From d123254ebf964b7740c8b96cb43867bf1f9e06af Mon Sep 17 00:00:00 2001 From: yotamleo <88616986+yotamleo@users.noreply.github.com> Date: Sun, 30 Aug 2026 18:43:12 +0200 Subject: [PATCH] fix(dedup): say what the entity predicate proves and what it assumes (#296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absence of a `node_kind` stamp is not proof that a node is an entity: an extractor can mint a node standing for part of its file without stamping one (detect.py's per-sheet document nodes do exactly that), and the predicate read those as entities while its name and docstring claimed proof. Renames it `_reads_as_file_entity` and splits the docstring into what is PROVEN (not the file's own node — an `_id_prefixes` reconstruction that holds for every stored-path spelling) and what is ASSUMED (not a section, resting on the producer stamping `node_kind`). Adds the producer contract next to `_FILE_STRUCTURE_NODE_KINDS` and a test pinning the stamped/unstamped split, so the limit is enforced rather than described. No behaviour change: the same nodes merge as before. Co-Authored-By: Claude --- graphify/dedup.py | 49 +++++++++++++++++++++++++++------------------ tests/test_dedup.py | 42 ++++++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/graphify/dedup.py b/graphify/dedup.py index 59aac4e1f..f6d193597 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -306,34 +306,43 @@ def _defines_id(node: dict) -> bool: # 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). +# +# PRODUCER CONTRACT: an extractor that mints a node standing for a *part* of its +# source file (a section, a sheet, a slide) must stamp one of these. The gate +# below reads an unstamped node as an entity, so an unstamped structural node is +# eligible to merge with its namesake in another file. _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). +def _reads_as_file_entity(node: dict) -> bool: + """True when the node reads as an entity found inside its file, rather than + as the file itself or a stamped structural part of it (#296). - Two things disqualify a node, and either alone is enough: + Be precise about what is proven and what is assumed, because the two halves + differ in strength: - * 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 + * PROVEN — it is not 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 ``_`` 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. + file carries an ``_`` suffix, so it never equals one. This is a + reconstruction, not a heuristic. + * ASSUMED — it is not a section of the file. That rests on ``node_kind``, + which only a producer that stamps it can attest. `heading`/`page` are + honoured when present, but ABSENCE OF THE MARKER IS NOT PROOF OF + ENTITY-NESS: a producer minting sub-file nodes without stamping + `node_kind` (see the contract above) yields structural nodes that this + returns True for, and two such nodes sharing a label in different files + would merge. The conservative fix is on the producer side — stamp + `node_kind` — not a guess here about what an unstamped node meant. - 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. + A node that cannot be checked at all (no ID, no provenance) answers False + and stays blocked. """ 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 + return False # uncheckable — 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) @@ -670,8 +679,8 @@ def deduplicate_entities( # it is provably safe (#2182). `concept` is the one file_type meant to # 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 + # blocked. rationale/document join `concept` here ONLY when the node + # reads as an entity inside its file (#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 @@ -688,7 +697,7 @@ def deduplicate_entities( (n for n in group if (n.get("file_type") == "concept" or (n.get("file_type") in _FILE_ANCHORED_NONCODE - and _provably_not_file_structure(n))) + and _reads_as_file_entity(n))) and (n.get("source_file") or "") and _entropy(n.get("label", "")) >= _ENTROPY_THRESHOLD), key=lambda n: n["id"], diff --git a/tests/test_dedup.py b/tests/test_dedup.py index ad40a30d8..e0ec4fc81 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -1219,31 +1219,32 @@ def test_same_word_variant_helper(): _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 +def test_reads_as_file_entity_helper(): + """A file's own node, a stamped `page`/`heading` node, and any node missing + an id or provenance all stay treated as file-anchored (#296). The own-node + half is a reconstruction and holds for every stored-path spelling; the + structural half rests on the producer stamping `node_kind`.""" + from graphify.dedup import _reads_as_file_entity # An entity extracted from a note: `_` never equals the path. - assert _provably_not_file_structure( + assert _reads_as_file_entity( {"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( + assert not _reads_as_file_entity( {"id": "journal_2024_03_01", "source_file": "journal/2024-03-01.md"}) - assert not _provably_not_file_structure( + assert not _reads_as_file_entity( {"id": "2024_03_01", "source_file": "journal/2024-03-01.md"}) # pre-#1504 bare stem - assert not _provably_not_file_structure( + assert not _reads_as_file_entity( {"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( + assert not _reads_as_file_entity( {"id": "anything", "node_kind": "page", "source_file": "docs/a.md"}) - assert not _provably_not_file_structure( + assert not _reads_as_file_entity( {"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": ""}) + assert not _reads_as_file_entity({"id": "", "source_file": "docs/a.md"}) + assert not _reads_as_file_entity({"id": "docs_a_thing", "source_file": ""}) def test_dedup_merges_crossfile_document_entity_variants(): @@ -1368,3 +1369,18 @@ def test_dedup_never_merges_repeated_headings_across_files(): ) assert {n["source_file"] for n in result_nodes} == { "docs/a.md", "docs/b.md", "docs/c.md"} + + +def test_reads_as_file_entity_trusts_the_node_kind_stamp_only(): + """The documented limit of the structural half (#296): `node_kind` is what + marks a sub-file node, so an UNSTAMPED structural node reads as an entity. + Pinned deliberately — the fix for such a producer is to stamp `node_kind`, + and this test is what fails if the predicate is ever quietly changed to + guess instead.""" + from graphify.dedup import _reads_as_file_entity + stamped = {"id": "book_xlsx_summary", "label": "Summary (sheet)", + "node_kind": "heading", "source_file": "book.xlsx"} + unstamped = dict(stamped) + del unstamped["node_kind"] + assert not _reads_as_file_entity(stamped) + assert _reads_as_file_entity(unstamped)