Skip to content

fix(explore): a Flow through an interface goes through the implementation the query names - #2453

Merged
colbymchenry merged 2 commits into
mainfrom
claude/fervent-noether-764445
Oct 7, 2026
Merged

colbymchenry merged 2 commits into
mainfrom
claude/fervent-noether-764445

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

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.

walkCalls in src/graph/named-symbol-flow.ts is 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):

  • gin mappingByPtr tryToSetValue setter.TrySet formSource.TrySet setByForm: the Flow ran setter.TrySet → headerSource.TrySet (binding/header.go:35) → setByForm. The named formSource.TrySet (binding/form_mapping.go:75) calls setByForm just as directly.
  • gin tryToSetValue setter.TrySet multipartRequest.TrySet setByForm: also went through headerSource.
  • Struct implementations hit it too. prometheus Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier went through tsdb's DB.Querier.

Change

  • Walk tie-break. When another node at the same depth reaches a node again, the new route wins if it ends on fewer unnamed hops, or failing that, if it passes more named symbols. A tie keeps the first route, as before. A node's same-depth rivals are all expanded before the node itself is, so nothing has been reached through it yet when its route changes. The visit order, the visited set and the NAMED_VISIT_CAP bound are unchanged, since replacing a route never adds a node.
  • Deepest-end pick. Each seed still keeps its deepest named end. Of two equally deep ends, it now keeps the one whose route passes more named symbols; before, it kept whichever was reached first. Without this, a query that names one implementation and two ends, one per implementation, still got the first-listed implementation's end.
  • Why fewer unnamed hops ranks first. It keeps the tie-break from spending bridge budget a later hop needs. With the default budget of one, only a route node's streak can differ, so in practice the named count decides.

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:

  • targeted: a caller of an interface method, the interface method, one of its implementations, and a callee that two or more implementations call (gin 6, prometheus 221, etcd 185).
  • random: 600 random call chains over static and synthesized edges, naming both ends plus a random subset of the middle.

Results on main 61f8b09. The other two indexes gave the same picture.

gin prometheus etcd
targeted: lead chain passes the named implementation, before → after 2 → 6 of 6 76 → 215 of 221 77 → 184 of 185
random: lead chains that changed 13 / 600 20 / 600 25 / 600
changed lead chains that changed length 0 0 0
changed lead chains with fewer named symbols 0 0 0
queries with identical getCallees lookups 600 / 600 600 / 600 600 / 600
  • Every changed lead chain kept its length and gained named symbols, and repeated runs gave identical results.
  • The 7 targeted misses left are token resolution, and both arms give the same answer for them. Either the token matched several nodes that are all named (two DB types, two sample types), 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.
  • The walk change alone reaches 171 of 185 (etcd) and 211 of 221 (prometheus). The deepest-end pick brings these to 184 and 215.
  • Timing: every query makes the same callee lookups. Totals per round were within −12% to +5% of main, which is this box's round-to-round noise (it ran at 100% CPU).
  • Viewer, 3 chains (unreleased, behind 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 unnamed UnimplementedAuthServer stub 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:

  • gin mappingByPtr … formSource.TrySet setByForm: header.go:35 → form_mapping.go:75
  • gin … multipartRequest.TrySet setByForm: header.go:35 → multipart_form_mapping.go:27 (when len(files) == 0)
  • prometheus Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier: tsdb/db.go:2617 → storage/fanout.go:75
  • etcd Periodic.Run Compactable.Compact kv.Compact KVClient.Compact: client/v3/retry.go:127 → client/v3/kv.go:197

Tests

  • New __tests__/flow-named-implementation.test.ts covers 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 checks codegraph_explore's Flow section end to end. On main's source 4 of 6 fail; with the fix all 6 pass.
  • The explore-*, flow-*, ui-flow-* and adaptive-explore suites (39 files, 472 tests) pass.
  • Full suite on the rebased tree (box at 100% CPU): the parallel run failed 122 tests in 42 files, 117 of them timeouts. A serial rerun of those 42 files with 120 s timeouts left 9 failures in 6 files. 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.
  • tsc is clean for src/ and for the new test file.

Overlap

  • Connect LuaJIT FFI calls to Rust operation handlers #2331 (open, LuaJIT FFI) edits the same walkCalls line: it adds isTransportEdge(c.edge) || to the skip that also held parent.has(c.node.id). Whichever PR lands second should keep the transport skip and drop parent.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

colbymchenry and others added 2 commits October 7, 2026 10:21
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant