diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f05b76c4..5a285b6ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In JavaScript and TypeScript, an import written `import { default as AppRoot } from './routes/app/root'`, as in bulletproof-react's router, now counts as the module's default import, the same as `import AppRoot from './routes/app/root'`. Before, it was read as an import of an export named `default`, which no module has, so a call, a route or a JSX attribute that used `AppRoot` was matched by its name alone: it could link to another file's `AppRoot`, or to nothing when the default export has a name of its own. Svelte, Vue and Astro script blocks are read the same way. - In React, a component's JSX no longer links it to a class or component that only shares a name with one of its type arguments or with a variable of its own. A type in angle brackets, like `Document` in ` items={…} />`, `User` in `useState()` or `Entry` in a generic `(…) =>` component, is no longer read as a tag. A tag naming a variable or parameter the component sets itself, like `` after `const Content = isDropdown ? DropdownMenu.SubContent : ContextMenu.SubContent` or `` in `widgets.map((Widget) => )`, now links to nothing, unless the component declares a component of that name inside itself. Before, these linked to an unrelated class or component elsewhere in the repository: outline's document lists showed up among the callers of its `Document` model class, and its menus among the callers of the command bar's `Content`, so `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading. - In JavaScript and TypeScript, a name imported from a package your `package.json` lists no longer links to a project symbol that only shares its name when `tsconfig.json` or `jsconfig.json` has a catch-all path alias, like `"*": ["./typings/*"]` or `"*": ["src/*", "node_modules/*"]`, or an alias that points the package at a file in `node_modules`, like `"lit/decorators": ["./node_modules/lit/decorators.js"]`. Such an alias made every package look like part of the project, so `import { Typography } from '@mui/material'` was linked to the project's own `Typography` and every lit `@property()` decorator to an unrelated class's `property` field, and `codegraph callers`, impact and `codegraph affected` listed code that never used them. An import the alias does map to a file of your project, like a `.d.ts` you keep for an untyped package or `components/Button` through `"*": ["src/*"]`, links as before. Re-index affected projects after upgrading. +- `codegraph_explore`'s Flow now goes through the implementation your query names when an interface method has several that lead to the same place. Before, it took whichever one came first in the index: asking about prometheus's `Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier` traced the call through the TSDB's `DB.Querier` and left the `fanout.Querier` you named off the Flow. Any two routes of the same length are now settled this way, in favor of the one that passes through more of the symbols you named. ## [1.6.2] - 2026-10-03 diff --git a/__tests__/flow-named-implementation.test.ts b/__tests__/flow-named-implementation.test.ts new file mode 100644 index 000000000..4f0560ff9 --- /dev/null +++ b/__tests__/flow-named-implementation.test.ts @@ -0,0 +1,228 @@ +/** + * When an interface method fans out to several implementations, the Flow goes + * through the one the query names. + * + * Explore's `named` walk is breadth-first, and an interface method calls every + * implementation at the same depth. Whichever implementation the index listed + * first used to claim everything reached after it, so on prometheus + * + * Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier + * + * came back as `Queryable.Querier → DB.Querier → NewMergeQuerier`: a route + * through the TSDB's implementation, which nobody named, while + * `fanout.Querier`, which was named and calls `NewMergeQuerier` just as + * directly, was left off the Flow. gin's form binding did the same once its map + * types counted as implementations: `setter.TrySet` went through + * `headerSource.TrySet` when the query named `formSource.TrySet`. + * + * Every case is asked twice, naming one implementation and then the other, so + * it fails whichever one the index happens to list first. + */ +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 = { + 'go.mod': 'module example.com/app\n\ngo 1.22\n', + // gin's binding package, with structs where gin has map types. + 'binding/form_mapping.go': `package binding + +type setter interface { + TrySet(key string) (bool, error) +} + +type formSource struct{ values map[string][]string } + +func (form formSource) TrySet(key string) (bool, error) { + return setByForm(form.values, key) +} + +func tryToSetValue(s setter, key string) (bool, error) { + return s.TrySet(key) +} + +func setByForm(values map[string][]string, key string) (bool, error) { + return len(values[key]) > 0, nil +} +`, + 'binding/header.go': `package binding + +type headerSource struct{ values map[string][]string } + +func (hs headerSource) TrySet(key string) (bool, error) { + return setByForm(hs.values, key) +} +`, + // Both implementations call the same function. + 'src/store.ts': `export interface Store { + save(key: string): boolean; +} + +export function persist(store: Store, key: string): boolean { + return store.save(key); +} + +export function writeEntry(key: string): boolean { + return key.length > 0; +} +`, + 'src/memory-store.ts': `import { Store, writeEntry } from './store'; + +export class MemoryStore implements Store { + save(key: string): boolean { + return writeEntry(key); + } +} +`, + 'src/disk-store.ts': `import { Store, writeEntry } from './store'; + +export class DiskStore implements Store { + save(key: string): boolean { + return writeEntry(key); + } +} +`, + // Each implementation calls its own function, and the query names both, so + // the two routes end at different symbols that are equally deep. + 'src/cache.ts': `export interface Cache { + evict(key: string): void; +} + +export function expire(cache: Cache, key: string): void { + cache.evict(key); +} + +export function dropFromMemory(key: string): void { + console.log(key); +} + +export function dropFromDisk(key: string): void { + console.log(key); +} +`, + 'src/memory-cache.ts': `import { Cache, dropFromMemory } from './cache'; + +export class MemoryCache implements Cache { + evict(key: string): void { + dropFromMemory(key); + } +} +`, + 'src/disk-cache.ts': `import { Cache, dropFromDisk } from './cache'; + +export class DiskCache implements Cache { + evict(key: string): void { + dropFromDisk(key); + } +} +`, +}; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-flow-named-impl-')); + 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, each step as `qualifiedName@file`. */ +function leadChain(query: string): string[] { + const flow = resolveNamedSymbolFlow(cg, query); + return (flow.chains[0]?.steps ?? []).map((s) => `${s.node.qualifiedName}@${s.node.filePath}`); +} + +/** 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 Flow through an interface goes through the implementation the query names', () => { + it('Go: the route through formSource.TrySet when the query names it', () => { + expect(leadChain('tryToSetValue setter.TrySet formSource.TrySet setByForm')).toEqual([ + 'tryToSetValue@binding/form_mapping.go', + 'setter::TrySet@binding/form_mapping.go', + 'formSource::TrySet@binding/form_mapping.go', + 'setByForm@binding/form_mapping.go', + ]); + }); + + it('Go: the route through headerSource.TrySet when the query names it', () => { + expect(leadChain('tryToSetValue setter.TrySet headerSource.TrySet setByForm')).toEqual([ + 'tryToSetValue@binding/form_mapping.go', + 'setter::TrySet@binding/form_mapping.go', + 'headerSource::TrySet@binding/header.go', + 'setByForm@binding/form_mapping.go', + ]); + }); + + it('TypeScript: the route through the named class when two classes call the same function', () => { + expect(leadChain('persist Store.save MemoryStore.save writeEntry')).toEqual([ + 'persist@src/store.ts', + 'Store::save@src/store.ts', + 'MemoryStore::save@src/memory-store.ts', + 'writeEntry@src/store.ts', + ]); + expect(leadChain('persist Store.save DiskStore.save writeEntry')).toEqual([ + 'persist@src/store.ts', + 'Store::save@src/store.ts', + 'DiskStore::save@src/disk-store.ts', + 'writeEntry@src/store.ts', + ]); + }); + + it('TypeScript: of two equally deep ends, the one reached through the named class', () => { + expect(leadChain('expire Cache.evict MemoryCache.evict dropFromMemory dropFromDisk')).toEqual([ + 'expire@src/cache.ts', + 'Cache::evict@src/cache.ts', + 'MemoryCache::evict@src/memory-cache.ts', + 'dropFromMemory@src/cache.ts', + ]); + expect(leadChain('expire Cache.evict DiskCache.evict dropFromMemory dropFromDisk')).toEqual([ + 'expire@src/cache.ts', + 'Cache::evict@src/cache.ts', + 'DiskCache::evict@src/disk-cache.ts', + 'dropFromDisk@src/cache.ts', + ]); + }); + + it('still bridges through an implementation when the query names none', () => { + const chain = leadChain('tryToSetValue setter.TrySet setByForm'); + expect(chain).toHaveLength(4); + expect(chain[0]).toBe('tryToSetValue@binding/form_mapping.go'); + expect(chain[1]).toBe('setter::TrySet@binding/form_mapping.go'); + expect(['formSource::TrySet@binding/form_mapping.go', 'headerSource::TrySet@binding/header.go']).toContain(chain[2]); + expect(chain[3]).toBe('setByForm@binding/form_mapping.go'); + }); + + it("codegraph_explore's Flow section lists the named implementation", async () => { + const formLine = cg.getNodesByName('TrySet').find((n) => n.qualifiedName === 'formSource::TrySet')!.startLine; + const headerLine = cg.getNodesByName('TrySet').find((n) => n.qualifiedName === 'headerSource::TrySet')!.startLine; + + const viaForm = await exploreFlow('tryToSetValue setter.TrySet formSource.TrySet setByForm'); + expect(viaForm).toHaveLength(4); + expect(viaForm[2]).toBe(`3. TrySet (binding/form_mapping.go:${formLine})`); + + const viaHeader = await exploreFlow('tryToSetValue setter.TrySet headerSource.TrySet setByForm'); + expect(viaHeader).toHaveLength(4); + expect(viaHeader[2]).toBe(`3. TrySet (binding/header.go:${headerLine})`); + }); +}); diff --git a/src/graph/named-symbol-flow.ts b/src/graph/named-symbol-flow.ts index 1652a2c6d..956c43ee5 100644 --- a/src/graph/named-symbol-flow.ts +++ b/src/graph/named-symbol-flow.ts @@ -389,6 +389,19 @@ function callSitesOf(steps: readonly FlowStep[]): Map { const NAMED_VISIT_CAP = 1500; const DIRECTED_VISIT_CAP = 12_000; +/** How the `named` walk reached a node, and what the route there carries. */ +interface WalkStep { + prev: string | null; + edge: Edge | null; + node: Node; + /** Hops from the seed. */ + depth: number; + /** Unnamed hops the route ends on, held against `maxBridge`. */ + streak: number; + /** Named symbols on the route, the seed and this node included. */ + named: number; +} + /** * Breadth-first over `calls` edges — synthesized ones included, which is what * carries a flow across a callback, a re-render or a JSX child. @@ -397,6 +410,19 @@ const DIRECTED_VISIT_CAP = 12_000; * 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). + * * Returns the parent map, so a caller can reconstruct any reached node's path. */ function walkCalls( @@ -405,25 +431,41 @@ function walkCalls( named: ReadonlySet, maxHops: number, maxBridge: number -): { parent: Map; reached: string[] } { - const parent = new Map(); - parent.set(seed.id, { prev: null, edge: null, node: seed }); - const queue: Array<{ id: string; depth: number; streak: number }> = [ - { id: seed.id, depth: 0, streak: 0 }, - ]; +): { parent: Map; reached: string[] } { + const parent = new Map(); + parent.set(seed.id, { + prev: null, + edge: null, + node: seed, + depth: 0, + streak: 0, + named: named.has(seed.id) ? 1 : 0, + }); + const queue: string[] = [seed.id]; const reached: string[] = []; for (let head = 0; head < queue.length && parent.size < NAMED_VISIT_CAP; head++) { - const { id, depth, streak } = queue[head]!; + const id = queue[head]!; + const at = parent.get(id)!; if (id !== seed.id && named.has(id)) reached.push(id); - if (depth >= maxHops - 1) continue; + if (at.depth >= maxHops - 1) continue; for (const c of cg.getCallees(id)) { - if (!FLOW_EDGE_KINDS.has(c.edge.kind) || parent.has(c.node.id)) continue; + if (!FLOW_EDGE_KINDS.has(c.edge.kind)) continue; + const isNamed = named.has(c.node.id); // A route node is a connector, not a symbol the reader would have named: // crossing one costs no bridge budget. - const newStreak = named.has(c.node.id) ? 0 : c.node.kind === 'route' ? streak : streak + 1; - if (newStreak > maxBridge) continue; - parent.set(c.node.id, { prev: id, edge: c.edge, node: c.node }); - queue.push({ id: c.node.id, depth: depth + 1, streak: newStreak }); + 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 }); } } return { parent, reached }; @@ -596,13 +638,20 @@ 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. - let deepest: FlowStep[] | null = null; + // 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 steps = chainTo(parent, id); - if (!deepest || steps.length > deepest.length) deepest = steps; + const at = parent.get(id)!; + if ( + !deepest || + at.depth > deepest.depth || + (at.depth === deepest.depth && at.named > deepest.named) + ) { + deepest = at; + } } - if (deepest) found.push(deepest); + if (deepest) found.push(chainTo(parent, deepest.node.id)); } }