From d91a987aa57063ac3705fa2e4b903523433f2ec0 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Mon, 27 Jul 2026 10:46:47 +0100 Subject: [PATCH] fix(extract): stamp target_file on Python imports and markdown refs so incremental targets canonicalize (#2211, #2213) The #2169 incremental canonicalization only rewrites edge targets that carry a target_file stamp. Python relative imports and markdown reference links emitted absolute-path-derived target ids without one, so on an incremental/subset extraction they dangled on an absolute id instead of resolving to the canonical root-relative node (dropping md->md references and leaving a dangling imports_from on --no-cluster). Both now stamp the resolved target (existence-gated); the stamp is popped before graph.json ships. Also register the unresolved target form in the remap loop so a symlinked root (macOS /tmp) can't cause an id-form mismatch. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/extract.py | 42 ++++++++++++-- graphify/extractors/markdown.py | 28 ++++++++-- tests/test_incremental.py | 97 +++++++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 9 deletions(-) diff --git a/graphify/extract.py b/graphify/extract.py index 0835a97..9de0c76 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -339,6 +339,7 @@ def _import_python(node, source: bytes, file_nid: str, stem: str, edges: list, s module_node = node.child_by_field_name("module_name") if module_node: raw = _read_text(module_node, source) + target_path: "Path | None" = None if raw.startswith("."): # Relative import - resolve to full path so IDs match file node IDs dots = len(raw) - len(raw.lstrip(".")) @@ -347,10 +348,11 @@ def _import_python(node, source: bytes, file_nid: str, stem: str, edges: list, s for _ in range(dots - 1): base = base.parent rel = (module_name.replace(".", "/") + ".py") if module_name else "__init__.py" - tgt_nid = _make_id(str(base / rel)) + target_path = base / rel + tgt_nid = _make_id(str(target_path)) else: tgt_nid = _make_id(raw) - edges.append({ + edge = { "source": file_nid, "target": tgt_nid, "relation": "imports_from", @@ -359,7 +361,22 @@ def _import_python(node, source: bytes, file_nid: str, stem: str, edges: list, s "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", "weight": 1.0, - }) + } + # Stamp the resolved target file (mirroring _import_js, #1814) so + # the #2169 remap pass can canonicalize this edge's target on an + # incremental run where the target file itself is not in the + # batch — without it the target keeps an absolute-path-derived id + # that matches no node in the merged graph and dangles (#2213). + # Existence-gated: a speculative import of a nonexistent sibling + # must stay dangling, exactly as before. The stamp is transient + # and popped before graph.json ships. + if target_path is not None: + try: + if target_path.is_file(): + edge["target_file"] = str(target_path) + except OSError: + pass + edges.append(edge) def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_path: str, scope_stack: list[str] | None = None) -> None: @@ -4698,11 +4715,18 @@ def extract( _tf = _e.get("target_file") if not _tf: continue + _raw_tp = Path(_tf) try: - _tp = Path(_tf).resolve() + _tp = _raw_tp.resolve() except (OSError, RuntimeError): continue if _tp in _remap_seen: + # Already covered: either the target is in this batch (its input + # form is the same form the extractors minted ids from, and the + # per-path loop registers both that and the resolved form) or an + # earlier stamped edge registered it. Re-appending it here would + # re-run its per-path iteration AFTER later batch files and could + # flip the last-writer of a colliding old-id key. continue _remap_seen.add(_tp) try: @@ -4719,6 +4743,16 @@ def extract( except OSError: continue remap_paths.append(_tp) + # Also register the AS-STAMPED (unresolved) form. The edge target id + # was minted from the stamped path exactly as written (e.g. + # ``str(base / rel)`` for a Python relative import), which under a + # symlinked root (macOS /tmp -> /private/tmp) or relative inputs + # differs from the resolved form; the per-path loop below derives the + # old ids from whichever Path it is given, so a missing form would + # leave the edge target unmapped and dangling. + if _raw_tp != _tp: + _remap_seen.add(_raw_tp) + remap_paths.append(_raw_tp) for path in remap_paths: old_id = _make_id(str(path)) try: diff --git a/graphify/extractors/markdown.py b/graphify/extractors/markdown.py index d6d45ac..e1b2440 100644 --- a/graphify/extractors/markdown.py +++ b/graphify/extractors/markdown.py @@ -94,10 +94,14 @@ def extract_markdown(path: Path) -> dict: "source_file": str_path, "source_location": f"L{line}"}) def add_edge(src: str, tgt: str, relation: str, line: int, - confidence: str = "EXTRACTED", weight: float = 1.0) -> None: - edges.append({"source": src, "target": tgt, "relation": relation, - "confidence": confidence, "source_file": str_path, - "source_location": f"L{line}", "weight": weight}) + confidence: str = "EXTRACTED", weight: float = 1.0, + target_file: "str | None" = None) -> None: + edge = {"source": src, "target": tgt, "relation": relation, + "confidence": confidence, "source_file": str_path, + "source_location": f"L{line}", "weight": weight} + if target_file is not None: + edge["target_file"] = target_file + edges.append(edge) file_nid = _make_id(str(path)) add_node(file_nid, path.name, 1) @@ -120,7 +124,21 @@ def extract_markdown(path: Path) -> dict: if tgt_nid == file_nid or tgt_nid in linked_targets: return linked_targets.add(tgt_nid) - add_edge(file_nid, tgt_nid, "references", line) + # Stamp the resolved target file (mirroring the JS/Python import + # stamps, #1814/#2213) so the #2169 remap pass can canonicalize this + # edge's target on an incremental run where the linked doc is not in + # the batch — without it the target keeps an absolute-path-derived id + # that matches no node in the merged graph and the md->md reference + # silently drops (#2211). Existence-gated: a link to a nonexistent + # doc must stay dangling, exactly as before. The stamp is transient + # and popped before graph.json ships. + target_file = None + try: + if resolved.is_file(): + target_file = str(resolved) + except OSError: + pass + add_edge(file_nid, tgt_nid, "references", line, target_file=target_file) # Track heading stack for nesting: [(level, nid), ...] heading_stack: list[tuple[int, str]] = [] diff --git a/tests/test_incremental.py b/tests/test_incremental.py index 91bf8e5..62c1203 100644 --- a/tests/test_incremental.py +++ b/tests/test_incremental.py @@ -211,6 +211,103 @@ def test_extract_no_cluster_incremental_code_only_preserves_doc_nodes(tmp_path): assert any("beta" in i for i in after_by_id), sorted(after_by_id) +def test_incremental_python_relative_import_target_canonicalizes(tmp_path): + """#2213 (defect 1, shared root with #2211): a Python relative import's + imports_from edge must stamp target_file so the #2169 remap canonicalizes + its target on an incremental extraction where the target file is NOT in + the batch — instead of leaving an absolute-path-derived dangling id.""" + from graphify.extract import extract, _make_id + + # realpath: on macOS the pytest tmp dir can sit behind a symlink + # (/tmp -> /private/tmp); anchor everything on the resolved form so the + # canonical-id assertions are deterministic. + tmp = Path(os.path.realpath(tmp_path)) + pkg = tmp / "pkg" + pkg.mkdir() + (pkg / "b.py").write_text( + "class Thing:\n def go(self):\n return 1\n", encoding="utf-8" + ) + a = pkg / "a.py" + a.write_text( + "from .b import Thing\n\n\ndef use():\n return Thing().go()\n", + encoding="utf-8", + ) + + full = extract([a, pkg / "b.py"], cache_root=tmp) + full_imports = [ + e for e in full["edges"] + if e.get("relation") == "imports_from" + and str(e.get("source_file", "")).endswith("a.py") + ] + assert full_imports, full["edges"] + canonical = full_imports[0]["target"] + assert canonical == "pkg_b", canonical + + # Incremental: only the importer is in the batch (b.py unchanged, so the + # #2169 merge path re-extracts a.py alone). Same cache/scan root. + inc = extract([a], cache_root=tmp) + inc_imports = [ + e for e in inc["edges"] + if e.get("relation") == "imports_from" + and str(e.get("source_file", "")).endswith("a.py") + ] + assert inc_imports, inc["edges"] + assert inc_imports[0]["target"] == canonical, inc_imports + # Not an absolute-path-shaped ghost id (…_pkg_b would end "_b", but the + # pre-fix dangling form was the full path with the extension folded in). + assert not inc_imports[0]["target"].endswith("_py"), inc_imports + + root_slug = _make_id(str(tmp)) + for e in inc["edges"]: + assert root_slug not in str(e.get("target", "")), e + # The target_file hint is transient and must never ship. + assert "target_file" not in e, e + + +def test_incremental_md_reference_target_canonicalizes(tmp_path): + """#2211: a markdown [link](docs/setup.md) references edge must stamp + target_file so the #2169 remap canonicalizes its target on an incremental + extraction where the linked doc is NOT in the batch — instead of the + md->md reference dangling on an absolute-path-derived id and dropping.""" + from graphify.extract import extract, _make_id + + tmp = Path(os.path.realpath(tmp_path)) + docs = tmp / "docs" + docs.mkdir() + setup = docs / "setup.md" + setup.write_text("# Setup\nInstall the thing.\n", encoding="utf-8") + claude = tmp / "CLAUDE.md" + claude.write_text( + "# Overview\nSee [setup](docs/setup.md) for install steps.\n", + encoding="utf-8", + ) + + full = extract([claude, setup], cache_root=tmp) + full_refs = [ + e for e in full["edges"] + if e.get("relation") == "references" + and str(e.get("source_file", "")).endswith("CLAUDE.md") + ] + assert full_refs, full["edges"] + canonical = full_refs[0]["target"] + assert canonical == "docs_setup", canonical + + # Incremental: only the linking doc is in the batch. + inc = extract([claude], cache_root=tmp) + inc_refs = [ + e for e in inc["edges"] + if e.get("relation") == "references" + and str(e.get("source_file", "")).endswith("CLAUDE.md") + ] + assert inc_refs, inc["edges"] + assert inc_refs[0]["target"] == canonical, inc_refs + + root_slug = _make_id(str(tmp)) + for e in inc["edges"]: + assert root_slug not in str(e.get("target", "")), e + assert "target_file" not in e, e + + def test_update_prunes_a_removed_imports_edge(tmp_path): """#1521: when an import is deleted from a file, `graphify update` must prune the edge it produced — preserving it (keyed only on endpoint membership) left a