diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cf6f7ba..032eb104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 `_` 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. diff --git a/graphify/dedup.py b/graphify/dedup.py index 9da26b30..79a1bfb8 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -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 `_` 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: diff --git a/tests/test_dedup.py b/tests/test_dedup.py index ee69cae0..57fe6c19 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -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)