Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Strengthens _GraphContextCache's file-identity key from (mtime_ns, size) to a five-field tuple of (st_dev, st_ino, st_ctime_ns, st_mtime_ns, st_size) via a new _file_key, so an equal-length, mtime-restored atomic replacement is detected instead of served stale. On a cold miss, load now builds the entry privately and re-stats after reading, retrying once if identity changed and raising RuntimeError("graph.json changed while loading") if the file keeps churning — with the old cache entry and LRU order preserved on failure. Adds tests/test_serve_cache_identity.py with in-process contract tests covering the load-time race, bounded churn, LRU eviction, pinned isolation, and parse/absence error mapping.
Worth a look
- File I/O performed while holding the cache lock serializes all loads —
graphify/serve.py:180· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Graph reload can now raise uncaught RuntimeError during file churn —
graphify/serve.py:184· 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 — 586 functions depend on the 199 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 125 callees - new:
_query_graph_text()— 25 callers, 10 callees - new:
_score_query()— 15 callers, 6 callees - new:
_query_terms()— 20 callers, 3 callees - new:
run_benchmark()— 16 callers, 3 callees - new:
_load_graph()— 14 callers, 3 callees - new:
_build_server()— 2 callers, 16 callees - new:
_query_subgraph_tokens()— 7 callers, 3 callees - …and 9 more — each is listed as a finding
Verification — 586 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: 401 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
9 of 287 test file(s) selected (3%) via static blast radius.
tests/test_benchmark.py— impacttests/test_benchmark_raw_graph.py— impacttests/test_file_label_disambiguation.py— impacttests/test_query_induced_edges.py— impacttests/test_query_mcp_direction.py— impacttests/test_query_names_its_graph.py— impacttests/test_serve.py— impacttests/test_serve_cache_identity.py— impact, changed-testtests/test_serve_http.py— impact
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 load.
The verifier did not have enough to check load, 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: not verifiable: all 69 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly __init__ raised on the synthesized receiver args — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_load\_entry.
The verifier did not have enough to check \_load\_entry, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly __init__ raised on the synthesized receiver args — names the real obstacle, not a sampling gap)
· 17 more finding(s) on lines outside this diff (see the check run).
An atomic replacement of
graph.jsoncan keep its byte length and restored mtime, causing a warm_GraphContextCacheto return the old graph. This change includes device, inode and ctime in the cache identity and rechecks that identity after loading. It retries one changed-identity load, then rejects a second race while preserving the previous entry and its LRU state.The patch adds 21 regression tests covering same-size/restored-mtime replacement, cold and warm load races, bounded retries, failed refresh, pinned entries and LRU behavior. It remains a metadata check: identical five-field collisions, hidden in-place rewrites, filesystem writer coordination and replacement after the final stat are outside its guarantee. Learning-sidecar invalidation in #3044 remains separate.
Validation on macOS / CPython 3.14.7 with frozen upstream dependencies:
pytest tests/ -q --tb=short: 5,784 passed, 14 skipped, 0 failures/errors. Existing platform, optional-service and feature-condition skips remain visible.The initial full run exposed an existing backend test's dependence on inherited
GRAPHIFY_GEMINI_MODEL. The unchanged test failed with that override, passed through the existing environment scrub, failed when the override was reintroduced, and passed again after removal. The full passing run used that scrub; no test expectation, source file, dependency pin or skip condition was changed to obtain it.The complete GitHub CI and Python/platform matrix remain for CI to evaluate. This change has only been exercised in an isolated checkout; it has not been adopted into an existing managed installation or attested as loaded by a running server.