docs(review): one telling per invariant — trim the rebuilt modules' header autopsies

.changeset/review-header-cleanup.md

@@ -0,0 +1,2 @@
+---
+---

src/app/review/producer.ts

@@ -8,16 +8,14 @@
  * against the shared contract (`core/review/generationOrder.ts`) before anything is
  * published, so a producer bug fails here rather than desynchronizing a reader.
  *
- * It deliberately does *not* own the review's live state. The terminal's controller owns
- * the store today, and moving that is a behavior change rather than a seam extraction; the
- * producer attaches to whichever store the host mounted and plans intents against it,
- * supplying the caller-owned facts core refuses to invent — identity, time, and the
- * annotation index (`ReviewIntentFacts.annotations`) — through the same derivation the
- * terminal uses.
+ * It does *not* own the review's live state. The terminal's controller owns the store
+ * today, and moving that is a behavior change rather than a seam extraction; the producer
+ * attaches to whichever store the host mounted and plans intents against it, supplying the
+ * caller-owned facts core refuses to invent (identity, time, and the annotation index,
+ * `ReviewIntentFacts.annotations`) through the same derivation the terminal uses.
  *
  * No transport lives here. Serving the session surface means answering method calls; HTTP,
- * SSE, and a browser client are later phases, and their absence is what keeps this module
- * about the review rather than about a protocol.
+ * SSE, and a browser client are later phases.
  */
 import {
   assertReviewPublicationAdvance,
@@ -158,9 +156,9 @@ export class ReviewProducer {
   /**
    * The review state this producer plans against, when a host has attached one.
    *
-   * Read-only, and deliberately the *store's* state rather than a copy: a caller
-   * validating a request against the current review — does this file exist, is this the
-   * draft I opened — must see exactly what the next intent will be planned against.
+   * Read-only, and the *store's* state rather than a copy: a caller validating a request
+   * against the current review — does this file exist, is this the draft I opened — must
+   * see exactly what the next intent will be planned against.
    */
   getReviewState() {
     return this.store?.getSnapshot();

src/app/review/publication.ts

@@ -7,10 +7,10 @@
  * mutating this one — which is what makes "which generation is this?" answerable rather
  * than a matter of timing.
  *
- * The document itself deliberately knows nothing about any of this
- * (`src/core/review/document.ts`); publication is layered on top so the shared model stays
- * a description of a review rather than of a transport. The vocabulary for addressing and
- * ordering those generations is `src/core/review/generationOrder.ts`.
+ * The document itself knows nothing about any of this (`src/core/review/document.ts`);
+ * publication is layered on top so the shared model stays a description of a review rather
+ * than of a transport. The vocabulary for addressing and ordering those generations is
+ * `src/core/review/generationOrder.ts`.
  */
 import {
   buildReviewContentManifest,

src/app/review/resourceStore.ts

@@ -1,7 +1,7 @@
 /**
  * Materializing and serving one generation's resources.
  *
- * Three rules shape this module, each of them a defect the prototype shipped:
+ * Three rules shape this module:
  *
  * - **Single flight per resource.** A resource is produced at most once per generation,
  *   and concurrent readers share that one production. It is not a cache bolted on top: a

src/core/commandCatalog.ts

@@ -13,17 +13,17 @@
  *   attached surface sees the result. Its effect is declared as data rather than as a
  *   function, which is what lets the same declaration drive the terminal's handler, an
  *   agent command, and later a wire action.
- * - `client-local` — deliberately per-client view state (scrolling, layout, theme, help).
+ * - `client-local` — per-client view state (scrolling, layout, theme, help).
  *   Each client implements its own handler; sharing identity is what keeps help screens
  *   and palettes agreeing about what the command is called and what it is bound to.
  * - `host-only` — it runs where the review is hosted (quitting, reloading the source,
- *   opening `$EDITOR`). Not invocable from a remote client without an explicit allowlist,
- *   which is a scope boundary rather than a missing feature (audit F4).
+ *   opening `$EDITOR`). Not invocable from a remote client without an explicit allowlist
+ *   (audit F4).
  *
- * This module is deliberately renderer-neutral and dependency-light: no OpenTUI, no React,
- * no Node builtins, chords as plain strings. It is not part of `src/core/review` because
- * it describes UI vocabulary rather than review semantics, and that module stays purely
- * about what a review *is*.
+ * This module is renderer-neutral and dependency-light: no OpenTUI, no React, no Node
+ * builtins, chords as plain strings. It is not part of `src/core/review` because it
+ * describes UI vocabulary rather than review semantics, and that module stays purely about
+ * what a review *is*.
  */
 import type { ReviewIntent } from "./review/intents";
 import type { ReviewSelectionScope } from "./review/navigation";

src/core/review/actions.ts

@@ -1,5 +1,5 @@
 /**
- * Declares the actions the reducer executes — decided state transitions only.
+ * Declares the actions the reducer executes: decided state transitions only.
  *
  * An action states what changes, not whether it should: lifecycle code and intent plans
  * decide that first. `reduceReviewState` applies one without further validation beyond

src/core/review/address.ts

@@ -16,6 +16,13 @@
  */
 import type { ReviewSide } from "./types";
 
+/**
+ * One addressable thing in a review, at the granularity the caller means.
+ *
+ * `file` and `hunk` name structure; `line` names a position in the content by side, which
+ * exists whether or not anything is anchored there; `note` names one anchored note by its
+ * id, which outlives the line it currently hangs from.
+ */
 export type ReviewAddress =
   | { kind: "file"; fileKey: string }
   | { kind: "hunk"; fileKey: string; hunkIndex: number }

src/core/review/anchors.ts

@@ -6,17 +6,21 @@
  * this resolver placed through its fallback is not silently dropped by a consumer that
  * re-derives placement (`docs/browser-review-seam-audit.md`, D3/B8).
  *
- * Resolution is deliberately permissive: an imported note may name a line the current
- * patch no longer shows, and losing it entirely would be worse than hanging it from the
- * nearest real hunk. Callers that must reject an unbacked target validate before
- * anchoring rather than reading a verdict out of the anchor.
+ * Resolution is permissive: an imported note may name a line the current patch no longer
+ * shows, and losing it entirely would be worse than hanging it from the nearest real hunk.
+ * Callers that must reject an unbacked target validate before anchoring rather than
+ * reading a verdict out of the anchor.
  */
 import { reviewHunkRange, reviewRangesOverlap, type ReviewHunkSpan } from "./geometry";
 import type { ReviewLineRange, ReviewRangeAnchorV1, ReviewSide } from "./types";
 
 export interface ReviewNoteAnchorInput {
   oldRange?: ReviewLineRange;
   newRange?: ReviewLineRange;
+  /**
+   * The line the note was placed on. When it lands inside a hunk it decides ownership,
+   * ahead of any range that also intersects one.
+   */
   preferred?: { side: ReviewSide; line: number };
   /**
    * The hunk that owns the note when no range intersects one — an expanded-gap line, or

src/core/review/annotations.ts

@@ -5,12 +5,12 @@
  * index, and core cannot compute it: notes arrive from sources the semantic document does
  * not carry (a sidecar loaded with the changeset, live agent comments, the reviewer's own
  * notes), and only the consumer that merged them onto the diff model knows the full set.
- * It is therefore a caller-supplied fact (`ReviewIntentFacts.annotations`) — but the
+ * It is therefore a caller-supplied fact (`ReviewIntentFacts.annotations`), but the
  * *derivation* is shared, so the terminal and the producer hand the planner the same
  * answer instead of two that agree by coincidence.
  *
- * File membership is deliberately broader than hunk membership: a file carrying review
- * context but no note inside any hunk is still a stop on the annotated-file tour.
+ * File membership is broader than hunk membership: a file carrying review context but no
+ * note inside any hunk is still a stop on the annotated-file tour.
  */
 import { reviewHunkRanges, reviewRangesOverlap, type ReviewHunkSpan } from "./geometry";
 import type { ReviewAnnotationIndex } from "./navigation";

src/core/review/canonicalFile.ts

@@ -2,11 +2,7 @@
  * Does this serialized file still describe the review it came from?
  *
  * A producer serves each reviewed file as a canonical JSON resource, and a reader has to
- * be able to check that what it received matches the review it was published with. The
- * prototype checked this three times with three different field lists — seventeen fields
- * at the producer, twelve in the browser, ten in the broker — and the browser's compared
- * two of them by `JSON.stringify`, so a lazily inserted key could spuriously fail a file
- * that had not changed at all. None of the three compared hunk *content*
+ * be able to check that what it received matches the review it was published with
  * (`docs/browser-review-seam-audit.md`, D4).
  *
  * There is one check here, and it does not carry a field list of its own: it projects the

src/core/review/contentManifest.ts

@@ -1,15 +1,13 @@
 /**
  * A deterministic semantic snapshot of one review document.
  *
- * The manifest is a parity instrument, not a validator: every consumer of the shared
- * model can be driven through the same fixture and compared against the same manifest,
- * so a renderer that re-derives geometry instead of consuming core fails visibly rather
- * than drifting quietly.
+ * The manifest exists so every consumer of the shared model can be driven through the same
+ * fixture and compared against the same snapshot, making a renderer that re-derives
+ * geometry instead of consuming core fail visibly rather than drift quietly.
  *
  * It therefore records *derived* geometry — hunk extents, gap addresses, default note
  * targets, the reason a file renders nothing — alongside the content those derivations
- * read. Renderer identity (runtime ids, rows, widths) is deliberately absent, since two
- * consumers agreeing on it would prove nothing.
+ * read. Renderer identity (runtime ids, rows, widths) is left out.
  */
 import {
   reviewExpansionSide,

src/core/review/document.ts

@@ -7,9 +7,8 @@
  * the store, the terminal's note projection, later a transport — reads the result rather
  * than the diff model behind it.
  *
- * Publication concerns (generations, resource descriptors, byte digests) deliberately do
- * not appear: they belong to the producer runtime that serves a document, not to the
- * document itself.
+ * Publication concerns (generations, resource descriptors, byte digests) do not appear:
+ * they belong to the producer runtime that serves a document, not to the document itself.
  */
 import type { DiffFile } from "../types";
 import {

src/core/review/generationOrder.ts

@@ -19,14 +19,14 @@
  * - Within one generation, state revisions strictly increase but need **not** be
  *   contiguous: a receiver that joined late, replayed a log, or took a fresh snapshot
  *   legitimately sees jumps. A revision that repeats is a replay, not an update.
- * - Across generations, revisions are not comparable at all — a new generation may restart
- *   them — which is why a generation change is classified as its own verdict rather than
- *   folded into revision comparison.
+ * - Across generations, revisions are not comparable at all, since a new generation may
+ *   restart them. A generation change is therefore classified as its own verdict rather
+ *   than folded into revision comparison.
  *
  * Non-semantic republication (a renderer width changed, nothing about the review did) is
- * deliberately *not* modelled here. It carries no new position, so it classifies as a
- * replay; whoever needs to re-emit it decides that on its own publication key rather than
- * by loosening this comparison.
+ * *not* modelled here. It carries no new position, so it classifies as a replay; whoever
+ * needs to re-emit it decides that on its own publication key rather than by loosening
+ * this comparison.
  */
 
 export interface ReviewGenerationIdentity {

src/core/review/geometry.ts

@@ -7,9 +7,9 @@
  * once, because a renderer that re-derives one silently disagrees with the state store
  * that validates against it (`docs/browser-review-seam-audit.md`, A3/A4/A6/A10).
  *
- * The inputs are deliberately structural rather than `ReviewFileV1`: a parsed diff hunk
- * and a projected `ReviewHunkV1` both satisfy them, so the terminal can call these
- * primitives from its render path without projecting a whole semantic document first.
+ * The inputs are structural rather than `ReviewFileV1`: a parsed diff hunk and a projected
+ * `ReviewHunkV1` both satisfy them, so the terminal can call these primitives from its
+ * render path without projecting a whole semantic document first.
  */
 import type { ReviewLineAddressV1, ReviewLineRange, ReviewSide } from "./types";
 

src/core/review/identity.ts

@@ -6,10 +6,10 @@
  * positions or renderer object identity. Every identity here is a pure function of the
  * facts it names, so two processes projecting the same content agree.
  *
- * The digest is an identity hash, not an integrity check: it is deliberately
- * platform-neutral arithmetic rather than a crypto primitive, so the shared model stays
- * importable from a browser bundle without a hashing runtime. Wire-integrity digests
- * belong beside the transport that verifies bytes, not here.
+ * The digest is an identity hash, not an integrity check: it is platform-neutral
+ * arithmetic rather than a crypto primitive, so the shared model stays importable from a
+ * browser bundle without a hashing runtime. Wire-integrity digests belong beside the
+ * transport that verifies bytes, not here.
  */
 
 /** Four independent 32-bit FNV-1a lanes, giving a 128-bit identity from one pass. */

src/core/review/intents.ts

@@ -38,6 +38,13 @@ import {
 import type { ReviewStore } from "./store";
 import type { ReviewFileV1, ReviewLineAddressV1, ReviewLineRange, ReviewSide } from "./types";
 
+/**
+ * The facts core refuses to invent, supplied by whoever submits an intent.
+ *
+ * Identity, time, and the annotation index all depend on the runtime a review is hosted
+ * in, so planning reads them from here instead of reaching for a clock, a UUID source, or
+ * a note set the semantic document does not carry.
+ */
 export interface ReviewIntentFacts {
   /** Caller-allocated identity for a newly persisted note. */
   noteId?: string;
@@ -62,14 +69,17 @@ export interface ReviewIntentFacts {
 }
 
 export type ReviewIntent =
+  /** Select one hunk outright, revealing it the way the caller asks. */
   | { type: "selection/select"; fileKey: string; hunkIndex: number; reveal: ReviewRevealRequest }
   /** Step the selection through one navigable scope; the scope decides wrap and reveal. */
   | { type: "selection/move"; scope: ReviewSelectionScope; delta: number }
   /** Jump to one file, landing on its first hunk. */
   | { type: "selection/select-file"; fileKey: string; reveal?: ReviewRevealRequest }
   /** Adopt the position a renderer's viewport settled on, without moving any viewport. */
   | { type: "selection/anchor"; fileKey: string; hunkIndex: number }
+  /** Replace the review's file filter, which decides the visible stream. */
   | { type: "filter/set"; filter: string }
+  /** Set whether agent notes are shown; reviewer-authored notes stay visible either way. */
   | { type: "notes/set-visibility"; visible: boolean }
   /** Open a draft at one hunk, defaulting to the line a whole-hunk note hangs from. */
   | {
@@ -81,7 +91,9 @@ export type ReviewIntent =
     }
   /** Persist the active draft; a blank body retires the draft instead. */
   | { type: "notes/create-user"; consumeDraft: true }
+  /** Delete one reviewer-authored note by id. */
   | { type: "notes/remove-user"; noteId: string }
+  /** Dismiss one live agent note by id, leaving reviewer notes untouched. */
   | { type: "notes/remove-live"; noteId: string }
   | { type: "notes/clear"; fileKey?: string; includeUser?: boolean }
   /** Flip one addressable collapsed gap between collapsed and expanded. */
@@ -90,12 +102,11 @@ export type ReviewIntent =
 /**
  * Every intent type, as a value rather than only as a type.
  *
- * The wire vocabulary is derived from this list instead of restated beside it: the
- * prototype hand-copied the action union into three more places, so an intent added to
- * one was silently unreachable from the others (`docs/browser-review-seam-audit.md`,
- * B12). The assertion below makes the list total — adding a member to `ReviewIntent`
- * without naming it here fails to typecheck — and `src/session/reviewProtocol.ts`
- * subtracts a named exclusion list from it rather than writing its own.
+ * The wire vocabulary is derived from this list instead of restated beside it
+ * (`docs/browser-review-seam-audit.md`, B12). The assertion below makes the list total —
+ * adding a member to `ReviewIntent` without naming it here fails to typecheck — and
+ * `src/session/reviewProtocol.ts` subtracts a named exclusion list from it rather than
+ * writing its own.
  */
 export const REVIEW_INTENT_TYPES = [
   "selection/select",

src/core/review/navigation.ts

@@ -18,10 +18,10 @@
  *   another file puts that file's header on screen; crossing backward reveals the hunk
  *   itself; annotated-hunk navigation asks for the note. Callers do not re-decide this.
  *
- * The model this plans over is deliberately structural — file keys and hunk counts, plus
- * an annotation index the consumer supplies. Which hunks count as annotated depends on
- * note sources the semantic document does not carry (an imported sidecar, a renderer's
- * merged live comments), so it arrives as a caller-owned fact rather than being guessed.
+ * The model this plans over is structural — file keys and hunk counts, plus an annotation
+ * index the consumer supplies. Which hunks count as annotated depends on note sources the
+ * semantic document does not carry (an imported sidecar, a renderer's merged live
+ * comments), so it arrives as a caller-owned fact rather than being guessed.
  */
 import type { ReviewRevealRequest, ReviewSemanticSelection } from "./state";
 

src/core/review/noteSize.ts

@@ -1,16 +1,9 @@
 /**
  * The one size a review note is measured against.
  *
- * The prototype measured notes in two units: action validation checked `body` and
- * `markup` separately, while the producer and broker checked the whole serialized note.
- * A note could therefore pass the check that admitted it and then fail the check that
- * published it — poisoning an entire snapshot with a capacity error rather than rejecting
- * one note (`docs/browser-review-seam-audit.md`, D1).
- *
- * So there is one measurement, and it is the whole note: everything that will be
- * serialized, counted together, in the unit the transport actually pays. A composer
- * checking a note it is about to create and a producer checking a note it is about to
- * publish call the same function and get the same answer.
+ * A note is measured whole — its serialized JSON, including framing — never field by
+ * field, so the check that admits a note and the check that publishes it cannot disagree.
+ * Composers and producers call the same function (`docs/browser-review-seam-audit.md`, D1).
  */
 import type { ReviewNoteV1 } from "./types";
 import { utf8ByteLength } from "./validation";
@@ -21,10 +14,9 @@ export const MAX_REVIEW_NOTE_BYTES = 256 * 1024;
 /**
  * The serialized size of one note.
  *
- * Measured over the note's JSON form, because that is what a snapshot carries — summing
- * the text fields alone would undercount the framing every one of them is wrapped in, and
- * undercounting is how the per-field check let an oversized note through. Key order does
- * not affect the total, so two encoders that order fields differently still agree.
+ * Measured over the note's JSON form, because that is what a snapshot carries: summing
+ * the text fields alone would undercount the framing every one of them is wrapped in. Key
+ * order does not affect the total, so two encoders that order fields differently agree.
  */
 export function reviewNoteByteLength(note: ReviewNoteV1) {
   return utf8ByteLength(JSON.stringify(note));