Repository navigation
ci: run perf-comment and pkg-pr-new jobs without the full Nix dev shell - #1258
Conversation
The `ccusage perf comment` and `ccusage rust perf comment` jobs entered the full `nix develop` dev shell (Rust toolchain, litellm, typescript-go, …) only to run a handful of bun/pnpm scripts. `compare-pr-performance.ts` already benchmarks the native binary bundled in the installed pkg.pr.new package (`installedNativePackageBinEntry` → node_modules/@ccusage/ccusage-<plat>-<arch>/bin/ccusage), so no Rust build — and therefore no cargo/toolchain — is involved. Provision only the tools these jobs actually use via `nix profile install --inputs-from . nixpkgs#pnpm nixpkgs#bun nixpkgs#hyperfine nixpkgs#just` (pinned to the flake's locked nixpkgs) plus an explicit `pnpm install --frozen-lockfile` (previously done by the dev shell's shellHook), and run the scripts directly. node continues to come from the runner image. This shrinks the cold closure from the whole dev shell to four small tools.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCI workflow updates replace ChangesWorkflow unwrapping and direct tooling execution
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant NixProfile
participant PNPM
participant Just
participant Bun
GitHubActions->>NixProfile: nix profile install pnpm,bun,hyperfine,just
GitHubActions->>PNPM: pnpm install --frozen-lockfile
GitHubActions->>Just: just generate-large-fixture
PNPM->>Bun: pnpm exec bun compare-pr-performance.ts
PNPM->>Bun: pnpm exec bun upsert-pr-comment.ts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | ace4c86 | Commit Preview URL Branch Preview URL |
Jun 10 2026, 11:25 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/ci.yaml (2)
223-225: 💤 Low valueConsider adding a step name for consistency.
The perf tooling installation and dependency setup look correct. The
--inputs-from .flag properly pins to the flake.lock, and all required tools (pnpm, bun, hyperfine, just) are installed.However, the
pnpm installstep on line 225 is missing aname:field. While valid YAML, adding a name would improve consistency with the rest of the workflow.📝 Suggested improvement
- name: Install perf tooling run: nix profile install --inputs-from . nixpkgs#pnpm nixpkgs#bun nixpkgs#hyperfine nixpkgs#just - - run: pnpm install --frozen-lockfile + - name: Install dependencies + run: pnpm install --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 223 - 225, The workflow step that runs "pnpm install --frozen-lockfile" is missing a name; add a descriptive "name:" field (e.g., "Install dependencies" or "Install pnpm dependencies") above the run line to match the rest of the workflow and improve readability and consistency with the preceding "Install perf tooling" step.
297-299: 💤 Low valueConsider adding a step name for consistency (mirrors ccusage-perf-comment).
Same perf tooling setup as the
ccusage-perf-commentjob. Thepnpm installstep on line 299 is also missing aname:field.📝 Suggested improvement
- name: Install perf tooling run: nix profile install --inputs-from . nixpkgs#pnpm nixpkgs#bun nixpkgs#hyperfine nixpkgs#just - - run: pnpm install --frozen-lockfile + - name: Install dependencies + run: pnpm install --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 297 - 299, The workflow step running "pnpm install --frozen-lockfile" is missing a name for consistency with the "Install perf tooling" step and the ccusage-perf-comment job; add a top-level "name:" key (e.g., "Install dependencies" or "Install pnpm deps") immediately above the run: pnpm install --frozen-lockfile line, matching indentation and style used by the "Install perf tooling" step so both steps have explicit names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 223-225: The workflow step that runs "pnpm install
--frozen-lockfile" is missing a name; add a descriptive "name:" field (e.g.,
"Install dependencies" or "Install pnpm dependencies") above the run line to
match the rest of the workflow and improve readability and consistency with the
preceding "Install perf tooling" step.
- Around line 297-299: The workflow step running "pnpm install
--frozen-lockfile" is missing a name for consistency with the "Install perf
tooling" step and the ccusage-perf-comment job; add a top-level "name:" key
(e.g., "Install dependencies" or "Install pnpm deps") immediately above the run:
pnpm install --frozen-lockfile line, matching indentation and style used by the
"Install perf tooling" step so both steps have explicit names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f2fe720-4cfc-4ded-985b-75fe93223a61
📒 Files selected for processing (1)
.github/workflows/ci.yaml
Code Coverage OverviewLanguages: Rust Rust / code-coverage/cargo-llvm-covThe overall coverage remains at 77%, unchanged from the Updated |
The pkg-pr-new dry-run job entered the full `nix develop` dev shell only to run `pnpm pkg-pr-new publish`. Packing repackages the native binaries restored from the build matrix, so no Rust toolchain is involved (bun is fetched via the engines.runtime entry during `pnpm install`). Provision just pnpm with `nix profile install --inputs-from . nixpkgs#pnpm` plus an explicit `pnpm install --frozen-lockfile`, matching the release and perf-comment jobs. Add comments to the pkg-pr-new and perf-comment jobs explaining why the dev shell is unnecessary, since the reasoning (prebuilt binaries, no cargo) is not obvious from the step alone.
ryoppippi
changed the title
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-x64
commit: |
Two CI tweaks: - Rename the `lint-check` job to `check` (and its action-timeline `needs` reference). `nix flake check` covers far more than linting — clippy, treefmt, schema drift, gitleaks, build — so `check` describes it better. - Add a `nix develop --command true` warm-up step to the `test` job so the dev shell realization and its pnpm install hook land in a dedicated step instead of bleeding into the first timed step and adding noise to action-timeline.
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-x64
commit: |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yaml">
<violation number="1" location=".github/workflows/ci.yaml:23">
P2: Renaming the CI job ID from `lint-check` to `check` changes the check-run context and can silently break required branch-protection checks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| uses: ./.github/actions/detect-code-changes | ||
|
|
||
| lint-check: | ||
| check: |
There was a problem hiding this comment.
P2: Renaming the CI job ID from lint-check to check changes the check-run context and can silently break required branch-protection checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 23:
<comment>Renaming the CI job ID from `lint-check` to `check` changes the check-run context and can silently break required branch-protection checks.</comment>
<file context>
@@ -20,7 +20,7 @@ jobs:
uses: ./.github/actions/detect-code-changes
- lint-check:
+ check:
needs: changes
if: needs.changes.outputs.code-changed == 'true'
</file context>
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
1 participant
Summary
Several CI jobs entered the full
nix developdev shell (Rust toolchain, litellm, typescript-go, …) just to run a few bun/pnpm scripts that never touch Rust. This slims theccusage perf comment,ccusage rust perf comment, andnpm-publish-dry-run-and-upload-pkg-pr-nowjobs down to the tools they actually use, provisioned withnix profile install --inputs-from . …(pinned to the flake's locked nixpkgs).Follows up #1257 (release publish off the dev shell + per-
github.jobsticky-disk keys). With per-job sticky disks, each job warms its own store; provisioning only what it needs keeps that store tiny instead of caching the whole dev shell.What changed
perf-comment jobs (
ccusage perf comment,ccusage rust perf comment)compare-pr-performance.tsalready benchmarks the native binary bundled in the installed pkg.pr.new package (installedNativePackageBinEntry→node_modules/@ccusage/ccusage-<plat>-<arch>/bin/ccusage) — no local Rust build, confirmed by job logs (package installs + "Installed native binary" benchmark, zerocargo/Compiling).nix developwithnix profile install --inputs-from . nixpkgs#pnpm nixpkgs#bun nixpkgs#hyperfine nixpkgs#just+pnpm install --frozen-lockfile, then run the scripts directly. No change tocompare-pr-performance.ts.pkg-pr-new dry-run job
engines.runtimeduringpnpm install).nix develop --command pnpm pkg-pr-new publishwithnix profile install --inputs-from . nixpkgs#pnpm+pnpm install --frozen-lockfile+ a directpnpm pkg-pr-new publish.Comments added to all three jobs explaining why the dev shell is unnecessary (prebuilt binaries, no cargo).
nodecontinues to come from the runner image. Thetypecheckandtestjobs keepnix develop— they genuinely need the full toolchain (cargo, clippy, cargo-llvm-cov, tsgo, vitest). The build matrix is untouched: linux/mac-arm use targetednix build .#ccusage[-static], and Intel mac / Windows alreadycargo buildwith the runner's system Rust (pinned byrust-toolchain.toml) — none use the dev shell.Why
Pulling the entire Rust dev shell to run a benchmarking or packaging script is pure overhead; on a cold sticky-disk miss it is ~4.5 min of download. The minimal tool set is a small closure (cold ~tens of seconds, instant when warm).
Testing / validation
actionlintclean onci.yaml;zizmorfindings identical before/after; pre-commit hooks pass.rust/target/release/ccusagepath is only a fallback / size-table entry, absent in CI).compare-pr-performance.tsspawnshyperfineand importsgunshi/fs-fixture;generate-large-fixture.tsimportsgunshi;upsert-pr-comment.tsusesfetchonly — all covered by the installed tools +pnpm install.All three jobs run on PR CI, so this PR exercises the changes directly.
Summary by CodeRabbit