Skip to content

Commit 3fcf7e2

Browse files
committed
test(knowledge): pin per-base document reads in the counted list plan
EXPLAIN the exact counted-list SQL with nested loops disabled and require every document scan to be correlated on its knowledge base, so a set-based join over the ACL index fails the build. Drop the comment's page-limit claim, which is the planner's cost choice rather than a guarantee, and a redundant mock-call assertion.
1 parent a263c6d commit 3fcf7e2

3 files changed

Lines changed: 132 additions & 2 deletions

File tree

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* The counted knowledge-base list must total each base through that base's own documents, never
3+
* through a set-based join over the whole document table. The reader's ACL tokens are shared by
4+
* every tenant, so a join the planner drives from the ACL index instead of `knowledge_base_id`
5+
* reads every tenant's documents to total one base. Nested loops are disabled while planning so
6+
* the planner reaches for exactly that join wherever the query shape still allows it.
7+
*/
8+
import type { Principal } from '@sim/auth/principal'
9+
import * as schema from '@sim/db/schema'
10+
import { document, workspace } from '@sim/db/schema'
11+
import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure'
12+
import { withUtcTimestamps } from '@sim/db/timestamps'
13+
import { generateId } from '@sim/utils/id'
14+
import { eq } from 'drizzle-orm'
15+
import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js'
16+
import postgres from 'postgres'
17+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
18+
19+
const database = vi.hoisted(() => ({
20+
current: undefined as PostgresJsDatabase<typeof import('@sim/db/schema')> | undefined,
21+
/** Every statement the app issues, so the test can EXPLAIN the exact SQL it ran. */
22+
statements: [] as { sql: string; params: unknown[] }[],
23+
}))
24+
vi.mock('@sim/db', async (importOriginal) => ({
25+
...(await importOriginal<typeof import('@sim/db')>()),
26+
get db() {
27+
if (!database.current) throw new Error('Test database not initialized')
28+
return database.current
29+
},
30+
}))
31+
32+
import {
33+
createKnowledgeAclFixtureIds,
34+
seedKnowledgeAclFixture,
35+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
36+
import { listKnowledgeBases } from '@/lib/knowledge/application/knowledge-bases'
37+
38+
interface PlanNode {
39+
'Node Type': string
40+
'Relation Name'?: string
41+
'Index Cond'?: string
42+
'Recheck Cond'?: string
43+
Filter?: string
44+
Plans?: PlanNode[]
45+
}
46+
47+
function documentScans(node: PlanNode): PlanNode[] {
48+
const own = node['Relation Name'] === 'document' ? [node] : []
49+
return [...own, ...(node.Plans ?? []).flatMap(documentScans)]
50+
}
51+
52+
const ids = createKnowledgeAclFixtureIds()
53+
const reader: Principal = { kind: 'session', userId: ids.bobId, sessionId: 'fixture-reader' }
54+
const connection = postgres(
55+
readTestDatabaseUrl(),
56+
withUtcTimestamps({
57+
max: 2,
58+
prepare: false,
59+
fetch_types: false,
60+
connection: {},
61+
onnotice: () => {},
62+
})
63+
)
64+
65+
describe('knowledge-base list totals plan', () => {
66+
beforeAll(async () => {
67+
vi.stubGlobal('fetch', async () => {
68+
throw new Error('Unexpected provider request in knowledge-base totals plan test')
69+
})
70+
database.current = drizzle(connection, {
71+
schema,
72+
logger: {
73+
logQuery(sql, params) {
74+
database.statements.push({ sql, params })
75+
},
76+
},
77+
})
78+
await seedKnowledgeAclFixture(ids, { connectorType: 'google_drive' })
79+
await database.current.insert(document).values({
80+
id: generateId(),
81+
knowledgeBaseId: ids.knowledgeBaseId,
82+
filename: 'Workspace handbook',
83+
fileUrl: 'https://fixture.test/shared',
84+
fileSize: 10,
85+
mimeType: 'text/plain',
86+
tokenCount: 10,
87+
processingStatus: 'completed',
88+
})
89+
})
90+
91+
afterAll(async () => {
92+
await database.current?.delete(workspace).where(eq(workspace.id, ids.workspaceId))
93+
await connection.end()
94+
vi.unstubAllGlobals()
95+
})
96+
97+
it('totals each base through its own documents even when the planner prefers a hash join', async () => {
98+
database.statements.length = 0
99+
const { knowledgeBases } = await listKnowledgeBases.execute({
100+
principal: reader,
101+
input: { workspaceId: ids.workspaceId },
102+
})
103+
expect(knowledgeBases.map(({ knowledgeBase }) => knowledgeBase)).toEqual([
104+
expect.objectContaining({ id: ids.knowledgeBaseId, docCount: 1, tokenCount: 10 }),
105+
])
106+
107+
const counted = database.statements.filter(
108+
({ sql }) =>
109+
/^select .* from "knowledge_base" /.test(sql) &&
110+
sql.includes('"document"."knowledge_base_id" = "knowledge_base"."id"')
111+
)
112+
expect(counted).toHaveLength(1)
113+
const plan = await connection.begin(async (tx) => {
114+
await tx`SET LOCAL enable_nestloop = off`
115+
const [row] = await tx.unsafe(
116+
`EXPLAIN (FORMAT JSON) ${counted[0].sql}`,
117+
counted[0].params as Parameters<typeof tx.unsafe>[1]
118+
)
119+
return (row['QUERY PLAN'] as { Plan: PlanNode }[])[0].Plan
120+
})
121+
122+
const scans = documentScans(plan)
123+
expect(scans.length).toBeGreaterThan(0)
124+
for (const scan of scans) {
125+
const conditions = [scan['Index Cond'], scan['Recheck Cond'], scan.Filter].join(' ')
126+
expect(conditions, `${scan['Node Type']} on document`).toContain(
127+
'(knowledge_base_id = knowledge_base.id)'
128+
)
129+
}
130+
})
131+
})

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@ describe('knowledge base counts with live source permissions', () => {
304304
expect(dbChainMockFns.select).not.toHaveBeenCalledWith({
305305
connectorId: schemaMock.knowledgeConnector.id,
306306
})
307-
expect(dbChainMockFns.groupBy).not.toHaveBeenCalled()
308307
})
309308

310309
it('does not retain stale totals when a live source no longer authorizes its documents', async () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ async function readCountedKnowledgeBaseRows(
203203
/**
204204
* A lateral aggregate, so each base is counted through its own `knowledge_base_id` index and
205205
* never through a bitmap of every tenant's documents sharing the `ws` token. An aggregate always
206-
* yields one row, so an empty base stays at zero, and the limit stops counting past the page.
206+
* yields one row, so an empty base stays at zero.
207207
*/
208208
const totals = db
209209
.select({

0 commit comments

Comments
 (0)