From 54b3b830c91bf94a4bbac6baa4e6e8ed432630d1 Mon Sep 17 00:00:00 2001 From: hassaanch23 Date: Tue, 28 Jul 2026 17:32:12 +0500 Subject: [PATCH 1/3] fix(llm): also drop semantic nodes whose source_file was never on disk (#2217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_out_of_scope` (#1895) dropped a node only when its `source_file` resolved to an EXISTING file outside the dispatched set: return p.is_file() and p not in _dispatched_resolved so a path the model invented — one that exists nowhere — was admitted, and the comment called it a "model-invented anchor" passing through by design. The cache disagrees. `save_semantic_cache` refuses the identical attribution: its write loop skips any group whose path is not a real file, and #1757's `allowed_source_files` guard skips any real file outside the dispatched batch. So graph.json accumulated nodes the cache would never persist, and on each `--update` the source was re-extracted and minted a *different* ghost — the non-convergence reported in #2217. Same script on base and on this commit, one dispatched notes.md, a node attributed to src/auth/session.py: base ghost in graph: True cache entries for ghost: 0 after ghost in graph: False cache entries for ghost: 0 (both: cache entries for notes.md: 1 — the probe does distinguish) The existence test is KEPT and a claim test added ALONGSIDE it, never in place of it: real FILE outside the dispatched set -> misattributed (#1895, unchanged) real DIRECTORY -> kept nothing on disk, and claims a file -> fabricated (#2217, new) anything else -> kept Replacing the existence arm instead of extending it looks equivalent and is not: it re-admits every real undispatched file whose name carries no corpus extension (`Makefile`, `LICENSE`, `pyproject.toml`, `data.csv`), which the cache still refuses — reopening the very divergence this closes. Pinned by test_filter_is_a_superset_of_the_1895_existence_check. Recognising a "claim" is deliberately conservative, because a false positive DELETES a node and cascades through its edges. A separator alone is not evidence — models put "N/A", "CI/CD", "TCP/IP", "and/or", "client/server architecture" in this field. A corpus suffix alone is not either — "Node.js", "Vue.js", "D3.js". So a claim requires both: a separator, and a final segment that is a single token ending in a known corpus extension. URLs and directory-shaped values are excluded on purpose; cache.py records that subagent fragments really do carry a directory here, and the base filter kept all of them. Accepted gap, stated plainly: a fabricated BARE basename ("session.py", no directory) is not recognised, because nothing syntactic separates it from the technology names above. When it names a real file the existence arm still drops it; when it names nothing it survives. Precision-first, matching `_bind_node_evidence`, which likewise prefers a missed fabrication to a destroyed node. Both halves canonicalise a source_file the same way, via cache.py's existing `_normalize_source_file_value` (#2197, same field, same problem). Normalising in only one half is worse than normalising in neither: on POSIX a node attributed to a genuinely dispatched file with Windows separators ("pkg\B.md") missed the dispatched set and the existence check, but the claim test — which does flip backslashes — called it a fabrication, so the node was deleted while save_semantic_cache stored it under the real path. That is this filter's own invariant inverted, and cache replay would put the node straight back. Five further defects found while testing the above, all reachable once dropping stops being rare: - a node id appearing twice (chunks are concatenated) could be dropped in one copy and kept in another; the cascade then stripped every edge from the surviving, correctly-attributed copy. Mirrors cache.py #1916. - a non-scalar id or edge endpoint (#1631's malformed-result class: a list where a scalar belongs) raised TypeError and aborted the run after every chunk had been paid for. cache.py guards the same spot. - an edge or hyperedge carrying its own bad source_file was filtered only when some node happened to be dropped, so a clean-nodes/dirty-edge response went unexamined. Now filtered unconditionally, and counted. - `_endpoint_dropped` judged both endpoints under one try/except, so an unhashable `source` masked a genuinely dropped `target`. - PRE-EXISTING on 0b2bd93, verified there and guarded here only because the filter above now guards the identical class two blocks up: a non-string source_file reached `Path()` in the #1890 reconciliation and aborted the run. Verdicts are memoised per source_file value — one corpus repeats the same path across thousands of items and each miss costs two stat syscalls. Counters: `out_of_scope_dropped` keeps its meaning as the node total; `provenance_dropped_misattributed` / `_fabricated` split the node failure modes; `provenance_dropped_edges` / `_hyperedges` report the edge side (both bad- attribution and endpoint-cascade removals). Co-Authored-By: Claude Opus 5 (1M context) --- graphify/llm.py | 316 ++++++++++++++++-- tests/test_provenance_invariant.py | 497 +++++++++++++++++++++++++++++ 2 files changed, 779 insertions(+), 34 deletions(-) create mode 100644 tests/test_provenance_invariant.py diff --git a/graphify/llm.py b/graphify/llm.py index 8c34cde6b..0f179b533 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -11,10 +11,11 @@ import re import sys import time +from collections import Counter from collections.abc import Callable from concurrent.futures import ThreadPoolExecutor, as_completed from dataclasses import dataclass, replace -from pathlib import Path +from pathlib import Path, PurePosixPath from graphify.file_slice import ( FileSlice, @@ -620,6 +621,93 @@ def _read_files(units: "list[Path | FileSlice]", root: Path) -> str: _UNVERIFIED_VALUE = "unverified" +# ── Provenance: does a source_file claim to be a file? ──────────────────────── +# `source_file` is spec'd (references/extraction-spec.md) as a corpus path copied +# verbatim from the dispatched FILE_LIST. Two things actually turn up there: +# +# 1. A path the model assembled itself — a plausible repo path reconstructed +# from a node id, a citation, or a basename mentioned INSIDE the document +# ("src/auth/session.py"). This is fabricated provenance (#2217): it makes a +# node look sourced when nothing sourced it. +# 2. A bare phrase on a concept node ("auth flow"). This asserts no file +# provenance at all, and #1895 deliberately kept such nodes. +# +# Only (1) is a provenance claim, so only (1) is held to the dispatched set — and +# ONLY as an addition to the #1895 existence arm, never as a replacement for it +# (see `_scope_verdict`), so nothing this misjudges can un-drop a real file. +# +# Recognising (1) has to be conservative in one specific direction: a false +# positive DELETES a node and cascades through its edges, so prose must never be +# mistaken for a path. Requiring a separator alone is not enough — models emit +# "N/A", "CI/CD", "TCP/IP", "and/or", "client/server architecture" and +# "read/write lock" into this field, and every one of those carries a slash. A +# suffix alone is not enough either: "Node.js", "Vue.js" and "D3.js" all end in a +# corpus extension. So a claim needs BOTH — a separator, and a final segment that +# is a single token ending in a known corpus extension. +# +# Known and accepted gap: a fabricated BARE basename ("session.py", no directory) +# is not recognised, because nothing syntactic separates it from the technology +# names above. When such a value names a real file it is still dropped by the +# existence arm; when it names nothing it survives. That is deliberate +# precision-first triage, matching `_bind_node_evidence` above, which also +# prefers a missed fabrication to a destroyed node. +_CORPUS_SUFFIXES: "frozenset[str] | None" = None + + +def _corpus_suffixes() -> "frozenset[str]": + """Lowercased union of every extension graphify treats as corpus input. + + Imported lazily and memoised: `detect` pulls in the extraction machinery, and + this is only ever needed on the merge path. + """ + global _CORPUS_SUFFIXES + if _CORPUS_SUFFIXES is None: + from graphify import detect as _detect + _CORPUS_SUFFIXES = frozenset( + suffix.lower() + for group in ( + _detect.CODE_EXTENSIONS, + _detect.DOC_EXTENSIONS, + _detect.PAPER_EXTENSIONS, + _detect.IMAGE_EXTENSIONS, + _detect.OFFICE_EXTENSIONS, + _detect.VIDEO_EXTENSIONS, + _detect.GOOGLE_WORKSPACE_EXTENSIONS, + ) + for suffix in group + ) + return _CORPUS_SUFFIXES + + +def _claims_a_file(source_file: "str | Path") -> bool: + """Whether *source_file* asserts a file provenance (vs. a bare concept marker). + + Requires a path separator AND a final segment that is one token ending in a + known corpus extension. False for empty values, for prose (with or without a + slash), and for directory-shaped values. + """ + sf = str(source_file).strip() + if not sf: + return False + if "://" in sf: + # A URL is a source_url, not a corpus path. Base kept these; a scheme is + # never evidence that a local file was claimed. + return False + normalized = sf.replace("\\", "/") + if "/" not in normalized: + # A single token is a name, not a path claim: "Node.js", "auth flow". + return False + last = normalized.rsplit("/", 1)[-1].strip() + if not last or " " in last or "\t" in last: + # "client/server architecture", "read/write lock", "src/auth/" — prose or + # a directory, neither of which asserts a file. + return False + try: + return PurePosixPath(last).suffix.lower() in _corpus_suffixes() + except (ValueError, OSError): # pathological value from an untrusted result + return False + + def _label_identifiers(label: str) -> list[str]: """Identifier tokens from a node label, stripped of a trailing call/args parenthesis (``foo()`` -> ``foo``, ``Cls.method(x)`` -> ``Cls``/``method``).""" @@ -2356,75 +2444,235 @@ def _checkpoint_chunk(result: dict, chunk: "list[Path | FileSlice]") -> None: # dispatched against the source_files that actually came back and surface the gap. dispatched = {unit_path(f) for chunk in chunks for f in chunk} - # Out-of-scope node filter (#1895). The #1757 cache guard already refuses - # to WRITE a cache entry for a node whose source_file is a real file that - # was not dispatched, but the node itself still flowed into the merged - # result and landed in graph.json. Mirror the #1757 condition here: resolve - # each source_file against root and drop the node only when it resolves to - # an existing file (.is_file()) outside the dispatched set — non-file - # source_files (concepts, model-invented anchors) pass through untouched. + # Provenance filter (#1895, #2217). The #1757 cache guard refuses to WRITE a + # cache entry for a node whose source_file is not a dispatched file, but the + # node itself still flowed into the merged result and landed in graph.json. + # + # This originally mirrored only HALF of the #1757 condition: it dropped a + # source_file that resolved to an existing file (.is_file()) outside the + # dispatched set, and let everything else through. A model-invented path that + # does not exist on disk therefore passed straight into the graph (#2217), + # even though `save_semantic_cache`'s `group_skipped` — `not p.is_file() or + # p not in allowed` — rejects that exact attribution. The two guards + # disagreed, and the graph took the lenient one: graph.json accumulated ghost + # nodes the cache would never replay, so each incremental --update re-extracted + # the source and minted a *different* ghost. That is the non-convergence + # reported in #2217, and the reason existence must not decide admission. + # + # The existence arm is KEPT and the claim arm is added to it — a union, not a + # replacement. Dropping the old test would have un-dropped every real + # undispatched file whose name carries no corpus extension (`Makefile`, + # `LICENSE`, `pyproject.toml`), which the cache still refuses, leaving exactly + # the divergence this is meant to close. So: + # + # resolves to a real FILE outside the dispatched set -> misattributed (#1895) + # resolves to a real DIRECTORY -> kept; cache.py records + # subagent fragments really do carry directory paths here, and the base + # filter kept them, so this must not start deleting them + # resolves to nothing AND claims a file -> fabricated (#2217) + # anything else (prose, concept markers) -> kept + # # Runs BEFORE the #1890 covered/uncovered reconciliation so that diff # reflects the post-filter graph. + _resolve_cache: dict = {} + def _resolve_against_root(value: "str | Path") -> Path: + key = str(value) + hit = _resolve_cache.get(key) + if hit is not None: + return hit p = Path(value) if not p.is_absolute(): p = root / p try: - return p.resolve() + resolved = p.resolve() except (OSError, RuntimeError): - return p + resolved = p + _resolve_cache[key] = resolved + return resolved _dispatched_resolved = {_resolve_against_root(p) for p in dispatched} - def _out_of_scope(item: dict) -> bool: + # Both halves of the decision must canonicalise a source_file the SAME way. + # `_claims_a_file` flips backslashes and strips each segment; if the + # dispatched-set lookup does not, then on POSIX a node attributed to a + # genuinely dispatched file with Windows separators ("sub\notes.md") misses + # the dispatched set, misses is_file()/is_dir(), and is then deleted as + # "fabricated" by the one half that DID normalise it. + # + # `save_semantic_cache` already applies `_normalize_source_file_value` + # (#2197, same field, same problem) to every item before grouping — so the + # cache stores such a node under the real dispatched path while the graph + # deletes it. That is this filter's own invariant inverted, and because the + # merged-result filter is bypassed on cache replay (cache.py), the next + # --update would put the node straight back. Reuse the repo's normaliser + # rather than a second, subtly different one. + from .cache import _normalize_source_file_value as _normalize_sf + + try: + _root_resolved = root.resolve() + except (OSError, RuntimeError): + _root_resolved = root + + def _spellings(raw: str) -> "set[str]": + """Every form of *raw* that could name the same dispatched file.""" + out = {raw, raw.strip()} + flat = raw.strip().replace("\\", "/") + out.add(flat) + # Segment-wise strip, matching `_claims_a_file`'s treatment of the last + # segment ("sub/ notes.md"). + out.add("/".join(seg.strip() for seg in flat.split("/"))) + for value in tuple(out): + try: + out.add(_normalize_sf(value, _root_resolved)) + except (OSError, ValueError, RuntimeError): + continue + return {v for v in out if v} + + _verdict_cache: dict = {} + + def _scope_verdict(item: dict) -> str: + """``""`` when in scope, else the reason it is not. + + ``"misattributed"`` — a real corpus file that was not dispatched (#1895). + ``"fabricated"`` — a file-claiming value with nothing on disk (#2217). + + The split is diagnostic only (both are dropped); it separates "the model + pointed at the wrong real file" from "the model invented a path", which + are different failure modes with different fixes. + + Memoised per source_file value: one corpus routinely repeats the same + path across thousands of nodes and edges, and each miss costs two stat + syscalls. + """ sf = item.get("source_file") - if not sf: + if not sf or not isinstance(sf, (str, Path)): + # A non-string here is the #1631 class of malformed result. There is + # nothing to judge and raising would abort an already-paid-for run. + return "" + raw = str(sf) + cached = _verdict_cache.get(raw) + if cached is not None: + return cached + verdict = "" + spellings = _spellings(raw) + if not any(_resolve_against_root(s) in _dispatched_resolved for s in spellings): + # Judge existence on the canonical spelling, not the raw one. + canonical = min(spellings, key=len) if spellings else raw + p = _resolve_against_root(canonical) + if p.is_file(): + verdict = "misattributed" + elif not p.is_dir() and _claims_a_file(raw): + verdict = "fabricated" + _verdict_cache[raw] = verdict + return verdict + + def _out_of_scope(item: dict) -> bool: + return bool(_scope_verdict(item)) + + def _hashable(value: "object") -> bool: + """Untrusted results really do put lists where scalars belong (#1631); + cache.py guards the same spot rather than aborting a paid-for run.""" + try: + hash(value) + except TypeError: return False - p = _resolve_against_root(sf) - return p.is_file() and p not in _dispatched_resolved + return True dropped_ids: set = set() dropped_files: set[str] = set() kept_nodes: list[dict] = [] + dropped_by_reason: Counter = Counter() for n in merged.get("nodes", []): - if _out_of_scope(n): - if n.get("id") is not None: - dropped_ids.add(n.get("id")) + verdict = _scope_verdict(n) + if verdict: + nid = n.get("id") + if nid is not None and _hashable(nid): + dropped_ids.add(nid) dropped_files.add(str(n.get("source_file"))) + dropped_by_reason[verdict] += 1 continue kept_nodes.append(n) dropped_node_count = len(merged.get("nodes", [])) - len(kept_nodes) + # A node id can legitimately appear twice in the merged result (chunks are + # concatenated, dedup happens downstream). If one copy was dropped and another + # survived, the id is still live — pruning its edges would strip a correctly + # attributed node of every relationship. cache.py fixed this exact hazard in + # its own prune (#1916, `skipped_ids -= written_ids`). + dropped_ids -= {n.get("id") for n in kept_nodes if _hashable(n.get("id"))} merged["out_of_scope_dropped"] = dropped_node_count + # Split counters, so a fabricated-provenance regression is visible as its own + # number instead of being averaged into the #1895 misattribution count. + merged["provenance_dropped_misattributed"] = dropped_by_reason["misattributed"] + merged["provenance_dropped_fabricated"] = dropped_by_reason["fabricated"] if dropped_node_count: merged["nodes"] = kept_nodes - # Keep the graph consistent: an edge or hyperedge referencing a - # dropped node's id (or itself attributed to an undispatched real - # file) must not survive its endpoint. - merged["edges"] = [ - e for e in merged.get("edges", []) - if not _out_of_scope(e) - and e.get("source") not in dropped_ids - and e.get("target") not in dropped_ids - ] - merged["hyperedges"] = [ - h for h in merged.get("hyperedges", []) - if not _out_of_scope(h) - and not (dropped_ids & set(h.get("nodes", []) or [])) - ] + + # An edge or hyperedge carries its own source_file, and the prompt holds it to + # the same rule as a node. Filtering it only when some NODE happened to be + # dropped left a clean-nodes/dirty-edge response entirely unexamined, so this + # runs unconditionally; only the endpoint cascade depends on dropped ids. + def _endpoint_dropped(*values: "object") -> bool: + # Judge each endpoint on its own: a single unhashable `source` must not + # mask a `target` that really was dropped and leave the edge dangling. + for v in values: + try: + if v in dropped_ids: + return True + except TypeError: # unhashable endpoint from a malformed result + continue + return False + + def _members_dropped(members: "object") -> bool: + try: + return bool(dropped_ids & set(members or [])) + except TypeError: + return False + + edges_before = len(merged.get("edges", [])) + hyper_before = len(merged.get("hyperedges", [])) + merged["edges"] = [ + e for e in merged.get("edges", []) + if not _out_of_scope(e) and not _endpoint_dropped(e.get("source"), e.get("target")) + ] + merged["hyperedges"] = [ + h for h in merged.get("hyperedges", []) + if not _out_of_scope(h) and not _members_dropped(h.get("nodes")) + ] + merged["provenance_dropped_edges"] = edges_before - len(merged["edges"]) + merged["provenance_dropped_hyperedges"] = hyper_before - len(merged["hyperedges"]) + + if dropped_node_count: shown = ", ".join(sorted(Path(f).name for f in dropped_files)[:5]) more = f" (+{len(dropped_files) - 5} more)" if len(dropped_files) > 5 else "" + # Name the two failure modes separately: a misattribution points at a real + # file you can go read, whereas a fabricated path has nothing behind it and + # usually signals a prompt/backend problem rather than a corpus one. + breakdown = ", ".join( + f"{count} {reason}" + for reason, count in ( + ("mis-attributed to another corpus file", dropped_by_reason["misattributed"]), + ("attributed to a path that does not exist", dropped_by_reason["fabricated"]), + ) + if count + ) print( f"[graphify] WARNING: dropped {dropped_node_count} out-of-scope node(s) " f"attributed to file(s) not dispatched for extraction: {shown}{more}. " - "The model mis-attributed them to another corpus file; they were " - "excluded from the graph (#1895).", + f"({breakdown}.) They were excluded from the graph (#1895, #2217).", file=sys.stderr, ) covered: set[Path] = set() for n in merged.get("nodes", []): sf = n.get("source_file") - if sf: + # PRE-EXISTING on 0b2bd93, not introduced here: a non-string source_file + # (the #1631 malformed-result class) reached `Path()` and raised + # TypeError, aborting the run after every chunk had been paid for. It is + # guarded rather than left alone because the node filter above now guards + # the identical class two blocks up, and a crash between them would be + # incoherent. + if sf and isinstance(sf, (str, Path)): p = Path(sf) covered.add(p if p.is_absolute() else (root / p)) uncovered = sorted( diff --git a/tests/test_provenance_invariant.py b/tests/test_provenance_invariant.py new file mode 100644 index 000000000..8efb4e406 --- /dev/null +++ b/tests/test_provenance_invariant.py @@ -0,0 +1,497 @@ +"""Tests for the semantic ``source_file`` provenance invariant (#2217). + +A semantic result's ``source_file`` is provenance: it is what tells a reader +which dispatched file a node came from. ``_out_of_scope`` (#1895) held only +attributions that resolved to an *existing* file outside the dispatched set, so +a path the model invented — one that exists nowhere — was admitted to the graph +while ``save_semantic_cache`` refused the identical attribution. These tests pin +the corrected rule: existence never decides admission, a *claimed* file does. +""" + +import warnings +from pathlib import Path +from unittest.mock import patch + +import pytest + +from graphify.cache import save_semantic_cache +from graphify.llm import _claims_a_file, extract_corpus_parallel + + +def _run(files, result, tmp_path): + """Drive extract_corpus_parallel with a faked per-chunk extraction result.""" + payload = {"input_tokens": 1, "output_tokens": 1, **result} + payload.setdefault("edges", []) + payload.setdefault("hyperedges", []) + with patch("graphify.llm.extract_files_direct", side_effect=lambda c, **k: dict(payload)): + return extract_corpus_parallel( + files, backend="kimi", root=tmp_path, + token_budget=None, chunk_size=len(files), max_concurrency=1, + ) + + +# ── _claims_a_file ──────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "value", + [ + "src/auth/session.py", # the #2217 shape: assembled repo path + "src\\auth\\session.py", # Windows separators + "docs/README.md", # known doc extension + "papers/paper.pdf", # known paper extension + "assets/diagram.png", # known image extension + "a/b/notes.DOCX", # case-insensitive suffix match + ], +) +def test_values_that_claim_a_file(value): + assert _claims_a_file(value) is True + + +@pytest.mark.parametrize( + "value", + [ + # Concept markers #1895 deliberately preserves. + "auth flow", + "authentication", + "React.Component", # dotted, but ".component" is not a corpus extension + "graphify v0.9.26", # version suffix must not read as an extension + "Section 3.2", + "", + " ", + # Slash-bearing prose. A separator alone must never imply a path: models + # routinely put these in source_file, and treating them as file claims + # deleted the nodes outright. + "N/A", + "TCP/IP", + "CI/CD", + "and/or", + "I/O", + "24/7", + "input/output", + "A/B testing", + "client/server architecture", + "read/write lock", + # A corpus suffix alone must not imply a path either — these are + # technology names, not files. + "Node.js", + "React.js", + "Vue.js", + "D3.js", + "Objective.C", + # Directory-shaped values. cache.py documents that subagent fragments + # really do carry a directory here; it is malformed provenance, not a + # fabricated file, and the base filter kept these. + "src/auth", + "src/auth/", + "docs/", + ], +) +def test_values_that_do_not_claim_a_file(value): + assert _claims_a_file(value) is False + + +def test_slash_bearing_prose_and_directories_survive_the_filter(tmp_path): + """End-to-end guard for the class above: these must reach the graph. + + A false positive here does not merely mislabel a node — it deletes it and + cascade-drops every edge that touches it. + """ + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + (tmp_path / "src").mkdir() + (tmp_path / "src" / "auth").mkdir() + + survivors = { + "na": "N/A", + "cicd": "CI/CD", + "tcpip": "TCP/IP", + "cs": "client/server architecture", + "nodejs": "Node.js", + "adir": "src/auth", + } + out = _run([doc], {"nodes": [ + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + *({"id": k, "source_file": v, "file_type": "concept"} for k, v in survivors.items()), + ]}, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"ok", *survivors} + assert out["out_of_scope_dropped"] == 0 + + +def test_filter_is_a_superset_of_the_1895_existence_check(tmp_path): + """The claim rule is an ADDITION to the existence check, never a replacement. + + A real corpus file that was not dispatched must still be dropped even when its + name carries no corpus extension (`Makefile`, `LICENSE`, `pyproject.toml`). + Replacing the existence arm silently re-admitted these while + `save_semantic_cache` went on refusing them — reopening, for that class, the + very graph-vs-cache divergence this change exists to close. + """ + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + for name in ("Makefile", "LICENSE", "pyproject.toml", "data.csv"): + (tmp_path / name).write_text("x\n", encoding="utf-8") + + out = _run([doc], {"nodes": [ + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + *({"id": n.lower(), "source_file": n, "file_type": "document"} + for n in ("Makefile", "LICENSE", "pyproject.toml", "data.csv")), + ]}, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"ok"} + # Real files, so they are misattributions (#1895), not fabrications. + assert out["provenance_dropped_misattributed"] == 4 + assert out["provenance_dropped_fabricated"] == 0 + + +def test_duplicate_id_keeps_the_surviving_copys_edges(tmp_path): + """Chunks are concatenated, so one id can appear twice in the merged result. + + If one copy is dropped and another survives, the id is still live. Pruning its + edges would leave a correctly-attributed node stripped of every relationship — + the hazard cache.py fixed for its own prune in #1916. + """ + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "dup", "source_file": "notes.md", "file_type": "document"}, + {"id": "dup", "source_file": "src/auth/session.py", "file_type": "code"}, + {"id": "peer", "source_file": "notes.md", "file_type": "document"}, + ], + "edges": [{"source": "dup", "target": "peer", "source_file": "notes.md"}], + "hyperedges": [{"id": "h", "nodes": ["dup", "peer"], "source_file": "notes.md"}], + }, tmp_path) + + assert out["provenance_dropped_fabricated"] == 1 + assert "dup" in {n["id"] for n in out["nodes"]} + assert len(out["edges"]) == 1, "surviving duplicate lost its edge" + assert len(out["hyperedges"]) == 1, "surviving duplicate lost its hyperedge" + + +def test_dirty_edge_is_filtered_even_when_no_node_is_dropped(tmp_path): + """The prompt holds edges to the same source_file rule as nodes, and the cache + groups them by their own source_file. Filtering them only when some node + happened to be dropped left a clean-nodes/dirty-edge response unexamined.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "a", "source_file": "notes.md", "file_type": "document"}, + {"id": "b", "source_file": "notes.md", "file_type": "document"}, + ], + "edges": [ + {"source": "a", "target": "b", "source_file": "notes.md"}, + {"source": "a", "target": "b", "source_file": "src/auth/session.py"}, + ], + "hyperedges": [ + {"id": "h_bad", "nodes": ["a", "b"], "source_file": "src/auth/session.py"}, + ], + }, tmp_path) + + assert out["out_of_scope_dropped"] == 0, "no node should have been dropped" + assert len(out["edges"]) == 1 + assert out["edges"][0]["source_file"] == "notes.md" + assert out["hyperedges"] == [] + assert out["provenance_dropped_edges"] == 1 + assert out["provenance_dropped_hyperedges"] == 1 + + +def test_unhashable_ids_do_not_abort_the_run(tmp_path): + """Untrusted results really do put lists where scalars belong (#1631). The + filter must degrade, not raise, after every chunk has already been paid for.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": ["a", "b"], "source_file": "src/auth/session.py", "file_type": "code"}, + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + ], + "edges": [{"source": ["a"], "target": "ok", "source_file": "notes.md"}], + }, tmp_path) + + assert "ok" in {str(n.get("id")) for n in out["nodes"]} + + +@pytest.mark.parametrize( + "spelling", + [ + " pkg/B.md ", # surrounding whitespace + "pkg/ B.md", # whitespace after the separator + "pkg\\B.md", # Windows separators on a POSIX host + " pkg\\B.md ", # both + ], +) +def test_alternate_spellings_of_a_dispatched_path_are_tolerated(tmp_path, spelling): + """Both halves of the decision must canonicalise identically. + + `_claims_a_file` flips backslashes and strips segments. If the dispatched-set + lookup does not, a node attributed to a genuinely dispatched file misses the + set, misses is_file()/is_dir(), and is deleted as "fabricated" by the one half + that did normalise — taking its edges with it. + + The path must be NESTED: a top-level ` notes.md ` can never reach the + fabricated arm (no separator means `_claims_a_file` is False), so it would + pass with the whole normalisation deleted. That earlier version of this test + was green against code that had been removed entirely. + """ + sub = tmp_path / "pkg" + sub.mkdir() + doc = sub / "B.md" + doc.write_text("b\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "ws", "source_file": spelling, "file_type": "document"}, + {"id": "peer", "source_file": "pkg/B.md", "file_type": "document"}, + ], + "edges": [{"source": "ws", "target": "peer", "source_file": "pkg/B.md"}], + }, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"ws", "peer"}, ( + f"{spelling!r} was not recognised as the dispatched pkg/B.md" + ) + assert out["provenance_dropped_fabricated"] == 0 + assert len(out["edges"]) == 1, "the node's edge was cascade-dropped with it" + + +def test_directory_attribution_that_looks_like_a_file_is_kept(tmp_path): + """Pins the `is_dir()` arm, which is otherwise unreachable in the suite. + + A value only reaches that arm when it BOTH claims a file and resolves to a + directory, so it needs a directory named like one. Deleting the arm must fail + a test rather than silently start dropping directory attributions. + """ + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + weird = tmp_path / "docs" / "guide.md" + weird.mkdir(parents=True) # a DIRECTORY named guide.md + + out = _run([doc], {"nodes": [ + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "dir", "source_file": "docs/guide.md", "file_type": "concept"}, + ]}, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"ok", "dir"} + assert out["out_of_scope_dropped"] == 0 + + +def test_url_shaped_source_file_is_not_a_fabrication(tmp_path): + """A URL is a `source_url`, not a corpus path. Base kept these; a scheme is + never evidence that a local file was claimed.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], {"nodes": [ + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "url", "source_file": "https://example.com/pkg/mod.py", "file_type": "document"}, + ]}, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"ok", "url"} + assert out["out_of_scope_dropped"] == 0 + + +def test_non_string_source_file_does_not_abort_the_run(tmp_path): + """The #1631 class again, on the source_file field rather than the id.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], {"nodes": [ + {"id": "ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "weird", "source_file": ["a/b.py"], "file_type": "document"}, + {"id": "num", "source_file": 42, "file_type": "document"}, + ]}, tmp_path) + + assert "ok" in {n["id"] for n in out["nodes"]} + + +# ── The #2217 defect ────────────────────────────────────────────────────────── + + +def test_invented_nonexistent_source_file_is_dropped(tmp_path, capsys): + """#2217: the model reconstructs a plausible repo path from a node id inside + a document. The path exists nowhere and was never dispatched, so the old + ``p.is_file()`` condition admitted it as a 'model-invented anchor'.""" + doc = tmp_path / "notes.md" + doc.write_text("see src_auth_session_validate for details\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "notes_ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "ghost", "source_file": "src/auth/session.py", "file_type": "code"}, + ], + }, tmp_path) + + ids = {n["id"] for n in out["nodes"]} + assert "ghost" not in ids, "invented source_file reached the merged graph (#2217)" + assert ids == {"notes_ok"} + assert out["out_of_scope_dropped"] == 1 + assert out["provenance_dropped_fabricated"] == 1 + assert out["provenance_dropped_misattributed"] == 0 + assert "does not exist" in capsys.readouterr().err + + +def test_invented_attribution_cascades_to_edges_and_hyperedges(tmp_path): + """A dropped node must not leave a dangling edge or hyperedge behind, and an + edge/hyperedge carrying the invented attribution itself is dropped too.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "notes_ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "also_ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "ghost", "source_file": "src/auth/session.py", "file_type": "code"}, + ], + "edges": [ + {"source": "notes_ok", "target": "also_ok", "source_file": "notes.md"}, + # endpoint was dropped + {"source": "notes_ok", "target": "ghost", "source_file": "notes.md"}, + # the edge itself carries the invented attribution + {"source": "notes_ok", "target": "also_ok", + "source_file": "src/auth/session.py"}, + ], + "hyperedges": [ + {"id": "h_ok", "nodes": ["notes_ok", "also_ok"], "source_file": "notes.md"}, + {"id": "h_ghost", "nodes": ["notes_ok", "ghost"], "source_file": "notes.md"}, + ], + }, tmp_path) + + node_ids = {n["id"] for n in out["nodes"]} + assert "ghost" not in node_ids + assert len(out["edges"]) == 1, f"dangling/invented edges survived: {out['edges']}" + assert out["edges"][0]["target"] == "also_ok" + assert [h["id"] for h in out["hyperedges"]] == ["h_ok"] + + +def test_misattributed_and_fabricated_are_counted_separately(tmp_path): + """The two failure modes need different fixes, so they get different counters: + a real file you can go read vs. a path with nothing behind it.""" + a = tmp_path / "A.md"; a.write_text("a\n", encoding="utf-8") + real_undispatched = tmp_path / "B.py" + real_undispatched.write_text("def b(): pass\n", encoding="utf-8") + + out = _run([a], { + "nodes": [ + {"id": "a_ok", "source_file": "A.md", "file_type": "document"}, + {"id": "mis", "source_file": "B.py", "file_type": "code"}, + {"id": "fab", "source_file": "src/nope.py", "file_type": "code"}, + ], + }, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"a_ok"} + assert out["provenance_dropped_misattributed"] == 1 + assert out["provenance_dropped_fabricated"] == 1 + assert out["out_of_scope_dropped"] == 2 + + +# ── What the invariant must NOT do ──────────────────────────────────────────── + + +def test_concept_marker_source_file_still_survives(tmp_path): + """Regression guard on the design choice. The literal invariant suggested in + #2217 — every non-empty source_file must resolve into the dispatched set — + deletes a concept node whose source_file is a bare phrase, which #1895 + deliberately kept (and which the #1895 test asserts). Only values that CLAIM + a file are held to the dispatched set.""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + + out = _run([doc], { + "nodes": [ + {"id": "notes_ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "auth_flow", "source_file": "auth flow", "file_type": "concept"}, + {"id": "no_anchor", "file_type": "concept"}, + ], + }, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"notes_ok", "auth_flow", "no_anchor"} + assert out["out_of_scope_dropped"] == 0 + + +def test_dispatched_files_are_never_dropped(tmp_path, capsys): + """A clean run must be untouched: no drops, no warning, counters at zero.""" + a = tmp_path / "A.md"; a.write_text("a\n", encoding="utf-8") + sub = tmp_path / "pkg"; sub.mkdir() + b = sub / "B.md"; b.write_text("b\n", encoding="utf-8") + + out = _run([a, b], { + "nodes": [ + {"id": "a_ok", "source_file": "A.md", "file_type": "document"}, + # nested attribution, given relative to root exactly as spec'd + {"id": "b_ok", "source_file": "pkg/B.md", "file_type": "document"}, + ], + }, tmp_path) + + assert {n["id"] for n in out["nodes"]} == {"a_ok", "b_ok"} + assert out["out_of_scope_dropped"] == 0 + assert out["provenance_dropped_fabricated"] == 0 + assert "out-of-scope" not in capsys.readouterr().err + + +# ── The property the bug violated ───────────────────────────────────────────── + + +def _cache_accepts(node: dict, doc: Path, tmp_path: Path, probe: Path) -> bool: + """Whether ``save_semantic_cache`` (#1757) will persist *node*. + + Uses the documented return value — the number of entries written — rather + than probing the on-disk layout, so the signal cannot silently go vacuous. + """ + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + return save_semantic_cache( + [dict(node)], [], [], root=tmp_path, cache_root=probe, + allowed_source_files=[doc], + ) > 0 + + +def test_file_claiming_attributions_agree_with_the_cache(tmp_path): + """The property whose violation caused #2217's non-convergence. + + ``save_semantic_cache`` (#1757) skips a group when ``not p.is_file() or p not + in allowed``; the merged-graph filter used to apply only the second half. A + node the graph admitted but the cache refused became a ghost: absent from the + cache, it was re-extracted on every ``--update``, minting a *different* ghost + each run. + + For every source_file that CLAIMS a file, graph admission and cache admission + must now be the same decision. (Bare concept markers are deliberately outside + this property: #1895 keeps them in the graph, while the cache — which is keyed + by real file — has always skipped them. That divergence is pre-existing, is + not a ghost, and is asserted separately below.)""" + doc = tmp_path / "notes.md" + doc.write_text("notes\n", encoding="utf-8") + real_undispatched = tmp_path / "other.py" + real_undispatched.write_text("x = 1\n", encoding="utf-8") + + file_claims = [ + {"id": "notes_ok", "source_file": "notes.md", "file_type": "document"}, + {"id": "ghost", "source_file": "src/auth/session.py", "file_type": "code"}, + {"id": "mis", "source_file": "other.py", "file_type": "code"}, + ] + concept = {"id": "concept", "source_file": "auth flow", "file_type": "concept"} + + out = _run([doc], {"nodes": [dict(n) for n in file_claims + [concept]]}, tmp_path) + survivors = {n["id"] for n in out["nodes"]} + + for i, node in enumerate(file_claims): + in_graph = node["id"] in survivors + in_cache = _cache_accepts(node, doc, tmp_path, tmp_path / f"probe{i}") + assert in_graph == in_cache, ( + f"{node['id']!r} ({node['source_file']!r}): graph={in_graph} " + f"cache={in_cache} — the #2217 divergence" + ) + + # Sanity: the probe distinguishes accept from refuse (guards against a + # vacuously-passing loop where the cache refuses everything). + assert _cache_accepts(file_claims[0], doc, tmp_path, tmp_path / "probe_ok") is True + assert _cache_accepts(file_claims[1], doc, tmp_path, tmp_path / "probe_bad") is False + + # The documented exception, pinned so it stays deliberate. + assert concept["id"] in survivors + assert _cache_accepts(concept, doc, tmp_path, tmp_path / "probe_concept") is False From 37ce04653aa9adac6a40901d609531b13d7ceff8 Mon Sep 17 00:00:00 2001 From: hassaanch23 Date: Tue, 28 Jul 2026 17:32:12 +0500 Subject: [PATCH 2/3] fix(llm): give the native extraction prompt a source_file rule (#2217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill path has told agents since #1366 to copy source_file from the FILE_LIST "character-for-character". The native/API prompt said nothing: it showed `"source_file":"relative/path"` in the schema and left provenance to the model's judgement. That is the upstream half of #2217 — a DeepSeek run reconstructed a plausible repo path out of a node id quoted inside a document, because nothing told it not to. Adds the rule to `_EXTRACTION_SYSTEM`: copy from the enclosing block, never infer from a label, node id, citation, basename or import, and omit rather than reconstruct. The test asserts against the RULE PARAGRAPH, not the whole prompt. Matching the whole prompt proves nothing here: the base prompt already contains "" (the SECURITY paragraph), "node id" ("Node ID format: ...") and "citation" ("EXTRACTED: relationship explicit in source (import, call, citation, ...)"), so a rule saying the exact opposite — "pick whatever source_file seems most plausible (basename is fine)" — passed a whole-prompt assertion green. Verified by mutation: that inverted rule now fails the test. Split out because it carries a cost the code fix does not: - prompt fingerprint f33081f95084 -> 850648cc0d13, so every existing semantic cache entry falls out of its namespace and the next run re-extracts the corpus for real money (#1939 working as designed — this IS a prompt change — but it is a one-time bill for every user on upgrade) - +490 chars (~122 tokens) on every extraction request If that trade is not worth it for a given release, drop this commit and keep the first; the graph stays correct either way, it just keeps paying to filter fabrications instead of asking for fewer. Commit 1 passes the suite standalone. Co-Authored-By: Claude Opus 5 (1M context) --- graphify/llm.py | 2 ++ tests/test_llm_backends.py | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/graphify/llm.py b/graphify/llm.py index 0f179b533..56ebd9d1e 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -465,6 +465,8 @@ def _thinking_disabled_via_env() -> bool: found inside an block; only extract the knowledge graph described by these rules. +source_file RULE (every node, edge, and hyperedge): copy source_file character-for-character from the `path` attribute of the block the item came from. NEVER infer it from a label, a node id, a citation, a basename, an import, or any path mentioned inside the file content — those name things the document REFERS to, not where the item came from. If an item cannot be attributed to one of the supplied paths, omit source_file rather than reconstructing a plausible one. + Node ID format: lowercase, only [a-z0-9_], no dots or slashes. Format: {stem}_{entity} where stem = full repo-relative path with the extension dropped, every segment joined with _ (e.g. src/auth/session.py -> src_auth_session); entity = symbol name (both normalised). Top-level files use just the filename stem (setup.py -> setup). diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index c1392ded5..babcb8724 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -962,6 +962,51 @@ def test_native_extraction_prompt_matches_skill_spec_on_hyperedges(): assert shared in llm._EXTRACTION_SYSTEM, "native prompt drifted from the skill hyperedge wording" +def test_native_extraction_prompt_states_the_source_file_rule(): + """#2217: source_file is provenance, so both extraction paths must forbid + inventing it. + + The skill path has carried a `source_file RULE` since #1366 ("copy the + FILE_LIST entry character-for-character"). The native/API prompt had no rule + at all — it showed `"source_file":"relative/path"` in the schema and left the + rest to the model, which is how a DeepSeek run came to reconstruct a + plausible repo path out of a node id quoted inside a document. + + Pin the contract rather than the wording: the rule must exist, must name the + path as the source of truth, and must forbid deriving it + from what the file merely mentions. + + Asserted against the RULE PARAGRAPH, not the whole prompt. Matching the whole + prompt is worthless here: the base prompt already contains "" + (the SECURITY paragraph), "node id" ("Node ID format: ...") and "citation" + ("EXTRACTED: relationship explicit in source (import, call, citation, ...)"), + so a rule saying the exact opposite — "pick whatever source_file seems most + plausible (basename is fine)" — passed the whole-prompt version green. + """ + spec = ( + Path(__file__).resolve().parents[1] + / "tools" / "skillgen" / "fragments" / "references" / "shared" / "extraction-spec.md" + ).read_text(encoding="utf-8") + assert "source_file RULE" in spec, "skill extraction-spec lost its source_file rule" + + for deep in (False, True): + prompt = llm._extraction_system(deep=deep) + assert "source_file RULE" in prompt, f"deep={deep}: native prompt has no source_file rule" + # Isolate the rule paragraph so nothing else in the prompt can satisfy it. + rule = prompt.split("source_file RULE", 1)[1].split("\n\n", 1)[0].lower() + + assert "untrusted_source" in rule, ( + f"deep={deep}: rule does not name the block the path must be copied from" + ) + assert "never" in rule, f"deep={deep}: rule states no prohibition" + # The failure mode #2217 reported: deriving provenance from something the + # document merely refers to, instead of from the dispatched path. + for forbidden in ("node id", "citation", "basename", "label"): + assert forbidden in rule, ( + f"deep={deep}: rule does not forbid inferring source_file from a {forbidden}" + ) + + # --- *_BASE_URL env overrides for kimi / gemini / deepseek (#1458) ------------- # BACKENDS reads the env at import time, so each case runs in a fresh interpreter # (subprocess) to avoid reload contamination of the test session. From 2b213380980a24c2ddac7655a961007411aa83dc Mon Sep 17 00:00:00 2001 From: hassaanch23 Date: Tue, 28 Jul 2026 17:32:12 +0500 Subject: [PATCH 3/3] test: audit the committed reference graphs for fabricated provenance (#2217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `worked/` holds the only production-shaped semantic output in the tree, so it is where a provenance regression would first be visible. Each graph's nodes are judged against what that run demonstrably knew about. `worked/rsl-siege-manager` ships a manifest, so all 1,886 nodes (1,268 AST, 618 LLM `rationale`) are checked against the 293 files the run walked; 1,880 carry a file-claiming source_file and all 1,880 resolve into the manifest. The manifest-less graphs have no independent ground truth for code nodes — a fabricated one would contribute its own path to the reference set and validate itself — so only the semantic layer is audited there, against the deterministic AST layer. Three things this deliberately does NOT do: - It does not pass vacuously. `worked/karpathy-repos`'s 33 non-code nodes carry no `source_file` at all (bare id+community stubs, not LLM output), so that fixture now SKIPS rather than reporting green on zero checks. httpx and mixed-corpus are pure AST and skip outright. Only one fixture carries signal and the test says so. - It does not match a bare basename. Reducing the manifest's absolute keys to corpus-relative form and comparing exactly replaced an `endswith` fallback that accepted any trailing segment. (Both spellings of the audit catch a fabricated FULL path; only this one would reject `source_file: "env.py"` against a manifest containing `...\alembic\env.py` — and "inferred it from a basename" is precisely what the new prompt rule forbids. Note the filter itself still permits that value: see commit 1's stated bare-basename gap.) - It does not mangle dot-directories. `lstrip("./")` strips a character SET, so `.github/workflows/ci.yml` became `github/...` and read as fabricated. Mutation-tested: injecting a fabricated `rationale` node with a full path fails the audit. The claim floor (>= 1,800 for rsl-siege-manager) is asserted BEFORE the skip, so a total collapse of `_claims_a_file` fails rather than quietly skipping the only fixture that checks anything — verified by forcing it False. Scope stated honestly in the module docstring: this cannot guard `_claims_a_file` against over-tightening, because that function only decides which values get checked and every attribution here is a real manifest path. Over-tightening is covered by test_values_that_do_not_claim_a_file and by #1895's own test. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_worked_graph_provenance.py | 151 ++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 tests/test_worked_graph_provenance.py diff --git a/tests/test_worked_graph_provenance.py b/tests/test_worked_graph_provenance.py new file mode 100644 index 000000000..09edc1a55 --- /dev/null +++ b/tests/test_worked_graph_provenance.py @@ -0,0 +1,151 @@ +"""Provenance audit of the committed reference graphs under ``worked/``. + +These are real graphs from real corpora (``worked/rsl-siege-manager`` alone +carries 1,268 AST nodes and 618 LLM-produced ``rationale`` nodes), so they are +the only production-shaped semantic output in the tree — and therefore where a +fabricated-provenance regression (#2217) would first be visible. + +Every ``source_file`` that claims a file must be one the same run demonstrably +knew about: the manifest where one was committed, otherwise the set of files the +deterministic AST layer attributed nodes to. A path the model invented appears in +neither. + +The corpora themselves are not vendored, so this asserts internal consistency +rather than on-disk existence — which is the right test anyway, since #2217 is +precisely about existence being the wrong question. + +Scope note: this does NOT guard `_claims_a_file` against over-tightening. That +function only decides which values get checked, and every attribution in the one +non-vacuous fixture is a real manifest path, so a stricter `_claims_a_file` could +never produce an offender here. Over-tightening is covered by +`test_values_that_do_not_claim_a_file` and by +`tests/test_chunking.py::test_out_of_scope_nodes_are_dropped_from_merged_result`. +The claim floor below is the only signal this file offers on that axis. +""" + +import json +from pathlib import Path + +import pytest + +from graphify.llm import _claims_a_file + +WORKED = Path(__file__).resolve().parents[1] / "worked" +GRAPHS = sorted(WORKED.glob("*/graph.json")) if WORKED.is_dir() else [] + +# Fixtures whose audited-claim count must not silently collapse. If a future +# change stops classifying ordinary attributions as file claims, this floor drops +# and the test fails rather than passing on an empty check. +CLAIM_FLOOR = {"rsl-siege-manager": 1800} + +pytestmark = pytest.mark.skipif(not GRAPHS, reason="worked/ reference graphs not present") + + +def _norm(value: str) -> str: + """Case- and separator-insensitive path key. + + Uses ``removeprefix("./")`` rather than ``lstrip("./")``: lstrip strips a + character SET, so it eats the leading dot of ``.github/workflows/ci.yml`` and + swallows ``../`` prefixes whole, making legitimate attributions look invented. + """ + return str(value).replace("\\", "/").lower().removeprefix("./") + + +def _strip_common_prefix(paths: "set[str]") -> "set[str]": + """Reduce absolute manifest keys to corpus-relative form. + + The manifest holds absolute paths from the machine that built the graph, while + nodes hold repo-relative ones. Stripping the longest shared directory prefix + lets the two be compared by exact equality — which matters, because an + ``endswith`` fallback would also accept a bare basename, and "inferred it from + a basename" is exactly the fabrication the extraction prompt now forbids. + """ + if len(paths) < 2: + return set(paths) + split = [p.split("/") for p in paths] + common = 0 + for parts in zip(*split): + if len(set(parts)) != 1: + break + common += 1 + if not common: + return set(paths) + return {"/".join(p[common:]) for p in split} + + +def _ground_truth(graph_dir: Path, nodes: list[dict]) -> "tuple[set[str], bool]": + """(known files, whether the truth is independent of the audited nodes). + + A committed ``manifest.json`` lists the files the run actually walked, so it + is independent evidence and every node can be judged against it. + + Without one, the only available reference is the AST layer's own + attributions. That is still ground truth *for the semantic layer* — the AST + engine is deterministic and never invents a path — but it cannot judge code + nodes, because a fabricated code node would contribute its own path to the + reference set and validate itself. Callers must restrict the audit + accordingly; the flag says which regime applies. + """ + manifest = graph_dir / "manifest.json" + if manifest.exists(): + keys = {_norm(k) for k in json.loads(manifest.read_text(encoding="utf-8"))} + return _strip_common_prefix(keys), True + return { + _norm(n["source_file"]) + for n in nodes + if n.get("file_type") == "code" and n.get("source_file") + }, False + + +def test_norm_preserves_dot_directories(): + """Regression guard for the lstrip character-set bug.""" + assert _norm(".github/workflows/ci.yml") == ".github/workflows/ci.yml" + assert _norm("./src/App.tsx") == "src/app.tsx" + assert _norm("..\\pkg\\mod.py") == "../pkg/mod.py" + + +@pytest.mark.parametrize("graph_path", GRAPHS, ids=lambda p: p.parent.name) +def test_no_node_claims_a_file_the_run_never_saw(graph_path: Path): + nodes = json.loads(graph_path.read_text(encoding="utf-8")).get("nodes", []) + assert nodes, f"{graph_path} has no nodes — fixture is empty" + known, independent = _ground_truth(graph_path.parent, nodes) + assert known, f"{graph_path}: could not establish the known-file set" + + # With only the AST layer as reference, audit the semantic layer alone — + # judging code nodes against a set they define would be circular. + audited = [n for n in nodes if independent or n.get("file_type") != "code"] + claims = [ + n for n in audited + if n.get("source_file") and _claims_a_file(n["source_file"]) + ] + # The floor is asserted BEFORE the skip, deliberately. Checking it afterwards + # made it unreachable in the one case it exists for: if `_claims_a_file` + # collapsed entirely, `claims` would be empty, the skip would fire first, and + # the only signal-bearing fixture would report as skipped rather than failed. + floor = CLAIM_FLOOR.get(graph_path.parent.name) + if floor is not None: + assert len(claims) >= floor, ( + f"{graph_path.parent.name}: only {len(claims)} file-claiming attribution(s) " + f"checked, expected >= {floor}. Either the fixture changed or " + "_claims_a_file stopped recognising ordinary corpus paths." + ) + + if not claims: + # Nothing to check. Skip rather than report green: a fixture whose audited + # nodes carry no file-claiming source_file (karpathy-repos) would + # otherwise pass vacuously, which is the failure mode this file exists to + # avoid. + pytest.skip( + f"{graph_path.parent.name}: {len(audited)} audited node(s), " + "none carrying a file-claiming source_file" + ) + + offenders = [ + (n.get("id"), n.get("file_type"), n["source_file"]) + for n in claims + if _norm(n["source_file"]) not in known + ] + assert not offenders, ( + f"{graph_path.parent.name}: {len(offenders)} of {len(claims)} checked node(s) " + f"claim a file the run never saw (fabricated provenance, #2217): {offenders[:5]}" + )