Files
temp/features/steps/plan_resume_fields_persistence_steps.py
freemo 77251e7623 fix(persistence): persist reversion_count, last_completed_step, and last_checkpoint_id on LifecyclePlanModel
Implemented persistence for three previously missing Plan fields to ensure proper resume and reversion behavior across restarts. The changes address the gaps in the database, ORM mapping, repository logic, and tests.

- Alembic migration
  - File: alembic/versions/m9_002_plan_resume_fields.py
  - Adds three columns to the v3_plans table:
    - reversion_count: INTEGER NOT NULL DEFAULT 0 (server_default)
    - last_completed_step: INTEGER NOT NULL DEFAULT -1 (server_default)
    - last_checkpoint_id: TEXT nullable (no server_default)
  - Migration chains from m9_001_session_name_column to align with existing plan schema evolution.

- SQLAlchemy model
  - File: src/cleveragents/infrastructure/database/models.py
  - Updated LifecyclePlanModel to include reversion_count, last_completed_step, and last_checkpoint_id columns.
  - from_domain(): updated to serialize all three fields.
  - to_domain(): updated to deserialize all three fields with proper cast() typing, ensuring correct domain conversions.

- Repository fix
  - File: src/cleveragents/infrastructure/database/repositories.py
  - Fixed LifecyclePlanRepository.update() which was silently dropping the three fields on every update.
  - This remediation ensures updates preserve reversion_count, last_completed_step, and last_checkpoint_id.

- Behave tests
  - Files: features/plan_resume_fields_persistence.feature and associated steps
  - Added 7 scenarios validating individual field persistence, combined persistence, default values, cross-reconnection persistence, and update persistence.
  - Tests ensure correctness of persistence behavior across restarts and updates.

Key Design Decisions
- server_default used for migration columns to backfill defaults automatically for existing rows without a separate backfill step.
- last_checkpoint_id is nullable (no server_default) because None is the correct default in the domain model.
- The update() fix in LifecyclePlanRepository was a bonus discovery; without it, updates could silently drop the resume fields and break persistence guarantees.

ISSUES CLOSED: #2864
2026-04-05 04:38:31 +00:00

114 lines
4.6 KiB
Python

"""Step definitions for plan resume fields persistence feature.
Tests round-trip persistence of reversion_count, last_completed_step,
and last_checkpoint_id through the LifecyclePlanRepository.
Background steps (fresh in-memory DB, prerequisite action, file-based DB,
close-and-reopen, new lifecycle plan, persist plan) are reused from
plan_persistence_steps.py — they share the same step text so Behave
automatically picks them up.
"""
from __future__ import annotations
from behave import given, then, when
from behave.runner import Context
# ---------------------------------------------------------------------------
# Given: mutate the plan under test
# ---------------------------------------------------------------------------
@given("the plan has reversion_count {count:d}")
def step_plan_has_reversion_count(context: Context, count: int) -> None:
"""Set reversion_count on the plan being built."""
context._pp_plan = context._pp_plan.model_copy(update={"reversion_count": count})
@given("the plan has last_completed_step {step:d}")
def step_plan_has_last_completed_step(context: Context, step: int) -> None:
"""Set last_completed_step on the plan being built."""
context._pp_plan = context._pp_plan.model_copy(update={"last_completed_step": step})
@given('the plan has last_checkpoint_id "{checkpoint_id}"')
def step_plan_has_last_checkpoint_id(context: Context, checkpoint_id: str) -> None:
"""Set last_checkpoint_id on the plan being built."""
context._pp_plan = context._pp_plan.model_copy(
update={"last_checkpoint_id": checkpoint_id}
)
# ---------------------------------------------------------------------------
# When: retrieve the plan
# ---------------------------------------------------------------------------
@when('I retrieve the plan by ID "{plan_id}"')
def step_retrieve_plan_by_id(context: Context, plan_id: str) -> None:
"""Retrieve the plan from the repository and store it for assertions."""
context.retrieved_plan = context._pp_plan_repo.get(plan_id)
assert context.retrieved_plan is not None, (
f"Plan {plan_id!r} not found in repository"
)
# ---------------------------------------------------------------------------
# When: update reversion_count on an already-persisted plan
# ---------------------------------------------------------------------------
@when("I update the plan reversion_count to {count:d}")
def step_update_plan_reversion_count(context: Context, count: int) -> None:
"""Update reversion_count on the persisted plan.
Uses ``context._pp_plan`` (the plan that was just persisted) so this
step can be called immediately after "I persist the plan via the plan
repository" without a prior retrieve step.
"""
updated = context._pp_plan.model_copy(update={"reversion_count": count})
context._pp_plan_repo.update(updated)
context._pp_session.commit()
# ---------------------------------------------------------------------------
# Then: assert resume field values
# ---------------------------------------------------------------------------
@then("the retrieved plan reversion_count should be {count:d}")
def step_retrieved_plan_reversion_count(context: Context, count: int) -> None:
"""Assert the retrieved plan's reversion_count."""
assert context.retrieved_plan.reversion_count == count, (
f"Expected reversion_count={count}, got {context.retrieved_plan.reversion_count}"
)
@then("the retrieved plan last_completed_step should be {step:d}")
def step_retrieved_plan_last_completed_step(context: Context, step: int) -> None:
"""Assert the retrieved plan's last_completed_step."""
assert context.retrieved_plan.last_completed_step == step, (
f"Expected last_completed_step={step}, "
f"got {context.retrieved_plan.last_completed_step}"
)
@then('the retrieved plan last_checkpoint_id should be "{checkpoint_id}"')
def step_retrieved_plan_last_checkpoint_id(
context: Context, checkpoint_id: str
) -> None:
"""Assert the retrieved plan's last_checkpoint_id matches the expected value."""
assert context.retrieved_plan.last_checkpoint_id == checkpoint_id, (
f"Expected last_checkpoint_id={checkpoint_id!r}, "
f"got {context.retrieved_plan.last_checkpoint_id!r}"
)
@then("the retrieved plan last_checkpoint_id should be None")
def step_retrieved_plan_last_checkpoint_id_none(context: Context) -> None:
"""Assert the retrieved plan's last_checkpoint_id is None."""
assert context.retrieved_plan.last_checkpoint_id is None, (
f"Expected last_checkpoint_id=None, "
f"got {context.retrieved_plan.last_checkpoint_id!r}"
)