Repository navigation
chore(playground): improve canvas generation UX - #2313
Merged
smakosh merged 8 commits intoMay 18, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR adds cookie-backed model preference helpers and SSR wiring, refactors error-message parsing, makes the canvas generate API resolve API keys from body/header/cookies, migrates clients to cookie-based model persistence, and updates the canvas client for streaming-aware lifecycle, auth/project side-effects, and skeleton UIs. A chat UI small change collapses reasoning by default. ChangesCanvas Playground Generation and Streaming
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/playground/src/app/api/canvas/generate/route.ts (1)
34-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize and validate
prompt/apiKeybefore use.
" "prompt and whitespaceapiKeycurrently bypass checks and can trigger avoidable upstream failures. Trim first, then validate.Suggested patch
- const { prompt, model, apiKey } = body; + const prompt = typeof body.prompt === "string" ? body.prompt.trim() : ""; + const model = body.model; + const apiKey = typeof body.apiKey === "string" ? body.apiKey.trim() : undefined; @@ - if (!prompt) { + if (!prompt) { return new Response(JSON.stringify({ error: "Missing prompt" }), { status: 400, }); } @@ - const finalApiKey = apiKey ?? headerApiKey ?? cookieApiKey; + const finalApiKey = apiKey || headerApiKey || cookieApiKey;🤖 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 `@apps/playground/src/app/api/canvas/generate/route.ts` around lines 34 - 55, Trim and validate the incoming prompt and API key values before use: call .trim() on the incoming prompt (the local variable prompt) and on apiKey/headerApiKey/cookieApiKey before computing finalApiKey, then reject if the trimmed prompt is empty or the trimmed finalApiKey is empty; update the checks that currently return 400 to use the trimmedPrompt and trimmedFinalApiKey and ensure you pass the trimmedPrompt (not the original) onward (references: prompt, apiKey, headerApiKey, cookieApiKey, finalApiKey, trimmedPrompt).
🤖 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 `@apps/playground/src/components/playground/canvas-page-client.tsx`:
- Around line 241-251: The state initialization and persistence for
selectedModel currently read/write localStorage (variables: selectedModel,
setSelectedModel, CANVAS_MODEL_KEY, DEFAULT_CANVAS_MODEL); replace that logic
with cookie-backed storage so SSR can read the preference: on init, read the
cookie named by CANVAS_MODEL_KEY (fall back to DEFAULT_CANVAS_MODEL) and set
selectedModel; when changing model via setSelectedModel, write the value to the
same cookie (handle cookie read/write errors similar to the existing try/catch).
Also update the equivalent logic referenced around the other occurrence (the
code at the later snippet lines) to use the same cookie-based read/write
approach. Ensure cookie lifecycle (expires/path) follows repo standards for
user-settings cookies.
- Around line 288-313: Replace the inline fetch inside the useEffect with a
TanStack Query mutation: create a mutation using your api provider (e.g., const
ensureKeyMutation = api.playground.ensurePlaygroundKey.useMutation()) and call
ensureKeyMutation.mutate({ projectId: selectedProject.id }) instead of fetch
inside ensureKey; onSuccess set ensuredProjectRef.current = selectedProject.id
and handle errors via onError (instead of the empty catch). Keep the useEffect
to gate invocation by isAuthenticated/selectedOrganization/selectedProject but
remove raw fetch and the ensureKey async implementation, using the mutation's
mutate/mutateAsync and callbacks to manage side effects for the symbols
ensureKey, ensuredProjectRef, and the "/api/ensure-playground-key" behavior.
---
Outside diff comments:
In `@apps/playground/src/app/api/canvas/generate/route.ts`:
- Around line 34-55: Trim and validate the incoming prompt and API key values
before use: call .trim() on the incoming prompt (the local variable prompt) and
on apiKey/headerApiKey/cookieApiKey before computing finalApiKey, then reject if
the trimmed prompt is empty or the trimmed finalApiKey is empty; update the
checks that currently return 400 to use the trimmedPrompt and trimmedFinalApiKey
and ensure you pass the trimmedPrompt (not the original) onward (references:
prompt, apiKey, headerApiKey, cookieApiKey, finalApiKey, trimmedPrompt).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 136eedb1-4ae8-4171-b481-49b318ce928b
📒 Files selected for processing (4)
apps/playground/src/app/api/canvas/generate/route.tsapps/playground/src/components/playground/canvas-page-client.tsxapps/playground/src/components/playground/chat-ui.tsxapps/playground/src/lib/utils.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/playground/src/components/playground/canvas-page-client.tsx`:
- Around line 293-306: The ensureKey function currently treats any resolved
fetch as success and sets ensuredProjectRef.current regardless of status and
request ordering; change it to check the fetch response (response.ok) before
marking ensuredProjectRef.current = selectedProject.id, and add an ordering
guard (e.g., a local requestId or AbortController tied to ensureKey) so that
only the latest request's successful response can update ensuredProjectRef;
ensure you reference ensureKey, ensuredProjectRef, selectedProject.id and the
POST to "/api/ensure-playground-key" when implementing these checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 179b9efb-c36a-4d72-bc81-35f7a70976be
📒 Files selected for processing (10)
apps/playground/src/app/api/canvas/generate/route.tsapps/playground/src/app/canvas/page.tsxapps/playground/src/app/image/page.tsxapps/playground/src/app/playground-shell.tsxapps/playground/src/app/video/page.tsxapps/playground/src/components/playground/canvas-page-client.tsxapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/image-page-client.tsxapps/playground/src/components/playground/video-page-client.tsxapps/playground/src/lib/model-preferences.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground/src/app/api/canvas/generate/route.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/playground/src/components/playground/video-page-client.tsx (1)
242-269:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the stale
selectedProjectcheck with a cancellation flag.The comparison
selectedProject.id === projectIddoesn't prevent stale updates becauseprojectIdis captured before the async operation. A response from a previous project selection can still arrive after the user changes projects, causingensuredProjectRef.currentto be overwritten with an outdated project ID. Use a cancellation flag cleared in the effect's cleanup function instead.Suggested fix
useEffect(() => { if (!isAuthenticated || !selectedProject) { ensuredProjectRef.current = null; return; } + let cancelled = false; const ensureKey = async () => { if (!selectedOrganization) { return; } const projectId = selectedProject.id; if (ensuredProjectRef.current === projectId) { return; } try { const response = await fetch("/api/ensure-playground-key", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ projectId }), }); - if (response.ok && selectedProject.id === projectId) { + if (!cancelled && response.ok) { ensuredProjectRef.current = projectId; } } catch { // ignore } }; void ensureKey(); + return () => { + cancelled = true; + }; }, [isAuthenticated, selectedOrganization, selectedProject]);🤖 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 `@apps/playground/src/components/playground/video-page-client.tsx` around lines 242 - 269, The effect's async ensureKey captures projectId and can race with later selections, so replace the stale selectedProject.id check with a local cancellation flag: inside the useEffect create let cancelled = false and set cancelled = true in the cleanup, then in the async function ensureKey (and before setting ensuredProjectRef.current) check if cancelled and return early; keep existing guards (isAuthenticated, selectedOrganization) and refer to ensuredProjectRef, ensureKey, selectedProject, selectedOrganization, and isAuthenticated to locate the logic to change.apps/playground/src/components/playground/image-page-client.tsx (1)
170-197:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a cleanup flag to prevent stale async requests from updating the ref after project switches.
The
response.ok && selectedProject.id === projectIdcheck doesn't prevent updates from stale responses because bothselectedProjectandprojectIdare captured in theensureKeyclosure when the effect runs. When a user switches projects mid-request, the old async operation completes with its captured values and still updatesensuredProjectRef.currentwith the outdated project ID.Introduce a
cancelledflag that the cleanup function sets totrue, preventing stale requests from updating the ref:Suggested fix
useEffect(() => { if (!isAuthenticated || !selectedProject) { ensuredProjectRef.current = null; return; } + let cancelled = false; const ensureKey = async () => { if (!selectedOrganization) { return; } const projectId = selectedProject.id; if (ensuredProjectRef.current === projectId) { return; } try { const response = await fetch("/api/ensure-playground-key", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ projectId }), }); - if (response.ok && selectedProject.id === projectId) { + if (!cancelled && response.ok) { ensuredProjectRef.current = projectId; } } catch { // ignore } }; void ensureKey(); + return () => { + cancelled = true; + }; }, [isAuthenticated, selectedOrganization, selectedProject]);🤖 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 `@apps/playground/src/components/playground/image-page-client.tsx` around lines 170 - 197, The effect in useEffect that runs ensureKey can let stale async responses overwrite ensuredProjectRef.current; add a local cancelled flag in the useEffect scope, set cancelled = true in the returned cleanup function, and inside ensureKey check the flag before writing ensuredProjectRef.current (and after awaiting fetch/response.ok). Update references in the closure (ensureKey, ensuredProjectRef.current, selectedProject, selectedOrganization) so the async callback returns early when cancelled to avoid updating the ref for a switched project.apps/playground/src/components/playground/chat-page-client.tsx (1)
916-946:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse AbortController or cleanup cancellation to prevent stale requests from overwriting state.
When
selectedProjectchanges while the async request is pending, the older request can still complete and updateensuredProjectRef.currenteven after switching projects. The comparisonselectedProject.id === projectIddoesn't fully protect against this because both references exist in the same closure.The suggested fix correctly prevents stale responses by cancelling pending operations in the cleanup function:
Suggested fix
useEffect(() => { // Reset ref when user logs out or project is unset if (!isAuthenticated || !selectedProject) { ensuredProjectRef.current = null; return; } + let cancelled = false; const ensureKey = async () => { if (!selectedOrganization) { return; } const projectId = selectedProject.id; // Skip if we've already ensured the key for this project if (ensuredProjectRef.current === projectId) { return; } try { const response = await fetch("/api/ensure-playground-key", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ projectId }), }); - if (response.ok && selectedProject.id === projectId) { + if (!cancelled && response.ok) { ensuredProjectRef.current = projectId; } } catch { // ignore for now } }; void ensureKey(); + return () => { + cancelled = true; + }; }, [isAuthenticated, selectedOrganization, selectedProject]);🤖 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 `@apps/playground/src/components/playground/chat-page-client.tsx` around lines 916 - 946, The ensureKey effect can let a stale fetch resolve and overwrite ensuredProjectRef.current; update the useEffect that defines ensureKey to use an AbortController (or a cancellation flag) and pass its signal to fetch, and in the cleanup return call controller.abort() (or flip the flag) so any late response is ignored; also ensure you check controller.signal.aborted (or the flag) before writing ensuredProjectRef.current and keep references to selectedProject.id via the captured projectId inside ensureKey to avoid cross-request races (functions: useEffect, ensureKey, ensuredProjectRef, selectedProject, selectedOrganization, isAuthenticated).
🤖 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.
Outside diff comments:
In `@apps/playground/src/components/playground/chat-page-client.tsx`:
- Around line 916-946: The ensureKey effect can let a stale fetch resolve and
overwrite ensuredProjectRef.current; update the useEffect that defines ensureKey
to use an AbortController (or a cancellation flag) and pass its signal to fetch,
and in the cleanup return call controller.abort() (or flip the flag) so any late
response is ignored; also ensure you check controller.signal.aborted (or the
flag) before writing ensuredProjectRef.current and keep references to
selectedProject.id via the captured projectId inside ensureKey to avoid
cross-request races (functions: useEffect, ensureKey, ensuredProjectRef,
selectedProject, selectedOrganization, isAuthenticated).
In `@apps/playground/src/components/playground/image-page-client.tsx`:
- Around line 170-197: The effect in useEffect that runs ensureKey can let stale
async responses overwrite ensuredProjectRef.current; add a local cancelled flag
in the useEffect scope, set cancelled = true in the returned cleanup function,
and inside ensureKey check the flag before writing ensuredProjectRef.current
(and after awaiting fetch/response.ok). Update references in the closure
(ensureKey, ensuredProjectRef.current, selectedProject, selectedOrganization) so
the async callback returns early when cancelled to avoid updating the ref for a
switched project.
In `@apps/playground/src/components/playground/video-page-client.tsx`:
- Around line 242-269: The effect's async ensureKey captures projectId and can
race with later selections, so replace the stale selectedProject.id check with a
local cancellation flag: inside the useEffect create let cancelled = false and
set cancelled = true in the cleanup, then in the async function ensureKey (and
before setting ensuredProjectRef.current) check if cancelled and return early;
keep existing guards (isAuthenticated, selectedOrganization) and refer to
ensuredProjectRef, ensureKey, selectedProject, selectedOrganization, and
isAuthenticated to locate the logic to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a091c957-183a-4688-8c11-da725f327378
📒 Files selected for processing (5)
apps/playground/src/components/playground/canvas-page-client.tsxapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/image-page-client.tsxapps/playground/src/components/playground/video-page-client.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground/src/components/playground/canvas-page-client.tsx
smakosh
approved these changes
May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements