update main from test#1603
Conversation
Integrates react-dropzone drag-and-drop with the /files page to upload files to GitHub sandbox repos via POST /api/sandboxes/files. Shows upload overlay, progress state, and toast notifications on completion. Co-Authored-By: Paperclip <noreply@paperclip.ing>
migrate to dedicated api for chat endpoints
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRemoved local chat API routes and proxy utility; updated client chat hook to call external chats API with access tokens. Added a sandbox drag-and-drop upload flow: new upload API, client components, upload hook, and helper libs to upload files to Vercel Blob and notify backend. Changes
Sequence DiagramsequenceDiagram
participant Client
participant DropZone as "DropZone UI"
participant Hook as "useSandboxFileDrop"
participant Blob as "Vercel Blob Upload\n(`@vercel/blob`)"
participant Backend as "External API\n/ api/sandboxes/files"
rect rgba(100,150,240,0.5)
Note over Client,Backend: Sandbox upload flow
Client->>DropZone: user drops files
DropZone->>Hook: handleFilesDropped(files)
Hook->>Client: fetch access token (usePrivy)
Hook->>Blob: upload each file (clientPayload includes token)
Blob-->>Hook: returns blob URLs
Hook->>Backend: POST /api/sandboxes/files + Authorization: Bearer <token> with blob refs
Backend-->>Hook: returns status (success|error)
Hook-->>Client: resolve mutation, show toasts and refetch filetree
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
- Add /api/sandbox-upload route for generating presigned blob tokens - Update uploadSandboxFiles to upload to Vercel Blob first, then pass blob URLs to the API (bypasses 4.5MB serverless limit) - Add @vercel/blob dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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)
hooks/useCreateChat.tsx (1)
95-103:⚠️ Potential issue | 🟠 MajorGuard
createChatto prevent duplicate POST writes.This effect can rerun while
isOptimisticChatItemis still true (dependency identity changes), which can trigger duplicatePOST /api/chatscalls. Add a one-shot/idempotency guard per chat id.🔧 Suggested fix
-import { useEffect } from "react"; +import { useEffect, useRef } from "react"; ... + const createdChatIdsRef = useRef<Set<string>>(new Set()); const accessToken = useAccessToken(); useEffect(() => { if (!isOptimisticChatItem || !accessToken) return; + const chatId = (chatRoom as Conversation).id; + if (!chatId || createdChatIdsRef.current.has(chatId)) return; + createdChatIdsRef.current.add(chatId); const createChat = async () => { try { ... - chatId: (chatRoom as Conversation).id, + chatId, ... - createChat(); + void createChat(); }, [ isOptimisticChatItem, - chatRoom, + (chatRoom as Conversation).id, selectedArtist?.account_id, accessToken, setDisplayName, refetchConversations, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useCreateChat.tsx` around lines 95 - 103, The effect calling createChat in useCreateChat can fire multiple times and cause duplicate POST /api/chats calls; add a one-shot/idempotency guard keyed by chat id (e.g., chatRoom.id or composed from chatRoom + selectedArtist?.account_id) using a module-level or hook-level ref/Set (e.g., createdChatIds or inFlightChatIds) and check it before invoking createChat, mark the id as in-flight/created immediately when calling, and only clear/reset on a determinative failure path; ensure the guard is consulted in the effect dependencies (where isOptimisticChatItem, chatRoom, selectedArtist?.account_id, accessToken, setDisplayName, refetchConversations are used) so duplicate POSTs are prevented while preserving retry on real failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useCreateChat.tsx`:
- Line 27: The early return on "if (!isOptimisticChatItem || !accessToken)
return;" silently abandons optimistic chats when token retrieval fails; change
this to explicitly handle the missing-token case by detecting the error path
from useAccessToken (e.g., accessToken === null or an error flag) and
dispatching a reconciliation/failure for the optimistic item instead of
returning. Update the flow in useCreateChat.tsx so that when
isOptimisticChatItem is true but accessToken is missing you call the existing
optimistic reconciliation routine (or add a new
markChatFailed/rollbackOptimisticChat function) and surface an error (e.g., set
a failed state or show a toast) so the optimistic chat isn’t left hanging while
still proceeding when a valid accessToken exists.
---
Outside diff comments:
In `@hooks/useCreateChat.tsx`:
- Around line 95-103: The effect calling createChat in useCreateChat can fire
multiple times and cause duplicate POST /api/chats calls; add a
one-shot/idempotency guard keyed by chat id (e.g., chatRoom.id or composed from
chatRoom + selectedArtist?.account_id) using a module-level or hook-level
ref/Set (e.g., createdChatIds or inFlightChatIds) and check it before invoking
createChat, mark the id as in-flight/created immediately when calling, and only
clear/reset on a determinative failure path; ensure the guard is consulted in
the effect dependencies (where isOptimisticChatItem, chatRoom,
selectedArtist?.account_id, accessToken, setDisplayName, refetchConversations
are used) so duplicate POSTs are prevented while preserving retry on real
failures.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3257bc5-37ab-4a4e-be17-8de75627c926
⛔ Files ignored due to path filters (2)
lib/chat/__tests__/proxyToApiChat.test.tsis excluded by!**/*.test.*and included bylib/**types/Chat.tsxis excluded by none and included by none
📒 Files selected for processing (5)
app/api/chat/create/route.tsapp/api/chat/generate/route.tsapp/api/chat/route.tshooks/useCreateChat.tsxlib/chat/proxyToApiChat.ts
💤 Files with no reviewable changes (4)
- app/api/chat/route.ts
- app/api/chat/generate/route.ts
- app/api/chat/create/route.ts
- lib/chat/proxyToApiChat.ts
|
|
||
| useEffect(() => { | ||
| if (!isOptimisticChatItem) return; | ||
| if (!isOptimisticChatItem || !accessToken) return; |
There was a problem hiding this comment.
Handle missing token with an explicit failure path.
At Line 27, returning on !accessToken silently strands optimistic chats when token retrieval fails (null is used for both “not loaded yet” and “error” in useAccessToken). Add an explicit error/reconciliation path so optimistic state is not left hanging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useCreateChat.tsx` at line 27, The early return on "if
(!isOptimisticChatItem || !accessToken) return;" silently abandons optimistic
chats when token retrieval fails; change this to explicitly handle the
missing-token case by detecting the error path from useAccessToken (e.g.,
accessToken === null or an error flag) and dispatching a reconciliation/failure
for the optimistic item instead of returning. Update the flow in
useCreateChat.tsx so that when isOptimisticChatItem is true but accessToken is
missing you call the existing optimistic reconciliation routine (or add a new
markChatFailed/rollbackOptimisticChat function) and surface an error (e.g., set
a failed state or show a toast) so the optimistic chat isn’t left hanging while
still proceeding when a valid accessToken exists.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 466550d50e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| useEffect(() => { | ||
| if (!isOptimisticChatItem) return; | ||
| if (!isOptimisticChatItem || !accessToken) return; |
There was a problem hiding this comment.
Persist optimistic chats without waiting on access token
This early return can drop newly-created optimistic chats when the user sends a message before Privy token hydration completes. handleSendMessage adds optimistic items immediately, but this hook now does nothing until accessToken exists; meanwhile conversation data is keyed by token, so when the key flips from null to a token the optimistic entry is replaced and the create call never runs. In that flow, the new chat can disappear from Recent Chats instead of being persisted via /api/chats.
Useful? React with 👍 / 👎.
- Extract file drop logic to useSandboxFileDrop hook (OCP) - Extract drag-n-drop wrapper to SandboxDropZone component (OCP) - Move route from sandbox-upload to sandbox/upload (KISS) - Add auth check to blob upload endpoint - Move JSON parsing inside try-catch - Fix file vs directory detection using FileNode.type instead of dot heuristic - Parallelize blob uploads with Promise.all - Add ARIA labels for accessibility - Use cn() utility for class merging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract NoSandboxFiles to its own component (SRP) - Extract findFileNode to lib/sandboxes/findFileNode.ts (SRP) - Use tanstack useMutation in useSandboxFileDrop for cleaner state management Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The blob client controls its own request headers, so we validate the access token passed via clientPayload in onBeforeGenerateToken. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…chat feat: drag-and-drop file upload for sandbox file tree
Summary by CodeRabbit
New Features
Refactor