fix: relativize manifest, .graphify_root, and cache source_file fields (#777)

[cornwe19]

Summary

Fixes #777. Three artifacts under graphify-out/ embedded absolute paths from the saving machine, breaking cross-machine portability of a committed graphify-out/:

1. manifest.json keys — absolute paths ⇒ detect_incremental missed on every other checkout, forcing a full rebuild on every CI run.
2. .graphify_root — absolute path ⇒ graphify update (no args) scanned a non-existent path or failed outright after clone.
3. cache/ast/*.json source_file fields — absolute paths inside cached extraction fragments, leaking user paths into committed diffs.

graph.json was already relativized via watch._relativize_source_files (since 0.5.0), proving the pattern. This PR mirrors that approach for the other three artifacts.

Approach

Relativize on persist, re-anchor against root on load. In-memory callers continue to see absolute paths, so internal code is unchanged — notably the AST symbol-prefix remap in extract.py (added by #1033/#1096) that depends on Path(source_file).resolve() to look up its prefix table. Out-of-root entries (symlinked external corpora) keep their absolute form so they round-trip on the saving machine even when they can't be portably encoded.

Changes

• detect.py: save_manifest / load_manifest accept an optional root= kwarg.

  • When provided, write relative posix keys, read them back as absolute.
  • Legacy absolute-keyed manifests pass through unchanged (no migration step needed).
  • detect_incremental forwards its root arg so the fix is on by default for the common path.
  • Callers that don't pass root keep the old behavior — skill-generated subagent scripts (tools/skillgen/expected/**/update.md etc.) that call save_manifest(incremental['files']) remain green.

• cache.py: save_cached serializes a deepcopy with relative source_file fields, leaving the caller's dict in its original absolute shape so downstream pipeline steps (AST prefix remap, graph merge) still see what they expect. load_cached re-anchors relative source_file fields back to absolute on read.

• watch.py: .graphify_root stores the user-supplied watch_path rather than the resolved absolute, so a committed graphify-out/.graphify_root with . works in any clone where the caller's CWD is the project root. Absolute caller paths are preserved as-is. The three save_manifest calls pass root=project_root.

• __main__.py: extract's two _save_manifest calls pass root=target.

Why deepcopy in save_cached?

I initially mutated the caller's result dict in-place to relativize source_file fields. Two existing tests then failed (test_rebuild_code_is_idempotent_when_cluster_ids_flap, test_rebuild_code_skips_cluster_when_topology_unchanged). The cause: the symbol-prefix remap at extract.py:10885 calls Path(sf).resolve() to look up the prefix table. With my mutation, that lookup got Path("app.py").resolve() which resolved against CWD instead of the project root, missing the table → IDs stayed in their absolute-prefixed form → topology comparison saw a fake change on the second build → infinite-rebuild loop.

deepcopy adds bounded overhead (one file's worth of nodes/edges per cache entry; typically a few KB, occasionally a few MB) and avoids the trap of leaking persistence concerns into the in-memory shape that other pipeline stages depend on.

Tests

12 new tests:

• test_save_manifest_relativizes_keys_when_root_given
• test_save_manifest_without_root_keeps_absolute_keys (back-compat)
• test_load_manifest_absolutizes_relative_keys
• test_load_manifest_passes_through_legacy_absolute_keys (back-compat)
• test_save_manifest_out_of_root_keeps_absolute
• test_detect_incremental_portable_across_paths (end-to-end)
• test_save_cached_relativizes_source_file
• test_load_cached_absolutizes_source_file
• test_load_cached_passes_through_legacy_absolute_source_file (back-compat)
• test_cache_portable_across_roots (end-to-end)
• test_graphify_root_preserves_relative_when_invoked_with_relative_path
• test_graphify_root_preserves_absolute_when_user_supplied

Full test suite: 1676 passed, 135 skipped (platform-dependent), 1 deselected (wheel build test needs pip install build), 0 failures attributable to this change. Two test failures unrelated to this PR appear when boto3 / AWS credentials are present in the env (test_ollama.py::test_detect_backend_*) — they assume no backend is configured. Both pass in a stripped env.

Backwards compatibility

• Legacy absolute-keyed manifests still load — the absolutize step is a no-op for already-absolute keys.
• Legacy cache entries with absolute source_file still load — same pattern.
• Callers that don't pass root to save_manifest / load_manifest keep their existing behavior.
• Migration to the new on-disk format happens automatically on the next save by any caller that does pass root (this PR updates all internal callers in __main__.py and watch.py).

Test plan

• pytest tests/test_detect.py tests/test_cache.py tests/test_watch.py — 161 passed, 2 skipped
• Full pytest suite — 1676 passed, 0 unrelated failures
• Manual: clone, graphify update ., verify manifest.json keys are relative posix paths
• Manual: clone same graphify-out/ to a different absolute prefix, run graphify update ., verify detect_incremental reports zero new files (cache portable)

Related: #722 (manifest absolute paths — original question issue), #952 (AST ID collisions — separate concern, not addressed here).

🤖 Generated with Claude Code

[safishamsi]

Thanks for this — the design is solid and the deepcopy rationale is well-documented. The relativize/absolutize round-trip, migration path, and back-compat behaviour all check out. Merge-ready modulo one issue below.

---

Minor issue: _to_relative_for_storage over-resolves symlinked files

In detect.py, _to_relative_for_storage calls .resolve() on the file key before relative_to(root):

return p.resolve().relative_to(Path(root).resolve()).as_posix()

detect() emits unresolved keys (from os.walk), so a symlinked file inside the root — e.g. alias.py → sub/target.py — gets stored under key sub/target.py instead of alias.py. On reload, alias.py is not found in the manifest → it gets re-extracted on every incremental run. Not a corruption risk, but a redundant full-extract-per-file regression for any repo that uses in-root symlinks.

The fix is to relativize without resolving the file:

try:
    return str(Path(os.path.relpath(p, Path(root).resolve()))).replace(os.sep, "/")
except (ValueError, OSError):
    return key  # out-of-root or Windows cross-drive

Same issue exists in _relativize_source_files_in in cache.py (lower-impact there since cache lookup is content-hashed, not key-matched, but worth fixing for consistency).

Ideally also add a regression test:

def test_save_manifest_in_root_symlink_roundtrips(tmp_path):
    target = tmp_path / "sub" / "target.py"
    target.parent.mkdir()
    target.write_text("x")
    link = tmp_path / "alias.py"
    link.symlink_to(target)
    manifest = {str(link): {"mtime": 1.0, "size": 1}}
    save_manifest(manifest, root=tmp_path)
    loaded = load_manifest(root=tmp_path)
    assert str(link.resolve()) in loaded or str(link) in loaded

Everything else looks good — happy to merge once this is addressed.

[cornwe19]

Addressed in 8f09326.

Switched _to_relative_for_storage (detect.py) and _relativize_source_files_in (cache.py) to os.path.relpath against a resolved-root only — the key itself stays symbolic, so in-root symlinks now round-trip under their own name. Preserved the prior out-of-root semantics by falling back to the absolute key when the computed relative path escapes root (.. prefix).

Two regression tests added:

• test_save_manifest_in_root_symlink_roundtrips — confirms alias.py (symlink to sub/target.py) stores under alias.py, not sub/target.py, and reloads to the symlink's absolute path.
• test_save_cached_in_root_symlink_keeps_symlink_name — same shape for cache source_file.

Both skip cleanly on filesystems that don't support symlinks. Full suite: 1678 passed, same two pre-existing AWS-creds-in-env failures (test_ollama.py::test_detect_backend_*) noted in the original PR description, no new failures.