Whitespace-only (git diff -w is empty); the #311 format job checks whole
touched files, and these two legacy files carried pre-existing violations
never caught before (no PR had touched them since the gate landed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Field-reference update skipped: docs/channels.md and docs/domain-model.md
have no per-field FFmpeg-profile catalogue (channels.md's Encoding bullet
is a one-line summary, not a field list; grep for QsvExtraHardwareFrames
or HardwareAcceleration finds no such list in either doc).
- Blocker 1: VA-API decode to SOFTWARE frames (drop -hwaccel_output_format,
new DecoderVaapiToSoftware) so the proven hwupload/vpp_qsv branch bridges to
the QSV encoder — the naive hardware-surface path emits a bare vpp_qsv on
VA-API frames and fails on ~all content.
- Blocker 2: bool? domain property + != false coercion (DeinterlaceVideo
pattern) so create-with-false actually persists false.
- High 3: REST DTOs bool?=null + ?? true for /api/v1 additive-compat.
- Medium 4: correct Task 4 test scaffolding (DefaultHardwareCapabilities).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors Jellyfin's default-on "prefer native decoder" hybrid via a new
QsvPreferNativeDecoder profile boolean. Reverified against code before
designing; records the rejected decode-family-enum alternative (Option C).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Whole-branch review found the stamping test was structurally vacuous: a bare
foreach over ChannelTemplates.Where(IsSystem) passes with zero iterations, so
the test would have stayed green if template seeding silently bailed out.
Assert the collection is non-empty first. Proven non-vacuous by a negative
control (forcing SeedChannelTemplates to bail makes exactly this test fail).
Also cover the ACTUAL production sequence -- adopt an existing hand-made row,
then delete it -- which the previous no-resurrect test did not exercise (it
covered seed-then-delete). The marker is written on the adopt path too, so
the deleted row must stay deleted.
docs: note that a deleted preset degrades to no default rather than failing,
and that the default applies to newly created channels, not retroactively.
Refs #67
- Add ChannelEditScreen tests exercising the geometry-fetch/preview path (previously
untested because the fixture's blank logo path kept BugPreview from ever rendering):
asserts the fetched watermark geometry lands on the preview image with asymmetric
values, and that an external logo URL suppresses the preview.
- Fix a toggle-off/toggle-on data loss: re-enabling "use logo as on-screen bug" used to
always rebind to the default 'Channel Bug' preset, discarding a channel's own
per-geometry ChannelLogo preset. Remember the last referenced logo-driven watermark id
in a ref and prefer it, falling back to the shared default.
- Strip the geometry cache's `id` key before it reaches BugPreview's props (was leaking
via a spread).
- Drop the "(see #502)" issue reference from the visible help text; keep it in the code
comment.
- Constrain findLogoBugWatermark's imageSource param to the generated
ChannelWatermarkImageSource union instead of `string`, and add unit tests for its
preference/fallback/none-found behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the raw <img> preview in the Watermarks editor's Image row with
the shared BugPreview component, so the on-screen bug's location/size/
margins/opacity render the same way as the other three preview spots.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WatermarkResponseModel gains ImageSource so a client can identify
logo-driven presets generically instead of matching a user-editable name.
Additive under the frozen-additive /api/v1 contract (#286).
Adding a positional record parameter is source-breaking for existing
constructor call sites, so the two test files that built the DTO
positionally are updated. WatermarkHandlerTests now seeds its two rows with
DIFFERENT image sources so the round-trip assertion proves the field is
actually carried through the mapper rather than matching a constant on both.
Regenerated v1.json, endpoint-index.md and v1.d.ts; check:api clean.
Stripped the inherited UTF-8 BOM from Mapper.cs (#311 fix-as-you-touch).
Refs #67
Extracts on-screen bug (watermark) placement math into a pure,
DOM-free bugPreviewStyle(geometry) function plus a BugPreview
component that renders it inside a 16:9 frame. Consumed by the
channel and playout watermark editor screens (tasks 4/5) so users
can preview bug placement before saving.
Fable review (cold, review-only) found a blocker in the plan's own code:
Task 5/6 selected the logo-bug preset by searching for the first
imageSource==='ChannelLogo' entry, but getWatermarks() sorts by name
(pickers.ts:14), so with a second logo-driven preset -- which this design
explicitly invites users to create -- the toggle would read OFF for a
logo-driven channel and REPOINT it on tick, from a control documented as a
pure reflection of stored state. Now resolved by id lookup, with the
tick-on target chosen by a shared findLogoBugWatermark() helper and pinned
by a two-preset regression test.
Also adopted:
- ConfigElement seed marker (watermark.channel_bug_seeded): ChannelWatermark
has no IsSystem flag and Initialize runs every startup, so a name-only
guard resurrected a deliberately deleted preset forever.
- Channel-editor preview now fetches the referenced preset's REAL geometry
via the existing GET /api/v1/watermarks/{id} instead of hardcoding the
seeded defaults -- which would have been wrong for exactly the users who
tuned theirs.
- The 're-save untouched leaves watermarkId unchanged' test the spec
promised and the plan had omitted, plus a create-path degrade test.
- Reversed the ChannelBuilder exclusion (operator decision): fresh installs
stamp the preset onto the templates the seed creates; existing installs
are untouched.
- External-URL logos never render a bug (File.Exists against a URL,
WatermarkSelector.cs:269-286) -- verified, filed as #502, preview no
longer promises it.
- Dropped Task 2's InternalsVisibleTo branch: already present
(ErsatzTV.Application.csproj:30-32).
Refs #67#502
Self-review against the spec caught that the spec named both SPA creation
paths, but ChannelBuilder does not hardcode a watermark -- it inherits
watermarkId from the selected ChannelTemplate. Defaulting it would mean
mutating the seeded Standard/Music videos template rows, which the seed's
name-guard deliberately skips on existing installs, so it would no-op on
prod while changing fresh installs. Excluded in both documents with the
reason recorded.
Refs #67
Recon showed ChannelWatermarkImageSource.ChannelLogo already resolves the
channel's logo artwork at render time, and prod already runs that pattern
by hand (43 channels -> one 'Channel Bug' preset). So #67 is productizing
a proven mechanism, not building one: seed the preset idempotently in
DbInitializer (adopting any existing same-name row untouched), default the
SPA creation paths to it, and add a shared <BugPreview> that renders the
resolved geometry in both the channel editor and WatermarksScreen.
No schema change, no WatermarkSelector change. One additive DTO field
(WatermarkResponseModel.imageSource) under the frozen-additive /api/v1.
Refs #67
The media transcoders (Jellyfin, ersatztv, ersatztv-test) moved from bumblebee
to jazz (192.168.1.29) on 2026-07-20, server-management#633. Our docs still sent
the release operator to bumblebee for security-scan.sh and named it as the docker
host, which would have scanned the wrong box.
Also fixes the inverse error: `jazz` was an EARLIER name for the .99 host, so
three pre-migration references meaning today's bumblebee read as jazz. Those are
now bumblebee, and a Hosts table + name-reuse warning is added at the top of
ci-cd.md so the next session resolves hostnames by IP rather than re-breaking it
in either direction.
Version table had drifted (stopped at v26.9.0) — adds v26.10.0 + v26.11.0.
Design spec and bite-sized TDD implementation plan for #489, whose
implementation landed in #493.
Docs-only. Kept as the record of how the design was reached: that Jellyfin
classifies mixed-library items server-side via includeItemTypes (so no
inference is needed), that MediaItem is TPT keyed on LibraryPathId (so no
migration is needed), that MediaKind is dispatch + presentation only, and why
the feature is deliberately scoped to Jellyfin rather than local libraries.
Also records the open risk the plan carried -- the music-video scanner's
untraced reconciliation -- which #494 subsequently answered.
Refs #489
Co-authored-by: Timothy <timothy.look@gmail.com>
Co-committed-by: Timothy <timothy.look@gmail.com>
Jellyfin libraries typed `mixed` were dropped by JellyfinApiClient.Project's
`_ => None` with no log line, so music and standup content could not be
ingested without a local-library workaround that bypassed Jellyfin entirely.
Adds LibraryMediaKind.Mixed, maps "mixed"/absent/blank CollectionType onto it,
and gives SynchronizeJellyfinLibraryByIdHandler a Mixed arm composing the three
existing per-kind scanners. Jellyfin classifies items server-side via
includeItemTypes, so the passes see disjoint sets; reconciliation is type-scoped
and cannot cross-delete. No new scanner and no DB migration -- MediaItem is TPT
keyed on LibraryPathId, so heterogeneous contents were already legal.
Segregation falls out of the model: a library is a place (one path <-> one
Jellyfin library <-> one ErsatzTV library), so music/standup cannot leak into
Movies or TV Shows.
Also removes the silent-success `_ => Unit.Default` from both scanner
dispatchers, which returned Right for an unhandled kind and stamped LastScan as
though a scan had run, and rejects Mixed for local libraries at the API.
Deliberately Jellyfin-only: local scanners share one video extension list and
would claim each other's files, and LibraryFolder etags are keyed by
LibraryPathId with no notion of kind.
Verified by live E2E against a real Jellyfin, including the interaction with
#494's reconciliation sweep. Four cold review rounds, all MERGEABLE.
fixes#489
Co-authored-by: Timothy <timothy.look@gmail.com>
Co-committed-by: Timothy <timothy.look@gmail.com>
JellyfinMusicVideoLibraryScanner.UpdateMetadata copied only scalar fields
for an EXISTING music video, so genres/tags/studios/artists edited in
Jellyfin never reached ErsatzTV — the update path silently dropped every
child collection (only the Add path ever persisted them). Root cause is
inherited from upstream: unlike the movie/episode Jellyfin path, which
reconciles collections inside the tracked repository GetOrAdd,
MusicVideoRepository.GetOrAdd is AsNoTracking and the scanner never
reconciled the collections itself.
Fix mirrors PlexMovieLibraryScanner.UpdateMetadata's remove-stale +
add-new idiom, reconciling exactly the collections that BOTH the Add path
persists AND GetOrAdd eager-loads: Genres, Tags, Studios, Artists. Guids
(add-persisted but not eager-loaded — would duplicate) and Directors
(eager-loaded but not add-persisted for music videos) are deliberately
out of scope.
Movie/Episode paths do NOT have this gap (they reconcile in the tracked
repo GetOrAdd), so no separate fix is needed there.
Test is an interaction test (substituted repos, canned existing item)
verifying the exact reconcile calls; proven non-vacuous. The real-DB
double-scan approach can't drive this: the in-memory harness shares one
SQLite connection across contexts and mid-scan GetOrAdd's First()-nav
path predicate mis-resolves once the existing item carries metadata
children — a harness-only quirk (prod uses per-context pooled
connections).
fixes#497
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(494): reconcile removed music videos in Jellyfin scanner (#495)
JellyfinMusicVideoLibraryScanner did add/update only, so a music video or
artist deleted on the Jellyfin side lingered forever. Add a library-scoped
trash sweep (TrashMissingMusicVideos) gated by the #477 empty-fetch guard.
fixes#494
Co-authored-by: Timothy <timothy.look@gmail.com>
Co-committed-by: Timothy <timothy.look@gmail.com>
GetOrAddFolder read libraryPath.LibraryFolders, a navigation collection only
eager-loaded on the local scan path (via GetLibrary). The Jellyfin remote sync
path takes its LibraryPath off the JellyfinLibrary entity, where LibraryFolders
is null, so .Filter(null) threw ArgumentNullException('source') on the first
item of every Jellyfin music-video scan — a feature that had therefore never
run in prod, CI, or locally.
Look the folder up from the DB by (LibraryPathId, Path) instead, removing the
implicit eager-load contract entirely (correct for all nine callers) and
documenting it on ILibraryRepository. null != empty is preserved so a re-scan
does not insert duplicate LibraryFolder rows. No new hot-path cost: local
scanners already query GetParentFolderId per folder just before this call.
Tests:
- LibraryRepositoryTests: GetOrAddFolder with a null LibraryFolders (the exact
remote-path shape) creates the folder, is idempotent on re-scan, and persists
a supplied ParentId.
- JellyfinMusicVideoLibraryScannerTests: an end-to-end scan of one synthetic
music video, wiring the REAL LibraryRepository/ArtistRepository/
MusicVideoRepository against in-memory SQLite (the existing MediaServer*
scanner tests mock every repo, which is why the bug escaped), asserts the scan
completes and creates Artist + MusicVideo rows with a real LibraryFolder.
Both proven non-vacuous against the reverted fix (each reproduces the issue's
ArgumentNullException). decisions.md entry added.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
External-JSON playout channels build their own /media/plex/{server}/{plexFile}
URL in ExternalJsonPlayoutItemProvider.StreamRemotely and the handler assigns it
without routing through ValidatePlayoutItemPath, so the #473 class survived here:
a media item gone from the server 404s under ffmpeg (exit 8) and the same dead
item is re-selected for its whole slot.
Route StreamRemotely through the same IRemoteStreamProber seam #473/PR #479 added
for the generated-playout path. Probe runs before the Plex metadata round-trip
(the URL needs only server.Id + plexFile), so a gone item skips it. An unavailable
stream returns PlayoutItemNotAvailableFromMediaServer, which the handler already
maps to a real-error card. The fail-open policy (redirected-404 only) lives inside
IRemoteStreamProber, so this second call site duplicates only the decision to probe.
Tests: ExternalJsonPlayoutItemProviderTests pins both directions; proven non-vacuous
by neutralizing the probe. docs/decisions.md gets a #480 entry closing the #473
scope gap (append-only: the old #473 entry is cross-referenced, not edited).
fixes#480
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Third-round review caught that
Stale_Playlist_Guard_Should_Take_Precedence_Over_An_Otherwise_Valid_Three_Way
describes an impossible case: ThreeWay requires
processLaunched <= playlistExists, which is exactly the negation of the
guard condition, so the guard can never preempt a ThreeWay. What the
test really pins is precedence over the progress branches
(TwoWayLateProgress) — still the ordering that matters.
That is the same "rationale misstates the mechanism" defect the previous
commit existed to fix, landed inside the fix itself. Renaming rather
than leaving a test whose name teaches the next reader something false.
Also broadens the escape-hatch caveat: a stale playlist that slips past
the guard lands as TwoWay more often than ThreeWay, since FFmpeg has
usually not reported progress that early.
Test name and comments only; no logic change.
A successful fetch returning zero items made existing.Except([]) flag the ENTIRE
library FileNotFound in one scan — feeding EmptyTrashHandler's permanent delete
and emptying every affected collection (dead channels). Add a shared
MediaServerReconciliationGuard that skips (and logs a Warning) the sweep when
incoming==0 while items exist, wired into the three library-level sweeps
(Television shows / Movie / OtherVideo).
An empty incoming set is indistinguishable at scan time from a mid-restore /
emptied-upstream error (both report a zero total), so this deliberately overrides
#476's degenerate "last item removed => empty incoming => flag" case. #476's
cascade still fires for partial deletions (survivors present); its characterization
test moves from an empty incoming to a survivor+removed partial-deletion case.
Tests: policy table (MediaServerReconciliationGuardTests) + per-scanner integration
proving the wiring (empty incoming + non-empty existing flags/reindexes nothing).
Proven non-vacuous by neutralizing the guard. Nested TV season/episode sweeps left
unguarded (bounded blast radius); ratio-threshold + projection-failure detection
deferred to a follow-up. docs/decisions.md updated.
Fixes#477
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-review of the fix commit returned MERGEABLE with one Medium: the
comment justifying the stale-playlist guard misstated the mechanism, in
three places. It claimed Run "warns about a non-empty transcode folder
but does not delete it" — but StartFFmpegSessionHandler.FolderMustBeEmpty
calls EmptyFolder BEFORE the worker spawns, and Run's finally empties it
again. Verified directly rather than taken on the reviewer's word.
The real residual path is EmptyFolder FAILING: it swallows every
exception into a LogWarning and continues. Say that instead.
On a PR whose entire value is that the numbers mean what they say, a
rationale comment that misstates the mechanism is the same class of
defect the PR exists to prevent, so it does not get to ship as a nit.
Also documents that the guard is best-effort rather than a proof (if the
wipe failed, before-or-after-launch is a scheduling race, so an unlucky
sample can still slip through as an implausibly fast ThreeWay), and adds
the test the reviewer noted was missing: guard precedence over an
otherwise-valid ThreeWay.
Comments and one test only; no logic change.
timothyandClaude Opus 4.8