fix(review): give every scope a description, derive the done-code unions, correct the sweep docs

Second CodeRabbit pass. Most of the batch was already fixed in fd506c1/09a5e90
and is skipped; what remained:

/login rendered `<dt>` with no `<dd>` for a scope `describeScope` does not know.
A `dt` alone is invalid in a `<dl>`, and AT groups a term with the next
definition it finds — so an undescribed scope was read as meaning whatever the
scope below it means, on the one page whose job is to say what is being granted.
The default case now returns an honest "no description here, ask the
deployment" line and the `<dd>` is unconditional. The e2e comment claiming the
dd/dt count catches a fall-through is corrected: the per-row text assertions
are what catch it now.

`DONE_CODES` and its union are derived from one `as const` tuple in both
`admin/accounts/view.ts` and `account/view.ts`, so the runtime guard and the
exhaustive switch cannot drift.

Docs: "365d ago" is 8 chars (four-digit days are the 9); the login srcset
proposal needs the 132px mobile slot in `sizes`; setNameAction belongs in the
useActionState list; the appraise confirmation cannot name a pool index or
total that `AppraiseActionState` does not carry; five of the six notices need
unconditional mounting, not four; `date_future` cannot hold a `<date>`
placeholder in a static map, and `max` goes stale across UTC midnight; the
th-anchor padding has to be 0.35rem to reach the 28px floor; COMPARISON's
"three" was four, its chain is 13 steps, and its test counts now match the
verified 1134/218. PREAMBLE's `.st` claim is marked corrected in place rather
than deleted — SYNTHESIS and COMPARISON both cite what it said.

typecheck, lint, format:check clean; npm test 77 files/1134 passed;
npx playwright test 218 passed.

Author: guarzo

docs/design-sweep/COMPARISON.md

@@ -68,9 +68,12 @@ Aug-4 ended at "§10. Recommended next commands." Nothing ran. The user's own
 account of that sweep was that they "ran the recommended commands" — manually,
 one at a time, after the fact.
 
-Aug-5 ran a 14-step chain from a single gate. Four blocking items (1–4) are
-closed. 1127 unit tests and 204 e2e tests pass, up from 201 — the three new ones
-cover `/login`, which had no spec at all before this.
+Aug-5 ran the 13-step chain in SYNTHESIS.md's "Proposed command chain" from a
+single gate. Four blocking items (1–4) are closed. On the branch's final state —
+which includes the review fixes made after the chain finished, so these are not
+the numbers any single step ended on — 1134 unit tests and 218 Playwright test
+cases pass, up from 201 e2e cases. Four of the new ones cover `/login`, which
+had no spec at all before this.
 
 Two things the chain taught that no amount of reviewing would have:
 
@@ -96,7 +99,7 @@ spec after every step, not just at the end.
 
 ## Where Aug-5 was not better
 
-Three of the 24 items did not survive contact with the code. The backlog should
+Four of the 24 items did not survive contact with the code. The backlog should
 not be read as 24-for-24.
 
 - **Item 18, second half.** "The queued marker misattributes its own age" is

docs/design-sweep/PREAMBLE.md

@@ -100,10 +100,14 @@ product's own nouns. Do not propose generic replacements.
   — capping the column moved the whole page 144px sideways on a nav click.
 - Body prose caps at 68ch.
 
-**Known open defect, already logged.** `.st` (the Status token) declares no
-`font-weight` and renders at 400 where the label register calls for 600.
-DESIGN.md names this "a defect, not a variant". You may cite it; you do not need
-to discover it.
+**Corrected after dispatch — do not reuse.** This brief told all eighteen
+reviewers that `.st` (the Status token) declares no `font-weight` and renders at
+400, as a known open defect they could cite without discovering. That is wrong,
+and was wrong when they were dispatched: `.st` declares `font-weight: 600`, and
+`DESIGN.md` was the stale half. Five reviewers pushed back; thirteen took it as
+given. The paragraph is kept here rather than deleted because SYNTHESIS.md's
+"Correction to the brief" and COMPARISON.md both refer to what this document
+said — but anyone reusing this preamble for another sweep must drop it.
 
 **Out of scope for this sweep.** Screenshots and starting a dev server. The
 source answers the questions, and a dev server rewrites `tsconfig.json` here.

docs/design-sweep/audit-admin-audit.md

@@ -80,7 +80,7 @@ figures below are computed from the OKLCH tokens, not eyeballed.
 - **Where:** `src/app/globals.css:3253-3301`, `src/app/_components/format-ago.ts:14-15`
 - **Cost:** Three years from now the oldest pages of a log that by design deletes nothing render "1095d ago" in a `white-space: nowrap` cell that is 72px of content box, and the overflow paints out of the pinned column onto the row beneath it — the exact failure the pin exists to prevent.
 - **Principle:** none.
-- **Fix:** The docblock computes the bound as "`365d ago` caps at 8ch"; the string is 9 characters, and `elapsedShort` has no cap on the day count at all. Either bound the formatter (`>= 999d` → `999d+`) or add `overflow: hidden; text-overflow: ellipsis` to `.log--audit td:first-child` so an over-long value truncates inside the pin instead of escaping it. The second is one declaration and makes the whole class of future overflow safe.
+- **Fix:** The docblock computes the bound as "`365d ago` caps at 8ch", which is right for three-digit days — `365d ago` is 8 characters — but `elapsedShort` has no cap on the day count at all, so a four-digit day (`1095d ago`) is 9 and nothing stops the next digit after that. Either bound the formatter (`>= 999d` → `999d+`) or add `overflow: hidden; text-overflow: ellipsis` to `.log--audit td:first-child` so an over-long value truncates inside the pin instead of escaping it. The second is one declaration and makes the whole class of future overflow safe.
 
 ## What is good and must survive
 

docs/design-sweep/audit-login.md

@@ -16,15 +16,15 @@
 - **Where:** `src/app/login/page.tsx:92-104`, `src/app/globals.css:2279-2308`
 - **Cost:** A member deciding whether to hand authGD their character reads `esi-characters.read_contacts.v1`, `esi-characters.write_contacts.v1`, `esi-ui.open_window.v1` and learns nothing they could act on, three lines below a paragraph that explains the same grant in plain English.
 - **Principle:** PRODUCT.md principle 2, "State before action" — the screen has to answer what is true before it offers something to press
-- **Fix:** Invert the `<dl>` so it carries the mapping instead of a heading. `dt` becomes the scope identifier, `dd` becomes the one sentence it buys ("read the contacts on your characters", "add, update and remove contacts under the `{label}` label", "open the in-game window when authGD sends you somewhere"), from a lookup keyed on the identifier with a fallback of no `dd` for a scope the map does not know. Move "Scopes requested" to a `<p>` above the list carrying `.launch__scopes-head` with the register's type, so the label register keeps its member. This preserves everything the docblock at `page.tsx:95-99` argues for (one row per scope, real boundaries, no space-joined blob) and it makes the `<dl>` semantically honest at the same time: today a screen reader announces each identifier as a *definition of* "Scopes requested" four times over, which is the wrong relationship. Keep `overflow-wrap: anywhere` on the identifier.
+- **Fix:** Invert the `<dl>` so it carries the mapping instead of a heading. `dt` becomes the scope identifier, `dd` becomes the one sentence it buys ("read the contacts on your characters", "add, update and remove contacts under the `{label}` label", "open the in-game window when authGD sends you somewhere"), from a lookup keyed on the identifier. An identifier the map does not know still gets a `dd` — a short, honest "no description here, ask the deployment" line rather than no `dd` at all: a `dt` with nothing under it is invalid in a `<dl>`, and AT groups a term with the next definition it finds, so an undescribed scope would be read as meaning whatever the scope below it means. On a consent screen that is the worst available failure. Move "Scopes requested" to a `<p>` above the list carrying `.launch__scopes-head` with the register's type, so the label register keeps its member. This preserves everything the docblock at `page.tsx:95-99` argues for (one row per scope, real boundaries, no space-joined blob) and it makes the `<dl>` semantically honest at the same time: today a screen reader announces each identifier as a *definition of* "Scopes requested" four times over, which is the wrong relationship. Keep `overflow-wrap: anywhere` on the identifier.
 
 ### 3. The LCP element is the one image on the page with no priority, while the 2.2 KB one is boosted
 
 - **Severity:** moderate
 - **Where:** `src/app/login/page.tsx:48-54` and `122-128`, `src/app/globals.css:2223-2246`
 - **Cost:** Every visitor watches an empty 180px box above the corp name while an 82 KB seal downloads behind an explicitly-prioritised 2.2 KB button mark, and the 620ms `seal-settle` entrance has usually finished playing on that empty box before the artwork arrives, so the seal pops in with no settle at all.
 - **Principle:** PRODUCT.md principle 5, "earn the artwork" — an entrance the artwork misses is worse than no entrance
-- **Fix:** Three parts, all small. (a) `fetchPriority="high"` belongs on the seal: at 180x180 (132x132 under 40rem) it is the largest paint candidate on this page, larger than the display-size `h1` at every viewport, and `/brand/emblem.webp` is 81,694 bytes against the button's 2,248. Move it, do not duplicate it, or neither is prioritised. (b) Emit a real preload so the fetch starts from the head rather than after body parse: `preload(brand.sealUrl, { as: "image", fetchPriority: "high" })` from `react-dom` at the top of the component. `BRAND_SEAL_URL` can point off-origin in a fork, which makes this more valuable, not less. (c) The docblock at `page.tsx:44-47` names the missing 1x srcset entry as a real cost paid deliberately to keep the encode untouched. That trade is not forced: `srcset="/brand/emblem-256.webp 256w, /brand/emblem.webp 512w" sizes="180px"` adds the 1x entry without re-encoding the 512 source at all, only cutting one new derivative. A 1x phone currently pulls 82 KB to draw 132 CSS px, a 3.9x oversample.
+- **Fix:** Three parts, all small. (a) `fetchPriority="high"` belongs on the seal: at 180x180 (132x132 under 40rem) it is the largest paint candidate on this page, larger than the display-size `h1` at every viewport, and `/brand/emblem.webp` is 81,694 bytes against the button's 2,248. Move it, do not duplicate it, or neither is prioritised. (b) Emit a real preload so the fetch starts from the head rather than after body parse: `preload(brand.sealUrl, { as: "image", fetchPriority: "high" })` from `react-dom` at the top of the component. `BRAND_SEAL_URL` can point off-origin in a fork, which makes this more valuable, not less. (c) The docblock at `page.tsx:44-47` names the missing 1x srcset entry as a real cost paid deliberately to keep the encode untouched. That trade is not forced: `srcset="/brand/emblem-256.webp 256w, /brand/emblem.webp 512w" sizes="(max-width: 40rem) 132px, 180px"` adds the 1x entry without re-encoding the 512 source at all, only cutting one new derivative. `sizes` has to carry the mobile slot as well as the desktop one — the seal draws at 132 CSS px under 40rem, and a flat `sizes="180px"` would tell the browser the slot is always 180, which on a 2x phone selects the 512 file for a 132px box and gives back none of the saving the new derivative exists for. A 1x phone currently pulls 82 KB to draw 132 CSS px, a 3.9x oversample.
 
 ### 4. A long corp name is clipped by `overflow: hidden` with no way to scroll to the rest
 

docs/design-sweep/audit-payout-detail.md

@@ -28,11 +28,11 @@
   sitting in" (`appraise-form.tsx:9-20`). The `?error=` redirect is the right
   channel for a failure that arrives on page load, and the wrong one for a
   rejection of a control that is 2,000px down a page full of open panels. Convert
-  the four field-level editors that reject on format — `setItemPriceAction`,
-  `setParticipantSharesAction`, `addFlatPoolAction`'s `total_invalid`, and
-  `addParticipantAction` — to return state through `useActionState` from a small
-  client leaf, the way `AppraiseForm` and `NoteForm` already do, and render the
-  message beside the field that produced it. `openInfoAction`'s five failure codes
+  the field-level editors that reject on format — `setNameAction`,
+  `setItemPriceAction`, `setParticipantSharesAction`, `addFlatPoolAction`'s
+  `total_invalid`, and `addParticipantAction` — to return state through
+  `useActionState` from a small client leaf, the way `AppraiseForm` and `NoteForm`
+  already do, and render the message beside the field that produced it. `openInfoAction`'s five failure codes
   are the harder case (they can't be predicted client-side); at minimum they should
   not navigate, since the action persists nothing by its own docblock's argument.
   Keep the redirect only for failures that genuinely have no form still on screen.
@@ -132,13 +132,16 @@
   text." The primitive grew an empty-slot mode specifically so call sites could
   mount unconditionally, and this page — the one with six of them — uses `&&` at
   all six.
-- **Fix:** Mount all six unconditionally and pass the empty value:
+- **Fix:** Mount them unconditionally and pass the empty value:
   `<Notice tone="warn">{poolsWithUnresolvedItems.length > 0 ? <>…</> : ""}</Notice>`.
-  The `errorMessage` one at line 251 is the exception worth arguing — it arrives by
-  navigation, so the document is new either way — but the four derived warnings
-  (unresolved items, the two roster clashes, and the `dropped` report after the
-  `?dropped=` remount) all appear as a result of an action the operator just took
-  on a page that did not reload, and those are the ones the region exists for.
+  Five of the six need it. The `errorMessage` one at line 251 is the single
+  exception worth arguing — it arrives by navigation, so the document is new
+  either way — but the other five all appear as a result of an action the
+  operator just took on a page that did not reload, and those are the ones the
+  region exists for: the unresolved-items warning, the two roster clashes
+  (duplicate unresolved names, and names that are both linked and unlinked), the
+  `dropped` report after the `?dropped=` remount, and the info notice saying
+  repricing is closed while the operation is finalized.
 
 ### 6. The pool-items table is the one table on the page that runs to hundreds of rows, and it is the one that gets no sticky header
 
@@ -210,10 +213,16 @@
   specifically so "a client leaf can tell that apart from either of those"
   (actions.ts:160-163), and nothing consumes it.
 - **Fix:** Consume the `ok: true` the action already returns: render a
-  `role="status"` confirmation naming what was added ("pool 3 added, 4.82b ISK"),
-  and clear the textarea on success — the paste is persisted as `rawPaste` on the
-  pool by then, so nothing is lost by dropping it from the form, and an empty box
-  is the strongest available signal that the submit landed.
+  `role="status"` confirmation saying the paste was appraised and added as a new
+  pool, and clear the textarea on success — the paste is persisted as `rawPaste`
+  on the pool by then, so nothing is lost by dropping it from the form, and an
+  empty box is the strongest available signal that the submit landed. Naming the
+  numbers ("pool 3 added, 4.82b ISK") would read better still, but
+  `AppraiseActionState` is `{ ok: true; dropped }` and carries neither the pool
+  index nor the total; either widen it to carry both, or keep the message to
+  what the existing result can actually back. Do not write the numbers from
+  anything the client already had — the whole point is to report what the server
+  did.
 
 ### 10. The notices name the affected items in the faintest, smallest type on the page
 

docs/design-sweep/audit-payouts-list.md

@@ -24,11 +24,22 @@ and `DESIGN.md` / `PRODUCT.md`.
   `bypassClientGuard` strips `max` by name. Every other client guard on this flow has a
   server twin (`name` → `name_required`, date parse → `date_invalid`, shares → four
   checks, price → `price_invalid`). `max` is the one that does not.
-- **Fix:** Add `date_future` to `NEW_OPERATION_ERRORS` ("An operation cannot be dated in
-  the future. EVE time is UTC, so today is <date>.") and the matching check in
+- **Fix:** Add a `date_future` code to `NEW_OPERATION_ERRORS` and the matching check in
   `createOperationAction` after the NaN test; add the same code and check to
   `setOccurredAtAction` / `OPERATION_ERRORS`, since the detail-page editor has the same
-  gap. Keep `max` as the fast path.
+  gap. The map entries are static strings that `lookupErrorMessage` reads verbatim, so
+  the date cannot live in them — an entry reading "…today is `<date>`" ships that
+  placeholder to the operator literally. Keep the map to the static half ("An operation
+  cannot be dated in the future. EVE time is UTC.") and let the surface that renders the
+  rejection append the day, from the same server-computed UTC date it already uses for
+  `max` — the client's own clock is the wrong source here, since the whole point of the
+  message is which day the *server* thinks it is.
+- **And the `max` guard itself goes stale.** `today` is computed once, when the page
+  renders, so a form left open across UTC midnight carries yesterday's `max` and the
+  browser now blocks a submit dated today — the valid case. Either recompute it at
+  validation time, or drop the native `max` and let the server check be the only guard.
+  Either way the server-side `date_future` rejection is what has to hold, rendered as a
+  persistent field error rather than a bubble that vanishes on the next keystroke.
 
 Two sub-cases the same fix covers, both currently unaddressed:
 

docs/design-sweep/audit-shell.md

@@ -165,14 +165,17 @@ findings rather than padded into them. Six findings, worst-first.
   ```css
   .log th a {
     display: inline-block;
-    padding-block: 0.22rem;
-    margin-block: -0.22rem;
+    padding-block: 0.35rem;
+    margin-block: -0.35rem;
   }
   ```
 
-  That is 24.09px of target at zero net layout cost. Making the anchor fill the
-  cell would be better still but is unsafe under `.log--audit`'s
-  `table-layout: fixed` with `nowrap` cells.
+  The anchor's own line box is ~17px, so 0.35rem (5.6px) of padding each side
+  lands at ~28.2px — the floor, rather than the ~24px a smaller value gives.
+  The equal negative margin is what keeps the net layout change at zero, so the
+  header height and the `scroll-margin-top: 3rem` figure both stay valid.
+  Making the anchor fill the cell would be better still but is unsafe under
+  `.log--audit`'s `table-layout: fixed` with `nowrap` cells.
 
 ### 6. The reduced-motion comment states a mechanism the stylesheet does not implement
 

e2e/login.spec.ts

@@ -21,10 +21,12 @@ test("each requested scope shows a plain-English description, not just the ident
   await expect(rows.nth(0)).toHaveText("esi-characters.read_contacts.v1");
   await expect(rows.nth(1)).toHaveText("esi-characters.write_contacts.v1");
 
-  // Every configured scope resolves to a description in this deployment: the
-  // regression this guards is a scope silently falling through describeScope's
-  // default case (a raw identifier with nothing under it) rather than a
-  // deliberate omission.
+  // Every configured scope resolves to a real description in this deployment.
+  // The count no longer proves that on its own — `describeScope`'s default now
+  // returns a fallback sentence, so a scope falling through still renders a
+  // `<dd>` (a `<dt>` without one is invalid in a `<dl>`, and AT would read the
+  // undescribed scope as meaning whatever the next definition says). The two
+  // text assertions below are what catches a fall-through now.
   const descriptions = page.locator(".launch__scopes dd");
   await expect(descriptions).toHaveCount(2);
   await expect(descriptions.nth(0)).toContainText("Reads the contacts");

src/app/account/view.ts

@@ -8,10 +8,15 @@
 
 /** The four outcomes `/account`'s server actions redirect back with. A code
  *  outside this set (hand-typed, or from a build that has since dropped one)
- *  renders no confirmation at all — see `accountConfirmation`'s default. */
-export type AccountDoneCode = "main" | "unlink" | "wake" | "discord";
+ *  renders no confirmation at all — see `accountConfirmation`'s default.
+ *
+ *  Tuple first, type derived from it: the guard below and the exhaustive
+ *  switch have to agree about the same four strings, and a separately-written
+ *  union is one edit away from disagreeing. Same shape in
+ *  `admin/accounts/view.ts`. */
+const DONE_CODES = ["main", "unlink", "wake", "discord"] as const;
 
-const DONE_CODES: readonly AccountDoneCode[] = ["main", "unlink", "wake", "discord"];
+export type AccountDoneCode = (typeof DONE_CODES)[number];
 
 function isDoneCode(value: string | undefined): value is AccountDoneCode {
   return value !== undefined && (DONE_CODES as readonly string[]).includes(value);