timothyandClaude Opus 5
b5dee26202
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 8s
PR Gates / Docs update reminder (pull_request) Successful in 13s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 19s
review-verdict/h10 Review-verdict: MERGEABLE @ b5dee26 (base: main)
PR Gates / decisions lifecycle (pull_request) Successful in 20s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 15s
Review verdict / Set review-verdict status (pull_request_target) Successful in 6s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 4m32s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m44s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m18s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m53s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 6s
Both search indexers opened UpdateSong with
metadata.AlbumArtists ??= [];
metadata.Artists ??= [];
Artists/AlbumArtists hold the whole list in ONE COLUMN rather than
being navigations. So unlike the same `??= []` idiom on
Genres/Tags/Artwork all around them, the property IS the column value:
assigning it on a TRACKED entity flips the entry to Modified and the
next SaveChanges writes [] over a NULL column. This is the mechanism an
adversarial review demonstrated in #691, which is why that issue's
entity-level guard was reverted in favour of guarding at the read site.
Measured rather than reasoned about, per the issue's first done-when
box. Restoring ONLY the `??= []` clause (the real predecessor lines,
not a hand-written mutant) reddens the new fixture on
`metadata.Artists should be null but was []`; a probe variant with the
first two assertions replaced by prints reports STATE=Modified and the
raw column moving from NULL to "[]". Today's two feeds are both
AsNoTracking (SearchRepository.GetItemToIndex and GetAllSongs), so no
shipped caller loses data -- but that is a property of two callers, not
of the indexer, and #691 already recorded it as a loaded gun. The
fixture pins the indexer's own contract instead.
Removing the assignment is not sufficient alone: it was load-bearing
for the four reads below it, and deleting it by itself converts a
silent write into a live throw on every untagged song. Measured by
deleting only those two lines from the real predecessor file:
NullReferenceException, thrown at the foreach (cited by symbol: a line
number in a mutant that exists in no committed tree is unreproducible
by construction). The
exception type follows the read FORM, not the field -- foreach yields
NRE, string.Join/ToList yield ArgumentNullException -- and this PR
contains two of each, which is why no single exception-name grep
characterises the class. So each site moves together with its reads:
- LuceneSearchIndex.UpdateSong / ElasticSearchIndex.UpdateSong: hoist
Optional(...).Flatten().ToList() locals and read those.
- RefreshChannelDataHandler: the Scriban context took the raw nullable
lists (the issue's second item). The shipped _song.sbntxt only does
array.join, but a custom template is free to do anything.
The population was derived from the MODEL rather than from the issue's
file list, and the obvious derivation is wrong: "the IList<string>
properties under ErsatzTV.Core/Domain" returns two of eight. It misses
the six value-converted collections (ProgramScheduleAlternate and
PlayoutTemplate each carrying DaysOfMonth, MonthsOfYear, DaysOfWeek),
declared as plain ICollection<T> and made single columns only in
Data/Configurations -- and their storage differs (comma-separated text
for the int converter, JSON for the enum one), so the shared property
is "one scalar column", not the serialization. No site applies `??=`
to any of the six, so this defect has no instance there; whether a null
can REACH one at runtime is unverified and is filed as #823 rather than
asserted either way. Only the SongMetadata pair is left NULL in
practice, by FallbackMetadataProvider. Every site touching either field
was then swept; the remaining readers were already guarded by #691.
The fixture carries two anti-vacuity guards, both witnessed:
- A POSITIVE CONTROL (`writer.NumDocs.ShouldBe(1)`). Every other
assertion says something did NOT happen, so all of them hold
vacuously if UpdateSong never runs -- and it silently stops running
if a future refactor gates UpdateItems on `_initialized`, which this
fixture bypasses by injecting the writer. Verified BOTH directions:
with that gate added the control fails `NumDocs should be 1 but was
0`, and with the control removed the whole test PASSES while the code
under test is unreachable.
- A capturing logger, because UpdateSong wraps its body in a catch that
assigns metadata.Song = null -- severing a required relationship and
cascading the metadata to Deleted. Without it the probe silently
measures the error path; on the first run it did exactly that (a bare
ILanguageCodeService substitute NPEs inside AddLanguages). The raw
column helper also fails loudly on a missing row, since ExecuteScalar
returns CLR null for both "NULL column" and "no such row".
ElasticSearchIndex has no equivalent fixture -- it needs a stubbed
transport -- so its change is by inspection against the Lucene one, and
the gap is filed as #824 rather than covered by a source-text guard.
The whitespace-only churn in ElasticSearchIndex.cs is the #311
fix-as-you-touch format gate: it scopes to whole changed FILES.
`git diff -w` over that file shows only the two hunks above.
Local gate: ErsatzTV.Tests 2006 passed / 4 pre-existing skips,
Core.Tests 685/1 skip, Infrastructure.Tests 114, Architecture.Tests 7,
Scanner.Tests 1504 -- 0 failures in each. scripts/tests 874 passed / 2
skipped. dotnet format whitespace --verify-no-changes clean on the four
touched files, no BOM on any. decisions_validate OK.
Fixes #701
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>