Skip to content

Commit 982c6b1

Browse files
committed
Write the report a completed review was always for
Work item V5 of the M3 plan. A review could be run and every finding decided, and then it produced nothing: no report, no export, nothing to send anyone. The renderer is pure, so the exact text is testable without a database, and an assembler gathers what it needs in one place rather than two routes drifting about what a report contains. Two things make it more than a list of findings. It says what was examined, not only what was found. A reader cannot otherwise tell "nothing is wrong" from "nothing was looked at", and that distinction is the whole value of a review. The counts come from the coverage ledger the pipeline already reconciled, so they are what the app counted rather than what a model claimed. It records what the person decided, dismissals included, with their reasons. A dismissal is evidence about the engine rather than an absence of one, and dropping it would throw away the only signal that says which prompts need work. Confirmed nitpicks stay too: what someone chose to keep is not the report's to second-guess. The protocol's own prose about output format is deliberately not parsed. Rendering arbitrary instructions is not tractable, and a report whose shape depended on prompt-shaped text would change meaning without anyone editing the code that writes it. Exports live outside the run directory, so a report outlives the worktrees, bundle and logs that deleting a review removes. The filename uses the UTC day and turns branch slashes into hyphens, so a second export the same day replaces the first instead of accumulating near-identical files. Eighteen unit tests on the text itself, including the no-em-dash rule that binds every surface, plus two end-to-end tests that build the report from a review which actually ran and write the export to disk. Three mutations checked and all caught: dropping dismissals, omitting the completeness statement, and reversing the severity order.
1 parent 033b4d9 commit 982c6b1

10 files changed

Lines changed: 827 additions & 1 deletion

File tree

docs/DECISIONS.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,3 +709,23 @@ verified evidence, in writing, here.
709709
automatically. When the dependency has a branch of the same name as the one
710710
being reviewed it is preselected and labelled as suggested, because that is
711711
almost always the other half of the change, and almost always is not always.
712+
- 2026-07-31 DECIDED (V5, D-38): the report is the app's own rendering of the
713+
finding format, and a protocol's prose about output format is not parsed.
714+
Rendering arbitrary instructions is not tractable, and a report whose shape
715+
depended on prompt-shaped text would change meaning without anyone editing
716+
the code that writes it.
717+
- 2026-07-31 DECIDED (V5): the report states what was examined as well as what
718+
was found. A reader cannot otherwise tell "nothing is wrong" from "nothing
719+
was looked at", and that distinction is the whole value of a review. The
720+
counts come from the coverage ledger the pipeline already reconciled, not
721+
from anything a model claimed.
722+
- 2026-07-31 DECIDED (V5): dismissed findings appear in the report with their
723+
reasons. A dismissal is evidence about the engine rather than an absence of
724+
one, and dropping it would throw away the only signal that says which
725+
prompts need work. Confirmed NITPICKs are kept too (D-11): what a person
726+
chose to keep is not the report's to second-guess.
727+
- 2026-07-31 DECIDED (V5, D-39): exports are written under the data root's
728+
exports directory, outside the run directory, so a report outlives the
729+
worktrees, bundle and logs that deleting a review removes. The filename uses
730+
the UTC day and turns branch slashes into hyphens, so a second export the
731+
same day replaces the first rather than accumulating near-identical files.

docs/plans/M3-FINISH-PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ confirms the wiring.
347347
| V2 | confirm/dismiss/complete/context routes, confirmation UI, keyboard map | V1 | DONE |
348348
| V3 | project detail page, fetch-now, links CRUD, project delete | V1 | DONE |
349349
| V4 | preflight route and panel, linked toggle with suggestion | V3 | DONE |
350-
| V5 | report renderer, report/export routes, report UI | V2 | |
350+
| V5 | report renderer, report/export routes, report UI | V2 | DONE |
351351
| V6 | resume/queued/merged/delete UI, settings editor, probe buttons | V2 | |
352352
| V7 | rulesets detail, enable toggle, snapshot filter, export | V1 | |
353353
| V8 | e2e journey, theme screenshots, design audit, CI --e2e | V2-V7 | |
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* Writes the report to a file.
3+
*
4+
* Exports live outside the run directory on purpose: deleting a review removes
5+
* its worktrees, bundle and logs, and the report it produced should outlive
6+
* all of that. It is the thing the review was for.
7+
*/
8+
9+
import { mkdir, writeFile } from "node:fs/promises";
10+
import { join } from "node:path";
11+
import { repoSlug } from "@/lib/git/url";
12+
import { exportsDir } from "@/lib/paths";
13+
import { exportFileName, renderReport } from "@/lib/review/report";
14+
import { requireProject } from "@/server/db/repositories/projects";
15+
import { requireReview, statusOf } from "@/server/db/repositories/reviews";
16+
import { buildReportInput } from "@/server/review/report-input";
17+
import { handler, ok } from "@/server/api/respond";
18+
import { runtime } from "@/server/runtime";
19+
20+
export const dynamic = "force-dynamic";
21+
22+
export async function POST(
23+
_request: Request,
24+
context: { params: Promise<{ id: string }> },
25+
): Promise<Response> {
26+
return handler(async () => {
27+
const { db, dataDir } = runtime();
28+
const { id } = await context.params;
29+
30+
const review = requireReview(db, id);
31+
const status = statusOf(review);
32+
if (status !== "complete") {
33+
return Response.json(
34+
{
35+
error: `This review is ${status.replace(/_/g, " ")}, so there is no report to export.`,
36+
code: "NotComplete",
37+
},
38+
{ status: 409 },
39+
);
40+
}
41+
42+
const markdown = renderReport(buildReportInput(db, id));
43+
const name = exportFileName({
44+
projectSlug: repoSlug(requireProject(db, review.projectId).name),
45+
fromBranch: review.fromBranch,
46+
intoBranch: review.intoBranch,
47+
at: review.completedAt ?? review.createdAt,
48+
});
49+
50+
const directory = exportsDir(dataDir);
51+
await mkdir(directory, { recursive: true });
52+
const path = join(directory, name);
53+
await writeFile(path, markdown, "utf8");
54+
55+
return ok({ path, markdown });
56+
});
57+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* The report for a completed review.
3+
*
4+
* Only for a completed one: a report is the record of what a person accepted,
5+
* so a review still waiting on decisions has nothing to report yet.
6+
*/
7+
8+
import { renderReport } from "@/lib/review/report";
9+
import { requireReview, statusOf } from "@/server/db/repositories/reviews";
10+
import { buildReportInput } from "@/server/review/report-input";
11+
import { handler, ok } from "@/server/api/respond";
12+
import { runtime } from "@/server/runtime";
13+
14+
export const dynamic = "force-dynamic";
15+
16+
export async function GET(
17+
_request: Request,
18+
context: { params: Promise<{ id: string }> },
19+
): Promise<Response> {
20+
return handler(async () => {
21+
const { db } = runtime();
22+
const { id } = await context.params;
23+
24+
const status = statusOf(requireReview(db, id));
25+
if (status !== "complete") {
26+
return Response.json(
27+
{
28+
error:
29+
`This review is ${status.replace(/_/g, " ")}. A report is the record of what a ` +
30+
"person accepted, so it exists once every finding has been decided.",
31+
code: "NotComplete",
32+
},
33+
{ status: 409 },
34+
);
35+
}
36+
37+
return ok({ markdown: renderReport(buildReportInput(db, id)) });
38+
});
39+
}

src/app/reviews/[id]/page.tsx

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ export default function ReviewPage({ params }: { params: Promise<{ id: string }>
5858
const { id } = use(params);
5959
const [snapshot, setSnapshot] = useState<Snapshot | null>(null);
6060
const [live, setLive] = useState<string[]>([]);
61+
const [report, setReport] = useState<string | null>(null);
62+
const [exported, setExported] = useState("");
6163

6264
async function reload() {
6365
const response = await fetch(`/api/reviews/${id}`);
@@ -221,6 +223,16 @@ export default function ReviewPage({ params }: { params: Promise<{ id: string }>
221223
</ol>
222224
</Card>
223225

226+
{review.status === "complete" ? (
227+
<ReportPanel
228+
reviewId={id}
229+
report={report}
230+
exported={exported}
231+
onLoaded={setReport}
232+
onExported={setExported}
233+
/>
234+
) : null}
235+
224236
{review.status === "awaiting_confirmation" && reported.length > 0 ? (
225237
<ConfirmationQueue
226238
reviewId={id}
@@ -277,6 +289,87 @@ export default function ReviewPage({ params }: { params: Promise<{ id: string }>
277289
);
278290
}
279291

292+
/**
293+
* The report, once a person has decided everything.
294+
*
295+
* Fetched on demand rather than with the page: it is the last thing anyone
296+
* looks at, and a review that is still running has no report to speak of.
297+
*/
298+
function ReportPanel({
299+
reviewId,
300+
report,
301+
exported,
302+
onLoaded,
303+
onExported,
304+
}: {
305+
reviewId: string;
306+
report: string | null;
307+
exported: string;
308+
onLoaded: (markdown: string) => void;
309+
onExported: (path: string) => void;
310+
}) {
311+
const [error, setError] = useState("");
312+
313+
useEffect(() => {
314+
if (report !== null) return;
315+
let cancelled = false;
316+
void (async () => {
317+
const response = await fetch(`/api/reviews/${reviewId}/report`);
318+
const body = (await response.json()) as { markdown?: string; error?: string };
319+
if (cancelled) return;
320+
if (response.ok && body.markdown) onLoaded(body.markdown);
321+
else setError(body.error ?? "The report could not be built.");
322+
})();
323+
return () => {
324+
cancelled = true;
325+
};
326+
}, [reviewId, report, onLoaded]);
327+
328+
return (
329+
<section className="mb-6">
330+
<header className="mb-3 flex flex-wrap items-center justify-between gap-3">
331+
<h2 className="text-sm font-semibold">Report</h2>
332+
<span className="flex items-center gap-2">
333+
<Button
334+
onClick={() => {
335+
if (report) void navigator.clipboard?.writeText(report);
336+
}}
337+
disabled={!report}
338+
>
339+
Copy
340+
</Button>
341+
<Button
342+
variant="primary"
343+
onClick={async () => {
344+
const response = await fetch(`/api/reviews/${reviewId}/export`, { method: "POST" });
345+
const body = (await response.json()) as { path?: string; error?: string };
346+
if (response.ok && body.path) onExported(body.path);
347+
else setError(body.error ?? "The report could not be exported.");
348+
}}
349+
>
350+
Export
351+
</Button>
352+
</span>
353+
</header>
354+
355+
{error ? <Problem>{error}</Problem> : null}
356+
{exported ? (
357+
<p className="mb-2 text-xs text-[var(--color-good)]">Written to {exported}</p>
358+
) : null}
359+
360+
{report === null ? (
361+
<p className="text-sm text-[var(--color-ink-muted)]">Building the report...</p>
362+
) : (
363+
<Card className="max-h-[32rem] overflow-auto p-4">
364+
<pre className="text-xs leading-5 whitespace-pre-wrap">
365+
<code className="font-[family-name:var(--font-mono)]">{report}</code>
366+
</pre>
367+
</Card>
368+
)}
369+
</section>
370+
);
371+
}
372+
280373
function Stat({ label, value }: { label: string; value: string }) {
281374
return (
282375
<div className="flex justify-between gap-4">

0 commit comments

Comments
 (0)