Skip to content

Commit 054d243

Browse files
j15zclaude
andcommitted
fix(review): harden docs corpus edges — trailing-slash glob, root-index drops, oversized-line reads
Review findings applied from the multi-agent pass on this branch: - glob("docs/") matched no key and silently returned empty; normalize now strips trailing slashes so it resolves like "docs" - unscoped search_docs no longer returns root-homepage chunks that would only be counted against topK and then dropped as stale (the manifest deliberately omits index.mdx) - a docs page whose single line exceeds the inline cap now fails with grep guidance instead of returning an over-cap payload as success - test coverage for the vfs docs routing (glob/read/grep dispatch, DocsCorpusError surfacing, truncation paths), the search_docs server tool's shortfall notes, the empty-embedding outcome, and the inert @docs context Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1ed1929 commit 054d243

8 files changed

Lines changed: 272 additions & 10 deletions

File tree

apps/sim/lib/copilot/chat/process-contents.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ describe('processContextsServer - skill contexts', () => {
7878
})
7979
})
8080

81+
describe('processContextsServer - docs contexts', () => {
82+
beforeEach(() => {
83+
vi.clearAllMocks()
84+
})
85+
86+
it('resolves a tagged docs context to nothing while @docs tagging is disabled', async () => {
87+
const result = await processContextsServer(
88+
[{ kind: 'docs', label: 'Docs' } as ChatContext],
89+
'user-1',
90+
'how do loops work @Docs',
91+
'ws-1'
92+
)
93+
94+
expect(result).toEqual([])
95+
})
96+
})
97+
8198
describe('processContextsServer - MCP contexts', () => {
8299
beforeEach(() => {
83100
vi.clearAllMocks()

apps/sim/lib/copilot/docs/docs-corpus.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ describe('globDocs', () => {
5858
expect(globDocs('docs/workflows.mdx')).toEqual(['docs/workflows.mdx'])
5959
expect(globDocs('docs/workflows/index.mdx')).toEqual([])
6060
})
61+
62+
it('treats a trailing-slash pattern like the bare directory instead of matching nothing', () => {
63+
expect(globDocs('docs/')).toEqual(['docs'])
64+
expect(globDocs('docs/integrations/')).toEqual(['docs/integrations'])
65+
})
6166
})
6267

6368
describe('readDocsPage', () => {

apps/sim/lib/copilot/docs/docs-corpus.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ const docsKeyView: Map<string, string> = new Map(
3838
)
3939

4040
function normalize(path: string): string {
41-
return path.trim().replace(/^\/+/, '')
41+
// Trailing slashes are stripped so `docs/` addresses the corpus the same way
42+
// `docs` does — otherwise a trailing-slash glob pattern matches no key and
43+
// silently returns an empty result instead of the corpus listing.
44+
return path.trim().replace(/^\/+/, '').replace(/\/+$/, '')
4245
}
4346

4447
/**

apps/sim/lib/copilot/docs/docs-search.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ vi.mock('drizzle-orm', () => {
2626
and: op('and'),
2727
or: op('or'),
2828
eq: op('eq'),
29+
ne: op('ne'),
2930
like: op('like'),
3031
notLike: op('notLike'),
3132
sql: (strings: TemplateStringsArray) => ({ op: 'sql', text: strings.join('?') }),
@@ -77,6 +78,12 @@ describe('searchDocs path scoping', () => {
7778
expect(whereText()).toContain('academy/%')
7879
})
7980

81+
it('excludes the root homepage when unscoped — its chunks have no live docs/ path', async () => {
82+
await searchDocs('cron')
83+
expect(whereText()).toContain('"op":"ne"')
84+
expect(whereText()).toContain('index.mdx')
85+
})
86+
8087
it('scopes a page to both on-disk layouts', async () => {
8188
await searchDocs('cron', { path: 'docs/workflows/blocks/agent.mdx' })
8289
const text = whereText()
@@ -171,6 +178,17 @@ describe('searchDocs results', () => {
171178
expect((await searchDocs('cron')).results).toEqual([])
172179
})
173180

181+
it('returns the zero-candidate outcome without querying when the embedding is empty', async () => {
182+
mockGenerateSearchEmbedding.mockResolvedValue({ embedding: [] })
183+
const outcome = await searchDocs('cron')
184+
expect(outcome).toEqual({
185+
results: [],
186+
candidatesConsidered: 0,
187+
droppedBelowThreshold: 0,
188+
droppedStale: 0,
189+
})
190+
})
191+
174192
it('drops chunks below the similarity threshold', async () => {
175193
mockRows.value = [
176194
{

apps/sim/lib/copilot/docs/docs-search.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { db } from '@sim/db'
22
import { docsEmbeddings } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { and, eq, like, notLike, or, sql } from 'drizzle-orm'
4+
import { and, eq, like, ne, notLike, or, sql } from 'drizzle-orm'
55
import { docsPathForSourceDocument, isDocsDir, isDocsPage } from '@/lib/copilot/docs/docs-corpus'
66
import { docsSourceCandidates, UNMOUNTED_DOCS_SECTIONS } from '@/lib/copilot/docs/docs-path'
77
import { generateSearchEmbedding } from '@/lib/knowledge/embeddings'
@@ -60,12 +60,16 @@ export class DocsSearchScopeError extends Error {
6060
*
6161
* An unscoped search excludes every {@link UNMOUNTED_DOCS_SECTIONS} section:
6262
* they are indexed but not mounted in the VFS, so a hit there would be a chunk
63-
* the agent cannot then read.
63+
* the agent cannot then read. The root homepage (`index.mdx`) is excluded for
64+
* the same reason — the manifest generator drops it (its URL is `/`, which
65+
* redirects), so its chunks would only ever be counted against topK and then
66+
* discarded as stale.
6467
*/
6568
function scopeCondition(path?: string) {
6669
const normalized = (path ?? '').trim().replace(/^\/+/, '').replace(/\/+$/, '')
6770
if (normalized === '' || normalized === 'docs') {
6871
return and(
72+
ne(docsEmbeddings.sourceDocument, 'index.mdx'),
6973
...UNMOUNTED_DOCS_SECTIONS.map((section) =>
7074
notLike(docsEmbeddings.sourceDocument, `${section}/%`)
7175
)

apps/sim/lib/copilot/tools/handlers/vfs.test.ts

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44

5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66
import { TOOL_RESULT_MAX_INLINE_CHARS } from '@/lib/copilot/constants'
77

88
const { getOrMaterializeVFS } = vi.hoisted(() => ({
@@ -405,3 +405,109 @@ describe('vfs uploads are opt-in (like recently-deleted/)', () => {
405405
expect(grepChatUpload).toHaveBeenCalledWith('report.json', 'chat-1', 'x', expect.any(Object))
406406
})
407407
})
408+
409+
describe('vfs handlers docs corpus routing', () => {
410+
const fetchMock = vi.fn()
411+
const DOCS_PAGE = 'docs/workflows/blocks/agent.mdx'
412+
413+
beforeEach(() => {
414+
vi.clearAllMocks()
415+
fetchMock.mockReset()
416+
vi.stubGlobal('fetch', fetchMock)
417+
})
418+
419+
afterEach(() => {
420+
vi.unstubAllGlobals()
421+
})
422+
423+
it('globs the docs corpus without materializing the workspace VFS', async () => {
424+
const result = await executeVfsGlob({ pattern: 'docs/**' }, GREP_CTX)
425+
426+
expect(result.success).toBe(true)
427+
expect((result.output as { files: string[] }).files).toContain(DOCS_PAGE)
428+
expect(getOrMaterializeVFS).not.toHaveBeenCalled()
429+
})
430+
431+
it('reads a docs page via the live-site fetch, not the workspace VFS', async () => {
432+
fetchMock.mockResolvedValue({ ok: true, status: 200, text: async () => 'line one\nline two' })
433+
434+
const result = await executeVfsRead({ path: DOCS_PAGE }, GREP_CTX)
435+
436+
expect(result.success).toBe(true)
437+
expect(result.output).toEqual({ content: 'line one\nline two', totalLines: 2 })
438+
expect(getOrMaterializeVFS).not.toHaveBeenCalled()
439+
})
440+
441+
it('surfaces DocsCorpusError messages verbatim from read, without fetching', async () => {
442+
const unknown = await executeVfsRead({ path: 'docs/not-a-real-page.mdx' }, GREP_CTX)
443+
expect(unknown.success).toBe(false)
444+
expect(unknown.error).toContain('Docs page not found')
445+
446+
const dir = await executeVfsRead({ path: 'docs/workflows/blocks' }, GREP_CTX)
447+
expect(dir.success).toBe(false)
448+
expect(dir.error).toContain('is a directory')
449+
expect(fetchMock).not.toHaveBeenCalled()
450+
})
451+
452+
it('greps exactly one docs page and rejects multi-page scopes verbatim', async () => {
453+
fetchMock.mockResolvedValue({
454+
ok: true,
455+
status: 200,
456+
text: async () => 'alpha\ncron beta\ngamma',
457+
})
458+
459+
const single = await executeVfsGrep({ pattern: 'cron', path: DOCS_PAGE }, GREP_CTX)
460+
expect(single.success).toBe(true)
461+
462+
const multi = await executeVfsGrep({ pattern: 'cron', path: 'docs/workflows' }, GREP_CTX)
463+
expect(multi.success).toBe(false)
464+
expect(multi.error).toContain('single page')
465+
expect(getOrMaterializeVFS).not.toHaveBeenCalled()
466+
})
467+
468+
it('truncates an oversized multi-line docs page to fit the inline cap', async () => {
469+
const line = 'y'.repeat(200)
470+
const totalLines = Math.ceil((TOOL_RESULT_MAX_INLINE_CHARS * 2) / (line.length + 1))
471+
fetchMock.mockResolvedValue({
472+
ok: true,
473+
status: 200,
474+
text: async () => Array.from({ length: totalLines }, () => line).join('\n'),
475+
})
476+
477+
const result = await executeVfsRead({ path: DOCS_PAGE }, GREP_CTX)
478+
479+
expect(result.success).toBe(true)
480+
const output = result.output as { content: string; totalLines: number }
481+
expect(output.totalLines).toBe(totalLines)
482+
expect(output.content).toContain('[Page truncated: returned lines 1-')
483+
expect(JSON.stringify(output).length).toBeLessThanOrEqual(TOOL_RESULT_MAX_INLINE_CHARS)
484+
})
485+
486+
it('fails a docs page whose single line cannot fit inline instead of returning it oversized', async () => {
487+
fetchMock.mockResolvedValue({
488+
ok: true,
489+
status: 200,
490+
text: async () => 'z'.repeat(TOOL_RESULT_MAX_INLINE_CHARS + 1000),
491+
})
492+
493+
const result = await executeVfsRead({ path: DOCS_PAGE }, GREP_CTX)
494+
495+
expect(result.success).toBe(false)
496+
expect(result.error).toContain('Grep this page')
497+
})
498+
499+
it('rejects an explicit window that still overflows instead of truncating it', async () => {
500+
const line = 'y'.repeat(200)
501+
const totalLines = Math.ceil((TOOL_RESULT_MAX_INLINE_CHARS * 2) / (line.length + 1))
502+
fetchMock.mockResolvedValue({
503+
ok: true,
504+
status: 200,
505+
text: async () => Array.from({ length: totalLines }, () => line).join('\n'),
506+
})
507+
508+
const result = await executeVfsRead({ path: DOCS_PAGE, offset: 0, limit: totalLines }, GREP_CTX)
509+
510+
expect(result.success).toBe(false)
511+
expect(result.error).toContain('still too large over the requested window')
512+
})
513+
})

apps/sim/lib/copilot/tools/handlers/vfs.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,14 @@ function hasModelAttachment(result: unknown): boolean {
9090
/**
9191
* Trim an oversized docs page to the largest whole-line prefix that fits the
9292
* inline budget, preserving the true `totalLines` so the model can page through
93-
* the rest with offset/limit.
93+
* the rest with offset/limit. Returns null when not even one line fits — a
94+
* single line longer than the cap — so the caller can fail instead of returning
95+
* an over-cap payload as success.
9496
*/
9597
function truncateDocsPageToInlineCap(page: { content: string; totalLines: number }): {
9698
output: { content: string; totalLines: number }
9799
returnedLines: number
98-
} {
100+
} | null {
99101
const lines = page.content.split('\n')
100102
// Route to ONE more fetch, not two. Telling the model to grep and then read
101103
// costs two more uncached fetches of a page it already partly has; grep and
@@ -105,17 +107,16 @@ function truncateDocsPageToInlineCap(page: { content: string; totalLines: number
105107
`\n\n[Page truncated: returned lines 1-${shown} of ${page.totalLines}. To continue, read this path with offset: ${shown}. To jump straight to a section, grep this path INSTEAD of reading it — grep is the same single fetch and returns only matching lines with their numbers.]`
106108

107109
let kept = lines.length
108-
let content = page.content
109110
while (kept > 0) {
110-
content = `${lines.slice(0, kept).join('\n')}${notice(kept)}`
111+
const content = `${lines.slice(0, kept).join('\n')}${notice(kept)}`
111112
if (
112113
serializedResultSize({ content, totalLines: page.totalLines }) <= TOOL_RESULT_MAX_INLINE_CHARS
113114
) {
114-
break
115+
return { output: { content, totalLines: page.totalLines }, returnedLines: kept }
115116
}
116117
kept = Math.floor(kept / 2)
117118
}
118-
return { output: { content, totalLines: page.totalLines }, returnedLines: kept }
119+
return null
119120
}
120121

121122
export async function executeVfsGrep(
@@ -317,6 +318,12 @@ export async function executeVfsRead(
317318
}
318319
}
319320
const truncated = truncateDocsPageToInlineCap(page)
321+
if (!truncated) {
322+
return {
323+
success: false,
324+
error: `${path} is too large to return inline even truncated. Grep this page for the section you need.`,
325+
}
326+
}
320327
logger.debug('vfs_read truncated oversized docs page', {
321328
path,
322329
totalLines: page.totalLines,
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import type { DocsSearchOutcome } from '@/lib/copilot/docs/docs-search'
6+
7+
const { mockSearchDocs } = vi.hoisted(() => ({
8+
mockSearchDocs: vi.fn(),
9+
}))
10+
11+
vi.mock('@/lib/copilot/docs/docs-search', () => ({
12+
searchDocs: mockSearchDocs,
13+
}))
14+
15+
import { searchDocsServerTool } from '@/lib/copilot/tools/server/docs/search-docs'
16+
17+
function outcome(overrides: Partial<DocsSearchOutcome>): DocsSearchOutcome {
18+
return {
19+
results: [],
20+
candidatesConsidered: 0,
21+
droppedBelowThreshold: 0,
22+
droppedStale: 0,
23+
...overrides,
24+
}
25+
}
26+
27+
const RESULT = {
28+
path: 'docs/agents.mdx',
29+
url: 'https://docs.sim.ai/agents',
30+
title: 'Agents',
31+
content: 'body',
32+
similarity: 0.9,
33+
}
34+
35+
describe('searchDocsServerTool', () => {
36+
beforeEach(() => {
37+
mockSearchDocs.mockReset()
38+
})
39+
40+
it('forwards query, path, and topK to the search layer', async () => {
41+
mockSearchDocs.mockResolvedValue(outcome({ results: [RESULT], candidatesConsidered: 1 }))
42+
43+
const output = await searchDocsServerTool.execute({
44+
query: 'how do agents work',
45+
path: 'docs/agents.mdx',
46+
topK: 7,
47+
})
48+
49+
expect(mockSearchDocs).toHaveBeenCalledWith('how do agents work', {
50+
path: 'docs/agents.mdx',
51+
topK: 7,
52+
})
53+
expect(output).toEqual({
54+
results: [RESULT],
55+
query: 'how do agents work',
56+
totalResults: 1,
57+
})
58+
})
59+
60+
it('omits the note when nothing was dropped', async () => {
61+
mockSearchDocs.mockResolvedValue(outcome({ results: [RESULT], candidatesConsidered: 1 }))
62+
63+
const output = await searchDocsServerTool.execute({ query: 'q' })
64+
65+
expect(output.note).toBeUndefined()
66+
})
67+
68+
it('explains an empty result set caused by filtering, so it does not read as missing docs', async () => {
69+
mockSearchDocs.mockResolvedValue(
70+
outcome({ candidatesConsidered: 2, droppedBelowThreshold: 1, droppedStale: 1 })
71+
)
72+
73+
const output = await searchDocsServerTool.execute({ query: 'q' })
74+
75+
expect(output.note).toContain('does NOT mean the docs lack this topic')
76+
expect(output.note).toContain('1 scored too low')
77+
expect(output.note).toContain('1 point at pages no longer in the docs')
78+
})
79+
80+
it('notes threshold-only drops on a partial result set', async () => {
81+
mockSearchDocs.mockResolvedValue(
82+
outcome({ results: [RESULT], candidatesConsidered: 3, droppedBelowThreshold: 2 })
83+
)
84+
85+
const output = await searchDocsServerTool.execute({ query: 'q' })
86+
87+
expect(output.note).toContain('Returned 1 of 3 candidate(s)')
88+
expect(output.note).toContain('2 scored too low')
89+
expect(output.note).not.toContain('no longer in the docs')
90+
})
91+
92+
it('notes stale-only drops on a partial result set', async () => {
93+
mockSearchDocs.mockResolvedValue(
94+
outcome({ results: [RESULT], candidatesConsidered: 2, droppedStale: 1 })
95+
)
96+
97+
const output = await searchDocsServerTool.execute({ query: 'q' })
98+
99+
expect(output.note).toContain('1 point at pages no longer in the docs')
100+
expect(output.note).not.toContain('scored too low')
101+
})
102+
})

0 commit comments

Comments
 (0)