The guard asserted EXACT completeness over a population enumerated by a directory walk, so an untracked .ts/.tsx under web/src/ entered it and failed as unregistered on that developer's checkout while CI — which only ever checks out tracked files — stayed green. The glob still supplies file CONTENT; the POPULATION is now the git index, read by web/vite-plugins/trackedSourceFiles.ts in Vite's own Node context and handed to the app project as a virtual module. That reaches the index without admitting @types/node to tsconfig.app.json, the obstacle that deferred this in #818. Three mechanisms carry the proof, each added because the previous was measured insufficient: a closed-form restatement of the shared scope predicate (sharing no helper at any depth with what it checks); a second independent `ls-files --others` query cross-checking the population; and real-git tests that execute the derivation against a temp repository. Six residuals are stated with their MEASURED fail-directions, and testing.guard-derives-population-from-source gains a bounded exception plus the closed-form criterion. fixes #819 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
85 KiB
SPA conventions — "Add a screen" playbook
Purpose: a precise playbook for adding a new screen (or sub-path editor) to the ChicoryTV React SPA
(web/), for an agent with no prior context in this repo. Update this doc in the same PR that
changes any convention below.
Companion to api-conventions.md (the API surface the SPA talks to) and docs/contributing.md
(general repo conventions).
1. Stack & layout
Vite + React + TypeScript, builds to ErsatzTV/wwwroot/app (see web/vite.config.ts:
base: '/app/', build.outDir: '../ErsatzTV/wwwroot/app'), served by the ASP.NET host at /app.
- Composition root:
web/src/App.tsx— owns the active route, approved Libraries sub-path, navigation guard integration, and theme; it composes the shell with the matched screen. - Routes + nav:
web/src/app/routes.tsx— the ONE stable module-level array ofScreenRouteobjects (path,label,title,kicker,icon, etc.) plusallowSubPaths?: boolean, the matcher, and sidebar group definitions. Route objects must never be rebuilt per render. - Shell + screen dispatch:
web/src/app/AppShell.tsxowns Sidebar/TopBar/Connect/version/theme chrome;web/src/app/ScreenContent.tsxexhaustively maps a matched route to its screen and owns the Media/Libraries route wrappers. - Screens:
web/src/screens/*.tsx, one file per top-level screen, generally with a colocated*.test.tsx. - API clients:
web/src/api/<domain>.ts(see §4). - Build-time plugins:
web/vite-plugins/*.ts— Vite plugins that run in NODE, not in the browser bundle, and are type-checked undertsconfig.node.json(nevertsconfig.app.json, which must stay free of@types/node). This is the seam for anything a test needs that only Node can answer —trackedSourceFiles.tsreadsgit ls-filessopageSizeCallSites.guard.test.ts, the one SPA guard that derives a file population, takes it from the git index rather than a directory walk (#819). - Styling:
web/src/shell.css(+web/src/components/components.css) — utility classes with actv-prefix (~690 occurrences across those two files). Reuse an existingctv-*class before inventing a new one.shell.csscarries the only base reset —html, body { margin: 0 }plusbody { background: var(--surface-app) }(the 8px default body margin otherwise frames every full-viewport layout with a light border, #373). There is no globalbox-sizingreset (the SPA is authored under the defaultcontent-box), so any element that combineswidth: 100%with padding/border must setbox-sizing: border-boxlocally or it overflows its container — e.g..ctv-nav-item(#377). Preferwidth: auto(shrink-to-fit) overwidth: 100%+ padding where you can.
2. CRITICAL: sub-path screens must own their own pathname state
If a route sets allowSubPaths: true (e.g. so /app/blocks/{id} works under the /app/blocks nav
entry), the screen component itself must track window.location.pathname and listen for
popstate — do not rely on the composition root re-rendering ScreenContent when the sub-path
changes.
Why: app/routes.tsx's routeFromLocation() matches an allowSubPaths route by prefix
(pathname.startsWith(\${route.path}/`)) and returns the **same ScreenRouteobject reference** for the base path and every sub-path under it.App's state update is setActiveRoute(routeFromLocation()); React's useStatesetter bails viaObject.iswhen the new value is reference-equal to the old one — so navigating from/app/blocksto/app/blocks/42(or between/app/blocks/42and/app/blocks/17) **never re-invokes ScreenContent** at the Applevel. See the comment block directly abovePlayoutsRouteScreeninscreens/PlayoutsScreen.tsx for the canonical explanation, and its implementation (useState(() => window.location.pathname)+useEffectwith apopstate` listener local to the wrapper component) for the fix.
Exemplars of screens that already do this correctly: BlocksScreen.tsx, TemplatesScreen.tsx,
DecosScreen.tsx, DecoTemplatesScreen.tsx, and the PlayoutsRouteScreen wrapper colocated in
PlayoutsScreen.tsx (which owns two sibling sub-paths, /playouts/{id}/alternate-schedules and
/playouts/{id}/templates, dispatching internally via parsePlayoutSubRoute).
Exception — a guarded sub-path route defers pathname ownership to App. When a sub-path route's
screens ALSO register a dirty guard (§8), the wrapper must not self-listen for popstate; App
owns the pathname and passes the approved sub-path down as a prop. The LibrariesRouteScreen wrapper
in app/ScreenContent.tsx is the exemplar (sub-path parsed by parseLibrariesSubRoute, dispatched
via a flat switch, sub-path supplied by App's librariesSubPath state). See §8 for why child-before-parent
effect order makes the self-listening pattern unsafe here.
3. Data loading pattern
Reference implementation: web/src/screens/LogsScreen.tsx. Structure to copy for any screen that
fetches from the API:
- A discriminated-union state type covering loading/success/error, e.g.
type LogsState = { status: 'loading'; ... } | { status: 'success'; ... } | { status: 'error'; ... }. - A
seqRef(monotonically incremented request counter) +activeRef(mount-tracking boolean, flipped in a mount/unmountuseEffect) pair — guards against a stale, slower request overwriting a newer one's result, and against setting state after unmount. - The actual fetch lives in a
useCallback(load), called from a separateuseEffect(() => { load(); }, [load]). - Lint rule —
react-hooks"no set-state-in-effect": never callsetStatesynchronously in the body of auseEffect. State transitions happen only inside event handlers or promise.then()/.catch()callbacks (as inLogsScreen'sload). This is enforced byeslint-plugin-react-hooksinweb/eslint.config.js— a synchronoussetStatein an effect body will failnpm run lint.
3a. "Keep results visible during refetch" ⇒ gate mutations + show a refreshing cue
Some grid screens deliberately keep the previous successful result set rendered while a refetch
is in flight (no full-screen loading state on a query/kind/page change), so the grid doesn't flash
empty. SearchScreen.tsx and MediaBrowseScreen.tsx do this. If such a screen also carries
mutation surfaces (per-card Add-to menu, Select/select-mode, a selection action bar, "Add all",
"Save as smart collection"), those surfaces would otherwise stay live over a stale result set —
an add/select action then targets the about-to-be-replaced items, or (worse) a query-wide "Add all"
bulk request resolves against the previous query. This was issue #221 (adversarial-reviewer#18).
Convention — when a screen keeps stale results visible during a refetch:
- Key the success state to the request params that produced it. Store the identifying params on
the
status: 'success'variant (SearchScreen: thequery;MediaBrowseScreen: akind|query|pagekey), set in the seq-guarded.then. Deriveconst refreshing = state.status === 'success' && state.<key> !== <current params>;in render. Prefer this over a synchronously-setrefreshingflag: setting state synchronously from the load path trips thereact-hooks"no set-state-in-effect" rule (§3). Invariant, not a guarantee: the derivation is only self-correcting when every value the current params can take will actually trigger a fetch. Ifload()early-returns for some param value (e.g.SearchScreen's blank-query guard),statenever updates for that value and a stalestatus: 'success'variant lingers — so the comparison must exclude params that suppress fetching, or gate the whole flag on the same condition that gates the fetch (SearchScreen:const refreshing = hasQuery && state.status === 'success' && state.query !== query.trim();— fixed post-review in #222 after the naive derivation got stucktrueonce the query was cleared to empty, see PR discussion for #221). - While
refreshing: show a visible cue (arole="status""Refreshing…" row with<Spinner>plus the.ctv-media-grid-dimopacity class on the grid) and disable every mutation surface — per-card Add-to menu (withhold theactionsnode), select toggle + in-grid selection (const canSelect = selectMode && !refreshing;gatesonToggleSelect), the selection action bar, "Add all", "Save as smart collection". Card navigation (onOpen) may stay live — but only outside select mode: whileselectMode && refreshing,MediaPosterCardfalls back toonOpenwheneveronToggleSelectis undefined, so both props must be withheld together or a mid-select click navigates away instead of no-op'ing. The select-mode toggle itself should only be disabled while refreshing when entering select mode (refreshing && !selectMode) — exiting only clears selection, not a mutation, so it must stay available. - Bind async bulk completions to their request params, not just mount. A whole-query/whole-set
request (e.g.
getSearchAllItems) must, on resolve, check that its snapshotted params are still current (compare against a ref that always holds the committed value —SearchScreenreuseslastQueryRef) and discard otherwise. Checking onlyactiveRef(mounted) is insufficient.
3b. Paged list endpoints clamp server-side — page to completeness ONLY for bounded lists; a media-library picker searches instead
Every paged /api/v1 list endpoint (rerun-collections, multi-collections, library/browse, search,
trakt-lists, …) clamps pageSize to its own controller's MaxPageSize (100, as of #644) regardless
of what the client requests. A screen that asks for pageSize: 1000 to "get everything in one call"
gets only the first MaxPageSize rows back, silently — no error, no truncation indicator, no paging
UI to notice the gap. This was issue #644 (following on from #634, which fixed the first instance —
SchedulesScreen's rerun-collections picker load).
Two classes of call site, treated differently (decision record:
docs/decisions/records/spa/library-pickers-resolve-by-search.md,
spa.library-pickers-resolve-by-search, superseding spa.list-completeness-vs-bounded-pickers —
the #644 follow-up got Class A right and Class B only half right):
-
Class A — bounded-by-construction lists (collections, multi-collections, smart collections, playlists — admin-created, hundreds of rows at most): genuinely need the complete list, and completeness is cheap. Use the shared
loadAllPageshelper (web/src/api/paging.ts, re-exported viaweb/src/api/index.ts) instead of an inflatedpageSize:const { items, complete } = await loadAllPages(getMultiCollections); // pages against totalCount, cap defaults to 100It pages
pageNumfrom 0 (per §"paging-zero-based" inapi-conventions.md) against the response'stotalCount, stopping — and reportingcomplete: false— on an empty page (defensive guard against atotalCountthat never converges) or on an abortedsignal. Always checkcomplete: a caller that needs the full list must not treat a resolved promise as proof the list is whole (a partial result is otherwise silently indistinguishable from a complete one — the same defect class as #644 itself, sinceGetLibraryBrowseItemsHandler.HydrateMediaItemscan legitimately drop stale Lucene hits and produce a short/empty page in normal operation). Rendercomplete: falseas its own copy ("List may be incomplete — retry to reload"), never through search-narrowing text — aloadPickerOptionsresult that can come from either class carries ahint: 'incomplete' | 'none'discriminator, not a boolean shared with an unrelated condition (#644 follow-up round-3 review F1). Pass anAbortSignal(4th arg) from the caller's effect cleanup so a superseded load stops issuing further page requests, and gate anyconsole.warnon!signal?.aborted— a superseded or user-aborted load returnscomplete: falsetoo, and that's expected, not a defect. Do not raise the server-side cap to work around any of this — theapi.search-allitems-pagingprecedent is that the client bounds itself and the server stays bounded. -
Class B — media-library pickers (Episode / Song / Image / Movie / MusicVideo / TelevisionShow / TelevisionSeason / Artist / OtherVideo / RemoteStream — the largest tables in an install, tens of thousands of rows possible): resolve by search, do not window the type at all (#651). Neither
loadAllPages(~200 serial requests for a 20k-row library, each more expensive than the last sinceLuceneSearchIndex.SearchcomputeshitsLimit = skip + limit) nor a single bounded page (an arbitrary alphabetical prefix, unusable as a picker even once the truncation is made visible) is acceptable. Render the sharedSearchPicker(web/src/schedules/pickers.tsx) oversearchLibraryPickerOptions(web/src/api/libraryBrowse.ts):const searchLibrary = useCallback( // memoize: SearchPicker lists `search` in its effect deps (q: string) => searchLibraryPickerOptions('Episode', q), [] );The helper owns both bounds: at most ONE
getLibraryBrowseItemsrequest per settled query, at mostLIBRARY_PICKER_RESULTS(25) rows, and no request at all belowLIBRARY_PICKER_MIN_QUERY(2) characters. Selecting a media-library type must issue zero requests. The per-kindLIBRARY_PICKER_RESULTScap is the only truncation this class has — there is no whole-type window left to hint at, so the oldShowing the first 100 of 5000 — use search to narrow.copy is gone from these pickers along with the window it described. Surfacing the per-kind cap is permitted wherever it is reachable, and required only where bulk selection makes the count actionable — see theAddItemsDialogsub-bullet below, which sums the cap across kinds and renders aShowing N of M matcheshint for exactly that reason. Prove the bound with a request-count assertion against a large (20k-row) fixture, not by inspection.-
SearchPickeris the single-select SHAPE, not the rule itself. A MULTI-select picker (CollectionsScreen'sAddItemsDialog— checkbox rows, many items added at once, fanned out over several kinds) cannot renderSearchPickerand must not be forced to. It satisfies this section by taking the same constraints the helper enforces for single-select — the gate onLIBRARY_PICKER_MIN_QUERY,titleContainsQuerythe typed text,LIBRARY_PICKER_RESULTSper kind — viasearchLibraryBrowseItems(web/src/api/libraryBrowse.ts), a sibling ofsearchLibraryPickerOptionsthat returns fullLibraryBrowseItemrows plustotalCountinstead of{id, name}, so the bound lives in the helper rather than the caller (#685 review finding 2). There is no post-fetchslice, but the per-kind cap can still truncate the real match count — this is a bulk multi-select add, where "add the 40 matching episodes" is a first-class use, soAddItemsDialogsums each kind'stotalCountand renders aShowing N of M matcheshint once it exceeds the rendered rows (finding 4 — an earlier revision of this bullet called the truncation nothing left to hint at). The gate's home is the shared HELPER, not the screen — however single-sink the screen's own function looks. #685 got this wrong twice in a row, and the second time is the instructive one: the check sat insiderunSearch, which genuinely IS the one sink both entry paths route through, so it read as correct. It was still a duplicate of the helper's gate, and the two masked each other: as of4be3f247d— which had no unit tests on the helper — deleting EITHER copy left the whole suite green, so the min-query boundary test pinned nothing. Removing the screen's copy is what made the helper's gate load-bearing. The invariant, not the count: every gate must have at least one test that reddens when that gate ALONE is removed. A guard you cannot redden is not a guard, and "it's the single sink" is not evidence that it is the only one. The async guards below reach this screen too (#740), and debouncing is not what triggers either of them — the two guards have different triggers and neither subsumes the other:seqRefis triggered by concurrent reachability: two or more entry points into one async sink, so a superseded response can settle after a newer one.AddItemsDialoghas exactly that — a form submit fanning out overDEFAULT_SEARCH_KINDS, and a kind-chip click issuing a single request — sorunSearchcarries it even though it is an explicit Search-button submission rather than the debounced typeahead "Debounced typeaheads" mandates it for.useIsMountedRef()is triggered by unmount-reachability, which is present with a single entry point too. Check whether the surface can close mid-request through a path that does not consult the in-flight flag: a disabled submit button does not settle it, becauseDialogcloses on Escape and on a backdrop click without consulting anything. BothrunSearchandsubmitcarry it for that reason.
Apply the guard to this component's OWN state only. A parent callback invoked after the await —
onAdded, reloading the list after a write that succeeded — belongs to the parent, which owns its own guarding; dropping it because THIS component went away leaves the list stale.searchingis written under the seq guard too, which is the half easily missed: a superseded search'sfinallyotherwise clears the spinner while the newer request is still in flight, and the dialog claims to be idle mid-request. So is the catch path — an error from a superseded search must not wipe the results a newer, successful one has already painted. -
Compile typed text; never forward raw Lucene. Send
titleContainsQuery(text)→title:*<escaped>*. The index's default field does not match bare title words (Alphafinds nothing for "Show Alpha" — seee2e-local.md), so a raw forward looks broken in a name picker. Reuse the helper; do not re-implement the escaping (same rule as the #440 Auto-Tune typeahead, same shapebuilder/rules/compile.tsemits forcontains). The escaped set includes&and|, because Lucene's boolean operators are&&/||and a title likeRock & Rollotherwise compiles to something Lucene parses as syntax. Drive the escaping test from the exported character set (LIBRARY_PICKER_LUCENE_SPECIALS), one character per case — a test carrying its own hand-copied "every special" sample cannot see what is missing from that sample. -
The bound belongs to the helper, not the caller.
searchLibraryPickerOptionsclampspageSizetoLIBRARY_PICKER_RESULTS; a documented bound a caller can exceed by passing a bigger number is not a bound. -
Render the current selection from the owning record, not from the result set. An item already selected but outside the current results must still display — losing it on edit is data loss, not a cosmetic defect. Rerun collections and playlist items carry
selectedNameon their own DTOs;FillerPresetFullResponseModelstores only an id, so its edit path resolves the name with a single by-id detail read (getShow/getSeason/getArtist) and degrades to#idon failure — never to a cleared field. -
A read model that derives an id and its name from the same eager-loaded navigation reports no selection at all when that navigation isn't loaded — a successful 200 indistinguishable from "the user cleared it". (
RerunCollectionsController.ProjectToResponseModeldoes exactly this, andMediaCollections/Mappermaps RemoteStream through_ => null; tracked as #671.) An earlier revision of this section required a client-side guard that preserved the id across such a response. That guard is gone and must not be rebuilt — it only ever preserved a value seeded from the list row, which is itself null in production for every row, and the reconciliation it required is what the initialize-once rule below replaced. The correct handling is to show the server's answer honestly: no selection, Save disabled, and the validation badge saying why. -
An id NEVER travels without its namespace — in results, in options, in cached result sets. A media/collection id only means anything inside the type that produced it, so any structure holding ids must hold the type too, and identity is compared as
(type, id). Three places this bites, all the same bug:- A typeahead's cached results must be keyed on
(source, query), not the query text. Same query, different source ⇒ the results are not stale, they are wrong: hide them and re-query. Keying on text alone lets a re-query guard suppress the new source's request and leave the old namespace's hit clickable under the new label. - List-backed
<select>options must carry the type they were loaded for and be dropped the moment the active type differs — otherwise the previous type's rows stay selectable during the replacement load on a slow connection. - A local edit whose type contradicts the server's is a conflict, not something to reconcile (below).
- A typeahead's cached results must be keyed on
-
Initialize an edit draft ONCE, from the detail read — never reconcile a late response against an open form. This supersedes an earlier prescription here for merging a refresh into a draft field-by-field/atomically with touched-field tracking. That reconciliation layer produced a HIGH finding in three consecutive review rounds of #651, including three cross-user lost updates, and the last of them (a merge with no immutable baseline, so it could not tell a local edit from a server change) is unfixable without adding a third-way baseline — more machinery on the surface that was generating the bugs. Instead:
draftstarts asnullfor an existing record and the form does not render until the detail GET lands. There is then no draft for a late response to reconcile against, and no window in which the user can edit something about to be replaced.- Do not seed from the list row. It is not authoritative: for rerun collections the list
handler applies zero
.Include()s while the detail handler applies fourteen, and both project through the same mapper, so the list response is a strict SUBSET of the detail one (#671). A seed can only add a race, never information. Verify that claim for your endpoint before relying on it. - Fail CLOSED on a missing concurrency token. Writing the ETag in the same callback that
sets the draft is not the same as "a draft implies an ETag" — the response can simply omit
the header, and then the PUT carries no
If-Matchand silently force-writes. No token ⇒ no editable draft (error + Retry/Back). Note this makes your test mocks load-bearing: a detail mock that omitsETagwas previously exercising the force-write path without saying so, so give every single-record GET mock a real ETag and test the absent case explicitly. - Bound the load and always offer a way out. A caller-supplied fetch with no abort signal can hang forever; race it against a deadline, and give the loading view a Back control so a hung request is never a dead end.
- Detect conflicts at save time via the existing
If-Match→ 412 → Reload path. Reload sets the draft back tonulland re-runs the same initialize-once load, so "replace" needs no separate policy and the form is unmounted while the replacement is in flight.
FillerPresetsScreenandPlaylistsScreenalready worked this way;RerunCollectionsScreenwas the outlier that seeded from its list row, which is where every one of these defects lived. -
A name resolved asynchronously must be keyed to the id it was resolved FOR, and must refuse to overwrite a label that already names a different id. A slow by-id read landing after the user has picked something else would otherwise label the new selection with the old item's title while the id — and therefore what gets saved — says otherwise. Keep the guard at the writer, where it is reachable and testable; a second render-time id comparison is unreachable once every writer sets the label and the id together, and an unreachable guard is an untested one.
-
Only Lucene-backed types.
GetLibraryBrowseItemsHandlerappliesqueryas a Lucene clause for media items but as a plain SQLLIKEonNamefor the collection-family types (Collection / SmartCollection / MultiCollection / RerunCollection / Playlist). A compiledtitle:*x*sent at those matches nothing literally. Keep the collection-family pickers on their Class A / bounded single-page loads —FillerPresetsScreen'sCOLLECTION_TYPESmarks the search-driven entries withsearchable: truefor exactly this reason.
-
If a screen shows a bounded preview or has real paging UI (a "load more" button, a page-size
selector, a fixed-size typeahead result list), a pageSize at or below the cap is correct as-is —
loadAllPages is only for "I need literally everything, and the list is small by construction"
call sites.
Debounced typeaheads: arm on focus, and guard on mounted as well as on sequence. A typeahead that
fetches on mount multiplies by the number of rows on screen (an N-rule tree fired N unrequested
facet lookups before #578); arm the effect on the input's onFocus instead. And pair the monotonic
seqRef stale-response guard with the shared useIsMountedRef() (web/src/hooks.ts) in every async
callback — seqRef drops an older response, but says nothing about whether the component still
exists.
A custom picker replacing a native control owes you its keyboard behaviour. A <select> is
fully keyboard-operable; swapping in a listbox-and-input is an accessibility regression unless it
implements the ARIA combobox pattern — role="combobox" + aria-expanded/aria-controls/
aria-autocomplete on the input, ArrowUp/ArrowDown to move a virtual cursor exposed via
aria-activedescendant, Enter to commit, Escape to dismiss, options as non-tab-stops
(tabIndex={-1}) marked with aria-selected. Note this changes what getAllByRole('combobox')
matches in tests: count <select> elements when that is what you mean. Two failure modes that only
appear once the widget is asynchronous:
- Freshness is
(source, query), and cached failures are not answers — but they are not licences to retry either. ASearchPicker-style cache must record which source produced the results and whether the attempt succeeded. Caching a failure as an authoritative empty result turns a transient 500 into a permanent "No matches" that reopening can never clear. But simply declining the cached failure re-runs the effect and schedules another request every debounce — a request storm on a persistent outage. Keep the two apart: aresultsFor.okflag says whether the held answer is authoritative, and a separate attempted key (a ref, so writing it doesn't re-render) suppresses automatic retries until an explicit user action — reopen, focus, or edit — re-arms it. - Put a validity predicate at the BOUNDARY the class crosses, not at the site the bug was found.
An entity-reference id (
selectedId,collectionId,mediaItemId, …) is bound by the API as a 32-bit integer, so1.5or2147483648renders and commits fine and then fails on write. Such ids enter editor state through several doors — search results, list-backed<select>options, and the selection restored from a detail read — so a check added to whichever one surfaced the defect leaves the others open (this is how #651 produced the same finding in two consecutive rounds). Share one predicate (isSelectionId/selectionIdOrNullinweb/src/api/selectionId.ts) and apply it on every path — including the ones that don't look like pickers, such as aplaylistGroupIdseeded from the wire into a create dialog. Treat an unbindable id as ABSENT, never coerce it — rounding1.5to1would submit a different record — and clear its label with it: a row still reading "Blade Runner" over a null id makes two contradictory statements about the same item. Drop rather than render an option that cannot be selected safely. "Surfaces as no selection" is only true if that screen's Save gate actually checks for one — onPlaylistsScreenit did not, so this claim was false there for a full round after being written here. Verify an invariant on every screen it names before writing it down. Prove it per ingress by asserting zero writes are reachable after attempting the write: a write-count assertion on a path that never attempts one is trivially true. And unit-test the predicate's INCLUSIVE endpoints directly — once it is the single point of failure for every ingress, a>for>=slip passes an entire screen suite. - A caller-supplied promise needs a deadline, and a 2xx body is not a contract.
client.tsturns malformed JSON intoundefinedrather than rejecting, sosetResults(undefined)throws on the next render. Validate the elements, not just the container:Array.isArrayaccepts[null], which then throws onoption.idduring render, and an element with a wrong-typedidcommits an invalid value throughonSelect. Treat any malformed payload as a failed attempt (so it stays retryable), not as an empty answer. And asearchprop carries no abort signal, so race it against a timeout — otherwise a never-settling request leaves the picker spinning with no way back. - A stale result set must not be committable — by ANY modality. Between a keystroke and its
response,
resultsstill describe the previous query, so highlighting an option, retyping, and pressing Enter commits the old option while the box reads the new text. Drop the highlight on input change (not when the next response arrives) and put the guard in the singlechoose()sink rather than on each call site — gating Enter and leavingonClickopen is the same defect in another modality, and the next path added would be ungated too. Keep the stale list visible (hiding it flickers on every keystroke) but genuinely inert:aria-disabledplus a dimmed style, not merely a handler that silently no-ops on a normal-looking button. - Escape must not strand the user. Closing the popup while focus stays in the input means
onFocuscan never re-arm it, so typing does nothing and the user has to blur and refocus to recover. Typing and ArrowDown must both reopen it — and reopening onto results that are already current must not re-query: the duplicate response lands later and resets the cursor the user has since moved, so Enter silently does nothing. Reopening also places the cursor (ARIA APG) rather than swallowing the keypress.
The web typecheck gate is npm run typecheck, never npx tsc --noEmit. web/tsconfig.json is
solution-style ("files": [] + references), so a bare tsc --noEmit resolves to zero input files
and exits 0 without checking anything — a green that means "I looked at nothing". CI runs
npm run typecheck (tsc -b --pretty false), which builds the referenced projects and includes the
test files. Verified by planting a deliberate type error: --noEmit stayed green, -b caught it.
Testing an is-mounted guard: React 19 does not warn on a setState-after-unmount, and an unmounted
tree renders nothing either way — so no DOM assertion can distinguish "the guard stopped it" from
"React discarded it". Prove the mechanism (a useIsMountedRef unit test, with a StrictMode
double-invoke for the re-arm) and the integration (mock the hook module and assert the
component actually read current — and saw false — when the late response landed). Verify each by
removing the mechanism and confirming the test fails.
4. API client modules
One file per domain in web/src/api/, e.g. logs.ts, blocks.ts, playouts.ts. Pattern (see
web/src/api/logs.ts):
- Re-export the generated response/DTO types from
./generated/v1:export type LogEntry = components['schemas']['LogEntryResponseModel']; - A typed params interface for the endpoint's query string (e.g.
GetLogsParams). - The fetch function builds a
URLSearchParamsfrom only the params that are set, then calls the sharedrequest<T>(url)helper from./client. - An error-message helper (e.g.
messageFromLogsError) that narrowsunknown→ApiError(from./client) → a human string, with a fallback message — screens use this instead of stringifying errors themselves. web/src/api/index.tsre-exports everything so screens import from'../api', not from the individual domain file directly.
Trust the generated key casing — it mirrors the runtime. Since #198 the OpenAPI spec is generated
to match the runtime Newtonsoft serializer exactly (a schema transformer runs the same contract
resolver; see api-conventions.md §5a), so the generated types carry the real wire keys — including
Newtonsoft's acronym quirks like ffmpegProfileId (channel FFmpeg-profile id) and ffmpegProfile
(channel FFmpeg-profile display name). Do not hand-cast responses to "fix" a key or dual-read a
spec-cased vs runtime-cased key (the old PlaybackTroubleshootingScreen #198 escape hatch that read
data.channel.fFmpegProfileId has been removed — read ffmpegProfileId straight off the typed
response). When you mock an API response in a test, use the generated (runtime) casing.
4a. Optimistic-concurrency editors (ETag / If-Match / 412)
Replace-all editors (Blocks is the reference; see api-conventions.md §7a for the server contract,
issue #253) must round-trip the aggregate's concurrency ETag so a stale tab can't silently overwrite a
fresher edit:
- Transport seam (
web/src/api/client.ts):requestWithMeta<T>(path, options)returns{ data, etag }(reads theETagresponse header).request<T>delegates to it and drops the meta — keep usingrequestfor endpoints without a concurrency token. - Domain module (
web/src/api/blocks.ts): expose a…WithMetaload (getBlockItemsWithMeta→{ data, etag }) and make the replace accept the last-seen ETag and return the new one:replaceBlock(id, body, ifMatch?)→requestWithMeta(..., { headers: ifMatch ? {'If-Match': ifMatch} : undefined }). - Editor: hold the ETag in a
useRef; set it from the load GET, and replace it from the PUT response's ETag on every successful save (a same-tab second save otherwise 412s against its own write). Keyed reload: areloadKeystate in the loaduseEffectdep array lets the conflict flow re-fetch. - 412 UX: catch
error instanceof ApiError && error.status === 412on save and open a blocking "changed elsewhere — reload (unsaved changes discarded)"ConfirmDialog(Reload bumpsreloadKey), distinct from a 409 ("build in progress — retry shortly"). All other errors stay the generic save-error path. Reference:web/src/screens/BlocksScreen.tsxBlockEditor. - List-derived editors (Multi/Rerun collections open the editor from a paged-list row, not a per-record
GET): fetch the single record via the
…WithMetaby-id helper (getMultiCollectionWithMeta/getRerunCollectionWithMeta) when the editor mounts, and build the draft from that response — so the ETag and the draft data come from one read and stay consistent (the fail-safe ordering; don't pair a list-row draft with a separately-fetched ETag). Editors that navigate back to the list on save (Playout, Multi/Rerun, Collection reorder) need only the 412 branch — no post-save ETag rotation. - Replace-all PUT → round-trip every persisted field, even ones the editor doesn't visibly edit. A
replace-all update (Multi/Rerun collection, schedule items, block items) submits the whole item list,
so any field the draft mapping drops is silently reset to its default server-side. The multi-collection
editor's per-source
weight(#404) is the canonical trap: it must be read initemsFromMultiCollectionand written intoItemRequest, or every source resets to weight 1 on the next save of an unrelated edit (rename, add a source). When you add a field to a replace-all DTO, add it to both the response→draft and draft→request mappings and cover the round-trip in a test. - Numeric inputs with API bounds: hold the value as a string in the draft so the field edits smoothly
(transient empty/partial), pass
type="number"+min/max/inputMode="numeric"toInputfor the native stepper, clamp to the API's validator range ononBlur(immediate feedback) and again at save (authoritative) — never let an out-of-range value reach the server and surface a raw 400. Mirror the server's bound as a shared const (e.g. multi-collection weightWEIGHT_MIN/WEIGHT_MAX= 1..1000, #404).
4b. Full-replace request bodies are built as Complete<T> (#807)
A full-replace endpoint writes the WHOLE entity, so a field the builder never sets is not left
alone — it is written as its default. Build every full-replace body against Complete<T>
(web/src/api/completeRequest.ts), which maps a generated request type so every member is
required:
function toReplaceRequest(draft: Draft): Complete<ReplaceBlockRequest> { … }
items.map((item): Complete<DecoTemplateItemRequest> => ({ … }))
Two rules, and the second is the one that gets skipped:
- Annotate the wrapper parameter (
body: Complete<ReplaceBlockRequest>) so every caller — including ones written later by someone who never read this — inherits the check. - Annotate each construction site too, including every
.mapcallback's return type. The wrapper annotation catches a missing field anywhere. The phantom direction (a field the schema does not accept) relies on TypeScript's excess-property check, which fires only on a fresh object literal in a contextually typed position — and a literal returned from a generic.mapcallback is not one, becausemap<U>infersUfrom the callback's return rather than from the target element type. A spread or an inferred local loses it too, but the.mapcallback is the common case and the easy one to misread as safe: several construction sites accepted a phantom field before #807, and three of them contained neither a spread nor an inferred local.
An optional member may be written field: undefined. The point is not to forbid omitting a value,
it is to forbid omitting the decision: an unmentioned field is an oversight, an explicit
undefined is a choice a reviewer can see.
Do NOT apply Complete<T> to a schema whose optional members are computed server-side.
ArtworkContentTypeModel (reachable from PUT /channels/{id} via UpdateChannelRequest.logo) has
isExternalUrl / hasContentType / urlWithContentType as get-only properties derived from
path. Nothing deserializes them, so omitting them drops nothing — and annotating the site would
force you to invent server-computed values in an outbound request. Check the schema's disposition
in scripts/tests/test_optional_request_members.py before annotating a new site.
Why this is needed even though most builders already typecheck: a member is omittable exactly when
the generated type marks it ?:, which comes from the required array of the schema in
ErsatzTV/wwwroot/openapi/v1.json — the generator script only passes it through. Most nullable
properties emit as required-and-nullable, so the gap looks closed on inspection while a minority
sits unchecked inside it. Do not trust a list of which schemas those are: two hand-written ones
were wrong (#807). scripts/tests/test_optional_request_members.py derives the set on every run
and fails until each has a stated disposition. See
docs/decisions/records/testing/full-replace-asserts-field-list.md, and
web/src/api/completeRequest.guard.test.ts for the executed proof.
5. Artwork rendering
Render item.artwork / item.poster (or whatever the DTO field is named) directly as an <img src> — since PR #181, API responses already return rooted, directly-usable URLs (see
api-conventions.md §4). Do not client-side-prefix artwork paths (no /artwork/posters/ string
building in SPA code) — if you see that pattern, it's stale/wrong.
5b. HLS video preview
Screens that preview an ErsatzTV HLS stream use the reusable HlsPlayer component
(web/src/media/HlsPlayer.tsx, introduced with #145). Pass it a src (the .m3u8 URL, or null
for idle) and — when the manifest GET itself starts a server-side session (e.g. troubleshooting
playback.m3u8) — a playToken you increment per play, so a repeat play with an identical URL
still tears down and re-attaches (an unchanged src alone is a state no-op that never issues a new
request); it attaches hls.js when Media Source Extensions are available and falls back to native
HLS (video.canPlayType('application/vnd.apple.mpegurl'), i.e. Safari) otherwise, and tears down the
hls.js instance on src change and unmount. Its config mirrors the legacy _Host.cshtml
previewChannel (liveDurationInfinity: true + an unbounded manifest maxTimeToFirstByteMs) because
the troubleshooting playback.m3u8 endpoint blocks until segments exist before it 302s to the live
manifest. In tests, mock hls.js wholesale (vi.mock('hls.js', …) with a class exposing
static isSupported(), static Events, and loadSource/attachMedia/on/destroy) so jsdom never
touches a real MediaSource; assert the manifest URL via the mocked loadSource spy (see
PlaybackTroubleshootingScreen.test.tsx).
It also takes an optional onError(message) wired to Hls.Events.ERROR (fatal errors only) and,
on the Safari native path, the <video> element's own error event. Fatal errors are reported,
never auto-recovered — a diagnostic surface must show the fault rather than retry past it. Pass a
stable onError (useCallback); it is in the attach effect's dependency array, so an unstable
identity restarts the stream every render. When mocking hls.js in a test that exercises errors,
include ERROR in the mock's static Events.
It also takes an optional onPlaying(), called once playback has actually started — wired to the
<video> element's own playing event on both the hls.js/MSE path and the Safari native-HLS
path. It is deliberately not wired to Hls.Events.MANIFEST_PARSED: that fires before any media
has decoded (the manifest for an HttpLiveStreamingDirect channel always parses, even over a black
video), so it only issues the video.play() kick. onPlaying is purely additive (omitting it is
safe) and mirrors onError's requirements exactly: pass a stable onPlaying (useCallback),
since it too sits in the attach effect's dependency array and an unstable identity would restart the
stream every render.
Resolving a /iptv/* src under JWT auth (#552). Before feeding an /iptv/* manifest URL to
HlsPlayer, pass it through withIptvToken(url) (web/src/media/iptvToken.ts): under a JWT-enabled
deployment it appends the short-lived ?access_token= the /iptv/* scheme requires (the ctv-session
cookie does not satisfy it), and it is a no-op when JWT is off (the endpoint answers 204, cached).
It is async, so resolve it into state and gate the player on the resolved src — the channel-preview
panel does this in an effect keyed on playToken; the troubleshooting screen awaits it inline in its
onPlay handler. Only the top-level manifest needs it (segments and the multi-variant→variant hop
carry or bypass the token server-side). In tests, mock ../media/iptvToken to identity
(withIptvToken: (url) => Promise.resolve(url)) so opening a preview fires no real iptv-token fetch;
the append logic itself is unit-tested in media/iptvToken.test.ts.
5c. Media "Add to…" affordances
Screens that let the user add media items to a collection/playlist/schedule use the shared layer in
web/src/media/addTo/ — AddToMenu (popover for a MediaPosterCard actions slot or a detail
page's action row) and the AddToCollectionDialog / AddToPlaylistDialog / AddToScheduleDialog /
SaveAsSmartCollectionDialog it drives. Do not build screen-local target pickers. Multi-select
on grid screens is an explicit "Select" toggle (see docs/decisions.md 2026-07-10 for the rationale
and the accepted deviations from Blazor).
5d. Client-local preferences: localStorage, namespaced ctv-* keys
Per-browser UI preferences (theme, an auth token, a screen's remembered page size) live in
window.localStorage under a namespaced ctv- key, not a round-trip through the API — the
established pattern is designSystem.ts's getStoredDesignSystemTheme/applyDesignSystemTheme
(ctv-theme): a small getStorage() helper that returns window.localStorage wrapped in a
try/catch (so a disabled/unavailable storage API degrades to the default instead of throwing), a
getter that validates the stored value against the known option set before trusting it, and a
setter that writes straight through. LogsScreen.tsx's page-size persistence (ctv-logs-page-size,
#213) follows the same shape. Reserve this for state that's genuinely local to the browser/user
session — if a preference needs to be shared across devices or is really server/business state
(e.g. Blazor's ConfigElement-backed settings), it belongs behind an API endpoint instead; see
docs/decisions.md 2026-07-11 for the specific reasoning on logs page-size.
5e. Session auth: the boot gate, the machine-key screen + the global 401 signal (#295)
Since #295 the browser authenticates with a session cookie, not an API key. The whole /api
surface still answers a missing/expired session with 401, and mutating verbs additionally require a
CSRF header. The former keyless-bootstrap ApiKeyScreen (the X-Api-Key/localStorage model of #197)
is gone. The SPA seams now are:
- The request client sends no API key; it relies on the cookie and adds CSRF automatically
(
web/src/api/client.ts): the session cookie rides along with every same-origin request, and every mutating verb (POST/PUT/PATCH/DELETE) gets anX-Csrfheader set automatically — screens making a POST get CSRF for free. There is nogetStoredApiKey/X-Api-Keypath anymore; the only residuallocalStoragetouch isclearLegacyStoredApiKey()(web/src/api/auth.ts), a one-shot cleanup the boot gate runs once to purge any stalectv-api-key. - The boot gate owns login/setup (
web/src/AuthGate.tsx), wrapping<App/>outside the shell/router. It asks the PUBLIC/api/v1/auth/config, then/api/v1/auth/session, and renders Setup / Login / the app. It also publishesAuthContext({ username, method, signOut, requireLogin }) — readmethodviauseContext(AuthContext)to branch on the auth kind (e.g.'local'vs OIDC). - The machine-key screen is a normal authenticated screen (
web/src/screens/ApiKeyScreen.tsx, route/app/api-key, System nav group). It loads the server machine key from/api/v1/auth/machine-key(viagetMachineKey()) and displays it masked (a masked<code>that never puts the key in the DOM until Reveal) with Reveal + Copy affordances — the key is for MCP / external REST clients; the browser no longer uses it. For local accounts only (AuthContext.method === 'local') it also renders a "Local admin password" card that callschangePassword(current, new), mapping a wrong- current-password 401 to the server ProblemDetails detail inline. - File-download endpoints go through a fetch-blob helper, never a browser tab. Endpoints that
return a binary body (the troubleshooting archive / media-sample POSTs) must not go through
request()(it JSON-parses) and must not be opened withwindow.open(a session-cookie GET in a new tab can't carry the CSRF header and bypasses error handling). Insteadfetch(path, { method: 'POST', headers: { 'X-Csrf': '1' } }), thenresponse.blob()→ object URL → anchor-click → revoke, reading the filename fromContent-Disposition; a non-ok response throws anApiError. SeedownloadTroubleshootingArchive/downloadTroubleshootingMediaSampleinweb/src/api/troubleshoot.ts. - 401 UX is one shell-level banner, not per-screen handling. Rather than special-casing 401 in
every screen's error path, the request client emits a single app-wide signal on 401
(
auth.tsnotifyUnauthorized/subscribeUnauthorized), andweb/src/UnauthorizedBanner.tsx(rendered once in the App shell) subscribes and prompts re-login. This is the DRY seam for "the server rejected us" — reuse it instead of adding bespoke 401 branches. Auth flows that expect a 401 as an inline answer (login, change-password) passsuppressUnauthorizedSignalso they don't trip it.
6. Tests
- vitest, colocated
*.test.ts/*.test.tsxnext to the source file. - Every screen with meaningful logic gets a screen test; every API client module gets a
param-mapping / URL-building test (e.g.
logs.test.tsnext tologs.ts). web/src/App.test.tsxcovers composition, navigation, shell behavior, and integration regressions such as sub-path-to-sub-path rendering and guarded popstate restoration. Detailed screen behavior belongs in the screen's colocated test.web/src/app/routes.test.tsxindependently pins stable route-object identity and query/path matching.- Nav-label test-selector care:
getByRole('link'/'button', { name: /Regex/ })matches by substring by default — a loose regex can match more than one nav item. Verified example: the System nav button is matched with an anchored regex (name: /^System/) rather than a bare/System/, specifically to avoid ambiguous matches against other labels that start with or contain "System". Anchor (^/$) or use exact strings ingetByRolename matchers whenever a new label could be a substring of (or share a substring with) an existing one — checkapp/routes.tsx's navlabel:list for collisions before picking a new label. - Extracted-screen tests own their own fetch mock (Schedules #207, Channels #244, Playouts
#245): when a
screen is pulled out of
App.tsxintoweb/src/screens/<Name>Screen.tsx, its colocated<Name>Screen.test.tsxbuilds a self-containedvi.spyOn(window, 'fetch')mock scoped to that screen's own endpoints (plus localjsonResponse/fixture-factory helpers) and renders the screen component directly — it must not import fromApp.test.tsxor reuse the monolithicmockDashboardApi().App.test.tsxkeeps only a thin nav-smoke test for the extracted screen (route to it, assert it mounted and hit its own endpoint) plus genuinely cross-cutting shell concerns (route table, sub-path ownership §2, the unsaved-changes/popstate guard §8, the TopBar action wiring). Detailed screen behavior lives in the screen's own test file. SeeChannelsScreen.test.tsx/SchedulesScreen.test.tsx/PlayoutsScreen.test.tsxfor the shape.
7. Verification gate — run before every commit touching web/
From web/:
npm test # vitest
npm run lint # eslint .
npm run build # tsc -b && vite build
Also run npm run check:api if you touched anything OpenAPI-relevant (see api-conventions.md §5)
— it regenerates src/api/generated/v1.d.ts and fails the build if it's out of sync with what's
committed.
8. Unsaved-changes navigation guard
Screens with a draft / explicit-Save model (edits accumulate locally, one Save flushes them —
e.g. the schedules editor, web/src/screens/SchedulesScreen.tsx) must guard against losing the
draft to navigation. The shared module web/src/navigationGuard.ts is the seam:
-
The screen registers a guard on mount:
registerNavigationGuard(() => !dirtyRef.current || window.confirm(...)), returning the unregister fn from the mountuseEffectfor cleanup. Only one guard is active at a time (the mounted screen); a stale unregister only clears its own guard. -
App.tsx'snavigatehandler callscanLeaveCurrentScreen()beforepushState— afalsereturn aborts the in-app sidebar/nav click. -
Browser Back/Forward (
popstate) is also covered.App.tsx'spopstatehandler consultscanLeaveCurrentScreen()too. Apopstatecannot be cancelled — by the time it fires the URL has already changed — so on a veto the handler re-pushes the pre-pop path (tracked in acurrentPathRefupdated on every approved navigation) viahistory.pushStateand leavesactiveRouteuntouched, effectively undoing the browser's URL change. This same handler also runs for the synthetic popnavigateToPath()dispatches, so programmatic in-app navigation is guarded as well. The re-push does not re-firepopstate; that's safe because only one screen is mounted at a time. -
Guarded sub-path routes: App owns pathname/popstate (resolves the old §2 caveat). A sub-path wrapper that BOTH tracks its own pathname (§2) AND whose sub-screens register a dirty guard cannot self-listen for
popstate: React commits child passive effects before the parent, so a wrapper-ownedpopstatelistener fires before App's guard-restore handler and would switch sub-screen before App could veto and re-push — desyncing the two. Resolution (design #202 §D.2, finding 4): App is the singlepopstateowner. On a pop it consultscanLeaveCurrentScreen(), and only on approval does it update acurrentSubPathstate value (librariesSubPath) that it passes down into the wrapper; on a veto it re-pushes the pre-pop path and updates nothing, so the wrapper never sees a vetoed path. The wrapper (LibrariesRouteScreen) derives its sub-path purely from that prop — never from the raw event — so effect-commit order is irrelevant. Scope the state write to the guarded route only: thesetLibrariesSubPathcall is gated on the arrived route beinglibraries, so unguarded sub-path routes (Playouts/Media) skip it entirely and stay byte-identical (they still bail App'ssetActiveRouteviaObject.isand self-own their pathname per §2). The regression test is the App-owned-popstate case inApp.test.tsx(dirty editor at/app/libraries/local/3:confirm→falsekeeps URL + mounted sub-screen;confirm→truenavigates). Unguarded sub-path screens (PlayoutsRouteScreen,MediaRouteScreen) keep self-owning their pathname — only screens that register a dirty guard defer to App. -
The screen also guards the paths the module still can't see — an in-screen action that would replace the draft (schedule switch, opening the properties editor) uses its own
window.confirm(...), and abeforeunloadlistener (installed whiledirty) covers full-page unloads (reload / close tab / external link).
Keep the guard predicate reading a ref (dirtyRef), not the dirty state value, so
canLeaveCurrentScreen() sees the current dirtiness synchronously at click time.
- Successful save + same-callback navigation:
useDirtyGuardreturns amarkCleancallback for the narrow case where a successful save queues the new draft/baseline and immediately callsnavigateToPath(...)in that same promise callback. React has not committed the clean render before the syntheticpopstate, so callmarkClean()after the durable save succeeds and immediately before navigating. Still queue the real clean draft/baseline state; the callback only transfers the synchronous guard truth across that one event boundary. Only do this while the successful request still owns the current draft: disable or otherwise gate draft mutations for the entire in-flight save, or revision-check the completion before marking clean. Otherwise the completion can discard edits made after the request started. Do not replace the callback with a timer or a forced render. Screens that remain mounted after saving do not need the callback.
9. Review checklist — temporal semantics
- For every effect / timer / async completion, ask: when does it fire (mount, dependency change,
unmount, StrictMode double-invoke) and which render/request does it still own? A debounce timer
fires on mount too (§3, the
lastQueryRefno-change guard exists precisely for that); a.thencan resolve after the params it was launched for have moved on (§3a, therefreshinggate and the Add-all query binding exist for that). A guard that only checks "still mounted" (activeRef) does not answer "still current". - For any "make X consistent with Y" change, re-validate the exemplar Y's temporal behavior before copying it. #221 came from copying a fetch model that keeps stale results visible onto screens that had gained mutation surfaces — the exemplar was safe read-only, the copy was not. Copying a pattern copies its assumptions; confirm they still hold in the new context.
10. TopBar primary-action button (usePrimaryAction)
The shell TopBar (app/AppShell.tsx) renders at most one primary-action button (top-right, Plus
icon) for the active screen. App.tsx wraps the shell and matched screen in the minimal
PrimaryActionProvider from web/src/primaryAction.ts: the active screen explicitly registers one
handler, and the TopBar reads that matching registration directly from React context. No window event,
string-keyed dispatcher, or generic screen-action framework participates in normal screen actions.
- A screen opts in by calling
usePrimaryAction(routeId, handler)at the top of the component, before any early return (it's a hook). The handler must be reachable there — a create/navigate handler or auseStatesetter, not something defined below a loading/errorreturn. Reference:SchedulesScreen(usePrimaryAction('schedules', () => setForm('create'))). - The provider owns exactly one registration. Each mounted hook gets an opaque owner token; registering a newer screen replaces the previous registration, and cleanup clears only the registration owned by that hook. A stale unmount therefore cannot erase the newer screen's action. The hook keeps the latest handler in a ref, so ordinary screen renders update behavior without reclaiming/churning ownership. Directly-rendered screen tests outside the provider remain harmless; there is simply no shell action consumer.
- The route must ALSO declare a matching non-empty
primaryActionlabel in the stableapp/routes.tsxtable. The TopBar renders the button only when that label is non-empty and the active route owns a matching registration — metadata alone can no longer produce a dead button, while a registration without a label remains intentionally invisible. Keep the two in lockstep. The#238tests inApp.test.tsxguard this: a data-drivenit.eachasserts each URL-navigating create screen's banner actually navigates (so a typo'd route id → a button that navigates nowhere → red), plus a drop test that an action screen shows no banner button.primaryAction.test.tsxseparately pins matching, latest-handler, route-change, cleanup, and stale-owner semantics. The dialog/editor create screens (schedules, multi/rerun collections, trakt) are exercised by their own create-flow tests. - When to WIRE vs DROP (issue #238). The Plus icon makes the button semantically a "create new item"
affordance. Keep + wire it only on list screens with a single, unambiguous create flow ("Add Channel",
"Add Schedule", "Add Multi-Collection", "Add Rerun Collection", "Add Trakt List", "Add Filler Preset", "Add
Profile", "Add Watermark"). Drop it (
primaryAction: '') where the primary action is not a create (Save / Refresh / Play / Validate / Reset / Scan — the "+" is wrong, and those screens already carry the correct in-body control), or where a single banner action would be ambiguous (Collections has two create types behind tabs), a silent no-op (the Channel builder's Create is disabled until the form is valid), or semantically misplaced (Dashboard is a status page; Libraries "Scan" is per-row). Coexisting with an in-body create control is fine (Schedules has both) — the banner is a convenience, not the sole entry point. Seedocs/decisions.md2026-07-12 for the full rationale.
11. Slide-over panels + the shared advanced-options model
SlideOver(right-edge detail/edit panel) lives inweb/src/components/overlay.tsxalongsideDialog. Use it for a per-item edit/detail surface layered over a screen (the Auto-Tune "Configure" panel is the reference, #386); useDialogfor a centered confirm/short form. Both share one privateuseOverlayBehavior(open, onClose, panelRef)hook (focus + body-scroll lock on the closed→open transition, Escape-to-close via a latest-onCloseref, scrim-click dismiss). It portals todocument.bodyand takestitle/subtitle/children/footer/width. Don't build a bespoke edge panel — extendSlideOver.- Draft-until-commit slide-overs need the §8 guard at the SCREEN, not the panel. When per-item
edits accumulate in the parent screen's state and are flushed by one later action (Auto-Tune's
bulk-create), closing the panel does not lose data (it's still in screen state) — so the
window.confirmguard belongs on screen navigation / unload while any uncommitted edit exists, not on panel close. Show an "Edited" badge on customised rows so the pending edits are visible. - Per-source correction rows: weight stepper + include/exclude toggle, keyed by the server's source id
(#440 wiring #425's backend into the Auto-Tune DetailPanel's Content-sources pane). The row model is
reusable wherever an item list carries per-item rotation weights:
- The row's draft (
SourceDraftinAutoTuneScreen.tsx) holdsweightas a string per §4a, plus a booleanexcluded; the boundWEIGHT_MIN/WEIGHT_MAX= 1..1000 mirrors the server'sMultiCollectionItemWeightvalidator — the same 1..1000 the multi-collection editor enforces (#404), but via its own screen-local pair: same values, separate consts, not a shared module. It is clamped ononBlurand again in the draft→request mapping. - Only genuinely customised rows go on the wire. The draft→request mapper (
sourcesRequest) drops any row that is still fair-share (weight 1, not excluded, not added) and returnsundefinedwhen nothing is left, so the field is omitted rather than sent as an all-default array. That predicate mirrors the server's owncustomizedcheck exactly — an all-default array is a documented no-op there, and omitting it keeps the channel on the cheaper single-SmartCollection shape. The same predicate drives the row's "Edited" badge and the §8 dirty guard, so a value touched and set back to the default is correctly not an edit (same rule as a name cleared back to empty). - A library picker compiles typed text; it never forwards raw Lucene. The "add a source not in the
base set" typeahead sends
title:*<escaped>*(titleContainsQuery), not the user's literal text: the search index's default field does not match bare title words (Alphafinds nothing for "Show Alpha" — seee2e-local.md), so a raw forward likeSearchScreen's (where the box is explicitly a query box) would look broken in a name picker. Escape every Lucene special + whitespace so the boundary stars are the only live wildcards — the same shapebuilder/rules/compile.tsemits forcontains. - Rows are read-keyed by the server's id (
GET /api/v1/channels/auto-tune/members), and an added id that turns out to already be a base member patches the existing row instead of appending a duplicate.
- The row's draft (
- Advanced channel-options overrides are shared, not duplicated (
web/src/builder/advancedOptions.tsx). TheCreateChannelFromLineupAdvancedOptionsRequestoverride model — the enum catalogs,ADVANCED_KEYS,effectiveValue, and the field adapters (useAdvancedOverrides) — is one module consumed by both the Channel Builder and the Auto-Tune DetailPanel, and must not be re-implemented per screen. Each screen writes its own field JSX over the shared hook;playbackOrder/playoutModeare surfaced as dedicated controls (Builder state / Auto-Tune's Shuffle + Always-playing toggles) and merged intoadvancedat create, not carried in the override map. - Three per-field states: INHERIT / set / CLEAR (#135,
api.from-lineup-clear-to-none). A select onINHERIT(or a cleared text input) omits the field so the create handler coalesces it with the template value. A concrete value overrides the template. TheCLEARsentinel ("None") is stored in the overrides map like any override but is folded into the request'sadvanced.clearlist at build time byapplyOverridesToRequest/collectClears— so the sentinel never reaches the wire as a field value, andeffectiveValuereads aCLEARoverride as none. Build the request only throughapplyOverridesToRequest, never a raw spread of the overrides map (a spread leaks theCLEARsentinel). Today only the five id selects (watermark + fillers) expose a "None" option; the backendclearenum also covers the preferred audio/subtitle language strings for machine clients, but the SPA text inputs keep "empty = inherit" (a tri-state text control is deferred) — API-ahead-of-UI, matching the "REST API is a real audience" posture.
12. Reusable rule builder (web/src/builder/rules/)
A visual rule builder for Lucene-backed queries, introduced for the SmartCollection create/edit dialog (#176) and built as a standalone, controlled component module — not a SmartCollection screen concern — so it can be embedded by later screens without duplicating the rule model.
types.ts— the rule tree:Rule(field/operator/value),Group(match: all|anyoverRules and/or recursively nested sub-Groups),Operator,FieldType, anisGroupnarrowing helper, andMAX_GROUP_DEPTH— the authoring cap on nesting (root group = depth 0, so up toMAX_GROUP_DEPTHlevels of sub-groups below it; currently 5). #436 replaced #176's one-level "Kodi" model with bounded-arbitrary depth — see thespa.rulebuilder-nestingdecision.MAX_GROUP_DEPTHis the single source of truth: the UI's "Add group" gate (RuleBuilder.tsx),parse.ts(anything deeper is out of subset ⇒null⇒ raw-text fallback) androundtrip.test.ts's generator all read it — never re-hardcode a depth.compile.ts/parse.ts—compile(group)turns a rule tree into a query string covering a closed subset of the Lucene grammar;parse(input, fieldTypes)is its exact inverse, returningnull(never a lossy best-effort tree) for any query outside that subset. Escaping is total, so a compile→parse round-trip is lossless for any builder-authored value, including Lucene special characters — pinned by a 500-tree property test (roundtrip.test.ts, which generates trees to the fullMAX_GROUP_DEPTHand asserts the corpus actually reached that depth).fieldCatalog.ts—useSearchFields()wrapsGET /api/v1/search/fieldsand exposes a clean, non-nullRuleField[](plus afieldTypeslookup and abyGroupgrouping) for field pickers; this is the only place the raw API response shape is unwrapped.RuleBuilder.tsx— the controlled component itself:{ group, onChange }in, add/remove rule and sub-group UI out. It holds no query-string state — the embedding screen owns the compiledquerystring (compile(group)on change) and, symmetrically, seedsgroupviaparse(query, fieldTypes)on load (falling back to raw-text editing on anullparse).
Usage pattern: a screen owns two things — the raw query string (what actually gets saved) and
the parsed Group | null (what the builder edits). Toggling between "Builder" and raw-text modes is
just switching which of the two is the source of truth for that render, re-deriving the other via
compile/parse at the toggle boundary. See SmartCollectionDialog for the reference integration.
SmartCollectionDialog.tsx(web/src/builder/) — the name + query authoring dialog wrappingRuleBuilder(Builder/Advanced toggle + a live preview count), extracted from CollectionsScreen so it is a shared, self-contained editor (#437). Its only coupling to a host is the interface{ open, busy, error, initial, onCancel, onSubmit({name, query}) }— the parent owns theonSubmitside effect (persist, add to a lineup, …). Consumed by CollectionsScreen (create/edit) and the ChannelBuilder (/app/new-channel, "New smart query" in the Collections source): the Builder authors an ad-hoc query,createSmartCollectionpersists it as a real named SmartCollection, and the new collection is added to the lineup by itssmartCollectionId— no new REST/MCP surface (the MCP already exposesersatztv_create_smart_collection).
Generalizing Auto-Tune's fixed axis picker into an arbitrary-field builder is a different
primitive (group-by, not single-query filtering) and a backend epic — designed separately in
docs/superpowers/specs/2026-07-23-auto-tune-arbitrary-field-design.md, tracked as its own issue,
not wired here.
- Relative-date operators (#435,
rulebuilder.relative-date-macros) —release_date/added_dategaininLast/notInLastalongsidebefore/after/between, each with a numeric value plus aday|week|month|yearunitpicker (types.ts'sunit?: DateUnit).dateMacro.tsis the single seam: afield↔macro-prefixtable (release_date↔released,added_date↔added) plus theinLast/notInLast ↔ inthelast/notinthelastsuffix maps, delegated to fromcompile.ts/parse.tsfor these two fields — the compiled query is the existingreleased_inthelast:"7 day"-styleCustomMultiFieldQueryParsermacro, so nothing downstream changes.validation.ts'sruleErrorrequires the value to parse as a positive integer before it's compiled. - Facet-value typeahead (#434/#578,
api.search-field-values-sources) — the value input for atextfield (not enum) is a combobox backed bygetSearchFieldValues(web/src/api/search.ts→GET /api/v1/search/fields/{name}/values?q=&limit=), debounced on keystroke, prefix-matching the in-progress value against distinct terms already in the index. It always allows free-text entry as a fallback — a 404 (non-text field) or an empty result list (e.g. ElasticSearch backend) degrades to a plain text input rather than blocking the rule. Since #578 (api.search-field-values-sources)album_artistreturns values instead of 404ing, andartistcovers free-text music-video/song credits as well as entity artists; for those two the server's list is bounded best-effort on a very large library, so the free-text fallback stays load-bearing — never treat an absent suggestion as an invalid value. - Single-child-group normalization (#438) —
normalizeGroup(validation.ts) coerces a group'smatchtoallwhenever it has fewer than two children, recursively. A one-childanygroup is semantically identical toallbut doesn't round-trip throughcompile→parse(the compiled Lucene for a lone child carries noAND/OR), so the builder normalizes on every change rather than let the UI and the compiled query silently diverge;roundtrip.test.ts's property test asserts againstnormalizeGroup(tree), not the raw generated tree.
13. Collapsible sidebar + nav-group accordions (#396)
The shell sidebar (web/src/app/AppShell.tsx) supports two independent, persisted collapse states.
Both are shell chrome — no screen participates.
- State + persistence live in
web/src/app/sidebarState.ts(useSidebarState()), consumed byAppShellalone. It follows the §5d client-local-prefs pattern (a try/catchgetStorage(), a validating getter, a write-through setter) over two namespaced keys:ctv-sidebar-collapsed—"1"/"0"; is the sidebar collapsed to the 60px icon rail?ctv-sidebar-groups— JSON{groupKey: boolean}where the boolean is collapsed. A labeled group with no stored entry defaults to collapsed, so a fresh load shows only the always-open Primary group. (Keys are thectv-hyphen form, not the prototype'sctv.sidebar.*— seedecisions.md2026-07-18.)
AppShellstampsctv-app-shell-collapsedon the shell root when collapsed; the rail look is entirely CSS-driven from that one class (shell.cssnarrows the tracked--sidebar-wto 60px and transitionsgrid-template-columns;@media (prefers-reduced-motion: reduce)drops the transition).- Nav is inventory-driven from
sidebarNavGroups(app/routes.tsx). Only labeled groups are collapsible; each has an explicit stablekey('media','system') used for the persisted map — don't derive the key from the label (a rename would orphan persisted state). The unlabeled Primary group is always rendered. - Accordions apply only in the expanded sidebar. In the rail, group-collapse is ignored: every
item renders (icon-only), groups separated by a
.ctv-nav-divider. The nav item's label stays in the a11y tree (visually hidden via CSS, notdisplay:none) so the accessible name — and everygetByRole('link', { name })test — still resolves; the label is also passed as the nativetitletooltip (NavItemgained atitleprop). Numeric badges collapse to a corner dot. The active-route indicator works in both states. - Testing note:
App.test.tsx's shell/nav suite clicks Media/System nav links directly, so itsbeforeEachseeds both groups open (ctv-sidebar-groups); the default-collapsed / accordion / rail behavior is covered in its owndescribe('collapsible sidebar (#396)'), and the persistence helpers have a colocatedsidebarState.test.ts.
14. Server-derived rollup verdict: one filter, per-item fault badges (#415)
When the server folds several fault classes into one verdict object (see api-conventions.md →
"server-derived rollup verdict" and decisions.md → api.channel-health-object), the SPA gets one
list filter/count off the rollup status, never a filter per individual fault:
- One rollup filter, not N fault filters.
ChannelsScreen.tsx'sproblemsfilter/count reads onlyhealth.status === 'Problems'(hasProblems(channel)); it replaced the earlier single-faultwillNeverPlay/"No playout" filter now that the taxonomy is real (#415superseded the "don't broaden the label" comment that guarded against a false all-clear). Do not add a second filter per fault class — that re-fragments the one signal the rollup exists to unify. - Per-row badges still name the specific fault(s), spelling out
health.faults(No playout/Never built/Build failed/Empty/Broken source) rather than a generic "Problems" chip — the rollup unifies the filter, not the detail a user reads on a given row.Unknownrenders no badge (it is deliberately notHealthyand notProblems). - Defensive coercion at the read boundary, even though the generated type is non-null.
healthis in the OpenAPIrequiredlist, so the generatedChannelSummary['health']types as{status, faults, playoutCount, brokenSourceItemCount}— not nullable (contrastcore-dtos-generate-nullable-in-spa, which covers other Core DTO fields that do generate| null | undefined). Still always read it through optional chaining with an explicit fallback at the point of use (channel.health?.status,channel.health?.faults ?? []) rather than asserting it non-null — a genuinely absenthealth(e.g. an older cached response predating this field) must read as "no verdict", never crash or silently read asHealthy. - Status/fault string unions are hand-maintained, matching
ChannelPreviewAvailability: the server emits plain strings backed by aconst stringclass (ChannelHealthStatus,ChannelFault), not a generated enum, so the SPA's local TS union ('Healthy' | 'Problems' | 'Unknown', and the fault union) is kept in sync by hand when the server adds a value.
15. Field-level progressive disclosure (FieldHelp, #734)
Settings fields — FFmpeg profiles above all — carry consequences that are severe and non-obvious, while the UI gives a bare label. The information exists (decision records, source comments) but is invisible to the person changing the value. The pattern below puts it one interaction away without cluttering the form, and its value is in being identical everywhere: one icon, one gesture, one place users learn to look. Applied ad-hoc per screen it is just visual noise, so adopt this shape or none.
Decision record: docs/decisions/records/spa/field-progressive-disclosure.md
(spa.field-progressive-disclosure).
The three levels
| Level | What | Where it lives | Length limit |
|---|---|---|---|
| 1 | Field name, optionally a one-sentence summary | the screen's own row markup (.ctv-settings-row-label / .ctv-settings-row-help) |
one short sentence, hard limit |
| 2 | Explainer panel behind a trigger next to the name | FieldHelp (web/src/components/fieldHelp.tsx) |
one short paragraph, hard limit |
| 3 | Deep link to external docs | FieldHelp's docsHref prop |
n/a — external |
Level 3 has no live target yet; the affordance is built and typed so a screen can adopt it the
day the docs exist, and no screen passes docsHref today. Do not invent a URL to fill it.
The trigger is an icon, never the label
FieldHelp renders a lucide-react Info icon inside a real <button type="button">, placed
immediately after the field name. Do not make the field name itself the trigger — a settings
label is frequently a <label> bound to its control (components/forms.tsx wraps the input in
one), so a label-wide trigger competes with click-to-focus. The trigger's click handler calls
preventDefault() as belt-and-braces for that nesting: per the HTML spec a label's activation
behaviour is skipped for events targeted at interactive content, and a <button> is interactive
content, so this guards a case no engine is currently known to hit — Chromium was verified to
behave as specified. It costs nothing and does not depend on every engine agreeing.
fieldHelp.test.tsx asserts the mechanism (defaultPrevented) rather than "the input did not
focus", because jsdom does not implement label activation at all and the obvious assertion would
pass vacuously.
Copy lives with the field definition
Put the level-2 paragraphs in a module-level FIELD_HELP object (declared as const, not
Record<string, string> — under an index signature a mistyped key types as string and renders a
trigger with an empty panel and no build error) in the screen module, next to the field
definitions it describes (see FFmpegProfilesScreen.tsx), and pass
detail={FIELD_HELP.someField} at the call site. Not in the shared component (it would collect
copy for screens it knows nothing about) and not from the API — copy that ships separately from
the field it describes drifts, and drifted copy is worse than none. Where a bound comes from a
constant the form already uses, interpolate it (MINIMUM_QSV_EXTRA_HARDWARE_FRAMES) rather than
restating the number.
The panel is portalled — do not put it back in the wrapper
.ctv-card sets overflow: hidden, and an overflow: hidden ancestor clips a positioned
descendant whatever its z-index. An in-flow panel on a card's last row was measured showing
12px of a 92px paragraph — the Audio card's final row always carries a detail, so one field's
explainer was destroyed in every state of that card. FieldHelp therefore renders the panel through
createPortal into document.body with position: fixed, positioned from the trigger's viewport
rect in a layout effect that also flips it above when it would run off the bottom and clamps it
inside the horizontal edges. Readability then stops depending on which row of which card adopted it.
The portal has two consequences worth knowing before you adopt it on a new screen. The panel no
longer sits inside a dialog's or slide-over's stacking context, so it carries z-index: 110 to
out-rank .ctv-dialog-overlay / .ctv-slideover-scrim (both fixed at 100) — at the original 60 it
painted behind the very surface whose field it was explaining. And a focusable element inside the
panel lands at the end of the document's tab order rather than next to its trigger.
Accessibility contract (not optional)
Hover-only content is invisible to keyboard and touch users, so FieldHelp opens on three
independent signals, none derived from another:
- click/tap — pins the panel open; the only gesture a touch user has. It survives the pointer leaving, and toggles closed on a second press.
- hover — for reading the paragraph in place. Unlike
.ctv-tooltipthe panel is notpointer-events: none, so a pointer that reaches it keeps it open; but hover does not reliably get you there. Across the 7px offset the pointer is over neither element and the panel closes under the cursor. A transparent::beforebridge was tried and withdrawn: it held for a strictly vertical descent and failed for a diagonal one, the natural reach toward a panel sitting below and to the right — measured, the safe sideways exit was the bottom 1.25px of an 18px icon. Do not re-add it; a mechanism that needs a 1.25px caveat is not a mechanism. Interacting with the panel — which today means adocsHreflink — is done by pinning it first (click/tap). That is the documented route, it survives any pointer motion, and it is the only one a touch user has anyway. - keyboard focus — opens on focus, closes on blur.
Plus: aria-expanded on the trigger, an accessible name of More about <field>, Escape closes
and returns focus to the trigger, and an outside pointer press dismisses. Two details are easy to
get wrong and are pinned by tests:
role="note"is not a live region and nothing announces the panel when it appears. What actually reads the paragraph out isaria-describedbyon the trigger, set only while the panel is open. Do not treat the role as satisfying the announcement requirement — it does not.aria-controls/aria-describedbyare set only while the panel exists. A reference to an id that is not in the document is worse than no reference at all.
Escape and outside-press are armed only when the panel is pinned or focused, never when it is merely hovered: a document-level Escape handler that fires whenever a pointer happens to rest on an info icon would steal focus from whatever the user was actually dismissing. Escape's re-focus deliberately does not re-open the panel — that regression is pinned by a test.
FieldHelp vs Tooltip
They coexist on purpose. Tooltip (components/feedback.tsx) is a single-line, nowrap,
pointer-events: none, hover/focus-only label for a control whose purpose is not otherwise stated —
an icon button, say. It cannot hold a paragraph, cannot hold a link, and touch users never see it.
Do not widen Tooltip into this role, and do not use FieldHelp where a two-word label would
do.
Adopting it on a new screen
- Give the row a level-1 summary if one short sentence genuinely helps; skip it otherwise.
- Add the paragraph to that screen's
FIELD_HELPrecord. Check it against the option list and the server-side rule, not against your recollection: the first cut of this screen's copy compared loudnorm to a dynamic-normalization mode this fork does not have, described scaling as a binary choice while omittingStretch, and asserted a range as known-bad that its own decision record calls untested. Copy that describes options the user cannot choose is exactly the drift this convention exists to prevent. - Render
<FieldHelp detail={…} label={fieldName} />inside the row's label element. - Reuse the
ctv-field-help*classes incomponents.css; do not restyle per screen. - If the field needs a
docsHref, remember the link is reached by pinning, and that the portal puts it at the END of the document's tab order rather than immediately after the trigger — a known limitation tracked in #840, whose trigger is exactly this: the first screen to passdocsHref. Pick it up rather than working around it locally.
Panel placement (not clipped, flips when it would run off the bottom) is currently verified by live
measurement, not by a committed test — #839 tracks pinning it in the browser harness. Re-measure
if you change the offset, the placement logic or .ctv-card's overflow.
Backfilling every screen is deliberately not this convention's job — adopt it where a field's
consequences are severe and non-obvious, which is what makes the icon meaningful rather than
decorative. FFmpegProfilesScreen.tsx is the reference implementation (eleven fields).