fix(dedup): deterministic collision survivor + bare-file definer (#1851 followup)

Two tweaks on top of @bchan84x's #1852:

1. The definer heuristic decided the survivor pairwise, so when several
   same-file nodes co-defined one ID the surviving label still depended on
   arrival order (the exact 3-node case #1851 reports). Choose the survivor
   by a total order (definer first, then shorter/canonical label, then
   lexically) via a min over _collision_rank, so it is order-independent.
   Direction preserves #1504 (lexically-first source path wins).

2. _defines_id now also recognizes a bare file-level node whose id is
   exactly the slugified path (nid == prefix), not only <path>_<entity>.

Adds an order-independence test over all 6 permutations of definer +
same-file relabel + cross-file reference, and a bare-file-node test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
safishamsi
2026-07-14 13:51:42 +01:00
parent 70bc9ca7b2
commit fd54f0ef0b
3 changed files with 59 additions and 2 deletions
+2
View File
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
## 0.9.16 (unreleased)
- Fix: the dedup ID-collision warning now reports in proportion to what is actually lost, and an ID collision resolves to a deterministic survivor (#1851 / #1852, thanks @bchan84x). Two nodes can mint the same `<path>_<entity>` ID when a document merely references an entity defined elsewhere; the old code kept whichever arrived first and always printed a scary "two files with the same name" warning that overstated the loss (edges rewire to the survivor, so a reference collapse loses nothing). Now the node whose `source_file` actually defines the ID wins, a harmless reference collapse is silent, a same-file relabel is a quiet note, and only a genuine cross-file collision warns. The survivor is chosen by a total order (definer first, then the shorter/canonical label, then lexically) so it no longer depends on chunk arrival order, including when several same-file nodes co-define one ID.
- Fix (regression from 0.9.15): a nested `.gitignore`/`.graphifyignore` no longer applies its patterns outside its own directory (#1873 / #1887 / #1885, thanks @Alwyn93). When 0.9.15 started reading nested ignore files, a non-anchored pattern was still matched against the path relative to the scan root, so a nested bare `*` (a common "ignore this scratch dir" idiom, e.g. what the hypothesis library writes into `.hypothesis/`) matched every file and `detect()` returned zero files, silently producing an empty graph. Patterns are now matched relative to the directory whose ignore file defined them and only apply within that subtree; the anchor directory itself is exempt.
- Fix (regression from 0.9.15): `graphify update` no longer emits 0 nodes and refuses to overwrite an existing `graph.json` when the source tree contains a nested broad `.gitignore` (#1880). This was the update-layer symptom of the scoping bug above: the zeroed re-scan produced an empty rebuild, which the shrink-guard then correctly refused. With subtree scoping fixed the rebuild sees the real files again; the shrink-guard is unchanged.
+28 -2
View File
@@ -218,7 +218,30 @@ def _defines_id(node: dict) -> bool:
source_file = node.get("source_file") or ""
if not nid or not source_file:
return False
return any(nid.startswith(f"{prefix}_") for prefix in _id_prefixes(source_file))
# `nid == prefix` covers a bare file-level node whose id is exactly the
# slugified path with no `_entity` suffix (a semantic node for the file
# itself); `startswith(prefix + "_")` covers the usual `<path>_<entity>` id.
return any(nid == prefix or nid.startswith(f"{prefix}_")
for prefix in _id_prefixes(source_file))
def _collision_rank(node: dict) -> tuple:
"""A total order for choosing the survivor of an ID collision, independent of
the order the colliding nodes arrive in.
The winner is the node with the SMALLEST rank. A node whose ``source_file``
defines the ID always outranks a mere reference; among equally-(non-)defining
nodes it prefers the shorter, more canonical label over a longer qualified
variant, then breaks any remaining tie lexically on label and then source_file
(so the lexically-first path wins) — fully deterministic regardless of order.
"""
label = node.get("label") or ""
return (
not _defines_id(node), # definers (False) sort before references (True)
len(label), # shorter, more canonical label first
label, # lexical tiebreak
node.get("source_file") or "", # lexically-first source path wins
)
def _report_id_collision(nid: str, survivor: dict, losers: list[dict]) -> None:
@@ -304,7 +327,10 @@ def deduplicate_entities(
incumbent = seen_ids.get(nid)
if incumbent is None:
seen_ids[nid] = node
elif _defines_id(node) and not _defines_id(incumbent):
elif _collision_rank(node) < _collision_rank(incumbent):
# Smallest-ranked node wins; the min over a total order is independent
# of the order nodes arrive in, so the survivor no longer depends on
# chunk ordering (#1851).
seen_ids[nid] = node
dropped[nid].append(incumbent)
else:
+29
View File
@@ -513,6 +513,35 @@ def test_same_file_relabel_is_noted(capsys):
assert "WARNING" not in captured.err
def test_collision_survivor_is_order_independent():
"""#1851: definer + same-file relabel + cross-file reference. Across every
insertion order the SAME node (source_file AND label) must survive the
definer heuristic alone left the label order-dependent among co-definers."""
import itertools
nid = "agents_make_batch_fixtures_make_batch_fixtures"
definer = {"id": nid, "label": "make-batch-fixtures agent",
"file_type": "concept", "source_file": "agents/make-batch-fixtures.md"}
relabel = {"id": nid, "label": "make-batch-fixtures helper agent",
"file_type": "concept", "source_file": "agents/make-batch-fixtures.md"}
xref = {"id": nid, "label": "make-batch-fixtures", "file_type": "concept",
"source_file": "available/diagnose-issue/SKILL.md"}
survivors = set()
for perm in itertools.permutations([definer, relabel, xref]):
out, _ = deduplicate_entities([dict(n) for n in perm], [], communities={})
assert len(out) == 1
survivors.add((out[0]["source_file"], out[0]["label"]))
assert survivors == {("agents/make-batch-fixtures.md", "make-batch-fixtures agent")}, (
f"non-deterministic collision survivor: {survivors}"
)
def test_bare_file_node_defines_its_own_id():
"""A file-level semantic node whose id is exactly the slugified path (no
`_entity` suffix) must be recognised as defining its id (#1851 tweak)."""
assert _defines_id({"id": "agents_make_batch_fixtures",
"source_file": "agents/make-batch-fixtures.md"})
def test_defines_id_helper():
assert _defines_id(_DEFINING)
assert not _defines_id(_REFERENCING)