Luis Mendes
f678d611bb
CI / benchmark-publish (pull_request) Has been skipped
CI / typecheck (pull_request) Successful in 58s
CI / quality (pull_request) Successful in 57s
CI / security (pull_request) Successful in 1m3s
CI / build (pull_request) Successful in 25s
CI / helm (pull_request) Successful in 34s
CI / lint (pull_request) Successful in 3m20s
CI / integration_tests (pull_request) Successful in 4m2s
CI / unit_tests (pull_request) Successful in 4m27s
CI / docker (pull_request) Successful in 1m18s
CI / coverage (pull_request) Successful in 8m39s
CI / e2e_tests (pull_request) Successful in 12m37s
CI / status-check (pull_request) Successful in 1s
CI / typecheck (push) Successful in 47s
CI / lint (push) Successful in 3m16s
CI / security (push) Successful in 4m7s
CI / quality (push) Successful in 3m46s
CI / build (push) Successful in 21s
CI / helm (push) Successful in 24s
CI / unit_tests (push) Successful in 4m19s
CI / integration_tests (push) Successful in 3m55s
CI / docker (push) Successful in 1m19s
CI / e2e_tests (push) Successful in 12m30s
CI / coverage (push) Successful in 11m48s
CI / status-check (push) Successful in 1s
CI / benchmark-publish (push) Successful in 29m36s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-regression (pull_request) Successful in 56m32s
Added CorrectionAttemptModel SQLAlchemy model with all spec-defined columns
(correction_attempt_id, plan_id, original_decision_id, new_decision_id,
mode, guidance, archived_artifacts_path, state, created_at, completed_at).
Added FK constraints to v3_plans and decisions tables. Created Alembic
migration and idx_corrections_plan index. Added repository layer for
CRUD operations.
Addressed code review feedback (rounds 1-12):
- Replaced Any type annotations with typed CorrectionAttemptRecord
signatures using TYPE_CHECKING imports.
- Replaced fragile time.sleep with deterministic timestamps.
- Added cascade deletion test with PRAGMA foreign_keys=ON.
- Changed update_state() to accept typed enum and datetime params.
- Added guidance non-empty validator with max_length=10_000.
- Added spec-aligned server_default for created_at column.
- Fixed timezone handling in to_domain() and from_domain().
- Added spec-defined lifecycle state transition validation via
validate_correction_state_transition() domain function.
- Improved FK-violation error messages in create() and update_state().
- Normalised timestamp to millisecond precision matching SQLite
server_default strftime('%f') output.
- Added auto-set completed_at on terminal transitions.
- Added CorrectionAttemptRecord field validators (strip, non-empty).
- Changed original_decision_id FK from CASCADE to RESTRICT matching
spec DDL default, preserving correction audit trail.
- Added input validation in update_state() for new_decision_id and
archived_artifacts_path (empty/whitespace rejection).
- Fixed dirty-session bug by moving validation before ORM mutations.
- Extracted _SQLITE_TIMESTAMP_MS_LEN constant for timestamp truncation.
Addressed thirteenth code review feedback:
- Changed InvalidCorrectionStateTransitionError base class from
DatabaseError to BusinessRuleViolation per CONTRIBUTING.md exception
semantics (state transition is a business rule, not a database error;
prevents incorrect retries by @database_retry decorator).
- Changed new_decision_id FK from SET NULL to RESTRICT matching the
spec DDL default (no ON DELETE clause) and consistent with the
RESTRICT approach used for original_decision_id.
- Changed update_state() input validation for new_decision_id and
archived_artifacts_path from DatabaseError to ValueError per
CONTRIBUTING.md argument validation guidelines.
- Defensive to_domain() coercion now defaults corrupted state to
'failed' (terminal) instead of 'pending', preventing re-execution
of completed/failed corrections with corrupted DB values.
- Extracted format_sqlite_timestamp() helper and SQLITE_TIMESTAMP_MS_LEN
public constant, removing duplicated timestamp formatting logic
between from_domain() and update_state().
- Added code comment explaining CASCADE on plan_id FK as a codebase
convention deviation from spec DDL default.
- Added BDD scenario verifying RESTRICT FK on original_decision_id
blocks decision deletion.
- Replaced weak cross-plan isolation test with stronger two-plan
scenario verifying list_by_plan returns only each plan's attempts.
- Fixed hardcoded assertion in step_check_archived_path to use
context variable.
- 45 BDD scenarios and 5 Robot integration tests.
Addressed fourteenth code review feedback:
- Added ORM-level relationship(cascade="all, delete-orphan") on
LifecyclePlanModel for CorrectionAttemptModel, consistent with all
other v3_plans child tables, ensuring ORM-level cascade deletes
work even when SQLite FK enforcement is disabled.
- Added defensive to_domain() coercion for corrupted guidance column
(defaults to "[corrupted]" with warning log), consistent with
existing mode/state coercion pattern.
- Added ValueError guard in format_sqlite_timestamp() rejecting naive
datetimes per CONTRIBUTING.md fail-fast argument validation.
- Fixed stale spec DDL line reference in CorrectionAttemptModel
docstring.
- Fixed duplicated docstring on SQLITE_TIMESTAMP_MS_LEN constant.
Addressed fifteenth code review feedback:
- Fixed update_state() to defensively handle corrupted DB state values
via try/except ValueError coercion to FAILED terminal state with
warning log, consistent with to_domain() defensive coercion pattern.
- Strengthened RESTRICT FK BDD assertion to verify exception type
(IntegrityError/DatabaseError) instead of only checking presence.
- Split multi-When/Then cross-plan isolation BDD scenario into
idiomatic single-When/Then scenarios per Gherkin best practice.
- 53 BDD scenarios (was 45) and 5 Robot integration tests.
ISSUES CLOSED: #920
45 lines
2.3 KiB
Plaintext
45 lines
2.3 KiB
Plaintext
*** Settings ***
|
|
Documentation Smoke tests for correction attempt persistence via CorrectionAttemptRepository
|
|
Resource ${CURDIR}/common.resource
|
|
Suite Setup Setup Test Environment
|
|
Suite Teardown Cleanup Test Environment
|
|
|
|
*** Variables ***
|
|
${HELPER_SCRIPT} robot/helper_correction_attempt_persistence.py
|
|
|
|
*** Test Cases ***
|
|
Create And Retrieve Correction Attempt
|
|
[Documentation] Create a revert correction attempt, persist, retrieve
|
|
[Tags] database correction persistence
|
|
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} create-retrieve cwd=${WORKSPACE} timeout=120s on_timeout=kill
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} create-retrieve-ok
|
|
|
|
List Correction Attempts By Plan
|
|
[Documentation] Create multiple correction attempts and list by plan
|
|
[Tags] database correction query persistence
|
|
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} list-by-plan cwd=${WORKSPACE} timeout=120s on_timeout=kill
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} list-by-plan-ok
|
|
|
|
Update Correction Attempt State
|
|
[Documentation] Update state from pending to executing to complete
|
|
[Tags] database correction state persistence
|
|
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} update-state cwd=${WORKSPACE} timeout=120s on_timeout=kill
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} update-state-ok
|
|
|
|
Delete Correction Attempt
|
|
[Documentation] Delete a correction attempt and verify it is gone
|
|
[Tags] database correction persistence
|
|
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} delete cwd=${WORKSPACE} timeout=120s on_timeout=kill
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} delete-ok
|
|
|
|
Domain Model Round-Trip
|
|
[Documentation] Verify all spec DDL columns survive domain-model round-trip
|
|
[Tags] database correction persistence
|
|
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} domain-roundtrip cwd=${WORKSPACE} timeout=120s on_timeout=kill
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} domain-roundtrip-ok
|