chore: re-cut as 0.0.1-alpha.1, and refresh the chart page from the results - #250
Merged
Conversation
…esults Two things. **The version.** `v0.1.0-alpha.1` was cut from #189 on a `Release-As:` footer I proposed after telling Felix that `0.0.1-alpha` was not a version this repo could produce. That was wrong — `Release-As:` forces any version and RELEASING.md documents it as lever 2 — so the choice got made from a menu that was missing the wanted option. The tag and its GitHub release are deleted (nothing had shipped: `PUBLISH_TO_PYPI` is unset, so that job skipped), the manifest and CHANGELOG are reset to `0.0.0-alpha.33`, and the footer here cuts `0.0.1-alpha.1` instead. Patch is 1 rather than 0, so the stream continues at `0.0.1-alpha.N` rather than sitting in the absorbing state the config notes describe. **`bench/plot.py`, 62 lines.** `docs/benchmarks-scaling.html` was hand-built, which made it numbers with no link to the run behind them — the failure `bench/README.md` opens by describing. This rewrites exactly one line of the page, the `const DATA = {...};` literal, and touches nothing else. Patching rather than templating is deliberate: the page is a tracked source file whose prose carries the actual claims ("the bands do not overlap at any rung" is an argument, not a rendering), and a template would move that into a file nobody opens while burying every prose edit under a regenerated page. Run against the committed results it reproduces the hand-built page byte for byte, which is the only evidence the two ever agreed. Release-As: 0.0.1-alpha.1
📝 WalkthroughWalkthroughThe benchmark workflow now separates ladder and density runs, reports both datasets, and adds a plotting script that updates the scaling page. Release metadata changes adjust the package version and remove the prior changelog release section. ChangesBenchmark plotting workflow
Release metadata
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bench/plot.py (1)
31-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse precise types for the measurement tables.
dict[Any, Any]hides the tuple-key and numeric-value contract used bypanel(), preventing Pyrefly from catching malformed records. Define typed aliases/TypedDicts and type the decoded timing records at the JSON boundary.As per coding guidelines,
**/*.pymust use Pyrefly for type checking; fix types rather than widening them.🤖 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 `@bench/plot.py` around lines 31 - 42, The measurement tables in best currently use dict[Any, Any], hiding their tuple-key and numeric-value contracts. Define precise type aliases or TypedDicts for timing records and the wall, peak, and cols tables, type the decoded JSON record at the boundary, and update best so its return type and intermediate values are precise enough for panel() and Pyrefly to validate malformed records.Source: Coding guidelines
🤖 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 `@bench/plot.py`:
- Around line 59-64: The benchmark input contract is inconsistent between the
plotting code and harness documentation. In bench/plot.py lines 59-64, update
the scaling input path and expected size/case keys to match the dataset produced
by the harness, or explicitly generate the required scaling dataset; in
bench/README.md lines 15-21, document and produce scaling.jsonl with dispatch
and xs–2xl entries, or change the plotting invocation to consume the density
dataset. Keep both sites aligned on the same contract.
---
Nitpick comments:
In `@bench/plot.py`:
- Around line 31-42: The measurement tables in best currently use dict[Any,
Any], hiding their tuple-key and numeric-value contracts. Define precise type
aliases or TypedDicts for timing records and the wall, peak, and cols tables,
type the decoded JSON record at the boundary, and update best so its return type
and intermediate values are precise enough for panel() and Pyrefly to validate
malformed records.
🪄 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: c729b8eb-9799-4603-8aa5-41d6f59e4fa5
📒 Files selected for processing (4)
.release-please-manifest.jsonCHANGELOG.mdbench/README.mdbench/plot.py
💤 Files with no reviewable changes (1)
- CHANGELOG.md
Comment on lines
+59
to
+64
| ladder = best(Path('bench/results/latest.jsonl'), 'highs') | ||
| scaling = best(Path('bench/results/scaling.jsonl'), 'lp') | ||
| cases = sorted({c for c, _, _ in ladder['wall']}) | ||
| data = { | ||
| 'scaling': panel(scaling, 'dispatch', SCALING, ('farkas', 'linopy', 'duckdb', 'duckdb@1GB')), | ||
| 'cases': {c: panel(ladder, c, LADDER, ('farkas', 'linopy', 'duckdb')) for c in cases}, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The benchmark producer and plotting consumer disagree on the input contract.
bench/plot.py#L59-L64: consume the file and size/case schema produced by the harness, or explicitly document and generate the required scaling run.bench/README.md#L15-L21: producescaling.jsonlwithdispatch/xs–2xl, or change the plotting invocation to match the density dataset.
📍 Affects 2 files
bench/plot.py#L59-L64(this comment)bench/README.md#L15-L21
🤖 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 `@bench/plot.py` around lines 59 - 64, The benchmark input contract is
inconsistent between the plotting code and harness documentation. In
bench/plot.py lines 59-64, update the scaling input path and expected size/case
keys to match the dataset produced by the harness, or explicitly generate the
required scaling dataset; in bench/README.md lines 15-21, document and produce
scaling.jsonl with dispatch and xs–2xl entries, or change the plotting
invocation to consume the density dataset. Keep both sites aligned on the same
contract.
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
1 participant
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.
Two things.
The version.
v0.1.0-alpha.1was cut from #189 on aRelease-As:footerI proposed after telling Felix that
0.0.1-alphawas not a version this repocould produce. That was wrong —
Release-As:forces any version andRELEASING.md documents it as lever 2 — so the choice got made from a menu that
was missing the wanted option. The tag and its GitHub release are deleted
(nothing had shipped:
PUBLISH_TO_PYPIis unset, so that job skipped), themanifest and CHANGELOG are reset to
0.0.0-alpha.33, and the footer here cuts0.0.1-alpha.1instead. Patch is 1 rather than 0, so the stream continues at0.0.1-alpha.Nrather than sitting in the absorbing state the config notesdescribe.
bench/plot.py, 62 lines.docs/benchmarks-scaling.htmlwas hand-built,which made it numbers with no link to the run behind them — the failure
bench/README.mdopens by describing. This rewrites exactly one line of thepage, the
const DATA = {...};literal, and touches nothing else.Patching rather than templating is deliberate: the page is a tracked source
file whose prose carries the actual claims ("the bands do not overlap at any
rung" is an argument, not a rendering), and a template would move that into a
file nobody opens while burying every prose edit under a regenerated page.
Run against the committed results it reproduces the hand-built page byte for
byte, which is the only evidence the two ever agreed.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores