diff --git a/features/invariant_model.feature b/features/invariant_model.feature deleted file mode 100644 index df541ded9..000000000 --- a/features/invariant_model.feature +++ /dev/null @@ -1,59 +0,0 @@ -Feature: Invariant data model and database schema - As a developer - I want an Invariant SQLAlchemy model with a corresponding database schema - So that invariant rules can be persisted and queried efficiently - - Background: - Given a fresh in-memory invariant database - - Scenario: Create an Invariant with all required fields - Given a new Invariant with description "All plans must have a goal" - When I persist the Invariant - Then I can retrieve the Invariant by its ID - And the persisted Invariant description should be "All plans must have a goal" - - Scenario: is_active defaults to True - Given a new Invariant with description "Default active invariant" - When I persist the Invariant - Then I can retrieve the Invariant by its ID - And the persisted Invariant is_active should be True - - Scenario: created_at is auto-populated on insert - Given a new Invariant with description "Timestamped invariant" - When I persist the Invariant - Then I can retrieve the Invariant by its ID - And the persisted Invariant created_at should not be empty - - Scenario: id is a UUID string - Given a new Invariant with description "UUID invariant" - When I persist the Invariant - Then I can retrieve the Invariant by its ID - And the persisted Invariant id should be a valid UUID - - Scenario: description is required and cannot be empty - Given a new Invariant with an empty description - When I try to persist the Invariant - Then a ValueError should be raised for empty description - - Scenario: Query active invariants - Given 3 active Invariants and 2 inactive Invariants - When I query Invariants filtered by is_active True - Then I should get 3 Invariants - - Scenario: Query inactive invariants - Given 3 active Invariants and 2 inactive Invariants - When I query Invariants filtered by is_active False - Then I should get 2 Invariants - - Scenario: Deactivate an Invariant - Given a persisted active Invariant - When I set is_active to False on the Invariant - Then the Invariant is_active should be False - - Scenario: Migration upgrade creates invariants table - Given a fresh in-memory invariant database - Then the invariants table should exist - - Scenario: Migration creates index on is_active - Given a fresh in-memory invariant database - Then the invariants table should have an index on is_active diff --git a/features/steps/invariant_model_steps.py b/features/steps/invariant_model_steps.py deleted file mode 100644 index 5bfe27e0c..000000000 --- a/features/steps/invariant_model_steps.py +++ /dev/null @@ -1,233 +0,0 @@ -"""Step definitions for invariant_model.feature. - -Tests the InvariantModel ORM class: field defaults, persistence, -querying by is_active, and schema validation. -""" - -from __future__ import annotations - -import uuid -from datetime import UTC, datetime - -from behave import given, then, when # type: ignore[import-untyped] -from behave.runner import Context -from sqlalchemy import create_engine, inspect -from sqlalchemy.orm import sessionmaker - -from cleveragents.infrastructure.database.models import Base, InvariantModel - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _setup_db(context: Context) -> None: - """Create an in-memory SQLite DB with the invariants table.""" - engine = create_engine("sqlite:///:memory:", echo=False) - Base.metadata.create_all(engine) - sm = sessionmaker(bind=engine) - session = sm() - context._inv_engine = engine - context._inv_session = session - - -def _make_invariant(description: str, is_active: bool = True) -> InvariantModel: - """Create an InvariantModel instance with a fresh UUID and timestamp.""" - return InvariantModel( - id=str(uuid.uuid4()), - description=description, - created_at=datetime.now(tz=UTC).isoformat(), - is_active=is_active, - ) - - -# --------------------------------------------------------------------------- -# Background -# --------------------------------------------------------------------------- - - -@given("a fresh in-memory invariant database") -def step_fresh_db(context: Context) -> None: - _setup_db(context) - - -# --------------------------------------------------------------------------- -# Create and retrieve -# --------------------------------------------------------------------------- - - -@given('a new Invariant with description "{description}"') -def step_new_invariant(context: Context, description: str) -> None: - context._inv_model = _make_invariant(description) - - -@given("a new Invariant with an empty description") -def step_new_invariant_empty_desc(context: Context) -> None: - context._inv_model = InvariantModel( - id=str(uuid.uuid4()), - description="", - created_at=datetime.now(tz=UTC).isoformat(), - is_active=True, - ) - - -@when("I persist the Invariant") -def step_persist_invariant(context: Context) -> None: - context._inv_session.add(context._inv_model) - context._inv_session.commit() - context._inv_id = context._inv_model.id - - -@when("I try to persist the Invariant") -def step_try_persist_invariant(context: Context) -> None: - context._inv_error = None - if not context._inv_model.description: - context._inv_error = ValueError("description cannot be empty") - return - context._inv_session.add(context._inv_model) - context._inv_session.commit() - context._inv_id = context._inv_model.id - - -@then("I can retrieve the Invariant by its ID") -def step_retrieve_invariant(context: Context) -> None: - inv = ( - context._inv_session.query(InvariantModel).filter_by(id=context._inv_id).first() - ) - assert inv is not None, f"Invariant with id={context._inv_id} not found" - context._inv_retrieved = inv - - -@then('the persisted Invariant description should be "{expected}"') -def step_check_description(context: Context, expected: str) -> None: - assert context._inv_retrieved.description == expected, ( - f"Expected '{expected}', got '{context._inv_retrieved.description}'" - ) - - -@then("the persisted Invariant is_active should be True") -def step_check_is_active_true(context: Context) -> None: - assert ( - context._inv_retrieved.is_active is True - or context._inv_retrieved.is_active == 1 - ), f"Expected is_active=True, got {context._inv_retrieved.is_active!r}" - - -@then("the persisted Invariant created_at should not be empty") -def step_check_created_at(context: Context) -> None: - assert context._inv_retrieved.created_at, "created_at should not be empty" - assert len(str(context._inv_retrieved.created_at)) > 0 - - -@then("the persisted Invariant id should be a valid UUID") -def step_check_uuid(context: Context) -> None: - inv_id = context._inv_retrieved.id - try: - uuid.UUID(str(inv_id)) - except ValueError as exc: - raise AssertionError(f"id '{inv_id}' is not a valid UUID") from exc - - -@then("a ValueError should be raised for empty description") -def step_check_value_error(context: Context) -> None: - assert context._inv_error is not None, "Expected a ValueError but none was raised" - assert isinstance(context._inv_error, ValueError) - - -# --------------------------------------------------------------------------- -# Filtering by is_active -# --------------------------------------------------------------------------- - - -@given("{active_count:d} active Invariants and {inactive_count:d} inactive Invariants") -def step_mixed_invariants( - context: Context, active_count: int, inactive_count: int -) -> None: - for i in range(active_count): - inv = _make_invariant(f"Active invariant {i}", is_active=True) - context._inv_session.add(inv) - for i in range(inactive_count): - inv = _make_invariant(f"Inactive invariant {i}", is_active=False) - context._inv_session.add(inv) - context._inv_session.commit() - - -@when("I query Invariants filtered by is_active True") -def step_query_active(context: Context) -> None: - context._inv_results = ( - context._inv_session.query(InvariantModel).filter_by(is_active=True).all() - ) - - -@when("I query Invariants filtered by is_active False") -def step_query_inactive(context: Context) -> None: - context._inv_results = ( - context._inv_session.query(InvariantModel).filter_by(is_active=False).all() - ) - - -@then("I should get {count:d} Invariants") -def step_check_count(context: Context, count: int) -> None: - actual = len(context._inv_results) - assert actual == count, f"Expected {count} Invariants, got {actual}" - - -# --------------------------------------------------------------------------- -# Deactivate -# --------------------------------------------------------------------------- - - -@given("a persisted active Invariant") -def step_persisted_active(context: Context) -> None: - inv = _make_invariant("Active invariant to deactivate", is_active=True) - context._inv_session.add(inv) - context._inv_session.commit() - context._inv_id = inv.id - context._inv_retrieved = inv - - -@when("I set is_active to False on the Invariant") -def step_deactivate(context: Context) -> None: - inv = ( - context._inv_session.query(InvariantModel).filter_by(id=context._inv_id).first() - ) - assert inv is not None - inv.is_active = False - context._inv_session.commit() - context._inv_retrieved = inv - - -@then("the Invariant is_active should be False") -def step_check_is_active_false(context: Context) -> None: - inv = ( - context._inv_session.query(InvariantModel).filter_by(id=context._inv_id).first() - ) - assert inv is not None - assert inv.is_active is False or inv.is_active == 0, ( - f"Expected is_active=False, got {inv.is_active!r}" - ) - - -# --------------------------------------------------------------------------- -# Schema validation -# --------------------------------------------------------------------------- - - -@then("the invariants table should exist") -def step_table_exists(context: Context) -> None: - inspector = inspect(context._inv_engine) - tables = inspector.get_table_names() - assert "invariants" in tables, ( - f"Table 'invariants' not found. Available tables: {tables}" - ) - - -@then("the invariants table should have an index on is_active") -def step_index_exists(context: Context) -> None: - inspector = inspect(context._inv_engine) - indexes = inspector.get_indexes("invariants") - index_names = [idx["name"] for idx in indexes] - # SQLite may also create implicit indexes; check for our named index - assert any("is_active" in name for name in index_names), ( - f"No index on is_active found. Indexes: {index_names}" - ) diff --git a/robot/helper_invariant_model.py b/robot/helper_invariant_model.py deleted file mode 100644 index 464d65adc..000000000 --- a/robot/helper_invariant_model.py +++ /dev/null @@ -1,188 +0,0 @@ -"""Helper script for Robot Framework invariant model smoke tests.""" - -from __future__ import annotations - -import sys -import uuid -from datetime import UTC, datetime -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 sqlalchemy import create_engine, inspect -from sqlalchemy.orm import sessionmaker - -from cleveragents.infrastructure.database.models import Base, InvariantModel - - -def _make_db() -> tuple[object, object]: - """Create an in-memory SQLite DB and return (engine, session).""" - engine = create_engine("sqlite:///:memory:", echo=False) - Base.metadata.create_all(engine) - sm = sessionmaker(bind=engine) - session = sm() - return engine, session - - -def _make_invariant(description: str, is_active: bool = True) -> InvariantModel: - return InvariantModel( - id=str(uuid.uuid4()), - description=description, - created_at=datetime.now(tz=UTC).isoformat(), - is_active=is_active, - ) - - -def _test_create_invariant() -> None: - """Create an Invariant with all required fields and verify persistence.""" - _, session = _make_db() - inv = _make_invariant("All plans must have a goal") - session.add(inv) - session.commit() - - retrieved = session.query(InvariantModel).filter_by(id=inv.id).first() - assert retrieved is not None, "Invariant not found after persist" - assert retrieved.description == "All plans must have a goal" - print("invariant-create-ok") - - -def _test_default_is_active() -> None: - """Verify that is_active defaults to True.""" - _, session = _make_db() - inv = _make_invariant("Default active invariant") - session.add(inv) - session.commit() - - retrieved = session.query(InvariantModel).filter_by(id=inv.id).first() - assert retrieved is not None - assert retrieved.is_active is True or retrieved.is_active == 1, ( - f"Expected is_active=True, got {retrieved.is_active!r}" - ) - print("invariant-default-active-ok") - - -def _test_created_at() -> None: - """Verify that created_at is populated on insert.""" - _, session = _make_db() - inv = _make_invariant("Timestamped invariant") - session.add(inv) - session.commit() - - retrieved = session.query(InvariantModel).filter_by(id=inv.id).first() - assert retrieved is not None - assert retrieved.created_at, "created_at should not be empty" - assert len(str(retrieved.created_at)) > 0 - print("invariant-created-at-ok") - - -def _test_uuid_id() -> None: - """Verify that the Invariant id is a valid UUID string.""" - _, session = _make_db() - inv = _make_invariant("UUID invariant") - session.add(inv) - session.commit() - - retrieved = session.query(InvariantModel).filter_by(id=inv.id).first() - assert retrieved is not None - try: - uuid.UUID(str(retrieved.id)) - except ValueError as exc: - raise AssertionError(f"id '{retrieved.id}' is not a valid UUID") from exc - print("invariant-uuid-ok") - - -def _test_query_active() -> None: - """Query Invariants filtered by is_active=True.""" - _, session = _make_db() - for i in range(3): - session.add(_make_invariant(f"Active {i}", is_active=True)) - for i in range(2): - session.add(_make_invariant(f"Inactive {i}", is_active=False)) - session.commit() - - results = session.query(InvariantModel).filter_by(is_active=True).all() - assert len(results) == 3, f"Expected 3 active, got {len(results)}" - print("invariant-query-active-ok") - - -def _test_query_inactive() -> None: - """Query Invariants filtered by is_active=False.""" - _, session = _make_db() - for i in range(3): - session.add(_make_invariant(f"Active {i}", is_active=True)) - for i in range(2): - session.add(_make_invariant(f"Inactive {i}", is_active=False)) - session.commit() - - results = session.query(InvariantModel).filter_by(is_active=False).all() - assert len(results) == 2, f"Expected 2 inactive, got {len(results)}" - print("invariant-query-inactive-ok") - - -def _test_deactivate() -> None: - """Set is_active to False on an existing Invariant.""" - _, session = _make_db() - inv = _make_invariant("Active invariant to deactivate", is_active=True) - session.add(inv) - session.commit() - - retrieved = session.query(InvariantModel).filter_by(id=inv.id).first() - assert retrieved is not None - retrieved.is_active = False - session.commit() - - updated = session.query(InvariantModel).filter_by(id=inv.id).first() - assert updated is not None - assert updated.is_active is False or updated.is_active == 0, ( - f"Expected is_active=False, got {updated.is_active!r}" - ) - print("invariant-deactivate-ok") - - -def _test_table_exists() -> None: - """Verify the invariants table exists after schema creation.""" - engine, _ = _make_db() - inspector = inspect(engine) - tables = inspector.get_table_names() - assert "invariants" in tables, f"Table 'invariants' not found. Available: {tables}" - print("invariant-table-ok") - - -def _test_index_exists() -> None: - """Verify the index on is_active exists after schema creation.""" - engine, _ = _make_db() - inspector = inspect(engine) - indexes = inspector.get_indexes("invariants") - index_names = [idx["name"] for idx in indexes] - assert any("is_active" in name for name in index_names), ( - f"No index on is_active found. Indexes: {index_names}" - ) - print("invariant-index-ok") - - -_TESTS = { - "create_invariant": _test_create_invariant, - "default_is_active": _test_default_is_active, - "created_at": _test_created_at, - "uuid_id": _test_uuid_id, - "query_active": _test_query_active, - "query_inactive": _test_query_inactive, - "deactivate": _test_deactivate, - "table_exists": _test_table_exists, - "index_exists": _test_index_exists, -} - -if __name__ == "__main__": - if len(sys.argv) < 2: - print(f"Usage: {sys.argv[0]} ") - print(f"Available tests: {', '.join(sorted(_TESTS))}") - sys.exit(1) - - test_name = sys.argv[1] - if test_name not in _TESTS: - print(f"Unknown test: {test_name}") - print(f"Available: {', '.join(sorted(_TESTS))}") - sys.exit(1) - - _TESTS[test_name]() diff --git a/robot/invariant_model.robot b/robot/invariant_model.robot deleted file mode 100644 index b74dbb61a..000000000 --- a/robot/invariant_model.robot +++ /dev/null @@ -1,63 +0,0 @@ -*** Settings *** -Documentation Smoke tests for Invariant data model persistence contract -Resource ${CURDIR}/common.resource -Suite Setup Setup Test Environment With Database Isolation -Suite Teardown Cleanup Test Environment - -*** Variables *** -${HELPER_SCRIPT} robot/helper_invariant_model.py - -*** Test Cases *** -Create Invariant With Required Fields - [Documentation] Create an Invariant with all required fields and verify persistence - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} create_invariant cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=create_invariant failed: ${result.stderr} - Should Contain ${result.stdout} invariant-create-ok - -Is Active Defaults To True - [Documentation] Verify that is_active defaults to True on a new Invariant - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} default_is_active cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=default_is_active failed: ${result.stderr} - Should Contain ${result.stdout} invariant-default-active-ok - -Created At Is Populated - [Documentation] Verify that created_at is auto-populated on insert - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} created_at cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=created_at failed: ${result.stderr} - Should Contain ${result.stdout} invariant-created-at-ok - -Id Is UUID - [Documentation] Verify that the Invariant id is a valid UUID string - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} uuid_id cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=uuid_id failed: ${result.stderr} - Should Contain ${result.stdout} invariant-uuid-ok - -Query Active Invariants - [Documentation] Query Invariants filtered by is_active=True - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} query_active cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=query_active failed: ${result.stderr} - Should Contain ${result.stdout} invariant-query-active-ok - -Query Inactive Invariants - [Documentation] Query Invariants filtered by is_active=False - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} query_inactive cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=query_inactive failed: ${result.stderr} - Should Contain ${result.stdout} invariant-query-inactive-ok - -Deactivate Invariant - [Documentation] Set is_active to False on an existing Invariant - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} deactivate cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=deactivate failed: ${result.stderr} - Should Contain ${result.stdout} invariant-deactivate-ok - -Table Exists After Migration - [Documentation] Verify the invariants table exists after schema creation - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} table_exists cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=table_exists failed: ${result.stderr} - Should Contain ${result.stdout} invariant-table-ok - -Index On Is Active Exists - [Documentation] Verify the index on is_active exists after schema creation - ${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} index_exists cwd=${WORKSPACE} - Should Be Equal As Integers ${result.rc} 0 msg=index_exists failed: ${result.stderr} - Should Contain ${result.stdout} invariant-index-ok diff --git a/src/cleveragents/infrastructure/database/migrations/versions/m3_001_invariants_table.py b/src/cleveragents/infrastructure/database/migrations/versions/m3_001_invariants_table.py index 14a13d7ad..0fe330b7a 100644 --- a/src/cleveragents/infrastructure/database/migrations/versions/m3_001_invariants_table.py +++ b/src/cleveragents/infrastructure/database/migrations/versions/m3_001_invariants_table.py @@ -1,8 +1,21 @@ -"""Add invariants table. +"""Placeholder for the original invariants-table migration. -Creates the ``invariants`` table for the invariant management system -(Stage M3 - issue #8524). Invariants are globally-scoped user-defined -constraints that must hold true across all planning sessions. +The original Stage M3 design created an ``invariants`` table here with the +legacy ``id / description / is_active`` schema. That schema was superseded +by ``m11_001_standalone_invariants`` which creates the same table name +with the current ``id / text / scope / source_name / active / +non_overridable / created_at`` columns expected by +``InvariantModel`` and ``InvariantRepository``. + +If both ``upgrade()`` bodies ran the second ``op.create_table('invariants', +...)`` would fail because the table already exists, which previously broke +every scenario that initialises the test database in ``before_scenario``. + +This migration is therefore kept as a no-op so the historical revision id +is still reachable by the downstream merge migrations +(``m3_002_merge_invariants_and_a5_006`` and +``m9_004_merge_invariants_branch``) and the upgrade chain can run through +to ``m11_001_standalone_invariants`` which now owns the table creation. Revision ID: m3_001_invariants_table Revises: m9_002_plan_resume_fields @@ -11,9 +24,6 @@ Create Date: 2026-04-24 from collections.abc import Sequence -import sqlalchemy as sa -from alembic import op - # revision identifiers, used by Alembic. revision: str = "m3_001_invariants_table" down_revision: str | Sequence[str] | None = "m9_002_plan_resume_fields" @@ -22,24 +32,8 @@ depends_on: str | Sequence[str] | None = None def upgrade() -> None: - """Create the invariants table with index on is_active.""" - op.create_table( - "invariants", - sa.Column("id", sa.String(36), nullable=False), - sa.Column("description", sa.Text, nullable=False), - sa.Column("created_at", sa.String(30), nullable=False), - sa.Column( - "is_active", - sa.Boolean, - nullable=False, - server_default=sa.text("1"), - ), - sa.PrimaryKeyConstraint("id"), - ) - op.create_index("ix_invariants_is_active", "invariants", ["is_active"]) + """No-op; ``m11_001_standalone_invariants`` owns the ``invariants`` table.""" def downgrade() -> None: - """Drop the invariants table.""" - op.drop_index("ix_invariants_is_active", table_name="invariants") - op.drop_table("invariants") + """No-op; pairs with the no-op ``upgrade()``.""" diff --git a/src/cleveragents/infrastructure/database/models.py b/src/cleveragents/infrastructure/database/models.py index be56635a9..ddc24a829 100644 --- a/src/cleveragents/infrastructure/database/models.py +++ b/src/cleveragents/infrastructure/database/models.py @@ -3784,4 +3784,3 @@ class IndexedFileModel(Base): # it for lookups already; no separate index needed. Index("ix_indexed_files_language", "language"), ) -