Track Node and Bun benchmark trends with calibrated gates - #6329
Merged
Conversation
📝 WalkthroughWalkthroughThe benchmark pipeline now runs five-sample Perry, Node, and optional Bun comparisons, validates versioned JSON artifacts, applies noise-aware regression gates, and records deterministic Fastify HTTP tracking results with raw and summarized outputs. ChangesBenchmark gating and tracking
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant CompareScript
participant BenchmarkGate
participant HonestBench
participant Oha
CIWorkflow->>CompareScript: run five-sample runtime benchmarks
CompareScript->>BenchmarkGate: build and compare benchmark artifacts
BenchmarkGate-->>CIWorkflow: return regression gate report
CIWorkflow->>HonestBench: run Fastify tracking workload
HonestBench->>Oha: collect fixed-load HTTP metrics
HonestBench-->>CIWorkflow: write raw results and metadata
CIWorkflow->>BenchmarkGate: summarize HTTP results
BenchmarkGate-->>CIWorkflow: write tracking summary
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_benchmark_gate.py (1)
259-298: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover unhealthy HTTP samples.
Add cases for a nonzero
exit_codeand the intended invalid-success-rate condition. The documented artifact contract invalidates unhealthy samples, while this suite currently only covers incomplete samples and missing metrics.🤖 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 `@tests/test_benchmark_gate.py` around lines 259 - 298, Extend the HTTP summarization tests around summarize_http with cases covering a nonzero exit_code and the documented invalid success-rate condition. Assert that each unhealthy sample raises ArtifactError, while preserving the existing incomplete-sample and missing-metric coverage.
🤖 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.
Inline comments:
In `@benchmarks/README.md`:
- Around line 123-126: Update the runtime metadata documentation around
build_artifact() to avoid describing Perry’s compile command as a required
schema field. Mark the compile-command metadata optional, or state that
validation enforces it only where applicable, while preserving the existing
required runtime fields and fallback behavior.
In `@tests/test_benchmark_gate.py`:
- Around line 18-28: Update the fixture helpers runtime and record to preserve
explicitly supplied empty command and RSS lists: replace truthiness-based
fallbacks in command and rss with None-only defaulting. Keep defaults for
omitted arguments while allowing [] to remain unchanged so malformed empty
inputs are detected.
---
Nitpick comments:
In `@tests/test_benchmark_gate.py`:
- Around line 259-298: Extend the HTTP summarization tests around summarize_http
with cases covering a nonzero exit_code and the documented invalid success-rate
condition. Assert that each unhealthy sample raises ArtifactError, while
preserving the existing incomplete-sample and missing-metric coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 619b104d-c3b6-43c7-b432-8ad294232d4c
⛔ Files ignored due to path filters (1)
benchmarks/honest_bench/workloads/4_http_fastify/node/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/benchmark.ymlREADME.mdbenchmarks/README.mdbenchmarks/benchmark_gate.pybenchmarks/compare.shbenchmarks/honest_bench/run.shbenchmarks/honest_bench/workloads/4_http_fastify/README.mdbenchmarks/honest_bench/workloads/4_http_fastify/node/package.jsontests/test_benchmark_gate.pytests/test_benchmark_peer_fallback.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@benchmarks/benchmark_gate.py`:
- Around line 268-269: Update validate_artifact so every completeness-only
validation, including sample-count checks, benchmark-set equality, and
derived-distribution requirements, is conditional on require_complete. Preserve
schema validation in both modes, while allowing evaluate_regressions to validate
incomplete baselines when it passes False; if leniency is not intended, remove
the unused parameter instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7162aadb-8e54-4fd7-8f10-5c200c8577a8
📒 Files selected for processing (9)
.github/workflows/benchmark.yml.github/workflows/test.ymlREADME.mdbenchmarks/README.mdbenchmarks/benchmark_gate.pybenchmarks/compare.shbenchmarks/honest_bench/workloads/4_http_fastify/README.mdtests/test_benchmark_gate.pytests/test_benchmark_peer_fallback.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- benchmarks/README.md
- benchmarks/honest_bench/workloads/4_http_fastify/README.md
- .github/workflows/benchmark.yml
- benchmarks/compare.sh
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/benchmark_gate.py (1)
347-347: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
runtime_results["perry"]will KeyError instead of raisingArtifactErrorif Perry is unavailable.Line 347 directly indexes
runtime_results["perry"]without a guard. If Perry's runtime metadata wereavailable: False, the loop at lines 301-306 would skip Perry, leaving it absent fromruntime_results. This would produce an unhandledKeyErrorrather than a descriptiveArtifactError. While Perry should always be available in practice, the validator should enforce this explicitly.🛡️ Proposed fix
perry_result = runtime_results.get("perry") - perry_result = runtime_results["perry"] + if not perry_result: + raise ArtifactError(f"{name}: Perry runtime is required but missing") ratios = _mapping(entry.get("ratios"), f"{name}: ratios")🤖 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 `@benchmarks/benchmark_gate.py` at line 347, Update the Perry validation flow around runtime_results and the perry_result assignment to explicitly verify Perry was available and produced a result before indexing it. Raise the existing ArtifactError with a descriptive message when Perry is missing, while preserving the current processing path when runtime_results contains "perry".
🤖 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.
Outside diff comments:
In `@benchmarks/benchmark_gate.py`:
- Line 347: Update the Perry validation flow around runtime_results and the
perry_result assignment to explicitly verify Perry was available and produced a
result before indexing it. Raise the existing ArtifactError with a descriptive
message when Perry is missing, while preserving the current processing path when
runtime_results contains "perry".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05eebc55-68bb-45d4-9047-d318d9768526
📒 Files selected for processing (3)
.github/workflows/benchmark.ymlbenchmarks/benchmark_gate.pytests/test_benchmark_gate.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/benchmark.yml
- tests/test_benchmark_gate.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens Perry's benchmark regression signal with complete Perry/Node/Bun distributions, peer-relative tracking, calibrated fast-benchmark noise handling, and deterministic Fastify HTTP diagnostics.
Changes
Related issue
N/A — implements performance audit findings #6 and #7.
Test plan
python3 -m unittest discover -s tests -p 'test_benchmark_gate.py'./tests/test_benchmark_peer_fallback.sh./tests/test_benchmark_output_verifier.shpython3 -m py_compile benchmarks/benchmark_gate.py tests/test_benchmark_gate.pybash -n benchmarks/compare.sh benchmarks/honest_bench/run.sh tests/test_benchmark_peer_fallback.sh tests/test_benchmark_output_verifier.sh.github/workflows/benchmark.ymland.github/workflows/test.ymlwith PyYAML.git diff --checkChecklist