fix(779): stop narrating mechanisms — the third causal story was wrong too

Re-review of the fix commit found that my correction repeated the very defect
it was correcting. The commit message said "Measured, not assumed" about a
mechanism I had not measured:

  "replacing the forwarded r.Id with null — which sends the handler down the
   create-new-rows path — leaves this test green, because a fresh database
   reassigns the same ids once the old rows are deleted"

The observation (green) was right; both halves of the explanation were wrong.
An all-null payload gives requestIds.Count == 0, which takes the POSITIONAL
FALLBACK, not the create-new-rows path — and that branch does
`built.Id = existing.Id` and SetValues, so no row is deleted and no id is
reassigned. No fresh database is involved. Read from
ReplaceProgramScheduleItemsHandler.cs:79-100 this time, not inferred.

The conclusion survives and is stronger than written: BOTH branches preserve ids
for a same-order, same-subtype payload, so no id-less mutation of this fixture
can redden the Id comparison. What separates the branches is a REORDERING
payload, which is what ReplaceProgramScheduleItemsReconcileTests pins in
Reorder_ById_Should_Move_State_With_The_Logical_Item_Not_The_Slot and
Insert_ById_In_Middle_Should_Keep_Existing_Ids_And_State — the previously cited
"in-place: id preserved" assertion sits in a test whose payload has no ids, so
it runs the positional path too and was the wrong citation.

Three drafts of one paragraph each supplied a confident causal story for a
correct observation and two were wrong, so the record now says the rule that
generalises: record the measurement and the code path you actually read; do not
narrate why the system behaves that way unless that was measured too.

Also from the re-review:
  - the BlockItemId counterexample no longer asserts which branch is taken. Ids
    can be REUSED after delete-and-reinsert, in which case the lookup resolves to
    a DIFFERENT block item rather than to nothing — worse than the "loses its
    collection type and name" previously claimed. Which occurs was not measured,
    so the record claims only that the reference is orphaned
  - the GuideGroup collision is marked a HAZARD, not a demonstrated consequence:
    unmeasured, and narrowed by the projector grouping only contiguous runs that
    also share GuideStart/GuideFinish
  - detector F's own proof is now stated honestly in its record: flipping the
    default reddens ONLY the absent-cell test, so the true/True/TRUE cases are
    complete coverage but are not independently load-bearing against it. A
    complete matrix is not one in which every cell discriminates
  - elementwise comparison holds for records OR value types (two of the four
    collection properties are List<int>)
  - the floor-versus-pin wording notes it is strictly stronger than, not in
    conflict with, testing.guard-derives-population-from-source
  - "must still exist on the record" now names ScheduleItemResponseModel

Empty exemption set with a live stale-exemption loop is kept deliberately: it is
a universally-quantified invariant over a legitimately empty set, not a narrowed
population, and it becomes live with the first exemption.

Refs #779

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ErsatzTV.Tests/Application/ProgramSchedules/ScheduleItemResponseRoundTripTests.cs

@@ -354,21 +354,26 @@ public class ScheduleItemResponseRoundTripTests
     //
     // Properties deliberately NOT compared. The set is EMPTY, and that is a finding rather than an
     // oversight. The first version exempted Id on the reasoning that "the PUT replaces the item
-    // set, so B's rows are new rows with new ids". That is not what this endpoint does:
-    // ToReplaceCommand forwards r.Id, so ReplaceProgramScheduleItemsHandler takes its id-based
-    // reconcile path and updates each row IN PLACE. So Id compares equal and the exemption was
+    // set, so B's rows are new rows with new ids". That is not what this endpoint does: both of
+    // ReplaceProgramScheduleItemsHandler's branches reuse the existing row for a same-order,
+    // same-subtype payload — the id-based reconcile and the positional fallback each do
+    // `built.Id = existing.Id` and SetValues. So Id compares equal and the exemption was
     // unnecessary, resting on a rationale that was the opposite of the code.
     //
-    // What comparing Id does NOT do, measured rather than assumed: it does not pin the #252
-    // id-preservation invariant. Replacing `r.Id` with `null` in ToReplaceCommand — which sends the
-    // handler down the create-new-rows path — leaves this test GREEN, because a fresh database
-    // reassigns the same ids after the old rows are deleted. The assertion is true and cheap, not
-    // discriminating. The test that actually pins in-place reconcile is
-    // ReplaceProgramScheduleItemsReconcileTests ("in-place: id preserved"); do not read this one as
-    // a second copy of it.
+    // What comparing Id does NOT do, and this is stated narrowly on purpose because two earlier
+    // versions of this comment asserted a MECHANISM that the code contradicted. Measured: replacing
+    // `r.Id` with `null` in ToReplaceCommand leaves this test GREEN. Read from the handler: an
+    // all-null payload gives `requestIds.Count == 0` and so takes the positional fallback, which
+    // also preserves ids in place. Both branches therefore agree on Id for this fixture, so NO
+    // id-less mutation of it can redden the Id comparison — the assertion is true and cheap, not
+    // discriminating. What separates the branches is a payload that REORDERS, which is what
+    // ReplaceProgramScheduleItemsReconcileTests covers in
+    // Reorder_ById_Should_Move_State_With_The_Logical_Item_Not_The_Slot and
+    // Insert_ById_In_Middle_Should_Keep_Existing_Ids_And_State. Those are the tests that pin
+    // in-place reconcile; do not read this one as a second copy of them.
     //
-    // Any name added here must still exist on the record (asserted below), so renaming a field
-    // cannot leave a stale exemption silently exempting nothing.
+    // Any name added here must still exist on ScheduleItemResponseModel (asserted below), so
+    // renaming a field cannot leave a stale exemption silently exempting nothing.
     private static readonly System.Collections.Generic.HashSet<string> RoundTripExemptProperties =
         new(StringComparer.Ordinal);
 
@@ -402,8 +407,8 @@ public class ScheduleItemResponseRoundTripTests
                 // Collection-valued members (WatermarkIds, Watermarks, GraphicsElementIds,
                 // GraphicsElements). The elementwise walk still delegates to each element's Equals,
                 // so it is value equality only because those elements are records
-                // (NamedIdResponseModel); a future non-record element type would silently be
-                // compared by REFERENCE inside this loop. It is also order-sensitive, which is
+                // (NamedIdResponseModel) or value types (the int id lists); a future element type that
+                // is neither would silently be compared by REFERENCE inside this loop. It is also order-sensitive, which is
                 // correct for these ordered lists but would be wrong for an unordered type such as
                 // a dictionary-valued property.
                 actual.ShouldNotBeNull($"{property.Name} was null on the round-tripped item");

docs/decisions/records/testing/deny-path-at-production-config-value.md

@@ -44,6 +44,17 @@ Ask not only "which value did the test use" but "which code did the value flow t
 `ResolveKey` returns before touching the live config volume; that is a test-isolation detail and
 deliberately not their subject.
 
+**What the mutation proof here does and does not show, because the honest version is weaker than
+the headline.** Flipping the shipped default `true` -> `false` reddens
+`Read_Gating_Is_Required_When_The_Setting_Is_Absent` and only that test. That establishes the
+*absent* cell is load-bearing. It also shows the three explicitly-configured `true`/`True`/`TRUE`
+cases — the cells this rule is named for — are NOT independently load-bearing against that mutation:
+a mistyped configuration key is caught, but by the `false` cases, not the `true` ones. The only
+mutation that would redden `"True"`/`"TRUE"` alone is a hand-rolled case-sensitive parse replacing
+`GetValue`. They are kept as behaviour coverage of the real binder rather than deleted, and the
+distinction is written down instead of being smoothed over: a matrix that is *complete* is not
+thereby a matrix in which every cell *discriminates*.
+
 **Why no check enforces this.** Deciding whether a test exercised a production value means reading
 test source and judging intent — a string-matching predicate over code, which
 `docs/defect-shapes-773.md` §4 argues against and which this repo has withdrawn twice after

docs/decisions/records/testing/full-replace-asserts-field-list.md

@@ -41,17 +41,30 @@ contradicted the code — the failure this record is about, committed inside the
 Two further details. Every exempted name is asserted to still *exist* on the record, because a stale
 exemption exempts nothing while reading in review as a considered decision. And the anti-vacuity
 check is a **pin** (`ShouldBe(55)`), not a `>=` floor: a floor lets properties vanish silently, which
-is the one-sided version of the both-directions rule this record invokes. Comparing against the
+is the one-sided version of the both-directions rule this record invokes. (`
+testing.guard-derives-population-from-source` endorses a floor on its canonical example; this is
+strictly stronger, not a contradiction of it.) Comparing against the
 reflected count minus exemptions would be tautological — both sides come from the same reflection —
 so the number is written down and must be bumped deliberately. It was obtained by raising the pin and
 reading the failure, not counted off the source.
 
-**What comparing `Id` does NOT do**, stated because the plausible version of this sentence is wrong.
-It is tempting to say it now pins #252's id-preservation invariant. It does not, and the check is
-cheap: replacing the forwarded `r.Id` with `null` — which sends the handler down the create-new-rows
-path — leaves this test **green**, because a fresh database reassigns the same ids once the old rows
-are deleted. The assertion is true and free, not discriminating.
-`ReplaceProgramScheduleItemsReconcileTests` is what pins in-place reconcile.
+**What comparing `Id` does NOT do**, stated narrowly because two earlier drafts of this paragraph
+asserted a MECHANISM the code contradicted — inside the record whose subject is exactly that. What
+was measured: replacing the forwarded `r.Id` with `null` leaves the round-trip test **green**. What
+the handler does, read rather than inferred: an all-null payload gives `requestIds.Count == 0` and
+takes the *positional fallback*, which — like the id-based reconcile — reuses the existing row
+(`built.Id = existing.Id`). Both branches preserve ids for a same-order, same-subtype payload, so no
+id-less mutation of that fixture can redden the `Id` comparison. It is true and cheap, not
+discriminating. What separates the branches is a payload that REORDERS, which is what
+`ReplaceProgramScheduleItemsReconcileTests` pins in
+`Reorder_ById_Should_Move_State_With_The_Logical_Item_Not_The_Slot` and
+`Insert_ById_In_Middle_Should_Keep_Existing_Ids_And_State`.
+
+The generalisable part is not the `Id` detail. It is that three successive drafts here each supplied
+a confident causal story for a correct observation, and two of them were wrong. **Record the
+measurement and the code path you actually read; do not narrate why the system behaves that way
+unless that too was measured.**
+
 
 **A completeness guarantee on the COMPARER is not one on the FIXTURE.** The seed builder feeding this
 round trip is still a hand-written list. A new nullable field is now reflected and compared — as
@@ -69,13 +82,17 @@ item ids. That is false, and the counterexamples live in the same subsystem the
 
 - `PlayoutItem.SchedulingContext` is a persisted column holding a serialized
   `BlockSchedulingContext(BlockId, **BlockItemId**, Enumerator, Seed, Index)`.
-  `ReplaceBlockItemsHandler` renumbers `BlockItem.Id`, after which
-  `ProcessSchedulingContextHandler` resolves nothing and falls back to
-  `new BlockContextBlockItem(id, null, null)` — so the troubleshooting view for an already-built
-  playout item loses its collection type and name.
+  `ReplaceBlockItemsHandler` deletes and reinserts, so the stored `BlockItemId` no longer refers to
+  the row it was written for. `ProcessSchedulingContextHandler` looks it up: it either resolves
+  nothing and falls back to `new BlockContextBlockItem(id, null, null)`, or — if the id was reused by
+  a different item — resolves to the WRONG one. Which of the two happens was not measured, so the
+  claim here is only that the reference is orphaned; the second case is the worse one and is the
+  reason not to write this off.
 - `PlayoutItem.GuideGroup` is set to `effectiveBlock.TemplateItemId` and persisted, while
-  `ReplaceTemplateItemsHandler` renumbers `TemplateItem.Id`. `GuideGroup` is the XMLTV grouping key,
-  so renumbered ids can collide with values already persisted on older items.
+  `ReplaceTemplateItemsHandler` deletes and reinserts. `GuideGroup` is the XMLTV grouping key, so a
+  reused id colliding with one already persisted on older items is a HAZARD rather than a
+  demonstrated consequence — unmeasured, and narrowed by the projector grouping only contiguous runs
+  that also share `GuideStart`/`GuideFinish`.
 
 The accurate statement is narrower: **no progression or enumerator state is keyed to those ids**, so
 the replace does not reset scheduling position — but two persisted *diagnostic/grouping* fields do