mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 16:09:26 +00:00
fix(dedup): say what the entity predicate proves and what it assumes (#296)
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 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+58
-33
No files matched your search
+29
-20
@@ -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 ``_<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.
|
||||
file carries an ``_<entity>`` 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"],
|
||||
|
||||
+29
-13
@@ -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: `<path>_<entity>` 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)
|
||||
Reference in new issue
Block a user