Skip to content

Phase 84: Dedup & Fingerprint-Progress Cutover#228

Merged
SimplicityGuy merged 43 commits into
mainfrom
SimplicityGuy/phase-84
Jul 10, 2026
Merged

Phase 84: Dedup & Fingerprint-Progress Cutover#228
SimplicityGuy merged 43 commits into
mainfrom
SimplicityGuy/phase-84

Conversation

@SimplicityGuy

Copy link
Copy Markdown
Owner

Summary

Phase 84: Dedup & Fingerprint-Progress Cutover
Goal: Cut services/dedup.py and get_fingerprint_progress over to the dedup marker / output tables, so dedup resolve/undo and the fingerprint progress bar derive from data rather than FileRecord.state.
Status: Verified ✓ · threats_open: 0 · nyquist_compliant: true · UAT 3/3 gaps closed

This phase moves the dedup readers and the fingerprint progress endpoint off FileRecord.state and onto the durable dedup_resolution marker and the fingerprint output tables.

Planning discovered that dedup_resolution has had no go-forward writer since migration 032 created itresolve_group stamped FileRecord.state and nothing else. So this is not a pure reader cutover: it adds the writer, converts undo into a compare-and-swap on the marker, reconciles the corpus in both directions, and only then flips the nine read sites.

Then UAT found something bigger: duplicate resolution has never persisted at all. See "The bug UAT found" below.


Changes

84-01 — Migration 035: bidirectional reconcile

Data-only, no DDL, down_revision = "034", documented no-op downgrade(). Re-runs 032's _BACKFILL_DEDUP verbatim (insert missing markers) plus a new DELETE of orphaned markers, so marker ≡ state exactly at the cutover instant. Must land before any reader flips — encoded as a wave dependency.

alembic/versions/035_reconcile_dedup_resolution.py, tests/integration/test_migrations/test_migration_035_reconcile_dedup_resolution.py

84-02 — dedup_resolved_clause() predicate

A stage-less, file-level correlated exists(...) in the single-source predicate module. Deliberately kept out of the five Stage dispatch ladders (dedup is not a stage; those ladders raise on unknown stages and are drift-locked by Phase 78's equivalence test).

src/phaze/services/stage_status.py, src/phaze/models/dedup_resolution.py (D-08 docstring)

84-03 — dedup.py cutover

  • The go-forward writer (pg_insert + on_conflict_do_nothing(index_elements=["file_id"]), explicit id=uuid4() because pg_insert bypasses the Python-side PK default), stamping the operator's actual canonical_file_id.
  • undo_resolve becomes a CAS: DELETE … RETURNING file_id; the previous_state restore applies only to ids that actually held a marker, so a stale-tab replay no-ops.
  • All nine read sites flipped to ~dedup_resolved_clause(). The D-00a dual-writer survives until Phase 90.

src/phaze/services/dedup.py, tests/integration/test_dedup_divergence.py, tests/integration/test_dedup_resolve_undo_shadow.py

84-04 — get_fingerprint_progress rewrite

All three keys derived from done_clause/failed_clause over a shared denominator (file_type IN MUSIC_VIDEO_TYPES AND NOT EXISTS(marker)), so completed ⊆ total. Every new import is function-local — services/fingerprint.py is imported by the agent worker, which cannot import phaze.models.

Numbers will change, and that is the fix: completed previously read state == FINGERPRINTED, whose sole writer is retry_analysis_failed, so it counted ≈nothing. failed was a row count over fingerprint_results, double-counting files and misclassifying one-success/one-failure files. Both are now FILE counts.

src/phaze/services/fingerprint.py, tests/integration/test_fingerprint_progress.py, tests/fingerprint/services/test_fingerprint.py (toothless mock replaced), tests/fingerprint/routers/test_pipeline_fingerprint.py

84-05 — AST source-scan guard

Allows exactly one FileState.DUPLICATE_RESOLVED write in dedup.py (the surviving dual-writer) and forbids every read — walking positional and keyword Call args, closing the blind spot that made two Phase-83 guards toothless. AST rather than grep, because fingerprint.py:268 carries FINGERPRINTED in a docstring.

tests/shared/test_dedup_fingerprint_source_scan.py

84-06 — Live-corpus shadow-compare (read-only)

Executed against production inside BEGIN TRANSACTION READ ONLY. No snapshot needed: shadow_compare has zero write calls. See "Findings" below.


The bug UAT found

POST /duplicates/{hash}/resolve returned HTTP 200, rendered a success partial, and updated the stats header to show the group gone — while writing zero rows. Reload, and the group is back. Same for undo, resolve-all, undo-all.

get_session (database.py:48-51) yields the session and never commits. services/ only flush()es (caller-owned transaction). routers/duplicates.py never committed. The transaction rolled back on session close. Duplicate resolution has never persisted. This predates Phase 84 — the pre-cutover FileRecord.state dual-write only flushed too.

It likely explains why production shows 6 duplicate groups and 0 duplicate_resolved files.

Why no test caught it: tests/conftest.py:216 overrides get_session with the test's own session, so assertions read uncommitted rows from inside the same transaction. Ten existing router tests pass identically with and without the commits.

Fixed in 74f1f12f. The two regression tests assert from an independent session, which sees only committed data — mutation-verified: removing the four commits turns both RED while the ten pre-existing tests stay GREEN.


Requirements Addressed

REQ-ID Description
READ-04 Dedup (services/dedup.py) and get_fingerprint_progress derive from the dedup marker / output tables rather than FileRecord.state.
SIDECAR-02 Dedup resolution (DUPLICATE_RESOLVED) is represented via a durable dedup marker, with resolve/undo preserved and backfilled from existing rows.

Verification

  • Automated verification: 84-VERIFICATION.mdstatus: passed, 7/7 must-haves
  • Code review: 84-REVIEW.md → 0 blockers; 2 warnings (WR-01 marker deleted before payload validation; WR-02 unhandled ValueError on malformed UUID) fixed in 5215d82c with 3 mutation-verified regression tests
  • Security: 84-SECURITY.md → 16/16 threats closed, threats_open: 0 (ASVS L1)
  • Validation: 84-VALIDATION.mdnyquist_compliant: true; dedup.py and stage_status.py at 100% coverage
  • UAT: 84-UAT.md → 8 tests, cold-start migration + live app + real HTTP endpoints; 3 issues found, 3 closed
  • Full suite: 3,189 tests green across all nine buckets; ruff and mypy clean

Every guard in this PR is mutation-tested in both directions — the source is broken, the guard observed RED, then restored. A green guard that was never broken proves nothing (Phase 83 shipped two such guards).


Key Decisions

  • D-00a — Writers dual-write. FileRecord.state keeps being stamped; only reliance on it is replaced. The write dies in Phase 90.
  • D-02/D-03/D-07 — Bulk pg_insert + ON CONFLICT (file_id) DO NOTHING, stamping the operator's actual canonical_file_id. First-writer-wins on a concurrent double-submit.
  • D-06 — The marker is the single CAS domain. DELETE … RETURNING decides what gets written; a stale replay matches zero rows and no-ops.
  • D-13 — The dedup predicate is file-level, not a Stage. It stays out of the dispatch ladders.
  • D-14 — On a consistent corpus no test can tell "reads the marker" from "reads state". The divergence test seeds a deliberately inconsistent corpus (marker + analyzed → excluded; duplicate_resolved + no marker → included).
  • D-17 — All three get_fingerprint_progress keys share one denominator, so the bar can never exceed 100%.

Findings for the release

1. 032 must not ship without this PR. 032 creates dedup_resolution; its only go-forward writer is this phase's resolve_group. A release carrying 032 but not 84 lets an operator resolve one of the 6 waiting groups, stamping state = 'duplicate_resolved' with no marker — recreating the exact bug this phase fixes. Phases 77–84 are all unreleased, so shipping them in one tag satisfies this; do not cherry-pick 032 ahead.

2. hard_fail_total = 0 is NOT reachable on the first deploy. Measured read-only on production: 1001 of 1050 analyzed files have analysis.analysis_completed_at IS NULL, and done_clause(ANALYZE) requires it non-NULL. Nothing in 032035 backfills it. analyzed is a hard invariant, so just shadow-compare will exit 1 with ~1001 divergences. This is not a Phase 84 regression — Phase 84's own invariant (duplicate_resolved) has zero exposure and is clean. Backlog item filed: .planning/todos/pending/analysis-completed-at-backfill.md.

Scope the post-deploy check to duplicate_resolved: 0 divergent until that lands.

3. Production is at Alembic 031. 032/033/034 have never been applied. 035 is therefore a no-op on the real corpus (0 inserts, 0 deletes) and remains the defensive reconcile for any environment where 032 ships ahead of the writer.

4. Three claims in the planning artifacts were fabricated and are corrected in-tree: a _test-suffix "destructive-write guard" that does not exist; an analysis_results table that does not exist; and "routers/duplicates.py relies on get_session to commit", which was never true.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd

SimplicityGuy and others added 30 commits July 9, 2026 17:37
6 plans in 4 waves. Wave 1: migration 035 reconcile + D-13 predicate.
Wave 2: dedup.py writer/undo/9-reader cutover + get_fingerprint_progress
rewrite. Wave 3: AST source-scan guard. Wave 4: live-corpus shadow-compare.
Covers READ-04, SIDECAR-02; all D-00a..D-17 decisions cited literally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
- Data-only, bidirectional reconcile of dedup_resolution against files.state
- Statement 1: 032's _BACKFILL_DEDUP verbatim (insert missing markers)
- Statement 2: DELETE orphaned markers (state <> 'duplicate_resolved')
- No DDL, static parameter-free SQL, empty autogenerate diff, no saq_jobs
- downgrade() is a documented no-op (D-04 Claude's Discretion)
- DB-free: bare-number revision, saq_jobs banner scan, static-SQL scan
- Real-PG both-direction corpus: resolved-no-marker gains marker (derived canonical),
  orphaned marker deleted, control stays row-less, pre-existing marker DO NOTHING
- files.state byte-unchanged snapshot, idempotency re-run, empty autogenerate diff
- no-op downgrade assertion; FOOTGUN docstring for port 5433
- Add 84-01-SUMMARY.md (both-direction reconcile, no-op downgrade, footgun)
- STATE.md position/activity lines only (progress counters untouched)
- ROADMAP plan progress: 1/6 summaries for Phase 84
- Add DedupResolution import + dedup_resolved_clause() correlated exists predicate (D-13)
- Stage-less file-level ColumnElement[bool]; existence of a dedup_resolution marker = resolved
- Deliberately kept OUT of the Stage dispatch ladders (done/failed/inflight/domain_completed/stage_status_case) so the Phase-78 equivalence drift-lock is untouched
- Reproduces shadow_compare._dedup_exists body verbatim as the new public single-source clause
…l (D-08)

- Document that scan_deletion.py deletes markers on EITHER FK (file_id OR canonical_file_id), so deleting a keeper's scan batch un-resolves its duplicates and they reappear for re-review
- Note this is accepted deliberately (safe failure mode) and that D-03 exposes every go-forward resolution to it
- Documentation only; scan_deletion.py intentionally left unchanged
- Add 84-02-SUMMARY.md (self-check PASSED)
- Update ROADMAP plan progress (2/6 summaries)
…do + nine reader flips)

- resolve_group: add the go-forward pg_insert(DedupResolution) writer (D-01/D-02/D-03)
  with explicit uuid id (pg_insert bypasses the Python default) + canonical_file_id =
  the operator's canonical_id, on_conflict_do_nothing(file_id) (D-07); dual-writer stays (D-00a)
- undo_resolve: DELETE(DedupResolution)...RETURNING file_id CAS; restore previous_state only
  for returned ids (D-05/D-06); coerce previous_state to a FileState member (T-84-03-02)
- flip the nine FileRecord.state != DUPLICATE_RESOLVED read sites to ~dedup_resolved_clause() (READ-04)
- update two discovery unit tests to seed markers (cutover makes the marker, not state, authority)
…ve readers (D-14)

Seeds marker≢state (File A: marker+analyzed→EXCLUDED; File B: duplicate_resolved+no-marker→INCLUDED)
across find_duplicate_groups, find_duplicate_groups_with_metadata, count_duplicate_groups,
get_duplicate_stats, and resolve_group's selection. Reverting any reader's ~dedup_resolved_clause()
to the state read inverts the assertions (mutation-tested: line 191 revert → count 1≠2 RED).
…(D-16.1)

Asserts run_shadow_compare(session).hard_fail_total == 0 after resolve, undo, and re-resolve on a
real-PG corpus (group files seeded at DISCOVERED — the one state with no shadow invariant). Adds the
D-06 stale-replay CAS case: re-resolving with a different canonical makes the stale payload's file a
keeper, so the replayed undo matches 0 rows and cannot clobber the re-resolution. Mutation-tested:
removing the pg_insert writer → hard_fail_total=1 (RED); restored → GREEN.
- SUMMARY.md with both mutation-check RED evidence + T-84-03-02 validation decision
- ROADMAP plan progress 3/6, 84-03 checked off; requirements READ-04/SIDECAR-02
…up marker

- total = music/video files AND ~dedup_resolved_clause() (D-10, no state read)
- completed = done_clause(Stage.FINGERPRINT) file count (D-11, was state==FINGERPRINTED)
- failed = failed_clause(Stage.FINGERPRINT) file count (D-11, was a fingerprint_results row count)
- all three share the denominator so completed/failed subset total (D-17)
- 3-key contract preserved (D-09); all new deps imported function-locally (D-00e)
…test

- delete the side_effect assert-your-own-dict stub (green through any rewrite, D-15)
- real-PG corpus pins D-10 denominator, D-11 file-vs-row units, DERIV-05 aggregation, D-17 subset
- marker-not-state proof: state=duplicate_resolved + no marker is INCLUDED in total
- mutation-tested: completed->state==FINGERPRINTED RED (2->0); failed->row count RED (1->3)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…tover (D-14)

- ast.walk guard: dedup.py allows exactly one DUPLICATE_RESOLVED writer, zero reads
- walks Call.args AND Call.keywords for where/filter/filter_by/having (closes Phase-83 blind spots)
- fingerprint.py: zero FileState.FINGERPRINTED (AST-blind to the docstring token)
- five mutation directions encoded hermetically (3 RED, 2 GREEN false-positive checks)
- 84-05-SUMMARY.md with five mutation results recorded
- ROADMAP plan progress (5/6 summaries)
- requirements READ-04, SIDECAR-02 satisfied
Executor 84-03 marked both requirements Complete in REQUIREMENTS.md before
Phase 84 passed verification. The repo's own anti-drift guard
(tests/shared/core/test_requirements_traceability.py) caught it:

  READ-04 marked Complete but Phase 84 not passed
  SIDECAR-02 marked Complete but Phase 84 not passed

Requirement completion is owned by `gsd-sdk query phase.complete`, which runs
only after VERIFICATION.md reports status: passed. Reverting to Pending; the
phase-completion step re-applies the marks at the right time.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…ract

The router-level test still encoded the pre-cutover contract: it seeded a file
with state=FINGERPRINTED and no fingerprint_results row, then asserted
completed == 1. Under D-11 `completed` is derived from done_clause(FINGERPRINT)
— an engine row with status IN ('success','completed') — so that file correctly
counts 0. Plan 84-04 replaced the service-level mock but did not enumerate this
router test.

Rewritten as a mini divergence guard: f3 has state=FINGERPRINTED with no engine
row (must NOT count), f4 has state=METADATA_EXTRACTED with a success row (must
count). Mutation-tested: reverting the endpoint to `state == FINGERPRINTED`
turns it RED.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…e gate

84-06 is autonomous: false and needs a production restore + operator DSN. Deferred,
not skipped: the committed CI test (D-16.1) provably cannot see the real post-032
resolved-without-marker rows, so only the live run proves migration 035 repaired them.

Phase 79 deferred this same run (79 D-02), which is why D-01 — dedup_resolution having
no go-forward writer — went unnoticed for two phases. Phase 84 is NOT complete until
this closes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…1, WR-02)

Code review found undo_resolve deleted markers BEFORE validating the
browser-held payload. Two defects:

WR-01 (defeats this phase's own SC#3): the DELETE ... RETURNING ran first,
then `FileState(entry["previous_state"])` was coerced per file. A coercion
failure hit `continue`, so the marker was already gone while FileRecord.state
stayed 'duplicate_resolved' — exactly the HARD divergence
services/shadow_compare.py:135 forbids. The committed D-16.1 test could not
catch it because it only ever feeds valid states.

WR-02: a malformed UUID or a missing "id" key raised an unhandled
ValueError/KeyError that escaped as an HTTP 500 before reaching the CAS.

Both close with one change: parse and validate every entry into
`restore_by_id` first, then scope the DELETE to only those ids. A marker is
now deleted only when its state restore is guaranteed to follow. The D-06 CAS
is unchanged — RETURNING still decides what actually gets written, so a stale
replay still no-ops. Duplicate payload entries collapse via the dict, and the
return count is the RETURNING cardinality rather than a loop counter.

Three regression tests added. Mutation-verified: all three go RED against the
previous implementation (commit a67ed16) and GREEN against this one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…rement

Ran D-16.2 against the live database read-only (BEGIN TRANSACTION READ ONLY,
transaction_read_only=on). No snapshot, no migration, no writes.

Three corrections to the plan, all verified in source:
  - the "_test-suffix destructive-write guard" does not exist; shadow_compare
    has zero write calls, so a restore was never required
  - hard_fail_total=0 was the wrong pass condition: it aggregates all 13 hard
    invariants, 12 of which this phase does not own
  - the repair is provable by construction, not empirical

The finding: production is at Alembic revision 031. Migrations 032/033/034 have
never been applied, dedup_resolution does not exist, and there are ZERO
duplicate_resolved files (despite 6 unresolved duplicate groups). D-01's premise
is vacuous on this corpus; 035 is a no-op here and remains a defensive reconcile.
fingerprint_results is empty, so the completed-jump/failed-drop is 0 -> 0.

Deploy-ordering constraint recorded: 032 must not ship without Phase 84, or an
operator resolving a duplicate would stamp state with no marker and reintroduce
D-01 exactly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
SimplicityGuy and others added 13 commits July 9, 2026 21:19
All 6 plans executed. SC#3 closed by the read-only live-corpus measurement in
84-06-SUMMARY.md: production is at Alembic revision 031, dedup_resolution does
not exist, and there are zero duplicate_resolved files — the invariant this
phase owns has zero exposure and cannot diverge.

VERIFICATION status human_needed -> passed. READ-04 and SIDECAR-02 marked
Complete; tests/shared/core/test_requirements_traceability.py green (the same
guard that caught the earlier premature marking).

Deploy-ordering constraint recorded as satisfied structurally: nothing reaches
production except via a tagged release, and no release is tagged before this
merges — so 032 and its go-forward writer land together.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
secure-phase found both threats had a mitigation present in source but nothing
testing it — the "closed in SECURITY.md != tested" failure mode.

T-84-04-01 (DoS, agent-worker import boundary): fingerprint.py's function-local
DB imports were honoured but locked by nothing. The Phase-84 AST source scan only
inspects FileState attribute access, and 84-04's sys.modules spot-check was done by
hand and never committed. Adds tests/shared/test_fingerprint_import_boundary.py:
an AST scan of direct module-level imports plus a fresh-interpreter import that
asserts phaze.models / phaze.database / services.pipeline / services.stage_status
never reach sys.modules. The second catches transitive drags the first cannot see.

T-84-03-03 (Tampering, concurrent HTMX double-submit): on_conflict_do_nothing was
present but unexercised. resolve_group filters ~dedup_resolved_clause(), so a
SEQUENTIAL second POST never reaches the INSERT — the conflict fires only when a
concurrent transaction's marker was invisible to our SELECT snapshot. Adds both
cases; the concurrent one models the race by blinding the clause, hitting the real
pg_insert statement.

Mutation-verified:
  hoist a module-level DB import into fingerprint.py -> both import-boundary tests RED
  drop .on_conflict_do_nothing(...) from resolve_group -> concurrent test RED
    (the sequential test stays GREEN — proof the race model was necessary)
Restored: 9 passed, src clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…ing)

The security audit closed T-84-06-02 but flagged that its ACTUAL control was
untested. The plan-time register claimed a "_test-suffix destructive-write guard"
that does not exist in src/phaze/ — the only such check is a pytest.skip in
tests/integration/test_dedup_resolve_undo_shadow.py, a test-suite guard.

What really makes `just shadow-compare --database-url <live-dsn>` safe against a
production database is that services/shadow_compare.py issues only select()s and
its CLI imports neither main nor Alembic. That property was relied on to run the
Phase-84 live-corpus check against real production data, and nothing tested it.

Adds a DB-free AST guard: no insert/update/delete/text import, no mutating Session
method. AST rather than grep on purpose — the module's docstring and a comment
contain the words `text()` and `update` as prose (lines 21, 66), which a
line-oriented grep would flag. Same trap that made two Phase-83 guards toothless.

Mutation-verified: importing `delete` -> RED; adding `session.add(...)` -> RED;
the docstring prose stays GREEN. Restored, src clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
validate-phase found the only uncovered lines in services/dedup.py (315, 325) sit
inside the T-84-03-02 undo-payload validation, which is attacker-reachable via the
browser-held [{id, previous_state}] payload. dedup.py now at 100%.

Adds two cases:
  test_undo_accepts_uuid_typed_id            -> a UUID-typed id (internal callers)
  test_undo_with_null_previous_state_keeps_marker -> JSON null / int / list

Finding recorded rather than papered over: the `isinstance(raw_state, str)` check
is NOT a runtime control. FileState is a StrEnum, so its members already satisfy
isinstance(x, str), and FileState(None|42|[...]|True) raises ValueError anyway.
That line exists only to narrow Any|None -> str for mypy; removing it changes no
behaviour and no mutation turns it RED. The load-bearing control is the
try/except ValueError, and removing THAT turns both new tests RED. A branch that
survives every mutation is type-checker scaffolding, not a guard — the test
docstring says so.

Mutation-verified:
  drop the UUID-typed id branch   -> test_undo_accepts_uuid_typed_id RED
  drop the try/except ValueError  -> both null/invalid-state tests RED
  drop only the isinstance narrow -> all GREEN (redundant, as documented)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
UAT against a real app + real database found that duplicate resolution has NEVER
persisted. POST /duplicates/{hash}/resolve returned HTTP 200, rendered a success
partial, and the stats header showed the group gone — while the database received
zero rows. A page reload brought the group back.

Root cause: `get_session` (database.py:48-51) yields the session and never commits;
`routers/duplicates.py` never committed either; `services/dedup.py` only flush()es
per the caller-owned-transaction discipline. The transaction was rolled back on
session close. All four write endpoints were affected: resolve, undo, resolve-all,
undo-all.

The planning docs, D-02, and resolve_group's own docstring all asserted that
"routers/duplicates.py relies on get_session to commit". That was never true. The
real convention is that mutating routers commit explicitly (routers/tags.py:369,
routers/tracklists.py).

This predates Phase 84 — before the cutover, resolve_group's FileRecord.state
dual-write also only flushed, so it never persisted either. It likely explains why
production shows 6 duplicate groups and 0 duplicate_resolved files.

Why every test missed it: tests/conftest.py:216 overrides get_session with the
test's OWN session, so assertions read uncommitted rows from inside the same
transaction. The two regression tests added here assert from an INDEPENDENT session,
which by definition sees only committed data. Mutation-verified: removing the four
commits turns both RED, while the 10 pre-existing tests stay GREEN — exactly the
blindness that let this ship.

Verified end-to-end against a fresh migrated DB and a live uvicorn: resolve writes
the marker (canonical_file_id = operator's pick) + dual-writes state and the group
leaves the page; undo deletes the marker and restores previous_state;
shadow-compare stays hard_fail_total=0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
…g item filed

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoehQo6XGjqCq9bt9JSGZd
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@SimplicityGuy SimplicityGuy merged commit 09cefc6 into main Jul 10, 2026
31 checks passed
@SimplicityGuy SimplicityGuy deleted the SimplicityGuy/phase-84 branch July 10, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant