Skip to content

Commit a7bdc75

Browse files
committed
improvement(knowledge): admit before embedding, budget the fill check, pass over emptied slices
1 parent a462b7d commit a7bdc75

7 files changed

Lines changed: 98 additions & 57 deletions

File tree

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,12 @@ async function exactProjectionNeighbors(vector: number[], limit: number, readerC
134134
JSON.stringify(vector),
135135
'text-embedding-3-small'
136136
)
137-
return db.execute<{ id: string }>(sql`SELECT s.id FROM ${embeddingSearch} s
138-
INNER JOIN document d ON d.id = s.document_id
139-
WHERE s.knowledge_base_id = ${ids.knowledgeBaseId} AND s.enabled ${readerClause ?? sql``}
140-
ORDER BY (${distance}) + 0, s.id
137+
/** Unaliased: the distance expression qualifies its column with the table's own name. */
138+
return db.execute<{ id: string }>(sql`SELECT ${embeddingSearch.id} AS id FROM ${embeddingSearch}
139+
INNER JOIN document d ON d.id = ${embeddingSearch.documentId}
140+
WHERE ${embeddingSearch.knowledgeBaseId} = ${ids.knowledgeBaseId}
141+
AND ${embeddingSearch.enabled} ${readerClause ?? sql``}
142+
ORDER BY (${distance}) + 0, ${embeddingSearch.id}
141143
LIMIT ${limit}`)
142144
}
143145
const captured: CapturedQuery[] = []
@@ -1197,7 +1199,7 @@ describe.skipIf(!enabled)('Knowledge search latency on a realistic indexed corpu
11971199
const expected = await exactProjectionNeighbors(
11981200
queryVector,
11991201
actual.length,
1200-
sql`AND s.document_id IN (${sql.join(
1202+
sql`AND ${embeddingSearch.documentId} IN (${sql.join(
12011203
documentIds.map((id) => sql`${id}`),
12021204
sql`, `
12031205
)})`

‎apps/sim/lib/knowledge/application/search.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,8 @@ describe('knowledge search application use case', () => {
356356
).rejects.toThrow('Search is not enabled for this organization')
357357
expect(mocks.recordActivity).not.toHaveBeenCalled()
358358
expect(mocks.requireOrganizationSearch).toHaveBeenCalledExactlyOnceWith('org-canonical')
359-
/** The gate runs beside the embedding call; billing and retrieval still never start after a refusal. */
360359
expect(mocks.resolveBilling).not.toHaveBeenCalled()
360+
expect(mocks.generateEmbedding).not.toHaveBeenCalled()
361361
expect(mocks.executeSearch).not.toHaveBeenCalled()
362362
})
363363

‎apps/sim/lib/knowledge/application/search.ts‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,9 @@ const searchKnowledgeUseCase = defineAuthorizedKnowledgeUseCase({
285285
)
286286
/**
287287
* Whether the organization may search at all, and whether this payer still may: neither
288-
* depends on the query, so both run beside the embedding call below instead of ahead of it.
289-
* A refusal still ends the search before any retrieval.
288+
* depends on the query, so both run beside the scope and defaults reads below instead of
289+
* ahead of them. Admission stays ahead of the embedding call, which a refused search must
290+
* never make.
290291
*/
291292
const admit = async (): Promise<BillingAttributionSnapshot | undefined> => {
292293
if (context.organizationId)
@@ -368,41 +369,41 @@ const searchKnowledgeUseCase = defineAuthorizedKnowledgeUseCase({
368369
: undefined
369370
const resultSecretRegistry = preparedRegistry ?? input.resultSecretRegistry
370371
input.signal?.throwIfAborted()
371-
const [queryEmbedding, access, searchDefaults, billingAttribution, tagDefinitions] =
372-
await Promise.all([
373-
hasQuery
374-
? measureSearchStage('embedding', () =>
375-
runWithKnowledgeModelInputProvenance(resultSecretRegistry, () =>
376-
generateSearchEmbedding(
377-
input.query!,
378-
embeddingTarget!,
379-
context.workspaceId,
380-
input.signal
381-
)
382-
)
383-
)
384-
: Promise.resolve(null),
385-
measureSearchStage('access_scope', () => context.access.get()),
386-
measureSearchStage('defaults', () =>
387-
resolveKnowledgeSearchDefaults({
388-
workspaceId: context.workspaceId,
389-
organizationId: context.organizationId,
372+
const [access, searchDefaults, billingAttribution, tagDefinitions] = await Promise.all([
373+
measureSearchStage('access_scope', () => context.access.get()),
374+
measureSearchStage('defaults', () =>
375+
resolveKnowledgeSearchDefaults({
376+
workspaceId: context.workspaceId,
377+
organizationId: context.organizationId,
390378

391-
/** The signed-in person, if any; never the billing owner or a key's creator. */
392-
userId: resolvePrincipalSubjectUserId(principal) ?? undefined,
393-
requestedMode: input.searchMode,
394-
})
395-
),
396-
admit(),
397-
/** The tag names the results are labelled with depend on the bases alone. */
398-
filters.length === 0
399-
? measureSearchStage('tag_definitions', () =>
400-
getDocumentTagDefinitionsByKnowledgeBaseIds(knowledgeBaseIds)
401-
)
402-
: Promise.resolve(definitionsByKnowledgeBase),
403-
])
379+
/** The signed-in person, if any; never the billing owner or a key's creator. */
380+
userId: resolvePrincipalSubjectUserId(principal) ?? undefined,
381+
requestedMode: input.searchMode,
382+
})
383+
),
384+
admit(),
385+
/** The tag names the results are labelled with depend on the bases alone. */
386+
filters.length === 0
387+
? measureSearchStage('tag_definitions', () =>
388+
getDocumentTagDefinitionsByKnowledgeBaseIds(knowledgeBaseIds)
389+
)
390+
: Promise.resolve(definitionsByKnowledgeBase),
391+
])
404392
definitionsByKnowledgeBase = tagDefinitions
405393
input.signal?.throwIfAborted()
394+
const queryEmbedding = hasQuery
395+
? await measureSearchStage('embedding', () =>
396+
runWithKnowledgeModelInputProvenance(resultSecretRegistry, () =>
397+
generateSearchEmbedding(
398+
input.query!,
399+
embeddingTarget!,
400+
context.workspaceId,
401+
input.signal
402+
)
403+
)
404+
)
405+
: null
406+
input.signal?.throwIfAborted()
406407
annotateSearchDiagnostics({
407408
accessScopeKind: access.kind,
408409
searchMode: searchDefaults.searchMode,

‎apps/sim/lib/knowledge/search/diagnostics.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ export type SearchStage =
5151
| 'vector.settings'
5252
| 'vector.probe'
5353
| 'vector.page'
54+
| 'vector.projection_filled'
55+
| 'keyword.projection_filled'
5456
| 'vector.exact_candidates'
5557
| 'vector.exact'
5658
| 'vector.candidate_search'

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,21 @@ describe('workspace-scoped vector retrieval', () => {
559559
expect(JSON.stringify(dbChainMockFns.leftJoin.mock.calls)).toContain('embeddingSearch')
560560
})
561561

562+
it('passes over a slice whose documents went away instead of ending the pool there', async () => {
563+
const execute = dbChainMockFns.execute.getMockImplementation()!
564+
let pages = 0
565+
dbChainMockFns.execute.mockImplementation(async (query) => {
566+
const statement = render(query)
567+
/** The first slice's documents are gone; the next slice still has the readable rows. */
568+
if (isPageStatement(statement.sql)) return pages++ === 0 ? [] : ranked
569+
return execute(query)
570+
})
571+
queueTableRows(schemaMock.embedding, [...ranked].reverse())
572+
expect((await handleVectorOnlySearch(params)).map((row) => row.id)).toEqual(['near', 'far'])
573+
expect(pages).toBe(2)
574+
expect(statements().filter((query) => isWalk(query.sql))).toHaveLength(1)
575+
})
576+
562577
it('sizes the pool to the pages asked for, doubling a pool the pages outran', () => {
563578
expect(vectorCandidatePoolLimit(20, undefined)).toBe(200)
564579
expect(vectorCandidatePoolLimit(150, undefined)).toBe(300)

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

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,20 @@ const PROJECTION_FILLED_TTL_MS = 60_000
117117
* Whether the ranking projection still holds rows the backfill has not filled. Read off the
118118
* unfilled-rows index in microseconds and remembered briefly: the answer only ever changes once.
119119
*/
120-
const projectionFilled = new LRUCache<ProjectionSourceAclTable, boolean>({
120+
const projectionFilled = new LRUCache<
121+
ProjectionSourceAclTable,
122+
boolean,
123+
{ budget: SearchBudget | undefined; stage: SearchStage }
124+
>({
121125
max: PROJECTION_SOURCE_ACL_TABLES.length,
122126
ttl: PROJECTION_FILLED_TTL_MS,
123-
fetchMethod: async (projection) => {
127+
/** The read that misses the cache spends the leg's own budget, like every other read of the leg. */
128+
fetchMethod: async (projection, _stale, { context }) => {
124129
const table = projection === 'embedding_search' ? embeddingSearch : embeddingKeywordTin
125-
const [row] = await db.execute<{ unfilled: boolean }>(sql`
130+
const [row] = await runSearchQuery(context.budget, context.stage, (executor) =>
131+
executor.execute<{ unfilled: boolean }>(sql`
126132
SELECT EXISTS (SELECT 1 FROM ${table} WHERE ${table.acl} IS NULL) AS unfilled`)
133+
)
127134
return !row?.unfilled
128135
},
129136
})
@@ -1770,7 +1777,11 @@ async function selectVectorResults(params: SearchParams): Promise<SearchResult[]
17701777
}
17711778
let selected: Array<{ id: string }>
17721779
const plan = params.access.kind === 'user' ? params.accessPlan : undefined
1773-
const filled = plan ? ((await projectionFilled.fetch('embedding_search')) ?? false) : false
1780+
const filled = plan
1781+
? ((await projectionFilled.fetch('embedding_search', {
1782+
context: { budget: params.budget, stage: 'vector.projection_filled' },
1783+
})) ?? false)
1784+
: false
17741785
/**
17751786
* A source the caller is a member of that has its own index is walked on its own, which
17761787
* beats ranking it exactly once it is large enough to have earned that index.
@@ -1891,29 +1902,32 @@ async function selectVectorResults(params: SearchParams): Promise<SearchResult[]
18911902
vectorCandidateScan: selected.length < candidateLimit ? 'underfilled' : 'planned',
18921903
})
18931904
}
1894-
const slice = candidatePool.ids.slice(offset, offset + limit)
1895-
if (!slice.length) return { candidates: [], nextOffset: offset }
18961905
/**
18971906
* The walk's order is the page's order: it ranked on the stored halfvec, and rescoring the
18981907
* pool against the original vectors read one out-of-line vector per candidate from storage
18991908
* no cache holds, seconds on a query nobody had run before. Only the page's identities are
1900-
* read here; the full read predicate follows at hydration, as before.
1909+
* read here; the full read predicate follows at hydration, as before. A slice whose
1910+
* documents all went away since the walk is passed over, not mistaken for the pool's end.
19011911
*/
1902-
const ranked = new Map(slice.map((candidate, index) => [candidate.id, index]))
1903-
const identities = await runSearchQuery(params.budget, 'vector.page', (executor) =>
1904-
executor.execute<SearchReadCandidate>(sql`
1912+
for (let start = offset; start < candidatePool.ids.length; start += limit) {
1913+
const slice = candidatePool.ids.slice(start, start + limit)
1914+
const ranked = new Map(slice.map((candidate, index) => [candidate.id, index]))
1915+
const identities = await runSearchQuery(params.budget, 'vector.page', (executor) =>
1916+
executor.execute<SearchReadCandidate>(sql`
19051917
SELECT ${embeddingSearch.id} AS id, ${document.id} AS "documentId",
19061918
${document.connectorId} AS "connectorId"
19071919
FROM ${embeddingSearch}
19081920
INNER JOIN ${document} ON ${document.id} = ${embeddingSearch.documentId}
19091921
WHERE ${embeddingSearch.id} = ANY(${textArrayLiteral(slice.map((candidate) => candidate.id))})
19101922
`)
1911-
)
1912-
const page = [...identities].sort((a, b) => (ranked.get(a.id) ?? 0) - (ranked.get(b.id) ?? 0))
1913-
return {
1914-
candidates: page,
1915-
nextOffset: offset + page.length,
1923+
)
1924+
if (!identities.length) continue
1925+
const page = [...identities].sort(
1926+
(a, b) => (ranked.get(a.id) ?? 0) - (ranked.get(b.id) ?? 0)
1927+
)
1928+
return { candidates: page, nextOffset: start + slice.length }
19161929
}
1930+
return { candidates: [], nextOffset: candidatePool.ids.length }
19171931
},
19181932
hydrate: (ids, authorized) =>
19191933
hydrateSearchCandidates(
@@ -2027,7 +2041,9 @@ export async function executeKeywordSearch(params: KeywordSearchParams): Promise
20272041
/** A filled projection decides readability on the ranked row alone; none of its rows needs the document. */
20282042
const tinFilled =
20292043
accessPlan && tinQuery
2030-
? ((await projectionFilled.fetch('embedding_keyword_tin')) ?? false)
2044+
? ((await projectionFilled.fetch('embedding_keyword_tin', {
2045+
context: { budget: params.budget, stage: 'keyword.projection_filled' },
2046+
})) ?? false)
20312047
: false
20322048
/** The projection predicate over the ranked CTE's mirrored columns, plus any excluded source. */
20332049
const onRowKeywordVisibility = (excludedSources: readonly string[]) =>

‎apps/sim/lib/navigation/organization-rollout.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@ vi.mock('@/lib/billing/core/access', () => ({
2222
isOrganizationBillingBlocked: vi.fn().mockResolvedValue(false),
2323
}))
2424

25-
import { requireOrganizationSearchAvailable } from '@/lib/knowledge/access/availability'
25+
import {
26+
forgetKnowledgeAccessAvailability,
27+
requireOrganizationSearchAvailable,
28+
} from '@/lib/knowledge/access/availability'
2629
import { resolveAppEntryPath } from '@/lib/navigation/resolve-app-entry'
2730

2831
afterAll(resetEnvFlagsMock)
2932

3033
describe('organization rollout during impersonation', () => {
3134
beforeEach(() => {
35+
/** Each case answers the same organization differently; the memo must not carry one across. */
36+
forgetKnowledgeAccessAvailability()
3237
vi.clearAllMocks()
3338
setEnvFlags({ isAppConfigEnabled: true, isHosted: true })
3439
mocks.landing.mockImplementation(async (userId: string) =>

0 commit comments

Comments
 (0)