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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
498b76ba32
commit
d91a987aa5
+38
-4
@@ -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:
|
||||
|
||||
@@ -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]] = []
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user