Skip to content

Commit d03d33e

Browse files
authored
fix(knowledge): give the reach count the probe's share of the deadline, not the leg's (#8111)
1 parent d2a4e47 commit d03d33e

2 files changed

Lines changed: 63 additions & 29 deletions

File tree

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2051,6 +2051,20 @@ describe('permitted-document planner', () => {
20512051
expect(reachCounts()).toHaveLength(2)
20522052
})
20532053

2054+
it('reports a leg whose own deadline passed during the count as short, not failed', async () => {
2055+
const budget = new SearchBudget('vector', performance.now() - 1)
2056+
await expect(
2057+
resolveReach(['org-index'], scope('spent-leg'), budget, {
2058+
connectors: { workspace: [], admin: [], members: [], liveProofRequired: [] },
2059+
observers: { confirmed: [], observed: [] },
2060+
memberSources: [],
2061+
connectorTypes: new Map(),
2062+
uploads: true,
2063+
})
2064+
).resolves.toEqual({ kind: 'unbounded', broad: true })
2065+
expect(budget.timedOut).toBe(true)
2066+
})
2067+
20542068
it('counts a resolved reach against a small index instead of assuming it broad', async () => {
20552069
/** A bound inside the probe limit proves nothing without a saturated probe. */
20562070
dbChainMockFns.execute.mockImplementation(async (query) => {
@@ -2075,9 +2089,10 @@ describe('permitted-document planner', () => {
20752089
)
20762090
expect(reach).toEqual({ kind: 'unbounded', broad: false })
20772091
expect(reachCounts()).toHaveLength(1)
2078-
/** The count is the search's own read: it runs inside the leg's deadline statement. */
2092+
/** The count is the search's own read, under the probe's share of the deadline, not the leg's. */
20792093
const countAt = statements().findIndex((query) => query.sql.includes(') reached'))
20802094
expect(statements()[countAt - 1].sql).toContain('statement_timeout')
2095+
expect(Number(statements()[countAt - 1].params[0])).toBeLessThanOrEqual(600)
20812096
})
20822097

20832098
it('does not remember a reach whose count ran out of time', async () => {
@@ -2102,6 +2117,8 @@ describe('permitted-document planner', () => {
21022117
})
21032118
expect(plan).toEqual({ kind: 'unbounded', broad: true })
21042119
expect(reachCounts()).toHaveLength(1)
2120+
/** Only the count's share of the deadline was spent; the leg is not the one that timed out. */
2121+
expect(budget.timedOut).toBe(false)
21052122
/** The next search counts again rather than trusting an answer that never came. */
21062123
await resolveReach(
21072124
['org-index'],

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

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,35 +1253,48 @@ async function estimateFilteredDocuments(
12531253
* documents. Counted once against that bound and remembered, so the first search after the
12541254
* window pays for it and the rest do not. A caller whose probe already saturated is known to
12551255
* reach past the probe's limit, so a bound inside that limit is met without counting.
1256+
*
1257+
* The count reads as many index entries as the caller reaches, so on a large index it can cost
1258+
* more than the leg it serves; it gets the probe's share of the deadline, never the whole leg's.
1259+
* A count that runs out of that share answers `null`: the leg keeps its time and its deadline
1260+
* intact, and the caller decides this search alone without remembering anything.
12561261
*/
12571262
async function reachIsBroad(
12581263
knowledgeBaseIds: string[],
12591264
access: KnowledgeAccessScope,
12601265
budget: SearchBudget | undefined,
12611266
plan: SearchAccessPlan | undefined,
12621267
saturated: boolean
1263-
): Promise<boolean> {
1268+
): Promise<boolean | null> {
12641269
if (access.kind !== 'user') return true
1265-
const total =
1266-
(await indexDocumentCounts.fetch([...knowledgeBaseIds].sort().join(','), {
1267-
context: budget,
1268-
})) ?? 0
1269-
const bound = Math.ceil(total * BROAD_REACH_SHARE)
1270-
if (saturated && bound <= VECTOR_PROBE_DOCUMENT_LIMIT) return true
1271-
const [row] = await runSearchQuery(budget, 'permitted_documents', (executor) =>
1272-
executor.execute<{ n: number }>(sql`
1273-
SELECT count(*) AS n FROM (
1274-
SELECT 1 FROM ${document}
1275-
WHERE ${and(
1276-
isNull(document.deletedAt),
1277-
knowledgeAclOverlapCondition(access),
1278-
inArray(document.knowledgeBaseId, knowledgeBaseIds),
1279-
planSourceCondition(plan)
1280-
)}
1281-
LIMIT ${bound}
1282-
) reached`)
1283-
)
1284-
return Number(row?.n ?? 0) >= bound
1270+
const countBudget = budget?.capped(VECTOR_PROBE_BUDGET_MS)
1271+
try {
1272+
const total =
1273+
(await indexDocumentCounts.fetch([...knowledgeBaseIds].sort().join(','), {
1274+
context: countBudget,
1275+
})) ?? 0
1276+
const bound = Math.ceil(total * BROAD_REACH_SHARE)
1277+
if (saturated && bound <= VECTOR_PROBE_DOCUMENT_LIMIT) return true
1278+
const [row] = await runSearchQuery(countBudget, 'permitted_documents', (executor) =>
1279+
executor.execute<{ n: number }>(sql`
1280+
SELECT count(*) AS n FROM (
1281+
SELECT 1 FROM ${document}
1282+
WHERE ${and(
1283+
isNull(document.deletedAt),
1284+
knowledgeAclOverlapCondition(access),
1285+
inArray(document.knowledgeBaseId, knowledgeBaseIds),
1286+
planSourceCondition(plan)
1287+
)}
1288+
LIMIT ${bound}
1289+
) reached`)
1290+
)
1291+
return Number(row?.n ?? 0) >= bound
1292+
} catch (error) {
1293+
if (!budget || !countBudget?.isTimeout(error)) throw error
1294+
/** Only the count's share was spent; the leg's own deadline still governs. */
1295+
budget.remaining()
1296+
return null
1297+
}
12851298
}
12861299

12871300
/**
@@ -1333,11 +1346,13 @@ export async function resolveReach(
13331346
if (remembered) return { kind: 'unbounded', broad: remembered.broad }
13341347
try {
13351348
const broad = await reachIsBroad(knowledgeBaseIds, access, budget, plan, false)
1349+
/** A count that ran out of time decides this search only; the next one counts again. */
1350+
if (broad === null) return { kind: 'unbounded', broad: true }
13361351
if (key) saturatedReach.set(key, { broad })
13371352
return { kind: 'unbounded', broad }
13381353
} catch (error) {
1354+
/** The leg's own deadline passed during the count: the leg is short, the search is not failed. */
13391355
if (!budget?.isTimeout(error)) throw error
1340-
/** A count that ran out of time decides this search only; the next one counts again. */
13411356
return { kind: 'unbounded', broad: true }
13421357
}
13431358
}
@@ -1393,17 +1408,21 @@ export async function resolvePermittedDocuments(params: {
13931408
}
13941409
if (probe.kind === 'saturated') {
13951410
try {
1396-
broad = await reachIsBroad(
1411+
const counted = await reachIsBroad(
13971412
params.knowledgeBaseIds,
13981413
params.access,
13991414
params.budget,
14001415
params.accessPlan,
14011416
true
14021417
)
1403-
if (key) saturatedReach.set(key, { broad })
1418+
/** A count that ran out of time decides this search only; the next one counts again. */
1419+
if (counted !== null) {
1420+
broad = counted
1421+
if (key) saturatedReach.set(key, { broad })
1422+
}
14041423
} catch (error) {
1424+
/** The leg's own deadline passed during the count: the leg is short, the search is not failed. */
14051425
if (!params.budget?.isTimeout(error)) throw error
1406-
/** A count that ran out of time decides this search only; the next one counts again. */
14071426
}
14081427
}
14091428
}
@@ -2658,9 +2677,7 @@ export async function retrieveKnowledgeSearch(
26582677
* readable documents enumerated ahead of ranking: its reach alone chooses between one
26592678
* walk over the whole graph and a search of each source.
26602679
*/
2661-
await measureSearchStage('permitted_documents', () =>
2662-
resolveReach(knowledgeBaseIds, access, budgets.vector, accessPlan)
2663-
)
2680+
await resolveReach(knowledgeBaseIds, access, budgets.vector, accessPlan)
26642681
: await resolvePermittedDocuments({
26652682
knowledgeBaseIds,
26662683
access,

0 commit comments

Comments
 (0)