Skip to content

perf(ci): trim plugin installs from CLI coverage - #11333

Merged
cv merged 2 commits into
mainfrom
codex/6237-ci-lightweight-installs
Sep 10, 2026
Merged

perf(ci): trim plugin installs from CLI coverage#11333
cv merged 2 commits into
mainfrom
codex/6237-ci-lightweight-installs

Conversation

@cjagwani

@cjagwani cjagwani commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Outcome

CLI coverage shards now install only the plugin's production dependencies, and the coverage merge job skips plugin dependencies entirely. A three-run local benchmark reduced the median plugin install from 20.6s to 11.5s per shard (44%) and node_modules from 121MB to 6.7MB; skipping it removes the full install from merge setup. Production behavior is unchanged.

Reason

All 12 CLI coverage shards and the merge job used the shared full dependency install even though shards only need plugin runtime packages and merge needs no plugin packages. That repeated avoidable work extends the coverage gate's critical path and consumes runner I/O.

Related issues

Part of #6237.

Changes

  • Add explicit full, production, and none plugin modes to the trusted CI dependency installer; existing callers retain the full default.
  • Use production in CLI coverage shards and none in the CLI coverage merge job.
  • Keep the plugin Vitest project contract runnable without plugin-local development dependencies by comparing lockfile versions and using the root TypeScript compiler already installed by the shard.
  • Remove the now-stale slow-test timing hint after the contract's measured median fell from 4.74s to 4.14s locally.
  • Extend repository and workflow contracts for every install mode and caller.

This recovers only the small dependency-install portion of closed PR #11098; broader shard balancing and test splitting remain separate changes.

Verification

  • npm run catalog:compile && npx vitest run --project integration test/repository/ci-install-dependencies.test.ts test/automation/pull-requests/pr-workflow-contract.test.ts test/repository/plugin-vitest-project.test.ts test/repository/cli-coverage-sequencer.test.ts --coverage=false — 4 files and 66 tests passed with nemoclaw/node_modules containing production dependencies only.
  • Three isolated npm ci runs with a warm shared cache — full median 20.574s; --omit=dev median 11.452s; install size 121MB vs 6.7MB.
  • Three focused contract runs — current-main median 4.74s; this change median 4.14s.
  • Normal pre-commit hooks — formatting, lint fixes, repository checks, ShellCheck, gitleaks, and source-shape checks passed.
  • Normal pre-push hooks — CLI TypeScript check passed.
  • GitHub commit verification — commit 9965cbabbcf685971e5d71ae03086fe4137ec990 is Verified.
  • The first CI run exposed three assertions that reproduced unchanged on clean main at f8f9a1b240; landed fix test(ci): refresh current main contracts #11332 changes exactly those assertions. After merging current main, all three files pass (69 tests).
  • The non-rewriting main merge commit 97630b8e662cc35378d4306a38f6c8298ddd6269 is also Verified.
  • Diff review and gitleaks confirm no secrets, API keys, or credentials are present.

Review notes

The install-time measurements are local macOS evidence; the exact hosted-runner saving should be confirmed by this PR's fresh CI timing. The change is fail-closed: unsupported modes and multiple arguments are rejected before any install runs.


Signed-off-by: Charan Jagwani cjagwani@nvidia.com

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cjagwani cjagwani self-assigned this Sep 9, 2026
@cjagwani cjagwani changed the title perf(ci)::// trim plugin installslyph perf(ci): trim plugin installs from CLI coverage Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 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: CHILL

Plan: Enterprise

Run ID: 1b6391f0-940d-462b-81f6-7f5481513514

📥 Commits

Reviewing files that changed from the base of the PR and between f8f9a1b and 9965cba.

📒 Files selected for processing (7)
  • .github/actions/ci-cli-coverage-merge/action.yaml
  • .github/actions/ci-cli-coverage-shard/action.yaml
  • .github/actions/ci-install-dependencies.sh
  • ci/cli-test-timing-hints.json
  • test/automation/pull-requests/pr-workflow-contract.test.ts
  • test/repository/ci-install-dependencies.test.ts
  • test/repository/plugin-vitest-project.test.ts
💤 Files with no reviewable changes (1)
  • ci/cli-test-timing-hints.json

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


📝 Walkthrough

Walkthrough

The CI dependency installer now accepts full, production, and none modes. Coverage actions pass explicit modes. Tests validate installer behavior and locked plugin dependency versions.

Changes

CI dependency installation modes

Layer / File(s) Summary
Installer mode handling
.github/actions/ci-install-dependencies.sh
The installer validates its mode argument and conditionally installs plugin dependencies with the matching npm options.
Action mode wiring and contracts
.github/actions/ci-cli-coverage-merge/action.yaml, .github/actions/ci-cli-coverage-shard/action.yaml, test/automation/pull-requests/pr-workflow-contract.test.ts
Coverage actions pass none or production. The workflow contract test expects the action-specific commands.
Installer and plugin validation
test/repository/ci-install-dependencies.test.ts, test/repository/plugin-vitest-project.test.ts, ci/cli-test-timing-hints.json
Tests cover installation modes, invalid arguments, locked dependency versions, and file separation. The plugin test timing hint was removed.

Priority: ⬇️ Low

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

Merge Risk: ⚪ Minimal · up to 97630

CI coverage workflows now install only the plugin dependencies they need while preserving the default full installation path. The updated installer and workflow contracts cover the new modes, with no remaining merge-blocking risk identified.

Suggested reviewers: cv, apurvvkumaria, prekshivyas

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reducing plugin installations in CLI coverage workflows.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/6237-ci-lightweight-installs

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

@github-code-quality

github-code-quality Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 97630b8 in the codex/6237-ci-lightw... branch remains at 96%, unchanged from commit ae5b2ca in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 97630b8 in the codex/6237-ci-lightw... branch remains at 83%, unchanged from commit ae5b2ca in the main branch.

Show a line coverage summary of the most impacted files.
File main ae5b2ca codex/6237-ci-lightw... 97630b8 +/-
src/lib/actions...ridge-output.ts 95% 89% -6%
src/lib/actions...e-validation.ts 88% 84% -4%
src/lib/actions...ard-recovery.ts 97% 95% -2%
src/lib/actions...ard-recovery.ts 85% 84% -1%
src/lib/onboard...uild-context.ts 75% 75% 0%
src/lib/actions...ess-recovery.ts 82% 84% +2%
src/lib/actions...eway-restart.ts 92% 94% +2%
src/lib/inferen...anaged-state.ts 82% 85% +3%
src/lib/state/o...config-merge.ts 85% 92% +7%
src/lib/actions...-diagnostics.ts 67% 81% +14%

Updated September 09, 2026 21:10 UTC

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cjagwani
cjagwani requested review from cv and prekshivyas September 9, 2026 21:13
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit 97630b8. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions chore Build, CI, dependency, or tooling maintenance labels Sep 9, 2026
@cv
cv enabled auto-merge (squash) September 10, 2026 01:27
@cv
cv merged commit a4265ab into main Sep 10, 2026
67 checks passed
@cv
cv deleted the codex/6237-ci-lightweight-installs branch September 10, 2026 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants