diff --git a/package.json b/package.json index 4366e644..c1e41b2e 100644 --- a/package.json +++ b/package.json @@ -43,9 +43,9 @@ "bench": "tsx bin/benchmark.ts", "scenario": "tsx bin/scenario.ts", "eval:structural-extract": "tsx bin/structuralExtractEval.ts", - "pretest:jest": "npm run build:info", - "test:jest": "jest", - "test:jest:watch": "jest --watch", + "pretest:jest": "npm run build:info && node bin/copyTreeSitterWasm.mjs", + "test:jest": "cross-env NODE_OPTIONS=--experimental-vm-modules jest", + "test:jest:watch": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --watch", "release": "release-it", "release:dry-run": "release-it --ci --dry-run --no-git --no-github.release --no-github.publish --no-npm.publish", "wiki:clone": "tsx bin/wiki.ts clone", @@ -73,6 +73,7 @@ "@types/yargs": "^17.0.33", "@typescript-eslint/eslint-plugin": "^7.13.1", "@typescript-eslint/parser": "^7.13.1", + "cross-env": "^10.1.0", "eslint": "^8.54.0", "ink-testing-library": "4.0.0", "jest": "^30.0.5", diff --git a/src/lib/parsers/default/__evals__/fixtures.ts b/src/lib/parsers/default/__evals__/fixtures.ts index a3687fc6..b9de4079 100644 --- a/src/lib/parsers/default/__evals__/fixtures.ts +++ b/src/lib/parsers/default/__evals__/fixtures.ts @@ -75,6 +75,33 @@ export const evalFixtures: readonly EvalFixture[] = [ ].join('\n')), ], }, + { + name: 'ts-arrow-fn-export', + // Marquee tree-sitter advantage case (#933 phase 1.1 + 2). + // + // The regex parser sees `export const handler = (...)` and + // classifies it as a `const` declaration, surfacing the symbol + // as `const handler` in the summary. Tree-sitter inspects the + // declarator's value, recognizes the `arrow_function` node, + // and classifies the binding as a function — so the same diff + // surfaces as `handler()` instead. + // + // Both parsers fire the fast path (so the LLM-call-saved metric + // is identical in mock-mode), but the qualitative output + // differs. Run the eval with the tree-sitter wasms present vs. + // absent (e.g. delete `dist/tree-sitter/` and re-run) to see the + // diff in the per-fixture JSON. + description: 'TS module replaces a function declaration with an arrow-function const — regex outputs `const handler`, tree-sitter outputs `handler()`.', + diffs: [ + buildDiff('src/api.ts', [ + '@@ -1,5 +1,5 @@', + ' import { Request, Response } from "./types"', + ' ', + '-export function handler(req: Request): Response { return process(req) }', + '+export const handler = (req: Request): Response => process(req)', + ].join('\n')), + ], + }, { name: 'python-class-method-add', description: 'Python module replaces a legacy helper with a class + factory.', diff --git a/src/lib/parsers/default/__tree_sitter__/runtime.ts b/src/lib/parsers/default/__tree_sitter__/runtime.ts index 1c427cae..472af6cf 100644 --- a/src/lib/parsers/default/__tree_sitter__/runtime.ts +++ b/src/lib/parsers/default/__tree_sitter__/runtime.ts @@ -123,7 +123,17 @@ export type TreeSitterRuntime = { } async function ensureRuntime(): Promise { - if (initPromise) return initPromise + // If an init is already in flight OR completed successfully, reuse it. + // We deliberately do NOT cache `undefined` results: a transient failure + // (e.g. jest tearing down its environment mid-dynamic-import in a + // sibling test file) shouldn't poison the cache for the whole process. + // Each call that finds no successful cached runtime tries again, + // paying the init cost at most one extra time per failure. + if (initPromise) { + const cached = await initPromise + if (cached) return cached + // Fall through and retry — the previous attempt surrendered. + } initPromise = (async () => { const locations = resolveWasmLocations() if (!locations) return undefined diff --git a/src/lib/parsers/default/__tree_sitter__/tsTreeSitterParser.test.ts b/src/lib/parsers/default/__tree_sitter__/tsTreeSitterParser.test.ts index 9e08c123..2a44015c 100644 --- a/src/lib/parsers/default/__tree_sitter__/tsTreeSitterParser.test.ts +++ b/src/lib/parsers/default/__tree_sitter__/tsTreeSitterParser.test.ts @@ -2,45 +2,40 @@ import { existsSync } from 'node:fs' import { join } from 'node:path' import type { FileDiff } from '../../../types' import { treeSitterTsParser } from './tsTreeSitterParser' -import { _resetTreeSitterRuntimeForTesting } from './runtime' function fileDiff(file: string, diff: string): FileDiff { return { file, diff, summary: '', tokenCount: Math.ceil(diff.length / 4) } } -// Tree-sitter integration tests require TWO things to actually exercise -// the .wasm code path: -// 1. The .wasm files copied into `dist/tree-sitter/` by the postbuild -// script (only present after `npm run build`). -// 2. Jest running with `NODE_OPTIONS=--experimental-vm-modules` so the -// `web-tree-sitter` package (`"type": "module"`) can be loaded via -// the dynamic-import shim in `runtime.ts`. Without the flag, Jest -// throws "A dynamic import callback was invoked without -// --experimental-vm-modules" and the runtime surrenders. +// Tree-sitter integration tests need the .wasm files copied to +// `dist/tree-sitter/` to actually exercise the code path. The +// `pretest:jest` npm script handles that automatically by running the +// postbuild copy step. ESM dynamic import (required to load +// `web-tree-sitter`, which is `"type": "module"`) works out of the box +// because `test:jest` runs with `NODE_OPTIONS=--experimental-vm-modules` +// (#933 phase 2). // -// When either is missing, the suite skips itself — the production code -// path (regex fallback) is still tested via the registry-level tests in -// `structuralParserRegistry.test.ts`, and end-to-end tree-sitter -// validation runs via the eval harness CLI (#934), which executes under -// vanilla Node with full ESM support and is the right tool for -// integration verification anyway. -// -// Opt in locally with: -// NODE_OPTIONS=--experimental-vm-modules COCO_TEST_TREE_SITTER=1 npx jest ... +// The wasm-available check stays as a defensive guard: if someone +// invokes `npx jest` directly without running `pretest:jest` first, the +// wasms won't be there. Skipping in that case keeps the runner honest +// about what was actually exercised, and the production code path +// (regex fallback) is still tested via the registry-level tests in +// `structuralParserRegistry.test.ts`. const wasmDir = join(__dirname, '..', '..', '..', '..', '..', 'dist', 'tree-sitter') const wasmAvailable = existsSync(join(wasmDir, 'web-tree-sitter.wasm')) && existsSync(join(wasmDir, 'tree-sitter-typescript.wasm')) && existsSync(join(wasmDir, 'tree-sitter-tsx.wasm')) -const esmEnabled = process.env.COCO_TEST_TREE_SITTER === '1' -const describeWithWasm = (wasmAvailable && esmEnabled) ? describe : describe.skip +const describeWithWasm = wasmAvailable ? describe : describe.skip describeWithWasm('treeSitterTsParser (.wasm-backed)', () => { - beforeEach(() => { - // Each test gets a fresh runtime so we exercise the init path - // at least once and don't rely on inter-test cache state. - _resetTreeSitterRuntimeForTesting() - }) + // Note: deliberately NO `beforeEach` reset of the runtime. The runtime + // is designed as process-lifetime (one init + one parser cache for the + // whole run), and resetting between tests has caused flakes under + // jest + ESM dynamic imports — a previous test file's teardown could + // race the init promise. The init failure path is now retry-friendly + // (see `ensureRuntime` in runtime.ts), and we trust the first test in + // this suite to exercise the init path cleanly. it('returns undefined for non-TS / non-JS file paths', async () => { expect(await treeSitterTsParser.summarize(fileDiff('README.md', '+x'))).toBeUndefined() diff --git a/yarn.lock b/yarn.lock index 7afe9df8..e7f4673e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -486,6 +486,11 @@ dependencies: tslib "^2.4.0" +"@epic-web/invariant@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@epic-web/invariant/-/invariant-1.0.0.tgz#1073e5dee6dd540410784990eb73e4acd25c9813" + integrity sha512-lrTPqgvfFQtR/eY/qkIzp98OGdNJu0m5ji3q/nJI8v3SXkRKEnWiOxMmbvcSoAIzv/cGiuvRy57k4suKQSAdwA== + "@esbuild/aix-ppc64@0.27.3": version "0.27.3" resolved "https://registry.yarnpkg.com/@esbuild/aix-ppc64/-/aix-ppc64-0.27.3.tgz#815b39267f9bffd3407ea6c376ac32946e24f8d2" @@ -2455,6 +2460,14 @@ create-require@^1.1.0: resolved "https://registry.npmjs.org/create-require/-/create-require-1.1.1.tgz" integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ== +cross-env@^10.1.0: + version "10.1.0" + resolved "https://registry.yarnpkg.com/cross-env/-/cross-env-10.1.0.tgz#cfd2a6200df9ed75bfb9cb3d7ce609c13ea21783" + integrity sha512-GsYosgnACZTADcmEyJctkJIoqAhHjttw7RsFrVoJNXbsWWqaq6Ym+7kZjq6mS45O0jij6vtiReppKQEtqWy6Dw== + dependencies: + "@epic-web/invariant" "^1.0.0" + cross-spawn "^7.0.6" + cross-spawn@7.0.6, cross-spawn@^7.0.2, cross-spawn@^7.0.3, cross-spawn@^7.0.6: version "7.0.6" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.6.tgz#8a58fe78f00dcd70c370451759dfbfaf03e8ee9f"