ADR 0008: Enrichment persistence simplification scope¶
Status: Accepted (refactoring plan Phase 2 Step 2, finding H7). Reviewed by codex, grok, and deepseek-v4-pro.
Context¶
A 4-model refactoring analysis (finding H7) identified the enrichment
persistence model as "enterprise event-sourcing" machinery applied to offline
LLM drafts that a single operator triages. The enrichment subsystem uses
advisory locks, monotonic generations, full-payload idempotency fingerprints,
supersession chains, evidence ordinals, and scope-XOR kinds across three
persistence families: draft candidates (drafts.py), run assets
(run_assets.py), and translations (translations.py). At 21 files and 18+
external import sites, it is the widest-reaching non-API package.
The question: is this proportionate to a single-operator, locally hosted system?
Three constraints make most of the machinery load-bearing:
-
Real concurrency. The Celery worker, the API server, and the operator can all trigger enrichment producers at the same time. Row locks cannot guard a scope with no prior rows (the first invocation for a new speaker or run), so advisory locks fill that gap.
-
Human decision immutability. An operator accept or reject is terminal. A producer rerun must never touch a decided candidate. The supersession mechanism stamps only still-proposed candidates and uses a two-statement
FOR UPDATEthenUPDATEpattern to defeat a READ COMMITTED snapshot anomaly where a concurrent decision would otherwise be invisible to the supersession UPDATE. -
Finalization-order guarantees. Generations are allocated at finalization time under the advisory lock:
MAX(generation) + 1. Supersession scopes togeneration < run.generation, so a late-arriving completion cannot supersede claims from a run that finalized more recently. This is finalization order, not invocation order or causal order. If invocation A starts before B but B finalizes first (generation 1) and A finalizes last (generation 2), A supersedes B. This is the intended policy for a single-operator system: the most recent completed analysis wins.
Not all three constraints apply equally to every family:
| Constraint | Drafts | Assets | Translations |
|---|---|---|---|
| Advisory lock + generation + append-only | yes | yes | yes |
superseded_by IS NULL read-time derivation |
yes | yes | yes |
Decided-candidate immunity (FOR UPDATE then UPDATE) |
yes | no | no |
Per-field covered_fields supersession scoping |
yes | no | no |
| Whole-row head retirement | no | yes | yes |
The remaining H7 "machinery" items, evidence ordinals and scope-XOR kinds, are domain-model rules (stable evidence ordering, target-scope integrity), not persistence-layer complexity. They are out of scope for this ADR.
Decision¶
1. The persistence model stays¶
The append-only generation chain with supersession stamps is the simplest correct model satisfying the three constraints above. A "one current row plus history table" alternative would require moving superseded rows to a history table (an extra write per supersession), checking the history table for decided-candidate immunity (an extra join), and synchronizing the move with the insert (an extra serialization point). For draft candidates, where decisions and evidence must retain stable foreign-key identities, the cross-table split is strictly more complex.
"Latest" continues to be derived at read time via superseded_by IS NULL for
all consumer surfaces. No schema change, no data migration.
2. Generations stay¶
Generation counters cost one MAX+1 query under an already-held lock and
provide clean write-time ordering independent of wall-clock. Removing them
would require replacing the supersession scope with created_at comparisons
(sensitive to clock skew between workers) or supersession-chain traversal (O(n)
in chain length, fragile). The latest_producer_run() query is the only
read-time consumer of generation ordering; everywhere else, supersession stamps
are the filter.
3. Full-payload replay-conflict detection stays¶
The current replay check compares every stored field when an idempotency key matches. This catches key-construction bugs that would otherwise silently adopt a row with different results, source hash, model, or configuration. The cost is one JSON comparison on the uncommon replay path. Reducing to key-only first-write-wins adoption would remove this diagnostic protection for no measurable performance gain.
For translations, where payloads can reach several megabytes, the replay check
should use a stored canonical digest rather than loading and comparing the full
lines JSONB.
The replay comparison surface should include identity, source hash, normalized
payload, schema version, producer/model provenance, and configuration. Process
timestamps (started_at, completed_at) participate only if the idempotency
key is derived from durable job state that also preserves those timestamps. If
a legitimate retry recomputes timestamps, they become a false-conflict source.
Implementation should verify this for each family.
4. Transaction choreography is extracted¶
The advisory-lock, generation-allocation, idempotency-lookup, and
savepoint-protected-persist dance repeats across all three families (~86 lines
in drafts, ~72 in assets, ~42 in translations). M10 already extracted the
savepoint skeleton (savepoint_adopt_or_conflict in idempotency.py). The next
step extends or complements that helper with advisory-lock acquisition and
post-lock idempotency re-lookup.
The extraction boundary is narrow by design: only the invariant transaction
choreography moves into the shared module. Validation, payload construction, row
building, fingerprint definition, and supersession SQL stay in each family
module. The drafts two-statement FOR UPDATE supersession is materially
different from the simple asset/translation head retirement; turning those into
generic predicates would hide concurrency invariants without removing much code.
Net-complexity gate: if the extracted helper saves fewer than ~30 net lines or needs more than three callbacks, keep the current family-local code and standardize only comments and tests.
5. Translation integrity gaps are closed¶
run_translations has two gaps relative to enrichment_candidates and
run_enrichment_assets:
Missing idempotency key. record_translation inserts directly with no
savepoint, no idempotency key, and no replay detection. The job system's
claim_job() CAS prevents LLM re-invocation on Celery redelivery (a succeeded
job returns no row to the claimer), so this is not a crash-retry data-loss
scenario provided record_translation and the job completion state update
happen in one transaction. That atomicity should be verified before treating
translation idempotency as purely defense-in-depth; if the boundary is not
atomic, idempotency becomes a primary correctness guard.
One migration adds a nullable idempotency_key TEXT UNIQUE column and a
replay_digest TEXT column (canonical sha256 of the translation payload, used
for replay comparison instead of loading the full multi-megabyte lines JSONB).
Existing rows receive no backfill (they are completed generations needing no
replay protection). The writer populates both for all new rows. The key derives
from the translation job ID, matching the asset-job convention.
Missing immutability trigger. enrichment_candidates and
run_enrichment_assets both have database triggers rejecting UPDATE (except the
write-once supersession stamp) and DELETE. run_translations documents the same
immutability contract but enforces it only at the application layer. The same
migration adds the trigger, bringing translations to parity.
6. Validation layering stays¶
Python-side validation in the writer modules mirrors the DB CHECK constraints.
This is defense-in-depth, not duplication: the Python layer gives earlier,
clearer error messages before the advisory lock is acquired and the expensive
work is committed. Removing it would push all error reporting to opaque
IntegrityError exceptions from the database.
Consequences¶
- No data migration for the main refactoring. Code-only helper extraction.
- One Alembic migration for
run_translations: nullableidempotency_keycolumn with UNIQUE constraint, areplay_digestcolumn for efficient replay comparison, plus an immutability/supersession-integrity trigger matching the asset pattern. - Reduced code duplication across the three persistence families, contingent on the net-complexity gate.
- Translation integrity brought to parity with assets and candidates.
- The finalization-order-wins policy is explicitly documented and accepted.
- Future enrichment producers follow the extracted protocol rather than copying the advisory-lock/generation/idempotency/supersession pattern by hand.
- Consumer surfaces (review workbench, speakers page, run detail, CLI) are
unaffected. They read through
queries.py,latest_assets(), andcurrent_translations(), none of which change.
Rejected alternatives¶
One current row plus history table. More complex for candidates (decisions and evidence reference candidate rows by FK; moving superseded rows breaks those references or requires duplicating them). Plausible for assets and translations alone via trigger-maintained history, but not worth losing the uniform append-only model across all three families.
Removing generation counters. Saves one MAX+1 query per finalization and
loses write-time ordering safety. The alternatives (wall-clock comparison or
chain traversal) are either clock-skew-sensitive or O(n).
Key-only first-write-wins idempotency. Removes the diagnostic check that catches key-construction bugs. Silent adoption of a row with different results is harder to diagnose than an explicit conflict error.
Verification¶
The refactoring plan requires concurrent test scenarios and validation plans. Since there is no data migration for the main refactoring, these are test scenarios for the translation gap closure and protocol extraction:
- Finalization-order supersession. Invocation A starts, B starts, B finalizes (generation 1), A finalizes (generation 2). Assert A's candidates supersede B's proposed candidates.
- Decision beats supersession. Covered by the existing integration test
test_decision_beats_concurrent_supersessionintests/integration/test_enrichment_drafts.py. - Translation idempotency replay. Insert a translation, call
record_translationagain with the same idempotency key and payload. Assert the existing row is returned, not a new generation. - Translation immutability trigger. Insert a translation row. Attempt
UPDATE of content columns and DELETE. Assert both are rejected. Verify that
stamping
superseded_by_translation_idonce succeeds and stamping it again is rejected. - Crash-after-claim recovery. Document that
claim_job()CAS handles Celery redelivery (a succeeded job returns no row), so result-row idempotency is defense-in-depth, not the primary retry mechanism. - Translation migration rollback. The migration is a single
ALTER TABLE ADD COLUMNplusCREATE TRIGGER. Postgres DDL is transactional:alembic downgradedrops both atomically.