Skip to content

chore(parser): tree-sitter integration polish (#933 phase 2)#956

Merged
gfargo merged 1 commit into
mainfrom
feat/933-phase2-polish
May 14, 2026
Merged

chore(parser): tree-sitter integration polish (#933 phase 2)#956
gfargo merged 1 commit into
mainfrom
feat/933-phase2-polish

Conversation

@gfargo
Copy link
Copy Markdown
Owner

@gfargo gfargo commented May 14, 2026

Summary

Closes the loose ends from #955 (phase 1.1) before phase 3 (lazy-load infrastructure).

Discussed scope: skip the redundant JS-grammar bundle (TS grammar is a superset and already handles .js/.mjs/.cjs), use this PR as the polish lap.

Changes

1. Integration tests run by default

  • `test:jest` script now sets `NODE_OPTIONS=--experimental-vm-modules` (via the new `cross-env` devDep), letting jest load `web-tree-sitter` (`"type": "module"`) via dynamic import.
  • `pretest:jest` now also runs `bin/copyTreeSitterWasm.mjs` so a clean checkout's first `npm run test:jest` runs the integration suite instead of skipping it.
  • Removed the `COCO_TEST_TREE_SITTER=1` env-var gate — only the wasm-available check remains as a defensive fence.

2. New `ts-arrow-fn-export` eval fixture

Targets the regex extractor's known weak spot:
```ts
-export function handler(req: Request): Response { return process(req) }
+export const handler = (req: Request): Response => process(req)
```

Regex classifies as `const handler`; tree-sitter sees the `arrow_function` in the declarator value and classifies as `handler()`. Both fire the fast path, but the per-fixture JSON shows the qualitative difference. Diff: run the eval with wasms present vs. removed (`rm -rf dist/tree-sitter/`).

3. Stability fix: retry-on-failure for runtime init

Found while validating: with ESM dynamic imports always on, sibling-test-file teardown races could leave `initPromise` permanently cached as `undefined`. `ensureRuntime` now retries when the cached result is undefined while still reusing successful inits process-wide. Also removed the `beforeEach _resetTreeSitterRuntimeForTesting` call — the runtime is process-lifetime by design.

Net effect: 1647/1647 tests pass on 3 consecutive runs (previously 4-6 tree-sitter integration tests would flake).

Test plan

  • `npx tsc --noEmit` → 0 errors
  • `npm run test:jest` → 1647/1647 pass, 3 consecutive clean runs
  • `npx eslint` on touched files → clean
  • `npm run eval:structural-extract -- --fixtures-only` → 8 fixtures including new arrow-fn case, 6 LLM calls saved

Out of scope (phase 3 queue)

  • Lazy-load infra for non-bundled languages (cache dir, first-use prompt, manifest, integrity check)
  • Surface qualitative regex-vs-tree-sitter delta in eval output (today: diff JSONs by hand)
  • Telemetry on tree-sitter init failures

Refs #933.

Closes out the loose ends from #955 (phase 1.1) before moving on
to the lazy-load infrastructure in phase 3.

1. **Integration tests run by default.**
   Added `cross-env NODE_OPTIONS=--experimental-vm-modules` to the
   `test:jest` script so jest can load the ESM-only
   `web-tree-sitter` package via dynamic import. Removed the
   `COCO_TEST_TREE_SITTER=1` opt-in gate from the test file — the
   .wasm-available check stays as the defensive fence.

   `pretest:jest` now also runs `node bin/copyTreeSitterWasm.mjs`
   to populate `dist/tree-sitter/` before jest starts, so a clean
   checkout's first `npm run test:jest` runs the integration suite
   instead of skipping it.

   Added `cross-env` as a devDep (small, standard solution for
   cross-platform env-var syntax — CI is Linux but contributors may
   be on Windows).

2. **Arrow-function eval fixture.**
   New `ts-arrow-fn-export` fixture in `__evals__/fixtures.ts`
   targeting the regex extractor's known weak spot:
   `export const handler = (...) => ...`. The regex parser
   classifies the symbol as `const handler`; the tree-sitter
   parser inspects the declarator value, recognizes the
   `arrow_function`, and classifies it as a function — surfacing
   the same diff as `handler()` instead.

   Both parsers fire the fast path on this fixture (so the LLM-
   call-saved metric is identical in the harness's mock-mode
   counter), but the per-fixture JSON now shows the qualitative
   difference between the two extractors. Run the eval with the
   wasms present vs. removed (`rm -rf dist/tree-sitter/`) to see
   the diff.

3. **Stability fix: retry-on-failure for the runtime init.**
   Discovered while validating Phase 2: when jest's full suite
   runs with ESM dynamic imports enabled, a transient init failure
   (e.g. a sibling test file's environment tearing down mid-
   import) was permanently caching `undefined` in the runtime's
   `initPromise`. Subsequent calls to `getTreeSitterParser` from
   any test file inherited the cached failure.

   `ensureRuntime` now awaits the cached promise and re-tries if
   the result is undefined, while still reusing a successful
   init across the whole process. Net cost: at most one extra
   init attempt per real failure. Removed the
   `beforeEach _resetTreeSitterRuntimeForTesting` call in the
   integration suite — the runtime is process-lifetime by design,
   and resetting it between tests created surface area for the
   exact teardown race the retry path now handles.

Validation:
- npx tsc --noEmit → 0 errors
- `npm run test:jest` → 1647/1647 pass (3 consecutive runs, all
  clean — previously 4-6 tree-sitter integration tests would flake)
- `npx eslint` on touched files → clean
- `npm run eval:structural-extract -- --fixtures-only` → 8
  fixtures, 6 LLM calls saved, including the new arrow-fn case.

Out of scope (queued for phase 3):
- Lazy-load infra for non-bundled languages (cache dir,
  first-use prompt, manifest, integrity check).
- Surface the qualitative regex-vs-tree-sitter delta in the eval
  output (today you have to diff JSONs by hand). Issue #934
  polish.
- Telemetry on tree-sitter init failures (today they're silently
  caught; would help diagnose user-environment issues like
  corrupted wasms).

Refs #933.
@gfargo gfargo merged commit b8b2ee9 into main May 14, 2026
5 of 7 checks passed
@gfargo gfargo deleted the feat/933-phase2-polish branch May 14, 2026 18:57
gfargo added a commit that referenced this pull request May 14, 2026
…phase 7) (#959)

Closes the tree-sitter integration feature (#933). Lazy-loaded
parsers gain a first-class CLI surface for cache management;
verbose mode surfaces a discoverability hint when the fast path
falls through to regex.

## New `coco cache` subcommands

Extends the existing `coco cache` (diff-summary info / clear)
with three tree-sitter subcommands:

- \`coco cache parsers\` — show every manifest language with its
  current cache status (cached size or "not cached" + the
  fetched-size estimate), version pin, and source URL. Footer
  summarizes total disk usage + quick-reference commands.
- \`coco cache prefetch [languages...]\` — download specific
  parsers (e.g. \`coco cache prefetch py rs go\` or
  \`coco cache prefetch all\`). When invoked with no args AND
  stdin is a TTY, opens an interactive checkbox picker. In
  non-interactive contexts (CI, pipes), no-arg invocations error
  out with usage hints instead of hanging on a prompt.
- \`coco cache clear-parsers\` — wipe \`~/.cache/coco/tree-sitter/\`.
  Idempotent; reports a per-language ✓ for each removed file.

Aliases mirror \`COCO_PREFETCH\` env grammar: \`py\` / \`python\`,
\`rs\` / \`rust\`, \`go\` / \`golang\`, \`all\`.

## Surrender telemetry

In verbose mode, when the language-aware fast path is enabled and
the parser chain falls through to LLM, emit a discoverability
hint:

  \`Tree-sitter parser surrendered for 'python'; using regex
   fallback. Hint: \`coco cache parsers\` to inspect,
   \`coco cache prefetch python\` to enable.\`

Quiet on the default path; visible only when the user is
debugging summary quality. Hint copy adapts: bundled-language
surrenders (\`ts\` / \`js\`) point at \`coco cache prefetch all\`
because TS / TSX wasms are always shipped (the surrender is from
a parser-init failure, not a missing download); lazy-loaded
languages get a per-language prefetch hint.

## Implementation

### \`cache.ts\` (lazy-load cache module)

- New \`getCachedParserStatus(language)\` returns
  \`{ language, cached, path, bytes?, mtime? }\` for the table
  renderer + interactive picker.
- New \`clearCachedParser(language)\` unlinks the cached .wasm.
  Idempotent; returns \`true\` when a file was actually removed.

### \`structuralParserRegistry.ts\`

- New \`hasTreeSitterParser(language)\` lets the LLM fallthrough
  path know whether a tree-sitter parser is registered for the
  language — used by the surrender-telemetry hint. Doesn't
  expose internals; the caller just needs the boolean.

### \`summarizeLargeFiles.ts\`

- Surrender-telemetry block fires after the registry returns
  undefined and BEFORE the cache lookup. Only emits when the
  chain includes a tree-sitter parser, so regex-only languages
  don't get a misleading hint.

### \`commands/cache/\`

- \`config.ts\` gains the \`CACHE_SUBCOMMANDS\` enum and a positional
  \`[languages..]\` for prefetch. Yargs validates the subcommand
  set; unknown tokens get caught by the language resolver.
- \`handler.ts\` adds three new branches:
  - \`parsers\` calls \`renderParsersTable\`
  - \`prefetch\` resolves tokens via \`parsePrefetchEnv\`
    (reusing the env-var grammar), prompts when interactive,
    and delegates to \`prefetchTreeSitterParsers\`. Failed
    downloads → \`process.exitCode = 1\`.
  - \`clear-parsers\` walks every manifest entry, calls
    \`clearCachedParser\`, reports per-language status.

### \`inquirerPrompts.ts\`

- New \`checkboxPrompt\` helper. Same dynamic-import shim as the
  other prompts; reuses the codebase's standard pattern for ESM
  inquirer modules under ts-jest.

## Tests

4 new test cases in \`handler.test.ts\` cover the new subcommands:
\`parsers\` lists every manifest language, \`prefetch\` warns on
unknown tokens, \`clear-parsers\` reports no-op when empty AND
removes cached files when present.

Test isolation: each test sets \`COCO_CACHE_DIR\` to the same tmp
dir the existing tests use for \`XDG_CACHE_HOME\`, so the
tree-sitter cache lives inside the per-test sandbox.

## Manual validation

\`\`\`
$ COCO_CACHE_DIR=/tmp/coco-phase7-smoke coco cache parsers
Tree-sitter parser cache

  Python   not cached          (448.0 KB when fetched)
  Rust     not cached          (1.05 MB when fetched)
  Go       not cached          (212.1 KB when fetched)

  cached: 0/3  total on disk: 0 B

$ coco cache prefetch py
· Python: downloading https://cdn.jsdelivr.net/.../tree-sitter-python.wasm…
✓ Python parser cached (447 KB)

Summary: 1 downloaded · 0 already cached · 0 failed

$ coco cache clear-parsers
✓ cleared Python

Cleared 1 parser(s) from ~/.cache/coco/tree-sitter/
\`\`\`

## Validation

- \`npx tsc --noEmit\` → 0 errors
- \`npm run test:jest\` → 1674/1674 pass (3 of 4 consecutive runs
  clean, 1 flake on the pre-existing scenarioInputs timeout
  pattern)
- \`npx eslint\` on touched files → clean
- Manual: all four subcommands round-trip cleanly

## Out of scope (genuine future work)

- **Eval-harness side-by-side regex-vs-tree-sitter comparison
  in the report output**. Today the eval reports per-fixture
  outcomes but doesn't discriminate WHICH parser produced each
  summary. Surfacing the regex vs. tree-sitter delta requires
  registry injection at eval time (the harness builds its own
  parser chain instead of using the global). Reasonable
  follow-up; not gating on #933 closure.

## #933 status: feature complete

| Phase | Status |
|---|---|
| 1.0 — Registry abstraction | ✓ #950 |
| 1.1 — TS/TSX bundled | ✓ #955 |
| 2 — Polish + ESM jest + arrow-fn fixture | ✓ #956 |
| 3 — Lazy-load infra + Python | ✓ #957 |
| 5 — Rust | ✓ #958 |
| 6 — Go | ✓ #958 |
| **7 — Cache CLI + telemetry** | **this PR** |

Closes #933.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant