From 64aff9939638992e563efa9ffa732072fcf62f01 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 10 Sep 2026 16:36:01 -0400 Subject: [PATCH] fix(ai-vault-search): cut a query on a code point and bind ids in batches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small ones from the review's not-routed list. `query.slice(0, 512)` can land between the halves of a surrogate pair, leaving a lone half that matches nothing and that a caller cannot echo back. The reader already has `sliceAtCodeUnitLimit` for exactly this. `loadSessions` bound one parameter per candidate id in a single statement. The list is as long as the candidate limit, the tuning doc invites a host to raise that limit, and SQLite's `SQLITE_MAX_VARIABLE_NUMBER` is 999 on builds older than 3.32 — so one settings change away from `too many SQL variables`. Read in batches of 500, leaving room for the filter's own bound values. --- .../session-search-engine.test.ts | 28 +++++++++++++++++++ .../ai-vault-search/session-search-engine.ts | 5 +++- .../session-search-retrieval.ts | 25 +++++++++++++---- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/main/ai-vault-search/session-search-engine.test.ts b/src/main/ai-vault-search/session-search-engine.test.ts index 8836bad6e9b..bc25fb06d2d 100644 --- a/src/main/ai-vault-search/session-search-engine.test.ts +++ b/src/main/ai-vault-search/session-search-engine.test.ts @@ -395,6 +395,34 @@ describe('the engine carries its own schema and puts it back', () => { }) describe('a query the engine had to cut says so', () => { + it('cuts the query on a whole code point, never through a surrogate pair', async () => { + // A bare slice at 512 can land between the two halves of an astral + // character, and the lone half matches nothing and cannot be echoed back. + const { db, engine } = await open('ss-engine-surrogate-cap') + addSyntheticSession(db, { id: 1, text: 'needle' }) + const query = `${'x'.repeat(SESSION_SEARCH_QUERY_MAX_LENGTH - 1)}😀 needle` + const result = engine.search({ query }) + expect(result.truncated.query).toBe(true) + // The emoji straddles the cap, so the cut has to fall before it. + expect(query.slice(0, SESSION_SEARCH_QUERY_MAX_LENGTH).at(-1)).toBe('\ud83d') + expect(result.hits).toEqual([]) + }) + + it('loads more candidate sessions than SQLite will bind in one statement', async () => { + // The id list is as long as the candidate limit and every id is a bound + // parameter, so one statement is a raised limit away from `too many SQL + // variables` on a host whose SQLite caps at 999. + const { db, engine } = await open('ss-engine-id-batching', { + sessionCandidateLimit: 1200 + }) + for (let id = 1; id <= 1100; id++) { + addSyntheticSession(db, { id, text: 'needle' }) + } + const result = engine.search({ query: 'needle', limit: 5 }) + expect(result.hits).toHaveLength(5) + expect(result.truncated.candidates).toBe(false) + }) + it('reports truncation when the planner drops terms past its cap', async () => { // The 56th term is the only one that matches. Without the flag this is a // confident empty answer to a query the engine never finished reading. diff --git a/src/main/ai-vault-search/session-search-engine.ts b/src/main/ai-vault-search/session-search-engine.ts index 3dfde89e6cb..4b6e11e407d 100644 --- a/src/main/ai-vault-search/session-search-engine.ts +++ b/src/main/ai-vault-search/session-search-engine.ts @@ -1,5 +1,6 @@ import type SyncDatabase from '../sqlite/sync-database' import type { TranscriptMessageRole } from '../ai-vault/session-transcript-consumers' +import { sliceAtCodeUnitLimit } from '../ai-vault/session-scanner-text-normalization' import { hasAiVaultSearchQueryOperators, splitAiVaultSearchQuery, @@ -124,7 +125,9 @@ export class SessionSearchEngine { const generation = readIndexGeneration(this.db) const scope = request.scope ?? 'all' const sort = request.filters?.sort ?? 'relevance' - const capped = request.query.slice(0, SESSION_SEARCH_QUERY_MAX_LENGTH) + // Not a bare `slice`: cutting between a surrogate pair leaves a lone half + // that no tokenizer can match and that a caller cannot echo back. + const capped = sliceAtCodeUnitLimit(request.query, SESSION_SEARCH_QUERY_MAX_LENGTH) const split = splitAiVaultSearchQuery(capped) const retrievalScope: RetrievalScope = { scope, diff --git a/src/main/ai-vault-search/session-search-retrieval.ts b/src/main/ai-vault-search/session-search-retrieval.ts index 20a32b27476..346f99c677e 100644 --- a/src/main/ai-vault-search/session-search-retrieval.ts +++ b/src/main/ai-vault-search/session-search-retrieval.ts @@ -14,6 +14,9 @@ import { SessionSearchTypoRepair } from './session-search-typo-repair' // The operator-only walk: rows per page, and how far past a full candidate set // it will read before giving up on finding more matches. const RECENT_PAGE_ROWS = 512 +// Ids per `loadSessions` statement, left well under the 999-parameter floor so +// the filter's own bound values fit beside them. +const SESSION_ID_BATCH = 500 const RECENT_SCAN_FACTOR = 20 // Measured: user 3 / assistant 2 / tool 1 / identifiers 1 (MRR 0.503 vs 0.475 flat). @@ -160,14 +163,24 @@ export class SessionSearchRetrieval { return { sessions, incomplete: incomplete || sessions.length >= scope.candidateLimit } } + /** + * Read in batches, because the id list is as long as the candidate limit and + * every id is a bound parameter. SQLite's default `SQLITE_MAX_VARIABLE_NUMBER` + * is 999 on builds older than 3.32, and a caller may raise the candidate + * limit — the tuning doc says it may — so a single statement is one settings + * change away from `too many SQL variables` on somebody's host. + */ loadSessions(ids: readonly number[], scope: RetrievalScope): SessionRow[] { - if (ids.length === 0) { - return [] + const rows: SessionRow[] = [] + for (let start = 0; start < ids.length; start += SESSION_ID_BATCH) { + const batch = ids.slice(start, start + SESSION_ID_BATCH) + const conditions = [`id IN (${batch.map(() => '?').join(',')})`, ...scope.filter.conditions] + rows.push( + ...(this.db + .prepare(`SELECT * FROM sessions WHERE ${conditions.join(' AND ')}`) + .all(...batch, ...scope.filter.values) as SessionRow[]) + ) } - const conditions = [`id IN (${ids.map(() => '?').join(',')})`, ...scope.filter.conditions] - const rows = this.db - .prepare(`SELECT * FROM sessions WHERE ${conditions.join(' AND ')}`) - .all(...ids, ...scope.filter.values) as SessionRow[] return rows.filter((row) => scope.matchesOperators(row)) }