ai/byre
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

Happy review

Browse Source
This commit is contained in:
Ian Gulliver
2026-10-15 20:07:36 -07:00
parent ccd55afe13
commit 35368242e3
14 changed files with 312 additions and 191 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/operations.md
+3 -3
View File
@@ -238,14 +238,14 @@ The fork's session guidance gets one more line (`forkGuidance` in `internal/agen
## Reviews
Every new snapshot of a task gets a review: a separate agent session on the vendor the task's agent doesn't use reads the change and reports on it (`internal/agents/review.go`, `internal/store/review.go`). A turn that changes no files makes no snapshot and so no review, and a snapshot whose change (the diff below) has no files, such as a task's first snapshot taken before its agent changed anything, gets no review either. Nothing else starts a review, and nothing waits for one: reviews ride along for the human to look at when they want.
Every new snapshot of a task gets a review: a separate agent session on the vendor the task's agent doesn't use reads the change and reports on it (`internal/agents/review.go`, `internal/store/review.go`). A turn that changes no files makes no snapshot and so no review, and a snapshot whose change (the diff below) has no files, such as a task's first snapshot taken before its agent changed anything, runs no reviewer: it gets a `reviews` row in state `skipped`, with no session, vendor or model. Nothing else starts a review, and nothing waits for one: reviews ride along for the human to look at when they want.
- **Starting.** After `takeSnapshot` creates a snapshot, `afterSnapshot` runs `startReviewer`: it picks the vendor opposite the task's latest implementer session (`reviewVendor`) and that vendor's model in `reviewModels`, checks the snapshot's commit out detached at `worktrees/<project-id>/review-<review-id>`, creates an `agent_sessions` row with `role` `reviewer` on the task (created by the task's creator), a `reviews` row (`running`, one per snapshot), and sends `reviewPrompt` as its first turn (actor `byre`): the change as `git diff` from the previous snapshot (the snapshot commit's parent for the first) with its changed files, the merge-base diff of all the task's work for context, and the task's first message to its agent.
- **Running.** Reviewer sessions get `reviewGuidance` and only the `submit_review` tool, whose arguments (`reviewSchema`: `change`, a few sentences of Markdown on what the change does, required and non-empty; a Markdown `summary`, which the guidance and schema require to be exactly "No significant findings." when there are no findings and otherwise a bulleted list with one line per finding; `findings`, each with `severity`, `category`, `title`, `detail`, `path`, `line`, `end_line` and `images`; and top-level `images`) become the review's `result` and mark it `done`. The guidance encourages, without requiring, Mermaid blocks for a data flow, call graph, state machine or schema the change alters, and screenshots when the change alters something visible.
- **Screenshots.** Reviewer sessions also get the Playwright MCP server (`playwrightMCP` in `internal/agents/review.go`: `npx @playwright/mcp` at a pinned version, headless Chrome, isolated profile, absolute file paths), passed to Claude as a `stdio` entry in `--mcp-config` and to Codex as `mcp_servers` in the thread's `config`. Its automatically named files go to `worktrees/<project-id>/review-<review-id>-media`, removed with the checkout. Each `images` entry is `{path, caption}`, the path absolute or relative to the checkout; at `submit_review` byre reads every one, refuses the whole submission (nothing recorded) for a missing file or one that isn't one of `store.ImageTypes` within `store.MaxBlobBytes`, then stores each as a blob and replaces its `path` with `blob`, since the files are deleted with the checkout. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or turn summaries, and their `user.message` events carry no `head`. When the reviewer's turn ends byre completes its session, kills its process and removes the worktree; a review still `running` then fails with why (no submission, with the final message; an error; an interrupt). A process that exits mid-turn fails the review the same way. Merging or discarding the task fails its running reviews ("the task was closed") and ends their reviewers. On startup, a running reviewer resumes like any session, and a running review whose reviewer isn't `running` fails.
- **Questions.** A question's `reviews` is the review of the first snapshot the task took after the question was filed, the one at the end of the turn that asked it, computed when the item is read; the Inbox tab shows it under the question once it starts.
- **Events.** `review.started` and `review.finished` (state `done` or `failed`) carry the whole review, with the task's ids and no `agent_session_id`. A reviewer's own events carry its session id and the task's ids: the transcript skips every event of a session whose `session.created` has `role` `reviewer`, and draws each `review.finished` as a note with its outcome; the lead's `read_transcript` leaves the reviewer's events out and shows each `review.finished` with its summary and findings (`reviewLine`). `GET /api/tasks/{taskId}/sessions` and everything that looks for the task's agent (`ListSessions`) see only implementer sessions.
- **UI.** The **Reviews** tab (`web/src/reviews.ts`) shows one review at a time, picked by snapshot (see "Snapshots" for the picker): its snapshot, state or severity counts, error, `change`, summary, screenshots and findings. A finding with `path` and `line` shows, under its title, the lines `line` to `end_line` plus 3 either side from that file's diff for the snapshot's step (`changes?snapshot=<n>&step=true`, the diff the reviewer was given), with new-file line numbers and the finding's own lines marked; it shows nothing when those lines aren't in the diff, and "diff not available" with the error once the task is closed. Screenshots link to the full image. Each finding has a **fix** button (also on review cards in the inbox) that sends the finding's severity, title, location and detail to the task's agent as a message from the user: through `POST /api/sessions/{id}/messages` when its latest live session is idle, starting a session when it has none, and refused with an alert while the agent is mid-turn. It loads when it opens, when the task changes, and on each `review.*` event in the task's stream. When the finished review of the highest-numbered snapshot has findings, the tab (or the More menu holding it) is highlighted with a badge of the finding count. Each snapshot row's **review** button gets the same treatment from that snapshot's `review.finished`: highlighted, with the count in a badge over its top-right corner. `GET /api/tasks/{taskId}/reviews` lists a task's reviews oldest first. `sqlite3 byre.db "SELECT id, task_id, snapshot_n, vendor, state, error FROM reviews ORDER BY created_at DESC LIMIT 20"` shows recent ones.
- **Events.** `review.started` and `review.finished` (state `done`, `failed` or `skipped`; a skipped review emits only `review.finished`) carry the whole review, with the task's ids and no `agent_session_id`. A reviewer's own events carry its session id and the task's ids: the transcript skips every event of a session whose `session.created` has `role` `reviewer`, and draws each `review.finished` as a note with its outcome; the lead's `read_transcript` leaves the reviewer's events out and shows each `review.finished` with its summary and findings (`reviewLine`). `GET /api/tasks/{taskId}/sessions` and everything that looks for the task's agent (`ListSessions`) see only implementer sessions.
- **UI.** The **Reviews** tab (`web/src/reviews.ts`) shows one review at a time, picked by snapshot (see "Snapshots" for the picker): its snapshot, state or severity counts, error, `change`, summary, screenshots and findings. A finding with `path` and `line` shows, under its title, the lines `line` to `end_line` plus 3 either side from that file's diff for the snapshot's step (`changes?snapshot=<n>&step=true`, the diff the reviewer was given), with new-file line numbers and the finding's own lines marked; it shows nothing when those lines aren't in the diff, and "diff not available" with the error once the task is closed. Screenshots link to the full image. Each finding has a **fix** button (also on review cards in the inbox) that sends the finding's severity, title, location and detail to the task's agent as a message from the user: through `POST /api/sessions/{id}/messages` when its latest live session is idle, starting a session when it has none, and refused with an alert while the agent is mid-turn. It loads when it opens, when the task changes, and on each `review.*` event in the task's stream. When the finished review of the highest-numbered snapshot has findings, the tab (or the More menu holding it) is highlighted with a badge of the finding count. Each snapshot row's **review** button carries a mark over its top-right corner from that snapshot's latest `review.*` event: a spinner while the review runs, a grey ● when it was skipped, a green ✓ when it finished with no findings, and, when it has findings, the button highlighted with the count in a badge. A failed review leaves the button unmarked. `GET /api/tasks/{taskId}/reviews` lists a task's reviews oldest first. `sqlite3 byre.db "SELECT id, task_id, snapshot_n, vendor, state, error FROM reviews ORDER BY created_at DESC LIMIT 20"` shows recent ones.
## Events