Skip to content

chore: make the binary-size check measure the pull request's own code - #1501

Merged
jdx merged 2 commits into
mainfrom
chore/size-separate-target-dirs
Sep 24, 2026
Merged

jdx merged 2 commits into
mainfrom
chore/size-separate-target-dirs

Conversation

@jdx

@jdx jdx commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

https://entire.io/gh/jdx/usage/trails/50

The size check has not been measuring pull requests. Every recent report shows exactly +0 bytes on every binary, including #1497, which adds a lot of code to usage-lib and the CLI. #1498 then failed the check with a compile error that its own build doesn't have (no field delegate on type SpecComplete).

Cause. tasks/size.sh builds the merge base (in a worktree) and the head into one target directory, so crates.io dependencies compile once. But Cargo names a workspace crate's artifacts by its path relative to the workspace root, so both checkouts produce the same artifact names. After the base build, the head build found a fresh usage-lib under the same name and reused it. The head binary was therefore built from the base's library code. That explains the +0s, and the #1498 failure: its usage-cli rebuilt (it adds a libc dependency) against the base's usage-lib, which has no delegate field. The same reuse can happen across CI runs through the rust-cache'd target directory, and locally across branches.

Fix. Before each build, the script now runs cargo clean --release -p <member> for every workspace member, taken from cargo metadata --no-deps. Each side's workspace crates always build from that side's source, and crates.io dependencies stay shared and cached. The 1% gate, the binaries measured and what is gated are unchanged.

Validation. Run locally against the two open pull requests, reusing one target directory across both runs on purpose, since reuse is what broke:

PR usage CLI derived mise CLI (usage's share)
#1497 +9,272 B (+0.08%) +0
#1498 +79,424 B (+0.69%) at f4d08fea91; +31,808 B (+0.28%) after 1e065f682c dropped an mpsc channel +0

Before this change, the #1497 run in that setup failed to compile the same way #1498 did in CI. Both pull requests are under the gate. The derived CLI doesn't move because neither touches the derive or argv runtime.

🤖 Generated with Claude Code


Note

Low Risk
Touches only the tasks/size.sh CI script; no runtime, auth, or application code paths.

Overview
Fixes the binary-size CI check so head and base builds are compared against their own workspace crate artifacts instead of silently reusing each other’s from a shared CARGO_TARGET_DIR.

In tasks/size.sh, build() now discovers workspace members with cargo metadata --no-deps and runs cargo clean --release for each member before cargo build, while still sharing the target directory for crates.io deps. Comments are updated to document why the clean is required (identical artifact paths across the base worktree and the PR checkout caused +0 byte deltas and wrong linkages, e.g. head CLI against base usage-lib).

The 1% gate, measured binaries, and gating rules are unchanged—only measurement correctness.

Reviewed by Cursor Bugbot for commit aba852a. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Chores
    • Build size comparisons now use artifacts from the correct checkout, avoiding misleading results caused by reusing cached build outputs. This makes reported size differences more accurately reflect the code being compared.

tasks/size.sh builds the merge base and the head into one target directory.
Cargo names workspace crates' artifacts by path relative to the workspace
root, so the base worktree's freshly built usage-lib looked up to date to
the head build, which then linked the base's code. Every report came out +0
bytes, and #1498 failed to compile once its head no longer built against
the base's library. Workspace crates are now cleaned before each build;
crates.io dependencies stay shared. The gate itself is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: f1223b24-c502-4d5c-846d-92334fe12b9d

📥 Commits

Reviewing files that changed from the base of the PR and between 373bfb9 and aba852a.

📒 Files selected for processing (1)
  • tasks/size.sh

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


📝 Walkthrough

Walkthrough

The size comparison build now identifies workspace packages with cargo metadata and cleans their release artifacts before building. The updated comment explains that separate checkouts can otherwise reuse artifacts and report a comparison of +0 bytes.

Changes

Size comparison

Layer / File(s) Summary
Clean workspace release artifacts
tasks/size.sh
The build step gets workspace package names from cargo metadata --locked --no-deps and runs cargo clean --release --locked for those packages before the release build. The comment describes the artifact reuse issue.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to aba85

The size comparison cleans workspace release artifacts before each checkout’s build, helping prevent stale artifacts from skewing results. No concrete merge-blocking risk is established; the PR is ready for normal checks.

🚥 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 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: ensuring the binary-size check measures the pull request's own code.
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.
  • Fix all pre-merge checks with AI

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.

@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no outstanding finding or new actionable issue remains.

Summary

The PR makes the binary-size check rebuild workspace crates from each checkout while retaining a shared target directory. The follow-up change applies --locked to metadata and clean, matching the build’s lockfile constraint.

Reviews (2) · Last reviewed commit: "chore: keep the size check from rewritin..."

Comment thread tasks/size.sh Outdated
`cargo clean -p` resolves the workspace and could update a lockfile that
no longer matches the manifests, so the build that follows would measure
dependencies the pull request never pinned. Both it and `cargo metadata`
now run with --locked, like the build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jdx
jdx merged commit ffce74d into main Sep 24, 2026
14 checks passed
@jdx
jdx deleted the chore/size-separate-target-dirs branch September 24, 2026 20:05
@github-actions

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▆▇▆▇▇█▁▂ 285,027,329 → 285,064,854 +0.01% 49.92 → 49.49ms -0.86%
startup ▁▃▃▃▆▆▂█ 1,053,355 → 1,060,174 +0.65% 1.46 → 1.47ms +0.84%

No instruction-count regression above 1%.

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

Shadow comparison

Parsing mise use -g node@20 against a shadow of mise's committed spec.
Reported, not gated: the shadow grows as the derive learns to express more, so
what to watch is the ratio rather than either column.

framework stripped binary, bytes
usage 1262000
bpaf 2494656
clap 3100904
framework instructions, cold parse vs usage
usage 8530 —
clap 6314978 740x
bpaf 21909298 2568x
                                              min       p01       p10    median
usage-rs: argv -> struct                      793       797       801       831  ns
clap: build tree + parse -> struct        1243479   1246841   1254212   1271254  ns
bpaf: build parser + parse -> struct      3415140   3415140   3478484   3500207  ns

usage: argv -> struct                             763 ns      0.76 µs
clap: build tree + parse -> struct            1259890 ns   1259.89 µs
clap: parse -> struct, tree reused              50897 ns     50.90 µs
clap: build tree only                          749413 ns    749.41 µs

aba852a706ac vs 373bfb909efd · measured on the runner, not pushed to the history.

jdx pushed a commit that referenced this pull request Sep 28, 2026
<!-- entire-trail-link-start -->
https://entire.io/gh/jdx/usage/trails/33
<!-- entire-trail-link-end -->

### 🚀 Features

- **(cli)** publish a usage agent skill with packslip by
[@jdx](https://github.com/jdx) in
[#1509](#1509)
- **(complete)** complete a wrapped command's arguments with its own
shell completion via delegate= by [@jdx](https://github.com/jdx) in
[#1498](#1498)
- **(lib)** add getters that read FlagMeta, CommandMeta and ArgMeta
fields wherever they live by [@jdx](https://github.com/jdx) in
[#1491](#1491)
- **(lib)** export the chosen subcommand to scripts as usage_cmd by
[@jdx](https://github.com/jdx) in
[#1505](#1505)
- **(spec)** let a complete node sit inside the arg it completes by
[@jdx](https://github.com/jdx) in
[#1496](#1496)
- **(spec)** list an arg's possible values from a command with `choices
run=` by [@jdx](https://github.com/jdx) in
[#1497](#1497)

### 🐛 Bug Fixes

- **(bash)** complete `--flag=` values when the cursor sits after `=` by
[@jdx](https://github.com/jdx) in
[#1499](#1499)
- **(cli)** run scripts passed to usage bash through a pipe or process
substitution by [@jdx](https://github.com/jdx) in
[#1494](#1494)
- **(cli)** keep usage explain from running a spec's choices run=
commands by [@jdx](https://github.com/jdx) in
[#1503](#1503)
- **(complete)** read --line=LINE in completion requests from typed
completers by [@jdx](https://github.com/jdx) in
[#1487](#1487)
- **(complete)** show completion parse errors without garbling the
prompt by [@jdx](https://github.com/jdx) in
[#1495](#1495)
- **(derive)** ignore closed pipes instead of panicking in generated
parse() by [@jdx](https://github.com/jdx) in
[#1484](#1484)
- **(derive)** keep generated async dispatch from reserving stack for
every command by [@jdx](https://github.com/jdx) in
[#1488](#1488)
- **(fish)** complete words the user has started quoting by
[@jdx](https://github.com/jdx) in
[#1500](#1500)
- **(lib)** resolve inherited usage aliases after multiline workspace
entries by [@jdx](https://github.com/jdx) in
[#1507](#1507)
- **(lib)** make published usage-rs tests self-contained by
[@jdx](https://github.com/jdx) in
[#1510](#1510)
- **(zsh)** stop the completion-init handler from breaking file
completion for other commands by [@jdx](https://github.com/jdx) in
[#1493](#1493)

### 📚 Documentation

- **(cli)** describe fig output as the legacy Fig format by
[@jdx](https://github.com/jdx) in
[2a1bef6](2a1bef6)
- clarify CLI frameworks and generators on the homepage by
[@jdx](https://github.com/jdx) in
[#1482](#1482)

### 🛡️ Security

- remove Entire trail runners by [@jdx](https://github.com/jdx) in
[#1485](#1485)

### 🔍 Other Changes

- float jdx tools and aube on latest without a release-age delay by
[@jdx](https://github.com/jdx) in
[#1486](#1486)
- run cargo-semver-checks on every published crate by
[@jdx](https://github.com/jdx) in
[#1490](#1490)
- fail pull requests that grow the usage CLI or a derived CLI by more
than 1% by [@jdx](https://github.com/jdx) in
[#1492](#1492)
- make the binary-size check measure the pull request's own code by
[@jdx](https://github.com/jdx) in
[#1501](#1501)

### 📦️ Dependency Updates

- bump jdx/renovate-config workflows to c736149 by
[@jdx](https://github.com/jdx) in
[06f2f1b](06f2f1b)
- bump jdx/renovate-config workflows to aa49efc by
[@jdx](https://github.com/jdx) in
[9aaedef](9aaedef)
- bump jdx/renovate-config workflows to 5b46432 by
[@jdx](https://github.com/jdx) in
[36f8681](36f8681)
- pin jdx/renovate-config workflows to v1.0.0 by
[@jdx](https://github.com/jdx) in
[a31ae2f](a31ae2f)
- update communique to 1.4.2 in mise.lock by
[@jdx](https://github.com/jdx) in
[805396f](805396f)
- upgrade locked mise tools by [@jdx](https://github.com/jdx) in
[d8b761d](d8b761d)
- update jdx/packslip action to v1.4.0 by [@jdx](https://github.com/jdx)
in [#1508](#1508)
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