Skip to content

fix(dedup): merge cross-file entity nodes typed by their file's extension (#296) - #3216

Open
yotamleo wants to merge 2 commits into
Graphify-Labs:v8from
yotamleo:fix/296-entity-merge-gate
Open

fix(dedup): merge cross-file entity nodes typed by their file's extension (#296)#3216
yotamleo wants to merge 2 commits into
Graphify-Labs:v8from
yotamleo:fix/296-entity-merge-gate

Conversation

@yotamleo

Copy link
Copy Markdown

Running graphify over a note vault leaves the same person or project sitting in the graph as many separate nodes — @cyrilXBT, @cyrilxbt and cyrilXBT all 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. _norm already casefolds and collapses [\W_]+, so all three spellings key to cyrilxbt and land in the same Pass 1 bucket. The bucket is then discarded, because the cross-file union added in #2182 is gated on 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. 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 typed concept, so the cross-file merge that already exists never gets to run on them.

Repro

from graphify.dedup import deduplicate_entities

nodes = [
    {"id": "journal_2024_03_01_cyrilxbt", "label": "@cyrilXBT",
     "file_type": "document", "source_file": "journal/2024-03-01.md"},
    {"id": "journal_2024_03_02_cyrilxbt", "label": "@cyrilxbt",
     "file_type": "document", "source_file": "journal/2024-03-02.md"},
    {"id": "journal_2024_03_03_cyrilxbt", "label": "cyrilXBT",
     "file_type": "document", "source_file": "journal/2024-03-03.md"},
]
print(len(deduplicate_entities(nodes, [], communities={})[0]))

Before: 3. After: 1. Flip file_type to "concept" on all three and it already printed 1 before this change — the type is the only thing separating them.

Evidence

Upstream HEAD 33362d969292b57eda82f3fbd9eb5f3f5bc9bbc2 (0.9.53), graphify/dedup.py:640 — the n.get("file_type") == "concept" filter on the Pass 1 cross-file mergeable list. 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,627 document nodes exactly 0 were their own file's node.

@adelaidasofia asked for option 1 of the three sketched there:

On your three options, 1 is the one I would want: widen the gate to entity nodes that are provably not the file's own node, entropy and provenance guards untouched. It keeps merge policy conservative and needs nothing retyped upstream.

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.py did not model it. It turns out not to need any new extractor coupling: _id_prefixes already 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 and never equals one.

The new _reads_as_file_entity requires two things, and either one failing keeps the node blocked:

  • Proven — it is not its file's own node, by the _id_prefixes reconstruction above. This is a reconstruction, not a heuristic, and it holds for every spelling a stored source_file may take.
  • Assumed — it is not a section of the file: 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_kind stamp. Absence of the marker is not proof of entity-ness: a producer that mints a node standing for part of its file without stamping node_kinddetect.py's per-sheet document nodes 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_KINDS and pins the stamped/unstamped split in a test, test_reads_as_file_entity_trusts_the_node_kind_stamp_only. Happy to stamp the detect.py path 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 steps sections across sibling documents are distinct sections, not duplicates; node_kind exists precisely because file_type cannot 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 False and 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 concept rows do. code (#1205) is still keyed by ID and never enters Pass 1 at all; image/paper still stay blocked, so shared basenames like logo.png in two directories do not merge. Pass 2's fuzzy _crossfile_fileanchored_blocked is 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_winner picks on ID shape and dedup.py never 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

  • The fixture above: 3 nodes to 1.
  • On a 15,197-node mixed code-and-markdown graph, cross-file exact-normalization groups that merge go 8 to 32 and nodes collapsed 27 to 52, with 133 document/rationale nodes 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 answers False whenever it cannot: the file's own node in each stored-path spelling, page and heading nodes, 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_variantsrationale rides 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 — two README.md stay 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_types changed meaning and are updated rather than deleted: the document and rationale rows 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 renamed document-own-file-node / rationale-own-file-node to say so.

Verified both directions: with dedup.py reverted the two merge tests fail; with the _reads_as_file_entity call stubbed out the four own-node/heading tests fail.

pytest tests/ -q was 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 on v8 in this environment (Windows: os.mkfifo/unix-socket/symlink cases in test_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 check passes and python -m tools.skillgen --check is 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.

yotamleo and others added 2 commits August 30, 2026 18:30
…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>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 filesgraphify/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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant