feat(food-search): All Providers aggregated search (mobile, phase 2)#1654
Conversation
PR Validation ResultsChange Detection
✅ All checks passed. Thank you! |
There was a problem hiding this comment.
Code Review
This pull request introduces an aggregated 'All Providers' search mode in the food search screen, allowing parallel queries across multiple food providers. It adds a new hook useAllProvidersSearch, query keys, and a providerColor utility to support this feature, along with UI updates for top matches and provider-specific accordions. Feedback on the changes highlights that the manual reference-stability logic using useRef inside useMemo in useAllProvidersSearch is a React anti-pattern and redundant, as React Query v5's useQueries already provides referential stability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
eda214d to
74cd177
Compare
|
Good catch on the render-phase ref reads/writes – that part is a legitimate anti-pattern, so I've removed the One correction on the premise, though: in React Query v5 So rather than drop the optimization, I moved the projection into Typecheck, lint, and the |
|
That's a solid improvement. Moving the projection into the |
apedley
left a comment
There was a problem hiding this comment.
Theres an issue with the header on food search screen only for iOS that actually came from phase 1. We need headerShown: false in the options for createStackScreenOptions or else we get double headers.
New logic untested, ie. the interleave and empty/error/etc states.
Result rows does not use the provider_id to determine the key, they are only in the section. Instead it uses item.online.source. There's a possible collision there.
If someone selects All, deletes the text in the input, and taps scan it forwards __all__ as the provider to FoodScan. I'm not sure why it's forwarding it.. maybe in case we want to have a provider override in the future.
| // before each By Source provider. Known providers get a curated brand-ish | ||
| // colour; anything else gets a deterministic hue so new providers still get a | ||
| // stable colour with no maintenance. | ||
| const PROVIDER_COLORS: Record<string, string> = { |
There was a problem hiding this comment.
Try to not hardcode hex colors. Use uniwind colors from global.css for consistency and theme awareness. There are category colors there that are used for settings icons that would work well here.
|
UX issue maybe from phase 1: if I search for something I have a lot of but want to find another of, the entire screen can be taken up by "your foods" making it seem like search didn't work |
74cd177 to
257a465
Compare
|
Thanks for the thorough pass. All addressed in the latest push: iOS double header. Fixed. Result row key collision. Fixed. The online row key now includes the provider id (
Hardcoded hex colours. Fixed. Untested new logic. I pulled the Top Matches interleave out into a pure |
Good catch, and you are right that this predates phase 2. The screen is one SectionList that stacks Your Foods, then Your Meals, then the online section (Top Matches / By Source in All mode), and the local sections are unbounded. So a query that matches a lot of your own foods fills the screen and pushes the online results below the fold, which reads as "search didn't find anything new." It affects single-provider mode too, not just All Providers. A few ways we could go: A (my preference): cap Your Foods / Your Meals to the first few matches with a "Show all N" expander, reusing the same affordance the By Source accordions and provider "show all" already use. That keeps the online section within a screen of the top and stays consistent with the existing UX language. I would lean toward making it conditional, so the cap only kicks in when there are online results to show and a pure local search is never artificially truncated. B (could complement A): keep the section headers sticky and add a count to them (e.g. "Your Foods (23)") so it is always obvious there is more below, including the online header. On its own it just signals the online results exist rather than bringing them up, but paired with A it reinforces it. C (considered, not keen): a fixed max-height inner scroll on the local sections. It fights the SectionList model and feels awkward on mobile, so I would rather not. Reordering to put online first is off the table since local-first is intentional. Two things I would want your call on before I build it:
Once you pick the shape I will wire it up. |
|
Fatsecret has some limits. Though we will not hit that in practical for selfhosted env, hitting all the providers for every search will not be ideal. May I suggest to have these options so we don't take out what is feasible today?
Sorry if this was already been discussed. Probably a checkbox/radio button. I know this is how we had previously. but instead of multiple tabs, in single search itself having the checkbox/radio will it make hybrid of both? OpenFoodFacts cloud already running into issues, we will also be contributing more. |
|
Purpose of this PR itself is to unify the search. Not sure my previous comment is not valid one. I am open to your suggestions. |
Hitting all providers is the only way to get that info. To each provider it looks exactly the same as if it were searched individually.
These options are there just automated to be more like retail apps where you don’t need to remember where to search. Even so, “All Providers” currently requires switching to it explicitly - it’s not available as a default.
OFF will actually be in better shape during this. It has its own rate limit that’s tighter than the default. |
|
All good calls, working through them now. OFF rate limiting. You're right, and it's worse than "maybe": the single-provider hook and the All Providers hook each construct their own Colours, hash to index. Agreed, switching to Colours, contrast. Good catch on the neon green, that was Capping local results. Thanks, will do, in this PR. Option A with the condition: capping Your Foods and Your Meals to six rows with a "Show all N" expander, and only when there are online results to show, so a pure local search is never truncated. I'll push all of this together and flag anything that needs an iOS eye, since I can't run it on a device here. Thanks, this is a fair thing to be careful about, and it's worth being explicit about how the load actually behaves here. A few things that should ease the concern: All Providers is opt-in, not the default. A normal search still hits a single provider; the user has to deliberately switch to "All Providers" to fan out. So the multi-provider path only runs when someone asks for it, not on every search. To each provider, an All Providers search looks identical to a normal individual search, one request, same shape. There's no amplification per provider; it's the same call we'd make if the user had picked that provider manually. So we're not generating a heavier or unusual load pattern, just running the existing per-provider searches in parallel. For OFF specifically, requests are rate-limited, and per Andrew's review I'm consolidating that into a single shared limiter (tighter than OFF's own default) that covers both single and All modes, so even heavy toggling can't push us past a safe rate. And there's already an escape hatch for a setup that wants to avoid an external provider's load entirely: providers carry an active flag, and search only ever queries active ones, so deactivating a provider in the configuration removes it from search completely. A rate- or bandwidth-sensitive install can run a purely local search today without any new control. On the explicit local / online / both selector: that's essentially what the unified search already does automatically. Local and the chosen online provider both come back in one search (the retail-app pattern that #1496 is aiming for), so the user doesn't have to remember where to look. Reintroducing an explicit mode toggle would step back toward the multi-tab decision the redesign is trying to remove. That said, the building blocks are all here, so if you'd want a "search scope" control in the future (say, an advanced toggle to restrict to local-only for low-bandwidth or rate-sensitive setups), that's a small follow-up rather than a rework. Happy to go that way if you'd prefer it. |
257a465 to
189b3c2
Compare
|
Pushed all of this.
Two things I could not check myself since I have no iOS device: the new category colours' contrast on the badge (especially the blue/teal/amber on dark), and the local "Show all" behaviour. A quick look when you get a chance would be appreciated. |
After those 2 quick things we're ready for part 3 |
|
Ha, fair, that file was overkill for one export. Moved the singleton into Top Matches now never caps below the number of providers that returned results, so every configured provider with a hit shows at least one match. The round robin already emits each provider's first item before any second item, so that guarantee just falls out of raising the floor. Added a couple of tests for it (eight providers all appear; empty or errored providers don't count toward the minimum). |
189b3c2 to
9331edb
Compare
|
Looks good. Some tests are failing though |
Adds an 'All Providers' mode to the unified food search: a single search fans out across every active food provider in parallel (useAllProvidersSearch via React Query useQueries), each provider streaming in independently so a slow or failing provider does not block the others. - Top Matches: interleaves each provider's top results (round-robin by rank, capped), each row badged with its source in a per-provider colour. - By Source: one collapsible accordion per provider with a colour dot, a result-count badge, a per-provider loading spinner and skeleton rows, and a 'show all N' link that switches into that single provider's full view. Errored providers show a muted 'Couldn't load' + tap-to-retry; providers that return nothing show 'No results'. - Per-provider signature colours (providerColor: curated map + deterministic fallback) differentiate sources at a glance. - The Top Matches header doubles as the All Providers / single-provider switcher, consistent with the phase 1 source selector. Single-provider search is disabled while All Providers is active. Phase 1 behaviour is otherwise unchanged.
9331edb to
24db2a8
Compare
|
Fixed, green now. The limiter move was the culprit: I'd put the singleton in |
|
Hope this is ready to merge. |


Description
What problem does this PR solve?
Phase 2 of the unified search (#1496). Phase 1 streamed in a single online provider; this adds an "All Providers" mode so a single search returns results from every configured food provider at once, without the user picking a source first.
How did you implement the solution?
Selecting "All Providers" in the source switcher fans the search out across all active food providers in parallel (
useAllProvidersSearch, built on React QueryuseQueries). Each provider is its own query, so results stream in independently and a slow or failing provider does not block the others (partial results). The view shows Top Matches (each provider's top results interleaved round-robin, capped, badged with the source) and a By Source section with one collapsible accordion per provider. Each accordion has a result-count badge, a per-provider loading spinner and skeleton, an errored state ("Couldn't load" + tap to retry), an empty state ("No results"), and a "show all N" link that opens that single provider's full paginated view. Providers are colour-coded (a tinted badge in Top Matches and a dot in By Source) via a smallproviderColorhelper (curated map + deterministic fallback). The fan-out is entirely client-side, so there is no new backend.Linked Issue: Closes #1653
How to Test
PR Type
Checklist
All PRs:
New features only:
Frontend changes (
SparkyFitnessFrontend/):pnpm run validateand it passes.en) translation file.Backend changes (
SparkyFitnessServer/):rls_policies.sqlfor any new user-specific tables.UI changes (components, screens, pages):
Mobile changes (
SparkyFitnessMobile/):Screenshots
Notes for Reviewers
Mobile only. Web parity is intentionally deferred to a follow-up so the mobile UX can settle first; no
SparkyFitnessFrontend/orSparkyFitnessServer/changes here (the Frontend and Backend checklist items above are left unchecked because they do not apply). Tests:FoodSearchScreen.test.tsxupdated and passing; typecheck and lint clean on the changed files.A few deliberate choices worth your eye, all easy to change: