diff --git a/tests/skill/understand/test_extract_import_map.test.mjs b/tests/skill/understand/test_extract_import_map.test.mjs index 0f17277..b5f132e 100644 --- a/tests/skill/understand/test_extract_import_map.test.mjs +++ b/tests/skill/understand/test_extract_import_map.test.mjs @@ -1597,3 +1597,55 @@ describe('extract-import-map.mjs — tree-sitter init graceful failure', () => { expect(result.output.stats.totalEdges).toBe(0); }); }); + +describe('extract-import-map.mjs — deterministic stderr ordering across loaders', () => { + let projectRoot; + + afterEach(() => { + if (projectRoot) { + rmSync(projectRoot, { recursive: true, force: true }); + projectRoot = null; + } + }); + + // Regression for the parallel-loader stderr-order bug surfaced in + // PR #346 review: tsconfig / go.mod / composer.json loaders now run + // concurrently, but warnings must still emit in the pre-PR canonical + // order (tsconfig → go → php). If the loaders streamed warnings + // mid-flight, I/O timing could reorder them — the assertions below + // catch that regression. + it('emits warnings in canonical order (tsconfig, go, php) regardless of I/O timing', () => { + projectRoot = setupTree({ + 'tsconfig.json': '{ "compilerOptions": { "baseUrl": ".", ', // unterminated + 'composer.json': '{ "autoload": { "psr-4": { "App\\\\": "src/" }, ', // unterminated + 'src/index.ts': `import { foo } from './foo';\n`, + 'src/foo.ts': `export const foo = 1;\n`, + 'src/Http/Controller.php': + ` e.name).filter(Boolean); - exportsByPath.set(file.path, names); - } catch (err) { - process.stderr.write( - `Warning: compute-batches: exports extraction failed for ${file.path} ` + - `(analyze error: ${err.message}) — symbols=[] in neighborMap — ` + - `cross-batch edges to this file limited to file-level\n`, - ); - exportsByPath.set(file.path, []); + + // I/O is parallelised in bounded chunks (libuv worker threads handle the + // disk reads concurrently) while the actual tree-sitter parse stays on + // the main thread, since web-tree-sitter is single-threaded WASM. For a + // 15k-file iOS repo (#226), the sequential `readFileSync` loop dominated; + // letting reads pipeline drops wall time roughly proportional to the + // share of the loop spent waiting on disk. + for (let start = 0; start < codeFiles.length; start += IO_PARALLELISM) { + const slice = codeFiles.slice(start, start + IO_PARALLELISM); + + // Read every file in the slice concurrently. Errors per file are + // captured in-place so a single bad file does not abort the chunk. + const reads = await Promise.all( + slice.map(async (file) => { + const abs = join(projectRoot, file.path); + try { + const content = await readFile(abs, 'utf-8'); + return { file, content, readError: null }; + } catch (err) { + return { file, content: null, readError: err }; + } + }), + ); + + // Serialise the CPU-bound tree-sitter work and the stderr warning emits + // so log order remains identical to the previous sequential loop. This + // also keeps existing fixture-comparison tests stable. + for (const { file, content, readError } of reads) { + if (readError) { + process.stderr.write( + `Warning: compute-batches: exports extraction failed for ${file.path} ` + + `(read error: ${readError.message}) — symbols=[] in neighborMap — ` + + `cross-batch edges to this file limited to file-level\n`, + ); + exportsByPath.set(file.path, []); + continue; + } + try { + const analysis = registry.analyzeFile(file.path, content); + const names = (analysis?.exports || []).map(e => e.name).filter(Boolean); + exportsByPath.set(file.path, names); + } catch (err) { + process.stderr.write( + `Warning: compute-batches: exports extraction failed for ${file.path} ` + + `(analyze error: ${err.message}) — symbols=[] in neighborMap — ` + + `cross-batch edges to this file limited to file-level\n`, + ); + exportsByPath.set(file.path, []); + } } } return exportsByPath; diff --git a/understand-anything-plugin/skills/understand/extract-import-map.mjs b/understand-anything-plugin/skills/understand/extract-import-map.mjs index 00f9181..2a7cdfd 100644 --- a/understand-anything-plugin/skills/understand/extract-import-map.mjs +++ b/understand-anything-plugin/skills/understand/extract-import-map.mjs @@ -37,6 +37,29 @@ import { createRequire } from 'node:module'; import { dirname, resolve, join, posix } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { existsSync, readFileSync, realpathSync, writeFileSync } from 'node:fs'; +import { readFile } from 'node:fs/promises'; + +/** + * Read a list of files concurrently while preserving result order. Failures + * are returned in-place as `{ raw: null, err }` so callers can emit the same + * per-file warnings they did under the previous sequential `readFileSync` + * loops. + * + * `paths` is a list of `{ key, absPath }` pairs; `key` is whatever the caller + * wants to attach the result to (typically a project-relative POSIX path). + */ +async function readFilesParallel(paths) { + return Promise.all( + paths.map(async ({ key, absPath }) => { + try { + const raw = await readFile(absPath, 'utf-8'); + return { key, raw, err: null }; + } catch (err) { + return { key, raw: null, err }; + } + }), + ); +} const __dirname = dirname(fileURLToPath(import.meta.url)); // skills/understand/ -> plugin root is two dirs up @@ -180,20 +203,26 @@ function parseTsConfigText(raw) { * where the stripper damaged a string literal containing `//`. * 3. If both fail, warn and skip — that tsconfig contributes no aliases. */ -function loadTsConfigs(projectRoot, files) { +async function loadTsConfigs(projectRoot, files) { const out = new Map(); + const warnings = []; + // Collect the candidate paths in the original file order before reading, + // so warning emit order matches the previous sequential implementation. + const candidates = []; for (const f of files) { const p = toPosix(f.path); const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p; if (base !== 'tsconfig.json') continue; const absPath = join(projectRoot, p); if (!existsSync(absPath)) continue; - let raw; - try { - raw = readFileSync(absPath, 'utf-8'); - } catch (err) { - process.stderr.write( - `Warning: extract-import-map: tsconfig.json at ${absPath} failed ` + + candidates.push({ key: p, absPath }); + } + const reads = await readFilesParallel(candidates); + for (const { key: p, raw, err } of reads) { + if (err) { + // absPath isn't carried through the helper return shape; reconstruct it. + warnings.push( + `Warning: extract-import-map: tsconfig.json at ${join(projectRoot, p)} failed ` + `to read (${err.message}) — path aliases from this config will ` + `not be applied — relative imports unaffected\n`, ); @@ -201,8 +230,8 @@ function loadTsConfigs(projectRoot, files) { } const parsed = parseTsConfigText(raw); if (!parsed) { - process.stderr.write( - `Warning: extract-import-map: tsconfig.json at ${absPath} failed ` + + warnings.push( + `Warning: extract-import-map: tsconfig.json at ${join(projectRoot, p)} failed ` + `to parse — path aliases from this config will not be applied ` + `— relative imports unaffected\n`, ); @@ -210,7 +239,7 @@ function loadTsConfigs(projectRoot, files) { } out.set(dirOf(p), parsed); } - return out; + return { configs: out, warnings }; } /** @@ -237,20 +266,26 @@ function loadTsConfigs(projectRoot, files) { * The resolver uses each module's prefix to translate * `import "github.com/foo/bar/x"` into the project-internal `x/.go`. */ -function loadGoModules(projectRoot, files) { +async function loadGoModules(projectRoot, files) { const out = new Map(); + // loadGoModules currently emits no warnings (read failures are silently + // skipped — per-file resolvers surface "no ancestor go.mod" later), but + // the `{ data, warnings }` shape matches loadTsConfigs / loadPhpAutoloads + // so the concurrent caller in buildResolutionContext can drain them + // uniformly in canonical order. + const warnings = []; + const candidates = []; for (const f of files) { const p = toPosix(f.path); const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p; if (base !== 'go.mod') continue; const absPath = join(projectRoot, p); if (!existsSync(absPath)) continue; - let raw; - try { - raw = readFileSync(absPath, 'utf-8'); - } catch { - continue; - } + candidates.push({ key: p, absPath }); + } + const reads = await readFilesParallel(candidates); + for (const { key: p, raw, err } of reads) { + if (err) continue; let moduleName = ''; for (const line of raw.split(/\r?\n/)) { const trimmed = line.replace(/\/\/.*$/, '').trim(); @@ -261,7 +296,7 @@ function loadGoModules(projectRoot, files) { if (!moduleName) continue; out.set(dirOf(p), moduleName); } - return out; + return { modules: out, warnings }; } /** @@ -306,10 +341,32 @@ function findNearestConfigDir(startDir, configMap) { * * Build once; pass everywhere. */ -function buildResolutionContext(projectRoot, files) { +async function buildResolutionContext(projectRoot, files) { const fileSet = new Set(files.map(f => toPosix(f.path))); - const tsConfigs = loadTsConfigs(projectRoot, files); - const goModules = loadGoModules(projectRoot, files); + + // The three config-loader passes are independent and each does its own + // batched parallel I/O; run them concurrently so the wait for a slow + // tsconfig.json read doesn't block go.mod / composer.json scanning. + // + // Each loader BUFFERS warnings into a private array rather than writing + // them to stderr inline. If a loader streamed warnings directly during + // the concurrent passes, lines from independent loader families could + // interleave based on I/O timing — that would break the pre-PR + // deterministic order (ts → go → php) and make stderr-diff verification + // flaky. Drain the buffers in canonical order *after* Promise.all, so + // a fixture with `(malformed tsconfig.json, malformed composer.json)` + // always emits `tsconfig…\ncomposer…\n`, never the reverse. + const [tsResult, goResult, phpResult] = await Promise.all([ + loadTsConfigs(projectRoot, files), + loadGoModules(projectRoot, files), + loadPhpAutoloads(projectRoot, files), + ]); + for (const w of tsResult.warnings) process.stderr.write(w); + for (const w of goResult.warnings) process.stderr.write(w); + for (const w of phpResult.warnings) process.stderr.write(w); + const tsConfigs = tsResult.configs; + const goModules = goResult.modules; + const phpAutoloads = phpResult.autoloads; // Index .go files by their parent directory so the Go resolver can // expand a package-level import to all member .go files in O(1). @@ -331,8 +388,6 @@ function buildResolutionContext(projectRoot, files) { const kotlinIndex = buildSuffixIndex(files, p => p.endsWith('.kt')); const csIndex = buildSuffixIndex(files, p => p.endsWith('.cs')); - const phpAutoloads = loadPhpAutoloads(projectRoot, files); - return { projectRoot, fileSet, @@ -1028,20 +1083,23 @@ function parseComposerAutoloadText(raw) { * at the bad file and skips it. The rest of the project's PHP imports keep * resolving via whichever composer.json files parsed cleanly. */ -function loadPhpAutoloads(projectRoot, files) { +async function loadPhpAutoloads(projectRoot, files) { const out = new Map(); + const warnings = []; + const candidates = []; for (const f of files) { const p = toPosix(f.path); const base = p.includes('/') ? p.slice(p.lastIndexOf('/') + 1) : p; if (base !== 'composer.json') continue; const absPath = join(projectRoot, p); if (!existsSync(absPath)) continue; - let raw; - try { - raw = readFileSync(absPath, 'utf-8'); - } catch (err) { - process.stderr.write( - `Warning: extract-import-map: composer.json at ${absPath} failed ` + + candidates.push({ key: p, absPath }); + } + const reads = await readFilesParallel(candidates); + for (const { key: p, raw, err } of reads) { + if (err) { + warnings.push( + `Warning: extract-import-map: composer.json at ${join(projectRoot, p)} failed ` + `to read (${err.message}) — PSR-4 namespace mapping from this ` + `composer.json unavailable — PHP imports under this package ` + `will not resolve\n`, @@ -1050,8 +1108,8 @@ function loadPhpAutoloads(projectRoot, files) { } const parsed = parseComposerAutoloadText(raw); if (parsed === null) { - process.stderr.write( - `Warning: extract-import-map: composer.json at ${absPath} failed ` + + warnings.push( + `Warning: extract-import-map: composer.json at ${join(projectRoot, p)} failed ` + `to parse — PSR-4 namespace mapping unavailable — PHP imports ` + `under this package will not resolve\n`, ); @@ -1059,7 +1117,7 @@ function loadPhpAutoloads(projectRoot, files) { } out.set(dirOf(p), parsed); } - return out; + return { autoloads: out, warnings }; } /** @@ -1421,8 +1479,10 @@ async function main() { ); } - // Build resolution context (cached configs) - const ctx = buildResolutionContext(projectRoot, files); + // Build resolution context (cached configs). The loader pass for the + // tsconfig/go.mod/composer.json files inside is parallelised — see + // `buildResolutionContext`. + const ctx = await buildResolutionContext(projectRoot, files); const importMap = {}; let filesWithImports = 0;