From 5e75ced771ed537e0e2084385f0c3063daa936a9 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Thu, 1 Oct 2026 19:34:53 +0300 Subject: [PATCH 1/2] perf(context): stop redoing per-query work per candidate on long prompts On a long prompt, explore (and so the Claude Code prompt hook) spent most of its time repeating work whose answer cannot change within one retrieval (#2184): - scorePathRelevance re-split the whole query into words and re-ran extractSearchTerms for every candidate path. The query half is now prepared once (preparePathRelevanceQuery) and scored per path (scorePreparedPathRelevance); scorePathRelevance keeps its signature. - The deprioritize predicate stat'ed codegraph.json for every candidate. The matcher set on QueryBuilder is now a source that returns a predicate for the config in force; searchNodes and ContextBuilder take one per pass. Config edits are still picked up on the next search. - getAllNodeNames re-read SELECT DISTINCT name FROM nodes on every fuzzy fallback, once per query term. It is now memoized per database change stamp, like getDominantFile (#1864), held through a WeakRef so a large index's name list is not pinned between retrievals. Ranking is unchanged: explore, buildContext and findRelevantContext outputs are byte-identical to main on 18 prompts (short symbol queries, 500 B to 8 KB prose and subagent reports), with and without a deprioritize config. Prompt-hook wall time on a 377-file index: 4 KB report 4.6 s -> 2.2 s, 8 KB report 8.6 s -> 3.0 s. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/node-names-cache.test.ts | 47 +++++++++++++++++++++++ src/context/index.ts | 28 ++++++++------ src/db/queries.ts | 60 ++++++++++++++++++++++-------- src/index.ts | 29 ++++++++++----- src/search/query-utils.ts | 57 +++++++++++++++++++++++----- 6 files changed, 177 insertions(+), 45 deletions(-) create mode 100644 __tests__/node-names-cache.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb9e73572..db1f577536 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,6 +154,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In React with TypeScript, a component that renders a generic component and passes it type arguments, like outline's ` items={…} />` or excalidraw's ` …>`, now links to the component it renders. Before, a tag followed by type arguments was skipped, so the generic component looked unused by the screens that render it, and `codegraph_explore`'s call paths, callers and impact stopped short of it. Re-index React projects after upgrading. - In React, a component that a file declares itself as a value is now what that file's JSX renders, whatever else in the repository shares its name. A lazily loaded page, like `const RegisterPage = Loadable(lazy(() => import('./pages/Register')))`, a `dynamic(() => import(…))` component or one loaded only in development (`IS_DEV ? lazy(…) : null`), links to the component its module exports, including the one its loader picks and one an `export default observer(Login)` wraps; a wrapped component like `const Settings = observer(function Settings() {…})`, `React.memo(Rows, areEqual)` or `forwardRef(ComboboxInner) as {…}` links to the function it wraps; and an alias like `const DialogPortal = DialogPrimitive.Portal` links to its own declaration. Before, `` was linked to any lone component of that name elsewhere, often in another app of the same repository, so `codegraph_explore`, callers and impact followed it into the wrong app while the page it really renders had no caller. React Router layouts declared this way follow the same rule, and a TSX type named like a value the file declares now stays in that file. Re-index React projects after upgrading. - In React, a JSX tag imported from one of your own modules now links to the component that module exports under that name, read the way a lazily loaded page's module already is. That includes going through an `index` file or barrel that passes the component on, like outline's `Sidebar/index.ts` with `import Sidebar from './App'; export default Sidebar` or its `Avatar/index.ts` with `const Avatar = AvatarWithHoverCard`, and past a wrapper such as `export default observer(AppSidebar)`, `React.memo(StaticCanvas, areEqual)` or mastodon's `const MemoizedBlurhash = memo(Blurhash)`. Before, a tag was matched by its name and the import only broke ties, so outline's `` linked to an unrelated styled `Sidebar` in a sibling file, a default import under another name linked to whatever other component its module exported, and an import through a barrel could land on another app's component of the same name. A tag whose module exports an unnamed component, like `export default () => …`, now links to nothing instead of to a namesake elsewhere. A lazily loaded React Router page exported as such a value or wrapper links to the component it holds too, the one with the markup. `codegraph sync` and the file watcher redraw these links when only the file that passes the component on changes. Re-index React projects after upgrading. +- `codegraph_explore` and the Claude Code prompt hook are much faster on long prompts, such as a pasted report several thousand characters long, which could run past the hook's 30-second timeout. The results are unchanged. (#2184) ## [1.6.2] - 2026-10-03 diff --git a/__tests__/node-names-cache.test.ts b/__tests__/node-names-cache.test.ts new file mode 100644 index 0000000000..165628972d --- /dev/null +++ b/__tests__/node-names-cache.test.ts @@ -0,0 +1,47 @@ +/** + * getAllNodeNames() is read once per database state (#2184). + * + * The fuzzy search fallback scans the distinct name list, and a long prose + * prompt runs it once per query term that FTS and LIKE miss — hundreds of + * re-reads of `SELECT DISTINCT name FROM nodes` per retrieval. The list is now + * memoized against the same change stamp as the dominant-file memo (#1864). + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import CodeGraph from '../src'; +import { QueryBuilder } from '../src/db/queries'; + +function queriesOf(cg: CodeGraph): QueryBuilder { + return (cg as unknown as { queries: QueryBuilder }).queries; +} + +describe('node names — read once per index state (#2184)', () => { + let dir: string; + let cg: CodeGraph | undefined; + + afterEach(() => { + cg?.close(); + cg = undefined; + vi.restoreAllMocks(); + if (dir) fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('a many-term query reads the name list once, and a sync is seen', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-node-names-')); + fs.writeFileSync(path.join(dir, 'engine.ts'), 'export function startEngine(): void {}\n'); + cg = await CodeGraph.init(dir, { index: true }); + const read = vi.spyOn(queriesOf(cg) as unknown as { readAllNodeNames: () => string[] }, 'readAllNodeNames'); + + // Every word misses FTS and LIKE, so each one falls through to the fuzzy pass. + await cg.findRelevantContext('how does zorblax quuxify the wibbler and frobnicate glorp'); + expect(read).toHaveBeenCalledTimes(1); + + fs.writeFileSync(path.join(dir, 'engine.ts'), 'export function stopEngine(): void {}\n'); + await cg.sync(); + expect(queriesOf(cg).getAllNodeNames()).toContain('stopEngine'); + expect(read).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/context/index.ts b/src/context/index.ts index 4f924eb40a..9edca8ecfe 100644 --- a/src/context/index.ts +++ b/src/context/index.ts @@ -25,7 +25,7 @@ import { GraphTraverser } from '../graph'; import { formatContextAsMarkdown, formatContextAsJson } from './formatter'; import { logDebug } from '../errors'; import { validatePathWithinRoot, isConfigLeafNode } from '../utils'; -import { isTestFile, extractSearchTerms, scorePathRelevance, getStemVariants, isDistinctiveIdentifier } from '../search/query-utils'; +import { isTestFile, extractSearchTerms, preparePathRelevanceQuery, scorePreparedPathRelevance, getStemVariants, isDistinctiveIdentifier } from '../search/query-utils'; import { LOW_CONFIDENCE_MARKER } from './markers'; import { findNamedCopybooks, isCopybookInclude } from '../graph/cobol-copybooks'; import { describeSynthesizedHop } from '../graph/synthesized-hop'; @@ -220,9 +220,13 @@ export class ContextBuilder { * and its non-production budget cap are deliberately NOT joined: those REMOVE * content, and `deprioritize` is a ranking lever by definition — `exclude` is * the lever for taking things out of reach. + * + * Taken once per retrieval and applied to every candidate: taking one reads + * the config, and doing that per candidate path cost seconds on a long + * prompt (#2184). */ - private isDeprioritized(filePath: string): boolean { - return this.queries.getDeprioritizedPathMatcher()?.(filePath) ?? false; + private deprioritizedPredicate(): (filePath: string) => boolean { + return this.queries.getDeprioritizedPathMatcher() ?? (() => false); } /** @@ -858,6 +862,10 @@ export class ContextBuilder { // Track per-node term hits for multi-term boosting const camelNodeTerms = new Map(); const maxCamelPerTerm = Math.ceil(opts.searchLimit / 2); + // Path relevance is scored against the whole query for every candidate + // below: split the query and read the deprioritize config once (#2184). + const pathQuery = preparePathRelevanceQuery(query); + const isDeprioritized = this.deprioritizedPredicate(); for (const sym of symbolsFromQuery) { const titleCased = sym.charAt(0).toUpperCase() + sym.slice(1).toLowerCase(); @@ -902,11 +910,10 @@ export class ContextBuilder { if (searchIdSet.has(r.node.id)) continue; if (isTestFile(r.node.filePath) && !isTestQuery) continue; - const pathScore = scorePathRelevance( + const pathScore = scorePreparedPathRelevance( r.node.filePath, - query, - undefined, - this.isDeprioritized(r.node.filePath), + pathQuery, + isDeprioritized(r.node.filePath), ); const brevityBonus = Math.max(0, 6 - (name.length - titleCased.length) / 4); termCandidates.push({ node: r.node, score: 8 + brevityBonus + pathScore }); @@ -994,11 +1001,10 @@ export class ContextBuilder { const compoundResults: SearchResult[] = []; for (const [, entry] of compoundTermMap) { if (entry.terms.size >= 2) { - const pathScore = scorePathRelevance( + const pathScore = scorePreparedPathRelevance( entry.node.filePath, - query, - undefined, - this.isDeprioritized(entry.node.filePath), + pathQuery, + isDeprioritized(entry.node.filePath), ); const brevityBonus = Math.max(0, 6 - entry.node.name.length / 8); compoundResults.push({ diff --git a/src/db/queries.ts b/src/db/queries.ts index 6421eec360..3ace68e327 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -18,7 +18,7 @@ import { SearchResult, } from '../types'; import { safeJsonParse } from '../utils'; -import { isTestFile, kindBonus, nameMatchBonus, scorePathRelevance } from '../search/query-utils'; +import { isTestFile, kindBonus, nameMatchBonus, preparePathRelevanceQuery, scorePreparedPathRelevance } from '../search/query-utils'; import { parseQuery, boundedEditDistance } from '../search/query-parser'; import { isGeneratedFile } from '../extraction/generated-detection'; import { splitIdentifierSegments } from '../search/identifier-segments'; @@ -236,7 +236,7 @@ export class QueryBuilder { // whole project, not a symbol, so it carries no discriminative signal (#720). // Set once by the CodeGraph instance; empty by default (no down-weighting). private projectNameTokens: Set = new Set(); - private isDeprioritizedPath: ((filePath: string) => boolean) | undefined; + private deprioritizedPathMatcher: (() => ((filePath: string) => boolean) | undefined) | undefined; // FTS5 availability flag — detected once at construction time (#1532) private _fts5Available: boolean | undefined; @@ -252,6 +252,12 @@ export class QueryBuilder { | { stamp: string; value: { filePath: string; edgeCount: number; nextEdgeCount: number } | null } | undefined; + // getAllNodeNames()'s list under the same change stamp. The fuzzy search + // pass asks for it once per query term, so it has to outlive one search + // (#2184); held weakly because on a multi-million-name index it is too large + // to pin in a long-lived server between retrievals. + private nodeNamesMemo: { stamp: string; names: WeakRef } | undefined; + // Prepared statements (lazily initialized) private stmts: { insertNode?: SqliteStatement; @@ -391,6 +397,7 @@ export class QueryBuilder { // different databases report the same one — the memo goes with the old // connection, or a worker following a rebuilt index keeps its answer (#1864). this.dominantFileMemo = undefined; + this.nodeNamesMemo = undefined; } private edgeKindStmt(sql: string): SqliteStatement { @@ -415,18 +422,22 @@ export class QueryBuilder { } /** - * Set the predicate that marks a path as de-prioritized by the project's - * `codegraph.json` `deprioritize` patterns (#982). Ranking-only: those paths - * stay indexed and findable, they just stop outranking first-party code. - * Called once when the project opens; undefined disables the lever. + * Set the source of the predicate that marks a path as de-prioritized by the + * project's `codegraph.json` `deprioritize` patterns (#982). Ranking-only: + * those paths stay indexed and findable, they just stop outranking + * first-party code. Called once when the project opens; undefined disables + * the lever. The source returns the predicate for the config in force when it + * is called (undefined: nothing is de-prioritized), so a ranking pass takes + * one and applies it to every candidate instead of re-reading the config per + * path (#2184). */ - setDeprioritizedPathMatcher(matcher: ((filePath: string) => boolean) | undefined): void { - this.isDeprioritizedPath = matcher; + setDeprioritizedPathMatcher(matcher: (() => ((filePath: string) => boolean) | undefined) | undefined): void { + this.deprioritizedPathMatcher = matcher; } - /** The `deprioritize` predicate (#982), so other rankers apply the same lever. */ + /** The `deprioritize` predicate (#982) for the current config, so other rankers apply the same lever. */ getDeprioritizedPathMatcher(): ((filePath: string) => boolean) | undefined { - return this.isDeprioritizedPath; + return this.deprioritizedPathMatcher?.(); } // =========================================================================== @@ -1631,6 +1642,8 @@ export class QueryBuilder { // Apply multi-signal scoring if (results.length > 0 && (text || query)) { const scoringQuery = text || query; + const pathQuery = preparePathRelevanceQuery(scoringQuery, this.projectNameTokens); + const isDeprioritized = this.getDeprioritizedPathMatcher(); results = results.map(r => { // A path the project de-prioritized is saying its symbol NAMES are not // the answer, so the exact-name bonus has to be damped too. The -15 path @@ -1638,14 +1651,14 @@ export class QueryBuilder { // on #982's repro, a `usage()` helper sat at 74.8 vs 51.2 for the top // product symbol — -15 lands at 59.8, still ahead). Damped, not zeroed, // so the tree stays findable when it genuinely is what you asked for. - // Evaluated once and reused: the predicate stats the config file. - const deprioritized = this.isDeprioritizedPath?.(r.node.filePath) ?? false; + // Evaluated once and reused. + const deprioritized = isDeprioritized?.(r.node.filePath) ?? false; const nameBonus = nameMatchBonus(r.node.name, scoringQuery); return { ...r, score: r.score + kindBonus(r.node.kind) - + scorePathRelevance(r.node.filePath, scoringQuery, this.projectNameTokens, deprioritized) + + scorePreparedPathRelevance(r.node.filePath, pathQuery, deprioritized) + (deprioritized ? Math.round(nameBonus * DEPRIORITIZED_NAME_BONUS_SCALE) : nameBonus), }; }); @@ -3787,9 +3800,26 @@ export class QueryBuilder { } /** - * Get all distinct node names (lightweight — just name strings for pre-filtering) + * Get all distinct node names (lightweight — just name strings for pre-filtering). + * Memoized per database change stamp, like {@link getDominantFile}; the list + * is shared between callers, so it must not be modified. */ - getAllNodeNames(): string[] { + getAllNodeNames(): readonly string[] { + // Same transaction rule as getDominantFile: a stamp read inside one could + // outlive a ROLLBACK. + if (this.db.inTransaction !== false) { + this.nodeNamesMemo = undefined; + return this.readAllNodeNames(); + } + const stamp = this.getChangeStamp(); + const memo = this.nodeNamesMemo?.stamp === stamp ? this.nodeNamesMemo.names.deref() : undefined; + if (memo) return memo; + const names = this.readAllNodeNames(); + this.nodeNamesMemo = { stamp, names: new WeakRef(names) }; + return names; + } + + private readAllNodeNames(): string[] { if (!this.stmts.getAllNodeNames) { this.stmts.getAllNodeNames = this.db.prepare('SELECT DISTINCT name FROM nodes'); } diff --git a/src/index.ts b/src/index.ts index 1f50d46ce7..4644341076 100644 --- a/src/index.ts +++ b/src/index.ts @@ -208,27 +208,36 @@ export class CodeGraph { // CodeGraph per root for its whole lifetime, so editing `codegraph.json` // would appear to do nothing until the process restarted — `exclude` and // `include` do not behave that way. `loadDeprioritizePatterns` is - // mtime-cached, so this costs one `stat`; the compiled matcher is memoized - // on the pattern array's identity, which the cache keeps stable. + // mtime-cached, so this costs one `stat` per ranking pass (a pass takes + // one predicate for all its candidates, #2184); the compiled matcher is + // memoized on the pattern array's identity, which the cache keeps stable. let cachedPatterns: string[] | undefined; let cachedMatcher: ReturnType | undefined; - this.queries.setDeprioritizedPathMatcher((filePath: string): boolean => { + this.queries.setDeprioritizedPathMatcher(() => { + let matcher: ReturnType | undefined; try { const patterns = loadDeprioritizePatterns(this.projectRoot); - if (patterns.length === 0) return false; + if (patterns.length === 0) return undefined; if (patterns !== cachedPatterns) { cachedPatterns = patterns; cachedMatcher = ignore().add(patterns); } - const rel = path.isAbsolute(filePath) - ? path.relative(this.projectRoot, filePath) - : filePath; - if (!rel || rel.startsWith('..')) return false; - return cachedMatcher!.ignores(rel.split(path.sep).join('/')); + matcher = cachedMatcher; } catch { // Ranking must never take the search down with it. - return false; + return undefined; } + return (filePath: string): boolean => { + try { + const rel = path.isAbsolute(filePath) + ? path.relative(this.projectRoot, filePath) + : filePath; + if (!rel || rel.startsWith('..')) return false; + return matcher!.ignores(rel.split(path.sep).join('/')); + } catch { + return false; + } + }; }); this.orchestrator = new ExtractionOrchestrator(this.projectRoot, this.queries); diff --git a/src/search/query-utils.ts b/src/search/query-utils.ts index 91bbdd09e1..a930153ee5 100644 --- a/src/search/query-utils.ts +++ b/src/search/query-utils.ts @@ -224,11 +224,31 @@ export function scorePathRelevance( projectNameTokens?: Set, isDeprioritized?: boolean, ): number { - const pathLower = filePath.toLowerCase(); - const fileName = path.basename(filePath).toLowerCase(); - const dirName = path.dirname(filePath).toLowerCase(); - let score = 0; + return scorePreparedPathRelevance( + filePath, + preparePathRelevanceQuery(query, projectNameTokens), + isDeprioritized, + ); +} + +/** + * The query's half of {@link scorePathRelevance}, which is the same for every + * path scored against that query. A ranker scoring many candidate paths for + * one query prepares it once: re-splitting a multi-KB prompt per candidate was + * most of the retrieval time on long prompts (#2184). + */ +export interface PathRelevanceQuery { + /** The query has no words: every path scores 0, penalties included. */ + readonly empty: boolean; + /** Sub-tokens of each scored query word, in query order; words with none are left out. */ + readonly wordTokens: readonly (readonly string[])[]; + readonly isTestQuery: boolean; +} +export function preparePathRelevanceQuery( + query: string, + projectNameTokens?: Set, +): PathRelevanceQuery { // Score per original query WORD, not per sub-token. A single PascalCase word // splits into many sub-tokens (a project name "SuperBizAgent" → // superbizagent / super / biz / agent) that all match the SAME path segment, @@ -239,7 +259,7 @@ export function scorePathRelevance( // camelCase/snake split per word (so `getUserName` still matches a // `get_user_name` path) — we just attribute each word's matches once. const allWords = query.split(/\s+/).filter((w) => w.length > 0); - if (allWords.length === 0) return 0; + if (allWords.length === 0) return { empty: true, wordTokens: [], isTestQuery: false }; // A query word that just names the PROJECT (its go.mod / package.json / repo // name) carries no discriminative path signal — drop it so the rest of the @@ -252,11 +272,32 @@ export function scorePathRelevance( : allWords; const scored = words.length > 0 ? words : allWords; + const wordTokens: string[][] = []; for (const word of scored) { // Use base terms only — stem variants inflate path scores by generating // many near-duplicate terms that all match the same path segments. const subtokens = extractSearchTerms(word, { stems: false }); - if (subtokens.length === 0) continue; + if (subtokens.length > 0) wordTokens.push(subtokens); + } + + const queryLower = query.toLowerCase(); + const isTestQuery = queryLower.includes('test') || queryLower.includes('spec'); + return { empty: false, wordTokens, isTestQuery }; +} + +/** {@link scorePathRelevance} against a query prepared by {@link preparePathRelevanceQuery}. */ +export function scorePreparedPathRelevance( + filePath: string, + query: PathRelevanceQuery, + isDeprioritized?: boolean, +): number { + if (query.empty) return 0; + const pathLower = filePath.toLowerCase(); + const fileName = path.basename(filePath).toLowerCase(); + const dirName = path.dirname(filePath).toLowerCase(); + let score = 0; + + for (const subtokens of query.wordTokens) { // Exact filename match (strongest) if (subtokens.some((t) => fileName.includes(t))) score += 10; // Directory match @@ -274,9 +315,7 @@ export function scorePathRelevance( // standing statement by the project, so it is NOT waived. The name-bonus // damping at the call site is what keeps such a tree findable. // - a path that is both is docked ONCE, not twice. - const queryLower = query.toLowerCase(); - const isTestQuery = queryLower.includes('test') || queryLower.includes('spec'); - const offTarget = (!isTestQuery && isTestFile(filePath)) || isDeprioritized === true; + const offTarget = (!query.isTestQuery && isTestFile(filePath)) || isDeprioritized === true; if (offTarget) { score -= 15; } From 263058a9e091f0f6e108ffd82bc4afcde06ef239 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Thu, 1 Oct 2026 19:44:24 +0300 Subject: [PATCH 2/2] test(context): don't pin how many times a sync reads the name list The sync's own resolution may read the name list again (any write after that read moves the change stamp), so the test now checks what the memo promises: the new name is seen after a sync, and a repeat call in the same state does not re-read. Co-Authored-By: Claude Opus 5.5 --- __tests__/node-names-cache.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/__tests__/node-names-cache.test.ts b/__tests__/node-names-cache.test.ts index 165628972d..de565d0acf 100644 --- a/__tests__/node-names-cache.test.ts +++ b/__tests__/node-names-cache.test.ts @@ -41,7 +41,12 @@ describe('node names — read once per index state (#2184)', () => { fs.writeFileSync(path.join(dir, 'engine.ts'), 'export function stopEngine(): void {}\n'); await cg.sync(); + // The sync's own resolution may read the list again; what matters is that + // the new name is seen and a repeat call in the same state is a memo hit. expect(queriesOf(cg).getAllNodeNames()).toContain('stopEngine'); - expect(read).toHaveBeenCalledTimes(2); + const afterSync = read.mock.calls.length; + expect(afterSync).toBeGreaterThan(1); + queriesOf(cg).getAllNodeNames(); + expect(read).toHaveBeenCalledTimes(afterSync); }); });