feat(favourites): search-led workspace, no ModeHome (#164) - #1983
Conversation
Ledger #164: /favourites becomes one dashboard + search page. Direction B from the comps — persistent search, sets as chips, Continue + Recent + sets on an empty query, in-place filter on a typed one. Product had already rejected a ModeHome treatment for this route, and none is reintroduced. - Retire the marketing lockup. The heart icon tile, the "Favourites command library" H1 and a sentence explaining the page to someone already standing on it cost roughly 90px of fold and said nothing the nav had not. The heading is now "Favourites" with the item count beside it as plain text, never a heading. - Real in-place filtering. The page reads the shared composer's live draft through useSearchCommand, seeded from the route's submitted ?q= so hard loads still server-render the exact list. Typing now filters the table without navigating and without a second input — the same pattern the tools results page already uses, so the one-composer contract in docs/search-chrome-behaviour.md is untouched and the route keeps the shell's hero composer. - Collapse the duplicated filter. filteredItems and the table's own tableRows were computed independently from identical inputs, so the band's match count and the table's count were two answers to one question. The page now derives the rows once and passes them down. - One chip rail replaces three navigation surfaces: FavouritesSidebar, FavouritesMobileQuickViews and FavouritesMobileBrowseRail. Sets, quick views and types are chips with counts, so browse reads identically at every width instead of three components disagreeing about what it means. Each chip still toggles its own dimension, so a set and a type compose as the sidebar allowed; "All" is the only chip that clears everything. favourites-library-nav had no remaining callers afterwards and is deleted rather than left dead. - Empty query shows Continue, then Recent and Your sets side by side. A typed query demotes that band to a collapsed disclosure and filters the table in place beneath it, with the table header switching to "N matches for …". Continue and Recent are rendered from the existing derivation and no new store was added. Worth stating plainly: lastUsedByItemId and pinnedItemIds are still hard-coded five-entry literals for demo slugs, and real registry items fall back to the string "Saved". Direction B leads with both surfaces, so they want a genuine per-item last-opened timestamp — that is a data-layer change, not a layout one, and inventing it here would have quietly changed the scope of this row. Captured as its own ledger row instead. Test updates land with the change rather than after it: the retired heading was asserted in six places, and two source-text pins needed adjusting for reasons unrelated to their intent — one sliced the mobile card's source to "wherever FavouritesTable happens to be" and so swallowed any component added between them, the other pinned the also-matches query expression when the test is about which surface owns that block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYphQZmsBBeqnidpSnAtjE
Assembled before the first push this time, so the PR head does not change under an in-flight CI run. - Branch review record for 0267232. - issues:done for #164, recording the delivered scope and the one part left deliberately undone. - issues:add for the Continue/Recent data gap that redesign exposed: both surfaces are now prominent but still ride hard-coded demo timestamps, so real saved items tie at one score and order arbitrarily. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYphQZmsBBeqnidpSnAtjE
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:38 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Comment |
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 15, 2026
Final Codex review snapshot Head:
The PR was merged by its pre-existing auto-merge workflow. Codex did not merge it or alter auto-merge. |
Summary
/favouritesbecomes one dashboard + search workspace, direction B from the comps. No ModeHome is reintroduced and no separate Favourites home route is created — product had already rejected that treatment for this route.Favouriteswith the item count beside it as plain text, never a heading.useSearchCommand, seeded from the route's submitted?q=so a hard load still server-renders the exact list. Typing filters the table without navigating and without a second input — the same patterntools-search-results-page.tsxalready uses — so the one-composer contract indocs/search-chrome-behaviour.mdis untouched and the route keeps the shell's hero composer.filteredItemsand the table's owntableRowswere computed independently from identical inputs, so the band's match count and the table's count were two answers to one question. The page now derives the rows once and passes them down.FavouritesSidebar,FavouritesMobileQuickViewsandFavouritesMobileBrowseRail. Sets, quick views and types become chips with counts, so browse reads identically at every width instead of three components disagreeing about what it means. Each chip still toggles its own dimension, so a set and a type compose the way the sidebar allowed; "All" is the only chip that clears everything.favourites-library-nav.tsxhad no remaining callers afterwards and is deleted rather than left as 529 lines of unreachable UI.Not done, deliberately, and worth knowing before review. Continue and Recent render from the existing derivation, and no new store was added.
lastUsedByItemIdandpinnedItemIdsare still hard-coded five-entry literals keyed to demo slugs; real registry items fall back to the literal string"Saved". Direction B leads with both surfaces, so they genuinely want a per-item last-opened timestamp — but that is a data-layer change, not a layout one, and building it inside this PR would have quietly rescoped the row. It is queued as its own ledger row instead, and this PR makes the gap more visible than it was, which is the honest trade.Verification
npm run verify:pr-localDesign-system contract, run because this deletes a component and adds three:
edgeOwnershipConflictsdrops 25 → 24 because the deletedfavourites-library-nav.tsxcarried one. That is a real paydown, not a baseline edit — the ratchet treats an absent path as zero, so the baseline is left alone here.UI verification not run: the container ships
chromium-1194while the lock pins revision1234, socheck:playwright-browser-revisionfails closed and no local Playwright run is possible; pointingPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHat the stale shell is forbidden byAGENTS.md. Browser proof rests on the CI Production UI job. Theui-smokefavourites journeys were updated in this PR and remain the coverage for the phone/desktop breakpoint contract.Risk and rollout
useSearchCommandwere ever absent the page falls back to the route's?q=prop, which is the pre-change behaviour, so the failure mode is "filtering needs a submit" rather than a blank page.favourites-library-nav.tsxreturns with the revert.useSavedRegistryFavouritesdata as before.Clinical Governance Preflight
The classifier returns
clinicalRisk: falsefor these paths, so this section is not required — completed anyway because favourites are a saved-clinical-content surface and the page's fault behaviour was touched.Clinical KB Database(sjrfecxgysukkwxsowpy)The conservative-failure behaviour is preserved exactly:
favouritesRegistryStatusstill folds a failed or unauthorized registry read topartialwhenever items exist, so a failed load can never render as "0 matches" and read as "you have no saved favourites". The empty-state branch still fires only on areadystatus. The demo boundary is unchanged — prototype favourites are still spread only underdemoMode, andtests/favourites-demo-boundary.test.tsstill pins it.Notes
RAG impact: no retrieval behaviour change — this PR touches only the favourites presentation layer and its tests. No retrieval, ranking, selection, chunking, or scoring code is involved; the page reads already-saved account favourites and filters them client-side with the samefilterAndSortItems` helper as before.tests/favourites-demo-boundary.test.tssliced the mobile card's source fromfunction FavouriteMobileCardtofunction FavouritesTable, so any component added between the two was pulled into the window and failed itsaria-pressedassertion for code that is not the mobile card; it now ends at the function's own closing brace.tests/audit-navigation-auth-regressions.test.tspinned the exactquery={query}expression on the favourites also-matches mount, when the test is about which surface owns that block; it now pins the mount, not the prop.Generated by Claude Code