Skip to content

Commit 2a333c7

Browse files
authored
fix(kb): added proper pagination for documents in kb (#937)
1 parent 41cc0cd commit 2a333c7

File tree

3 files changed

+76
-126
lines changed

3 files changed

+76
-126
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1121,7 +1121,7 @@ export function KnowledgeBase({
11211121
key={page}
11221122
onClick={() => goToPage(page)}
11231123
disabled={isLoadingDocuments}
1124-
className={`font-medium text-sm transition-colors hover:text-foreground disabled:cursor-not-allowed disabled:opacity-50 ${
1124+
className={`font-medium text-sm transition-colors hover:text-foreground disabled:opacity-50 ${
11251125
page === currentPage ? 'text-foreground' : 'text-muted-foreground'
11261126
}`}
11271127
>

apps/sim/hooks/use-knowledge.ts

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export function useKnowledgeBase(id: string) {
4141
}
4242

4343
// Constants
44-
const MAX_DOCUMENTS_LIMIT = 10000
4544
const DEFAULT_PAGE_SIZE = 50
4645

4746
export function useKnowledgeBaseDocuments(
@@ -54,69 +53,61 @@ export function useKnowledgeBaseDocuments(
5453
const [error, setError] = useState<string | null>(null)
5554

5655
const documentsCache = getCachedDocuments(knowledgeBaseId)
57-
const allDocuments = documentsCache?.documents || []
5856
const isLoading = loadingDocuments.has(knowledgeBaseId)
59-
const hasBeenLoaded = documentsCache !== null // Check if we have any cache entry, even if empty
6057

61-
// Load all documents on initial mount
58+
// Load documents with server-side pagination and search
59+
const requestLimit = options?.limit || DEFAULT_PAGE_SIZE
60+
const requestOffset = options?.offset || 0
61+
const requestSearch = options?.search
62+
6263
useEffect(() => {
63-
if (!knowledgeBaseId || hasBeenLoaded || isLoading) return
64+
if (!knowledgeBaseId || isLoading) return
6465

6566
let isMounted = true
6667

67-
const loadAllDocuments = async () => {
68+
const loadDocuments = async () => {
6869
try {
6970
setError(null)
70-
await getDocuments(knowledgeBaseId, { limit: MAX_DOCUMENTS_LIMIT })
71+
await getDocuments(knowledgeBaseId, {
72+
search: requestSearch,
73+
limit: requestLimit,
74+
offset: requestOffset,
75+
})
7176
} catch (err) {
7277
if (isMounted) {
7378
setError(err instanceof Error ? err.message : 'Failed to load documents')
7479
}
7580
}
7681
}
7782

78-
loadAllDocuments()
83+
loadDocuments()
7984

8085
return () => {
8186
isMounted = false
8287
}
83-
}, [knowledgeBaseId, hasBeenLoaded, isLoading, getDocuments])
84-
85-
// Client-side filtering and pagination
86-
const { documents, pagination } = useMemo(() => {
87-
let filteredDocs = allDocuments
88-
89-
// Apply search filter
90-
if (options?.search) {
91-
const searchLower = options.search.toLowerCase()
92-
filteredDocs = filteredDocs.filter((doc) => doc.filename.toLowerCase().includes(searchLower))
93-
}
94-
95-
// Apply pagination
96-
const offset = options?.offset || 0
97-
const limit = options?.limit || DEFAULT_PAGE_SIZE
98-
const total = filteredDocs.length
99-
const paginatedDocs = filteredDocs.slice(offset, offset + limit)
88+
}, [knowledgeBaseId, isLoading, getDocuments, requestSearch, requestLimit, requestOffset])
10089

101-
return {
102-
documents: paginatedDocs,
103-
pagination: {
104-
total,
105-
limit,
106-
offset,
107-
hasMore: offset + limit < total,
108-
},
109-
}
110-
}, [allDocuments, options?.search, options?.limit, options?.offset])
90+
// Use server-side filtered and paginated results directly
91+
const documents = documentsCache?.documents || []
92+
const pagination = documentsCache?.pagination || {
93+
total: 0,
94+
limit: requestLimit,
95+
offset: requestOffset,
96+
hasMore: false,
97+
}
11198

11299
const refreshDocumentsData = useCallback(async () => {
113100
try {
114101
setError(null)
115-
await refreshDocuments(knowledgeBaseId, { limit: MAX_DOCUMENTS_LIMIT })
102+
await refreshDocuments(knowledgeBaseId, {
103+
search: requestSearch,
104+
limit: requestLimit,
105+
offset: requestOffset,
106+
})
116107
} catch (err) {
117108
setError(err instanceof Error ? err.message : 'Failed to refresh documents')
118109
}
119-
}, [knowledgeBaseId, refreshDocuments])
110+
}, [knowledgeBaseId, refreshDocuments, requestSearch, requestLimit, requestOffset])
120111

121112
const updateDocumentLocal = useCallback(
122113
(documentId: string, updates: Partial<DocumentData>) => {

apps/sim/stores/knowledge/store.ts

Lines changed: 47 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,18 @@ export const useKnowledgeStore = create<KnowledgeStore>((set, get) => ({
261261
) => {
262262
const state = get()
263263

264-
// Return cached documents if they exist (no search-based caching since we do client-side filtering)
264+
// Check if we have cached data that matches the exact request parameters
265265
const cached = state.documents[knowledgeBaseId]
266-
if (cached && cached.documents.length > 0) {
266+
const requestLimit = options?.limit || 50
267+
const requestOffset = options?.offset || 0
268+
const requestSearch = options?.search
269+
270+
if (
271+
cached &&
272+
cached.searchQuery === requestSearch &&
273+
cached.pagination.limit === requestLimit &&
274+
cached.pagination.offset === requestOffset
275+
) {
267276
return cached.documents
268277
}
269278

@@ -277,11 +286,11 @@ export const useKnowledgeStore = create<KnowledgeStore>((set, get) => ({
277286
loadingDocuments: new Set([...state.loadingDocuments, knowledgeBaseId]),
278287
}))
279288

280-
// Build query parameters
289+
// Build query parameters using the same defaults as caching
281290
const params = new URLSearchParams()
282-
if (options?.search) params.set('search', options.search)
283-
if (options?.limit) params.set('limit', options.limit.toString())
284-
if (options?.offset) params.set('offset', options.offset.toString())
291+
if (requestSearch) params.set('search', requestSearch)
292+
params.set('limit', requestLimit.toString())
293+
params.set('offset', requestOffset.toString())
285294

286295
const url = `/api/knowledge/${knowledgeBaseId}/documents${params.toString() ? `?${params.toString()}` : ''}`
287296
const response = await fetch(url)
@@ -299,15 +308,15 @@ export const useKnowledgeStore = create<KnowledgeStore>((set, get) => ({
299308
const documents = result.data.documents || result.data // Handle both paginated and non-paginated responses
300309
const pagination = result.data.pagination || {
301310
total: documents.length,
302-
limit: options?.limit || 50,
303-
offset: options?.offset || 0,
311+
limit: requestLimit,
312+
offset: requestOffset,
304313
hasMore: false,
305314
}
306315

307316
const documentsCache: DocumentsCache = {
308317
documents,
309318
pagination,
310-
searchQuery: options?.search,
319+
searchQuery: requestSearch,
311320
lastFetchTime: Date.now(),
312321
}
313322

@@ -515,11 +524,15 @@ export const useKnowledgeStore = create<KnowledgeStore>((set, get) => ({
515524
loadingDocuments: new Set([...state.loadingDocuments, knowledgeBaseId]),
516525
}))
517526

518-
// Build query parameters - for refresh, always start from offset 0
527+
// Build query parameters using consistent defaults
528+
const requestLimit = options?.limit || 50
529+
const requestOffset = options?.offset || 0
530+
const requestSearch = options?.search
531+
519532
const params = new URLSearchParams()
520-
if (options?.search) params.set('search', options.search)
521-
if (options?.limit) params.set('limit', options.limit.toString())
522-
params.set('offset', '0') // Always start fresh on refresh
533+
if (requestSearch) params.set('search', requestSearch)
534+
params.set('limit', requestLimit.toString())
535+
params.set('offset', requestOffset.toString())
523536

524537
const url = `/api/knowledge/${knowledgeBaseId}/documents${params.toString() ? `?${params.toString()}` : ''}`
525538
const response = await fetch(url)
@@ -534,87 +547,33 @@ export const useKnowledgeStore = create<KnowledgeStore>((set, get) => ({
534547
throw new Error(result.error || 'Failed to fetch documents')
535548
}
536549

537-
const serverDocuments = result.data.documents || result.data
550+
const documents = result.data.documents || result.data
538551
const pagination = result.data.pagination || {
539-
total: serverDocuments.length,
540-
limit: options?.limit || 50,
541-
offset: 0,
552+
total: documents.length,
553+
limit: requestLimit,
554+
offset: requestOffset,
542555
hasMore: false,
543556
}
544557

545-
set((state) => {
546-
const currentDocuments = state.documents[knowledgeBaseId]?.documents || []
547-
548-
// Create a map of server documents by filename for quick lookup
549-
const serverDocumentsByFilename = new Map()
550-
serverDocuments.forEach((doc: DocumentData) => {
551-
serverDocumentsByFilename.set(doc.filename, doc)
552-
})
553-
554-
// Filter out temporary documents that now have real server equivalents
555-
const filteredCurrentDocs = currentDocuments.filter((doc) => {
556-
// If this is a temporary document (starts with temp-) and a server document exists with the same filename
557-
if (doc.id.startsWith('temp-') && serverDocumentsByFilename.has(doc.filename)) {
558-
return false // Remove the temporary document
559-
}
560-
561-
// If this is a real document that still exists on the server, keep it for merging
562-
if (!doc.id.startsWith('temp-')) {
563-
const serverDoc = serverDocuments.find((sDoc: DocumentData) => sDoc.id === doc.id)
564-
if (serverDoc) {
565-
return false // Will be replaced by server version in merge below
566-
}
567-
}
568-
569-
// Keep temporary documents that don't have server equivalents yet
570-
return true
571-
})
572-
573-
// Merge server documents with any remaining local documents
574-
const mergedDocuments = serverDocuments.map((serverDoc: DocumentData) => {
575-
const existingDoc = currentDocuments.find((doc) => doc.id === serverDoc.id)
576-
577-
if (!existingDoc) {
578-
// New document from server, use it as-is
579-
return serverDoc
580-
}
581-
582-
// Merge logic for existing documents (prefer server data for most fields)
583-
return {
584-
...existingDoc,
585-
...serverDoc,
586-
// Preserve any local optimistic updates that haven't been reflected on server yet
587-
...(existingDoc.processingStatus !== serverDoc.processingStatus &&
588-
['pending', 'processing'].includes(existingDoc.processingStatus) &&
589-
!serverDoc.processingStartedAt
590-
? { processingStatus: existingDoc.processingStatus }
591-
: {}),
592-
}
593-
})
594-
595-
// Add any remaining temporary documents that don't have server equivalents
596-
const finalDocuments = [...mergedDocuments, ...filteredCurrentDocs]
597-
598-
const documentsCache: DocumentsCache = {
599-
documents: finalDocuments,
600-
pagination,
601-
searchQuery: options?.search,
602-
lastFetchTime: Date.now(),
603-
}
604-
605-
return {
606-
documents: {
607-
...state.documents,
608-
[knowledgeBaseId]: documentsCache,
609-
},
610-
loadingDocuments: new Set(
611-
[...state.loadingDocuments].filter((loadingId) => loadingId !== knowledgeBaseId)
612-
),
613-
}
614-
})
558+
const documentsCache: DocumentsCache = {
559+
documents,
560+
pagination,
561+
searchQuery: requestSearch,
562+
lastFetchTime: Date.now(),
563+
}
564+
565+
set((state) => ({
566+
documents: {
567+
...state.documents,
568+
[knowledgeBaseId]: documentsCache,
569+
},
570+
loadingDocuments: new Set(
571+
[...state.loadingDocuments].filter((loadingId) => loadingId !== knowledgeBaseId)
572+
),
573+
}))
615574

616575
logger.info(`Documents refreshed for knowledge base: ${knowledgeBaseId}`)
617-
return serverDocuments
576+
return documents
618577
} catch (error) {
619578
logger.error(`Error refreshing documents for knowledge base ${knowledgeBaseId}:`, error)
620579

0 commit comments

Comments
 (0)