Conversation
- make run-benchmarks tolerate missing REPORT_DICT_TRAIN rows\n- keep benchmark report sections explicit with n/a placeholders\n- publish benchmark-report and benchmark-delta artifacts to gh-pages/dev/bench\n- add direct README links to published delta/full reports
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Updates CI benchmark reporting so runs that omit dictionary-training output still produce publishable reports, and publishes the latest benchmark delta/full reports to GitHub Pages (with README links).
Changes:
- Make the benchmark parser non-fatal when
REPORT_DICT_TRAINlines are missing, and keep dictionary sections explicit via placeholder rows. - Publish
benchmark-report.md,benchmark-delta.md, andbenchmark-delta.jsontogh-pagesunderdev/bench/. - Add README links to the published “latest” benchmark delta and full reports.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| README.md | Adds links to the latest published benchmark delta/full reports on GitHub Pages. |
| .github/workflows/ci.yml | Checks out gh-pages on main pushes and commits/pushes the generated benchmark report artifacts. |
| .github/scripts/run-benchmarks.sh | Makes dict-train parsing non-fatal and emits placeholder rows when dictionary sections are missing. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBenchmark script now warns (does not exit) on missing dictionary-training rows and inserts Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as CI Runner
participant BenchScript as run-benchmarks.sh
participant Workspace as Job Workspace
participant GhPages as gh-pages (subdir)
Runner->>BenchScript: start benchmark job
BenchScript->>Workspace: generate artifacts (benchmark-report.md, benchmark-delta.json, benchmark-delta.md)
Runner->>GhPages: checkout `gh-pages` into subdir
Runner->>GhPages: create `dev/bench/` and copy artifacts into it
Runner->>GhPages: git add files
Runner->>GhPages: git diff --cached || git commit
Runner->>GhPages: push using app token
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/run-benchmarks.sh:
- Line 299: Update the warning string currently set to "WARN: No
REPORT_DICT_TRAIN lines parsed; dictionary training section will be empty." so
it accurately reflects that a placeholder row is emitted; change the message to
indicate "no data rows parsed; placeholder row will be emitted." Locate the
exact literal in the shell script (the string on line with WARN: No
REPORT_DICT_TRAIN) and replace it with the new wording; keep surrounding
formatting and quoting identical and run the script to verify the updated
message appears where previously the old warning printed.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc1fa69e-3d48-4be3-a72c-c4f219db7860
📒 Files selected for processing (3)
.github/scripts/run-benchmarks.sh.github/workflows/ci.ymlREADME.md
![sw-release-bot[bot]](https://avatars.githubusercontent.com/u/228206483?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'structured-zstd vs C FFI'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.15.
| Benchmark suite | Current: ce3624a | Previous: b280757 | Ratio |
|---|---|---|---|
compress/best/small-4k-log-lines/matrix/c_ffi |
0.344 ms |
0.28 ms |
1.23 |
compress/fastest/low-entropy-1m/matrix/c_ffi |
0.17 ms |
0.142 ms |
1.20 |
compress/fastest/large-log-stream/matrix/c_ffi |
2.55 ms |
2.149 ms |
1.19 |
decompress/fastest/high-entropy-1m/c_stream/matrix/c_ffi |
0.033 ms |
0.027 ms |
1.22 |
decompress/default/high-entropy-1m/rust_stream/matrix/c_ffi |
0.156 ms |
0.109 ms |
1.43 |
decompress/best/high-entropy-1m/rust_stream/matrix/c_ffi |
0.144 ms |
0.12 ms |
1.20 |
decompress/best/high-entropy-1m/c_stream/matrix/c_ffi |
0.161 ms |
0.038 ms |
4.24 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @polaz
- update REPORT_DICT_TRAIN warning to mention _n/a_ placeholder output - rename gh-pages publish commit message to benchmark reports
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/run-benchmarks.sh:
- Around line 484-485: The REPORT_DICT warning text is inconsistent with the
code path that emits a placeholder row when dictionary_rows is empty; update the
message referenced by REPORT_DICT (or the variable/constant used at the previous
message location) so it no longer claims the section “will be empty” and instead
reflects that an “_n/a_” placeholder row is written (search for REPORT_DICT,
dictionary_rows, and the lines.append call that writes "| _n/a_ | _no dictionary
rows emitted in this run_ |" and change the warning text accordingly).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d00833d3-988d-4b9d-8996-e3976a4f5442
📒 Files selected for processing (2)
.github/scripts/run-benchmarks.sh.github/workflows/ci.yml
- update REPORT_DICT warning to mention _n/a_ placeholder rows when section has no data
Summary
REPORT_DICT_TRAINrows are absent, while keeping ratio/memory sections strict_n/a_placeholder rows inbenchmark-report.mdwhen dictionary sections have no rowsbenchmark-report.md,benchmark-delta.json, andbenchmark-delta.mdtogh-pages/dev/benchonmainpushesValidation
Related #56
Summary by CodeRabbit
New Features
Bug Fixes
Documentation