From 7ca058f83bd12ce739818e2f78622398f073fffe Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 27 Jul 2026 21:02:24 +0200 Subject: [PATCH] fix(668): review round 2 -- remove a second false comment, harden a vacuous assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review of the round-1 fix commit returned MERGEABLE with three findings, all about claims rather than behaviour. All three applied. 1. A stale FALSE parenthetical survived round 1. The docstring on Unicode_Fold_Agrees_With_The_Ordinal_Filter still claimed it catches "one that stops filtering the extras out". It does not. Mutation-verified: delete the Where in FilterSortTake and all EIGHT cases stay green, because each is either a positive SQL alone returns or an ASCII-query negative SQL alone rejects. The same mutation turns the new over-match test RED, so the pair does cover both directions -- but only the corrected wording says so. This is the same species of error round 1 fixed, one paragraph above it; swept by subject this time. 2. Unicode_Fold_Over_Match_Is_Discarded_By_The_Ordinal_Filter asserts an EMPTY result, so it passes vacuously if the fold never runs. Its premises are now asserted explicitly (the query is non-ASCII, and ToUpperInvariant maps ſ to S), so a fold that quietly stopped mapping ſ would fail rather than go green for the wrong reason. 3. The comment on IsSqlite overstated its enforcement. ProviderStaticsWiringTests parses the composition roots for ASSIGNMENTS only; nothing mechanically stops a read of TvContext.IsSqlite here. The real reason stands -- such a read would falsify that test's prose exemption while the test stayed green -- so the comment now says that instead of implying a guard that does not exist. Decisions-Edit: yes --- .../Search/Queries/GetSearchFieldValuesHandler.cs | 8 +++++--- .../Search/GetSearchFieldValuesHandlerTests.cs | 11 +++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs b/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs index 8a852e571..017b8e8a5 100644 --- a/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs +++ b/ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs @@ -213,9 +213,11 @@ public class GetSearchFieldValuesHandler(IDbContextFactory dbContextF return false; } - // Derived per-context rather than read from the TvContext.IsSqlite static on purpose: that static is - // scoped by ProviderStaticsWiringTests to host-only readers, and reading it here would falsify its - // scanner exemption. Do not "simplify" this to IsSqlite. + // Derived per-context rather than read from the TvContext.IsSqlite static on purpose. Nothing MECHANICALLY + // stops that read -- ProviderStaticsWiringTests only parses the two composition roots for ASSIGNMENTS, not + // readers -- but that test's scanner exemption for IsSqlite is justified in prose as "read only by + // DbInitializer + DatabaseMigratorService, both host-only", and reading it here would make that reason + // false while the test stayed green. Do not "simplify" this to IsSqlite. private static bool IsSqlite(TvContext dbContext) => (dbContext.Database.ProviderName ?? string.Empty).Contains("Sqlite", StringComparison.OrdinalIgnoreCase); diff --git a/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs b/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs index e2b701993..1e34d59ac 100644 --- a/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Search/GetSearchFieldValuesHandlerTests.cs @@ -711,8 +711,10 @@ public class GetSearchFieldValuesHandlerTests /// ersatztv#668. The Unicode fold added for the non-ASCII branch may OVER-match — the in-memory /// filter runs afterwards and drops the extras — /// but it must never UNDER-match. Each case pins the endpoint's answer against what that filter - /// alone would say, so a fold that starts dropping rows (or one that stops filtering the extras out) - /// fails here. + /// alone would say, so a fold that starts dropping rows fails here. It does NOT catch removal of the + /// in-memory filter — every case here is either a positive that SQL alone returns, or an ASCII-query + /// negative that SQL alone rejects. That direction is + /// 's job. /// /// The negative cases here have ASCII queries, so they exercise the FAST PATH (the fold is /// skipped entirely) and pin that it is exact: "ſweet".StartsWith("S", OrdinalIgnoreCase) @@ -768,6 +770,11 @@ public class GetSearchFieldValuesHandlerTests await context.SaveChangesAsync(); } + // Premises, asserted because the expectation is an EMPTY list and would otherwise pass for the + // wrong reason -- e.g. if the branch stopped running, or a hand-rolled fold stopped mapping ſ to S, + // SQL would return nothing and this test would still be green. + GetSearchFieldValuesHandler.ContainsNonAscii("\u017F").ShouldBeTrue(); + char.ToUpperInvariant('\u017F').ShouldBe('S'); "Sword".StartsWith("\u017F", StringComparison.OrdinalIgnoreCase).ShouldBeFalse(); var handler = new GetSearchFieldValuesHandler(_db.Factory);