Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 17 additions & 37 deletions apps/sim/app/api/copilot/chat/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -802,49 +802,29 @@ export async function POST(req: NextRequest) {
toolNames: toolCalls.map((tc) => tc?.name).filter(Boolean),
})

// Save messages to database after streaming completes (including aborted messages)
// NOTE: Messages are saved by the client via update-messages endpoint with full contentBlocks.
// Server only updates conversationId here to avoid overwriting client's richer save.
if (currentChat) {
const updatedMessages = [...conversationHistory, userMessage]

// Save assistant message if there's any content or tool calls (even partial from abort)
if (assistantContent.trim() || toolCalls.length > 0) {
const assistantMessage = {
id: crypto.randomUUID(),
role: 'assistant',
content: assistantContent,
timestamp: new Date().toISOString(),
...(toolCalls.length > 0 && { toolCalls }),
}
updatedMessages.push(assistantMessage)
logger.info(
`[${tracker.requestId}] Saving assistant message with content (${assistantContent.length} chars) and ${toolCalls.length} tool calls`
)
} else {
logger.info(
`[${tracker.requestId}] No assistant content or tool calls to save (aborted before response)`
)
}

// Persist only a safe conversationId to avoid continuing from a state that expects tool outputs
const previousConversationId = currentChat?.conversationId as string | undefined
const responseId = lastSafeDoneResponseId || previousConversationId || undefined

// Update chat in database immediately (without title)
await db
.update(copilotChats)
.set({
messages: updatedMessages,
updatedAt: new Date(),
...(responseId ? { conversationId: responseId } : {}),
})
.where(eq(copilotChats.id, actualChatId!))
if (responseId) {
await db
.update(copilotChats)
.set({
updatedAt: new Date(),
conversationId: responseId,
})
.where(eq(copilotChats.id, actualChatId!))

logger.info(`[${tracker.requestId}] Updated chat ${actualChatId} with new messages`, {
messageCount: updatedMessages.length,
savedUserMessage: true,
savedAssistantMessage: assistantContent.trim().length > 0,
updatedConversationId: responseId || null,
})
logger.info(
`[${tracker.requestId}] Updated conversationId for chat ${actualChatId}`,
{
updatedConversationId: responseId,
}
)
}
Comment on lines +812 to +827
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server now only updates conversationId when responseId is truthy, but there's no update to updatedAt when responseId is falsy. This means if a chat completes without a valid responseId, the database won't reflect that the chat was recently active.

Consider always updating updatedAt to track chat activity:

Suggested change
if (responseId) {
await db
.update(copilotChats)
.set({
updatedAt: new Date(),
conversationId: responseId,
})
.where(eq(copilotChats.id, actualChatId!))
logger.info(`[${tracker.requestId}] Updated chat ${actualChatId} with new messages`, {
messageCount: updatedMessages.length,
savedUserMessage: true,
savedAssistantMessage: assistantContent.trim().length > 0,
updatedConversationId: responseId || null,
})
logger.info(
`[${tracker.requestId}] Updated conversationId for chat ${actualChatId}`,
{
updatedConversationId: responseId,
}
)
}
if (responseId) {
await db
.update(copilotChats)
.set({
updatedAt: new Date(),
conversationId: responseId,
})
.where(eq(copilotChats.id, actualChatId!))
logger.info(
`[${tracker.requestId}] Updated conversationId for chat ${actualChatId}`,
{
updatedConversationId: responseId,
}
)
} else {
// Still update the timestamp even without conversationId
await db
.update(copilotChats)
.set({ updatedAt: new Date() })
.where(eq(copilotChats.id, actualChatId!))
}

This ensures chat activity is properly tracked even in edge cases.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/copilot/chat/route.ts
Line: 812:827

Comment:
The server now only updates `conversationId` when `responseId` is truthy, but there's no update to `updatedAt` when `responseId` is falsy. This means if a chat completes without a valid `responseId`, the database won't reflect that the chat was recently active.

Consider always updating `updatedAt` to track chat activity:

```suggestion
              if (responseId) {
                await db
                  .update(copilotChats)
                  .set({
                    updatedAt: new Date(),
                    conversationId: responseId,
                  })
                  .where(eq(copilotChats.id, actualChatId!))

                logger.info(
                  `[${tracker.requestId}] Updated conversationId for chat ${actualChatId}`,
                  {
                    updatedConversationId: responseId,
                  }
                )
              } else {
                // Still update the timestamp even without conversationId
                await db
                  .update(copilotChats)
                  .set({ updatedAt: new Date() })
                  .where(eq(copilotChats.id, actualChatId!))
              }
```

This ensures chat activity is properly tracked even in edge cases.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

}
} catch (error) {
logger.error(`[${tracker.requestId}] Error processing stream:`, error)
Expand Down
12 changes: 12 additions & 0 deletions apps/sim/app/api/copilot/chat/update-messages/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ export async function POST(req: NextRequest) {

const { chatId, messages, planArtifact, config } = UpdateMessagesSchema.parse(body)

// Debug: Log what we're about to save
const lastMsgParsed = messages[messages.length - 1]
if (lastMsgParsed?.role === 'assistant') {
logger.info(`[${tracker.requestId}] Parsed messages to save`, {
messageCount: messages.length,
lastMsgId: lastMsgParsed.id,
lastMsgContentLength: lastMsgParsed.content?.length || 0,
lastMsgContentBlockCount: lastMsgParsed.contentBlocks?.length || 0,
lastMsgContentBlockTypes: lastMsgParsed.contentBlocks?.map((b: any) => b?.type) || [],
})
}

// Verify that the chat belongs to the user
const [chat] = await db
.select()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ export const DiffControls = memo(function DiffControls() {
!isResizing && 'transition-[bottom,right] duration-100 ease-out'
)}
style={{
bottom: 'calc(var(--terminal-height) + 8px)',
right: 'calc(var(--panel-width) + 8px)',
bottom: 'calc(var(--terminal-height) + 16px)',
right: 'calc(var(--panel-width) + 16px)',
}}
>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,17 +470,8 @@ const CopilotMessage: FC<CopilotMessageProps> = memo(
{/* Content blocks in chronological order */}
{memoizedContentBlocks}

{/* Show streaming indicator if streaming but no text content yet after tool calls */}
{isStreaming &&
!message.content &&
message.contentBlocks?.every((block) => block.type === 'tool_call') && (
<StreamingIndicator />
)}

{/* Streaming indicator when no content yet */}
{!cleanTextContent && !message.contentBlocks?.length && isStreaming && (
<StreamingIndicator />
)}
{/* Always show streaming indicator at the end while streaming */}
{isStreaming && <StreamingIndicator />}

{message.errorType === 'usage_limit' && (
<div className='flex gap-1.5'>
Expand Down
Loading