fix(watch): write manifest.json to the target's graphify-out (#2316) - #2345
fix(watch): write manifest.json to the target's graphify-out (#2316)#2345Rishet11 wants to merge 2 commits into
Conversation
…y-Labs#2316) _rebuild_code computed the correct output dir as `out = watch_path / _GRAPHIFY_OUT` and routed every artifact through it except the three save_manifest calls, which omitted manifest_path= and fell back to detect._MANIFEST_PATH — a module-import-time constant built from the relative out_path("graphify-out/manifest.json"), so it resolved against the process CWD. `graphify update <target>` therefore wrote the target's manifest into the invoking project and left the target with none. save_manifest also read-modify-writes that path and full-scan callers pass scan_corpus (Graphify-Labs#1908 pruning), so the invoking project's own rows were pruned as out-of-corpus and overwritten. Both projects lost their incremental baseline, not just the target. Anchor the manifest keys to watch_root at the same three calls. root= previously followed project_root, which is CWD-derived for a relative invocation, so `graphify update ../other` stored out-of-root files under absolute keys and broke Graphify-Labs#777/Graphify-Labs#1964 portability. project_root itself is left alone: the persisted graph deliberately rehomes source_file against it across invocation styles. Also pass cwd= to _git_head. Without it `git rev-parse HEAD` inherited the caller's directory, stamping the invoking repo's commit into the target's graph.json built_at_commit (cf. Graphify-Labs#2081).
…raphify-Labs#2316) Seven tests, all red on 4fe1109 and green with the fix: - location, parametrized over the clustered and --no-cluster write sites - the unchanged-topology early return, the third write site; asserts the "No code-graph topology changes detected" line so the test cannot silently drift onto a different branch and keep passing - the invoking project's own manifest survives an update of another project (the data-loss half) - a relative target stores relative keys - the resulting manifest is actually consumable by detect_incremental - built_at_commit matches the target repo's HEAD, not the caller's
There was a problem hiding this comment.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR changes _rebuild_code in graphify/watch.py so that manifest writes and git-HEAD lookups are anchored to the watch/target directory rather than the process CWD: the three save_manifest calls now pass an explicit manifest_path and root=watch_root, and _git_head gains a cwd parameter that is passed the target's watch_root. It also adds a new test file tests/test_watch_manifest_location.py covering where the manifest lands, non-destruction of the CWD project's manifest, relative-key portability, and the commit stamped into the target graph. The surface area is the manifest/provenance handling for graphify update <target> when invoked from a different directory. Reviewers should confirm the intended anchoring behavior across the clustered, --no-cluster, and unchanged-topology early-return paths.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 283 functions depend on the 77 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
_rebuild_code()— 62 callers, 48 callees
Verification — 283 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: 157 function(s) in the blast radius were not formally verified this run
· 1 more finding(s) on lines outside this diff (see the check run).
Closes #2316
graphify update <target>wrotemanifest.jsoninto the process CWD'sgraphify-out/while every other artifact went to the target. Reproduced onv84fe1109(0.9.31), and the reproduction turned up two more defects in the same function that the report does not cover. All three are fixed here;graphify/watch.pyis the only file touched.Root cause
_rebuild_codecomputes the correct output dir atwatch.py:897:and routes every artifact through it, including one line above each manifest write (
out / ".graphify_root",out / "needs_update"). The threesave_manifest(...)calls omittedmanifest_path=, so they fell back todetect._MANIFEST_PATH(detect.py:29) — a module-import-time constant built from the relativeout_path("manifest.json")(paths.py:288), resolved against the process CWD.cli.py:2815is the reference-correct wiring and passes it explicitly at2853/3404/3510/3657.Three defects
1. Location — as reported. The reporter's line numbers (970/1009/1087, from 0.9.13) have drifted to 1232/1275/1421 at
4fe1109.2. It is data loss on the invoking project, not just a stray file.
save_manifestread-modify-writes the same path (detect.py:1615) and full-scan callers passscan_corpus(#1908 pruning), so the invoking project's own rows are pruned as out-of-corpus and overwritten:Both projects then need a full rescan, not only the target.
3. A relative target stored absolute keys.
root=followedproject_root, which is CWD-derived for a relative invocation, sographify update ../projBput the target's files out-of-root and_to_relative_for_storagekept them absolute — breaking the #777/#1964 portability guarantee even once the file is in the right directory:Fixed by passing
root=watch_rootat those three calls.project_rootitself is deliberately left alone — the persisted graph rehomessource_fileagainst it when the user switches between absolute and relative invocation styles (tests/test_watch.py:1389,:1428). The manifest and the persisted graph are different artifacts with legitimately different anchors; the manifest's consumer isdetect_incremental(root=target).4.
built_at_commitcame from the wrong repo._git_head()rangit rev-parse HEADwith nocwd=, so it read the caller's repository:This is #2081 in the watch path — wrong provenance data rather than a misplaced file. One
cwd=parameter.Before / after, the reporter's exact repro
projA/graphify-out/manifest.json, containing projB's filesprojApre-existing manifestprojB/graphify-out/manifest.jsonb_file.py/private/tmp/.../projB/b_file.pyb_file.pyprojBbuilt_at_commitVerification
tests/test_watch_manifest_location.py, 7 tests, all 7 red at4fe1109and green with the fix (verified by reverting onlywatch.py). They cover all three write sites: clustered,--no-cluster, and the unchanged-topology early return. The early-return test asserts on the"No code-graph topology changes detected"line so it cannot silently drift onto another branch and keep passing.-p no:randomly: base 4 failed / 3895 passed / 3 skipped → 4 failed / 3902 passed / 3 skipped, failure-set diff empty. The 4 are pre-existing backend-detection tests that read real env vars.ruff check .clean.tools.skillgen --check(134 artifacts),--audit-coverage,--schema-singletonall clean.Known gaps, stated openly
except Exception: pass, so a manifest write that fails for any other reason is still silent. The reporter raises this; it is a different change and I left it alone rather than widen the diff. Happy to add a log line here if you want it.tools/skillgen/fragments/references/shared/update.md:163has the same missingmanifest_pathin the agent runbook (save_manifest(..., root='INPUT_PATH')). Same bug class, different artifact, and it needs regenerated skill markdown — not folded in here.export.py:173has its own_git_head()with the same no-cwd=shape, used by theextractpath. Untouched._report_root_label(watch.py:268) falls back toPath.cwd().namefor a relative path. Cosmetic label only, left as is.