Findings from the systematic gotcha-class review before the browser
pass:
- The collection smart editor's Apply derived the collection NAME from
document.title — which parses to "Smart Collection", RENAMING the
collection on every apply. The server now renders the real name
(data-collection-name) and the write path refuses to proceed without
it.
- TestRenderingPartialsRender pinned the pre-episode-collections
empty-state copy ("No books in this collection yet.") — updated to
the shipped label ("Nothing in this collection yet."; collections
hold episodes now).
- The deleted handler-local rule comparators (evaluateRule/
compareValues — removed in item 3.4's single-evaluator refactor)
left behind two unit-test files testing the duplicates; deleted.
Their semantics remain covered at the service level
(TestEvaluateRules_* over FieldMatches/EvaluateRules) and
TestTextToString (which actually pins utils.PgTextString) stays.
Everything else reviewed clean against the gotcha classes: templ
whole-expression attrs, ?={ boolean attrs, JSONArg quoting in
x-init/data attrs, Alpine CSP store paths, htmx form-encoding, the
" assert escaping, and the aria-label selector contract on the
player's reload listener.
checkRulesAgainstBook/evaluateRule and the service's EvaluateRules
existed in two parallel universes: one over database.MediaItems (what
every query actually returns) and one over ListMediaItemsRow (what two
hand-written 45-field mediaItemsToListMediaItemsRow converters
produced — verbatim twins in the handler and the dashboard service,
with a URL-resolution side effect baked into a "converter"). The rule
fields (genre/series/author/language/publisher/copyright_year/tags)
all exist on MediaItems, so the evaluators now take the real row and
the converters, their three call-site loops, and the dead
ApplyAutoAssignRules entry point (zero callers) are gone.
The rules surface is wire-covered by the collections rules scenarios;
handlers+services suites and the full pin/H1/H3 battery green.
handlers textToString, router textToString, router getText, and utils
normText were four copies of the same NULL-to-empty unwrap (99 call
sites). One exported utils.PgTextString replaces them all; the private
normText (comparison normalizer) stays but is now the odd one out.
Harness fix found on the way: setupTestServer built a full wiring
literal for route registration and then RETURNED a second, sparser
literal that never copied AnnotationService — setup.AnnotationService
was always nil on the struct every test receives. Nobody noticed until
cross_library_move_test became the first caller (it previously used
the now-retired plain CreateMediaHighlight query); it now exercises
the real SaveHighlight path through the wired service.
John O'Keefe
john-okeefe