Feed the PR reviewer the linked issue + test-runner facts so it stops punting to the human #438

New issue

Closed

opened 2026-07-28 03:00:13 +00:00 by igor · 0 comments

igor commented

2026-07-28 03:00:13 +00:00

Collaborator

The shadow reviewer ends at COMMENT far more often than it should, and COMMENT means a human is asked. Over three days: 30 COMMENT, 18 APPROVE, 11 REQUEST_CHANGES — per PR, 16 finished APPROVE against 30 finishing non-APPROVE. Roughly two thirds of PRs land in the operator's queue.

The reason is usually MISSING CONTEXT, not a defect. Verbatim, from two PRs merged 2026-07-27 after a human confirmed both were fine:

igor#435: "I can't see issue #433's text, so I can't verify the claimed 'requirement 6' is actually what was asked for."
igor#436: "Is bin/test-agent-block.sh actually executed by CI? The Makefile isn't in the diff."

Neither is a code problem. The first is answerable by reading the linked issue; the second by reading bin/check-sync.sh, which globs bin/test-*.sh. Both were settled by a human in about a minute each. The reviewer hedged because it was honest about not knowing — the right instinct with the wrong inputs.

Decision: give the reviewer the context it keeps saying it lacks. Two additions to the review prompt:

The linked issue body. Parse the closing keyword from the PR body (Closes #NN) and include that issue's body. Requirements the PR claims to satisfy are unverifiable without it, and "I can't verify the requirement" is currently a standing reason to punt to the human.
The repo's test-runner facts. At minimum whether the new/changed test files are actually executed — for this repo, that check-sync.sh globs bin/test-*.sh, so a new bin/test-*.sh runs in CI automatically. "Is this test even run" is a recurring question the diff cannot answer.

Scope discipline: this is about giving the reviewer facts it asks for, NOT about loosening its standards. Do not touch the verdict thresholds or tell it to approve more. If it still has a real doubt after being handed the issue and the runner facts, COMMENT remains correct.

Acceptance criteria:

The review prompt includes the body of any issue named by a Closes #NN in the PR body (bounded in size; skip gracefully if it cannot be fetched).
The review prompt states whether changed test files are picked up by the repo's test runner.
A test asserts both are present in the built prompt for a PR that closes an issue.
No change to verdict thresholds or approve criteria.

Why it matters: an operator asked for a loop that codes, reviews, merges, then picks up the next ticket. At a 35% approve rate that loop stalls on a human two thirds of the time. This is the lever that changes the rate.

The shadow reviewer ends at `COMMENT` far more often than it should, and `COMMENT` means a human is asked. Over three days: **30 COMMENT, 18 APPROVE, 11 REQUEST_CHANGES** — per PR, 16 finished APPROVE against 30 finishing non-APPROVE. Roughly two thirds of PRs land in the operator's queue. The reason is usually MISSING CONTEXT, not a defect. Verbatim, from two PRs merged 2026-07-27 after a human confirmed both were fine: - igor#435: *"I can't see issue #433's text, so I can't verify the claimed 'requirement 6' is actually what was asked for."* - igor#436: *"Is `bin/test-agent-block.sh` actually executed by CI? The Makefile isn't in the diff."* Neither is a code problem. The first is answerable by reading the linked issue; the second by reading `bin/check-sync.sh`, which globs `bin/test-*.sh`. Both were settled by a human in about a minute each. The reviewer hedged because it was honest about not knowing — the right instinct with the wrong inputs. **Decision: give the reviewer the context it keeps saying it lacks.** Two additions to the review prompt: 1. **The linked issue body.** Parse the closing keyword from the PR body (`Closes #NN`) and include that issue's body. Requirements the PR claims to satisfy are unverifiable without it, and "I can't verify the requirement" is currently a standing reason to punt to the human. 2. **The repo's test-runner facts.** At minimum whether the new/changed test files are actually executed — for this repo, that `check-sync.sh` globs `bin/test-*.sh`, so a new `bin/test-*.sh` runs in CI automatically. "Is this test even run" is a recurring question the diff cannot answer. Scope discipline: this is about giving the reviewer facts it asks for, NOT about loosening its standards. Do not touch the verdict thresholds or tell it to approve more. If it still has a real doubt after being handed the issue and the runner facts, COMMENT remains correct. Acceptance criteria: - The review prompt includes the body of any issue named by a `Closes #NN` in the PR body (bounded in size; skip gracefully if it cannot be fetched). - The review prompt states whether changed test files are picked up by the repo's test runner. - A test asserts both are present in the built prompt for a PR that closes an issue. - No change to verdict thresholds or approve criteria. Why it matters: an operator asked for a loop that codes, reviews, merges, then picks up the next ticket. At a 35% approve rate that loop stalls on a human two thirds of the time. This is the lever that changes the rate.

igor self-assigned this

2026-07-28 03:54:23 +00:00

igor referenced this issue from a pull request that will close it,

2026-07-28 04:11:55 +00:00

feat: feed the shadow PR reviewer the linked issue body + test-runner facts #442

igor added spent time

2026-07-28 04:11:56 +00:00

17 minutes 28 seconds

igor removed their assignment

2026-07-28 04:11:56 +00:00

igor referenced this issue from a commit

2026-07-28 04:11:57 +00:00