Conversation
``_to_relative_for_storage`` compares text. A stored absolute key naming a file under ``root`` through a symlinked checkout path, or with different case on a case-insensitive filesystem, relativizes to ``../...`` and is kept absolute for ever. The next save writes the fresh relative key beside it, so the manifest describes the same file twice and the stale row never expires. When the lexical answer says out-of-root, ask the filesystem before accepting it: walk the key's ancestors for the one that ``os.path.samefile`` matches the resolved root, and join the remainder with its original casing. Keys with no such ancestor still round-trip absolute, and the lexical fast path is unchanged, so in-root keys pay nothing.
![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. No changes could be formally verified in this run.
Graphify review — findings
Fixes manifest key relativization so an absolute key that spells root differently — a case variant on Windows/APFS or a symlinked checkout — is recognized as in-root by filesystem identity rather than lexical text, preventing an upgrade from leaving a stale absolute key beside its new relative twin (#3581). Adds _anchor_under_root, which walks the key's ancestors for one that os.path.samefile-matches the resolved root and rejoins the remainder with original casing; it runs only after the lexical relpath attempt yields ../, returns None for genuinely out-of-root paths or any ancestor it can't stat, and leaves those keys absolute as before.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2496 functions depend on the 538 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 637 callers, 45 callees - new:
_rebuild_code()— 123 callers, 53 callees - new:
detect()— 112 callers, 15 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
save_manifest()— 42 callers, 11 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 45 more — each is listed as a finding
Verification — 2496 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: 984 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
52 of 283 test file(s) selected (18%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_atomic_writes.py— impacttests/test_build.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_claude_cli_backend.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_detect.py— impact, changed-testtests/test_dotnet.py— impacttests/test_evidence_binding.py— impacttests/test_extract.py— impacttests/test_extract_cli.py— impacttests/test_file_slice.py— impacttests/test_ignore_file_encoding.py— impacttests/test_image_vision.py— impacttests/test_import_extension_resolution.py— impacttests/test_incremental_mtime_collision.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_llm_backends.py— impacttests/test_long_path_hashing.py— impacttests/test_manifest_ingest.py— impacttests/test_mcp_ingest.py— impacttests/test_multilang.py— impacttests/test_non_regular_files.py— impacttests/test_office_incremental.py— impacttests/test_office_limits.py— impacttests/test_ollama.py— impacttests/test_out_dir_evidence.py— impacttests/test_oversized_document_slicing.py— impacttests/test_package_json_subpath_imports.py— impacttests/test_pdf_slicing.py— impacttests/test_pdf_token_estimate.py— impacttests/test_phantom_external_import.py— impacttests/test_pipeline.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_stale_prune.py— impacttests/test_swift_computed_properties.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/test_unmapped_at_alias_resolution.py— impacttests/test_vue_extraction.py— impact- … and 2 more
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.
Formal verification
Could not verify: Could not verify \_to\_relative\_for\_storage.
The verifier did not have enough to check \_to\_relative\_for\_storage, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
· 53 more finding(s) on lines outside this diff (see the check run).
…Linux The case-sensitive branch asserted the variant key stays absolute, but on a case-sensitive filesystem that path does not exist at all, so save_manifest's existing "drop rows whose file is gone" prune removed the row before anything relativized it. The branch was measuring that prune rather than the anchoring decision, and failed on every CI job. Create the variant file when the probe says case-sensitive, so what the branch measures is that `os.path.samefile` refuses to anchor a genuinely different file. Verified against the same shape locally with two distinct directory names: the fresh key relativizes, the other keeps its absolute key.
1 participant
What
_to_relative_for_storagedecides in-root vs out-of-root withos.path.relpath, which compares text. A stored absolute key can name a file that is underrootwhile spelling it differently:~/work -> ~/checkouts/repo, or/tmp -> /private/tmpon macOS)relpathanswers../…, the key is kept absolute, and the next save writes the fresh relative key beside it. The manifest then holds two keys for one file, and the stale one never expires: it is not pruned (the file exists), and it is relativized to../…again on every future save.Measured on
v8(symlinked checkout,save_manifest(..., root=real, scan_corpus=…)):How
Only when the lexical answer is
../…, ask the filesystem instead of accepting it: walk the key's ancestors for the one thatos.path.samefilematches the resolved root, and join the remainder with its original casing preserved. No ancestor matches means genuinely out-of-root, and the key round-trips absolute exactly as before.The lexical branch is untouched and still runs first, so an in-root key costs one string operation as it does today; the ancestor walk only happens on the branch that previously gave up.
samefileis the right primitive here because it is platform-neutral:os.path.normcaseis a no-op off Windows, so a case-based fix could be verified neither locally nor by this repo's Ubuntu CI, whilesamefileresolves the symlinked form on every platform and the case-variant form on every case-insensitive one.Tests
tests/test_detect.py:test_save_manifest_relativizes_key_reached_through_symlinked_root— end-to-end, deterministic on every filesystem including CI: the legacy key collapses onto the fresh relative key instead of duplicating it.test_save_manifest_legacy_case_variant_key_does_not_duplicate— the case shape. Asserts the collapse on a case-insensitive filesystem and, on a case-sensitive one, that both keys survive because there the two paths really are different files. No skip, so the test says something on both.test_anchor_under_root_returns_none_when_no_ancestor_is_root— the negative side, unit-level.save_manifestprunes rows whose file no longer exists before relativizing, so the unresolvable out-of-root shape cannot be reached end-to-end and is pinned here.test_save_manifest_out_of_root_keeps_absoluteandtest_save_manifest_full_scan_keeps_out_of_root_rowsalready pin that real out-of-root rows are neither relativized nor pruned; both still pass.Mutation battery over the new branch, 5 of 7 killed:
samefilestats both sides, so the two spellings are equivalent; resolving once up-front is kept because the value is already computed forrelpathstatper ancestor per fileFull suite: 5350 passed, 25 failed — the same 25 failures
origin/v8produces on this machine (missing optionalterraform/ollamaextras), unchanged by this branch.Relation to #3581
This is not the shape reported in #3581 and does not close it. Reproducing that report separates cleanly into two cases, both measured on
v8:In the reporter's re-clone case the old checkout still exists on disk, so the stale rows survive the
Path(f).exists()prune and are then protected by the_in_root(f)guard in the seed loop, which deliberately never prunes out-of-root rows (pinned bytest_save_manifest_full_scan_keeps_out_of_root_rows). Changing that is a semantics decision about--include/symlinked corpora, so I have left it alone and written the measurements up on the issue instead.What this PR fixes is the adjacent case that has no such trade-off: a key that names a file which really is inside
root, only spelled differently. There the duplicate row is unambiguously wrong.