fix: correctness fixes from a codebase review (#498)

* fix(router): preserve path encoding and query on festival redirect

go_router hands back percent-decoded path parameters, so the six
per-route onInvalidFestival closures — each rebuilding its own path by
string interpolation — silently lost the encoding. A style containing a
slash ("Porter/Stout", encoded %2F by buildStylePath) split into two
segments and stopped matching its route; a "?" became a query string and
a "#" became a fragment, truncating the id. Reaching a stale link to a
retired festival was enough to trigger it.

All six closures did the same thing: swap segment 0 for the current
festival. Doing that once in _festivalScopeRedirect, from state.uri with
each segment re-encoded, fixes the corruption and collapses the
duplication. It also carries the query string across, which previously
only /:festivalId did and the five nested routes dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* fix(drinks): sort names, breweries and styles case-insensitively

DrinkSortService used a raw String.compareTo, which orders every
capitalised value ahead of every lowercase one. In the live cbf2026
feed that puts the brewery "d'Achouffe" at index 277 of 279 — below
"Zotler" — instead of alphabetically between "Cydefx" and "Daleside".

The style facet and the My Festival list already sort case-insensitively
via StringComparisonHelper, so DrinkSortService was the outlier. Use the
same comparator for all three text sorts.

Tie order is still unspecified (equal-ABV drinks are not alphabetical);
adding a secondary tiebreak is left as a separate change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* fix(festivals): make Retry and Refresh work in the festival sheet

FestivalSelectorSheet renders provider state from a reference captured
via context.read, which subscribes to nothing, and a modal route is not
rebuilt by its opener. Every other sheet in this file gets away with that
because its controls pop the sheet before the notification lands — but
Retry and Refresh deliberately stay open, so loadFestivals() ran and
nothing on screen changed. Both were dead buttons.

Wrap the body in a ListenableBuilder on the injected provider. Consumer
would work too, but listening to the instance directly keeps the widget
usable without an ancestor Provider, which is how its tests build it.

The two tests covering this asserted provider state only; one carried a
comment explaining that the sheet would not rebuild. Both now assert the
rendered UI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* refactor(utils): rename compareLocaleAware to compareCaseInsensitive

The function lower-cases both operands and calls String.compareTo, which
compares UTF-16 code units. That is not collation: 'é' is greater than
every ASCII letter, so "Rosé" sorts after "Rosz" rather than next to
"Rose", and the brewery "Ārpus" sorts below "Zötler". The old name and
docstring promised locale-aware ordering the code never delivered, and
the docstring's examples ("Café comes right after Cafe") were true only
because the base word is a prefix of the accented one.

Behaviour is unchanged — this renames the function and rewrites the
docstring to describe what it does, including the limitation. The test
that asserted the coincidental accent ordering now also pins the real
behaviour, so introducing a collator later is a deliberate change with a
visible diff rather than a silent reordering of the style filter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* fix(provider): never strand the app on the startup spinner

BeerProvider.initialize() awaits SharedPreferences, two one-time
migrations, the festival cache and the saved selection, with no error
handling. If any of them threw, _isInitialized was never set, so the
router's '/' redirect kept returning null and the app sat on the
CircularProgressIndicator forever. Crashlytics recorded it as fatal, but
the user had no way out short of reinstalling.

Move the restore sequence into _restoreState() and wrap it: a failure is
logged, surfaced as provider.error so the drinks screen offers a Retry,
and startup completes either way. loadDrinks() now also refuses to
dereference a null repository, which is the state left behind when
initialize() fails before the repositories are built.

Also wraps three fire-and-forget futures in unawaited() while in these
files — including the logError call in _handlePostInitRedirect, the only
analytics call in lib/ that was not wrapped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* fix(screens): guard every festival-scoped screen against stale data

MyFestivalScreen was the only screen holding the festival-flash guard
from #397. The router schedules setFestival in a post-frame callback, so
on a URL-driven festival change — a cross-festival deep link on a warm
app, browser back/forward, or the post-init redirect — the other four
screens built once against the previous festival's catalogue. The drinks
list showed the old festival's name and drinks; the detail screens
resolved the wrong entity or a spurious "not found".

The in-app switcher was already safe (it calls setFestival before
router.go), which is why this never showed up in normal use.

Adds the guard to DrinksScreen, DrinkDetailScreen, BreweryScreen and
StyleScreen, and gives the invariant one test file covering all five so a
new festival-scoped screen has an obvious place to be added.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* docs: correct navigateToRoute and stale symbol references

navigateToRoute stopped branching on platform in #470 (PR #478) — it is
now context.push on every platform — but AGENTS.md and two skill files
still described the web/mobile branch in the present tense, with line
citations that had also drifted. Skills steer future agents, so this sent
them looking for code that no longer exists.

Also corrects references to the FavoritesScreen class (renamed to
MyFestivalScreen; the route stays /:festivalId/favorites, and that
distinction is now stated as such), and marks BottomActionBar,
ActionButton and BreadcrumbBar in the reuse table as currently unused —
no screen wires them up, so the table was recommending widgets that are
not in fact an established pattern here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* fix: address review feedback and remaining review findings

Router:
- Preserve the URL fragment across the invalid-festival redirect. The
  docstring claimed "everything else about the URL" but dropped it.
  Uri.query and Uri.fragment both return the raw, still-encoded form, so
  appending round-trips exactly (verified — re-encoding double-escapes).
  This retires a documented known limitation: the test that asserted the
  lossy behaviour, and its TODO, are replaced by one pinning the fix.
- Stop double-decoding the style path parameter. go_router already
  percent-decodes path parameters, so safeDecodeComponent turned a style
  literally containing "%20" into a space. The helper existed only for
  that call site and is removed with it.

Provider:
- Guard loadFestivals() against a null repository explicitly. This was
  not a crash — the null dereference was already caught and surfaced as
  festivalsError — but relying on a TypeError for control flow hid the
  intent and produced a generic message. Tests pin loadFestivals and
  refreshIfStale completing without repositories.
- Cover the loadDrinks null-repository guard, which Codecov correctly
  flagged as the untested part of the previous commit.

Repository:
- Report each unknown status_text once per session. getDrinks runs on
  every cold start, festival switch, staleness refresh and pull-to-
  refresh, so one new phrase from the organisers logged a Crashlytics
  error from every user roughly hourly for the whole festival.

UI:
- Replace Colors.grey / Colors.red with theme colours. The fixed grey is
  2.7:1 against a light surface, below the 4.5:1 WCAG AA requires for
  body text (it only passed in dark mode).

Docs:
- Fix a broken markdown table row introduced in the previous commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

* refactor: remove duplicated filter predicates and theme sheet

DrinkFilterService defined eight single-purpose filters and then
re-implemented every one of those predicates inline in filterDrinks, so
each rule existed twice: the copy with unit tests had no production
caller, and the copy the app actually ran had none of its own. A fix to
filterByAvailability could pass its test while changing nothing on
screen. filterDrinks now composes them, leaving one copy of each rule.
The service's 67 existing tests pass unchanged, which is the evidence
the composition is behaviour-identical.

about_screen carried a 77-line verbatim copy of ThemeSelectorSheet;
it now uses the shared widget.

Also adds a test for the drinks screen's full error view — one of the
four documented loading/error signals, previously asserted only as
"not showing" and never rendered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MoEMyawRmvhA2zuSDBmLdr

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: richardthe3rd

.claude/skills/architecture-contract/SKILL.md

@@ -281,7 +281,7 @@ run on every launch.
 | Personal state (favourites/ratings/tastings) is catalogue-independent (#390) | `DrinkRepository.getPersonalEntries` doc (`drink_repository.dart:56-65`): "the caller can enumerate a user's favourites, ratings, and tasting history purely from the personal-data store, before (or without) the drink catalogue being fetched." Fixed the favourites-flash bug family (#310/#397) as a side effect, because the My Festival list stopped being `_allDrinks.where(...)` (whichever festival happened to be loaded) and became its own festival-scoped query. |
 | Stale-while-revalidate (SWR) with two independent per-type caches | `cache_service.dart:8-19` class doc: render last-good data instantly, refresh in background, keep cache on failure. Per-*type* (not per-festival) caching specifically so a flaky `cider.json` fetch can't wipe out a good `beer.json` cache — see invariant 11. |
 | The `/` route must have a `builder`, not just a `redirect` | `router.dart:76-83` comment, citing issue #386: a redirect-only route that stays put (because the provider hasn't initialized yet) leaves go_router with an empty `pages` list and no `onGenerateRoute`, which crashes with "Null check operator used on a null value" in **release** builds only. The minimal `CircularProgressIndicator` builder at `router.dart:84-85` is the fix; removing it reintroduces a release-only crash invisible in debug/tests. |
-| `navigateToRoute()` branches `context.go()` (web) vs `context.push()` (mobile) | `navigation_helpers.dart:228-240`: `push` from inside a `ShellRoute` doesn't update the browser URL bar on web, but `push` is preferred on mobile to preserve the native back-stack. Always use this helper for drill-down navigation (drink detail, brewery) rather than calling `context.go`/`context.push` directly — see AGENTS.md's Navigation pattern. |
+| `navigateToRoute()` pushes on every platform (no web/mobile branch since #470) | `navigation_helpers.dart:237-239` is now just `context.push(path)`. It used to branch to `context.go()` on web because `push` from inside a `ShellRoute` didn't update the browser URL bar; enabling `GoRouter.optionURLReflectsImperativeAPIs` (`router.dart`) fixed that, and `go` was disposing the calling screen and losing its scroll position (#470, PR #478). The one-line helper is kept deliberately: it is the only place that rationale is recorded, and the single seam if that flag ever has to come back off. Always use it for drill-down navigation (drink detail, brewery) rather than calling `context.go`/`context.push` directly. |
 | Analytics only fires in production | `AnalyticsService._isAnalyticsEnabled = isProduction()` (`analytics_service.dart:21`); `EnvironmentService.isProduction()`/`isProductionHost()` (`environment_service.dart`). Fixed issue #269: unknown hostnames used to default to "production", polluting real analytics with staging/preview traffic; now unknown → NOT production (under-count is the safe failure direction). `logError` is the one exception — it runs in **every** environment so Crashlytics still sees staging crashes. |
 | `DefaultFestivals` hard-coded fallback | `models/festival.dart:262+` — four literal `Festival` objects (`cbf2026` active, `cbf2025`, `cbfw2025`, `cbf2024`) used only when both the network *and* the festival cache are unavailable (`FestivalController.currentFestival` getter, `festival_controller.dart:41-47`, and `BeerProvider.loadDrinks`, `beer_provider.dart:391-398`). This is a last-resort constant, not a data source to keep in sync with `data/festivals.json` — do not add new festivals here expecting them to appear in the switcher; that's the registry's job. |
 
@@ -303,11 +303,13 @@ found" — they're tracked.
   (`docs/todos.md:57-68`).
 - **No way to navigate back from the `/about` deep link** — archived todo
   H6 (`docs/todos.md:136-147`).
-- **URL fragments are lost during the post-init redirect.** Explicitly
-  marked with a `TODO` in `test/router_test.dart:666`: "Fix this by
-  preserving `currentUri.fragment` in redirect URL construction." The test
-  at lines 655-664 documents the current (lossy) behaviour as a known
-  limitation, not a passing spec for correct behaviour.
+- **URL fragments in the *post-init* redirect** (`main.dart`'s
+  `_handlePostInitRedirect`) are still dropped — it rebuilds the path from
+  `segments` + query only. The router's own invalid-festival redirect no
+  longer loses them: `_redirectToCurrentFestival` (`router.dart`) carries
+  query and fragment across verbatim, and `test/router_test.dart`'s "URL
+  fragments survive the invalid-festival redirect" pins that. The old
+  lossy-behaviour test and its TODO are gone.
 - **Flutter web renders to `<canvas>`**, so Playwright (the only E2E tool
   in use, per ADR 0005) can never assert on rendered UI content — a route
   can return HTTP 200 and still be showing an error state underneath.
@@ -316,13 +318,13 @@ found" — they're tracked.
 - **AGENTS.md architecture doc drift** — see the callout at the top of this
   file. `FavoritesService`/`RatingsService`/`TastingLogService` are gone;
   `UserDataStore` is reality.
-- **The class is called `FavoritesScreen` but the file is
-  `lib/screens/my_festival_screen.dart`.** PR #448 renamed the underlying
+- **The screen class is `MyFestivalScreen` but its route is still
+  `/:festivalId/favorites`.** PR #448 renamed the underlying
   model (`FavoriteDrinkEntry` → `MyFestivalEntry`) and generalised
-  `favoriteDrinks` → `myFestivalEntries`, but the screen class name and its
-  URL path (`/:festivalId/favorites`) are unchanged **on purpose** — URLs
-  are a public contract (see skill `change-control`'s unwritten rule #1).
-  Don't be surprised the class name and file name disagree; don't rename
+  `favoriteDrinks` → `myFestivalEntries`, and the class has since been renamed
+  to match its file, but the URL path (`/:festivalId/favorites`) is unchanged
+  **on purpose** — URLs are a public contract (see skill `change-control`'s
+  unwritten rule #1). Don't be surprised the class and route disagree; don't rename
   the class without checking every import, and never rename the route.
 - **The `/v1alpha` catalogue API is contract-only** (issue #432/PR #433):
   proto + generated OpenAPI + a read-only worker endpoint exist, but there
@@ -440,8 +442,8 @@ grep "^version:" pubspec.yaml
 # Confirm known-weak-points are still open (re-check state, not just existence)
 gh issue view 432 --json state,title 2>/dev/null || echo "use mcp__github__issue_read method=get issue_number=432 instead"
 
-# Re-read the fragment-loss TODO to confirm it hasn't been fixed
-sed -n '650,667p' test/router_test.dart
+# Confirm the router redirect still preserves query + fragment
+grep -n "hasFragment\|hasQuery" lib/router.dart
 ```
 
 If any of these disagree with the text above, the code has moved on —

.claude/skills/my-festival-campaign/SKILL.md

@@ -112,7 +112,7 @@ Ground rules for the whole track:
 - **#412 is already largely landed.** `MyFestivalEntry`
   (`lib/models/my_festival_entry.dart`), `BeerProvider.myFestivalEntries` /
   `favoriteEntries`, and `lib/screens/my_festival_screen.dart` (which today
-  contains class `FavoritesScreen`) already exist. Don't re-create them; extend
+  contains class `MyFestivalScreen`) already exist. Don't re-create them; extend
   them. Note the issues say `FavoriteDrinkEntry` — the code already renamed it to
   `MyFestivalEntry`.
 
@@ -223,7 +223,7 @@ want-to-try / tasted badge` — Fixes #413.
 favourites body. Rename the nav tab.
 
 **Files:** `lib/screens/my_festival_screen.dart` (extend the existing
-`FavoritesScreen`), `lib/screens/screens.dart`, `lib/main.dart` (nav tab
+`MyFestivalScreen`), `lib/screens/screens.dart`, `lib/main.dart` (nav tab
 icon/label/semantics ~406–417), `lib/router.dart`,
 `test/screens/my_festival_screen_test.dart` (new).
 

.claude/skills/ui-and-accessibility/SKILL.md

@@ -52,7 +52,7 @@ that failure mode is expensive and it has already happened.
    skill `change-control` for the full unwritten-rules list. Concretely for UI
    work: the "My Festival" rename (issue #414) renames the **nav label and
    icon**, not the route. The route stays `/:festivalId/favorites`
-   (`lib/router.dart:111`, `FavoritesScreen` class in
+   (`lib/router.dart`, `MyFestivalScreen` class in
    `lib/screens/my_festival_screen.dart`) even though the tab text changes to
    "My Festival." A visual/branding rename is never a licence to touch
    `router.dart` path strings.
@@ -62,8 +62,7 @@ that failure mode is expensive and it has already happened.
    deliberately small shared-component set (the identity hero panels
    `DrinkHeroPanel`/`BreweryHeroPanel`/`StyleHeroPanel` with their shared
    `FactsStrip`/`FactCell`, `InfoChip`, `SectionHeader`,
-   `BottomActionBar`/`ActionButton`, `BreadcrumbBar`, `buildOverflowMenu`, the
-   filter sheets). A second widget that does 90% of
+   `buildOverflowMenu`, the filter sheets). A second widget that does 90% of
    what an existing one does is scope creep and a maintenance burden for a
    solo maintainer.
 
@@ -140,8 +139,8 @@ unless noted.
 | Divided strip of "key facts" (centred value over an uppercase label) inside a hero | `FactsStrip` + `FactCell` | `facts_strip.dart` | `FactsStrip` owns the top/bottom/left dividers; a `FactCell` becomes a navigation button when given `onTap` + `semanticLabel` |
 | Small metadata pill (style, dispense, bar location) | `InfoChip` | `info_chip.dart` | Optional `onTap` makes it a `Semantics(button: true)` link |
 | Section title with underline on a detail screen | `SectionHeader` | `section_header.dart` | `showSeparator` toggles the underline |
-| Sticky bottom row of actions (tasting log, rate, favourite, share) | `BottomActionBar` + `ActionButton` | `bottom_action_bar.dart` | `ActionButton.isActive` drives colour + `FontWeight`; `semanticLabel` overrides the visible label for screen readers |
-| Back-navigation header on a detail screen (drink/brewery/style) | `BreadcrumbBar` | `breadcrumb_bar.dart` | Only the `IconButton` gets `Semantics`, never the text row — see the "BAD" example in `docs/code/widget-standards.md`. 28px icon → 48×48 touch target. Text segments only become tappable/underlined when a callback is provided |
+| Sticky bottom row of actions (tasting log, rate, favourite, share) | `BottomActionBar` + `ActionButton` | `bottom_action_bar.dart` | **Currently unused** — no screen wires it up (the drink detail screen uses a FAB + `YourTakeCard` instead). Available, but check it still fits before adopting. `ActionButton.isActive` drives colour + `FontWeight`; `semanticLabel` overrides the visible label for screen readers |
+| Back-navigation header on a detail screen (drink/brewery/style) | `BreadcrumbBar` | `breadcrumb_bar.dart` | **Currently unused** — detail screens use `CollapsingDetailAppBar` + `buildHomeLeadingButton`. Only the `IconButton` gets `Semantics`, never the text row — see the "BAD" example in `docs/code/widget-standards.md`. 28px icon → 48×48 touch target. Text segments only become tappable/underlined when a callback is provided |
 | Three-dot menu for festival switch / settings / about | `buildOverflowMenu(context)` | `overflow_menu.dart` | A function, not a widget class — `docs/code/ui-components.md` documents where to include it (Drinks, My Festival screen) and where not to (detail screens, About, modals) |
 | Modal filter pickers (category, style, sort, visibility) | `showCategoryFilter` / `showStyleFilter` / `showSortOptions` / `showVisibilityFilter` | `drink_filter_sheets.dart` | All route through the private `_showSheet` helper (`isScrollControlled: true`) and share `_SheetHandle` — add a new filter type by adding a sheet class + show-function here, not a bespoke `showModalBottomSheet` call elsewhere |
 | Star rating display or picker | `StarRating` | `star_rating.dart` | `isEditable` toggles read-only vs tap-to-rate; semantic `value` is always `'$rating out of 5 stars'` |
@@ -276,7 +275,7 @@ citable bug. Breaking one silently reintroduces a fixed defect.
 
 ### 1. Festival-flash guard (REQUIRED in any festival-scoped screen)
 
-`lib/screens/my_festival_screen.dart:15` (inside `FavoritesScreen.build`):
+`lib/screens/my_festival_screen.dart` (first thing in `MyFestivalScreen.build`):
 ```dart
 final provider = context.watch<BeerProvider>();
 if (provider.currentFestival.id != festivalId) {
@@ -314,10 +313,11 @@ invent a fifth loading state or collapse two of these into one.
 
 ### 3. `navigateToRoute` + typed path builders
 
-`lib/utils/navigation_helpers.dart:234` — `navigateToRoute(context, path)`
-picks `context.go(path)` on web and `context.push(path)` on mobile, because
-`push` from inside a `ShellRoute` doesn't update the browser URL bar on web,
-while `push` is preferred on mobile to preserve the native back stack. Use it
+`lib/utils/navigation_helpers.dart:237` — `navigateToRoute(context, path)`
+calls `context.push(path)` on every platform. It used to branch to
+`context.go()` on web (the URL bar didn't follow a `push` from inside a
+`ShellRoute`); `GoRouter.optionURLReflectsImperativeAPIs` fixed that in #470,
+and `go` was disposing the calling screen and losing its scroll. Use it
 for any drill-down navigation to content (drink detail, brewery, style). For
 root/tab navigation that replaces the stack (bottom nav taps, "go home"), call
 `context.go()` directly instead — that's a deliberate exception, not an

AGENTS.md

@@ -206,7 +206,7 @@ Container(
 
 **Provider reads** — `context.watch<BeerProvider>()` in `build()` only (subscribes to rebuilds). `context.read<BeerProvider>()` in callbacks, `initState`, and post-frame callbacks (one-shot, no rebuild subscription). Analytics calls in `initState` must be deferred via `WidgetsBinding.instance.addPostFrameCallback()`.
 
-**Navigation** — for drill-down navigation to content (drink detail, brewery), use `navigateToRoute()` from `lib/utils/navigation_helpers.dart`; it selects `context.go()` (web) or `context.push()` (mobile) automatically. For root/tab navigation that replaces the route stack (bottom nav, home button), use `context.go()` directly. Build URL paths with the typed helpers (`buildFestivalPath()`, `buildDrinkDetailPath()`, etc.) — never interpolate raw strings.
+**Navigation** — for drill-down navigation to content (drink detail, brewery), use `navigateToRoute()` from `lib/utils/navigation_helpers.dart`; it pushes the route on every platform, so the calling screen keeps its scroll position (#470). For root/tab navigation that replaces the route stack (bottom nav, home button), use `context.go()` directly. Build URL paths with the typed helpers (`buildFestivalPath()`, `buildDrinkDetailPath()`, etc.) — never interpolate raw strings.
 
 **Loading/error states** — four mutually exclusive signals on `BeerProvider`:
 

lib/domain/controllers/drink_filter_controller.dart

@@ -62,7 +62,7 @@ class DrinkFilterController {
 
   /// Unique styles in the source drinks, narrowed to the selected category when
   /// one is active, sorted case-insensitively (via
-  /// [StringComparisonHelper.compareLocaleAware]) so styles order in a stable,
+  /// [StringComparisonHelper.compareCaseInsensitive]) so styles order in a stable,
   /// human-friendly way regardless of capitalisation. Presentation consumes
   /// this directly — no sorting in the UI.
   List<String> get availableStyles {
@@ -71,7 +71,7 @@ class DrinkFilterController {
         .map((d) => d.style!)
         .toSet()
         .toList()
-      ..sort(StringComparisonHelper.compareLocaleAware);
+      ..sort(StringComparisonHelper.compareCaseInsensitive);
   }
 
   /// Drink count per category across the full source.

lib/domain/repositories/api_drink_repository.dart

@@ -20,6 +20,10 @@ class ApiDrinkRepository implements DrinkRepository {
 
   static const Uuid _uuid = Uuid();
 
+  /// Availability phrases already reported to Crashlytics this session, so a
+  /// novel `status_text` is logged once rather than on every refresh.
+  final Set<String> _reportedUnknownStatuses = {};
+
   ApiDrinkRepository({
     required BeerApiService apiService,
     required UserDataStore userDataStore,
@@ -85,12 +89,19 @@ class ApiDrinkRepository implements DrinkRepository {
 
     _applyUserState(update.drinks, festival.id);
 
+    // Report only phrases this session hasn't already reported. getDrinks runs
+    // on every cold start, festival switch, staleness refresh and pull-to-
+    // refresh, so without this a single new phrase coined by the organisers
+    // would log an error from every user roughly hourly for the whole run of
+    // the festival — drowning real crashes in the Crashlytics dashboard.
     final unknownStatuses = update.drinks
         .where((d) => d.availabilityStatus == AvailabilityStatus.unknown)
         .map((d) => d.statusText)
         .whereType<String>()
-        .toSet();
+        .toSet()
+        .difference(_reportedUnknownStatuses);
     if (unknownStatuses.isNotEmpty) {
+      _reportedUnknownStatuses.addAll(unknownStatuses);
       final sample = unknownStatuses.take(5).join(', ');
       final count = unknownStatuses.length;
       unawaited(

lib/domain/services/drink_filter_service.dart

@@ -128,8 +128,14 @@ class DrinkFilterService {
   /// 5. Allergen exclusions
   /// 6. Search filter
   ///
-  /// Each filter is only applied if its criteria is active.
-  /// Uses Iterable chaining to avoid intermediate list allocations.
+  /// Each filter is only applied if its criteria is active (each `filterByX`
+  /// short-circuits on an inactive criterion).
+  ///
+  /// Composed from the single-purpose filters above rather than re-testing each
+  /// predicate inline: there is exactly one copy of every rule, so a fix to
+  /// `filterByAvailability` (say) cannot pass its own unit test while leaving
+  /// the list the app actually renders unchanged. Still lazy — the `Iterable`
+  /// chain materialises once at the end.
   List<Drink> filterDrinks(
     List<Drink> drinks, {
     String? category,
@@ -139,43 +145,27 @@ class DrinkFilterService {
     Set<String> excludedAllergens = const {},
     String searchQuery = '',
   }) {
-    Iterable<Drink> result = drinks;
-
-    if (category != null) {
-      result = result.where((d) => d.category == category);
-    }
-
-    if (styles != null && styles.isNotEmpty) {
-      result = result.where((d) => d.style != null && styles.contains(d.style));
-    }
-
-    if (favoritesOnly) {
-      result = result.where((d) => d.isFavorite);
-    }
-
-    if (visibilityFilters.contains(DrinkVisibilityFilter.availableOnly)) {
-      result = result.where(
-        (d) => d.availabilityStatus != AvailabilityStatus.out,
-      );
-    }
-    if (visibilityFilters.contains(DrinkVisibilityFilter.notTasted)) {
-      result = result.where((d) => !d.isTasted);
-    }
-    if (visibilityFilters.contains(DrinkVisibilityFilter.veganOnly)) {
-      result = result.where((d) => d.isVegan == true);
-    }
-
-    if (excludedAllergens.isNotEmpty) {
-      result = result.where(
-        (d) => excludedAllergens.every((a) => (d.allergens[a] ?? 0) == 0),
-      );
-    }
-
-    if (searchQuery.isNotEmpty) {
-      final lowerQuery = searchQuery.toLowerCase();
-      result = result.where((d) => _matchesSearch(d, lowerQuery));
-    }
-
+    Iterable<Drink> result = filterByCategory(drinks, category);
+    result = filterByStyles(result, styles ?? const {});
+    result = filterByFavorites(result, favoritesOnly: favoritesOnly);
+    result = filterByAvailability(
+      result,
+      hideUnavailable: visibilityFilters.contains(
+        DrinkVisibilityFilter.availableOnly,
+      ),
+    );
+    result = filterByNotTasted(
+      result,
+      notTastedOnly: visibilityFilters.contains(
+        DrinkVisibilityFilter.notTasted,
+      ),
+    );
+    result = filterByVegan(
+      result,
+      veganOnly: visibilityFilters.contains(DrinkVisibilityFilter.veganOnly),
+    );
+    result = filterByExcludedAllergens(result, excludedAllergens);
+    result = filterBySearch(result, searchQuery);
     return result.toList();
   }
 }