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

fix(review): handle imperative metadata and quoted feedback #279

Merged
jwilger merged 5 commits from fix/metadata-feedback-intent into main 2026-05-19 21:16:28 -07:00
Conversation 1 Commits 5 Files changed 8 +507 -6
jwilger commented 2026-05-19 21:14:12 -07:00
Owner
Copy link

Why:

  • Valid imperative PR metadata such as Add auto_review CI trigger was blocked when the Cheap metadata gate returned a false negative.
  • Quoted natural-language corrections and approval requests were falling through to generic freeform replies instead of acknowledging the user's feedback.
  • Rust RED tests can live inside source files under #[cfg(test)], so the RGR guardrail needed a scoped test-author path.

What:

  • Add a delegated, scoped Rust in-source test-author lease for RGR plugin workflows.
  • Accept imperative PR titles with substantive Summary and Verification/Why metadata in the deterministic metadata fallback.
  • Route quoted approval/correction feedback through review correction with best-effort structured Cheap-tier classification.
  • Add regression coverage for the observed metadata and chat feedback bugs.

Validation:

  • just opencode-test
  • just fmt
  • just clippy
  • cargo nextest run -p ar-review -p ar-chat

Fixes #273
Fixes #274

Why: - Valid imperative PR metadata such as `Add auto_review CI trigger` was blocked when the Cheap metadata gate returned a false negative. - Quoted natural-language corrections and approval requests were falling through to generic freeform replies instead of acknowledging the user's feedback. - Rust RED tests can live inside source files under `#[cfg(test)]`, so the RGR guardrail needed a scoped test-author path. What: - Add a delegated, scoped Rust in-source test-author lease for RGR plugin workflows. - Accept imperative PR titles with substantive Summary and Verification/Why metadata in the deterministic metadata fallback. - Route quoted approval/correction feedback through review correction with best-effort structured Cheap-tier classification. - Add regression coverage for the observed metadata and chat feedback bugs. Validation: - just opencode-test - just fmt - just clippy - cargo nextest run -p ar-review -p ar-chat Fixes #273 Fixes #274
Why:
- Rust RED tests can live inside #[cfg(test)] modules in source files, so path-only production gates blocked valid test-author work.

What:
- Add a scoped rgr-test-author lease for one in-source Rust unit test file.
- Require reviewer/test-author guidance to distinguish test-only Rust source edits from production edits.

Validation:
- just opencode-test
Why:
- Valid imperative PR titles with substantive descriptions were blocked when the Cheap metadata check returned a false negative.
- This incorrectly requested changes for PRs such as 'Add auto_review CI trigger'.

What:
- Extend the deterministic clearly-acceptable metadata fallback to cover imperative titles with substantive Summary and Verification/Why content.
- Add a regression test for the observed false-positive shape.

Validation:
- cargo nextest run -p ar-review imperative_pr_metadata_does_not_request_changes_when_cheap_model_over_blocks
Why:
- Quoted natural-language corrections and approval requests were falling through to generic freeform replies, losing the user's actual feedback.
- The observed approval request should acknowledge review feedback and keep the approval path instead of summarizing the diff.

What:
- Route approval-oriented quoted feedback to the review-correction flow.
- Add a structured Cheap-tier feedback classification call that is best-effort and non-fatal.
- Cover structured classification and failure fallback with focused chat tests.

Validation:
- rtk cargo test -p ar-chat --test review_correction quoted_feedback_with_approval_request_uses_structured_cheap_classification_and_approval_path -- --exact
- rtk cargo test -p ar-chat --test review_correction cheap_classification_failure_still_stores_learning_and_posts_approval -- --exact
- cargo nextest run -p ar-chat review_correction
Why:
- Rust formatting must remain clean after the bug-fix commits.

What:
- Apply cargo fmt output to the metadata and chat regression changes.

Validation:
- just fmt
- cargo nextest run -p ar-review -p ar-chat
test(chat): release classification request lock before await
All checks were successful
CI / Classify changed paths (pull_request) Successful in 1s
Details
CI / Format check (pull_request) Successful in 6s
Details
CI / Clippy (pull_request) Successful in 40s
Details
CI / Test (pull_request) Successful in 54s
Details
CI / Build (pull_request) Successful in 31s
Details
CI / opencode plugin tests (pull_request) Successful in 14s
Details
CI / Dependency policy (pull_request) Successful in 9s
Details
CI / Request auto_review semantic review (pull_request) Successful in 1s
Details
auto_review auto_review: no findings
CI / Build PR artifacts (no token) (pull_request) Successful in 2s
Details
46fe34ad83
 
Why:
- The pre-push clippy gate rejects holding a MutexGuard across an await in the chat regression test.

What:
- Clone the captured response format while the lock is held, then drop the guard before awaiting Forgejo request inspection.

Validation:
- just fmt
- just clippy
auto-review approved these changes 2026-05-19 21:15:37 -07:00
auto-review left a comment
Copy link

The PR addresses issues with handling imperative metadata and quoted feedback in the review process. It introduces a scoped test-author lease for Rust in-source tests and improves the classification of quoted feedback. The changes appear safe to merge.

Walkthrough

  • .opencode/agents/rgr-test-author.md & rgr-test-reviewer.md: Updated to clarify the scope of Rust in-source tests.
  • .opencode/plugin-tests/auto-review-discipline-rgr.test.ts: Added tests to ensure the correct handling of Rust in-source test author leases.
  • .opencode/plugins/auto-review-discipline.ts: Introduced logic to manage test-author leases and ensure edits are scoped correctly.
  • crates/ar-chat/src/command.rs & handler.rs: Enhanced feedback classification to better handle quoted approval requests.
  • crates/ar-review/src/pipeline.rs: Improved metadata validation to accept imperative PR titles with substantive content.

LLM usage and cost

The PR addresses issues with handling imperative metadata and quoted feedback in the review process. It introduces a scoped test-author lease for Rust in-source tests and improves the classification of quoted feedback. The changes appear safe to merge. ## Walkthrough - **.opencode/agents/rgr-test-author.md & rgr-test-reviewer.md**: Updated to clarify the scope of Rust in-source tests. - **.opencode/plugin-tests/auto-review-discipline-rgr.test.ts**: Added tests to ensure the correct handling of Rust in-source test author leases. - **.opencode/plugins/auto-review-discipline.ts**: Introduced logic to manage test-author leases and ensure edits are scoped correctly. - **crates/ar-chat/src/command.rs & handler.rs**: Enhanced feedback classification to better handle quoted approval requests. - **crates/ar-review/src/pipeline.rs**: Improved metadata validation to accept imperative PR titles with substantive content. ## LLM usage and cost - Reasoning (gpt-4o) in=8618 out=532 cost=$0.051070 - Cheap (gpt-4o-mini) in=460 out=53 cost=$0.000101 Estimated total USD: $0.051171 via https://api.openai.com and https://api.openai.com
jwilger deleted branch fix/metadata-feedback-intent 2026-05-19 21:16:28 -07:00
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!279
No description provided.