Skip to content

Commit 1d66a7d

Browse files
committed
fix(knowledge): avoid count-query fan-out for unpaged lists
1 parent 0bc7f98 commit 1d66a7d

7 files changed

Lines changed: 86 additions & 47 deletions

File tree

‎apps/sim/lib/knowledge/__integration__/knowledge-base-list.integration.ts‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { getWorkspaceKnowledgeBases } from '@/lib/knowledge/service'
1818
*/
1919
const ids = createKnowledgeAclFixtureIds()
2020
const foreign = createKnowledgeAclFixtureIds()
21+
const manyBases = createKnowledgeAclFixtureIds()
2122
const offPageId = generateId()
2223
const emptyId = generateId()
2324
const archivedId = generateId()
@@ -106,6 +107,16 @@ beforeAll(async () => {
106107
SELECT ${baseId} || '-' || n, ${baseId}, 'bulk.txt', 'https://fixture.invalid/bulk',
107108
1, 'text/plain', ARRAY['ws'], 1 FROM generate_series(1, 10000) AS n`)
108109
}
110+
await seedKnowledgeAclFixture(manyBases, { connectorType: 'google_drive' })
111+
await db.execute(sql`INSERT INTO knowledge_base (id, workspace_id, user_id, name)
112+
SELECT ${manyBases.knowledgeBaseId} || '-' || n, ${manyBases.workspaceId},
113+
${manyBases.aliceId}, 'Scale fixture ' || n FROM generate_series(1, 10000) AS n`)
114+
await db.execute(sql`INSERT INTO document
115+
(id, knowledge_base_id, filename, file_url, file_size, mime_type, acl, token_count)
116+
VALUES (${generateId()}, ${manyBases.knowledgeBaseId}, 'first.txt',
117+
'https://fixture.invalid/first', 1, 'text/plain', ARRAY['ws'], 13),
118+
(${generateId()}, ${`${manyBases.knowledgeBaseId}-10000`}, 'last.txt',
119+
'https://fixture.invalid/last', 1, 'text/plain', ARRAY['ws'], 17)`)
109120
await db.execute(sql`ANALYZE knowledge_base`)
110121
await db.execute(sql`ANALYZE document`)
111122
}, 60_000)
@@ -114,7 +125,7 @@ afterAll(async () => {
114125
const reportPath = process.env.KNOWLEDGE_BASE_LIST_REPORT_PATH
115126
if (reportPath) writeFileSync(reportPath, JSON.stringify(reports, null, 2))
116127
try {
117-
for (const fixture of [ids, foreign]) {
128+
for (const fixture of [ids, foreign, manyBases]) {
118129
await db.delete(workspace).where(eq(workspace.id, fixture.workspaceId))
119130
await db.delete(organization).where(eq(organization.id, fixture.organizationId))
120131
await db.delete(user).where(inArray(user.id, [fixture.aliceId, fixture.bobId]))
@@ -191,4 +202,34 @@ describe('knowledge base list counts on real Postgres', () => {
191202
})
192203
expect(archived.data.map(({ id }) => id)).toEqual([archivedId])
193204
})
205+
206+
it('counts a large unpaged workspace within a fixed database round-trip budget', async () => {
207+
let documentQueries = 0
208+
const previousDebug = db.$client.options.debug
209+
db.$client.options.debug = (_connection, query) => {
210+
if (query.includes('"document"')) documentQueries++
211+
}
212+
try {
213+
const all = await getWorkspaceKnowledgeBases(manyBases.workspaceId, 'active', {
214+
countsFor: access,
215+
})
216+
expect(all.data).toHaveLength(10001)
217+
expect(all.nextCursorKeys).toBeNull()
218+
expect(all.data.find((kb) => kb.id === manyBases.knowledgeBaseId)).toMatchObject({
219+
docCount: 1,
220+
tokenCount: 13,
221+
})
222+
expect(all.data.find((kb) => kb.id === `${manyBases.knowledgeBaseId}-10000`)).toMatchObject({
223+
docCount: 1,
224+
tokenCount: 17,
225+
})
226+
expect(all.data.reduce((total, kb) => total + kb.docCount, 0)).toBe(2)
227+
expect(all.data.reduce((total, kb) => total + kb.tokenCount, 0)).toBe(30)
228+
reports.push({ unpagedBases: all.data.length, documentQueries })
229+
expect(documentQueries).toBeGreaterThan(0)
230+
expect(documentQueries).toBeLessThan(10)
231+
} finally {
232+
db.$client.options.debug = previousDebug
233+
}
234+
})
194235
})

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { db } from '@sim/db'
22
import { document, knowledgeBase, knowledgeConnector, workspaceFiles } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
5-
import { chunkArray } from '@sim/utils/helpers'
65
import { generateId } from '@sim/utils/id'
76
import { filterUndefined } from '@sim/utils/object'
87
import type { SQL } from 'drizzle-orm'
@@ -34,7 +33,7 @@ import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
3433
import { generateRestoreName } from '@/lib/core/utils/restore-name'
3534
import { findActiveFolder, resolveRestoredFolderId } from '@/lib/folders/queries'
3635
import { isKnowledgeMemberAccessAvailable } from '@/lib/knowledge/access/availability'
37-
import { knowledgeAccessCondition } from '@/lib/knowledge/access/predicate'
36+
import { knowledgeAccessCondition, textArrayLiteral } from '@/lib/knowledge/access/predicate'
3837
import type { KnowledgeAccessProvider } from '@/lib/knowledge/access/types'
3938
import { mirrorsSourceAcls } from '@/lib/knowledge/connectors/access-modes'
4039
import {
@@ -53,7 +52,6 @@ import type {
5352
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
5453

5554
const logger = createLogger('KnowledgeBaseService')
56-
const KNOWLEDGE_BASE_COUNT_BATCH_SIZE = 100
5755

5856
/**
5957
* Every caller-fixable knowledge-base failure is an {@link OrchestrationError},
@@ -203,17 +201,14 @@ async function readCountedKnowledgeBaseRows(
203201
> {
204202
const scope = 'get' in access ? await access.get() : access
205203
const rows = await readKnowledgeBaseRows(where, orderBy, limit)
206-
const counts = new Map<string, { docCount: number; tokenCount: number }>()
207-
for (const batch of chunkArray(rows, KNOWLEDGE_BASE_COUNT_BATCH_SIZE)) {
208-
const totals = await countDocumentsByKnowledgeBase(
209-
inArray(
210-
document.knowledgeBaseId,
211-
batch.map((kb) => kb.id)
212-
),
213-
knowledgeAccessCondition(scope)
214-
)
215-
for (const total of totals) counts.set(total.knowledgeBaseId, total)
216-
}
204+
const totals =
205+
rows.length > 0
206+
? await countDocumentsByKnowledgeBase(
207+
sql`${document.knowledgeBaseId} = ANY(${textArrayLiteral(rows.map((kb) => kb.id))})`,
208+
knowledgeAccessCondition(scope)
209+
)
210+
: []
211+
const counts = new Map(totals.map((total) => [total.knowledgeBaseId, total]))
217212

218213
/**
219214
* The counts above already include everything the reader's stored ACL admits. Only a

‎apps/sim/lib/selectors/server/providers/powerbi.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ describe('Power BI selector provider boundary', () => {
136136
manualWorkspaceId: 'stale-manual-workspace',
137137
[field]: workspace,
138138
}
139-
const picker = PowerBIBlock.subBlocks.find((subBlock) => subBlock.id === 'datasetSelector')!
139+
const picker = PowerBIBlock.subBlocks.find((subBlock) => subBlock.id === 'datasetSelector')
140+
if (!picker) throw new Error('Power BI semantic-model selector is missing')
140141
const context = buildSelectorContextFromValues({
141142
selectorKey: 'powerbi.datasets',
142143
contextConfigs: getSelectorContextSubBlocks(PowerBIBlock.subBlocks, values),

‎apps/sim/tools/powerbi/__fixtures__/provider-fixture.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export type PowerBIFixtureScenario =
3131
| 'missing-identity'
3232
| 'missing-query'
3333

34-
export interface PowerBIFixtureRequest {
34+
interface PowerBIFixtureRequest {
3535
method: string
3636
path: string
3737
body: unknown

‎apps/sim/tools/powerbi/execute-query.test.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { jsonResponse } from '@sim/testing/helpers/http'
22
import { describe, expect, it } from 'vitest'
33
import { powerbiExecuteQueryTool } from '@/tools/powerbi/execute-query'
44

5+
const transformResponse = powerbiExecuteQueryTool.transformResponse
6+
if (!transformResponse) throw new Error('Power BI query tool must transform its response')
7+
58
const params = {
69
accessToken: 'provider-token',
710
groupId: 'workspace-id',
@@ -32,7 +35,7 @@ describe('Power BI DAX result handling', () => {
3235
],
3336
})
3437

35-
const result = await powerbiExecuteQueryTool.transformResponse!(response, params)
38+
const result = await transformResponse(response, params)
3639

3740
expect(result).toMatchObject({
3841
success: false,
@@ -52,7 +55,7 @@ describe('Power BI DAX result handling', () => {
5255
})
5356

5457
it('preserves provider column names, blank values and falsy values in complete results', async () => {
55-
const result = await powerbiExecuteQueryTool.transformResponse!(
58+
const result = await transformResponse(
5659
jsonResponse({
5760
results: [
5861
{
@@ -79,7 +82,7 @@ describe('Power BI DAX result handling', () => {
7982
})
8083

8184
it('does not fabricate a query error message when Microsoft supplies only a code', async () => {
82-
const result = await powerbiExecuteQueryTool.transformResponse!(
85+
const result = await transformResponse(
8386
jsonResponse({ error: { code: 'DatasetExecuteQueriesError' } }),
8487
params
8588
)
@@ -98,7 +101,7 @@ describe('Power BI DAX result handling', () => {
98101
})
99102

100103
it.each(['omitted', 'empty'])('accepts one result table with %s rows', async (kind) => {
101-
const result = await powerbiExecuteQueryTool.transformResponse!(
104+
const result = await transformResponse(
102105
jsonResponse({ results: [{ tables: [kind === 'empty' ? { rows: [] } : {}] }] }),
103106
params
104107
)
@@ -117,7 +120,7 @@ describe('Power BI DAX result handling', () => {
117120
{ code: 'AnalysisServicesErrorCode', detail: { type: 1, value: '3238920194' } },
118121
{ code: 'DetailsMessage', detail: { type: 1, value: 'The DAX query is invalid.' } },
119122
]
120-
const result = await powerbiExecuteQueryTool.transformResponse!(
123+
const result = await transformResponse(
121124
jsonResponse({
122125
error: {
123126
code: 'DatasetExecuteQueriesError',
@@ -174,9 +177,7 @@ describe('Power BI DAX result handling', () => {
174177
: undefined
175178
)
176179

177-
await expect(powerbiExecuteQueryTool.transformResponse!(response, params)).rejects.toThrow(
178-
/maximum size.*20971520/
179-
)
180+
await expect(transformResponse(response, params)).rejects.toThrow(/maximum size.*20971520/)
180181
expect(canceled).toBe(true)
181182
}
182183
)
@@ -191,7 +192,7 @@ describe('Power BI DAX result handling', () => {
191192
},
192193
})
193194
)
194-
const transformed = powerbiExecuteQueryTool.transformResponse!(response, params, {
195+
const transformed = transformResponse(response, params, {
195196
signal: abort.signal,
196197
})
197198
abort.abort(new Error('Execution canceled'))

‎apps/sim/tools/powerbi/types.ts‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { OutputProperty, ToolResponse } from '@/tools/types'
22

3-
export interface PowerBIAuthParams {
3+
interface PowerBIAuthParams {
44
accessToken: string
55
}
66

@@ -27,7 +27,7 @@ export interface PowerBIExecuteQueryParams extends PowerBIDatasetParams {
2727
includeNulls?: boolean
2828
}
2929

30-
export type PowerBINotifyOption = 'NoNotification' | 'MailOnFailure' | 'MailOnCompletion'
30+
type PowerBINotifyOption = 'NoNotification' | 'MailOnFailure' | 'MailOnCompletion'
3131

3232
export interface PowerBIRefreshDatasetParams extends PowerBIDatasetParams {
3333
notifyOption?: PowerBINotifyOption
@@ -71,7 +71,7 @@ export interface PowerBIDataset {
7171
webUrl: string | null
7272
}
7373

74-
export interface PowerBIRefreshAttempt {
74+
interface PowerBIRefreshAttempt {
7575
attemptId: number | null
7676
type: string | null
7777
startTime: string | null
@@ -96,7 +96,7 @@ export interface PowerBIQueryError {
9696
details: unknown | null
9797
}
9898

99-
export interface PowerBIInformationProtectionLabel {
99+
interface PowerBIInformationProtectionLabel {
100100
id: string | null
101101
name: string | null
102102
}
@@ -206,21 +206,6 @@ export const POWERBI_DATASET_OUTPUT_PROPERTIES = {
206206
webUrl: { ...nullableString, description: 'Semantic model URL in Power BI, when available' },
207207
} satisfies Record<string, OutputProperty>
208208

209-
export const POWERBI_REFRESH_ATTEMPT_OUTPUT_PROPERTIES = {
210-
attemptId: {
211-
type: 'number',
212-
nullable: true,
213-
description: 'Refresh attempt index',
214-
},
215-
type: { ...nullableString, description: 'Provider refresh attempt type' },
216-
startTime: { ...nullableString, description: 'Attempt start timestamp' },
217-
endTime: { ...nullableString, description: 'Attempt end timestamp, when available' },
218-
serviceExceptionJson: {
219-
...nullableString,
220-
description: 'Serialized provider failure details, when available',
221-
},
222-
} satisfies Record<string, OutputProperty>
223-
224209
export const POWERBI_REFRESH_OUTPUT_PROPERTIES = {
225210
requestId: { ...nullableString, description: 'Provider refresh request ID' },
226211
refreshType: { ...nullableString, description: 'Provider refresh trigger type' },
@@ -237,7 +222,23 @@ export const POWERBI_REFRESH_OUTPUT_PROPERTIES = {
237222
refreshAttempts: {
238223
type: 'array',
239224
description: 'Refresh attempts supplied by the provider',
240-
items: { type: 'object', properties: POWERBI_REFRESH_ATTEMPT_OUTPUT_PROPERTIES },
225+
items: {
226+
type: 'object',
227+
properties: {
228+
attemptId: {
229+
type: 'number',
230+
nullable: true,
231+
description: 'Refresh attempt index',
232+
},
233+
type: { ...nullableString, description: 'Provider refresh attempt type' },
234+
startTime: { ...nullableString, description: 'Attempt start timestamp' },
235+
endTime: { ...nullableString, description: 'Attempt end timestamp, when available' },
236+
serviceExceptionJson: {
237+
...nullableString,
238+
description: 'Serialized provider failure details, when available',
239+
},
240+
},
241+
},
241242
},
242243
} satisfies Record<string, OutputProperty>
243244

‎apps/sim/tools/powerbi/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
} from '@/tools/powerbi/types'
1111
import { safeUrlPathSegment } from '@/tools/url-path'
1212

13-
export const MAX_POWERBI_RESPONSE_BYTES = 20 * 1024 * 1024
13+
const MAX_POWERBI_RESPONSE_BYTES = 20 * 1024 * 1024
1414

1515
export const POWERBI_ACCESS_TOKEN_PARAM = {
1616
type: 'string',

0 commit comments

Comments
 (0)