diff --git a/CHANGELOG.md b/CHANGELOG.md index d78fe393..660fcf4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,15 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) -## 0.9.36 (unreleased) +## 0.9.37 (unreleased) + +- Fix: TypeScript member calls no longer fabricate a high-confidence `calls` edge by matching a receiver type by name alone (#2553, thanks @Earthfreedom). A member call now resolves only when the receiver's type is defined in the same file or actually imported by the caller's file, so a third-party `import type { Repo }` can no longer bind to an unrelated local `class Repo`; table-inferred receivers are tiered to INFERRED rather than EXTRACTED. +- Fix: TypeScript/JavaScript calls inside a callback body passed to another call (for example `export const handler = wrapper(async (req) => { helper() })`) are no longer dropped (#2552, thanks @Earthfreedom). The callback body is now walked and its calls attributed to the declaration, through the same import-gated resolution so it cannot fabricate edges. +- Fix: Kotlin imports, fully-qualified calls, and one-line type bodies (#2526, #2550, #2551, thanks @spaceBrownie, @thomasrengot-hub, and @Mustaqeem66 for #2531). The extractor now matches the bundled grammar's `import` node (imports were silently dropped) and resolves each import to the real target node; a fully-qualified call like `com.example.Foo.bar()` now produces a `calls` edge; and a file with syntax the bundled grammar cannot parse (such as a one-line `class C { val x }`) now emits a warning instead of silently extracting nothing, and declarations recovered inside an error span keep their enclosing class. +- Fix: `graphify update` now retries a file whose extractor failed on a previous run instead of stamping it up-to-date forever (#2543, thanks @michaelxer). A failed extraction is no longer recorded in the manifest as processed, a manifest already poisoned by the old behavior is healed on the next run, and the fix avoids re-processing a genuinely-unchanged file. +- Fix: the claude-cli backend now surfaces an API error carried in the stdout envelope (for example a rate limit returned with a zero exit code) instead of treating it as an empty success (#2554, thanks @annieyii). The error is raised on both the zero and non-zero exit paths. + +## 0.9.36 (2026-08-07) - Fix: four commands that failed silently while exiting 0 now surface the problem (#2534, thanks @elecnix). `cluster-only` warns when `--backend`/`--model`/`--batch-size` are ignored because saved labels are being reused; the community-label prompt no longer collides with the discard sentinel (a model echoing the key back is no longer silently dropped); `tree --root` exits non-zero when the root matches no source file instead of silently flattening the tree; and `cluster-only` stamps `built_at_commit` from the analysed graph rather than the shell's working directory. Also folds in the `cluster-only` refused-write guard from #2522 (thanks @aniJani). - Fix: a Swift `extension Foo` in a different file from `Foo` no longer drops static and singleton call edges into the type (#2538, thanks @pawelo446). The extension node id is now remapped consistently so the extension merges onto its base type before call resolution, and the merge is gated so it never absorbs a same-named type from another language. diff --git a/graphify/cli.py b/graphify/cli.py index 4b18c7fb..441e4ca3 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -90,6 +90,7 @@ def _stamped_manifest_files( sem_result: dict, root: Path, partial_source_files: "set[str] | None" = None, + failed_ast_sources: "set[str] | list[str] | None" = None, ) -> dict[str, list[str]]: """Manifest-safe files dict: only stamp semantic files that actually produced output (cache hit or fresh extraction). Files whose chunk failed @@ -115,6 +116,10 @@ def _stamped_manifest_files( doc unstamped, so detect_incremental re-queued it on every run. The stamping condition mirrors the cache-write keying (a hyperedge carries its own ``source_file``); do not derive it from member nodes. + + ``failed_ast_sources`` (#2543): code files whose AST extractor errored + (missing optional extra, etc.) or returned zero nodes. They must not be + stamped as up-to-date or a later install of the extra will never re-run. """ root = Path(root) @@ -134,12 +139,16 @@ def _stamped_manifest_files( if sf: sem_extracted.add(_resolve(sf)) partial_resolved = {_resolve(p) for p in (partial_source_files or set())} + failed_ast_resolved = {_resolve(p) for p in (failed_ast_sources or [])} sem_types = {"document", "paper", "image"} return { ftype: [ f for f in flist - if ftype not in sem_types - or (_resolve(f) in sem_extracted and _resolve(f) not in partial_resolved) + if _resolve(f) not in failed_ast_resolved + and ( + ftype not in sem_types + or (_resolve(f) in sem_extracted and _resolve(f) not in partial_resolved) + ) ] for ftype, flist in files_by_type.items() } @@ -330,6 +339,88 @@ def _stale_graph_sources( return stale +def _zero_node_stamped_code_sources( + graph_path: Path, + scan_root: Path, + unchanged_code: list[str], +) -> list[str]: + """Manifest-stamped code files with a registered extractor but ZERO nodes + in the existing graph.json (#2543 heal). + + The failed-source unstamping only covers failures that happen AFTER it + shipped; a manifest poisoned by an earlier run (extraction failed, hashes + stamped anyway) keeps reporting the file unchanged forever, and the only + documented recovery was deleting graphify-out/. A stamped file that the + graph has no nodes for, despite an extractor being wired up for it, is + exactly that state — re-queue it as changed. Bounded by the same no-wedge + property: if it fails again this run it is now left unstamped, and if it + succeeds its nodes enter graph.json so the next scan stops re-queuing it. + + Membership mirrors the ``source_file`` spellings extracts store (#1897/ + #1941: scan-root-relative, forward slash; absolute for out-of-root) and + compares NFC-normalized (#2210/#2221). + """ + if not unchanged_code: + return [] + from graphify.paths import nfc + try: + data = json.loads(graph_path.read_text(encoding="utf-8")) + except Exception: + return [] + if not isinstance(data, dict): + return [] + try: + root_res = scan_root.resolve() + except (OSError, RuntimeError): + root_res = scan_root + # /graphify-out/graph.json — legacy relative source_files may be + # anchored here instead of the scan root (<=0.9.16, #555/#1899). + out_base = graph_path.parent.parent + try: + out_base = out_base.resolve() + except (OSError, RuntimeError): + pass + + present: set[str] = set() + for n in data.get("nodes", []): + if not isinstance(n, dict): + continue + sf = n.get("source_file") + if not sf or not isinstance(sf, str): + continue + present.add(nfc(sf)) + p = Path(sf) + if p.is_absolute(): + try: + present.add(nfc(str(p.resolve()))) + except (OSError, RuntimeError): + pass + else: + rel = sf.replace("\\", "/") + for base in (root_res, out_base): + present.add(nfc(os.path.normpath(str(base / rel)))) + + from graphify.extract import _get_extractor + healed: list[str] = [] + for f in unchanged_code: + p = Path(f) + if _get_extractor(p) is None: + continue # no extractor: absence from the graph is expected + spellings = {nfc(str(p))} + try: + spellings.add(nfc(str(p.resolve()))) + except (OSError, RuntimeError): + pass + try: + spellings.add(nfc(p.resolve().relative_to(root_res).as_posix())) + except (ValueError, OSError, RuntimeError): + pass + if spellings & present: + continue # the graph has this file: stamp is honest + healed.append(f) + return healed + + def _prune_graph_json_sources(graph_path: Path, stale_sources: list[str]) -> int: """Drop nodes/edges/hyperedges owned by ``stale_sources`` from graph.json in place. Returns the number of nodes removed. @@ -2991,6 +3082,24 @@ def dispatch_command(cmd: str) -> None: graph_stale_sources = _stale_graph_sources( existing_graph_path, target, _seen_files, detection=detection ) + # #2543 heal: manifests poisoned BEFORE failed-source unstamping + # existed carry live hashes for code files whose extraction failed + # (missing extra, crash) — stamped up-to-date yet absent from + # graph.json, so the incremental gate skips them forever. Treat + # such a file as changed and re-queue it; if it fails again this + # run it is now left unstamped, so this cannot wedge. + _healed_sources = _zero_node_stamped_code_sources( + existing_graph_path, + target, + detection.get("unchanged_files", {}).get("code", []), + ) + if _healed_sources: + print( + f"[graphify extract] re-queuing {len(_healed_sources)} " + f"manifest-stamped code file(s) with no nodes in graph.json " + f"(prior failed extraction, #2543)" + ) + code_files.extend(Path(p) for p in _healed_sources) else: print(f"[graphify extract] scanning {target}") detection = _detect( @@ -3528,8 +3637,16 @@ def dispatch_command(cmd: str) -> None: # Path normalization against the scan root happens inside the helper # (#1897) so fresh root-relative source_files match detect()'s # absolute file lists. - _manifest_files = _stamped_manifest_files(files_by_type, sem_result, target, - partial_source_files=_partial_semantic_files) + # #2543: also drop AST sources that failed (missing optional extra / + # zero-node anomaly) so they are not frozen as up-to-date. + _failed_ast_sources = list(ast_result.get("failed_sources") or []) + _manifest_files = _stamped_manifest_files( + files_by_type, + sem_result, + target, + partial_source_files=_partial_semantic_files, + failed_ast_sources=_failed_ast_sources, + ) # Files dispatched this run but dropped by _stamped_manifest_files # above (failed chunk, LLM omission, or any future exclusion) still @@ -3546,6 +3663,9 @@ def dispatch_command(cmd: str) -> None: f for _flist in _manifest_files.values() for f in _flist } _cleared_semantic = {str(p) for p in semantic_files} - _stamped_semantic + # #2543: AST failures need both hashes blanked (clear_ast), not just + # semantic_hash — otherwise a prior bad stamp keeps the file "unchanged". + _cleared_ast = set(_failed_ast_sources) # Full-scan manifest saves prune rows for in-root files that left the # scan corpus but still exist on disk (#1908). The corpus must be the @@ -3607,7 +3727,7 @@ def dispatch_command(cmd: str) -> None: "(--no-cluster); outputs left untouched." ) try: - _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic) + _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic, clear_ast=_cleared_ast or None) except Exception as exc: print(f"[graphify extract] warning: could not write manifest: {exc}", file=sys.stderr) stages.total() @@ -3713,7 +3833,7 @@ def dispatch_command(cmd: str) -> None: ) try: if has_path: - _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic) + _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic, clear_ast=_cleared_ast or None) except Exception as exc: print(f"[graphify extract] warning: could not write manifest: {exc}", file=sys.stderr) if global_merge: @@ -3860,7 +3980,7 @@ def dispatch_command(cmd: str) -> None: _wja(analysis_path, analysis, indent=2) try: if has_path: - _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic) + _save_manifest(_manifest_files, manifest_path=str(manifest_path), kind="both", root=target, scan_corpus=_scan_corpus, clear_semantic=_cleared_semantic, clear_ast=_cleared_ast or None) except Exception as exc: print(f"[graphify extract] warning: could not write manifest: {exc}", file=sys.stderr) diff --git a/graphify/detect.py b/graphify/detect.py index b0ea7623..31d38b62 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -1699,6 +1699,7 @@ def save_manifest( root: Path | None = None, scan_corpus: set[str] | list[str] | None = None, clear_semantic: set[str] | list[str] | None = None, + clear_ast: set[str] | list[str] | None = None, ) -> None: """Save current file mtimes + content hashes for change detection. @@ -1733,6 +1734,12 @@ def save_manifest( and making detect_incremental(kind="semantic") report them unchanged. Pass the set of such files (any path form ``scan_corpus`` accepts) to force their seeded semantic_hash to "" instead of inheriting it. + + ``clear_ast`` (#2543): same idea for AST failures (missing optional extra, + zero-node anomalous extract). Blanks BOTH ``ast_hash`` and + ``semantic_hash`` on the seeded row so either detect_incremental kind + re-queues the file after the failure is fixed, without deleting + graphify-out/. """ existing = load_manifest(manifest_path, root=root) @@ -1749,6 +1756,7 @@ def save_manifest( scan_set = _path_index(scan_corpus) clear_set = _path_index(clear_semantic) + clear_ast_set = _path_index(clear_ast) try: root_res: Path | None = Path(root).resolve() if root is not None else None except (OSError, RuntimeError): @@ -1764,6 +1772,8 @@ def save_manifest( return False def _in_clear(path_str: str) -> bool: + if clear_set is None: + return False if path_str in clear_set or _nfc(path_str) in clear_set: return True try: @@ -1772,6 +1782,17 @@ def save_manifest( except (OSError, RuntimeError): return False + def _in_clear_ast(path_str: str) -> bool: + if clear_ast_set is None: + return False + if path_str in clear_ast_set or _nfc(path_str) in clear_ast_set: + return True + try: + resolved = str(Path(path_str).resolve()) + return resolved in clear_ast_set or _nfc(resolved) in clear_ast_set + except (OSError, RuntimeError): + return False + def _in_root(path_str: str) -> bool: # Without a root we cannot tell in-root from out-of-root; fail open # (keep the row) so out-of-root corpora are never pruned by accident. @@ -1817,7 +1838,11 @@ def save_manifest( continue if scan_set is not None and not _in_scan(f) and _in_root(f): continue # excluded-but-alive: drop the stale row (#1908) - if clear_set is not None and _in_clear(f): + if clear_ast_set is not None and _in_clear_ast(f): + # AST failure this run (missing extra / zero nodes, #2543): blank + # both hashes so either detect_incremental kind re-queues. + normalised = {**normalised, "ast_hash": "", "semantic_hash": ""} + elif clear_set is not None and _in_clear(f): # Dispatched-but-omitted this run: don't inherit the stale # semantic_hash, or detect_incremental would call it unchanged (#1948). normalised = {**normalised, "semantic_hash": ""} diff --git a/graphify/llm.py b/graphify/llm.py index 5dab0aa2..2640429c 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -1373,6 +1373,30 @@ def _claude_cli_envelope(stdout: str) -> dict: return envelope +def _claude_cli_error(stdout: str) -> str: + """Return the CLI's own error text when the envelope flags `is_error`. + + `claude -p` reports API failures (rate limits, auth) in the stdout JSON + envelope with `is_error: true` and leaves stderr EMPTY — and on a rate limit + it still exits 0. So the two obvious checks both miss it: a non-zero exit + printed a bare "exited 1: " with no cause, and a zero exit fed the error + string to the JSON parser, producing an empty graph that `_response_is_hollow` + misread as truncation and adaptive retry then bisected, re-issuing requests + that were still being refused (#2554). Best-effort: unparseable stdout is not + this function's problem, the caller's `_claude_cli_envelope` reports that. + """ + try: + envelope = _claude_cli_envelope(stdout) + except RuntimeError: + return "" + if not envelope.get("is_error"): + return "" + detail = envelope.get("result") + if isinstance(detail, str) and detail.strip(): + return detail.strip() + return "unspecified error" + + # A JSON Schema pinning the top-level shape graphify consumes. Passed to # `claude -p --json-schema` (structured output) so the CLI CONSTRAINS the model # to emit the object directly instead of relying on it CHOOSING to honour a @@ -1538,10 +1562,12 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo check=False, **_no_window_kwargs(), ) + cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - raise RuntimeError( - f"claude -p exited {proc.returncode}: {proc.stderr.strip()[:500]}" - ) + detail = proc.stderr.strip() or cli_error or "(no stderr, no error envelope)" + raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") + if cli_error: + raise RuntimeError(f"claude -p reported an error: {cli_error[:500]}") envelope = _claude_cli_envelope(proc.stdout) @@ -2597,8 +2623,14 @@ def _call_llm( check=False, **_no_window_kwargs(), ) + cli_error = _claude_cli_error(proc.stdout) if proc.returncode != 0: - raise RuntimeError(f"claude -p exited {proc.returncode}: {proc.stderr.strip()[:500]}") + detail = proc.stderr.strip() or cli_error or "(no stderr, no error envelope)" + raise RuntimeError(f"claude -p exited {proc.returncode}: {detail[:500]}") + if cli_error: + # Without this the error text is returned as the model's reply and + # the caller writes it into the graph as a community label (#2554). + raise RuntimeError(f"claude -p reported an error: {cli_error[:500]}") envelope = _claude_cli_envelope(proc.stdout) cli_usage = envelope.get("usage") or {} if cli_usage: diff --git a/graphify/watch.py b/graphify/watch.py index 933bdf9d..4984bfd7 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -1325,6 +1325,41 @@ def _rebuild_code( } _rebase_relative_source_files(result, watch_root, project_root) + # #2543: AST sources that failed this run (error result, or extractor + # present but zero nodes) must not be stamped kind="ast" below, and any + # prior stamp must be blanked (clear_ast) — otherwise the incremental + # gate reports them unchanged forever and only deleting graphify-out/ + # recovers. Mirrors the extract CLI's _stamped_manifest_files handling. + _failed_ast_sources = set(result.get("failed_sources") or []) + + def _ast_manifest_files() -> dict[str, list[str]]: + """detected["files"] minus this run's failed AST sources (#2543). + + Only the STAMPED set shrinks; scan_corpus at the save sites stays + the raw detect output so #1908 pruning is unaffected. + """ + if not _failed_ast_sources: + return detected["files"] + failed_res = set(_failed_ast_sources) + for p in _failed_ast_sources: + try: + failed_res.add(str(Path(p).resolve())) + except (OSError, RuntimeError): + pass + + def _failed(f: str) -> bool: + if f in failed_res: + return True + try: + return str(Path(f).resolve()) in failed_res + except (OSError, RuntimeError): + return False + + return { + ftype: [f for f in flist if not _failed(f)] + for ftype, flist in detected["files"].items() + } + # Preserve semantic nodes/edges from a previous full run. # AST-only rebuild replaces nodes for changed files; everything else is kept. # Filter by node ID membership in the new AST output, not by file_type — @@ -1439,10 +1474,13 @@ def _rebuild_code( # pass it as the scan corpus too: rows for files that left the # scan but still exist on disk (newly excluded) are pruned # instead of surviving as phantom "deleted" entries (#1908). + # Failed AST sources are dropped from the stamped set and + # their prior hashes blanked (#2543). save_manifest( - detected["files"], manifest_path=str(out / "manifest.json"), + _ast_manifest_files(), manifest_path=str(out / "manifest.json"), kind="ast", root=watch_root, scan_corpus={f for _fl in detected["files"].values() for f in _fl}, + clear_ast=_failed_ast_sources or None, ) except Exception: pass @@ -1485,11 +1523,13 @@ def _rebuild_code( if same_topology: try: from graphify.detect import save_manifest - # Full-scan save: prune excluded-but-alive rows (#1908). + # Full-scan save: prune excluded-but-alive rows (#1908); + # leave failed AST sources unstamped (#2543). save_manifest( - detected["files"], manifest_path=str(out / "manifest.json"), + _ast_manifest_files(), manifest_path=str(out / "manifest.json"), kind="ast", root=watch_root, scan_corpus={f for _fl in detected["files"].values() for f in _fl}, + clear_ast=_failed_ast_sources or None, ) except Exception: pass @@ -1632,11 +1672,13 @@ def _rebuild_code( try: from graphify.detect import save_manifest - # Full-scan save: prune excluded-but-alive rows (#1908). + # Full-scan save: prune excluded-but-alive rows (#1908); + # leave failed AST sources unstamped (#2543). save_manifest( - detected["files"], manifest_path=str(out / "manifest.json"), + _ast_manifest_files(), manifest_path=str(out / "manifest.json"), kind="ast", root=watch_root, scan_corpus={f for _fl in detected["files"].values() for f in _fl}, + clear_ast=_failed_ast_sources or None, ) except Exception: pass diff --git a/pyproject.toml b/pyproject.toml index f99f81e8..8b184ac2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "graphifyy" -version = "0.9.36" +version = "0.9.37" description = "AI coding assistant skill (Claude Code, CodeBuddy, Codex, OpenCode, Kilo Code, Cursor, Gemini CLI, Aider, OpenClaw, Factory Droid, Trae, Hermes, Kiro, Pi, Devin CLI, Google Antigravity) - turn any folder of code, docs, papers, images, or videos into a queryable knowledge graph" readme = "README.md" license = "Apache-2.0" diff --git a/tests/test_claude_cli_backend.py b/tests/test_claude_cli_backend.py index 7699ad82..4f5187ab 100644 --- a/tests/test_claude_cli_backend.py +++ b/tests/test_claude_cli_backend.py @@ -87,6 +87,91 @@ def test_raises_on_nonzero_exit(): llm._call_claude_cli("dummy", max_tokens=8192) +_ERROR_ENVELOPE = { + "type": "result", + "subtype": "success", + "is_error": True, + "result": "API Error: Rate limit reached", + "stop_reason": "stop_sequence", + "usage": {"input_tokens": 0, "output_tokens": 0}, + "modelUsage": {}, +} + + +def test_nonzero_exit_surfaces_envelope_error_when_stderr_empty(): + # The CLI reports API failures in the stdout JSON envelope, not on stderr. + # Without reading it the user gets a bare "exited 1: " and no cause (#2554). + completed = MagicMock( + returncode=1, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached"): + llm._call_claude_cli("dummy", max_tokens=8192) + + +def test_raises_on_error_envelope_with_zero_exit(): + # `claude -p` exits 0 on a rate limit and flags it as is_error in the + # envelope. Parsing `result` as model output yields an empty graph that + # _response_is_hollow then misreads as truncation, so adaptive retry + # bisects the chunk while every request is still being refused (#2554). + completed = MagicMock( + returncode=0, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached"): + llm._call_claude_cli("dummy", max_tokens=8192) + + +def test_raises_on_array_shaped_error_envelope(): + # Newer CLIs (>= ~2.1) stream a JSON ARRAY of events with the result + # object last; the is_error flag must be honoured in that shape too (#2554). + streamed = [ + {"type": "system", "subtype": "init"}, + dict(_ERROR_ENVELOPE), + ] + completed = MagicMock(returncode=0, stdout=json.dumps(streamed), stderr="") + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached"): + llm._call_claude_cli("dummy", max_tokens=8192) + + +def test_call_llm_raises_on_error_envelope(): + # _call_llm returns envelope["result"] verbatim, so a rate-limited call + # hands "API Error: Rate limit reached" back to its callers as if it were + # model output — the dedup tiebreaker and community labeling then write + # that string into the graph as a label (#2554). + completed = MagicMock( + returncode=0, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached"): + llm._call_llm("dummy", backend="claude-cli") + + +def test_call_llm_nonzero_exit_surfaces_envelope_error(): + completed = MagicMock( + returncode=1, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", + ) + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + with pytest.raises(RuntimeError, match="Rate limit reached"): + llm._call_llm("dummy", backend="claude-cli") + + +def test_call_llm_success_still_returns_result_text(): + # Genuine success (exit 0, is_error false) must keep returning the + # envelope's result text untouched. + envelope = dict(_ENVELOPE, result="a fine label") + completed = MagicMock(returncode=0, stdout=json.dumps(envelope), stderr="") + with patch("shutil.which", return_value="/fake/bin/claude"), \ + patch("subprocess.run", return_value=completed): + assert llm._call_llm("dummy", backend="claude-cli") == "a fine label" + + def test_raises_on_garbage_envelope(): completed = MagicMock(returncode=0, stdout="not json", stderr="") with patch("shutil.which", return_value="/fake/bin/claude"), \ diff --git a/tests/test_detect.py b/tests/test_detect.py index a2794c73..3dfad3cc 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -1452,6 +1452,49 @@ def test_save_manifest_clear_semantic_erases_stale_hash_for_omitted_file(tmp_pat ) +def test_save_manifest_clear_ast_blanks_both_hashes_for_failed_extra(tmp_path): + """#2543: AST failure (missing optional extra) must blank both hashes so + the next extract re-queues the file without deleting graphify-out/.""" + import json + + sql = tmp_path / "schema.sql" + sql.write_text("CREATE TABLE users (id INT);\n") + py = tmp_path / "main.py" + py.write_text("def main():\n return 1\n") + manifest_path = str(tmp_path / "graphify-out" / "manifest.json") + corpus = {str(sql), str(py)} + + # Run 1: both files stamped as if a prior full extract succeeded. + save_manifest( + {"code": [str(sql), str(py)]}, + manifest_path, + root=tmp_path, + scan_corpus=corpus, + ) + manifest = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert manifest["schema.sql"]["ast_hash"] != "" + assert manifest["schema.sql"]["semantic_hash"] != "" + assert manifest["main.py"]["ast_hash"] != "" + + # Run 2: sql fails (missing extra) — omitted from stamped files, listed in clear_ast. + save_manifest( + {"code": [str(py)]}, + manifest_path, + root=tmp_path, + scan_corpus=corpus, + clear_ast={str(sql)}, + ) + manifest = json.loads(Path(manifest_path).read_text(encoding="utf-8")) + assert manifest["schema.sql"]["ast_hash"] == "", "failed AST source must lose ast_hash" + assert manifest["schema.sql"]["semantic_hash"] == "", "failed AST source must lose semantic_hash" + assert manifest["main.py"]["ast_hash"] != "", "successful code must keep its stamp" + + inc = detect_incremental(tmp_path, manifest_path, kind="semantic") + new_names = {Path(f).name for f in inc["new_files"].get("code", [])} + assert "schema.sql" in new_names, "failed-extra file must be re-queued" + assert "main.py" not in new_names, "unchanged successful code must stay warm" + + def test_save_manifest_without_filter_unchanged_for_code(tmp_path): """Code files must be stamped in the manifest regardless of semantic cache.""" import json diff --git a/tests/test_extract.py b/tests/test_extract.py index 28c97b27..29348576 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -3068,6 +3068,20 @@ def test_extract_warns_when_sql_extra_missing(tmp_path, capsys, monkeypatch): # the Python file still extracts normally labels = [n.get("label") for n in result["nodes"]] assert any(str(l).startswith("main") for l in labels) + # #2543: failed sql sources must be surfaced so the CLI can leave them + # unstamped in the incremental manifest. + failed = {Path(p).name for p in result.get("failed_sources", [])} + assert failed == {"schema.sql", "views.sql"} + assert "main.py" not in failed + + +def test_extract_failed_sources_empty_when_sql_installed(tmp_path): + """#2543: successful extracts do not appear in failed_sources.""" + pytest.importorskip("tree_sitter_sql") + s = tmp_path / "schema.sql"; s.write_text("CREATE TABLE users (id INT);\n") + py = tmp_path / "main.py"; py.write_text("def main():\n return 1\n") + result = extract([s, py], cache_root=tmp_path) + assert result.get("failed_sources") == [] def test_extract_no_missing_dep_warning_when_sql_installed(tmp_path, capsys): diff --git a/tests/test_extract_cli.py b/tests/test_extract_cli.py index bac501f5..fc98dc6e 100644 --- a/tests/test_extract_cli.py +++ b/tests/test_extract_cli.py @@ -1156,6 +1156,194 @@ def test_no_cluster_incremental_prunes_newly_excluded_file( assert any("keep.py" in s for s in sources) +# --------------------------------------------------------------------------- +# #2543: a code file whose AST extraction FAILED (error result — e.g. missing +# optional extra — or extractor-present zero nodes) must not be stamped in the +# incremental manifest, or detect_incremental reports it unchanged forever and +# only `rm -rf graphify-out` recovers. The extractor is swapped through +# extract._DISPATCH so the tests run without tree-sitter-sql installed. +# --------------------------------------------------------------------------- + +def _sql_failure_corpus(tmp_path): + project = tmp_path / "project" + project.mkdir() + (project / "keep.py").write_text( + "def kept():\n return still_here()\n\n" + "def still_here():\n return 1\n" + ) + (project / "schema.sql").write_text("CREATE TABLE users (id INT);\n") + return project + + +def _failing_sql(path): + # Mirrors extractors/sql.py's missing-extra result (#1745). + return {"nodes": [], "edges": [], + "error": "tree_sitter_sql not installed. Run: pip install tree-sitter-sql"} + + +def _ok_sql(path): + return { + "nodes": [{"id": "sql_schema_users", "label": path.name, "file_type": "code", + "source_file": str(path), "source_location": None}], + "edges": [], + } + + +def _manifest_row(manifest_path, name): + import json + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + for key, entry in manifest.items(): + if name in key: + return entry + return None + + +def test_failed_extra_is_retried_and_recovers(monkeypatch, tmp_path, capsys): + """Run 1 fails on schema.sql (missing extra) -> no live manifest hash; + run 2 with the extra 'installed' re-queues it and the graph gains its + nodes; run 3 settles at 0 re-extracted (no requeue loop).""" + import graphify.extract as extractmod + + project = _sql_failure_corpus(tmp_path) + out_dir = tmp_path / "out" + _clear_backend_keys(monkeypatch) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + graph_path = out_dir / "graphify-out" / "graph.json" + manifest_path = out_dir / "graphify-out" / "manifest.json" + argv = ["graphify", "extract", str(project), "--out", str(out_dir)] + + # Run 1: extraction of schema.sql fails. + failing = pytest.MonkeyPatch() + failing.setitem(extractmod._DISPATCH, ".sql", _failing_sql) + try: + _run_extract(monkeypatch, argv) + finally: + failing.undo() + capsys.readouterr() + assert not any("schema.sql" in s for s in _node_sources(graph_path)) + row = _manifest_row(manifest_path, "schema.sql") + assert row is None or (not row.get("ast_hash") and not row.get("semantic_hash")), ( + f"failed schema.sql must carry no live hash in the manifest, got {row}" + ) + assert _manifest_row(manifest_path, "keep.py")["ast_hash"] != "" + + # Run 2: the extra is now 'installed' — the file must be re-queued. + monkeypatch.setitem(extractmod._DISPATCH, ".sql", _ok_sql) + _run_extract(monkeypatch, argv) + out_text = capsys.readouterr().out + assert "1 code" in out_text, f"schema.sql must be in the changed set: {out_text}" + assert any("schema.sql" in s for s in _node_sources(graph_path)), ( + "recovered schema.sql must contribute nodes to graph.json" + ) + assert _manifest_row(manifest_path, "schema.sql")["ast_hash"] != "", ( + "recovered schema.sql must be stamped up-to-date" + ) + + # Run 3: steady state — nothing re-extracted, no heal/requeue loop. + _run_extract(monkeypatch, argv) + out_text = capsys.readouterr().out + assert "0 re-extracted" in out_text, f"run 3 must be a no-op: {out_text}" + assert "re-queuing" not in out_text + assert any("schema.sql" in s for s in _node_sources(graph_path)) + + +def test_permanent_failure_does_not_wedge(monkeypatch, tmp_path, capsys): + """A file that keeps failing is retried on every run (exactly 1 file), the + runs complete, and the rest of the graph stays stable — no wedge, no loop.""" + import graphify.extract as extractmod + + project = _sql_failure_corpus(tmp_path) + out_dir = tmp_path / "out" + _clear_backend_keys(monkeypatch) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setitem(extractmod._DISPATCH, ".sql", _failing_sql) + graph_path = out_dir / "graphify-out" / "graph.json" + manifest_path = out_dir / "graphify-out" / "manifest.json" + argv = ["graphify", "extract", str(project), "--out", str(out_dir)] + + _run_extract(monkeypatch, argv) # seed (full scan) + capsys.readouterr() + + for run in (2, 3): + _run_extract(monkeypatch, argv) + out_text = capsys.readouterr().out + assert "1 code" in out_text, ( + f"run {run}: the failed file must be retried, not frozen: {out_text}" + ) + assert "1 re-extracted" in out_text, f"run {run}: {out_text}" + sources = _node_sources(graph_path) + assert any("keep.py" in s for s in sources), f"run {run}: graph must stay stable" + assert not any("schema.sql" in s for s in sources) + row = _manifest_row(manifest_path, "schema.sql") + assert row is None or (not row.get("ast_hash") and not row.get("semantic_hash")), ( + f"run {run}: still-failing schema.sql must never gain a live hash, got {row}" + ) + + +def test_success_and_unchanged_unaffected(monkeypatch, tmp_path, capsys): + """Healthy corpus: second run re-extracts nothing and hashes stay live.""" + project = _two_file_corpus(tmp_path) + out_dir = tmp_path / "out" + _clear_backend_keys(monkeypatch) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + manifest_path = out_dir / "graphify-out" / "manifest.json" + argv = ["graphify", "extract", str(project), "--out", str(out_dir)] + + _run_extract(monkeypatch, argv) + capsys.readouterr() + _run_extract(monkeypatch, argv) + out_text = capsys.readouterr().out + assert "0 re-extracted" in out_text, f"warm healthy run must be a no-op: {out_text}" + for name in ("x.py", "keep.py"): + assert _manifest_row(manifest_path, name)["ast_hash"] != "", ( + f"{name} must keep its live stamp on a no-op run" + ) + + +def test_poisoned_manifest_is_healed(monkeypatch, tmp_path, capsys): + """A manifest poisoned BEFORE the #2543 fix (live hash stamped, file absent + from graph.json) must be re-queued and healed by the next run.""" + import json + import graphify.extract as extractmod + from graphify.detect import save_manifest + + project = _sql_failure_corpus(tmp_path) + out_dir = tmp_path / "out" + _clear_backend_keys(monkeypatch) + monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None) + monkeypatch.setitem(extractmod._DISPATCH, ".sql", _ok_sql) + graph_path = out_dir / "graphify-out" / "graph.json" + manifest_path = out_dir / "graphify-out" / "manifest.json" + argv = ["graphify", "extract", str(project), "--out", str(out_dir)] + + _run_extract(monkeypatch, argv) # healthy seed: schema.sql stamped + in graph + capsys.readouterr() + assert any("schema.sql" in s for s in _node_sources(graph_path)) + + # Poison: pre-fix state — manifest keeps the live hash while the graph has + # no nodes for the file (the old stamping path never saw the failure). + graph = json.loads(graph_path.read_text(encoding="utf-8")) + graph["nodes"] = [n for n in graph["nodes"] if "schema.sql" not in n.get("source_file", "")] + for key in ("links", "edges"): + if key in graph: + graph[key] = [e for e in graph[key] if "schema.sql" not in e.get("source_file", "")] + graph_path.write_text(json.dumps(graph), encoding="utf-8") + save_manifest( + {"code": [str(project / "schema.sql")]}, + manifest_path=str(manifest_path), kind="both", root=project, + ) + assert _manifest_row(manifest_path, "schema.sql")["ast_hash"] != "" + + _run_extract(monkeypatch, argv) + out_text = capsys.readouterr().out + assert "re-queuing 1" in out_text, ( + f"poisoned stamped file must be healed via re-queue (#2543): {out_text}" + ) + assert any("schema.sql" in s for s in _node_sources(graph_path)), ( + "healed schema.sql must be back in graph.json" + ) + + def test_cache_check_prompt_file_scopes_hits_to_that_prompt(monkeypatch, tmp_path, capsys): """#1939: cache-check --prompt-file only counts entries produced by that same extraction prompt, so an upgraded prompt reports a miss (re-extract) rather diff --git a/tests/test_partial_cache.py b/tests/test_partial_cache.py index 66f4f765..04609458 100644 --- a/tests/test_partial_cache.py +++ b/tests/test_partial_cache.py @@ -150,6 +150,28 @@ def test_stamped_manifest_excludes_partial_files(): assert out["code"] == ["x.py"] +def test_stamped_manifest_excludes_failed_ast_sources(): + """#2543: code files whose AST extract failed (missing extra) stay unstamped.""" + from pathlib import Path + from graphify.cli import _stamped_manifest_files + + files_by_type = { + "document": ["a.md"], + "code": ["/abs/ok.py", "/abs/schema.sql"], + } + sem_result = { + "nodes": [{"id": "1", "source_file": "a.md"}], + "edges": [], "hyperedges": [], + } + out = _stamped_manifest_files( + files_by_type, sem_result, Path("/abs"), + failed_ast_sources={"/abs/schema.sql"}, + ) + assert out["document"] == ["a.md"] + assert out["code"] == ["/abs/ok.py"] + assert "/abs/schema.sql" not in out["code"] + + def test_group_has_partial_marker(): assert _group_has_partial_marker({"nodes": [{"_partial": True}]}) is True assert _group_has_partial_marker({"edges": [{"_partial": True}]}) is True