Skip to content

test: complete Vitest 5 coverage tooling - #215

Merged
steipete merged 1 commit into
mainfrom
chore/clawpatch-vitest5-coverage
Sep 13, 2026
Merged

steipete merged 1 commit into
mainfrom
chore/clawpatch-vitest5-coverage

Conversation

@steipete

@steipete steipete commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Vitest 5.0.0 is already installed on main (adopted in 536acaf), but the repository had no matching coverage provider or CI coverage run, and its development instructions permitted Node versions the test runner cannot run on. This completes the tooling against the official Vitest 5 migration guide.

Adds @vitest/coverage-v8 5.0.0 and pnpm test:coverage, covering all 85 production TypeScript files under src/, including unimported files. The Node 26 CI job runs the full suite with coverage; the existing Node 22/24 full suites and Windows execution check remain. Coverage prints a text report and writes JSON summary/HTML under coverage/; Vitest 5's .vitest/ artifacts are ignored.

Development docs now distinguish Vitest's Node 22.x (22.12+), 24.x, or 26+ support from the published CLI's unchanged engines.node >=22 contract. There was no Node 25 matrix entry or third-party pool to migrate. No coverage thresholds existed; no test assertions, skips, timeouts, production code, runtime dependencies, or package version changed. The Unreleased changelog records the completed tooling. Vitest and the V8 provider resolve to 5.0.0; Vite remains 8.3.0.

Validation on head 3590234d66a367e2d4f3eb3dda0c1ace372c3100:

Full-suite proof Result Vitest duration Wall time
Before, Node 24.21.0, pnpm test --maxWorkers=2 50 files; 957 passed, 2 existing platform skips 208.81 s 213.51 s
After, same Node and command 50 files; 957 passed, same 2 skips 213.35 s 219.55 s
After, Node 24.21.0, pnpm test:coverage --maxWorkers=2 50 files; 957 passed, same 2 skips 112.14 s 116.77 s
Linux CI, Node 26, pnpm test:coverage 50 files; 957 passed, same 2 skips 39.00 s See linked job

These local runs were on a busy shared macOS host; timings are observations, not a performance claim. The before/after comparison starts from the existing Vitest 5 baseline, not Vitest 4.

Local coverage: 89.15% statements, 82.26% branches, 91.90% functions, 89.45% lines. Verified that the JSON report includes exactly all 85 production TypeScript files, with no missing or unexpected files, and that the HTML report exists. Linux Node 26 coverage: 89.17% statements, 82.25% branches, 91.98% functions, 89.45% lines.

Frozen install, typecheck, lint, formatting, and dependency audit (zero vulnerabilities) pass. Node 26 build and built CLI --help pass; pnpm pack:smoke installs the actual tarball offline and maps 13 features, including 3 CUDA features, in 43.68 seconds locally. The same packaged CLI smoke passes in CI. Independent Codex autoreview is scoped-clean through P2.

Exact-head CI is green for Node 22/24/26 and Windows. CodeQL, dependency review, and secret scanning pass. No CI retries were needed.

@steipete
steipete requested a review from a team as a code owner September 13, 2026 21:01
@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 13, 2026
@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed September 13, 2026, 5:04 PM ET / 21:04 UTC.

ClawSweeper review

What this changes

Adds Vitest V8 coverage reporting, runs it in Node 26 CI, and documents development requirements without changing the published CLI’s runtime requirements.

Merge readiness

Blocked before merge - 2 items remain

The tooling is still missing from main and v0.8.0, and the patch has no identified correctness defect. After-change coverage output remains a merge blocker.

Likely related people: steipete (medium-confidence routing candidate based on prior tooling work).

Priority: P3
Reviewed head: 3590234d66a367e2d4f3eb3dda0c1ace372c3100

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and appears correct, but evidence for the newly introduced reporting path is still pending.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The changed owner is the Vitest coverage command and provider configuration. The captured evidence does not show an after-change run producing terminal, JSON-summary, and HTML reports; static checks and pre-change test timings do not exercise that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The changed owner is the Vitest coverage command and provider configuration. The captured evidence does not show an after-change run producing terminal, JSON-summary, and HTML reports; static checks and pre-change test timings do not exercise that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Applicable repository policy: Read the complete root AGENTS.md at the checkout and base revision. No nested AGENTS.md or maintainer-notes directory was found. Applied guidance on focused changes, generated artifacts, validation, and changelog documentation.
Introduced coverage configuration: The complete pinned diff adds the matching development dependency, coverage command and reporters; only the Node 26 test step changes. Runtime dependencies, CLI engines, test assertions, Node 22/24 jobs, Windows checks, Action references, and permissions remain unchanged.
Main and release still lack coverage configuration: Both fetched main and v0.8.0 have only the test include pattern, mock restoration, and timeout configuration. Current-tree searches found no competing coverage implementation. No merged replacement was established.
Findings None None.
Security None None.

How this fits together

Clawpatch’s development tooling runs the TypeScript test suite locally and in CI. The coverage provider measures exercised source files and produces terminal, JSON, and HTML reports.

flowchart LR
  A[TypeScript source and tests] --> B[Vitest runner]
  C[Local coverage command] --> B
  D[Node 26 CI] --> B
  B --> E[V8 coverage collection]
  E --> F[Terminal and file reports]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The changed owner is the Vitest coverage command and provider configuration. The captured evidence does not show an after-change run producing terminal, JSON-summary, and HTML reports; static checks and pre-change test timings do not exercise that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Complete next step (P2) - Add the promised after-change coverage output and report evidence to the PR body. Terminal output or linked artifacts suffice; redact private paths, endpoints, and credentials. Updating the body should trigger another review; otherwise ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Coverage rollout 1 CI job changed; 0 production source or test assertions changed The change adds reporting while retaining the existing runtime compatibility checks.

Technical review

Best possible solution:

Keep coverage within the existing Vitest configuration and verify that the full suite produces the documented reports, including unimported source files.

Do we have a high-confidence way to reproduce the issue?

Not applicable to a reported runtime bug: this PR adds development tooling, and source inspection confirms the missing coverage setup on main.

Is this the best way to solve the issue?

Yes. Using the matching official coverage provider and existing CI job is a focused solution; no competing supported coverage setup was found.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 6c57d8bd2adf.

Labels

Label changes:

  • add P3: This is development-tooling completion without a demonstrated user-facing runtime defect.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed owner is the Vitest coverage command and provider configuration. The captured evidence does not show an after-change run producing terminal, JSON-summary, and HTML reports; static checks and pre-change test timings do not exercise that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P3: This is development-tooling completion without a demonstrated user-facing runtime defect.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed owner is the Vitest coverage command and provider configuration. The captured evidence does not show an after-change run producing terminal, JSON-summary, and HTML reports; static checks and pre-change test timings do not exercise that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Applicable repository policy: Read the complete root AGENTS.md at the checkout and base revision. No nested AGENTS.md or maintainer-notes directory was found. Applied guidance on focused changes, generated artifacts, validation, and changelog documentation. (AGENTS.md:1, 3590234d66a3)
  • Introduced coverage configuration: The complete pinned diff adds the matching development dependency, coverage command and reporters; only the Node 26 test step changes. Runtime dependencies, CLI engines, test assertions, Node 22/24 jobs, Windows checks, Action references, and permissions remain unchanged. (vitest.config.ts:8, 3590234d66a3)
  • Main and release still lack coverage configuration: Both fetched main and v0.8.0 have only the test include pattern, mock restoration, and timeout configuration. Current-tree searches found no competing coverage implementation. No merged replacement was established. (vitest.config.ts:4, 6c57d8bd2adf)
  • Dependency contract and package identity: The affirmative dependency signal is package.json’s @vitest/coverage-v8 addition and vitest.config.ts’s V8 provider selection. npm metadata identifies vitest-dev/vitest as the repository, matches the locked provider integrity and Vitest 5.0.0 peer, and confirms Node ^22.12.0 || ^24.0.0 || >=26.0.0. The official coverage reference supports the include, exclude, reporter, and default output-directory configuration; the migration guide documents the upgraded prerequisites.
  • Captured validation does not yet exercise coverage: The supplied body at sourceRevision 79ba8dd833e04ade0b49e63ee2bd6f04ac439a448362d6a48440f59f9db22b45 reports pre-change tests and static checks, explicitly leaving after-change coverage results pending. A read-only live PR read returned the same body and head; comments contained only review acknowledgments, and the scratch directory contained no proof artifacts. (3590234d66a3)
  • Prior tooling history supports routing: Main history records Peter Steinberger’s dependency and CI work, including b979f94. GitHub commit metadata maps its author to steipete, and its patch adds the Node 22/24 runtime matrix. Some deeper local pickaxe/blame reads failed while retrieving historical objects; the successful history and GitHub patch reads support routing without asserting feature introduction. (.github/workflows/ci.yml:50, b979f94701be)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add redacted after-change terminal output or linked report artifacts showing successful full-suite coverage collection and the documented report outputs.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 0885c93 into main Sep 13, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant