mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-21 03:11:42 +00:00
fix(dedup): keep a dropped-folder doc next to a top-level same-id file (#4281)
With protocol.md next to docs/protocol.md and docs/firmware/protocol.md, all
three minted `protocol` and the label tiebreak dropped a real document.
A file whose full stem is the id now defines it and keeps the bare id; the
other files are path-qualified when they have no other node spelled with
their own full stem. If they do, the bare id may be a reference, so the file
keeps today's collapse (known boundary). A genuine reference to the top-level
file still collapses onto it.
A bare path slice from a file that has its own page ranks as a reference, so the
real file survives whatever the labels are.
(cherry picked from commit a3fa3c47c4)
This commit is contained in:
1 parent
e8eb76e797
commit
8d5b139da4
2 files changed
+172
-27
No files matched your search
+67
-13
@@ -464,7 +464,10 @@ def _collision_rank(node: dict, root: Path | None = None,
|
||||
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.
|
||||
ids, so the survivor of every other collision is chosen exactly as before. It
|
||||
also holds a shorter path-slice id from a file that has its own page
|
||||
(``_slice_references``), so a bare ``protocol`` inside docs/protocol.md cannot
|
||||
outrank the real top-level protocol.md on label length.
|
||||
"""
|
||||
label = node.get("label") or ""
|
||||
rank_path = _rank_path(node.get("source_file") or "", root)
|
||||
@@ -524,6 +527,43 @@ def _mentions(nodes: list[dict], edges: list[dict],
|
||||
return found
|
||||
|
||||
|
||||
def _own_page_files(nodes: list[dict], root: Path | None) -> set[str]:
|
||||
"""Root-relative files that have a node spelled with their own full stem (#4281).
|
||||
|
||||
Such a file already has its page, so a shorter path-slice id it also mints
|
||||
(``protocol`` inside docs/protocol.md, next to ``docs_protocol``) may be a
|
||||
reference to another file rather than that file's own page."""
|
||||
from graphify.build import _is_abs # local: build imports dedup
|
||||
|
||||
files: set[str] = set()
|
||||
for node in nodes:
|
||||
nid, sf = node.get("id"), node.get("source_file")
|
||||
if not nid or not isinstance(nid, str) or not sf or not isinstance(sf, str):
|
||||
continue
|
||||
k = _rank_path(sf, root)
|
||||
if _is_abs(k):
|
||||
continue
|
||||
full = _full_stem(k)
|
||||
if nid == full or nid.startswith(f"{full}_"):
|
||||
files.add(k)
|
||||
return files
|
||||
|
||||
|
||||
def _slice_references(nodes: list[dict], root: Path | None) -> set[tuple[str, str]]:
|
||||
"""``(id, file)`` of nodes whose id is a SHORTER path slice of a file that has its
|
||||
own page (#4281). They rank as references, like a ``_mentions`` entry."""
|
||||
own_page = _own_page_files(nodes, root)
|
||||
found: set[tuple[str, str]] = set()
|
||||
for node in nodes:
|
||||
nid, sf = node.get("id"), node.get("source_file")
|
||||
if not nid or not isinstance(nid, str) or not sf or not isinstance(sf, str):
|
||||
continue
|
||||
k = _rank_path(sf, root)
|
||||
if k in own_page and nid in _id_prefixes(k) and nid != _full_stem(k):
|
||||
found.add((nid, k))
|
||||
return found
|
||||
|
||||
|
||||
def _same_source_entity(survivor: dict, duplicate: dict) -> bool:
|
||||
"""True when exact-ID records came from the same source file.
|
||||
|
||||
@@ -709,10 +749,6 @@ def _path_qualify_dropped_collisions(
|
||||
location cannot leak into an id;
|
||||
- a group acts only when its files have at least two distinct full stems; a
|
||||
lone candidate is left to ``build_from_json``'s semantic id remap;
|
||||
- a group is skipped when ANY node in the input comes from the file whose
|
||||
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
|
||||
@@ -721,9 +757,15 @@ def _path_qualify_dropped_collisions(
|
||||
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 file whose full stem IS the id (``protocol.md`` for ``protocol``) defines it,
|
||||
keeps the bare id, and counts toward the collision, so ``docs/protocol.md``
|
||||
next to it is still renamed. A candidate whose bare id names another input
|
||||
file is renamed only if its own file has no other node spelled with its own
|
||||
full stem. If it has one (``docs_protocol`` next to ``protocol``), the bare id
|
||||
may be a reference, so it stays on the definer-wins collapse-and-warn path as
|
||||
before. Known boundary: when the LLM drops the folder AND also emits another
|
||||
node spelled with the file's full stem, the file counts as having its own page
|
||||
and falls back to the collapse it got before this change (#4281);
|
||||
- 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
|
||||
@@ -777,20 +819,32 @@ def _path_qualify_dropped_collisions(
|
||||
else:
|
||||
by_id[nid].append(node)
|
||||
|
||||
# Files with a node spelled with their own full stem: their page already exists,
|
||||
# so a shorter-prefix id from such a file may be a reference to another file.
|
||||
own_page = _own_page_files(nodes, root)
|
||||
|
||||
renames: dict[tuple[str, str], str] = {}
|
||||
claimed: dict[str, str] = {}
|
||||
for nid in sorted(by_id):
|
||||
members = by_id[nid]
|
||||
if len({key(m["source_file"]) for m in members}) < 2:
|
||||
# The file whose full stem IS the id defines it; it keeps the bare id and
|
||||
# counts toward the collision even though it is not a candidate.
|
||||
definers = {o for o in owners[nid] if o and not _is_abs(o) and _full_stem(o) == nid}
|
||||
files = {key(m["source_file"]) for m in members} | definers
|
||||
if len(files) < 2:
|
||||
continue
|
||||
if len({_full_stem(key(m["source_file"])) for m in members}) < 2:
|
||||
continue
|
||||
if any((qualified_id(m) or ("", ""))[1] in named_files for m in members):
|
||||
if len({_full_stem(f) for f in files}) < 2:
|
||||
continue
|
||||
for m in sorted(members, key=lambda m: key(m["source_file"])):
|
||||
k = key(m["source_file"])
|
||||
if (nid, k) in renames or (nid, k) in kept_bare:
|
||||
continue
|
||||
# The bare id names another input file (``protocol`` while protocol.md
|
||||
# exists). It is this file's own page only if the file has no other
|
||||
# page node; otherwise it may be a reference, so leave it to collapse.
|
||||
if (qualified_id(m) or ("", ""))[1] in named_files and k in own_page:
|
||||
kept_bare.add((nid, k))
|
||||
continue
|
||||
qualified = qualified_id(m)
|
||||
new_id = qualified[0] if qualified else None
|
||||
if (new_id is None
|
||||
@@ -938,7 +992,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)
|
||||
mentions = _mentions(nodes, edges, root_resolved) | _slice_references(nodes, root_resolved)
|
||||
seen_ids: dict[str, dict] = {}
|
||||
dropped: dict[str, list[dict]] = defaultdict(list)
|
||||
for node in nodes:
|
||||
|
||||
+105
-14
@@ -1604,10 +1604,8 @@ def test_bare_id_across_different_paths_is_path_qualified(capsys):
|
||||
assert "minted by two different files" not in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_bare_id_naming_an_existing_file_is_not_renamed(capsys):
|
||||
"""`protocol.md` exists, so a bare `protocol` may be a genuine reference to it:
|
||||
the group is not path-qualified and takes the definer-wins collapse path."""
|
||||
nodes = [
|
||||
def _three_bare_protocols():
|
||||
return [
|
||||
{"id": "protocol", "label": "protocol", "file_type": "document",
|
||||
"source_file": "protocol.md"},
|
||||
{"id": "protocol", "label": "protocol (docs)", "file_type": "document",
|
||||
@@ -1615,9 +1613,19 @@ def test_bare_id_naming_an_existing_file_is_not_renamed(capsys):
|
||||
{"id": "protocol", "label": "protocol (firmware)", "file_type": "document",
|
||||
"source_file": "docs/firmware/protocol.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert [(n["id"], n["source_file"]) for n in result_nodes] == [("protocol", "protocol.md")]
|
||||
assert "WARNING" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_bare_ids_in_three_files_keep_one_node_per_file(capsys):
|
||||
"""protocol.md exists and the other two files each have only a bare `protocol`.
|
||||
Neither has a page of its own, so the bare id is their page: protocol.md keeps
|
||||
`protocol` and the others are path-qualified. Nothing is dropped (#4281)."""
|
||||
result_nodes, _ = deduplicate_entities(_three_bare_protocols(), [], communities={})
|
||||
assert {(n["id"], n["source_file"]) for n in result_nodes} == {
|
||||
("protocol", "protocol.md"),
|
||||
("docs_protocol", "docs/protocol.md"),
|
||||
("docs_firmware_protocol", "docs/firmware/protocol.md"),
|
||||
}
|
||||
assert "minted by two different files" not in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_build_with_absolute_edge_paths_moves_edges_to_renamed_node(tmp_path):
|
||||
@@ -1961,17 +1969,100 @@ def test_mention_without_an_edge_gives_the_same_two_documents():
|
||||
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)."""
|
||||
def test_top_level_file_keeps_its_own_node_next_to_dropped_folder_docs():
|
||||
"""Case R_toplevel. protocol.md, docs/protocol.md and docs/firmware/protocol.md
|
||||
all mint `protocol`. Each file keeps its own node, and the firmware reference
|
||||
lands on the top-level `protocol` node (#4281)."""
|
||||
from graphify.build import build
|
||||
|
||||
G = build([_protocol_reference_input(top_level=True)])
|
||||
assert sorted(G.nodes) == ["docs_firmware_protocol", "docs_protocol"]
|
||||
assert sorted(G.nodes) == ["docs_firmware_protocol", "docs_protocol", "protocol"]
|
||||
assert {frozenset(e) for e in G.edges} == {
|
||||
frozenset(("docs_firmware_protocol", "docs_protocol"))}
|
||||
frozenset(("docs_firmware_protocol", "protocol"))}
|
||||
|
||||
|
||||
def _reference_to_top_level(*, edge=True, docs_label="protocol", top_label="Protocol"):
|
||||
docs = "docs/protocol.md"
|
||||
nodes = [_mention_node("docs_protocol", "Docs protocol", docs),
|
||||
_mention_node("protocol", docs_label, docs),
|
||||
_mention_node("protocol", top_label, "protocol.md")]
|
||||
edges = [_mention_edge("docs_protocol", "protocol", docs)] if edge else []
|
||||
return nodes, edges
|
||||
|
||||
|
||||
def test_genuine_reference_to_top_level_file_still_collapses():
|
||||
"""docs/protocol.md has its own `docs_protocol` page and a `protocol` it
|
||||
references. That bare id is the top-level file, not a second page of docs/:
|
||||
it collapses onto protocol.md's node and the edge ends there (#4281)."""
|
||||
nodes, edges = _reference_to_top_level()
|
||||
kept, kept_edges = deduplicate_entities(nodes, edges, communities={})
|
||||
assert {(n["id"], n["source_file"]) for n in kept} == {
|
||||
("docs_protocol", "docs/protocol.md"), ("protocol", "protocol.md")}
|
||||
assert [(e["source"], e["target"]) for e in kept_edges] == [("docs_protocol", "protocol")]
|
||||
|
||||
|
||||
def test_reference_to_top_level_file_without_an_edge_still_collapses():
|
||||
"""Same input with no `references` edge: the docs `protocol` still collapses
|
||||
into protocol.md's node, because docs/protocol.md has its own page (#4281)."""
|
||||
nodes, edges = _reference_to_top_level(edge=False)
|
||||
kept, kept_edges = deduplicate_entities(nodes, edges, communities={})
|
||||
assert {(n["id"], n["source_file"]) for n in kept} == {
|
||||
("docs_protocol", "docs/protocol.md"), ("protocol", "protocol.md")}
|
||||
assert kept_edges == []
|
||||
|
||||
|
||||
def test_top_level_file_survives_whatever_the_labels_are():
|
||||
"""Case R_toplevel, label-swap. The bare `protocol` from docs/protocol.md has the
|
||||
SHORTER label. docs/protocol.md has its own page, so that node is a reference and
|
||||
protocol.md's node survives; the docs_protocol edge ends on it (#4281)."""
|
||||
nodes, edges = _reference_to_top_level(docs_label="Protocol", top_label="Protocol (top-level)")
|
||||
kept, kept_edges = deduplicate_entities(nodes, edges, communities={})
|
||||
assert {(n["id"], n["label"], n["source_file"]) for n in kept} == {
|
||||
("docs_protocol", "Docs protocol", "docs/protocol.md"),
|
||||
("protocol", "Protocol (top-level)", "protocol.md")}
|
||||
assert [(e["source"], e["target"]) for e in kept_edges] == [("docs_protocol", "protocol")]
|
||||
|
||||
|
||||
def test_top_level_file_survives_without_a_references_edge():
|
||||
"""Same label-swap input with no `references` edge, so `_mentions` has no
|
||||
evidence. The ranking alone must keep protocol.md's node: a bare slice from a
|
||||
file that has its own page is a reference, whatever the labels (#4281)."""
|
||||
nodes, _ = _reference_to_top_level(edge=False, docs_label="Protocol",
|
||||
top_label="Protocol (top-level)")
|
||||
for order in (nodes, list(reversed(nodes))):
|
||||
kept, _ = deduplicate_entities([dict(n) for n in order], [], communities={})
|
||||
survivor = next(n for n in kept if n["id"] == "protocol")
|
||||
assert (survivor["label"], survivor["source_file"]) == ("Protocol (top-level)", "protocol.md")
|
||||
|
||||
|
||||
def test_top_level_collision_result_does_not_depend_on_input_order():
|
||||
"""The three-file case fed in reverse order keeps the same nodes (#4281)."""
|
||||
nodes = _three_bare_protocols()
|
||||
fwd, _ = deduplicate_entities([dict(n) for n in nodes], [], communities={})
|
||||
rev, _ = deduplicate_entities([dict(n) for n in reversed(nodes)], [], communities={})
|
||||
assert {(n["id"], n["source_file"]) for n in fwd} == {
|
||||
(n["id"], n["source_file"]) for n in rev}
|
||||
|
||||
|
||||
def test_dropped_folder_doc_with_own_stem_entity_falls_back_to_collapse():
|
||||
"""KNOWN BOUNDARY, pinned so a change is noticed. docs/protocol.md emits `protocol`
|
||||
AND `docs_protocol_header` (an entity spelled with its full stem), so the file
|
||||
counts as having its own page and the bare id may be a reference. It collapses
|
||||
into protocol.md's node, as before this change (#4281)."""
|
||||
nodes = [_mention_node("protocol", "protocol", "docs/protocol.md"),
|
||||
_mention_node("docs_protocol_header", "Header", "docs/protocol.md"),
|
||||
_mention_node("protocol", "Protocol", "protocol.md")]
|
||||
kept, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert {(n["id"], n["source_file"]) for n in kept} == {
|
||||
("docs_protocol_header", "docs/protocol.md"), ("protocol", "protocol.md")}
|
||||
|
||||
|
||||
def test_lone_bare_id_without_a_top_level_file_is_left_to_build():
|
||||
"""A single `protocol` from docs/protocol.md and no protocol.md anywhere: this
|
||||
pass does not act, the id stays `protocol` for build's remap (#4281)."""
|
||||
nodes = [_mention_node("protocol", "Protocol", "docs/protocol.md")]
|
||||
kept, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert [n["id"] for n in kept] == ["protocol"]
|
||||
|
||||
|
||||
def test_entity_described_in_two_documents_collapses_to_one_node(capsys):
|
||||
|
||||
Reference in new issue
Block a user