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

graphify/dedup.py

@@ -301,6 +301,53 @@ 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).
+#
+# 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 _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).
+
+    Be precise about what is proven and what is assumed, because the two halves
+    differ in strength:
+
+    * 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. 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.
+
+    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  # 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)
+
+
 # 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 +677,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
+        # 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
+        # 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 _reads_as_file_entity(n)))
              and (n.get("source_file") or "")
              and _entropy(n.get("label", "")) >= _ENTROPY_THRESHOLD),
             key=lambda n: n["id"],

tests/test_dedup.py

@@ -851,13 +851,18 @@ def test_crossfile_one_char_typo_concepts_still_merge():
 
 
 @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 @@ def test_crossfile_one_char_typo_concepts_still_merge():
       "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,183 @@ 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_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 _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 _reads_as_file_entity(
+        {"id": "journal_2024_03_01", "source_file": "journal/2024-03-01.md"})
+    assert not _reads_as_file_entity(
+        {"id": "2024_03_01", "source_file": "journal/2024-03-01.md"})  # pre-#1504 bare stem
+    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 _reads_as_file_entity(
+        {"id": "anything", "node_kind": "page", "source_file": "docs/a.md"})
+    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 _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():
+    """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"}
+
+
+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)