Skip to content

refactor(bench): consolidate setup abort handling#180

Merged
acgetchell merged 1 commit into
mainfrom
refactor/165-bench-or-abort
Jul 11, 2026
Merged

refactor(bench): consolidate setup abort handling#180
acgetchell merged 1 commit into
mainfrom
refactor/165-bench-or-abort

Conversation

@acgetchell

@acgetchell acgetchell commented Jul 11, 2026

Copy link
Copy Markdown
Owner
  • Share OrAbort across exact and comparison benchmarks while preserving contextual panic behavior.
  • Centralize Clippy policy in Cargo.toml for consistent local and CI checks.
  • Keep unreachable_pub enforced while allowing intentional crate-visible helpers.
  • Enable CodeRabbit's request-changes review workflow.

Closes #165

Summary by CodeRabbit

  • Developer Experience

    • Standardized benchmark failure handling with clearer operation-specific error messages.
    • Updated lint configuration so Clippy rules are managed centrally and consistently.
    • Simplified Clippy workflow and command-line configuration.
  • Testing

    • Added coverage for benchmark error-handling behavior.
    • Reorganized testing guidance for improved clarity.

- Share `OrAbort` across exact and comparison benchmarks while preserving contextual panic behavior.
- Centralize Clippy policy in Cargo.toml for consistent local and CI checks.
- Keep `unreachable_pub` enforced while allowing intentional crate-visible helpers.
- Enable CodeRabbit's request-changes review workflow.

Closes #165
@acgetchell acgetchell self-assigned this Jul 11, 2026
@acgetchell acgetchell enabled auto-merge July 11, 2026 19:14
@acgetchell acgetchell disabled auto-merge July 11, 2026 19:14
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The benchmark suite now shares an OrAbort trait for contextual Result and Option failure handling. Exact and comparison benchmarks use it throughout setup and execution. Clippy lint levels are centralized in Cargo.toml, and review configuration and testing guidance are updated.

Benchmark abort handling

Layer / File(s) Summary
Shared OrAbort helper
benches/common/bench_utils.rs, tests/exact_bench_config.rs
Adds and tests OrAbort implementations for Result and Option, then wires the helper into benchmark-related modules.
Exact benchmark migration
benches/common/exact.rs, benches/exact.rs
Replaces local panic helpers across exact benchmark setup, arithmetic operations, corpus generation, and validation.
Comparison benchmark migration
benches/vs_linalg.rs
Uses or_abort across factorization, solve, determinant, norm, and stress benchmark paths.

Lint and review configuration

Layer / File(s) Summary
Clippy lint ownership
Cargo.toml, clippy.toml, justfile, .github/workflows/rust-clippy.yml, src/scaled_product.rs
Centralizes lint levels in Cargo.toml, removes command-line lint flags and a redundant suppression, and updates the SARIF workflow.
Review and testing guidance
.coderabbit.yaml, AGENTS.md
Enables the request-changes workflow and reorders two testing guidance bullets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several non-benchmark changes were added, including Clippy policy, CodeRabbit workflow, and AGENTS.md edits, which are outside issue #165. Split the benchmark helper refactor from config/docs changes or link the extra work to separate issues.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main bench refactor: consolidating abort handling into a shared postfix helper.
Linked Issues check ✅ Passed The shared OrAbort trait replaces the duplicated bench helpers, supports Result and Option, and the exact/vs_linalg call sites were migrated.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/165-bench-or-abort

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.75%. Comparing base (2ffa19c) to head (b9e453e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #180   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files           8        8           
  Lines        4624     4624           
=======================================
  Hits         4520     4520           
  Misses        104      104           
Flag Coverage Δ
unittests 97.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@acgetchell acgetchell enabled auto-merge July 11, 2026 19:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@justfile`:
- Around line 352-361: Update the clippy-all-targets and clippy-exact recipes to
pass -D warnings to cargo clippy, ensuring every configured Clippy invocation
fails on warnings while preserving the existing workspace, feature, and target
arguments.
🪄 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: CHILL

Plan: Pro

Run ID: b2fc8cab-7e0e-476b-9572-b0a83deecf63

📥 Commits

Reviewing files that changed from the base of the PR and between 2ffa19c and b9e453e.

📒 Files selected for processing (12)
  • .coderabbit.yaml
  • .github/workflows/rust-clippy.yml
  • AGENTS.md
  • Cargo.toml
  • benches/common/bench_utils.rs
  • benches/common/exact.rs
  • benches/exact.rs
  • benches/vs_linalg.rs
  • clippy.toml
  • justfile
  • src/scaled_product.rs
  • tests/exact_bench_config.rs
💤 Files with no reviewable changes (1)
  • src/scaled_product.rs

Comment thread justfile
@acgetchell acgetchell merged commit 701032a into main Jul 11, 2026
20 checks passed
@acgetchell acgetchell deleted the refactor/165-bench-or-abort branch July 11, 2026 19:31
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.

refactor(bench): consolidate setup unwrap helpers behind a postfix OrAbort trait

1 participant