Skip to content

Commit b8409d5

Browse files
j15zclaude
andcommitted
fix(review): act on docs-vfs review findings
Multi-agent review of the docs/ VFS change. Applied the behavior-preserving fixes plus two agent-facing bugs that made real pages unreadable. - docs read no longer hard-fails on oversized pages. Six+ live integration references exceed the inline cap (github.mdx is 354KB, sportmonks 513KB), so a plain read of them ALWAYS failed and cost a second fetch to recover. Truncate to the largest whole-line prefix that fits, keep the true totalLines, and tell the model how to page. An explicit offset/limit that still overflows is still an error — that one is a caller mistake. - classify docs fetch failures. Everything collapsed to null, so a permanent 404 was reported to the agent as "temporarily unavailable, retry shortly", inviting a retry loop on a page that will never exist. 4xx (except 429) is now permanent and says so; 5xx/429/network/timeout keep the retry wording. - register search_documentation as a transitional alias for search_docs. sim and mothership deploy independently and the rename deleted the old id on both sides, so BOTH deploy orders broke docs lookup for the window between them. Old params are a subset of the new. Remove once both ship. - extract the index-page fold (X/index.mdx <-> X.mdx) into docs-path.ts. It was re-derived in three places — the manifest generator, the source_document reverse mapping, and the search scope filter — which is the hand-synced-duplicate shape that has drifted in this repo before. - grepDocsPage now goes through grepReadResult, the primitive files/ and uploads/ grep already use, instead of calling grep directly. - couldMatchDocsScope delegates to isDocsPath; the bodies were identical. - drop the dead 'docs' member from AgentContextType. Tests: 404-vs-5xx-vs-429 classification, network failure, and a docs-path round-trip asserting one source candidate reproduces every manifest entry. Verified: tsc clean, 932 copilot tests, biome clean, docs-manifest:check, check:utils and check:api-validation:strict both pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 611d3c9 commit b8409d5

9 files changed

Lines changed: 174 additions & 24 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ type AgentContextType =
4242
| 'table'
4343
| 'file'
4444
| 'workflow_block'
45-
| 'docs'
4645
| 'folder'
4746
| 'filefolder'
4847
| 'active_resource'

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,29 @@ describe('readDocsPage', () => {
9393
expect(fetchMock).not.toHaveBeenCalled()
9494
})
9595

96-
it('surfaces a docs-site failure as a retryable error', async () => {
96+
it('surfaces a docs-site outage as a retryable error', async () => {
9797
fetchMock.mockResolvedValue({ ok: false, status: 502, text: async () => '' })
9898
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/temporarily unavailable/)
9999
})
100+
101+
it('treats a network failure as retryable', async () => {
102+
fetchMock.mockRejectedValue(new Error('socket hang up'))
103+
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/temporarily unavailable/)
104+
})
105+
106+
it('reports a page the site no longer serves as permanent, not retryable', async () => {
107+
fetchMock.mockResolvedValue({ ok: false, status: 404, text: async () => '' })
108+
const error = await readDocsPage(`docs/${SAMPLE_PAGE}`).catch((e) => e)
109+
expect(error).toBeInstanceOf(DocsCorpusError)
110+
expect(error.message).toMatch(/does not serve it/)
111+
expect(error.message).toMatch(/retrying will not help/)
112+
expect(error.message).not.toMatch(/temporarily unavailable/)
113+
})
114+
115+
it('still treats 429 as retryable rather than permanent', async () => {
116+
fetchMock.mockResolvedValue({ ok: false, status: 429, text: async () => '' })
117+
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/temporarily unavailable/)
118+
})
100119
})
101120

102121
describe('grepDocsPage', () => {

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

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3+
import { foldDocsIndexPath } from '@/lib/copilot/docs/docs-path'
34
import { DOCS_MANIFEST } from '@/lib/copilot/generated/docs-manifest'
45
import type { GrepCountEntry, GrepMatch, GrepOptions } from '@/lib/copilot/vfs/operations'
5-
import { glob as globPaths, grep as grepFiles } from '@/lib/copilot/vfs/operations'
6+
import { glob as globPaths, grepReadResult } from '@/lib/copilot/vfs/operations'
67

78
const logger = createLogger('DocsCorpus')
89

@@ -56,12 +57,11 @@ export function isDocsPath(path: string | undefined): boolean {
5657
* True when a glob `pattern` could match the docs corpus. Like `uploads/` and
5758
* `recently-deleted/`, the corpus is opt-in: only a pattern that explicitly
5859
* starts with `docs/` (or is exactly `docs`) sees it, so a broad `**` glob never
59-
* drags 300+ doc pages into the result.
60+
* drags 300+ doc pages into the result. Same rule as {@link isDocsPath}; the
61+
* separate name reads correctly at the glob call site.
6062
*/
6163
export function couldMatchDocsScope(pattern: string | undefined): boolean {
62-
if (!pattern) return false
63-
const normalized = normalize(pattern)
64-
return normalized === 'docs' || normalized.startsWith(DOCS_PREFIX)
64+
return isDocsPath(pattern)
6565
}
6666

6767
/** Manifest paths (and their virtual directories) matching an explicit `docs/` pattern. */
@@ -82,7 +82,7 @@ export function isDocsPage(path: string): boolean {
8282
*/
8383
export function docsPathForSourceDocument(sourceDocument: string | null): string | null {
8484
if (!sourceDocument) return null
85-
const path = `${DOCS_PREFIX}${sourceDocument.replace(/^\/+/, '').replace(/\/index\.mdx$/, '.mdx')}`
85+
const path = `${DOCS_PREFIX}${foldDocsIndexPath(sourceDocument.replace(/^\/+/, ''))}`
8686
return docsKeyView.has(path) ? path : null
8787
}
8888

@@ -108,9 +108,16 @@ export interface DocsPage {
108108
* to its raw-markdown route), so no mapping table is needed. Returns null when
109109
* the page is not in the manifest or the site does not serve it.
110110
*/
111-
async function fetchDocsPage(path: string): Promise<string | null> {
111+
type DocsFetchResult =
112+
| { outcome: 'ok'; content: string }
113+
/** The site will not serve this path however many times we ask. */
114+
| { outcome: 'missing' }
115+
/** Transient: 5xx, 429, network error, or timeout. */
116+
| { outcome: 'unavailable' }
117+
118+
async function fetchDocsPage(path: string): Promise<DocsFetchResult> {
112119
const key = normalize(path)
113-
if (!docsKeyView.has(key)) return null
120+
if (!docsKeyView.has(key)) return { outcome: 'missing' }
114121
const url = `${DOCS_BASE_URL}/${key.slice(DOCS_PREFIX.length)}`
115122
try {
116123
const response = await fetch(url, {
@@ -119,12 +126,13 @@ async function fetchDocsPage(path: string): Promise<string | null> {
119126
})
120127
if (!response.ok) {
121128
logger.warn('Docs page fetch returned a non-OK status', { url, status: response.status })
122-
return null
129+
const permanent = response.status >= 400 && response.status < 500 && response.status !== 429
130+
return { outcome: permanent ? 'missing' : 'unavailable' }
123131
}
124-
return await response.text()
132+
return { outcome: 'ok', content: await response.text() }
125133
} catch (err) {
126134
logger.warn('Docs page fetch failed', { url, error: toError(err).message })
127-
return null
135+
return { outcome: 'unavailable' }
128136
}
129137
}
130138

@@ -144,13 +152,18 @@ export async function readDocsPage(path: string): Promise<DocsPage> {
144152
`Docs page not found: ${path}. Use glob("docs/**") to list the docs corpus.`
145153
)
146154
}
147-
const content = await fetchDocsPage(key)
148-
if (content === null) {
155+
const result = await fetchDocsPage(key)
156+
if (result.outcome === 'missing') {
157+
throw new DocsCorpusError(
158+
`${key} is in the docs index but ${DOCS_BASE_URL} does not serve it — the page was likely moved or removed. Use glob("docs/**") to find the current path; retrying will not help.`
159+
)
160+
}
161+
if (result.outcome === 'unavailable') {
149162
throw new DocsCorpusError(
150163
`Could not load ${key} from ${DOCS_BASE_URL} — the docs site is temporarily unavailable. Retry shortly.`
151164
)
152165
}
153-
return { content, totalLines: content.split('\n').length }
166+
return { content: result.content, totalLines: result.content.split('\n').length }
154167
}
155168

156169
/**
@@ -170,5 +183,5 @@ export async function grepDocsPage(
170183
)
171184
}
172185
const page = await readDocsPage(key)
173-
return grepFiles(new Map([[key, page.content]]), pattern, undefined, options)
186+
return grepReadResult(key, page, pattern, key, options)
174187
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { docsSourceCandidates, foldDocsIndexPath } from '@/lib/copilot/docs/docs-path'
6+
import { DOCS_MANIFEST } from '@/lib/copilot/generated/docs-manifest'
7+
8+
describe('foldDocsIndexPath', () => {
9+
it('folds a section overview onto the section path', () => {
10+
expect(foldDocsIndexPath('workflows/index.mdx')).toBe('workflows.mdx')
11+
expect(foldDocsIndexPath('platform/enterprise/index.mdx')).toBe('platform/enterprise.mdx')
12+
})
13+
14+
it('leaves a plain page untouched', () => {
15+
expect(foldDocsIndexPath('workflows/blocks/agent.mdx')).toBe('workflows/blocks/agent.mdx')
16+
expect(foldDocsIndexPath('agents.mdx')).toBe('agents.mdx')
17+
})
18+
19+
it('does not fold a page merely named index', () => {
20+
expect(foldDocsIndexPath('index.mdx')).toBe('index.mdx')
21+
})
22+
})
23+
24+
describe('docsSourceCandidates', () => {
25+
it('is the inverse of the fold — one candidate always reproduces the input', () => {
26+
for (const publicPath of DOCS_MANIFEST) {
27+
const candidates = docsSourceCandidates(publicPath)
28+
expect(candidates.map(foldDocsIndexPath)).toContain(publicPath)
29+
}
30+
})
31+
32+
it('offers both on-disk layouts for a section path', () => {
33+
expect(docsSourceCandidates('workflows.mdx')).toEqual(['workflows.mdx', 'workflows/index.mdx'])
34+
})
35+
})
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* The single definition of how a docs source file maps onto its public path.
3+
*
4+
* Fumadocs folds a section's `index.mdx` into the section URL itself, so
5+
* `workflows/index.mdx` on disk is `/workflows` on the site (and
6+
* `/workflows/index.mdx` is a 404). Three places need that rule — the manifest
7+
* generator, the `source_document` -> VFS reverse mapping, and the vector
8+
* search's scope filter — and hand-syncing it has bitten this repo before, so
9+
* it lives here.
10+
*
11+
* Deliberately dependency-free: `scripts/sync-docs-manifest.ts` imports this by
12+
* relative path, and it must not pull in the manifest it generates.
13+
*/
14+
15+
/** Suffix that marks a section overview page on disk. */
16+
export const DOCS_INDEX_SUFFIX = '/index.mdx'
17+
18+
/**
19+
* Fold an `en`-relative mdx file path onto its public path — the value used as
20+
* both the `docs/`-relative VFS path and the docs.sim.ai URL path.
21+
*/
22+
export function foldDocsIndexPath(mdxPath: string): string {
23+
return mdxPath.endsWith(DOCS_INDEX_SUFFIX)
24+
? `${mdxPath.slice(0, -DOCS_INDEX_SUFFIX.length)}.mdx`
25+
: mdxPath
26+
}
27+
28+
/**
29+
* The inverse of {@link foldDocsIndexPath}: the on-disk file names a public
30+
* path could have come from. A page is stored either as `<stem>.mdx` or, when
31+
* it is a section overview, as `<stem>/index.mdx`.
32+
*/
33+
export function docsSourceCandidates(publicPath: string): [string, string] {
34+
const stem = publicPath.replace(/\.mdx$/, '')
35+
return [`${stem}.mdx`, `${stem}${DOCS_INDEX_SUFFIX}`]
36+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { docsEmbeddings } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { and, eq, like, notLike, or, sql } from 'drizzle-orm'
55
import { docsPathForSourceDocument, isDocsDir, isDocsPage } from '@/lib/copilot/docs/docs-corpus'
6+
import { docsSourceCandidates } from '@/lib/copilot/docs/docs-path'
67
import { generateSearchEmbedding } from '@/lib/knowledge/embeddings'
78

89
const logger = createLogger('DocsSearch')
@@ -65,10 +66,10 @@ function scopeCondition(path?: string) {
6566

6667
if (isDocsPage(normalized)) {
6768
// One page: on disk it is either `<tail>.mdx` or `<tail>/index.mdx`.
68-
const stem = tail.replace(/\.mdx$/, '')
69+
const [pageFile, indexFile] = docsSourceCandidates(tail)
6970
return or(
70-
eq(docsEmbeddings.sourceDocument, `${stem}.mdx`),
71-
eq(docsEmbeddings.sourceDocument, `${stem}/index.mdx`)
71+
eq(docsEmbeddings.sourceDocument, pageFile),
72+
eq(docsEmbeddings.sourceDocument, indexFile)
7273
)
7374
}
7475

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,33 @@ function hasModelAttachment(result: unknown): boolean {
8787
)
8888
}
8989

90+
/**
91+
* Trim an oversized docs page to the largest whole-line prefix that fits the
92+
* inline budget, preserving the true `totalLines` so the model can page through
93+
* the rest with offset/limit.
94+
*/
95+
function truncateDocsPageToInlineCap(page: { content: string; totalLines: number }): {
96+
output: { content: string; totalLines: number }
97+
returnedLines: number
98+
} {
99+
const lines = page.content.split('\n')
100+
const notice = (shown: number) =>
101+
`\n\n[Page truncated: showing lines 1-${shown} of ${page.totalLines}. Grep this path for the section you need, then read with offset/limit.]`
102+
103+
let kept = lines.length
104+
let content = page.content
105+
while (kept > 0) {
106+
content = `${lines.slice(0, kept).join('\n')}${notice(kept)}`
107+
if (
108+
serializedResultSize({ content, totalLines: page.totalLines }) <= TOOL_RESULT_MAX_INLINE_CHARS
109+
) {
110+
break
111+
}
112+
kept = Math.floor(kept / 2)
113+
}
114+
return { output: { content, totalLines: page.totalLines }, returnedLines: kept }
115+
}
116+
90117
export async function executeVfsGrep(
91118
params: Record<string, unknown>,
92119
context: ExecutionContext
@@ -274,10 +301,24 @@ export async function executeVfsRead(
274301
const page = await readDocsPage(path)
275302
const windowed = applyWindow(page)
276303
if (serializedResultSize(windowed) > TOOL_RESULT_MAX_INLINE_CHARS) {
277-
return {
278-
success: false,
279-
error: `${path} is too large to return inline. Grep that one page for the relevant section, then retry read with offset/limit.`,
304+
// Several real docs pages (the largest integration references) exceed the
305+
// inline cap, so failing here would make a plain read of them always fail
306+
// and cost a second fetch to recover. Truncate to what fits instead and
307+
// tell the model how to page — but only when it did not ask for a window,
308+
// since an explicit offset/limit that still overflows is a caller error.
309+
if (offset !== undefined || limit !== undefined) {
310+
return {
311+
success: false,
312+
error: `${path} is still too large over the requested window. Narrow offset/limit, or grep this page for the section you need.`,
313+
}
280314
}
315+
const truncated = truncateDocsPageToInlineCap(page)
316+
logger.debug('vfs_read truncated oversized docs page', {
317+
path,
318+
totalLines: page.totalLines,
319+
returnedLines: truncated.returnedLines,
320+
})
321+
return { success: true, output: truncated.output }
281322
}
282323
logger.debug('vfs_read resolved docs page', { path, totalLines: page.totalLines })
283324
return { success: true, output: windowed }

apps/sim/lib/copilot/tools/server/router.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ const baseServerToolRegistry: Record<string, BaseServerTool> = {
165165
[queryLogsServerTool.name]: queryLogsServerTool,
166166
[getJobLogsServerTool.name]: getJobLogsServerTool,
167167
[searchDocsServerTool.name]: searchDocsServerTool,
168+
// Transitional alias: sim and mothership deploy independently, so during the
169+
// rollout of the search_documentation -> search_docs rename one side is still
170+
// emitting the old id. The old params are a subset of the new, so routing them
171+
// here is safe. Remove once both repos have shipped the rename.
172+
search_documentation: searchDocsServerTool,
168173
[searchOnlineServerTool.name]: searchOnlineServerTool,
169174
[setEnvironmentVariablesServerTool.name]: setEnvironmentVariablesServerTool,
170175
[getCredentialsServerTool.name]: getCredentialsServerTool,

scripts/sync-docs-manifest.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import { readdir, readFile, writeFile } from 'node:fs/promises'
2727
import { dirname, resolve } from 'node:path'
2828
import { fileURLToPath } from 'node:url'
29+
import { foldDocsIndexPath } from '../apps/sim/lib/copilot/docs/docs-path'
2930
import { formatGeneratedSource } from './format-generated-source'
3031

3132
const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url))
@@ -55,7 +56,7 @@ async function collectMdxPaths(dir: string, prefix = ''): Promise<string[]> {
5556
/** Map an `en`-relative mdx file path to its docs.sim.ai URL path, or null to drop it. */
5657
function toDocsPath(mdxPath: string): string | null {
5758
if (mdxPath === 'index.mdx') return null
58-
return mdxPath.replace(/\/index\.mdx$/, '.mdx')
59+
return foldDocsIndexPath(mdxPath)
5960
}
6061

6162
function render(paths: string[]): string {

0 commit comments

Comments
 (0)