cleveragents/cleveragents-core

Fork 3

UAT: `decisions` table missing `UNIQUE(plan_id, sequence_number)` constraint — duplicate sequence numbers can be inserted silently #2976

New issue

Open

opened 2026-04-05 03:01:20 +00:00 by freemo · 2 comments

freemo commented

2026-04-05 03:01:20 +00:00

Owner

Metadata

Branch: fix/decisions-unique-plan-seq-constraint
Commit Message: fix(persistence): add UNIQUE(plan_id, sequence_number) constraint to decisions table
Milestone: v3.7.0
Parent Epic: #394

Background

During UAT testing of the Decision Tree & Corrections feature area, the decisions table schema was analyzed. The domain model explicitly states that sequence number uniqueness within a plan is enforced at the persistence layer, but the database schema only has a non-unique index (not a unique constraint) on (plan_id, sequence_number).

Expected Behaviour (from spec and domain model)

The Decision domain model docstring states:

sequence_number: int = Field(
    ...,
    ge=0,
    description=(
        "Monotonic order within the plan's decisions.  "
        "Uniqueness within a plan is enforced at the persistence "
        "layer, not the domain model."
    ),
)

This explicitly promises that the persistence layer enforces uniqueness. The database must have a UNIQUE(plan_id, sequence_number) constraint.

Actual Behaviour

The migration (alembic/versions/m4_001_decision_tables.py) and ORM model (src/cleveragents/infrastructure/database/models.py) only create a non-unique index:

Index("ix_decisions_plan_seq", "plan_id", "sequence_number"),

There is no UniqueConstraint("plan_id", "sequence_number"). This means:

Two decisions with the same plan_id and sequence_number can be inserted without a database error.
The SequenceConflictError in DecisionService is the only guard, but it relies on in-memory state and can be bypassed in concurrent scenarios or after service restart in persisted mode.
The domain model's documented contract ("Uniqueness within a plan is enforced at the persistence layer") is violated.

Steps to Reproduce

In persisted mode, directly insert two decisions with the same plan_id and sequence_number via the repository.
No database error is raised — the duplicate is silently accepted.
DecisionService.list_decisions() returns both decisions with the same sequence number.

Code Locations

File	Detail
`src/cleveragents/domain/models/core/decision.py`	Lines 287–295 — `sequence_number` field with uniqueness promise
`alembic/versions/m4_001_decision_tables.py`	Missing `UniqueConstraint`
`src/cleveragents/infrastructure/database/models.py`	`DecisionModel.__table_args__` (line 2762) — only `Index`, not `UniqueConstraint`
`src/cleveragents/application/services/decision_service.py`	`SequenceConflictError` — in-memory guard only, not a DB-level guarantee

Subtasks

Add a new Alembic migration that drops the existing ix_decisions_plan_seq index and replaces it with a UniqueConstraint("plan_id", "sequence_number") (named uq_decisions_plan_seq)
Update DecisionModel.__table_args__ in src/cleveragents/infrastructure/database/models.py to use UniqueConstraint instead of (or in addition to) the plain Index
Verify that DecisionService correctly surfaces the DB-level IntegrityError as a SequenceConflictError so callers receive a meaningful domain exception rather than a raw SQLAlchemy error
Write/update Behave unit tests covering the duplicate-sequence-number scenario (both in-memory and via repository)
Write/update Robot Framework integration tests that attempt to insert duplicate (plan_id, sequence_number) pairs and assert the correct error is raised
Run nox to confirm all stages pass and coverage remains ≥ 97%

Definition of Done

A new Alembic migration exists that replaces the plain index with a UniqueConstraint on (plan_id, sequence_number)
DecisionModel.__table_args__ reflects the unique constraint in the ORM model
Attempting to insert a duplicate (plan_id, sequence_number) via the repository raises SequenceConflictError (not a raw DB error)
All Behave unit tests for the decisions domain pass
All Robot Framework integration tests for the decision persistence layer pass
nox -e typecheck passes with zero errors
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/decisions-unique-plan-seq-constraint` - **Commit Message**: `fix(persistence): add UNIQUE(plan_id, sequence_number) constraint to decisions table` - **Milestone**: v3.7.0 - **Parent Epic**: #394 ## Background During UAT testing of the Decision Tree & Corrections feature area, the `decisions` table schema was analyzed. The domain model explicitly states that sequence number uniqueness within a plan is enforced at the persistence layer, but the database schema only has a non-unique index (not a unique constraint) on `(plan_id, sequence_number)`. ### Expected Behaviour (from spec and domain model) The `Decision` domain model docstring states: ```python sequence_number: int = Field( ..., ge=0, description=( "Monotonic order within the plan's decisions. " "Uniqueness within a plan is enforced at the persistence " "layer, not the domain model." ), ) ``` This explicitly promises that the persistence layer enforces uniqueness. The database **must** have a `UNIQUE(plan_id, sequence_number)` constraint. ### Actual Behaviour The migration (`alembic/versions/m4_001_decision_tables.py`) and ORM model (`src/cleveragents/infrastructure/database/models.py`) only create a non-unique index: ```python Index("ix_decisions_plan_seq", "plan_id", "sequence_number"), ``` There is no `UniqueConstraint("plan_id", "sequence_number")`. This means: 1. Two decisions with the same `plan_id` and `sequence_number` can be inserted without a database error. 2. The `SequenceConflictError` in `DecisionService` is the only guard, but it relies on in-memory state and can be bypassed in concurrent scenarios or after service restart in persisted mode. 3. The domain model's documented contract ("Uniqueness within a plan is enforced at the persistence layer") is violated. ### Steps to Reproduce 1. In persisted mode, directly insert two decisions with the same `plan_id` and `sequence_number` via the repository. 2. No database error is raised — the duplicate is silently accepted. 3. `DecisionService.list_decisions()` returns both decisions with the same sequence number. ### Code Locations | File | Detail | |------|--------| | `src/cleveragents/domain/models/core/decision.py` | Lines 287–295 — `sequence_number` field with uniqueness promise | | `alembic/versions/m4_001_decision_tables.py` | Missing `UniqueConstraint` | | `src/cleveragents/infrastructure/database/models.py` | `DecisionModel.__table_args__` (line 2762) — only `Index`, not `UniqueConstraint` | | `src/cleveragents/application/services/decision_service.py` | `SequenceConflictError` — in-memory guard only, not a DB-level guarantee | ## Subtasks - [ ] Add a new Alembic migration that drops the existing `ix_decisions_plan_seq` index and replaces it with a `UniqueConstraint("plan_id", "sequence_number")` (named `uq_decisions_plan_seq`) - [ ] Update `DecisionModel.__table_args__` in `src/cleveragents/infrastructure/database/models.py` to use `UniqueConstraint` instead of (or in addition to) the plain `Index` - [ ] Verify that `DecisionService` correctly surfaces the DB-level `IntegrityError` as a `SequenceConflictError` so callers receive a meaningful domain exception rather than a raw SQLAlchemy error - [ ] Write/update Behave unit tests covering the duplicate-sequence-number scenario (both in-memory and via repository) - [ ] Write/update Robot Framework integration tests that attempt to insert duplicate `(plan_id, sequence_number)` pairs and assert the correct error is raised - [ ] Run `nox` to confirm all stages pass and coverage remains ≥ 97% ## Definition of Done - [ ] A new Alembic migration exists that replaces the plain index with a `UniqueConstraint` on `(plan_id, sequence_number)` - [ ] `DecisionModel.__table_args__` reflects the unique constraint in the ORM model - [ ] Attempting to insert a duplicate `(plan_id, sequence_number)` via the repository raises `SequenceConflictError` (not a raw DB error) - [ ] All Behave unit tests for the `decisions` domain pass - [ ] All Robot Framework integration tests for the decision persistence layer pass - [ ] `nox -e typecheck` passes with zero errors - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo added this to the v3.7.0 milestone

2026-04-05 03:01:34 +00:00

freemo added a new dependency

2026-04-05 03:01:53 +00:00

#394 Epic: Decision Framework

freemo commented

2026-04-05 03:15:00 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Confirmed
MoSCoW: Should Have

Valid finding verified during batch triage.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Confirmed - **MoSCoW**: Should Have Valid finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 06:15:22 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo commented

2026-04-05 07:15:48 +00:00

Author

Owner

Starting implementation on branch fix/decisions-unique-plan-seq-constraint.

Plan:

Wave 1 (parallel): Add Alembic migration + Update ORM model + Verify DecisionService error handling
Wave 2 (parallel): Write Behave unit tests + Write Robot Framework integration tests
Wave 3: Run nox quality gates

Difficulty assessment: Medium → starting at sonnet tier.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Starting implementation on branch `fix/decisions-unique-plan-seq-constraint`. **Plan:** - Wave 1 (parallel): Add Alembic migration + Update ORM model + Verify DecisionService error handling - Wave 2 (parallel): Write Behave unit tests + Write Robot Framework integration tests - Wave 3: Run nox quality gates Difficulty assessment: Medium → starting at sonnet tier. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo referenced this issue

2026-04-05 07:21:47 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue

2026-04-05 17:35:29 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377