From 31ee494c4168bfa5a50333ff77ab3c036a5608f7 Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Mon, 19 Oct 2026 19:05:19 -0700 Subject: [PATCH] reviewing is work --- docs/operations.md | 4 ++-- mac/Sources/Byre/Chat.swift | 13 ++++++++++--- mac/Sources/Byre/Format.swift | 2 -- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index b482363..766a3d8 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -274,7 +274,7 @@ Agent activity becomes events with `agent_session_id` set: `user.message`, `agen Event payloads are stored whole in `events.payload`, and `agent.tool_result` content (full command output, file reads, diffs) dominates the log's size; the app loads every active task's whole history when it starts (see "Events"), so this is what its startup costs. `sqlite3 byre.db "SELECT kind, count(*), sum(length(payload)) FROM events GROUP BY kind ORDER BY 3 DESC"` shows the split, and `sqlite3 byre.db "SELECT kind, count(*), sum(length(payload)) FROM events WHERE task_id = '' GROUP BY kind ORDER BY 3 DESC"` one task's. -Messages byre itself sends an agent (actor `byre`: the restart notice, Sync and Merge prompts, Compact's handoff request) show collapsed like a tool call, labelled `byre` with the first line as the brief; expanding it shows the full text. Review findings forwarded to the agent come this way too, so the transcript shows them separately: a `review.finished` event with findings draws its counts note with the review's `summary` (one bullet per finding) expanded under it, behind a border-coloured rule, and then each finding as its own row (see "Reviews"). The transcript view has a border in the task's colour (see "Task colours"; the accent in the lead pane) while the session is `running` and its transcript has an open turn (`open` in `TranscriptBuilder`: a drawn turn-starting `user.message`, not mid-turn, with no `turn.ended` after it), and a Cylon-style scanner in the same colour: a row of 2pt dots, 5pt apart along an 84pt track, where a glow sweeping back and forth every 1.2s lights up the dots under it, drawn below the last message, in the transcript's bottom padding, so it scrolls with the transcript and sits just above the composer's top border when scrolled to the bottom. It is an overlay 8pt below the transcript's last row, so showing or hiding it never changes the transcript's scroll height or position, and it is drawn with Core Animation (`ScannerView` in `mac/Sources/Byre/Chat.swift`: a gradient layer masked by the dots, its position animated by a repeating `CABasicAnimation`), so the render server animates it and SwiftUI never redraws the transcript for it. The session list (`GET /api/me/lead/sessions` or the task's sessions) is fetched separately from the event stream, so it says `running` a little before the turn's `user.message` arrives and a little after its `turn.ended`; requiring the open turn as well keeps the scanner and border from flashing on in those gaps. Consecutive tool calls collapse into one group whose summary counts calls per tool and file, and file paths show relative to the worktree. Thinking output appears as expanded Markdown text with a small muted "thinking" label and ends the current tool-call group just like an agent message, including inside subagent rows. While the scanner shows, a tool call still waiting for its result, and any group or subagent row holding one, gets a border in the task's colour, so a running tool reads apart from waiting on the model without expanding anything (`working` in `mac/Sources/Byre/Chat.swift`). The braille spinner the Project tab shows for task states is drawn with Core Animation too (`SpinnerView` in `mac/Sources/Byre/Format.swift`: its ten glyphs rendered once into images in the surrounding font and colour, cycled every 80ms by a repeating discrete `CAKeyframeAnimation` on the layer's contents), so it never makes SwiftUI redraw or lay out the views around it. When a turn ends, or a turn-starting `user.message` (not mid-turn) arrives, any call still without a result, other than those inside an active subagent row (below), reads "no result" instead of "running…", so an interrupted call never spins during a later turn. The second case covers a turn a restart cut off: it has no `turn.ended`, and the restart notice starts the next turn (`endTools` in `mac/Sources/Byre/TranscriptBuilder.swift`). byre's own tools show without their `mcp__byre__` prefix, in title case (`mcp__byre__create_task` shows as Create Task; `toolLabel` in `mac/Sources/Byre/TranscriptBuilder.swift`); other tools show under the name the agent used. A Claude Code subagent call (the `Agent` or `Task` tool) stands on its own, collapsed, labelled with its `subagent_type` and `description`; expanding it shows the prompt and the call's result. Its summary carries a status from the `agent.vendor` `system` events whose `tool_use_id` names it (`task_started`: running, or running in the background; `task_progress`: its `description`; `task_notification`: its `status`, red unless `completed`). From its `task_started` to its `task_notification` the row is active, and an active row gets the task-coloured border while the scanner shows or while the session's `subagents` count is above zero, as do the waiting tool calls inside it in the latter case; so a background subagent still reads as running after the turn that started it has ended. A turn's end leaves the waiting calls inside an active row waiting; the row's end (its `task_notification`, or a `session.subagents` event with 0, which ends every still-active row as "ended with its process", replayed history included) marks the calls inside it still waiting "no result". The subagent's own messages, thinking and tool calls (events whose `parent_tool_use_id` names the call) go inside that call's row, between the prompt and the result, so parallel subagents don't interleave in the main stream; its consecutive tool calls group there the same way, its messages get an accent rule down their left edge, and a subagent it starts nests inside it in turn. An event naming a subagent call the transcript never saw stays in the main stream marked `↳ subagent`. A subagent call ends the main agent's run of tool calls, so it never joins a group. A subagent's messages never end the main agent's live message, count as a turn's final message or hide a quiet turn. Claude Code streams a background subagent's activity, but for a foreground one only its prompt, so a foreground subagent shows only as its call, its status and its result. Selecting a task that has never had a session focuses the composer, and so does picking any task in the ⌘K switcher when it lands on the transcript (`Store.composerFocus`). In the composer, Enter sends and Shift-Enter adds a newline; unsent text is kept per task (see "Drafts" below), so it survives switching tabs or tasks and relaunching the app. The composer has no `@`-file mentions or path picker: text reaches the agent exactly as typed (stream-json doesn't expand `@path` the way the interactive CLI does), and the agent reads any file the message names with its own tools. The composer is the same whether or not the task has a live session, and stays enabled while a turn runs: Send posts to the session's messages, or starts a session with the text when there is none or the last one completed. A sent message shows at once as a bubble reading "queued…" while the session is running and "sending…" otherwise, replaced by the server's `user.message` event with the same text when it arrives, and removed (text and images back in the composer) if the request fails. A queued message's `user.queued` event takes over its bubble (the oldest local one with the same text), and the transcript shows a "queued…" bubble below everything else for every `user.queued` not yet followed by a `user.message` with the same text, each `user.message` clearing the oldest such bubble first (queued messages are delivered oldest first), then a local one, and `task.merged` or `task.discarded` clearing them all, since closing the task deletes its queue, so a queued message stays visible across switching tasks, relaunching the app, and on every view. A bubble from a `user.queued` event has a × that withdraws the message (`ChatModel.withdraw`): it first downloads the message's images, so a failed download leaves the message queued, then sends the `DELETE`; on success the bubble goes, its text goes back into that task's composer ahead of whatever is there, separated by a blank line, and its images are added back as composer attachments. A `user.unqueued` event removes the bubble with its `id` on every other view. A refusal (the agent already has the message) shows as an alert and the bubble stays until the `user.message` clears it. +Messages byre itself sends an agent (actor `byre`: the restart notice, Sync and Merge prompts, Compact's handoff request) show collapsed like a tool call, labelled `byre` with the first line as the brief; expanding it shows the full text. Review findings forwarded to the agent come this way too, so the transcript shows them separately: a `review.finished` event with findings draws its counts note with the review's `summary` (one bullet per finding) expanded under it, behind a border-coloured rule, and then each finding as its own row (see "Reviews"). The transcript view has a border in the task's colour (see "Task colours"; the accent in the lead pane) while the session is `running` and its transcript has an open turn (`open` in `TranscriptBuilder`: a drawn turn-starting `user.message`, not mid-turn, with no `turn.ended` after it) or the task's state is `review_pending` or `reviewing` (see "Task state in the store"), and, only in the first case, a Cylon-style scanner in the same colour: a row of 2pt dots, 5pt apart along an 84pt track, where a glow sweeping back and forth every 1.2s lights up the dots under it, drawn below the last message, in the transcript's bottom padding, so it scrolls with the transcript and sits just above the composer's top border when scrolled to the bottom. It is an overlay 8pt below the transcript's last row, so showing or hiding it never changes the transcript's scroll height or position, and it is drawn with Core Animation (`ScannerView` in `mac/Sources/Byre/Chat.swift`: a gradient layer masked by the dots, its position animated by a repeating `CABasicAnimation`), so the render server animates it and SwiftUI never redraws the transcript for it. The session list (`GET /api/me/lead/sessions` or the task's sessions) is fetched separately from the event stream, so it says `running` a little before the turn's `user.message` arrives and a little after its `turn.ended`; requiring the open turn as well keeps the scanner and border from flashing on in those gaps. Consecutive tool calls collapse into one group whose summary counts calls per tool and file, and file paths show relative to the worktree. Thinking output appears as expanded Markdown text with a small muted "thinking" label and ends the current tool-call group just like an agent message, including inside subagent rows. While the scanner shows, a tool call still waiting for its result, and any group or subagent row holding one, gets a border in the task's colour, so a running tool reads apart from waiting on the model without expanding anything (`working` in `mac/Sources/Byre/Chat.swift`). The braille spinner the Project tab shows for task states is drawn with Core Animation too (`SpinnerView` in `mac/Sources/Byre/Format.swift`: its ten glyphs rendered once into images in the surrounding font and colour, cycled every 80ms by a repeating discrete `CAKeyframeAnimation` on the layer's contents), so it never makes SwiftUI redraw or lay out the views around it. When a turn ends, or a turn-starting `user.message` (not mid-turn) arrives, any call still without a result, other than those inside an active subagent row (below), reads "no result" instead of "running…", so an interrupted call never spins during a later turn. The second case covers a turn a restart cut off: it has no `turn.ended`, and the restart notice starts the next turn (`endTools` in `mac/Sources/Byre/TranscriptBuilder.swift`). byre's own tools show without their `mcp__byre__` prefix, in title case (`mcp__byre__create_task` shows as Create Task; `toolLabel` in `mac/Sources/Byre/TranscriptBuilder.swift`); other tools show under the name the agent used. A Claude Code subagent call (the `Agent` or `Task` tool) stands on its own, collapsed, labelled with its `subagent_type` and `description`; expanding it shows the prompt and the call's result. Its summary carries a status from the `agent.vendor` `system` events whose `tool_use_id` names it (`task_started`: running, or running in the background; `task_progress`: its `description`; `task_notification`: its `status`, red unless `completed`). From its `task_started` to its `task_notification` the row is active, and an active row gets the task-coloured border while the scanner shows or while the session's `subagents` count is above zero, as do the waiting tool calls inside it in the latter case; so a background subagent still reads as running after the turn that started it has ended. A turn's end leaves the waiting calls inside an active row waiting; the row's end (its `task_notification`, or a `session.subagents` event with 0, which ends every still-active row as "ended with its process", replayed history included) marks the calls inside it still waiting "no result". The subagent's own messages, thinking and tool calls (events whose `parent_tool_use_id` names the call) go inside that call's row, between the prompt and the result, so parallel subagents don't interleave in the main stream; its consecutive tool calls group there the same way, its messages get an accent rule down their left edge, and a subagent it starts nests inside it in turn. An event naming a subagent call the transcript never saw stays in the main stream marked `↳ subagent`. A subagent call ends the main agent's run of tool calls, so it never joins a group. A subagent's messages never end the main agent's live message, count as a turn's final message or hide a quiet turn. Claude Code streams a background subagent's activity, but for a foreground one only its prompt, so a foreground subagent shows only as its call, its status and its result. Selecting a task that has never had a session focuses the composer, and so does picking any task in the ⌘K switcher when it lands on the transcript (`Store.composerFocus`). In the composer, Enter sends and Shift-Enter adds a newline; unsent text is kept per task (see "Drafts" below), so it survives switching tabs or tasks and relaunching the app. The composer has no `@`-file mentions or path picker: text reaches the agent exactly as typed (stream-json doesn't expand `@path` the way the interactive CLI does), and the agent reads any file the message names with its own tools. The composer is the same whether or not the task has a live session, and stays enabled while a turn runs: Send posts to the session's messages, or starts a session with the text when there is none or the last one completed. A sent message shows at once as a bubble reading "queued…" while the session is running and "sending…" otherwise, replaced by the server's `user.message` event with the same text when it arrives, and removed (text and images back in the composer) if the request fails. A queued message's `user.queued` event takes over its bubble (the oldest local one with the same text), and the transcript shows a "queued…" bubble below everything else for every `user.queued` not yet followed by a `user.message` with the same text, each `user.message` clearing the oldest such bubble first (queued messages are delivered oldest first), then a local one, and `task.merged` or `task.discarded` clearing them all, since closing the task deletes its queue, so a queued message stays visible across switching tasks, relaunching the app, and on every view. A bubble from a `user.queued` event has a × that withdraws the message (`ChatModel.withdraw`): it first downloads the message's images, so a failed download leaves the message queued, then sends the `DELETE`; on success the bubble goes, its text goes back into that task's composer ahead of whatever is there, separated by a blank line, and its images are added back as composer attachments. A `user.unqueued` event removes the bubble with its `id` on every other view. A refusal (the agent already has the message) shows as an alert and the bubble stays until the `user.message` clears it. **Drafts.** Unsent composer text (task and lead) and unsent inbox answers (checked choices and Other text) are kept in memory by `Drafts` (`mac/Sources/Byre/Drafts.swift`) and saved to a SwiftData `DraftRecord` per server and login (`docs/macos.md`, "Drafts"), one JSON object with `composer` (keyed by `message:` or `message:lead`, each `{text, images}`), `checked` and `texts` (both keyed by inbox item id), and `comments`, the Changes view's pending line comments keyed by task id, each `{anchor: {path, line, old, quote}, text}`. A comment shows only under a diff line whose path, line number, side and text all still match, so after the agent moves lines a comment never attaches to a different line. Every pending comment not shown under a line (its file closed or not loaded, or its line gone from the diff) is listed at the top of the Changes view with its `path:line`, the quoted line, its editable text and **remove**, so nothing is sent unseen. Writes are debounced (400 ms) and flushed at once when a composer or answer box loses focus, on send and on quit. A composer draft is cleared only once its send succeeds, and an inbox answer only once its response is accepted; each send in flight is tracked on its own and the stored draft holds the text of every unconfirmed send (oldest first, then the composer's current text, separated by blank lines) and counts its images, so a relaunch mid-send brings back everything not yet confirmed. A failed send's text goes back into the composer ahead of whatever is there now (text typed since, or earlier failures), separated by a blank line, rather than replacing it. The composer follows its `Drafts` text as it changes, so text put back from outside it (a withdrawn queued message, a draft synced in from another Mac) shows at once. Drafts for tasks that are no longer active and for inbox items no longer open are pruned whenever the task list or inbox reloads. A record that doesn't decode is logged and ignored, as is a write that fails; drafts then live only in memory. Pending images are not stored, only their count: after a relaunch the composer shows how many need re-attaching, until dismissed or the next successful send. @@ -457,7 +457,7 @@ Every new snapshot of a task gets a review: a separate agent session on the vend - **Events.** `review.started`, `review.finished` (state `done`, `failed` or `skipped`; a skipped review emits only `review.finished`) and `review.finding` (a finding's state set, with the user as actor) 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.** Reviews live in the task's transcript (`renderReview` in `mac/Sources/Byre/TranscriptBuilder.swift`, rows in `mac/Sources/Byre/Reviews.swift`). A finished review is a note at its `review.finished` event, "review of snapshot #n:" with its state or severity counts or error; with findings, the note carries the summary and screenshots, and each finding follows as its own row, numbered from 1 as in the message to the agent. The `change` field is not shown; it goes to the lead and into notes. 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 against the default branch (`changes?snapshot=`, 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. Clicking a screenshot opens it full size. A finding with no answer from the user or the agent has **fix** and **ignore** buttons. **fix** sends the finding's number, severity, title, location and detail to the task's agent as a message from the user: through `POST /api/sessions/{id}/messages` to its latest live session (queued like any message while the agent is mid-turn, see "Agent sessions"), or starting a session when it has none; the view stays where it is, so several findings can be sent in a row. Then it, or **ignore** on its own, records the finding's state (`fixed` or `ignored`) with `PUT /api/reviews/{reviewId}/findings/{index}`, the index being the finding's position in `result.findings`; the review's `finding_states` (the `reviews.finding_states` column, a JSON object from index to state) holds them, and the buttons give way to "fix sent" or "ignored". Only a `done` review's findings take a state; the user can't clear one, and only the agent's answer clears a `fixed` one (below). - **Agent responses.** The message forwarding findings to the agent numbers them and tells it to answer each with the task agent's `respond_to_finding` MCP tool (`respondToFindingTool` in `internal/agents/review.go`) before changing anything: `snapshot`, `finding` (from 1), `response` (`fixing` or `ignoring`) and a one-line `reason`. byre records it in the review's `finding_responses` (the `reviews.finding_responses` column, a JSON object from index to `{response, reason}`; a later call for the same finding replaces it) and emits `review.finding` with actor `agent`. The answer also clears the finding's `fixed` state, since it answers that fix request: an ignore after the user's **fix** brings **fix** back so the user can overrule it again. A user's `ignored` stays. A call naming a snapshot with no `done` review, or a finding it doesn't have, is refused. On the finding, an answer shows as "agent fixing: …" or "agent ignoring: …" and dims the finding's diff, detail and screenshots. Fixing takes away both buttons; ignoring keeps **fix**, so the user can overrule it, and drops **ignore**. The tool call itself is its own transcript row (not folded into a group of tool calls): what the agent answered for which finding, its title and reason, a **show** button that scrolls to the finding, and, for an ignore, the same **fix** button while the user hasn't answered the finding. A call that fails is hidden. The Merge confirmation's review warning counts findings the user hasn't ignored, whatever the agent answered. -- **Snapshot row.** Each snapshot row's **review** button shows that snapshot's latest `review.*` event: a fill in the accent tint (`Palette.accentTint`, the same in every task) while the review runs, and otherwise a mark over its top-right corner: a grey ● when it was skipped, a green ✓ when it finished with no open findings (none, or each answered by the user or the agent), and, when it has open findings, the button highlighted with their count in a badge. A failed review leaves the button unmarked. While any of a task's reviews is `running` and its agent is idle, the task's state is `reviewing` (see "Task state in the store"), which the task switcher and the Project tab show as **Reviewing** (see "Task state"). `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. +- **Snapshot row.** Each snapshot row's **review** button shows that snapshot's latest `review.*` event: a border in the task's colour while the review runs, and otherwise a mark over its top-right corner: a grey ● when it was skipped, a green ✓ when it finished with no open findings (none, or each answered by the user or the agent), and, when it has open findings, the button highlighted with their count in a badge. A failed review leaves the button unmarked. While any of a task's reviews is `running` and its agent is idle, the task's state is `reviewing` (see "Task state in the store"), which the task switcher and the Project tab show as **Reviewing** (see "Task state"). `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. ## Workflows diff --git a/mac/Sources/Byre/Chat.swift b/mac/Sources/Byre/Chat.swift index c858f4b..2ae27d6 100644 --- a/mac/Sources/Byre/Chat.swift +++ b/mac/Sources/Byre/Chat.swift @@ -125,6 +125,11 @@ final class ChatModel { builder.open && session?.state == .running } + var reviewing: Bool { + let state = taskId.flatMap { store.findTask($0) }?.state + return state == .reviewing || state == .reviewPending + } + var subagents: Bool { (session?.subagents ?? 0) > 0 } @@ -742,6 +747,7 @@ private struct TranscriptScroll: View { var body: some View { let running = model.running + let active = running || model.reviewing ScrollView { VStack(alignment: .leading, spacing: 0) { if model.key == nil { @@ -845,8 +851,8 @@ private struct TranscriptScroll: View { } } .overlay { - Rectangle().stroke(running ? accent : .clear, lineWidth: 1) - .animation(.easeInOut(duration: 0.3), value: running) + Rectangle().stroke(active ? accent : .clear, lineWidth: 1) + .animation(.easeInOut(duration: 0.3), value: active) } .overlay(alignment: .topTrailing) { AttentionBar(model: model) @@ -1581,6 +1587,7 @@ private struct SnapshotRow: View { let showReview: () -> Void @Environment(Store.self) private var store @Environment(Overlays.self) private var overlays + @Environment(\.paneAccent) private var accent @ViewState private var forking = false var body: some View { @@ -1658,7 +1665,7 @@ private struct SnapshotRow: View { } } } - .buttonStyle(SmallLinkButtonStyle(color: n > 0 ? Palette.warn : Palette.accent, fill: review?.state == .running ? Palette.accentTint : .clear)) + .buttonStyle(SmallLinkButtonStyle(color: n > 0 ? Palette.warn : Palette.accent, border: review?.state == .running ? accent : Palette.border)) .disabled(review == nil || review?.state == .running) .help(review?.state == .running ? "Review in progress" : review == nil ? "Not reviewed" : "Scroll to the review of snapshot \(snap.n)") } diff --git a/mac/Sources/Byre/Format.swift b/mac/Sources/Byre/Format.swift index 33f88f6..3fa02b4 100644 --- a/mac/Sources/Byre/Format.swift +++ b/mac/Sources/Byre/Format.swift @@ -403,7 +403,6 @@ private struct WebButton: View { struct SmallLinkButtonStyle: ButtonStyle { var color = Palette.accent var border = Palette.border - var fill = Color.clear @Environment(\.isEnabled) private var enabled func makeBody(configuration: ButtonStyleConfiguration) -> some View { @@ -411,7 +410,6 @@ struct SmallLinkButtonStyle: ButtonStyle { .font(Fonts.font(11)) .foregroundStyle(color) .padding(.horizontal, 6) - .background(fill, in: RoundedRectangle(cornerRadius: 4)) .overlay(RoundedRectangle(cornerRadius: 4).stroke(border)) .opacity(enabled ? 1 : 0.45) .contentShape(Rectangle())