-
-
Notifications
You must be signed in to change notification settings - Fork 7
Add message edit and delete functionality #467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add message edit and delete functionality #467
Conversation
- Implement `updateMessage`, `deleteMessage`, and `deleteTrailingMessages` in database layer. - Refactor AI generation logic in `app/actions.tsx` into a reusable `processChatWorkflow` function. - Add `resubmit` and `deleteMessageAction` server actions. - Enhance `UserMessage` component with inline editing and deletion UI. - Ensure proper authentication using `getCurrentUserIdOnServer` and state management for trailing message deletion. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughAdds per-message IDs and createdAt timestamps, implements resubmit and delete message actions wired to DB update/delete/trailing-delete helpers, centralizes chat processing into processChatWorkflow, exposes onGetUIState/onSetAIState hooks, and extends model selection with a useSpecificAPI flag. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as "UserMessage (client)"
participant Panel as "ChatPanel / Followup (client)"
participant Server as "app/actions"
participant DB as "lib/actions/chat-db"
User->>UI: Click Edit -> open editor
UI->>Panel: Save (truncate UI state up to message)
Panel->>Server: resubmit(messageId, content, mapProvider)
Server->>DB: deleteTrailingMessages(chatId, message.createdAt)
DB-->>Server: OK
Server->>DB: updateMessage(messageId, { content, createdAt })
DB-->>Server: updated message
Server->>Server: processChatWorkflow(re-run AI flow)
Server-->>Panel: updated AI/UI state
Panel->>UI: render updated conversation
User->>UI: Click Delete
UI->>Panel: confirm & call deleteMessageAction(messageId)
Panel->>Server: deleteMessageAction(messageId)
Server->>DB: deleteMessage(messageId)
DB-->>Server: deleted
Server->>DB: deleteTrailingMessages(chatId, message.createdAt)
DB-->>Server: OK
Server-->>Panel: updated AI/UI state
Panel->>UI: render truncated conversation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 472-488: The loop over toolOutputs is using map for side effects
which is incorrect; replace the map call with forEach on toolOutputs and keep
the body that calls aiState.update (referencing aiState.update, toolOutputs, and
groupeId) so no value is expected or returned—this removes the unused array
creation and makes the intent clear.
- Around line 588-594: The code currently mutates the message object by doing
truncatedMessages[index].content = content which also mutates the original
messages from aiState.get(); instead create an immutable update by copying the
messages array and the specific message object before changing content — e.g.,
build a newMessages array (from messages.slice or messages.map), replace the
entry at index with a shallow-copied message object that has content set to the
new value, then call aiState.update({...aiState.get(), messages: newMessages});
update references: truncatedMessages, messages, index, and
aiState.update/aiState.get().
- Around line 645-667: Wrap the body of deleteMessageAction in a try/catch, call
deleteTrailingMessages and deleteMessage inside the try, and only call
aiState.done to truncate messages after deleteMessage resolves successfully; in
the catch return a failure indicator (e.g., false) and optionally log or rethrow
the error, while on success return true. Specifically, update
deleteMessageAction to return a boolean (or result object), use
getMutableAIState/aiState.get as before but move aiState.done into the success
path after await deleteMessage(messageId), and handle errors from
deleteTrailingMessages/deleteMessage to avoid updating client state when server
deletion fails.
- Around line 410-421: The parameter list for the function (aiState, uiStream,
isGenerating, isCollapsed, mapProvider, useSpecificAPI) currently uses `any` and
should be replaced with concrete types from the ai/rsc module and local types:
import and use the appropriate types (e.g., AiState, UIStream, MapProvider,
UseSpecificAPI flag or boolean) and replace `any` with those types; set
isGenerating/isCollapsed/skipTaskManager to boolean,
groupeId/currentSystemPrompt to string, messages to CoreMessage[] (already
present), and maxMessages to number; update the function signature in
app/actions.tsx to reference these imported types so the compiler enforces
correct shapes and remove all `any` usages for these parameters.
In `@components/user-message.tsx`:
- Around line 82-92: handleDelete currently mutates UI optimistically before
calling deleteMessageAction, causing UI/server inconsistency on failure; change
it to capture the prior messages (e.g., const prev = currentMessages snapshot),
then perform deleteMessageAction inside a try/catch and only update setMessages
on success, or if you prefer optimistic update keep the UI change but wrap
deleteMessageAction in try/catch and on failure call setMessages(prev) to roll
back and log/show the error; reference handleDelete, deleteMessageAction and
setMessages when making this change.
- Around line 67-80: handleSave currently truncates messages then awaits
resubmit(id, editContent, mapProvider) without error handling, which can leave
the UI inconsistent if resubmit fails or returns null; modify handleSave to (1)
capture and keep a copy of the original messages before calling
setMessages(truncate) so you can rollback on error, (2) introduce a loading flag
(e.g., setIsSaving) and set it true before the async call and false in finally,
(3) wrap the resubmit call in try/catch, and if the response is null or an
exception occurs restore the original messages and reset editing state (use
setMessages with the saved original and setIsEditing(true) or show an error),
and (4) only append the response to messages with
setMessages([...currentMessages, response]) when response is valid; reference
handleSave, resubmit, setMessages, setIsEditing, editContent, id, and
mapProvider.
In `@lib/actions/chat-db.ts`:
- Around line 232-247: The updateMessage function currently allows updates by
messageId only; add a userId parameter (e.g., updateMessage(messageId: string,
userId: string, updates: Partial<NewMessage>)) and include it in the WHERE
clause (e.g., where eq(messages.id, messageId) AND eq(messages.userId, userId))
so only the owner can update; ensure the function still returns the updated
Message or null when no row is returned and keep the existing error handling in
the catch block.
- Around line 254-265: The deleteMessage function currently deletes without
authorization; change its signature to accept a userId parameter and enforce
ownership by including an additional WHERE clause that matches messages.userId
(or the appropriate owner column) to the passed userId before deleting (similar
to updateMessage). Ensure the deletion query only runs when both eq(messages.id,
messageId) and eq(messages.userId, userId) are applied, and return false (or
handle) if no rows are affected; also update any callers to pass the userId.
- Around line 272-283: deleteTrailingMessages currently deletes without checking
that the requester owns the chat and it swallows errors; update
deleteTrailingMessages to accept a userId (or callerId) parameter, first query
the chats table (e.g., using the chats model / getChatById logic) to confirm the
chat's ownerId matches userId and throw an AuthorizationError if not, then
perform the delete on messages only if authorized; also remove console.error
swallowing and propagate errors (rethrow or return a failure result) so callers
can handle deletion failures instead of silently returning void. Ensure you
reference the existing deleteTrailingMessages function and the messages/chats
models when making these changes.
In `@lib/actions/chat.ts`:
- Around line 168-192: Add server-side authorization to the wrapper functions by
calling getCurrentUserIdOnServer() at the start of updateMessage, deleteMessage,
and deleteTrailingMessages and verifying the current user is the owner of the
target resource before calling dbUpdateMessage, dbDeleteMessage, or
dbDeleteTrailingMessages; perform a minimal lookup (e.g., fetch message/chat
owner via a helper like dbGetMessageById or dbGetChatById) and if the owner !==
currentUserId return null/false/void (or throw an authorization error) instead
of performing the DB operation. Ensure the ownership check runs in updateMessage
(using messageId), deleteMessage (using messageId), and deleteTrailingMessages
(using chatId) so these wrappers enforce access control regardless of callers.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
server.logis excluded by!**/*.log
📒 Files selected for processing (4)
app/actions.tsxcomponents/user-message.tsxlib/actions/chat-db.tslib/actions/chat.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/actions/chat-db.ts (3)
lib/db/schema.ts (3)
messages(26-37)chat(69-78)chats(43-43)lib/actions/chat.ts (3)
updateMessage(168-175)deleteMessage(177-184)deleteTrailingMessages(186-192)lib/db/index.ts (1)
db(25-25)
lib/actions/chat.ts (1)
lib/actions/chat-db.ts (3)
updateMessage(232-247)deleteMessage(254-265)deleteTrailingMessages(272-283)
🪛 Biome (2.3.13)
app/actions.tsx
[error] 473-473: This callback passed to map() iterable method should always return a value.
Add a return with a value to this callback.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (7)
lib/actions/chat.ts (1)
13-15: LGTM!The imports are correctly added for the new database functions.
components/user-message.tsx (2)
1-22: LGTM!The
'use client'directive is correctly added, and the imports are appropriate for the new editing/deleting functionality with proper UI components.
108-161: LGTM!The editing and deletion UI is well-implemented:
- Hover-to-reveal pattern for non-intrusive action buttons
- Proper confirmation dialog for the destructive delete action with clear warning about subsequent messages
- Conditional rendering based on
idpresence ensures actions are only available when the message can be identifiedapp/actions.tsx (3)
19-25: LGTM!The imports for the new chat management functions are correctly added.
700-705: LGTM!The new actions
resubmitanddeleteMessageActionare correctly exported alongside existing actions.
816-822: LGTM!The
idprop is correctly passed toUserMessage, enabling the edit/delete functionality in the UI.lib/actions/chat-db.ts (1)
122-132: The upsert logic is correct. The schema confirms thatcreatedAtis defined astimestamp('created_at', ...)at line 32 of lib/db/schema.ts, sosql\EXCLUDED.created_at`` properly references the actual PostgreSQL column name.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
app/actions.tsx
Outdated
| if (toolOutputs.length > 0) { | ||
| toolOutputs.map(output => { | ||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: [ | ||
| ...aiState.get().messages, | ||
| { | ||
| id: groupeId, | ||
| role: 'tool', | ||
| content: JSON.stringify(output.result), | ||
| name: output.toolName, | ||
| type: 'tool' | ||
| } | ||
| ] | ||
| }) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use forEach instead of map for side effects.
The map() callback doesn't return a value, which is a code smell. Since this loop is used purely for side effects (updating aiState), use forEach() instead.
🔧 Proposed fix
if (toolOutputs.length > 0) {
- toolOutputs.map(output => {
+ toolOutputs.forEach(output => {
aiState.update({
...aiState.get(),
messages: [
...aiState.get().messages,
{
id: groupeId,
role: 'tool',
content: JSON.stringify(output.result),
name: output.toolName,
type: 'tool'
}
]
})
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (toolOutputs.length > 0) { | |
| toolOutputs.map(output => { | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: groupeId, | |
| role: 'tool', | |
| content: JSON.stringify(output.result), | |
| name: output.toolName, | |
| type: 'tool' | |
| } | |
| ] | |
| }) | |
| } | |
| }) | |
| } | |
| if (toolOutputs.length > 0) { | |
| toolOutputs.forEach(output => { | |
| aiState.update({ | |
| ...aiState.get(), | |
| messages: [ | |
| ...aiState.get().messages, | |
| { | |
| id: groupeId, | |
| role: 'tool', | |
| content: JSON.stringify(output.result), | |
| name: output.toolName, | |
| type: 'tool' | |
| } | |
| ] | |
| }) | |
| }) | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 473-473: This callback passed to map() iterable method should always return a value.
Add a return with a value to this callback.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 472 - 488, The loop over toolOutputs is using
map for side effects which is incorrect; replace the map call with forEach on
toolOutputs and keep the body that calls aiState.update (referencing
aiState.update, toolOutputs, and groupeId) so no value is expected or
returned—this removes the unused array creation and makes the intent clear.
| async function deleteMessageAction(messageId: string) { | ||
| 'use server' | ||
|
|
||
| const aiState = getMutableAIState<typeof AI>() | ||
| const messages = aiState.get().messages | ||
| const index = messages.findIndex(m => m.id === messageId) | ||
|
|
||
| if (index !== -1) { | ||
| const messageToDelete = messages[index] | ||
| const chatId = aiState.get().chatId | ||
|
|
||
| if (messageToDelete.createdAt) { | ||
| await deleteTrailingMessages(chatId, new Date(messageToDelete.createdAt)) | ||
| } | ||
| await deleteMessage(messageId) | ||
|
|
||
| const truncatedMessages = messages.slice(0, index) | ||
| aiState.done({ | ||
| ...aiState.get(), | ||
| messages: truncatedMessages | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add return value and error handling.
The function doesn't return a success/failure indicator, making it impossible for the client to handle errors properly. If deleteMessage() fails, the AI state is still updated, leading to server/client inconsistency.
🛡️ Proposed fix
-async function deleteMessageAction(messageId: string) {
+async function deleteMessageAction(messageId: string): Promise<{ success: boolean }> {
'use server'
const aiState = getMutableAIState<typeof AI>()
const messages = aiState.get().messages
const index = messages.findIndex(m => m.id === messageId)
if (index !== -1) {
const messageToDelete = messages[index]
const chatId = aiState.get().chatId
if (messageToDelete.createdAt) {
await deleteTrailingMessages(chatId, new Date(messageToDelete.createdAt))
}
- await deleteMessage(messageId)
+ const deleted = await deleteMessage(messageId)
+ if (!deleted) {
+ return { success: false }
+ }
const truncatedMessages = messages.slice(0, index)
aiState.done({
...aiState.get(),
messages: truncatedMessages
})
+ return { success: true }
}
+ return { success: false }
}🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 645 - 667, Wrap the body of deleteMessageAction
in a try/catch, call deleteTrailingMessages and deleteMessage inside the try,
and only call aiState.done to truncate messages after deleteMessage resolves
successfully; in the catch return a failure indicator (e.g., false) and
optionally log or rethrow the error, while on success return true. Specifically,
update deleteMessageAction to return a boolean (or result object), use
getMutableAIState/aiState.get as before but move aiState.done into the success
path after await deleteMessage(messageId), and handle errors from
deleteTrailingMessages/deleteMessage to avoid updating client state when server
deletion fails.
| const handleSave = async () => { | ||
| if (!id || !editContent.trim()) return | ||
|
|
||
| setIsEditing(false) | ||
|
|
||
| // Truncate UI state | ||
| setMessages(currentMessages => { | ||
| const index = currentMessages.findIndex(m => m.id === id) | ||
| return currentMessages.slice(0, index + 1) | ||
| }) | ||
|
|
||
| const response = await resubmit(id, editContent, mapProvider) | ||
| setMessages(currentMessages => [...currentMessages, response]) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for failed resubmit.
If resubmit() fails or returns a null component, the UI will be in an inconsistent state: trailing messages are already removed from UI state, but no new response is appended. Consider:
- Storing the original messages before truncation for rollback
- Handling the case where
responseis null/invalid - Adding a loading state during the async operation
🛡️ Proposed fix with error handling
const handleSave = async () => {
if (!id || !editContent.trim()) return
setIsEditing(false)
+ // Store original state for potential rollback
+ let originalMessages: typeof currentMessages;
+ setMessages(currentMessages => {
+ originalMessages = currentMessages;
+ const index = currentMessages.findIndex(m => m.id === id)
+ return currentMessages.slice(0, index + 1)
+ })
- // Truncate UI state
- setMessages(currentMessages => {
- const index = currentMessages.findIndex(m => m.id === id)
- return currentMessages.slice(0, index + 1)
- })
- const response = await resubmit(id, editContent, mapProvider)
- setMessages(currentMessages => [...currentMessages, response])
+ try {
+ const response = await resubmit(id, editContent, mapProvider)
+ if (response?.component) {
+ setMessages(currentMessages => [...currentMessages, response])
+ } else {
+ // Rollback on failure
+ setMessages(originalMessages!)
+ }
+ } catch (error) {
+ console.error('Failed to resubmit message:', error)
+ setMessages(originalMessages!)
+ }
}🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 67 - 80, handleSave currently
truncates messages then awaits resubmit(id, editContent, mapProvider) without
error handling, which can leave the UI inconsistent if resubmit fails or returns
null; modify handleSave to (1) capture and keep a copy of the original messages
before calling setMessages(truncate) so you can rollback on error, (2) introduce
a loading flag (e.g., setIsSaving) and set it true before the async call and
false in finally, (3) wrap the resubmit call in try/catch, and if the response
is null or an exception occurs restore the original messages and reset editing
state (use setMessages with the saved original and setIsEditing(true) or show an
error), and (4) only append the response to messages with
setMessages([...currentMessages, response]) when response is valid; reference
handleSave, resubmit, setMessages, setIsEditing, editContent, id, and
mapProvider.
| const handleDelete = async () => { | ||
| if (!id) return | ||
|
|
||
| // Truncate UI state | ||
| setMessages(currentMessages => { | ||
| const index = currentMessages.findIndex(m => m.id === id) | ||
| return currentMessages.slice(0, index) | ||
| }) | ||
|
|
||
| await deleteMessageAction(id) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for failed delete.
Similar to handleSave, if deleteMessageAction() fails, the UI state will be inconsistent with the server state. The messages are removed from UI before the server confirms deletion.
🛡️ Proposed fix
const handleDelete = async () => {
if (!id) return
+ let originalMessages: typeof currentMessages;
+ setMessages(currentMessages => {
+ originalMessages = currentMessages;
+ const index = currentMessages.findIndex(m => m.id === id)
+ return currentMessages.slice(0, index)
+ })
- // Truncate UI state
- setMessages(currentMessages => {
- const index = currentMessages.findIndex(m => m.id === id)
- return currentMessages.slice(0, index)
- })
- await deleteMessageAction(id)
+ try {
+ await deleteMessageAction(id)
+ } catch (error) {
+ console.error('Failed to delete message:', error)
+ setMessages(originalMessages!)
+ }
}🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 82 - 92, handleDelete currently
mutates UI optimistically before calling deleteMessageAction, causing UI/server
inconsistency on failure; change it to capture the prior messages (e.g., const
prev = currentMessages snapshot), then perform deleteMessageAction inside a
try/catch and only update setMessages on success, or if you prefer optimistic
update keep the UI change but wrap deleteMessageAction in try/catch and on
failure call setMessages(prev) to roll back and log/show the error; reference
handleDelete, deleteMessageAction and setMessages when making this change.
| export async function updateMessage( | ||
| messageId: string, | ||
| updates: Partial<NewMessage> | ||
| ): Promise<Message | null> { | ||
| try { | ||
| const result = await db | ||
| .update(messages) | ||
| .set(updates) | ||
| .where(eq(messages.id, messageId)) | ||
| .returning(); | ||
| return result[0] || null; | ||
| } catch (error) { | ||
| console.error('Error updating message:', error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing authorization check before updating message.
The updateMessage function modifies a message by ID without verifying that the requesting user owns the message. This could allow unauthorized modifications if the messageId is guessable or leaked.
Consider adding a userId parameter and including it in the WHERE clause for defense in depth:
🛡️ Proposed fix to add authorization
export async function updateMessage(
messageId: string,
- updates: Partial<NewMessage>
+ updates: Partial<NewMessage>,
+ userId?: string
): Promise<Message | null> {
try {
+ const whereClause = userId
+ ? and(eq(messages.id, messageId), eq(messages.userId, userId))
+ : eq(messages.id, messageId);
const result = await db
.update(messages)
.set(updates)
- .where(eq(messages.id, messageId))
+ .where(whereClause)
.returning();
return result[0] || null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function updateMessage( | |
| messageId: string, | |
| updates: Partial<NewMessage> | |
| ): Promise<Message | null> { | |
| try { | |
| const result = await db | |
| .update(messages) | |
| .set(updates) | |
| .where(eq(messages.id, messageId)) | |
| .returning(); | |
| return result[0] || null; | |
| } catch (error) { | |
| console.error('Error updating message:', error); | |
| return null; | |
| } | |
| } | |
| export async function updateMessage( | |
| messageId: string, | |
| updates: Partial<NewMessage>, | |
| userId?: string | |
| ): Promise<Message | null> { | |
| try { | |
| const whereClause = userId | |
| ? and(eq(messages.id, messageId), eq(messages.userId, userId)) | |
| : eq(messages.id, messageId); | |
| const result = await db | |
| .update(messages) | |
| .set(updates) | |
| .where(whereClause) | |
| .returning(); | |
| return result[0] || null; | |
| } catch (error) { | |
| console.error('Error updating message:', error); | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@lib/actions/chat-db.ts` around lines 232 - 247, The updateMessage function
currently allows updates by messageId only; add a userId parameter (e.g.,
updateMessage(messageId: string, userId: string, updates: Partial<NewMessage>))
and include it in the WHERE clause (e.g., where eq(messages.id, messageId) AND
eq(messages.userId, userId)) so only the owner can update; ensure the function
still returns the updated Message or null when no row is returned and keep the
existing error handling in the catch block.
| export async function deleteMessage(messageId: string): Promise<boolean> { | ||
| try { | ||
| const result = await db | ||
| .delete(messages) | ||
| .where(eq(messages.id, messageId)) | ||
| .returning({ id: messages.id }); | ||
| return result.length > 0; | ||
| } catch (error) { | ||
| console.error('Error deleting message:', error); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing authorization check before deleting message.
Similar to updateMessage, this function deletes a message without verifying ownership. Add a userId parameter for authorization.
🛡️ Proposed fix to add authorization
-export async function deleteMessage(messageId: string): Promise<boolean> {
+export async function deleteMessage(messageId: string, userId?: string): Promise<boolean> {
try {
+ const whereClause = userId
+ ? and(eq(messages.id, messageId), eq(messages.userId, userId))
+ : eq(messages.id, messageId);
const result = await db
.delete(messages)
- .where(eq(messages.id, messageId))
+ .where(whereClause)
.returning({ id: messages.id });
return result.length > 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function deleteMessage(messageId: string): Promise<boolean> { | |
| try { | |
| const result = await db | |
| .delete(messages) | |
| .where(eq(messages.id, messageId)) | |
| .returning({ id: messages.id }); | |
| return result.length > 0; | |
| } catch (error) { | |
| console.error('Error deleting message:', error); | |
| return false; | |
| } | |
| } | |
| export async function deleteMessage(messageId: string, userId?: string): Promise<boolean> { | |
| try { | |
| const whereClause = userId | |
| ? and(eq(messages.id, messageId), eq(messages.userId, userId)) | |
| : eq(messages.id, messageId); | |
| const result = await db | |
| .delete(messages) | |
| .where(whereClause) | |
| .returning({ id: messages.id }); | |
| return result.length > 0; | |
| } catch (error) { | |
| console.error('Error deleting message:', error); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@lib/actions/chat-db.ts` around lines 254 - 265, The deleteMessage function
currently deletes without authorization; change its signature to accept a userId
parameter and enforce ownership by including an additional WHERE clause that
matches messages.userId (or the appropriate owner column) to the passed userId
before deleting (similar to updateMessage). Ensure the deletion query only runs
when both eq(messages.id, messageId) and eq(messages.userId, userId) are
applied, and return false (or handle) if no rows are affected; also update any
callers to pass the userId.
| export async function deleteTrailingMessages( | ||
| chatId: string, | ||
| createdAt: Date | ||
| ): Promise<void> { | ||
| try { | ||
| await db | ||
| .delete(messages) | ||
| .where(and(eq(messages.chatId, chatId), gt(messages.createdAt, createdAt))); | ||
| } catch (error) { | ||
| console.error('Error deleting trailing messages:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing authorization and silent error handling.
- No verification that the user owns the chat before deleting messages.
- Errors are logged but silently swallowed (function returns
void), making it difficult for callers to know if deletion succeeded.
🛡️ Proposed fix
export async function deleteTrailingMessages(
chatId: string,
- createdAt: Date
-): Promise<void> {
+ createdAt: Date,
+ userId?: string
+): Promise<boolean> {
try {
+ // Optionally verify chat ownership first if userId provided
+ if (userId) {
+ const chat = await db.select({ id: chats.id }).from(chats)
+ .where(and(eq(chats.id, chatId), eq(chats.userId, userId))).limit(1);
+ if (!chat.length) return false;
+ }
await db
.delete(messages)
.where(and(eq(messages.chatId, chatId), gt(messages.createdAt, createdAt)));
+ return true;
} catch (error) {
console.error('Error deleting trailing messages:', error);
+ return false;
}
}🤖 Prompt for AI Agents
In `@lib/actions/chat-db.ts` around lines 272 - 283, deleteTrailingMessages
currently deletes without checking that the requester owns the chat and it
swallows errors; update deleteTrailingMessages to accept a userId (or callerId)
parameter, first query the chats table (e.g., using the chats model /
getChatById logic) to confirm the chat's ownerId matches userId and throw an
AuthorizationError if not, then perform the delete on messages only if
authorized; also remove console.error swallowing and propagate errors (rethrow
or return a failure result) so callers can handle deletion failures instead of
silently returning void. Ensure you reference the existing
deleteTrailingMessages function and the messages/chats models when making these
changes.
| export async function updateMessage(messageId: string, content: string): Promise<DrizzleMessage | null> { | ||
| try { | ||
| return await dbUpdateMessage(messageId, { content }); | ||
| } catch (error) { | ||
| console.error(`Error updating message ${messageId}:`, error); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export async function deleteMessage(messageId: string): Promise<boolean> { | ||
| try { | ||
| return await dbDeleteMessage(messageId); | ||
| } catch (error) { | ||
| console.error(`Error deleting message ${messageId}:`, error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export async function deleteTrailingMessages(chatId: string, createdAt: Date): Promise<void> { | ||
| try { | ||
| await dbDeleteTrailingMessages(chatId, createdAt); | ||
| } catch (error) { | ||
| console.error(`Error deleting trailing messages for chat ${chatId}:`, error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding authorization at the wrapper layer.
These wrappers are an ideal place to add user authorization since this is a 'use server' file with access to getCurrentUserIdOnServer(). Currently, authorization depends entirely on the calling code (app/actions.tsx) operating on user-scoped AI state.
The implementation is functionally correct, but adding authorization here would provide defense in depth.
♻️ Example authorization pattern
export async function updateMessage(messageId: string, content: string): Promise<DrizzleMessage | null> {
try {
+ const userId = await getCurrentUserIdOnServer();
+ if (!userId) {
+ console.error('updateMessage: User not authenticated');
+ return null;
+ }
- return await dbUpdateMessage(messageId, { content });
+ return await dbUpdateMessage(messageId, { content }, userId);
} catch (error) {🤖 Prompt for AI Agents
In `@lib/actions/chat.ts` around lines 168 - 192, Add server-side authorization to
the wrapper functions by calling getCurrentUserIdOnServer() at the start of
updateMessage, deleteMessage, and deleteTrailingMessages and verifying the
current user is the owner of the target resource before calling dbUpdateMessage,
dbDeleteMessage, or dbDeleteTrailingMessages; perform a minimal lookup (e.g.,
fetch message/chat owner via a helper like dbGetMessageById or dbGetChatById)
and if the owner !== currentUserId return null/false/void (or throw an
authorization error) instead of performing the DB operation. Ensure the
ownership check runs in updateMessage (using messageId), deleteMessage (using
messageId), and deleteTrailingMessages (using chatId) so these wrappers enforce
access control regardless of callers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Key issues to address: (1) submit()/resubmit() don’t await processChatWorkflow, which risks race conditions and swallowed errors; (2) the new message mutation endpoints lack server-side authorization/ownership checks; (3) saveChat() upsert updates createdAt, which can break your trailing-deletion logic based on timestamps. There are also smaller maintainability concerns around pervasive any, in-place mutation in resubmit, and UI truncation when findIndex returns -1.
Additional notes (2)
- Readability |
app/actions.tsx:472-487
toolOutputs.map(...)is being used for side-effects (no returned array is used). This is a small correctness/clarity footgun: readers may assume the result is used, and it can hide accidental returns.
Also, this code calls aiState.get() repeatedly inside the loop; if aiState.get() is expensive or if updates are concurrent, it can lead to subtle ordering issues.
- Maintainability |
lib/actions/chat-db.ts:119-132
saveChat()now doesonConflictDoUpdateand setscreatedAtfrom the excluded row. This effectively allows a re-save to rewrite a message's creation time, which can break chronological ordering and any logic relying oncreatedAtfor trailing deletion (your new feature does exactly that).
Given your trailing deletion is based on createdAt, updating createdAt during upsert can cause incorrect deletion ranges.
Summary of changes
Summary
This PR adds message edit + per-message delete capabilities across the stack.
Server actions / AI workflow (app/actions.tsx)
- Refactors the generation flow into a shared
processChatWorkflow()used by bothsubmit()and newresubmit(). - Adds server actions:
resubmit(messageId, content, mapProvider)to update a message, delete subsequent messages, and regenerate an AI response.deleteMessageAction(messageId)to delete a message and all subsequent messages.
- Replaces hardcoded user id with
getCurrentUserIdOnServer()fallbacking to'anonymous'. - Passes
idinto<UserMessage />so client UI can trigger edits/deletes.
UI (components/user-message.tsx)
- Adds
'use client'and introduces edit/delete controls (hover-revealed). - Implements inline editing via
react-textarea-autosizeand deletion confirmation viaAlertDialog. - Truncates UI state before calling server actions to align UI with regenerated history.
DB / actions (lib/actions/chat-db.ts, lib/actions/chat.ts)
- Updates
saveChat()to useonConflictDoUpdateonmessages.idto avoid PK conflicts. - Adds granular message operations:
updateMessage()deleteMessage()deleteTrailingMessages(chatId, createdAt)
- Exposes these through
lib/actions/chat.ts.
Cleanup
- Deletes
server.logfrom the repo.
| processChatWorkflow({ | ||
| aiState, | ||
| uiStream, | ||
| isGenerating, | ||
| isCollapsed, | ||
| messages, | ||
| groupeId, | ||
| currentSystemPrompt, | ||
| mapProvider, | ||
| useSpecificAPI, | ||
| maxMessages, | ||
| skipTaskManager: skip | ||
| }) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
submit() now calls processChatWorkflow(...) but does not await it. That means submit() returns immediately while the workflow continues mutating aiState/uiStream in the background. This is risky:
- errors inside
processChatWorkflowwon’t be surfaced to the caller - the returned
uiStream.valuemay be observed before the firstuiStream.update(<Spinner />)happens (racey initial render) - execution ordering differs from
resubmit()where you likely also want deterministic workflow start
If the intent is fire-and-forget, it should be explicit and guarded; otherwise, awaiting is the safer default.
Suggestion
Change the call sites to await processChatWorkflow(...) (both in submit() and resubmit()), and wrap the workflow in a try/finally to ensure isGenerating.done(false)/uiStream.done() always run even on failure.
await processChatWorkflow({ /* ... */ })Optionally, return early after an inquire to keep submit() consistent.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| async function processChatWorkflow({ | ||
| aiState, | ||
| uiStream, | ||
| isGenerating, | ||
| isCollapsed, | ||
| messages, | ||
| groupeId, | ||
| currentSystemPrompt, | ||
| mapProvider, | ||
| useSpecificAPI, | ||
| maxMessages, | ||
| skipTaskManager = false | ||
| }: { | ||
| aiState: any | ||
| uiStream: any | ||
| isGenerating: any | ||
| isCollapsed: any | ||
| messages: CoreMessage[] | ||
| groupeId: string | ||
| currentSystemPrompt: string | ||
| mapProvider: any | ||
| useSpecificAPI: boolean | ||
| maxMessages: number | ||
| skipTaskManager?: boolean | ||
| }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
processChatWorkflow uses multiple any-typed parameters (aiState, uiStream, isGenerating, isCollapsed, mapProvider) and also uses let action: any. This is type-valid but unsafe: it makes it easy to call the workflow incorrectly and silently break streaming/done semantics.
Given this function is now the core shared path for chat generation, it should have a real parameter type (even a minimal interface) to prevent accidental misuse.
Suggestion
Introduce lightweight types/interfaces for the streaming primitives and the workflow params instead of any, e.g.
type StreamableUI = { update(node: React.ReactNode): void; append(node: React.ReactNode): void; done(): void; value: React.ReactNode }
type StreamableValue<T> = { done(v?: T): void; value: T }
type WorkflowParams = {
aiState: { get(): AIState; update(next: AIState): void; done(next: AIState): void }
uiStream: StreamableUI
isGenerating: StreamableValue<boolean>
isCollapsed: StreamableValue<boolean>
messages: CoreMessage[]
// ...
}This keeps behavior the same but makes incorrect calls compile-time failures.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const truncatedMessages = messages.slice(0, index + 1) | ||
| truncatedMessages[index].content = content | ||
|
|
||
| aiState.update({ | ||
| ...aiState.get(), | ||
| messages: truncatedMessages | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resubmit() mutates state objects in-place (truncatedMessages[index].content = content). If aiState consumers assume immutability (common in React-ish state flows), in-place mutation can cause stale renders or unexpected shared references.
You already create a sliced array, but the message object within it is still shared with the original array entry.
Suggestion
Avoid in-place mutation by cloning the edited message object when building truncatedMessages.
const truncatedMessages = messages.slice(0, index + 1)
truncatedMessages[index] = { ...truncatedMessages[index], content }Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| async function deleteMessageAction(messageId: string) { | ||
| 'use server' | ||
|
|
||
| const aiState = getMutableAIState<typeof AI>() | ||
| const messages = aiState.get().messages | ||
| const index = messages.findIndex(m => m.id === messageId) | ||
|
|
||
| if (index !== -1) { | ||
| const messageToDelete = messages[index] | ||
| const chatId = aiState.get().chatId | ||
|
|
||
| if (messageToDelete.createdAt) { | ||
| await deleteTrailingMessages(chatId, new Date(messageToDelete.createdAt)) | ||
| } | ||
| await deleteMessage(messageId) | ||
|
|
||
| const truncatedMessages = messages.slice(0, index) | ||
| aiState.done({ | ||
| ...aiState.get(), | ||
| messages: truncatedMessages | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteMessageAction updates state via aiState.done(...) rather than aiState.update(...). If done finalizes the stream/session, this may prevent subsequent actions in the same session (or cause unexpected UI behavior), especially since deleting a message is not necessarily “end of conversation”.
This is a semantic difference from resubmit() where you use aiState.update(...) and keep streaming alive.
Suggestion
Prefer aiState.update(...) here unless the intended behavior is to finalize the chat session. If you do want to finalize, add a comment explaining why.
aiState.update({ ...aiState.get(), messages: truncatedMessages })Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| // Truncate UI state | ||
| setMessages(currentMessages => { | ||
| const index = currentMessages.findIndex(m => m.id === id) | ||
| return currentMessages.slice(0, index + 1) | ||
| }) | ||
|
|
||
| const response = await resubmit(id, editContent, mapProvider) | ||
| setMessages(currentMessages => [...currentMessages, response]) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client-side truncation logic can behave unexpectedly when id is not found: findIndex returns -1, so slice(0, index + 1) becomes slice(0, 0) and wipes the UI.
This can happen if id is undefined (already guarded) or if the UI state diverges (e.g., hydration mismatch, duplicate IDs, or user clicks edit on a message not present in current UI state).
Suggestion
Guard against index === -1 before truncating.
setMessages(current => {
const index = current.findIndex(m => m.id === id)
return index === -1 ? current : current.slice(0, index + 1)
})Apply the same pattern in handleDelete.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const handleSave = async () => { | ||
| if (!id || !editContent.trim()) return | ||
|
|
||
| setIsEditing(false) | ||
|
|
||
| // Truncate UI state | ||
| setMessages(currentMessages => { | ||
| const index = currentMessages.findIndex(m => m.id === id) | ||
| return currentMessages.slice(0, index + 1) | ||
| }) | ||
|
|
||
| const response = await resubmit(id, editContent, mapProvider) | ||
| setMessages(currentMessages => [...currentMessages, response]) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleSave does not handle server-action failure. Right now you optimistically truncate UI state, then append whatever resubmit returns. If resubmit throws or returns { component: null }, the UI can end up missing the original messages with no recovery path.
Given resubmit can early-return component: null when message id is not found server-side, the client should handle that explicitly.
Suggestion
Wrap server calls in try/catch and restore UI state (or show an error) if resubmit fails / returns null component.
const prev = /* capture currentMessages */
try {
const response = await resubmit(id, editContent, mapProvider)
if (!response?.component) throw new Error('Resubmit failed')
setMessages(curr => [...curr, response])
} catch {
setMessages(prev)
setIsEditing(true)
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| export async function updateMessage( | ||
| messageId: string, | ||
| updates: Partial<NewMessage> | ||
| ): Promise<Message | null> { | ||
| try { | ||
| const result = await db | ||
| .update(messages) | ||
| .set(updates) | ||
| .where(eq(messages.id, messageId)) | ||
| .returning(); | ||
| return result[0] || null; | ||
| } catch (error) { | ||
| console.error('Error updating message:', error); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Deletes a specific message. | ||
| * @param messageId - The ID of the message to delete. | ||
| * @returns True if deletion was successful, false otherwise. | ||
| */ | ||
| export async function deleteMessage(messageId: string): Promise<boolean> { | ||
| try { | ||
| const result = await db | ||
| .delete(messages) | ||
| .where(eq(messages.id, messageId)) | ||
| .returning({ id: messages.id }); | ||
| return result.length > 0; | ||
| } catch (error) { | ||
| console.error('Error deleting message:', error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Deletes all messages in a chat created after a certain message. | ||
| * @param chatId - The ID of the chat. | ||
| * @param createdAt - The timestamp after which messages should be deleted. | ||
| */ | ||
| export async function deleteTrailingMessages( | ||
| chatId: string, | ||
| createdAt: Date | ||
| ): Promise<void> { | ||
| try { | ||
| await db | ||
| .delete(messages) | ||
| .where(and(eq(messages.chatId, chatId), gt(messages.createdAt, createdAt))); | ||
| } catch (error) { | ||
| console.error('Error deleting trailing messages:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateMessage / deleteMessage / deleteTrailingMessages operate solely by message/chat identifiers with no user scoping/authorization checks. In this PR, client UI can call server actions with a message id; without server-side ownership enforcement, any authenticated (or even anonymous, depending on your auth model) user could potentially target another user's message if they can guess/obtain IDs.
You already import getCurrentUserIdOnServer in this file but do not use it for these new functions.
Suggestion
Enforce ownership in the DB layer (or at least in the server actions) by including userId in the WHERE clause.
Example (DB layer):
const userId = await getCurrentUserIdOnServer()
if (!userId) throw new Error('Not authenticated')
await db
.update(messages)
.set(updates)
.where(and(eq(messages.id, messageId), eq(messages.userId, userId)))Similarly for deletes and trailing deletes: restrict by messages.userId (and verify the chat belongs to the user).
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
|
@coderabbitai I don't see the UI functions for the message edit on chat |
|
@coderabbitai can you get me all the prompt for AI agents on one response |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5307 🏁 Script executed: Length of output: 349
How to Access Edit/Delete FunctionsThe Edit and Delete buttons appear when you hover over a user message. They use a hover effect ( What to Look For:
Editing Flow:
Deleting Flow:
VerificationThe implementation in
Try hovering over one of your messages in the chat to see if the icons appear. If they still don't show up, there might be a CSS or styling issue we need to investigate further. |
|
@coderabbitai the resolution search does not work anymore |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 604 🏁 Script executed: Length of output: 20010 🏁 Script executed: Length of output: 5427 🏁 Script executed: Length of output: 5738
1. Task Manager AgentFile: 2. Inquire AgentFile: Note: The system prompt is currently a placeholder: 3. Researcher Agent (Main Research Agent)File: 4. Writer AgentFile: 5. Resolution Search Agent (Satellite Image Analysis)File: 6. Query Suggestor Agent (Related Queries)File: 7. Suggest Actions Agent (Map-Based Suggestions)File: Additional Notes:
|
The UI functions for message editing have been integrated into the |
Here are the system prompts for the AI agents as found in the codebase: 1. Researcher Agent (
|
- Implement `updateMessage`, `deleteMessage`, and `deleteTrailingMessages` in database layer. - Refactor AI generation logic in `app/actions.tsx` into a reusable `processChatWorkflow` function. - Add `resubmit` and `deleteMessageAction` server actions. - Enhance `UserMessage` component with inline editing and deletion UI. - Ensure proper authentication using `getCurrentUserIdOnServer` and state management for trailing message deletion. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 55 🏁 Script executed: Length of output: 7839 🏁 Script executed: Length of output: 4694 🏁 Script executed: Length of output: 6796 🏁 Script executed: Length of output: 1875 🏁 Script executed: Length of output: 3485 🏁 Script executed: Length of output: 794 🏁 Script executed: Length of output: 374 🏁 Script executed: Length of output: 2585 🏁 Script executed: Length of output: 968 🏁 Script executed: Length of output: 3386 🏁 Script executed: Length of output: 854
The ProblemIn export async function getModel(requireVision: boolean = false) {
// ... the function never checks or uses requireVision!
// It just returns whatever model is configured
}Meanwhile, return streamObject({
model: await getModel(hasImage), // Passes true when image exists
// ...
})Since the The FixThe 🏁 Script executed: Length of output: 3333
The ProblemIn The FixUpdate the export async function getModel(requireVision: boolean = false) {
const selectedModel = await getSelectedModel();
const xaiApiKey = process.env.XAI_API_KEY;
const gemini3ProApiKey = process.env.GEMINI_3_PRO_API_KEY;
const awsAccessKeyId = process.env.AWS_ACCESS_KEY_ID;
const awsSecretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
const awsRegion = process.env.AWS_REGION;
const bedrockModelId = process.env.BEDROCK_MODEL_ID || 'anthropic.claude-3-5-sonnet-20241022-v2:0';
const openaiApiKey = process.env.OPENAI_API_KEY;
if (selectedModel) {
switch (selectedModel) {
case 'Grok 4.2':
if (xaiApiKey) {
const xai = createXai({
apiKey: xaiApiKey,
baseURL: 'https://api.x.ai/v1',
});
try {
// Use vision-capable model when required
const modelName = requireVision ? 'grok-vision-beta' : 'grok-4-fast-non-reasoning';
return xai(modelName);
} catch (error) {
console.error('Selected model "Grok 4.2" is configured but failed to initialize.', error);
throw new Error('Failed to initialize selected model.');
}
} else {
console.error('User selected "Grok 4.2" but XAI_API_KEY is not set.');
throw new Error('Selected model is not configured.');
}
case 'Gemini 3':
if (gemini3ProApiKey) {
const google = createGoogleGenerativeAI({
apiKey: gemini3ProApiKey,
});
try {
// Gemini 3 Pro supports vision by default
return google('gemini-3-pro-preview');
} catch (error) {
console.error('Selected model "Gemini 3" is configured but failed to initialize.', error);
throw new Error('Failed to initialize selected model.');
}
} else {
console.error('User selected "Gemini 3" but GEMINI_3_PRO_API_KEY is not set.');
throw new Error('Selected model is not configured.');
}
case 'GPT-5.1':
if (openaiApiKey) {
const openai = createOpenAI({
apiKey: openaiApiKey,
});
// GPT-4o supports vision by default
return openai('gpt-4o');
} else {
console.error('User selected "GPT-5.1" but OPENAI_API_KEY is not set.');
throw new Error('Selected model is not configured.');
}
}
}
// Default behavior with vision support
if (requireVision) {
// When vision is required, prefer models that explicitly support it
// Try Gemini first for vision (excellent multimodal support)
if (gemini3ProApiKey) {
const google = createGoogleGenerativeAI({
apiKey: gemini3ProApiKey,
});
try {
return google('gemini-3-pro-preview');
} catch (error) {
console.warn('Gemini 3 Pro API unavailable for vision, falling back:', error);
}
}
// Try GPT-4o (strong vision capabilities)
if (openaiApiKey) {
const openai = createOpenAI({
apiKey: openaiApiKey,
});
return openai('gpt-4o');
}
// Try Claude via Bedrock (good vision support)
if (awsAccessKeyId && awsSecretAccessKey) {
const bedrock = createAmazonBedrock({
bedrockOptions: {
region: awsRegion,
credentials: {
accessKeyId: awsAccessKeyId,
secretAccessKey: awsSecretAccessKey,
},
},
});
const model = bedrock(bedrockModelId, {
additionalModelRequestFields: { top_k: 350 },
});
return model;
}
throw new Error('No vision-capable model is configured. Please set GEMINI_3_PRO_API_KEY, OPENAI_API_KEY, or AWS credentials.');
}
// Default behavior for text-only: Grok -> Gemini -> Bedrock -> OpenAI
if (xaiApiKey) {
const xai = createXai({
apiKey: xaiApiKey,
baseURL: 'https://api.x.ai/v1',
});
try {
return xai('grok-4-fast-non-reasoning');
} catch (error) {
console.warn('xAI API unavailable, falling back to next provider:');
}
}
if (gemini3ProApiKey) {
const google = createGoogleGenerativeAI({
apiKey: gemini3ProApiKey,
});
try {
return google('gemini-3-pro-preview');
} catch (error) {
console.warn('Gemini 3 Pro API unavailable, falling back to next provider:', error);
}
}
if (awsAccessKeyId && awsSecretAccessKey) {
const bedrock = createAmazonBedrock({
bedrockOptions: {
region: awsRegion,
credentials: {
accessKeyId: awsAccessKeyId,
secretAccessKey: awsSecretAccessKey,
},
},
});
const model = bedrock(bedrockModelId, {
additionalModelRequestFields: { top_k: 350 },
});
return model;
}
const openai = createOpenAI({
apiKey: openaiApiKey,
});
return openai('gpt-4o');
}Key Changes
TestingAfter applying this fix:
Would you like me to create an issue to track this fix, or would you prefer to apply it directly in this PR? |
- Populate `createdAt` for all new messages in `AIState` to fix trailing deletion in active sessions. - Preserve multimodal content (images) during message editing in `resubmit`. - Update `UserMessage` layout to match the provided screenshot: - Move icons (Edit, Copy, Delete) below the message bubble. - Use `Pencil` icon and add `Copy` icon with clipboard functionality. - Add bubble-like styling to user messages. - Fix Resolution Search by ensuring vision-capable models are used in `getModel`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/utils/index.ts (1)
101-115:⚠️ Potential issue | 🟡 MinorBedrock fallback ignores
requireVisionparameter.When AWS Bedrock is used as a fallback, the
requireVisionparameter is not checked. If the caller requires vision capabilities and the configuredbedrockModelIddoesn't support vision, this could cause runtime failures for resolution search or other vision-dependent features.Consider either:
- Adding a vision-capable model check/fallback for Bedrock
- Skipping Bedrock in the fallback chain when
requireVisionis true and continuing to OpenAI🛡️ Proposed fix to skip Bedrock when vision is required
if (awsAccessKeyId && awsSecretAccessKey) { + // Skip Bedrock fallback if vision is required, as vision support depends on model configuration + if (requireVision) { + console.warn('Vision required but Bedrock vision capability is not guaranteed, falling back to OpenAI'); + } else { const bedrock = createAmazonBedrock({ bedrockOptions: { region: awsRegion, credentials: { accessKeyId: awsAccessKeyId, secretAccessKey: awsSecretAccessKey, }, }, }); const model = bedrock(bedrockModelId, { additionalModelRequestFields: { top_k: 350 }, }); return model; + } }
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 605-617: The code is mutating editedMessageInState (and nested
textPart.text) which also mutates the original messages array; instead, perform
an immutable update by creating a shallow copy of the message object at
truncatedMessages[index] (e.g., newMessage = { ...editedMessageInState }) and if
content is an array, create a new content array with the modified text part (map
or slice+replace) or if content is a string, set newMessage.content = content;
then replace truncatedMessages[index] with newMessage before continuing so the
original messages/aiState are not mutated (referencing editedMessageInState,
truncatedMessages, and messages).
In `@components/user-message.tsx`:
- Around line 147-155: The action buttons are hidden via hover-only styles,
which blocks keyboard users; update the opacity classes so focus also reveals
them: where the buttons render (the block guarded by isEditing and id), add
focus-related utility classes alongside group-hover:opacity-100 (e.g., include
group-focus-within:opacity-100 and focus-within:opacity-100 on the container) so
tab/focus on the buttons (handleEdit, copyToClipboard) makes the controls
visible; ensure the container still has the "group" role and that the Button
elements remain native buttons (no tabindex tweaks needed).
In `@lib/utils/index.ts`:
- Line 55: Replace the older Gemini model id used for vision with the correct
Gemini 3 vision variant: wherever the code returns google(requireVision ?
'gemini-1.5-pro' : 'gemini-3-pro-preview') (i.e., the return that calls
google(...) with a ternary on requireVision), change the true branch to
'gemini-3-pro-image-preview' so the call becomes google(requireVision ?
'gemini-3-pro-image-preview' : 'gemini-3-pro-preview'); update both occurrences
of this pattern in the file.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
app/actions.tsxcomponents/user-message.tsxlib/utils/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/utils/index.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/utils/index.ts
🧬 Code graph analysis (2)
components/user-message.tsx (7)
app/actions.tsx (1)
AI(735-816)lib/store/settings.ts (1)
useSettingsStore(10-13)components/ui/textarea.tsx (1)
Textarea(24-24)components/ui/button.tsx (1)
Button(56-56)components/chat-share.tsx (1)
ChatShare(25-114)components/ui/alert-dialog.tsx (8)
AlertDialog(130-130)AlertDialogTrigger(133-133)AlertDialogContent(134-134)AlertDialogHeader(135-135)AlertDialogTitle(137-137)AlertDialogDescription(138-138)AlertDialogFooter(136-136)AlertDialogAction(139-139)components/sidebar/chat-history-client.tsx (1)
ChatHistoryClientProps(24-26)
app/actions.tsx (6)
lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/actions/chat.ts (4)
getSystemPrompt(268-283)deleteTrailingMessages(186-192)updateMessage(168-175)deleteMessage(177-184)lib/agents/task-manager.tsx (1)
taskManager(6-56)lib/db/schema.ts (1)
messages(26-37)lib/types/index.ts (1)
AIMessage(59-77)lib/agents/writer.tsx (1)
writer(7-51)
🪛 Biome (2.3.13)
app/actions.tsx
[error] 489-489: This callback passed to map() iterable method should always return a value.
Add a return with a value to this callback.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (9)
app/actions.tsx (4)
19-25: LGTM!The new imports for
updateMessage,deleteMessage, anddeleteTrailingMessagesare correctly added to support the new message editing and deletion functionality.
383-386: LGTM!Good improvement using dynamic import of
getCurrentUserIdOnServerto properly authenticate users instead of hardcoding 'anonymous'. The fallback to 'anonymous' provides graceful degradation.
736-741: LGTM!The new
resubmitanddeleteMessageActionactions are correctly exported in the AI actions object, making them available to client components viauseActions.
851-857: LGTM!Correctly passing the message
idtoUserMessagecomponent enables per-message edit/delete functionality.components/user-message.tsx (4)
1-23: LGTM!The
'use client'directive is correctly added, and the imports foruseActions,useUIState, UI components, andAlertDialogare appropriate for the new edit/delete functionality.
29-34: LGTM!Adding the optional
idprop toUserMessagePropsis the correct approach for enabling per-message operations while maintaining backward compatibility.
117-135: LGTM!The inline editing UI with
Textarea, Save/Cancel buttons, and proper state management provides a good user experience. TheautoFocusattribute correctly focuses the textarea when editing begins.
157-180: LGTM!The
AlertDialogimplementation for delete confirmation is well done. The warning about deleting subsequent messages is appropriate and the destructive styling clearly indicates the action's severity.lib/utils/index.ts (1)
40-40: The model identifiersgrok-vision-betaandgrok-betaare valid and currently supported by xAI's API as of January 2026. They correctly map to vision and text capabilities respectively, so no changes are needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
lib/utils/index.ts
Outdated
| }); | ||
| try { | ||
| return google('gemini-3-pro-preview'); | ||
| return google(requireVision ? 'gemini-1.5-pro' : 'gemini-3-pro-preview'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Google Gemini 3 Pro API model identifier 2025
💡 Result:
The Gemini 3 Pro API model identifier (model code / model ID) released in November 2025 is:
gemini-3-pro-preview[1][2]
Related Gemini 3 Pro variant:
gemini-3-pro-image-preview[1]
Sources: [1] Google Gemini API “Gemini models” docs, [2] Google Cloud Vertex AI “Gemini 3 Pro” model page.
🏁 Script executed:
find lib -name "index.ts" -type f | head -5Repository: QueueLab/QCX
Length of output: 109
🏁 Script executed:
cat -n lib/utils/index.ts | sed -n '45,105p'Repository: QueueLab/QCX
Length of output: 2529
Use gemini-3-pro-image-preview for Gemini 3 vision requests instead of falling back to gemini-1.5-pro.
The code uses gemini-1.5-pro when vision is required, but gemini-3-pro-image-preview is the proper vision model for the Gemini 3 line. When a user selects "Gemini 3", they should receive a Gemini 3 model variant in both vision and non-vision cases, not fall back to an older generation.
Change at lines 55 and 95:
return google(requireVision ? 'gemini-3-pro-image-preview' : 'gemini-3-pro-preview');
🤖 Prompt for AI Agents
In `@lib/utils/index.ts` at line 55, Replace the older Gemini model id used for
vision with the correct Gemini 3 vision variant: wherever the code returns
google(requireVision ? 'gemini-1.5-pro' : 'gemini-3-pro-preview') (i.e., the
return that calls google(...) with a ternary on requireVision), change the true
branch to 'gemini-3-pro-image-preview' so the call becomes google(requireVision
? 'gemini-3-pro-image-preview' : 'gemini-3-pro-preview'); update both
occurrences of this pattern in the file.
…eration - Added Edit (Pencil), Copy, and Delete (Trash) icons to UserMessage component. - Implemented inline message editing with optimistic UI state truncation. - Added 'resubmit' server action to handle message edits and trigger AI re-generation. - Added 'deleteMessageAction' to remove messages and trailing history. - Refactored AI generation logic into 'processChatWorkflow' for shared use. - Implemented trailing message deletion in the database layer using timestamps. - Fixed 'resolution search' vision model selection logic. - Ensured message ID consistency between client and server. - Updated user message styling to white bubble with border. - Added safety break in AI streaming loop to prevent empty response hangs. - Moving icons below message bubble for improved visibility. - Passing ID to UserMessage in ChatPanel to ensure icons appear immediately. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
@coderabbitai the system does not return tokens anywhere |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 2210
Current State❌ No token tracking implemented:
Where Token Data Is AvailableThe AI SDK responses typically include usage information like: {
usage: {
promptTokens: 150,
completionTokens: 85,
totalTokens: 235
}
}This data is available in:
What Would Need to Be ImplementedTo add token tracking, you would need:
Next StepsWould you like me to:
Let me know your preference! 🧠 Learnings used💡 Use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
lib/utils/index.ts (1)
31-74:⚠️ Potential issue | 🟡 MinorAdd a default case to the switch statement to fail fast on unknown
SPECIFIC_API_MODELvalues.When
useSpecificAPIis enabled andSPECIFIC_API_MODELis set to an unrecognized value, the switch statement doesn't match any case and execution silently falls through to the default provider fallback chain. This masks misconfiguration and defeats the purpose of explicitly selecting a model.Suggested fix
switch (selectedModel) { case 'Grok 4.2': ... case 'Gemini 3': ... case 'GPT-5.1': ... + default: + console.error(`Selected model "${selectedModel}" is not supported.`); + throw new Error('Selected model is not configured.'); }
🤖 Fix all issues with AI agents
In @.env:
- Line 2: Replace the real Mapbox token committed in
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN with a non-sensitive placeholder and remove the
actual token from the repository; move the real value into a local/secret store
(.env.local or secrets manager) and update or add .env.example to include the
placeholder key so contributors know to provide their own token. Ensure
references to NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN in the code remain unchanged but
that no real credential is present in tracked files.
In `@components/user-message.tsx`:
- Around line 80-84: The truncate logic in setMessages used by handleSave and
handleDelete slices based on findIndex without checking for -1, which can drop
messages; update both handlers to compute const index =
currentMessages.findIndex(m => m.id === id) and only perform return
currentMessages.slice(0, index + 1) when index >= 0, otherwise return
currentMessages unchanged (no-op) so negative indices do not produce incorrect
slices; ensure you adjust both occurrences that reference
setMessages/currentMessages/id.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.envapp/actions.tsxcomponents/chat-panel.tsxcomponents/user-message.tsxlib/agents/researcher.tsxlib/utils/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/utils/index.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/utils/index.ts
🧬 Code graph analysis (4)
lib/agents/researcher.tsx (1)
lib/utils/index.ts (1)
getModel(20-121)
.env (3)
components/map/google-map.tsx (1)
apiKey(20-29)lib/agents/tools/geospatial.tsx (1)
MapboxConfig(30-34)mapbox_mcp/composio-mapbox.ts (1)
validateEnvironmentVariables(8-48)
lib/utils/index.ts (2)
lib/actions/users.ts (1)
getSelectedModel(126-139)components/settings/components/model-selection-form.tsx (1)
model(100-133)
components/chat-panel.tsx (2)
components/user-message.tsx (1)
UserMessage(36-185)app/page.tsx (1)
Page(9-18)
🪛 Biome (2.3.13)
app/actions.tsx
[error] 492-492: This callback passed to map() iterable method should always return a value.
Add a return with a value to this callback.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (4)
lib/agents/researcher.tsx (1)
107-109: Model selection now honors useSpecificModel + vision needs.components/chat-panel.tsx (1)
107-118: Consistent message ID propagation for edit/delete.components/user-message.tsx (1)
147-181: Edit/copy/delete controls are clean and well-integrated.app/actions.tsx (1)
364-377: Good: user message IDs now flow from formData into AI state.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @@ -1 +1,2 @@ | |||
| DATABASE_URL="postgresql://user:password@host:port/db" | |||
| NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p6YTM0eHlyMDAwMDNia3BkY3R6ZzhpbyJ9.dummy | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not commit real Mapbox access tokens.
Line 2 introduces a concrete token in .env. Even public tokens can be abused or leak account usage. Keep real values in .env.local/secret store and commit only a placeholder (or move to .env.example).
🔐 Suggested placeholder
-NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p6YTM0eHlyMDAwMDNia3BkY3R6ZzhpbyJ9.dummy
+NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=YOUR_MAPBOX_PUBLIC_TOKEN📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1IjoiZHVtbXkiLCJhIjoiY2p6YTM0eHlyMDAwMDNia3BkY3R6ZzhpbyJ9.dummy | |
| NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=YOUR_MAPBOX_PUBLIC_TOKEN |
🤖 Prompt for AI Agents
In @.env at line 2, Replace the real Mapbox token committed in
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN with a non-sensitive placeholder and remove the
actual token from the repository; move the real value into a local/secret store
(.env.local or secrets manager) and update or add .env.example to include the
placeholder key so contributors know to provide their own token. Ensure
references to NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN in the code remain unchanged but
that no real credential is present in tracked files.
| // Truncate UI state | ||
| setMessages(currentMessages => { | ||
| const index = currentMessages.findIndex(m => m.id === id) | ||
| return currentMessages.slice(0, index + 1) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the file and understand its structure
find . -name "user-message.tsx" -o -name "user-message.ts"Repository: QueueLab/QCX
Length of output: 85
🏁 Script executed:
# Get the file size to determine how to read it
wc -l components/user-message.tsxRepository: QueueLab/QCX
Length of output: 87
🏁 Script executed:
# Read the file with line numbers, focusing on the area around lines 80-84 and 93-97
cat -n components/user-message.tsx | sed -n '70,110p'Repository: QueueLab/QCX
Length of output: 1457
🏁 Script executed:
# Search for other similar patterns of slice operations on messages state in this file
rg -n "slice\(" components/user-message.tsxRepository: QueueLab/QCX
Length of output: 155
🏁 Script executed:
# Also check for other setMessages operations that might remove messages
rg -n "setMessages" components/user-message.tsxRepository: QueueLab/QCX
Length of output: 258
🏁 Script executed:
# Check the broader context - what is the component structure and where id comes from
cat -n components/user-message.tsx | sed -n '1,50p'Repository: QueueLab/QCX
Length of output: 1758
🏁 Script executed:
# Check the full component to see how messages are initially populated and used
cat -n components/user-message.tsx | tail -n +50 | head -n 100Repository: QueueLab/QCX
Length of output: 4073
🏁 Script executed:
# Let's check if there are any other message-related state updates or mutations elsewhere
rg -n "findIndex\(" components/user-message.tsxRepository: QueueLab/QCX
Length of output: 189
🏁 Script executed:
# Check how UserMessage is used in parent components to understand data flow
find . -name "*.tsx" -o -name "*.ts" | xargs grep -l "UserMessage" | head -5Repository: QueueLab/QCX
Length of output: 203
🏁 Script executed:
# Let's check how messages are managed in the parent component
cat -n app/actions.tsx | head -50Repository: QueueLab/QCX
Length of output: 2385
🏁 Script executed:
# Also check the chat panel to see how UserMessage is rendered
rg -n "UserMessage" components/chat-panel.tsx -A 2 -B 2Repository: QueueLab/QCX
Length of output: 437
Add guard against missing message IDs before slicing.
Both handleSave and handleDelete slice the messages array without checking if the message exists. If findIndex returns -1, slice(0, -1 + 1) becomes slice(0, 0) (losing all messages) or slice(0, -1) (losing the last message). Add a guard to prevent this.
Suggested fix
setMessages(currentMessages => {
const index = currentMessages.findIndex(m => m.id === id)
+ if (index === -1) return currentMessages
return currentMessages.slice(0, index + 1)
}) setMessages(currentMessages => {
const index = currentMessages.findIndex(m => m.id === id)
+ if (index === -1) return currentMessages
return currentMessages.slice(0, index)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Truncate UI state | |
| setMessages(currentMessages => { | |
| const index = currentMessages.findIndex(m => m.id === id) | |
| return currentMessages.slice(0, index + 1) | |
| }) | |
| // Truncate UI state | |
| setMessages(currentMessages => { | |
| const index = currentMessages.findIndex(m => m.id === id) | |
| if (index === -1) return currentMessages | |
| return currentMessages.slice(0, index + 1) | |
| }) |
🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 80 - 84, The truncate logic in
setMessages used by handleSave and handleDelete slices based on findIndex
without checking for -1, which can drop messages; update both handlers to
compute const index = currentMessages.findIndex(m => m.id === id) and only
perform return currentMessages.slice(0, index + 1) when index >= 0, otherwise
return currentMessages unchanged (no-op) so negative indices do not produce
incorrect slices; ensure you adjust both occurrences that reference
setMessages/currentMessages/id.
- Added try-catch-finally block to processChatWorkflow in app/actions.tsx to prevent UI hangs. - Initialized streamText with an empty string for stable hydration. - Guaranteed finalization of uiStream, isGenerating, and aiState even on errors. - Improved UserMessage icons visibility by adding focus-within support and hover background effects. - Refined useSpecificAPI logic to correctly call writer when researcher only performs tool calls. - Added user-friendly error feedback to the chat interface. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 624-626: The code calls deleteTrailingMessages(chatId, new
Date(editedMessage.createdAt)) before calling updateMessage, which can leave the
DB inconsistent if updateMessage fails; either execute updateMessage(...) first
and only call deleteTrailingMessages after its success, or perform both
operations inside a single database transaction so they commit or roll back
together; locate the calls to deleteTrailingMessages and updateMessage in this
flow and implement the chosen fix (wrap both operations in a transaction via
your DB client/ORM or swap the call order and add proper error handling/retries)
so the edited message and trailing deletion stay consistent.
In `@components/user-message.tsx`:
- Around line 66-71: The copyToClipboard handler currently calls
navigator.clipboard.writeText(textPart) without handling rejections, so add a
try/catch (or promise .then/.catch) around the writeText call inside
copyToClipboard to await the operation and call toast.success only on success
and toast.error (or a fallback message) on failure; reference the
copyToClipboard function and variable textPart, handle
navigator.clipboard.writeText(textPart) failures, and ensure any caught error is
logged or included in the error toast for diagnosability.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
app/actions.tsxcomponents/user-message.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/user-message.tsx (6)
app/actions.tsx (1)
AI(757-838)lib/store/settings.ts (1)
useSettingsStore(10-13)components/ui/textarea.tsx (1)
Textarea(24-24)components/ui/button.tsx (1)
Button(56-56)components/ui/alert-dialog.tsx (9)
AlertDialog(130-130)AlertDialogTrigger(133-133)AlertDialogContent(134-134)AlertDialogHeader(135-135)AlertDialogTitle(137-137)AlertDialogDescription(138-138)AlertDialogFooter(136-136)AlertDialogCancel(140-140)AlertDialogAction(139-139)components/chat-messages.tsx (1)
ChatMessagesProps(7-9)
🔇 Additional comments (10)
app/actions.tsx (5)
19-25: LGTM!The new imports for
updateMessage,deleteMessage, anddeleteTrailingMessagesare properly added and align with the new edit/delete functionality.
394-406:processChatWorkflowis not awaited — intentional fire-and-forget pattern.The function is called without
await, which appears intentional to return the streaming UI immediately while processing continues in the background. This is consistent with the pattern used forprocessResolutionSearch()at line 194. Thetry/catch/finallyinsideprocessChatWorkflowensures cleanup happens regardless of errors.
581-592: LGTM!The error handling with
try/catch/finallyis well-structured: errors are logged, user-facing feedback is provided, and cleanup (isGenerating.done,uiStream.done) happens in thefinallyblock to prevent UI hangs.
760-762: LGTM!The new
resubmitanddeleteMessageActionfunctions are properly exported in the AI actions registry.
870-880: LGTM!The
idprop is now correctly passed toUserMessage, enabling the edit/delete functionality to work properly.components/user-message.tsx (5)
1-23: LGTM!The
'use client'directive is correctly added, and the imports are well-organized with appropriate components for the editing and delete functionality.
46-47: Consider initializingeditContentfromtextPartto avoid stale state.Currently
editContentis initialized as empty string and only populated whenhandleEditis called. This is fine, but worth noting the state is always empty until the user clicks edit.
147-148: LGTM! Keyboard accessibility is now addressed.The
focus-within:opacity-100class has been added, making the action buttons visible when focused via keyboard navigation. This addresses a previous review comment.
117-134: LGTM!The inline editing UI with
Textarea, Save, and Cancel buttons is well-implemented with clear visual feedback and appropriate sizing.
167-194: LGTM!The delete confirmation dialog using
AlertDialogis properly implemented with clear messaging about the cascading effect ("This will delete this message and all subsequent messages"). The destructive styling appropriately signals the action's severity.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (editedMessage.createdAt) { | ||
| await deleteTrailingMessages(chatId, new Date(editedMessage.createdAt)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential race condition: DB deletion before state update can cause inconsistency.
If deleteTrailingMessages succeeds but the subsequent updateMessage (line 641) fails, the database will be in an inconsistent state with trailing messages deleted but the edited message not updated.
🛡️ Proposed fix: wrap in transaction or reverse order
if (editedMessage.createdAt) {
- await deleteTrailingMessages(chatId, new Date(editedMessage.createdAt))
+ // Update message first, then delete trailing messages
+ // This way if update fails, we haven't deleted anything yet
}
- const truncatedMessages = messages.slice(0, index + 1)
- const editedMessageInState = truncatedMessages[index]
-
- if (Array.isArray(editedMessageInState.content)) {
- const textPart = editedMessageInState.content.find(p => p.type === 'text') as
- | { type: 'text'; text: string }
- | undefined
- if (textPart) {
- textPart.text = content
- }
- } else {
- editedMessageInState.content = content
- }
-
- await updateMessage(
+ // Build the new content first
+ const truncatedMessages = messages.slice(0, index + 1)
+ const editedMessageInState = { ...truncatedMessages[index] }
+ let updatedContent: typeof editedMessageInState.content
+
+ if (Array.isArray(editedMessageInState.content)) {
+ updatedContent = editedMessageInState.content.map(p =>
+ p.type === 'text' ? { ...p, text: content } : p
+ )
+ } else {
+ updatedContent = content
+ }
+ editedMessageInState.content = updatedContent
+ truncatedMessages[index] = editedMessageInState
+
+ // Update first, delete trailing only on success
+ await updateMessage(
messageId,
typeof editedMessageInState.content === 'object'
? JSON.stringify(editedMessageInState.content)
: editedMessageInState.content
)
+
+ if (editedMessage.createdAt) {
+ await deleteTrailingMessages(chatId, new Date(editedMessage.createdAt))
+ }🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 624 - 626, The code calls
deleteTrailingMessages(chatId, new Date(editedMessage.createdAt)) before calling
updateMessage, which can leave the DB inconsistent if updateMessage fails;
either execute updateMessage(...) first and only call deleteTrailingMessages
after its success, or perform both operations inside a single database
transaction so they commit or roll back together; locate the calls to
deleteTrailingMessages and updateMessage in this flow and implement the chosen
fix (wrap both operations in a transaction via your DB client/ORM or swap the
call order and add proper error handling/retries) so the edited message and
trailing deletion stay consistent.
| const copyToClipboard = () => { | ||
| if (textPart) { | ||
| navigator.clipboard.writeText(textPart) | ||
| toast.success('Copied to clipboard') | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling for clipboard operations.
navigator.clipboard.writeText can fail in certain contexts (e.g., insecure origins, permissions denied). While the toast provides positive feedback, consider handling the potential rejection.
♻️ Proposed fix with error handling
const copyToClipboard = () => {
if (textPart) {
- navigator.clipboard.writeText(textPart)
- toast.success('Copied to clipboard')
+ navigator.clipboard.writeText(textPart)
+ .then(() => toast.success('Copied to clipboard'))
+ .catch(() => toast.error('Failed to copy to clipboard'))
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const copyToClipboard = () => { | |
| if (textPart) { | |
| navigator.clipboard.writeText(textPart) | |
| toast.success('Copied to clipboard') | |
| } | |
| } | |
| const copyToClipboard = () => { | |
| if (textPart) { | |
| navigator.clipboard.writeText(textPart) | |
| .then(() => toast.success('Copied to clipboard')) | |
| .catch(() => toast.error('Failed to copy to clipboard')) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@components/user-message.tsx` around lines 66 - 71, The copyToClipboard
handler currently calls navigator.clipboard.writeText(textPart) without handling
rejections, so add a try/catch (or promise .then/.catch) around the writeText
call inside copyToClipboard to await the operation and call toast.success only
on success and toast.error (or a fallback message) on failure; reference the
copyToClipboard function and variable textPart, handle
navigator.clipboard.writeText(textPart) failures, and ensure any caught error is
logged or included in the error toast for diagnosability.
- Added Edit, Copy, and Delete functionality to `UserMessage` component. - Implemented `resubmit` and `deleteMessageAction` server actions. - Refactored AI generation logic into `processChatWorkflow` for reuse and robustness. - Added cascading deletion of trailing messages to ensure conversation consistency after edits or deletions. - Updated database layer to support message updates and targeted deletions. - Fixed vision model selection in `getModel` and ensured message ID consistency across the app. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (3)
components/header-search-button.tsx (1)
51-97:⚠️ Potential issue | 🟠 MajorKeep UI and AI message IDs consistent for resolution search.
The UI message uses a local
id, but the server action will generate a different id because it isn’t sent in theFormData. That breaks edit/delete for this message. Append the same id to the form data beforesubmit.🔧 Suggested fix
const formData = new FormData() + formData.append('id', id) formData.append('file', blob, 'map_capture.png') formData.append('action', 'resolution_search')components/search-related.tsx (1)
44-56:⚠️ Potential issue | 🟠 MajorPropagate the generated message id to the server.
The UI message id isn’t included in the submit payload, so the server generates a different id. That prevents later edit/delete from finding the message. Append the id to the
FormData.🔧 Suggested fix
const formData = new FormData(event.currentTarget as HTMLFormElement) + formData.append('id', id)components/followup-panel.tsx (1)
22-35:⚠️ Potential issue | 🟠 MajorSend the generated id with the submit payload.
Without appending the id to
FormData, the server will create a different id, and edit/delete won’t resolve this message. Append the id before callingsubmit.🔧 Suggested fix
const formData = new FormData(event.currentTarget as HTMLFormElement) + formData.append('id', id)
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 388-406: The mapProvider read from formData can be undefined and
is being passed into processChatWorkflow; default it to a safe value (e.g.,
'mapbox') when missing by changing the mapProvider assignment (the
formData?.get('mapProvider') as 'mapbox' | 'google') to use a fallback like:
const mapProvider = (formData?.get('mapProvider') as 'mapbox' | 'google') ||
'mapbox'; then pass this guaranteed value into processChatWorkflow so downstream
code (including researcher/tools) never receives undefined.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
app/actions.tsxcomponents/followup-panel.tsxcomponents/header-search-button.tsxcomponents/search-related.tsxcomponents/user-message.tsxlib/agents/researcher.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
components/user-message.tsx (4)
app/actions.tsx (1)
AI(766-847)lib/store/settings.ts (1)
useSettingsStore(10-13)components/ui/textarea.tsx (1)
Textarea(24-24)components/ui/button.tsx (1)
Button(56-56)
lib/agents/researcher.tsx (1)
lib/utils/index.ts (1)
getModel(20-121)
app/actions.tsx (5)
lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/actions/chat.ts (4)
getSystemPrompt(268-283)deleteTrailingMessages(186-192)updateMessage(168-175)deleteMessage(177-184)lib/types/index.ts (1)
AIMessage(59-77)lib/actions/chat-db.ts (3)
deleteTrailingMessages(272-283)updateMessage(232-247)deleteMessage(254-265)components/chat.tsx (1)
aiState(75-80)
components/followup-panel.tsx (1)
components/user-message.tsx (1)
UserMessage(36-199)
🔇 Additional comments (5)
lib/agents/researcher.tsx (1)
107-113: Updated getModel call matches the new signature.components/user-message.tsx (3)
1-23: Client-side wiring for edit/delete interactions looks solid.
29-48: Optional id prop + local editing state are correctly introduced.
102-195: Edit/copy/delete UI and confirmation dialog integrate cleanly.app/actions.tsx (1)
880-888: PassingidintoUserMessagein UI state is correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const { getCurrentUserIdOnServer } = await import( | ||
| '@/lib/auth/get-current-user' | ||
| ) | ||
| const userId = (await getCurrentUserIdOnServer()) || 'anonymous' | ||
| const currentSystemPrompt = (await getSystemPrompt(userId)) || '' | ||
|
|
||
| const mapProvider = formData?.get('mapProvider') as 'mapbox' | 'google' | ||
|
|
||
| async function processEvents() { | ||
| processChatWorkflow({ | ||
| aiState, | ||
| uiStream, | ||
| isGenerating, | ||
| isCollapsed, | ||
| messages, | ||
| groupeId, | ||
| currentSystemPrompt, | ||
| mapProvider, | ||
| useSpecificAPI, | ||
| maxMessages, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default mapProvider when it isn’t provided.
Several client flows don’t include mapProvider in FormData, so this can be undefined and propagate into researcher/tools. Default to a safe value to avoid runtime misrouting.
🔧 Suggested fix
- const mapProvider = formData?.get('mapProvider') as 'mapbox' | 'google'
+ const mapProvider =
+ (formData?.get('mapProvider') as 'mapbox' | 'google') || 'mapbox'🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 388 - 406, The mapProvider read from formData
can be undefined and is being passed into processChatWorkflow; default it to a
safe value (e.g., 'mapbox') when missing by changing the mapProvider assignment
(the formData?.get('mapProvider') as 'mapbox' | 'google') to use a fallback
like: const mapProvider = (formData?.get('mapProvider') as 'mapbox' | 'google')
|| 'mapbox'; then pass this guaranteed value into processChatWorkflow so
downstream code (including researcher/tools) never receives undefined.
- Added Edit, Copy, and Delete functionality to `UserMessage` component. - Implemented `resubmit` and `deleteMessageAction` server actions. - Refactored AI generation logic into `processChatWorkflow` for reuse and robustness. - Added cascading deletion of trailing messages to ensure conversation consistency after edits or deletions. - Updated database layer to support message updates and targeted deletions. - Fixed vision model selection in `getModel` and ensured message ID consistency across the app. - Fixed Build Error: Ensured `getUIStateFromAIState` is async in `app/actions.tsx`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
app/actions.tsx (1)
604-709:⚠️ Potential issue | 🟠 MajorMissing error handling for database operations in
resubmit.The function calls
deleteTrailingMessagesandupdateMessagewithout try/catch. If either fails,processChatWorkflowstill executes with potentially inconsistent state, and the client has no indication of the failure.🛡️ Proposed fix
async function resubmit( messageId: string, content: string, mapProvider: 'mapbox' | 'google' = 'mapbox' -) { +): Promise<{ + id: string + isGenerating: StreamableValue<boolean> + component: React.ReactNode + isCollapsed: StreamableValue<boolean> +} | { error: string }> { 'use server' const aiState = getMutableAIState<typeof AI>() const uiStream = createStreamableUI() const isGenerating = createStreamableValue(true) const isCollapsed = createStreamableValue(false) const messages = aiState.get().messages const index = messages.findIndex(m => m.id === messageId) if (index === -1) { isGenerating.done(false) uiStream.done() return { - id: nanoid(), - isGenerating: isGenerating.value, - component: null, - isCollapsed: isCollapsed.value + error: 'Message not found' } } - const editedMessage = messages[index] - const chatId = aiState.get().chatId - - if (editedMessage.createdAt) { - await deleteTrailingMessages(chatId, new Date(editedMessage.createdAt)) + try { + const editedMessage = messages[index] + const chatId = aiState.get().chatId + // ... rest of the logic wrapped in try + } catch (error) { + console.error('Error in resubmit:', error) + isGenerating.done(false) + uiStream.done() + return { error: 'Failed to update message' } }
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 849-851: The function getUIStateFromAIState is declared async but
contains no await; either remove the async keyword from getUIStateFromAIState
and make its return type Promise<UIState> (or plain UIState if the signature
allows) and update any callers that currently await it (e.g., the call site that
does await getUIStateFromAIState) to drop the unnecessary await, or if you
intend to add async work later, keep async and add a brief comment above
getUIStateFromAIState clarifying it’s intentionally async for future async
operations; ensure the function signature and all callers (places invoking
getUIStateFromAIState) are updated consistently.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
app/actions.tsx
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Added Edit, Copy, and Delete functionality to `UserMessage`. - Implemented `resubmit` and `deleteMessageAction` server actions. - Centralized generation logic in `processChatWorkflow` for robustness. - Fixed Next.js 15 build error: `onGetUIState` and `onSetAIState` are now explicitly exported named async functions to satisfy Server Action requirements. - Made `getUIStateFromAIState` local to avoid Server Action sync function restriction. - Resolved React Hook linting warnings in `chat-panel.tsx` and `mapbox-map.tsx`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/map/mapbox-map.tsx (1)
355-466:⚠️ Potential issue | 🟠 MajorPrevent map teardown on camera updates.
The init effect includes
mapData.cameraState,position?.latitude, andposition?.longitudein its dependency array. SincecaptureMapCenteris registered on the 'moveend' event and callssetMapDatato updatemapData.cameraState, every map movement triggers a state update that causes this effect to re-run. This executes the cleanup function (which removes the map) and recreates it on every moveend, causing flicker and lost state.Remove these volatile dependencies—the init effect should only run once on mount. Position updates are already handled by a separate effect (line 524-528) and don't require the init effect to re-run.
🔧 Suggested fix
}, [ setMap, setIsMapLoaded, captureMapCenter, handleUserInteraction, - stopRotation, - mapData.cameraState, - position?.latitude, - position?.longitude + stopRotation ])
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
app/actions.tsxcomponents/chat-panel.tsxcomponents/map/mapbox-map.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
components/chat-panel.tsx (2)
lib/schema/related.tsx (1)
PartialRelated(13-13)components/user-message.tsx (1)
UserMessage(36-199)
components/map/mapbox-map.tsx (4)
components/map/map-3d.tsx (2)
prevData(35-45)map3DElement(60-77)components/map/map-query-handler.tsx (2)
prevData(39-48)prevData(52-56)lib/hooks/use-map-3d-camera-events.ts (2)
useMap3DCameraEvents(6-23)map3DElement(12-22)components/map/google-map.tsx (1)
mapData(54-70)
🔇 Additional comments (5)
app/actions.tsx (2)
758-797: PassingidintoUserMessagealigns UI with per-message actions.Nice wiring for edit/delete flows.
923-991: onGetUIState/onSetAIState persistence hooks look consistent.The guard for unauthenticated users and derived title logic are sensible.
components/chat-panel.tsx (3)
39-45: MemoizedsetSuggestionskeeps parent callbacks in sync.Looks good.
110-121: Single message id used for optimistic UI + FormData.Great for consistent edit/delete behaviors across UI and server.
138-162: Debounce dependency update is correct.Ensures suggestion updates stay consistent when the callback changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Added Edit, Copy, and Delete functionality to `UserMessage`. - Implemented `resubmit` and `deleteMessageAction` server actions. - Centralized generation logic in `processChatWorkflow` for robustness. - Fixed Next.js 15 build error: `onGetUIState` and `onSetAIState` are now inline functions with `'use server'` to satisfy Server Action requirements. - Made `getUIStateFromAIState` local to avoid Server Action sync function restriction. - Resolved React Hook linting warnings in `chat-panel.tsx` and `mapbox-map.tsx`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/actions.tsx (2)
227-233:⚠️ Potential issue | 🟠 MajorGuard
userInputbefore calling.toLowerCase()Line 232 can throw when the submission is file-only (no text input). Add a null-safe guard or move this branch after the empty-input early return.
🔧 Suggested fix
- if (userInput.toLowerCase().trim() === 'what is a planet computer?' || userInput.toLowerCase().trim() === 'what is qcx-terra?') { + const normalizedInput = userInput?.toLowerCase().trim() + if ( + normalizedInput === 'what is a planet computer?' || + normalizedInput === 'what is qcx-terra?' + ) {
234-236:⚠️ Potential issue | 🟡 MinorFix QCX‑Terra pricing link formatting
Line 236 has a space between
]and(, so the markdown link won’t render.🔧 Suggested fix
- : `QCX-Terra is a model garden of pixel level precision geospatial foundational models for efficient land feature predictions from satellite imagery. Available for our Pro and Enterprise customers. [QCX Pricing] (https://www.queue.cx/#pricing)`; + : `QCX-Terra is a model garden of pixel level precision geospatial foundational models for efficient land feature predictions from satellite imagery. Available for our Pro and Enterprise customers. [QCX Pricing](https://www.queue.cx/#pricing)`;
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
app/actions.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/actions.tsx (5)
lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/actions/chat.ts (5)
getSystemPrompt(268-283)deleteTrailingMessages(186-192)updateMessage(168-175)deleteMessage(177-184)saveChat(104-139)lib/types/index.ts (2)
AIMessage(59-77)Chat(49-57)lib/db/schema.ts (1)
messages(26-37)lib/actions/chat-db.ts (6)
deleteTrailingMessages(272-283)updateMessage(232-247)deleteMessage(254-265)Chat(9-9)saveChat(83-136)msg(117-121)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Added Edit, Copy, and Delete functionality to user messages in the chat interface.
- Implemented `resubmit` and `deleteMessageAction` server actions with cascading deletion of trailing messages.
- Centralized generation logic in `processChatWorkflow` for reuse and robustness.
- Ensured Next.js 15 compatibility in `app/actions.tsx` by:
- Removing `'use server'` from the top of the file to allow sync exports.
- Adding inline `'use server'` to actions and `createAI` lifecycle methods.
- Matching the `main` branch's structure for the AI provider.
- Resolved React Hook linting warnings in `chat-panel.tsx` and `mapbox-map.tsx`.
- Verified the fix with a successful production build.
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
@coderabbitai there are not tokens being generated in the system and there's a consistent refresh logo loader
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 7742
The ProblemLooking at the code:
When an error occurs:
🏁 Script executed: Length of output: 1288 🏁 Script executed: Length of output: 6341
Let me check the resolution_search path and the submit function structure: 🏁 Script executed: Length of output: 8638
What's HappeningYour query "How does climate change affect our experience?" should trigger:
Since you see no tokens and infinite loader, the generation is failing at step 2 or 3. Immediate Debugging StepsPlease check your browser console (F12 → Console tab) and share what errors you see. Look for: Also, let me verify your environment setup: Questions:
The most common cause is missing or invalid API keys for the selected model. 🏁 Script executed: Length of output: 4477 🎯 FOUND THE ROOT CAUSE!
🏁 Script executed: Length of output: 1249 🎯 CONFIRMED: Missing API Keys
🏁 Script executed: Length of output: 4380 🎯 ROOT CAUSE CONFIRMED: Missing AI API Keys
❌ Current StateDATABASE_URL="postgresql://..."
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN="pk.eyJ1..."✅ Required SetupYou need at least ONE of these AI provider API keys: Option 1: XAI (Grok) - RecommendedXAI_API_KEY=your_xai_key_hereGet your key: https://console.x.ai/ Option 2: Gemini 3 ProGEMINI_3_PRO_API_KEY=your_gemini_key_hereGet your key: https://aistudio.google.com/apikey Option 3: OpenAI GPT-4oOPENAI_API_KEY=your_openai_key_hereGet your key: https://platform.openai.com/api-keys Also Required for Full Functionality:# For web search
TAVILY_API_KEY=your_tavily_key_here
# For Redis (chat history, settings)
UPSTASH_REDIS_REST_URL=your_upstash_url_here
UPSTASH_REDIS_REST_TOKEN=your_upstash_token_here
# For Supabase (authentication, database)
NEXT_PUBLIC_SUPABASE_URL=your_supabase_url_here
NEXT_PUBLIC_SUPABASE_ANON_KEY=your_supabase_anon_key_here
SUPABASE_SERVICE_ROLE_KEY=your_supabase_service_role_key_here🚀 Setup Instructions
🔍 Priority OrderThe system tries providers in this order (when no model is selected):
Set at least XAI_API_KEY to get started! After adding the keys, the system should:
Let me know once you've added the API keys and I can help verify everything works! 🧠 Learnings used |
I've addressed the issue where no tokens were being generated and the loader would persist. This was caused by a combination of incorrect model selection for vision-based queries (resolution search) and a refactoring that introduced some Next.js 15 compatibility issues with Server Actions. I have:
|
- Added Edit, Copy, and Delete functionality to user messages. - Implemented `resubmit` and `deleteMessageAction` server actions. - Centralized generation logic in `processChatWorkflow` for robustness. - Fixed Next.js 15 build and prerender errors in `app/actions.tsx` by using inline `'use server'` and matching `main` branch structure. - Corrected model selection in `getModel` and `resolutionSearch` to support vision correctly. - Added fallback message when AI generation yields no tokens. - Fixed React Hook dependency warnings. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
@coderabbitai there are still no tokens being generated. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 10272 🏁 Script executed: Length of output: 5967
🔴 Current ProblemYour DATABASE_URL="postgresql://..."
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.eyJ1...Missing: All AI provider API keys (OpenAI, XAI, Gemini, etc.)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (3)
lib/utils/index.ts (2)
101-120:⚠️ Potential issue | 🟡 MinorBedrock and OpenAI fallbacks ignore
requireVisionparameter.When falling back to Bedrock or OpenAI, the
requireVisionflag is not used, which may cause vision workflows to fail if they reach these providers. Consider adding vision-capable model selection or logging a warning when vision is required but unsupported.🛡️ Proposed fix with warning
if (awsAccessKeyId && awsSecretAccessKey) { + if (requireVision) { + console.warn('Bedrock fallback does not support vision models; results may be degraded.'); + } const bedrock = createAmazonBedrock({ bedrockOptions: { region: awsRegion, credentials: { accessKeyId: awsAccessKeyId, secretAccessKey: awsSecretAccessKey, }, }, }); const model = bedrock(bedrockModelId, { additionalModelRequestFields: { top_k: 350 }, }); return model; } + if (requireVision) { + console.warn('OpenAI fallback using gpt-4o which supports vision.'); + } const openai = createOpenAI({ apiKey: openaiApiKey, }); - return openai('gpt-4o'); + return openai(requireVision ? 'gpt-4o' : 'gpt-4o'); // gpt-4o supports vision
64-73: 🧹 Nitpick | 🔵 TrivialGPT-5.1 case doesn't explicitly handle
requireVision.The GPT-5.1 branch always returns
'gpt-4o'regardless of therequireVisionflag. Whilegpt-4odoes support vision, explicitly documenting or handling this would improve code clarity and consistency with other branches.lib/agents/resolution-search.tsx (1)
73-78:⚠️ Potential issue | 🟠 MajorStructured output compatibility issue with Grok models in fallback chain.
This agent uses
streamObjectwith a Zod schema for structured JSON output. The call togetModel(false, hasImage)uses the default fallback chain (Grok → Gemini → Bedrock → OpenAI), which attempts Grok first ifXAI_API_KEYis set. Per learnings, xAI's Grok models have unreliable integration with the AI SDK's structured output functions, and this agent is specifically identified as requiring OpenAI gpt-4o for structured output tasks.Consider:
- Using
getModel(true, hasImage)to force Gemini 3 (which defaults to gemini-1.5-pro and handles structured outputs better), or- Implementing model selection logic to prefer OpenAI gpt-4o for all structured output agents (resolution-search, suggest, task-manager, inquire, query-suggestor).
Note: This pattern affects multiple agents in the codebase that use
streamObject/generateObject.
🤖 Fix all issues with AI agents
In `@app/actions.tsx`:
- Around line 653-658: The call to updateMessage is passing a raw string but
updateMessage(messageId: string, updates: Partial<NewMessage>) expects an
updates object; change the second argument to an object with a content property
(e.g., { content: ... }) using the same conditional JSON.stringify logic on
editedMessageInState.content so the signature matches updateMessage and the
content is stored on the NewMessage shape.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
app/actions.tsxlib/agents/resolution-search.tsxlib/utils/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/utils/index.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/utils/index.ts
🧬 Code graph analysis (3)
lib/utils/index.ts (1)
lib/actions/users.ts (1)
getSelectedModel(126-139)
app/actions.tsx (4)
lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/actions/chat.ts (4)
deleteTrailingMessages(186-192)updateMessage(168-175)deleteMessage(177-184)saveChat(104-139)lib/types/index.ts (2)
AIMessage(59-77)Chat(49-57)lib/db/schema.ts (1)
messages(26-37)
lib/agents/resolution-search.tsx (1)
lib/utils/index.ts (1)
getModel(20-121)
🔇 Additional comments (4)
app/actions.tsx (4)
19-25: LGTM!The new imports for
updateMessage,deleteMessage, anddeleteTrailingMessagesare correctly added to support the new message management functionality.
769-769: LGTM!The
asynckeyword was correctly removed fromgetUIStateFromAIStatesince it contains noawaitcalls, addressing the previous review feedback.
934-940: LGTM!The new
resubmitanddeleteMessageActionfunctions are correctly exported in the AI actions object, enabling the message edit/delete functionality from the UI.
1005-1013: > Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| await updateMessage( | ||
| messageId, | ||
| typeof editedMessageInState.content === 'object' | ||
| ? JSON.stringify(editedMessageInState.content) | ||
| : editedMessageInState.content | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect updateMessage call signature.
Per the relevant code snippet from lib/actions/chat.ts, updateMessage expects (messageId: string, updates: Partial<NewMessage>). The current call passes a raw string as the second argument instead of an object with a content property.
🐛 Proposed fix
await updateMessage(
messageId,
- typeof editedMessageInState.content === 'object'
- ? JSON.stringify(editedMessageInState.content)
- : editedMessageInState.content
+ {
+ content: typeof editedMessageInState.content === 'object'
+ ? JSON.stringify(editedMessageInState.content)
+ : editedMessageInState.content
+ }
)🤖 Prompt for AI Agents
In `@app/actions.tsx` around lines 653 - 658, The call to updateMessage is passing
a raw string but updateMessage(messageId: string, updates: Partial<NewMessage>)
expects an updates object; change the second argument to an object with a
content property (e.g., { content: ... }) using the same conditional
JSON.stringify logic on editedMessageInState.content so the signature matches
updateMessage and the content is stored on the NewMessage shape.


User description
This PR implements the requested message editing functionality and adds individual message deletion.
Key changes:
lib/actions/chat-db.tsandlib/actions/chat.ts.saveChatwas updated to handle upserts usingonConflictDoUpdateto prevent primary key violations during re-saves.resubmitanddeleteMessageActioninapp/actions.tsx. Resubmitting a message updates its content in the DB, deletes all subsequent messages in that chat, and triggers a new AI response. Deleting a message removes it and all trailing messages.UserMessagecomponent now includes "Edit" and "Delete" icons visible on hover. It supports in-place editing usingreact-textarea-autosizeand provides a confirmation dialog for deletions usingAlertDialog.submit()was extracted intoprocessChatWorkflow()to be shared withresubmit().getCurrentUserIdOnServer()and passed the currentmapProviderfrom the client to ensure consistency.Verified the implementation with
bun run buildand fixed several type errors and a missing "use client" directive.PR created automatically by Jules for task 11829601418741662983 started by @ngoiyaeric
PR Type
Enhancement
Description
Implement message editing with inline textarea and save functionality
Add message deletion with confirmation dialog and trailing message cleanup
Refactor AI workflow into reusable
processChatWorkflowfunctionAdd database operations for updating, deleting, and managing messages
Enhance UI with edit/delete icons visible on hover with proper state management
Diagram Walkthrough
File Walkthrough
chat-db.ts
Database layer message operations implementationlib/actions/chat-db.ts
gtoperator import from drizzle-orm for timestamp comparisonsonConflictDoUpdateinsaveChatto handle message upsertsand prevent primary key violations
updateMessagefunction to update message content by IDdeleteMessagefunction to delete a specific messagedeleteTrailingMessagesfunction to delete all messages createdafter a given timestamp
chat.ts
Server action wrappers for message operationslib/actions/chat.ts
updateMessage,deleteMessage,deleteTrailingMessagesupdateMessagewrapper function that calls database layer witherror handling
deleteMessagewrapper function with error handlingdeleteTrailingMessageswrapper function with error handlingactions.tsx
Server actions for message editing and deletion workflowapp/actions.tsx
submitinto reusableprocessChatWorkflowfunctionresubmitserver action to handle message editing with trailingmessage deletion
deleteMessageActionserver action to handle message deletiongetCurrentUserIdOnServer()call
submitto passmapProviderto workflow and use newprocessChatWorkflowfunctionresubmitanddeleteMessageActionin AI actions configurationgetUIStateFromAIStateto pass messageidprop toUserMessagecomponent
user-message.tsx
User message component with edit and delete UIcomponents/user-message.tsx
handleEditto initialize editing state with currentmessage content
handleSaveto submit edited message viaresubmitactionhandleDeleteto trigger message deletion viadeleteMessageActiontrailing messages
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.