Plan: Persist QA artefacts linked to canonical episodes (2.2.7) - #279
Plan: Persist QA artefacts linked to canonical episodes (2.2.7)#279leynos wants to merge 3 commits into
Conversation
Draft the execution plan for roadmap item 2.2.7, covering the QA artefact domain model, PostgreSQL persistence, recording and query services, REST retrieval filtered by evaluator and compliance status, and the first first-party command-line surface. Records three deviations from upstream artefacts for ADR capture: a unified `qa_evaluations` plus `qa_findings` schema in place of the design document's `brand_compliance_results` table, correlation to the existing cost ledger instead of duplicating usage data, and a CLI implemented as a REST client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Apply a six-lens pre-implementation review. The substantive corrections: Drop the proposed `episodic.llm` Hecate prefix. Hecate classifies a module by the first configured group whose prefix contains it, so a bare `episodic.llm` in `domain_ports` silently reclassifies the OpenAI adapters out of `outbound_adapter` while `make check-architecture` still exits 0. Add a guard test for that whole class of mistake. Slim `episodic/qa/__init__.py` in the first milestone. Importing any evaluator contract currently loads LangGraph and httpx transitively, which would have made the plan's own domain-purity constraint decorative. Split `QaEvaluationSummary` from `QaEvaluation` and store `finding_count`. The previous revision specified a findings-free list query alongside a response body carrying a finding count, which no adapter could have produced. Give `artefact_schema_version` stated semantics, add `compliance_policy_version` so a stored compliance verdict records the rule that produced it, derive the idempotency key deterministically, and let a successful evaluation supersede an earlier errored one so a transient failure is not permanent. Add store metrics and structured logging, narrow the IntegrityError handler by constraint name, and rebuild the index set around the queries actually issued. Replace an infeasible Hypothesis state machine and two negative controls that would have failed for the wrong reason; add obligations for blocking-set independence, mapping fidelity, key determinism, errored supersession, constraint enforcement, timezone handling, JSONB round-tripping, count agreement, and enum taxonomy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Reviewer's GuideThis PR adds a single, very large execution-plan document describing how to implement persistent QA artefacts linked to canonical episodes, including milestones, domain and storage design, HTTP and CLI interfaces, verification strategy, and architectural constraints; there are no production code changes yet, only the plan. Sequence diagram for recording and retrieving a QA evaluationsequenceDiagram
participant Evaluator
participant Recorder as QA_recording_service
participant Store as PostgreSQL_store
participant API as HTTP_API
participant CLI
Evaluator->>Recorder: evaluation_from_pedante(result)
Recorder->>Store: record_evaluation(evaluation)
Store-->>Recorder: stored evaluation with findings
Recorder-->>Evaluator: recorded QA artefact
API->>Store: list_evaluations(request)
Store-->>API: summaries and total
API-->>CLI: HTTP response
CLI->>CLI: render evaluations
Entity relationship diagram for persisted QA artefactserDiagram
CANONICAL_EPISODE ||--o{ QA_EVALUATION : has
GENERATION_RUN o|--o{ QA_EVALUATION : correlates
QA_EVALUATION ||--o{ QA_FINDING : contains
CANONICAL_EPISODE {
uuid id PK
}
GENERATION_RUN {
uuid id PK
}
QA_EVALUATION {
uuid id PK
uuid episode_id FK
uuid generation_run_id FK
enum evaluator
enum compliance_status
int finding_count
int compliance_policy_version
}
QA_FINDING {
uuid id PK
uuid evaluation_id FK
int ordinal
boolean is_blocking
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Both questions put to the reviewer have been ruled on, so neither should still read as an open proposal. The deviation from the design document's Data Model section is accepted: `qa_evaluations` plus `qa_findings` with a `compliance_status` column replaces the named `brand_compliance_results` table. Record the affected identifiers, the downstream impact on the schema and filter contract, and the upstream document edits the acceptance obliges, so EP-M6 cannot be marked complete while the design document still names a table that was never built. The CLI stays in scope for 2.2.7. Splitting the bootstrap out up front was offered and declined; the budget-triggered escalation to a roadmap addendum item remains available and needs no further approval when taken. The plan's status stays DRAFT: these two decisions are settled, but the plan as a whole has not yet been approved for implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
This branch carries the pre-implementation execution plan for roadmap item
2.2.7, "Persist QA artefacts linked to canonical episodes". No production code
changes; the single deliverable is the plan itself.
Today the service can run two quality-assurance evaluators — Pedante and
Chrono — but their results live only in an in-memory LangGraph state object and
are discarded when the graph finishes. Nothing reaches the database, so no
operator can ask what an evaluator said about an episode. The plan authorizes
building a durable QA artefact linked to a canonical episode, plus retrieval
over both HTTP and a new first-party command-line interface, filtered by
evaluator and compliance status.
Roadmap task: (2.2.7)
Execplan: docs/execplans/2-2-7-persist-qa-artefacts-linked-to-canonical-episodes.md
What the plan authorizes
Seven milestones, each ending in a coherent repository state: import-purity and
architecture grouping (EP-M0); the domain model and ports (EP-M1); the
PostgreSQL adapter, migration, and store observability (EP-M2); the recording
and query services with the Pedante and Chrono projections (EP-M3); two REST
endpoints (EP-M4); one CLI command (EP-M5); and two architecture decision
records plus the documentation and roadmap updates (EP-M6).
Wiring evaluators into the generation-run execution path is explicitly out of
scope; that remains roadmap item 4.4.1.
Review walkthrough
Reviewers short of time should read the decision log and the three structural
decisions it records; everything else follows from those.
Purpose / big picture
for the observable outcome, then
Constraints
for what the plan promises not to disturb — in particular that
QaStatusandQualityModestay as ADR 017 left them.Decision log.
Three decisions deserve scrutiny because each deviates from an upstream
artefact or fixes a semantic that will be hard to change later: replacing the
design document's
brand_compliance_resultstable with acompliance_statuscolumn on a unified evaluation record; recording acompliance_policy_versionso a stored verdict states the rule that producedit; and treating an errored artefact as provisional so a transient evaluator
failure is not permanent.
records what reconnaissance and review turned up, all verified against the
tree. Two findings change the work rather than describing it: Hecate
classifies a module by the first matching group, so an ancestor prefix
silently voids a later group's rules while
make check-architecturestillexits 0; and importing any evaluator contract currently loads LangGraph and
httpxtransitively, becauseepisodic/qa/__init__.pyimports the graphbuilders.
is the longest section and the one worth reviewing hardest. Each obligation
names its method, artefact, command, and a negative control that must fail
for the intended reason. Two obligations exist purely to stop other
obligations being vacuous:
INV-BLOCKING-SETpins the blocking supportlevels against an independently written literal, and
INV-COUNT-AGREEScross-checks the listed finding count against the detail representation.
gives the prescriptive shapes: the domain types, the port protocol, the
versioning semantics, the SQL schema with its two indexes, the adapter's
narrowed
IntegrityErrorhandling, the HTTP contract, and the CLI contract.and
Module budget
close the loop on scope. The budget is honest about the size of this item:
roughly 46 files against a 50-file tolerance, driven by the CLI bootstrap.
Validation
Documentation gates on the branch head:
Claims in the plan that were checked against the tree rather than asserted:
Code gates were not run because this branch changes no code.
Notes
The plan was produced with reconnaissance across the QA, persistence, API, and
orchestration subsystems, then revised after a six-lens design review covering
structural integrity, contracts, failure modes, verification non-vacuity,
alternatives, and long-term viability. The
Revision note
records what that review changed and why.
Two design questions were put to the reviewer and have since been ruled on;
both acceptances are recorded in the plan's decision log, so neither is an open
proposal any more:
brand_compliance_resultsdeviation — accepted. The plan updatesthe design document's Data Model bullet rather than building the table it
names, because that table's only producer, Anthem, is unimplemented, and a
two-table split would turn the roadmap's own filtered-retrieval requirement
into a union query. The acceptance names the upstream edits it obliges:
EP-M6 cannot be marked complete while the design document still names a
table that was never built.
retrieval "via API and CLI", but no CLI exists and roadmap 4.6.1
("Extend CLI client") assumes one does. Splitting the bootstrap out up
front was offered and declined. The budget-triggered escalation to a roadmap
addendum item 2.2.8 remains available to the implementer and needs no
further approval when taken.
The plan's status remains
DRAFT: these two decisions are settled, but theplan as a whole has not yet been approved for implementation.
Two divergences from
docs/episodic-tui-api-design.mdare recorded rather thanfixed here: an unauthenticated request receives 404 rather than 401, following
the existing
episode_tei.pyprecedent, and the plan raises 401 handling as aseparate item.
References
🤖 Generated with Claude Code
Summary by Sourcery
Define the implementation plan for durably storing and retrieving QA evaluator artefacts associated with canonical episodes without changing production code.
Enhancements:
Documentation:
Tests: