From fc1184c524e213643f0ee24718646c90e26eb5bf Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Thu, 2 Jul 2026 11:18:02 -0700 Subject: [PATCH 1/4] feat: add concurrency handling for document fetching and redirect collection --- src/utils/docs.functions.ts | 132 +++++++++++++++++------- tests/docs-manifest-concurrency.test.ts | 129 +++++++++++++++++++++++ 2 files changed, 225 insertions(+), 36 deletions(-) create mode 100644 tests/docs-manifest-concurrency.test.ts diff --git a/src/utils/docs.functions.ts b/src/utils/docs.functions.ts index 0e399cd90..74a265a40 100644 --- a/src/utils/docs.functions.ts +++ b/src/utils/docs.functions.ts @@ -17,7 +17,7 @@ import { isValidRepoPath, MAX_REPO_PATH_LENGTH } from './repo-path' import { removeLeadingSlash } from './utils' import type { DocsRedirectManifest } from './docs-redirects' -type DocsTreeNode = { +export type DocsTreeNode = { path: string children?: Array } @@ -92,6 +92,33 @@ const docsRedirectInput = v.object({ docsPaths: v.array(v.pipe(v.string(), v.maxLength(512))), }) +// Matches RAW_FETCH_CONCURRENCY in github-example.server.ts. +const DOCS_MANIFEST_FETCH_CONCURRENCY = 6 + +export async function mapWithConcurrency( + values: Array, + concurrency: number, + fn: (value: T) => Promise, +) { + const results = new Array(values.length) + let index = 0 + + const workers = Array.from( + { length: Math.min(concurrency, values.length) }, + async () => { + while (index < values.length) { + const currentIndex = index + index += 1 + results[currentIndex] = await fn(values[currentIndex]) + } + }, + ) + + await Promise.all(workers) + + return results +} + const temporarilyUnavailableMarkdown = `# Content temporarily unavailable We are having trouble fetching this document from GitHub right now. Please try again in a minute.` @@ -146,6 +173,62 @@ function isDocsManifest(value: unknown): value is DocsManifest { ) } +// Extracted so tests can inject a fake fetchFile without hitting real +// GitHub network/cache. +export async function collectRedirectEntriesForFile( + node: DocsTreeNode, + opts: { + docsRoot: string + fetchFile: (filePath: string) => Promise + onCanonicalPath: (canonicalPath: string) => void + }, +): Promise> { + const canonicalPath = getCanonicalDocsPath(node.path, opts.docsRoot) + + if (canonicalPath === null) { + return [] + } + + opts.onCanonicalPath(canonicalPath) + + let file: string | null + try { + file = await opts.fetchFile(node.path) + } catch (error) { + if (!isRecoverableGitHubContentError(error)) { + throw error + } + + return [] + } + + if (!file) { + return [] + } + + const frontMatter = extractFrontMatter(file) + const entries: Array = [] + + for (const redirectFrom of frontMatter.data.redirectFrom ?? []) { + const normalizedRedirect = normalizeDocsRedirectPath( + redirectFrom, + opts.docsRoot, + ) + + if (!normalizedRedirect || normalizedRedirect === canonicalPath) { + continue + } + + entries.push({ + from: normalizedRedirect, + to: canonicalPath, + source: node.path, + }) + } + + return entries +} + async function buildDocsManifest({ repo, branch, @@ -165,46 +248,23 @@ async function buildDocsManifest({ node.path.endsWith('.md'), ) const paths = new Set() - const redirects: Array = [] - - for (const node of markdownFiles) { - const canonicalPath = getCanonicalDocsPath(node.path, docsRoot) - - if (canonicalPath === null) { - continue - } - - paths.add(canonicalPath) - - const file = await fetchRepoFile(repo, branch, node.path) - - if (!file) { - continue - } - - const frontMatter = extractFrontMatter(file) - for (const redirectFrom of frontMatter.data.redirectFrom ?? []) { - const normalizedRedirect = normalizeDocsRedirectPath( - redirectFrom, + // A recoverable error on one file must not fail the whole manifest build + // (see collectRedirectEntriesForFile). + const redirectsByFile = await mapWithConcurrency( + markdownFiles, + DOCS_MANIFEST_FETCH_CONCURRENCY, + (node) => + collectRedirectEntriesForFile(node, { docsRoot, - ) - - if (!normalizedRedirect || normalizedRedirect === canonicalPath) { - continue - } - - redirects.push({ - from: normalizedRedirect, - to: canonicalPath, - source: node.path, - }) - } - } + fetchFile: (filePath) => fetchRepoFile(repo, branch, filePath), + onCanonicalPath: (canonicalPath) => paths.add(canonicalPath), + }), + ) return { paths: Array.from(paths), - redirects: buildRedirectManifest(redirects, { + redirects: buildRedirectManifest(redirectsByFile.flat(), { label: `docs redirects for ${repo}@${branch}:${docsRoot}`, }), } diff --git a/tests/docs-manifest-concurrency.test.ts b/tests/docs-manifest-concurrency.test.ts new file mode 100644 index 000000000..b0e8d77de --- /dev/null +++ b/tests/docs-manifest-concurrency.test.ts @@ -0,0 +1,129 @@ +import assert from 'node:assert/strict' +import test from 'node:test' +import { + collectRedirectEntriesForFile, + mapWithConcurrency, + type DocsTreeNode, +} from '../src/utils/docs.functions' +import { GitHubContentError } from '../src/utils/documents.server' + +test('mapWithConcurrency preserves result order regardless of completion order', async () => { + const delays = [30, 5, 20, 1, 15] + + const results = await mapWithConcurrency(delays, 3, async (delay) => { + await new Promise((resolve) => setTimeout(resolve, delay)) + return delay + }) + + assert.deepEqual(results, delays) +}) + +test('mapWithConcurrency bounds concurrency instead of running everything in parallel', async () => { + const items = Array.from({ length: 12 }, (_, index) => index) + let inFlight = 0 + let maxInFlight = 0 + + await mapWithConcurrency(items, 4, async () => { + inFlight += 1 + maxInFlight = Math.max(maxInFlight, inFlight) + await new Promise((resolve) => setTimeout(resolve, 10)) + inFlight -= 1 + }) + + assert.ok( + maxInFlight <= 4, + `expected at most 4 concurrent calls, saw ${maxInFlight}`, + ) + assert.equal(maxInFlight, 4, 'expected concurrency to reach the cap') +}) + +test('mapWithConcurrency runs in a bounded number of batches, not one call per item', async () => { + const items = Array.from({ length: 18 }, (_, index) => index) + const perItemDelayMs = 20 + const concurrency = 6 + + const start = Date.now() + await mapWithConcurrency(items, concurrency, async () => { + await new Promise((resolve) => setTimeout(resolve, perItemDelayMs)) + }) + const elapsed = Date.now() - start + + const sequentialWorstCase = items.length * perItemDelayMs + assert.ok( + elapsed < sequentialWorstCase / 2, + `expected concurrent execution well under ${sequentialWorstCase}ms, took ${elapsed}ms`, + ) +}) + +console.log('docs manifest concurrency tests passed') + +test('collectRedirectEntriesForFile returns no entries (not a rejection) when the file fetch throws a recoverable GitHub error', async () => { + const node: DocsTreeNode = { path: 'docs/guide/old-page.md' } + const paths: Array = [] + + const entries = await collectRedirectEntriesForFile(node, { + docsRoot: 'docs', + fetchFile: async () => { + throw new GitHubContentError('rate-limit', 'GitHub rate limited this file') + }, + onCanonicalPath: (path) => paths.push(path), + }) + + assert.deepEqual(entries, []) + assert.deepEqual(paths, ['guide/old-page']) +}) + +test('collectRedirectEntriesForFile rethrows non-recoverable errors instead of silently swallowing them', async () => { + const node: DocsTreeNode = { path: 'docs/guide/old-page.md' } + + await assert.rejects( + () => + collectRedirectEntriesForFile(node, { + docsRoot: 'docs', + fetchFile: async () => { + throw new TypeError('this is a real bug, not a flaky GitHub call') + }, + onCanonicalPath: () => {}, + }), + TypeError, + ) +}) + +test('a mix of one failing file and several succeeding files still produces every succeeding redirect', async () => { + const nodes: Array = [ + { path: 'docs/guide/a.md' }, + { path: 'docs/guide/flaky.md' }, + { path: 'docs/guide/b.md' }, + ] + + const fileContents: Record = { + 'docs/guide/a.md': '---\nredirect_from:\n - guide/old-a\n---\n# A', + 'docs/guide/b.md': '---\nredirect_from:\n - guide/old-b\n---\n# B', + } + + const paths: Array = [] + + const results = await mapWithConcurrency(nodes, 3, (node) => + collectRedirectEntriesForFile(node, { + docsRoot: 'docs', + fetchFile: async (filePath) => { + if (filePath === 'docs/guide/flaky.md') { + throw new GitHubContentError('server', 'GitHub 5xx for this file') + } + + return fileContents[filePath] ?? null + }, + onCanonicalPath: (path) => paths.push(path), + }), + ) + + // The whole build did not reject just because one file was flaky. + assert.deepEqual( + results.flat().map((entry) => entry.from), + ['guide/old-a', 'guide/old-b'], + ) + // All three files' paths were still recorded, including the flaky one. + assert.deepEqual(paths.sort(), ['guide/a', 'guide/b', 'guide/flaky']) +}) + +console.log('buildDocsManifest per-file fault tolerance tests passed') From a49742012d90a9bae43a4a8017bd04315bc9a9db Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Thu, 2 Jul 2026 11:19:09 -0700 Subject: [PATCH 2/4] format --- tests/docs-manifest-concurrency.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/docs-manifest-concurrency.test.ts b/tests/docs-manifest-concurrency.test.ts index b0e8d77de..40e7e83e1 100644 --- a/tests/docs-manifest-concurrency.test.ts +++ b/tests/docs-manifest-concurrency.test.ts @@ -64,7 +64,10 @@ test('collectRedirectEntriesForFile returns no entries (not a rejection) when th const entries = await collectRedirectEntriesForFile(node, { docsRoot: 'docs', fetchFile: async () => { - throw new GitHubContentError('rate-limit', 'GitHub rate limited this file') + throw new GitHubContentError( + 'rate-limit', + 'GitHub rate limited this file', + ) }, onCanonicalPath: (path) => paths.push(path), }) From 6c0012ea1ef4bd5ce7ecb3778bb39eac8604e894 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:29:34 +0000 Subject: [PATCH 3/4] fix: keep docs server imports out of client bundle path --- src/utils/docs.functions.ts | 40 ++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/utils/docs.functions.ts b/src/utils/docs.functions.ts index 74a265a40..7437c6c4c 100644 --- a/src/utils/docs.functions.ts +++ b/src/utils/docs.functions.ts @@ -3,19 +3,12 @@ import { createServerFn } from '@tanstack/react-start' import { setResponseHeader } from '@tanstack/react-start/server' import removeMarkdown from 'remove-markdown' import * as v from 'valibot' -import { - extractFrontMatter, - fetchApiContents, - fetchRepoFile, - isRecoverableGitHubContentError, - shouldUseLocalDocsFiles, -} from '~/utils/documents.server' import { extractFrameworksFromMarkdown } from './markdown/filterFrameworkContent' -import { getCachedDocsArtifact } from './github-content-cache.server' import { buildRedirectManifest, type RedirectManifestEntry } from './redirects' import { isValidRepoPath, MAX_REPO_PATH_LENGTH } from './repo-path' import { removeLeadingSlash } from './utils' import type { DocsRedirectManifest } from './docs-redirects' +import type { GitHubFileNode } from './documents.server' export type DocsTreeNode = { path: string @@ -123,6 +116,14 @@ const temporarilyUnavailableMarkdown = `# Content temporarily unavailable We are having trouble fetching this document from GitHub right now. Please try again in a minute.` +async function loadDocumentsServerModule() { + return import('./documents.server') +} + +async function loadGitHubContentCacheServerModule() { + return import('./github-content-cache.server') +} + function buildUnavailableFile(filePath: string) { if (filePath.toLowerCase().endsWith('.md')) { return temporarilyUnavailableMarkdown @@ -136,6 +137,9 @@ async function readRepoFileOrFallback( branch: string, filePath: string, ) { + const { fetchRepoFile, isRecoverableGitHubContentError } = + await loadDocumentsServerModule() + try { return await fetchRepoFile(repo, branch, filePath) } catch (error) { @@ -183,6 +187,8 @@ export async function collectRedirectEntriesForFile( onCanonicalPath: (canonicalPath: string) => void }, ): Promise> { + const { extractFrontMatter, isRecoverableGitHubContentError } = + await loadDocumentsServerModule() const canonicalPath = getCanonicalDocsPath(node.path, opts.docsRoot) if (canonicalPath === null) { @@ -238,6 +244,7 @@ async function buildDocsManifest({ branch: string docsRoot: string }): Promise { + const { fetchApiContents, fetchRepoFile } = await loadDocumentsServerModule() const nodes = await fetchApiContents(repo, branch, docsRoot) if (!nodes) { @@ -279,6 +286,7 @@ async function buildDocsPathManifest({ branch: string docsRoot: string }): Promise { + const { fetchApiContents } = await loadDocumentsServerModule() const nodes = await fetchApiContents(repo, branch, docsRoot) if (!nodes) { @@ -302,6 +310,11 @@ export const fetchDocsManifest = createServerFn({ method: 'GET' }) .validator(docsManifestInput) .handler(async ({ data }) => { const { repo, branch, docsRoot } = data + const [{ shouldUseLocalDocsFiles }, { getCachedDocsArtifact }] = + await Promise.all([ + loadDocumentsServerModule(), + loadGitHubContentCacheServerModule(), + ]) if (shouldUseLocalDocsFiles()) { return buildDocsManifest({ repo, branch, docsRoot }) @@ -322,6 +335,11 @@ export const fetchDocsPathManifest = createServerFn({ method: 'GET' }) .validator(docsManifestInput) .handler(async ({ data }) => { const { repo, branch, docsRoot } = data + const [{ shouldUseLocalDocsFiles }, { getCachedDocsArtifact }] = + await Promise.all([ + loadDocumentsServerModule(), + loadGitHubContentCacheServerModule(), + ]) if (shouldUseLocalDocsFiles()) { return buildDocsPathManifest({ repo, branch, docsRoot }) @@ -341,6 +359,7 @@ export const fetchDocsPathManifest = createServerFn({ method: 'GET' }) export const fetchDocsRedirect = createServerFn({ method: 'GET' }) .validator(docsRedirectInput) .handler(async ({ data }) => { + const { isRecoverableGitHubContentError } = await loadDocumentsServerModule() let manifest: DocsManifest try { @@ -389,6 +408,7 @@ export const fetchDocs = createServerFn({ method: 'GET' }) throw notFound() } + const { extractFrontMatter } = await loadDocumentsServerModule() const frontMatter = extractFrontMatter(file) const description = frontMatter.userDescription ?? removeMarkdown(frontMatter.excerpt ?? '') @@ -446,7 +466,9 @@ export const fetchRepoDirectoryContents = createServerFn({ .validator(repoDirectoryInput) .handler(async ({ data }: { data: RepoDirectoryRequest }) => { const { repo, branch, startingPath } = data - let githubContents: Awaited> + const { fetchApiContents, isRecoverableGitHubContentError } = + await loadDocumentsServerModule() + let githubContents: Array | null try { githubContents = await fetchApiContents(repo, branch, startingPath) From d4dec2ef5e8b124a8926fe0f71ff86d909c4e81a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:31:54 +0000 Subject: [PATCH 4/4] chore: finalize CI fix validation --- src/utils/docs.functions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/docs.functions.ts b/src/utils/docs.functions.ts index 7437c6c4c..fb17fb3e1 100644 --- a/src/utils/docs.functions.ts +++ b/src/utils/docs.functions.ts @@ -359,7 +359,8 @@ export const fetchDocsPathManifest = createServerFn({ method: 'GET' }) export const fetchDocsRedirect = createServerFn({ method: 'GET' }) .validator(docsRedirectInput) .handler(async ({ data }) => { - const { isRecoverableGitHubContentError } = await loadDocumentsServerModule() + const { isRecoverableGitHubContentError } = + await loadDocumentsServerModule() let manifest: DocsManifest try {