Skip to content

Commit 6dba116

Browse files
authored
improvement(search): reduce redundant metadata reads (#7728)
1 parent bba8f90 commit 6dba116

15 files changed

Lines changed: 778 additions & 78 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ jobs:
187187
bunx vitest run --mode integration
188188
lib/knowledge/__integration__/search-source-progress.integration.ts
189189
lib/knowledge/__integration__/search-source-pagination.integration.ts
190+
lib/knowledge/__integration__/search-reference-batching.integration.ts
190191
lib/core/outbox/service.integration.ts
191192
lib/knowledge/__integration__/connector-upload.integration.ts
192193

apps/sim/app/api/v2/knowledge/search/route.provenance.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ vi.mock('@/lib/knowledge/application/contexts', () => ({
4848
}))
4949

5050
vi.mock('@/lib/knowledge/service', () => ({
51-
getActiveKnowledgeBaseReference: mocks.getKnowledgeBase,
51+
getActiveKnowledgeBaseReferences: (ids: string[]) =>
52+
Promise.all(ids.map((id) => mocks.getKnowledgeBase(id))),
5253
}))
5354

5455
vi.mock('@/lib/knowledge/embeddings', () => ({
@@ -63,7 +64,8 @@ vi.mock('@/lib/knowledge/search/queries', () => ({
6364
}))
6465

6566
vi.mock('@/lib/knowledge/tags/service', () => ({
66-
getDocumentTagDefinitions: mocks.getTagDefinitions,
67+
getDocumentTagDefinitionsByKnowledgeBaseIds: async (ids: string[]) =>
68+
new Map(await Promise.all(ids.map(async (id) => [id, await mocks.getTagDefinitions(id)]))),
6769
}))
6870

6971
vi.mock('@/lib/knowledge/tags/utils', () => ({
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { db } from '@sim/db'
2+
import {
3+
knowledgeBase,
4+
knowledgeBaseTagDefinitions,
5+
organization,
6+
user,
7+
workspace,
8+
} from '@sim/db/schema'
9+
import { generateId } from '@sim/utils/id'
10+
import { and, eq, inArray } from 'drizzle-orm'
11+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
12+
import {
13+
createKnowledgeAclFixtureIds,
14+
seedKnowledgeAclFixture,
15+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
16+
import {
17+
getActiveKnowledgeBaseReference,
18+
getActiveKnowledgeBaseReferences,
19+
} from '@/lib/knowledge/service'
20+
import {
21+
getDocumentTagDefinitions,
22+
getDocumentTagDefinitionsByKnowledgeBaseIds,
23+
} from '@/lib/knowledge/tags/service'
24+
25+
describe('batched search reference reads', () => {
26+
const fixture = createKnowledgeAclFixtureIds()
27+
const baseIds = [fixture.knowledgeBaseId, ...Array.from({ length: 19 }, () => generateId())]
28+
const deletedBaseId = generateId()
29+
30+
beforeAll(async () => {
31+
await seedKnowledgeAclFixture(fixture)
32+
await db.insert(knowledgeBase).values(
33+
[...baseIds.slice(1), deletedBaseId].map((id, index) => ({
34+
id,
35+
userId: fixture.aliceId,
36+
workspaceId: fixture.workspaceId,
37+
name: `Batched search fixture ${index}`,
38+
deletedAt: id === deletedBaseId ? new Date() : null,
39+
}))
40+
)
41+
await db.insert(knowledgeBaseTagDefinitions).values(
42+
baseIds.slice(0, -1).flatMap((knowledgeBaseId) =>
43+
(['tag3', 'tag2'] as const).map((tagSlot) => ({
44+
id: generateId(),
45+
knowledgeBaseId,
46+
tagSlot,
47+
displayName: tagSlot,
48+
fieldType: 'text' as const,
49+
}))
50+
)
51+
)
52+
})
53+
54+
afterAll(async () => {
55+
vi.restoreAllMocks()
56+
await db.delete(workspace).where(eq(workspace.id, fixture.workspaceId))
57+
await db.delete(organization).where(eq(organization.id, fixture.organizationId))
58+
await db.delete(user).where(inArray(user.id, [fixture.aliceId, fixture.bobId]))
59+
await db.$client.end()
60+
})
61+
62+
it('returns identical active references with one query instead of twenty', async () => {
63+
const select = vi.spyOn(db, 'select')
64+
try {
65+
const expected = await Promise.all(baseIds.map(getActiveKnowledgeBaseReference))
66+
expect(select).toHaveBeenCalledTimes(20)
67+
select.mockClear()
68+
expect(await getActiveKnowledgeBaseReferences(baseIds)).toEqual(expected)
69+
expect(select).toHaveBeenCalledOnce()
70+
} finally {
71+
select.mockRestore()
72+
}
73+
})
74+
75+
it('preserves input order, duplicates, missing identities, and soft deletion', async () => {
76+
const ids = [baseIds[8], generateId(), baseIds[0], deletedBaseId, baseIds[8]]
77+
expect(await getActiveKnowledgeBaseReferences(ids)).toEqual(
78+
await Promise.all(ids.map(getActiveKnowledgeBaseReference))
79+
)
80+
})
81+
82+
it('returns identical ordered tag definitions with one query instead of twenty', async () => {
83+
const select = vi.spyOn(db, 'select')
84+
try {
85+
const expected = new Map(
86+
await Promise.all(
87+
baseIds.map(async (id) => [id, await getDocumentTagDefinitions(id)] as const)
88+
)
89+
)
90+
expect(select).toHaveBeenCalledTimes(20)
91+
select.mockClear()
92+
expect(await getDocumentTagDefinitionsByKnowledgeBaseIds(baseIds)).toEqual(expected)
93+
expect(select).toHaveBeenCalledOnce()
94+
expect(expected.get(baseIds.at(-1)!)).toEqual([])
95+
} finally {
96+
select.mockRestore()
97+
}
98+
})
99+
100+
it('does not cache updated references or tag definitions across reads', async () => {
101+
await getActiveKnowledgeBaseReferences(baseIds)
102+
await getDocumentTagDefinitionsByKnowledgeBaseIds(baseIds)
103+
await db
104+
.update(knowledgeBase)
105+
.set({ name: 'Updated reference' })
106+
.where(eq(knowledgeBase.id, baseIds[0]))
107+
await db
108+
.update(knowledgeBaseTagDefinitions)
109+
.set({ displayName: 'Updated definition' })
110+
.where(
111+
and(
112+
eq(knowledgeBaseTagDefinitions.knowledgeBaseId, baseIds[0]),
113+
eq(knowledgeBaseTagDefinitions.tagSlot, 'tag1')
114+
)
115+
)
116+
const references = await getActiveKnowledgeBaseReferences(baseIds)
117+
const tags = await getDocumentTagDefinitionsByKnowledgeBaseIds(baseIds)
118+
expect(references[0]?.name).toBe('Updated reference')
119+
expect(
120+
tags.get(baseIds[0])?.find((definition) => definition.tagSlot === 'tag1')?.displayName
121+
).toBe('Updated definition')
122+
})
123+
})

apps/sim/lib/knowledge/application/documents.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ vi.mock('@/lib/knowledge/documents/service', () => ({
7979

8080
vi.mock('@/lib/knowledge/tags/service', () => ({
8181
getDocumentTagDefinitions: mocks.getDocumentTagDefinitions,
82+
getDocumentTagDefinitionsByKnowledgeBaseIds: async (ids: string[]) =>
83+
new Map(
84+
await Promise.all(ids.map(async (id) => [id, await mocks.getDocumentTagDefinitions(id)]))
85+
),
8286
}))
8387

8488
vi.mock('@/lib/knowledge/orchestration/documents', () => ({

apps/sim/lib/knowledge/application/search.test.ts

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@ const mocks = vi.hoisted(() => ({
1313
requireOrganizationSearch: vi.fn(),
1414
resolvePermission: vi.fn(),
1515
getKnowledgeBase: vi.fn(),
16+
getKnowledgeBases: vi.fn(),
1617
resolveBilling: vi.fn(),
1718
checkUsage: vi.fn(),
1819
checkActorUsage: vi.fn(),
1920
generateEmbedding: vi.fn(),
2021
executeSearch: vi.fn(),
2122
getDocumentMetadata: vi.fn(),
2223
getTagDefinitions: vi.fn(),
24+
getTagDefinitionsBatch: vi.fn(),
2325
recordEmbeddingUsage: vi.fn(),
2426
importProvenance: vi.fn(),
2527
rerank: vi.fn(),
@@ -72,7 +74,7 @@ vi.mock('@/lib/permission-groups/resolve.server', () => ({
7274
}))
7375

7476
vi.mock('@/lib/knowledge/service', () => ({
75-
getActiveKnowledgeBaseReference: mocks.getKnowledgeBase,
77+
getActiveKnowledgeBaseReferences: mocks.getKnowledgeBases,
7678
}))
7779

7880
vi.mock('@/lib/knowledge/embeddings', () => ({
@@ -87,7 +89,7 @@ vi.mock('@/lib/knowledge/search/queries', () => ({
8789
}))
8890

8991
vi.mock('@/lib/knowledge/tags/service', () => ({
90-
getDocumentTagDefinitions: mocks.getTagDefinitions,
92+
getDocumentTagDefinitionsByKnowledgeBaseIds: mocks.getTagDefinitionsBatch,
9193
}))
9294

9395
vi.mock('@/lib/knowledge/tags/utils', () => ({
@@ -130,6 +132,13 @@ describe('knowledge search application use case', () => {
130132
mocks.resolveWorkspace.mockResolvedValue(workspace)
131133
mocks.resolvePermission.mockResolvedValue('read')
132134
mocks.getKnowledgeBase.mockResolvedValue(knowledgeBase)
135+
mocks.getKnowledgeBases.mockImplementation((ids: string[]) =>
136+
Promise.all(ids.map((id) => mocks.getKnowledgeBase(id)))
137+
)
138+
mocks.getTagDefinitionsBatch.mockImplementation(
139+
async (ids: string[]) =>
140+
new Map(await Promise.all(ids.map(async (id) => [id, await mocks.getTagDefinitions(id)])))
141+
)
133142
mocks.resolveBilling.mockResolvedValue({
134143
actorUserId: 'user-1',
135144
workspaceId: 'workspace-1',
@@ -554,6 +563,118 @@ describe('knowledge search application use case', () => {
554563

555564
expect(mocks.resolveWorkspace).not.toHaveBeenCalled()
556565
expect(mocks.getKnowledgeBase).not.toHaveBeenCalled()
566+
expect(mocks.getKnowledgeBases).not.toHaveBeenCalled()
567+
expect(mocks.getTagDefinitionsBatch).not.toHaveBeenCalled()
568+
})
569+
570+
it('loads references and tags once for twenty bases while preserving requested order', async () => {
571+
const ids = Array.from({ length: 20 }, (_, index) => `knowledge-${20 - index}`)
572+
mocks.getKnowledgeBases.mockResolvedValue(ids.map((id) => ({ ...knowledgeBase, id })))
573+
mocks.getTagDefinitionsBatch.mockResolvedValue(new Map(ids.map((id) => [id, []])))
574+
575+
const result = await searchKnowledge.execute({
576+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
577+
input: { knowledgeBaseIds: ids, query: 'answer', topK: 5 },
578+
})
579+
580+
expect(mocks.getKnowledgeBases).toHaveBeenCalledExactlyOnceWith(ids)
581+
expect(mocks.getTagDefinitionsBatch).toHaveBeenCalledExactlyOnceWith(ids)
582+
expect(result.knowledgeBaseIds).toEqual(ids)
583+
expect(result.knowledgeBaseId).toBe(ids[0])
584+
expect(result.knowledgeBases.map((base) => base.id)).toEqual(ids)
585+
expect(mocks.executeSearch).toHaveBeenCalledWith(
586+
expect.objectContaining({ knowledgeBaseIds: ids })
587+
)
588+
})
589+
590+
it('preserves duplicate requested bases in retrieval and the response', async () => {
591+
const ids = ['knowledge-2', 'knowledge-1', 'knowledge-2']
592+
mocks.getKnowledgeBases.mockResolvedValue(ids.map((id) => ({ ...knowledgeBase, id })))
593+
594+
const result = await searchKnowledge.execute({
595+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
596+
input: { knowledgeBaseIds: ids, query: 'answer', topK: 5 },
597+
})
598+
599+
expect(result.knowledgeBaseIds).toEqual(ids)
600+
expect(mocks.executeSearch).toHaveBeenCalledWith(
601+
expect.objectContaining({ knowledgeBaseIds: ids })
602+
)
603+
})
604+
605+
it('preserves missing-id order and duplicates in the concealed error before authorization or billing', async () => {
606+
mocks.getKnowledgeBases.mockResolvedValue([null, knowledgeBase, null, null])
607+
608+
await expect(
609+
searchKnowledge.execute({
610+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
611+
input: {
612+
knowledgeBaseIds: ['missing-2', 'knowledge-1', 'missing-1', 'missing-2'],
613+
query: 'answer',
614+
topK: 5,
615+
},
616+
})
617+
).rejects.toMatchObject({
618+
code: 'not_found',
619+
message: 'Knowledge bases not found or access denied: missing-2, missing-1, missing-2',
620+
})
621+
expect(mocks.resolvePermission).not.toHaveBeenCalled()
622+
expect(mocks.resolveBilling).not.toHaveBeenCalled()
623+
expect(mocks.executeSearch).not.toHaveBeenCalled()
624+
})
625+
626+
it('rejects a batch spanning different canonical workspaces before billing', async () => {
627+
mocks.getKnowledgeBases.mockResolvedValue([
628+
knowledgeBase,
629+
{ ...knowledgeBase, id: 'knowledge-2', workspaceId: 'workspace-2' },
630+
])
631+
632+
await expect(
633+
searchKnowledge.execute({
634+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
635+
input: { knowledgeBaseIds: ['knowledge-1', 'knowledge-2'], query: 'answer', topK: 5 },
636+
})
637+
).rejects.toMatchObject({
638+
code: 'validation',
639+
message: 'Selected knowledge bases must belong to the same workspace',
640+
})
641+
expect(mocks.resolveBilling).not.toHaveBeenCalled()
642+
expect(mocks.executeSearch).not.toHaveBeenCalled()
643+
})
644+
645+
it('reuses the tag filter batch when naming result metadata', async () => {
646+
const ids = ['knowledge-1', 'knowledge-2']
647+
mocks.getKnowledgeBases.mockResolvedValue(ids.map((id) => ({ ...knowledgeBase, id })))
648+
mocks.getTagDefinitionsBatch.mockResolvedValue(
649+
new Map(
650+
ids.map((id) => [
651+
id,
652+
[{ knowledgeBaseId: id, tagSlot: 'tag1', displayName: 'team', fieldType: 'text' }],
653+
])
654+
)
655+
)
656+
mocks.executeSearch.mockResolvedValue([
657+
{
658+
id: 'chunk-1',
659+
documentId: 'document-1',
660+
knowledgeBaseId: ids[0],
661+
content: 'answer',
662+
tag1: 'docs',
663+
},
664+
])
665+
666+
const result = await searchKnowledge.execute({
667+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
668+
input: {
669+
knowledgeBaseIds: ids,
670+
topK: 5,
671+
tagFilters: [{ tagName: 'team', operator: 'eq', value: 'docs' }],
672+
},
673+
})
674+
675+
expect(mocks.getTagDefinitionsBatch).toHaveBeenCalledExactlyOnceWith(ids)
676+
expect(result.results[0].metadata).toEqual({ team: 'docs' })
677+
expect(mocks.generateEmbedding).not.toHaveBeenCalled()
557678
})
558679

559680
it('rejects multi-knowledge-base tag filters without embedding spend', async () => {

apps/sim/lib/knowledge/application/search.ts

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ import {
4949
import { importKnowledgeSearchResultSecretProvenance } from '@/lib/knowledge/secret-provenance'
5050
import {
5151
type ActiveKnowledgeBaseReference,
52-
getActiveKnowledgeBaseReference,
52+
getActiveKnowledgeBaseReferences,
5353
} from '@/lib/knowledge/service'
5454
import {
5555
type KnowledgeTagNameFilter,
5656
resolveKnowledgeTagFilters,
5757
} from '@/lib/knowledge/tags/filter-resolution'
58-
import { getDocumentTagDefinitions } from '@/lib/knowledge/tags/service'
58+
import { getDocumentTagDefinitionsByKnowledgeBaseIds } from '@/lib/knowledge/tags/service'
5959
import type { DocumentTagDefinition } from '@/lib/knowledge/tags/types'
6060
import type { StructuredFilter } from '@/lib/knowledge/types'
6161
import { estimateTokenCount } from '@/lib/tokenization/estimators'
@@ -189,9 +189,7 @@ async function resolveKnowledgeSearchContext(
189189
`topK must be an integer between 1 and ${KNOWLEDGE_SEARCH_COST_POLICY.maxTopK}`
190190
)
191191
}
192-
const knowledgeBases = await Promise.all(
193-
input.knowledgeBaseIds.map(getActiveKnowledgeBaseReference)
194-
)
192+
const knowledgeBases = await getActiveKnowledgeBaseReferences(input.knowledgeBaseIds)
195193
const missingIds = input.knowledgeBaseIds.filter((_, index) => {
196194
const knowledgeBase = knowledgeBases[index]
197195
return !knowledgeBase || (!knowledgeBase.workspaceId && !knowledgeBase.organizationId)
@@ -558,18 +556,16 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({
558556
}
559557
}
560558

561-
const tagDefinitionEntries = await Promise.all(
562-
knowledgeBaseIds.map(async (knowledgeBaseId) => {
563-
const definitions =
564-
definitionsByKnowledgeBase.get(knowledgeBaseId) ??
565-
(await getDocumentTagDefinitions(knowledgeBaseId))
566-
return [
567-
knowledgeBaseId,
568-
new Map(definitions.map((definition) => [definition.tagSlot, definition.displayName])),
569-
] as const
570-
})
559+
if (filters.length === 0) {
560+
definitionsByKnowledgeBase =
561+
await getDocumentTagDefinitionsByKnowledgeBaseIds(knowledgeBaseIds)
562+
}
563+
const tagMaps = new Map(
564+
[...definitionsByKnowledgeBase].map(([knowledgeBaseId, definitions]) => [
565+
knowledgeBaseId,
566+
new Map(definitions.map((definition) => [definition.tagSlot, definition.displayName])),
567+
])
571568
)
572-
const tagMaps = new Map(tagDefinitionEntries)
573569
/**
574570
* Always read: the provenance snapshot vouches for the name, URL, and tags
575571
* a model may see, but the source card's modified time and connector type

0 commit comments

Comments
 (0)