fix(dedup): merge cross-file entity nodes typed by their file's extension (#296) - #3216
fix(dedup): merge cross-file entity nodes typed by their file's extension (#296)#3216yotamleo wants to merge 2 commits into
Conversation
…sion (Graphify-Labs#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 Graphify-Labs#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. Graphify-Labs#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` (Graphify-Labs#1205), `image`/`paper` shared basenames, and Pass 2's fuzzy `_crossfile_fileanchored_blocked` — so Graphify-Labs#1284's near-identical boilerplate and Graphify-Labs#3094's repeated `## Decisions` sections stay per-file, both now covered by tests. Co-Authored-By: Claude <noreply@anthropic.com>
…raphify-Labs#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>
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Widens Pass 1's cross-file exact-merge gate so rationale/document nodes join concept when they read as an entity inside their file, fixing the bug where an entity extracted from a note (typed by the file's extension) never merged across files. Adds _reads_as_file_entity, which proves a node isn't its file's own node via _id_prefixes across every stored-path spelling and honours the node_kind markers page/heading (_FILE_STRUCTURE_NODE_KINDS) so a file's own node and its sections still never merge, while any node lacking an id, provenance, or a stamped node_kind stays blocked. Pass 2's fuzzy anchoring, the entropy gate, and the provenance requirement are untouched.
Worth a look
- Exact document/rationale entities now merge across files —
graphify/dedup.py:699· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 476 functions depend on the 213 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 71 callers, 22 callees - new:
build_merge()— 62 callers, 13 callees - new:
dispatch_command()— 2 callers, 123 callees - new:
build()— 40 callers, 4 callees - new:
_prune()— 9 callers, 3 callees - new:
_llm_tiebreak()— 1 callers, 10 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 476 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 255 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify deduplicate\_entities.
The verifier did not have enough to check deduplicate\_entities, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
· 7 more finding(s) on lines outside this diff (see the check run).
Running graphify over a note vault leaves the same person or project sitting in the graph as many separate nodes —
@cyrilXBT,@cyrilxbtandcyrilXBTall survive as distinct entities, one per file that mentions them. Community detection then splits what should be one cluster into fragments and god-node analysis misses the real hubs, because a single entity's mentions are scattered across aliases; on the graph measured in #296 this left 1,741 collision groups unmerged.Normalization is not what fails.
_normalready casefolds and collapses[\W_]+, so all three spellings key tocyrilxbtand land in the same Pass 1 bucket. The bucket is then discarded, because the cross-file union added in #2182 is gated onfile_type == "concept"— and an entity the extractor pulls out of a.mdnote inheritsdocumentfrom the file's extension, not from anything about the entity. Note-heavy corpora do not have a canonicalization problem that code repos lack; they have the same one, but almost none of their entity nodes are typedconcept, so the cross-file merge that already exists never gets to run on them.Repro
Before:
3. After:1. Flipfile_typeto"concept"on all three and it already printed1before this change — the type is the only thing separating them.Evidence
Upstream HEAD
33362d969292b57eda82f3fbd9eb5f3f5bc9bbc2(0.9.53),graphify/dedup.py:640— then.get("file_type") == "concept"filter on the Pass 1 cross-filemergeablelist. The measurement behind the numbers above is in #296: #296 (comment) — classifying every surviving cross-file_norm-collision group by which gate rejects it gave 1,741 blocked by this filter, 2 by the entropy gate and 0 by provenance, and of 39,627documentnodes exactly 0 were their own file's node.@adelaidasofia asked for option 1 of the three sketched there:
That is what this implements — nothing retyped, no migration, no new subcommand.
The predicate
"Provably not the file's own node" needs something concrete, and
dedup.pydid not model it. It turns out not to need any new extractor coupling:_id_prefixesalready enumerates the ID a node standing forsource_fileitself 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 and never equals one.The new
_reads_as_file_entityrequires two things, and either one failing keeps the node blocked:_id_prefixesreconstruction above. This is a reconstruction, not a heuristic, and it holds for every spelling a storedsource_filemay take.node_kind: "heading", or"page"for the file node the markdown extractor labels outright.I have kept those two labelled separately in the code rather than calling the whole thing "provable", because the second half is only as good as the
node_kindstamp. Absence of the marker is not proof of entity-ness: a producer that mints a node standing for part of its file without stampingnode_kind—detect.py's per-sheetdocumentnodes are the in-tree example — yields structural nodes this predicate reads as entities, and two of them sharing a label in different files would merge. I did not paper over that with a guess about what an unstamped node meant; the conservative fix is on the producer side, so the PR states a short producer contract next to_FILE_STRUCTURE_NODE_KINDSand pins the stamped/unstamped split in a test,test_reads_as_file_entity_trusts_the_node_kind_stamp_only. Happy to stamp thedetect.pypath too if you would rather close it here — I left it alone because that code sits behind a not-yet-wired feature flag.The second condition is why the widening does not touch the guard verified in #3094. Repeated
## Decisions/## Next stepssections across sibling documents are distinct sections, not duplicates;node_kindexists precisely becausefile_typecannot carry that distinction, so it is the field that separates a heading from an entity found in the same file. Three documents each carrying both headings still yield six heading nodes, each attributed to its own file — there is a test for exactly that shape.A node that cannot be checked (no ID, no provenance) answers
Falseand stays blocked. The predicate can therefore only ever narrow what the gate treats as file-anchored, never widen it on a guess.What is deliberately unchanged
The entropy and provenance guards are byte-untouched, and the widened rows go through them exactly as
conceptrows do.code(#1205) is still keyed by ID and never enters Pass 1 at all;image/paperstill stay blocked, so shared basenames likelogo.pngin two directories do not merge. Pass 2's fuzzy_crossfile_fileanchored_blockedis untouched, so #1284's near-identical parallel-module boilerplate stays blocked — the widening is scoped to the exact-normalization pass, where the label is identical after normalization rather than merely similar.The survivor heuristic ("prefer the node with non-generated body content as the merge target") is not in this PR.
_pick_winnerpicks on ID shape anddedup.pynever sees body content, so doing it properly is its own change; a file's own node — usually the hand-curated note — is held out of the merge entirely here rather than being made to win it.Effect
document/rationalenodes held back by the predicate as file structure.Tests
Added to
tests/test_dedup.py, all in the repo's existing style:test_reads_as_file_entity_helper— the predicate proves entity-ness and answersFalsewhenever it cannot: the file's own node in each stored-path spelling,pageandheadingnodes, and nodes missing an ID or provenance.test_dedup_merges_crossfile_document_entity_variants— the reported shape, three spellings across three notes, collapsing to one.test_dedup_merges_crossfile_rationale_entity_variants—rationalerides the same gate.test_dedup_never_merges_a_files_own_node_away— the boundary: a curated note whose own node carries the entity's label survives alongside a mention of it elsewhere.test_dedup_never_merges_two_files_own_nodes— twoREADME.mdstay two documents.test_dedup_never_merges_repeated_headings_across_files— the Entity dedup merges nodes across different source files, silently removing whole documents from the graph #3094 verification case.test_dedup_crossfile_entity_merge_keeps_the_entropy_gate/..._provenance_gate— negative controls proving those guards still fire on the widened rows.test_dedup_crossfile_fuzzy_fileanchored_block_is_untouched— Pass 2 still blocks near-identical cross-file document labels.test_reads_as_file_entity_trusts_the_node_kind_stamp_only— pins the documented limit above: a stamped structural node stays blocked, an unstamped one reads as an entity.Two existing rows in
test_crossfile_identical_labels_stay_distinct_for_guarded_typeschanged meaning and are updated rather than deleted: thedocumentandrationalerows asserted that an identical label never merges cross-file for those types, which is the behaviour this PR is changing. They now carry the IDs the files' own nodes would be minted with, so they keep guarding those types at the boundary that still holds, and the ids are renameddocument-own-file-node/rationale-own-file-nodeto say so.Verified both directions: with
dedup.pyreverted the two merge tests fail; with the_reads_as_file_entitycall stubbed out the four own-node/heading tests fail.pytest tests/ -qwas run on the clean base commit and again on this branch, on the same machine: 17 failed, 5250 passed before and 17 failed, 5259 passed after — the same 17 failures, identical sets, and +9 passed for the tests added here (a tenth landed in the follow-up commit). Those 17 are pre-existing onv8in this environment (Windows:os.mkfifo/unix-socket/symlink cases intest_non_regular_files.py, POSIX-path install/uninstall cases, and two unicode-normalization tests) and are unrelated to this change — no test that passed on the baseline fails after it.ruff checkpasses andpython -m tools.skillgen --checkis clean.Addresses the option-1 half of #296. The issue as filed also asks for a configurable canonicalization subcommand and edge-dedup pass, which this does not add, so it should not auto-close.