Slipstream/auto_review
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases 27 Packages Activity Actions

fix(review): approve clean re-reviews #53

Merged
jwilger merged 3 commits from fix/clean-rereview-approval into main 2026-05-03 09:27:38 -07:00
Conversation 3 Commits 3 Files changed 3 +32 -4
jwilger commented 2026-05-03 09:19:03 -07:00
Owner
Copy link

Summary

  • Map clean review outputs with no findings to Forgejo APPROVED reviews so clean re-runs supersede stale Auto Review REQUEST_CHANGES records.
  • Keep error findings as REQUEST_CHANGES and warning/note-only findings as COMMENT.
  • Document the fix in the changelog for issue #52.

Verification

  • nix develop --command cargo fmt --all -- --check
  • nix develop --command cargo nextest run -p ar-review
  • nix develop --command cargo clippy --workspace --all-targets -- -D warnings

Closes #52.

## Summary - Map clean review outputs with no findings to Forgejo APPROVED reviews so clean re-runs supersede stale Auto Review REQUEST_CHANGES records. - Keep error findings as REQUEST_CHANGES and warning/note-only findings as COMMENT. - Document the fix in the changelog for issue #52. ## Verification - nix develop --command cargo fmt --all -- --check - nix develop --command cargo nextest run -p ar-review - nix develop --command cargo clippy --workspace --all-targets -- -D warnings Closes #52.
fix(review): approve clean re-reviews
Some checks failed
auto_review auto_review: no findings
CI / Nix flake check (pull_request) Has been cancelled
Details
2469d3c08d
auto-review requested changes 2026-05-03 09:19:43 -07:00
Dismissed
auto-review left a comment
Copy link

This PR updates the review mapping logic to post an Approved review for clean Auto Review runs, addressing issue #52. The changes appear well-documented and tested, making them safe to merge.

Walkthrough

  • CHANGELOG.md:
    • Added an entry documenting the new behavior where clean Auto Review runs post an Approved review.
  • crates/ar-review/src/mapping.rs:
    • Modified the output_to_review_request function to map clean review outputs to Approved reviews.
    • Updated tests to reflect the new behavior, ensuring that clean outputs result in an Approved review.

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
Linters
  • gitleaks — ok, 8 findings
  • semgrep — ok, 0 findings
  • trivy — ok, 0 findings
  • osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
  • ast-grep — ok, 0 findings
  • typos — ok, 25 findings
This PR updates the review mapping logic to post an `Approved` review for clean Auto Review runs, addressing issue #52. The changes appear well-documented and tested, making them safe to merge. ## Walkthrough - **CHANGELOG.md**: - Added an entry documenting the new behavior where clean Auto Review runs post an `Approved` review. - **crates/ar-review/src/mapping.rs**: - Modified the `output_to_review_request` function to map clean review outputs to `Approved` reviews. - Updated tests to reflect the new behavior, ensuring that clean outputs result in an `Approved` review. ## 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 <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>
test(review): cover clean approval mapping
All checks were successful
auto_review auto_review: 1 warning
CI / Nix flake check (pull_request) Successful in 2m3s
Details
0fbb6de1d9
auto-review reviewed 2026-05-03 09:22:10 -07:00
auto-review left a comment
Copy link

This PR updates the review mapping logic to post an Approved review for clean re-reviews, addressing issue #52. The changes appear well-structured and include necessary test updates, but ensure the logic aligns with the intended behavior.

Walkthrough

  • CHANGELOG.md:
    • Added an entry documenting the change in review behavior for clean outputs.
  • crates/ar-review/src/mapping.rs:
    • Modified the logic to map clean review outputs to Approved events instead of Comment.
    • Updated test names and expectations to reflect this change.
  • crates/ar-review/tests/review_mapping.rs:
    • Added a new test to verify that clean review outputs result in an Approved review event.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
Linters
  • gitleaks — ok, 8 findings
  • semgrep — ok, 0 findings
  • trivy — ok, 0 findings
  • osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
  • ast-grep — ok, 0 findings
  • typos — ok, 25 findings
This PR updates the review mapping logic to post an `Approved` review for clean re-reviews, addressing issue #52. The changes appear well-structured and include necessary test updates, but ensure the logic aligns with the intended behavior. ## Walkthrough - **CHANGELOG.md**: - Added an entry documenting the change in review behavior for clean outputs. - **crates/ar-review/src/mapping.rs**: - Modified the logic to map clean review outputs to `Approved` events instead of `Comment`. - Updated test names and expectations to reflect this change. - **crates/ar-review/tests/review_mapping.rs**: - Added a new test to verify that clean review outputs result in an `Approved` review event. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>
crates/ar-review/src/mapping.rs Outdated
@ -12,4 +12,4 @@
/// - Each finding becomes one inline `ReviewComment` anchored at
/// `new_position = line_start`. Multi-line ranges are rendered as a
/// `**Lines N–M:**` prefix in the body since Forgejo's per-line position
/// schema doesn't carry an end line.
auto-review commented 2026-05-03 09:22:10 -07:00
Owner
Copy link

🟡 Warning: The logic for determining the review event has changed. Ensure that this logic aligns with the intended behavior for handling clean reviews.

🟡 **Warning:** The logic for determining the review event has changed. Ensure that this logic aligns with the intended behavior for handling clean reviews.
jwilger commented 2026-05-03 09:24:06 -07:00
Author
Owner
Copy link

Addressed by documenting the intent directly at the event-mapping seam: clean outputs intentionally post Approved because Forgejo branch protection treats that as superseding this bot's stale RequestChanges after a clean re-review. Focused verification passed: nix develop --command cargo nextest run -p ar-review clean_review_output_posts_approval any_error_severity_promotes_to_request_changes only_warnings_and_notes_stay_comment.

Addressed by documenting the intent directly at the event-mapping seam: clean outputs intentionally post `Approved` because Forgejo branch protection treats that as superseding this bot's stale `RequestChanges` after a clean re-review. Focused verification passed: `nix develop --command cargo nextest run -p ar-review clean_review_output_posts_approval any_error_severity_promotes_to_request_changes only_warnings_and_notes_stay_comment`.
jwilger marked this conversation as resolved
docs(review): explain clean approval intent
All checks were successful
auto_review auto_review: no findings
CI / Nix flake check (pull_request) Successful in 1m59s
Details
f879a80e47
auto-review approved these changes 2026-05-03 09:24:30 -07:00
auto-review left a comment
Copy link

This PR updates the review mapping logic to post an Approved review for clean re-reviews, addressing issue #52. The changes are well-documented and tested, ensuring that clean re-reviews can supersede stale RequestChanges reviews. The PR appears safe to merge.

Walkthrough

  • CHANGELOG.md:
    • Added an entry documenting the new behavior of posting an Approved review for clean re-reviews, which addresses issue #52.
  • crates/ar-review/src/mapping.rs:
    • Updated the output_to_review_request function to post an Approved review when there are no findings, ensuring clean re-reviews supersede stale RequestChanges reviews.
    • Modified the test clean_output_is_an_approved_review_to_supersede_stale_request_changes to verify the new behavior.
  • crates/ar-review/tests/review_mapping.rs:
    • Added a new test clean_review_output_posts_approval to ensure that clean review outputs result in an Approved review event.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
Linters
  • gitleaks — ok, 8 findings
  • semgrep — ok, 0 findings
  • trivy — ok, 0 findings
  • osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings
  • ast-grep — ok, 0 findings
  • typos — ok, 25 findings
This PR updates the review mapping logic to post an `Approved` review for clean re-reviews, addressing issue #52. The changes are well-documented and tested, ensuring that clean re-reviews can supersede stale `RequestChanges` reviews. The PR appears safe to merge. ## Walkthrough - **CHANGELOG.md**: - Added an entry documenting the new behavior of posting an `Approved` review for clean re-reviews, which addresses issue #52. - **crates/ar-review/src/mapping.rs**: - Updated the `output_to_review_request` function to post an `Approved` review when there are no findings, ensuring clean re-reviews supersede stale `RequestChanges` reviews. - Modified the test `clean_output_is_an_approved_review_to_supersede_stale_request_changes` to verify the new behavior. - **crates/ar-review/tests/review_mapping.rs**: - Added a new test `clean_review_output_posts_approval` to ensure that clean review outputs result in an `Approved` review event. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers <details> <summary>Linters</summary> - gitleaks — ok, 8 findings - semgrep — ok, 0 findings - trivy — ok, 0 findings - osv-scanner — skipped: osv-scanner not found in sandbox image, 0 findings - ast-grep — ok, 0 findings - typos — ok, 25 findings </details>
jwilger deleted branch fix/clean-rereview-approval 2026-05-03 09:27:38 -07:00
jwilger referenced this pull request from a commit 2026-05-05 12:18:07 -07:00
chore: release v0.1.0 (#94)
Sign in to join this conversation.
Reviewers
No reviewers
auto-review
Labels
Clear labels   
bug

   
discussion

   
enhancement
  
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
discussion
enhancement
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
Slipstream/auto_review!53
No description provided.