Skip to content

fix(benchmark): avoid negative curl/cargo cases that fail the benchmark job - #3430

Open
ousamabenyounes wants to merge 1 commit into
rtk-ai:developfrom
ousamabenyounes:fix/benchmark-negative-curl-cargo
Open

fix(benchmark): avoid negative curl/cargo cases that fail the benchmark job#3430
ousamabenyounes wants to merge 1 commit into
rtk-ai:developfrom
ousamabenyounes:fix/benchmark-negative-curl-cargo

Conversation

@ousamabenyounes

Copy link
Copy Markdown
Contributor

Problem

The shared benchmark CI job (scripts/benchmark.sh) fails with BENCHMARK FAILED: 1 filter(s) produced more tokens than raw output. The offending case is curl json, which hits https://mockhttp.org/json/1 — a response so tiny (a handful of tokens) that RTK's curl formatting adds a few tokens rather than saving any (35 → 38, a negative case). Because this lives in the shared benchmark script, every branch based on develop inherits the red job, regardless of its own diff.

A second latent negative existed on the cargo side: cargo build on a trivial project (only a Finished line) could render a summary larger than the raw output.

Fix

  • scripts/benchmark.sh: replace the micro mockhttp.org/json/1 fixture with a small local JSON payload big enough to exercise real formatting savings, and point the wget case at a stable path.
  • src/cmds/rust/cargo_cmd.rs: guard both the streamed and non-streamed cargo build success summaries with core::guard::never_worse(raw, summary) so RTK never emits a summary larger than the raw output when the input is already tiny.

Test verification (RED → GREEN)

Two new unit tests assert the raw output is preserved when the summary would be larger:

test cmds::rust::cargo_cmd::tests::test_filter_cargo_build_success_uses_raw_when_summary_is_larger ... ok
test cmds::rust::cargo_cmd::tests::test_cargo_build_stream_success_uses_raw_when_summary_is_larger ... ok

RED (before the never_worse guard): the cargo build success path returned the rendered summary unconditionally, which is larger than the raw single-line Finished ... input.
GREEN (after): assert_eq!(result, output) — raw output is returned unchanged.

Full local suite on this branch (off develop):

cargo fmt --all --check   → clean
cargo clippy --all-targets → clean
cargo test                 → 2564 passed; 0 failed; 8 ignored

@aeppling

aeppling commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Hey @ousamabenyounes , thanks for this, actually @KuSh already opened an issue on mockhttp repo and this should be solved now that it is merged , jaredwray/mockhttp#181 , so apparently we should get a stable /json/1

@KuSh

KuSh commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Still missing a release on mockhttp side for the stable endpoint to be available though

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.

3 participants