fix(build): make definition_file portable, like its sibling source_file (#3223) - #3243
fix(build): make definition_file portable, like its sibling source_file (#3223)#3243abhay-codes07 wants to merge 1 commit into
Conversation
…le (Graphify-Labs#3223) The Graphify-Labs#2990 decl/def merge stamps definition_file from the implementation's then-absolute source_file, and nothing ever relativized it: graph.json shipped "source_file": "src/Foo.h" beside "definition_file": "/home/ci/build/.../src/Foo.cpp" - a path no other machine can open, leaking the build host's layout into every consumer including MCP get_node's "Defined in:" line. definition_file now goes through the same _norm_source_file normalization as source_file at both build sites (fresh nodes, and loaded-graph attrs - which also heals a graph written before this fix on its next rebuild), and the cache portability round-trip relativizes and re-anchors it beside source_file. Out-of-root paths keep their absolute form, exactly as source_file does.
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 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Makes definition_file a portable path everywhere source_file already is, so the decl/def merge (#2990) no longer leaks the build host's absolute layout into graph.json and MCP get_node. build_from_json relativizes it against the root when adding both nodes and edge attrs, healing already-written graphs on rebuild, and _relativize_source_files_in/_absolutize_source_files_in round-trip it through the cache; out-of-root paths stay absolute as before. Adds tests/test_definition_file_portability.py covering the repro, prebuilt-absolute normalization, out-of-root passthrough, and cache round-trip.
Worth a look
- build_from_json now changes definition_file from absolute to root-relative —
graphify/build.py:979· 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 — 2253 functions depend on the 171 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 522 callers, 43 callees - new:
_rebuild_code()— 113 callers, 50 callees - new:
build_from_json()— 196 callers, 18 callees - new:
detect()— 108 callers, 15 callees - new:
build_merge()— 62 callers, 13 callees - new:
save_semantic_cache()— 58 callers, 9 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
extract_files_direct()— 17 callers, 20 callees - …and 58 more — each is listed as a finding
Verification — 2253 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: 1568 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, 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: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_absolutize\_source\_files\_in.
The verifier did not have enough to check \_absolutize\_source\_files\_in, 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
Could not verify: Could not verify \_relativize\_source\_files\_in.
The verifier did not have enough to check \_relativize\_source\_files\_in, 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
· 66 more finding(s) on lines outside this diff (see the check run).
Closes #3223.
The problem
The #2990 decl/def merge stamps
definition_filefrom the implementation's then-absolutesource_file— and nothing ever relativized it.graph.jsonshipped"source_file": "src/Foo.h"beside"definition_file": "/home/ci/build/.../src/Foo.cpp": a path no other machine or checkout can open, leaking the build host's directory layout into every consumer — including MCPget_node'sDefined in:line, which is exactly where an agent reads it.The change
definition_filenow takes the same_norm_source_filenormalization assource_file, at both build sites — fresh extraction nodes, and loaded-graph attributes, so a graph written before this fix heals on its next rebuild. The cache portability round-trip relativizes and re-anchors it besidesource_filein both directions. Out-of-root paths keep their absolute form (separators normalized), exactly assource_file's contract has always been.Tests
tests/test_definition_file_portability.py— 4 tests: the issue's ownFoo.h/Foo.cpprepro through real extraction yieldsdefinition_file: "src/Foo.cpp"besidesource_file: "src/Foo.h"; a prebuilt absolute value is normalized at build; an out-of-root value keeps its absolute form; and the cache relativize/absolutize round-trip keeps it portable both ways. With the fix reverted, 3 of 4 fail.test_build,test_cache,test_stat_index_portabilityand the C/C++ suites are unchanged (201 passed); the full suite matches the freshv8(0.9.53) baseline.