diff --git a/CHANGELOG.md b/CHANGELOG.md index cb65991657..c9dcc22c25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## 0.9.61 (unreleased) +- Fix: `save_semantic_cache` now recovers a group whose reported `source_file` never resolves to a real file, by an unambiguous basename match against the already dispatched allowlist, instead of silently dropping it — an incremental run driven by a weak or local backend no longer redoes already processed chunks forever when the adaptive-retry split path loses track of a source file (#2973, thanks @ayushcodes10). - Fix: `graphify.serve` now imports cleanly on Python 3.12 and 3.13. The `chinese` extra pins `jieba-py` from 3.12 onward (0.9.60 mistakenly kept the old `jieba` until 3.14, and its invalid regex escapes are a hard error on 3.12+), and the jieba import now suppresses the tokenizer's `SyntaxWarning` regardless of message or line so it never escalates under `-W error`. - Fix: the git hook's rebuild-root guard now rejects a symlink-loop or dangling `.graphify_root` on Python 3.13, whose `Path.resolve()` no longer raises on a loop — the saved root must resolve to a real directory inside the repo before it is adopted. diff --git a/graphify/cache.py b/graphify/cache.py index a70cff03dc..63afc98eed 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -1490,6 +1490,31 @@ def resolved_source_path(value: str | Path) -> Path: if allowed_source_files is not None: allowed_paths = {source_path(path) for path in allowed_source_files} + def _recover_group_path(fpath: str) -> tuple[Path, Path]: + """Return ``(cache_path, resolved_path)`` for one ``by_file`` group, + recovering an unresolvable ``source_file`` via an unambiguous + basename match against ``allowed_paths`` when one is available (#2973). + + The adaptive-retry split path (``llm.py``'s bisect-and-retry on a + chunk that overflowed a weak/local backend's context) sometimes + re-prompts with a reduced file subset and loses track of which of + the original chunk's files a given node came from, so its + ``source_file`` never resolves to a real path at all. Recovering it + against the known-good, already-dispatched allowlist -- and ONLY + when the basename is unambiguous there -- lets that group's nodes + and edges reach the cache instead of silently vanishing on every + incremental run. A genuinely bogus or ambiguous basename still + falls through unrecovered to the existing skip behavior below. + """ + cache_path = source_path(fpath) + resolved = resolved_source_path(fpath) + if not resolved.is_file() and allowed_paths is not None: + candidates = [ap for ap in allowed_paths if ap.name == cache_path.name] + if len(candidates) == 1: + cache_path = candidates[0] + resolved = candidates[0] + return cache_path, resolved + partial_paths = None if partial_source_files is not None: partial_paths = {source_path(path) for path in partial_source_files} @@ -1506,9 +1531,9 @@ def resolved_source_path(value: str | Path) -> Path: def group_skipped(fpath: str) -> bool: """Mirror the write-loop skip condition for one source_file group.""" - p = resolved_source_path(fpath) + cache_path, p = _recover_group_path(fpath) return not p.is_file() or ( - allowed_paths is not None and source_path(fpath) not in allowed_paths + allowed_paths is not None and cache_path not in allowed_paths ) # Dangling-reference pruning (#1916). A node group is skipped by the write @@ -1565,9 +1590,26 @@ def hyperedge_dangles(h: dict) -> bool: saved = 0 skipped_not_file = 0 for fpath, result in by_file.items(): - cache_path = source_path(fpath) - p = resolved_source_path(fpath) + cache_path, p = _recover_group_path(fpath) if p.is_file(): + if cache_path != source_path(fpath): + # #2973: recovery only redirected the WRITE KEY. Each item in + # this group still carries the original unresolvable + # source_file string, and _semantic_entry_matches_path + # rejects an entry on read if any item's source_file doesn't + # match the path it was loaded under -- so leaving the old + # value in place would write a "successful" entry that can + # never actually be read back, silently reproducing the same + # loss this recovery exists to fix. + corrected = _normalize_value(str(cache_path)) + result = { + **result, + "nodes": [{**n, "source_file": corrected} for n in result["nodes"]], + "edges": [{**e, "source_file": corrected} for e in result["edges"]], + "hyperedges": [ + {**h, "source_file": corrected} for h in result["hyperedges"] + ], + } if allowed_paths is not None and cache_path not in allowed_paths: warnings.warn( "semantic cache skipped out-of-scope source_file " diff --git a/tests/test_semantic_cache_basename_recovery.py b/tests/test_semantic_cache_basename_recovery.py new file mode 100644 index 0000000000..7ee0c1b8fa --- /dev/null +++ b/tests/test_semantic_cache_basename_recovery.py @@ -0,0 +1,91 @@ +"""#2973 — save_semantic_cache must recover a group whose reported +source_file never resolves to a real file, by an unambiguous basename match +against the already dispatched allowlist, instead of silently dropping it. + +A weak or local backend's adaptive-retry split path (llm.py bisecting a +chunk that overflowed context and retrying) sometimes re-prompts with a +reduced file subset and loses track of which of the original chunk's files +a given node came from, so its source_file drifts to something that never +resolves at all. Without recovery this silently discarded the group's nodes +and edges from the cache on every incremental run. +""" +from __future__ import annotations + +import pytest + +from graphify.cache import load_cached, save_semantic_cache + + +def test_malformed_but_basename_unique_path_recovers(tmp_path): + real = tmp_path / "sub" / "weird_named_file.py" + real.parent.mkdir(parents=True) + real.write_text("def f(): pass\n") + + nodes = [{"id": "n1", "label": "f", "source_file": "lost_dir/weird_named_file.py"}] + saved = save_semantic_cache(nodes, [], root=tmp_path, allowed_source_files=[real]) + assert saved == 1 + + cached = load_cached(real, root=tmp_path, kind="semantic") + assert cached is not None + assert {n["id"] for n in cached["nodes"]} == {"n1"} + + +def test_recovered_group_edges_are_not_pruned_as_dangling(tmp_path): + # group_skipped (used by the dangling-reference pruning pass) and the + # write loop must agree a recovered group is WRITTEN, not skipped -- + # otherwise an edge between two nodes in that same recovered group would + # be wrongly pruned as referencing a "skipped" id. + real = tmp_path / "sub" / "weird_named_file.py" + real.parent.mkdir(parents=True) + real.write_text("def f(): pass\ndef g(): pass\n") + + nodes = [ + {"id": "n1", "label": "f", "source_file": "lost_dir/weird_named_file.py"}, + {"id": "n2", "label": "g", "source_file": "lost_dir/weird_named_file.py"}, + ] + edges = [ + {"source": "n1", "target": "n2", "relation": "calls", + "source_file": "lost_dir/weird_named_file.py"}, + ] + saved = save_semantic_cache(nodes, edges, root=tmp_path, allowed_source_files=[real]) + assert saved == 1 + + cached = load_cached(real, root=tmp_path, kind="semantic") + assert cached is not None + assert len(cached["edges"]) == 1 + + +def test_ambiguous_basename_stays_skipped(tmp_path): + a = tmp_path / "pkg_a" / "shared.py" + b = tmp_path / "pkg_b" / "shared.py" + a.parent.mkdir(parents=True) + b.parent.mkdir(parents=True) + a.write_text("def f(): pass\n") + b.write_text("def g(): pass\n") + + nodes = [{"id": "n1", "label": "f", "source_file": "lost_dir/shared.py"}] + with pytest.warns(RuntimeWarning, match="do not resolve to real files"): + saved = save_semantic_cache(nodes, [], root=tmp_path, allowed_source_files=[a, b]) + assert saved == 0 + assert load_cached(a, root=tmp_path, kind="semantic") is None + assert load_cached(b, root=tmp_path, kind="semantic") is None + + +def test_unscoped_call_with_no_allowlist_is_unaffected(tmp_path): + # No allowed_source_files at all: recovery must never run, so a + # genuinely bogus path is skipped exactly as before this fix, and a + # normal well formed path still resolves and saves. + real = tmp_path / "sub" / "weird_named_file.py" + real.parent.mkdir(parents=True) + real.write_text("def f(): pass\n") + + nodes = [ + {"id": "n1", "label": "f", "source_file": "sub/weird_named_file.py"}, + {"id": "n2", "label": "g", "source_file": "totally/does/not/exist.py"}, + ] + saved = save_semantic_cache(nodes, [], root=tmp_path) + assert saved == 1 + + cached = load_cached(real, root=tmp_path, kind="semantic") + assert cached is not None + assert {n["id"] for n in cached["nodes"]} == {"n1"}