fix(cockpit): a chapter too big to draw opens folded, so a huge PR still reads

SPEC.md

@@ -499,7 +499,7 @@ review this rule cannot reach: the review that graded nothing at all and only
 asks questions.
 
 Severity is advisory: no code derives or enforces the verdict from it. The
-cockpit only *points out* disagreement (§17.5), and the blocker count is shown
+cockpit only *points out* disagreement (§17.6), and the blocker count is shown
 beside the verdict wherever there is room, because that is the fact a reader
 can check the verdict against.
 
@@ -1386,7 +1386,26 @@ keeps ringing, since that machine's tap lands where nobody is looking. The
 favicon shows a dot exactly while the walkable set is non-empty, re-derived
 on every queue fetch so no screen can leave it stale.
 
-### 17.5 Truth-Telling Surfaces
+### 17.5 The Walkthrough
+
+Chapters render open — the walkthrough is the point of the page — with one
+exception: a chapter holding more than 2,000 diff lines opens folded, and its
+header MUST say so and say why ("34,961 diff lines, folded to keep the page
+quick"). A rendered diff line is a table row and a dozen DOM nodes, so the
+catch-all chapter of a 368-file PR is 600,000 of them: the browser then spends
+its time on layout rather than on the review, and scrolling collapses. The
+fold MUST be decided while rendering, not corrected afterwards — folding a
+chapter that has already been drawn pays the whole cost it exists to avoid.
+
+The fold is a default, not a refusal: one click opens it, and the user's
+choice stands for as long as they are on that review. It does not outlive the
+review — a chapter opened or folded here MUST NOT carry its id (`__other`
+above all) onto the next PR's page. Nor may anything else the last review put
+on screen: the detail view clears the artifact and the load error when the key
+changes, so no review is ever drawn under another's URL and no failure to load
+one is reported over the next.
+
+### 17.6 Truth-Telling Surfaces
 
 - The verdict cell shows recommendation + confidence; the blocker count is
   shown wherever there is room (detail chip, queue strip) as the checkable

src/core/diff.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, it } from "vitest";
 import {
+  diffLineCounts,
   newSideLineText,
   oldSideLineText,
   patchForFiles,
@@ -86,3 +87,17 @@ describe("oldSideLineText", () => {
     expect(newSideLineText(SAMPLE).get("src/old.ts")!.size).toBe(0);
   });
 });
+
+describe("diffLineCounts", () => {
+  it("counts each file's diff lines, which is what the cockpit has to draw", () => {
+    const counts = diffLineCounts(SAMPLE);
+    expect([...counts.keys()]).toEqual(splitDiffByFile(SAMPLE).map((p) => p.path));
+    for (const p of splitDiffByFile(SAMPLE)) {
+      expect(counts.get(p.path)).toBe(p.patch.split("\n").length);
+    }
+  });
+
+  it("has nothing to count in an empty diff", () => {
+    expect(diffLineCounts("").size).toBe(0);
+  });
+});

src/core/diff.ts

@@ -120,3 +120,11 @@ export function unclaimedFiles(diff: string, chapters: { files: string[] }[]): s
     .map((p) => p.path)
     .filter((p) => !claimed.has(p));
 }
+
+/**
+ * How many diff lines each file contributes, by path. The cockpit renders one
+ * table row per line, so this is what a chapter costs a browser to draw.
+ */
+export function diffLineCounts(diff: string): Map<string, number> {
+  return new Map(splitDiffByFile(diff).map((p) => [p.path, p.patch.split("\n").length]));
+}

web/src/Detail.tsx

@@ -1,7 +1,7 @@
 import { html } from "diff2html";
 import { ReactNode, useEffect, useMemo, useRef, useState } from "react";
 import { createPortal } from "react-dom";
-import { patchForFiles, splitDiffByFile, unclaimedFiles } from "../../src/core/diff";
+import { diffLineCounts, patchForFiles, splitDiffByFile, unclaimedFiles } from "../../src/core/diff";
 import { withGrade } from "../../src/core/severity";
 import {
   addComment,
@@ -585,12 +585,25 @@ function AddComment({
   );
 }
 
+/**
+ * Beyond this many diff lines a chapter opens folded rather than rendered.
+ *
+ * The cockpit draws one table row per diff line and a dozen DOM nodes per row,
+ * so a catch-all chapter holding a 368-file PR lands 600,000 nodes on the page
+ * and the browser spends its time on style and layout instead of on the
+ * review. Measured over 314 chapters of real reviews, 2 are past this line —
+ * a normal walkthrough opens exactly as it did, and the ones that would grind
+ * are one click away.
+ */
+const foldChapterOverLines = 2000;
+
 function ChapterSection({
   chapter,
   n,
   diff,
   comments,
   open,
+  heavy,
   onToggle,
   onUpdateComment,
   onDeleteComment,
@@ -607,6 +620,8 @@ function ChapterSection({
   diff: string;
   comments: ReviewComment[];
   open: boolean;
+  /** Diff lines, when there are too many of them to have opened with. */
+  heavy: number | null;
   onToggle: () => void;
   flash: string | null;
   onUpdateComment: (id: string, patch: { body?: string; status?: string }) => void;
@@ -649,6 +664,11 @@ function ChapterSection({
           {comments.length > 0
             ? ` · ${comments.length} comment${comments.length === 1 ? "" : "s"}`
             : " · no comments"}
+          {heavy != null && !open && (
+            <span title="Drawing this many lines at once would leave the page too slow to scroll. Open it if you want it — nothing else on the page is affected.">
+              {` · ${heavy.toLocaleString()} diff lines, folded to keep the page quick`}
+            </span>
+          )}
         </span>
         <span className="grow" />
         {onDiscuss && (
@@ -1317,8 +1337,13 @@ export function Detail({ reviewKey }: { reviewKey: string }) {
   // reach the one chat panel at the bottom.
   const [chatRefs, setChatRefs] = useState<ChatRef[]>([]);
   const chatInput = useRef<HTMLTextAreaElement | null>(null);
-  // Chapters are open by default — the walkthrough is the point of the page.
-  const [collapsed, setCollapsed] = useState<Set<string>>(new Set());
+  // Chapters are open by default — the walkthrough is the point of the page —
+  // except for one too big to draw (see foldChapterOverLines). This records
+  // only the chapters the user has since flipped the other way, so the default
+  // is decided while rendering rather than corrected after it: seeding it from
+  // an effect would draw the giant diff once before folding it away, which is
+  // the whole cost the fold exists to avoid.
+  const [flipped, setFlipped] = useState<Set<string>>(new Set());
   const [focused, setFocused] = useState(0);
   const chapterEls = useRef<Map<string, HTMLElement>>(new Map());
   const verdictEl = useRef<HTMLDivElement | null>(null);
@@ -1354,6 +1379,11 @@ export function Detail({ reviewKey }: { reviewKey: string }) {
     // Walking to another review starts at its top, not wherever the last one
     // left the page.
     window.scrollTo({ top: 0 });
+    // Nothing of the last review outlives its URL. Keeping it until the fetch
+    // lands renders the wrong PR's chapters under this one's key — briefly
+    // drawing a diff this page has no business drawing.
+    setArtifact(null);
+    setError(null);
     setFreshness(null);
     setFreshnessError(null);
     setEventOverride(null);
@@ -1458,15 +1488,37 @@ export function Detail({ reviewKey }: { reviewKey: string }) {
       : artifact.chapters;
   }, [artifact]);
 
+  /** How many diff lines each chapter is asking the browser to draw. */
+  const weights = useMemo(() => {
+    const counts = diffLineCounts(artifact?.diff ?? "");
+    return new Map(
+      chapters.map((ch) => [ch.id, ch.files.reduce((n, f) => n + (counts.get(f) ?? 0), 0)]),
+    );
+  }, [artifact?.diff, chapters]);
+  /** Lines, when there are enough of them that this chapter opens folded. */
+  const heavy = (id: string) => {
+    const lines = weights.get(id) ?? 0;
+    return lines > foldChapterOverLines ? lines : null;
+  };
+  const isOpen = (id: string) => (flipped.has(id) ? heavy(id) != null : heavy(id) == null);
+  const flip = (id: string) =>
+    setFlipped((s) => {
+      const next = new Set(s);
+      if (next.has(id)) next.delete(id);
+      else next.add(id);
+      return next;
+    });
+
+  // A fold belongs to the review it was made in. The cockpit walks from one
+  // review to the next without remounting, so without this a chapter opened
+  // here would carry its id — "__other" above all — onto the next PR's page.
+  useEffect(() => setFlipped(new Set()), [reviewKey]);
+
   const openChapter = (i: number) => {
     const ch = chapters[i];
     if (!ch) return null;
     setFocused(i);
-    setCollapsed((s) => {
-      const next = new Set(s);
-      next.delete(ch.id);
-      return next;
-    });
+    if (!isOpen(ch.id)) flip(ch.id);
     return ch;
   };
 
@@ -1787,7 +1839,7 @@ export function Detail({ reviewKey }: { reviewKey: string }) {
               {chapters.map((ch, i) => (
                 <div key={ch.id} className="rail-group">
                   <button
-                    className={`rail-item${!collapsed.has(ch.id) ? " rail-item-on" : ""}`}
+                    className={`rail-item${isOpen(ch.id) ? " rail-item-on" : ""}`}
                     onClick={() => goChapter(i)}
                   >
                     <span className="grow">
@@ -1905,15 +1957,9 @@ export function Detail({ reviewKey }: { reviewKey: string }) {
               n={i + 1}
               diff={artifact.diff}
               comments={artifact.comments.filter((c) => c.chapterId === ch.id)}
-              open={!collapsed.has(ch.id)}
-              onToggle={() =>
-                setCollapsed((s) => {
-                  const nextSet = new Set(s);
-                  if (nextSet.has(ch.id)) nextSet.delete(ch.id);
-                  else nextSet.add(ch.id);
-                  return nextSet;
-                })
-              }
+              open={isOpen(ch.id)}
+              heavy={heavy(ch.id)}
+              onToggle={() => flip(ch.id)}
               onUpdateComment={onUpdateComment}
               onDeleteComment={onDeleteComment}
               onAddComment={onAddComment}