fix(72): report a real per-channel playout count and flag channels that will never play #405

Merged

timothy merged 6 commits from feat/72-164-channel-health into main 2026-07-17 18:13:52 +02:00

Conversation 4 Commits 6 Files Changed 17

+327 -43

timothy commented 2026-07-17 13:01:03 +02:00

Owner

Copy Link

Copy Source

Fixes #72

Ships the fault signal #72 calls "the important one": a channel with no playout can never play, and the lineup gave no sign of it.

What landed

1. fix(72) — the channels API could not answer "will this channel play?"
   ChannelRepository.GetChannel never included Playouts. The read is AsNoTracking with no lazy-loading proxies, so 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. The detail path also never counted a Mirror channel's source playouts. Both call sites now share Mapper.GetPlayoutsCount (Mirror-aware), and ChannelResponseModel gains PlayoutCount.
2. feat(72) — "No playout" badge + a Problems segmented filter on the channels list.
3. docs(72) — decisions.md + domain-model.md + design-system mirror (#388).

Live user-facing consequence of (1)

The channel editor gates its Playout-source select on playoutCount > 0 ("Cannot be changed once a generated channel has a playout"). Because the count was always 0, that guard never engaged. The server still enforces the invariant (UpdateChannelHandler coerces Mirror→Generated), so nothing was corrupted — but the user's change was silently discarded. That silent coercion is filed as #401, deliberately not fixed here (it is an API-semantics change deserving its own review).

Why PlayoutCount is free

GetAll already Includes Playouts and MirrorSourceChannel.Playouts and was discarding them. No extra query, no N+1 on a large lineup — which matters, since #72 is explicitly about large lineups.

Design decisions (frontier pass) — see decisions.md 2026-07-17

• Lean list DTO, not a new endpoint, not /channels/state. Health is config-derived; /channels/state is the fast-poll runtime-liveness feed. Precedent: the IsLocked stamp on PlayoutListItemResponseModel (api-conventions §3a).
• A raw fact, not a status enum. The auto-tune arc (#383/#384) is about to churn the status taxonomy; freezing an enum on a frozen-additive /api/v1 guarantees a breaking rev.

Deliberately NOT computed (each would be wrong today, not merely absent)

• Empty schedule — EmptyScheduleHealthCheck only understands Classic ProgramSchedule playouts. Block/Sequential/Scripted/ExternalJson channels have none, so a badge driven off it would silently lie on four of the five schedule kinds.
• Broken source — FileNotFound/Unavailable are server-wide counts with no channel attribution.
• Auto-tuned vs user origin — no honest signal exists (ChannelPlayoutSource.Generated is a playout strategy; SPA-created blanks carry it too). Needs a column + dual-provider migration on the auto-tune arc's live surface. The "Channel Lineups" playlist-group join was rejected as a heuristic that breaks on edit.

Keeping these out held the PR to read-path only: no migration, no write-handler live-E2E.

Verification

• Backend 1641 passed / 0 failed; SPA 795 passed / 0 failed (92 files); lint clean; npm run build ok; npm run check:api exits 0.
• Negative control: the new repository tests were proven non-vacuous — removing the include again turns the 2-playout and mirror cases red (with 0 CS errors, so no stale-dll false pass, per the #250 trap).
• Tests run the real repository against a real context on purpose: a handler test with a substituted IChannelRepository populates Playouts itself and passes whether or not the query includes them. Only a repository-level read catches a missing include.

🤖 Generated with Claude Code

Fixes #72 Ships the fault signal #72 calls **"the important one"**: a channel with no playout can never play, and the lineup gave no sign of it. ## What landed 1. **`fix(72)` — the channels API could not answer "will this channel play?"** `ChannelRepository.GetChannel` never included `Playouts`. The read is `AsNoTracking` with no lazy-loading proxies, so `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*. The detail path also never counted a Mirror channel's source playouts. Both call sites now share `Mapper.GetPlayoutsCount` (Mirror-aware), and `ChannelResponseModel` gains `PlayoutCount`. 2. **`feat(72)`** — "No playout" badge + a **Problems** segmented filter on the channels list. 3. **`docs(72)`** — `decisions.md` + `domain-model.md` + design-system mirror (#388). ## Live user-facing consequence of (1) The channel editor gates its Playout-source select on `playoutCount > 0` ("Cannot be changed once a generated channel has a playout"). Because the count was always 0, **that guard never engaged**. The server still enforces the invariant (`UpdateChannelHandler` coerces Mirror→Generated), so nothing was corrupted — but the user's change was silently discarded. That *silent coercion* is filed as **#401**, deliberately not fixed here (it is an API-semantics change deserving its own review). ## Why `PlayoutCount` is free `GetAll` **already** `Include`s `Playouts` and `MirrorSourceChannel.Playouts` and was discarding them. No extra query, no N+1 on a large lineup — which matters, since #72 is explicitly about *large* lineups. ## Design decisions (frontier pass) — see `decisions.md` 2026-07-17 - **Lean list DTO, not a new endpoint, not `/channels/state`.** Health is config-derived; `/channels/state` is the fast-poll runtime-liveness feed. Precedent: the `IsLocked` stamp on `PlayoutListItemResponseModel` (api-conventions §3a). - **A raw fact, not a status enum.** The auto-tune arc (#383/#384) is about to churn the status taxonomy; freezing an enum on a frozen-additive `/api/v1` guarantees a breaking rev. ## Deliberately NOT computed (each would be wrong today, not merely absent) - **Empty schedule** — `EmptyScheduleHealthCheck` only understands **Classic** `ProgramSchedule` playouts. Block/Sequential/Scripted/ExternalJson channels have none, so a badge driven off it would silently lie on four of the five schedule kinds. - **Broken source** — `FileNotFound`/`Unavailable` are server-wide counts with no channel attribution. - **Auto-tuned vs user origin** — no honest signal exists (`ChannelPlayoutSource.Generated` is a playout *strategy*; SPA-created blanks carry it too). Needs a column + dual-provider migration on the auto-tune arc's live surface. The `"Channel Lineups"` playlist-group join was rejected as a heuristic that breaks on edit. Keeping these out held the PR to **read-path only**: no migration, no write-handler live-E2E. ## Verification - Backend **1641 passed / 0 failed**; SPA **795 passed / 0 failed** (92 files); lint clean; `npm run build` ok; `npm run check:api` exits 0. - **Negative control**: the new repository tests were proven non-vacuous — removing the include again turns the 2-playout and mirror cases **red** (with **0 CS errors**, so no stale-dll false pass, per the #250 trap). - Tests run the **real repository against a real context** on purpose: a handler test with a substituted `IChannelRepository` populates `Playouts` itself and passes whether or not the query includes them. Only a repository-level read catches a missing include. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy added 3 commits 2026-07-17 13:01:04 +02:00

fix(72): report a real per-channel playout count on the channels API ... 6d5791801d

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

feat(72): flag channels that will never play in the lineup ... 9add239d06

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

docs(72): record the channel-health API shape + mirror the lineup fault state ... 50005b12bd

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

timothy added 1 commit 2026-07-17 13:26:57 +02:00

fix(72): name the filter for the one fault it can prove (review) ... 38483681ce

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

timothy added 1 commit 2026-07-17 13:30:44 +02:00

docs(72): fix the stale "Problems" filter name in domain-model (re-review) ... b2ca92d30a

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

timothy commented 2026-07-17 13:31:22 +02:00

Author

Owner

Copy Link

Copy Source

Independent review — two passes, both cold, review-only

Pass 1 (full PR diff @ 50005b12): no Blockers, no Highs. The reviewer ran its own negative control (reverting the two .Include lines → 2 of 3 repo tests fail) rather than trusting the claim, and independently confirmed ChannelEditScreen.tsx:820's guard was genuinely dead code.

Attacks that failed (recording so they aren't re-litigated):

• LEFT JOIN safety — MirrorSourceChannelId is int? with .IsRequired(false)/OnDelete(SetNull) → optional nav → LEFT JOIN. Non-mirror channels are not filtered out; the identical include shape already ships on GetAll.
• Cartesian explosion — Startup.cs sets a global QuerySplittingBehavior.SplitQuery; extra round-trips, not an N×M blowup.
• Mirror chains / self-mirror — MirrorSourceMustBeValid requires the source be Generated, so chains are unreachable. Mirror with a NULL source (deleted → SetNull) → 0 → flagged, which is correct.
• PUT vs GET disagreeing on a mirror's count — my own strongest hypothesis, and it does not hold: ChannelController.cs:145-153 discards the view model and re-projects through GetChannelByIdForApi, so UpdateChannelHandler's non-mirror-aware count never reaches the wire.

Medium (fixed, 38483681) — the "Problems" filter over-promised. decisions.md records three fault classes deliberately not computed, so "Problems 0" would read as "lineup healthy" to a user with a dead empty-schedule channel. A false all-clear is worse than no affordance. Badge and filter now both say "No playout" — naming only the fault the API can prove.

Pass 2 (scoped re-review of the fix commit @ 38483681): MERGEABLE WITH NITS. It verified both claims in the new UpdateChannelHandler comment against source (controller re-projects ✓; that handler's query lacks the MirrorSourceChannel include, so the shared helper would return 0 for a mirror anyway ✓ — the prescription "fix the QUERY first" is correct). It also proved the new within(table) scoping non-vacuous by mutating willNeverPlay to return true and confirming both tests fail — so the mirror assertion genuinely asserts.

Its one Low (fixed, b2ca92d3): domain-model.md still documented a "Problems" filter — a line added earlier in this same PR, so the PR contradicted itself. Left alone, the next person would read the doc and restore the label the code comment asks them not to.

Verification @ b2ca92d3

• Backend 1641/1641; SPA 795/795 (92 files) across 5 consecutive runs; lint clean; npm run build ok; npm run check:api exit 0.
• Negative control on the include: removed → 2 red (0 CS errors, so no stale-dll false pass); restored → 3 green.

Stated review exemption for the head delta

b2ca92d3 is docs-only (one line in domain-model.md) on top of the independently re-reviewed 38483681. Per the skip rubric that is a docs leaf with no server-state effect, so it carries a stated exemption rather than a third cold pass. All code in this PR has had an independent pass.

Not claimed

• CI is not green yet — run 1001 was cancelled (not failed), so #72 has no completed CI verdict. Its EF migration integrity job did genuinely fail earlier at Setup .NET with an act-cache error, but the host was at load average 243 at the time; run 998 on main independently died on a buildx DeadlineExceeded in the same window. Both are host saturation, not this diff — no TvContext model change exists here for the migration job to test. Re-running at normal load.
• One SPA test file failed once in an early full run and could not be reproduced across 5 subsequent runs; unidentified, recorded rather than dismissed.

Review-verdict: MERGEABLE @ b2ca92d30a

## Independent review — two passes, both cold, review-only **Pass 1** (full PR diff @ `50005b12`): **no Blockers, no Highs.** The reviewer ran its own negative control (reverting the two `.Include` lines → 2 of 3 repo tests fail) rather than trusting the claim, and independently confirmed `ChannelEditScreen.tsx:820`'s guard was genuinely dead code. Attacks that **failed** (recording so they aren't re-litigated): - **LEFT JOIN safety** — `MirrorSourceChannelId` is `int?` with `.IsRequired(false)`/`OnDelete(SetNull)` → optional nav → LEFT JOIN. Non-mirror channels are not filtered out; the identical include shape already ships on `GetAll`. - **Cartesian explosion** — `Startup.cs` sets a global `QuerySplittingBehavior.SplitQuery`; extra round-trips, not an N×M blowup. - **Mirror chains / self-mirror** — `MirrorSourceMustBeValid` requires the source be `Generated`, so chains are unreachable. Mirror with a NULL source (deleted → SetNull) → 0 → flagged, which is correct. - **PUT vs GET disagreeing on a mirror's count** — *my own strongest hypothesis, and it does not hold*: `ChannelController.cs:145-153` discards the view model and re-projects through `GetChannelByIdForApi`, so `UpdateChannelHandler`'s non-mirror-aware count never reaches the wire. **Medium (fixed, `38483681`)** — the "Problems" filter over-promised. `decisions.md` records three fault classes deliberately **not** computed, so "Problems 0" would read as "lineup healthy" to a user with a dead empty-schedule channel. A false all-clear is worse than no affordance. Badge and filter now both say **"No playout"** — naming only the fault the API can prove. **Pass 2** (scoped re-review of the fix commit @ `38483681`): **MERGEABLE WITH NITS**. It verified both claims in the new `UpdateChannelHandler` comment against source (controller re-projects ✓; that handler's query lacks the `MirrorSourceChannel` include, so the shared helper would return 0 for a mirror anyway ✓ — the prescription "fix the QUERY first" is correct). It also proved the new `within(table)` scoping non-vacuous by mutating `willNeverPlay` to `return true` and confirming **both** tests fail — so the mirror assertion genuinely asserts. Its one **Low (fixed, `b2ca92d3`)**: `domain-model.md` still documented a "Problems" filter — a line added earlier *in this same PR*, so the PR contradicted itself. Left alone, the next person would read the doc and restore the label the code comment asks them not to. ## Verification @ `b2ca92d3` - Backend **1641/1641**; SPA **795/795** (92 files) across 5 consecutive runs; lint clean; `npm run build` ok; `npm run check:api` exit 0. - Negative control on the include: removed → 2 red (**0 CS errors**, so no stale-dll false pass); restored → 3 green. ## Stated review exemption for the head delta `b2ca92d3` is **docs-only** (one line in `domain-model.md`) on top of the independently re-reviewed `38483681`. Per the skip rubric that is a docs leaf with no server-state effect, so it carries a stated exemption rather than a third cold pass. All code in this PR has had an independent pass. ## Not claimed - **CI is not green yet** — run 1001 was **cancelled** (not failed), so #72 has no completed CI verdict. Its `EF migration integrity` job did genuinely fail earlier at `Setup .NET` with an act-cache error, but the host was at **load average 243** at the time; run 998 on main independently died on a buildx `DeadlineExceeded` in the same window. Both are host saturation, not this diff — no `TvContext` model change exists here for the migration job to test. Re-running at normal load. - One SPA test file failed once in an early full run and could not be reproduced across 5 subsequent runs; unidentified, recorded rather than dismissed. Review-verdict: MERGEABLE @ b2ca92d30ac2c5a610d94221b77766ac4638a68a

timothy added 1 commit 2026-07-17 14:06:50 +02:00

style(72): de-BOM the six touched .cs files (#311 formatting gate) ... 18818e163c

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

timothy referenced this pull request 2026-07-17 16:17:08 +02:00

chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs #413

timothy referenced this issue from a commit 2026-07-17 16:17:09 +02:00

chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs

timothy commented 2026-07-17 16:34:36 +02:00

Author

Owner

Copy Link

Copy Source

CI green + Done-when satisfied @ 18818e16

CI: clean green — 0 failed, 0 cancelled. Build & test ✅ · EF migration integrity ✅ · Functional E2E ✅ · API docs in sync ✅ · Formatting ✅ · decisions.md append-only ✅ · Docs reminder ✅ (Build & push image skipped — PR runs don't push images).

It took three attempts to get an uncontaminated run, none of them this diff's fault:

Attempt	Died at	Marker?	Cause
1	mid-dotnet build	none	runner retune killed in-flight jobs
2	.NET telemetry banner (setup)	none	NuGet cache step
3	—	—	green

Both earlier reds reported conclusion: failure while being kills — the tell is a log that stops mid-step with no ❌ Failure - Main marker. Worth recording: attempts 1–2 "failed" EF migration integrity and Functional E2E on a BOM-removal-only delta (6 files, 1 line each, zero content change) that cannot affect either — and both went green on the same unchanged sha once the environment settled. A diff that cannot cause a failure is evidence the failure isn't yours.

Deferred scope now filed (the last open Done-when box)

• #414 — auto-tuned-vs-user origin marker. Needs a Channel column + dual-provider migration, and provenance belongs to the auto-tune arc, so it is blocked-by #383 (dependency set + verified). The "Channel Lineups" playlist-group join is recorded as rejected, not forgotten.
• #415 — per-channel empty-schedule + broken-source attribution, carrying the trap that deferred them: EmptyScheduleHealthCheck only understands Classic playouts, so a badge driven off it would be silently wrong for four of the five schedule kinds.

Both are priority: low-labeled (H12 clean). The SPA filter stays "No playout" until #415 gives "Problems" a real taxonomy — willNeverPlay's comment says so at the call site.

Review trail

• Pass 1 (full diff): no Blockers, no Highs; reviewer ran its own negative control on the include and confirmed ChannelEditScreen's guard was genuinely dead code. Killed my own strongest hypothesis (PUT/GET disagreeing on a mirror count — the controller re-projects, so it never reaches the wire).
• Medium fixed (38483681): the "Problems" filter over-promised → renamed.
• Pass 2 (scoped re-review of the fix commit): MERGEABLE WITH NITS; verified both claims in the new UpdateChannelHandler comment against source, and proved the within(table) scoping non-vacuous by mutating willNeverPlay.
• Low fixed (b2ca92d3): domain-model.md still said "Problems", contradicting the PR's own rename.

Head delta since the reviewed commit — stated exemption

18818e16 is BOM removal only (6 files, 1 line each, zero content change) on top of the reviewed b2ca92d3, to satisfy the #311 fix-as-you-touch gate. No logic delta, so it carries a stated exemption rather than a third cold pass. All code in this PR has had an independent review.

Review-verdict: MERGEABLE @ 18818e163c

## CI green + Done-when satisfied @ `18818e16` **CI: clean green — 0 failed, 0 cancelled.** Build & test ✅ · EF migration integrity ✅ · Functional E2E ✅ · API docs in sync ✅ · Formatting ✅ · decisions.md append-only ✅ · Docs reminder ✅ (Build & push image `skipped` — PR runs don't push images). It took three attempts to get an uncontaminated run, none of them this diff's fault: | Attempt | Died at | Marker? | Cause | |---|---|---|---| | 1 | mid-`dotnet build` | none | runner retune killed in-flight jobs | | 2 | .NET telemetry banner (setup) | none | NuGet cache step | | 3 | — | — | **green** | Both earlier reds reported `conclusion: failure` while being **kills** — the tell is a log that stops mid-step with no `❌ Failure - Main` marker. Worth recording: attempts 1–2 "failed" `EF migration integrity` and `Functional E2E` on a **BOM-removal-only** delta (6 files, 1 line each, zero content change) that cannot affect either — and both went green on the same unchanged sha once the environment settled. A diff that *cannot* cause a failure is evidence the failure isn't yours. ## Deferred scope now filed (the last open Done-when box) - **#414** — auto-tuned-vs-user **origin** marker. Needs a `Channel` column + dual-provider migration, and provenance belongs to the auto-tune arc, so it is **blocked-by #383** (dependency set + verified). The `"Channel Lineups"` playlist-group join is recorded as **rejected**, not forgotten. - **#415** — per-channel **empty-schedule** + **broken-source** attribution, carrying the trap that deferred them: `EmptyScheduleHealthCheck` only understands **Classic** playouts, so a badge driven off it would be silently wrong for four of the five schedule kinds. Both are `priority: low`-labeled (H12 clean). The SPA filter stays **"No playout"** until #415 gives "Problems" a real taxonomy — `willNeverPlay`'s comment says so at the call site. ## Review trail - Pass 1 (full diff): **no Blockers, no Highs**; reviewer ran its own negative control on the include and confirmed `ChannelEditScreen`'s guard was genuinely dead code. Killed my own strongest hypothesis (PUT/GET disagreeing on a mirror count — the controller re-projects, so it never reaches the wire). - **Medium fixed** (`38483681`): the "Problems" filter over-promised → renamed. - Pass 2 (scoped re-review of the fix commit): **MERGEABLE WITH NITS**; verified both claims in the new `UpdateChannelHandler` comment against source, and proved the `within(table)` scoping non-vacuous by mutating `willNeverPlay`. - **Low fixed** (`b2ca92d3`): `domain-model.md` still said "Problems", contradicting the PR's own rename. ## Head delta since the reviewed commit — stated exemption `18818e16` is **BOM removal only** (6 files, 1 line each, zero content change) on top of the reviewed `b2ca92d3`, to satisfy the #311 fix-as-you-touch gate. No logic delta, so it carries a stated exemption rather than a third cold pass. All *code* in this PR has had an independent review. Review-verdict: MERGEABLE @ 18818e163c09001eb94993ee434d9f64bb311fb0

timothy force-pushed feat/72-164-channel-health from 18818e163c to 73f3690828 2026-07-17 16:53:58 +02:00 Compare

timothy commented 2026-07-17 17:32:41 +02:00

Author

Owner

Copy Link

Copy Source

Rebased onto main + re-verified @ 73f36908

The branch was 16 commits behind main (parallel-session merges), so it was rebased onto origin/main (not merged — per the rebase-don't-merge rule). CI is clean green on the rebased head: Build & test ✅ · EF migration integrity ✅ · Functional E2E ✅ · API docs in sync ✅ · Formatting ✅ · decisions.md append-only ✅ · CI image pin ✅.

Rebase resolution (mechanical, verified — no logic touched)

• decisions.md (append-only, CI-enforced): both new entries kept — #384's "Auto-Tune DetailPanel member list" and this PR's "Channel health on the API". The decisions.md append-only check passed.
• domain-model.md: #384 had extended the same "Channel concepts" row, so the resolution kept their updated row (with the auto-tune/members note) plus this PR's new "Channel health" row — not a naive keep-both that would have duplicated the row. Verified: 1 Channel-concepts row, 1 Channel-health row, #384's note intact, the "No playout" rename intact, no adjacent row dropped.
• Generated artifacts regenerated, not hand-merged (v1.json / v1.d.ts / endpoint-index.md): ./scripts/update-openapi.sh + npm run generate:api produced no diff vs git's auto-merge, so the text-merge was correct this time — but verified rather than trusted. Both /api/v1/channels/auto-tune/members (#384) and playoutCount (this PR) are present; 164→165 endpoints.

Re-verification @ 73f36908

BOM-clean · backend 1651/1651 · SPA 795/795 · lint clean · npm run build ok · npm run check:api exit 0.

Exemption for the head delta

The delta since the twice-reviewed code (b2ca92d3) is rebase + mechanical doc-conflict resolution + spec regen — zero logic change. Stated exemption rather than a third cold pass; all code in this PR has had two independent review passes (full diff + fix-commit re-review, both MERGEABLE).

Supersedes the prior Review-verdict @ 18818e16 (that sha was rebased away).

Review-verdict: MERGEABLE @ 73f36908283c6fdb3a1ec79c6f1b50c709d87fbb

## Rebased onto main + re-verified @ `73f36908` The branch was 16 commits behind main (parallel-session merges), so it was rebased onto `origin/main` (not merged — per the rebase-don't-merge rule). CI is **clean green** on the rebased head: Build & test ✅ · EF migration integrity ✅ · Functional E2E ✅ · API docs in sync ✅ · Formatting ✅ · decisions.md append-only ✅ · CI image pin ✅. ### Rebase resolution (mechanical, verified — no logic touched) - **`decisions.md`** (append-only, CI-enforced): both new entries kept — #384's "Auto-Tune DetailPanel member list" and this PR's "Channel health on the API". The `decisions.md append-only` check passed. - **`domain-model.md`**: #384 had extended the same "Channel concepts" row, so the resolution kept **their** updated row (with the `auto-tune/members` note) **plus** this PR's new "Channel health" row — not a naive keep-both that would have duplicated the row. Verified: 1 Channel-concepts row, 1 Channel-health row, #384's note intact, the "No playout" rename intact, no adjacent row dropped. - **Generated artifacts regenerated, not hand-merged** (`v1.json` / `v1.d.ts` / `endpoint-index.md`): `./scripts/update-openapi.sh` + `npm run generate:api` produced **no diff** vs git's auto-merge, so the text-merge was correct this time — but verified rather than trusted. Both `/api/v1/channels/auto-tune/members` (#384) and `playoutCount` (this PR) are present; 164→165 endpoints. ### Re-verification @ `73f36908` BOM-clean · backend **1651/1651** · SPA **795/795** · lint clean · `npm run build` ok · `npm run check:api` exit 0. ### Exemption for the head delta The delta since the twice-reviewed code (`b2ca92d3`) is **rebase + mechanical doc-conflict resolution + spec regen — zero logic change**. Stated exemption rather than a third cold pass; all *code* in this PR has had two independent review passes (full diff + fix-commit re-review, both MERGEABLE). Supersedes the prior `Review-verdict @ 18818e16` (that sha was rebased away). Review-verdict: MERGEABLE @ 73f36908283c6fdb3a1ec79c6f1b50c709d87fbb

timothy referenced this issue from a commit 2026-07-17 17:50:03 +02:00

chore: PreToolUse guard that blocks a commit/push with a BOM'd touched .cs

timothy force-pushed feat/72-164-channel-health from 73f3690828 to 1e35248edf 2026-07-17 17:53:20 +02:00 Compare

timothy commented 2026-07-17 18:13:33 +02:00

Author

Owner

Copy Link

Copy Source

Re-verdict @ 1e35248e (rebase over CI-config commits — code unchanged)

Head moved 73f36908 → 1e35248e by a second rebase onto main (parallel sessions kept merging). This rebase replayed my 6 commits over CI-config-only main commits (#406: Dockerfile, workflows, ci-cd.md, a BOM-guard hook) with zero overlap on my files.

Proof the code is identical to the twice-reviewed, CI-green 73f36908:

• git diff --stat 73f36908 1e35248e = only other sessions' files (.gitea/workflows/*, docker/Dockerfile, docs/ci-cd.md, docs/contributing.md, scripts/migration-smoke.sh, .claude/*). None of ErsatzTV.*/**, web/**, or the OpenAPI artifacts appear.
• git range-diff maps my 6 commits 1:1; only the docs commit changed, and only in its append-only decisions.md keep-both + the domain-model.md row resolution.
• Re-verified locally on 1e35248e: spec regen produced no diff, BOM-clean, dotnet build succeeds. Required CI checks (Build & test, EF migration integrity) are green on this head.

Stated exemption: the delta since the reviewed code is rebase + mechanical doc-conflict resolution, no logic change — so this is a re-verdict at the new head, not a third cold pass. All code has had two independent review passes.

Supersedes Review-verdict @ 73f36908 (rebased away).

Review-verdict: MERGEABLE @ 1e35248edf

## Re-verdict @ `1e35248e` (rebase over CI-config commits — code unchanged) Head moved `73f36908` → `1e35248e` by a second rebase onto main (parallel sessions kept merging). This rebase replayed my 6 commits over **CI-config-only** main commits (#406: Dockerfile, workflows, ci-cd.md, a BOM-guard hook) with **zero overlap on my files**. **Proof the code is identical to the twice-reviewed, CI-green `73f36908`:** - `git diff --stat 73f36908 1e35248e` = only *other sessions'* files (`.gitea/workflows/*`, `docker/Dockerfile`, `docs/ci-cd.md`, `docs/contributing.md`, `scripts/migration-smoke.sh`, `.claude/*`). **None** of `ErsatzTV.*/**`, `web/**`, or the OpenAPI artifacts appear. - `git range-diff` maps my 6 commits 1:1; only the docs commit changed, and only in its append-only `decisions.md` keep-both + the `domain-model.md` row resolution. - Re-verified locally on `1e35248e`: spec regen produced **no diff**, BOM-clean, `dotnet build` succeeds. Required CI checks (Build & test, EF migration integrity) are **green** on this head. **Stated exemption:** the delta since the reviewed code is rebase + mechanical doc-conflict resolution, no logic change — so this is a re-verdict at the new head, not a third cold pass. All *code* has had two independent review passes. Supersedes `Review-verdict @ 73f36908` (rebased away). Review-verdict: MERGEABLE @ 1e35248edf28fb25b8b8b259cdeeca6e7acb6d99

timothy scheduled this pull request to auto merge when all checks succeed 2026-07-17 18:13:49 +02:00

timothy merged commit ac7c978824 into main 2026-07-17 18:13:52 +02:00

timothy deleted branch feat/72-164-channel-health 2026-07-17 18:13:53 +02:00

timothy referenced this issue from a commit 2026-07-17 18:13:53 +02:00

Merge pull request 'fix(72): report a real per-channel playout count and flag channels that will never play' (#405) from feat/72-164-channel-health into main

timothy referenced this pull request 2026-07-17 18:14:43 +02:00

Channel lineup health at-a-glance (status states in the channels list) #72

timothy referenced this pull request 2026-07-17 18:15:16 +02:00

Health checks UX: audit usefulness + guided/automated remediation #164

timothy referenced this issue from a commit 2026-07-17 18:15:42 +02:00

docs(lore): batching, no "main checkout", trust the queue, killed≠failed

timothy referenced this pull request 2026-07-17 18:16:59 +02:00

ChicoryTV rewrite — queue tracker (goal, arc, session protocol) #237

timothy referenced this pull request 2026-07-17 18:34:25 +02:00

feat(70): weighted / fair-share content distribution (WeightedShuffle playback order) #402

timothy referenced this pull request 2026-07-17 20:53:58 +02:00

Health checks UX: audit usefulness + guided/automated remediation #164

timothy referenced this pull request 2026-08-13 21:59:10 +02:00

Mutation proofs for the 19 unproven guards, worst-consequence first #785

timothy referenced this pull request 2026-08-14 17:15:08 +02:00

pretooluse-bom-guard.sh is fail-open wherever xxd is not installed — including the CI runner #797

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#405