diff --git a/CHANGELOG.md b/CHANGELOG.md index f58218b4c7..0d462ceffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph_explore`'s Flow section now keeps a symbol you named even when a shorter route reaches it first through a function you did not name. Before, a step reachable both directly and through a named intermediate was dropped, so the flow stopped short of a symbol in your query; now the chain that visits the most of the symbols you named wins, and a longer chain that visits fewer no longer outranks it. (#2970) - In TypeScript, an awaited call with type arguments like `await fetchApi("/me")` or `await api.get(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586) - In JavaScript and TypeScript, a call or `new` in a parameter's default value, like `function renderWithNote(note = buildNote())`, a destructured `{ size = defaultSize() }` or `constructor(private svc = new Service())`, now counts as the function's own call. Before, it was dropped, so `buildNote` was missing that caller and impact on it came back incomplete. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and the pull request. (#2588) - In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render()` or `render()`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587) diff --git a/__tests__/explore-exact-target.test.ts b/__tests__/explore-exact-target.test.ts index d54d36798f..d598916a63 100644 --- a/__tests__/explore-exact-target.test.ts +++ b/__tests__/explore-exact-target.test.ts @@ -235,10 +235,12 @@ class IExact: return result.content?.[0]?.text ?? ''; }; - it('fixture sanity: the flow runs as_sql → get_qualify_sql → get_select and compiler.py goes focused', async () => { + it('fixture sanity: the flow runs through every named step and compiler.py goes focused', async () => { const text = await explore('SQLCompiler.as_sql pre_sql_setup get_select'); expect(text).toContain('**Flow (call path among the symbols you queried)'); - expect(text).toMatch(/1\. as_sql \(compiler\.py:\d+\)[\s\S]*2\. get_qualify_sql[\s\S]*3\. get_select/); + // #2970: the named `pre_sql_setup` is on the path (via the unnamed bridge + // `setup_query`) even though `get_qualify_sql` reaches `get_select` sooner. + expect(text).toMatch(/1\. as_sql \(compiler\.py:\d+\)[\s\S]*2\. pre_sql_setup[\s\S]*3\. setup_query[\s\S]*4\. get_select/); expect(sectionFor(text, COMPILER), 'the family file renders as a per-symbol view').toContain('· focused'); }); diff --git a/__tests__/flow-named-longer-route.test.ts b/__tests__/flow-named-longer-route.test.ts new file mode 100644 index 0000000000..e81b463f81 --- /dev/null +++ b/__tests__/flow-named-longer-route.test.ts @@ -0,0 +1,133 @@ +/** + * Explore's Flow keeps a named symbol even when a SHORTER route reaches it + * first (#2970). + * + * `walkCalls` used to keep one route per node: once a node was reached, only a + * better route of the SAME length replaced it. On the issue's shape + * + * entry → mid1 → deep → leaf + * entry → mid2 → leaf + * + * `leaf` is first reached at depth 2 through the unnamed `mid2`, so the route + * that actually passes the named `deep` (depth 3, one more named symbol) was + * rejected for being longer and `leaf` fell off the Flow. + * + * The walk now keeps a route per (node, named-count, bridge-streak) STATE, and + * the chain that visits the MOST named symbols wins — the longer one only + * breaks a tie. Flow stays a single longest-chain answer, never an all-paths + * listing, and the one-unnamed-bridge budget is unchanged. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { ToolHandler } from '../src/mcp/tools'; +import { resolveNamedSymbolFlow } from '../src/graph/named-symbol-flow'; + +const FILES: Record = { + // The issue's reproduction, verbatim. + 'src/a.ts': `export function entry() { mid1(); mid2(); } +export function mid1() { deep(); } +export function mid2() { leaf(); } +export function deep() { leaf(); } +export function leaf() { return 1; } +`, + // Two chains the query names: one short and fully named, one longer with + // fewer named symbols (two unnamed bridges). Named coverage must win. + 'src/hops.ts': `export function boot() { load(); } +export function load() { parse(); } +export function parse() { save(); } +export function save() { return 1; } + +export function push() { halt(); } +function halt() { spool(); } +export function spool() { yieldRow(); } +function yieldRow() { flush(); } +export function flush() { return 1; } +`, + // One bridge connects, two consecutive bridges do not: the budget is intact. + 'src/budget.ts': `export function alpha() { middle(); } +function middle() { omega(); } +export function omega() { return 1; } + +export function origin() { relay(); } +function relay() { linker(); } +function linker() { dest(); } +export function dest() { return 1; } +`, +}; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-flow-longer-route-')); + for (const [rel, content] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** The lead chain, as bare names. */ +function leadChain(query: string): string[] { + const flow = resolveNamedSymbolFlow(cg, query); + return (flow.chains[0]?.steps ?? []).map((s) => s.node.name); +} + +/** The numbered lines of explore's Flow section. */ +async function exploreFlow(query: string): Promise { + const res = await new ToolHandler(cg).execute('codegraph_explore', { query }); + const text = res.content?.[0]?.text ?? ''; + const start = text.indexOf('**Flow (call path among the symbols you queried)**'); + if (start < 0) return []; + const section = text.slice(start).split(/\n\n(?=\*\*|>)/)[0] ?? ''; + return section.split(/\r?\n/).filter((line) => /^\d+\. /.test(line)); +} + +describe('a named symbol reached first by a shorter route stays on the Flow (#2970)', () => { + it('walks through the named deep to reach leaf, one unnamed bridge', () => { + expect(leadChain('entry deep leaf')).toEqual(['entry', 'mid1', 'deep', 'leaf']); + }); + + it("codegraph_explore's Flow section lists all three named symbols", async () => { + const lines = await exploreFlow('entry deep leaf'); + expect(lines).toHaveLength(4); + expect(lines[0]).toContain('entry'); + expect(lines[2]).toContain('deep'); + expect(lines[3]).toContain('leaf'); + }); +}); + +describe('named coverage ranks above raw length', () => { + it('picks the shorter, fully named chain over a longer chain with fewer names', () => { + // `boot → load → parse → save` names four; `push → halt → spool → yieldRow + // → flush` is one hop longer but names only three. Coverage wins. + expect(leadChain('boot load parse save push spool flush')).toEqual([ + 'boot', + 'load', + 'parse', + 'save', + ]); + }); + + it('leaves a chain with no unnamed intermediate exactly as it was', () => { + expect(leadChain('boot load parse save')).toEqual(['boot', 'load', 'parse', 'save']); + }); +}); + +describe('the bridge budget still holds', () => { + it('connects across one unnamed symbol', () => { + expect(leadChain('alpha omega')).toEqual(['alpha', 'middle', 'omega']); + }); + + it('does not connect across two consecutive unnamed symbols', () => { + expect(resolveNamedSymbolFlow(cg, 'origin dest').chains).toHaveLength(0); + }); +}); diff --git a/src/graph/named-symbol-flow.ts b/src/graph/named-symbol-flow.ts index 34ae8adf56..cc3ed549c9 100644 --- a/src/graph/named-symbol-flow.ts +++ b/src/graph/named-symbol-flow.ts @@ -1,9 +1,10 @@ /** * The call path among a bag of named symbols — the one path finder. * - * `codegraph_explore` leads its answer with a "Flow" section: the longest call - * chain among the symbols an agent named, riding synthesized dynamic-dispatch - * edges so a controller reaches its implementation through the interface. The + * `codegraph_explore` leads its answer with a "Flow" section: the call chain + * among the symbols an agent named that visits the most of them — the longest + * such chain when several tie — riding synthesized dynamic-dispatch edges so a + * controller reaches its implementation through the interface. The * viewer's Flow strip (`/api/flow`, design spec §3.5) draws the same thing as * cards. They must never disagree, so the search lives here once and both * callers ride it: same token parsing, same overload disambiguation, same @@ -14,9 +15,10 @@ * * - **`mode: 'named'`** is exactly what explore does. Every resolved symbol is * both a possible start and a possible end, at most ONE unnamed symbol may - * bridge two named ones ({@link DEFAULT_MAX_BRIDGE}), and the LONGEST chain - * wins. The bridge cap is what stops the search wandering a god-function's - * fan-out: the agent's own naming is the evidence that a hop is on-topic. + * bridge two named ones ({@link DEFAULT_MAX_BRIDGE}), and the chain that + * visits the MOST named symbols wins (the longest of those on a tie). The + * bridge cap is what stops the search wandering a god-function's fan-out: the + * agent's own naming is the evidence that a hop is on-topic. * - **`mode: 'directed'`** is "how does X reach Y", which the agent has no way * to ask and the viewer's search box does. Both ends are pinned, so the * evidence the bridge cap was standing in for is already there and the search @@ -386,6 +388,7 @@ const DIRECTED_VISIT_CAP = 12_000; /** How the `named` walk reached a node, and what the route there carries. */ interface WalkStep { + /** The previous route STATE's key, not a bare node id — see {@link walkStateKey}. */ prev: string | null; edge: Edge | null; node: Node; @@ -397,6 +400,27 @@ interface WalkStep { named: number; } +/** + * Identity of a route prefix in the `named` walk: the node, the named symbols + * the route collected, and the unnamed-hop streak it ends on. + * + * A node alone is not enough. `parent` used to hold one route per node, replaced + * only by a better route of the SAME length, so depth decided first: issue + * #2970 — `entry → mid2 → leaf` reaches `leaf` at depth 2 and the route that + * passes the named `deep` (`entry → mid1 → deep → leaf`, depth 3, one more + * named symbol) was rejected for being longer. Keying on the named count keeps + * both and lets the final choice prefer the fuller one. + * + * The streak is keyed too, so a route that spends a bridge hop and one that + * does not are distinct prefixes, each keeping its own remaining budget. Depth + * is not keyed: the first arrival at a state is its shortest, and a longer + * route to the same (node, named, streak) is dominated — same reward, same + * budget shape, less depth left to spend. + */ +function walkStateKey(nodeId: string, namedCount: number, streak: number): string { + return `${nodeId}#${namedCount}#${streak}`; +} + /** * Breadth-first over `calls` edges — synthesized ones included, which is what * carries a flow across a callback, a re-render or a JSX child. @@ -405,20 +429,16 @@ interface WalkStep { * at most `maxBridge` unnamed symbols may sit between two of them. That cap is * what bounds the frontier, so {@link NAMED_VISIT_CAP} is generous. * - * Two routes of the same length to one node are settled by the agent's naming, - * not by the order the index lists callees in: the route through more named - * symbols wins. An interface method calls every implementation, so - * prometheus's `Queryable.Querier` reaches `NewMergeQuerier` through - * `fanout.Querier` and the TSDB's `DB.Querier` alike; keeping whichever was - * listed first took the Flow through `DB.Querier` when the query named - * `fanout.Querier`. A rival route comes from another node at the same depth, - * and every node at one depth is expanded before any node at the next, so the - * choice is final before anything is reached through it. Ending on fewer - * unnamed hops ranks first, so a choice never spends bridge budget that a later - * hop needs (with the default budget of one, only a route node's streak can - * differ). + * Routes are per STATE, not per node ({@link walkStateKey}). Two routes that + * reach a node are the same prefix only when they collected as many named + * symbols and end on the same bridge streak; then the shorter one wins (it has + * more hops left to spend) and the later one is dropped. A route that visits + * MORE named symbols is a different state and is never lost to a shorter one, + * however much longer it is — which is what keeps every named symbol on the + * Flow when a shorter route reaches one of them first (#2970). * - * Returns the parent map, so a caller can reconstruct any reached node's path. + * Returns the parent map keyed by state, so a caller can reconstruct any + * reached state's path. */ function walkCalls( cg: CodeGraph, @@ -428,20 +448,26 @@ function walkCalls( maxBridge: number ): { parent: Map; reached: string[] } { const parent = new Map(); - parent.set(seed.id, { + const seedNamed = named.has(seed.id) ? 1 : 0; + const seedKey = walkStateKey(seed.id, seedNamed, 0); + parent.set(seedKey, { prev: null, edge: null, node: seed, depth: 0, streak: 0, - named: named.has(seed.id) ? 1 : 0, + named: seedNamed, }); - const queue: string[] = [seed.id]; + const queue: string[] = [seedKey]; const reached: string[] = []; - for (let head = 0; head < queue.length && parent.size < NAMED_VISIT_CAP; head++) { - const id = queue[head]!; - const at = parent.get(id)!; - if (id !== seed.id && named.has(id)) reached.push(id); + // The cap is about how far the search fans out, so it counts distinct NODES + // as before; one node may carry several states, each expanded at most once. + const seen = new Set([seed.id]); + for (let head = 0; head < queue.length && seen.size < NAMED_VISIT_CAP; head++) { + const key = queue[head]!; + const at = parent.get(key)!; + const id = at.node.id; + if (id !== seed.id && named.has(id)) reached.push(key); if (at.depth >= maxHops - 1) continue; for (const c of cg.getCallees(id)) { if (!FLOW_EDGE_KINDS.has(c.edge.kind)) continue; @@ -450,17 +476,19 @@ function walkCalls( // crossing one costs no bridge budget. const streak = isNamed ? 0 : c.node.kind === 'route' ? at.streak : at.streak + 1; if (streak > maxBridge) continue; - const depth = at.depth + 1; - const namedOnRoute = at.named + (isNamed ? 1 : 0); - const prior = parent.get(c.node.id); - if (prior) { - // Reached already: only a better route of the same length replaces it. - if (prior.depth !== depth) continue; - if (streak > prior.streak || (streak === prior.streak && namedOnRoute <= prior.named)) continue; - } else { - queue.push(c.node.id); - } - parent.set(c.node.id, { prev: id, edge: c.edge, node: c.node, depth, streak, named: namedOnRoute }); + const namedCount = at.named + (isNamed ? 1 : 0); + const nextKey = walkStateKey(c.node.id, namedCount, streak); + if (parent.has(nextKey)) continue; + parent.set(nextKey, { + prev: key, + edge: c.edge, + node: c.node, + depth: at.depth + 1, + streak, + named: namedCount, + }); + queue.push(nextKey); + seen.add(c.node.id); } } return { parent, reached }; @@ -580,10 +608,7 @@ function walkBidirectional( return null; } -function chainTo( - parent: Map, - target: string -): FlowStep[] { +function chainTo(parent: Map, target: string): FlowStep[] { const steps: FlowStep[] = []; let cur: string | null = target; while (cur) { @@ -633,25 +658,39 @@ export function resolveNamedSymbolFlow( } else { for (const seed of [...flow.named.values()].slice(0, MAX_SEEDS)) { const { parent, reached } = walkCalls(cg, seed, namedIds, maxHops, maxBridge); - // Explore's rule: the DEEPEST named sink this seed can reach. Of two as - // deep, the one whose route passes more named symbols, as in the walk. - let deepest: WalkStep | null = null; - for (const id of reached) { - const at = parent.get(id)!; + // Explore's rule: of every named sink this seed reaches, the route that + // visits the MOST named symbols — the header's promise — and, of two + // that visit as many, the deeper one. #2970: the route through the + // named `deep` carries one more named symbol than the shorter one + // through `mid2`, so it wins even though it is longer. + let best: WalkStep | null = null; + let bestKey: string | null = null; + for (const key of reached) { + const at = parent.get(key)!; if ( - !deepest || - at.depth > deepest.depth || - (at.depth === deepest.depth && at.named > deepest.named) + !best || + at.named > best.named || + (at.named === best.named && at.depth > best.depth) ) { - deepest = at; + best = at; + bestKey = key; } } - if (deepest) found.push(chainTo(parent, deepest.node.id)); + if (bestKey) found.push(chainTo(parent, bestKey)); } } if (found.length === 0) return flow; - found.sort((a, b) => (directed ? a.length - b.length : b.length - a.length)); + // Explore ranks by named coverage first: a chain that visits more of the + // symbols the agent named is the fuller answer, even when a longer chain + // visits fewer (#2970). Of two that visit as many, the longer wins. + const namedOnChain = (steps: FlowStep[]): number => + steps.reduce((n, s) => n + (namedIds.has(s.node.id) ? 1 : 0), 0); + found.sort((a, b) => + directed + ? a.length - b.length + : namedOnChain(b) - namedOnChain(a) || b.length - a.length + ); // Identical chains, and chains that are just a shorter run along one // already kept, are the same answer twice: `a → b → c` and `b → c` differ diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 89fa53101f..f288140e48 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -3500,10 +3500,11 @@ export class ToolHandler { * Flow-from-named-symbols: an agent's codegraph_explore query is a bag of * symbol names that usually spans the flow it's investigating (e.g. * "PmsProductController getList PmsProductService list PmsProductServiceImpl"). - * Surface the longest call chain AMONG those named symbols — scoped to what the - * agent explicitly named, so (unlike a fuzzy relevance set) there's no - * wrong-feature wandering. Rides synthesized edges, so controller→service- - * interface→impl shows up. Returns '' if no chain of >=3 nodes exists. + * Surface the call chain AMONG those named symbols that visits the most of + * them (the longest such chain on a tie) — scoped to what the agent explicitly + * named, so (unlike a fuzzy relevance set) there's no wrong-feature wandering. + * Rides synthesized edges, so controller→service→interface→impl shows up. + * Returns '' if no chain of >=3 nodes exists. * * Ambiguous tokens (Java `list` → dozens of nodes) are disambiguated by * CO-NAMING: the agent names the class too, so we keep only `list` candidates @@ -6185,15 +6186,22 @@ export class ToolHandler { // names ~7 Session.swift methods — the build spine PLUS off-path // task/didCompleteTask — far past the whole response budget). Engage the // per-symbol view to keep the SPINE full and collapse the off-path named - // methods to signatures. Only when there IS off-path content to shed — - // otherwise the spine is irreducible (a sequential flow has no redundancy), - // so leave it to the normal full render. + // methods to signatures. + // + // A file that DEFINES a polymorphic supertype is content to shed by itself + // (see `definesPolymorphicSupertype`): its subclasses are redundant whatever + // the spine covers. Requiring off-path named content dropped it out of this + // view once #2970 put every named step ON the fuller spine — Django's + // compiler.py then fell back to the cluster render, dumping the + // `SQLInsertCompiler` / `SQLDeleteCompiler` bodies the per-symbol view keeps + // as signatures. A sequential spine with no subtree to shed still renders + // whole; a family base does not. const namedBodyChars = group.nodes .filter(n => CALLABLE_BODY.has(n.kind) && (flow.pathNodeIds.has(n.id) || flow.uniqueNamedNodeIds.has(n.id))) .reduce((s, n) => s + fileLines.slice(n.startLine - 1, n.endLine).join('\n').length, 0); + const offPathNamed = group.nodes.some(n => CALLABLE_BODY.has(n.kind) && flow.uniqueNamedNodeIds.has(n.id) && !flow.pathNodeIds.has(n.id)); const onSpineGodFile = hasSpineNode - && namedBodyChars > allowance - && group.nodes.some(n => CALLABLE_BODY.has(n.kind) && flow.uniqueNamedNodeIds.has(n.id) && !flow.pathNodeIds.has(n.id)); + && (fileDefinesSuper || (namedBodyChars > allowance && offPathNamed)); // A line RANGE the agent asked for (`lines 900-1003`) is not a symbol, so // the per-symbol view has no way to show it — such a file takes the // cluster path, which renders anchor spans first. diff --git a/src/ui-server/api/flow.ts b/src/ui-server/api/flow.ts index cc73838966..5dcc87eab5 100644 --- a/src/ui-server/api/flow.ts +++ b/src/ui-server/api/flow.ts @@ -19,8 +19,9 @@ * * - `?from=&to=` — a directed question, from the search box's flow grammar. * Both ends pinned, shortest path wins. - * - `?symbols=a,b,c` — explore's own question, verbatim. Longest chain among - * the named symbols wins, at most one unnamed bridge. + * - `?symbols=a,b,c` — explore's own question, verbatim. The chain that visits + * the most of the named symbols wins (longest on a tie), at most one unnamed + * bridge. * - `?hop=s&hop=d…` — the trail, read as a flow. Nothing is searched: * the hops are the ones the reader walked, and the work is finding the edge * that already connects each consecutive pair. diff --git a/ui/src/views/FlowView.svelte b/ui/src/views/FlowView.svelte index 99d81440cc..789d2836f8 100644 --- a/ui/src/views/FlowView.svelte +++ b/ui/src/views/FlowView.svelte @@ -223,7 +223,7 @@ if (p.query.kind === 'directed') { return 'Every card is a call the graph recorded. A dashed link is a hop no one can see in the source — a callback, an interface, a re-render — and it names where it was wired.'; } - return 'The longest call path among the symbols you named, the same one codegraph_explore leads with.'; + return 'The call path among the symbols you named that visits the most of them, the same one codegraph_explore leads with.'; } /**