fix(update): file-aware shrink-guard so removed symbols prune without --force
`graphify update` after deleting a function left the stale node in graph.json. The build correctly dropped it (#1116), but _check_shrink then refused to write the smaller graph ("Refusing to overwrite — you may be missing chunk files"), so the deletion never persisted without --force. That also starved the work-memory node-existence gate, which relies on graph.json reflecting deletions. The shrink-guard now takes the set of source files re-extracted this run (rebuilt_sources). A net shrink is allowed when every lost node belongs to a rebuilt source (a symbol genuinely removed) or a deleted file; it is still refused when a node vanishes from a file we did NOT touch — the silent failed/partial-extraction case the guard exists to catch. The #1116 e2e test now asserts the prune happens with force=False (was force=True); added two _check_shrink unit tests (allowed within rebuilt sources, refused outside). Full suite 2339 passed; skillgen --check clean; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
b448c16cbd
commit
533859dc51
@@ -4,6 +4,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Fix: `graphify update` now prunes a function/symbol removed from a still-present file without needing `--force`. The build already dropped the stale node (#1116), but the shrink-guard then refused to write the smaller graph ("new graph has N nodes but existing has M … Refusing to overwrite"), so the deletion silently never persisted unless you passed `--force` — leaving stale nodes (and the work-memory node-existence gate) lagging until a forced rebuild. The guard is now file-aware: a net shrink is allowed when every lost node belongs to a file re-extracted this run (or deleted), and still refused when a node disappears from a file that was *not* touched (the failed/partial-extraction case it exists to catch).
|
||||
- Fix: `validate_extraction` and `build_from_json` no longer crash on a non-hashable node `id` or edge `source`/`target` (e.g. a list emitted by a malformed LLM extraction) — previously a single bad node raised `TypeError: unhashable type` and aborted the entire build of an otherwise-complete corpus. The validator now reports the bad id/endpoint as an error string (its documented contract), and the build skips the malformed entry with a stderr warning while keeping every well-formed node/edge; non-dict nodes are still left to raise so shape diagnostics are unchanged (#1447, thanks @dschwartzi).
|
||||
- Fix: Python qualified class-method calls (`ClassName.method(...)`) now produce an EXTRACTED `calls` edge to the class-qualified method node (#1446). Previously these cross-class static/qualified calls were dropped: the shared cross-file pass skips all member calls (the #543/#1219 god-node guard against bare `obj.method()` collisions), and when the called method shared its name with an in-file node — e.g. a viewset action `approve()` delegating to a service `Service.approve()` — the bare-name lookup matched the caller's own node and silently dropped it. The Python extractor now captures a simple-identifier receiver, defers capitalized-receiver member calls to a new receiver-based resolver (`_resolve_python_member_calls`, mirroring the Swift pass), and emits the edge only when the receiver resolves to exactly one class that owns the method (single-definition god-node guard); instance/module calls (`self.x()`, `obj.x()`, lowercase receivers) are unaffected.
|
||||
- Feat: new first-class `agents` platform installs the skill to the generic cross-framework Agent-Skills locations. `graphify install --platform agents` (alias `--platform skills`) writes the spec's user-global `~/.agents/skills/graphify/SKILL.md` — the directory `npx skills` and spec-compliant frameworks read — and `--project` writes `./.agents/skills/graphify/SKILL.md`; `graphify uninstall` removes them. Previously that user-global location was only reachable as an accidental side effect of the gemini-on-Windows branch. The skill bundle re-homes amp's agents-md body (registered in `tools/skillgen/platforms.toml`, rendered through the skillgen drift/coverage guards); the body is identical to amp's, and only the on-demand hooks reference differs — it points at `graphify agents install`, which (as the amp-twin subcommand) wires the skill plus an AGENTS.md always-on section. Bare `graphify install` is unchanged — still single-platform (claude/windows) (#1432, closes #1405).
|
||||
|
||||
+49
-14
@@ -355,6 +355,7 @@ def _check_shrink(
|
||||
tmp: "Path | None" = None,
|
||||
*,
|
||||
had_explicit_deletions: bool = False,
|
||||
rebuilt_sources: "set[str] | None" = None,
|
||||
) -> bool:
|
||||
"""Return True (ok to proceed) or False (shrink refused).
|
||||
|
||||
@@ -366,23 +367,43 @@ def _check_shrink(
|
||||
has declared which files were removed (e.g. the post-commit hook saw
|
||||
a ``D`` in ``git diff --name-only``) and a smaller graph is the expected
|
||||
outcome — skip the guard so legitimate refactors don't require ``--force``.
|
||||
|
||||
``rebuilt_sources`` (when given) is the set of source files re-extracted this
|
||||
run. A net shrink is legitimate — not a failed chunk — when every *lost* node
|
||||
belonged to one of those files (a symbol removed from a re-extracted file) or
|
||||
carries no source_file. Only an unexplained loss (a node from a file we did
|
||||
NOT touch — e.g. a dropped semantic/doc node) refuses the write. This lets a
|
||||
plain ``graphify update`` after deleting a function refresh the graph without
|
||||
``--force`` (#1116 left stale nodes write-blocked even though build dropped them).
|
||||
"""
|
||||
if force or not existing_data or had_explicit_deletions:
|
||||
return True
|
||||
existing_n = len(existing_data.get("nodes", []))
|
||||
new_n = len(new_data.get("nodes", []))
|
||||
if new_n < existing_n:
|
||||
if tmp is not None:
|
||||
tmp.unlink(missing_ok=True)
|
||||
print(
|
||||
f"[graphify] WARNING: new graph has {new_n} nodes but existing "
|
||||
f"graph.json has {existing_n}. Refusing to overwrite — you may be "
|
||||
f"missing chunk files from a previous session. "
|
||||
f"Pass --force to override.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return False
|
||||
return True
|
||||
existing_nodes = existing_data.get("nodes", [])
|
||||
new_nodes = new_data.get("nodes", [])
|
||||
if len(new_nodes) >= len(existing_nodes):
|
||||
return True
|
||||
if rebuilt_sources is not None:
|
||||
from graphify.build import _norm_source_file
|
||||
new_ids = {n.get("id") for n in new_nodes}
|
||||
lost = [n for n in existing_nodes if n.get("id") not in new_ids]
|
||||
|
||||
def _accounted(n: dict) -> bool:
|
||||
sf = n.get("source_file")
|
||||
return (not sf
|
||||
or sf in rebuilt_sources
|
||||
or _norm_source_file(sf) in rebuilt_sources)
|
||||
if all(_accounted(n) for n in lost):
|
||||
return True
|
||||
if tmp is not None:
|
||||
tmp.unlink(missing_ok=True)
|
||||
print(
|
||||
f"[graphify] WARNING: new graph has {len(new_nodes)} nodes but existing "
|
||||
f"graph.json has {len(existing_nodes)}. Refusing to overwrite — you may be "
|
||||
f"missing chunk files from a previous session. "
|
||||
f"Pass --force to override.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return False
|
||||
|
||||
|
||||
def _report_for_compare(report_text: str) -> str:
|
||||
@@ -635,6 +656,18 @@ def _rebuild_code(
|
||||
pass # corrupt graph.json - proceed with AST-only
|
||||
|
||||
_relativize_source_files(result, project_root)
|
||||
# Source files re-extracted this run — their symbol sets may legitimately
|
||||
# shrink (a removed function), so the shrink-guard should not block the
|
||||
# write when every lost node belongs to one of them (or a deleted file).
|
||||
_rebuilt_root = str(project_root)
|
||||
if changed_paths is None:
|
||||
rebuilt_sources = {
|
||||
_nsf(str(p.relative_to(project_root)), _rebuilt_root)
|
||||
for p in code_files if p.is_relative_to(project_root)
|
||||
}
|
||||
else:
|
||||
rebuilt_sources = {(_nsf(str(p), _rebuilt_root) or str(p)) for p in extract_targets}
|
||||
rebuilt_sources |= set(deleted_paths)
|
||||
out.mkdir(exist_ok=True)
|
||||
# Write the user-supplied path rather than the resolved absolute form
|
||||
# so a committed ``graphify-out/.graphify_root`` is portable across
|
||||
@@ -670,6 +703,7 @@ def _rebuild_code(
|
||||
if not _check_shrink(
|
||||
force, existing_graph_data, candidate_graph_data,
|
||||
had_explicit_deletions=bool(deleted_paths),
|
||||
rebuilt_sources=rebuilt_sources,
|
||||
):
|
||||
return False
|
||||
existing_graph.write_text(candidate_graph_text, encoding="utf-8")
|
||||
@@ -776,6 +810,7 @@ def _rebuild_code(
|
||||
force, existing_graph_data, candidate_graph_data,
|
||||
tmp=graph_tmp,
|
||||
had_explicit_deletions=bool(deleted_paths),
|
||||
rebuilt_sources=rebuilt_sources,
|
||||
):
|
||||
return False
|
||||
from graphify.export import backup_if_protected as _backup
|
||||
|
||||
+34
-1
@@ -260,7 +260,10 @@ def test_rebuild_code_evicts_removed_symbol_from_surviving_file(tmp_path):
|
||||
# Remove foo() from a.py (keep bar); leave b.py untouched.
|
||||
(corpus / "a.py").write_text("def bar(): pass\n", encoding="utf-8")
|
||||
|
||||
assert _rebuild_code(corpus, acquire_lock=False, force=True) is True
|
||||
# No force=True: a symbol removed from a re-extracted file is a legitimate
|
||||
# shrink, so the shrink-guard must let `graphify update` refresh the graph
|
||||
# without --force (the lost node belongs to a rebuilt source).
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
after_data = json.loads(graph_path.read_text(encoding="utf-8"))
|
||||
after = labels(after_data)
|
||||
|
||||
@@ -550,6 +553,36 @@ def test_check_shrink_allows_no_existing_data():
|
||||
assert ok is True
|
||||
|
||||
|
||||
def test_check_shrink_allows_shrink_within_rebuilt_sources(capsys):
|
||||
"""#1116: a symbol removed from a re-extracted file is a legitimate shrink —
|
||||
every lost node belongs to a rebuilt source, so the write proceeds (no --force)."""
|
||||
existing = {"nodes": [
|
||||
{"id": "a", "source_file": "m.py"},
|
||||
{"id": "b", "source_file": "m.py"},
|
||||
{"id": "c", "source_file": "other.py"},
|
||||
], "links": []}
|
||||
new = {"nodes": [
|
||||
{"id": "a", "source_file": "m.py"},
|
||||
{"id": "c", "source_file": "other.py"},
|
||||
], "links": []}
|
||||
ok = _check_shrink(False, existing, new, rebuilt_sources={"m.py"})
|
||||
assert ok is True
|
||||
assert "Refusing to overwrite" not in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_check_shrink_blocks_shrink_outside_rebuilt_sources(capsys):
|
||||
"""The guard's real job is intact: a node lost from a file we did NOT re-extract
|
||||
(the failed-chunk signal) is still refused even with rebuilt_sources set."""
|
||||
existing = {"nodes": [
|
||||
{"id": "a", "source_file": "m.py"},
|
||||
{"id": "z", "source_file": "untouched.py"},
|
||||
], "links": []}
|
||||
new = {"nodes": [{"id": "a", "source_file": "m.py"}], "links": []}
|
||||
ok = _check_shrink(False, existing, new, rebuilt_sources={"m.py"})
|
||||
assert ok is False
|
||||
assert "Refusing to overwrite" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_check_shrink_allows_growth():
|
||||
"""new > existing is always fine."""
|
||||
ok = _check_shrink(
|
||||
|
||||
Reference in New Issue
Block a user