Skip to content

Commit f7949f3

Browse files
waleedlatif1claude
andcommitted
fix(knowledge): count list documents once instead of per 400-base batch
#7730 switched the knowledge base lists to a KnowledgeAccessProvider, which turned the single joined count into a per-400-base loop of count plus candidate-discovery queries. The staging integ workspace holds 21k archived bases, so GET /api/knowledge?scope=archived went from ~4s to ~100s and the archive-kb integ check timed out at 30s. The stored-ACL count moves back into the list's own join. Only documents a live source (GitHub, Confluence) authorizes beyond that are counted afterwards, once for the whole list, bounded by the list filter (unpaged) or the page's ids. The provider now reports whether the reader holds any live-source credential, and the shared batch generator skips candidate discovery entirely when it does not, since the discovered predicate would be provably empty. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lm5mdDU9bi1vb4oAvPtbjn
1 parent f45cab0 commit f7949f3

7 files changed

Lines changed: 174 additions & 50 deletions

File tree

apps/sim/lib/knowledge/access/scope.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,25 @@ describe('createKnowledgeAccessProvider', () => {
266266
expect(dbChainMockFns.select).toHaveBeenCalledTimes(2)
267267
})
268268

269+
it('reports live-source readers only when a member-scoped source credential exists', async () => {
270+
queueSubjects([{ providerId: 'slack', providerTenantId: 'T1', providerSubjectId: 'U1' }])
271+
await expect(
272+
createKnowledgeAccessProvider(SESSION, WORKSPACE).hasLiveSourceReaders?.()
273+
).resolves.toBe(false)
274+
275+
queueSubjects([
276+
{
277+
providerId: 'confluence',
278+
providerTenantId: 'site-1',
279+
providerSubjectId: 'account-1',
280+
credentialId: 'credential-1',
281+
},
282+
])
283+
await expect(
284+
createKnowledgeAccessProvider(SESSION, WORKSPACE).hasLiveSourceReaders?.()
285+
).resolves.toBe(true)
286+
})
287+
269288
it('retries after a failed lookup rather than caching the failure', async () => {
270289
dbChainMockFns.where.mockRejectedValueOnce(new Error('connection reset'))
271290
const provider = createKnowledgeAccessProvider(SESSION, WORKSPACE)

apps/sim/lib/knowledge/access/scope.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,10 @@ function createAccessProvider(
397397
async get() {
398398
return (await identity()).access
399399
},
400+
async hasLiveSourceReaders() {
401+
const { access, githubReaders, confluenceReaders } = await identity()
402+
return access.kind === 'user' && (githubReaders.length > 0 || confluenceReaders.length > 0)
403+
},
400404
async getForConnectors(connectorIds, signal) {
401405
const ids = boundedIds(connectorIds)
402406
const cancellation =

apps/sim/lib/knowledge/access/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ export interface KnowledgeAccessProvider {
9393
documentIds: readonly string[],
9494
signal?: AbortSignal
9595
): Promise<KnowledgeAccessScope>
96+
/**
97+
* Whether the reader holds any credential a live source (GitHub, Confluence) could
98+
* authorize beyond the stored ACL. Without one, candidate discovery can only re-prove
99+
* the ordinary predicate, so readers skip it. Absent means unknown: discover.
100+
*/
101+
hasLiveSourceReaders?(): Promise<boolean>
96102
}
97103

98104
/** Two existing search legs each contribute at most 200 candidates to one authorization batch. */

apps/sim/lib/knowledge/read-access.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@ describe('knowledgeReadAccessBatches', () => {
4949
expect(gt).toHaveBeenCalledWith(document.connectorId, first.at(-1)!.connectorId)
5050
})
5151

52+
it('yields only the ordinary predicate for a reader without live-source credentials', async () => {
53+
const resolve = vi.fn(async () => identity)
54+
const provider: KnowledgeAccessProvider = {
55+
get: async () => identity,
56+
getForConnectors: resolve,
57+
getForDocuments: async () => identity,
58+
hasLiveSourceReaders: async () => false,
59+
}
60+
const batches = []
61+
for await (const predicate of knowledgeReadAccessBatches(provider, [])) batches.push(predicate)
62+
expect(batches).toHaveLength(1)
63+
expect(resolve).not.toHaveBeenCalled()
64+
expect(dbChainMockFns.selectDistinct).not.toHaveBeenCalled()
65+
})
66+
5267
it('does not enumerate sources after a satisfied ordinary existence probe', async () => {
5368
const resolve = vi.fn(async () => identity)
5469
const provider: KnowledgeAccessProvider = {

apps/sim/lib/knowledge/read-access.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export async function* knowledgeReadAccessBatches(
3030
const ordinary = knowledgeAccessCondition(scope)
3131
yield ordinary
3232
if (!provider || scope.kind !== 'user') return
33+
if (provider.hasLiveSourceReaders && !(await provider.hasLiveSourceReaders())) return
3334

3435
let cursor: string | undefined
3536
while (true) {

apps/sim/lib/knowledge/service.test.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,11 @@ describe('knowledge base counts with live source permissions', () => {
551551
id: 'kb-1',
552552
workspaceId: 'ws-1',
553553
chunkingConfig: {},
554-
docCount: 99,
555-
tokenCount: 999,
554+
docCount: 2,
555+
tokenCount: 10,
556556
createdAt: new Date('2026-01-01'),
557557
},
558558
])
559-
queueTableRows(schemaMock.document, [{ knowledgeBaseId: 'kb-1', docCount: 2, tokenCount: 10 }])
560559
queueTableRows(schemaMock.document, [{ connectorId: 'confluence-source' }])
561560
queueTableRows(schemaMock.document, [{ knowledgeBaseId: 'kb-1', docCount: 3, tokenCount: 20 }])
562561
const result = await getWorkspaceKnowledgeBases('ws-1', 'active', { access, limit: 2 })
@@ -587,6 +586,56 @@ describe('knowledge base counts with live source permissions', () => {
587586
).toBe(true)
588587
})
589588

589+
it('counts an unpaged list in one joined query and one discovery pass bounded by the list filter', async () => {
590+
const { access, getForConnectors } = reader()
591+
const bases = Array.from({ length: 1000 }, (_, index) => ({
592+
id: `kb-${index}`,
593+
workspaceId: 'ws-1',
594+
chunkingConfig: {},
595+
docCount: 1,
596+
tokenCount: 1,
597+
createdAt: new Date('2026-01-01'),
598+
}))
599+
queueTableRows(schemaMock.knowledgeBase, bases)
600+
queueTableRows(schemaMock.document, [{ connectorId: 'confluence-source' }])
601+
queueTableRows(schemaMock.document, [{ knowledgeBaseId: 'kb-7', docCount: 3, tokenCount: 20 }])
602+
const result = await getWorkspaceKnowledgeBases('ws-1', 'archived', { access })
603+
expect(result.data).toHaveLength(1000)
604+
expect(result.data[7]).toMatchObject({ docCount: 4, tokenCount: 21 })
605+
expect(result.data[8]).toMatchObject({ docCount: 1, tokenCount: 1 })
606+
expect(getForConnectors).toHaveBeenCalledOnce()
607+
expect(dbChainMockFns.selectDistinct).toHaveBeenCalledOnce()
608+
expect(dbChainMockFns.groupBy).toHaveBeenCalledTimes(2)
609+
expect(
610+
dbChainMockFns.where.mock.calls.some(([condition]) =>
611+
hasMockCondition(
612+
condition,
613+
(node) => node.type === 'inArray' && node.column === schemaMock.knowledgeBase.id
614+
)
615+
)
616+
).toBe(false)
617+
})
618+
619+
it('never discovers live sources for a reader without live-source credentials', async () => {
620+
const { access, getForConnectors } = reader()
621+
access.hasLiveSourceReaders = async () => false
622+
queueTableRows(schemaMock.knowledgeBase, [
623+
{
624+
id: 'kb-1',
625+
workspaceId: 'ws-1',
626+
chunkingConfig: {},
627+
docCount: 2,
628+
tokenCount: 10,
629+
createdAt: new Date('2026-01-01'),
630+
},
631+
])
632+
const result = await getWorkspaceKnowledgeBases('ws-1', 'archived', { access })
633+
expect(result.data[0]).toMatchObject({ docCount: 2, tokenCount: 10 })
634+
expect(getForConnectors).not.toHaveBeenCalled()
635+
expect(dbChainMockFns.selectDistinct).not.toHaveBeenCalled()
636+
expect(dbChainMockFns.groupBy).toHaveBeenCalledOnce()
637+
})
638+
590639
it('does not retain stale totals when a live source no longer authorizes its documents', async () => {
591640
const { access, getForConnectors } = reader()
592641
queueTableRows(schemaMock.document, [])

apps/sim/lib/knowledge/service.ts

Lines changed: 77 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { generateRestoreName } from '@/lib/core/utils/restore-name'
3434
import { findActiveFolder, resolveRestoredFolderId } from '@/lib/folders/queries'
3535
import { isKnowledgeMemberAccessAvailable } from '@/lib/knowledge/access/availability'
3636
import { knowledgeAccessCondition } from '@/lib/knowledge/access/predicate'
37-
import { MAX_KNOWLEDGE_ACCESS_CANDIDATES } from '@/lib/knowledge/access/types'
37+
import type { KnowledgeAccessProvider } from '@/lib/knowledge/access/types'
3838
import { mirrorsSourceAcls } from '@/lib/knowledge/connectors/access-modes'
3939
import { type KnowledgeReadAccess, knowledgeReadAccessBatches } from '@/lib/knowledge/read-access'
4040
import type {
@@ -174,6 +174,7 @@ async function readKnowledgeBaseRows(
174174
): Promise<
175175
Array<Omit<KnowledgeBaseWithCounts, 'connectorTypes' | 'hasPermissionScopedConnector'>>
176176
> {
177+
const scope = access && 'get' in access ? await access.get() : access
177178
const query = db
178179
.select({
179180
id: knowledgeBase.id,
@@ -201,7 +202,7 @@ async function readKnowledgeBaseRows(
201202
eq(document.userExcluded, false),
202203
isNull(document.archivedAt),
203204
isNull(document.deletedAt),
204-
access ? ('get' in access ? sql`false` : knowledgeAccessCondition(access)) : undefined
205+
scope ? knowledgeAccessCondition(scope) : undefined
205206
)
206207
)
207208
.where(where)
@@ -210,59 +211,85 @@ async function readKnowledgeBaseRows(
210211

211212
const rows = limit === undefined ? await query : await query.limit(limit)
212213

213-
const counts =
214-
access && 'get' in access
215-
? await readKnowledgeBaseDocumentCounts(
216-
rows.map((kb) => kb.id),
214+
/**
215+
* The join above already counted everything the reader's stored ACL admits. Only a
216+
* provider can add documents a live source (GitHub, Confluence) authorizes beyond that,
217+
* and that supplement is resolved once for the whole list: an unpaged list is bounded by
218+
* its own filter, a page by its row IDs, so a workspace with tens of thousands of bases
219+
* never turns into hundreds of per-batch round trips.
220+
*/
221+
const liveCounts =
222+
access && 'get' in access && rows.length > 0
223+
? await readLiveSourceDocumentCounts(
224+
limit === undefined && where
225+
? where
226+
: inArray(
227+
knowledgeBase.id,
228+
rows.map((kb) => kb.id)
229+
),
217230
access
218231
)
219232
: undefined
220233
return rows.map((kb) => ({
221234
...kb,
222235
chunkingConfig: kb.chunkingConfig as ChunkingConfig,
223-
docCount: counts ? (counts.get(kb.id)?.docCount ?? 0) : Number(kb.docCount),
224-
tokenCount: counts ? (counts.get(kb.id)?.tokenCount ?? 0) : kb.tokenCount,
236+
docCount: Number(kb.docCount) + (liveCounts?.get(kb.id)?.docCount ?? 0),
237+
tokenCount: kb.tokenCount + (liveCounts?.get(kb.id)?.tokenCount ?? 0),
225238
}))
226239
}
227240

228-
/** Counts only hydrated access batches, keeping candidate discovery free of document metadata. */
229-
async function readKnowledgeBaseDocumentCounts(
230-
knowledgeBaseIds: readonly string[],
231-
access: KnowledgeReadAccess
241+
const ACTIVE_DOCUMENT_CONDITIONS = [
242+
eq(document.userExcluded, false),
243+
isNull(document.archivedAt),
244+
isNull(document.deletedAt),
245+
] as const
246+
247+
/**
248+
* Document totals per knowledge base for one access predicate, restricted to the bases
249+
* `subject` selects. `subject` may reference `knowledge_base` columns.
250+
*/
251+
async function countDocumentsByKnowledgeBase(
252+
subject: SQL,
253+
accessCondition: SQL
254+
): Promise<Array<{ knowledgeBaseId: string; docCount: number; tokenCount: number }>> {
255+
return db
256+
.select({
257+
knowledgeBaseId: document.knowledgeBaseId,
258+
docCount: count(),
259+
tokenCount: sql<number>`COALESCE(SUM(${document.tokenCount}), 0)`.mapWith(Number),
260+
})
261+
.from(document)
262+
.innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id))
263+
.where(and(subject, ...ACTIVE_DOCUMENT_CONDITIONS, accessCondition))
264+
.groupBy(document.knowledgeBaseId)
265+
}
266+
267+
/**
268+
* Totals for documents only a live source authorizes, on top of the reader's stored ACL.
269+
* The ordinary predicate is skipped because every caller has already counted it; candidate
270+
* discovery stays free of document metadata and returns nothing for a reader without
271+
* live-source credentials.
272+
*/
273+
async function readLiveSourceDocumentCounts(
274+
subject: SQL,
275+
access: KnowledgeAccessProvider
232276
): Promise<Map<string, { docCount: number; tokenCount: number }>> {
233277
const counts = new Map<string, { docCount: number; tokenCount: number }>()
234-
for (
235-
let offset = 0;
236-
offset < knowledgeBaseIds.length;
237-
offset += MAX_KNOWLEDGE_ACCESS_CANDIDATES
238-
) {
239-
const conditions = [
240-
inArray(
241-
knowledgeBase.id,
242-
knowledgeBaseIds.slice(offset, offset + MAX_KNOWLEDGE_ACCESS_CANDIDATES)
243-
),
244-
eq(document.userExcluded, false),
245-
isNull(document.archivedAt),
246-
isNull(document.deletedAt),
247-
]
248-
for await (const accessCondition of knowledgeReadAccessBatches(access, conditions)) {
249-
const rows = await db
250-
.select({
251-
knowledgeBaseId: document.knowledgeBaseId,
252-
docCount: count(),
253-
tokenCount: sql<number>`COALESCE(SUM(${document.tokenCount}), 0)`.mapWith(Number),
254-
})
255-
.from(document)
256-
.innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id))
257-
.where(and(...conditions, accessCondition))
258-
.groupBy(document.knowledgeBaseId)
259-
for (const row of rows) {
260-
const previous = counts.get(row.knowledgeBaseId)
261-
counts.set(row.knowledgeBaseId, {
262-
docCount: (previous?.docCount ?? 0) + Number(row.docCount),
263-
tokenCount: (previous?.tokenCount ?? 0) + Number(row.tokenCount),
264-
})
265-
}
278+
let ordinary = true
279+
for await (const accessCondition of knowledgeReadAccessBatches(access, [
280+
subject,
281+
...ACTIVE_DOCUMENT_CONDITIONS,
282+
])) {
283+
if (ordinary) {
284+
ordinary = false
285+
continue
286+
}
287+
for (const row of await countDocumentsByKnowledgeBase(subject, accessCondition)) {
288+
const previous = counts.get(row.knowledgeBaseId)
289+
counts.set(row.knowledgeBaseId, {
290+
docCount: (previous?.docCount ?? 0) + Number(row.docCount),
291+
tokenCount: (previous?.tokenCount ?? 0) + Number(row.tokenCount),
292+
})
266293
}
267294
}
268295
return counts
@@ -1019,11 +1046,14 @@ export async function attachKnowledgeBaseConnectors(
10191046
): Promise<KnowledgeBaseWithCounts> {
10201047
let visible = knowledgeBase
10211048
if (access) {
1022-
const counts = await readKnowledgeBaseDocumentCounts([knowledgeBase.id], access)
1049+
const subject = eq(document.knowledgeBaseId, knowledgeBase.id)
1050+
const scope = 'get' in access ? await access.get() : access
1051+
const [ordinary] = await countDocumentsByKnowledgeBase(subject, knowledgeAccessCondition(scope))
1052+
const live = 'get' in access ? await readLiveSourceDocumentCounts(subject, access) : undefined
10231053
visible = {
10241054
...knowledgeBase,
1025-
docCount: counts.get(knowledgeBase.id)?.docCount ?? 0,
1026-
tokenCount: counts.get(knowledgeBase.id)?.tokenCount ?? 0,
1055+
docCount: Number(ordinary?.docCount ?? 0) + (live?.get(knowledgeBase.id)?.docCount ?? 0),
1056+
tokenCount: (ordinary?.tokenCount ?? 0) + (live?.get(knowledgeBase.id)?.tokenCount ?? 0),
10271057
}
10281058
}
10291059
const [withConnectors] = await attachConnectorTypes([visible])

0 commit comments

Comments
 (0)