fix(668): review round 4 -- sweep the retracted claim by SUBJECT, not by memory
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 21s
PR Gates / decisions lifecycle (pull_request) Successful in 25s
PR Gates / Script tests (pytest) (pull_request) Successful in 44s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 34s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 19s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m55s
Review verdict / Set review-verdict status (pull_request) Failing after 10m59s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 18m54s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 21m38s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped

Round 3 said "correct the claim everywhere" and missed two places, which is
the same mistake in a new coat: I fixed the spots I remembered instead of
grepping for the subject.

- The class-level summary of the very file round 3 edited still said MySQL's
  "ci collation OVER-matches instead", contradicting the method docstring
  forty lines below it. Reworded.
- The PR body still carried the retracted over-match story -- and round 3's
  commit message claimed it had been corrected. It had not. Now corrected,
  with the measurement table and the retraction stated openly.

This time the sweep was `grep -i over-match` across every file the branch
touches; the remaining hits are the SQLite-fold invariant and #578 history,
which are correct and stay.

Also softened two overclaims the reviewer flagged. Round 3 deleted the
predecessor's "configuration-incidental, not designed" hedge and replaced it
with a firmer statement than the evidence supports: that MySQL cannot
over-match is contingent on MySqlConnector fixing the connection collation to
utf8mb4_bin, not a property of MySQL. A driver, protocol or prepared-statement
change could restore it. The record and the fixture docstring now say
"driver-contingent, not a law"; the code is safe either way because the ordinal
filter stays regardless.

Filed #689 for the source of the copied falsehood -- LibraryFolderDedupeMigrationTests
on main carries the same "CI sets ETV_REQUIRE_MYSQL_TESTS=1" sentence.

Decisions-Edit: yes
This commit is contained in:
2026-07-27 22:09:26 +02:00
parent dda98efcc4
commit 07723e418b
2 changed files with 10 additions and 7 deletions
@@ -17,7 +17,8 @@ namespace ErsatzTV.Tests.Integration;
/// <summary>
/// ersatztv#668, EXECUTED on both providers. The bug was a collation/fold difference, so it lives exactly
/// where a single-provider test cannot see it: SQLite's <c>LOWER()</c> folds ASCII only and UNDER-matched
/// a stored <c>Édith</c>, while MySQL's is Unicode-aware and its ci collation OVER-matches instead.
/// a stored <c>Édith</c>, while MySQL's is Unicode-aware and reaches it unaided. (Its column collation
/// is accent-INsensitive, but the executed comparison is not — see the method docstring below.)
/// <para>
/// <see cref="ErsatzTV.Tests.Application.Search.GetSearchFieldValuesHandlerTests" /> covers the
/// SQLite semantics in depth against in-memory SQLite, and
@@ -158,8 +159,10 @@ public class SearchFieldValuesProviderTests(TestProvider provider)
/// binds the LIKE pattern with a BINARY collation and the executed comparison is therefore
/// accent-SENSITIVE. (A literal pattern typed by hand DOES over-match — a different query from
/// the one the handler runs.) So the row pins the accent-sensitive result on both providers and
/// documents the near miss; it does NOT exercise an over-match correction, because on this path
/// there is nothing to correct.
/// documents the near miss; it does NOT exercise an over-match correction, because with the
/// CURRENT driver there is nothing to correct. That is a driver-contingent fact, not a law: a
/// driver or protocol change that made the pattern ci-collated would restore the over-match, and
/// the ordinal filter — which stays regardless — would then be doing real work here.
/// </para>
/// </summary>
[TestCase("é", TestName = "Uppercase_Accent_Reachable_From_Lowercase_Query")]
@@ -26,8 +26,8 @@ Verified on a live MySQL 8.4: `LOWER('Édith')` is `édith`, so the existing pre
also matches `Edith` (the column is accent-insensitive `utf8mb4_0900_ai_ci`), and an earlier revision of
this record concluded from exactly that probe that MySQL over-matches and the ordinal filter corrects it.
It does not: through EF the driver binds the pattern with a BINARY collation, so the executed comparison
is accent-SENSITIVE and returns `Édith` alone. MySQL's correctness rests on Unicode-aware `LOWER()`, not
on the collation.
is accent-SENSITIVE and returns `Édith` alone — a driver-contingent fact, not a law. MySQL's correctness
rests on Unicode-aware `LOWER()`, not on the collation.
## Why a fold, and not the #578 walk
@@ -65,8 +65,8 @@ interceptor, so that seam would miss exactly this query.
## Residuals, stated rather than glossed
**Crowding**: a SQL `LIMIT` can fill with rows the ordinal filter then discards, under-DELIVERING the
count (never a wrong value). Unreachable on MySQL through the binary-bound pattern above; the SQLite fold
has it in principle when limit-many stored values are upper-equal but ordinal-unequal to the prefix (a
count (never a wrong value). Not reachable on MySQL under the CURRENT driver behaviour above (a ci-collated
pattern would restore it); the SQLite fold has it when limit-many values are upper-equal but ordinal-unequal (
`ſ`/`K`/`İ` class), so "no accepted loss" means no unreachable VALUE, not a guaranteed count. An
over-fetch was rejected (it perturbs the pinned `"apple"`/`"Zulu"` examples). **Ordering stays
best-effort** per #578.