mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-06 05:35:54 +00:00
fix(watch): require deletion evidence before evicting a missing source (#1795)
_reconcile_existing_graph treated "source identity absent from the collected corpus" as deletion and evicted its nodes/edges/hyperedges. But corpus absence is ambiguous: it's also what you see when a file still exists and merely stopped being collected (ignore rules or filters changed). Upgrading into the merged- .gitignore scan semantics (#1363) mass-evicted 655 nodes from a deliberately- built, .gitignore'd docs dir whose files were present the whole time — reported as a successful rebuild. Fail-closed: before evicting a corpus-absent identity, require Path(identity) .exists() is False (identity is an absolute path). Alive-but-excluded sources are preserved (nodes, edges, hyperedges) and a loud line reports how many were kept and why. True deletions and renames still evict (old path gone from disk); a full extract --force still purges deliberate exclusions via the AST ownership rule. Existence is memoized (one stat per file that left the corpus). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
0efb2a443c
commit
591da764a1
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## 0.9.13 (unreleased)
|
||||
|
||||
- Fix: incremental `graphify update` no longer silently evicts nodes for a file that left the scan corpus but still exists on disk (#1795, thanks @CJNA). `_reconcile_existing_graph` read "source absent from the collected corpus" as "deleted", but that's also what an ignore-rule/filter change looks like (e.g. an upgrade that starts honoring `.gitignore`) — in one 27k-node graph the first rebuild after such an upgrade mass-evicted 655 nodes whose files were present the whole time. Eviction now fails closed: a corpus-absent source is only evicted when `Path(identity).exists()` is False (true deletion), otherwise its nodes/edges/hyperedges are preserved and a loud line reports how many were kept and why. True deletions and renames evict as before; a full `extract --force` still purges deliberate exclusions.
|
||||
|
||||
- Fix: `build_merge` no longer silently deletes a re-extracted file's fresh nodes when that file is also passed in `prune_sources` (#1796, thanks @erichkusuki). A file present in `new_chunks` is being replaced, not deleted, so it's now excluded from the prune set — "replace" wins over a contradictory "delete" of the same source. Previously, following the old edit-workflow (pass the changed file in `prune_sources`) deleted the just-built concept whenever an edit kept a node's label. Genuine deletions (a file in `prune_sources` but not `new_chunks`) still prune.
|
||||
|
||||
- Fix: `graphify path` resolves each endpoint to the first candidate whose label contains every query token, instead of blindly taking the top-scored node (#1785, thanks @CJNA). `_score_nodes`' full-query bonus only fires when the query equals/prefixes a label, so a query that is a token *subset* of the intended label (`"Reject-everything judge"` vs `"Degenerate Reject-Everything Judge"`) got no bonus and a node prefix-matching one rare token could outscore it — anchoring the path on an unrelated, often disconnected node and yielding a false "No path found". When the top candidate already full-matches (the common case) the pick is unchanged. Applied to both the `path` CLI and the MCP shortest-path tool; the close-runner-up ambiguity warning now fires only when the score head is what was actually picked.
|
||||
|
||||
@@ -418,6 +418,17 @@ def _reconcile_existing_graph(
|
||||
# lists can contain only a rename destination, so explicit paths alone
|
||||
# cannot identify the stale source. Keep the comparison scoped to the
|
||||
# watched root so subfolder updates preserve records outside that subtree.
|
||||
#
|
||||
# Fail-closed eviction: a source identity missing from the corpus is only
|
||||
# DELETION evidence when the file is actually gone from disk. A file that
|
||||
# still exists but stopped being collected was *excluded* (ignore rules or
|
||||
# filters changed — e.g. a .gitignore the scanner newly honors), and
|
||||
# treating that as deletion silently mass-evicts good nodes. Preserve
|
||||
# instead and say so; a full re-extraction still purges deliberately
|
||||
# excluded sources via the AST ownership rule below.
|
||||
excluded_alive_files: set[str] = set()
|
||||
excluded_alive_nodes = 0
|
||||
_alive_cache: dict[str, bool] = {}
|
||||
for node in existing.get("nodes", []):
|
||||
source_file = node.get("source_file")
|
||||
if not source_file or _get_extractor(Path(source_file)) is None:
|
||||
@@ -426,6 +437,15 @@ def _reconcile_existing_graph(
|
||||
if not source_paths.in_watch_root(source_file):
|
||||
continue
|
||||
if identity not in current_sources:
|
||||
if identity:
|
||||
alive = _alive_cache.get(identity)
|
||||
if alive is None:
|
||||
alive = Path(identity).exists()
|
||||
_alive_cache[identity] = alive
|
||||
if alive:
|
||||
excluded_alive_files.add(identity)
|
||||
excluded_alive_nodes += 1
|
||||
continue
|
||||
normalized = source_paths.normalize(source_file)
|
||||
if normalized:
|
||||
deleted_paths.add(normalized)
|
||||
@@ -433,6 +453,13 @@ def _reconcile_existing_graph(
|
||||
node_evicted_source_identities.add(identity)
|
||||
edge_evicted_source_identities.add(identity)
|
||||
hyperedge_evicted_source_identities.add(identity)
|
||||
if excluded_alive_files:
|
||||
print(
|
||||
f"[graphify watch] fail-closed: kept {excluded_alive_nodes} node(s) "
|
||||
f"from {len(excluded_alive_files)} file(s) that left the scan corpus "
|
||||
"but still exist on disk (ignore rules or filters changed?). "
|
||||
"Run a full re-extraction to purge them if the exclusion is intentional."
|
||||
)
|
||||
|
||||
# A full re-extraction owns every AST node under watch_root. Incremental
|
||||
# extraction owns only nodes from rebuilt or deleted sources. Semantic
|
||||
|
||||
@@ -1410,3 +1410,59 @@ def test_merge_changed_paths_dedupes_in_order():
|
||||
[Path("a.py")],
|
||||
)
|
||||
assert [p.as_posix() for p in merged] == ["a.py", "b.py", "c.py"]
|
||||
|
||||
|
||||
def test_rebuild_code_preserves_nodes_from_excluded_but_alive_file(tmp_path, capsys):
|
||||
"""Fail-closed eviction: a file that leaves the scan corpus (newly ignored)
|
||||
but still exists on disk was EXCLUDED, not deleted — its nodes must survive
|
||||
an incremental rebuild, with a loud message, instead of being silently
|
||||
mass-evicted as stale sources (the docs/brainstorms incident: an upgrade
|
||||
started honoring .gitignore and evicted 655 nodes whose files were present).
|
||||
"""
|
||||
import json
|
||||
from graphify.watch import _rebuild_code
|
||||
|
||||
corpus = tmp_path / "corpus"
|
||||
(corpus / "notes").mkdir(parents=True)
|
||||
(corpus / "auth.py").write_text("def login(): pass\n", encoding="utf-8")
|
||||
(corpus / "notes" / "brainstorm.md").write_text(
|
||||
"# Brainstorm\n\nA local-only design note.\n", encoding="utf-8"
|
||||
)
|
||||
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
graph_path = corpus / "graphify-out" / "graph.json"
|
||||
labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]}
|
||||
assert "brainstorm.md" in labels
|
||||
|
||||
# The file becomes ignored (leaves the corpus) but stays on disk.
|
||||
(corpus / ".graphifyignore").write_text("notes/\n", encoding="utf-8")
|
||||
capsys.readouterr()
|
||||
|
||||
assert _rebuild_code(corpus, changed_paths=[Path("auth.py")], acquire_lock=False) is True
|
||||
labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]}
|
||||
assert "brainstorm.md" in labels, (
|
||||
"nodes from an excluded-but-alive file must be preserved, not evicted"
|
||||
)
|
||||
assert "fail-closed: kept" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_rebuild_code_still_evicts_when_excluded_file_is_also_deleted(tmp_path):
|
||||
"""The fail-closed preserve must not weaken true-deletion eviction: once the
|
||||
excluded file is actually gone from disk, its nodes are evicted as before."""
|
||||
import json
|
||||
from graphify.watch import _rebuild_code
|
||||
|
||||
corpus = tmp_path / "corpus"
|
||||
(corpus / "notes").mkdir(parents=True)
|
||||
(corpus / "auth.py").write_text("def login(): pass\n", encoding="utf-8")
|
||||
(corpus / "notes" / "brainstorm.md").write_text("# Brainstorm\n", encoding="utf-8")
|
||||
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
graph_path = corpus / "graphify-out" / "graph.json"
|
||||
|
||||
(corpus / "notes" / "brainstorm.md").unlink()
|
||||
|
||||
assert _rebuild_code(corpus, changed_paths=[Path("auth.py")], acquire_lock=False) is True
|
||||
labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]}
|
||||
assert "brainstorm.md" not in labels, "deleted file's nodes must still be evicted"
|
||||
assert "login()" in labels
|
||||
|
||||
Reference in New Issue
Block a user