From c969a924be34ff2464f439bb9b4deca8720eb264 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Fri, 4 Sep 2026 09:36:32 +0200 Subject: [PATCH 01/11] feat(cockpit): write the review's own comment yourself, in the send panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The body GitHub gets was composed and only composed — summary, walkthrough, folded notes, footer — so the one comment on a review that is not attached to a line was the one thing the user could not write. Now the send preview has an editor: it opens seeded with the composed body, and what you save is exactly what posts, footer and all. Half-honouring a hand-written body — keeping a footer you deleted, re-appending notes you cut — would post something nobody wrote. It changes the body alone. Inline comments still post, the event still applies, and the draft underneath is untouched: `build it from the review again` is one click, and a re-review clears the override the same way it drops the pre-chat snapshot. A chat turn never writes one, so writing or clearing one while a turn runs is the user's and stands. Because this is the one place what GitHub gets stops being derived from what the cockpit shows, the panel says so wherever it describes the payload — the strip reads "body you wrote" instead of claiming folded comments the body may no longer carry, the note under it says it no longer follows the summary, and a body you wrote opens shown rather than behind "see what gets posted". Two things go from the panel while it is being honest about what posts, both of them saying twice what is already said once: the "send as…" switch and its "follows the verdict" line, since the verdict buttons sit directly above the button and the way to post an approve is to say the review approves; and the verdict bar's "what you send follows this unless you say otherwise", which explained the switch. `cerber send -e` keeps its override — a terminal has no verdict buttons above it. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 11 ++ SPEC.md | 45 ++++++-- docs/lifecycle.md | 10 +- src/core/artifact.ts | 13 +++ src/core/autosend.test.ts | 1 + src/core/history.test.ts | 8 ++ src/core/history.ts | 9 ++ src/core/refresh.test.ts | 1 + src/core/revise.test.ts | 9 ++ src/core/revise.ts | 3 + src/core/send.test.ts | 32 +++++- src/core/send.ts | 8 +- src/core/state.test.ts | 1 + src/runner/chat.test.ts | 1 + src/runner/review-merge.test.ts | 1 + src/runner/review.test.ts | 1 + src/runner/review.ts | 3 + src/server/chat.test.ts | 1 + src/server/daemon.ts | 1 + src/server/guards.test.ts | 30 ++++++ src/server/index.ts | 7 ++ web/src/Detail.tsx | 182 ++++++++++++++++++++++---------- web/src/api.ts | 10 +- web/src/review.test.ts | 10 ++ web/src/review.ts | 7 ++ web/src/styles.css | 100 ++++++------------ web/src/types.ts | 6 ++ 27 files changed, 374 insertions(+), 137 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index adb31a7..741ab9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -192,6 +192,17 @@ left unwritten. Code and tests win when they disagree. rather than overwriting whatever they edited while the turn ran. The difference is deliberate: a re-review is asking for a new draft, a chat turn is asking for an edit to this one +- Don't put a second control over one decision. What Send posts follows the + verdict and the verdict buttons are right above it, so the cockpit has no + "send as…" switch beside the button — the way to post an approve is to say + the review approves. The summary is cerber's own reading of the PR, not + something to hand-edit; the body that posts is the thing worth editing. +- Don't let what posts drift from what the cockpit shows without saying so. + The GitHub body is composed from the draft (summary, walkthrough, the + comments that can't post inline, footer). The one exception is a body the + user wrote by hand in the send panel (`bodyOverride`) — it replaces the + composed one outright, footer included, and every line of that panel that + describes the payload has to say which of the two is about to post. - Don't add an accept step to a revision the user asked for. The chat agent writes the draft directly; Send is where a human vouches for what reaches GitHub, and one pre-chat snapshot is the way back. A per-turn undo is diff --git a/SPEC.md b/SPEC.md index 6a4c7ff..e067e44 100644 --- a/SPEC.md +++ b/SPEC.md @@ -182,6 +182,7 @@ absent on read and materialize with the stated value. | `chapters` | Chapter[] | default `[]` | | `comments` | Comment[] | default `[]` | | `verdict` | Verdict \| null | default `null` | +| `bodyOverride` | string \| null | default `null`; the review body to post, written by hand — null means composed at send time (§14.4) | | `run` | RunInfo \| null | default `null` | | `sent` | SentInfo \| null | default `null` | | `filed` | FiledInfo \| null | default `null`; never set on a sent artifact | @@ -389,9 +390,10 @@ review began at that moment. and next artifacts over a fixed watchlist — appearance in the inbox, status changes, head movement, PR state/draft flips, run start (with its shape: model, source, trust, who asked) and finish (reviewed SHA, cost) or failure, -verdict set/changed, comment churn summarized as one line (added from the -review / written by you / edited / re-graded / dropped / restored / gone), -send, filing, refresh. A generic diff would bury the timeline under a running +verdict set/changed, the body to post leaving the review's composition or +returning to it, comment churn summarized as one line +(added from the review / written by you / edited / re-graded / dropped / +restored / gone), send, filing, refresh. A generic diff would bury the timeline under a running turn's narration, which is rewritten every couple of seconds. **Decision notes.** The poll's deliberate silences — the only kind of history @@ -621,8 +623,10 @@ another look — and then a new head means a new run. ### 8.5 What a Re-Review Destroys and What Survives -A re-review replaces the whole draft — summary, chapters, verdict, and **all -comments, the user's own included**. That is a decision, not a gap: +A re-review replaces the whole draft — summary, chapters, verdict, a +hand-written `bodyOverride` (it described a body for a +draft that no longer exists — the same reason the pre-chat snapshot goes), and +**all comments, the user's own included**. That is a decision, not a gap: half-keeping them (carrying on success, losing on failure) costs the code and still loses the work, so cerber does neither and says so plainly. @@ -1112,7 +1116,8 @@ folded onto the *current* artifact, three-way (`before` = at turn start, the first rule. - `status` is always `current`'s — a "mark reviewed" clicked mid-turn stands. - The verdict is the turn's only if the turn actually revised it; otherwise - `current`'s. + `current`'s. `bodyOverride` is always `current`'s — a turn never writes one, so writing or + clearing one mid-turn is the user's decision and stands. ### 12.6 Snapshot and Reset @@ -1225,6 +1230,14 @@ Payload construction (pure, previewable without side effects): - The body is `## Summary`, then `## Walkthrough` (chapter titles and explanations), then the folded notes, then a fixed footer crediting cerber: "drafted by AI, sent by a human." +- **Unless the user wrote one.** A non-null `bodyOverride` replaces the + composed body outright — footer and folded notes included. Half-honouring it + (keeping a footer they deleted, re-appending notes they cut) would post + something nobody wrote. It changes the body alone: inline comments still + post, the event still applies, and the draft it replaced is untouched and + can compose the body again at any time. The split into inline and folded is + still computed, because the cockpit needs it to say which comments have no + line of their own to land on. - Grades are rendered into comment text via the one badge function (§7.4). - `commit_id` is the artifact's `pr.headSha` — so a review of a stale head fails with a 422 rather than landing inline comments on the wrong code. @@ -1311,7 +1324,7 @@ static assets included. No CORS: same-origin only. | `GET /api/reviews` | queue list items | derived counts: comments, drifted, blockers, graded | | `POST /api/reviews` | pull a PR in by URL/ref | **202** + artifact (run started); 200 existing; 409 in flight; 502 fetch failed, nothing left behind | | `GET /api/reviews/:key` | one artifact | 404 | -| `PATCH /api/reviews/:key` | settle | only `reviewed`/`skipped` accepted (§8.1); stamps `settledAt`, clears `filed` | +| `PATCH /api/reviews/:key` | settle · set the verdict · write the body to post | only `reviewed`/`skipped` accepted (§8.1); stamps `settledAt`, clears `filed`; `bodyOverride` takes a string or `null` (back to composed) | | `POST/PATCH/DELETE …/comments[/:id]` | comment CRUD | delete is user-origin only in the UI | | `POST …/refresh` | §13.2 | `{stale, changed, …}`; never an error for "nothing to do" | | `POST …/rerun?source=0\|1` | re-review, always forced | **202**; 409 sent; 409 in flight | @@ -1423,6 +1436,11 @@ one is reported over the next. or request-changes with none), the cockpit points it out and offers a one-click chat turn asking the reviewer to re-true the verdict. It MUST NOT rewrite the verdict itself. +- What Send posts follows the verdict, with nothing in between and no second + control for it: the verdict buttons sit directly above the button, so a + "send as…" switch beside it would be two controls over one decision — and + the way to post an approve is to say the review approves. (`cerber send -e` + keeps its own override; a terminal has no verdict buttons above it.) - A comment that cannot post inline — drifted, or with no line at all, which is what a comment on a line the PR *removed* becomes, GitHub taking inline comments on the new side only — MUST say so where it is read; nothing else @@ -1442,6 +1460,19 @@ one is reported over the next. that is the body GitHub gets. It MUST be suppressed when the render reads back word-for-word as the source (whitespace runs flattened): a plain note is told nothing by a second copy of itself. +- The review body — GitHub's own comment on the review, the one part of the + payload attached to no line — is writable by hand from the send panel, which + is where the user is standing when they read that it says the wrong thing. + The editor MUST be seeded with the composed body, so writing one starts from + what would otherwise post. Because this is the one place what GitHub gets + stops being derived from what the cockpit shows, the panel MUST say which of + the two is about to be posted wherever it says anything about the payload: + the body strip says who wrote it (never "N folded into the body" over a body + the user may have cut those notes out of), the note under it says the body no + longer follows the summary or the comments, and building it from the review + again is always one click away. A body the user wrote MUST open shown rather + than behind "see what gets posted" — it is the one fact about the payload + that nothing else on the page carries. - The review's history renders as a collapsed card at the foot of the review, newest first — it is what you open when a review is not where you expected it, not part of reading one — with a rail jump that opens it on the way. An diff --git a/docs/lifecycle.md b/docs/lifecycle.md index 77243ca..4b79a64 100644 --- a/docs/lifecycle.md +++ b/docs/lifecycle.md @@ -194,8 +194,8 @@ push convinced the guard the draft was current and the poll never re-reviewed that PR again. An artifact written before `reviewedSha` existed has none, and falls back to the old comparison. -**A re-review replaces the whole draft, including comments you wrote.** They -are dropped when the run starts and they do not come back — not on success, not +**A re-review replaces the whole draft, including comments you wrote and a send +body you wrote yourself.** They are dropped when the run starts and they do not come back — not on success, not if the run fails. This is a decision rather than an oversight, and it is stated here rather than left to be discovered: if you have written comments you want to keep, send the review or copy them out before pressing re-review. @@ -414,6 +414,12 @@ Written by other paths: - Every comment edit, add and drop is saved immediately (`editedByUser` is set when you rewrite an AI comment — it is the calibration signal *and* what makes the comment survive a re-review). +- A review body you write yourself (`bodyOverride`, from the send panel) is + stored beside the draft rather than replacing it: the summary, walkthrough and + comments stay as they read, and clearing the field composes the body again. + While it is set, that text is what posts — the folded comments included in the + composed body are only sent if the body you wrote kept them, which is why the + panel stops claiming them. A re-review clears it. - A chat turn writes `pendingChat` **before** it starts (so a reload mid-turn still shows it), streams `pendingChat.progress`, then folds its result onto whatever the artifact says now (`mergeConcurrentEdits`) and appends a diff --git a/src/core/artifact.ts b/src/core/artifact.ts index 6e01f36..9ad0c8d 100644 --- a/src/core/artifact.ts +++ b/src/core/artifact.ts @@ -312,6 +312,19 @@ export const ArtifactSchema = z.object({ chapters: z.array(ChapterSchema).default([]), comments: z.array(CommentSchema).default([]), verdict: VerdictSchema.nullable().default(null), + /** + * The review body to post, written by hand. Null — the normal case — means + * the body is composed from the draft at send time (§14.4: summary, + * walkthrough, the comments that cannot post inline, footer). + * + * Everywhere else in cerber, what GitHub gets is derived from what the + * cockpit shows. This is the one place the user can cut that link, so it is + * a field rather than a rewrite of the summary: the draft underneath stays + * exactly as the review wrote it, the body can always be built again, and + * the send panel says which of the two is about to be posted. A re-review + * clears it — it described a body for a draft that no longer exists. + */ + bodyOverride: z.string().nullable().default(null), run: RunInfoSchema.nullable().default(null), /** Set once the review was sent to GitHub (explicitly, or via opt-in auto-send). */ sent: SentInfoSchema.nullable().default(null), diff --git a/src/core/autosend.test.ts b/src/core/autosend.test.ts index af092f6..bee0ee6 100644 --- a/src/core/autosend.test.ts +++ b/src/core/autosend.test.ts @@ -32,6 +32,7 @@ function makeArtifact(overrides: Partial = {}): Artifact { chapters: [], comments: [], verdict: { recommendation: "approve", confidence: 95, reasoning: "clean" }, + bodyOverride: null, run: null, sent: null, refresh: null, diff --git a/src/core/history.test.ts b/src/core/history.test.ts index f81943b..7bd5aaa 100644 --- a/src/core/history.test.ts +++ b/src/core/history.test.ts @@ -31,6 +31,7 @@ function artifact(over: Partial = {}): Artifact { chapters: [], comments: [], verdict: null, + bodyOverride: null, run: null, sent: null, refresh: null, @@ -89,6 +90,13 @@ describe("describeChange", () => { ]); }); + it("records the body that posts leaving the review, and coming back", () => { + const plain = artifact(); + const own = artifact({ bodyOverride: "Ran it locally, ship it." }); + expect(describeChange(plain, own)).toEqual(["the body to post was written by hand"]); + expect(describeChange(own, plain)).toEqual(["the body to post follows the review again"]); + }); + it("records a push under the review", () => { const before = artifact(); const after = artifact({ pr: { ...before.pr, headSha: "5502944aaaa" } }); diff --git a/src/core/history.ts b/src/core/history.ts index 07e79b2..06d1589 100644 --- a/src/core/history.ts +++ b/src/core/history.ts @@ -176,6 +176,15 @@ export function describeChange(before: Artifact | null, after: Artifact): string ); } + // What posts stopped following the draft, or started following it again. + if ((before?.bodyOverride ?? null) !== (after.bodyOverride ?? null)) { + lines.push( + after.bodyOverride == null + ? "the body to post follows the review again" + : "the body to post was written by hand", + ); + } + const comments = describeComments(before?.comments ?? [], after.comments); if (comments) lines.push(comments); diff --git a/src/core/refresh.test.ts b/src/core/refresh.test.ts index ed428db..cf8dd8a 100644 --- a/src/core/refresh.test.ts +++ b/src/core/refresh.test.ts @@ -66,6 +66,7 @@ function makeArtifact(overrides: Partial = {}): Artifact { chapters: [], comments: [], verdict: null, + bodyOverride: null, run: null, sent: null, refresh: null, diff --git a/src/core/revise.test.ts b/src/core/revise.test.ts index b1febc4..3ecb214 100644 --- a/src/core/revise.test.ts +++ b/src/core/revise.test.ts @@ -48,6 +48,7 @@ function artifact(over: Partial = {}): Artifact { chapters: [{ id: "one", title: "One", explanation: "Explains one.", files: ["src/a.ts"] }], comments: [comment()], verdict: { recommendation: "comment", confidence: 70, reasoning: "because" }, + bodyOverride: null, run: null, sent: null, refresh: null, @@ -331,6 +332,14 @@ describe("mergeConcurrentEdits", () => { expect(merged.verdict).toEqual(mine); }); + it("keeps a send body the user wrote while the turn was running", () => { + // A turn never writes one, so whatever the artifact says now is the user's. + const b = before(); + const { artifact: after } = applyRevisions(b, [{ kind: "summary", body: "Rewritten." }]); + const current = artifact({ ...b, bodyOverride: "My own words." }); + expect(mergeConcurrentEdits(b, after, current).bodyOverride).toBe("My own words."); + }); + it("takes the turn's verdict when the turn did revise it", () => { const b = before(); const theirs = { recommendation: "request_changes" as const, confidence: 80, reasoning: "found one" }; diff --git a/src/core/revise.ts b/src/core/revise.ts index ea6d874..8eed3ae 100644 --- a/src/core/revise.ts +++ b/src/core/revise.ts @@ -91,6 +91,9 @@ export function mergeConcurrentEdits( // there only when it actually revised one. status: current.status, verdict: after.verdict === before.verdict ? current.verdict : after.verdict, + // A turn never writes a send body, so one written while it ran is the + // user's and stands — as does clearing one. + bodyOverride: current.bodyOverride, }; } diff --git a/src/core/send.test.ts b/src/core/send.test.ts index b562233..5b5e590 100644 --- a/src/core/send.test.ts +++ b/src/core/send.test.ts @@ -44,15 +44,16 @@ function makeArtifact(overrides: Partial = {}): Artifact { chapters: [{ id: "core", title: "Core", explanation: "Adds a const.", files: ["src/a.ts"] }], comments: [], verdict: { recommendation: "comment", confidence: 80, reasoning: "ok" }, + bodyOverride: null, run: null, sent: null, refresh: null, filed: null, settledAt: null, calibration: null, - chat: [], - preChat: null, - pendingChat: null, + chat: [], + preChat: null, + pendingChat: null, ...overrides, }; } @@ -117,6 +118,31 @@ describe("buildReviewPayload", () => { expect(payload.body).toContain("src/a.ts:~2"); }); + it("posts the body the user wrote instead of the composed one", () => { + // The whole body, footer included: a body half-honoured is one nobody + // wrote. The draft underneath is untouched — only what posts changed. + const artifact = makeArtifact({ + bodyOverride: "Looks good to me. I ran the migration locally.\n", + comments: [ + { id: "1", path: "src/a.ts", line: 2, body: "inline ok", chapterId: "core", severity: null, origin: "ai", status: "draft", editedByUser: false, originalLine: null, drifted: false }, + { id: "2", path: "src/a.ts", line: 999, body: "bad line", chapterId: "core", severity: null, origin: "ai", status: "draft", editedByUser: false, originalLine: null, drifted: false }, + ], + }); + const payload = buildReviewPayload(artifact, "APPROVE"); + expect(payload.body).toBe("Looks good to me. I ran the migration locally."); + expect(payload.body).not.toContain("## Summary"); + expect(payload.body).not.toContain("cerber"); + // Inline comments are a separate half of the payload and still post, and + // the cockpit still needs to know which comments had no line to land on. + expect(payload.comments).toEqual([{ path: "src/a.ts", line: 2, side: "RIGHT", body: "inline ok" }]); + expect(payload.folded.map((c) => c.id)).toEqual(["2"]); + }); + + it("composes the body again once the override is cleared", () => { + const payload = buildReviewPayload(makeArtifact({ bodyOverride: null }), "COMMENT"); + expect(payload.body).toContain("## Summary"); + }); + it("anchors the review to the reviewed head commit", () => { expect(buildReviewPayload(makeArtifact(), "COMMENT").commitId).toBe("abc"); const noSha = makeArtifact({ pr: { ...makeArtifact().pr, headSha: "" } }); diff --git a/src/core/send.ts b/src/core/send.ts index 4e33bab..cf7d81c 100644 --- a/src/core/send.ts +++ b/src/core/send.ts @@ -76,7 +76,13 @@ export function buildReviewPayload(artifact: Artifact, event: ReviewEvent): Revi return { event, - body: parts.join("\n").trim(), + // A body the user wrote by hand replaces the composed one outright, + // footer and folded notes included: it is the review's own comment, and + // half-honouring it — keeping a footer they deleted, re-appending notes + // they cut — would post something nobody wrote. The composition above + // still runs, because `folded` is what the cockpit uses to say which + // comments have no line of their own to land on. + body: (artifact.bodyOverride ?? parts.join("\n")).trim(), comments: inline, folded, commitId: artifact.pr.headSha || undefined, diff --git a/src/core/state.test.ts b/src/core/state.test.ts index 91500a5..2a11875 100644 --- a/src/core/state.test.ts +++ b/src/core/state.test.ts @@ -40,6 +40,7 @@ function artifact(over: Partial = {}): Artifact { chapters: [], comments: [], verdict: null, + bodyOverride: null, run: null, sent: null, refresh: null, diff --git a/src/runner/chat.test.ts b/src/runner/chat.test.ts index 4c852fe..234bb7f 100644 --- a/src/runner/chat.test.ts +++ b/src/runner/chat.test.ts @@ -54,6 +54,7 @@ function artifact(over: Partial = {}): Artifact { chapters: [{ id: "one", title: "One", explanation: "Explains one.", files: ["src/a.ts"] }], comments: [comment()], verdict: { recommendation: "comment", confidence: 70, reasoning: "because" }, + bodyOverride: null, run: { model: "opus", startedAt: "2026-08-19T00:00:00Z", diff --git a/src/runner/review-merge.test.ts b/src/runner/review-merge.test.ts index 9b86179..20b604d 100644 --- a/src/runner/review-merge.test.ts +++ b/src/runner/review-merge.test.ts @@ -82,6 +82,7 @@ function ready(comments: Comment[], headSha = "old-sha"): Artifact { chapters: [], comments, verdict: null, + bodyOverride: null, run: null, sent: null, filed: null, diff --git a/src/runner/review.test.ts b/src/runner/review.test.ts index 72df7cb..a9f9d0a 100644 --- a/src/runner/review.test.ts +++ b/src/runner/review.test.ts @@ -56,6 +56,7 @@ function artifact(status: ArtifactStatus, headSha: string): Artifact { chapters: [], comments: [], verdict: null, + bodyOverride: null, run: null, sent: null, filed: null, diff --git a/src/runner/review.ts b/src/runner/review.ts index 18d49ab..95dbdb1 100644 --- a/src/runner/review.ts +++ b/src/runner/review.ts @@ -234,6 +234,9 @@ async function performReview( chapters: [], comments: [], verdict: null, + // Not carried over. A hand-written send body described the draft this run + // is replacing — the same reason the pre-chat snapshot goes. + bodyOverride: null, run: { model: opts.model ?? null, startedAt: now(), diff --git a/src/server/chat.test.ts b/src/server/chat.test.ts index 8452e85..f14641d 100644 --- a/src/server/chat.test.ts +++ b/src/server/chat.test.ts @@ -49,6 +49,7 @@ function artifact(over: Partial = {}): Artifact { chapters: [], comments: [], verdict: { recommendation: "comment", confidence: 70, reasoning: "r" }, + bodyOverride: null, run: null, sent: null, refresh: null, diff --git a/src/server/daemon.ts b/src/server/daemon.ts index 5d8128f..e02610e 100644 --- a/src/server/daemon.ts +++ b/src/server/daemon.ts @@ -151,6 +151,7 @@ export function stubArtifact(ref: DiscoveredPr): Artifact { chapters: [], comments: [], verdict: null, + bodyOverride: null, run: null, sent: null, refresh: null, diff --git a/src/server/guards.test.ts b/src/server/guards.test.ts index 8c05bc9..c15bce3 100644 --- a/src/server/guards.test.ts +++ b/src/server/guards.test.ts @@ -58,6 +58,7 @@ function artifact(status: ArtifactStatus): Artifact { chapters: [], comments: [], verdict: { recommendation: "approve", confidence: 90, reasoning: "fine" }, + bodyOverride: null, run: null, sent: null, filed: null, @@ -88,6 +89,15 @@ const patchStatus = async (status: string) => { }); }; +const patchBody = async (body: Record) => { + const app = await buildApp({}); + return app.request(`/api/reviews/${KEY}`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +}; + beforeEach(() => { vi.clearAllMocks(); submit.mockResolvedValue({ url: "https://github.com/acme/widgets/pull/42#r1" }); @@ -198,4 +208,24 @@ describe("PATCH /api/reviews/:key — only the statuses that are your decision", expect((await patchStatus(status)).status).toBe(400); } }); + + // The review's own comment, written by hand. It is the one thing GitHub gets + // that is not derived from the draft, so it is stored rather than composed — + // and `null` is how the user hands the body back to the composition. + it("takes a body the user wrote, and gives it back on null", async () => { + await saveArtifact(artifact("ready")); + + expect((await patchBody({ bodyOverride: "Ran it locally, ship it." })).status).toBe(200); + expect((await loadArtifact(ID))!.bodyOverride).toBe("Ran it locally, ship it."); + // The draft it replaces is untouched — this changes what posts, not the review. + expect((await loadArtifact(ID))!.summary).toBe("a draft"); + + expect((await patchBody({ bodyOverride: null })).status).toBe(200); + expect((await loadArtifact(ID))!.bodyOverride).toBeNull(); + // Settling is the other half of this route; writing a body is not a settle. + expect((await loadArtifact(ID))!.status).toBe("ready"); + expect((await loadArtifact(ID))!.settledAt).toBeNull(); + expect(submit).not.toHaveBeenCalled(); + }); + }); diff --git a/src/server/index.ts b/src/server/index.ts index a6bbc49..1d167a0 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -309,6 +309,7 @@ export async function buildApp( chapters: [], comments: [], verdict: null, + bodyOverride: null, run: { model: null, startedAt: now, @@ -386,6 +387,12 @@ export async function buildApp( // the moment you click. Same rule the poll's reopen follows. next.filed = null; } + // The review body as the user rewrote it — or `null` to hand the body + // back to the composition it came from. Only ever set from the send + // panel, where the text being replaced is on screen. + if (body.bodyOverride !== undefined) { + next.bodyOverride = body.bodyOverride === null ? null : String(body.bodyOverride); + } if (body.verdictRecommendation !== undefined && next.verdict) { next.verdict = { ...next.verdict, diff --git a/web/src/Detail.tsx b/web/src/Detail.tsx index 2b4cf1b..67751fa 100644 --- a/web/src/Detail.tsx +++ b/web/src/Detail.tsx @@ -1331,33 +1331,101 @@ function ChatPanel({ ); } +/** + * A textarea that grows to its text, up to most of the window. + * + * The two long-form boxes in a review — the summary and the body that gets + * posted — are paragraphs that wrap, so a row count guessed from newlines + * opens the user's own prose on a scrollbar, with a line cut in half at the + * bottom edge. + */ +function useGrowToFit(value: string) { + const box = useRef(null); + useEffect(() => { + const el = box.current; + if (!el) return; + el.style.height = "auto"; + el.style.height = `${Math.min(el.scrollHeight + 2, window.innerHeight * 0.7)}px`; + }, [value]); + return box; +} + +/** + * The review's own comment — the one paragraph GitHub gets that is not attached + * to a line — rewritten by hand. + * + * It is seeded with the composed body and replaces it outright, footer and + * folded notes included: a body half-honoured is one nobody wrote. What it is + * not is an edit of the review — the summary, walkthrough and comments stay + * exactly as they read, and `build it from the review again` is always one + * click away. + */ +function BodyEditor({ + initial, + onSave, + onCancel, +}: { + initial: string; + onSave: (text: string) => void; + onCancel: () => void; +}) { + const [draft, setDraft] = useState(initial); + const box = useGrowToFit(draft); + return ( +
+