V2.4.1 Improve code quality, add list thumbnail, improve UI, share to others, add generic import, add Playwright tests by devpro · Pull Request #468 · devpro/keeptrack

devpro · 2026-07-28T21:30:17Z

No description provided.

┌────────────────────────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────┐ │ Fix │ Verification │ ├────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────┤ │ S2365 — PlaylistDetail.razor rename │ PlaylistSmokeTest passed against a real browser + real Blazor Server circuit │ ├────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────┤ │ ASP0025 ×2 — AddAuthorizationBuilder │ ReferenceDataAdminResourceTest (AdminOnly) + PlaylistResourceTest/BookResourceTest │ │ │ (MemberOnly) — 9/9 passed over real HTTP with real Firebase auth │ ├────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────┤ │ CA1862 ×2 — │ AlbumReferenceRepositoryTest/BookReferenceRepositoryTest — 7/7 passed against real MongoDB │ │ StringComparison.OrdinalIgnoreCase │ │ ├────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────┤ │ CA1859 ×3 — concrete return types │ 235/235 unit tests passed │ ├────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────┤ │ JS S2486 — logged exception │ No test infra exists for this file — still an honest gap, noted in the docs │ └────────────────────────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────┘ docker itself isn't on PATH in either shell here (confirmed again), but Test-NetConnection -Port 27017 proved Mongo was reachable, which was enough to run everything through it directly. Updated docs/code-quality-findings.md with the real test results (replacing the earlier "couldn't verify" caveat), and saved a memory note on the working recipe for WebApi.IntegrationTests (not just Playwright) via Local.runsettings env-var loading + --filter-class, since that'll save time next session.

Every image-bearing list page (Movies, TV Shows, Books, Albums, Video Games, Cars, Houses, Health Profiles, Gear, Collectibles) gains a list/thumbnail toggle in the search bar. The thumbnail view is a responsive poster grid that leverages each type's cover art (portrait posters, square album art, wide game imagery) with title + meta captions. View mode lives in the URL as ?view=grid, following the existing search/sort/filter URL-state convention: bookmarkable and restored on back-nav. It is deliberately kept out of the query signature so switching views never refetches, and unlike a filter it does not reset the page. Playlists (no cover art) stays list-only. MobileScreenshotTest gains a cover-art showcase seed and grid captures at both phone (390x844) and desktop (1280x900) viewports, verified visually. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvLYeTtX526Y5QZqrrz1o6

Close the top UI-coverage gap from the new testing assessment: the TV Time, Amazon, and generic video-game import pages had API-level coverage but were never driven through the browser. - Support/*FixtureCsvBuilder + TvTimeImportFixtureZipBuilder: minimal GUID-suffixed in-memory fixtures so every run imports a genuinely new item it then deletes (no dedup-hidden "already imported" rows, no accumulation). - Pages/ImportPage + Amazon/GenericVideoGame page objects, plus a PageBase.OpenImportAsync nav helper. - End2EndFixture.GetItemIdsAsync: one reusable list-query helper all three tests use for API cleanup (books, video games, tv-shows, episodes). - docs/testing-assessment.md: the assessment itself, with these gaps marked closed. All three pass in self-hosted mutating mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PxyaxubjRzm2PSLMz7AE24

Covers the admin page's provider-free, deterministic surfaces (recommendation #2 from the testing assessment): page load via the Admin nav link, the System status panel resolving, unresolved-queue type switching, and the export -> import round-trip (idempotent upsert-by-id, so it changes no data). The provider search/link flow is deliberately left to the per-type detail-page smoke tests (same endpoints, real providers), and a full sync-now poll is left out as known-flaky on provider latency. Passes in self-hosted mutating mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PxyaxubjRzm2PSLMz7AE24

Close the last genuine coverage gaps from the assessment (#4 account page, #5 user-preferences UI) - Manage.razor previously had no test at any level. ManageAccountSmokeTest asserts the signed-in identity renders, then round-trips a preference toggle through the real UI (toggle -> persisted -> a fresh load reflects it) and restores the original value via the API so the shared account is left unchanged. The toggle re-clicks through the prerender->interactive gap the same way ClickUntilAsync does for buttons. Also records in the assessment doc why per-type Quick Add scenarios (original recommendation #3) were withdrawn: they would duplicate coverage the suite already provides, which the quality bar rejects. Passes in self-hosted mutating mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PxyaxubjRzm2PSLMz7AE24

- Inset the grid from the panel edges (was flush): 1rem desktop, 0.75rem mobile, matching the list rows' horizontal padding. - Size grid columns per cover shape so wide (16:9) tiles are no longer tiny strips: wide uses a 260px min (3 large columns on desktop, 2 on mobile) for video games/cars/houses/health/gear/collectibles; square (albums) 160px; portrait unchanged. The grid container now carries its ItemImageShape class so the CSS can target it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvLYeTtX526Y5QZqrrz1o6

Space the last row of grid items off the card's bottom border (1.5rem desktop, 1.25rem mobile); the grid previously had no bottom padding so items sat flush against the panel edge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvLYeTtX526Y5QZqrrz1o6

Fixed the problem was in the shared DateTimeFields.razor time input (used by both the health record form and car-history form). It's a free-text HH:mm field with inputmode="numeric", so the phone's numeric keypad shows digits but no : key, making the colon impossible to type. The owner deliberately chose free-text over the native <input type="time"> picker to guarantee 24h display (documented in the component and CLAUDE.md), so rather than override that decision, I made the colon optional on input: - SetTimeTextAsync now also accepts bare digits — 1430 → 14:30, 930 → 09:30 — via a new TryParseTime helper, while still accepting 14:30 typed on a desktop keyboard. - The field reformats to the canonical HH:mm on blur, so the stored/displayed value is unchanged. - Loosened the pattern to allow the optional colon and added a title hint ("Type HH:mm, or just the digits"). - An unparseable entry is still ignored, leaving the previous value untouched — same behavior as before. Because it's the shared component, car-history time entry on mobile benefits from the same fix.

The view choice is now remembered instead of defaulting to list on every page. It is a global user preference (like a theme), not list state: it never changes which items show or their order, only their presentation. Stored per-device in localStorage and mirrored in a new circuit-scoped ListViewPreference so every list page in the session shares one choice. The app renders InteractiveServer over a WebSocket circuit, so in-app navigation carries no fresh HttpContext to read a cookie from - hence localStorage (read once per circuit on first interactive render, since it is not reachable during prerender) rather than a server cookie. Chose per-device deliberately: thumbnails suit a desktop while a phone may prefer the compact list. The ?view= URL parameter is removed; toggling now re-renders in place (no navigation, no refetch) and writes the preference. MobileScreenshotTest drives the captures via the toggle + localStorage instead of ?view=, which also verifies the preference carries across pages and full reloads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvLYeTtX526Y5QZqrrz1o6

These two pages build their own layout (not InventoryList) but render the same media rows, so they gained the same view toggle and poster grid. To keep one copy of the logic, the toggle's seed/persist plumbing moved into a shared ListViewToggle component (used by InventoryList too, which no longer inlines it or the seeding in InventoryPageBase), and the poster card moved into a shared ItemGridCard used by all three grids. The card's caption render-fragment is named MetaContent, not Meta, because <Meta> collides with the HTML <meta> void element in Razor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WvLYeTtX526Y5QZqrrz1o6

Fixed. The Reference field in the owned-copy editor (OwnedVersionFields.razor) was misaligned because when an Amazon ASIN is detected, the "Open on Amazon" link renders as a .kt-icon-btn that's 2.25rem (~36px) tall — taller than the ~30px input beside it. The flex row wrapping them used align-items-center, which vertically centered the shorter input inside the taller row, dropping it a few pixels below the Price/Acquired/Vendor inputs in the same row. Switching that row to align-items-start top-aligns the input so it lines up with its siblings again.

Root cause: In the thumbnail (grid) view, the delete button lived inside .kt-grid-cover. On hover, that element gets transform: translateY(-3px) (app.css:479), and a transform creates a new stacking context. That trapped the button's z-index: 2 within the cover's context, so it could no longer sit above the card-level Bootstrap stretched-link (z-index: 1). The link painted over the whole cover — button visible, but the click landed on the link. The list-row view avoids this because its delete button is a direct sibling of the stretched-link with no transformed ancestor between them. Fix: Moved @actions in ItemGridCard.razor out of .kt-grid-cover to be a direct child of .kt-grid-card — the same stacking context as the stretched-link — so its z-index: 2 genuinely wins. No CSS change needed: .kt-grid-delete is position: absolute; top/right, which now anchors to .kt-grid-card (still position-relative) and stays in the same top-right corner over the cover.

Before: three loose stacked rows (state buttons, a bare fully-completed toggle, playthroughs) with unlabelled date pickers appearing inline with no context. After: - A <hr> divider separates the shared owned-copy fields from the game-specific progress controls, so the two areas read as distinct groups. - State and Completion now sit side-by-side in a two-column row (col-md-6), each under an uppercase form-label matching the rest of the app's fields. On mobile they stack. - The bare date pickers get contextual muted hints — "Completed on" and "on" (kt-card-meta) — so a lone date box is no longer mysterious. Widened to 170px so full dates aren't cramped. - A second <hr> sets off Playthroughs, which now shows a "No playthroughs recorded yet." empty state instead of just a bare "+ Add" button, and its remove ✕ got an aria-label for parity with the other remove buttons. Everything reuses your existing design tokens (form-label, kt-card-meta, .kt-icon-btn, Bootstrap grid), so it's consistent with the other detail pages.

Personal (car/house/health) sharing — list and read-only detail (your key distinction: media = list only; personal = list + full detail with history/metrics/charts). Backend (SharedWithMeController) - GET /{shareId}/{cars|houses|health-profiles} (list) and GET /{shareId}/…/{itemId} (parent + full child history + computed metrics), via two generic helpers (ReadPersonalListAsync, LoadSharedParentAsync) — grant resolution stays the single security choke point. Metrics reuse the existing static Car/House/HealthMetricsService. No copy routes for personal categories (view-only, enforced by absence + the classifier). - Contracts: one generic SharedDetailDto<TParent,TChild,TMetrics> (+ OwnerDisplayName for the breadcrumb). Recipient UI — reuses the real detail pages (as you steered, "it takes an id to load data") - CarDetail/HouseDetail/HealthProfileDetail gained a ShareId param → CanEdit => ShareId is null. When set, they load from the ownership-scoped shared endpoints (the one wrinkle: /api/cars/{id} is scoped to the caller's own user_id, so a recipient would 404), and every edit affordance is gated off. Three thin route wrappers own /account/manage/shared/{shareId}/…/{id} + MemberOnly auth, leaving the owner pages' own route/auth untouched. - History rows gained ReadOnly (hides edit/delete). SharedCollectionPage gained Cars/Houses/Health tabs → new generic SharedPersonalList. Breadcrumb reads Shared with me › <owner> › <item> per your note. Owner UI — SharingPage now has a Personal group; Health requires an explicit confirm before it can be enabled (never bundled).

The shared personal list (SharedPersonalList.razor) was a plain <a>-per-row list — no covers, no thumbnails, no grid, nothing resembling the media tabs. It now reuses the same InventoryList component the owner's own Cars/Houses/Health pages use, so it gets cover thumbnails, the list/grid toggle, search, sort, and per-type meta lines for free. Concretely: 1. InventoryList gained an optional DetailHref selector. Its read-only mode was built for the media shared list, where rows deliberately don't navigate anywhere. Personal items do have a full read-only detail page, so when DetailHref is set, read-only rows and grid cards link to it (media rows, with no DetailHref, are unchanged). 2. SharedPersonalList now wraps InventoryList in read-only mode with cover thumbnails (ImageUrl, wide shape, matching the owner pages), the per-type meta row, and detail links. Since a person has only a handful of cars/houses/profiles, the set is fetched once and search/sort run client-side over it (no server paging needed). 3. Extracted CarMetaRow / HouseMetaRow / HealthMetaRow into Components/Inventory/Meta/, mirroring the existing MovieMetaRow/etc. pattern, and pointed both the owner list pages and the shared list at them — so the meta line is defined once, not duplicated between the two views (per the no-duplication quality bar). 4. SharedCollectionPage passes the image/meta/detail params for the three personal tabs; removed the now-dead ItemSubtitle/CarSubtitle code. Both BlazorApp and the Playwright test project build clean with zero warnings. The existing SharedCollectionViewPage locators (.kt-item-row, row click) still work — the new rows keep that class and the stretched-link makes the whole row clickable, and the default view is the list view. One note: the grid/list toggle in this shared view respects the same circuit-wide ListViewPreference as every other list page, so if the recipient has switched to grid elsewhere, the shared personal tabs will show cover cards too — consistent with the media tabs' behavior.

Domain - ShareCategory (Domain + Contracts): added Collectibles and Gears. - ShareKind: added a third value Collection — a view-only list — alongside Media (copyable list) and Personal (view-only detail). ShareCategoryClassifier maps the two new categories to Collection, and IsCopyable stays the single rule KindOf == Media, so they're non-copyable. Backend (SharedWithMeController) - Injected the collectible/gear repos + DTO mappers. - New generic helper ReadOwnedListAsync<TModel,TDto> — the media read path minus reference-image hydration and copy-dedup (constraint is plain IHasId, not IReferenceLinkedDto), still routing through the one ResolveGrantAsync security choke point. - GET /{shareId}/collectibles and GET /{shareId}/gear. No copy routes. Search/sort/favourite/owned filters work via the existing repo GetFilters. Recipient UI - SharedCategoryList.Copy is now optional — when null, no per-row "add" action renders; it's the same full InventoryList (search/sort/filters/thumbnails/grid) but purely read-only. - SharedCollectionPage gained Collectibles/Gear tabs. - Extracted CollectibleMetaRow.razor / GearMetaRow.razor and reused them in the owner list pages too (same meta-extraction pattern as the media rows). Owner UI - SharingPage gained a third "Collections (view-only)" group; SharingLabels maps Gears → "Gear". Tests - ShareCategoryClassifierTest: added a Collection-kind/never-copyable theory. - ShareResourceTest.CollectionShare_IsReadableAsAFilterableList_ButNeverCopyable: paged/searchable/favourite-filtered read, empty AlreadyInCollectionIds, out-of-scope 404, and copy route absent.

What was happening The shared collection's tabs were rendered straight from share.IncludedCategories, whose stored order reflects the order the owner clicked the category buttons when creating the share. That's because SharingPage.razor builds the grant from a HashSet<ShareCategory> (IncludedCategories = [.. _selected]), so click order leaks through to storage and then to the recipient's tabs. Fix Added a single canonical ordering helper, SharingLabels.Ordered(...), whose order matches the left nav menu (NavMenu.razor): Movies, TV shows, Books, Albums, Video games, Cars, Houses, Health, Collectibles, Gear. (Note the enum's own declaration order differs — it lists Collectibles/Gear before Cars/Houses/Health — so sorting by the enum wouldn't have matched the menu.) Applied it everywhere categories are displayed, so nothing depends on the stored click order anymore: - SharedCollectionPage.razor — the recipient's tab bar, and the default-selected tab fallback (so the landing tab is the first one shown). - SharingPage.razor — the owner's "Active shares" category summary. - SharedWithMeListPage.razor — the recipient's "shared with me" list summary. The stored data is untouched (categories are stored by name), this is purely a display-order change. One note: SharingLabels lives in BlazorApp, which has no unit-test project (tests are WebApi/integration/Playwright only), so the ordering helper isn't covered by an automated test — it's straightforward display logic.

The wishlist wasn't showing books' custom images because WishlistController.BuildWishlistAsync only hydrated the cover from the linked reference document, never applying the tenant-owned CustomImageUrl override that books (and video games) carry. The override logic (hydrate reference cover, then let a non-empty CustomImageUrl win) was already duplicated in four places — BookController, VideoGameController, AlbumController, and a private helper in SharedWithMeController — and simply missing from the wishlist. Rather than add a fifth copy, I: - Promoted the helper into the shared ReferenceImageHydrator.HydrateWithCustomOverrideAsync (the one place that already owns the reference-image batch-lookup logic). - Routed every call site through it: the three CRUD controllers' OnListMappedAsync, SharedWithMeController's three reads (removing its now-redundant private helper), and — the actual fix — the wishlist's Books and VideoGames. Movies and TV shows in the wishlist stay on the plain HydrateAsync path since neither has a CustomImageUrl concept. Net result: the algorithm now lives once, and custom book/game covers render in the wishlist (and shared wishlist) just as they do on the list pages.

The fix Books' (and video games') tenant-owned CustomImageUrl override was applied on the per-type list endpoints but not in the wishlist, so custom covers silently fell back to the reference cover (or nothing). - Promoted the "hydrate reference cover, then let CustomImageUrl win" logic into a shared ReferenceImageHydrator.HydrateWithCustomOverrideAsync — it previously existed as a private copy in SharedWithMeController plus three near-identical inline copies in Book/VideoGame/Album controllers. - Routed all of them through it, plus the actual bug fix in WishlistController for Books and VideoGames (movies/TV shows stay on the plain path — they have no CustomImageUrl). Tests New WishlistResourceTest with two cases asserting a CustomImageUrl beats the linked reference cover in the wishlist payload, for books and video games. Both pass against local MongoDB. Docs - Fixed the now-inaccurate BookController.OnListMappedAsync doc comment (it claimed the override was "Book-specific, not shared via ReferenceImageHydrator" — the opposite of what it now is). - CLAUDE.md doesn't document CustomImageUrl, so nothing needed changing there for the feature itself. The test-filter gotcha you hit Documented in CLAUDE.md's commands section: --settings Local.runsettings and --filter-method can't be combined — --settings forces legacy VSTest mode, which rejects the Microsoft.Testing.Platform filter flags and silently runs zero tests (exit 5, "error: 1"). Added the working recipe: load the runsettings' env vars into the shell, then filter without --settings. A full unfiltered run can still use --settings. One thing I did not do: I left the AlbumResourceTest/BookResourceTest/VideoGameResourceTest existing override tests untouched since they still pass and cover the list-endpoint path. The shared-with-me endpoints already had override coverage via their own reads. If you'd like belt-and-suspenders coverage for the shared wishlist anonymous path (/api/wishlist/shared/{token}) specifically, I can add that too — but it exercises the identical BuildWishlistAsync code path, so I judged it redundant.

Summary of this round - Shorter messages (ReferenceRefreshMessage.cs): No match found → No match, Unlinked - no match → Unlinked, Already linked → No change. Linked! unchanged. - Narrower slot (app.css): reserved width 8.5rem → 6rem, which comfortably fits all four messages now. - Mobile: no reservation — inside the existing @media (max-width: 767px) block, .kt-title-toast-slot collapses to width: auto; flex: 0 1 auto, so on phones the (already small) title keeps its full width and the slot only takes room while a message is actually showing. Desktop now keeps the title stable with a modest 6rem reserved gap; mobile behaves as it did before, minus the permanent squeeze.

Root cause The error is a DI scope mismatch, not a Blazor-lifecycle timing issue. AuthenticationTokenHandler is wired into every API client with AddHttpMessageHandler<AuthenticationTokenHandler>(). IHttpClientFactory builds the message-handler pipeline in its own handler scope, separate from the component render scope. So the NavigationManager you constructor-injected into the handler was a fresh RemoteNavigationManager that the renderer never called .Initialize() on. That instance is fine as long as nobody touches it — but RedirectToLogin() reads navigationManager.Uri. That path only fires when the Firebase token is stale/expired (past the ~1h token life, before the client-side refresh caught up) and the API returns 401, or when there's no token. Exactly the intermittent condition you saw. During the SSR/prerender pass (WaitForNonStreamingPendingTasks in your stack trace), .Uri on the uninitialized manager throws RemoteNavigationManager has not been initialized, the framework's clean-redirect path never runs, and you get the red error until you refresh (a refresh re-issues the token via the cookie, so the 401 doesn't recur). Fix src/BlazorApp/Components/Account/AuthenticationTokenHandler.cs: resolve NavigationManager from httpContext.RequestServices instead of the constructor. That's the request's own scope — the one the endpoint renderer already initialized before running OnInitializedAsync — so NavigateTo throws the proper NavigationException the framework converts into a real redirect to login. httpContext is already guaranteed non-null on this path (the guard at the top of SendAsync), and I made RedirectToLogin static since it no longer needs instance state. Net effect: a stale token now cleanly bounces the user to the login page (re-running Firebase sign-in and re-issuing the cookie) instead of showing the red error. Worth noting I kept the change surgical. Two things I'd flag rather than silently change: - This handler throws InvalidOperationException("HttpContext is not available") when HttpContext is null, which is the case inside a live interactive circuit. So these authenticated API calls effectively only succeed during the SSR/prerender pass. That's a broader design characteristic, not something this bug requires touching — but if you ever see failures on interactive re-fetches (paging/search after the circuit is live), that's the thing to look at. - I didn't add a test: this is a rendering-scope/SSR behavior that a mocked unit test can't reproduce (it needs a real prerender host), and per your quality notes a mock here would prove nothing. If you want coverage, the right home is a Playwright smoke test that forces a stale token and asserts the login redirect

The fix src/BlazorApp/Components/Account/AuthenticationTokenHandler.cs — resolve NavigationManager from httpContext.RequestServices (the request's render scope, where the endpoint renderer already initialized it) instead of constructor-injecting it (which came from IHttpClientFactory's separate handler scope, where the RemoteNavigationManager was never initialized). That uninitialized instance is what threw RemoteNavigationManager has not been initialized on .Uri during the SSR/prerender pass whenever a stale token produced a 401 — the red error you had to refresh past. Now the properly-initialized manager throws the NavigationException the framework converts into a clean redirect to login. The test test/BlazorApp.PlaywrightTests/Smoke/StaleTokenRedirectSmokeTest.cs (+ a ForgeStaleTokenMemberCookie() helper on End2EndFixture). It forges an auth cookie with a valid member principal but a Firebase token WebApi rejects, hits the exact reported page (/account/manage/shared/{id}), and asserts a redirect to login. Verified both directions: - With the fix: passes (~5–12s). - With the fix reverted: fails, reproducing your exact stack trace (SharedCollectionPage.OnInitializedAsync → WaitForNonStreamingPendingTasks → uninitialized RemoteNavigationManager). Recipe updated I rewrote the e2e-run memory so this doesn't cost time again. The checks I ran (provider keys, Mongo port, browsers) were the recipe's gap — it now leads with "preconditions are already satisfied, don't re-verify," states plainly that no provider keys need setting (the host loads real ones from appsettings.Development.json; placeholder x values are pointless), and gives a copy-paste PowerShell block for a read-only single-class run. One thing I'll flag again since it's adjacent: this handler throws HttpContext is not available when HttpContext is null (inside a live circuit), so these authenticated API calls effectively only succeed during SSR. Not in scope for this bug, but worth knowing if interactive re-fetches ever misbehave.

What was already there The sharing feature already had a committed SharingSmokeTest (from c89eae2 Share car/house/health) plus its page objects, covering two of the three sharing shapes: - Media (Movies) — copyable list with the "In collection" badge - Personal (Cars) — list plus a read-only detail page The actual gap The plan's progress log flagged one deferred Playwright leg: the collection categories (Collectibles + Gear), which are a genuinely third shape — a full read-only list, but view-only (no shared reference to copy, so no "add to my collection" action and no read-only detail page). Nothing exercised those two tabs. What I added 1. SharedCollectionViewPage.AddButton(itemTitle) — a page-object locator for the per-row data-testid="share-add" action (kept on the page object per your locator-discipline convention). 2. SharingSmokeTest.ShareCollections_RecipientSeesReadOnlyListWithNoAddAction — self-shares Collectibles + Gear through the owner UI, then as the recipient opens each tab and asserts the item is listed read-only with the add button absent (ToHaveCountAsync(0)) and no "In collection" badge. That absence is the whole point — it's what separates a view-only collection tab from a copyable media tab. 3. Updated the plan's progress log to mark the leg as written. test/BlazorApp.PlaywrightTests builds clean (0 warnings). Like every other smoke test it self-skips unless E2E_ENABLED=true, so I couldn't run it here — it needs your WSL E2E env (MongoDB + Firebase creds + browsers). Per your run recipe, that's where it should actually execute. I've left everything uncommitted for you to review first. One note on scope: the existing test covers Cars but not Houses/Health for the personal shape. Those three go through the identical SharedPersonalList + read-only-detail mechanism, so Cars is representative — I didn't add redundant House/Health legs. Say the word if you'd rather have explicit coverage for those too.

Done You were right — the WantToWatch flag was dead weight on TV shows. WatchNextService.ComputeInProgressShows drives shows purely from State == Current + a confirmed unseen episode and never looked at the flag; there was no "shows to watch" surface for it to feed either. It's a genuinely movie-only concept, which is left untouched. Removed across every layer (build clean, 288 unit tests green): - Model / Entity / DTO — TvShowModel, TvShow (want_to_watch BSON), TvShowDto. Mapperly stayed balanced, so the build itself is the proof nothing was missed. - Repository filter — the input.WantToWatch clause in TvShowRepository.GetFilter (it wasn't even exposed as a list filter button). - UI — the "To watch" badge in TvShowMetaRow and the "Watchlist" toggle button + ToggleWantToWatchAsync in TvShowDetail. - TV Time import — ShowEnrichment no longer maps the export's for_later status into shows; the now-unused ShowStatusCsvParser.ForLaterStatus constant went too (parser test keeps its for_later row coverage via the literal, since the parser still must read that column faithfully for favorite). - Index — dropped tvshow_want_to_watch from mongodb-create-index.js, with a note explaining why. Data cleanup: added scripts/unset-tvshow-want-to-watch.js (idempotent $unset, same run-once style as the poster-url migration) to strip the orphaned field from existing tvshow documents — run once per environment. Docs: updated the stale CLAUDE.md import line and added a short design note recording that WantToWatch is movie-only by design, so it doesn't get reintroduced as a flag later (if you ever want a "shows I want to start" view, it should be a real Watch Next section, not a flag with no reader).

Summary #1 — Killed the N+1 reference lookup (WatchNextController.cs) The old code awaited a separate FindByIdAsync per in-progress show (N serial round trips to tvshow_reference). Now it collects the candidate shows' reference ids and does one batched FindByIdsAsync, then maps references back per show — the same batched pattern the movie-poster hydration one line below already used. #2 (cheap) — Stopped fetching the whole episode history (EpisodeRepository + IEpisodeRepository) The old code pulled every episode the owner has (int.MaxValue), then WatchNextService discarded every non-Current show's episodes in memory — cost scaled with total lifetime watch history. Added a batched, owner-scoped FindByShowIdsAsync(ownerId, showIds); the controller now fetches episodes only for the Current+linked shows that can actually appear in the result. The owner_id + tv_show_id IN(...) filter rides the leading fields of the existing episode_last_watched index. Output is behaviorally identical — non-Current and unlinked shows were already excluded downstream. Tests - New EpisodeRepositoryTest (integration, real MongoDB): verifies FindByShowIdsAsync returns only the requested shows' episodes, is owner-scoped (a different owner tracking the same show id is excluded), and returns empty for no ids. Both pass. - Updated the FakeEpisodeRepository in TvTimeImportServiceIdempotencyTest to implement the new interface member. - Full unit suite (288) green; the two new integration tests green against your local Mongo. Not done (as agreed): the aggregation version of #2 (server-side max (season,episode) per show) and reference caching — hold those unless this isn't enough. If you want to sanity-check the win, the Watch Next page load should now issue a small constant number of queries regardless of how many finished shows/episodes are in the collection. One note: ITvShowReferenceRepository.FindByIdAsync is no longer called from this controller but is still used elsewhere, so I left it in place. the movies tab is already in good shape, unlike the show-reference path we just fixed. I checked all three layers: What's already correct 1. Server-side filter, with a matching index. WantToWatch = true is pushed into the Mongo query (MovieRepository.GetFilter:34), and there's a dedicated partial index movie_want_to_watch (owner_id, want_to_watch, filtered to want_to_watch: true) backing exactly that shape. So it fetches only the watchlist, never the whole movie collection. 2. Batched poster hydration, no N+1. ReferenceImageHydrator.HydrateAsync collects all reference ids and does a single FindByIdsAsync — the same batched pattern I just applied to the shows. No per-movie round trip. So the two things that mattered on the shows side (a server-side filter and avoiding N+1) are both already right here. Two marginal nits (I'd leave both) - The "already seen" exclusion runs in memory. FilterMoviesToWatch drops movies with FirstSeenAt != null in C#, even though GetFilter already supports pushing it down via IsUnseen = true (line 37). Pushing it to the DB would transfer slightly fewer docs — but the set is already just the watchlist (small), the FirstSeenAt condition wouldn't be index-covered anyway (only want_to_watch is in the partial index), and FilterMoviesToWatch is a documented, unit-tested method that exists specifically because the flag goes stale. Net payoff ≈ zero. Not worth the change. - A wasted count query — but this is not movie-specific and not really an inefficiency of the movies tab per se: FindAllAsync always runs a CountDocumentsAsync before the Find to populate PagedResult metadata that Watch Next discards. Both the shows fetch and the movies fetch pay it (2 round trips where 1 would do). It's indexed on owner_id and cheap; removing it would mean adding a non-paged read method to the repos — more API surface than the tiny saving justifies. Bottom line: the movies tab doesn't have the kind of scaling problem the episode/reference path had — its cost is bounded by watchlist size, not by total library size. I wouldn't change anything there. If you want me to push the seen-filter down anyway (turning FilterMoviesToWatch into a DB filter) I can, but I'd consider it cosmetic.

OMDb's free tier is a hard 1000 calls/day and nothing counted them. Two consumers spend the key on the same 24h tick (the reference sync's IMDb backfill, then the Explore catalogue backfill) and neither could see what the other had spent, so Explore carried a hardcoded ImdbBackfillBudget of 250 per domain - 500/day whatever else was happening - while the sync's backfill was uncapped entirely. Worse, an exhausted key answers HTTP 401, which GetFromJsonAsync throws for. AddImdbRatingAsync is awaited unguarded inside ResolveTvShowAsync/ ResolveMovieAsync, so an over-quota day turned admin manual linking and Explore "add" into 500s - despite IOmdbClient promising a missing rating is never an error. The same was true of any OMDb timeout or outage. - OmdbCallBudget gates every call from inside OmdbClient, so no call site can bypass it. The count is one MongoDB document per (provider, UTC day) reserved with the same atomic filtered upsert LeaseRepository uses: an in-process counter would let every replica spend the full allowance, since interactive resolves land on whichever replica served the request. The day is part of the _id, so the allowance renews with no reset job. - Priority replaces per-consumer caps: Interactive may reach the whole allowance, Background stops short of Omdb:InteractiveReserve, and the Explore pass now sizes itself to whatever the sync left instead of guessing. - OmdbClient returns OmdbLookupResult for every outcome, catching the enumerated failures OMDb/the network/the resilience pipeline can raise (the caller's own cancellation still propagates). Both 401s - quota and rejected key - write the day off through the shared counter, so a blown quota costs one call rather than hundreds. - OmdbLookupResult.Attempted separates "OMDb has nothing for this title" from "we never asked", so a spent budget can no longer stamp a rating attempt and suppress a title for the whole 90-day re-attempt window. Covered by unit tests for the client's failure modes and the Explore pass's budget behaviour, and by a real-MongoDB ProviderQuotaRepositoryTest including a 50-way concurrent race for 20 calls - the ceiling rests on server-side atomicity a mock could only restate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TP6JmUK4BJPhafojZQCZFV

The periodic sync read every reference document with FindAllAsync() and filtered LastEnrichedAt in memory, in MongoDB's natural order. That materialized whole collections each tick - for TV, every show's entire embedded episode guide - to then discard most of them, and it meant a pass interrupted partway always re-walked the same head of the collection. FindStaleAsync(cutoff, limit) makes it a server-side filter and sort: never-enriched first, then least-recently-enriched, capped at 500 per domain. The ordering is what makes the cap safe - a pass takes the stalest end, so whatever it doesn't reach leads the next one - and it means an exhausted provider budget delays documents rather than starving them. The null half of the filter can't be folded into the comparison: MongoDB compares within a type, so $lte against a date matches neither a null nor a missing field, and a never-enriched reference - the one most in need of a pass - would be the one document the query could never return. Verified against a real server, and pinned by ReferenceStalenessRepositoryTest, since a mocked repository cannot prove either the filter or the ordering. The five byte-for-byte copies of the per-domain sync loop collapse into one generic SyncDomainAsync, the same shape RecomputeReferenceRatingsAsync already used; each domain now supplies only its repository and its refresh. The query itself lives once in ReferenceStalenessQueries, beside ExploreExclusionQueries, with each repository passing a field expression. Adds last_enriched_at indexes on all five reference collections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TP6JmUK4BJPhafojZQCZFV

A tenant item carried ReferenceRating/ReferenceRatingScale but no record of which source produced them, so nothing could tell an item already on the selected source from one that still needed re-stamping. The admin's "recompute" therefore had to assume the worst on every click: read the whole reference collection and fire one UpdateMany per document, all of them setting values that were already correct. ModifiedCount kept the reported counts honest, so the waste was in round trips, not the numbers - and the ordinary case is a no-op, since the button sits next to the source picker and gets pressed again "just in case". ReferenceRatingSource joins the pair on all five models, entities and DTOs. Everything that writes the value writes the source: SetReferenceLinkAsync, SetReferenceRatingAsync, the TryLinkExisting* direct updates, and the clear-on-unlink/no-match branches, which clear all three. PrimaryRating returns the source alongside the value so the two can't drift apart at a call site. The source is stamped even when that source has no value for the reference. It records which source the copy was computed from, not where a number came from - stamping it only alongside a value would leave every unrated item looking permanently mismatched, and the no-op below could never trigger. RecomputeReferenceRatingsAsync now opens with CountLinkedOnOtherRatingSource and returns (0, 0) without reading the reference collection at all when nothing is mismatched. An item stamped with no source (linked before the field existed) counts as mismatched, so the first recompute backfills the domain and every later one costs a single count - no migration script. It is deliberately not a value-drift repair: a value that moved while the source stayed put is the periodic sync's job, through the same SetReferenceRatingAsync. The five byte-for-byte copies of the propagation body collapse into ReferenceRatingQueries over the new IHasReferenceRating entity interface. Unlike ExploreExclusionQueries this needs no per-domain field expressions, because the four fields are named identically on every entity. Mapping needed no attributes either - the member matches by name in both directions on the storage and DTO mappers. Books stay the one domain with no selectable source: BookPrimaryRating reads whichever provider key the reference happens to store, and a reference with no rating at all genuinely has no source to name, which is why the source is nullable end to end. Covered by unit tests for the early-out (the reference collection is never read) and for the stamp surviving a source with no value, plus real-MongoDB coverage that the source round-trips and that a never-stamped item counts as mismatched - a $ne filter silently missing a missing field is exactly the failure a mocked repository would not catch.

SharingSmokeTest / SharingOwnerPage — the screenshot confirmed it: _recipientEmail was empty when CreateShareAsync ran server-side, even though the test had already called FillRecipientEmailAsync. Root cause is the same Blazor Server race PageBase.ClickUntilAsync was written for (documented in ReferenceDataAdminPage.SelectUnresolvedTypeAsync too): a Fill/Click that lands before the SignalR circuit connects only touches the DOM, and gets silently wiped back to the (still-empty) bound field on the first real render. SharingOwnerPage never had a protected first click to "warm up" the circuit before its two unprotected Fills ran. Fixed by making ToggleCategoryAsync retry via ClickUntilAsync (checking for the .active class, mirroring the existing SelectUnresolvedTypeAsync pattern — Health is exempted since it opens a confirmation modal instead) and reordering both tests to toggle categories before filling the text fields. VideoGameSmokeTest (and every other Movie/TvShow/Album smoke test sharing SearchAndLinkFirstResultAsync) — your screenshot showed the search genuinely still running, not stuck. The provider clients' resilience pipeline has its own total-timeout ceiling (30s for TMDB/RAWG/Discogs, 40s for the book providers), but the Playwright wait was only 20s — shorter than the server's own worst case. Bumped to 45s so a real (if slow/retried) search has room to finish.

What changed 1. Attempt stamp on TV/movie reference documents RatingsCheckedAt (ratings_checked_at, source → last attempt) on both models/entities, sharing Explore's 90-day window — which moved out of ExploreCatalogueRefreshService into RatingSourceCatalog.RatingReattemptAfter so there's one declaration, not two. BackfillImdbRatingAsync checks it before the TMDB external-ids lookup, so a title IMDb has nothing for now costs zero calls per pass instead of two (TMDB + OMDb). Only an attempt OMDb actually answered is stamped; a call that never happened leaves no trace. A re-resolve carries the existing stamps over instead of restarting them, and the Interactive paths (admin link, Explore add) ignore the window entirely. No index, no migration script: unlike Explore's map this is never a query filter, only read from a document the sync already loaded, and a missing field deserializes to "never attempted". While in there I fixed an adjacent bug: a full fetch rebuilds Ratings from TMDB, and when OMDb was unreachable it dropped a known IMDb rating — on exactly the days the budget is tight, leaving the cheap backfill to buy it back later. RebuildRatingsAsync (now shared by both full-fetch paths) keeps it when the call never happened; "OMDb has nothing" is a real answer and still clears it. One behavioural consequence worth naming: a forced "sync now" also honours the 90-day window, so it won't re-ask OMDb about a title stamped recently. That follows directly from your rule that the button does exactly what the sync does — no special-casing. Manual linking still always asks. 2. Recompute — and the scaling angle Per batch of 500 it's now one projected read plus one bulk write, instead of a whole-collection read plus one UpdateMany round trip per reference: - FindRatingsAsync(afterId, limit) projects _id + ratings and pages by _id cursor (for TV, the old read hauled every show's entire embedded episode guide to get at two fields). - SetReferenceRatingsAsync writes the page back as one unordered BulkWrite of UpdateMany entries. On your scaling point: the tenant items are still re-stamped server-side inside each entry, so a growing user base multiplies documents written but not round trips, payload, or API memory. The cheap no-op when nothing is mismatched is untouched and still comes first. Added to Movie/TvShow/VideoGame only — the selectable domains — rather than propagating dead methods to Book/Album. Tests - Unit: recent stamp skips both calls; past-window re-attempts; "no rating" stamps, "never asked" doesn't; full fetch keeps/drops the known rating correctly; re-resolve carries stamps; recompute pages from the cursor and stops on the first short page. The four existing recompute tests moved to capture-then-assert (Moq's It.Is can't hold tuple literals). - Integration (for you to run, in MovieReferenceRatingRepositoryTest): the batched re-stamp in one bulk write, FindRatingsAsync projection + id cursor, and RatingsCheckedAt round-tripping as UTC — a value read back as Local would silently shift the 90-day window by the host's offset, which no mocked test can catch. Docs updated: CLAUDE.md, docs/reference-ratings-plan.md, docs/code-quality-findings.md. Nothing committed. One thing I left alone: the recompute is still a synchronous admin request. Batched, that's fine well into the tens of thousands of references; past that it belongs on the existing JobStore pattern. Say the word if you want that now rather than later.

What changed: - TvShowController.OnDeletedAsync (src/WebApi/Controllers/TvShowController.cs:96) deletes the show's episodes, matching Car/House/HealthProfile. - IEpisodeRepository.DeleteAllForShowAsync + its EpisodeRepository implementation. - Rather than a fourth copy of the same owner-scoped DeleteMany, the algorithm now lives once as MongoDbRepositoryBase.DeleteAllByParentAsync(parentIdField, parentId, ownerId) — it takes the parent-id expression, not an element-name string, so the BSON name mapping stays with the entity. CarHistory/HouseHistory/HealthRecord were refactored onto it and are one-liners now. Tests/docs: - New real-MongoDB case TvShowResourceTest.TvShowResourceDelete_CascadesToItsEpisodes_IsOk, mirroring the existing House one (not run, per your instruction). - The two Playwright tests that hand-deleted episodes before deleting the show no longer need to — both cleanups simplified, with the reason noted. - Fake IEpisodeRepository in TvTimeImportServiceIdempotencyTest implements the new method. - CLAUDE.md: the "deleting a TV show does not cascade" gotcha is now the opposite statement, and the "Child entities" section documents the shared helper as the way to write a cascade.

What's covered ExploreSmokeTest drives four flows against a seeded explore_catalogue, all with the actions a real user performs — no reload, no browser back, no view switching: - Dismiss: the card shows the entry's year and rating, dismissing drops it, and the page's own Refresh re-fetches with it still gone (the half that proves the dismissal was recorded server-side rather than spliced out of a client list). - Load more: 24 cards, click, 26, first card unchanged (appended, not reshuffled), and the button goes once the ranking is exhausted. - Tabs: each domain shows only its own ranking, switching back by clicking the other tab. - Add: a real TMDB id (278), the card disappears, then Movies -> search -> open the item -> a cover on its detail page, which is the evidence the add resolved the reference by provider id rather than creating a bare movie. Video games are left out while RAWG is unavailable: the listing could be seeded around it, the add path could not, and half a domain isn't worth a test that reads as complete. Test infrastructure - End2EndFixture.SeedExploreCatalogueAsync/RemoveExploreCatalogueEntriesAsync write the ranking through the hosted IExploreCatalogueRepository. It's the only way in: that collection is otherwise written solely by the weekly refresh pass, which the e2e host never runs, so a seeded ranking is the whole ranking. Self-hosted mode only, hence CanSeedDatabaseDirectly and a self-skip under E2E_TARGET_URL. - ExplorePage page object; PageBase gains OpenExploreAsync and a Func<Task> overload of ClickUntilAsync, so a page object can retry until the URL changed or a card went away without a second copy of the retry loop. - SmokeTestBase.RemoveItemsMatchingAsync — TrackItemsMatching's body, extracted so it can also run up front. - NavigationSmokeTest covers /explore (read-only safe: the empty state is a load too); MobileScreenshotTest captures it, and no longer seeds "The Shawshank Redemption", which collides with the Add test's fixed title. Two findings, both documented in CLAUDE.md - A smoke test must stay in the default list view. ItemGridCard covers its card with an empty Bootstrap stretched-link, so the anchor has no size and Playwright refuses to click it — "element is not visible" on an element it just resolved by accessible name. ListPage.OpenItemAsync only works in list view. - The Add test clears any existing copy of that title before it starts. Explore's contract is to hide what the caller already tracks, so "the tenant doesn't hold this" is a precondition no amount of cleanup can establish afterwards. It's the one sanctioned place a test deletes what it didn't create, safe only because TestDatabaseGuard rejects any dev/prod/staging/preprod database. The lesson came from the test itself: it failed before TrackOpenItem claimed the id, and every later run then found an empty page. Cleanup: catalogue entries by external id, dismissals via the undo endpoint registered at the moment they're recorded, and the created movie by its own id. The movie_reference the link earns is deliberately left in place, like every other real-provider reference.

RAWG has been unreachable since 2026-08-02: api.rawg.io times out entirely and rawg.io returns a Cloudflare 522 (edge up, origin unreachable). It was the only video game provider, so game enrichment, admin linking, the periodic reference sync and Explore's game tab were all dead, and the failure was silent - a reference simply never refreshed. IGDB (Twitch/Amazon) is the only like-for-like replacement: it is the only provider that keeps both a browsable catalogue and two aggregate scores. Giant Bomb has no score field at all, which would kill the two-source rating model; MobyGames is commercially licensed and rate-limited to ~1 req/s; TheGamesDB's data is thin; Steam is PC-only; OpenCritic has no open API tier. The shape of this change deliberately mirrors what books did when they went from Open Library to Google Books: the new provider is *added* and made the default, the old one stays registered, and nothing already linked loses data. Video games are now the second multi-provider domain, and the machinery is shared rather than copied. ================================================================================ 1. Provider abstraction, now shared between books and video games ================================================================================ - New IReferenceProviderClient declares ProviderKey/DisplayName, the two members every provider client has in common. - New generic ReferenceClientRegistry<TClient> replaces BookReferenceClientRegistry (deleted). It resolves a provider by key, case-insensitively, falling back to the deployment default. Books and video games close over the same class rather than having one registry each - the alternative was 29 lines of identical resolution logic duplicated per domain. - IRawgClient (deleted) becomes the provider-agnostic IVideoGameReferenceClient, with RawgSearchResult/RawgGameDetails/RawgTopRatedItem renamed to VideoGameSearchResult/VideoGameDetails/VideoGameTopRatedItem. - IVideoGameReferenceClient adds SupportedRatingSources: the source keys a provider owns and can order by. It is read in three places - keying the Ratings map, deciding which stored ratings a refresh may overwrite, and declaring the domain's Explore rankings - so a new provider needs no second list anywhere. - VideoGameDetails carries a Ratings map built *by the client*, unlike BookDetails which carries a single scalar. Each game provider reports two scores on scales that differ per provider (RAWG's 0-5 user score beside Metacritic's 0-100, IGDB's two 0-100 scores); that knowledge belongs to the provider rather than to a switch in the shared enrichment service. ================================================================================ 2. IgdbClient ================================================================================ - New IgdbClient implementing IVideoGameReferenceClient: search, details by id, and a ranked page for Explore. - Queries are POST bodies in IGDB's Apicalypse language rather than query-string parameters. A tenant-typed title is escaped (quotes, backslashes, control chars dropped) before being embedded in a string literal - a bare quote would otherwise end the literal and let the rest of the title parse as query syntax. - An id is parsed as a number before being interpolated into a where clause, for the same reason. - first_release_date is a unix timestamp in seconds; summary is the synopsis; genres.name and platforms.name are expanded inline. - Search applies no year filter, deliberately. Apicalypse rejects sort alongside search, and narrowing a relevance search by exact release year is the same trap OpenLibraryClient already documents: an edition or regional release routinely carries a different year from the one a tenant typed. The year is returned per candidate for the caller to tie-break with. - RawgClient is kept and now implements the same interface, unchanged apart from the renames. It stays registered so an admin can still search/link with it and so the rawg/metacritic ratings it already produced keep rendering. ================================================================================ 3. Twitch authentication and rate limiting ================================================================================ IGDB is the one provider that authenticates with a token rather than an api key (it is part of Twitch and has no key of its own). All of it is handled outside the client so IgdbClient stays an ordinary typed HttpClient. - IgdbTokenProvider (singleton) exchanges client_credentials for an app access token and caches it, guarded by a SemaphoreSlim so a cold-start burst does not start one exchange per caller. Token and expiry travel together as one immutable record so the fast path reads both with a single atomic reference read. - Deliberately in-process, not in MongoDB - unlike OmdbCallBudget. A quota is a shared fact every replica spends from; a token is not. Twitch issues one per caller and several remain valid at once, so each replica holding its own costs nothing and coordinating them would buy nothing. - The renewal margin is capped at half the token's lifetime. A fixed margin longer than the lifetime puts the renewal point in the past the instant the token arrives, so every call fetches a new one - caching nothing while doubling traffic to Twitch. Real Twitch tokens last ~60 days so this only bites on short-lived ones; a unit test caught it. - It resolves its HTTP client from IHttpClientFactory by name rather than taking a typed HttpClient, because capturing one in a singleton pins its handler forever and defeats the factory's rotation. - It never throws: a missing key, a rejected secret, a failed request all come back as null, which every caller already treats as "IGDB is unavailable". Throwing would turn a transient Twitch outage into a 500 from an admin search. - IgdbAuthenticationHandler attaches Client-ID and the bearer token, and on a 401 drops the cached token and retries exactly once with a fresh one. A revoked or early-expired token would otherwise fail every request until the process restarted, since nothing else invalidates the cache. A second 401 means the credentials themselves are wrong, which retrying cannot fix. The request body is buffered up front because a sent HttpRequestMessage cannot be resent and its content is disposed after sending. - IgdbRateLimiter/IgdbRateLimitHandler pace requests to IGDB's documented 4/s using a TokenBucketRateLimiter. The bucket lives in a singleton, not on the handler, because IHttpClientFactory rebuilds the handler chain on every rotation and a limiter owned by the handler would silently reset its budget. - Handler order is load-bearing: authentication -> resilience -> rate limiter, outermost first. The limiter goes *innermost* for two reasons. Its queue wait is then covered by the resilience handler's total-request timeout instead of being an unbounded wait behind an infinite client timeout (these clients set HttpClient.Timeout to InfiniteTimeSpan by design so the resilience pipeline owns the bound) - a pod stuck in an unbreakable wait is worse than one that fails and restarts. And the 429 it synthesizes when the queue is full is retried with backoff rather than escaping to the caller as a hard failure. A retry also re-acquires a token, which is correct: a retry is another request against the ceiling. The queue is bounded for the same reason. - AddProviderResilienceHandler now returns the builder so that ordering is expressible. No other client's behaviour changes. - IgdbSettings has nullable credentials and a missing Igdb section is a supported state, the same optional shape as OmdbSettings. Every IGDB call path leads with IsConfigured and short-circuits to an empty result. This matters more than it does for a secondary provider: a hard requirement on the *default* provider's settings would take the whole API down on one unset value. ================================================================================ 4. Rating sources ================================================================================ - RatingSourceCatalog: VideoGame's selectable sources become igdb (default), igdbcritic and metacritic. New constants Igdb and IgdbCritic, both scale 100. - IGDB reports no Metacritic score. Its aggregated_rating is IGDB's own aggregation of external critic scores, so it gets its own igdbcritic key and that number is never written under Metacritic's - which would misattribute it. - rawg stays declared, with its scale-5 entry, and is only removed from the *selectable* list. ScaleOf throws on an unknown source, and references linked through RAWG still carry and display rawg-keyed values on detail pages; dropping the declaration would turn every one of those into an exception rather than a stale number. - Dropping it from the selectable list is what makes Resolve ignore a stored RAWG-era override and fall back to the current default. The existing recompute then re-stamps every tenant item on its first run, so the switch needs no migration script at all. - New MergeProviderRatings in ReferenceEnrichmentService: a provider may replace only the sources SupportedRatingSources says it owns. An IGDB refresh leaves a reference's rawg/metacritic values untouched, and vice versa. Every video game resolve/refresh path routes its Ratings assignment through it. Previously both paths assigned the fetched map wholesale, which would have silently discarded every stored RAWG and Metacritic score on first refresh - scores that are still displayed and that cost a call to a now-unreachable provider to obtain. - The other half of the rule: a source the provider *does* own but no longer reports is correctly dropped. That is the provider answering "I have no value", which is an answer, not an absence. - UI labels for igdb/igdbcritic added to ReferenceRatings.razor and the admin page. ================================================================================ 5. Enrichment: resolve, refresh, and carrying the existing catalogue across ================================================================================ ReferenceEnrichmentService.VideoGames.cs now mirrors .Books.cs: - TryAutoResolveVideoGameAsync searches the deployment default (unattended path, no admin to pick a provider), unchanged single-candidate rule. - ResolveVideoGameAsync takes an optional providerKey, looks up FindByExternalIdAsync against that provider's key, and writes the id under it. - New TryAdoptDefaultVideoGameProviderAsync: a reference carrying no id for the current default provider gets one adopted during the nightly sync. It searches once and adopts only when exactly one candidate's *normalized* title equals the reference's with a compatible year. This is stricter than ordinary auto- resolution on purpose: a reference's title and year are canonical provider data rather than tenant-typed text, so an exact match is a genuine confirmation - but two different games sharing a title is ordinary in this domain, so anything ambiguous is left for an admin to link by hand. This is what carries an existing catalogue across a provider change with no migration script and no new endpoint, costing one search per not-yet-adopted reference per pass and nothing once adopted. - RefreshVideoGameReferenceAsync now calls *only* the default provider. This is the one deliberate divergence from RefreshBookReferenceAsync's "refresh through whichever provider linked it". That rule is right for books, where every registered provider is reachable. This domain gained a second provider *because the first went down*, so falling back to it meant every not-yet-adopted reference paid a full retry-and-timeout cycle against a dead host on every pass - observed in the running app as a stream of GET api.rawg.io/api/games/... An operator who selects a provider must not see traffic to another one. - A reference that cannot be adopted keeps the data and the provider ids it already has, and is stamped as checked (new StampCheckedAsync). The stamp matters: FindStaleAsync serves least-recently-enriched first under a per-pass cap of 500, so a document whose LastEnrichedAt never moved would park at the head of that queue forever and starve everything behind it - the same re-walking-the-same-head failure the cap's ordering exists to prevent. - ReferenceEnrichmentService gains an ILogger. Adoption declining was previously silent, which made "why is nothing adopting?" invisible; it now logs the candidates and how many matched, which distinguishes ambiguous editions sharing a title from a year disagreement between the two providers. ================================================================================ 6. Images ================================================================================ - A reference that carries a RAWG id keeps its RAWG image (new PreferredImageUrl, applied on both the resolve and refresh paths). RAWG's background_image is curated landscape key art and its image CDN is still serving those URLs even though its API is not, so for a RAWG-linked reference the stored image is both good and still working. Replacing it is a downgrade, and it is irreversible: the RAWG URL cannot be recomputed from the RAWG id without RAWG's API. A small cosmetic gain against permanent data loss is what makes "keep what we have" the default rather than a special case. - Everything else uses IGDB's cover at t_1080p (810x1080), the largest token IGDB offers. Measured on the live CDN: t_cover_big 264x352, t_cover_big_2x 528x704, t_720p 540x720, t_1080p 810x1080 (~107 KB). Every token preserves the source's own 3:4 aspect, so this is purely a resolution choice, and box art is detailed artwork shown large on the detail page. - IGDB's landscape collections were both tried and rejected on direct inspection. artworks is contributed rather than curated and unreliable in aspect and quality (Red Dead Redemption 2's first artwork is posterised fan art at 720x720 - square, so a 16:9 crop mangles it; Baldur's Gate III's is a bare logo on black). screenshots are consistently 1280x720 but are raw in-game frames with HUD visible. Neither is the curated key art RAWG's background_image was; IGDB has no equivalent of it. ================================================================================ 7. Explore ================================================================================ - ExploreRankings becomes an injected service rather than a static class: the discovery provider, the stored rankings and the displayable sources for video games all now come from whichever provider is registered as that domain's default - a deployment-time choice, not a compile-time one. ToReferenceItemType stays static. - New DisplaySource: a rating source the catalogue genuinely cannot carry falls back to the ranking's own. Metacritic selected while IGDB discovers for the domain would otherwise leave every single card with no number at all, permanently. IMDb for movies/TV stays displayable because the refresh pass backfills it. Detail pages are unaffected and still show every stored rating. - ExploreCatalogueRefreshService builds its ratings dict from the client's own SupportedRatingSources instead of a hardcoded (rawg, metacritic) pair. - New IExploreCatalogueRepository.DeleteRankingsExceptAsync, called after a completed pass: DeleteStaleAsync only prunes *within* a ranking it just rewrote, so a ranking abandoned entirely by a provider change would sit in the collection forever, stale and unreadable. Keyed on what the configuration declares rather than on what one pass fetched, so a skipped or failed ranking is never affected. An empty keep-set is refused - that can only be a caller bug, and emptying Explore is worse than leaving it. - Dismissals need no cleanup: they are keyed on the discovery provider explicitly, so records written under RAWG stay unambiguously attributed to it. - The ranking query's only filter is a real vote-count floor (MinUserRatingCount/MinCriticRatingCount). IGDB reports a vote count per game, so unlike RAWG this is an actual minimum rather than RAWG's "has a Metacritic score at all" proxy. IGDB can also restrict to parent titles with game_type = 0 and deliberately does not: DLC and remasters are first-class records in this app, so a well-reviewed expansion is a legitimate suggestion rather than noise beside its own parent. ================================================================================ 8. Admin surface (breaking API changes) ================================================================================ - GET /api/reference-data/book-providers is replaced by GET /api/reference-data/providers?type={type}, returning the registered providers for any domain and an empty list for the single-provider ones (which is what tells the UI not to render a picker at all). - BookProviderDto is renamed ReferenceProviderDto. - Search and Link thread the chosen provider for VideoGame exactly as they already did for Book. - The provider picker in ReferenceDataAdminPage and InlineReferenceLinker is no longer Book-only: it renders whenever the domain has more than one provider, so an admin can search IGDB or RAWG per request. - SystemStatusDto gains a required VideoGameProvider, reported beside BookProvider and shown on the admin page. - Admin search results deliberately stay on the IGDB cover: that candidate list renders a small 60x90 portrait thumb, which box art fits. ================================================================================ 9. Configuration ================================================================================ - Igdb:ClientId and Igdb:ClientSecret (Igdb__ClientId / Igdb__ClientSecret) - a Twitch application registered at dev.twitch.tv. Both optional; see section 3. - ReferenceData:VideoGameProvider, default igdb when unset, so an existing deployment needs no config change to move off RAWG. - Rawg:ApiKey is still read and still used for admin search/linking through RAWG. - appsettings.json and CONTRIBUTING.md updated with both, including where to get the Twitch pair. ================================================================================ 10. Database ================================================================================ - scripts/mongodb-create-index.js declares videogame_reference_igdb_id: unique, with partialFilterExpression on external_ids.igdb, alongside the existing rawg index. Video games carry one index per provider rather than one overall, because a reference can legitimately hold both ids (RAWG linked it, IGDB was adopted onto it later) and each id space needs its own uniqueness guarantee. - No migration script is needed for anything else: the rating re-stamp rides the existing recompute, and provider ids are adopted by the sync. ================================================================================ 11. Robustness fixes this work uncovered ================================================================================ - The sync-now background job now runs on IHostApplicationLifetime.ApplicationStopping. It was fire-and-forget with no cancellation token at all, so a pass - which takes minutes - kept working against a container being torn down at shutdown: the singletons it depends on (the Mongo client, the HTTP clients, IGDB's rate limiter) are disposed out from under it and every remaining step throws ObjectDisposedException, including the final job-store write, which left the job reading "Running" forever. Observed in the running app as a disposed TokenBucketRateLimiter deep inside an IGDB call. Cancellation is now reported as "The API shut down before the sync finished" rather than a disposal stack. - The per-item catches in ReferenceSyncService and ExploreCatalogueRefreshService no longer swallow OperationCanceledException. One failing document must never abort a run, but a shutdown is not one failing document: swallowing it walks the rest of the page against a container that is already being disposed, turning one shutdown into a burst of failures. ================================================================================ 12. Tests (400 unit tests passing) ================================================================================ - FakeRawgClient becomes FakeVideoGameReferenceClient, parameterized by provider key and rating sources so a single test can register two providers and exercise the multi-provider behaviour without a second fake class. - New coverage: MergeProviderRatings keeps another provider's values and drops its own stale ones; refresh never calls a non-default provider; adoption links on an exact match and declines on both an ambiguous title and a year mismatch; adoption stops searching once it has succeeded; a reference with no resolvable id is still stamped as checked; DisplaySource falls back for Metacritic but not for IMDb; rawg is unselectable but keeps its scale; a RAWG-era override resolves to the current default. - New IgdbAuthenticationTest: the token cache serves repeat calls without a second exchange, coalesces a concurrent burst into one, refetches after Invalidate and past the renewal point, returns null rather than throwing when Twitch rejects the credentials; the handler attaches both headers, refreshes and retries once on 401 with the body intact, does not retry twice, and never sends a request when there is no token. - Integration: FindByExternalIdAsync resolves the same document from either provider's id space and rejects an id from the wrong one. A duplicate-key test was written and then removed - nothing in the suite creates indexes, so it would have passed or failed on whether someone had run the index script, testing the environment rather than the code. The reason is recorded in the file so it is not re-added. - Playwright: the e2e fixture now hard-requires Igdb__ClientId/Igdb__ClientSecret instead of Rawg__ApiKey. A smoke test links through whichever provider is the default, and no e2e path reaches RAWG unless a test picks it explicitly, so requiring its key would fail runs for a provider they never call. ExploreSmokeTest asks the hosted app for the ranking key rather than computing it statically. ================================================================================ 13. Documentation ================================================================================ - New docs/igdb-api-notes.md: the confirmed query shapes, the image findings with their evidence, size-token measurements, and the game_type/category story. - CLAUDE.md: IGDB added to the per-provider findings, the multi-provider video game section, the rating-source rules, the Explore changes, and the shutdown rule for detached background jobs. - docs/reference-ratings-plan.md carries a "superseded in part" note - its video-game arm describes the RAWG-era defaults. - CONTRIBUTING.md: the Twitch setup, both new config keys, and the changed e2e requirements. ================================================================================ Confirmed against the live IGDB API ================================================================================ Everything below was verified with real credentials rather than taken from documentation: - The details, search and ranked query shapes all return what the client expects. - first_release_date is unix seconds; cover.image_id builds the CDN URL. - search is relevance-ordered and noisy: "Half-Life 2" returns three MMod variants above the canonical game, which is why admin search shows several candidates and automatic resolution only ever acts on a single one. - category no longer exists and IGDB does not say so: it is silently dropped from responses, and where category = 0 parses fine and matches zero documents. A stale field name in a where clause therefore fails silently - and in the Explore refresh pass especially quietly, because a pass returning nothing deliberately keeps the previous catalogue. Its replacement is game_type (0 = main game). - Critic rating counts run an order of magnitude below user counts (8-27 against thousands for the same titles), which is why the two rankings have very different vote floors. - The cover size tokens and their exact pixel dimensions. ================================================================================ After deploying ================================================================================ 1. Register a Twitch application and set Igdb__ClientId / Igdb__ClientSecret. 2. Run scripts/mongodb-create-index.js against every database. Adoption starts writing external_ids.igdb immediately, and the unique partial index is what guarantees two references cannot claim the same IGDB id. 3. Trigger a sync (or wait for the nightly pass). Existing references adopt IGDB ids where the match is unambiguous; the rest keep their RAWG data and are reported in the logs. 4. Use the admin recompute once after picking a rating source, to re-stamp denormalized ratings on tenant items. Known limitations: - Adoption declines on ambiguous or year-mismatched titles by design; those references keep RAWG data and are logged for manual linking. - References that adopted an IGDB id before the image fix in this commit had their RAWG image URL overwritten. That URL cannot be recovered without RAWG's API and needs a database restore if the images matter. - IGDB has no curated landscape key art, so game covers are portrait box art while the detail page and list tile are shaped for landscape. Storing both a cover and a landscape image per reference is the clean fix and is noted as a follow-up in docs/igdb-api-notes.md.

What changed: - src/WebApi/ReferenceData/ReferenceSyncWindows.cs (new) — the single declaration of both staleness windows: Periodic (3 days references / 7 days Explore), Forced (zero/zero), For(force). The background service now reads it instead of holding its own constants, so the incremental "sync now" and the 24h tick physically cannot drift apart. - ReferenceDataAdminController.SyncNow([FromQuery] bool force = false) → RunSyncJobAsync(jobId, ReferenceSyncWindows.For(force)), which feeds both SyncStaleReferencesAsync and the Explore refresh. Everything else (own DI scope, ApplicationStopping, reconciliation, cancellation handling) untouched. Sync-now still doesn't take the lease — an admin asking for it now shouldn't be turned away by another replica. - Blazor: a Force checkbox beside the button (unchecked by default, disabled while running), StartSyncAsync(bool force), a rewritten explanation of both modes, and a mode line in the result alert captured from the run that started — so an all-zeros result reads as "nothing was stale" rather than "broken". - Contract note: an unparameterized POST /api/reference-data/sync-now now runs the incremental pass, not the full re-check. Pass ?force=true for the old behavior. 1. The fallback is now actually best-effort. AddOpenLibraryRatingFallbackAsync catches everything except OperationCanceledException (a shutdown must still propagate, same exclusion ReferenceSyncService makes per document), logs a warning, and lets the refresh finish. Both call sites — RefreshBookReferenceAsync and the interactive ResolveBookAsync — are covered by the one guard. 2. A rating that couldn't be re-fetched is no longer deleted. With the refresh no longer aborting, it wrote back a Ratings map rebuilt from the linking provider — which never carries Open Library's number. The live run proved it: The Hobbit's stored 4.29/498 and Psion's 4.5/2 were wiped by a refresh that simply couldn't reach Open Library. Both callers now pass the previously stored rating in; a lookup that never answered (failure, or no ISBN to ask with) keeps it, while a real "no rating for this ISBN" response still clears it — the same distinction RebuildRatingsAsync already makes for OMDb. Evidence - Live forced sync against keeptrack_dev with Open Library still degraded: booksChecked: 7, booksUpdated: 7, zero Failed to refresh reference entries, 4 Open Library rating lookup failed warnings (the 4 books with an ISBN), and all 7 last_enriched_at stamped at 09:52–09:55 — so they leave the head of the staleness queue instead of re-paying the 40s timeout every pass. - 3 new unit tests; the two guard tests were verified to fail with the fix stashed and pass with it. Full suite 406 passed, build 0 warnings. - docs/code-quality-findings.md gets a new Fixed entry (both halves), and CLAUDE.md's Open Library bullet records the rule. One consequence to flag: those two dev ratings are gone from keeptrack_dev right now — dropped by the forced run I made before the second half existed. They'll return on the next successful refresh once Open Library recovers (books are re-checked every 3 days, or immediately with a forced sync). I left them alone rather than hand-writing values back into your database; say the word if you'd rather I restore them directly.

What changed Export: untouched — I verified it's complete first. All six collections are exported, and since it serializes the models straight through and Mapperly escalates unmapped members to build errors, no field can silently go missing. New Domain/Services/ReferenceDataImportService.cs — one generic algorithm over all six collections (the repositories share no base interface, so it takes FindAllAsync/UpsertAsync as delegates): - Matches on any provider key the document carries, so nothing names a provider; _id survives only as the fallback for a document with no provider id at all (in your dev DB: 0 of 1,307 documents). - Keeps the target's _id, so tenant ReferenceIds stay valid. - Imports people first and remaps Cast/Author/Artist references onto the target's own person ids. - Merges: external ids unioned, aliases unioned, ratings per-source with target-only keys kept, RatingsCheckedAt keeps the later stamp, and anything the import has no value for leaves the target's alone. - A provider id another document already claims is skipped and reported (SkippedExternalIds, shown in the admin UI and logged) instead of failing the run — that means two reference documents for one work, which only you can merge. Also: ReferenceMatchModel.Matches now holds the alias-equality rule once (enrichment's MergeMatchedAliases uses it too); the result DTO reports created/updated per collection instead of one count; scripts/mongodb-create-index.js declares the index per provider that can write each collection. Verification ReferenceDataImportResourceTest — 8 tests over real HTTP and real MongoDB (a mocked repository can't prove this; the failure being prevented is the unique index firing): match-by-provider-id per domain (tmdb / igdb / discogs / googlebooks), keeps the target's _id, keeps target-only aliases+ratings+synopsis, remaps cast, skips a conflicting id, keeps a provider id the export lacks, runs twice cleanly. All pass; document counts across the test DB are unchanged before/after. Two things worth flagging: - The full integration suite has 1 unrelated failure: BookProviderSearchAndLinkResourceTest.SearchThenLink_ResolvesARealBook_ThroughOpenLibrary, a live-provider test — Open Library is currently timing out at 10s per attempt and returning 503s (the degradation already documented in CLAUDE.md). It fails at the search call, before any import code. - One run of the new class failed all 8 at once in 8s, then passed on four consecutive re-runs. The signature (every test, immediately, at the shared Authenticate()) points at a transient Firebase sign-in, not the code — but I couldn't capture the message, so I'm telling you rather than asserting it.

@page

What I found first There was already a 404 mechanism — but only the .NET 10 template scaffold: Router NotFoundPage + UseStatusCodePagesWithReExecute("/not-found") wired to a bare, unstyled <h3>Not Found</h3> page. So your instinct was half right. More importantly, that mechanism was never what you were hitting. The error page you see doesn't come from unknown URLs — it comes from dead item links, via two separate bugs: - GetOneAsync used GetFromJsonAsync, whose built-in EnsureSuccessStatusCode turns a 404 into a throw. All eleven detail pages already have a "Movie not found." state — it was unreachable. The throw killed the circuit on in-app navigation, or blew up the prerender into /error on a direct load. - A malformed id (hand-edited, truncated URL) never even got that far: every entity behind MongoDbRepositoryBase maps _id as an ObjectId, so the driver raises FormatException from ObjectId.Parse → 500. I confirmed this against your real MongoDB, then reproduced it as 4 failing tests before the fix (GET/PUT/DELETE /api/movies/not-an-object-id all 500). Specificities that shaped the design - [ExcludeFromInteractiveRouting] is load-bearing. Your App.razor picks the render mode from it. Without it the page renders interactively and the cascaded HttpContext isn't reliably there — so the status can't be read. It also means a bogus URL never opens a SignalR circuit just to say "no". I verified: 0 interactive markers on the 404 page vs 2 on Home. - The middleware re-executes for every 400-599 with no body, not just 404s. The page reads IStatusCodeReExecuteFeature.OriginalStatusCode so it doesn't promise "page not found" for, say, a rejected antiforgery token. - /not-found must keep its own @page route (the middleware re-executes onto it), so it's directly reachable at HTTP 200 — the page relabels anything under 400 as 404 rather than displaying "Error 200". - No [Authorize] — bouncing a signed-out visitor to login would tell them the page exists. - TvShowDetail was alone in querying its child collection before checking the parent, so the episode query (filtered on the same id) failed the whole page. Car/House/HealthProfile already had the right order. Verified live Ran the app and checked with curl (anonymous, no cookies): unknown URL → real HTTP 404 with the page rendering, <title>Page not found - Keeptrack</title>, no error UI, scoped CSS serving with the matching scope id. Tests - InventoryApiClientBaseTest (5) — 404→null, 200→item, and 500/403/400 still throw. - MalformedIdResourceTest (8) — real MongoDB, GET/PUT/DELETE plus the cascade path, each paired with a well-formed-but-absent id as control. Proven to fail without the fix. - NotFoundSmokeTest (2) — e2e status+render guard, self-skips without E2E_ENABLED. - No regressions: 406 WebApi unit, 19 BlazorApp unit, and Movie/TvShow/Car/Health/House resource tests all green. Confirmed nothing leaked into keeptrack_integrationtests. One thing I deliberately left, documented in docs/code-quality-findings.md: a raw API caller can still pass a malformed id as a filter (GET /api/episodes?TvShowId=garbage) and get a 500. No UI path reaches it, and fixing it means touching four child repositories rather than one shared method — your call whether that's worth it. I also updated CLAUDE.md with the mechanism and gotchas.

ReferenceDataImportService writes those one InsertOne/ReplaceOne at a time (after reading all six collections whole first) — minutes of work, and it was running inside a single blocking HTTP request. HttpClient's default 100s timeout cancelled the BlazorApp side while the WebApi kept importing. So your import very likely succeeded server-side after the UI reported failure. About the missing WebApi logs — that's expected and not evidence the request didn't arrive: src/WebApi/appsettings.json sets "Microsoft": "Warning", so ASP.NET Core never logs request start/finish, and the import path only logs its skipped-external-id warnings, at the very end. Fix Moved it onto the JobStore pattern the project already mandates for long-running work (same shape as TV Time import and sync-now): - POST /api/reference-data/import buffers the upload, starts the work on a fresh DI scope, returns 202 + job id; GET /api/reference-data/import/{jobId} polls. src/WebApi/ReferenceData/ReferenceDataAdminController.cs:87 - The job runs on ApplicationStopping and the import checks that token per document, so a shutdown mid-import stops promptly and says so instead of writing against a disposed Mongo client — safe because re-running the same zip is idempotent. - Domain reports which collection it's writing (ReferenceDataImportCollection); the web layer names that as a job stage. The admin page now shows a real per-collection progress bar, with people flagged as "most of the wait". - Secondary issue on the same path: the page handed IBrowserFile.OpenReadStream() straight to StreamContent, so the browser fed the file down the SignalR circuit during the POST, against the same timeout. It's buffered first now. - Signature bundling into ReferenceRepositorySet so ImportAsync stays under Sonar's S107 param limit, same fix as OwnedItemImportAdapter. Updated CLAUDE.md and docs/code-quality-findings.md. Verified: build clean (0 warnings), 406 unit tests pass, and all 9 ReferenceDataImportResourceTest cases pass against real MongoDB through the new 202-plus-polling flow. Two things to note: 1. Still one round trip per document, so a full export will take a couple of minutes in the background. Batching into BulkWrite pages would cut that by an order of magnitude but touches all six repositories and the merge loop's incremental indexing — I left it and recorded it as open in the findings doc. Say the word if you want it. The new test passes: Import_WhenOneDocumentIsKnownAndOneIsNot_CountsEachAgainstWhatItActuallyDid — passed (3s) It runs both branches in one import over the same collection and asserts what the numbers claim about the database: the updated show is still under the target's original _id and still the only document carrying its TMDB id (nothing was inserted beside it), while the created one now exists. To answer your question directly — the count comes from one variable that also decides the write: var match = FindMatch(document, byExternalId, byId); // dictionaries built from findAllAsync(), before any write if (match is not null) { merge(match, document); document.Id = match.Id; } // → ReplaceOne on an existing _id ... if (match is not null) counts.Updated++; else counts.Created++; // → ReplaceOne(IsUpsert) inserts There's no path where an insert increments Updated. The one asymmetry: two entries in the same zip sharing a provider id count 1 created + 1 updated, because the second merges into the one just inserted — but that needs Created > 0 in that collection, which is why it couldn't explain what you saw.

src/WebApi/Filters/ApiExceptionFilterAttribute.cs — TimeoutRejectedException / BrokenCircuitException / HttpRequestException now map to 502 Bad Gateway, logged as a warning. A 500 claims the fault is ours; it also made a provider outage indistinguishable from a real defect, for a log reader and for a test. Everything else still 500, argument exceptions still 400. Covered by a new theory in ApiExceptionFilterAttributeTest (7 tests pass). BookProviderSearchAndLinkResourceTest — now a [Theory] over googlebooks (the deployment default, so the provider a real user's search reaches) and bnf (keyless, quota-free), each case skipping itself on a 502. Narrow on purpose: only 502 skips, a 500 still fails. ResourceTestBase — GetThroughLiveProviderAsync / PostNoContentThroughLiveProviderAsync carry that guard; the four repeated StringContent/deserialize blocks collapsed into JsonBody/ReadJsonAsync on the way.

    public async Task<TDto?> GetOneAsync(string id)
    {
-        return await http.GetFromJsonAsync<TDto>($"{ApiResourceName}/{id}");
+        var response = await http.GetAsync($"{ApiResourceName}/{id}");


+            // pulled down the SignalR circuit in small chunks, and doing that *during* the POST would count the
+            // whole upload against the request's own timeout.
+            using var buffer = new MemoryStream();
+            await using (var stream = e.File.OpenReadStream(MaxImportFileSize))


…ures honestly Reported as a broken ISBN search (9782265002104) failing with "Google Books search failed (... 502 (Bad Gateway))". Not a regression: Google Books' search endpoint was answering 503 to everything, reproduced with curl and no application code involved - q=a and q=dune failed identically, with and without the fields= parameter, on both hosts, with the key in the query string and in a header, while volumes/{id} on the same key answered 200 and a bogus key answered 400. The key, the project and the API enablement were all fine; only search was down. Two real defects underneath it, both fixed here. An ISBN was only ever searchable through Google Books. OpenLibraryClient and BnfClient accepted the isbn parameter and silently ignored it, so switching provider in the admin picker - the one advertised way around a provider outage - quietly downgraded an exact-identifier search to a fuzzy title match. Both now search by it (q=isbn: and bib.isbn all, each confirmed against the real API). The ordering and fallbacks every book provider shares moved into the new BookReferenceClientBase (isbn, then title+author, then title, widening only on an empty result); a client now supplies just its own two query shapes. The author-retry loop was the same algorithm copy-pasted into all three clients. An ISBN miss widens to the title search rather than short-circuiting, which does change Google Books' previous behaviour. BnF holds no record for that ISBN while Open Library resolves it in one call, so reporting "no results" for a book the same provider can find by title would make supplying an ISBN worse than leaving it blank. No new mis-linking risk: the fallback runs the exact query an ISBN-less item already runs, and automatic resolution still only acts on a single confident candidate. The error text also named the wrong system. ApiExceptionFilterAttribute already returned 502 with an { error } body, but the Blazor client used EnsureSuccessStatusCode, which throws with only the status line and discards the body - so the API's explanation reached nobody and the admin saw our gateway status with the provider's real one nowhere in it. The filter now describes what the provider actually did (returned 503, unreachable, timed out, circuit open), ApiResponseExtensions reads that body into an ApiRequestException, and InlineReferenceLinker names the provider and points at the picker when the failure was upstream and the domain has another provider. Tests: BookReferenceClientBaseTest proves the shared policy once against a recording fake; OpenLibraryClientTest/BnfClientTest cover each ISBN query shape; ApiExceptionFilterAttributeTest covers the new messages; ApiResponseExtensionsTest covers reading the error body and falling back to the status line. 452/452 unit tests pass. Note for the next outage: BookProviderSearchAndLinkResourceTest does cover googlebooks search+link live, but skips on a 502 by design, so it goes green-by-skipping during exactly this scenario. That is deliberate and should not be narrowed, but it means no test will warn that a provider is down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012yXBT2z6j5sy9fGMTpepKK

…ed its backfill Reported one day after a deploy: half the movie list showed no rating, and the admin's rating recompute kept answering "0 references checked, 0 items updated" however often it was clicked. The recompute is a no-op by design there and always would have been - it re-stamps the denormalized rating on tenant items from what the reference document already holds, makes no provider call, and early-outs on CountLinkedOnOtherRatingSourceAsync, which returned 0 because every linked movie was already correctly stamped imdb. It can never produce a rating value. The missing half was ratings.imdb on the reference documents: 1516 linked movies (999 rated, 517 not), 509 movie references holding an imdb id, no imdb rating and no attempt stamp, against provider_quota showing omdb:2026-08-04 at 1000/1000. The bug is in the no-change short-circuit of RefreshTvShowReferenceAsync and RefreshMovieReferenceAsync. BackfillImdbRatingAsync correctly declines to spend a call it cannot afford, and correctly leaves RatingsCheckedAt unstamped so the title is not written off - but both callers then set LastEnrichedAt = UtcNow regardless. That marks a document the pass admittedly did nothing for as freshly enriched and drops it out of FindStaleAsync for the whole 3-day window, so a quota-capped day converges in 3-day steps instead of daily. That is what left the catalogue at half coverage with no admin action able to move it. BackfillImdbRatingAsync now returns an ImdbBackfillOutcome instead of a bool, and LastEnrichedAt is stamped for every outcome but Deferred. Two properties keep that from being the starvation this codebase otherwise stamps to avoid. It is narrow: only a spent allowance defers, checked before the call and re-read after it, since the budget can run out mid-pass through another replica or OMDb's own "Request limit reached!" 401. A missing key or a failed request deliberately still stamps - neither can be retried into working, so deferring on them would pin every reference at the head of the queue forever. And it is self-limiting: the allowance renews at UTC midnight, so a deferred reference costs one cheap TMDB /changes call per pass until then, and guarantees the next affordable calls are spent on the references actually missing a rating. The full-fetch path keeps stamping on purpose. Its expensive half - details, cast and the person upserts behind it - genuinely completed, so re-paying all of it every pass to retry one OMDb call would be the wrong trade; such a reference is left holding a TMDB rating, which routes it through the cheap short-circuit next time round. Tests: five cases in ReferenceEnrichmentServiceTest - the quota regression for movies and TV, the budget running out during the pass, and both narrowness guards (an unreachable OMDb for any other reason still stamps, and so does a reference that needed no call). The three pinning the fix were confirmed to fail without it. FakeOmdbClient gained an opt-in ReportsLimitReached that writes the day off through the shared budget, mirroring OmdbClient. 431/431 unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122bNmGMVVdaCTn1HPtNFhY

The guard keeping a video game reference's RAWG key art from being replaced by an IGDB cover keyed on "this document carries a rawg id", which is only a proxy for "the stored image is a RAWG image". The two diverge as soon as a document holds both ids - the normal state after the sync adopts the default provider - and the proxy then inverted the rule in the case where the operator acted deliberately: the admin picker passes its provider straight into ResolveVideoGameAsync, which writes that provider's id before computing the image, so re-linking through RAWG made the guard fire on the rawg id it had just written and discard the key art it had just fetched. That also left a dead RAWG URL unrepairable by anything short of unlinking (which deletes the shared reference document), and froze every image permanently on a deployment running RAWG as the default provider. PreferredImageUrl now takes the fetching client's ProviderKey and exempts RAWG. Every other provider still may not overwrite a stored image on a rawg-linked reference; RAWG stays authoritative for its own data. Covered by four RefreshVideoGameReferenceAsync cases and one ResolveVideoGameAsync case, the latter failing against the old predicate. The rule had no test at all before this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Explore excludes a suggestion two ways, and the RAWG -> IGDB switch broke both on the same documents. The id half asks each linked reference for the discovery provider's id, so a reference still carrying only a rawg id contributes nothing; the title half compared titles with exact normalized equality, which fails on precisely the spelling divergences that had blocked adoption in the first place. 105 of 344 references were stuck, 76 of them invisible to both halves - so the owner's own games, Elden Ring and Red Dead Redemption 2 among them, led their Explore feed. Movies and TV were never affected: TMDB is both their discovery provider and the one that linked every reference. Restore the invariant (every reference carries the current default provider's id) rather than working around it in Explore: - TitleNormalizer.NormalizeLoose/LooselyEqual for provider-to-provider matching, and StripDisambiguator for re-querying without a "(1997)" suffix - confirmed live, IGDB answers nothing at all to either query shape for such a title. Normalize stays strict; it keys aliases matched against tenant-typed text. - IVideoGameReferenceClient.FindGamesByExactTitleAsync (IGDB `where name ~`, RAWG search_exact plus a client-side equality check), and a query ladder in adoption that widens only on an empty result. Measured against live IGDB over the real stuck set: 49 of 105 now adopt unattended, and the rest reach the admin queue with candidates where they previously produced none. - ProviderAdoptionCheckedAt, so a fruitless attempt is remembered for 7 days instead of re-paid every pass. Add an admin provider-reconciliation surface for what adoption refuses to guess: the gap queue with per-row candidates and one-click adopt (writing onto the existing document, never minting a second), and duplicate groups with a merge that re-points every tenant's item before deleting the absorbed document. The merge keeps a RAWG-linked document's cover whichever one survives, computed before the ids are unioned - afterwards the survivor carries a rawg id whatever its own cover is. Report the import's blind spot rather than closing it: matching is by provider id, so an IGDB-era export lands beside a target's RAWG-era copies of the same games. Title text is not identity, so an import must not fuse them on its own. Finally, derive the video game domain's selectable rating sources from its registered default provider (new RatingSourceOptions) instead of a hardcoded list. That list was wrong in both directions: it offered Metacritic long after IGDB became the default although IGDB cannot produce it - so every game linked since resolved to no rating, while preserved RAWG-era values hid the breakage - and it would equally have kept offering IGDB's scores on a deployment back on RAWG. A stored override that is not currently on offer is ignored but never erased, so setting ReferenceData:VideoGameProvider=rawg brings rawg/metacritic back and honours the admin's existing choice untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QU4wnTX63eL6J6wpu3MmBy

Reported from the running app: an album search by title and artist returned results where the searched title occurred in the artist name rather than in any release title. q= is Discogs' free-text parameter, matching the artist name, label, credits and tracklist - artist= narrows the pool but does not constrain what q= matched on, and the pre-existing zero-result retry drops artist= entirely, leaving nothing but free text. Confirmed live: q=Discovery with artist=Daft Punk returns "Live @ Rex Club, Paris" and "MP3 Collection" beside the album, and q=Sabbath returns 2350 hits whose only occurrence of the word is "Black Sabbath" in the artist name. That noise made albums unlinkable rather than merely untidy. Automatic resolution acts on a single candidate, so a title colliding with a prolific artist's back catalogue never auto-resolved however unambiguous it was, and the admin picker only shows five candidates - noise ranked above the real master pushed it off the list, leaving no way to link by hand either. Re-check every candidate's own parsed release title client-side and discard mismatches, the same treatment BnfClient.AuthorMatches already applies to a provider clause that is not a strict filter. Running it on the parsed title from SplitArtistTitle is what excludes a match that only ever occurred in the artist half of Discogs' combined "Artist - Title" string. The filter sits inside SearchAlbumsCoreAsync, so the widening step needs no new condition: "answered, but nothing was actually titled that" reaches the artist retry as the same state as an empty response. TitleNormalizer.LooselyContains joins LooselyEqual for this - whole-word containment under NormalizeLoose, so "Nevermind (Demo & Outtakes)" and "Homework / Discovery" are kept while "Blue" does not match "Blueprint", and a title normalizing to nothing matches everything rather than filtering a caller down to none. Switching to the field-scoped release_title= instead was measured and rejected: precise (2350 hits down to 209 for "Sabbath", all genuine title matches) but it reorders badly - release_title=Nevermind with artist=Nirvana ranks the canonical 1991 album fourth, behind "Nevermind Sessions" - which the five-candidate cap turns into a worse failure than the noise. Filtering keeps q='s relevance order. Open Library's q= has the identical problem and is deliberately left alone: the book ladder is multi-provider and widens only on an empty step, so a filter changes which rung it lands on, and the ISBN-first fallback that carried book search through the Google Books outage is not worth risking for this. Documented in docs/code-quality-findings.md with the constraints for picking it up later. Covered by DiscogsClientTest (verbatim real-API response shapes, including the retry path) and TitleNormalizerTest.LooselyContains_*. The client had no test at all before this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XzsptC25NT8QvjGceAzW6r

sonarqubecloud · 2026-08-05T22:48:32Z

Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

An Explore suggestion is by definition not in the collection yet, so there was nothing to click: no detail page exists for it, and deciding whether to add or dismiss meant leaving the app and searching for the title by hand. Each card and row is now a link to the title's own provider page, opened in a new tab so the list being worked through survives the detour. It follows the rating the card displays wherever it can, which is the point: a movie showing an IMDb score opens IMDb, not TMDB, so the reader lands where the number came from. It falls back to the discovery provider's page otherwise, since the bounded IMDb backfill is always behind the ranking and an entry it hasn't reached must still open somewhere. The URLs are stored per source on the catalogue entry (web_urls, merged key-by-key exactly like ratings) rather than derived at read time, because two of the four cannot be derived: IGDB and RAWG key their pages on a slug, so those come back from the listing itself. ProviderWebLinks holds only the ones an id does determine. The IMDb link is a free by-product of the rating backfill, built from the id that lookup has to resolve anyway, and stored even when the OMDb call never happened - withholding a fact already in hand because a separate, budgeted call failed would leave the card pointing at the wrong site for another week. FindMissingRatingOrLinkAsync also takes an entry that has a rating but no link, deliberately without the 90-day window on that half: those are entries from before links were stored, one lookup closes each for good, and making them wait would leave the top of the ranking linking to TMDB. It still can't loop, because a title the provider has no id for gets no rating either and so can only match through the windowed rating branch; and an entry pulled in for its link alone spends no OMDb call. Existing entries gain links on the next pass that rewrites them (weekly, or sync-now with Force); until then the card renders as plain text rather than as an anchor pointing nowhere. Also drops the provider name from the page blurb, which had said "from RAWG" since IGDB became the default - the discovery provider is a deployment-time choice the client cannot know, and each card now names its own destination. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TRHuQKQwHLtpZZ6LMXK5WP

Provider reconciliation left rows nothing could clear, because the adoption ladder trusted a provider's search too far. Every finding below was measured against the live IGDB API using this database's stuck rows. - Widen on "nothing *matched*", not on "nothing came back", and keep every rung's candidates instead of the last reply replacing them. A relevance search hands back an unrelated non-empty answer as readily as an empty one - IGDB answers "NieR:Automata" with a lone "Untitled NieR:Automata Project" - and that ended the ladder before the rung that finds the game ever ran. - Ask with TitleNormalizer.ToProviderQuery: a search is far more punctuation-sensitive than the catalogue behind it. `search "NieR Automata"` returns the game first where `search "NieR:Automata"` never returns it at all, so that reference now adopts unattended. - Then, only while nothing has matched, retry with trailing words dropped (bounded, floor of two words), and finally with every word as a substring - the one shape that survives the provider spelling a title with punctuation the reference omits. Those rungs surface the right game for "NieR Replicant v1.22474487139", "Pokemon: Let's Go, Pikachu! and Eevee!" and "Marvel's Avengers", none of which had any candidate before. Substring results are unranked, so they are shortlisted to the eight closest to the title asked for. - Confirmation is unchanged and still judges a candidate against the reference's own title, so none of this widens what links itself. Apostrophes are now dropped rather than spaced in NormalizeLoose, which was producing "assassin s creed". - Give the admin row its own search box and accept a pasted provider page URL or id, since a search has genuine dead ends. Candidates are ordered closest-title-first so the likeliest answer leads the row. Also adds ?exploreOnly=true to sync-now, so rebuilding the Explore rankings no longer costs a full reference pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Gjyjjf1cN2BFP7QsBYhcE

The failure screenshot every Playwright test was supposed to leave was reaching the wrong page, and chasing one real failure with it turned up two suites quietly writing to each other's database. Failure diagnostics, for every test in the suite: - Capture walks every context the test opened, not just the shared signed-in one, and every open page in it. Four tests built their own anonymous context and disposed it inside the test body, so on failure they screenshotted the untouched shared page and traced a context nothing had happened in. They now open it through SmokeTestBase.NewAnonymousPageAsync, which keeps it alive until the diagnostics have run and removes the same four copies of the same context options. - Capturing can no longer replace the failure being diagnosed: a page that has crashed or is mid-navigation is reported and skipped, where before it threw out of DisposeAsync and cost the trace that would have explained the run. - The written paths are printed to the failing test's own output, so a CI log names its evidence instead of assuming the reader knows the convention (bin/<config>/net10.0/e2e-diagnostics). Then what that screenshot showed: 26 seeded Explore suggestions interleaved with 18 real films. - The sync-now tests get a 202 and finish while the job they started runs on against the live providers, writing the real TMDB ranking into the shared explore_catalogue minutes later - documents no cleanup can register at creation time, because they don't exist yet. Nothing there asserts anything a provider returns, so those classes are hosted through ProviderlessWebAppFactory and now write nothing, call nobody and spend none of OMDb's daily budget. ReferenceSyncPollingResourceTest, the one test that wants the live pass, moves to its own class and keeps the ordinary host. - ServerDerivedDataSweep (an assembly fixture, so it disposes after every class and every host - nothing can write behind it) empties the derived collections at the end of a run, as the backstop for that one test. It is best-effort by design: a cleanup that fails an assembly fixture turns 198 passes into 198 failures. - ExploreSmokeTest now checks the premise it rests on and names this cause, instead of failing on a card count that matches nothing. Database isolation, so neither of the above can recur: - Each suite settles its own database - keeptrack_integrationtests and keeptrack_e2e - and pushes the resolved name into its host's configuration, which is what stops the silent fallback to keeptrack_dev. Infrastructure__MongoDB__DatabaseName and E2E_MONGODB_DATABASE still override; neither is required any more. - Defaulting rather than demanding is the point. An IDE sets test environment variables once for the whole solution, so a variable both suites read could only ever hand them the same database, and removing it to stop that failed all 198 integration tests at once. The guard now vouches for the name a run will actually use, so a dev/prod name still fails fast. And the import smoke test that started this: /import renders a section per importer, each with its own level-1 heading and a link, so a sub-page object waiting for "Amazon" was ready while the browser was still on the hub - and the upload then ran against the hub's DOM, which has three file inputs. Sub-pages wait for their route (PageBase.Route) and scope the input by accept, like ImportPage already did. Also carries the empty param tags added to two ReferenceData files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BePNQ16PETbqLm23ESFC4D

devpro and others added 21 commits July 26, 2026 21:04

Add sonar issues in code quality findings docs

db5d796

Bump version 2.4.1

58686f8

Add generic import

064d096

Share media items with another user

620cfaa

github-advanced-security AI found potential problems Jul 28, 2026

View reviewed changes

Comment thread src/BlazorApp/Components/Import/GenericImportPage.razor Fixed

devpro added 8 commits July 29, 2026 10:10

devpro and others added 18 commits August 3, 2026 01:34

Fix invalid md

28f8089

Fix line length

63da11f

Fix invalid order

8d599f3

Add env var in ci for integration tests

c56549d

Update CI

872f700

github-advanced-security AI found potential problems Aug 4, 2026

View reviewed changes

devpro and others added 6 commits August 5, 2026 11:27

Fix format in test file

a9f0045

devpro and others added 4 commits August 6, 2026 14:45

Rename env var for mobile test

0ba5208

Navigation Menu

V2.4.1 Improve code quality, add list thumbnail, improve UI, share to others, add generic import, add Playwright tests - #468

V2.4.1 Improve code quality, add list thumbnail, improve UI, share to others, add generic import, add Playwright tests#468
devpro wants to merge 80 commits into
mainfrom
feature/code-improv

devpro commented Jul 28, 2026

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

devpro commented Jul 28, 2026

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 5, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants