fix(extract): drop internal origin_file so it stops leaking into graph.json#1516
Closed
TPAteeq wants to merge 1 commit into
Closed
fix(extract): drop internal origin_file so it stops leaking into graph.json#1516TPAteeq wants to merge 1 commit into
TPAteeq wants to merge 1 commit into
Conversation
…h.json Graphify-Labs#1462 tags each sourceless cross-file stub with an internal origin_file field (the referencing file's absolute path) so _disambiguate_colliding_node_ids can keep same-named cross-file references distinct. The colliding-id pass is its only consumer, but the field was never dropped, so it shipped into graph.json verbatim as an absolute, machine-specific path -- violating the same "no absolute paths in output" contract that already relativizes source_file (Graphify-Labs#555, Graphify-Labs#932). No exporter caught it: to_json passes node fields through unchanged, and to_graphml / the graph-db pushers strip only _-prefixed keys, which origin_file is not. Drop origin_file from the combined node list after the colliding-id pass consumes it, right before the _origin tagging loop. One strip at the source covers every export surface and the cache-hit path and cannot re-enter via build_from_json. The per-file AST cache keeps its own copy (written before the combine), which is what disambiguation reads on a cache hit, so cross-file stubs stay distinct. _origin is deliberately left in place: the incremental watcher reads it from the persisted graph.json to evict stale AST nodes (Graphify-Labs#1116), and it is the constant "ast", not a path. Adds a regression test that fails pre-fix (origin_file present with an absolute path) and passes post-fix, while asserting the two cross-file stubs stay distinct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
safishamsi
pushed a commit
that referenced
this pull request
Jun 28, 2026
…h.json (#1516) The origin_file disambiguation field (#1462) is internal — it's consumed by the colliding-id pass and must not be persisted. In 0.9.0 it shipped into graph.json as an absolute, machine-specific path (the same portability bug class as #555, #932), breaking on clone/move. It's now popped at the single extract() output chokepoint, AFTER _disambiguate_colliding_node_ids has consumed it, so all persist paths (clustered, --no-cluster, cluster-only) emit clean output. _origin is kept (the incremental watcher relies on it, #1116). Ported from PR #1516 by @TPAteeq. Verified: fresh extract (clustered and --no-cluster) produces graph.json with zero origin_file fields; the #1462 same- label disambiguation still works (it runs before the strip). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
safishamsi
pushed a commit
that referenced
this pull request
Jun 28, 2026
…d extractors (#1515) The #1462 same-label cross-file stub disambiguation (the origin_file key) only existed in the generic extractor, so the six dedicated extractors — Julia, Fortran, Go, Rust, PowerShell, ObjC — still collapsed same-named imported-type stubs from different files into one conflated bare-id node (a false cross-package link). Each now sets origin_file on its sourceless stub, identical to the generic extractor; the generic _node_disambiguation_source_key consumes it, so two files importing the same type stay distinct while source_file stays empty (the #1402 rewire onto a real definition is unchanged). Ported from PR #1515 by @TPAteeq. Must ship with #1516: this widens origin_file to six more languages, and #1516 is what strips it from graph.json. Verified: a 2-file Go corpus now yields 2 distinct Widget stubs AND graph.json carries no origin_file. Resolved a test-insertion conflict with #1516 by keeping both tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
Thanks @TPAteeq — confirmed: |
safishamsi
added a commit
that referenced
this pull request
Jun 28, 2026
Patch over 0.9.0: completes the node-ID work (fully closes #1504 via injective salt #1522), stops origin_file leaking into graph.json (#1516), extends cross-file stub disambiguation to the six dedicated extractors (#1515), Java type-param skip (#1518) + record component refs (#1519), prunes a deleted import's edge on update (#1521), and retries rate-limited (429) requests instead of dropping chunks (#1523). All non-breaking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1462.
ensure_named_node's cross-file fallback tags each sourceless stubwith an internal
origin_filefield — the referencing file's absolute path — so_disambiguate_colliding_node_idscan keep same-named cross-file references distinct.That field is consumed only by the colliding-id pass during extraction, but it was never
dropped, so it shipped into
graph.jsonverbatim as an absolute, machine-specific path,violating the "no absolute paths in output" contract that already relativizes
source_file(#555, #932). No exporter caught it:
to_jsonpasses node fields through unchanged, andto_graphml(and the Neo4j/FalkorDB pushers) strip only_-prefixed keys —origin_filehas no underscore.
Change
Drop
origin_filefrom the combined node list after the colliding-id pass consumes it,right before the
_origintagging loop inextract()(one strip at the source — coversevery export surface and the cache-hit path; cannot re-enter via
build_from_json).Why
_originis left aloneThe original observation also named
_origin. It is intentionally kept: theincremental watcher reads
n.get("_origin") == "ast"from the persistedgraph.jsontoevict stale AST nodes on a full rebuild (#1116,
watch.py). Stripping it would reintroducethat bug. It is the constant
"ast"— not a path — so it is not an absolute-path leak.Result
origin_file(abs path)/Users/you/proj/pkg/a.pysource_filedistinctness_origin"ast""ast"(unchanged)The colliding-id pass runs before the strip, so cross-file stubs stay distinct; the
per-file AST cache keeps its own
origin_filecopy for the cache-hit path.Tests
test_origin_file_is_not_serialized_into_extract_output: asserts no node carriesorigin_file(or any absolute sandbox path) while the two cross-filePathstubs staydistinct. Fails pre-fix, passes post-fix.
Out of scope
origin_fileas an absolute path(
cache.save_cachedrelativizessource_fileper bug: absolute paths in manifest.json, .graphify_root, and cache/ast/*.json break git-shared graphify-out/ across machines #777 but notorigin_file). The cacheis a private, content-hashed, version-namespaced artifact that never ships, and the strip
above keeps it out of
graph.jsonregardless — so this is a separate portability/hygienefollow-up, not part of this fix.
referencing path; under a fully-resolved scan root that slug is root-relative. This is
Bug: AST extractor produces duplicate node IDs for same-name imported symbols (collision, not fragmentation) #1462's existing disambiguation behavior, unchanged here.