fix(review): show linter run summaries in reviews #40

jwilger · 2026-05-02T13:10:58-07:00

jwilger commented

2026-05-02 13:10:58 -07:00

Summary

Add linter run summaries to posted review bodies in a collapsed Linters section, including zero-finding runs, skips, and failures.
Thread linter status from ar-tools through ar-review routing and ar-orchestrator into the review pipeline.
Surface missing host binaries and sandbox-image exit 127 as skipped linter runs while preserving existing finding collection behavior.

RGR evidence

RED:

cargo test -p ar-review pipeline::tests::review_body_includes_linter_summary_section -- --nocapture
missing LinterRunSummary / LinterRunStatus / render_linter_section symbols

Reviewer-driven RED:

cargo test -p ar-tools runner::tests::run_all_with_status_marks_missing_binary_as_skipped -- --nocapture
missing Skipped variants

GREEN / verification:

cargo test -p ar-review pipeline::tests::review_body_includes_linter_summary_section -- --nocapture
cargo test -p ar-tools runner::tests::run_all_with_status_marks_missing_binary_as_skipped -- --nocapture
cargo test -p ar-tools runner::tests::run_all_with_status_marks_sandbox_exit_127_as_skipped -- --nocapture
cargo check -p ar-orchestrator
cargo fmt --all -- --check
git diff --check -- CHANGELOG.md crates/ar-orchestrator/src/dispatcher.rs crates/ar-review/src/lib.rs crates/ar-review/src/pipeline.rs crates/ar-review/src/routing.rs crates/ar-tools/src/lib.rs crates/ar-tools/src/runner.rs

Closes #11

## Summary - Add linter run summaries to posted review bodies in a collapsed Linters section, including zero-finding runs, skips, and failures. - Thread linter status from ar-tools through ar-review routing and ar-orchestrator into the review pipeline. - Surface missing host binaries and sandbox-image exit 127 as skipped linter runs while preserving existing finding collection behavior. ## RGR evidence RED: ``` cargo test -p ar-review pipeline::tests::review_body_includes_linter_summary_section -- --nocapture missing LinterRunSummary / LinterRunStatus / render_linter_section symbols ``` Reviewer-driven RED: ``` cargo test -p ar-tools runner::tests::run_all_with_status_marks_missing_binary_as_skipped -- --nocapture missing Skipped variants ``` GREEN / verification: - cargo test -p ar-review pipeline::tests::review_body_includes_linter_summary_section -- --nocapture - cargo test -p ar-tools runner::tests::run_all_with_status_marks_missing_binary_as_skipped -- --nocapture - cargo test -p ar-tools runner::tests::run_all_with_status_marks_sandbox_exit_127_as_skipped -- --nocapture - cargo check -p ar-orchestrator - cargo fmt --all -- --check - git diff --check -- CHANGELOG.md crates/ar-orchestrator/src/dispatcher.rs crates/ar-review/src/lib.rs crates/ar-review/src/pipeline.rs crates/ar-review/src/routing.rs crates/ar-tools/src/lib.rs crates/ar-tools/src/runner.rs Closes #11

jwilger added 1 commit

2026-05-02 13:10:58 -07:00

fix(review): show linter run summaries in reviews

auto_review auto_review: 2 warnings

CI / Nix flake check (pull_request) Successful in 1m45s

Details

a07947d173

Thread linter execution summaries through routing and orchestration so posted reviews disclose which tools ran, which were skipped, and how many findings each produced.

RED: cargo test -p ar-review pipeline::tests::review_body_includes_linter_summary_section -- --nocapture initially failed with missing linter summary/rendering API; later reviewer-driven RED covered missing binary skip status.

auto-review reviewed

2026-05-02 13:11:16 -07:00

auto-review left a comment

This PR introduces linter run summaries to the review process, enhancing transparency by showing which linters ran, their findings, and any skips or failures. The changes appear well-structured and include necessary tests, but ensure all new fields and functions are consistently integrated across the codebase.

Walkthrough

CHANGELOG.md: Updated to reflect the new feature of linter transparency in review bodies.
crates/ar-orchestrator/src/dispatcher.rs: Modified to thread linter_runs through the review process.
crates/ar-review/src/lib.rs: Exposed new types and functions related to linter run summaries.
crates/ar-review/src/pipeline.rs: Added LinterRunSummary and LinterRunStatus types, and integrated linter summaries into the review body.
crates/ar-review/src/routing.rs: Introduced lint_workspace_report_via to return detailed linter run reports.
crates/ar-tools/src/runner.rs: Enhanced to handle skipped linters and report their status, including new test cases to verify behavior.

Pre-merge checks

CHANGELOG updated — CHANGELOG.md is in the diff
Tests touched — source changed but no test file appears in the diff
No new TODO/FIXME comments — no new TODO/FIXME markers

This PR introduces linter run summaries to the review process, enhancing transparency by showing which linters ran, their findings, and any skips or failures. The changes appear well-structured and include necessary tests, but ensure all new fields and functions are consistently integrated across the codebase. ## Walkthrough - **CHANGELOG.md**: Updated to reflect the new feature of linter transparency in review bodies. - **crates/ar-orchestrator/src/dispatcher.rs**: Modified to thread `linter_runs` through the review process. - **crates/ar-review/src/lib.rs**: Exposed new types and functions related to linter run summaries. - **crates/ar-review/src/pipeline.rs**: Added `LinterRunSummary` and `LinterRunStatus` types, and integrated linter summaries into the review body. - **crates/ar-review/src/routing.rs**: Introduced `lint_workspace_report_via` to return detailed linter run reports. - **crates/ar-tools/src/runner.rs**: Enhanced to handle skipped linters and report their status, including new test cases to verify behavior. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers

crates/ar-review/src/routing.rs

					
				@ -1,6 +1,7 @@

				//! File-extension-based routing from a PR's changed files to the linters

🟡 Warning: The lint_workspace_report_via function now returns a LintWorkspaceReport containing both findings and run summaries. Ensure that this change is reflected in all parts of the code that consume this function's output.

🟡 **Warning:** The `lint_workspace_report_via` function now returns a `LintWorkspaceReport` containing both findings and run summaries. Ensure that this change is reflected in all parts of the code that consume this function's output.

Verified. This is reflected at the only production call site: prepare_and_lint now calls lint_workspace_report_via, destructures both findings and linter_runs, stores both in LintPhaseOutput, and passes linter_runs into ReviewArgs so the review body can render them. Existing lint_workspace_via remains available for callers that only need findings. cargo check -p ar-orchestrator passed after the integration.

Verified. This is reflected at the only production call site: `prepare_and_lint` now calls `lint_workspace_report_via`, destructures both `findings` and `linter_runs`, stores both in `LintPhaseOutput`, and passes `linter_runs` into `ReviewArgs` so the review body can render them. Existing `lint_workspace_via` remains available for callers that only need findings. `cargo check -p ar-orchestrator` passed after the integration.

crates/ar-tools/src/runner.rs

					
				@ -16,6 +16,8 @@ pub enum RunnerError {

				    },

🟡 Warning: The RunnerError enum now includes a Skipped variant. Ensure that all match statements handling RunnerError are updated to handle this new variant appropriately.

🟡 **Warning:** The `RunnerError` enum now includes a `Skipped` variant. Ensure that all match statements handling `RunnerError` are updated to handle this new variant appropriately.

Verified. The new Skipped variant is handled in both batch paths: run_all explicitly swallows RunnerError::Skipped(_) to preserve existing findings-only behavior, while run_all_with_status converts it into LinterRunStatus::Skipped(reason) for the review-body summary. The ar-review routing adapter also maps ar_tools::runner::LinterRunStatus::Skipped into the review pipeline summary type. Focused tests cover both host missing-binary and sandbox exit-127 skipped paths.

Verified. The new `Skipped` variant is handled in both batch paths: `run_all` explicitly swallows `RunnerError::Skipped(_)` to preserve existing findings-only behavior, while `run_all_with_status` converts it into `LinterRunStatus::Skipped(reason)` for the review-body summary. The `ar-review` routing adapter also maps `ar_tools::runner::LinterRunStatus::Skipped` into the review pipeline summary type. Focused tests cover both host missing-binary and sandbox exit-127 skipped paths.

jwilger merged commit 37acfff192 into main

2026-05-02 13:15:01 -07:00

jwilger referenced this pull request from a commit

2026-05-02 13:15:01 -07:00

fix(review): show linter run summaries in reviews (#40)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jwilger/auto_review!40

				`@ -1,6 +1,7 @@`
				`//! File-extension-based routing from a PR's changed files to the linters`

Rows
Columns