Keep recipe generation alive across tab switches#116
Conversation
Move the generation state and request out of the GenerateFlow route into a RecipeGenerationProvider mounted above AppLayout. The /generate route unmounts on tab switch, which orphaned the in-flight request and its state updates; the provider stays mounted across tabs, so results that arrive while on another tab are kept and shown on return. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🧹 The preview environment was removed because this PR was closed. |
|
Hmm, I've tested it on both chrome and firefox, but it works for me on neither. |
|
@imol-ai Note that when jumping back to the recipes page, you always see the tags first, but now there should be the "view recipes" button at the top right as soon as it's done. |
|
Woops, sorry, completely missed that - I just expected it to be the default view on the page. Looks good! |
…ation-across-tabs # Conflicts: # web-client/src/pages/GeneratePage.tsx
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new shared recipe-generation provider centralizes prompt, tag, recipe, status, and loading state. The app and recipe-related pages now read from that provider instead of outlet context, and the tests were updated to render under the new provider and use sessionStorage-backed recipe data. ChangesRecipe Generation Context Refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratePage
participant RecipeGenerationProvider
participant Backend
GeneratePage->>RecipeGenerationProvider: generate()
RecipeGenerationProvider->>RecipeGenerationProvider: set loading, clear recipes, persist prompt/tags
RecipeGenerationProvider->>GeneratePage: navigate to /generate/results
RecipeGenerationProvider->>Backend: POST /ai/recipes
Backend-->>RecipeGenerationProvider: response
RecipeGenerationProvider->>RecipeGenerationProvider: update recipes/status, persist to sessionStorage
RecipeGenerationProvider->>RecipeGenerationProvider: clear loading (finally)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-client/tests/pages/GeneratePage.test.tsx (1)
29-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMissing
sessionStorage.clear()inafterEach— test isolation gap.Test 1 seeds
sessionStorage['generated_recipes']but nothing here clears it afterward, unlike the siblingRecipePage.test.tsxwhich addedsessionStorage.clear()to itsafterEachin this same cohort. Currently masked becausegenerate()always resetsrecipesto[]synchronously, but any future test asserting on a clean initial recipe list will be order-dependent/flaky.🧹 Proposed fix
afterEach(() => { fetchMock.mockReset() vi.unstubAllGlobals() + sessionStorage.clear() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/tests/pages/GeneratePage.test.tsx` around lines 29 - 32, The GeneratePage test cleanup is missing sessionStorage reset, so seeded generated_recipes can leak into later tests and create order-dependent behavior. Update the afterEach in GeneratePage.test.tsx to also clear sessionStorage, matching the sibling RecipePage.test.tsx cleanup, alongside fetchMock.mockReset() and vi.unstubAllGlobals() so each test starts with a clean storage state.
🧹 Nitpick comments (1)
web-client/src/recipeGeneration.tsx (1)
47-56: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPersist
prompt/selectedTagsconsistently withrecipes.
recipesis synced to sessionStorage on every state change (Lines 47-49), butprompt/selectedTagsare only written at the momentgenerate()fires (Lines 55-56). Typed input or tag selections that haven't triggered a generation are lost on a full page refresh, unlikerecipes.♻️ Proposed fix
+useEffect(() => { + sessionStorage.setItem('recipe_prompt', prompt) +}, [prompt]) + +useEffect(() => { + sessionStorage.setItem('recipe_tags', JSON.stringify(selectedTags)) +}, [selectedTags]) + useEffect(() => { sessionStorage.setItem('generated_recipes', JSON.stringify(recipes)) }, [recipes])And drop the now-redundant writes inside
generate()(Lines 55-56).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/recipeGeneration.tsx` around lines 47 - 56, The `recipeGeneration` state persistence is inconsistent: `recipes` is kept in sessionStorage via the existing `useEffect`, but `prompt` and `selectedTags` are only saved inside `generate()`. Add sessionStorage sync for `prompt` and `selectedTags` alongside the `recipes` effect in `recipeGeneration.tsx`, and remove the redundant writes from `generate()` so `generate`, `useEffect`, and the `recipe_prompt`/`recipe_tags` keys all stay in one persistence path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-client/src/recipeGeneration.tsx`:
- Around line 35-43: The `RecipeGenerationProvider` state initializers for
`selectedTags` and `recipes` are parsing `sessionStorage` JSON unsafely, which
can now crash the whole app when mounted above `AppLayout`. Update the
`recipeGeneration.tsx` initial state logic to guard the `JSON.parse` calls with
try/catch or a safe parsing helper, and fall back to empty arrays when
`recipe_tags` or `generated_recipes` contain invalid JSON.
---
Outside diff comments:
In `@web-client/tests/pages/GeneratePage.test.tsx`:
- Around line 29-32: The GeneratePage test cleanup is missing sessionStorage
reset, so seeded generated_recipes can leak into later tests and create
order-dependent behavior. Update the afterEach in GeneratePage.test.tsx to also
clear sessionStorage, matching the sibling RecipePage.test.tsx cleanup,
alongside fetchMock.mockReset() and vi.unstubAllGlobals() so each test starts
with a clean storage state.
---
Nitpick comments:
In `@web-client/src/recipeGeneration.tsx`:
- Around line 47-56: The `recipeGeneration` state persistence is inconsistent:
`recipes` is kept in sessionStorage via the existing `useEffect`, but `prompt`
and `selectedTags` are only saved inside `generate()`. Add sessionStorage sync
for `prompt` and `selectedTags` alongside the `recipes` effect in
`recipeGeneration.tsx`, and remove the redundant writes from `generate()` so
`generate`, `useEffect`, and the `recipe_prompt`/`recipe_tags` keys all stay in
one persistence path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 104959c1-2b47-4459-a5c7-68b59aa7e1cf
📒 Files selected for processing (6)
web-client/src/App.tsxweb-client/src/pages/GeneratePage.tsxweb-client/src/pages/RecipePage.tsxweb-client/src/recipeGeneration.tsxweb-client/tests/pages/GeneratePage.test.tsxweb-client/tests/pages/RecipePage.test.tsx
Summary
During recipe generation, switching to another tab (e.g. Library/Profile) used to lose the in-flight request: returning back to
/generateshowed an empty list.This PR moves the generation state (
prompt,selectedTags,recipes,status,loading) and thegenerate()request out ofGenerateFlowinto a newRecipeGenerationProvidermounted aboveAppLayout. The provider stays mounted across tab switches, so a response that arrives while you're on another tab is kept and shown when you navigate back, and the loading state is preserved mid-flight.Type of change
API changes
api/openapi.yamlupdated andapi/scripts/gen-all.shre-runDefinition of Done
🤖 Generated with Claude Code
Summary by CodeRabbit