100 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
timothyandClaude Opus 4.8	2977f86c25	fix(409): report never-scanned LastScan as null for API/MCP parity (data migration + read coercion + SPA simplify) ... Data migration nulls the historical 0001-01-01 sentinel rows on Library/LibraryPath (both providers); GetAllMediaSourcesForApiHandler coerces any residual sentinel to null; SPA drops the client-side heuristic now that the API is honest. fixes #409 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 21:20:18 +02:00
timothyandClaude Opus 4.8	dd55f00ed4	docs(420): sharpen the skip's honest hit-rate (rebase-before-merge) + cite live combined-status check ... Review nits from the scoped #420 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 20:53:23 +02:00
timothy	922b1ef53c	docs(398,420): document the #420 skip + record build-once measured-and-rejected	2026-07-18 20:53:23 +02:00
timothy	eb12176fe1	ci(420): skip re-validating an already-green identical tree on merge-to-main	2026-07-18 20:52:44 +02:00
timothyandClaude Opus 4.8	1d95cbfee6	fix(403): accurate Classic fallback message + engine-coverage tripwire ... Review follow-ups (cold review of PR #457): - Marathon can reach PlayoutBuilder's default arm via `goto default` when its enumerator can't be built; the new warning claimed "Marathon is not supported by classic scheduling", which is false. Distinguish supported-but-failed-to-build (logs "could not build") from genuinely unsupported using PlaybackOrderSupport, which also makes Classic a runtime consumer of the matrix. - PlaybackOrderSupportTests now asserts every SchedulingEngineKind has a matrix entry, so a new engine kind fails the test instead of throwing KeyNotFoundException at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 18:32:14 +02:00
timothyandtimothy	ca87336600	fix(401): reject Mirror channel with playout (422) + repair dead else-branch (#456) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:27:26 +00:00
timothyandtimothy	f62ff0eebc	fix(327): validate playlist name on rename (reject empty/whitespace/too-long) (#454) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:27:19 +00:00
timothyandClaude Opus 4.8	0f34c86afa	feat(403): make unsupported PlaybackOrder loud at build time + tripwire ... Adding a new PlaybackOrder was unsafe by construction: three build-time dispatch sites turned an unknown value into an enumerator silently. Classic substituted RandomizedMediaCollectionEnumerator (the // TODO default arm), PlaylistEnumerator had no default arm so the item was dropped, and BlockPlayoutBuilder's allow-list continue skipped it. (#70 already made YAML/Scripted log a warning and MultiCollectionGroup throws.) - each silent site now logs a Warning naming the order + engine + the fallback taken; the fallback itself is preserved so a live channel never goes dark on one misconfigured item and scheduler goldens do not move. - PlaylistEnumerator.Create gained an optional Option<ILogger> (it was static with no logger -- why the drop was unreportable); loggered callers pass it. - BlockPlayoutBuilder gained an explicit Random arm (it previously reached an enumerator only via the coincidental _ => fallback) and a loud defensive fallback. - new PlaybackOrderSupport matrix (per SchedulingEngineKind) + tripwire PlaybackOrderSupportTests: Supported ∪ Unsupported must partition the enum for every engine, so a new order fails the test until classified. BlockPlayoutBuilder consumes the matrix for its allow-list. - write-path rejection left unchanged (#70 closed the persistence hole; the perimeter has been wrong three times per decisions.md); reverse _ => None mappings reviewed and deferred (different axis; making them loud would warn on legit enumerator types). docs/decisions.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 18:23:21 +02:00
timothyandtimothy	b1521f9047	fix(409,447): LibrariesScreen 'Never scanned' label + stabilize flaky scan-poll test (#451) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:18:45 +00:00
timothyandtimothy	ebb9a40dc3	fix(410): log scan cancellation below ERROR (user-initiated, not a failure) (#453) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:18:02 +00:00
timothyandtimothy	ab8e5d7a91	ci(338): distinguish ZAP warning (exit 2) from failure (exit 1) in security-scan (#452) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:16:41 +00:00
timothyandtimothy	11ec07ee1b	fix(367): correct Plex budget-exhausted guidance when no servers exist (#450) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:15:37 +00:00
timothyandtimothy	6462c36983	fix(310): strip UTF-8 BOM from the legacy .cs files #269 touched (#449) ... Co-authored-by: Timothy <timothy.look@gmail.com> Co-committed-by: Timothy <timothy.look@gmail.com>	2026-07-18 16:15:35 +00:00
timothyandClaude Opus 4.8	8cef07a673	fix(396): mark active route in a default-collapsed group; fix mobile brand bar ... Cold-review findings: - #1 (CONFIRMED): deep-linking/reloading into a route inside a default-collapsed labeled group (e.g. /app/settings, /app/libraries) left the expanded sidebar with no active indicator, violating "active route marked in both states". Now the group that contains the active route is force-shown (and its header shows expanded) WITHOUT persisting — navigating away reverts to the stored preference. Regression test added. - #2 (mobile): a persisted-collapsed state hid the brand logo+wordmark while the toggle is also hidden at <=980px, leaving an empty header. Restore the brand in the mobile query (collapse is desktop-only). Verified live (Playwright): /app/settings deep-link shows Settings active + aria-current, System auto-revealed, Media stays collapsed, nothing persisted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 16:43:33 +02:00
timothyandClaude Opus 4.8	eb47aed767	feat(396): collapsible sidebar + nav-group accordions ... Add two independent, persisted collapse states to the shell sidebar: - a header toggle that collapses it to a 60px icon rail - collapsible accordions per labeled nav group (Media, System); the unlabeled Primary group is always open, default-collapsed groups State + persistence live in web/src/app/sidebarState.ts (useSidebarState); AppShell stamps ctv-app-shell-collapsed on the shell root and the rail look is CSS-driven. Two namespaced localStorage keys (ctv-sidebar-collapsed, ctv-sidebar-groups) per the persisted-UI-state convention. In the rail, accordions are ignored (all items shown icon-only, label kept in the a11y tree + surfaced as a title tooltip, badges as a corner dot); active-route indicator works in both states; grid-column transition respects prefers-reduced-motion. Tests: colocated sidebarState.test.ts + a new describe in App.test.tsx (default-collapsed, accordion toggle+persist, rail, reload persistence). Docs: spa-conventions §13 + decisions.md 2026-07-18. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 16:43:33 +02:00
timothyandClaude Opus 4.8	a1b75c1f2c	docs(363): sweep the scan-flow drift the re-review caught ... The fix commit softened the script's scan-409 wording + dropped ?deep=true but left the docs describing the old behavior. Match them: - docs/e2e-local.md: POST .../scan (no ?deep; note local scans always ForceScan) + "409, deterministic bar a tiny residual TOCTOU gap" (was "guaranteed 409") - docs/ci-cd.md: same "guaranteed 409" -> "409 (deterministic bar ...)" Docs-only; harness behavior unchanged (still 38/38 green, deterministic). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:44:47 +02:00
timothyandClaude Opus 4.8	4345180a56	review(363): robustness + wording fixes from cold review ... - check seed_library_path succeeded (print lastrowid) so a silent seeding failure surfaces as a FAIL instead of degrading Flow A to an advisory skip with no diagnostic (Medium) - anchor the scan-status match to '"libraryId":2,' so it can't substring-match "libraryId":20/23 if the suite ever creates more libraries (Low) - drop the no-op ?deep=true (local scans always ForceScan; deep only affects Plex/Jellyfin/Emby) + note why (Low) - soften "guaranteed 409" for the scan flow to note the tiny residual TOCTOU gap the multi-second scan covers; Flow B stays race-free by construction (Low) - correct the "WAL tolerates a second writer" wording to the real reason (the busy-timeout retry serializes the writer) in the script + both docs (Nit) - use TEST-NET-1 192.0.2.1 (RFC 5737) instead of RFC1918 10.255.255.1 for the non-routable Jellyfin address (Nit) Re-verified: fresh-instance harness runs green (38/38), lock section deterministic. Functional E2E CI job already green on the prior head. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:33:39 +02:00
timothyandClaude Opus 4.8	6c8c7feeba	feat(350): instrument HLS cold-start latency (phase split + feature flags) ... Adds one Information-level structured log per HLS tune-in cold-start so the real driver breakdown can be measured on prod before optimizing the transcode pipeline (measure-before-optimize). Log-only; no transcode behavior change. - WaitForPlaylistSegments returns a PlaylistSegmentsResult: Phase A (process startup -> playlist exists) vs Phase B (segment fill), segments reached, deadline-expired. - StartFFmpegSessionHandler emits one summary: total = setup + startup + fill, plus cleanly-detectable feature flags (subtitle burn-in, hwaccel family). - ColdStartFeatures: pure, unit-tested args->features helper (14 cases). Watermark / HDR->SDR / image-subtitle burn-in are deliberately not flagged (all reduce to overlay= in the args, indistinguishable); the full ffmpeg arguments remain available at Debug. Refs #350 (instrumentation slice; optimization deferred pending real data). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:30:56 +02:00
timothyandClaude Opus 4.8	8a85f9ddb5	test(363): functional-E2E harness — add deterministic scan-lock + collections-lock 409 flows ... Extends scripts/e2e-functional.sh with the two IEntityLocker 409 contracts the first cut (ersatztv#299) deferred as "racy", made DETERMINISTIC by firing the racing request only once the lock is provably held (no sleep-and-hope): - library-scan "already scanning" 409: seed ~60 tiny ffmpeg clips into the built-in Shows library so the scanner subprocess runs a few seconds, poll GET /libraries/scan-status until the library is active (that window is a strict subset of the scan lock's held window — StartScan after LockLibrary, EndScan before UnlockLibrary), then a second POST .../scan is a guaranteed 409. Self-skips (advisory) when ffmpeg is absent. - external-collections "already scanning" 409: seed a Jellyfin media-source row pointing at a non-routable address so the background sync hangs and the per-family lock stays held; the lock is taken synchronously before the 202, so the 202 proves it held. collections-scan-status corroborates; unknown source 404. Seeding uses python3's stdlib sqlite3 (already a harness dep) to insert rows the API can't create (LibraryPath, media-source); WAL mode tolerates the second writer. No new CI step/dependency — ffmpeg ships in the toolchain image. Verified: 4/4 fresh-instance runs green (38/38), lock section deterministic. Still deferred to #363 follow-ups: the playout-build lock 409 + isLocked projection (#215) and the UI-interactive Playwright flows. Docs updated same PR: docs/e2e-local.md, docs/ci-cd.md, the functional-e2e job comment in .gitea/workflows/docker-build.yml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 15:17:21 +02:00
timothyandClaude Opus 4.8	c57fbf9826	docs(293): note the pageNum upper clamp (MaxAllItemsPageNum) in the decision + api-conventions ... Fix delta re-review flagged the decision entry + §5 note still described the pre-fix pageNum = Math.Max(0, pageNum); the shipped code clamps the upper bound too (0..2_000_000) to stop pageNum*pageSize overflowing int to a 500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 13:40:20 +02:00
timothyandClaude Opus 4.8	ac7965dee4	feat(293): paginate GET /api/v1/search/all-items to cap DoS exposure ... The all-items endpoint fired ten index searches with limit:0 (every hit), so a broad authenticated query materialized the whole index into one response. Add optional pageNum/pageSize (clamped 1..1000; pageNum 0..2_000_000 so skip can't overflow int) and an additive per-kind Totals on the response; the SPA add-all flow now pages to completeness instead of a single unbounded fetch. - SearchController.SearchAllItems: clamp params (Logs §1 precedent), map Totals - QuerySearchIndexAllItemsHandler: skip=pageNum*pageSize, limit=pageSize, read SearchResult.TotalCount per kind - SearchResultAllItemsResponseModel: additive Totals (frozen-v1-safe) - web/src/api/search.ts: getSearchAllItems paging params + getAllSearchItemIds (pages until each kind hits its total; empty-page safety break) - tests: controller clamp/thread/totals, handler skip/limit/totals, SPA paging - docs: decisions.md 2026-07-18 (#293), api-conventions.md §5; regenerated OpenAPI Design: issue option (a) full pagination, operator-confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 13:09:06 +02:00
timothyandClaude Opus 4.8	2281f2e764	fix(308): idempotent concurrent Add*ToCollection instead of a composite-PK 500 ... Two concurrent adds of the same item both membership-check it absent, both insert the CollectionItem composite key, and the loser's SaveChangesForcingVersion threw an uncaught DbUpdateException (SQLite 19 / MySQL 1062) -> 500. Now the loser is an idempotent no-op. - ConcurrencyExtensions.TrySaveChangesForcingVersion: bool-returning sibling that catches only a classified unique/PK violation and returns false. - 10 single-item Add*ToCollection handlers: return Unit.Default (no-op, skip fan-out) on false — the racing winner already inserted + rotated + rebuilt. - Bulk AddItemsToCollection: retry on a fresh context against recomputed membership so a partial-overlap collision doesn't drop the non-colliding items (bounded loop; common no-collision path runs once). - Provider detection via a TvContext.IsUniqueConstraintViolation static delegate (matches the existing IsSqlite/LastInsertedRowId provider seam), wired from Startup to SqliteErrorClassifier / MySqlErrorClassifier. - Add*ToPlaylist is NOT affected (PlaylistItem has its own identity PK; a playlist may legitimately contain the same item more than once). Tests: a negative-control anchor proves the race genuinely throws a classified exception; end-to-end handler tests reproduce a real cross-connection race via a shared-cache SQLite harness + a SavingChanges interceptor (the single-conn in-memory fixture cannot). Every fix-dependent test verified to fail with the catch disabled. Docs: api-conventions.md §7a (idempotent insert under concurrency) + decisions/optimistic-concurrency.md. fixes #308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 13:02:14 +02:00
timothyandClaude Opus 4.8	3bca000e27	fix(425): address adversarial review — hide owned artifacts from schedule picker ... Independent review found a blocker + two mediums: - BLOCKER: SearchSmartCollections/SearchMultiCollections (the scheduling collection picker) didn't filter OwnedByChannelId, so a user could select a system-owned auto-tune artifact into their own schedule item — a later channel delete would then cascade-delete that schedule item (ProgramScheduleItem-> MultiCollection is OnDelete(Cascade)). Both handlers now exclude owned rows; regression test added (CollectionPickerHidesOwnedTests). - MEDIUM: TryStampOwnership now runs on CancellationToken.None (post-commit convention #254) and stamps the MC + member SmartCollections in one transaction, so a late cancel or mid-way failure can't leave a permanent orphan / list leak. - Documented the movie-remainder weight clamp bound (>1000 un-touched movies can't express the exact ratio — the 1..1000 weight-column bound from #70). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 03:52:06 +02:00
timothyandClaude Opus 4.8	e364b338e6	feat(425): per-source rotation weights + query corrections for auto-tune channels ... Auto-tune channels can now carry per-content-source rotation weights (weighted round-robin, e.g. 3x Show A / 1x Show B) and query corrections (exclude / add-untagged), supplied at bulk-create time via an optional `sources: [{sourceId, weight, excluded}]` on each AutoTunedChannelRequest. Design (Option A, reuse #70): when a source is customized the channel is backed by a system-owned MultiCollection of per-source SmartCollections carrying the weights, with PlaybackOrder.WeightedShuffle -- the exact path WeightedShuffleCollectionEnumerator already consumes. All-default weights keep the #69 single-SmartCollection fair-share shape. - Discriminators: TV -> live show_title:"X" (episodes carry no parent-show id in the index); movies -> stable id:{mediaItemId}. - Materialization is axis-dependent: TV materializes every base show individually (un-weighted shows keep per-show fair-share) + a live remainder at weight 1; MovieGenre materializes only touched movies + one count-weighted remainder. - Remainder = (base) AND NOT (materialized union excluded) -- a partition. - New nullable OwnedByChannelId on SmartCollection + MultiCollection (dual-provider migration); owned rows are hidden from the collection lists and cascade-cleaned on channel delete. Tests: AutoTuneAxisMap query/partition units; DB-backed weighted-path handler tests (TV materialize-all, movie count-remainder, exclusion, no-customization fallback); delete-cleanup. Docs: decisions.md, domain-model.md, api-conventions.md; OpenAPI trio regenerated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 03:52:06 +02:00
timothy	3b5a78a08f	fix(176): compile date values to index yyyyMMdd format; guard Advanced toggle	2026-07-18 02:43:54 +02:00
timothy	c91c34f113	docs(176): decisions + spa-conventions for the SmartCollection rule builder	2026-07-18 02:43:54 +02:00
timothy	ea6b509016	fix(176): guard Builder toggle for out-of-subset queries; resync query on reseed	2026-07-18 02:41:50 +02:00
timothy	8483af41fc	docs(176): plan Task 8 — guard Builder toggle + resync query (matches shipped fix)	2026-07-18 02:41:50 +02:00
timothy	60e8655a00	feat(176): Builder|Advanced toggle in the SmartCollection dialog	2026-07-18 02:41:50 +02:00
timothy	2763f65b35	test(176): cover RuleBuilder field-change operator/value reset	2026-07-18 02:41:50 +02:00
timothy	ceb9069bae	feat(176): RuleBuilder component	2026-07-18 02:41:50 +02:00
timothy	7d553ef00c	refactor(176): expose clean non-null RuleField from useSearchFields	2026-07-18 02:41:50 +02:00
timothy	279c0d6ce5	docs(176): plan — hook exposes clean non-null RuleField (Core DTOs are nullable-disabled)	2026-07-18 02:41:50 +02:00
timothy	30a8f67e8e	feat(176): field-catalog api + useSearchFields hook ... Task 6: getSearchFields() client fn + SearchField type in api/search.ts, and the useSearchFields() hook in builder/rules/fieldCatalog.ts that reshapes the catalog into fieldTypes (name -> FieldType) and byGroup views for the rule builder (Tasks 7-8). Generated SearchFieldResponseModel has all-nullable fields (name/label/ type/group/values), unlike the brief's non-null assumption -- adapted by dropping fields with a null name and defaulting group to 'Other' / type to 'text' rather than indexing under undefined.	2026-07-18 02:41:50 +02:00
timothy	03646a2ed9	test(176): compile/parse round-trip property test	2026-07-18 02:41:50 +02:00
timothy	5186bed910	docs(176): plan — broaden round-trip generator to special chars; escaping now total	2026-07-18 02:41:50 +02:00
timothy	218e1810cb	fix(176): total escaping so wildcard/quoted values round-trip (parse review)	2026-07-18 02:41:50 +02:00
timothy	0b5258533a	feat(176): Lucene-subset parser (exact inverse of compiler)	2026-07-18 02:41:50 +02:00
timothy	bc524d0b53	fix(176): split fulltext matches/notMatches from text contains	2026-07-18 02:41:50 +02:00
timothy	7f7062729d	docs(176): fix plan operator model — split fulltext matches/notMatches from text contains (Task 3 review Critical)	2026-07-18 02:41:50 +02:00
timothy	fd786da649	feat(176): rule model + Lucene-subset compiler	2026-07-18 02:41:50 +02:00
timothy	ae22107d98	feat(176): GET /api/v1/search/fields endpoint + regenerated api artifacts	2026-07-18 02:41:50 +02:00
timothy	41df582032	feat(176): search field catalog query + curated list	2026-07-18 02:41:50 +02:00
timothy	aa4127fcef	docs(176): fix plan test-project path + parse.ts self-review nits; add SDD ledger	2026-07-18 02:41:50 +02:00
timothyandClaude Opus 4.8	8c7af0e202	docs(176): implementation plan — SmartCollection visual rule builder ... Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 02:41:50 +02:00
timothyandClaude Opus 4.8	71159c1a8a	docs(176): design spec — SmartCollection visual rule builder (sub-project #1) ... Scopes #176 down after recon (Autotune #69 + composite create-channel #63 already shipped): the remaining leg is the visual WYSIWYG rule builder. Kodi one-level-nested model, compile-only to a closed Lucene subset (no schema change), backed by a new read-only GET /api/v1/search/fields catalog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 02:41:50 +02:00
timothy	56fffacfd4	fix(386): address cold-review findings on the DetailPanel ... Cold adversarial review (no blockers, 3 real Mediums): - Query&size "Order" row ignored the Shuffle toggle (dead ?? fallback showing the static axis order); now reflects shuffleOn, consistent with the subtitle. - Clearing Name/Number to '' flagged the row Edited + armed the unsaved-changes guard, but the payload reverted to the proposal default — the two "edited" derivations disagreed. overrideEdited now ignores an empty (inherited) value. - runPreview did not clear per-channel overrides, so edits (incl. pinned numbers) silently carried into a re-generated batch → collision risk. Fresh preview now resets overrides/detailKey/logo cache. Tests: empty-name-not-Edited + re-preview-clears-overrides. Live-E2E (local instance, seeded TV library): preview → Configure → toggle Shuffle → Create verified end-to-end; the created channel's schedule shows PlaybackOrder=Shuffle (overridden) vs SeasonEpisode (axis default), confirming the per-channel override flows UI → SPA → backend → playout. Refs #386	2026-07-18 01:31:40 +02:00
timothy	f44eee85c5	feat(386): Auto-Tune per-channel DetailPanel slide-over (SPA) ... Adds a right-hand "Configure" slide-over to each Auto-Tune Preview row, making a proposed channel editable before bulk-create — against the shipped #384/#385 backend only, so no control lacks a wire target. - New reusable SlideOver primitive (components/overlay.tsx), sharing a useOverlayBehavior hook (focus/scroll-lock/Escape/scrim) with Dialog. - Extract the Channel Builder's advanced-options model to builder/advancedOptions.tsx (enum catalogs, ADVANCED_KEYS, effectiveValue, INHERIT/omit useAdvancedOverrides hook); ChannelBuilder imports it unchanged (its tests pass byte-for-byte). The DetailPanel writes its own field JSX over the same hook — shared logic, per-screen layout. - Panes: identity (name/number + logo upload), Playback (Shuffle/Always-playing → advanced.playbackOrder/playoutMode), per-channel template picker, Advanced disclosure, lean read-only Query&size, read-only Content-sources via GET /members. - getAutoTuneChannelMembers API client (#384 read endpoint) + tests. - Screen-scoped §8 unsaved-changes guard + "Edited" row badge. - Dropped as backend-less decoration: MiniEpg, bug-initials generator, query text. Deferred to #425 with an in-pane hint: per-source weight steppers + corrections. - Docs: spa-conventions §11 (SlideOver + shared advanced-options), decisions.md. Refs #386	2026-07-18 01:31:40 +02:00
timothyandClaude Opus 4.8	ed6c43065f	feat(164): guided remediation for health checks (server-declared {Kind, Target}) ... Make the ~14 health checks actionable: each check that has a fix now declares where to go, and the SPA acts on it. Backend: - Widen domain HealthCheckLink (string Link) -> (string Target, HealthCheckLinkKind Kind) with ExternalDoc|AppRoute + factories; only the 4 link-building checks and the API mapper touched .Link. - Evolve HealthCheckResponseModel additively (/api/v1 frozen-additive): keep deprecated string? Link (still populated), add Brief (the BriefMessage the mapper was silently dropping) and nested Remediation {Kind, Target}. Kind is a mapped string, not a wire enum. - Make Mapper.GetStatus total: NotApplicable no longer throws (defensive; handler still filters it). InternalsVisibleTo(ErsatzTV.Tests) added to unit-test totality. - Fix 2 stale Blazor route links (media/trash -> /app/trash, search?query -> /app/search); add AppRoute remediation to actionable checks that had none (libraries / schedules / ffmpeg-profiles / settings). SPA: - DashboardScreen health panel renders remediation: AppRoute -> client-side nav button, ExternalDoc -> new-tab anchor; detail text truncates with title-hover. - Remove the dead "Open Classic UI" -> /system/health row from SettingsScreen (a #91b leftover that just 302'd to /app); update its regression test. Docs: decisions.md (#164), api-conventions.md (deprecate-in-place DTO evolution), blazor-route-parity.md (Section 4 correction); v1.json/v1.d.ts/endpoint-index regenerated. fixes #164 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-18 00:51:40 +02:00
timothyandClaude Opus 4.8	b9ab2d79f3	test(385): cover override-driven per-channel failure isolation ... Adds the one coverage case the cold review flagged (LOW): a two-channel batch where the first channel's advanced override is rejected downstream and the second, override-free channel is still Created — directly exercising the partial-success / batch-independence guarantee for the new override path. Test-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 23:33:10 +02:00
timothyandClaude Opus 4.8	8f61ad6530	feat(385): per-channel overrides in auto-tune bulk-create ... Auto-Tune DetailPanel backend (#385), additive half. The create request `AutoTunedChannelRequest` gains three optional per-channel fields, all backward-compatible (omit = PR1 behavior): - `templateId` — overrides the batch template per channel - `advanced` — reuses the manual Channel Builder's `CreateChannelFromLineupAdvancedOptionsRequest` verbatim (24-field override set, `advanced.X ?? template.X` stamp contract). Axis default fills `PlaybackOrder` only when the caller leaves it null. - `logo` — uploaded channel image, `Sanitized()` at the request boundary (#283 stored-XSS defense), forwarded to `CreateChannelFromLineup.Logo` Resolved per channel inside `CreateAutoTunedChannelsHandler.CreateOne`, so one channel's bad override still yields a per-channel Failed/Skipped without aborting the batch. Per-source rotation weights + query corrections are split out to #425 (they need a MultiCollection-of-per-source-SmartCollections redesign — #70's WeightedShuffle reads weights only off MultiCollection join rows, and an auto-tuned channel is one SmartCollection). Bug-initials/colour generated logo also deferred (needs persisted Channel state + FFmpeg-pipeline wiring). Tests: handler override-threading (per-channel wins, axis default preserved, no-override baseline) + request `ToCommand()` logo sanitization. OpenAPI trio regenerated. Docs: decisions.md, api-conventions.md, domain-model.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 23:33:10 +02:00
timothy	1df08e86bb	ci: pre-push guard against pushing an uncommitted working-tree change (H13) ... A file in the pushed diff that still has uncommitted working-tree/index changes means the pushed commit doesn't match what you built/reviewed — the #416 index/worktree trap, where a --no-renames review fix lived only in the working tree and shipped without being committed (CI, the push, and the reviewer each saw a different tree). New fail-open pre-push hook blocks that precisely (only files in the branch's diff vs origin/main), after the H11 rebase check. Escape: ETV_ALLOW_DIRTY_PUSH=1. Documented in decisions.md. Refs #416	2026-07-17 23:05:30 +02:00
timothy	eb1aba99c1	docs(lore): run local gate + cold review BEFORE pushing to Gitea CI ... The handoff PR-routine and HARD-CONSTRAINTS placed the cold adversarial review AFTER push/PR-open. New standing rule (Timothy, 2026-07-17): local gate + cold review run BEFORE the push; fold pre-push fixes into the branch; CI only confirms. Runners should see only already-validated work — a Gitea run can't be cancelled, so a run spent on a lint/review-caught issue is pure waste. Also notes docs-only PRs are seconds now (ersatztv#416).	2026-07-17 22:39:42 +02:00
timothy	082124f706	docs(416): note shallow-checkout FETCH_HEAD/two-dot fix (ci-cd + decisions) ... Refs #416	2026-07-17 22:02:06 +02:00
timothy	74005cc952	fix(416): detect against FETCH_HEAD with a two-dot diff (shallow-checkout safe) ... The test/migrations jobs check out fetch-depth:1. A shallow clone has NO origin/<base> tracking ref and no merge-base, so the three-dot origin/main...HEAD errored -> empty diff -> docs_only=false -> EVERY docs-only PR silently ran the full matrix (safe but the skip never fired). git fetch always writes FETCH_HEAD, which resolves in a shallow clone; diff against it with a two-dot tree diff (no merge-base). Confirmed in a real shallow file:// clone: origin/main did NOT resolve and three-dot errored, while FETCH_HEAD two-dot correctly returned the docs file. api-docs/format were unaffected only because they use fetch-depth:0. Refs #416	2026-07-17 21:33:53 +02:00
timothyandClaude Opus 4.8	de63603aab	test(320): don't blanket-delete foreign *.ts in the shared troubleshooting folder ... Re-review nit: the regression test deleted every *.ts in the machine-global TranscodeTroubleshootingFolder, which could nuke a live troubleshooting session's segments on a dev machine (reaping files it didn't create). Drop the sweep and keep only Directory.CreateDirectory — the folder-exists guarantee is what closes the false-pass hole; NUnit runs serially and no test leaves >= 2 stray .ts, so determinism is unaffected (negative control re-verified: still fails in ~10s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 20:03:10 +02:00
timothyandClaude Opus 4.8	5f8525eed7	fix(320): address cold-review nits — drop redundant cancel term, harden test ... Cold review (PR #423) found two LOW nits: - The segment-wait break also checked cancellationToken.IsCancellationRequested, which could exit into the Ok gate and return a spurious 200 for a cancelled request with no segments. The term is redundant (Task.Delay already throws on cancel -> caught -> 404), so break on notifier.IsFailed only. - The regression test's non-vacuousness was filesystem-state dependent (absent troubleshooting folder -> Directory.GetFiles throws -> false pass pre-fix). Seed an empty folder like the sibling success test so the spin path is deterministic regardless of NUnit run order. Negative control re-verified: removing the IsFailed break fails the test in ~10s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 19:57:55 +02:00
timothyandClaude Opus 4.8	b2059bd2a6	fix(320): break troubleshoot segment-wait loop on ffmpeg failure ... The second (segment-readiness) poll in POST /api/v1/troubleshoot/playback/start only checked !hasSegments. If ffmpeg died after writing the HLS playlist but before the first segments appeared, the loop spun until the client cancelled — tying up the request thread and holding the troubleshooting lock (worse since #301 moved the manifest fetch server-side per #320's writeup). Mirror the first (playlist) wait loop's exit check so a dead ffmpeg or client cancellation breaks the wait; the existing !IsFailed guard then falls through to the terminal NotFoundProblem, releasing the thread + lock. Adds a non-vacuous regression test (verified it fails on the pre-fix spinning loop via a bounded cancellation deadline). fixes #320 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 19:50:54 +02:00
timothy	f7b97adce8	ci(416): harden docs-only detection with --no-renames (review finding) ... The changed-set git diff had rename detection on by default, so a code->docs rename (Foo.cs -> docs/Foo.md) showed only the destination and was misclassified as docs-only, skipping required tests on a code change. --no-renames surfaces the source deletion -> full matrix. Empirically verified. This is the cold-review MEDIUM; it was applied in the working tree but never committed before the first push (index/worktree mismatch) — committing it now. Refs #416	2026-07-17 19:44:25 +02:00
timothy	92bb63b6da	docs(416): document docs-only CI skip ... ci-cd.md gains a 'Docs-only skip' section + triggers-table note; decisions.md records the decision. Cross-refs the separate PR-vs-main rerun redundancy (#420) and the within-run triple build (#398). Refs #416	2026-07-17 19:44:25 +02:00
timothy	5ba737bec7	ci(416): skip heavy jobs on docs-only changes ... Docs-only changes (docs/** or *.md) ran the full docker-build matrix (~9 min). Each heavy job (test, migrations, functional-e2e, build) now runs scripts/ci-detect-docs-only.sh as its first post-checkout step and gates every real step on docs_only!='true'. The jobs still RUN and report success in seconds, so the two required contexts keep reporting — a docs-only PR stays mergeable (never an if:-skipped required job; Gitea 1.25.4 reports if-skip as 'skipped', verified with a throwaway probe PR). build skips its image steps on a docs-only push to main; tag builds force docs_only=false. Detection uses --no-renames so a code->docs rename can never be misclassified as docs-only. Refs #416	2026-07-17 19:44:25 +02:00
timothyandClaude Opus 4.8	bf15677dc6	fix(376): XML-escape access_token value in XMLTV guide output ... `GetChannelGuideHandler` interpolated `request.AccessToken` (HTTP-request- derived, from `?access_token=`) raw into the pre-built XMLTV cache fragments. A token containing `&`, `<`, `>`, or `"` would emit invalid XML and malform the entire guide. Escape it with `SecurityElement.Escape`, consistent with how #340 escaped `{RequestBase}`. The M3U path (`ChannelPlaylist.ToM3U`) also interpolates the token but M3U is not XML, so escaping there is neither needed nor correct — left unchanged. Regression test `Guide_xml_escapes_access_token` drives the real handler with a token containing all four XML-special chars and asserts the output is escaped (sibling to the #340 `Guide_xml_escapes_advertised_base_url` test). Verified non-vacuous: it fails with the escape reverted. fixes #376 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 19:04:42 +02:00
3 people	9752111240	docs(70): fix the last stale claim — found by reading every weight comment, not by grepping verbs ... Round-4 review returned BLOCKED, and its diagnosis is the sharpest of the four: my "re-derived by grep" claim was false. The reviewer's grep included `dropped`; mine had `drop(ped)? every` and missed the bare word — so MultiCollectionConfiguration.cs:38 ("a 0-weight source is dropped by the enumerator") never matched. That is the same failure as the previous round, one level up: I replaced a list of SITES with a list of VERBS I guessed and called it a class. Fourth consecutive incomplete completeness claim, and again the code twin of a doc line I had already corrected. The site is now accurate. The DB default of 1 is still right, but for the reason decisions.md already gives: a 0 backfill means nothing on a share-of-airtime scale. The enumerator clamps such a row to the floor, so it rotates rather than vanishing -- the drop it cited cannot happen since EffectiveWeight replaced the Weight > 0 filter, and A_Non_Positive_Weight_Does_Not_Delete_The_Source pins that. Method changed, not just the text: instead of guessing which verb a stale claim might use, enumerate EVERY comment/doc line mentioning weight across the diff and read them. 41 lines, one false. Both automated passes then produced false positives on different subjects -- a line-based check flagged EffectiveWeight's "must not drop..." (the framing verb wraps to the previous line), and a sentence-aware one flagged ReplacePlaylistItemsHandler's "the item is dropped from the playlist" (true, and about unknown ORDERS, not weights: PlaylistEnumerator really has 0 default arms and gates on `enumerator is not null` at :210). Both were read and cleared rather than counted, which is the only reason this message can say what it says. Verification: Build 0 errors; Core.Tests 566; ErsatzTV.Tests 1673; 0 failed; no BOM on any touched non-generated .cs; decisions-guard exit 0. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	caaae4cd00	docs(70): re-derive the stale-claim fix by grep instead of working the review's list ... Round-3 review returned BLOCKED: must-fix (b) was not closed. It was right, and the root cause it named is the point of this commit — the previous correction "was scoped to the four sites the reviewer listed rather than re-derived by grep". Fixing the list is not fixing the class. That is the same failure as B1, where the gate covered the two writers already in hand and missed CreateChannelFromLineup. Re-grepped the behavior class instead. Three survivors, two of them missed and one freshly introduced by the correction itself: - CreateMultiCollectionHandler.cs — the create twin of a comment whose UPDATE twin I corrected and whose create twin I never opened. Present tense, and contradicted by two tests in this same PR. - decisions.md — corrected one line in that file and left its sibling. - MultiCollectionItemWeight.cs (and its decisions.md mirror) — the ceiling rationale still claimed unbounded weights overflow the sum. They cannot: EffectiveWeight clamps before every sum and CycleLength widens to long. The earlier pass pattern-matched on the word "filtered" and left the identical defect on the ceiling. The ceiling's real job is the floor's argument — a billion is not a share of airtime any more than 0 is — so it now says that, and credits the clamp with the arithmetic safety it actually provides. Also corrected the writer claim to the right predicate: not "two persisting writers" (Add*ToPlaylist and Trakt persist it too, hardcoded) but two writers that persist a CALLER-SUPPLIED order. The full set is now classified persists-caller-value / persists-hardcoded / in-memory, including Engine/PlaylistHelper, which the previous "two Preview handlers" phrasing missed. That bullet has been wrong three times in the same shape; it now says so, since a lesson that keeps being re-learned is worth recording as a pattern rather than a fact. The BOM check caught this commit re-adding a BOM to the one file patched with utf-8-sig — the same trap, an hour after writing it down. Stripped; the mechanical pre-push check is what makes that survivable. Core.Tests 566, ErsatzTV.Tests 1673, 0 failed. Format verify exit 0. decisions.md +90/-0 (append-only guard green). Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	7446ce0293	test(70): classic-weighted golden — weight beats collection size end-to-end ... The unit tests pin the enumerator's sequence in isolation; this pins that the real PlayoutBuilder actually distributes by weight, through the whole chain: MultiCollection -> MediaCollectionRepository -> CollectionWithItems.Weight -> ShuffleSourceBuilder -> WeightedShuffleCollectionEnumerator -> PlayoutItems. The fixture is deliberately lopsided: the HEAVY source (weight 3) is the SMALL collection (2 items) and the LIGHT source (weight 1) is the LARGE one (4). Over the pinned 2-day window that yields 81 : 27 = exactly 3.00 : 1 — the smaller collection taking the larger share, which is the entire point of the feature and is not reachable by any existing order: Shuffle is size-proportional (~2:4 here) and ShuffleInOrder plays every item exactly once per cycle. Longest consecutive same-source run is 3, so it interleaves smoothly rather than draining blocks. Same determinism contract as Classic_shuffle: pinned Playout.Seed + Continue mode (Reset randomizes the seed). Channel number/GUID 5 — every golden fixture shares one in-memory DB, so those must be globally unique. The net is armed, not decorative: with the golden absent the test fails (missing golden is a hard failure by design), and regenerating added only classic-weighted — the three existing goldens are byte-identical, so nothing was silently re-baselined. Core.Tests 566 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	0295a8a9f6	style(70): strip UTF-8 BOM from the .cs files this PR touches ... CI's Formatting job failed: 19 touched files carried a BOM, which .editorconfig forbids (charset=utf-8). Pure encoding change — one byte per file, no semantic diff (verified: every hunk is `-namespace` -> `+namespace`). Self-inflicted. The patches that edited these legacy files wrote them back as utf-8-sig to "preserve the existing style", but the #311 fix-as-you-touch gate requires a file to be normalized when you touch it — that is the whole point of scoping the gate to changed files instead of reformatting the ~2500 legacy BOM files at once. dotnet format leaves the EF-generated Designer/snapshot files alone as generated code, and its verify skips them the same way, so they stay as ef emitted them. Two corrections to what I believed going in: - `dotnet format --include` does NOT no-op here. It reported `error CHARSET` for each file and exit 2, reproducing CI exactly, and fixed them in place. The note claiming otherwise is wrong for this invocation. - My first BOM check reported all files clean. The od pattern was wrong; reading the first three bytes directly found 19. A detector that can only say "ok" is worse than no detector. Core.Tests 565, ErsatzTV.Tests 1673, Architecture.Tests 5 — all passed. API artifacts still in sync. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	ccef0ca88a	fix(70): test the weight gate; correct rationale my own clamp made false ... Re-review of the fix commit returned MERGEABLE-WITH-NITS. It verified the gate is now complete by enumerating the writers itself (no fourth persisting writer) and proved B2's fix works by writing throwaway handler tests — which was also its point: the fix shipped with none. B2 was create and update silently DISAGREEING on the same input, and the fix re-established agreement with nothing pinning it. Both paths are now driven from one shared case list, plus an explicit test that create and update agree on every case — the per-path tests would both have passed while the two diverged, which is how the bug existed in the first place. Non-vacuity proven: inverting only the update path's validation fails 10 of 20 on a clean build (0 errors, so not a stale-dll pass), and the agreement test is among the failures. The rest is my own prose contradicting my own code. The commit that added EffectiveWeight removed the weight filter, then left four statements asserting a 0-weight source "is filtered out" — two of them authored by that same commit, including the stated justification for Minimum=1 in MultiCollectionItemWeight. A future agent could have read that and deleted the clamp or the floor as redundant; they are belt-and-braces and neither is. Corrected to describe what the code now does: the gate refuses input that means nothing on a share-of-airtime scale, the clamp protects rows predating the gate. Also corrected the writer count in the very bullet whose lesson is "grep every writer of the field": ReplaceBlockItems writes BlockItem.PlaybackOrder, not PlaylistItem.PlaybackOrder. There are TWO persisting writers of PlaylistItem's, and the correction itself had miscounted by conflating the two fields — so the lesson now says to grep each field separately. Core.Tests 565 passed, ErsatzTV.Tests 1673 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	c0da414a4c	fix(70): close the review blockers — third playlist writer, weight bounds, overflow ... Adversarial review of PR #402 returned BLOCKED. It could not break the WRR math or the stateless-restore claim (it probed restore across wraps at indices 12/13/20/37 — all held, and the clamp preserves a 1000:1 ratio exactly). What it broke was the perimeter. B1 — the validation gate had a hole, so the silent-drop bug shipped. CreateChannelFromLineup is a THIRD writer of PlaylistItem.PlaybackOrder; its own guard only covered MultiCollection entries, so a 2+ entry lineup of plain collections persisted WeightedShuffle straight through to PlaylistEnumerator's null-drop. My decisions.md claim that "the silent sites never see it" was false as written — corrected in place, with the lesson recorded: grep every writer of the field, the non-obvious composite handler is the one that gets missed. The Add*ToPlaylist handlers are safe only because they hardcode their order. B2 — Weight had no validation at all, and create/update disagreed on the same input. EF's HasDefaultValue(1) substitutes 1 for a 0 on INSERT (0 reads as "not set") but an UPDATE writes the 0 through — and a 0-weight source was filtered out of the rotation, deleting it from the channel silently. Exactly the failure this order is careful to avoid everywhere else. Now bounded 1..1000 by a shared MultiCollectionItemWeight used by both paths so they cannot drift, and clamped again in the enumerator for rows that predate the gate. B3 — Sum(weights) is checked arithmetic, so two int.MaxValue weights threw OverflowException from inside a playout build. Reachable through the API precisely because of B2. The ceiling fixes both; the sum also widens to long. M1 the lineup mirror now allows WeightedShuffle for multi collections, matching the PlayoutModeMustBeValid change it claims to mirror. M3 ScheduleAsGroup is documented as deliberately unread by this order. L1 MinimumDuration is computed over every source instead of the current rotation — under the clamp a rotation is a strict subset and is rebuilt each wrap, so caching over it went stale. L2 the retry guard keys off the rotation, not the raw collection count. N1 the tautological default test is gone: it built entities in C#, so it asserted the property initializer, not the migration — it could not have failed. Replaced with clamp, overflow, and cross-wrap restore cases (the property the review proved but found unpinned). H1 the two follow-ups the PR body claimed were "filed" did not exist. Now filed: #403 (silent dispatch-fallback hardening) and #404 (SPA weight UI, blocked-by #388). Core.Tests 565 passed, ErsatzTV.Tests 1643 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	953481c177	docs(70): record the WeightedShuffle design and correct the ShuffleInOrder misreading ... decisions.md entry covers why one enum value rather than two or a separate setting, why the weight lives on both multi-collection join entities with a DB default of 1, why it can only be applied on the ShuffleInOrder-shaped path (source identity is destroyed on the Shuffle path), why it is stateless, and why cross-engine exposure is closed by write-path validation instead of by changing five shipped fallbacks. The load-bearing part is the ShuffleInOrder clarification. Its balanced-shuffle padding reads as equalization, but the spacers emit nothing — one cycle plays every item once, so airtime stays proportional to collection size. It is anti-clumping, not fair-share. That was misread during this issue's own design pass and would have collapsed #70 to "already shipped"; the distinction is the whole justification for the feature, so it is now written down in both decisions.md and the domain-model glossary rather than left to be rediscovered. domain-model.md gains a Playback order row contrasting Shuffle / ShuffleInOrder / WeightedShuffle, and notes on the multi-collection row that the two join entities are mirrors. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	ef9bba8a52	feat(70): accept per-source weight on the multi-collection API ... Without this the weight is only reachable by editing the database, so the enumerator has nothing to distribute by. Weight is threaded through create and update (all four handler branches: add and update, plain and smart) and defaults to 1, so it is optional on the wire and /api/v1 stays additive under the freeze. It is returned on the read path too, which is load-bearing rather than symmetry: the update replaces the item list, so a client that GETs, edits a name, and PUTs back would silently reset every weight to the default if the GET didn't carry it. Weight edits ride the existing MultiCollection Version token, so If-Match/412 concurrency needs no new design. Regenerated v1.json + v1.d.ts + endpoint-index via update-openapi.sh and generate:api (never hand-edited). The spec picks up weight on both request and response models and WeightedShuffle in the PlaybackOrder enum; weight is emitted optional. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	dd7c155b39	test(70): prove per-source Weight reaches the scheduler for both member kinds ... A multi collection holds plain collections and smart collections through two separate join entities that build CollectionWithItems at separate call sites. Only the plain one is obvious, and forgetting the smart mirror un-weights smart-collection members with nothing reporting it — so it gets a test rather than a comment. Three tests: both weights survive the round trip; an unweighted row reads back as 1, not 0 (a 0-weight source is filtered out by the enumerator, so a 0 default would silently drop every pre-existing member from a weighted rotation); and the real MediaCollectionRepository hands the weight to the scheduler — the mapping tests alone would pass even if the repo dropped it on the floor. ErsatzTV.Tests: 1641 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	f394c721a8	feat(70): refuse WeightedShuffle where it isn't implemented, report where it can't be ... WeightedShuffle is implemented for classic schedule items only, and every other engine mishandles an order it doesn't know *silently*: PlaylistEnumerator has no default arm so the item is dropped from the playlist; BlockPlayoutBuilder filters block items against an allow-list and `continue`s past the rest; YAML and Scripted return None, which their callers' foreach reads as "no content". A weighted order degrading to unweighted random or to nothing is the worst failure mode here, because the output is supposed to look arbitrary — nobody would notice. Rather than change those shipped fallbacks (a real defect class, but pre-existing and wider than this feature — filed separately, non-goal here), this closes the new exposure at the write path: ReplacePlaylistItems and ReplaceBlockItems reject WeightedShuffle with an error naming where it is available. If it can't be persisted where it isn't handled, the silent sites never see it. Defence in depth for the two engines that address orders by name: YAML and Scripted now log a warning when a parsed order falls through unhandled, so an empty schedule explains itself. Enum.Parse accepts "weightedShuffle" the moment the value exists, so the gate above can't cover them. EnumeratorForContent becomes an instance method to reach the logger. ProgramScheduleItemCommandBase lists WeightedShuffle explicitly as valid for multi collections — it already passed by falling through the switch, and implicit-by- omission is how this subsystem grew its silent paths. Core.Tests: 558 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	07cb8c0287	test(70): pin WeightedShuffle semantics; fix unbounded reshuffle retry on wrap ... Ten tests pin the distribution contract exactly rather than statistically, because the sequence IS the product decision: 3:1 emits A A B A (spread, not drained); equal weights air a 2-item source as often as a 20-item one; ties break to the earliest source; a defaulted weight behaves as fair-share (guarding the migration default); restoring at an index equals advancing to it (the stateless contract that lets CollectionEnumeratorState carry this order). Fixes a hang found by the non-vacuity control. When a rotation wraps, MoveNext retried the rebuild to avoid an immediate repeat. ShuffleInOrder can do that unbounded because its reshuffle randomizes the lead item — but this order's lead is decided by weight, so the heaviest source always leads, and when it holds a single item the lead never changes and the retry never terminates. Two single-item collections with unequal weights would wedge the playout build. The retry is now bounded: avoiding a back-to-back repeat is a nicety, not terminating is not. Regression test walks several wraps under a timeout. Non-vacuity proven per repo lore: inverting the real WRR pick (max -> min, never if(true), which trips CS0219 under warnings-as-errors and silently serves a stale dll to --no-build) failed 5 of 10 tests on a clean build (0 errors, so not a stale-dll false pass). Reverted; 10/10 green. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
3 people	68be319403	feat(70): WeightedShuffle playback order — weighted / fair-share distribution ... Adds PlaybackOrder.WeightedShuffle = 9 (Classic engine only) plus the per-source weight it distributes by. Why a new order rather than making ShuffleInOrder weight-aware: ShuffleInOrder's balanced shuffle pads sources to equal length with Option.None spacers, and spacers emit nothing — so one cycle plays every item exactly once and airtime stays proportional to collection size. It prevents *clumping*, not *domination*. Retrofitting weights onto it would silently change shipped users' output. WeightedShuffleCollectionEnumerator picks a source by smooth weighted round-robin (acc += weight; richest wins; pays the total), then takes that source's next item. Weights 3:1 emit A A B A; ties break to the earliest source in list order. Fair-share is the equal-weights default, so one mechanism covers both behaviors in the issue. One rotation is sized so the source needing the most picks works through all its items at its share; smaller sources loop within it — that looping is what makes equal weights mean equal airtime regardless of library size. The sequence stays a pure function of (Seed, Index), so it restores by replay like its siblings and needs no per-source persisted counters. It consumes ShuffleSourceBuilder.GetCollectionItemsForShuffleInOrder unchanged — the schedule-entity-free entry point #380 reserved for this issue. Source identity is destroyed on the Shuffle path (MultiCollectionGrouper collapses to GroupedMediaItem + Distinct), so the weight is only reachable via CollectionWithItems on the ShuffleInOrder-shaped path. Weight is persisted on MultiCollectionItem AND MultiCollectionSmartItem — the mirror is mandatory; omitting the smart item silently un-weights smart-collection members. Both carry a DB default of 1: without it existing rows would migrate to 0, and a 0-weight source is dropped by the enumerator. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 17:00:46 +00:00
timothy	fe6e2722f8	docs(lore): batching, no "main checkout", trust the queue, killed≠failed ... Operator-requested after PR #405 pushed 5 times, orphaning live runs the operator had to cancel by hand. Batches every standing-lore correction this session produced into one commit (per the batching rule it adds). New HARD CONSTRAINTS: - BATCH PUSHES. Cancellation is impossible from the agent side on Gitea 1.25.4 — REST .../runs/{id}/cancel and MCP cancel_run both 404, and the web-UI route needs a session+CSRF that doesn't script. Only the operator can cancel, so an orphaned run holds a runner slot until it finishes. If you must supersede a live run, SAY SO. - TRUST THE GITEA BUILD QUEUE. Do not gate/throttle a push on host health; the runners were retuned for stability. Batch because you can't cancel what you orphan, not to protect the host. - BOM-CHECK touched .cs before pushing. The #311 gate is fix-as-you-touch, and it bit two sessions the same day (PR #405 ×6; #70/PR #402 ×19 via Python utf-8-sig writing BOMs back). Verify your detector — an od-based grep reported all-clean while 19 files were dirty. Corrects a claim I nearly published: `dotnet format --include` DOES work here. The apparent no-op was the SHELL — CI's mapfile is bash-only, zsh has no mapfile → empty array → zero files → exit 0. Run it under bash -c. THERE IS NO "main checkout" — the biggest correction here. /Users/timothy/ersatztv is a shared mutable working tree whose HEAD is whatever the last session left there. Its name lies, and it bit TWO sessions on 2026-07-17, both doing the obvious thing: one assumed main and committed onto the #604/CI session's branch 24s after that session's own commit; another read git log there and concluded main was "4 behind origin" — a phantom. Framed as a design flaw, not a discipline failure: "check git status first" appears to confirm the false assumption and then goes stale (it WAS on main at 12:46 and wasn't by 14:17). Worktree discipline itself is healthy — 10 feature worktrees. Diagnosing CI reds — three ways to misread one, all hit this session: - A KILLED job reports conclusion=failure, not cancelled. The tell is a log that stops mid-step with NO error and NO failure marker. A runner retune killed run 1006's migration + E2E on a BOM-only diff that couldn't break them. Log timestamps are UTC, host is UTC+2 — convert before correlating. - cancelled ≠ failure: a cancel is NO verdict, and a run marked failure may hold a genuine job failure from BEFORE the cancel. Monitors must count FAILED and CANCELLED separately. - "Unable to pull refs/heads/v4" is act refreshing its action cache and is followed by "Cloned …" — noise, not a cause. Grep the failure marker, not the word "error". An infra-shaped red (setup/cache step, before your code compiles) is not a code failure; don't file a CI bug off one sample. Also: the cheap selector's failure modes are wider than deps+priority — it also misses in-progress claim state and umbrella-vs-child. Docs-only.	2026-07-17 18:15:32 +02:00
timothy	1e35248edf	style(72): de-BOM the six touched .cs files (#311 formatting gate) ... CI's "Formatting (changed .cs conform to .editorconfig)" job failed with `error CHARSET: Fix file encoding` on all six pre-existing BOM'd files this PR touches. The #311 gate is fix-as-you-touch: any .cs a PR touches must conform to .editorconfig (charset=utf-8), and these carried BOMs inherited from upstream. BOM removal only — six files, one line each, zero content change. The three files already without a BOM (GetChannelByIdForApiHandler + both new/changed Tests files) needed nothing. Note for the next person: this cannot be validated locally on this Mac — `dotnet format --include` silently no-ops here, so the gate is only observable in CI. Check `head -c3 <file> | xxd -p` for `efbbbf` on every touched .cs before pushing instead of trusting a local format run. Refs #72	2026-07-17 17:51:43 +02:00
timothy	d92e717ebb	docs(72): fix the stale "Problems" filter name in domain-model (re-review) ... Re-review, Low: domain-model.md still documented a "Problems" filter — a line added by 50005b12 earlier in this same PR, so the PR contradicted itself once 38483681 renamed the filter to "No playout". Ironic in exactly the way the rename is about: the next person implementing the deferred fault classes would read the doc, see "Problems" documented, and restore the over-promising label that ChannelsScreen's own comment asks them not to. Stale docs re-introduce the bug the code comment defends against. decisions.md needed no change — it only ever named the badge, never the filter. Refs #72	2026-07-17 17:51:43 +02:00
timothy	7e9c52b035	fix(72): name the filter for the one fault it can prove (review) ... Adversarial review, Medium: the "Problems" filter over-promised. The badge is honestly named ("No playout"), but the filter claimed a taxonomy that does not exist — decisions.md enumerates three fault classes this deliberately does NOT compute (empty schedule behind a playout, broken source, origin). A user whose Classic schedule is empty would read "Problems 0" as "lineup healthy" and ship a dead channel. A false all-clear is worse than no affordance. Renamed the filter to "No playout" so badge and filter both name exactly the one fault the API can prove, leaving "Problems" free for when the taxonomy behind it actually exists. Rationale recorded at the predicate so the next person doesn't "improve" the label back. Also from the review: - The zero-playout test's comment claimed include coverage it does not provide (it passes with or without the include — 0 == 0). Re-stated as what it is: a mapper boundary check. Its two siblings are the include coverage. - UpdateChannelHandler:179 is a fourth call site the "shared by all three" framing excluded. Harmless (the controller discards the view model and re-projects through GetChannelByIdForApi), but a trap: its query lacks the MirrorSourceChannel include, so swapping in the shared helper would report 0 for a working mirror. Documented that the fix there is a QUERY change. - Mirrored the rename into the design-system prototype. The rename collided the badge and filter labels, so the screen tests now scope the badge assertion to the table (spa-conventions §6). This also fixed a real weakness: the mirror test's unscoped queryByText would have matched the filter button and asserted nothing. Review note: the reviewer's strongest hypothesis — that PUT and GET could disagree on a mirror channel's count — was investigated and does NOT hold. Refs #72	2026-07-17 17:51:43 +02:00
timothy	9b3121f0c2	docs(72): record the channel-health API shape + mirror the lineup fault state ... Same-PR docs obligations for #72: - decisions.md — why health is the raw `PlayoutCount` fact on the lean list DTO rather than a derived status enum or a new endpoint, and, more importantly, why empty-schedule / broken-source / auto-tuned-origin are deliberately NOT computed. The empty-schedule one is the trap worth writing down: it only understands Classic ProgramSchedule playouts, so a badge driven off it would silently lie on the other four schedule kinds. - domain-model.md — a glossary row for channel health, drawing the line against /channels/state's OnAir (runtime liveness, not "would play if tuned"). - design-system — mirror the "No playout" badge + Problems filter into the Channels prototype so design-system/ stays a faithful mirror of prod (design-sync #388). Nature Docs carries playouts: 0 so the fault state is visible in the canvas. Refs #72	2026-07-17 17:51:43 +02:00
timothy	6cd5471500	feat(72): flag channels that will never play in the lineup ... Surfaces the per-channel playout count the previous commit put on the API as the fault state #72 calls "the important one": a channel with no playout can never play, and until now the lineup gave no sign of it. - "No playout" renders as a Badge with visible text rather than another single-letter marker like D/H. D/H flag states the user chose deliberately; this is a fault they need to notice and act on, and a tooltip-only glyph is invisible on touch and easy to miss in a long lineup. - A "Problems" segmented filter answers the issue's actual goal — seeing the health of a LARGE lineup at a glance — by collapsing it to just the faults. - `willNeverPlay` is one predicate shared by the badge, the count and the filter, so the three cannot disagree. Mirror channels relay their source's playouts and the server folds those into playoutCount, so a working mirror is not false-flagged (covered by a test). Deliberately not approximated here, per the design pass: an empty schedule behind an existing playout (EmptyScheduleHealthCheck only understands Classic ProgramSchedule playouts — a badge driven by it would silently lie on Block, Sequential, Scripted and ExternalJson channels) and broken/missing sources (FileNotFound/Unavailable are server-wide counts with no channel attribution). Both stay open on #72 rather than shipping a signal that is wrong for four of the five schedule kinds. The test fixture now sends playoutCount, matching the API, so the fault state is opt-in per test instead of depending on an absent field. Refs #72	2026-07-17 17:51:26 +02:00
timothy	1a40b4a8e6	fix(72): report a real per-channel playout count on the channels API ... The channels API could not answer "will this channel play?", which #72 needs to flag a broken channel in the lineup at a glance. Two defects, one root cause each: 1. `ChannelRepository.GetChannel` never included `Playouts`. The read is AsNoTracking with no lazy-loading proxies, so the navigation came back empty and `GetChannelByIdForApiHandler`'s `channel.Playouts?.Count ?? 0` could only ever evaluate to 0 — `GET /api/v1/channels/{id}` reported `playoutCount: 0` for every channel on the system. That silently disabled the channel editor's playout-source guard (ChannelEditScreen:820, gated on `playoutCount > 0`), so the "Cannot be changed once a generated channel has a playout" control was always live. The server still enforces the invariant (UpdateChannelHandler coerces Mirror back to Generated), so nothing was corrupted — but the user's change was silently discarded. That silent coercion is filed separately as #401. 2. The detail path counted only the channel's own playouts, never the mirror source's, so a working Mirror channel would read as "no playout" even once the include landed. Both call sites now share `Mapper.GetPlayoutsCount` (previously private to GetAllChannelsHandler), which handles the Mirror case. `ChannelResponseModel` gains `PlayoutCount` so the list — #72's actual surface — can render it; the count is free there, since `GetAll` already includes `Playouts` and `MirrorSourceChannel.Playouts` and simply discarded them. Tests run the real repository against a real context on purpose: a handler test with a substituted IChannelRepository populates `Playouts` itself, so it passes whether or not the query includes them. Proven non-vacuous — removing the include again turns the 2-playout and mirror cases red (0 CS errors, so no stale-dll false pass). Refs #72	2026-07-17 17:51:26 +02:00
3 people	6052626316	chore: widen the BOM-guard remedy to staged + dirty files (review nit) ... Cold review (MERGEABLE-WITH-NITS) flagged that the printed fix snippet and the docs by-hand check used `git diff origin/main...HEAD`, which excludes staged-but-uncommitted files. A brand-new staged .cs is exactly what fires the pre-commit deny yet is absent from that diff, so the copy-paste remedy would strip nothing → deny again → loop. (The deny still names the file, so manual stripping always worked; this just makes the printed remedy match the guard's own detection set: branch diff ∪ staged ∪ dirty.) Verified on a fresh fixture: a brand-new staged BOM file is denied, and the widened snippet strips it. Left as accepted (fail-open, Medium, CI backstops; shared with sibling hooks): `git -C <path>` isn't parsed for the target tree (only `cd` is — matches the `cd <wt> && git` usage here), and the heredoc-body false-deny vector that bash-guard/worktree-guard also carry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 15:50:01 +00:00
3 people	139bdaff5a	chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs ... The #311 fix-as-you-touch rule is already enforced by a pre-commit hook and a blocking CI job, and both missed the same mistake twice on 2026-07-17: PR #405 shipped 6 BOM'd files, PR #402 shipped 19. The gap is that the pre-commit hook is routinely skipped — worktree hook friction makes `git commit --no-verify` the normal path here — which leaves CI, a ~10-minute round trip, as the first thing that tells you. This guard fires on Bash before git runs, so --no-verify can't skip it. It denies `git commit`/`git push` when a .cs the branch touches still carries a BOM, names the files, and gives the strip command. Why a hook rather than a note: a memory describing this exact trap did not prevent either failure — the PR #402 session re-added a BOM an hour after writing that memory down, because the usual ways of touching a legacy file re-add it silently (Python io.open(..., encoding='utf-8-sig') WRITES a BOM back; sed/perl round-trips keep it). A check that runs beats one you have to remember. Follows the existing gate pattern: a fail-open script in .claude/hooks/ registered on the PreToolUse/Bash matcher alongside bash-guard and worktree-guard. Any parse or lookup trouble → allow; this must never be the reason a commit can't happen, and CI remains the backstop. Generated *.Designer.cs / TvContextModelSnapshot.cs are exempt, matching what dotnet format itself skips. Scoped to this repo, and it resolves the target tree from an in-command `cd` because commits here run as `cd <worktree> && git ...` and the harness resets the shell cwd between calls. Verified against a throwaway fixture: denies commit and push for a BOM'd touched file (naming it); allows a clean tree, a BOM in generated files only, `git status` with a BOM present, and `echo 'run git push later'` (no false-trip on the words). Proved live via a sentinel — it fires on the Bash matcher — sentinel removed. docs/contributing.md updated: three enforcement layers, why the third exists, the utf-8-sig re-add trap, a by-hand check, and the bash-not-zsh caveat for `dotnet format --include` (mapfile is bash-only; under zsh the file list is empty and the tool looks like it silently did nothing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 15:50:01 +00:00
timothyandClaude Opus 4.8	469d725559	ci(406): apply the memory-swap rule to our own two sites; stop leaning on the peak reading [decisions-edit] ... Re-review of the fix commits came back MERGEABLE with three findings worth acting on. MEDIUM -- the PR documented a standing rule ("--memory without --memory-swap silently grants 2x in swap") and then didn't apply it to the two sites this repo owns: docker-build.yml's smoke container and scripts/migration-smoke.sh, both `--memory 2g` with no --memory-swap. Pre-existing rather than a regression, but a rule you don't follow in your own repo isn't a rule. The migration-smoke one matters most: it runs on the PROD host in the release path, so a runaway migration should die against its cap rather than quietly swap out the box serving media. LOW -- and this is the important one: the docs leaned "peak 8305 MiB is probably mostly reclaimable cache". An independent probe (full solution build, same CI image, shared compilation off) measured peak 9457 MiB / anon 7134 MiB / file 421 MiB. ANON DOMINATED. Having verified the *mechanism* (peak overstates because it counts page cache), I guessed the *magnitude* in the direction I preferred -- the exact failure this entry criticises, committed inside the entry criticising it. Corrected in ci-cd.md, decisions.md and on server-management#604 (where the previous comment could have led to an unsafe 6g cap). Consequences now recorded honestly: a 6g cap looks UNSAFE, #570's "6g proved too tight" is the rule not an outlier, and #406's premise ("if this brings peak RSS well under 6 GiB the whole budget loosens") is looking DEAD -- the 7134 MiB anon was measured with shared compilation already off. The switches remain right; the looser budget they were meant to buy does not follow. NIT -- dropped the unverified claim that this also disables the Razor build server. The UseRazorBuildServer -> UseSharedCompilation fallback is .NET 5-era; Razor has been an in-process source generator since .NET 6, so there is likely no separate server to disable on .NET 10. Unverified, zero impact, so it has no business in a doc arguing for measurement over assumption. [decisions-edit]: the touched docs/decisions.md lines were added by this PR's own earlier commits, not settled entries on main -- net vs origin/main remains a pure insertion (0 deletions, verified). Also the sanctioned reason: the entry was factually wrong (see LOW). Verified: both workflows parse; migration-smoke.sh passes bash -n; the parsed mysql option string is `--memory=2g --memory-swap=2g --cpus=2`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 16:13:39 +02:00
timothyandClaude Opus 4.8	7859510c78	ci(406): report anon/file split — memory.peak counts page cache, not just RSS [decisions-edit] ... The peak-memory step from the previous commit worked on its first real CI run and immediately produced a number that would have led #604 to the WRONG decision: peak container memory: 8305 MiB (8708448256 bytes, from /sys/fs/cgroup/memory.peak) Read naively that says "the test job needs 8.1 GiB, so the 10g cap must stay and #406's 'the budget loosens' premise is dead". That reading is probably wrong, because `memory.peak` is the high-water mark of `memory.current`, which charges PAGE CACHE to the cgroup as well as anonymous memory. It is not peak RSS, and a build job does heavy NuGet/npm/obj/bin/coverage I/O. Proven on bumblebee -- a container with ZERO anonymous memory that merely reads an 800MB file: memory.peak : 826 MiB anon : 0 MiB file : 800 MiB This inverts the decision: page cache is RECLAIMED under a tighter cap, not OOM-killed, so a large peak that is mostly `file` is NOT evidence that the cap must stay high. `anon` is what forces an OOM. So the step now prints the anon/file split and says, in the log and the docs, to size caps on anon rather than peak. Honest about what this still isn't: the split is read at end-of-job, so it is the composition then, not at the peak instant -- a job peaking mid-`dotnet test` and freeing after would under-report anon. Sampling a true peak-anon is ersatztv#412, filed with the options. Also recorded there and in ci-cd.md: there is NO pre-change baseline from this instrument (7.8 GB VBCSCompiler was measured host-wide across concurrent jobs, not inside one job container), so #406's premise is neither confirmed nor killed yet. What IS established: no persistent compiler server survives a build, and migrations is green with mysql capped at 2g with swap disabled. The irony is the point, and it is now written into decisions.md: this repo's CI perf work keeps stating numbers from plausibility rather than measurement (#390's "2-4min" apt-ffmpeg estimate; real 110s). Measuring the wrong quantity precisely is the same failure wearing a lab coat -- and it bit this very change, whose whole purpose was to replace an inherited number with a measured one. Verified: shellcheck clean; the real step body run inside a --memory=2g container under `bash -e -o pipefail` reads memory.peak, prints the split, writes the step summary, exits 0; and it degrades to a skip + exit 0 where no cgroup file exists. [decisions-edit]: the modified docs/decisions.md lines are ones THIS PR added in its own earlier commits, not a settled entry on main -- net vs origin/main is still a pure insertion (0 deletions, verified). It is also the sanctioned reason: the entry as written was factually wrong about what memory.peak measures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 15:53:24 +02:00
timothyandClaude Opus 4.8	04e3a5249d	ci(406): disable swap on the capped mysql; fix peak-step placement and failure mode [decisions-edit] ... Review fixes from the cold adversarial pass on PR #411. Verdict was MERGEABLE, but one HIGH is a genuine defect worth the round-trip. [decisions-edit] rationale: the 12 "modified" lines in docs/decisions.md are lines THIS PR added in its own previous commit (e33205e8), not a settled entry on main. The hook diffs against HEAD; net against origin/main this file is still a pure insertion (65 added, 0 deleted -- verified, and CI's decisions-guard diffs against the PR base). It is also the sanctioned reason regardless: the entry as first written contained a factual error (see HIGH below). HIGH -- `--memory=2g` alone silently grants 2g of SWAP as well. Docker defaults an unset --memory-swap to 2x --memory. Verified live on bumblebee: `--memory=2g` alone gives memory.max=2147483648 AND memory.swap.max=2147483648; adding `--memory-swap=2g` gives memory.swap.max=0. Capping RAM while permitting swap is close to the worst outcome on the one host whose swap thrash is the entire reason for the cap -- and a swapping mysqld mid-DDL is exactly the pathology behind the known `Command Timeout expired` migrations flake, so the naive cap could have made that flake worse rather than better. The comment promising "if this ever OOM-kills the service" described a failure mode that would not have happened first: it would have swapped, silently. Now --memory-swap=2g (swap off); prefer a loud OOM over silent degradation. The same 2x applies to the runners' 10g job slots (10 GiB RAM + 10 GiB swap each), which is a plausible direct mechanism for the incident's 21 GiB swapped. That is #604's boundary -- reporting it there. MEDIUM -- `if: always()` does not make a step advisory. It controls whether the step RUNS, not whether its failure fails the job, and defaults.run.shell: bash means -e is on, so a failed cat/redirect in an informational step could redden a green test job. Use continue-on-error: true, matching the neighbouring Coverage summary step. MEDIUM -- the peak step must run LAST. memory.peak read at step N reports the peak only up to N, so sitting before Coverage summary excluded reportgenerator's memory -- under- reporting the very number #604 sizes caps on. Moved after it. LOW -- dependency-scan.yml gets the env too; workflow env does not cross workflow files, and that one runs unattended on a cron against the prod media host. NIT -- Dockerfile ENV moved below the openapi-generator wget: an ENV invalidates every layer under it, and that ~30MB download is deliberately ordered early to stay cached (#190). Nothing between it and restore compiles, so this placement is free. NIT -- docs no longer imply this shrinks `format`. dotnet format loads Roslyn in-process via MSBuildWorkspace and never spawns csc, so its 3.95 GiB is untouched; sizing the small lane on the assumption it shrank would be a trap. Also record honestly that 543 MiB is init+idle rather than the 787-migration replay (so 2g is a measured floor + headroom, not a measured ceiling -- the migrations job going green is what validates it) and that --cpus=2 is judgement with no measurement behind it. The entry criticises dressing estimates as measurements; it should not do that itself. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 15:36:18 +02:00
timothyandClaude Opus 4.8	e33205e8eb	ci(406): disable persistent compiler servers, cap the mysql service, report peak RSS ... Refs #406, server-management#604. Items 1 and 2 of #406 (item 3 landed in PR #407). Root cause of the 2026-07-17 bumblebee incident (load 340, 21 GiB swapped, prod media down until reboot) is CI-induced memory pressure, and the largest single consumer was Roslyn's persistent VBCSCompiler at 7.8 GB RSS. It is a compiler *server*: it outlives the build that started it to keep its heap warm for the next one. In CI there is no next build -- each job container is torn down at the end of the run -- so it is pure cost, and it is the actual reason each job needed a 10 GiB cap. 1. Disable the compiler servers via the workflow's top-level env (UseSharedCompilation, DOTNET_CLI_USE_MSBUILD_SERVER, MSBUILDDISABLENODEREUSE). MSBuild surfaces env vars as properties and only defaults UseSharedCompilation when empty, so the env var wins and every dotnet call is covered without touching each call site. Also set as ENV in the Dockerfile's SDK stage: the workflow env reaches runner-side jobs only, and the `build` job compiles inside `docker build` where it does not propagate -- that is the job server-management#570 measured pegging 5.999/6 GiB, so it is the one that most needs this. Build stage only; the final image is FROM runtime-base, so nothing lands in the shipped image. Verified locally: a default build leaves 1 VBCSCompiler alive, the same build under these vars leaves 0, and ErsatzTV.sln still builds clean (0 errors). Trade-off: csc runs per project instead of via the server, costing some build time. 2. Cap the mysql service (--memory=2g --cpus=2). A runner's container.options applies to the job container ONLY: a live migrations job reported HostConfig.Memory=10737418240 for the job and mem=0 nanocpus=0 for its mysql service, i.e. unbounded. 2g is sized on measurement -- mysql:8.4 with this exact env peaked at 543 MiB during init, 481 MiB idle (probed on bumblebee) -- not on inheritance. Also add a `Report peak container memory` step to the test job (cgroup memory.peak, always(), never fails a build). server-management#604 sizes both runners' caps on that number and it was inherited rather than measured; now every run reports it. Docs: ci-cd.md gains the compiler-server + services-cap sections and the lane table is corrected (PR #407 reverted the #390 lane move without updating it). decisions.md records all three, incl. the standing rule that a services: container never inherits a cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 15:22:31 +02:00
timothy	71708633f5	test(264): pin the path-scan-error log; correct testing.md accuracy ... Re-review of the previous fix commit (986ccfaf, verdict MERGEABLE) raised one fair hit and one claim that did not survive checking. Fair hit — that commit's message asserted "LogError is an extension method that NSubstitute can't cleanly verify" as the reason for adding no test. That is FALSE, and this repo disproves it: ErsatzTV.Tests' ReleaseMemoryHandlerTests.ShouldHaveLogged asserts on ILogger via ReceivedCalls(), which works precisely because the LogError extension dispatches to the substituted ILogger.Log. Reusing that idiom here costs 4 lines, so pin the log the previous commit added. Verified non-vacuous: asserting a message the handler never logs fails the test. Not applied — the same review called the row's "1471 tests" misleading on the grounds that TranscodingTests is [Explicit] and contributes most of that count. TranscodingTests is indeed [Explicit], but filtering it out yields exactly 1471, so 1471 is already the runnable count and the row was accurate. Kept the number; documented TranscodingTests as [Explicit]/opt-in instead, since it was a genuine omission from a table that claims to be authoritative. Also rewrap the verification-gate paragraph the previous commit left over-long, and name the two always-run projects instead of "both". Deferred (filed separately): a canceled local scan now logs at ERROR per path via this log. It mirrors the remote handlers exactly, so diverging here would be the inconsistency, not the fix.	2026-07-17 14:35:09 +02:00
timothy	986ccfaf6c	fix(264): log local path scan errors (review finding) + map Scanner.Tests ... Adversarial review (MERGEABLE, no blockers) raised two items worth folding in rather than deferring: Medium — ScanLocalLibraryHandler silently swallowed path scan errors, while the three remote scanners it mirrors all log result.LeftToSeq(). That mattered less when a failed path only skipped the path-level LastScan, but the previous commit makes a failed path suppress the library-level scan time too — so the user would see exactly the #264 symptom ("Never scanned") with nothing in the log explaining why. That is a diagnosis dead-end of the same class as the bug being fixed, so log it here rather than file a follow-up. No test: the sibling Synchronize*LibraryByIdHandlerTests don't assert on logging either, and LogError is an extension method that NSubstitute can't cleanly verify. Low — docs/testing.md bills itself as the authoritative map of what each test project covers but omitted ErsatzTV.Scanner.Tests entirely (1471 pre-existing tests). This PR adds a file to that project, so add the row and include it in the per-PR verification gate.	2026-07-17 14:27:50 +02:00
timothy	5c8dd64acf	fix(264): record library-level LastScan after a successful local scan ... Local libraries permanently showed "Never scanned" in the SPA libraries hub regardless of successful scans, even as item counts updated. Root cause: ScanLocalLibraryHandler wrote only the path-level LibraryPath.LastScan (which gates the per-path refresh interval) and never the library-level Library.LastScan. The read API (GetAllMediaSourcesForApiHandler) populates the hub's scan-time badge from Library.LastScan, so that value stayed null forever. The three remote scanners (Jellyfin/Emby/Plex) already set the library-level value; only the local scanner did not. Both sides predate #202 — the SPA hub merely made the missing value visible. Mirror the remote scanners' semantics: record the library-level scan time only when the scan actually ran and every path that ran succeeded, so a skipped (unforced, interval not elapsed) or partially-failed scan does not claim a successful scan time. Also de-BOM the touched handler per the fix-as-you-touch charset gate (#311). fixes #264	2026-07-17 14:18:14 +02:00
timothyandClaude Opus 4.8	17d65909b6	ci: move api-docs and format back to ubuntu-latest (refs #406) ... #390 moved these two onto the `small` lane to dodge a ~29 min queue in the contended `ubuntu-latest` lane. The queue was real, but the lane was the wrong place to fix it, and #390's own rationale flagged why: Caveat: on an API-touching PR this job does a full `dotnet build`, so it is not always a "small" job; capacity 4 absorbs that. "Capacity 4 absorbs that" held only because nothing enforced the SUM of the runners' per-job memory caps. Each job container is correctly capped at --memory=10g, but 6 slots x 10g = 60 GiB sat on a 25 GiB host that also runs prod media. On 2026-07-17 it went to load 713 with 21 GiB swapped, and telegraf could no longer get scheduled to report it. server-management#604 fixes that at the source rather than by shuffling lanes: the `ubuntu-latest` lane now has 5 slots (ci-runner grown to 48 GiB at capacity 4, plus a bumblebee overflow slot) against this repo's 5 ubuntu-latest jobs, and the `small` lane is sized for genuinely-tiny jobs. So the queue #390 was dodging is gone, and these two dotnet jobs go back where they can actually be given memory. Note this does NOT by itself shrink the `small` lane's caps: `build` (Build & push image) still runs there and is the very job #570 measured pegging 5.999/6 GiB, so that lane keeps a 10g-capable slot either way. The real unlock is disabling Roslyn's persistent VBCSCompiler in CI (#406) — 7.8 GB RSS observed — which would let every cap drop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 14:17:28 +02:00
timothyandClaude Opus 4.8	1be45fbe88	fix(384): guard out-of-range axis (L1 review) + tidy page clamp ... Cold adversarial review found a crafted numeric `?axis=5` binds past [ApiController]'s auto-400 (ModelState valid), then AutoTuneAxisMap.GenerateQuery's `_ => throw` surfaces as a 500 (no global exception filter). Short-circuit an undefined axis to an empty result in the handler — matching #69's EnumerateAxis `_ => []` graceful-empty pattern. Adds a regression test asserting no search runs. Also simplifies the redundant pageSize lower clamp (review N4): the `<= 0 ? 100` guard already floors it, so `Math.Clamp(_, 1, 200)` -> `Math.Min(_, 200)`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 09:40:44 +02:00
timothyandClaude Opus 4.8	0009607a09	feat(384): auto-tune DetailPanel content-source member read endpoint ... GET /api/v1/channels/auto-tune/members lists the distinct content sources a proposed auto-tune channel's server-generated SmartCollection resolves to — parent shows for the TV axes (ItemCount = query-matching episodes), movies for the movie-genre axis — reusing the existing PagedLibraryBrowseItemsResponseModel (no new schema). The handler runs the server-owned AutoTuneAxisMap.GenerateQuery through ISearchIndex (client never sends Lucene, per #69 PR1) and rolls matching leaf items up to their distinct sources, mirroring GetSmartCollectionItems so the DetailPanel preview matches what the built playout will contain. Backend child of #383 (Auto-Tune DetailPanel milestone); read-only, cold review acceptable. Handler + controller tests (9 new). OpenAPI + endpoint-index regenerated; d.ts unchanged (reuses existing schema). Docs: api-conventions §5, decisions.md 2026-07-17, domain-model. fixes #384 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 09:40:44 +02:00
timothy	ed52892d68	ci(390): bump pin to ersatztv-ci:07048b8; compare full shas in pin guard ... Pin bump for the image rebuilt from the review-fix commit (the two-step working as designed — the new ci-image-pin guard demanded it). Also fixes a real bug in that guard, caught locally before CI: it compared abbreviated shas, but git auto-scales abbreviation length with object count, so the tag ci-image.yml builds from a fetch-depth:1 shallow clone is 7 chars while %h on the guard's full clone is 8 — it would have failed on every run. Now resolves both sides to full shas via rev-parse, which also catches a pin that names no commit at all. Verified non-vacuous locally: PASS on the real pin, FAIL on a stale pin, FAIL on an unresolvable one. Refs #390	2026-07-17 09:37:05 +02:00
timothy	07048b8c96	ci(390): address cold review — pin-drift guard, honest cron, doc fixes ... Cold adversarial review findings: - MEDIUM (Renovate generates pin drift): add a blocking ci-image-pin job. Renovate manages docker/ci/Dockerfile's base pins but cannot bump an opaque :<sha> in container.image, so a base bump would publish a new image, test the OLD one, and merge with the Dockerfile disagreeing with the pin. The guard fails when the pin isn't the last commit to touch docker/ci, or when the 5 jobs pin different tags — making the documented two-step enforced rather than remembered. - MEDIUM (cron was a no-op): the weekly rebuild updated nothing (jobs pin :<sha>) and buildcache would have restored the apt layer verbatim, collecting none of the base updates it existed for. Added no-cache on the schedule path and rewrote the comment to state what it actually is: a build canary + a fresh :latest for the next bump. - LOW: FFMPEG_TAG was referenced in the Dockerfile + docs but never existed (the FROM is hardcoded); reworded both. - LOW: paths: filtered the exact file while the docs claimed docker/ci/** — use **. - NIT: docs oversold ENTRYPOINT reset as a gotcha; act overrides it anyway. Marked defensive. Refs #390	2026-07-17 09:34:46 +02:00
timothy	84756eebab	ci(390): default run steps to bash — act_runner uses sh inside container: ... The migrations job died in 0.13s with 'set: Illegal option -o pipefail'. Root cause: inside a container: act_runner does not default to bash, it falls back to 'sh -e {0}' (dash), since it cannot assume bash exists in an arbitrary image. Every multi-line script in this workflow is bash (set -o pipefail, arrays, shopt, mapfile), so dash fails them immediately. The format job passed only because it already declared shell: bash explicitly. Declare it once at the workflow level instead of per step; non-container jobs already defaulted to bash, so nothing changes for them. Documented in docs/ci-cd.md as a container gotcha — it surfaces only once a job is containerized and does not look like a shell problem. Refs #390	2026-07-17 09:26:43 +02:00
timothyandClaude Opus 4.8	d38181cba4	docs(lore): verify the cheap selector's deps + priority, not just the winner ... The #73 selector packet reported #385/#386 as deps:clear when both were blocked, and missed #383 (the only priority:medium in tier 1) by ranking on issue number. Either error alone produces a bad pickup, and the kickoff's winner-only recheck cannot catch a wrong winner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 09:23:51 +02:00
timothyandClaude Opus 4.8	62c15da490	docs(73): document seasonal / date-conditional scheduling (already implemented) ... fixes #73 #73's premise ("ErsatzTV has no native date-conditional scheduling today") is false. The predicate (IAlternateScheduleItem) is shipped on both engines, evaluated by AlternateScheduleSelector, reachable from the SPA, and already unit-tested — so the only real gap was discoverability, and it was a docs gap: the mechanism was documented as a mechanism, never as a task. - channels.md: task-shaped "Recipe: seasonal / holiday programming" for both engines, plus the gotchas — chiefly that blank StartYear/EndYear is what makes a range repeat every year (AlternateScheduleSelector.cs:32-40), that the override branch needs BOTH years, and that explicit years disable wrap-around detection. - domain-model.md: glossary row (mirrors the #77 "already exists" row). - decisions.md: entry recording the verdict and the rejected asks — notably that "prioritize collection X during a date range" is #70's weighting problem, not a second primitive built here while #70 is mid-flight. No production code changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-17 09:20:05 +02:00
timothy	afa42b0b6f	ci(390): run toolchain jobs in the shared CI image; drop 110s apt-ffmpeg; rebalance lanes ... Converts test/migrations/functional-e2e/api-docs/format to run inside ersatztv-ci:6a9e27c via container:, removing per-run setup-dotnet, setup-node, apt-get ffmpeg and dotnet tool install. NuGet/npm caches stay (project deps); setup-node's implicit npm cache is replaced with an explicit actions/cache. Rolled-up wins found while auditing every job's steps (evidence in #390): - Drop the apt-ffmpeg step (110s/run). It was never load-bearing: the app's only startup ffmpeg touch is a LogWarning from FFmpegLocatorService, and the harness drives curl-only contracts that never transcode. The image ships prod-identical ffmpeg 8.1.2 anyway, which #299's seeded-media follow-ups will need. - Bake dotnet-ef + reportgenerator into the image (2 per-run tool installs gone; versions now live in one tracked place). - Move api-docs (5s) and format (37s) to the idle 'small' lane — they were each queueing ~29min in the saturated ubuntu-latest lane. Also drops that lane from 5 jobs to 3. Only possible because container: makes them self-contained. Measured, not assumed: the image itself is worth only ~15-40s/job. The dominant costs are queue wait (server-management#604) and 742s of redundant compilation (#398) — both filed rather than silently bundled here. Refs #390	2026-07-17 09:16:04 +02:00
timothy	6a9e27c6b8	ci(390): fix CI-image smoke test — reportgenerator --version exits 1 ... The build-time smoke test failed the image build even though the toolchain was healthy: reportgenerator prints its version banner then exits 1 ("No report files specified"), so it has no clean version probe. Probe the dotnet-tool shim on PATH instead. dotnet --info confirmed the SDK + .NET 10.0.10 runtimes resolve correctly on the ffmpeg base (ICU/glibc fine). Refs #390	2026-07-17 09:13:09 +02:00