fix(clean): editing a canonical silently lost its duplicate's content - #41
Conversation
`test_classify_pending_promotes_orphaned_duplicate` states the invariant: when a
duplicate's canonical stops holding the shared content, "the content it still holds
gets its own page (otherwise it is lost forever)". It was enforced for only one of
the two ways that happens — the canonical leaving the inventory — and not for the
canonical's bytes CHANGING. Two loci, one pass apart.
state.py: `orphaned_dup` tested `duplicateOf not in inventory`. Edit the canonical and
the duplicate is not re-pended at all:
pending after A changed: [('A', 'changed')] <- B never reconsidered
B still holds the old bytes, so that document has no page anywhere. And there is no
route back: B's own bytes never change again, and its canonical stays in the inventory
forever, so no later pass reconsiders it. Silent, permanent, no error.
main.py: `dedup_pending` seeded the canonical index from the RECORDED rawHash of any
processed file still in the inventory — including a file that is in `pending` precisely
because its hash changed, i.e. one whose recorded hash it no longer serves:
state: A processed rawHash=hOLD; pending: A(hNEW, changed), B(hOLD, new)
-> kept ['A'], B filed as duplicate of A
So a new document carrying those old bytes is deduped onto a page that does not have
them. Same loss, reached from the other direction.
Fixes: dedup skips any pending file's recorded hash when seeding (a pending doc with a
rawHash is by definition stale), and classify_pending hashes the whole present
inventory up front so it can compare the CANONICAL's current hash rather than only its
presence. `hashes.get(canon, raw_hash)` is what keeps the pinned second half of the
existing test green — a canonical that is in the inventory but missing on disk stays
deduped, no churn. Hashing up front is the same total work the loop already did.
Verified in both directions and end to end through run_once: B is reprocessed and gets
its own page, C carrying A's original content is processed rather than deduped, and
both settle with nothing pending on the following pass. Mutation-tested: reverting
either locus fails 2 tests, and dropping the missing-on-disk default fails the pinned
contract.
clean 255 (+3). scorecard 25/25 (curation dedup metric included), ruff clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng safe Dedup exists to avoid LLM calls, so a fix that re-pends duplicates it should not costs money on every 30-minute pass, forever. Convergence is therefore the load- bearing property of the previous commit, and nothing asserted it. Verified by hand across seven passes and now pinned: with THREE identical files, editing the canonical promotes exactly ONE duplicate and re-dedups the other onto it (A processed, B promoted, C -> duplicateOf B), the next pass has nothing pending, and reverting the canonical's bytes settles too (A becomes a duplicate of B, zero pending after). What this rules out: both duplicates getting pages, both staying lost, and oscillation between passes. The mutation set now shows 3 failures for either locus instead of 2. clean 256 (+1). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gate review: the comment claimed a tighter invariant than the expression. `restale` is every pending file carrying a hash, which includes `new` docs and duplicates — what makes it correct is its overlap with the seeding candidates, which also require `status == "processed"`, and a processed file can only be pending because its recorded hash differs. So the files actually removed from the seed are exactly the processed ones whose hash is stale; the rest were never seeding candidates. The reviewer built the tighter variant (restrict to reason == "changed") and proved it a semantically EQUIVALENT mutant, not a coverage gap: byte-identical output across a 60-seed x 6-mutation differential fuzz. Left as is, with the comment now describing the mechanism rather than asserting the conclusion. Comment only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gate verdictVERDICT: SURVIVES — the first clean verdict in eight iterations. Worth recording what it took to earn it, because the reviewer attacked the thing that actually mattered. Churn was the risk, and it was attacked properlyDedup exists to avoid LLM calls, so a fix that re-pends duplicates it shouldn't costs money on every 30-minute pass, forever. That is why this bug sat on the deferred list through several earlier reviews. The reviewer ran nine hand-built scenarios plus a differential fuzz: 60 seeds × 6 random mutations over 4 files, 3 passes per step plus 3 further idle passes as a convergence invariant, replayed identically against Every one of those 354 diff lines has the same shape: main leaves a duplicate pointing at a canonical whose content moved on and no page serves its bytes; the branch gives it a page. Total cost: 20 extra LLM calls out of 234 across 360 mutation steps, all one-time. Not one pass anywhere re-issued a call for a settled file. It also confirmed the three-identical-files case resolves in a single pass (B promoted, C re-dedups onto B), that flip-flopping the canonical six times costs at most one call per flip, and that The regression I was most worried about does not existI'd checked the requeued-with-unchanged-bytes case myself; the reviewer proved why it can't bite, which is better than my empirical answer: a Hashing cost claim verified2000 files × 200 KB: branch 543 ms vs main 584 ms — within noise. Nothing hashed that wasn't hashed before. Third locus checked: promotion restores page and facts
Fixed from the review
Recorded, not fixed
Also noted: promotion adds the doc to Mutation set: 4 of 5 killed, the survivor proven equivalent. clean 256 (+4), coverage 94%; corpus 48 · graph 40 · slack 13 · fetch 33 · answer 66 · benchmark 3; ruff clean; scorecard 25/25 including the |
Eighth iteration of the autonomous
bughuntsweep, moving to the pipeline packages. Silent, permanent content loss with no error and no route back.Bug
test_classify_pending_promotes_orphaned_duplicatealready states the invariant:That was enforced for only one of the two ways a canonical stops holding the shared content — leaving the inventory — and not for its bytes changing. Two loci, one pass apart.
state.py—orphaned_duptestedduplicateOf not in inventory. Edit the canonical and the duplicate is never reconsidered:Bstill holds the old bytes, so that document has no page anywhere. And there is no route back:B's own bytes never change again, and its canonical stays in the inventory forever, so no later pass revisits it. Silent, permanent, no error.main.py—dedup_pendingseeded the canonical index from the recordedrawHashof any processed file still in the inventory, including a file that is inpendingprecisely because its hash changed — i.e. one whose recorded hash it no longer serves:So a new document carrying those old bytes is deduped onto a page that does not have them. Same loss, reached from the other direction.
Fix
dedup_pendingskips any pending file's recorded hash when seeding the index. A pending doc carrying arawHashis by definition one whose recorded hash is stale, so it cannot vouch for that content.classify_pendinghashes the whole present inventory up front, so it can compare the canonical's current hash rather than only its presence.hashes.get(canon, raw_hash)is what keeps the pinned second half of the existing test green: a canonical that is in the inventory but missing on disk stays deduped, exactly as before, with no churn. Hashing up front is the same total work the loop already did file-by-file — nothing is hashed that was not hashed before.Test
All three confirmed to fail with the source changes stashed.
test_dedup_never_points_a_duplicate_at_changed_content— end to end throughrun_once: pass 1 dedups B onto A, thena.mdis edited; B must be processed and get its own page, and it must settle with nothing pending on the next pass. Today:assert ['A'] == ['A', 'B'].test_dedup_does_not_seed_the_index_with_a_changed_docs_old_hash— the other direction: C carries A's original content while A has moved on; C must be processed, not deduped. Today:assert 'duplicate' == 'processed'.test_classify_pending_promotes_a_duplicate_whose_canonical_changed— the unit-level half, extending the file that already pins the canonical-gone case.test_dedup_converges_when_a_canonical_of_several_duplicates_changes— convergence, which is the property that makes re-pending safe at all: with three identical files, editing the canonical promotes exactly one duplicate and re-dedups the other onto it, the next pass is zero-pending, and reverting the bytes settles too.Mutation-tested: reverting either locus fails 3 tests, and dropping the missing-on-disk default fails the pinned contract — so the fix cannot drift into re-pending duplicates whose canonical is merely absent from disk.
clean 256 (+4) · corpus 48 · graph 40 · slack 13 · fetch 33 · answer 66 · benchmark 3 — bare
pytest(CI mode). ruff clean; golden scorecard 25/25, including thecuration: dedup + allowlistmetric.Why this was safe to fix now
It sat on the deferred backlog through several earlier reviews, and the reason was churn: re-pending duplicates that should stay deduped costs LLM calls on every 30-minute pass, forever. So convergence was verified before anything was claimed — by hand across seven passes, and then by the gate with a differential fuzz of 60 seeds × 6 mutations against
main, with three idle passes after each step as a convergence invariant. Zero non-convergence on either revision, and the total price of the fix is 20 one-time LLM calls out of 234 across 360 mutation steps. Every behavioural difference frommainis a document that used to have no page getting one.Keying the decision off a computed hash rather than mere presence is what satisfies both halves of the existing test at once.
Deliberately still open
hashes.get(canon, raw_hash)leaves one hole: a duplicate whose canonical is in the inventory but absent from disk still holds content no page serves. That is exactly whattest_classify_pending_promotes_orphaned_duplicate's second assertion pins, so preserving it is correct for this change — but it means one of two remaining holes is closed here, not both. The open question is whether in-inventory-but-missing-on-disk is transient (a mid-sync raw dir) or permanent; the answer decides whether that default is right or is the next bug. Recorded on the backlog.Minor, unmentioned before: promotion adds the doc to
touched, so theversionsanddossierspost-pass phases fire on it once. Bounded, and neither writes anythingclassify_pendingreads, so it cannot cause churn.