Commit Graph
3 Commits
Author SHA1 Message Date
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
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
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