fix(review): accept repo-scoped review corrections #204

Merged

jwilger merged 6 commits from process-pilens-deferred-format-guardrail into main 2026-05-15 05:25:33 -07:00

Conversation 5 Commits 6 Files changed 10 +1237 -819

jwilger commented 2026-05-14 17:16:26 -07:00

Owner

Copy link

Summary

• add the pi-lens deferred-finalize guardrail: after write/edit work, wait for the follow-up turn, re-check status/diff, and rerun relevant verification before committing, pushing, or opening/updating PRs
• add versioned safe_commit and safe_push Pi tools that stage only explicit files, preserve hooks/signing, validate Forgejo push destinations, reject main pushes, and block unsafe push URLs/pathspecs/pre-staged commits
• deny direct Pi bash execution by default and guide agents to add or modify reviewed typed Pi tools under RGR/tests/review for missing executable capabilities
• add a versioned lefthook.yml pre-commit hook that runs full nix flake check, expose lefthook from the Nix dev shell, auto-install hooks from nix develop, and have the Pi session-start guardrail ensure lefthook install is run
• accept valid unscoped conventional PR titles such as docs: apply threat model markdown formatting when the PR body has substantive Summary plus Verification/Why content, even if the cheap-tier metadata checker false-negatives them
• update the cheap-tier PR metadata prompt so conventional commit scope is optional and includes both scoped and unscoped examples
• recognize Forgejo quote-attribution corrections to any auto-review comment/finding, store the correction as repository-scoped learning for the current repo, and post an approving review on the current PR head

Verification

• git diff --check origin/main...HEAD
• nix develop -c cargo nextest run -p ar-review pr_metadata
• nix develop -c cargo nextest run -p ar-chat
• nix develop -c cargo fmt --all -- --check
• nix develop -c cargo clippy -p ar-chat -p ar-review --all-targets -- -D warnings
• nix develop -c cargo clippy -p ar-chat --all-targets -- -D warnings
• node tests/release_tooling/pi_guardrails_contract_test.mjs
• GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=commit.gpgSign GIT_CONFIG_VALUE_0=false tests/release_tooling/release_script_flake_test.sh test_pi_guardrails_route_git_commit_and_push_through_safe_tools
• GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=commit.gpgSign GIT_CONFIG_VALUE_0=false tests/release_tooling/release_script_flake_test.sh test_pi_guardrails_deny_bash_and_route_capabilities_through_reviewed_tools
• pre-commit lefthook ran nix flake check successfully while creating commits 04e898c, 606f3db, and c3dd58b

## Summary - add the pi-lens deferred-finalize guardrail: after write/edit work, wait for the follow-up turn, re-check status/diff, and rerun relevant verification before committing, pushing, or opening/updating PRs - add versioned `safe_commit` and `safe_push` Pi tools that stage only explicit files, preserve hooks/signing, validate Forgejo push destinations, reject main pushes, and block unsafe push URLs/pathspecs/pre-staged commits - deny direct Pi `bash` execution by default and guide agents to add or modify reviewed typed Pi tools under RGR/tests/review for missing executable capabilities - add a versioned `lefthook.yml` pre-commit hook that runs full `nix flake check`, expose lefthook from the Nix dev shell, auto-install hooks from `nix develop`, and have the Pi session-start guardrail ensure `lefthook install` is run - accept valid unscoped conventional PR titles such as `docs: apply threat model markdown formatting` when the PR body has substantive `Summary` plus `Verification`/`Why` content, even if the cheap-tier metadata checker false-negatives them - update the cheap-tier PR metadata prompt so conventional commit scope is optional and includes both scoped and unscoped examples - recognize Forgejo quote-attribution corrections to any auto-review comment/finding, store the correction as repository-scoped learning for the current repo, and post an approving review on the current PR head ## Verification - `git diff --check origin/main...HEAD` - `nix develop -c cargo nextest run -p ar-review pr_metadata` - `nix develop -c cargo nextest run -p ar-chat` - `nix develop -c cargo fmt --all -- --check` - `nix develop -c cargo clippy -p ar-chat -p ar-review --all-targets -- -D warnings` - `nix develop -c cargo clippy -p ar-chat --all-targets -- -D warnings` - `node tests/release_tooling/pi_guardrails_contract_test.mjs` - `GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=commit.gpgSign GIT_CONFIG_VALUE_0=false tests/release_tooling/release_script_flake_test.sh test_pi_guardrails_route_git_commit_and_push_through_safe_tools` - `GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=commit.gpgSign GIT_CONFIG_VALUE_0=false tests/release_tooling/release_script_flake_test.sh test_pi_guardrails_deny_bash_and_route_capabilities_through_reviewed_tools` - pre-commit lefthook ran `nix flake check` successfully while creating commits `04e898c`, `606f3db`, and `c3dd58b`

jwilger added 1 commit 2026-05-14 17:16:26 -07:00

docs: add pi-lens finalize guardrail edd0822998

auto-review approved these changes 2026-05-14 17:16:56 -07:00

Dismissed

auto-review left a comment

Copy link

The PR introduces a new guardrail for pi-lens deferred formatting/autofix, requiring a follow-up turn after write/edit work. This change aims to ensure that the post-hook working tree is reviewed and verified before any commit, push, or PR creation.

Walkthrough

• .pi/APPEND_SYSTEM.md:
  • Added a new guideline for when pi-lens is enabled, requiring a follow-up turn to recheck toolchain_status, review the post-hook diff, and rerun relevant verification before proceeding with commit, push, or PR creation.

The PR introduces a new guardrail for pi-lens deferred formatting/autofix, requiring a follow-up turn after write/edit work. This change aims to ensure that the post-hook working tree is reviewed and verified before any commit, push, or PR creation. ## Walkthrough - **.pi/APPEND_SYSTEM.md**: - Added a new guideline for when pi-lens is enabled, requiring a follow-up turn to recheck `toolchain_status`, review the post-hook diff, and rerun relevant verification before proceeding with commit, push, or PR creation.

jwilger added 1 commit 2026-05-14 19:17:14 -07:00

fix(review): accept metadata corrections ... 9ebed528fe

Accept unscoped conventional PR titles with substantive metadata, teach the metadata prompt that scope is optional, and handle user corrections by remembering the feedback and posting an approving review.

jwilger dismissed auto-review's review 2026-05-14 19:17:14 -07:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

jwilger changed title from docs: add pi-lens finalize guardrail to fix(review): accept metadata corrections and pi-lens guardrail 2026-05-14 19:18:03 -07:00

auto-review approved these changes 2026-05-14 19:21:15 -07:00

Dismissed

auto-review left a comment

Copy link

This PR introduces a new feature to handle user corrections for metadata review failures, allowing unscoped conventional commit titles if the PR body is substantive. It also updates the metadata validation logic and adds relevant tests. The changes appear well-integrated and safe to merge.

Walkthrough

Δ since edd0822:

• Command Handling
  • Added ReviewCorrection command to handle user corrections for metadata review failures.
  • Introduced parse_review_correction function to extract corrections from user comments.
• Handler Logic
  • Implemented handle_review_correction to update learning store and post approval reviews.
• Testing
  • Added tests to verify the new review correction handling logic.
• Metadata Validation
  • Updated validation to accept unscoped conventional commit titles with substantive PR bodies.

This PR introduces a new feature to handle user corrections for metadata review failures, allowing unscoped conventional commit titles if the PR body is substantive. It also updates the metadata validation logic and adds relevant tests. The changes appear well-integrated and safe to merge. ## Walkthrough ### Δ since edd0822: - **Command Handling** - Added `ReviewCorrection` command to handle user corrections for metadata review failures. - Introduced `parse_review_correction` function to extract corrections from user comments. - **Handler Logic** - Implemented `handle_review_correction` to update learning store and post approval reviews. - **Testing** - Added tests to verify the new review correction handling logic. - **Metadata Validation** - Updated validation to accept unscoped conventional commit titles with substantive PR bodies.

jwilger added 1 commit 2026-05-14 19:39:26 -07:00

fix(chat): generalize review corrections ... 6cbb2710f6

Treat quoted corrections to any auto-review finding as repository-specific learnings, instead of limiting correction handling to PR metadata failures.

jwilger dismissed auto-review's review 2026-05-14 19:39:26 -07:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

jwilger changed title from fix(review): accept metadata corrections and pi-lens guardrail to fix(review): accept repo-scoped review corrections 2026-05-14 19:39:39 -07:00

jwilger added 1 commit 2026-05-14 20:47:26 -07:00

fix(ci): format review correction test ... cfe55bd4c1

Apply rustfmt's wrapping for the new review correction regression test so the flake cargo-fmt check passes.

auto-review approved these changes 2026-05-14 20:51:36 -07:00

Dismissed

auto-review left a comment

Copy link

The PR expands the handling of review corrections to include repository-specific context and broader correction keywords. It appears safe to merge, but ensure that the changes align with the intended functionality and documentation.

Walkthrough

Δ since 9ebed52:

• Command Parsing: Expanded the ReviewCorrection command to handle broader cases, removing specific checks for metadata failures.
• Handler Changes: Updated embedding text to include repository-specific context, which may affect how corrections are stored and retrieved.
• Testing: Added tests for new correction scenarios, ensuring that corrections to review findings are correctly parsed and handled.

The PR expands the handling of review corrections to include repository-specific context and broader correction keywords. It appears safe to merge, but ensure that the changes align with the intended functionality and documentation. ## Walkthrough ### Δ since 9ebed52: - **Command Parsing**: Expanded the `ReviewCorrection` command to handle broader cases, removing specific checks for metadata failures. - **Handler Changes**: Updated embedding text to include repository-specific context, which may affect how corrections are stored and retrieved. - **Testing**: Added tests for new correction scenarios, ensuring that corrections to review findings are correctly parsed and handled.

jwilger added 1 commit 2026-05-14 21:03:40 -07:00

chore(hooks): enforce lefthook verification ... 04e898c764

Install lefthook from the Nix dev shell and Pi session start, and run full nix flake verification from the versioned pre-commit hook.

jwilger dismissed auto-review's review 2026-05-14 21:03:40 -07:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

auto-review approved these changes 2026-05-14 21:07:56 -07:00

Dismissed

auto-review left a comment

Copy link

The PR introduces a deferred-finalize guardrail for pi-lens, integrates lefthook into the development workflow, and updates PR title validation to accept unscoped conventional titles. It also enhances the handling of review corrections by storing them as repository-specific learnings. The changes appear safe to merge.

Walkthrough

Δ since cfe55bd:

• auto-review-guardrails.ts:
  • Added a function to ensure lefthook is installed at session start, aligning Pi sessions with the dev shell setup.
• flake.nix:
  • Included lefthook in the development environment and ensured its installation during nix develop.
• lefthook.yml:
  • Introduced a pre-commit hook to run nix flake check.
• release_script_flake_test.sh:
  • Added a test to verify that nix develop enables the versioned lefthook pre-commit format guardrail.

The PR introduces a deferred-finalize guardrail for pi-lens, integrates `lefthook` into the development workflow, and updates PR title validation to accept unscoped conventional titles. It also enhances the handling of review corrections by storing them as repository-specific learnings. The changes appear safe to merge. ## Walkthrough ### Δ since cfe55bd: - **auto-review-guardrails.ts**: - Added a function to ensure `lefthook` is installed at session start, aligning Pi sessions with the dev shell setup. - **flake.nix**: - Included `lefthook` in the development environment and ensured its installation during `nix develop`. - **lefthook.yml**: - Introduced a pre-commit hook to run `nix flake check`. - **release_script_flake_test.sh**: - Added a test to verify that `nix develop` enables the versioned `lefthook` pre-commit format guardrail.

jwilger added 1 commit 2026-05-14 21:31:02 -07:00

chore(nix): preserve formatted flake ... 7c1f3f97a9

Keep the formatter-produced flake layout and make the ar-cli flake contract whitespace-tolerant so the protected branch can receive the change as a normal follow-up commit.

jwilger dismissed auto-review's review 2026-05-14 21:31:02 -07:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

auto-review approved these changes 2026-05-14 21:34:55 -07:00

auto-review left a comment

Copy link

The changes primarily involve refactoring and reformatting the flake.nix file to improve readability and maintainability. The modifications appear safe to merge as they do not alter the core functionality.

Walkthrough

Δ since 04e898c:

• crates/ar-cli/src/cli.rs:
  • Normalized whitespace in a string comparison to ensure consistent formatting.
• flake.nix:
  • Reformatted the file for better readability by adjusting indentation and line breaks.
  • No functional changes were made, ensuring that the behavior remains consistent.

The changes primarily involve refactoring and reformatting the `flake.nix` file to improve readability and maintainability. The modifications appear safe to merge as they do not alter the core functionality. ## Walkthrough ### Δ since 04e898c: - **crates/ar-cli/src/cli.rs**: - Normalized whitespace in a string comparison to ensure consistent formatting. - **flake.nix**: - Reformatted the file for better readability by adjusting indentation and line breaks. - No functional changes were made, ensuring that the behavior remains consistent.

jwilger merged commit 573cefcadd into main 2026-05-15 05:25:33 -07:00

jwilger referenced this pull request from a commit 2026-05-15 05:25:33 -07:00

fix(review): accept repo-scoped review corrections (#204)

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

Subscribe

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!204

No description provided.