mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 16:02:35 +00:00
fix(ai-vault-search): cut a query on a code point and bind ids in batches
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user