mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 07:59:40 +00:00
fix(dedup): merge cross-file entity nodes typed by their file's extension (#296)
Per-file extraction over a note vault mints one node per mention of the same person or project, and they never merge. Normalization is not the problem: `_norm` already casefolds and collapses `[\W_]+`, so `@cyrilXBT`, `@cyrilxbt` and `cyrilXBT` land in the same Pass 1 bucket. The bucket is then dropped, because the cross-file union added in #2182 is gated to `file_type == "concept"` — and an entity the extractor pulls out of a `.md` note inherits `document` from the file's extension, not from anything about the entity. #1284's reasoning for that gate is right about files: two `README.md` in different folders are two documents. It does not follow for a node that merely lives in a document. This widens the Pass 1 cross-file residue to `document`/`rationale` nodes that are provably not part of their file's own structure, via a new `_provably_not_file_structure`: * not the file's own node — reusing `_id_prefixes`, which already enumerates the ID a node standing for `source_file` would carry in every spelling a stored path may take; * not one of the file's sections — `node_kind: "heading"`, and `"page"` for the file node the markdown extractor labels outright. A node that cannot be checked (no ID, no provenance) answers False and stays blocked, so the predicate can only narrow what the gate treats as file-anchored, never widen it on a guess. Deliberately unchanged: the entropy and provenance guards, `code` (#1205), `image`/`paper` shared basenames, and Pass 2's fuzzy `_crossfile_fileanchored_blocked` — so #1284's near-identical boilerplate and #3094's repeated `## Decisions` sections stay per-file, both now covered by tests. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2 files changed
+238
-15
No files matched your search
+57
-6
@@ -301,6 +301,44 @@ def _defines_id(node: dict) -> bool:
|
||||
for prefix in _id_prefixes(source_file))
|
||||
|
||||
|
||||
# `node_kind` values marking a node that is STRUCTURE of its source file rather
|
||||
# than an entity mentioned inside it: `page` is the file's own node, `heading`
|
||||
# one of its sections. The markdown extractor stamps these precisely because
|
||||
# `file_type` cannot carry the distinction, and both are file-anchored in the
|
||||
# #1284 sense — two files' `## Decisions` sections are two sections (#3094).
|
||||
_FILE_STRUCTURE_NODE_KINDS = frozenset({"page", "heading"})
|
||||
|
||||
|
||||
def _provably_not_file_structure(node: dict) -> bool:
|
||||
"""True only when the node is PROVABLY an entity found inside its file,
|
||||
rather than a structural part of the file itself (#296).
|
||||
|
||||
Two things disqualify a node, and either alone is enough:
|
||||
|
||||
* It is its file's OWN node. ``_id_prefixes`` enumerates the ID a node
|
||||
standing for ``source_file`` itself would carry, in every spelling a
|
||||
stored path may take (absolute, repo-relative, or the pre-#1504 bare
|
||||
stem). A file's own node IS one of those; an entity extracted from that
|
||||
file carries an ``_<entity>`` suffix, so it never equals one.
|
||||
* It is a section of the file — ``node_kind: "heading"`` — or the file node
|
||||
the extractor labelled outright, ``node_kind: "page"``. Repeated headings
|
||||
across sibling documents are distinct sections, not duplicates
|
||||
(#1284, re-verified on #3094), and stay blocked.
|
||||
|
||||
Fail-safe by construction: a node that cannot be checked (no ID, no
|
||||
provenance) answers False and stays blocked, so this can only ever *narrow*
|
||||
the set of nodes the cross-file gate treats as file-anchored — never widen
|
||||
it on a guess.
|
||||
"""
|
||||
nid = node.get("id") or ""
|
||||
source_file = node.get("source_file") or ""
|
||||
if not nid or not source_file:
|
||||
return False # unprovable — leave the file-anchored block in place
|
||||
if node.get("node_kind") in _FILE_STRUCTURE_NODE_KINDS:
|
||||
return False # the extractor says this node is part of the file's structure
|
||||
return nid not in _id_prefixes(source_file)
|
||||
|
||||
|
||||
# Path-segment lifecycle markers used by _collision_rank (#2532). Lower penalty
|
||||
# wins. Without them, pure lexical source_file order makes ``plans/_done/…``
|
||||
# beat ``plans/in-progress/…`` because "_" < "i" in ASCII. Active-vs-archived
|
||||
@@ -630,14 +668,27 @@ def deduplicate_entities(
|
||||
exact_merges += len(file_group) - 1
|
||||
# Cross-file residue: union exact matches across files, but only where
|
||||
# it is provably safe (#2182). `concept` is the one file_type meant to
|
||||
# unify across files (#1284) — code is keyed by ID (#1205), rationale/
|
||||
# document are file-anchored (#1284), and image/paper labels are often
|
||||
# shared basenames (logo.png). Provenance is required (#1178), and the
|
||||
# entropy gate mirrors Pass 2 so short generic labels ("API") stay
|
||||
# distinct. Sorting by id keeps the winner order-independent.
|
||||
# unify across files (#1284) — code is keyed by ID (#1205) and
|
||||
# image/paper labels are often shared basenames (logo.png), so both stay
|
||||
# blocked. rationale/document join `concept` here ONLY when the node is
|
||||
# provably not part of its file's own structure (#296): a file-anchored
|
||||
# *file_type* does not make an individual node file-anchored. An entity
|
||||
# extracted from a note — a person, a project — inherits `document` from
|
||||
# the file's extension, not from anything about itself, so in note-heavy
|
||||
# corpora almost no entity node is typed `concept` and this merge never
|
||||
# got to run on them. A file's own node and its headings still never
|
||||
# merge (#1284, #3094).
|
||||
# Provenance is required (#1178), and the entropy gate mirrors Pass 2 so
|
||||
# short generic labels ("API") stay distinct — both untouched here.
|
||||
# Scoped to this exact-normalization pass: Pass 2's fuzzy
|
||||
# `_crossfile_fileanchored_blocked` is unchanged, so #1284's
|
||||
# near-identical boilerplate and heading siblings stay blocked.
|
||||
# Sorting by id keeps the winner order-independent.
|
||||
mergeable = sorted(
|
||||
(n for n in group
|
||||
if n.get("file_type") == "concept"
|
||||
if (n.get("file_type") == "concept"
|
||||
or (n.get("file_type") in _FILE_ANCHORED_NONCODE
|
||||
and _provably_not_file_structure(n)))
|
||||
and (n.get("source_file") or "")
|
||||
and _entropy(n.get("label", "")) >= _ENTROPY_THRESHOLD),
|
||||
key=lambda n: n["id"],
|
||||
|
||||
+181
-9
@@ -851,13 +851,18 @@ _RATIONALE_BOILER = ("Django app config for apps.platform.cards. No business "
|
||||
|
||||
|
||||
@pytest.mark.parametrize("a,b", [
|
||||
({"id": "d1", "label": "Getting Started Installation Guide",
|
||||
# #296 narrowed the document/rationale rows here to their boundary case: an
|
||||
# identical label on two nodes that are provably NOT their files' own nodes
|
||||
# now merges (that is the bug the concept-only gate was causing). What must
|
||||
# still never merge is a file's OWN node, so these two rows now carry the
|
||||
# ids the file nodes themselves would be minted with.
|
||||
({"id": "docs_a", "label": "Getting Started Installation Guide",
|
||||
"file_type": "document", "source_file": "docs/a.md"},
|
||||
{"id": "d2", "label": "Getting Started Installation Guide",
|
||||
{"id": "docs_b", "label": "Getting Started Installation Guide",
|
||||
"file_type": "document", "source_file": "docs/b.md"}),
|
||||
({"id": "r1", "label": _RATIONALE_BOILER,
|
||||
({"id": "apps_platform_cards_apps", "label": _RATIONALE_BOILER,
|
||||
"file_type": "rationale", "source_file": "apps/platform/cards/apps.py"},
|
||||
{"id": "r2", "label": _RATIONALE_BOILER,
|
||||
{"id": "apps_platform_cores_apps", "label": _RATIONALE_BOILER,
|
||||
"file_type": "rationale", "source_file": "apps/platform/cores/apps.py"}),
|
||||
({"id": "backend_a_render_frame", "label": "render_frame",
|
||||
"file_type": "code", "source_file": "backend_a.py"},
|
||||
@@ -879,12 +884,15 @@ _RATIONALE_BOILER = ("Django app config for apps.platform.cards. No business "
|
||||
"file_type": "concept", "source_file": "doc1.md"},
|
||||
{"id": "api_b", "label": "API",
|
||||
"file_type": "concept", "source_file": "doc2.md"}),
|
||||
], ids=["document", "rationale", "code", "image-basename", "concept-image-mixed",
|
||||
"empty-source-file", "low-entropy-concept"])
|
||||
], ids=["document-own-file-node", "rationale-own-file-node", "code",
|
||||
"image-basename", "concept-image-mixed", "empty-source-file",
|
||||
"low-entropy-concept"])
|
||||
def test_crossfile_identical_labels_stay_distinct_for_guarded_types(a, b):
|
||||
"""The #2182 fix is gated to high-entropy `concept` nodes with provenance
|
||||
on BOTH sides. Identical labels must NOT merge for: file-anchored types
|
||||
(document/rationale, #1284), code (#1205), images sharing a basename in
|
||||
"""The #2182 cross-file merge requires provenance and high entropy on BOTH
|
||||
sides, and #296 widened its type gate only as far as nodes provably not
|
||||
their file's own node. Identical labels must still NOT merge for: a
|
||||
document/rationale node that IS its file's own node (#296, the boundary of
|
||||
#1284's file-anchored guard), code (#1205), images sharing a basename in
|
||||
different dirs, mixed concept+image pairs, provenance-less nodes (#1178),
|
||||
and low-entropy generic labels."""
|
||||
result_nodes, _ = deduplicate_entities([dict(a), dict(b)], [], communities={})
|
||||
@@ -1196,3 +1204,167 @@ def test_same_word_variant_helper():
|
||||
# Accepted trade: a length-differing 5-char spelling variant reads as two
|
||||
# words and stays unmerged, per the never-merge-distinct-entities bar.
|
||||
assert not _same_word_variant("colour", "color")
|
||||
|
||||
|
||||
# -- #296: cross-file merge for entity nodes typed by their file's extension --
|
||||
#
|
||||
# Reported shape: per-file extraction over a note vault mints one node per
|
||||
# mention of the same person/project, and they never merge. Normalization was
|
||||
# never the problem -- `_norm` already buckets the variants together -- the
|
||||
# Pass 1 cross-file union was gated to `file_type == "concept"`, and an entity
|
||||
# pulled out of a `.md` note inherits `document` from the file's extension, not
|
||||
# from anything about the entity. The @cyrilXBT variant family below is the
|
||||
# reported corpus's shape, reduced to the spellings that occur in it.
|
||||
|
||||
_CYRIL_VARIANTS = ["@cyrilXBT", "@cyrilxbt", "cyrilXBT"]
|
||||
|
||||
|
||||
def test_provably_not_file_structure_helper():
|
||||
"""The predicate proves entity-ness, and answers False whenever it cannot
|
||||
(#296): a file's own node, a structural `page`/`heading` node, and any node
|
||||
missing an id or provenance all stay treated as file-anchored."""
|
||||
from graphify.dedup import _provably_not_file_structure
|
||||
# An entity extracted from a note: `<path>_<entity>` never equals the path.
|
||||
assert _provably_not_file_structure(
|
||||
{"id": "journal_2024_03_01_cyrilxbt", "label": "@cyrilXBT",
|
||||
"source_file": "journal/2024-03-01.md"})
|
||||
# The file's own node, in each spelling a stored source_file may take.
|
||||
assert not _provably_not_file_structure(
|
||||
{"id": "journal_2024_03_01", "source_file": "journal/2024-03-01.md"})
|
||||
assert not _provably_not_file_structure(
|
||||
{"id": "2024_03_01", "source_file": "journal/2024-03-01.md"}) # pre-#1504 bare stem
|
||||
assert not _provably_not_file_structure(
|
||||
{"id": "vault_journal_2024_03_01", "source_file": 'C:\\vault\\journal\\2024-03-01.md'})
|
||||
# The markdown extractor states it outright, for the file and its sections.
|
||||
assert not _provably_not_file_structure(
|
||||
{"id": "anything", "node_kind": "page", "source_file": "docs/a.md"})
|
||||
assert not _provably_not_file_structure(
|
||||
{"id": "docs_a_decisions", "node_kind": "heading",
|
||||
"label": "Decisions", "source_file": "docs/a.md"})
|
||||
# Unprovable -- no id, or no provenance.
|
||||
assert not _provably_not_file_structure({"id": "", "source_file": "docs/a.md"})
|
||||
assert not _provably_not_file_structure({"id": "docs_a_thing", "source_file": ""})
|
||||
|
||||
|
||||
def test_dedup_merges_crossfile_document_entity_variants():
|
||||
"""The reported bug (#296): case/prefix variants of one entity, extracted
|
||||
from three different notes and typed `document` by extension, must collapse
|
||||
to a single node."""
|
||||
nodes = [
|
||||
{"id": "journal_2024_03_0%d_cyrilxbt" % i, "label": variant,
|
||||
"file_type": "document", "source_file": "journal/2024-03-0%d.md" % i}
|
||||
for i, variant in enumerate(_CYRIL_VARIANTS, start=1)
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 1, (
|
||||
"cross-file `document` entity variants did not merge -- the #296 gate "
|
||||
"widening is not reaching Pass 1's cross-file residue"
|
||||
)
|
||||
|
||||
|
||||
def test_dedup_merges_crossfile_rationale_entity_variants():
|
||||
"""`rationale` rides the same gate as `document` (#296): entity nodes of
|
||||
that type, provably not their files' own nodes, merge on an exact label."""
|
||||
nodes = [
|
||||
{"id": "svc_alpha_py_retention_window", "label": "Retention Window",
|
||||
"file_type": "rationale", "source_file": "svc/alpha.py"},
|
||||
{"id": "svc_beta_py_retention_window", "label": "retention window",
|
||||
"file_type": "rationale", "source_file": "svc/beta.py"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 1
|
||||
|
||||
|
||||
def test_dedup_never_merges_a_files_own_node_away():
|
||||
"""The boundary the widened gate is defined against (#296): a curated note
|
||||
whose OWN node carries the entity's label must survive as its own node, even
|
||||
when a same-label entity node exists in another file."""
|
||||
nodes = [
|
||||
# The curated page: its id is exactly the slugified source path.
|
||||
{"id": "people_cyrilxbt", "label": "@cyrilXBT",
|
||||
"file_type": "document", "source_file": "people/@cyrilXBT.md"},
|
||||
# A mention of the same entity, extracted from a journal note.
|
||||
{"id": "journal_2024_03_01_cyrilxbt", "label": "cyrilXBT",
|
||||
"file_type": "document", "source_file": "journal/2024-03-01.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 2, (
|
||||
"a file's own node merged away -- the #296 widening must never reach it"
|
||||
)
|
||||
assert {n["id"] for n in result_nodes} == {
|
||||
"people_cyrilxbt", "journal_2024_03_01_cyrilxbt"}
|
||||
|
||||
|
||||
def test_dedup_never_merges_two_files_own_nodes():
|
||||
"""Two README.md in different folders are genuinely two documents (#1284's
|
||||
original reasoning), and stay two under #296."""
|
||||
nodes = [
|
||||
{"id": "web_readme", "label": "README.md", "file_type": "document",
|
||||
"node_kind": "page", "source_file": "web/README.md"},
|
||||
{"id": "api_readme", "label": "README.md", "file_type": "document",
|
||||
"node_kind": "page", "source_file": "api/README.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 2
|
||||
|
||||
|
||||
def test_dedup_crossfile_entity_merge_keeps_the_entropy_gate():
|
||||
"""Entropy guard untouched (#296): a short generic label stays distinct for
|
||||
`document` entity nodes exactly as it does for `concept`."""
|
||||
nodes = [
|
||||
{"id": "docs_a_api", "label": "API", "file_type": "document",
|
||||
"source_file": "docs/a.md"},
|
||||
{"id": "docs_b_api", "label": "API", "file_type": "document",
|
||||
"source_file": "docs/b.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 2
|
||||
|
||||
|
||||
def test_dedup_crossfile_entity_merge_keeps_the_provenance_gate():
|
||||
"""Provenance guard untouched (#296, #1178): without a source_file the node
|
||||
cannot be proven to be an entity, so it stays out of the merge."""
|
||||
nodes = [
|
||||
{"id": "orphan_cyrilxbt", "label": "@cyrilXBT",
|
||||
"file_type": "document", "source_file": ""},
|
||||
{"id": "journal_2024_03_01_cyrilxbt", "label": "cyrilXBT",
|
||||
"file_type": "document", "source_file": "journal/2024-03-01.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 2
|
||||
|
||||
|
||||
def test_dedup_crossfile_fuzzy_fileanchored_block_is_untouched():
|
||||
"""#296 widens only the exact-normalization pass. Pass 2's fuzzy
|
||||
`_crossfile_fileanchored_blocked` is unchanged, so near-identical (not
|
||||
identical) document labels in different files still stay distinct -- the
|
||||
#1284 guard keeps doing its job on entity nodes too."""
|
||||
nodes = [
|
||||
{"id": "docs_a_guide", "label": "Getting Started Installation Guide",
|
||||
"file_type": "document", "source_file": "docs/a.md"},
|
||||
{"id": "docs_b_setup", "label": "Getting Started Installation Setup",
|
||||
"file_type": "document", "source_file": "docs/b.md"},
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 2
|
||||
|
||||
|
||||
def test_dedup_never_merges_repeated_headings_across_files():
|
||||
"""#3094's verification case must keep holding under #296: sibling documents
|
||||
that each carry the same `## Decisions` / `## Next steps` sections keep one
|
||||
heading node per file, attributed to that file."""
|
||||
nodes = [
|
||||
{"id": "docs_%s_%s" % (doc, slug), "label": heading,
|
||||
"file_type": "document", "node_kind": "heading",
|
||||
"source_file": "docs/%s.md" % doc}
|
||||
for doc in ("a", "b", "c")
|
||||
for slug, heading in (("decisions", "Decisions"),
|
||||
("next_steps", "Next steps"))
|
||||
]
|
||||
result_nodes, _ = deduplicate_entities(nodes, [], communities={})
|
||||
assert len(result_nodes) == 6, (
|
||||
"repeated headings merged across files -- #296 must not reach section "
|
||||
"nodes (#1284, #3094)"
|
||||
)
|
||||
assert {n["source_file"] for n in result_nodes} == {
|
||||
"docs/a.md", "docs/b.md", "docs/c.md"}
|
||||
Reference in new issue
Block a user