Conversation
![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.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 2 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds a machine-local incremental-state file (cache/incremental-state.json) that stores per-file size, mtime_ns, and indexed_at_ns alongside the content hashes now kept in manifest.json, letting detect_incremental skip hashing via a stat fastpath while still hashing files whose write tick may not have safely closed (_is_racily_clean, tunable through GRAPHIFY_MTIME_GRANULARITY_MS). Drops the old mtime/hash fields from manifest entries, normalizing legacy rows into ast_hash/semantic_hash only, and reworks save_manifest to compute both artifacts in one stat+MD5 pass (_stat_and_hash_for_manifest) and persist state atomically with deterministic, root-relative keys. State keys re-anchor to absolute paths on load and back to forward-slash relative on save, mirroring manifest storage.
Worth a look
- save_manifest sorts manifest keys after relativize but state keys use unrelativized keys, breaking state prune —
graphify/detect.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- save_manifest hashed-entry tuple contract changed —
graphify/detect.py:2373· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- save_manifest no longer writes mtime in manifest rows —
graphify/detect.py:2317· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Incremental state save can clobber concurrent state updates —
graphify/detect.py:2664· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2549 functions depend on the 571 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 645 callers, 45 callees - new:
_rebuild_code()— 129 callers, 54 callees - new:
detect()— 123 callers, 15 callees - new:
save_manifest()— 52 callers, 13 callees - new:
_extract_generic()— 18 callers, 29 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 — 2549 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: 1020 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
53 of 287 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_stable_manifest.py— impact, changed-testtests/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— impact- … and 3 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 detect\_incremental.
The verifier did not have enough to check detect\_incremental, 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: no capturable inputs from the test suite; property tier: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_mtime\_may\_hide\_a\_rewrite (not a proof).
The verifier ran both versions of \_mtime\_may\_hide\_a\_rewrite 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.
No difference found (not proven): No behavior difference found in save\_manifest (not a proof).
The verifier ran both versions of save\_manifest 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; 52 more finding(s) on lines outside this diff (see the check run).
| write_json_atomic(str(state_file), disk_state, indent=2) | ||
|
|
||
|
|
||
| def save_manifest( |
There was a problem hiding this comment.
save_manifest()
fans out to 13 callees (efferent coupling); 52 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
Fixes #3643 by separating portable manifest data from machine-local filesystem state.
Previously,
manifest.jsonpersisted volatilemtimeandseenfields, causing unnecessary changes across updates and checkouts. This change keeps the manifest content-based and moves incremental filesystem metadata into a local cache.Changes
Remove
mtimeandseenfrom persistedmanifest.jsonentries.Add
graphify-out/cache/incremental-state.jsonfor:sizemtime_nsindexed_at_nsKeep
manifest.jsondeterministically ordered and byte-stable.Add atomic, no-op-aware persistence for incremental state.
Use
mtime_nswith a 2-second racily-clean window for the incremental fastpath.Fall back to MD5 content verification when local state is missing, stale, malformed, or racily-clean.
Preserve legacy manifest compatibility and safely normalize old
mtime/seenentries.Preserve the existing
_stat_and_hash()contract.Leave the existing
cache/stat-index.jsoninfrastructure untouched.Add comprehensive regression coverage for manifest stability, cache recovery, rapid rewrites, migration, deletion, exclusion, and corrupted state.
Validation
tests/test_stable_manifest.py— 12 passedtests/test_incremental_mtime_collision.py— 5 passedtests/test_long_path_hashing.py— 4 passedtests/test_detect.py— 257 passed, 1 known environment-specific failuretests/test_serve.py tests/test_build.py tests/test_dedup.py— 342 passedgit diff --check— cleanThe single
test_graphifyignore_hermetic_without_vcsfailure is unrelated to this change and is caused by the host environment containing a.gitdirectory under the user's home directory.