Skip to content

Daily branch 2025 11 20 #109

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.

Sign up for GitHub

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

Merged
rossmanko merged 4 commits into from
Nov 21, 2025
Merged

Daily branch 2025 11 20 #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
433192f
refactor(system-prompt): move getResumeSection to end of prompt to im…
rossmanko Nov 20, 2025
d08dee1
refactor(chat-handler): provide different number of steps based on su…
rossmanko Nov 20, 2025
80aba28
refactor(providers): use gemini 2.5 flash for ask mode
rossmanko Nov 20, 2025
03c96f6
fix: prevent deletion of valid messages when regenerating after error
rossmanko Nov 20, 2025
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
26 changes: 22 additions & 4 deletions app/hooks/useChatHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,17 @@ export const useChatHandlers = ({
: todos;
if (cleanedTodos !== todos) setTodos(cleanedTodos);

// Check if the last assistant message has actual content
// If it's empty or has no parts, it was never saved (error occurred)
// In this case, we shouldn't delete anything from the database
const hasContent = lastAssistant?.parts && lastAssistant.parts.length > 0;

if (!temporaryChatsEnabled) {
// Delete last assistant message and update todos in a single transaction
await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
// Only delete if the last assistant message has content
// This prevents deleting previous valid messages when an error occurred
if (hasContent) {
await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
}
Comment on lines +301 to +311
Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential bug: Empty assistant messages may still be saved to the database.

The guard assumes that if hasContent is false, the message was never saved to the database. However, handleStop (lines 266-276) calls saveAssistantMessage without checking whether parts is empty. This means an empty assistant message can be saved if an error occurs immediately after the stream starts.

When regenerating, if the empty message exists in the database but hasContent is false in the UI state, the deletion will be skipped. The backend will then fetch the empty assistant message from the database, which could cause incorrect regeneration context.

Consider one of these approaches:

Option 1: Always attempt deletion (simplest)

-    // Check if the last assistant message has actual content
-    // If it's empty or has no parts, it was never saved (error occurred)
-    // In this case, we shouldn't delete anything from the database
-    const hasContent = lastAssistant?.parts && lastAssistant.parts.length > 0;
-
     if (!temporaryChatsEnabled) {
-      // Only delete if the last assistant message has content
-      // This prevents deleting previous valid messages when an error occurred
-      if (hasContent) {
-        await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
-      }
+      // Always delete the last assistant message if it exists
+      // The mutation safely handles the case where it doesn't exist in DB
+      if (lastAssistant) {
+        await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
+      }

Option 2: Prevent saving empty messages in handleStop
Modify handleStop to check lastMessage.parts?.length > 0 before calling saveAssistantMessage.

Option 3: Delete when lastAssistant exists (regardless of content)

     if (!temporaryChatsEnabled) {
-      // Only delete if the last assistant message has content
-      // This prevents deleting previous valid messages when an error occurred
-      if (hasContent) {
+      // Delete the last assistant message if one exists in the UI state
+      // This includes empty messages that may have been saved during errors
+      if (lastAssistant) {
         await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
       }
📝 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.

Suggested change
// Check if the last assistant message has actual content
// If it's empty or has no parts, it was never saved (error occurred)
// In this case, we shouldn't delete anything from the database
const hasContent = lastAssistant?.parts && lastAssistant.parts.length > 0;
if (!temporaryChatsEnabled) {
// Delete last assistant message and update todos in a single transaction
await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
// Only delete if the last assistant message has content
// This prevents deleting previous valid messages when an error occurred
if (hasContent) {
await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
}
if (!temporaryChatsEnabled) {
// Always delete the last assistant message if it exists
// The mutation safely handles the case where it doesn't exist in DB
if (lastAssistant) {
await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
}
🤖 Prompt for AI Agents 
In app/hooks/useChatHandlers.ts around lines 301-311 (and related handleStop at
~266-276), the UI assumes empty assistant parts mean the message was never
saved, but handleStop may save an empty assistant message; modify handleStop to
avoid persisting empty messages by checking lastMessage.parts?.length > 0 before
calling saveAssistantMessage, and only call saveAssistantMessage when there is
at least one non-empty part; this prevents orphan empty records and keeps the
existing deletion guard intact.

// For persisted chats, backend fetches from database - explicitly send no messages
regenerate({
body: {
Expand Down Expand Up @@ -348,11 +356,21 @@ export const useChatHandlers = ({
},
});
} else {
// For temporary chats, send all messages
// For temporary chats, filter out empty assistant message if present (from error)
// Check if last message is an empty assistant message
const lastMessage = messages[messages.length - 1];
const isLastMessageEmptyAssistant =
lastMessage?.role === "assistant" &&
(!lastMessage.parts || lastMessage.parts.length === 0);

const messagesToSend = isLastMessageEmptyAssistant
? messages.slice(0, -1)
: messages;

regenerate({
body: {
mode: chatMode,
messages,
messages: messagesToSend,
Comment on lines +359 to +373
Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Good implementation for temporary chats, but consider persisted chats consistency.

The filtering logic for empty assistant messages in temporary chats is well-implemented with clear variable naming and proper conditional handling.

However, there's an inconsistency: persisted chats (lines 347-357) don't handle empty assistant messages at all. If an empty assistant message exists in the database (due to the issue flagged above), the backend will fetch it during retry, while temporary chats would filter it out. This creates divergent behavior between the two modes.

For consistency, consider deleting empty assistant messages in persisted chats as well:

   if (!temporaryChatsEnabled) {
+    // Check if last message is an empty assistant message from an error
+    const lastMessage = messages[messages.length - 1];
+    const isLastMessageEmptyAssistant =
+      lastMessage?.role === "assistant" &&
+      (!lastMessage.parts || lastMessage.parts.length === 0);
+    
+    // Delete empty assistant message if present
+    if (isLastMessageEmptyAssistant) {
+      await deleteLastAssistantMessage({ chatId, todos: cleanedTodos });
+    }
+    
     // For persisted chats, backend fetches from database - explicitly send no messages
     regenerate({

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In app/hooks/useChatHandlers.ts around lines 347-373, persisted chats (lines
~347-357) don't handle empty assistant messages while temporary chats (lines
359-373) filter them out; make persisted behavior consistent by detecting if the
last message is an assistant with no parts (lastMessage?.role === "assistant" &&
(!lastMessage.parts || lastMessage.parts.length === 0)), then remove it from the
messages array before calling regenerate and also delete that empty message from
the database (or mark it removed) within the persisted-chat code path so retries
won't re-fetch the empty assistant message.

todos: cleanedTodos,
regenerate: true,
temporary: true,
Expand Down
12 changes: 6 additions & 6 deletions lib/ai/providers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import PostHogClient from "@/app/posthog";
import type { SubscriptionTier } from "@/types";

const baseProviders = {
"ask-model": openrouter("qwen/qwen3-coder:exacto"),
"ask-model-free": openrouter("qwen/qwen3-coder"),
"agent-model": xai("grok-code-fast-1"),
"agent-model-with-vision": xai("grok-4-fast-reasoning"),
"vision-model": openrouter("google/gemini-2.5-flash-preview-09-2025"),
"vision-model-for-pdfs": openrouter(
"ask-model": openrouter("google/gemini-2.5-flash-preview-09-2025"),
"ask-model-free": openrouter("google/gemini-2.5-flash-preview-09-2025"),
"ask-vision-model": openrouter("google/gemini-2.5-flash-preview-09-2025"),
"ask-vision-model-for-pdfs": openrouter(
"google/gemini-2.5-flash-preview-09-2025",
),
"agent-model": xai("grok-code-fast-1"),
"agent-vision-model": xai("grok-4-fast-reasoning"),
"title-generator-model": openai("gpt-4.1-mini-2025-04-14"),
"summarization-model": xai("grok-4-fast-non-reasoning"),
};
Expand Down
3 changes: 2 additions & 1 deletion lib/api/chat-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import {
} from "@/lib/utils/stream-writer-utils";
import { ConvexHttpClient } from "convex/browser";
import { api } from "@/convex/_generated/api";
import { getMaxStepsForUser } from "@/lib/chat/chat-processor";

const convex = new ConvexHttpClient(process.env.NEXT_PUBLIC_CONVEX_URL!);

Expand Down Expand Up @@ -347,7 +348,7 @@ export const createChatHandler = () => {
},
},
experimental_transform: smoothStream({ chunking: "word" }),
stopWhen: stepCountIs(mode === "ask" ? 5 : 20),
stopWhen: stepCountIs(getMaxStepsForUser(mode, subscription)),
onChunk: async (chunk) => {
// Track all tool calls immediately (no throttle)
if (chunk.chunk.type === "tool-call") {
Expand Down
33 changes: 30 additions & 3 deletions lib/chat/chat-processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,33 @@ import type { ChatMode, SubscriptionTier } from "@/types";
import { UIMessage } from "ai";
import { processMessageFiles } from "@/lib/utils/file-transform-utils";

/**
* Get maximum steps allowed for a user based on mode and subscription tier
* Agent mode: Always 20 steps (for all paid users)
* Ask mode: Free: 5 steps, Pro/Team: 10 steps, Ultra: 15 steps
*/
export const getMaxStepsForUser = (
mode: ChatMode,
subscription: SubscriptionTier,
): number => {
// Agent mode always gets 20 steps regardless of subscription
if (mode === "agent") {
return 20;
}

// Ask mode steps vary by subscription tier
if (subscription === "free") {
return 5; // Free users limited to 5 steps
}

if (subscription === "ultra") {
return 15; // Ultra users get 15 steps
}

// Pro and Team users get 10 steps
return 10;
};

/**
* Selects the appropriate model based on mode and file content
* @param mode - Chat mode (ask or agent)
Expand All @@ -17,15 +44,15 @@ export function selectModel(
): string {
// Prefer a dedicated PDF vision model for PDFs in ask mode
if (containsPdfFiles && mode === "ask") {
return "vision-model-for-pdfs";
return "ask-vision-model-for-pdfs";
}

// If there are media files (images or otherwise), choose appropriate vision model
if (containsMediaFiles && mode === "ask") {
return "vision-model";
return "ask-vision-model";
}
if (containsMediaFiles && mode === "agent") {
return "agent-model-with-vision";
return "agent-vision-model";
}

// Otherwise, choose based on mode
Expand Down
4 changes: 2 additions & 2 deletions lib/system-prompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ The current date is ${currentDateTime}.`;
if (mode === "ask") {
sections.push(getToneAndFormattingSection());
sections.push(getKnowledgeCutoffSection());
sections.push(getResumeSection(finishReason));
} else {
sections.push(getCommunicationSection());
sections.push(getToolCallingSection());
Expand All @@ -281,7 +280,6 @@ The current date is ${currentDateTime}.`;
sections.push(getTaskManagementSection());
sections.push(getSummarySection());
sections.push(getSandboxEnvironmentSection());
sections.push(getResumeSection(finishReason));
sections.push(getFinalInstructionsSection());
}

Expand All @@ -293,5 +291,7 @@ The current date is ${currentDateTime}.`;
sections.push(`<personality>\n${personalityInstructions}\n</personality>`);
}

sections.push(getResumeSection(finishReason));

return sections.filter(Boolean).join("\n\n");
};