Add per-snapshot review link and snapshot picker to Reviews tab
This commit is contained in:
Ian Gulliver
+3
-3
@@ -226,7 +226,7 @@ Merging or discarding a task stops new snapshots for it, waits for one in progre
|
||||
|
||||
`GET /api/tasks/{taskId}/changes?snapshot=<n>` reports a snapshot instead of the live worktree: its `tree` is the snapshot's tree, and `base` is the merge-base of the snapshot's commit and the default branch, or with `step=true` the previous snapshot's tree (the snapshot commit's parent for snapshot 1). It reads the stored shas, so it works only while the task is active and its refs keep the objects; `since` with `snapshot` is a 400. `GET /api/tasks/{taskId}/diff` takes that `base` and `tree` as usual.
|
||||
|
||||
The transcript is the timeline. The UI learns a task's snapshots from the `snapshot.created` events in its transcript (`snapshots` in `web/src/state.ts`). A snapshot is a point between the agent's events, not part of one, so each gets its own row in the transcript, a divider reading `snapshot #n` (its trigger and time in the tooltip), placed where its `snapshot.created` event falls in the task's event order: just after the turn end it belongs to (after the triggering text for an old `message` snapshot). A snapshot row ends the current group of tool calls, so calls after it start a new group. The row's **changes** button pins the snapshot (`state.pinned`, cleared when the selected task changes) and switches the right pane to the Changes tab; its **fork** button forks it (see "Forking") and points the right pane at the new task. Pinned, the Changes header reads `snapshot <n> · <trigger> · <time>` with ← and → to step to the neighbouring snapshots, and the range buttons are **Since main** and **This step** (`step=true`, the default when pinning) plus **Live**, which unpins. A pinned view doesn't refresh on tool results, since a snapshot never changes.
|
||||
The transcript is the timeline. The UI learns a task's snapshots from the `snapshot.created` events in its transcript (`snapshots` in `web/src/state.ts`). A snapshot is a point between the agent's events, not part of one, so each gets its own row in the transcript, a divider reading `snapshot #n` (its trigger and time in the tooltip), placed where its `snapshot.created` event falls in the task's event order: just after the turn end it belongs to (after the triggering text for an old `message` snapshot). A snapshot row ends the current group of tool calls, so calls after it start a new group. The row's **changes** button pins the snapshot (`state.pinned`, cleared when the selected task changes) and switches the right pane to the Changes tab; its **fork** button forks it (see "Forking") and points the right pane at the new task; its **review** button sets `state.reviewed` (cleared when the selected task changes) and switches the right pane to the Reviews tab. The Reviews tab shows one snapshot's review at a time: a header with a snapshot picker and ← → steps to the neighbouring reviewed snapshots (all setting `state.reviewed`), then that snapshot's latest review, or the newest review when nothing is picked. Pinned, the Changes header reads `snapshot <n> · <trigger> · <time>` with ← and → to step to the neighbouring snapshots, and the range buttons are **Since main** and **This step** (`step=true`, the default when pinning) plus **Live**, which unpins. A pinned view doesn't refresh on tool results, since a snapshot never changes.
|
||||
|
||||
## Forking
|
||||
|
||||
@@ -243,8 +243,8 @@ Every new snapshot of a task gets a review: a separate agent session on the vend
|
||||
- **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`: a Markdown `summary` and `findings`, each with `severity`, `category`, `title`, `detail`, `path` and `line`) become the review's `result` and mark it `done`. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or reports, 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 a **reviews** button; 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`) lists the task's reviews newest first (snapshot, state or severity counts, error, summary and findings). It loads when it opens, when the task changes, and on each `review.*` event in the task's stream. `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` 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, summary and findings. It loads when it opens, when the task changes, and on each `review.*` event in the task's stream. `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
|
||||
|
||||
|
||||
Vendored
+27
-8
@@ -1,7 +1,7 @@
|
||||
import { get } from "./api.js";
|
||||
import { ago, errorText, h, replace } from "./dom.js";
|
||||
import { markdown } from "./markdown.js";
|
||||
import { onEach, state } from "./state.js";
|
||||
import { onEach, pickReview, state } from "./state.js";
|
||||
const severities = ["critical", "high", "medium", "low", "info"];
|
||||
export function isReviewEvent(ev) {
|
||||
return ev.kind.startsWith("review.");
|
||||
@@ -38,7 +38,8 @@ function reviewResult(r, taskId) {
|
||||
: null);
|
||||
}
|
||||
export function mountReviews(root) {
|
||||
const list = h("div", { class: "review-list" });
|
||||
const head = h("header", { class: "reviews-header" });
|
||||
const body = h("div", { class: "review-list" });
|
||||
let reviews;
|
||||
let error = "";
|
||||
let target = 0;
|
||||
@@ -69,23 +70,40 @@ export function mountReviews(root) {
|
||||
};
|
||||
const render = () => {
|
||||
const taskId = state.selectedTaskId;
|
||||
replace(head);
|
||||
head.hidden = true;
|
||||
if (!taskId) {
|
||||
replace(list, h("div", { class: "empty" }, "No task selected."));
|
||||
replace(body, h("div", { class: "empty" }, "No task selected."));
|
||||
return;
|
||||
}
|
||||
if (error) {
|
||||
replace(list, h("div", { class: "empty error-text" }, error));
|
||||
replace(body, h("div", { class: "empty error-text" }, error));
|
||||
return;
|
||||
}
|
||||
if (!reviews) {
|
||||
replace(list, h("div", { class: "empty" }, "Loading…"));
|
||||
replace(body, h("div", { class: "empty" }, "Loading…"));
|
||||
return;
|
||||
}
|
||||
if (reviews.length === 0) {
|
||||
replace(list, h("div", { class: "empty" }, "No reviews yet. Each new snapshot of the task is reviewed by an agent from the other vendor."));
|
||||
replace(body, h("div", { class: "empty" }, "No reviews yet. Each new snapshot of the task is reviewed by an agent from the other vendor."));
|
||||
return;
|
||||
}
|
||||
replace(list, ...[...reviews].reverse().map((r) => reviewCard(r, taskId)));
|
||||
const n = state.reviewed ?? reviews.at(-1).snapshot_n;
|
||||
const shown = reviews.findLast((r) => r.snapshot_n === n);
|
||||
const ns = [...new Set([n, ...reviews.map((r) => r.snapshot_n)])].sort((a, b) => b - a);
|
||||
const picker = h("select", { title: "Snapshot whose review to show" }, ...ns.map((s) => h("option", { value: String(s) }, `snapshot #${s}`)));
|
||||
picker.value = String(n);
|
||||
picker.onchange = () => pickReview(Number(picker.value));
|
||||
const step = (to, label) => {
|
||||
const b = h("button", { type: "button", class: "snap-step", disabled: !ns.includes(to) }, label);
|
||||
b.onclick = () => pickReview(to);
|
||||
return b;
|
||||
};
|
||||
const next = ns.filter((s) => s > n).at(-1);
|
||||
const prev = ns.find((s) => s < n);
|
||||
head.hidden = false;
|
||||
replace(head, picker, step(prev ?? n - 1, `← #${prev ?? n - 1}`), step(next ?? n + 1, `#${next ?? n + 1} →`));
|
||||
replace(body, shown ? reviewCard(shown, taskId) : h("div", { class: "empty" }, `No review of snapshot #${n} yet.`));
|
||||
};
|
||||
const offs = [
|
||||
onEach(["selection"], () => {
|
||||
@@ -100,8 +118,9 @@ export function mountReviews(root) {
|
||||
void load();
|
||||
}
|
||||
}),
|
||||
onEach(["review"], render),
|
||||
];
|
||||
replace(root, h("div", { class: "reviews-view" }, list));
|
||||
replace(root, h("div", { class: "reviews-view" }, head, body));
|
||||
render();
|
||||
void load();
|
||||
return () => {
|
||||
|
||||
Vendored
+1
-1
File diff suppressed because one or more lines are too long
Vendored
+10
@@ -9,6 +9,7 @@ export const state = {
|
||||
selectedTaskId: undefined,
|
||||
transcript: [],
|
||||
pinned: undefined,
|
||||
reviewed: undefined,
|
||||
connected: false,
|
||||
focus: { tabs: ["transcript"], tab: "transcript" },
|
||||
leadSessions: [],
|
||||
@@ -62,6 +63,14 @@ export function pinSnapshot(n) {
|
||||
state.pinned = n;
|
||||
notify("pin");
|
||||
}
|
||||
export function pickReview(n) {
|
||||
state.reviewed = n;
|
||||
notify("review");
|
||||
}
|
||||
export function showReview(n) {
|
||||
pickReview(n);
|
||||
focusTab("reviews");
|
||||
}
|
||||
export async function loadMe() {
|
||||
state.me = await get("/api/me");
|
||||
notify("me");
|
||||
@@ -203,6 +212,7 @@ export function selectTask(taskId) {
|
||||
taskFeed.reset();
|
||||
state.selectedTaskId = taskId;
|
||||
state.pinned = undefined;
|
||||
state.reviewed = undefined;
|
||||
notify("selection");
|
||||
if (!taskId) {
|
||||
return;
|
||||
|
||||
Vendored
+1
-1
File diff suppressed because one or more lines are too long
Vendored
+5
-5
@@ -3,7 +3,7 @@ import { ago, copyable, errorText, h, replace, showAlert, showConfirm } from "./
|
||||
import { markdown } from "./markdown.js";
|
||||
import { previewURL } from "./preview.js";
|
||||
import { reviewCounts } from "./reviews.js";
|
||||
import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, state } from "./state.js";
|
||||
import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, showReview, state } from "./state.js";
|
||||
import { taskStyle } from "./taskcolor.js";
|
||||
export const models = [
|
||||
"claude-opus-5-5",
|
||||
@@ -776,8 +776,10 @@ function snapRow(snap, taskId) {
|
||||
void showAlert(errorText(err));
|
||||
}
|
||||
};
|
||||
const review = h("button", { type: "button", title: `Show the review of snapshot ${snap.n} in the Reviews tab` }, "review");
|
||||
review.onclick = () => showReview(snap.n);
|
||||
const time = new Date(snap.created_at).toLocaleTimeString([], { hour12: false });
|
||||
return h("div", { class: "snap-row", title: `${snap.trigger.replace("_", " ")} · ${time}` }, `snapshot #${snap.n}`, changes, fork);
|
||||
return h("div", { class: "snap-row", title: `${snap.trigger.replace("_", " ")} · ${time}` }, `snapshot #${snap.n}`, changes, fork, review);
|
||||
}
|
||||
function runSummary(calls) {
|
||||
const counts = new Map();
|
||||
@@ -890,10 +892,8 @@ function renderEvent(ev, ctx) {
|
||||
return h("div", { class: "note" }, `${ev.actor} set the merge commit title to "${String(p["merge_title"])}" `, when);
|
||||
case "review.finished": {
|
||||
const r = p;
|
||||
const open = h("button", { type: "button", title: "Show the task's reviews" }, "reviews");
|
||||
open.onclick = () => focusTab("reviews");
|
||||
const outcome = r.state === "failed" ? `failed: ${r.error}` : reviewCounts(r);
|
||||
return h("div", { class: `note${r.state === "failed" ? " error" : ""}` }, `review of snapshot #${r.snapshot_n}: ${outcome} `, when, " ", open);
|
||||
return h("div", { class: `note${r.state === "failed" ? " error" : ""}` }, `review of snapshot #${r.snapshot_n}: ${outcome} `, when);
|
||||
}
|
||||
case "task.merge_queued":
|
||||
return h("div", { class: "note" }, `${ev.actor} asked to merge; held until the merge in progress in this project is done `, when);
|
||||
|
||||
Vendored
+1
-1
File diff suppressed because one or more lines are too long
+27
-8
@@ -1,7 +1,7 @@
|
||||
import { get, type ByreEvent, type Schemas } from "./api.js";
|
||||
import { ago, errorText, h, replace } from "./dom.js";
|
||||
import { markdown } from "./markdown.js";
|
||||
import { onEach, state } from "./state.js";
|
||||
import { onEach, pickReview, state } from "./state.js";
|
||||
|
||||
type Review = Schemas["Review"];
|
||||
|
||||
@@ -80,7 +80,8 @@ function reviewResult(r: Review, taskId: string | undefined): HTMLElement {
|
||||
}
|
||||
|
||||
export function mountReviews(root: HTMLElement): () => void {
|
||||
const list = h("div", { class: "review-list" });
|
||||
const head = h("header", { class: "reviews-header" });
|
||||
const body = h("div", { class: "review-list" });
|
||||
let reviews: Review[] | undefined;
|
||||
let error = "";
|
||||
let target = 0;
|
||||
@@ -112,23 +113,40 @@ export function mountReviews(root: HTMLElement): () => void {
|
||||
|
||||
const render = () => {
|
||||
const taskId = state.selectedTaskId;
|
||||
replace(head);
|
||||
head.hidden = true;
|
||||
if (!taskId) {
|
||||
replace(list, h("div", { class: "empty" }, "No task selected."));
|
||||
replace(body, h("div", { class: "empty" }, "No task selected."));
|
||||
return;
|
||||
}
|
||||
if (error) {
|
||||
replace(list, h("div", { class: "empty error-text" }, error));
|
||||
replace(body, h("div", { class: "empty error-text" }, error));
|
||||
return;
|
||||
}
|
||||
if (!reviews) {
|
||||
replace(list, h("div", { class: "empty" }, "Loading…"));
|
||||
replace(body, h("div", { class: "empty" }, "Loading…"));
|
||||
return;
|
||||
}
|
||||
if (reviews.length === 0) {
|
||||
replace(list, h("div", { class: "empty" }, "No reviews yet. Each new snapshot of the task is reviewed by an agent from the other vendor."));
|
||||
replace(body, h("div", { class: "empty" }, "No reviews yet. Each new snapshot of the task is reviewed by an agent from the other vendor."));
|
||||
return;
|
||||
}
|
||||
replace(list, ...[...reviews].reverse().map((r) => reviewCard(r, taskId)));
|
||||
const n = state.reviewed ?? reviews.at(-1)!.snapshot_n;
|
||||
const shown = reviews.findLast((r) => r.snapshot_n === n);
|
||||
const ns = [...new Set([n, ...reviews.map((r) => r.snapshot_n)])].sort((a, b) => b - a);
|
||||
const picker = h("select", { title: "Snapshot whose review to show" }, ...ns.map((s) => h("option", { value: String(s) }, `snapshot #${s}`)));
|
||||
picker.value = String(n);
|
||||
picker.onchange = () => pickReview(Number(picker.value));
|
||||
const step = (to: number, label: string) => {
|
||||
const b = h("button", { type: "button", class: "snap-step", disabled: !ns.includes(to) }, label);
|
||||
b.onclick = () => pickReview(to);
|
||||
return b;
|
||||
};
|
||||
const next = ns.filter((s) => s > n).at(-1);
|
||||
const prev = ns.find((s) => s < n);
|
||||
head.hidden = false;
|
||||
replace(head, picker, step(prev ?? n - 1, `← #${prev ?? n - 1}`), step(next ?? n + 1, `#${next ?? n + 1} →`));
|
||||
replace(body, shown ? reviewCard(shown, taskId) : h("div", { class: "empty" }, `No review of snapshot #${n} yet.`));
|
||||
};
|
||||
|
||||
const offs = [
|
||||
@@ -144,9 +162,10 @@ export function mountReviews(root: HTMLElement): () => void {
|
||||
void load();
|
||||
}
|
||||
}),
|
||||
onEach(["review"], render),
|
||||
];
|
||||
|
||||
replace(root, h("div", { class: "reviews-view" }, list));
|
||||
replace(root, h("div", { class: "reviews-view" }, head, body));
|
||||
render();
|
||||
void load();
|
||||
return () => {
|
||||
|
||||
@@ -12,6 +12,7 @@ export type Topic =
|
||||
| "connection"
|
||||
| "event"
|
||||
| "pin"
|
||||
| "review"
|
||||
| "focus"
|
||||
| "lead.sessions"
|
||||
| "lead.transcript"
|
||||
@@ -28,6 +29,7 @@ export const state = {
|
||||
selectedTaskId: undefined as string | undefined,
|
||||
transcript: [] as ByreEvent[],
|
||||
pinned: undefined as number | undefined,
|
||||
reviewed: undefined as number | undefined,
|
||||
connected: false,
|
||||
focus: { tabs: ["transcript"], tab: "transcript" } as Focus,
|
||||
leadSessions: [] as AgentSession[],
|
||||
@@ -92,6 +94,16 @@ export function pinSnapshot(n: number | undefined): void {
|
||||
notify("pin");
|
||||
}
|
||||
|
||||
export function pickReview(n: number): void {
|
||||
state.reviewed = n;
|
||||
notify("review");
|
||||
}
|
||||
|
||||
export function showReview(n: number): void {
|
||||
pickReview(n);
|
||||
focusTab("reviews");
|
||||
}
|
||||
|
||||
export async function loadMe(): Promise<void> {
|
||||
state.me = await get("/api/me");
|
||||
notify("me");
|
||||
@@ -254,6 +266,7 @@ export function selectTask(taskId: string | undefined): void {
|
||||
taskFeed.reset();
|
||||
state.selectedTaskId = taskId;
|
||||
state.pinned = undefined;
|
||||
state.reviewed = undefined;
|
||||
notify("selection");
|
||||
|
||||
if (!taskId) {
|
||||
|
||||
+5
-5
@@ -3,7 +3,7 @@ import { ago, copyable, errorText, h, replace, showAlert, showConfirm } from "./
|
||||
import { markdown } from "./markdown.js";
|
||||
import { previewURL } from "./preview.js";
|
||||
import { reviewCounts } from "./reviews.js";
|
||||
import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, state, type Topic } from "./state.js";
|
||||
import { currentSession, findTask, focusTab, focusTask, onEach, pinSnapshot, setFocus, showReview, state, type Topic } from "./state.js";
|
||||
import { taskStyle } from "./taskcolor.js";
|
||||
|
||||
export type Usage = { used: number; window: number; model: string };
|
||||
@@ -907,8 +907,10 @@ function snapRow(snap: Snapshot, taskId: string | undefined): HTMLElement {
|
||||
void showAlert(errorText(err));
|
||||
}
|
||||
};
|
||||
const review = h("button", { type: "button", title: `Show the review of snapshot ${snap.n} in the Reviews tab` }, "review");
|
||||
review.onclick = () => showReview(snap.n);
|
||||
const time = new Date(snap.created_at).toLocaleTimeString([], { hour12: false });
|
||||
return h("div", { class: "snap-row", title: `${snap.trigger.replace("_", " ")} · ${time}` }, `snapshot #${snap.n}`, changes, fork);
|
||||
return h("div", { class: "snap-row", title: `${snap.trigger.replace("_", " ")} · ${time}` }, `snapshot #${snap.n}`, changes, fork, review);
|
||||
}
|
||||
|
||||
function runSummary(calls: Call[]): HTMLElement {
|
||||
@@ -1052,10 +1054,8 @@ function renderEvent(ev: ByreEvent, ctx: Ctx): HTMLElement | null {
|
||||
|
||||
case "review.finished": {
|
||||
const r = p as unknown as Review;
|
||||
const open = h("button", { type: "button", title: "Show the task's reviews" }, "reviews");
|
||||
open.onclick = () => focusTab("reviews");
|
||||
const outcome = r.state === "failed" ? `failed: ${r.error}` : reviewCounts(r);
|
||||
return h("div", { class: `note${r.state === "failed" ? " error" : ""}` }, `review of snapshot #${r.snapshot_n}: ${outcome} `, when, " ", open);
|
||||
return h("div", { class: `note${r.state === "failed" ? " error" : ""}` }, `review of snapshot #${r.snapshot_n}: ${outcome} `, when);
|
||||
}
|
||||
|
||||
case "task.merge_queued":
|
||||
|
||||
@@ -1612,6 +1612,18 @@ pre.file-text .line::before {
|
||||
background: var(--raised);
|
||||
}
|
||||
|
||||
.reviews-header {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 8px;
|
||||
padding: 8px 16px;
|
||||
border-bottom: 1px solid var(--border);
|
||||
}
|
||||
|
||||
.reviews-header[hidden] {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.card .review {
|
||||
background: var(--surface);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user