Skip to content

Commit 8e2623d

Browse files
fix(search): bound retrieval and progressively read document evidence (#7789)
* fix(search): bound retrieval and progressively read document evidence * fix(search): protect query output and validate document continuation
1 parent 45ab0f2 commit 8e2623d

26 files changed

Lines changed: 1296 additions & 425 deletions

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ vi.mock('@/lib/knowledge/embeddings', () => ({
5959

6060
vi.mock('@/lib/knowledge/search/queries', () => ({
6161
generateSearchEmbedding: mocks.generateEmbedding,
62-
executeKnowledgeSearch: mocks.executeSearch,
62+
retrieveKnowledgeSearch: async (...args: unknown[]) => ({
63+
rows: await mocks.executeSearch(...args),
64+
retrieval: { status: 'complete', timedOutLegs: [] },
65+
}),
6366
getDocumentMetadataByIds: mocks.getDocumentMetadata,
6467
}))
6568

‎apps/sim/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/page.tsx‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,37 @@
11
import { ChipLink } from '@sim/emcn'
22
import { notFound, redirect } from 'next/navigation'
3+
import type { SearchParams } from 'nuqs/server'
34
import { readSearchDocumentResultSchema } from '@/lib/api/contracts/knowledge/documents'
45
import { getSession } from '@/lib/auth'
56
import { OrchestrationError } from '@/lib/core/orchestration/types'
67
import { readSearchDocument } from '@/lib/knowledge/application/read-search-document'
78
import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect'
9+
import {
10+
loadDocumentReadParams,
11+
serializeDocumentReadParams,
12+
} from '@/app/o/[organizationId]/knowledge/[knowledgeBaseId]/[documentId]/search-params'
813
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
914
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
1015

1116
interface OrganizationDocumentPageProps {
1217
params: Promise<{ organizationId: string; knowledgeBaseId: string; documentId: string }>
13-
searchParams: Promise<{ offset?: string }>
18+
searchParams: Promise<SearchParams>
1419
}
1520

1621
export default async function OrganizationDocumentPage({
1722
params,
1823
searchParams,
1924
}: OrganizationDocumentPageProps) {
2025
const { organizationId, knowledgeBaseId, documentId } = await params
21-
const { offset: rawOffset } = await searchParams
22-
const offset = rawOffset === undefined ? 0 : Number(rawOffset)
23-
if (!Number.isInteger(offset) || offset < 0 || offset > 5000) notFound()
26+
const position = await loadDocumentReadParams(searchParams, { strict: true }).catch(() =>
27+
notFound()
28+
)
2429
const href = `/o/${encodeURIComponent(organizationId)}/knowledge/${encodeURIComponent(knowledgeBaseId)}/${encodeURIComponent(documentId)}`
2530
const session = await getSession()
2631
if (!session?.user) {
2732
redirect(
2833
buildAuthCrossLink('/login', {
29-
callbackUrl: offset ? `${href}?offset=${offset}` : href,
34+
callbackUrl: serializeDocumentReadParams(href, position),
3035
isInviteFlow: false,
3136
})
3237
)
@@ -39,15 +44,15 @@ export default async function OrganizationDocumentPage({
3944
input: {
4045
documentId,
4146
assertedOrganizationId: organizationId,
42-
offset,
43-
limit: 20,
47+
...position,
48+
limit: 3,
4449
resultSecretRegistry: registry,
4550
},
4651
})
4752
} catch (error) {
4853
if (
4954
error instanceof OrchestrationError &&
50-
(error.code === 'not_found' || error.code === 'forbidden')
55+
(error.code === 'not_found' || error.code === 'forbidden' || error.code === 'validation')
5156
)
5257
notFound()
5358
throw error
@@ -71,11 +76,11 @@ export default async function OrganizationDocumentPage({
7176
</p>
7277
))}
7378
<nav className='flex gap-2' aria-label='Document pages'>
74-
{offset > 0 && (
75-
<ChipLink href={`${href}?offset=${Math.max(0, offset - 20)}`}>Previous</ChipLink>
79+
{(position.startChunkIndex > 0 || position.startOffset > 0) && (
80+
<ChipLink href={href}>Start</ChipLink>
7681
)}
77-
{document.nextOffset !== null && (
78-
<ChipLink href={`${href}?offset=${document.nextOffset}`}>Next</ChipLink>
82+
{document.next && (
83+
<ChipLink href={serializeDocumentReadParams(href, document.next)}>Next</ChipLink>
7984
)}
8085
</nav>
8186
</article>
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { createLoader, createParser, createSerializer } from 'nuqs/server'
2+
3+
const parseAsDocumentPosition = createParser({
4+
parse: (value) => {
5+
if (!/^\d+$/.test(value)) return null
6+
const position = Number(value)
7+
return Number.isSafeInteger(position) && position <= 2147483647 ? position : null
8+
},
9+
serialize: String,
10+
}).withDefault(0)
11+
12+
export const documentReadParams = {
13+
startChunkIndex: parseAsDocumentPosition,
14+
startOffset: parseAsDocumentPosition,
15+
}
16+
17+
const documentReadUrlKeys = {
18+
urlKeys: {
19+
startChunkIndex: 'start-chunk-index',
20+
startOffset: 'start-offset',
21+
},
22+
} as const
23+
24+
export const loadDocumentReadParams = createLoader(documentReadParams, documentReadUrlKeys)
25+
export const serializeDocumentReadParams = createSerializer(documentReadParams, documentReadUrlKeys)

‎apps/sim/lib/api/contracts/knowledge/documents.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,23 @@ export const readSearchDocumentResultSchema = z.object({
429429
knowledgeBaseId: z.string().min(1),
430430
documentName: z.string().nullable(),
431431
sourceUrl: z.string().nullable(),
432-
chunks: z.array(z.object({ content: z.string(), chunkIndex: z.number().int().min(0) })).max(50),
432+
chunks: z
433+
.array(
434+
z.object({
435+
content: z.string().max(8000),
436+
chunkIndex: z.number().int().min(0),
437+
startOffset: z.number().int().min(0),
438+
endOffset: z.number().int().min(0),
439+
totalCharacters: z.number().int().min(0),
440+
})
441+
)
442+
.max(8),
433443
hasMore: z.boolean(),
434-
nextOffset: z.number().int().min(0).nullable(),
444+
next: z
445+
.object({
446+
startChunkIndex: z.number().int().min(0),
447+
startOffset: z.number().int().min(0),
448+
})
449+
.nullable(),
435450
})
436451
export type ReadSearchDocumentResult = z.output<typeof readSearchDocumentResultSchema>

‎apps/sim/lib/copilot/generated/tool-catalog-v1.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4750,16 +4750,24 @@ export const ReadDocument: ToolCatalogEntry = {
47504750
type: 'string',
47514751
},
47524752
limit: {
4753-
default: 20,
4754-
description: 'Maximum number of chunks to read.',
4755-
maximum: 50,
4753+
default: 3,
4754+
description:
4755+
'Maximum chunks to read; the server may return fewer to fit its text budget. Follow next when more context is needed.',
4756+
maximum: 8,
47564757
minimum: 1,
47574758
type: 'integer',
47584759
},
4759-
offset: {
4760-
default: 0,
4761-
description: 'Number of chunks to skip.',
4762-
maximum: 5000,
4760+
startChunkIndex: {
4761+
description:
4762+
"Inclusive chunk index from search or a previous read's next object. Gaps from disabled chunks are skipped.",
4763+
maximum: 2147483647,
4764+
minimum: 0,
4765+
type: 'integer',
4766+
},
4767+
startOffset: {
4768+
description:
4769+
'UTF-16 character offset within startChunkIndex. Omit to read the chunk from its start, or copy next.startOffset to continue a partial chunk.',
4770+
maximum: 2147483647,
47634771
minimum: 0,
47644772
type: 'integer',
47654773
},
@@ -5616,7 +5624,8 @@ export const SearchWorkspace: ToolCatalogEntry = {
56165624
},
56175625
topK: {
56185626
default: 20,
5619-
description: 'Maximum number of matching chunks to return.',
5627+
description:
5628+
'Maximum number of matching passage previews to return. Retrieval ranking is independent of preview length.',
56205629
maximum: 50,
56215630
minimum: 1,
56225631
type: 'integer',

‎apps/sim/lib/copilot/generated/tool-schemas-v1.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4694,16 +4694,24 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
46944694
type: 'string',
46954695
},
46964696
limit: {
4697-
default: 20,
4698-
description: 'Maximum number of chunks to read.',
4699-
maximum: 50,
4697+
default: 3,
4698+
description:
4699+
'Maximum chunks to read; the server may return fewer to fit its text budget. Follow next when more context is needed.',
4700+
maximum: 8,
47004701
minimum: 1,
47014702
type: 'integer',
47024703
},
4703-
offset: {
4704-
default: 0,
4705-
description: 'Number of chunks to skip.',
4706-
maximum: 5000,
4704+
startChunkIndex: {
4705+
description:
4706+
"Inclusive chunk index from search or a previous read's next object. Gaps from disabled chunks are skipped.",
4707+
maximum: 2147483647,
4708+
minimum: 0,
4709+
type: 'integer',
4710+
},
4711+
startOffset: {
4712+
description:
4713+
'UTF-16 character offset within startChunkIndex. Omit to read the chunk from its start, or copy next.startOffset to continue a partial chunk.',
4714+
maximum: 2147483647,
47074715
minimum: 0,
47084716
type: 'integer',
47094717
},
@@ -5518,7 +5526,8 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
55185526
},
55195527
topK: {
55205528
default: 20,
5521-
description: 'Maximum number of matching chunks to return.',
5529+
description:
5530+
'Maximum number of matching passage previews to return. Retrieval ranking is independent of preview length.',
55225531
maximum: 50,
55235532
minimum: 1,
55245533
type: 'integer',

‎apps/sim/lib/copilot/generated/trace-attributes-v1.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ export const TraceAttr = {
478478
LlmStreamChunks: 'llm.stream.chunks',
479479
LlmStreamFirstChunkBytes: 'llm.stream.first_chunk_bytes',
480480
LlmStreamFirstChunkMs: 'llm.stream.first_chunk_ms',
481+
LlmStreamFirstTokenMs: 'llm.stream.first_token_ms',
481482
LlmStreamOpenMs: 'llm.stream.open_ms',
482483
LlmStreamTotalMs: 'llm.stream.total_ms',
483484
LockAcquired: 'lock.acquired',
@@ -1159,6 +1160,7 @@ export const TraceAttrValues: readonly TraceAttrValue[] = [
11591160
'llm.stream.chunks',
11601161
'llm.stream.first_chunk_bytes',
11611162
'llm.stream.first_chunk_ms',
1163+
'llm.stream.first_token_ms',
11621164
'llm.stream.open_ms',
11631165
'llm.stream.total_ms',
11641166
'lock.acquired',

‎apps/sim/lib/copilot/tools/server/knowledge/workspace-search.test.ts‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ vi.mock('@/lib/knowledge/application/read-search-document', () => ({
3535
execute: mocks.read,
3636
},
3737
}))
38-
vi.mock('@/executor/utils/resolved-secret-content-projection', () => ({
39-
projectResolvedSecretModelContent: (value: unknown) => ({ safe: true, value }),
40-
}))
4138

4239
import {
4340
readDocumentServerTool,
@@ -61,6 +58,7 @@ describe('Assistant retrieval tools', () => {
6158
beforeEach(() => {
6259
vi.clearAllMocks()
6360
mocks.search.mockResolvedValue({
61+
retrieval: { status: 'complete', timedOutLegs: [] },
6462
knowledgeBases: [{ id: 'index', name: 'Enterprise Search' }],
6563
results: [
6664
{
@@ -83,7 +81,7 @@ describe('Assistant retrieval tools', () => {
8381
sourceUrl: 'https://source.test/doc',
8482
chunks: [{ content: 'body', chunkIndex: 0 }],
8583
hasMore: false,
86-
nextOffset: null,
84+
next: null,
8785
})
8886
})
8987
it('pins organization and private chat while reusing the canonical search index and citations', async () => {
@@ -187,11 +185,29 @@ describe('Assistant retrieval tools', () => {
187185
})
188186
)
189187
})
188+
it('returns only the projected query to the model', async () => {
189+
const secret = 'private-resolved-query-token'
190+
const registry = new ResolvedSecretTraceRegistry([
191+
{ name: 'TOKEN', plaintext: secret, encryptedValue: 'ciphertext' },
192+
])
193+
registry.recordResolved('TOKEN', secret)
194+
const result = await searchWorkspaceServerTool.execute(
195+
{ query: `Find ${secret}` },
196+
{ ...context, resolvedSecretTraceRegistry: registry }
197+
)
198+
expect(result).toMatchObject({ success: true, data: { query: 'Find {{TOKEN}}' } })
199+
expect(mocks.search).toHaveBeenCalledWith(
200+
expect.objectContaining({ input: expect.objectContaining({ query: 'Find {{TOKEN}}' }) })
201+
)
202+
expect(JSON.stringify(result)).not.toContain(secret)
203+
})
204+
190205
it.each([0, 20, 50])(
191206
'measures UTF-8 bytes for %i passages without logging their content',
192207
async (count) => {
193208
const content = 'Confidential passage é🔎'.repeat(100)
194209
mocks.search.mockResolvedValueOnce({
210+
retrieval: { status: 'complete', timedOutLegs: [] },
195211
knowledgeBases: [{ id: 'index', name: 'Enterprise Search' }],
196212
results: Array.from({ length: count }, (_, index) => ({
197213
knowledgeBaseId: 'index',
@@ -217,8 +233,9 @@ describe('Assistant retrieval tools', () => {
217233
expect.objectContaining({
218234
toolCallId: 'call',
219235
toolResultBytes: Buffer.byteLength(JSON.stringify(output)),
220-
passageBytes: count * Buffer.byteLength(content),
221-
maxPassageBytes: count ? Buffer.byteLength(content) : 0,
236+
passageBytes: count * Buffer.byteLength(content.slice(0, 1200)),
237+
originalPassageBytes: count * Buffer.byteLength(content),
238+
maxPassageBytes: count ? Buffer.byteLength(content.slice(0, 1200)) : 0,
222239
uniqueDocumentCount: Math.min(count, 4),
223240
})
224241
)
@@ -245,6 +262,7 @@ describe('Assistant retrieval tools', () => {
245262
})
246263
it('projects the provider name for connected-source citations instead of the index name', async () => {
247264
mocks.search.mockResolvedValueOnce({
265+
retrieval: { status: 'complete', timedOutLegs: [] },
248266
knowledgeBases: [{ id: 'index', name: 'Sim Search' }],
249267
results: [
250268
{
@@ -292,20 +310,20 @@ describe('Assistant retrieval tools', () => {
292310
})
293311
it('reads a selected document through the shared use case and rejects unbounded pages', async () => {
294312
expect(
295-
await readDocumentServerTool.execute({ documentId: 'doc', offset: 20 }, context)
313+
await readDocumentServerTool.execute({ documentId: 'doc', startChunkIndex: 20 }, context)
296314
).toMatchObject({ success: true })
297315
expect(mocks.read).toHaveBeenCalledWith(
298316
expect.objectContaining({
299317
input: expect.objectContaining({
300318
assertedWorkspaceId: 'workspace',
301319
filters: context.assistantSearch,
302-
offset: 20,
303-
limit: 20,
320+
startChunkIndex: 20,
321+
limit: 3,
304322
}),
305323
})
306324
)
307325
expect(
308-
await readDocumentServerTool.execute({ documentId: 'doc', limit: 10000 }, context)
326+
await readDocumentServerTool.execute({ documentId: 'doc', limit: 9 }, context)
309327
).toMatchObject({ success: false })
310328
expect(mocks.read).toHaveBeenCalledOnce()
311329
})

0 commit comments

Comments
 (0)