---
title: "Review rules"
source: "agents/review-rules.md"
---

> Part of [botgumi docs](https://botgumi.com/docs/llms.txt). Every page: [llms.txt](https://botgumi.com/llms.txt).

# Review rules

Every PR gets an agent review before a human merges it. The reviewer checks these fixed rules, not taste.

## Who reviews

- The reviewer is **never the author**, and by default runs on a **different runtime** from the author:
  Claude writes and Codex reviews, or the other way round.
- Reviewers come from `agents.reviewers` in `botgumi.config.ts` (or the track's override).
  If `primary` shares the author's runtime, `second` reviews instead.
- **High-risk PRs get both reviewers:** payments, auth, contract changes, data migrations (the high tier).
  Mid and low tier PRs get one.

## What the reviewer checks

Each failed check is a finding. The default severity is given; a reviewer may raise a non-blocking finding
to blocking with a reason, never the other way round.

| # | Check | Default |
|---|---|---|
| 1 | Matches the OpenAPI contract and the ADRs. Any contract change edits `docs/api/openapi.yaml` in the same PR and is called out. | blocking |
| 2 | Every screen or endpoint touched handles loading, empty, error (retryable or not) and offline. | blocking |
| 3 | Every new or changed user-facing string exists in en and zh-Hans, in the localization layer. No CJK-only logic. | blocking |
| 4 | No placeholder numbers in production code paths. Seed data is labelled as seed. | blocking |
| 5 | Every changed behaviour has a test. | blocking |
| 6 | PR sections filled in: **What**, **Why**, **How tested** (with pasted CI output), **Data-binding / contract changes**, **Known gaps**. | blocking |
| 7 | App code imports provider interfaces only, never a vendor SDK. | blocking |
| 8 | Docs updated for changed behaviour (ADR-0005): a PR that changes behaviour, a contract or a design updates its module's pages, lists any new page in `docs/manifest.json`, and commits the rebuilt `docs/public/`; `pnpm docs:check` passes. | blocking |
| 9 | Naming, clarity, small simplifications. | non-blocking |

### Dashboard track

On top of the checks above, every PR that touches `dashboard/` (ADR-0003):

| # | Check | Default |
|---|---|---|
| D1 | Counts only. No email, user ID, device ID, position, trace or review text in the page, its tables, the collector's output or its logs. | blocking |
| D2 | No number typed into the page. Every number comes from a source or a named calculation over sources, and carries its provenance (source, query or calculation, as-of time, link when there is one). | blocking |
| D3 | A failing source is named on its own panel and the others still load. The collector catches each source on its own and names the ones that failed. | blocking |
| D4 | Every panel handles loading, empty, error, offline, not connected (the missing key and the step that sets it) and stale (a morning source older than 48 hours). These replace check 2 for the dashboard. | blocking |
| D5 | A goal drawn on the page comes from the "Quality bar" section of the PRD and cites its line. No goal without one. | blocking |
| D6 | Keys are read-only, listed in the secrets inventory, and never reach the page or the tables. The collector only reads. | blocking |
| D7 | Cloud spend is labelled as an estimate. | non-blocking |

### Design track

On top of the checks above, every PR that touches `design/`, and every PR that adds or changes a screen in
`ios/` or `android/` (ADR-0004):

| # | Check | Default |
|---|---|---|
| G1 | The PR's stage follows a passed gate: brand brief, design system, lo-fi, hi-fi, screenshot direction, in order. | blocking |
| G2 | Tokens are in `design/system/tokens.source.json`. A colour, size or font on a board or in screen code is a token, not a literal. | blocking |
| G3 | Every screen in `design/lofi/screens.md` lists loading, empty, error and offline; every hi-fi screen has a board for each listed state. | blocking |
| G4 | Every hi-fi screen has a data-binding map (`bindings.md`), and every value on its boards comes from an example API response, local state or the device. No made-up numbers. | blocking |
| G5 | No screen code in `ios/` or `android/` before the screen's row is `approved` with its gate issue. Screen code matches its approved boards and binding map; a difference is a design issue first. | blocking |
| G6 | Strings on the boards have keys and exist in en and zh-Hans; the `ready` board is also drawn in zh-Hans. | blocking |
| G7 | `node design/check.mjs` passes, output pasted. | blocking |

## Posting

- One review per round, through `gh pr review`: `--request-changes` while blocking findings are open, `--comment` otherwise.
- Then the `botgumi/review` commit status on the head commit: `success` only with zero blocking findings.
- Each finding is a line: `**blocking**` or `**non-blocking**`, the check number, `file:line`, the problem, the fix.

## Rounds

1. The author fixes every blocking finding and pushes. CI must pass again.
2. The reviewer reviews again. That is round 2.
3. After **2 rounds** with blocking findings still open, the orchestrator escalates to the human with the open findings.

## Merging

The author never merges and never approves its own work. The orchestrator may merge only under
`merge-policy.md`, which requires `botgumi/review` = success. Everything else waits for a human.