Skip to content

Commit bb75936

Browse files
improvement(kb): pagination for docs + remove kb token count problematic query (#689)
* added pagination to docs in kb * ack PR comments * fix failing tests * fix(kb): remove problematic query updating kb token count --------- Co-authored-by: Waleed Latif <walif6@gmail.com>
1 parent a7a2056 commit bb75936

File tree

10 files changed

+398
-119
lines changed

10 files changed

+398
-119
lines changed

apps/sim/app/api/knowledge/[id]/documents/route.test.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ describe('Knowledge Base Documents API Route', () => {
3030
from: vi.fn().mockReturnThis(),
3131
where: vi.fn().mockReturnThis(),
3232
orderBy: vi.fn().mockReturnThis(),
33+
limit: vi.fn().mockReturnThis(),
34+
offset: vi.fn().mockReturnThis(),
3335
insert: vi.fn().mockReturnThis(),
3436
values: vi.fn().mockReturnThis(),
3537
update: vi.fn().mockReturnThis(),
@@ -99,7 +101,12 @@ describe('Knowledge Base Documents API Route', () => {
99101
it('should retrieve documents successfully for authenticated user', async () => {
100102
mockAuth$.mockAuthenticatedUser()
101103
mockCheckKnowledgeBaseAccess.mockResolvedValue({ hasAccess: true })
102-
mockDbChain.orderBy.mockResolvedValue([mockDocument])
104+
105+
// Mock the count query (first query)
106+
mockDbChain.where.mockResolvedValueOnce([{ count: 1 }])
107+
108+
// Mock the documents query (second query)
109+
mockDbChain.offset.mockResolvedValue([mockDocument])
103110

104111
const req = createMockRequest('GET')
105112
const { GET } = await import('./route')
@@ -108,16 +115,21 @@ describe('Knowledge Base Documents API Route', () => {
108115

109116
expect(response.status).toBe(200)
110117
expect(data.success).toBe(true)
111-
expect(data.data).toHaveLength(1)
112-
expect(data.data[0].id).toBe('doc-123')
118+
expect(data.data.documents).toHaveLength(1)
119+
expect(data.data.documents[0].id).toBe('doc-123')
113120
expect(mockDbChain.select).toHaveBeenCalled()
114121
expect(mockCheckKnowledgeBaseAccess).toHaveBeenCalledWith('kb-123', 'user-123')
115122
})
116123

117124
it('should filter disabled documents by default', async () => {
118125
mockAuth$.mockAuthenticatedUser()
119126
mockCheckKnowledgeBaseAccess.mockResolvedValue({ hasAccess: true })
120-
mockDbChain.orderBy.mockResolvedValue([mockDocument])
127+
128+
// Mock the count query (first query)
129+
mockDbChain.where.mockResolvedValueOnce([{ count: 1 }])
130+
131+
// Mock the documents query (second query)
132+
mockDbChain.offset.mockResolvedValue([mockDocument])
121133

122134
const req = createMockRequest('GET')
123135
const { GET } = await import('./route')
@@ -130,7 +142,12 @@ describe('Knowledge Base Documents API Route', () => {
130142
it('should include disabled documents when requested', async () => {
131143
mockAuth$.mockAuthenticatedUser()
132144
mockCheckKnowledgeBaseAccess.mockResolvedValue({ hasAccess: true })
133-
mockDbChain.orderBy.mockResolvedValue([mockDocument])
145+
146+
// Mock the count query (first query)
147+
mockDbChain.where.mockResolvedValueOnce([{ count: 1 }])
148+
149+
// Mock the documents query (second query)
150+
mockDbChain.offset.mockResolvedValue([mockDocument])
134151

135152
const url = 'http://localhost:3000/api/knowledge/kb-123/documents?includeDisabled=true'
136153
const req = new Request(url, { method: 'GET' }) as any

apps/sim/app/api/knowledge/[id]/documents/route.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import crypto from 'node:crypto'
2-
import { and, desc, eq, inArray, isNull } from 'drizzle-orm'
2+
import { and, desc, eq, inArray, isNull, sql } from 'drizzle-orm'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { z } from 'zod'
55
import { getSession } from '@/lib/auth'
@@ -210,6 +210,9 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id:
210210

211211
const url = new URL(req.url)
212212
const includeDisabled = url.searchParams.get('includeDisabled') === 'true'
213+
const search = url.searchParams.get('search')
214+
const limit = Number.parseInt(url.searchParams.get('limit') || '50')
215+
const offset = Number.parseInt(url.searchParams.get('offset') || '0')
213216

214217
// Build where conditions
215218
const whereConditions = [
@@ -222,6 +225,23 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id:
222225
whereConditions.push(eq(document.enabled, true))
223226
}
224227

228+
// Add search condition if provided
229+
if (search) {
230+
whereConditions.push(
231+
// Search in filename
232+
sql`LOWER(${document.filename}) LIKE LOWER(${`%${search}%`})`
233+
)
234+
}
235+
236+
// Get total count for pagination
237+
const totalResult = await db
238+
.select({ count: sql<number>`COUNT(*)` })
239+
.from(document)
240+
.where(and(...whereConditions))
241+
242+
const total = totalResult[0]?.count || 0
243+
const hasMore = offset + limit < total
244+
225245
const documents = await db
226246
.select({
227247
id: document.id,
@@ -250,14 +270,24 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id:
250270
.from(document)
251271
.where(and(...whereConditions))
252272
.orderBy(desc(document.uploadedAt))
273+
.limit(limit)
274+
.offset(offset)
253275

254276
logger.info(
255-
`[${requestId}] Retrieved ${documents.length} documents for knowledge base ${knowledgeBaseId}`
277+
`[${requestId}] Retrieved ${documents.length} documents (${offset}-${offset + documents.length} of ${total}) for knowledge base ${knowledgeBaseId}`
256278
)
257279

258280
return NextResponse.json({
259281
success: true,
260-
data: documents,
282+
data: {
283+
documents,
284+
pagination: {
285+
total,
286+
limit,
287+
offset,
288+
hasMore,
289+
},
290+
},
261291
})
262292
} catch (error) {
263293
logger.error(`[${requestId}] Error fetching documents`, error)

apps/sim/app/api/knowledge/utils.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ describe('Knowledge Utils', () => {
176176
{}
177177
)
178178

179-
expect(dbOps.order).toEqual(['insert', 'updateDoc', 'updateKb'])
179+
expect(dbOps.order).toEqual(['insert', 'updateDoc'])
180180

181181
expect(dbOps.updatePayloads[0]).toMatchObject({
182182
processingStatus: 'completed',

apps/sim/app/api/knowledge/utils.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import crypto from 'crypto'
2-
import { and, eq, isNull, sql } from 'drizzle-orm'
2+
import { and, eq, isNull } from 'drizzle-orm'
33
import { processDocument } from '@/lib/documents/document-processor'
44
import { retryWithExponentialBackoff } from '@/lib/documents/utils'
55
import { env } from '@/lib/env'
@@ -522,14 +522,6 @@ export async function processDocumentAsync(
522522
processingError: null,
523523
})
524524
.where(eq(document.id, documentId))
525-
526-
await tx
527-
.update(knowledgeBase)
528-
.set({
529-
tokenCount: sql`${knowledgeBase.tokenCount} + ${processed.metadata.tokenCount}`,
530-
updatedAt: now,
531-
})
532-
.where(eq(knowledgeBase.id, knowledgeBaseId))
533525
})
534526
})(),
535527
TIMEOUTS.OVERALL_PROCESSING,

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export function Document({
117117
setError(null)
118118

119119
const cachedDocuments = getCachedDocuments(knowledgeBaseId)
120-
const cachedDoc = cachedDocuments?.find((d) => d.id === documentId)
120+
const cachedDoc = cachedDocuments?.documents?.find((d) => d.id === documentId)
121121

122122
if (cachedDoc) {
123123
setDocument(cachedDoc)

0 commit comments

Comments
 (0)