diff --git a/graphify/dedup.py b/graphify/dedup.py index 56958ab42..041e5b7b9 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -445,7 +445,8 @@ def _rank_path(source_file: str, root: Path | None) -> str: return normalized -def _collision_rank(node: dict, root: Path | None = None) -> tuple: +def _collision_rank(node: dict, root: Path | None = None, + mentions: "frozenset | set" = frozenset()) -> tuple: """A total order for choosing the survivor of an ID collision, independent of the order the colliding nodes arrive in. @@ -458,11 +459,18 @@ def _collision_rank(node: dict, root: Path | None = None) -> tuple: in-repo colliders by segments present in both path forms, so absolute and repo-relative spellings of the same layout order identically — fully deterministic regardless of arrival order (#1851) or checkout location. + + ``mentions`` (see ``_mentions``) holds the ``(id, file)`` of nodes read as a + mention of a document rather than the document itself. Such a node ranks as a + reference even though its id is a path slice, so a real document outranks it + whatever the label lengths are (#4281). The set only ever contains path-slice + ids, so the survivor of every other collision is chosen exactly as before. """ label = node.get("label") or "" rank_path = _rank_path(node.get("source_file") or "", root) return ( - not _defines_id(node), # definers (False) sort before references (True) + # definers (False) sort before references (True) + not _defines_id(node) or (node.get("id"), rank_path) in mentions, _lifecycle_penalty(rank_path), # active paths beat archived ones (#2532) len(label), # shorter, more canonical label first label, # lexical tiebreak @@ -470,6 +478,52 @@ def _collision_rank(node: dict, root: Path | None = None) -> tuple: ) +def _mentions(nodes: list[dict], edges: list[dict], + root: Path | None) -> set[tuple[str, str]]: + """``(id, root-relative file)`` of nodes read as a MENTION of a document (#4281). + + A semantic fragment sometimes mints the bare id of a file it only points at: + ``protocol`` inside docs/firmware/protocol.md, standing for docs/protocol.md. A + node is read as such a mention when both hold: + + - another node of its OWN file points at it with a ``references`` edge whose own + source_file is that file, and + - its id is itself a path slice of that file (``_id_prefixes``), so it names a + document. An entity id such as ``x_intro`` is the target of ordinary intra-file + ``references`` edges and never counts. + + ``_path_qualify_dropped_collisions`` and ``_collision_rank`` both read this one + set, so a single definition decides whether a mention is renamed and whether it + loses a survivor tie. + + Heuristic boundary: this is INFERRED from the edges an LLM chose to emit, not + stated by the source. A missing or reversed edge gives no evidence, and the id is + then renamed per file like any other. A real document whose own node is linked to + its file-level id the same way (an entity named like its file, which + ``_doc_twin_remap`` already treats as the file itself) reads as a mention, and + that collision collapses exactly as it did before. + """ + def key(sf): + return _rank_path(sf, root) if sf and isinstance(sf, str) else "" + + own: dict[str, set[str]] = defaultdict(set) + for node in nodes: + nid = node.get("id") + if nid and isinstance(nid, str): + own[key(node.get("source_file"))].add(nid) + found: set[tuple[str, str]] = set() + for edge in edges: + if not isinstance(edge, dict) or edge.get("relation") != "references": + continue + k = key(edge.get("source_file")) + source, target = edge.get("source"), edge.get("target") + if (k and isinstance(source, str) and isinstance(target, str) + and source != target and source in own[k] and target in own[k] + and target in _id_prefixes(k)): + found.add((target, k)) + return found + + def _same_source_entity(survivor: dict, duplicate: dict) -> bool: """True when exact-ID records came from the same source file. @@ -659,6 +713,17 @@ def _path_qualify_dropped_collisions( full stem is the id's shorter prefix (``protocol`` while ``protocol.md`` exists): that file is the one the bare id names, so the id may be a genuine reference to it and stays on the definer-wins collapse-and-warn path; + - a node read as a MENTION of a document (``_mentions``: a path-slice id that + another node of its own file ``references``) is not a candidate, so it is not + renamed onto the mentioning file's own page. That reading is inferred from LLM + edges: with a missing or reversed edge the node is renamed like any other, and + a collision with a real document whose own node is linked to its file-level id + collapses as it did before. If two or more other files also mint the mention's + id it cannot be pointed at any one of them, so it stays on the bare id and + ``build_from_json`` then folds it into its own file's page; + - a top-level file that mints the same id (``protocol.md`` next to + ``docs/protocol.md``) is a separate collision this pass does not resolve: the + group is skipped and the label tiebreak drops one of the real documents; - a reference to the bare id from ANOTHER file cannot be attributed to one file, so it is left unresolved (one warning per id) rather than guessed; - if the new id is already used by a node of a different file, or two files @@ -695,18 +760,24 @@ def _path_qualify_dropped_collisions( _full_stem(key(n["source_file"])) for n in nodes if isinstance(n.get("source_file"), str) and n["source_file"] } + mentions = _mentions(nodes, edges, root) by_id: dict[str, list[dict]] = defaultdict(list) owners: dict[str, set[str]] = defaultdict(set) + kept_bare: set[tuple[str, str]] = set() for node in nodes: nid = node.get("id") if not nid or not isinstance(nid, str): continue - owners[nid].add(key(node.get("source_file"))) - if qualified_id(node): + k = key(node.get("source_file")) + owners[nid].add(k) + if not qualified_id(node): + continue + if (nid, k) in mentions: + kept_bare.add((nid, k)) # stays on the bare id; its own-file edges are fine + else: by_id[nid].append(node) renames: dict[tuple[str, str], str] = {} - kept_bare: set[tuple[str, str]] = set() claimed: dict[str, str] = {} for nid in sorted(by_id): members = by_id[nid] @@ -867,6 +938,7 @@ def deduplicate_entities( # Genuine cross-file ID collisions stay isolated and are reported below (#1504). nodes, edges = _disambiguate_defining_collisions(nodes, edges, hyperedges, root_resolved) nodes, edges = _path_qualify_dropped_collisions(nodes, edges, hyperedges, root_resolved) + mentions = _mentions(nodes, edges, root_resolved) seen_ids: dict[str, dict] = {} dropped: dict[str, list[dict]] = defaultdict(list) for node in nodes: @@ -876,7 +948,8 @@ def deduplicate_entities( incumbent = seen_ids.get(nid) if incumbent is None: seen_ids[nid] = node - elif _collision_rank(node, root_resolved) < _collision_rank(incumbent, root_resolved): + elif (_collision_rank(node, root_resolved, mentions) + < _collision_rank(incumbent, root_resolved, mentions)): # 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). @@ -894,7 +967,7 @@ def deduplicate_entities( survivor = seen_ids[nid] same_source = sorted( (l for l in losers if _same_source_entity(survivor, l)), - key=lambda l: _collision_rank(l, root_resolved), + key=lambda l: _collision_rank(l, root_resolved, mentions), ) for loser in same_source: survivor = _merge_missing_attributes(survivor, loser) diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 7248adb2c..67d36498f 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -1587,7 +1587,12 @@ def test_extension_rename_is_order_independent(): def test_bare_id_across_different_paths_is_path_qualified(capsys): """A bare `protocol` shared by docs/protocol.md and docs/firmware/protocol.md, with no protocol.md anywhere: neither file is named by the bare id, so both are - definers and each gets its own path-qualified id (#4281 remaining case).""" + definers and each gets its own path-qualified id (#4281 remaining case). + + No own-file `references` edge points at either copy here, so there is no + evidence that one of them is a mention of the other file, and both are renamed. + With that edge the same ids stay one node (a mention is not a second document): + see test_reference_to_another_file_does_not_become_a_second_document.""" nodes = [ {"id": "protocol", "label": "protocol", "file_type": "document", "source_file": "docs/protocol.md"}, @@ -1888,3 +1893,120 @@ def test_path_dropped_foreign_reference_edge_is_dropped_by_build(capsys): assert not G.has_edge("c_note", "b_x") assert ("node 'x' was minted by several files and renamed per file; " "1 reference(s)") in capsys.readouterr().err + + +# ── #4281: a same-file reference is a mention, not a second document ────────── +# Repro cases (notes 03-repro): D -> test_path_dropped_through_build, +# F -> test_path_dropped_foreign_reference_edge_is_dropped_by_build, +# R, R_labelswap, R_noedge, R_toplevel, G -> the tests below. + +def _mention_node(nid, label, source_file): + return {"id": nid, "label": label, "file_type": "document", "source_file": source_file} + + +def _mention_edge(source, target, source_file): + return {"source": source, "target": target, "relation": "references", + "confidence": "EXTRACTED", "source_file": source_file} + + +def _protocol_reference_input(*, edge=True, docs_label="Protocol", + mention_label="protocol (top-level)", top_level=False): + """docs/firmware/protocol.md mints its own `docs_firmware_protocol` plus a bare + `protocol` that stands for another file; docs/protocol.md mints `protocol`.""" + fw = "docs/firmware/protocol.md" + nodes = [_mention_node("protocol", docs_label, "docs/protocol.md"), + _mention_node("docs_firmware_protocol", "Firmware protocol", fw), + _mention_node("protocol", mention_label, fw)] + if top_level: + nodes.append(_mention_node("protocol", "Protocol (top-level)", "protocol.md")) + edges = [_mention_edge("docs_firmware_protocol", "protocol", fw)] if edge else [] + return {"nodes": nodes, "edges": edges} + + +def _assert_reference_lands_on_the_other_file(G): + assert sorted(G.nodes) == ["docs_firmware_protocol", "docs_protocol"] + # one edge, from the firmware doc to docs/protocol.md; a self-loop would collapse + # to a one-element frozenset and fail this comparison. + assert {frozenset(e) for e in G.edges} == { + frozenset(("docs_firmware_protocol", "docs_protocol"))} + + +def test_reference_to_another_file_does_not_become_a_second_document(): + """Case R. A bare id that another node of its own file `references`, and that is + itself a path slice (`protocol`), is a mention of a document: it is not renamed + into the mentioning file's own page, so the reference ends on docs/protocol.md. + Renaming it made `docs_firmware_protocol` reference itself (#4281).""" + from graphify.build import build + + _assert_reference_lands_on_the_other_file(build([_protocol_reference_input()])) + + +def test_reference_outcome_does_not_depend_on_label_length(): + """Case R_labelswap. Same graph as above with the labels swapped, so the mention + has the shorter one. v8 kept the shorter label and dropped docs/protocol.md here.""" + from graphify.build import build + + G = build([_protocol_reference_input(docs_label="Protocol overview", mention_label="protocol")]) + _assert_reference_lands_on_the_other_file(G) + + +def test_mention_without_an_edge_gives_the_same_two_documents(): + """Case R_noedge. With no `references` edge there is no evidence the bare id is a + mention, so both files are renamed per file; the result is still the two real + documents and no edge. The evidence only ever withholds a rename.""" + from graphify.build import build + + G = build([_protocol_reference_input(edge=False)]) + assert sorted(G.nodes) == ["docs_firmware_protocol", "docs_protocol"] + assert list(G.edges) == [] + + +def test_top_level_collision_keeps_todays_result(): + """Case R_toplevel. KNOWN LIMITATION, pinned so a change is noticed: with a + top-level protocol.md that also mints `protocol`, the three files collide and the + label tiebreak drops the top-level document's node. This is a separate genuine + collision from the mention rule and is not fixed here (#4281).""" + from graphify.build import build + + G = build([_protocol_reference_input(top_level=True)]) + assert sorted(G.nodes) == ["docs_firmware_protocol", "docs_protocol"] + assert {frozenset(e) for e in G.edges} == { + frozenset(("docs_firmware_protocol", "docs_protocol"))} + + +def test_entity_described_in_two_documents_collapses_to_one_node(capsys): + """Case G, the shape of the reporter's warnings. `sender` is derived from neither + path and each document points at it, so the two copies are one entity: one node + with both edges, and the merge warning still prints (#4281).""" + from graphify.build import build + + ext = { + "nodes": [_mention_node("docs_a_doc", "Doc A", "docs/a.md"), + _mention_node("sender", "Sender service", "docs/a.md"), + _mention_node("docs_b_doc", "Doc B", "docs/b.md"), + _mention_node("sender", "Sender", "docs/b.md")], + "edges": [_mention_edge("docs_a_doc", "sender", "docs/a.md"), + _mention_edge("docs_b_doc", "sender", "docs/b.md")], + } + G = build([ext]) + assert sorted(G.nodes) == ["docs_a_doc", "docs_b_doc", "sender"] + assert {frozenset(e) for e in G.edges} == { + frozenset(("docs_a_doc", "sender")), frozenset(("docs_b_doc", "sender"))} + assert "node 'sender' is minted by two different files" in capsys.readouterr().err + + +def test_reference_evidence_does_not_change_survivors_of_other_collisions(): + """The mention evidence is gated to ids that are themselves a path slice. `x_intro` + is derived from a/x.md (so a/x.md defines it and wins the collision) but is not a + slice. Both copies are the target of an own-file `references` edge. Applied to every + id, that evidence would demote a/x.md's copy and let the shorter label from c.md + win. The survivor must be what v8 picks: a/x.md's node (#4281).""" + nodes = [_mention_node("x", "X", "a/x.md"), + _mention_node("x_intro", "Introduction to X", "a/x.md"), + _mention_node("c_doc", "Doc C", "c.md"), + _mention_node("x_intro", "Intro", "c.md")] + edges = [_mention_edge("x", "x_intro", "a/x.md"), + _mention_edge("c_doc", "x_intro", "c.md")] + kept, _ = deduplicate_entities(nodes, edges, communities={}) + survivor = next(n for n in kept if n["id"] == "x_intro") + assert (survivor["label"], survivor["source_file"]) == ("Introduction to X", "a/x.md")