Skip to content

fix(watch): resolve the graphify root marker when GRAPHIFY_OUT is shared and absolute - #3735

Closed
ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-3375-graphify-root-marker-not-resolved
Closed

ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-3375-graphify-root-marker-not-resolved

Conversation

@ayushcodes10

Copy link
Copy Markdown
Contributor

Fixes #3375.

_rebuild_code in graphify/watch.py writes the caller-supplied watch_path straight into .graphify_root without resolving it. #777 deliberately chose that over a resolved absolute path so a committed graphify-out/.graphify_root stays portable across clones and CI runners — every normal reader of the marker (the generated git hooks, an unqualified graphify watch) runs with the same CWD it was written from, so a relative value like . is harmless there.

That assumption breaks once GRAPHIFY_OUT is set to an absolute, shared location — the multi-worktree setup from #686. The same marker file then becomes reachable from any worktree's CWD, not just the one that wrote it, so a relative value silently resolves against whichever worktree happens to read it later instead of the one that was actually scanned.

I initially looked at just always resolving the path (matching the two cli.py writers, which already do this for the #2012/#1571 custom --out support), but that would revert #777's fix and break its existing regression test (test_graphify_root_preserves_relative_when_invoked_with_relative_path), losing the git-portability guarantee for the common, single-clone case.

Instead this adds a small helper, _graphify_root_marker_value, used at both of watch.py's .graphify_root writer call sites: it only resolves to an absolute path when GRAPHIFY_OUT is itself absolute, and otherwise preserves the existing relative behavior untouched. cli.py's writers are left as-is since no portability issue has been filed against them.

Two regression tests: one sets a shared absolute GRAPHIFY_OUT and confirms the marker is resolved; a companion confirms the ordinary relative GRAPHIFY_OUT case still preserves the caller-supplied path exactly as the existing #777 test expects. Both were checked to fail/pass correctly against the pre-fix code. Full test_watch.py + test_watch_manifest_location.py (185 tests) and the full suite (5716 passed) are green.

🤖 Generated with Claude Code

https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh

ayushcodes10 and others added 3 commits September 22, 2026 13:21
…lute

Update writes a raw, caller supplied watch path into the graphify_root
marker, which Graphify-Labs#777 deliberately chose over a resolved absolute so a
committed marker file stays portable across clones and CI runners.
That is fine as long as every reader of the marker runs with the same
CWD it was written from, which normally holds since the generated git
hooks and an unqualified graphify watch both assume it.

It stops holding once GRAPHIFY_OUT is set to an absolute, shared
location, the multi worktree setup from Graphify-Labs#686. The same marker file is
then reachable from any worktree's CWD, so a relative value like the
dot from graphify update dot resolves against whichever worktree
happens to read it later instead of the one that was actually
scanned, silently falling back to the wrong scan root.

Add a small helper that only resolves in that shared, absolute case
and otherwise keeps the existing relative behaviour untouched, so the
777 portability guarantee for the common, single clone case is
preserved rather than reverted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
One test sets a shared, absolute GRAPHIFY_OUT and confirms the marker
written by an update from a relative path is resolved to an absolute
form. A companion test confirms the ordinary, relative GRAPHIFY_OUT
case still preserves the caller supplied relative path exactly as the
existing 777 test already expects, so this fix only changes the
shared output case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Resolves the .graphify_root marker to an absolute path when GRAPHIFY_OUT points at an absolute, shared output location, so a shared-output worktree setup no longer records a relative marker that resolves against whichever worktree reads it later. Adds _graphify_root_marker_value to make this decision and routes both marker writes in _rebuild_code through it, still preserving the caller-supplied relative value in the default relative-GRAPHIFY_OUT case so committed markers stay portable across clones.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 848 functions depend on the 654 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 144 callers, 55 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • new: watch() — 5 callers, 7 callees
  • new: _reconcile_graph_html() — 6 callers, 5 callees
  • new: _reconcile_existing_graph() — 1 callers, 8 callees
  • new: _reconcile_markdown_links() — 1 callers, 7 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 848 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: 676 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

290 of 290 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — full-run-safety
  • tests/test_astro_import_ids.py — full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_codebuddy.py — full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — full-run-safety
  • tests/test_cross_language_call_resolution.py — full-run-safety
  • tests/test_cross_repo_external_call_guards.py — full-run-safety
  • tests/test_cross_repo_member_calls.py — full-run-safety
  • tests/test_cross_repo_shared_types.py — full-run-safety
  • tests/test_csharp_call_site_generic_args.py — full-run-safety
  • … and 240 more

non-code file(s) changed (CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

· 7 more finding(s) on lines outside this diff (see the check run).

@safishamsi

Copy link
Copy Markdown
Member

Shipped in v0.9.66 (on PyPI). Cherry-picked with authorship preserved so it shows under your GitHub contributions. Thanks @ayushcodes10!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants