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
318 lines
10 KiB
Python
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()
|