onebrc/lane-j: typed GridlakeCarrierError (addresses #641 review)#642
Conversation
Addresses the #641 review: as_gridlake_columns returned Result<_, ()> with a clippy allow, losing which grid failed alignment. Replace with a GridlakeCarrierError::UnalignedGrid { grid } that surfaces the offending grid size, drops the #[allow(clippy::result_unit_err)], and impls Display + std::error::Error. Kept a plain enum rather than a snafu type: onebrc-probe is a workspace-excluded standalone probe whose lanes A/C are dependency-free by design (its Cargo.toml), so a snafu type would drag a new dep into a crate that deliberately carries none — the enum gives the same diagnostic context without that cost. The unaligned-grid test now asserts the variant (grid=72). 2/2 gridlake tests green; lane_j.rs fmt + clippy clean. Not addressed: the STATUS_BOARD "append-only" comment is a false positive — the board's sections are newest-first and this prepended a NEW section without reordering or rewriting any prior entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLBnPuScZy6w9di2QEjsXM
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
What
Addresses the CodeRabbit review on #641 (merged):
GridBatch::as_gridlake_columnsreturnedResult<GridlakeColumns, ()>with a#[allow(clippy::result_unit_err)], which lost the diagnostic context of which grid failed alignment.GridlakeCarrierError::UnalignedGrid { grid: usize }— surfaces the offending grid size, drops the#[allow], and implsDisplay+std::error::Error.grid = 72).Why a plain enum, not
snafuThe reviewer suggested a
snafu-derived error.onebrc-probeis a workspace-excluded standalone probe whose lanes A/C are dependency-free by design (documented in itsCargo.toml) — asnafutype would drag a new dependency into a crate that deliberately carries none. A plain enum gives the same diagnostic context (grid size,Display,Error) without that cost, which fits this crate better than the repo-widesnafuguideline the auto-review applied.The other #641 comment
CodeRabbit also flagged the
STATUS_BOARD.mdchange as breaking append-only ordering. That's a false positive: the board's sections are ordered newest-first (the 2026-07-04 section on top, then 07-02, then older), and this prepended a new section — it did not reorder or rewrite any prior entry, so provenance is preserved. No change made there.Verification
cargo test --manifest-path crates/onebrc-probe/Cargo.toml --features lane-j— 2/2 gridlake tests green (LE roundtrip + unaligned-grid variant assertion).lane_j.rsfmt + clippy clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01MLBnPuScZy6w9di2QEjsXM
Generated by Claude Code