the shadow reviewer reads a PR body that a rework round can never update #450

Closed
opened 2026-07-28 18:09:15 +00:00 by igor · 1 comment
Collaborator

Two things I hit on #449 that look like the same gap.

review_build_prompt (lib/review.sh) hands the model the PR title, the PR
description, and the diff. In rework mode I push commits to the branch and
AGENTS.md tells me not to write .agent/PR_BODY.md, so the description stays
frozen at whatever I wrote when the PR opened.

On #449 that body said, accurately at the time, that the scan glue had no test
coverage. Round 2 quoted the line back at me as a blocking finding. I have since
covered it, but the body still says the opposite, so round 3 gets a description
arguing against its own diff. The diff should win, and probably will. It is
still a prompt pointed at the wrong thing.

Second: round 2's third blocking item asked me to land part of the change as a
stacked PR first. I cannot. Rework mode is explicitly "stay on this branch, do
not open a new PR". So a scope finding on an already-open PR has no path to
resolution except the 3-round escalation, and a round gets spent on a request I
was never able to act on.

Three options, smallest first:

  1. Let a rework run replace the description. A .agent/PR_BODY.md written
    during rework updates the body instead of being ignored. Fixes the stale
    prompt, at the cost of a bot overwriting anything you edited by hand.
  2. Add the rework commit messages to the reviewer prompt. Right now they are the
    only place my answer to a finding lives, and nothing reads them.
  3. Route scope findings on an open PR straight to you instead of into a rework
    round, since splitting is not something rework can do.

No strong preference. Mostly I want to know whether the frozen body is a known
tradeoff before I start working around it.

<!-- agent:question --> Two things I hit on #449 that look like the same gap. `review_build_prompt` (lib/review.sh) hands the model the PR title, the PR description, and the diff. In rework mode I push commits to the branch and AGENTS.md tells me not to write `.agent/PR_BODY.md`, so the description stays frozen at whatever I wrote when the PR opened. On #449 that body said, accurately at the time, that the scan glue had no test coverage. Round 2 quoted the line back at me as a blocking finding. I have since covered it, but the body still says the opposite, so round 3 gets a description arguing against its own diff. The diff should win, and probably will. It is still a prompt pointed at the wrong thing. Second: round 2's third blocking item asked me to land part of the change as a stacked PR first. I cannot. Rework mode is explicitly "stay on this branch, do not open a new PR". So a scope finding on an already-open PR has no path to resolution except the 3-round escalation, and a round gets spent on a request I was never able to act on. Three options, smallest first: 1. Let a rework run replace the description. A `.agent/PR_BODY.md` written during rework updates the body instead of being ignored. Fixes the stale prompt, at the cost of a bot overwriting anything you edited by hand. 2. Add the rework commit messages to the reviewer prompt. Right now they are the only place my answer to a finding lives, and nothing reads them. 3. Route scope findings on an open PR straight to you instead of into a rework round, since splitting is not something rework can do. No strong preference. Mostly I want to know whether the frozen body is a known tradeoff before I start working around it.
Author
Collaborator

Answered — closing without a change.

Yes, the frozen PR body is a known tradeoff. The diff is ground truth; treat the description as what the author believed when the PR opened, not as a current claim. Do not work around it, and do not let a stale line in the body override what the diff shows.

A scope finding on an already-open PR is not actionable in rework, and should not be treated as one. Rework is "stay on this branch" by design, so "split this into stacked PRs" has no path to resolution — and answering it by adding code makes the PR bigger, which is the opposite of what was asked. #449 went from 85 to 627 lines across three rounds under exactly that pressure. If a scope finding comes back, say so plainly and stop; do not grow the diff to satisfy it.

Why no fix is being built. All three proposed options modify the review loop, which has been changed three times in two days, and every harness fix this week has spawned the next one (#438#442#444#445; #441#447 + #448; #439#443#449 → this). The gap is also downstream of over-scoping: it only bites on PRs that go multiple rework rounds. Thinner tickets and one open PR per repo remove most of it without touching lib/review.sh.

Good question, and the diagnosis was right — the answer is that it is accepted, not that it is unnoticed.

Answered — closing without a change. **Yes, the frozen PR body is a known tradeoff.** The diff is ground truth; treat the description as what the author believed when the PR opened, not as a current claim. Do not work around it, and do not let a stale line in the body override what the diff shows. **A scope finding on an already-open PR is not actionable in rework, and should not be treated as one.** Rework is "stay on this branch" by design, so "split this into stacked PRs" has no path to resolution — and answering it by adding code makes the PR bigger, which is the opposite of what was asked. #449 went from 85 to 627 lines across three rounds under exactly that pressure. If a scope finding comes back, say so plainly and stop; do not grow the diff to satisfy it. **Why no fix is being built.** All three proposed options modify the review loop, which has been changed three times in two days, and every harness fix this week has spawned the next one (#438 → #442 → #444 → #445; #441 → #447 + #448; #439 → #443 → #449 → this). The gap is also downstream of over-scoping: it only bites on PRs that go multiple rework rounds. Thinner tickets and one open PR per repo remove most of it without touching `lib/review.sh`. Good question, and the diagnosis was right — the answer is that it is accepted, not that it is unnoticed.
igor closed this issue 2026-07-28 22:54:59 +00:00
Sign in to join this conversation.
No milestone
No assignees
1 participant
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
joshtronic/igor#450
No description provided.