Skip to content

Steer PR review to the search tools, not shell grep#29

Merged
gobetti merged 1 commit into
mainfrom
chore/steer-review-to-search-tools
Jul 6, 2026
Merged

Steer PR review to the search tools, not shell grep#29
gobetti merged 1 commit into
mainfrom
chore/steer-review-to-search-tools

Conversation

@gobetti

@gobetti gobetti commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

The review allow-list permits bash only for git log/git diff/git show, but Sonnet 5 still reaches for grep -rn, find, cat, and python when verifying a finding across files. Each of those is denied and costs a turn plus a retry. On a repo with a tight max_turns the waste is enough to exhaust the budget mid-review on even a 4-file PR — the run dies with error_max_turns and posts no review at all.

Tell the model up front that bash is git-only and to search and read with the Read/Grep/Glob tools, reviewing primarily from the diff and opening extra files only when a finding needs cross-file confirmation. Removes the denial class and curbs over-exploration without raising any cap. Follow-up to 8fe29fd.

Evidence: reproducing a failing 4-file ht-crm review (max_turns: 20) with the same restricted tool-set showed Sonnet 5 making exactly 3 non-git shell attempts (grep -rn --include, a find, and grep -n) — matching that run's permission_denials_count: 3. The review content itself was trivially small (a clean LGTM), so the budget was lost to denied-shell retries, not review volume.

Ticket

N/A — ad hoc infra fix, not tied to a ticket.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing Done

Changes only the shared review workflow's prompt (no functional or schema change). Validated by reproducing the ht-crm review locally with the restricted tool-set: without steering, Sonnet 5 spends turns on denied shell grep/find/python calls; the added guidance directs it to the Read/Grep/Glob tools instead. Will confirm on live PR reviews across repos by watching turn and denial counts.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated documentation as needed

The review allow-list permits bash only for git log/diff/show, but
Sonnet 5 still reaches for `grep -rn`, `find`, `cat`, and `python`
when verifying a finding across files. Each is denied and costs a
turn plus a retry. On a repo with a tight max_turns the waste is
enough to exhaust the budget mid-review on even a 4-file PR — the
run dies with error_max_turns and posts no review at all.

Tell the model up front that bash is git-only and to search and
read with the Read/Grep/Glob tools, reviewing primarily from the
diff and opening extra files only when a finding needs cross-file
confirmation. Removes the denial class and curbs over-exploration
without raising any cap. Follow-up to 8fe29fd.
@gobetti gobetti force-pushed the chore/steer-review-to-search-tools branch from dec0468 to 01343f1 Compare July 6, 2026 19:39

@jaysonv0341 jaysonv0341 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.

Hopefully this helps reduce the number of turns. LGTM

@gobetti gobetti merged commit c313b7c into main Jul 6, 2026
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.

2 participants