From 533859dc51976b9c050754c62d2a5fe6177bb6c3 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Wed, 24 Jun 2026 11:29:08 +0100 Subject: [PATCH] fix(update): file-aware shrink-guard so removed symbols prune without --force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- CHANGELOG.md | 1 + graphify/watch.py | 63 +++++++++++++++++++++++++++++++++++---------- tests/test_watch.py | 35 ++++++++++++++++++++++++- 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b66812b..a7dbe18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/graphify/watch.py b/graphify/watch.py index 1b7fadb..046857b 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -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 diff --git a/tests/test_watch.py b/tests/test_watch.py index d1fcfb1..f1b8451 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -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(