Skip to content

Commit 9d00669

Browse files
authored
v0.8.49: knowledge search improvements, tables perf improvements
2 parents 7fc9e79 + d2a4e47 commit 9d00669

35 files changed

Lines changed: 57941 additions & 559 deletions

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -877,14 +877,4 @@ describe('Knowledge Search Utils', () => {
877877
)
878878
})
879879
})
880-
881-
describe('getDocumentMetadataByIds', () => {
882-
it('should handle empty input gracefully', async () => {
883-
const { getDocumentMetadataByIds } = await import('@/lib/knowledge/search/queries')
884-
885-
const result = await getDocumentMetadataByIds([])
886-
887-
expect(result).toEqual({})
888-
})
889-
})
890880
})

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ interface KnowledgeBaseData {
88
userId: string
99
workspaceId?: string | null
1010
name: string
11+
isSearchIndex: boolean
1112
description?: string | null
1213
tokenCount: number
1314
embeddingModel: string
@@ -22,7 +23,13 @@ export interface KnowledgeBaseAccessResult {
2223
hasAccess: true
2324
knowledgeBase: Pick<
2425
KnowledgeBaseData,
25-
'id' | 'userId' | 'workspaceId' | 'name' | 'embeddingModel' | 'embeddingDimension'
26+
| 'id'
27+
| 'userId'
28+
| 'workspaceId'
29+
| 'name'
30+
| 'isSearchIndex'
31+
| 'embeddingModel'
32+
| 'embeddingDimension'
2633
>
2734
}
2835

@@ -52,6 +59,7 @@ async function resolveKnowledgeBaseAccess(
5259
userId: knowledgeBase.userId,
5360
workspaceId: knowledgeBase.workspaceId,
5461
name: knowledgeBase.name,
62+
isSearchIndex: knowledgeBase.isSearchIndex,
5563
embeddingModel: knowledgeBase.embeddingModel,
5664
embeddingDimension: knowledgeBase.embeddingDimension,
5765
})

‎apps/sim/app/api/table/capability-gate.test.ts‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,26 @@ import {
2020
import { NextRequest } from 'next/server'
2121
import { beforeEach, describe, expect, it, vi } from 'vitest'
2222

23-
const { mockGetTableById, mockGetUserEntityPermissions, mockAddTableColumn, mockListTableViews } =
23+
const { mockGetTableById, mockCheckWorkspaceAccess, mockAddTableColumn, mockListTableViews } =
2424
vi.hoisted(() => ({
2525
mockGetTableById: vi.fn(),
26-
mockGetUserEntityPermissions: vi.fn(),
26+
mockCheckWorkspaceAccess: vi.fn(),
2727
mockAddTableColumn: vi.fn(),
2828
mockListTableViews: vi.fn(),
2929
}))
3030

31+
/** The shape `checkAccess` reads: the viewer's permission plus the workspace it just loaded. */
32+
function workspaceAccess(permission: string | null, organizationId: string | null = 'org-1') {
33+
return {
34+
exists: true,
35+
hasAccess: permission !== null,
36+
canWrite: permission === 'admin' || permission === 'write',
37+
canAdmin: permission === 'admin',
38+
workspace: { id: 'ws-1', organizationId },
39+
permission,
40+
}
41+
}
42+
3143
vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScopeMock)
3244

3345
vi.mock('@/lib/table', () => ({
@@ -44,7 +56,7 @@ vi.mock('@/lib/table/events', () => ({ signalTableSchemaChanged: vi.fn() }))
4456
vi.mock('@/lib/table/orchestration', () => ({ performUpdateTableColumn: vi.fn() }))
4557
vi.mock('@/lib/table/wire', () => ({ normalizeColumn: (column: unknown) => column }))
4658
vi.mock('@/lib/workspaces/permissions/utils', () => ({
47-
getUserEntityPermissions: mockGetUserEntityPermissions,
59+
checkWorkspaceAccess: mockCheckWorkspaceAccess,
4860
}))
4961
vi.mock('@/lib/workspaces/utils', () => ({ getWorkspaceOrganizationId: vi.fn() }))
5062

@@ -96,11 +108,42 @@ describe('tables.use gate on the raw /api/table routes', () => {
96108
authType: 'session',
97109
})
98110
mockGetTableById.mockResolvedValue(TABLE)
99-
mockGetUserEntityPermissions.mockResolvedValue('admin')
111+
mockCheckWorkspaceAccess.mockResolvedValue(workspaceAccess('admin'))
100112
mockAddTableColumn.mockResolvedValue({ schema: { columns: [{ name: 'expires_at' }] } })
101113
mockListTableViews.mockResolvedValue([])
102114
})
103115

116+
/**
117+
* The capability resolver looks the workspace up itself when the organization is omitted, so a
118+
* call site that already access-checked the workspace and drops the id pays a second read of a
119+
* value it is holding — once on every raw table route. Asserted on the resolver rather than on
120+
* a query count because that is where the omission would show.
121+
*/
122+
it('hands the capability resolver the organization it just loaded, not undefined', async () => {
123+
mockCheckWorkspaceAccess.mockResolvedValue(workspaceAccess('admin', 'org-42'))
124+
125+
await listViews()
126+
127+
expect(permissionGroupScopeMockFns.mockResolvePermissionGroupConfig).toHaveBeenCalledWith(
128+
expect.any(String),
129+
expect.any(String),
130+
'org-42'
131+
)
132+
})
133+
134+
/** A personal workspace has no organization; `null` is the answer, and still not a lookup. */
135+
it('passes null for a workspace that belongs to no organization', async () => {
136+
mockCheckWorkspaceAccess.mockResolvedValue(workspaceAccess('admin', null))
137+
138+
await listViews()
139+
140+
expect(permissionGroupScopeMockFns.mockResolvePermissionGroupConfig).toHaveBeenCalledWith(
141+
expect.any(String),
142+
expect.any(String),
143+
null
144+
)
145+
})
146+
104147
describe('when the group withholds Tables', () => {
105148
beforeEach(() => {
106149
permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue({
@@ -131,7 +174,7 @@ describe('tables.use gate on the raw /api/table routes', () => {
131174
})
132175

133176
it('still conceals a table the caller cannot reach, rather than naming the capability', async () => {
134-
mockGetUserEntityPermissions.mockResolvedValue(null)
177+
mockCheckWorkspaceAccess.mockResolvedValue(workspaceAccess(null))
135178

136179
const response = await listViews()
137180

‎apps/sim/app/api/table/utils.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { TableLockedError } from '@/lib/table/mutation-locks'
2323
import { isTablePredicate } from '@/lib/table/query-builder/converters'
2424
import { validateStoragePredicate } from '@/lib/table/query-builder/validate'
2525
import type { TableLockKind } from '@/lib/table/types'
26-
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
26+
import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
2727
import { getWorkspaceOrganizationId } from '@/lib/workspaces/utils'
2828

2929
/**
@@ -338,12 +338,14 @@ export async function checkAccess(
338338
return { ok: false, status: 404 }
339339
}
340340

341-
const permission = await getUserEntityPermissions(
342-
roleSubjectUserId(principal),
343-
'workspace',
344-
table.workspaceId
345-
)
346-
if (!permissionSatisfies(permission, level)) {
341+
/**
342+
* Resolved through {@link checkWorkspaceAccess} rather than `getUserEntityPermissions`, which
343+
* delegates to it and returns the permission alone. Same single resolution, but it also hands
344+
* back the workspace this check just loaded — and with it the owning organization the
345+
* capability gate below would otherwise look up for itself.
346+
*/
347+
const access = await checkWorkspaceAccess(table.workspaceId, roleSubjectUserId(principal))
348+
if (!permissionSatisfies(access.permission, level)) {
347349
return { ok: false, status: 403 }
348350
}
349351

@@ -352,7 +354,18 @@ export async function checkAccess(
352354
if (
353355
governedUserId &&
354356
table.workspaceId &&
355-
(await isWorkspaceCapabilityWithheld(governedUserId, table.workspaceId, 'tables.use'))
357+
/**
358+
* The organization is passed, not re-derived: omitting it makes the resolver load this very
359+
* workspace a second time (see `getUserPermissionConfig`), which is one extra round trip on
360+
* every raw table route. `access.workspace` is non-null on this line — a missing workspace
361+
* resolves to a null permission, which the gate above already refused.
362+
*/
363+
(await isWorkspaceCapabilityWithheld(
364+
governedUserId,
365+
table.workspaceId,
366+
'tables.use',
367+
access.workspace?.organizationId ?? null
368+
))
356369
) {
357370
return { ok: false, status: 403, capability: 'tables.use' }
358371
}

‎apps/sim/app/api/v1/knowledge/search/route.test.ts‎

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const {
1616
mockExecuteKnowledgeSearch,
1717
mockRetrievalStatus,
1818
mockGenerateSearchEmbedding,
19-
mockGetDocumentMetadataByIds,
2019
mockGetDocumentTagDefinitions,
2120
mockAuthenticateRequest,
2221
mockValidateWorkspaceAccess,
@@ -28,7 +27,6 @@ const {
2827
mockExecuteKnowledgeSearch: vi.fn(),
2928
mockRetrievalStatus: vi.fn(() => ({ status: 'complete', timedOutLegs: [] })),
3029
mockGenerateSearchEmbedding: vi.fn(),
31-
mockGetDocumentMetadataByIds: vi.fn(),
3230
mockGetDocumentTagDefinitions: vi.fn(),
3331
mockAuthenticateRequest: vi.fn(),
3432
mockValidateWorkspaceAccess: vi.fn(),
@@ -60,9 +58,7 @@ vi.mock('@/lib/knowledge/search/queries', () => ({
6058
retrieveKnowledgeSearch: async (params: { access: unknown }) => ({
6159
rows: await mockExecuteKnowledgeSearch(params),
6260
retrieval: mockRetrievalStatus(),
63-
readAccess: params.access,
6461
}),
65-
getDocumentMetadataByIds: mockGetDocumentMetadataByIds,
6662
}))
6763

6864
vi.mock('@/app/api/knowledge/utils', () => knowledgeApiUtilsMock)
@@ -132,7 +128,6 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
132128
isBYOK: false,
133129
})
134130
mockExecuteKnowledgeSearch.mockResolvedValue([])
135-
mockGetDocumentMetadataByIds.mockResolvedValue({})
136131
mockGetDocumentTagDefinitions.mockResolvedValue([])
137132
mockResolveBillingAttribution.mockImplementation(
138133
({ actorUserId, workspaceId }: { actorUserId: string; workspaceId: string }) =>
@@ -164,7 +159,6 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
164159
)
165160
expect(mockExecuteKnowledgeSearch).toHaveBeenCalledOnce()
166161
expect(response.status).toBe(500)
167-
expect(mockGetDocumentMetadataByIds).not.toHaveBeenCalled()
168162
})
169163

170164
it('retains the reader provider for ranked results and returned document metadata', async () => {
@@ -193,17 +187,11 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
193187
accessProvider: provider,
194188
})
195189
)
196-
expect(mockGetDocumentMetadataByIds).toHaveBeenCalledWith([], access)
197190
})
198191

199-
it.each([
200-
['query', false],
201-
['query', true],
202-
['filters', false],
203-
['filters', true],
204-
] as const)(
205-
'omits newly denied content from %s results and counts when all denied is %s',
206-
async (mode, allDenied) => {
192+
it.each(['query', 'filters'] as const)(
193+
'renders the source card each %s result row carries',
194+
async (mode) => {
207195
const access = { kind: 'user' as const, userId: 'user-1', tokens: ['reader-token'] }
208196
const provider = {
209197
get: vi.fn().mockResolvedValue(access),
@@ -219,26 +207,17 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
219207
{ tagSlot: 'tag1', displayName: 'category', fieldType: 'text' },
220208
])
221209
mockExecuteKnowledgeSearch.mockResolvedValue([
222-
{
223-
documentId: 'revoked-document',
224-
knowledgeBaseId: 'kb-1',
225-
content: 'revoked page content',
226-
tag1: 'revoked tag',
227-
chunkIndex: 0,
228-
distance: 0.1,
229-
},
230210
{
231211
documentId: 'allowed-document',
232212
knowledgeBaseId: 'kb-1',
233213
content: 'allowed page content',
214+
filename: 'Allowed page',
215+
sourceUrl: null,
234216
tag1: 'docs',
235217
chunkIndex: 0,
236218
distance: 0.2,
237219
},
238220
])
239-
mockGetDocumentMetadataByIds.mockResolvedValue(
240-
allDenied ? {} : { 'allowed-document': { filename: 'Allowed page', sourceUrl: null } }
241-
)
242221
const response = await POST(
243222
createMockRequest('POST', {
244223
workspaceId: 'ws-1',
@@ -250,24 +229,19 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
250229
)
251230
const body = await response.json()
252231
expect(response.status).toBe(200)
253-
expect(mockGetDocumentMetadataByIds).toHaveBeenCalledWith(
254-
['revoked-document', 'allowed-document'],
255-
access
256-
)
257-
expect(body.data.results).toEqual(
258-
allDenied
259-
? []
260-
: [
261-
expect.objectContaining({
262-
documentId: 'allowed-document',
263-
documentName: 'Allowed page',
264-
content: 'allowed page content',
265-
metadata: { category: 'docs' },
266-
}),
267-
]
232+
expect(mockExecuteKnowledgeSearch).toHaveBeenCalledWith(
233+
expect.objectContaining({ access, accessProvider: provider })
268234
)
269-
expect(body.data.totalResults).toBe(allDenied ? 0 : 1)
270-
expect(JSON.stringify(body)).not.toContain('revoked')
235+
expect(body.data.results).toEqual([
236+
expect.objectContaining({
237+
documentId: 'allowed-document',
238+
documentName: 'Allowed page',
239+
sourceUrl: null,
240+
content: 'allowed page content',
241+
metadata: { category: 'docs' },
242+
}),
243+
])
244+
expect(body.data.totalResults).toBe(1)
271245
}
272246
)
273247

@@ -372,7 +346,7 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
372346
expect(mockGenerateSearchEmbedding).not.toHaveBeenCalled()
373347
})
374348

375-
it('surfaces sourceUrl from document metadata in search results', async () => {
349+
it('surfaces the sourceUrl a result row carries', async () => {
376350
mockCheckKnowledgeBaseAccess.mockResolvedValueOnce({
377351
hasAccess: true,
378352
knowledgeBase: baseKb('kb-confluence', 'text-embedding-3-small'),
@@ -382,16 +356,12 @@ describe('v1 knowledge search route — per-KB embedding model', () => {
382356
documentId: 'doc-confluence',
383357
knowledgeBaseId: 'kb-confluence',
384358
content: 'page content',
359+
filename: 'Runbook.md',
360+
sourceUrl: 'https://example.atlassian.net/wiki/spaces/DOCS/pages/12345',
385361
chunkIndex: 0,
386362
distance: 0.1,
387363
},
388364
])
389-
mockGetDocumentMetadataByIds.mockResolvedValue({
390-
'doc-confluence': {
391-
filename: 'Runbook.md',
392-
sourceUrl: 'https://example.atlassian.net/wiki/spaces/DOCS/pages/12345',
393-
},
394-
})
395365

396366
const req = createMockRequest('POST', {
397367
workspaceId: 'ws-1',

‎apps/sim/app/api/v1/knowledge/search/route.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
import { SearchDeadlineError } from '@/lib/knowledge/search/budget'
1818
import { resolveKnowledgeSearchDefaults } from '@/lib/knowledge/search/defaults'
1919
import {
20-
getDocumentMetadataByIds,
2120
type KnowledgeRetrievalResult,
2221
retrieveKnowledgeSearch,
2322
type SearchResult,
@@ -267,6 +266,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
267266
accessProvider,
268267
searchMode,
269268
boostRecency,
269+
searchIndexOnly: accessibleKbs.every((kb) => kb.isSearchIndex),
270270
query,
271271
queryVector: {
272272
vector: JSON.stringify(queryEmbeddingResult.embedding),
@@ -316,14 +316,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
316316
/** v1 cannot express an incomplete search, so a leg that ran out of time fails the request. */
317317
if (retrieved.retrieval.status === 'partial') throw new SearchDeadlineError()
318318
const results = retrieved.rows
319-
const documentIds = results.map((r) => r.documentId)
320-
const documentMetadataMap = await getDocumentMetadataByIds(documentIds, retrieved.readAccess)
321-
const readableResults = results.filter((result) => documentMetadataMap[result.documentId])
322319

323320
return NextResponse.json({
324321
success: true,
325322
data: {
326-
results: readableResults.map((result) => {
323+
results: results.map((result) => {
327324
const kbTagMap = tagDefinitionsMap[result.knowledgeBaseId] || {}
328325
const tags: Record<string, string | number | boolean | Date | null> = {}
329326

@@ -335,11 +332,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
335332
}
336333
})
337334

338-
const docMeta = documentMetadataMap[result.documentId]
339335
return {
340336
documentId: result.documentId,
341-
documentName: docMeta?.filename || undefined,
342-
sourceUrl: docMeta?.sourceUrl ?? null,
337+
documentName: result.filename || undefined,
338+
sourceUrl: result.sourceUrl,
343339
content: result.content,
344340
chunkIndex: result.chunkIndex,
345341
metadata: tags,
@@ -349,7 +345,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
349345
query: query || '',
350346
knowledgeBaseIds: accessibleKbIds,
351347
topK,
352-
totalResults: readableResults.length,
348+
totalResults: results.length,
353349
},
354350
})
355351
} catch (error) {

0 commit comments

Comments
 (0)