Files
cleveragents-core/robot/helper_correction_attempt_persistence.py
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
feat(db): add correction_attempts table per specification DDL
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
2026-03-29 16:23:04 +01:00

318 lines
10 KiB
Python

"""Helper script for Robot Framework correction attempt persistence tests.
Usage:
python robot/helper_correction_attempt_persistence.py <subcommand>
Subcommands:
create-retrieve Create + retrieve a correction attempt
list-by-plan Create multiple, list by plan ID
update-state Update state transitions
delete Delete a correction attempt
domain-roundtrip Verify all spec DDL columns round-trip
"""
from __future__ import annotations
import sys
from datetime import UTC, datetime, timedelta
from pathlib import Path
# Ensure src is importable when run from workspace root
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
from collections.abc import Callable
from typing import Any
from sqlalchemy import create_engine, event
from sqlalchemy.orm import Session, sessionmaker
from cleveragents.domain.models.core.action import (
Action,
ActionState,
)
from cleveragents.domain.models.core.correction import (
CorrectionAttemptRecord,
CorrectionAttemptState,
CorrectionMode,
)
from cleveragents.domain.models.core.decision import (
ContextSnapshot,
Decision,
DecisionType,
)
from cleveragents.domain.models.core.plan import (
NamespacedName,
Plan,
PlanIdentity,
PlanPhase,
PlanTimestamps,
ProcessingState,
)
from cleveragents.infrastructure.database.models import Base
from cleveragents.infrastructure.database.repositories import (
ActionRepository,
CorrectionAttemptNotFoundError,
CorrectionAttemptRepository,
DecisionRepository,
LifecyclePlanRepository,
)
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
_PLAN_ID = "01HV000000000000000000RC01"
_DECISION_ID = "01HV000000000000000000RD01"
def _setup() -> tuple[Session, Callable[[], Session]]:
"""Create an in-memory SQLite DB with all tables and FK enforcement."""
engine = create_engine("sqlite:///:memory:", echo=False)
@event.listens_for(engine, "connect")
def _enable_fk(dbapi_conn: Any, _rec: Any) -> None:
cursor = dbapi_conn.cursor()
cursor.execute("PRAGMA foreign_keys=ON")
cursor.close()
Base.metadata.create_all(engine)
sm = sessionmaker(bind=engine)
session = sm()
factory: Callable[[], Session] = lambda: session # noqa: E731
return session, factory
def _create_prerequisites(session: Session, factory: Callable[[], Session]) -> None:
"""Create action + plan + decision needed to satisfy FK constraints."""
now = datetime.now(UTC)
action_repo = ActionRepository(session_factory=factory)
action = Action(
namespaced_name=NamespacedName.parse("local/robot-correction-action"),
description="Robot correction test action",
definition_of_done="Correction is applied",
strategy_actor="strategy-actor",
execution_actor="execution-actor",
state=ActionState.AVAILABLE,
)
action_repo.create(action)
session.commit()
plan_repo = LifecyclePlanRepository(session_factory=factory)
plan = Plan(
identity=PlanIdentity(plan_id=_PLAN_ID, attempt=1),
namespaced_name=NamespacedName.parse("local/robot-test-plan"),
action_name="local/robot-correction-action",
description="Robot test plan for correction persistence",
phase=PlanPhase.EXECUTE,
processing_state=ProcessingState.PROCESSING,
timestamps=PlanTimestamps(created_at=now, updated_at=now),
)
plan_repo.create(plan)
session.commit()
dec_repo = DecisionRepository(session_factory=factory)
decision = Decision(
decision_id=_DECISION_ID,
plan_id=_PLAN_ID,
sequence_number=0,
decision_type=DecisionType.PROMPT_DEFINITION,
question="What approach?",
chosen_option="Build REST API",
context_snapshot=ContextSnapshot(
hot_context_hash="sha256:robot",
hot_context_ref="ref:robot",
relevant_resources=[],
actor_state_ref="",
),
)
dec_repo.create(decision)
session.commit()
# ---------------------------------------------------------------------------
# Subcommands
# ---------------------------------------------------------------------------
def cmd_create_retrieve() -> None:
"""Create a correction attempt and retrieve it."""
session, factory = _setup()
_create_prerequisites(session, factory)
repo = CorrectionAttemptRepository(session_factory=factory)
record = CorrectionAttemptRecord(
plan_id=_PLAN_ID,
original_decision_id=_DECISION_ID,
mode=CorrectionMode.REVERT,
guidance="Fix the broken implementation",
)
created = repo.create(record)
session.commit()
retrieved = repo.get(created.correction_attempt_id)
assert retrieved.correction_attempt_id == created.correction_attempt_id
assert retrieved.mode == CorrectionMode.REVERT
assert retrieved.state == CorrectionAttemptState.PENDING
assert retrieved.plan_id == _PLAN_ID
assert retrieved.original_decision_id == _DECISION_ID
print("create-retrieve-ok")
def cmd_list_by_plan() -> None:
"""Create multiple correction attempts and list by plan."""
session, factory = _setup()
_create_prerequisites(session, factory)
repo = CorrectionAttemptRepository(session_factory=factory)
base_time = datetime(2026, 1, 1, 0, 0, 0, tzinfo=UTC)
for i in range(3):
record = CorrectionAttemptRecord(
plan_id=_PLAN_ID,
original_decision_id=_DECISION_ID,
mode=CorrectionMode.REVERT,
guidance=f"Guidance {i}",
created_at=base_time + timedelta(seconds=i),
)
repo.create(record)
session.commit()
results = repo.list_by_plan(_PLAN_ID)
assert len(results) == 3
# Check ordering
for i in range(len(results) - 1):
assert results[i].created_at <= results[i + 1].created_at
print("list-by-plan-ok")
def cmd_update_state() -> None:
"""Update state transitions."""
session, factory = _setup()
_create_prerequisites(session, factory)
repo = CorrectionAttemptRepository(session_factory=factory)
record = CorrectionAttemptRecord(
plan_id=_PLAN_ID,
original_decision_id=_DECISION_ID,
mode=CorrectionMode.REVERT,
guidance="Fix it",
)
created = repo.create(record)
session.commit()
# Update to executing
updated = repo.update_state(
created.correction_attempt_id,
state=CorrectionAttemptState.EXECUTING,
)
session.commit()
assert updated.state == CorrectionAttemptState.EXECUTING
# Update to complete with timestamp
updated = repo.update_state(
created.correction_attempt_id,
state=CorrectionAttemptState.COMPLETE,
completed_at=datetime.now(UTC),
)
session.commit()
assert updated.state == CorrectionAttemptState.COMPLETE
assert updated.completed_at is not None
print("update-state-ok")
def cmd_delete() -> None:
"""Delete a correction attempt."""
session, factory = _setup()
_create_prerequisites(session, factory)
repo = CorrectionAttemptRepository(session_factory=factory)
record = CorrectionAttemptRecord(
plan_id=_PLAN_ID,
original_decision_id=_DECISION_ID,
mode=CorrectionMode.REVERT,
guidance="Delete me",
)
created = repo.create(record)
session.commit()
result = repo.delete(created.correction_attempt_id)
session.commit()
assert result is True
# Verify deleted
try:
repo.get(created.correction_attempt_id)
msg = "Should have raised CorrectionAttemptNotFoundError"
raise AssertionError(msg)
except CorrectionAttemptNotFoundError:
pass
# Delete non-existent
result = repo.delete("01HV000000000000000NONEXIST")
assert result is False
print("delete-ok")
def cmd_domain_roundtrip() -> None:
"""Verify all spec DDL columns survive domain-model round-trip."""
session, factory = _setup()
_create_prerequisites(session, factory)
repo = CorrectionAttemptRepository(session_factory=factory)
now = datetime.now(UTC)
record = CorrectionAttemptRecord(
plan_id=_PLAN_ID,
original_decision_id=_DECISION_ID,
mode=CorrectionMode.APPEND,
guidance="Detailed guidance text for correction",
archived_artifacts_path="/tmp/archived/artifacts",
state=CorrectionAttemptState.PENDING,
created_at=now,
)
created = repo.create(record)
session.commit()
retrieved = repo.get(created.correction_attempt_id)
# Verify all columns
assert retrieved.correction_attempt_id == created.correction_attempt_id
assert retrieved.plan_id == _PLAN_ID
assert retrieved.original_decision_id == _DECISION_ID
assert retrieved.new_decision_id is None
assert retrieved.mode == CorrectionMode.APPEND
assert retrieved.guidance == "Detailed guidance text for correction"
assert retrieved.archived_artifacts_path == "/tmp/archived/artifacts"
assert retrieved.state == CorrectionAttemptState.PENDING
assert retrieved.created_at is not None
# Verify created_at value survives the round-trip within 10ms
# (from_domain truncates to millisecond precision)
delta = abs((retrieved.created_at - now).total_seconds())
assert delta < 0.01, f"created_at drift {delta}s exceeds 10ms tolerance"
assert retrieved.completed_at is None
print("domain-roundtrip-ok")
# ---------------------------------------------------------------------------
# Dispatch
# ---------------------------------------------------------------------------
_COMMANDS = {
"create-retrieve": cmd_create_retrieve,
"list-by-plan": cmd_list_by_plan,
"update-state": cmd_update_state,
"delete": cmd_delete,
"domain-roundtrip": cmd_domain_roundtrip,
}
def main() -> None:
"""Dispatch to the requested subcommand."""
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", file=sys.stderr)
sys.exit(1)
_COMMANDS[sys.argv[1]]()
if __name__ == "__main__":
main()