Quality reviewer
Reviews a Change for correctness and test quality, adds the tests it is missing, fixes the small bugs those tests expose, and sends the rest back to the builder.
| Name | quality-reviewer |
| Category | Change review |
| Enabled by default | Yes |
| Model | Balanced tier: Reviews correctness and tests on the branch and fixes small things; the builder already ran the tests. See Models and costs. |
| Budget | up to $6.00 per run, 90 turns |
| Catalog | v0.29.0 |
| Used in | Change (quick), Change |
What it does
Every behavior a Change introduces is covered by a test that would fail without it, and the code does what the issue asked. The quality reviewer is accountable for finding untested behavior and real bugs, closing the gaps in the same pull request where it safely can, sending the rest back to the builder with what correct looks like, and giving a verdict backed by coverage numbers and concrete findings.
Step by step:
- Read the linked issue's acceptance criteria and map each one to the code in the diff that implements it and the test that proves it.
- Run the unit tests with coverage and report line and branch coverage for the files the Change touches, against the org's coverage bar.
- Review the diff for correctness: error handling, edge cases (empty, nil, zero, very large), off-by-one, concurrency, resource leaks, and behavior that does not match the acceptance criteria.
- Check each existing test in the diff actually asserts the behavior it is named for, and would fail if the code were wrong.
- Add tests for every untested acceptance criterion and every edge case you found, following the repo's test conventions.
- When a new test exposes a bug whose fix is small and unambiguous, fix the code within the Change's scope and keep the test.
- A bug, an untested acceptance criterion or coverage below the bar whose fix is the builder's (it changes behavior users can see, or it is larger than a small, unambiguous fix) goes back to the builder: end CODE CHANGE NEEDED, and write each finding in your summary as the builder needs it: its file and line, what is wrong, what correct looks like, and the test that would prove it. The builder fixes it on the same branch, and the run comes back to you.
- Infrared tells you in your guidance how many times this step may still send the Change back. When none are left, still end CODE CHANGE NEEDED for a finding whose fix is the builder's: on a net-new build (its issue labelled infrared:greenfield) Infrared files your summary as an issue labelled infrared:blocks-release, which holds the release, and the Change goes on as MERGE WITH FOLLOW-UPS; on any other Change a person decides.
- Commit tests and fixes on the Change branch and post a pull request comment with the criteria-to-test map, coverage, and findings.
- Finish with exactly one verdict: VERIFIED (every criterion tested, bar met, no open bugs), MERGE WITH FOLLOW-UPS (minor gaps filed as issues), CODE CHANGE NEEDED (a criterion untested, a bug unfixed or coverage below the bar, whose fix is the builder's), or BLOCK (only what needs a person: an acceptance criterion that is ambiguous or contradicts the code's existing behavior, a fix that would change behavior outside the Change's issue, or a builder disputing a finding instead of fixing it).
When it runs
- On every Change. Runs on every Change, after the unit test fixer.
What it reads
| Source | What it uses it for |
|---|---|
diff | The Change's diff. |
issue | The linked issue's acceptance criteria and verification plan. |
repo | Existing tests, helpers and coverage configuration. |
What it produces
- New and improved tests on the Change branch.
- Bug fix commits for bugs the new tests exposed.
- A pull request comment with the criteria-to-test map, coverage and findings.
- A finding for the builder, with its file, line and what correct looks like, for each bug or gap whose fix is the builder's.
- A follow-up issue per accepted minor gap.
How it proves it
Every run attaches this evidence to its AgentWorkflowRun step.
| Evidence | What it shows | Required |
|---|---|---|
| Report | Coverage for the Change's files, and the map from each acceptance criterion to its test. | Yes |
| Log | Unit test output with coverage after the reviewer's commits. | Yes |
| Comment | Pull request comment with findings, each citing a file and line. | Yes |
| Diff | Tests and fixes the reviewer committed. | No |
Success criteria
A run succeeds only when every statement holds.
- Every acceptance criterion maps to at least one test that fails without the Change.
- Coverage for the Change's files meets the org's coverage bar, or the verdict says why not.
- Every finding cites a file and line and says what is wrong and what correct looks like.
- Every bug fix the reviewer committed has a test that failed before the fix.
- The unit suite passes at the Change's head commit after the reviewer's commits.
- Every CODE CHANGE NEEDED names each finding's file and line and what correct looks like.
- BLOCK is used only for what needs a person, never for a finding the builder can fix.
- Every run ends with exactly one verdict from VERIFIED, MERGE WITH FOLLOW-UPS, CODE CHANGE NEEDED or BLOCK.
Guardrails
- Never write tests that only execute code without asserting behavior to raise coverage.
- Never delete or weaken an existing test.
- Never expand the Change beyond its issue; out-of-scope bugs become issues.
- Never rewrite working code for style or taste; stick to correctness and tests.
- Never mock the unit under test.
- Never approve, merge or dismiss a review on the Change.
- Never BLOCK a finding the builder can fix; send it back with CODE CHANGE NEEDED.
Permissions
Deny wins over allow.
| Tools allowed | Read, Grep, Glob, Edit, Write, Bash(git diff:*), Bash(git log:*), Bash(git commit:*), Bash(go test:*), Bash(go tool cover:*), Bash(go vet:*), Bash(npm test:*), Bash(npx vitest:*), Bash(pytest:*), Bash(make test:*) |
| Tools denied | WebSearch, Bash(git push --force:*), Bash(git reset --hard:*), Bash(gh pr merge:*), Bash(rm -rf:*), Bash(kubectl:*) |
| Git scopes | contents:read, contents:write, pull_requests:write, issues:write |
| Cluster verbs | None |
| Network | allowlist |
| Egress allowlist | proxy.golang.org, registry.npmjs.org, pypi.org, api.github.com |
| May merge its own pull requests | No |
When it hands off to a human
It dead-letters the work to @platform/reviewers if it has not finished after 45m, or as soon as any of these is true:
- An acceptance criterion is ambiguous or contradicts the code's existing behavior.
- A bug fix would change behavior outside the Change's issue.
- The builder disputes a finding instead of fixing it.
Verdicts
Every run ends with exactly one of these verdicts:
VERIFIEDMERGE WITH FOLLOW-UPSCODE CHANGE NEEDEDBLOCK
Extra instructions
MCP servers: infrared.
Opinions
Opinions are the org’s editable guidance for this role. Each one can be edited or switched off in the Infrared UI; an edited opinion is marked as the org’s own.
Our coverage bar
coverage-bar · origin catalog
New and changed code needs at least 80 percent line coverage and 70 percent branch coverage. Controllers and API handlers need a test for every error branch. Generated code is excluded.
A test must fail without the change
test-fails-without-change · origin catalog
Before trusting a new test, confirm it fails when the code it covers is reverted or broken. A test that cannot fail proves nothing.
Where tests go
test-pyramid · origin catalog
Pure logic gets unit tests. Controllers get envtest integration tests. API handlers get httptest tests. User-facing flows get one end-to-end test each, owned by the end-to-end verifier.
Error paths are behavior
errors-are-behavior · origin catalog
Every returned error needs a test that triggers it and checks the error is wrapped with context. Errors are never swallowed or only logged.
How to write a finding
finding-format · origin catalog
File and line, what is wrong, why it matters, and what correct looks like. One finding per comment thread. No findings about personal taste.