Build ErsatzTV Image / Build & test (.NET) (pull_request) Failing after 3m9s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 11s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m47s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
228 lines
16 KiB
Markdown
228 lines
16 KiB
Markdown
# Decisions — append-only log
|
||
|
||
Purpose: why the codebase does what it does, so agents don't "fix" an established convention or
|
||
relitigate a settled call. Append new entries at the bottom in date order; never edit or delete
|
||
past entries except to fix a factual error. **Update this doc in the same PR that changes any fact
|
||
below (or that establishes a new convention worth recording).**
|
||
|
||
## 2026-06 — REST API wraps existing MediatR handlers 1:1, no service layer
|
||
|
||
The REST API (#2, `docs/rest-api.md`) is thin controllers over the existing MediatR
|
||
Create/Update/Delete handlers — no new service/business-logic layer was introduced, since nearly
|
||
every handler already returns `Either<BaseError, T>`, which maps cleanly to HTTP status codes.
|
||
Latent handler bugs (missing existence checks, `KeyNotFoundException` risk, etc.) are fixed **at
|
||
the handler**, converting what would have 500'd into a proper 404/422 — not papered over in the
|
||
controller. Established across the #2a–#2e gap-issue PRs. Deep FK ids nested inside item-list
|
||
request bodies (e.g. a schedule item's `CollectionId`) are deliberately **not** existence-checked at
|
||
that depth, to avoid N+1 validation queries — precedent set by the schedules endpoints (#172); see
|
||
`docs/api-conventions.md` §3 for the up-to-date statement of this rule.
|
||
|
||
## 2026-06 — UI rebuild is a React SPA (ChicoryTV) on the REST API, not a Blazor reskin
|
||
|
||
#59 committed to a full SPA rebuild rather than reskinning Blazor Server pages. Blazor removal is
|
||
split into two phases under #91: **(a)** root-flip (SPA becomes `/`) + legacy-route redirects —
|
||
DONE, merged via PR #148 (`ErsatzTV/LegacyUiRedirects.cs`, `feat/91-cutover` → main). **(b)** full
|
||
Blazor removal — gated on every route having an SPA equivalent; tracked route-by-route in
|
||
`docs/blazor-route-parity.md`.
|
||
|
||
## 2026-07 — Response DTOs live in `ErsatzTV.Core/Api`, file-scoped `#nullable enable`
|
||
|
||
New REST response DTOs go in `ErsatzTV.Core/Api/<Domain>/*ResponseModel.cs` and mirror the shape of
|
||
the corresponding Application-layer ViewModel — controllers never expose VM types directly. Because
|
||
`ErsatzTV.Core.csproj` sets `<Nullable>disable</Nullable>` project-wide, any response-model file
|
||
with an optional member needs its own `#nullable enable` pragma at the top (most already have one).
|
||
`ErsatzTV.Application` has no nullable context at all — do not add `?` annotations to types living
|
||
there; that's a Core/Api-layer-only convention. Full detail: `docs/api-conventions.md` §2.
|
||
|
||
## 2026-07 — PUT-replace list endpoints derive `Index` from array order; alternate-schedules last row = catch-all default
|
||
|
||
For "replace the whole list" endpoints (PUT over a collection — schedule items, template items,
|
||
etc.), the item's `Index` is derived from its position in the request array, not from a
|
||
client-supplied index/order field — established by `ReplaceScheduleItemsRequest.ToCommand`
|
||
(`Items.Select((item, index) => item.ToReplaceCommand(index))`). Separately, `ProgramScheduleAlternate`
|
||
and `PlayoutTemplate` rows (both `IAlternateScheduleItem`) are evaluated in `Index` order,
|
||
first-match-wins; the convention is to place the least-conditional (or unconditional) row **last**
|
||
so it acts as the catch-all default. Established by the alternate-schedules work (PR #179,
|
||
`AlternateScheduleSelector.cs`).
|
||
|
||
## 2026-07 — Templates editor in the SPA is a table, not Blazor's drag-calendar
|
||
|
||
The legacy Blazor `TemplateEditor.razor` used a drag-and-drop day-grid calendar UI. The SPA
|
||
equivalent (`/app/templates/{id}`, PR #173) renders the same day/block assignment as a table
|
||
instead. This is an accepted, deliberate parity deviation — don't "fix" it to match Blazor's
|
||
interaction model without discussing it first.
|
||
|
||
## 2026-07-07 — API artwork contract: rooted URLs produced server-side
|
||
|
||
API response DTOs return artwork as rooted, directly-usable URLs (`/artwork/posters/...`,
|
||
`/artwork/thumbnails/...`, `/artwork/fanart/...`), plus passthrough for absolute `http(s)://` URLs
|
||
and Jellyfin/Emby proxy variants. Established by PR #181
|
||
(`ErsatzTV.Application/LibraryBrowse/Queries/GetLibraryBrowseItemsHandler.cs`, private `Artwork(...)`
|
||
helper — comment: *"Returns a rooted, directly-usable artwork URL for the SPA's `<img src>`... the
|
||
SPA [needs it pre-rooted]"*), then generalized into the reusable `ApiArtwork` helper
|
||
(`ErsatzTV.Core/Api/ApiArtwork.cs`, PR #183). Root cause: the SPA has no `<base href>`, unlike
|
||
Blazor, so relative artwork paths that worked for Blazor pages 404 in the SPA. Do **not** reuse the
|
||
Application-layer Mappers used by Blazor (e.g. `MediaCards`/`Television` mappers) for new API
|
||
DTOs — those still return old Blazor-convention relative paths; map from the domain/VM directly and
|
||
root the path via `ApiArtwork`.
|
||
|
||
## 2026-07-07 — Decode-style endpoints take a row id and look up server-side
|
||
|
||
Endpoints that decode/expand opaque stored state accept a database row id and resolve server-side,
|
||
rather than accepting client-supplied serialized state to decode. Established by
|
||
`GET /api/playouts/history/{id}` (`PlayoutController.GetHistoryDetails`, PR #182) — the row's raw
|
||
JSON (`Key`/`Details`) is decoded server-side into `PlayoutHistoryDetailsResponseModel`, the client
|
||
never round-trips the raw payload itself.
|
||
|
||
## 2026-07-07 — Season/episode/music-video drill-in via `parentId`, not new child-listing endpoints
|
||
|
||
Rather than adding dedicated child-listing endpoints per media kind (e.g. "list episodes of a
|
||
season"), the library-browse endpoint takes an optional `parentId` query param and the SPA drills
|
||
in by re-querying with it. Established across PRs #181/#183 (library-picker season drill-in, then
|
||
media-detail's season/episode/artist/music-video browsing). Avoids a combinatorial explosion of
|
||
per-kind child endpoints.
|
||
|
||
## 2026-07-07 — Convention docs read at session start, updated in-PR
|
||
|
||
`docs/api-conventions.md`, `docs/spa-conventions.md`, `docs/e2e-local.md`,
|
||
`docs/blazor-route-parity.md`, `docs/domain-model.md`, `docs/decisions.md`, and `docs/README.md`
|
||
are the standing reference set every ChicoryTV session should read before starting work, and each
|
||
one carries an explicit "update this doc in the same PR" rule rather than deferring doc updates to
|
||
a follow-up. These docs **replace per-session recon** — an agent reads the index
|
||
(`docs/README.md`) and the relevant convention doc instead of re-deriving conventions from the code
|
||
each time it starts API/SPA/E2E/parity work. A testing map and a generated-endpoint index are
|
||
tracked as still-to-come under #185. Drafting this doc set also surfaced a drift in
|
||
`ApiControllerSecurityTests.cs`'s hardcoded controller registry (several controllers under
|
||
`ErsatzTV/Controllers/Api/` are missing from it — see `docs/api-conventions.md` §6) — tracked as a
|
||
follow-up under #184 rather than fixed inline, since it's a pre-existing gap, not something this
|
||
doc-drafting pass caused.
|
||
|
||
## 2026-07-09 — Playback-troubleshooting completion feedback: poll status, no push channel
|
||
|
||
The SPA playback-troubleshooting screen (`PlaybackTroubleshootingScreen.tsx`, #145) reports FFmpeg
|
||
completion by **polling `GET /api/troubleshoot/playback/status` every ~2s** while a session is
|
||
running (plus one poll on mount so a session started elsewhere still gates Play), rather than a
|
||
server push. The status endpoint returns `{ state, exitCode, speed, logs }`; the screen captures the
|
||
running→completed/failed transition in local component state and surfaces a completion notice
|
||
(success on exit 0, warning otherwise) — the SPA equivalent of the Blazor page's MediatR
|
||
`ICourier`/`ISnackbar` `PlaybackTroubleshootingCompletedNotification`. Chosen over SignalR/SSE
|
||
because the SPA has **no push channel** and troubleshooting sessions are short and user-initiated, so
|
||
a lightweight poll (started on Play, stopped on settle/unmount) is simpler than standing up a new
|
||
real-time transport. Speed thresholds and the "(Speed: Nx)" badge colors are copied verbatim from the
|
||
Blazor `GetSpeedClass` (red <0.9, green >1.1, amber otherwise).
|
||
|
||
## 2026-07-09 — datetime-local instead of Chronic natural-language start parsing
|
||
|
||
The channel-mode "Date and Time" input in the SPA playback-troubleshooting screen uses a native
|
||
`<input type="datetime-local">`, a **deliberate deviation** from the Blazor page, which parsed a
|
||
free-text field with `Chronic.Core.Parser` (natural language like "yesterday at 8pm"). The SPA has no
|
||
Chronic dependency and a picker is unambiguous; the selected local datetime is sent to
|
||
`playback.m3u8` as an ISO-8601 `start` param via `new Date(value).toISOString()`, which the
|
||
controller binds to `DateTimeOffset?` exactly as the Blazor round-trip (`"o"`) format did.
|
||
|
||
## 2026-07-09 — SPA gates Download Media Sample while a session is active
|
||
|
||
Minor intentional deviation: the SPA playback-troubleshooting screen disables **Download Media
|
||
Sample** (alongside Download Results) while a troubleshooting session is starting/running; Blazor
|
||
only gated Download Results. Both downloads compete with the live transcode for I/O and the sample
|
||
archiver reads the same media file, so gating both during a session is strictly safer and costs
|
||
nothing (sessions are short).
|
||
|
||
## 2026-07-09 — OpenAPI spec mirrors the runtime Newtonsoft serializer (#198)
|
||
|
||
The generated OpenAPI document is made to follow the **runtime** JSON contract, not the reverse. Runtime
|
||
`/api/*` responses are serialized by Newtonsoft via `CustomContractResolver`/`CustomNamingStrategy`
|
||
(camelCase + a `FFmpegProfileId`→`ffmpegProfileId` special case + `[JsonProperty]` overrides such as
|
||
`ChannelResponseModel.FFmpegProfile`→`ffmpegProfile`), while `Microsoft.AspNetCore.OpenApi` generates the
|
||
spec from System.Text.Json metadata, whose camelCase drifted (`fFmpegProfileId`, `fFmpegProfile`). That
|
||
drift fed the SPA the wrong key. Rather than hand-patch the spec or change the wire format (breaking clients),
|
||
we added `NewtonsoftSchemaNamingTransformer` — an OpenAPI schema transformer registered on all three
|
||
documents that renames each schema property through the *same* Newtonsoft contract resolver the runtime uses,
|
||
so the spec matches the wire format by construction. A contract test
|
||
(`OpenApiSerializerContractTests`) serializes representative DTOs through the real runtime settings and pins
|
||
the spec property sets to them. Decision: **the wire format is the source of truth; the spec follows it via the
|
||
real contract resolver.** This also fixed a latent SPA bug (the channel-list "FFmpeg profile" column read
|
||
`fFmpegProfile` and always showed "Unassigned"). Issue #198.
|
||
|
||
## 2026-07-09 — YAML playout validator: paste-textarea instead of a server file path
|
||
|
||
The legacy Blazor YAML playout validator took a **server-side file path** (read directly off the
|
||
container's filesystem). The SPA's `YamlValidatorScreen.tsx` instead uses a paste `<textarea>` — a
|
||
deliberate deviation, not an oversight. The SPA runs entirely client-side against `/api/*` and has
|
||
no access to the server's filesystem, so a file-path field would either need a new
|
||
filesystem-browsing endpoint or silently fail; pasting the YAML directly is simpler and matches how
|
||
every other SPA editor already round-trips content through the API instead of the disk.
|
||
|
||
## 2026-07-09 — Channel numbers: prompt-driven sequential renumber instead of drag-to-reorder
|
||
|
||
The legacy Blazor channel list let you drag-and-drop rows to reorder channel numbers. The SPA
|
||
(`App.tsx`) instead offers a "Renumber" action that walks the list and asks for each channel's new
|
||
number via a sequence of native `prompt()` calls. Deliberate deviation: drag-to-reorder needs a
|
||
dedicated drag library and a bespoke reorder-persistence endpoint; a sequential prompt reuses the
|
||
existing per-channel update call and needs no new UI dependency. Revisit only if channel counts grow
|
||
large enough that prompt-per-channel becomes tedious.
|
||
|
||
## 2026-07-09 — "Table, not calendar" convention also covers the deco-templates editor
|
||
|
||
Extends the 2026-07 "Templates editor in the SPA is a table, not Blazor's drag-calendar" entry
|
||
above (not editing that entry — this generalizes it): the deco-templates editor
|
||
(`DecoTemplatesScreen.tsx`) follows the same convention, rendering its day/deco assignment as a
|
||
table rather than reproducing Blazor's drag-and-drop calendar grid. Same rationale, same
|
||
accepted-deviation status — don't "fix" either editor to match Blazor's interaction model without
|
||
discussing it first.
|
||
|
||
## 2026-07-09 — Trash "see all" is capped at 100 items per kind; true paging deferred
|
||
|
||
`TrashScreen.tsx` requests `GET /api/search?query=state:FileNotFound&pageSize=100` — 100 is
|
||
`SearchController`'s `MaxPageSize`, and the endpoint has **no page-number parameter**, so a client
|
||
can't page past the first 100 matches of a given media kind. The SPA shows the first 100 per kind
|
||
(movies, shows, episodes, etc. are separate groups, so in practice most trash lists fit well within
|
||
that per-kind cap); this mirrors the legacy Blazor Trash page, which had the same underlying search
|
||
behavior and cap. True paging is deferred until the search API grows a page param — not attempted
|
||
here, since it would mean adding a paging contract server-side, out of scope for this pass.
|
||
|
||
## 2026-07-09 — Per-playout "Schedule reset" button dropped; Reset uses the server-default build mode
|
||
|
||
Blazor's playouts page had both a per-playout **Reset** and a separate **Schedule Reset** control
|
||
(setting the daily rebuild time). The SPA keeps Reset — `POST
|
||
/api/channels/{channelNumber}/playout/reset` with no `mode` param, so the server picks the same
|
||
default Blazor used (Classic → Refresh, everything else → Reset) — but drops the dedicated
|
||
"Schedule reset" button: the daily rebuild time is already editable through the playout's
|
||
Edit-details flow, so a second entry point would duplicate an existing capability. Deliberate
|
||
deviation, not a lost capability. Issue #210.
|
||
|
||
## 2026-07-09 — Collection custom order: move up/down buttons, any-kind collections
|
||
|
||
Blazor reordered collection items with SortableJS drag-and-drop and only enabled custom ordering
|
||
for movies-only collections. The SPA (`CollectionsScreen.tsx`) uses per-row **Move up / Move
|
||
down** buttons in an explicit reorder mode instead of drag (no new drag dependency; the mode loads
|
||
ALL items first because `PUT /api/collections/{id}/custom-order` replaces the whole order from
|
||
array position — submitting a partial page would scramble the rest), and does **not** replicate
|
||
the movies-only gate: the API and the playback-side `CustomOrderCollectionEnumerator` sort by
|
||
`CustomIndex` regardless of item kind, so the SPA offers reorder for any manual collection with
|
||
custom order enabled. Issue #211.
|
||
|
||
## 2026-07-10 — Shared "Add to…" layer lives in `web/src/media/addTo/`; select-mode is an explicit toggle
|
||
|
||
The media mutation surface (#208/#209) is built on one reusable component group,
|
||
`web/src/media/addTo/` (media-domain components, like `MediaPosterCard` — not generic
|
||
`components/`): `AddToCollectionDialog` (existing-collection Select + inline "(New collection)"
|
||
create, Blazor `AddToCollectionDialog.razor` parity), `AddToPlaylistDialog` (group → playlist
|
||
Selects, no inline create), `AddToScheduleDialog` (schedule Select; payload replicates Blazor's
|
||
`AddProgramScheduleItem.ForMediaItem` defaults — see `addTo/scheduleItem.ts`),
|
||
`SaveAsSmartCollectionDialog`, and `AddToMenu` (the drop-in popover for cards/detail pages via
|
||
`MediaPosterCard`'s `actions` slot). New screens wanting add-to affordances use this layer —
|
||
don't build screen-local pickers. Two deliberate deviations from Blazor, applied consistently on
|
||
the search and browse screens: **(1) multi-select is an explicit screen-level "Select" toggle**
|
||
(off = cards open, on = cards select) rather than Blazor's always-on corner-select, because
|
||
`MediaPosterCard`'s select handler takes over the card's single click gesture; **(2) the
|
||
per-card menu offers collection/playlist for a single item of any kind, plus schedule only for
|
||
shows/seasons/artists** — collection/playlist is a superset of Blazor's per-card collection-only
|
||
menu, while the schedule target is gated to exactly the kinds Blazor's
|
||
`AddProgramScheduleItem.ForMediaItem` call sites offer, because the server validator
|
||
(`ProgramScheduleItemCommandBase.CollectionTypeMustBeValid`) accepts only the
|
||
TelevisionShow/TelevisionSeason/Artist per-media-item CollectionTypes and 422s the rest.
|
||
"Add All" (query-wide) mirrors Blazor's two-step: materialize ids
|
||
via `GET /api/search/all-items`, then reuse the id-list add endpoints — no query-based add
|
||
command exists server-side. Issues #208/#209.
|