Skip to content

[Chore] Cache extension coverage by ownership - #1631

Merged
edelauna merged 4 commits into
mainfrom
feature/cache-coverage-lanes-2hyscfg2ezngj
Sep 14, 2026
Merged

edelauna merged 4 commits into
mainfrom
feature/cache-coverage-lanes-2hyscfg2ezngj

Conversation

@zoomote

@zoomote zoomote Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

​Created by Roomote. Follow up by mentioning @roomote, in the web UI, or in Discord.

What changed

  • Split extension unit coverage into independently cached API, core, services, misc/platform, and tree-sitter lanes.
  • Give ownership lanes complete upstream workspace-build dependencies and conservative shared-production inputs while excluding other lanes' test files.
  • Enforce exact resolved Vitest collection partitioning and the verified LCOV population in the coverage contract.
  • Run at most two coverage processes concurrently after profiling showed five-way execution causes resource-contention failures.
  • Preserve the same-commit/OS serialization that prevents duplicate Windows runs racing the Turbo cache save.

Why this change was made

The prior tree-sitter/general split left 414 general test files coupled to one cache key. Profiling found stable ownership boundaries of API, core, services, and misc tests, while production import cones overlap heavily. The implementation therefore isolates test-only invalidation but conservatively reruns all ownership lanes for shared production changes rather than risking false cache hits.

Impact

The five lane configs resolve to exactly the monolithic unit population: 478 discovered files, 8,411 tests, 469 LCOV source records, and 30,229 instrumented lines.

Measured isolated local cold times are API 16.2s, core 48.0s, services 36.6s, and misc 31.5s. The complete five-lane graph with tree-sitter completes in 1m56 cold and 310ms warm. An API-test-only change completes in 17.7s with five of six Turbo tasks restored; an API production change conservatively invalidates all four ownership lanes while retaining the tree-sitter cache.

Hosted Windows run 34793268975 completed successfully in 7m06s total. Its cold extension coverage step took 4m40s, down from 6m55s and 8m58s on the earlier two-lane cold runs (32% and 48% faster). The exhaustive coverage contract took 12s. A hosted partial-cache run was not manufactured with a no-op test edit; the deterministic local partial run is the measured evidence for that path.

Closes #115

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 1ccce659-be83-4749-a58a-942432268c6d

📥 Commits

Reviewing files that changed from the base of the PR and between ba46d1f and 110a107.

📒 Files selected for processing (9)
  • .github/workflows/code-qa.yml
  • src/package.json
  • src/scripts/verify-coverage-contract.mjs
  • src/turbo.json
  • src/vitest.api.config.ts
  • src/vitest.core.config.ts
  • src/vitest.misc.config.ts
  • src/vitest.services.config.ts
  • src/vitest.tree-sitter.config.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (4)
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

⚙️ CodeRabbit configuration file

Files:

  • src/vitest.services.config.ts
  • src/vitest.tree-sitter.config.ts
  • src/vitest.core.config.ts
  • src/vitest.misc.config.ts
  • src/vitest.api.config.ts
  • src/scripts/verify-coverage-contract.mjs
Require full commit SHA pins, least-privilege permissions, safe expression and shell interpolation, and trusted metadata handling.

⚙️ CodeRabbit configuration file

Files:

  • .github/workflows/code-qa.yml
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

⚙️ CodeRabbit configuration file

Files:

  • src/vitest.services.config.ts
  • src/package.json
  • src/vitest.tree-sitter.config.ts
  • src/turbo.json
  • src/vitest.core.config.ts
  • src/vitest.misc.config.ts
  • src/vitest.api.config.ts
  • src/scripts/verify-coverage-contract.mjs
Act as an adversarial second-opinion reviewer.

⚙️ CodeRabbit configuration file

Files:

  • src/vitest.services.config.ts
  • src/package.json
  • src/vitest.tree-sitter.config.ts
  • src/turbo.json
  • src/vitest.core.config.ts
  • src/vitest.misc.config.ts
  • src/vitest.api.config.ts
  • src/scripts/verify-coverage-contract.mjs
🔇 Additional comments (9)
src/vitest.api.config.ts (1)

1-15: LGTM!

src/vitest.core.config.ts (1)

1-15: LGTM!

src/vitest.services.config.ts (1)

1-16: LGTM!

src/vitest.misc.config.ts (1)

1-26: LGTM!

src/vitest.tree-sitter.config.ts (1)

1-15: LGTM!

src/package.json (1)

450-454: LGTM!

src/turbo.json (1)

23-40: LGTM!

Also applies to: 41-58, 59-77, 78-88, 89-106

src/scripts/verify-coverage-contract.mjs (1)

1-4: LGTM!

Also applies to: 6-6, 14-26, 35-84, 87-166

.github/workflows/code-qa.yml (1)

128-131: LGTM!

Also applies to: 159-160, 169-175, 197-201, 238-242


📝 Summary

Summary by CodeRabbit

  • Tests
    • Split coverage testing into five parallel areas: API, core, services, miscellaneous, and tree-sitter.
    • Added separate coverage reports for each testing area, with validation and upload support.
    • Improved coverage checks to verify test ownership, boundaries, task dependencies, and report completeness.
    • Added safeguards to prevent duplicate cache saves during concurrent test runs.

Walkthrough

The pull request replaces monolithic unit coverage with five area-specific lanes: API, core, services, miscellaneous, and tree-sitter. It adds dedicated Vitest and Turbo configurations, validates lane partitioning and aggregate coverage, and updates CI report publication.

Changes

Coverage slicing

Layer / File(s) Summary
Area-specific test configurations
src/vitest.*.config.ts, src/package.json
Adds five Vitest configurations and scripts. Each lane selects its test files and writes coverage to a dedicated directory.
Turbo tasks and coverage contract
src/turbo.json, src/scripts/verify-coverage-contract.mjs
Adds five Turbo coverage tasks. The contract validates task inputs, dependencies, test partitioning, collection equivalence, and aggregate LCOV baselines.
CI execution and report publication
.github/workflows/code-qa.yml
Runs the five coverage lanes with concurrency of two. It verifies, uploads, and archives one LCOV report per lane. It also adds per-commit, OS-specific cache-save concurrency control.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant Turbo
  participant Vitest
  participant Codecov
  GitHubActions->>Turbo: Run five coverage tasks
  Turbo->>Vitest: Start area-specific test configuration
  Vitest->>GitHubActions: Write five LCOV reports
  GitHubActions->>Codecov: Upload per-lane LCOV reports
Loading

Merge Risk: ⚪ Minimal · up to 110a1

This change splits the extension's unit-test coverage collection into five independently cached lanes (API, core, services, misc, tree-sitter) with matching CI publication and validation logic. The lane definitions, build dependencies, and coverage outputs were checked for consistency and no functional gaps were found, so this is safe to merge as a build/tooling change with no expected impact on production behavior.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Linked Issues check ❌ Error Issue #115 requires area-scoped test and coverage inclusion. The new API and misc configs override test.include and coverage.reportsDirectory, but they inherit the base coverage.include of `src/… Set test.coverage.include to the matching source boundaries in each area config. Ensure the services config covers the complete services ownership area, or provide an issue-backed requirement for a separate tree-sitter lane and update the…
Lifecycle Resource Cleanup ⚠️ Warning The new coverage verifier can leave child tasks running after the verification path fails. collect() starts six execFile commands, and Promise.all rejects when one pnpm exec vitest list comman… Track the spawned child processes and cancel all pending collection commands when any collection fails or the verifier receives termination. Await Promise.allSettled for every collection before removing collectionDirectory. Use `AbortCo…
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: caching extension coverage by ownership lanes.
Description check ✅ Passed The description explains the changes, rationale, implementation details, performance impact, test evidence, and linked issue. It does not use all template headings or include the pre-submission checkl…
Out of Scope Changes check ✅ Passed The additional tree-sitter lane, Turbo tasks, workflow serialization, and coverage-contract checks support the same coverage slicing, caching, and parallel execution objective. The reviewed evidence d…
Regression Evidence ✅ Passed PASS — The reviewed range changes only CI orchestration, Turbo/Vitest coverage configuration, package scripts, and the coverage-contract verifier. It changes no application behavior or visible UI. The…
Security Boundaries ✅ Passed No changed path meets the security failure conditions. The new coverage configurations contain only static include, exclude, and report paths. verify-coverage-contract.mjs invokes pnpm/node with…
Persistence Integrity ✅ Passed No changed application persistence path introduces the stated failure conditions. The PR adds separate coverage outputs, and each lane propagates the same coverage/{lane} destination through the Vit…
Full details: Linked Issues check

Explanation

Issue #115 requires area-scoped test and coverage inclusion. The new API and misc configs override test.include and coverage.reportsDirectory, but they inherit the base coverage.include of src/**/*.ts, src/**/*.tsx, and eslint-rules/**/*.mjs from src/vitest.config.ts. The services config also excludes services/tree-sitter/**, so the four requested ownership configs do not cover the complete services area. The configs do provide isolated output directories and inherit the LCOV reporter. The existing full-suite config remains unchanged.

Resolution

Set test.coverage.include to the matching source boundaries in each area config. Ensure the services config covers the complete services ownership area, or provide an issue-backed requirement for a separate tree-sitter lane and update the four-area contract accordingly. Retain the per-area coverage/{area} directories and LCOV reporter.

Full details: Lifecycle Resource Cleanup

Explanation

The new coverage verifier can leave child tasks running after the verification path fails. collect() starts six execFile commands, and Promise.all rejects when one pnpm exec vitest list command fails. The finally block only removes the temporary directory; it does not abort, kill, or await the remaining child processes. A failing lane configuration with a slower or hanging sibling therefore leaves unnecessary Vitest/pnpm work active and can keep the verifier process alive. An isolated Node check reproduced this behavior: the aggregate promise rejected at about 147 ms, but the sibling process kept the parent active for about 1.2 seconds.

Resolution

Track the spawned child processes and cancel all pending collection commands when any collection fails or the verifier receives termination. Await Promise.allSettled for every collection before removing collectionDirectory. Use AbortController with execFile where possible, and explicitly terminate the underlying process tree when required by the platform.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/cache-coverage-lanes-2hyscfg2ezngj

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review status

This PR was opened by an automated account. A human maintainer must verify the change intent, provenance, and validation before merging.

Current step: Required CI passed. Waiting for automated review of the latest commit.

If automated review does not start, a maintainer must restart it.

Review-state labels are managed by this workflow; do not edit them manually.

@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@edelauna
edelauna marked this pull request as ready for review September 13, 2026 22:08
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 13, 2026
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit and removed coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 13, 2026
@github-actions github-actions Bot removed coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 14, 2026
@zoomote zoomote Bot changed the title [Chore] Cache tree-sitter coverage independently [Chore] Cache extension coverage by ownership Sep 14, 2026
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 14, 2026
@edelauna
edelauna added this pull request to the merge queue Sep 14, 2026
@edelauna
edelauna removed this pull request from the merge queue due to a manual request Sep 14, 2026
@edelauna
edelauna added this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit 99025b1 Sep 14, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit coderabbit-review-active Required CI passed; CodeRabbit review is active

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Add four area-scoped Vitest configs (api, core, services, misc)

2 participants