Skip to content

Commit 23b7670

Browse files
authored
v0.8.46: keyword search optimizations
2 parents 1b96097 + 6904e58 commit 23b7670

17 files changed

Lines changed: 822 additions & 226 deletions

apps/sim/lib/knowledge/__integration__/search-latency.integration.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ async function sample(
477477
item.query.includes('CROSS JOIN LATERAL') ||
478478
isVectorCandidateQuery(item.query) ||
479479
item.query.includes('WITH scored_search_candidates') ||
480-
item.query.includes('WITH visible_keyword_documents'))
480+
item.query.includes('WITH matched_keyword_chunks'))
481481
)
482482
const plans: Array<
483483
CapturedQuery & {
@@ -550,7 +550,7 @@ async function sample(
550550
assertIndexedCandidates(parsedPlan[0].Plan, diagnostics.vectorCandidateLimit!, width)
551551
}
552552
}
553-
if (query.query.includes('WITH visible_keyword_documents')) {
553+
if (query.query.includes('WITH matched_keyword_chunks')) {
554554
assertScalarKeywordSorts(parsedPlan[0].Plan)
555555
}
556556
}

apps/sim/lib/knowledge/search/queries.test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,7 @@ describe('live repository authorization follows ranked candidates', () => {
884884
const statement = render(query).sql
885885
if (statement.includes('AS visible')) return candidatePages.shift() ?? []
886886
if (statement.includes('WITH scored_search_candidates')) return rerankPages.shift() ?? []
887-
if (statement.includes('WITH visible_keyword_documents')) return keywordPages.shift() ?? []
887+
if (statement.includes('WITH matched_keyword_chunks')) return keywordPages.shift() ?? []
888888
if (isExactRanking(statement)) return exactPages.shift() ?? []
889889
if (statement.includes('AS id FROM')) return probePages.shift() ?? []
890890
return []
@@ -1102,8 +1102,8 @@ describe('live repository authorization follows ranked candidates', () => {
11021102
expect(getForConnectors).toHaveBeenCalledWith(['allowed-source'], undefined)
11031103
if (mode === 'keyword') {
11041104
const ranking = render(dbChainMockFns.execute.mock.calls[0][0]).sql
1105-
expect(ranking).toContain('scored_keyword_candidates AS MATERIALIZED')
1106-
expect(ranking).toContain('ORDER BY keyword_rank DESC, id LIMIT')
1105+
expect(ranking).toContain('matched_keyword_chunks AS MATERIALIZED')
1106+
expect(ranking).toContain('ORDER BY keyword_rank DESC, matched_keyword_chunks.id')
11071107
expect(ranking).not.toContain('<=>')
11081108
expect(ranking).not.toContain('"content"')
11091109
} else if (mode === 'tags') {
@@ -1240,6 +1240,22 @@ describe('live repository authorization follows ranked candidates', () => {
12401240
expect(refillPredicate).toContain('revoked-source')
12411241
})
12421242

1243+
it('matches keyword chunks before the visibility predicate and ranks only what survives it', async () => {
1244+
keywordPages.push([candidate('selected', 'allowed-source')])
1245+
queueTableRows(schemaMock.embedding, [{ id: 'selected', content: 'verified result' }])
1246+
await executeKeywordSearch({ ...params, query: 'release', queryVector: params.queryVector! })
1247+
const ranking = render(dbChainMockFns.execute.mock.calls[0][0]).sql
1248+
const matched = ranking.indexOf('matched_keyword_chunks AS MATERIALIZED')
1249+
const visible = ranking.indexOf('visible_keyword_documents AS MATERIALIZED')
1250+
expect(matched).toBeGreaterThanOrEqual(0)
1251+
expect(visible).toBeGreaterThan(matched)
1252+
expect(ranking.slice(matched, visible)).not.toContain('keyword_rank')
1253+
expect(ranking.slice(visible)).toContain('FROM matched_keyword_chunks INNER JOIN')
1254+
/** The predicate fragments are parameterized, so the restriction is read off the query tree. */
1255+
const fragments = JSON.stringify(dbChainMockFns.execute.mock.calls[0][0])
1256+
expect(fragments).toContain('= ANY (ARRAY(SELECT document_id FROM matched_keyword_chunks))')
1257+
})
1258+
12431259
it('recomputes keyword candidates after excluding a revoked source and rechecks content access', async () => {
12441260
getForConnectors.mockResolvedValueOnce(identity)
12451261
keywordPages.push(

apps/sim/lib/knowledge/search/queries.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,19 @@ export interface KeywordSearchParams {
11001100
* with `topK` (measured at ~59x the buffer reads on a 20k-chunk base for a term
11011101
* matching every row). Ranking therefore touches no vectors, and only the rows
11021102
* that survive the limit are hydrated.
1103+
*
1104+
* The live-scope ranking query runs in three stages: match, authorize, rank. The
1105+
* visibility predicate carries correlated subqueries — one per connector, one per
1106+
* search-integration decision — so evaluating it across a base ahead of the query costs a table
1107+
* pass priced by how many documents the base holds rather than by how many the query matched.
1108+
* Matching first restricts that predicate to the documents the query actually matched.
1109+
*
1110+
* Two details keep that ordering from paying the saving back. Restricting the predicate with
1111+
* `document.id = ANY (...)` rather than a subquery keeps the narrowed lookup on a bitmap scan,
1112+
* which prefetches, where a plain `IN (SELECT ...)` plans as an index walk that does not. And
1113+
* the match stage carries identifiers only: ranking every match rather than every *visible*
1114+
* match would detoast one text-search vector per match, which on a mid-frequency term costs
1115+
* more than the pass it replaces.
11031116
*/
11041117
export async function executeKeywordSearch(params: KeywordSearchParams): Promise<SearchResult[]> {
11051118
const { knowledgeBaseIds, topK, query, queryVector, structuredFilters, access } = params
@@ -1133,38 +1146,41 @@ export async function executeKeywordSearch(params: KeywordSearchParams): Promise
11331146
selectPage: async (limit, offset, excludedSources) => {
11341147
const candidates = await runSearchQuery(params.budget, 'keyword.sql', (executor) =>
11351148
executor.execute<SearchReadCandidate>(sql`
1136-
WITH visible_keyword_documents AS MATERIALIZED (
1137-
SELECT ${document.id} AS id FROM ${document}
1138-
WHERE ${and(
1139-
inArray(document.knowledgeBaseId, knowledgeBaseIds),
1140-
...getDocumentVisibilityConditions(
1141-
access,
1142-
params.filters,
1143-
knowledgeMetadataCandidateAccessCondition(access)
1144-
),
1145-
excludeSearchSources(excludedSources)
1146-
)}
1147-
), scored_keyword_candidates AS MATERIALIZED (
1149+
WITH matched_keyword_chunks AS MATERIALIZED (
11481150
SELECT ${embeddingKeywordSearch.id} AS id,
1149-
${embeddingKeywordSearch.documentId} AS document_id,
1150-
${candidateRank} AS keyword_rank
1151+
${embeddingKeywordSearch.documentId} AS document_id
11511152
FROM ${embeddingKeywordSearch}
11521153
WHERE ${and(
11531154
inArray(embeddingKeywordSearch.knowledgeBaseId, knowledgeBaseIds),
11541155
eq(embeddingKeywordSearch.enabled, true),
11551156
sql`${embeddingKeywordSearch.contentTsv} @@ ${tsQuery}`,
1156-
sql`${embeddingKeywordSearch.documentId} IN (SELECT id FROM visible_keyword_documents)`,
1157-
sql`EXISTS (SELECT 1 FROM visible_keyword_documents)`,
11581157
tagFilterConditions.length
11591158
? sql`EXISTS (
11601159
SELECT 1 FROM ${embedding} WHERE ${embedding.id} = ${embeddingKeywordSearch.id}
11611160
AND ${and(...tagFilterConditions)}
11621161
)`
11631162
: undefined
11641163
)}
1164+
), visible_keyword_documents AS MATERIALIZED (
1165+
SELECT ${document.id} AS id FROM ${document}
1166+
WHERE ${and(
1167+
inArray(document.knowledgeBaseId, knowledgeBaseIds),
1168+
sql`${document.id} = ANY (ARRAY(SELECT document_id FROM matched_keyword_chunks))`,
1169+
...getDocumentVisibilityConditions(
1170+
access,
1171+
params.filters,
1172+
knowledgeMetadataCandidateAccessCondition(access)
1173+
),
1174+
excludeSearchSources(excludedSources)
1175+
)}
11651176
), ranked_keyword_candidates AS MATERIALIZED (
1166-
SELECT * FROM scored_keyword_candidates
1167-
ORDER BY keyword_rank DESC, id LIMIT ${limit} OFFSET ${offset}
1177+
SELECT matched_keyword_chunks.id, matched_keyword_chunks.document_id,
1178+
${candidateRank} AS keyword_rank
1179+
FROM matched_keyword_chunks INNER JOIN ${embeddingKeywordSearch}
1180+
ON ${embeddingKeywordSearch.id} = matched_keyword_chunks.id
1181+
WHERE matched_keyword_chunks.document_id IN (SELECT id FROM visible_keyword_documents)
1182+
ORDER BY keyword_rank DESC, matched_keyword_chunks.id
1183+
LIMIT ${limit} OFFSET ${offset}
11681184
)
11691185
SELECT ranked_keyword_candidates.id, ${document.id} AS "documentId",
11701186
${document.connectorId} AS "connectorId",
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/** @vitest-environment node */
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
const mocks = vi.hoisted(() => ({
5+
load: vi.fn(),
6+
permission: vi.fn(),
7+
search: vi.fn(),
8+
folders: vi.fn(),
9+
}))
10+
vi.mock('@sim/platform-authz/workspace', () => ({
11+
permissionSatisfies: (actual: string | null) => actual !== null,
12+
resolveEffectiveWorkspacePermission: mocks.permission,
13+
}))
14+
vi.mock('@/lib/uploads/contexts/workspace', () => ({ loadActiveWorkspaceContext: mocks.load }))
15+
vi.mock('@/lib/workspace-files/search/repository', () => ({
16+
searchWorkspaceFileIndex: mocks.search,
17+
}))
18+
vi.mock('@/lib/workspace-files/resolve-folder-scope', () => ({
19+
resolveWorkspaceFolderScope: mocks.folders,
20+
}))
21+
22+
import { searchWorkspaceFileContent } from '@/lib/workspace-files/application/search-workspace-file-content'
23+
24+
const principal = { kind: 'session', userId: 'user-1', sessionId: 'session-1' } as const
25+
const input = {
26+
workspaceId: 'workspace-1',
27+
query: 'needle',
28+
mode: 'exact',
29+
maxResults: 10,
30+
} as const
31+
32+
describe('searchWorkspaceFileContent cancellation', () => {
33+
beforeEach(() => {
34+
vi.clearAllMocks()
35+
mocks.load.mockResolvedValue({
36+
workspaceId: 'workspace-1',
37+
workspaceOrganizationId: null,
38+
allowPersonalApiKeys: true,
39+
billedAccountUserId: 'user-1',
40+
})
41+
mocks.permission.mockResolvedValue('read')
42+
mocks.search.mockResolvedValue({ results: [] })
43+
})
44+
45+
it.each(['request', 'input'] as const)(
46+
'propagates the %s signal through the authorized application operation',
47+
async (source) => {
48+
const controller = new AbortController()
49+
await searchWorkspaceFileContent.execute({
50+
principal,
51+
input: { ...input, ...(source === 'input' ? { signal: controller.signal } : {}) },
52+
request: {
53+
headers: new Headers(),
54+
...(source === 'request' ? { signal: controller.signal } : {}),
55+
},
56+
})
57+
expect(mocks.search).toHaveBeenCalledWith(
58+
expect.objectContaining({ signal: controller.signal })
59+
)
60+
}
61+
)
62+
63+
it('does not resolve folders or enqueue database work for a cancelled HTTP request', async () => {
64+
const signal = AbortSignal.abort(new Error('cancelled'))
65+
await expect(
66+
searchWorkspaceFileContent.execute({
67+
principal,
68+
input: { ...input, folderPaths: ['/notes'] },
69+
request: { headers: new Headers(), signal },
70+
})
71+
).rejects.toBe(signal.reason)
72+
expect(mocks.folders).not.toHaveBeenCalled()
73+
expect(mocks.search).not.toHaveBeenCalled()
74+
})
75+
})

apps/sim/lib/workspace-files/application/search-workspace-file-content.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,13 @@ import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace'
33
import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case'
44
import { fileOperations } from '@/lib/workspace-files/application/operations'
55
import { resolveWorkspaceFolderScope } from '@/lib/workspace-files/resolve-folder-scope'
6+
import { WorkspaceFileSearchUnavailableError } from '@/lib/workspace-files/search/errors'
67
import {
78
compileFileSearchPattern,
89
type FileSearchMode,
910
FileSearchPatternError,
1011
} from '@/lib/workspace-files/search/pattern'
11-
import {
12-
searchWorkspaceFileIndex,
13-
WorkspaceFileSearchUnavailableError,
14-
} from '@/lib/workspace-files/search/repository'
12+
import { searchWorkspaceFileIndex } from '@/lib/workspace-files/search/repository'
1513

1614
export interface SearchWorkspaceFileContentInput {
1715
workspaceId: string
@@ -37,14 +35,16 @@ export const searchWorkspaceFileContent = defineAuthorizedWorkspaceFileUseCase({
3735
operation: fileOperations.searchContent,
3836
resolveContext: ({ input }: { input: SearchWorkspaceFileContentInput }) =>
3937
resolveSearchWorkspaceFileContext(input),
40-
execute: async ({ principal, input, context }) => {
41-
/*
38+
execute: async ({ principal, input, context, request }) => {
39+
const signal = input.signal ?? request?.signal
40+
signal?.throwIfAborted()
41+
/**
4242
* Resolved here rather than at the surface so every caller (the File
4343
* block, the v2 route) is confined by the same check. A folder tree
4444
* holding one subtree per user makes this scope the isolation boundary,
4545
* not a convenience filter.
4646
*/
47-
/*
47+
/**
4848
* `!== undefined`, not a length check: an explicitly empty list is a scope
4949
* that names no folder, which must match nothing. Treating it as "absent"
5050
* would answer a request for nothing with the whole workspace.
@@ -58,15 +58,15 @@ export const searchWorkspaceFileContent = defineAuthorizedWorkspaceFileUseCase({
5858
includeSubfolders: input.includeSubfolders,
5959
})
6060
: undefined
61-
input.signal?.throwIfAborted()
61+
signal?.throwIfAborted()
6262

6363
try {
6464
return await searchWorkspaceFileIndex({
6565
workspaceId: context.workspaceId,
6666
pattern: compileFileSearchPattern(input.query, input.mode),
6767
maxResults: input.maxResults,
6868
folderScope,
69-
signal: input.signal,
69+
signal,
7070
})
7171
} catch (error) {
7272
/**

apps/sim/lib/workspace-files/search/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ Search joins the current file revision and resolved workspace/folder scope. A re
2222

2323
For regular chunks, PostgreSQL checks the pattern with newline-aware semantics, then verifies individual logical lines. Long-line fragments use only necessary three-character literals as a conservative prefilter, including all required alternation branches. Two-code-point overlap preserves those literals at every boundary. PostgreSQL reconstructs the complete candidate line and evaluates the original regex, so anchors, word boundaries, repetitions, and arbitrarily long match spans retain line semantics. Fixed overlap alone is never treated as proof of a match. The supported regex grammar and minimum literal requirement are unchanged.
2424

25-
Regular blocks are verified in batches of at most 16 (128 KiB of indexed text); long lines are reconstructed one at a time. Only bounded match-centered previews leave PostgreSQL: at most 201 rows to detect truncation, and at most 2 KiB per rendered result. A single search has a ten-second application deadline with per-statement guards. PostgreSQL 17 additionally enforces a total transaction timeout; PostgreSQL 16 uses the compatible idle-transaction guard. Transaction advisory locks admit at most two simultaneous searches per workspace and ten globally per database. Busy and timed-out searches fail explicitly; they never report an incomplete scan as an authoritative empty result. The reader uses the normal application database connection, so admission is coordinated on the same database as the index.
25+
Regular blocks are verified in batches of at most 16 (128 KiB of indexed text); long lines are reconstructed one at a time. Only bounded match-centered previews leave PostgreSQL: at most 201 rows to detect truncation, and at most 2 KiB per rendered result. A single search has a fifteen-second caller deadline covering queueing, connection acquisition, and execution; SQL runs for at most ten seconds within that budget, with per-statement guards. PostgreSQL 17 additionally enforces a total transaction timeout; PostgreSQL 16 uses the compatible idle-transaction guard. Transaction advisory locks admit at most 20 simultaneous searches per workspace and 5,000 globally per database. Search transactions use the dedicated `dbFor('search')` primary pool, with five connections per process, so they cannot occupy the application or execution client pools. Before acquiring a connection, a process admits five active searches and at most 100 waiting requests, capped at 20 waiting requests per workspace so one burst cannot fill the entire queue. Queued workspaces rotate after each grant; waiting requests expire after five seconds or leave immediately on cancellation. The local active budget comes from the same pool profile as the driver. A slot is released only when the transaction settles, including errors. Cancellation reaches this boundary from both HTTP requests and File-tool execution. The existing deadline helper ends the caller’s wait promptly even when connection acquisition stalls. A late transaction checks cancellation before running search SQL; its active slot remains reserved until the driver settles, so a timed-out request cannot start replacement work on top of a still-running query. The driver and upstream pooler still own physical connection cleanup; the application deadline does not cancel a queued PostgreSQL protocol command. Busy and timed-out searches fail explicitly; they never report an incomplete scan as an authoritative empty result. These bounds apply identically to exact and regex search and do not change result or line-number semantics.
26+
27+
The 20/workspace and 5,000/global advisory ceilings bound admitted transactions across processes; they are not promises of simultaneous execution or throughput. Local queues absorb short bursts without holding database connections. They are not durable jobs or a fleet-wide fair scheduler. The dedicated client pool isolates connection ownership, not PostgreSQL CPU, memory, I/O, or an upstream PgBouncer server pool. Its default URL is the process primary URL; any `DATABASE_URL_SEARCH` override must target the same primary database so current revisions and advisory admission remain coherent. Independent PgBouncer server budgets require separate database/user pool configuration. Total client connections can increase by five per participating process. Before raising execution capacity, measure the number of processes, backend pool budget, queue wait/rejection rates, search latency, and database resource headroom under representative exact and broad-regex workloads. More queueing cannot increase sustained throughput.
2628

2729
Arbitrary regex cannot have a fixed latency guarantee. Common terms, broad alternatives, and punctuation-only literals may require scanning significant scoped text. Larger capacity decisions need representative query plans and workload measurements; neither a per-file byte cap nor a PostgreSQL row-count claim establishes a total corpus capacity.
2830

0 commit comments

Comments
 (0)