Repository navigation
fix(explore): a Flow through an interface goes through the implementation the query names - #2453
Merged
Merged
Conversation
…tion the query names Explore's named walk is breadth-first, and the first callee listed claimed each node it reached. An interface method calls every implementation at the same depth, so when several of them call the next named symbol, the Flow went through whichever implementation the index listed first, even when the query named another one. On prometheus, `Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier` went through the TSDB's `DB.Querier`; on gin, once defined types count as implementers, `setter.TrySet` went through `headerSource.TrySet` when the query named `formSource.TrySet`. A node reached again from the same depth now takes the route that ends on fewer unnamed hops, then the one through more named symbols, and keeps the first otherwise. Of two equally deep named ends, the seed keeps the one whose route passes more named symbols. The expansion order, the visited set and the NAMED_VISIT_CAP bound are unchanged, so the walk makes the same callee lookups. Co-Authored-By: Claude Opus 5.5 <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.
Summary
codegraph_explore's Flow section could route a flow through an interface implementation the query never named, while leaving out the one it did name. The viewer's Flow strip uses the same path finder, so it did the same.walkCallsinsrc/graph/named-symbol-flow.tsis a plain BFS. The first callee listed claimed each node (parent.set), and the bridge budget only limits unnamed hops; it never ranks one route against another. An interface method calls all of its implementations at the same depth. So when several of them call the next named symbol, whichever implementation the index listed first won.Repro on current main (gin needs #2419's defined-type implementers, now merged):
mappingByPtr tryToSetValue setter.TrySet formSource.TrySet setByForm: the Flow ransetter.TrySet → headerSource.TrySet (binding/header.go:35) → setByForm. The namedformSource.TrySet(binding/form_mapping.go:75) callssetByFormjust as directly.tryToSetValue setter.TrySet multipartRequest.TrySet setByForm: also went throughheaderSource.Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerierwent through tsdb'sDB.Querier.Change
NAMED_VISIT_CAPbound are unchanged, since replacing a route never adds a node.Validation
Kit: gin, prometheus and etcd, each with three indexes: main 31c3328, #2419's branch, and main 61f8b09 (with #2414, #2416 and #2419) re-indexed with this branch's build (wasm path). The change is query-time only, so both arms read the same index.
Query sets per repo:
Results on main 61f8b09. The other two indexes gave the same picture.
getCalleeslookupsDBtypes, twosampletypes), or the named implementation fell outside the six-candidates-per-token cap (SDConfig.NewDiscoverer,Discovery.refresh). The walk can't prefer what was never named.CODEGRAPH_UI=1): the lead chain matches explore. A dropped alternative is usually a sub-run of the better lead chain, which the existing dedupe removes, or the same route rerouted at the same length. On etcd, 41 alternatives were replaced by equally long chains through more named symbols, for example a route through the unnamedUnimplementedAuthServerstub replaced by one through the named servers. I added no viewer-launch changelog entry because the strip is unreleased; say if you want one.Explore spot-checks, before → after:
mappingByPtr … formSource.TrySet setByForm: header.go:35 → form_mapping.go:75… multipartRequest.TrySet setByForm: header.go:35 → multipart_form_mapping.go:27 (when len(files) == 0)Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier: tsdb/db.go:2617 → storage/fanout.go:75Periodic.Run Compactable.Compact kv.Compact KVClient.Compact: client/v3/retry.go:127 → client/v3/kv.go:197Tests
__tests__/flow-named-implementation.test.tscovers a Go fixture shaped like gin (with structs) and two TypeScript fixtures. Each one is asked twice, naming one implementation and then the other, so it fails whichever one the index lists first. It also covers the bridge when no implementation is named, and checkscodegraph_explore's Flow section end to end. On main's source 4 of 6 fail; with the fix all 6 pass.function-ref's Caller/impact graph misses methods passed as first-class references (callbacks), e.g. executor.submit(obj.method, …) #1820 case hits its own 60 s limit on main's source too. The other five files (mcp-writer-lock,mcp-projectpath-lifecycle,mcp-initialize,mcp-staleness-banner,watcher) pass when run on their own, 127 of 127.src/and for the new test file.Overlap
walkCallsline: it addsisTransportEdge(c.edge) ||to the skip that also heldparent.has(c.node.id). Whichever PR lands second should keep the transport skip and dropparent.has(…):if (isTransportEdge(c.edge) || !FLOW_EDGE_KINDS.has(c.edge.kind)) continue;. Its doc paragraph sits beside the new one in the same comment, so keep both.🤖 Generated with Claude Code