fix(graph): an approved merge was reverted by the next registry load

[sturlese]

Ninth iteration of the autonomous bughunt sweep. A human-approved identity decision, silently undone by the next load.

Bug

apply_merge folded the absorbed names into the new canonical but left the entity that already owned them in place. save_registry then wrote a file where two entities each claim the same alias, and load_registry resolves that by last-writer-wins over entities iteration order — which, because save sorts by cid, is alphabetical cid order rather than the human's decision:

before                 : {'globex': 'globex-industries', 'globex industries': 'globex-industries'}
in-memory after approve: {'globex': 'globex',            'globex industries': 'globex'}
after save + reload    : {'globex': 'globex-industries', 'globex industries': 'globex-industries'}
entities in the file   : ['globex', 'globex-industries']

A human approved a merge into globex; the next load silently undid it.

The other sort direction is not better — the merge stands, but the curated entity's own display name is orphaned from its alias and build_entities emits two nodes for what a human declared one entity. ADR 008 wants identity decisions "auditable in one file's git history"; a file whose meaning flips on cid sort order is neither auditable nor decided.

Fix

The resolution depends on what the merge claimed, because this file is human-owned and removing a record needs more warrant than one shared string:

• the other entity's own name or id is claimed → the merge is asserting they are the same entity → absorb it (its id and names become aliases) and remove it;
• only one of its aliases is claimed → they are still distinct entities that happened to share a spelling → retarget that alias to the canonical and leave the entity otherwise alone.

Its old id is folded in on absorption. normalize keeps hyphens, so the slug globex-industries is a different key from the display name globex industries — dropping it would stop a reference written as the old id from resolving, and an absorbed entity should keep answering to everything it used to.

My first version absorbed on any overlap, and the gate refuted it. Two genuinely distinct entities can share a short alias, and it deleted one of them:

before        : ['acme-bank', 'acme-foods']     # acme-bank is a person, aliased "Acme"
after (v1)    : ['acme-foods']
ABG resolves  : acme-foods                      # a person's alias, now a food company

That folded a curated record's id, display name, type and every other alias into an unrelated entity, and vanished its graph node. Three more defects came from the same over-reach: a merge with absorbs: [] still deleted an entity; several proposals made the surviving identity whichever ran last (main was order-independent); and the canonical inherited the absorbed entity's other aliases, so a third entity claiming those recreated this very bug one hop out.

Nothing destructive is silent. apply_merge takes a log callable and merges approve passes print, because absorbing or retargeting somebody else's entity is a change the approver did not name:

note: alias(es) ['Acme'] moved from 'acme-bank' ('Acme Bank') to 'acme-foods';
      'acme-bank' is otherwise unchanged

by_alias is now derived in one place (_reindex), used by both load_registry and apply_merge. Built incrementally it kept entries pointing at ids that no longer exist — and merges approve applies every chosen proposal against one in-memory registry before saving once, so a later proposal would reason about a phantom entity. That is pinned in memory, not only across a save/load, because it is the live path:

assert reg.canonical_id("globex-industries") == "globex"
assert reg.entities.get("globex-industries") is None

Test

All three new tests confirmed to fail with the source change stashed.

• test_apply_merge_absorbs_an_already_registered_entity — the headline case, asserted both in memory and across a save/load round trip, including that the old id still resolves.
• test_apply_merge_absorbing_the_other_direction_also_settles — the mirror, where the absorbed id sorts first: the merge "survived" on main but left a contradictory file, so both names must resolve to the approved canonical.
• test_apply_merge_leaves_unrelated_entities_alone — the guard against over-absorbing: an entity sharing no name with the merge keeps its id, aliases and resolution. Without it, "absorb everything" would pass the other two.

Mutation-tested, all six caught — including the refuted v1 design itself (absorb on any overlap: 3 failures), never absorbing (4), dropping the old id (1), skipping the reindex (3), claimed omitting the canonical's own id/name (1), and retargeting without stripping (3).

Two of those initially survived and both were my tests' fault rather than the code's: the realistic reindex mutation (keep main's incremental update, don't refactor) passed because my first tests all went through save/load, which reindexes anyway; and claimed omitting the canonical's own id/name passed because I asserted "nothing was deleted" rather than "the contradiction is resolved".

graph 47 (+7) · clean 256 · corpus 48 · slack 13 · fetch 33 · answer 66 · benchmark 3 — bare pytest (CI mode). ruff clean; golden scorecard 25/25, including the graph: canonical entity nodes metric.

Scope note

The existing test_apply_merge_and_save_roundtrip only ever merged into an empty registry, which is why this survived: the absorbed-existing-entity case was the untested half of the same function. Its idempotent-re-apply assertion still holds unchanged.

Deliberately not fixed

• A registry already damaged by the old approve is not repaired. It still holds two entities claiming one alias and still resolves by cid sort order; only future merges are correct. A merges doctor that reports contradictions would be the honest remedy — backlog.
• A contradiction a human hand-authors between entities no merge touches is carried forward, not silently resolved. Resolving someone else's ambiguity is not a merge's business.
• Two contradictory proposals approved in one run are last-wins — deterministic given the human's approval order rather than an arbitrary sort, which is the improvement, but arguably it should refuse.

[sturlese]

Gate verdict

VERDICT: REFUTED — and the reviewer was right on every count. My fix was worse than the bug it fixed. Redesigned in 346f2ed.

The core error: I gave apply_merge authority to delete curated records

The registry is a human-owned identity file (ADR 008). My version absorbed any entity whose normalized names overlapped the merge's — and two genuinely distinct entities can carry the same short alias:

before        : ['acme-bank', 'acme-foods']     # acme-bank is a person, aliased "Acme"
after (v1)    : ['acme-foods']
ABG resolves  : acme-foods                      # a person's alias, now a food company

Its id, display name, type person and every other alias were folded into an unrelated company; its graph node entities/person/acme-bank vanished and links to it dangled. The judgment upstream was "these spellings are the same entity", not "delete Acme Bank".

Three more defects fell out of the same over-reach, all verified:

• A merge absorbing nothing still deleted an entity — claimed always includes the canonical's own id and name, so absorbs: [] was enough.
• New order dependence — with several proposals the surviving canonical identity became whichever ran last. Main was order-independent here; I traded sensitivity to cid sort order for sensitivity to approval order.
• The headline symptom survived one hop out — the canonical inherited the absorbed entity's other aliases, so a third entity claiming those recreated the exact cid-sort-order contradiction. The reviewer reproduced my own before/after table on the fix.

The redesign

Resolution now depends on what the merge claimed, which is the warrant question:

• the other entity's own name or id is claimed → the merge is asserting they are the same entity → absorb and remove it, as before;
• only one of its aliases is claimed → they are still distinct entities that shared a spelling → retarget that alias to the canonical and leave the entity otherwise alone.

That fixes all four findings at once. The person survives with its type and ABG intact; the genuine same-entity case still absorbs and its old id still resolves; a no-op merge deletes nothing; and the one-hop case survives a reload because the canonical now claims only what the merge named.

Nothing destructive is silent any more

The reviewer's point that byte-identical CLI output while destroying a record fails the minimum bar was fair. apply_merge takes a log callable and merges approve passes print:

note: alias(es) ['Acme'] moved from 'acme-bank' ('Acme Bank') to 'acme-foods';
      'acme-bank' is otherwise unchanged

Mutation testing, including the refuted design

All six caught now, where two survived before:

mutant	result
never absorb / never retarget	4 failed
absorb on ANY overlap (the refuted v1)	3 failed
drop the old id from the fold	1 failed
skip the reindex in apply_merge	3 failed
claimed omits the canonical's own id/name	1 failed
retarget but don't strip the alias	3 failed

Two corrections to my earlier claims the reviewer forced. My mutation report said skipping the reindex was caught — the realistic form (keep main's incremental update) survived; it's caught now. And claimed omitting the canonical's id/name survived because my test only asserted "nothing was deleted"; it now asserts the contradiction is actually resolved, which is the property that matters.

Confirmed sound, and worth recording

• _reindex is behaviour-preserving for load_registry: on a pre-existing shared alias, main's incremental build and _reindex produce identical maps.
• The phantom-entity resurrection across save/load that main had is genuinely fixed.
• by_alias is written in exactly one place now; no sibling writer missed.
• Absorption is one-hop and independent of reg.entities iteration order (claimed is computed before the loop).

Recorded, not fixed

• Registries already damaged by main are not repaired. A file written by the old approve still holds two entities claiming one alias and still resolves by cid sort order. Only future merges are correct. A merges doctor that reports contradictions would be the honest remedy — backlog.
• A contradiction a human hand-authors between entities no merge touches still resolves by cid order at load. Separate, pre-existing.
• Two contradictory proposals approved in one run are still last-wins. Deterministic given the human's approval order rather than an arbitrary sort, which is an improvement, but arguably it should refuse.

graph 47 (+7) · clean 256 · corpus 48 · slack 13 · fetch 33 · answer 66 · benchmark 3, bare pytest (CI mode) · ruff clean · scorecard 25/25.