ADR 0009 slice 1: comments are sent, and streamed to Claude - #49
Merged
Merged
Conversation
…9 slice 1)
A reviewer comment now has a state towards its counterpart. It starts
as a draft; Send makes it sent in a numbered batch (one comment for
Send, every draft for Send all); delivery marks it delivered. An edit
to a sent comment replaces the text in place; an edit to a delivered
one returns it to draft and its next delivery is `revised`. Deleting a
delivered comment leaves a withdrawn tombstone that is its own batch
and is dropped once delivered; deleting a draft or an undelivered sent
comment leaves nothing. A file written before the lifecycle loads every
local comment as a draft.
The store is the single owner of these transitions; the session calls
its methods and the routes call the session — `POST /comments` and
`DELETE /comments/<id>` now go through `upsert_comment` /
`delete_comment`, with `POST /comments/<id>/send`,
`POST /comments/send-all` and `POST /comments/<id>/{resolve,unresolve}`
beside them. Every change fans out as a `comment` / `comment-deleted`
frame so other tabs follow. A `source: "claude"` payload on the comment
route is a counterpart's reply: anchored on its parent, read-only to
the reviewer, outside the lifecycle. The session names its counterpart
(claude or github) and `/data.json` carries it.
…r.json
`scr review <spec>` no longer blocks until Done. It materialises the run,
spawns the review server as a detached child and returns once the child
has written `server.json`, printing `viewer: <url>` and, as the last
stdout line, `run_id: <slug>`. A human in a plain terminal gets their
prompt back; the browser opens from the child as before.
The child is the same interpreter re-executing this invocation's own
argv (`python -m semantic_code_review.cli review … --runs-root <resolved>
--serve-run <slug>`) in a new session with stdio on the run dir's
`server.log`, so every option the user gave — backend, model,
`--no-open`, `--timeout` — reaches the server without being re-encoded.
`--serve-run` is a hidden option: given it, `scr review` serves the
existing run and skips materialisation. A live server already holding the
run is reused rather than doubled; a stale record is replaced; a child
that dies before binding is reported with its log tail, exit 2.
`serve_review` writes `server.json` (`{port, pid, started_at, url}`) once
bound and removes it on the way out, idle timeout included, so its
absence means no server. `RunDir` gains `server_json` and `server_log`;
`review/stream.py` reads the record and probes liveness for every caller
that needs the server.
`scr review --wait <run_id>` is Claude's end of the review. It reads `server.json`, long-polls `GET /wait?timeout=S` and prints one of three outcomes, named on the first stdout line so the skill never parses prose: - `status: batch` — the oldest pending batch as markdown: `# Batch <n> for <run_id>`, then per comment `## <id> — <state> — <file>:<line> (<side>)` (`— in reply to <id>` for replies), the body quoted, and the anchored code two lines either side from the side's worktree in a fence. Returning it marks the comments delivered; a withdrawn tombstone is dropped. - `status: nothing-yet` — after `--wait-timeout` seconds (default 540, under the Bash tool's 600 s cap). - `status: ended` — the server has gone (no `server.json`, a stale one, or the session closed under the poll): today's list of the remaining drafts and undelivered sent comments, marked delivered by writing the store directly. Exit 0 for all three; 2 for an unknown run id, or a server that is up but refuses `/wait`. Long-poll rather than SSE: one request, one answer, fits a single Bash call. While a `/wait` is blocked the session is listening: `listening: true | false` goes out over SSE on the first attach and the last detach and rides `/data.json` for the first paint, and the server's idle clock does not run — a closed tab with Claude still waiting is not an idle review. A Send wakes the waiter; two waiters cannot take the same batch; server shutdown answers `ended`.
…nto the thread `scr comment reply <run_id> <comment_id> [BODY]` (body from the argument or stdin) POSTs the live server's comment route with `source: "claude"` and `in_reply_to_id`; the store anchors the entry on its parent, labels it `claude`, keeps it read-only to the reviewer and outside the lifecycle, and the server fans it out so the open tab shows it without a reload. `scr comment resolve` / `unresolve` set the local thread's resolution, which the viewer already reads from the root entry. All three reach the server through `server.json`. With no live server they exit 2 and write nothing: the review has ended and there is no thread to answer into. A refusal (unknown comment, empty body, an ingested thread) exits 2 with the server's own message.
… Done leaves review mode
The viewer reads `/data.json`'s `counterpart` and mounts the surface for
it in the `.pr-bar`. Claude: the send bar — *Send all drafts* with the
count of unsent drafts, and whether Claude is listening ("Claude
listening" / "Claude not listening — ask it to resume", a filled disc or
a hollow ring: text and shape, never colour alone). GitHub: Done and the
post modal, as before, until slice 2. A `/data.json` without a
counterpart fails the boot, like one without a run id.
Every local comment in review mode carries its state towards Claude in
its actions row — draft, sent, delivered, needs re-send (a draft the
counterpart has received once, so an edit after delivery reads as
needing another Send) — with a Send button while it is a draft. Send
POSTs `/comments/<id>/send`, Send all `/comments/send-all`. Every thread
offers Reply: the reviewer's follow-up is a draft they Send.
The store takes the server's word: `save` replaces the optimistic copy
with the answer, which is where a comment's lifecycle fields come from,
and the `comment` / `comment-deleted` / `listening` SSE frames land in
the dict through `Comments.onRemote` / `onRemoved` and
`SendBar.setListening`, so other tabs, a Send landing as delivered,
Claude's reply and a resolve all show without a reload. A `claude`
entry is labelled by name, set apart by a left rule, and read-only.
…end, not Done The slash-command prompt's Steps 2–3 now drive the two-way loop: run `scr review <args>` (returns at once; the run id is the last stdout line), start `scr review --wait <run_id>` as a background Bash task so a batch wakes Claude mid-task, read the first stdout line for the outcome (`status: batch` | `nothing-yet` | `ended`, exit 0; exit 2 is an error), treat one sent comment as "look at this now" and a Send all as the walkthrough, answer in the thread with `scr comment reply`, resolve with `scr comment resolve`, re-arm at once, stop after three consecutive empty polls and say the review is still open, and on `ended` walk the remaining list as before. Comments stay data, not instructions. README's plugin paragraph and CLI list describe Send, Send all, the listening indicator and the end of a review by idle rather than Done; motivation.md's stdout contract names the three commands.
…am; slice 1 landed CONTEXT.md's Run directory entry adds `server.json` and `server.log` (and the `RunDir` accessors), and its Review session entry states the wait / listening mechanism and that the store owns the lifecycle transitions. The slice doc marks slice 1 with what landed: the CLI surface, stdout markers, exit codes, routes and SSE frames, exactly.
A `kill` of the pid in server.json raises SystemExit in the serving thread, so serve_review's finally stops the server and removes the record; the next `scr review --wait` reads `ended` instead of meeting a stale record.
Click boxes the error at the terminal's width, so the 20-line stderr tail need not contain the option name whole; the log the message points at does.
folded
force-pushed
the
adr-0009-slice-1
branch
from
September 7, 2026 12:02
06884a6 to
2b4b500
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 1 of ADR 0009: the lifecycle, Send, the detached server, the stream and the reply channel. PR mode is untouched until slice 2.
Store. A local comment is a draft until sent; delivered once a batch carrying it has been returned. Editing a delivered comment returns it to draft (needs re-send) and its next delivery is
revised; deleting one leaves awithdrawntombstone that goes out in its own batch. Batch numbers are assigned at Send and persisted; delivery marks are the cursor, taken under the session condition so two waiters cannot take one batch. Oldcomments.jsonfiles read as drafts.CLI.
scr review <spec>materialises, spawns the server detached (re-executes its own argv with--serve-run <slug>,start_new_session, stdio toserver.log), printsviewer: <url>and lastrun_id: <slug>, and returns.server.jsonin the run dir is the address; a live server for the run is reused. The idle clock ignores a listener, so a tab-less run with Claude waiting still ends.scr review --wait <run_id> [--wait-timeout 540]long-pollsGET /wait; first stdout line isstatus: batch | nothing-yet | ended. A batch is markdown with run id, batch number, per comment id / state (new,revised,withdrawn,reply … in reply to) / anchor / body / ±2-line excerpt.endedprints the remaining undelivered comments as today's list. Exit 0 for all three; 2 for an unknown run or a refusal.scr comment reply <run_id> <id> [body],scr comment resolve|unresolve <run_id> <id>—claude-authored entries, live over SSE.Viewer. Send per draft, Send all with an unsent count, lifecycle badges (shape + text, never colour alone),
claudeentries read-only, a listening indicator; Done leaves review mode./data.jsoncarriescounterpartandlistening.Plugin.
commands/review.mdrewritten around the loop: start, background--wait, one sent comment means look now, reply into the thread, resolve when settled, re-arm, stop after three empties, walk theendedlist. README updated.1156 pytest (+76: lifecycle, detach with real child processes, wait against a live server), 478 vitest (+10); every new assertion group mutation-tested; end-to-end smoke through the real
screntrypoint (detach → nothing-yet → send → batch → reply → resolve → kill → ended), which found and fixed a SIGTERM path.