mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 23:49:14 +00:00
fix(dedup): keep a same-file reference as a mention (#4281)
A semantic fragment sometimes mints the bare id of a file it only points at: docs/firmware/protocol.md emits `protocol` for docs/protocol.md. Path-qualifying that id per file renamed it onto the mentioning file's own page, so the reference became a link from the file to itself. A collapse that picked the survivor by label length could instead drop the real document. Rule: a node is read as a mention when another node of its own file points at it with a `references` edge from that same file AND its id is itself a path slice of that file (it names a document). A mention is not a rename candidate, and it ranks as a reference in _collision_rank, so the real document outranks it whatever the label lengths are. One helper, _mentions, decides both places. The path-slice gate matters: entity ids such as `x_intro` are the target of ordinary intra-file `references` edges. Without the gate the rename skips them and two real documents collapse, and the ranking changes the survivor of unrelated collisions. A test pins the survivor. Fixes R (reference to another document) and R_labelswap: no self-link, and no dropped document decided by label length. Two real documents (D), a reference from a third file (F) and one entity described in two documents (G) are unchanged. Limits: - The evidence is inferred from the edges an LLM chose to emit. A missing or reversed edge gives no evidence, so both files are renamed as before. - A real document whose own node is linked to its file-level id the same way reads as a mention and collapses as on v8. - A top-level protocol.md that also mints `protocol` is a separate collision and still drops one document; pinned by a test. - If two or more other files also mint the mention's id, it stays on the bare id and build folds it into its own page.
This commit is contained in:
1 parent
58f844e57f
commit
2d69f5e541
2 files changed
+203
-8
No files matched your search
+80
-7
@@ -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)
|
||||
|
||||
+123
-1
@@ -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")
|
||||
Reference in new issue
Block a user