Skip to content

Commit 66c972d

Browse files
committed
improvement(knowledge): let an on-row walk run to its cap, widen a narrow reader's keyword window stepwise, apply scan settings with the deadline
- an on-row walk keeps walking, up to a 100k-tuple cap, until its limit is met; the separate wider walk and its diagnostic are gone - the on-row predicate tests the mirrored ACL alone — a member's source is no longer admitted whole, so a document re-owned after its chunk was mirrored is refused at the row - a reach count that ran out of time decides that search only; it is not remembered - a narrow reader ranks the narrowest keyword window first and widens to the wide one only when the page is short; resolved scopes leave the widest window short instead of ranking every match - the HNSW scan settings ride in the deadline statement, one round trip fewer per vector statement
1 parent 46e88eb commit 66c972d

6 files changed

Lines changed: 212 additions & 177 deletions

File tree

‎apps/sim/lib/knowledge/access/predicate.postgres.test.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,13 +600,28 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
600600
const onRow = new PgDialect().sqlToQuery(
601601
projectionCandidateAccessCondition(schema.embeddingSearch, scope, plan)
602602
)
603-
for (const id of [...cases.map(([documentId]) => documentId), 'upload-doc']) {
603+
const onRowAdmits = async (id: string) => {
604604
const rows = await connection.unsafe(
605605
`SELECT 1 FROM embedding_search WHERE ${onRow.sql} AND document_id = $${onRow.params.length + 1}`,
606606
[...(onRow.params as string[]), id]
607607
)
608-
if (await admits(perRow, id)) expect([id, rows.length > 0]).toEqual([id, true])
608+
return rows.length > 0
609+
}
610+
for (const id of [...cases.map(([documentId]) => documentId), 'upload-doc']) {
611+
if (await admits(perRow, id)) expect([id, await onRowAdmits(id)]).toEqual([id, true])
609612
}
613+
/**
614+
* A source the caller is a member of still holds documents that name other members only; the
615+
* mirrored ACL keeps those out of the ranking rather than leaving them for hydration to drop.
616+
*/
617+
await connection.unsafe(
618+
"INSERT INTO document(id, connector_id, acl, acl_verified_at) VALUES ('members-other', 'members', ARRAY['s:slack:-:bob'], statement_timestamp())"
619+
)
620+
await connection.unsafe(
621+
"INSERT INTO embedding_search SELECT id || '-chunk', id, connector_id, acl FROM document WHERE id = 'members-other'"
622+
)
623+
expect(await admits(perRow, 'members-other')).toBe(false)
624+
expect(await onRowAdmits('members-other')).toBe(false)
610625
/**
611626
* Candidate ranking defers the live source proof, as the per-row candidate predicate does: a
612627
* caller holds those grants only after authorization, so applying the clause during ranking

‎apps/sim/lib/knowledge/access/predicate.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,11 @@ export function knowledgeCandidateAccessConditionForConnectors(
307307
* resolved: `connectorId` and `acl` are mirrored there from the document, so a walk or a keyword
308308
* window decides readability on the row it scores instead of joining `document` per candidate.
309309
*
310-
* It admits a superset of the document predicate, never a subset: a member's source is admitted
311-
* whole (the observation that vouches for each document is checked at hydration), and requirement
312-
* clauses live on the document. Both are refused there, under the full predicate, before content is
313-
* returned — this predicate only decides what is worth ranking.
310+
* It admits a superset of the document predicate, never a subset: a mirrored ACL names the members
311+
* who observe a document, so overlap with the caller's tokens is the per-row test without the
312+
* observation's freshness, and requirement clauses live on the document. Both are refused there,
313+
* under the full predicate, before content is returned — this predicate only decides what is worth
314+
* ranking.
314315
*/
315316
export function projectionCandidateAccessCondition(
316317
projection: { connectorId: AnyPgColumn | SQL; acl: AnyPgColumn | SQL },
@@ -329,11 +330,8 @@ export function projectionCandidateAccessCondition(
329330
...plan.connectors.admin,
330331
...plan.connectors.members,
331332
]
332-
return sql`(
333-
${inSources(plan.memberSources)}
334-
OR (${projection.acl} && ${tokens}
335-
AND (${projection.connectorId} IS NULL OR ${inSources(mirrored)}))
336-
)`
333+
return sql`(${projection.acl} && ${tokens}
334+
AND (${projection.connectorId} IS NULL OR ${inSources(mirrored)}))`
337335
}
338336

339337
/**

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { db } from '@sim/db'
22
import { getPostgresErrorCode } from '@sim/utils/errors'
3-
import { sql } from 'drizzle-orm'
3+
import { type SQL, sql } from 'drizzle-orm'
44
import type { DbTransaction } from '@/lib/db/types'
55
import {
66
measureSearchStage,
@@ -67,7 +67,8 @@ export class SearchBudget {
6767
*/
6868
async query<T>(
6969
stage: SearchStage,
70-
run: (executor: SearchExecutor) => PromiseLike<T>
70+
run: (executor: SearchExecutor) => PromiseLike<T>,
71+
settings: readonly SQL[] = []
7172
): Promise<T> {
7273
const started = performance.now()
7374
const remaining = this.remaining()
@@ -97,9 +98,16 @@ export class SearchBudget {
9798
if (expired) throw new SearchDeadlineError()
9899
recordSearchStageDuration(`${this.leg}.connection_acquire`, performance.now() - started)
99100
const timeout = String(this.remaining())
100-
/** Interactive retrieval cannot amortize compilation of the access predicates. */
101+
/**
102+
* Interactive retrieval cannot amortize compilation of the access predicates. A stage's
103+
* own session settings ride in the same statement: each is a round trip otherwise.
104+
*/
101105
await tx.execute(
102-
sql`SELECT set_config('statement_timeout', ${timeout}, true), set_config('jit', 'off', true)`
106+
sessionSettingsStatement([
107+
sql`set_config('statement_timeout', ${timeout}, true)`,
108+
sql`set_config('jit', 'off', true)`,
109+
...settings,
110+
])
103111
)
104112
this.remaining()
105113
return measureSearchStage(stage, () => run(tx))
@@ -120,6 +128,11 @@ export class SearchBudget {
120128
}
121129
}
122130

131+
/** One statement applying `set_config` fragments to the transaction they run in. */
132+
export function sessionSettingsStatement(settings: readonly SQL[]): SQL {
133+
return sql.join([sql`SELECT`, sql.join([...settings], sql`, `)], sql` `)
134+
}
135+
123136
/** Execute SQL with stage diagnostics and, for live search, the shared retrieval deadline. */
124137
export function runSearchQuery<T>(
125138
budget: SearchBudget | undefined,

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ export interface SearchDiagnosticMetadata {
107107
vectorSourcesSliced?: number
108108
/** The sliced sources held more readable documents than one exact ranking may enumerate. */
109109
vectorSlicedSaturated?: boolean
110-
/** The whole-graph walk a broad reach earned came back short, so its sources were searched instead. */
111-
vectorBroadWalkUnderfilled?: boolean
112110
vectorSourcesWalked?: number
113111
/**
114112
* Which index ranked an unbounded keyword leg: `tin` ranks by BM25 and checks access on the top

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

Lines changed: 114 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
handleVectorOnlySearch,
4141
type PermittedDocuments,
4242
resolvePermittedDocuments,
43+
resolveReach,
4344
retrieveKnowledgeSearch,
4445
type SearchParams,
4546
VECTOR_PROBE_DOCUMENT_LIMIT,
@@ -678,7 +679,19 @@ describe('workspace-scoped vector retrieval', () => {
678679
statements()
679680
.filter((query) => query.sql.includes('statement_timeout'))
680681
.map((query) => query.params[0])
681-
).toEqual(['100', '100', '40', '20'])
682+
).toEqual(['100', '40', '20'])
683+
})
684+
685+
it('applies the scan settings in the deadline statement rather than one of their own', async () => {
686+
queueTableRows(schemaMock.embedding, ranked)
687+
await handleVectorOnlySearch({
688+
...params,
689+
budget: new SearchBudget('vector', performance.now() + 8000),
690+
})
691+
const settings = statements().filter((query) => query.sql.includes('hnsw.iterative_scan'))
692+
expect(settings.length).toBeGreaterThan(0)
693+
/** Each round trip is latency on the critical path; the settings never earn one alone. */
694+
for (const statement of settings) expect(statement.sql).toContain('statement_timeout')
682695
})
683696

684697
it('does not convert an unexpected candidate failure into partial retrieval', async () => {
@@ -1316,73 +1329,6 @@ describe('permitted-document planner', () => {
13161329
expect(statements().some((query) => query.sql.includes('WITH readable_chunks'))).toBe(false)
13171330
})
13181331

1319-
it('widens the walk of a broad caller whose whole-graph walk came back short', async () => {
1320-
const eligibility = { workspace: [], admin: ['other-src'], members: ['member-src'] }
1321-
indexedSourceRows = [{ name: 'idx', connectorId: 'member-src' }]
1322-
/** The graph walk finds one readable neighbour where the pool wants hundreds. */
1323-
traversedRows = [{ id: 'short-hit', distance: 0.4 }]
1324-
rerankRows = [hit('short-hit', 'member-src')]
1325-
queueTableRows(schemaMock.embedding, rerankRows)
1326-
await handleVectorOnlySearch({
1327-
...params,
1328-
topK: 2,
1329-
permitted: { kind: 'unbounded', broad: true },
1330-
accessPlan: {
1331-
connectors: eligibility,
1332-
observers: { confirmed: [{ id: 'm-1', connectorId: 'member-src' }], observed: [] },
1333-
memberSources: ['member-src'],
1334-
},
1335-
})
1336-
/** The same walk twice, the second with the wider scan; no source is searched on its own. */
1337-
const walks = statements().filter((query) => isWalk(query.sql))
1338-
expect(walks).toHaveLength(2)
1339-
expect(JSON.stringify(walks[1])).not.toContain('"right":"member-src"')
1340-
const settings = statements().filter((query) => query.sql.includes('hnsw.max_scan_tuples'))
1341-
expect(JSON.stringify(settings.at(-1))).toContain('100000')
1342-
expect(statements().some((query) => query.sql.includes('WITH readable_chunks'))).toBe(false)
1343-
})
1344-
1345-
it('keeps what a short broad walk found when the wider walk runs out of budget', async () => {
1346-
const eligibility = { workspace: [], admin: ['other-src'], members: ['member-src'] }
1347-
indexedSourceRows = [{ name: 'idx', connectorId: 'member-src' }]
1348-
traversedRows = [{ id: 'short-hit', distance: 0.4 }]
1349-
rerankRows = [hit('short-hit', 'member-src')]
1350-
queueTableRows(schemaMock.embedding, rerankRows)
1351-
/** The per-source search is cancelled by the server's statement timeout; the leg has time left. */
1352-
const budget = new SearchBudget('vector', performance.now() + 10_000)
1353-
const base = dbChainMockFns.execute.getMockImplementation()!
1354-
let walksSeen = 0
1355-
dbChainMockFns.execute.mockImplementation(async (query) => {
1356-
const statement = render(query).sql
1357-
if (isWalk(statement) && walksSeen++ > 0) {
1358-
throw Object.assign(new Error('canceling statement due to statement timeout'), {
1359-
code: '57014',
1360-
})
1361-
}
1362-
return base(query)
1363-
})
1364-
const rows = await handleVectorOnlySearch({
1365-
...params,
1366-
budget,
1367-
permitted: { kind: 'unbounded', broad: true },
1368-
accessPlan: {
1369-
connectors: eligibility,
1370-
observers: { confirmed: [{ id: 'm-1', connectorId: 'member-src' }], observed: [] },
1371-
memberSources: ['member-src'],
1372-
},
1373-
})
1374-
expect(rows.map((row) => row.id)).toEqual(['short-hit'])
1375-
/** The wider walk ran under a share of the leg's budget, not all of it. */
1376-
const timeouts = statements()
1377-
.filter((query) => query.sql.includes("'statement_timeout'"))
1378-
.map((query) =>
1379-
Number(query.params.find((param) => typeof param === 'string' && /^\d+$/.test(param)))
1380-
)
1381-
.filter((value) => Number.isFinite(value))
1382-
expect(Math.min(...timeouts)).toBeLessThanOrEqual(5000)
1383-
expect(Math.max(...timeouts)).toBeGreaterThan(5000)
1384-
})
1385-
13861332
it('walks an indexed source a bounded caller is a member of instead of ranking it exactly', async () => {
13871333
const eligibility = { workspace: [], admin: ['small-src'], members: ['member-src'] }
13881334
indexedSourceRows = [{ name: 'idx', connectorId: 'member-src' }]
@@ -1583,8 +1529,32 @@ describe('permitted-document planner', () => {
15831529
expect(statement).toContain('ranked_tin_chunks.acl')
15841530
})
15851531

1586-
it('takes one wide window for a narrow resolved scope and leaves a short page short', async () => {
1587-
tinPages = [{ ranked: 20_000, candidates: [hit('a', 'src-a')] }]
1532+
it('widens the window for a broad resolved scope whose first page came back short', async () => {
1533+
tinPages = [
1534+
{ ranked: 2000, candidates: [] },
1535+
{ ranked: 4000, candidates: [hit('b', 'src-a')] },
1536+
]
1537+
queueTableRows(schemaMock.embedding, [{ ...hit('b', 'src-a'), content: 'release notes' }])
1538+
const results = await keyword({
1539+
permitted: { kind: 'unbounded', broad: true },
1540+
accessPlan: {
1541+
connectors: { workspace: [], admin: ['src-a'], members: [] },
1542+
observers: { confirmed: [], observed: [] },
1543+
memberSources: [],
1544+
},
1545+
})
1546+
expect(results.map((row) => row.id)).toEqual(['b'])
1547+
const windows = tinStatements().map((query) => JSON.stringify(query))
1548+
expect(windows).toHaveLength(2)
1549+
expect(windows[0]).toContain('2000')
1550+
expect(windows[1]).toContain('10000')
1551+
})
1552+
1553+
it('widens a narrow resolved scope straight to its wide window and leaves a short page short', async () => {
1554+
tinPages = [
1555+
{ ranked: 2000, candidates: [] },
1556+
{ ranked: 20_000, candidates: [hit('a', 'src-a')] },
1557+
]
15881558
queueTableRows(schemaMock.embedding, [{ ...hit('a', 'src-a'), content: 'release notes' }])
15891559
const results = await keyword({
15901560
permitted: { kind: 'unbounded', broad: false },
@@ -1595,12 +1565,57 @@ describe('permitted-document planner', () => {
15951565
},
15961566
})
15971567
expect(results.map((row) => row.id)).toEqual(['a'])
1598-
/** One statement, at the widest window; nothing narrower first, and no ranking of every match after. */
1568+
/** The narrowest window, then the wide one; nothing between, and no ranking of every match after. */
1569+
const windows = tinStatements().map((query) => JSON.stringify(query))
1570+
expect(windows).toHaveLength(2)
1571+
expect(windows[0]).toContain('2000')
1572+
expect(windows[1]).toContain('20000')
1573+
expect(ginStatements()).toHaveLength(0)
1574+
/** Each window is ranked once for several pages' worth of readable rows, not once per page. */
1575+
expect(windows[1]).toContain('1000')
1576+
})
1577+
1578+
it('stops a narrow resolved scope at the narrowest window that fills its page', async () => {
1579+
const page = Array.from({ length: 20 }, (_, i) => hit(`k-${i}`, 'src-a'))
1580+
tinPages = [{ ranked: 2000, candidates: page }]
1581+
queueTableRows(
1582+
schemaMock.embedding,
1583+
page.map((row) => ({ ...row, content: 'release notes' }))
1584+
)
1585+
const results = await keyword({
1586+
topK: 20,
1587+
permitted: { kind: 'unbounded', broad: false },
1588+
accessPlan: {
1589+
connectors: { workspace: [], admin: ['src-a'], members: [] },
1590+
observers: { confirmed: [], observed: [] },
1591+
memberSources: [],
1592+
},
1593+
})
1594+
expect(results).toHaveLength(20)
1595+
/** Ranking costs grow with the window; a page the narrowest fills never pays for the wide one. */
15991596
expect(tinStatements()).toHaveLength(1)
1600-
expect(JSON.stringify(tinStatements()[0])).toContain('20000')
1597+
expect(JSON.stringify(tinStatements()[0])).toContain('2000')
1598+
expect(JSON.stringify(tinStatements()[0])).not.toContain('20000')
1599+
})
1600+
1601+
it('leaves a broad resolved scope short at its widest window instead of ranking every match', async () => {
1602+
tinPages = [
1603+
{ ranked: 2000, candidates: [] },
1604+
{ ranked: 10_000, candidates: [] },
1605+
{ ranked: 50_000, candidates: [hit('a', 'src-a')] },
1606+
]
1607+
queueTableRows(schemaMock.embedding, [{ ...hit('a', 'src-a'), content: 'release notes' }])
1608+
const results = await keyword({
1609+
permitted: { kind: 'unbounded', broad: true },
1610+
accessPlan: {
1611+
connectors: { workspace: [], admin: ['src-a'], members: [] },
1612+
observers: { confirmed: [], observed: [] },
1613+
memberSources: [],
1614+
},
1615+
})
1616+
expect(results.map((row) => row.id)).toEqual(['a'])
1617+
expect(tinStatements()).toHaveLength(3)
16011618
expect(ginStatements()).toHaveLength(0)
1602-
/** The window is ranked once for several pages' worth of readable rows, not once per page. */
1603-
expect(JSON.stringify(tinStatements()[0])).toContain('1000')
16041619
})
16051620

16061621
it('hydrates an oversized keyword page in slices and stops at the results it needs', async () => {
@@ -1777,6 +1792,31 @@ describe('permitted-document planner', () => {
17771792
expect(reachCounts()).toHaveLength(2)
17781793
})
17791794

1795+
it('does not remember a reach whose count ran out of time', async () => {
1796+
dbChainMockFns.execute.mockImplementation(async (query) => {
1797+
const statement = render(query).sql
1798+
if (statement.includes('EXPLAIN'))
1799+
return [{ 'QUERY PLAN': [{ Plan: { 'Plan Rows': 1_000_000 } }] }]
1800+
if (statement.includes(') reached'))
1801+
throw Object.assign(new Error('canceling statement due to statement timeout'), {
1802+
code: '57014',
1803+
})
1804+
return []
1805+
})
1806+
const budget = new SearchBudget('vector', performance.now() + 10_000)
1807+
const reachCounts = () => statements().filter((query) => query.sql.includes(') reached'))
1808+
const plan = await resolveReach(['org-index'], scope('timed-reach'), budget)
1809+
expect(plan).toEqual({ kind: 'unbounded', broad: true })
1810+
expect(reachCounts()).toHaveLength(1)
1811+
/** The next search counts again rather than trusting an answer that never came. */
1812+
await resolveReach(
1813+
['org-index'],
1814+
scope('timed-reach'),
1815+
new SearchBudget('vector', performance.now() + 10_000)
1816+
)
1817+
expect(reachCounts()).toHaveLength(2)
1818+
})
1819+
17801820
it('is remembered, so a broad caller skips the probe on the next search', async () => {
17811821
probeRows = [{ id: null, connectorId: null, saturated: true }]
17821822
const broad = scope('broad')

0 commit comments

Comments
 (0)