fix(cli): clean build intermediates with pathlib instead of rm - #2797
fix(cli): clean build intermediates with pathlib instead of rm#2797hudsonwa wants to merge 1 commit into
Conversation
…ify-Labs#2790) Permission-gated hosts deny `rm` and `find -delete`, so Step 9 left stale .graphify_chunk_*.json files that the next --update would merge. `graphify clean` unlinks the same files in Python.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
This PR replaces the shell-based cleanup of Step 9 build intermediates (rm/find -delete) with a Python/pathlib implementation, motivated by permission-gated hosts that deny those shell verbs. It adds a cleanup_build_intermediates helper in paths.py, a new clean [dir] CLI subcommand (with help text and dispatch handling), updates skill.md to call graphify clean instead of the shell commands, and adds unit tests covering the new cleanup function. The surface area touches graphify/__main__.py (help output), graphify/cli.py (command dispatch), graphify/paths.py (new constant and function), graphify/skill.md (documentation), and a new tests/test_cleanup_intermediates.py.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1534 functions depend on the 116 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 463 callers, 41 callees - new:
_rebuild_code()— 95 callers, 51 callees - new:
build_from_json()— 151 callers, 18 callees - new:
build_merge()— 41 callers, 14 callees - new:
save_manifest()— 34 callers, 11 callees - new:
to_obsidian()— 29 callers, 12 callees - new:
to_wiki()— 41 callers, 7 callees - new:
to_json()— 44 callers, 6 callees - …and 38 more
Verification — 1534 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1197 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).
The verifier ran both versions of \_run\_cli on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 45 more finding(s) on lines outside this diff (see the check run).
| @@ -803,6 +803,19 @@ def _reenter_main() -> None: | |||
|
|
|||
|
|
|||
| def dispatch_command(cmd: str) -> None: | |||
There was a problem hiding this comment.
dispatch_command()
fans out to 118 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Heads up on the skillgen-check failure @hudsonwa — Worth knowing while you're in there: the same One thing to watch on the windows render: gen.py's POSIX->PowerShell translator is strict and raises on any bash line it doesn't recognise, so it's worth checking I'd been working on the same issue from the fragments side and have it passing all five validators — happy to hand that over or close mine in favour of your |
|
Thank you for this, and I'm sorry I walked across work you already had going. I hadn't realised Please don't close #2801 on my account. If the maintainers prefer yours, I'll happily close this. Thank you for offering either way. |
2 participants
Summary
Step 9 told the agent to
rm/find -deleteintermediates. On permission-gated hosts those verbs are denied, so.graphify_chunk_*.jsonsurvived and the next--updatecould merge stale chunks.Cleanup now runs in Python (
Path.unlink), same as the rest of the pipeline.Related
Fixes #2790
Type
Changes
graphify/paths.py:cleanup_build_intermediates()graphify clean [dir]CLIskill.mdStep 9 (and the mid-pipeline temp cleanup) callgraphify clean graphify-outHow to Test
3 passed. Does not delete
graph.json. Does not walk into subdirs.Notes
Did not regenerate every
skill-*.mdhost file in this PR — source of truth isskill.md(what most installs copy). Happy to run skillgen in a follow-up if you want the platform clones updated in the same change.