The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").
Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.
Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).
One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
E32 S3. The compiler skipped 93 functions; it now skips 29 (try/finally shapes, two suppressions,
VirtualGrid). react-hooks/refs hits 245 -> 31.
- Popover engine: `ref={pop.refs.setReference}` made the compiler treat the whole popover handle as
a ref, so every component holding one was skipped (21 call sites, the engine and its surface
among them). The setter is destructured now; the same for useScrollFade and useReturnFocus.
- Render-phase `ref.current = x` writes moved to `useEffectEvent` where only effects read them,
else to the new `useLatest` (written in a layout effect) / `useStableCallback`.
- FocusSafeButton (under every Button) read the focused element during render; it reads it through
useSyncExternalStore, which React asks on every render instead of a cached memo.
Bugs this surfaced, fixed:
- Combobox: a chip could keep showing its raw id. The label cache was a ref an effect filled after
the render that needed it; it is state merged during render now (test seen RED).
- useUndoable rendered from a ref plus a forced re-render; compiled, that memoizes a stale value.
The history is state now, mirrored in a ref for the stable callbacks.
- ConfigPanel and the saved-views editor re-seeded their drafts in an effect, painting the stale
draft for a frame; both adjust during render against the remembered key.
- Toaster's handover is written after commit instead of during render; a new test pins that the
inherited set is taken once, at the takeover (seen RED on a per-render re-read).
- IssueComposer mutated a ref prop as a one-shot focus token; it is plain state.
- PlexBrowse wrote its info-scroll memory from an effect declared before the ref, past its own
"one writer" rule; the writer comes first now and the effect goes through it.
The compiler ratchet drops to the measured 29 and now also requires FocusSafeButton and the popover
engine to compile.
eslint now also enforces:
- the React Compiler diagnostics that `recommended` leaves out (todo,
memo-dependencies and ten zero-hit tripwires);
- a description on every suppression, and no blanket disables;
- no focused or skipped tests (vitest and playwright);
- the typed pair no-floating-promises and no-deprecated.
Lint also covers e2e/ and the TypeScript build configs, which `eslint .`
had been skipping in silence.
Every hit is fixed or suppressed inline with a named reason:
- The 31 try statements the compiler cannot lower yet carry a suppression
until S3 rewrites them. Five other compiler skips were plain fixes:
a module counter bumped inside a hook, a ref read in a default
parameter, and a local named like the <section> it renders.
- The real defects behind floating promises are fixed. ChatDock threw at
nobody when the device key store could not be opened. VideoEditor's
play() rejection went unhandled. loadYouTubeApi never rejected, so a
blocked IFrame API left the player on an empty stage forever; it now
fails on a script error or after 15 s, shows its own error with the
way out, and lets the next open try again.
- MutableRefObject, FormEvent and lucide's deprecated brand icon are
replaced. The YouTube glyph now lives in lib/brandIcons.
- The lint-probe harness lets its virtual file names into TypeScript's
default project, so the contract tests still run the real config.
The second half of the button sweep: the remaining hand-rolled buttons in
feature components, the design lab and the components/ui composites now
stand on Button, IconButton, Pressable, Chip or DragHandle.
- Chip + DismissibleChip (filter / token / tag / suggest / ghost, 24px
floor, 44 on touch) replace five chip geometries; FilterChipGroup
renders Chips; overflow lanes measure a mirror painted by chipClass.
- The Plex control row is IconButton's player rung (36, 20px glyph,
round, overlay; 44 on touch); the YT modal bar is on the sm rung with
pressed cycle toggles and names that survive a folded label.
- New variants: IconButton ghost-accent, danger-soft; Button warn on a
new state-warn utility. The .btn-{tone} recipes and btnCls are gone.
- Fixed on the way: membership rows (collection editor, Plex playlist
add) announce checked/mixed; a one-letter tag chip was 21px wide;
descriptionLinks moved out of lib/ (it renders components).
- local/button-primitive: a raw <button>/<FocusSafeButton> outside the
control-primitive files is a lint error (contract-tested, seen red).
Burns the first 14 of the 113 hook suppressions. Three verdicts, applied per site:
A useEffect that should not exist (6):
- Avatar remembered "the image failed" as a boolean an effect reset on src change. The
component is recycled across items in the virtualized feed, so after one avatar 429s the
next item's slot committed the fallback initial over a perfectly good image and only the
post-paint effect cleared it. The failure is now keyed BY URL; derived state cannot be stale.
- Combobox cleared the typed query from an effect on `open`. Moved into setOpenAnd, the single
funnel every close goes through. That also stops a spurious onQueryChange("") AT MOUNT, which
told a controlled caller its own initial query had been cleared before the user touched it.
- CommandPalette reset query + picked row from an effect. Closed now means UNMOUNTED, not
hidden, so a fresh open starts blank because mounting already means that -- and the ranking
memo over every channel, playlist and saved view stops running while the palette is shut.
- Pager's page box held a copy of the page that an effect re-synced; it now derives it and
keeps a draft only while being edited.
- App's ?link= / ?oauth= / ?admin= one-shots asserted "once per mount" with an empty dep list.
The ref guard the codebase already uses in 22 places ENFORCES it, so the honest deps can
include `t` -- which a language switch legitimately changes, and which would otherwise have
re-fired the toast.
Honest deps that also fix something (5):
- App's module preload was keyed on the account id, so a permission change that alters the
module set without moving the id never re-preloaded. `preload()` is idempotent, so depending
on `me` costs nothing.
- App's access-request nudge now re-issues on a language switch; it used to stay in the old
language until the pending count changed.
- App's diagnostics effect: applyDiagnostics already no-ops on an unchanged effective level,
so picking four fields out by hand was guarding nothing and missed any field added later.
- DataTable's two filter effects: a ref holds the latest onFiltersChange so commitFilters can
be stable, which is what lets the effects name it instead of switching the check off.
Genuine escape hatches (7) moved OUT of eslint-suppressions.json into inline directives that
name the invariant at the site -- the toast handover's render-phase global, the live region's
deliberate arming tick, the docked one-slot component, the popover engine's floating-ui list
state, dnd-kit's per-drag `moved` flag, BackToTop's DOM read and the palette's debounced
announcement. A reader of the code now sees why; a JSON file no one opens does not.
Tests: Avatar (3, new file) and Pager (3) pin the derived-state frames with a layout-phase
probe -- layout effects run before any passive effect, so they read the frame the browser would
paint. Combobox (2) pins the close-clears-query contract and the no-callback-at-mount rule.
All three sets mutation-checked against the effect-based originals.
Suppressions 113 -> 99; exhaustive-deps 45 -> 35.
The send, the draft and the screenshots take the session's row lock, so an
autosave or a paste during a send waits for it and then sees it, and two sends
cannot open two issues. A save keeps the delivery's progress; a failed edit
that shows the screenshots is finished by the retry; an answer that is not the
tracker's API (a redirect, a proxy page) is refused, not a crash that loses the
issue's record. Only an attempt that opened an issue counts toward the pace.
Screenshots: deleting one after the send is refused, a user's storage is
capped across sessions, new sessions and zip exports are paced, images are
stored uncompressed in the zip. One liveness rule, one session view, one image
type table.
Logging: a session's elevation is tracked apart from the user's own, so the
browser's own requests (no header) no longer lower the floor a session needs;
Stop ends it at once.
Composer: an image pasted anywhere is taken; what is still unsaved is saved on
close, and "saved" is shown only when it is; the draft is held while it is
sent; a failed screenshot read says so instead of reading as none; focus moves
to what stays; the preview follows the text size. Pill: the step prompt and a
running Stop survive the pill moving between hosts, the prompt keeps the
dialog's Tab trap, the toast region lets clicks through, other tabs hear of a
Start or Stop, the last seconds are sent before the cap, a capture with no
frame is an error.
The toast stack already has a host in the page and in every overlay (dialogs,
both players, the fullscreen player), the topmost one shows it, and inside an
overlay it sits above the player's control band. lib/toastDock lets one control
use exactly that placement; it renders outside the live region, so a ticking
control is not read out every second.
The Feed's chips + Sort + View row did not fit one line at 375px and pushed
the page 21px wide; the toast stack capped its width by the viewport while
it sits right of the sidebar. The toolbar rows (Feed, Plex browse) wrap,
and the toasts are capped by their host.
The 43 react-refresh/only-export-components warnings the gate let through meant an edit to a
provider (or AnchoredPopover, Toaster, plexDetailUi, …) could not be hot-swapped: Fast Refresh
reloads a module that also exports hooks or helpers.
- Each provider's contexts, the types its hooks return and the hooks themselves live in a sibling
context module (navigationContext.ts, channelsContext.ts, …); the provider file keeps the
component. loadAccountFilters moved next to the feed filters' defaults (feedFiltersInit.ts),
following navigationInit/playlistsInit.
- Helpers that sat in component files have their own modules: welcomeMode, downloadTitle,
plexPlayable, channelSync, ui/anchoredPopoverHooks, ui/pageToolbarSlot, ui/pagerContent,
ui/toastLevels, ui/overlayOrder, ui/tableAlign, lib/plexDetail; the cells of the channel columns
and the registry's page wrappers are components files of their own (channelCells, modulePages).
- usePersistedTab, an alias of useAccountPersistedState, is gone; its callers use the original.
npm run lint is now eslint --max-warnings 0, so a warning fails the gate like an error does.
Targeted round over 3cd0c525's fix surfaces: 14 raised, 14 fixed.
- VideoEditor: the "only X of Y clips" toast went silent exactly on the job-cap/quota refusal it was
written for (req had already reported the error); it now always shows. The cut list holds still while
Create queues, and a retry continues the "(i/N)" series.
- ChannelPage: Subscribe/Unsubscribe use Button `loading`, "Load more" and ChannelDiscovery's Subscribe
block the click with aria-disabled instead of native disabled; busy Block/Hide no longer light up on
hover.
- listNav: an item can name itself (`navItemAttrs(i, name)`) — Plex posters, seasons, episodes, message
rows and download rows pass their title; focus on a toast's Undo keeps the removed card's slot.
- AnchoredPopover: the hand-back only for focus-managed surfaces (a hover card no longer pulls focus
back); the dead panel-ref check is gone.
- AccountPanel: no confirmation mail offered to the demo account; send_failed copy no longer blames
the message for an unreachable server. Pager comment points at scrollParent.
Round 5 extracted `ui/dialogFocus` so no shell would hand-roll the focus
contract, then introduced four ways to lose the focus anyway — all four measured
in jsdom against the real components.
**Ownership was narrower than the trap.** `ui/Modal` traps Tab over the whole
dialog on purpose, so a toast's Undo or ✕ stays reachable; ownership was tracked
over the panel alone. Tabbing to that ✕ therefore read as "the user left the
dialog", and since removing a toast fires no `focusin` to re-arm, closing
afterwards skipped the restore and left the focus on <body>. Before this hook
existed the restore was unconditional, so it was a regression.
**A dialog stacked above us is still ours.** A nested `ui/Modal` portals into its
own host, so it is outside the outer dialog's root however wide that scope is
drawn — widening alone could not fix it. Both closing in one commit left the
focus on <body>: the outer skipped its restore, and the child's own target (a
control inside the outer panel) was already detached, because passive cleanups
run parent-first. Ownership now means "inside my dialog, or inside a modal layer
above it", which is what the rule always intended.
**The wizard disagreed with `ui/Modal` on its first line.** `trapTab(e, panel)`,
with neither the dialog-root scope (so its own toast host was outside the cycle
— the connection notice never auto-dismisses, and its ✕ was unreachable from the
keyboard) nor the `container` argument the JSDoc exists for: `panel.contains(
panel)` is true, so focus sitting ON the tabIndex=-1 panel matched no branch and
a Shift+Tab walked into the app behind the scrim. Both call sites now pass the
same three arguments.
**The toast handover was a render-phase read-modify-write.** A `useMemo` read a
module global that the next line overwrote; React re-invokes a component body
and every `useMemo` factory in it on a StrictMode double render, and the LAST
invocation commits — so the second pass re-read what the first had just written,
the takeover host classified its OWN new toast as inherited, and the dev build
behaved exactly like the code round 5 replaced. Snapshotted through a ref keyed
on `isTop`, so a repeated invocation of one render is a no-op.
And the fourth `aria-modal` shell, `Welcome`'s screenshot lightbox, is finally
swept into the same contract: it declared itself a modal dialog while leaving
the focus on the thumbnail behind an 80%-black scrim and letting Tab walk the
landing page underneath.
The headline fix of the previous commit — StrictMode's re-mount taking the focus
off an `autoFocus` field — had no test at all; deleting it left all 1009 green.
It has one now.
9 tests, mutation-checked against 8 mutations, each red on the test that names it.
Deep-review round 5, over round 4's own fixes.
- Round 4 gave `ui/Modal` a persistent portal host so a fullscreen transition would stop tearing
the dialog down. The host was attached in a `useLayoutEffect` — one commit phase AFTER React
runs `autoFocus` in `commitMount`, and `focus()` on a disconnected node is a no-op. Every
autoFocus in every dialog was dead: Mod+K opened the command palette with the focus on the ✕,
so typing searched nothing and Enter closed it; the same for the download rename field and
every non-danger confirm. The host is attached in a `useInsertionEffect` now, a whole phase
earlier, and the whole re-parent — including the focus save/restore across the move, which
`appendChild` needs because it is remove-then-insert — lives in one `useFullscreenPortalHost`.
`ui/AnchoredPopover` had a second copy of that re-parent WITHOUT the focus restore, and a
comment claiming focus survived it; both now come from the hook.
- `ui/Toaster` classified a toast raised in the SAME commit as a host takeover among the ones it
inherits, because it read the store live at the takeover render. A dialog that notifies and
closes in one handler (UploadDialog) therefore put its toast up with no role, into a region
still `aria-live="off"` — announced by neither path. "Inherited" is now what the outgoing host
last had on screen, and a toast that is NOT inherited is announced even inside the unarmed
window.
- The onboarding wizard renders its own shell, and Escape ordering was all of the dialog contract
it had: no role, no label, no initial focus, no trap, no restore. A keyboard user tabbed out of
the first-login wizard into the app behind the scrim. The focus half of the contract is now one
shared `ui/dialogFocus` that both `ui/Modal` and the wizard use.
Every fix has a regression test, each mutation-checked (6 mutations, all red on the assertion
they are named after).
Deep-review round 4, the overlays.
- Toast hosts were ranked by mount time, and PlayerModal swaps its host on every fullscreen change:
leaving true fullscreen with the download dialog open put the player's host back on top, so toasts
were drawn under that dialog's scrim and a click on one closed the dialog. Each overlay takes a
rank once, when it opens (`useOverlayOrder`), and hands it to every host it renders.
- A dialog open while fullscreen is left keeps a stable portal container (moved, not re-created):
React tore the portal down when `fsRoot` changed, resetting a half-filled form and dropping focus
to <body>. Focus is put back after the move.
- The onboarding wizard is a layer, not a raw document listener: one Escape used to dismiss the
error dialog over it AND end onboarding, skipping the write-scope step.
- `navigateTo` (a toast's "View", clicked inside the player's own host) goes over the overlays it
unmounts, so the player's Back-to-close cleanup no longer pops the page it just opened — the same
fix Shift+W already had. The access-requests toast action too.
- The live region arms itself a task after it takes over inherited toasts: React flushes effects in
the same task as that render, so the "silent inheritance" arming was a no-op.
- The overlay-host guard matches class tokens in any order (PlexPlayer's root was never checked),
globs `ui/` too (ui/Modal was not checked either), and requires the rank.
Deep-review round 3 of bug hunt 2026-09 (the round-2 overlay toast host, finished).
- Inside an overlay the toast stack sits above the bottom band (bottom-24, the UpNextCard rung) and
lets clicks between cards through: at the app's own spot it covered the Plex transport row and
PlayerModal's docked bar and took their clicks while a toast was up (measured: play/pause, ±10,
stop, prev/next, mute all hit the toast).
- ui/Modal and the onboarding wizard mount an overlay host too, so a toast raised under any dialog
is readable and a click on it no longer closes the dialog; the Modal's Tab trap spans the whole
dialog, so a toast's action and dismiss are keyboard-reachable. A test now enforces the
convention: a component that covers the viewport at z-overlay must mount <Toaster overlay />.
- PlayerModal's host moves into the stage in true fullscreen (outside the element holding the lock it
was painted under it).
- A host that takes over shows the toasts it inherits silently (no second role=alert, its region
aria-live=off until they are in), and each countdown bar starts as far in as its toast already is
instead of restarting while the real dismiss timer does not.
- The overlay-host registry uses lib/store instead of a hand-rolled listener triad.
The app's Toaster lives in the content column, which is a stacking context (its `@container` implies
layout containment), so both full-screen players — portalled to <body> at the same z-index — painted
over every toast. Round 1's "a failed Remove is reported" fix was therefore still silent inside the
player: its toast sat under an 80% black, blurred scrim (elementFromPoint returned the scrim), and a
click on it closed the player.
An overlay now mounts its own `<Toaster overlay />`, and exactly one host shows the toasts: the most
recently mounted overlay, else the app's. Same component, same look — and inside the aria-modal
dialog, where a screen reader can still reach it. A click on a toast inside an overlay stays there.
Both players get one (the Plex player's also works in native fullscreen, where nothing outside the
fullscreen element is drawn). Verified on localdev: the removal failure reads above the scrim,
bottom-left, and clicking it leaves the player open.
The app HAS a platform layer -- AnchoredPopover (22 importers, with
arrows/typeahead/Escape/anchor-flip), QueryState (21), Modal (17),
PageShell, DataTable, Tabs, Tooltip and ~26 more -- but it sat flat among
~160 feature components with nothing marking it as one. Several of those
files describe themselves as 'the one place' or 'canonical' and were
still bypassed by hand-rolled markup, because a layer nobody can see is a
layer nobody uses; three separate inventory passes of this codebase
misread it the same way.
Moves the 33 domain-free modules into components/ui/, rewrites the
imports in 89 files, and makes the boundary a lint rule rather than a
convention: inside ui/ a one-level-up import (a feature component) and
any lib/api slice are errors. The rule was verified to fire on both and
to leave generic lib/ helpers alone. README.md states which side of the
line a component belongs on and why.
No behaviour change: 3x tsc, eslint, knip, prettier and 687 vitest tests
all green; no backend file touched.
peter