Skip to content

Add per-chat settings for chat mode and selected model#2544

Open
princeaden1 wants to merge 2 commits intodyad-sh:mainfrom
princeaden1:store-chatmode
Open

Add per-chat settings for chat mode and selected model#2544
princeaden1 wants to merge 2 commits intodyad-sh:mainfrom
princeaden1:store-chatmode

Conversation

@princeaden1
Copy link
Collaborator

@princeaden1 princeaden1 commented Feb 6, 2026

Closes: #1844


Open with Devin

Summary by cubic

Adds per‑chat settings for chat mode and selected model so each chat can have its own configuration, with global settings used as fallback. Streaming and UI now respect these per‑chat overrides.

  • New Features

    • Store per-chat chat_mode and selected_model on the chats table.
    • Added useChatSettings hook with global fallback and update mutations.
    • Updated ChatModeSelector and ModelPicker to read/write per-chat when a chat is selected.
    • New IPC: chat:get-settings and chat:update-settings; stream handler merges per-chat with global settings.
    • Added e2e test verifying mode/model isolation between chats.
  • Migration

    • Run Drizzle migrations to add chat_mode and selected_model.
    • Existing chats continue using global settings until a per-chat override is saved.

Written for commit 4658c55. Summary will update on new commits.

@wwwillchen
Copy link
Collaborator

@BugBot run

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @princeaden1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant enhancement by enabling per-chat configuration for both the chat mode and the selected language model. This change allows users to tailor the behavior of each conversation independently, providing greater flexibility and a more personalized experience without affecting global application settings. The implementation includes database schema modifications, new frontend hooks, updated UI components, and backend logic to ensure these settings are consistently applied and persisted.

Highlights

  • Database Schema Update: The chats table in the database has been extended with two new columns: chat_mode and selected_model. These columns will store the specific chat mode and selected language model for each individual chat, allowing for per-chat customization.
  • Per-Chat Settings Management: A new useChatSettings React hook has been introduced to provide an interface for fetching and updating these per-chat settings. This hook intelligently falls back to global user settings if no specific setting is defined for a given chat.
  • UI Integration: The ChatModeSelector and ModelPicker components have been updated to utilize the new per-chat settings. When a chat is active, these components will display and allow modification of the chat's specific mode and model, overriding global preferences.
  • IPC Handlers and Stream Processing: New IPC handlers (getChatSettings, updateChatSettings) were added to facilitate communication between the renderer and main processes for managing per-chat settings. The chat stream processing logic was also updated to respect these per-chat settings during AI interactions.
  • End-to-End Testing: A new end-to-end test (per_chat_settings.spec.ts) has been added to verify that chat mode settings are correctly isolated and persisted across different chat sessions, ensuring the feature works as intended.
Changelog
  • drizzle/0026_fluffy_iron_monger.sql
    • Added chat_mode column of type text to the chats table.
    • Added selected_model column of type text to the chats table.
  • drizzle/meta/0026_snapshot.json
    • Updated the Drizzle ORM snapshot to reflect the new chat_mode and selected_model columns in the chats table schema.
  • drizzle/meta/_journal.json
    • Added a new migration entry (0026_fluffy_iron_monger) to the Drizzle migration journal.
  • e2e-tests/per_chat_settings.spec.ts
    • Added a new end-to-end test to verify that chat mode settings are isolated between different chats and persist correctly.
  • src/components/ChatModeSelector.tsx
    • Imported useChatSettings hook and selectedChatIdAtom.
    • Modified selectedMode logic to prioritize effectiveChatMode from useChatSettings if a chat is selected, otherwise fall back to global settings.
    • Updated handleModeChange to call updatePerChatMode if a chat is selected, otherwise update global settings.
  • src/components/ModelPicker.tsx
    • Imported useChatSettings hook and selectedChatIdAtom.
    • Modified onModelSelect to call updatePerChatModel if a chat is selected, otherwise update global settings.
    • Updated selectedModel logic to prioritize effectiveModel from useChatSettings if a chat is selected, otherwise fall back to global settings.
  • src/db/schema.ts
    • Added chatMode column to the chats table with an enum type for chat modes.
    • Added selectedModel column to the chats table with a JSON type for storing model details.
  • src/hooks/useChatSettings.ts
    • Created a new React hook useChatSettings to manage per-chat chatMode and selectedModel.
    • Implemented logic to fetch chat-specific settings from the IPC layer.
    • Implemented mutation to update chat-specific settings via IPC.
    • Provided effective settings that fall back to global user settings if per-chat settings are not defined.
    • Added functions to update chatMode and selectedModel for the current chat.
  • src/ipc/handlers/chat_handlers.ts
    • Added getChatSettings IPC handler to retrieve chatMode and selectedModel for a given chat ID from the database.
    • Added updateChatSettings IPC handler to dynamically update chatMode and/or selectedModel for a given chat ID in the database.
  • src/ipc/handlers/chat_stream_handlers.ts
    • Modified chat stream processing to fetch chat-specific settings (chatMode, selectedModel) from the database.
    • Integrated per-chat settings into the settings object used for AI interactions, allowing them to override global settings.
    • Removed redundant readSettings() calls where per-chat aware settings are now available.
  • src/ipc/types/chat.ts
    • Imported ChatModeSchema and LargeLanguageModelSchema.
    • Extended ChatSchema to include optional chatMode and selectedModel properties.
    • Defined UpdateChatSettingsParamsSchema for updating chat settings.
    • Defined ChatSettingsSchema for retrieving chat settings.
    • Added new IPC contract definitions for getChatSettings and updateChatSettings.
  • src/lib/queryKeys.ts
    • Added a new query key chats.settings for caching per-chat settings based on chatId.
Activity
  • The author princeaden1 initiated this pull request to implement per-chat settings.
  • The pull request directly addresses and closes issue [bug] Isolate the chat mode used for a particular chat #1844, indicating a resolution to a reported feature request or bug.
  • Database schema migrations were generated and applied to introduce new columns for chat-specific settings.
  • New IPC communication channels and handlers were established to support the retrieval and modification of these settings.
  • Key UI components were refactored to dynamically adjust their behavior based on the active chat's settings.
  • A new end-to-end test was developed to validate the isolation and persistence of chat settings.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 6, 2026

Greptile Overview

Greptile Summary

This PR implements per-chat settings for chat mode and selected model, allowing each chat to maintain its own configuration independent of global settings. When a chat is selected, the UI components (ChatModeSelector and ModelPicker) read and write per-chat settings; otherwise they use global settings.

Key Changes:

  • Database migration adds nullable chat_mode and selected_model columns to chats table
  • New useChatSettings hook provides per-chat settings with automatic fallback to global settings
  • IPC handlers getChatSettings and updateChatSettings enable reading/writing per-chat settings
  • Streaming handler merges per-chat settings with global settings before processing requests
  • Comprehensive E2E test verifies mode isolation between different chats
  • Migration function properly converts deprecated "agent" mode to "build"

Architecture:
The implementation follows established patterns: TanStack Query for state management, proper query invalidation, Zod schemas for type safety, and IPC contracts for renderer-main communication. Settings are stored as nullable columns, with null indicating use of global defaults.

Confidence Score: 4/5

  • Safe to merge with one validation issue to address
  • The implementation is solid with proper patterns, comprehensive testing, and correct fallback logic. One logical issue exists: the updateChatSettings handler lacks chat existence validation (unlike getChatSettings), which could silently fail for invalid chat IDs. This should be fixed to maintain consistency and prevent bugs.
  • Pay attention to src/ipc/handlers/chat_handlers.ts - the updateChatSettings handler needs chat existence validation

Important Files Changed

Filename Overview
src/db/schema.ts Adds chatMode enum field and selectedModel JSON field to chats table schema with proper typing
src/hooks/useChatSettings.ts New hook managing per-chat settings with global fallback, query invalidation on updates
src/ipc/handlers/chat_handlers.ts Adds getChatSettings and updateChatSettings handlers; missing chat existence validation in update handler
src/ipc/handlers/chat_stream_handlers.ts Merges per-chat settings with global settings for streaming, applies chat mode migration correctly
src/components/ChatModeSelector.tsx Updated to read/write per-chat mode when chat selected, falls back to global settings otherwise
src/components/ModelPicker.tsx Updated to read/write per-chat model when chat selected, maintains query invalidation pattern

Sequence Diagram

sequenceDiagram
    participant User
    participant ChatModeSelector
    participant useChatSettings
    participant IpcClient
    participant ChatHandlers
    participant Database

    User->>ChatModeSelector: Select chat mode
    ChatModeSelector->>useChatSettings: updateChatMode(mode)
    useChatSettings->>IpcClient: updateChatSettings(chatId, mode)
    IpcClient->>ChatHandlers: chat:update-settings
    ChatHandlers->>Database: UPDATE chats SET chat_mode
    Database-->>ChatHandlers: success
    ChatHandlers-->>IpcClient: void
    IpcClient-->>useChatSettings: void
    useChatSettings->>useChatSettings: invalidateQueries(chat settings)
    useChatSettings->>useChatSettings: invalidateQueries(token count)
    useChatSettings-->>ChatModeSelector: updated

    Note over User,Database: When sending a message
    User->>ChatModeSelector: Send message
    ChatModeSelector->>ChatHandlers: chat:stream-request
    ChatHandlers->>Database: SELECT chat settings
    Database-->>ChatHandlers: chatMode, selectedModel
    ChatHandlers->>ChatHandlers: Merge with global settings
    ChatHandlers->>ChatHandlers: Use effective settings for stream
Loading

Last reviewed commit: ac87ef3

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ Creating new chat modifies global selectedChatMode affecting other chats without per-chat overrides (src/components/ChatList.tsx:104)

In ChatList.tsx:104, when creating a new chat, the code calls updateSettings({ selectedChatMode: effectiveDefaultMode }) which modifies the global selectedChatMode setting rather than setting a per-chat mode for the new chat.

Root Cause

With the new per-chat settings feature, the global selectedChatMode serves as a fallback for chats that don't have an explicit per-chat mode set. When ChatList.tsx modifies the global setting on every new chat creation, it can inadvertently change the displayed mode for all other existing chats that rely on the global fallback.

Example scenario:

  1. User sets global default to "build"
  2. User creates Chat #1, explicitly sets it to "ask" (stored as per-chat setting)
  3. User creates Chat #2 - code resets global to "local-agent" (from getEffectiveDefaultChatMode)
  4. Now Chat #1 still shows "ask" (has per-chat override), but any chat without per-chat settings will show "local-agent" instead of the user's intended "build" default

Impact: The global chat mode setting gets overwritten every time a new chat is created, causing unexpected mode changes for chats relying on the global fallback.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 12 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/hooks/useChatSettings.ts">

<violation number="1" location="src/hooks/useChatSettings.ts:71">
P2: `isUsingGlobalSettings` reports false when `chatSettings` is undefined (e.g., disabled or loading query), even though the hook falls back to global settings. Treat both `null` and `undefined` as “no per-chat override.”</violation>
</file>

<file name="src/ipc/handlers/chat_handlers.ts">

<violation number="1" location="src/ipc/handlers/chat_handlers.ts:207">
P2: Guard against empty updates. If no fields are provided, return early (or throw a validation error) before calling update; otherwise this can execute an UPDATE with no SET columns and fail at runtime.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


// Check if using global settings (no per-chat override)
isUsingGlobalSettings: {
chatMode: chatSettings?.chatMode === null,
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 6, 2026

Choose a reason for hiding this comment

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

P2: isUsingGlobalSettings reports false when chatSettings is undefined (e.g., disabled or loading query), even though the hook falls back to global settings. Treat both null and undefined as “no per-chat override.”

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/useChatSettings.ts, line 71:

<comment>`isUsingGlobalSettings` reports false when `chatSettings` is undefined (e.g., disabled or loading query), even though the hook falls back to global settings. Treat both `null` and `undefined` as “no per-chat override.”</comment>

<file context>
@@ -0,0 +1,84 @@
+
+    // Check if using global settings (no per-chat override)
+    isUsingGlobalSettings: {
+      chatMode: chatSettings?.chatMode === null,
+      model: chatSettings?.selectedModel === null,
+    },
</file context>
Fix with Cubic

updates.selectedModel = selectedModel;
}

await db.update(chats).set(updates).where(eq(chats.id, chatId));
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 6, 2026

Choose a reason for hiding this comment

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

P2: Guard against empty updates. If no fields are provided, return early (or throw a validation error) before calling update; otherwise this can execute an UPDATE with no SET columns and fail at runtime.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ipc/handlers/chat_handlers.ts, line 207:

<comment>Guard against empty updates. If no fields are provided, return early (or throw a validation error) before calling update; otherwise this can execute an UPDATE with no SET columns and fail at runtime.</comment>

<file context>
@@ -172,5 +172,40 @@ export function registerChatHandlers() {
+      updates.selectedModel = selectedModel;
+    }
+
+    await db.update(chats).set(updates).where(eq(chats.id, chatId));
+  });
+
</file context>
Suggested change
await db.update(chats).set(updates).where(eq(chats.id, chatId));
if (Object.keys(updates).length === 0) {
return;
}
await db.update(chats).set(updates).where(eq(chats.id, chatId));
Fix with Cubic

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully introduces per-chat settings for chat mode and the selected model, which is a great enhancement for user experience. The changes are well-structured across the database, backend, and frontend. The new useChatSettings hook is a clean abstraction for managing these settings with a proper fallback mechanism to global settings. The addition of an end-to-end test is excellent and ensures the core functionality is robust. I have one suggestion to improve the correctness of the new hook for future use.

Comment on lines +71 to +72
chatMode: chatSettings?.chatMode === null,
model: chatSettings?.selectedModel === null,
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic for isUsingGlobalSettings is not entirely correct for all states. When chatId is null or when the per-chat settings are still loading, chatSettings is undefined. In this case, chatSettings?.chatMode is undefined, and undefined === null evaluates to false. This incorrectly implies that global settings are not being used, when in fact they are.

To make this property reliable for future use, you can use == null which checks for both null and undefined.

Suggested change
chatMode: chatSettings?.chatMode === null,
model: chatSettings?.selectedModel === null,
chatMode: chatSettings?.chatMode == null,
model: chatSettings?.selectedModel == null,

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4658c55d7e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +48 to +50
const selectedMode = selectedChatId
? effectiveChatMode
: settings?.selectedChatMode || "build";

Choose a reason for hiding this comment

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

P1 Badge Use route chat id before applying per-chat settings

These selectors now switch to per-chat state whenever selectedChatId is non-null, but that atom is not cleared on every non-chat navigation (for example AppList.handleNewApp only calls navigate({ to: "/" })). In that case, HomeChatInput still renders these controls and changing mode/model on the home screen mutates the previously opened chat instead of updating global defaults, which is a data-scope regression introduced by the new selectedChatId gate.

Useful? React with 👍 / 👎.

Comment on lines 266 to 268
selectedChatMode: chat.chatMode ?? globalSettings.selectedChatMode,
// Per-chat model overrides global if set
selectedModel: chat.selectedModel ?? globalSettings.selectedModel,

Choose a reason for hiding this comment

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

P2 Badge Keep token counting consistent with per-chat overrides

Streaming now applies per-chat selectedChatMode/selectedModel, but chat:count-tokens still reads only global settings (src/ipc/handlers/token_count_handlers.ts:61-74), so chats with overrides get incorrect context-window estimates and warning behavior in the UI (useCountTokens). This divergence is introduced here by making generation settings chat-specific without updating the token-count path to use the same per-chat values.

Useful? React with 👍 / 👎.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

🔍 Multi-Agent Code Review

Found 7 issue(s) flagged by consensus of 3 independent reviewers.
(0 issue(s) skipped - already commented)

Summary

Severity Count
🟡 MEDIUM 5
🟢 LOW 2

Issues to Address

Severity File Issue
🟡 MEDIUM src/ipc/handlers/chat_handlers.ts:194 updateChatSettings does not verify chat exists before update
🟡 MEDIUM src/ipc/handlers/chat_handlers.ts:198 Empty updates object causes pointless DB call; Record<string, unknown> loses type safety
🟡 MEDIUM src/hooks/useChatSettings.ts:70 isUsingGlobalSettings returns incorrect values during loading state
🟡 MEDIUM src/db/schema.ts:77 selectedModel column type duplicates LargeLanguageModel type definition
🟡 MEDIUM src/components/ModelPicker.tsx:88 getModelDisplayName has fragile forward reference to selectedModel
🟢 Low Priority Issues (2 items)
  • settings.selectedModel.name could be undefined if stored JSON is malformed - src/ipc/handlers/chat_stream_handlers.ts:484 — consider adding a fallback: settings.selectedModel?.name ?? 'unknown'
  • Settings read once at stream start is a behavioral change - src/ipc/handlers/chat_stream_handlers.ts:262 — previously readSettings() was called at multiple points during the stream, meaning mid-stream settings changes would be picked up. Now settings are captured once. Add a comment if intentional.

See inline comments for details.

Generated by multi-agent consensus review (3 agents, consensus threshold: 2)

Comment on lines +194 to +207
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;

// Build the update object dynamically
const updates: Record<string, unknown> = {};

if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}

await db.update(chats).set(updates).where(eq(chats.id, chatId));
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUMupdateChatSettings does not verify the chat exists before updating (3/3 agents flagged)

Unlike getChatSettings which throws "Chat not found", updateChatSettings will silently succeed with 0 rows affected if called with a non-existent chatId. This could mask bugs where the frontend tries to update settings for a deleted chat.

Additionally, if both chatMode and selectedModel are undefined, the updates object will be empty and an unnecessary db.update(chats).set({}) executes.

Suggested change
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;
// Build the update object dynamically
const updates: Record<string, unknown> = {};
if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}
await db.update(chats).set(updates).where(eq(chats.id, chatId));
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;
// Build the update object dynamically
const updates: Partial<typeof chats.$inferInsert> = {};
if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}
if (Object.keys(updates).length === 0) {
return;
}
await db.update(chats).set(updates).where(eq(chats.id, chatId));
});

Comment on lines +70 to +73
isUsingGlobalSettings: {
chatMode: chatSettings?.chatMode === null,
model: chatSettings?.selectedModel === null,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUMisUsingGlobalSettings returns incorrect values during loading state (3/3 agents flagged)

When chatSettings is undefined (query loading or chatId is null), chatSettings?.chatMode === null evaluates to false (undefined === null is false). This incorrectly reports the user is NOT using global settings when the data hasn't loaded yet.

This field doesn't appear to be consumed anywhere in the codebase currently. Consider removing it, or if needed, fix the logic to handle the loading state:

Suggested change
isUsingGlobalSettings: {
chatMode: chatSettings?.chatMode === null,
model: chatSettings?.selectedModel === null,
},
isUsingGlobalSettings: {
chatMode: !chatSettings || chatSettings.chatMode === null,
model: !chatSettings || chatSettings.selectedModel === null,
},

Comment on lines +77 to +81
selectedModel: text("selected_model", { mode: "json" }).$type<{
name: string;
provider: string;
customModelId?: number;
} | null>(),
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUM — Inline type duplicates LargeLanguageModel definition (3/3 agents flagged)

The structural type { name: string; provider: string; customModelId?: number } duplicates the LargeLanguageModel type from src/lib/schemas.ts. If LargeLanguageModel gains new fields in the future, this type won't be updated, causing a silent type mismatch between the DB layer and the rest of the app.

Consider importing and referencing the canonical type:

import type { LargeLanguageModel } from "@/lib/schemas";
// ...
selectedModel: text("selected_model", { mode: "json" }).$type<LargeLanguageModel | null>(),

Comment on lines 152 to +155
if (!settings) {
return null;
}
const selectedModel = settings?.selectedModel;
// Use per-chat model if available, otherwise fall back to global settings
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 MEDIUMgetModelDisplayName (defined at line 87) has a fragile forward reference to selectedModel (2/3 agents flagged)

The getModelDisplayName function references selectedModel which isn't assigned until here. While it works because the function is only called after this assignment, a future refactor that calls getModelDisplayName() before this line would cause a ReferenceError.

Consider moving this selectedModel declaration above the getModelDisplayName function definition, or refactoring getModelDisplayName to accept selectedModel as a parameter.

@wwwillchen
Copy link
Collaborator

@BugBot run

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

13 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +199 to +213
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;

// Build the update object dynamically
const updates: Record<string, unknown> = {};

if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}

await db.update(chats).set(updates).where(eq(chats.id, chatId));
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing chat existence validation before update. The getChatSettings handler verifies the chat exists and throws an error if not found, but updateChatSettings silently succeeds even for non-existent chat IDs. This inconsistency could mask bugs.

Suggested change
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;
// Build the update object dynamically
const updates: Record<string, unknown> = {};
if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}
await db.update(chats).set(updates).where(eq(chats.id, chatId));
});
createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
const { chatId, chatMode, selectedModel } = params;
// Verify chat exists before updating
const chat = await db.query.chats.findFirst({
where: eq(chats.id, chatId),
columns: { id: true },
});
if (!chat) {
throw new Error("Chat not found");
}
// Build the update object dynamically
const updates: Record<string, unknown> = {};
if (chatMode !== undefined) {
updates.chatMode = chatMode;
}
if (selectedModel !== undefined) {
updates.selectedModel = selectedModel;
}
await db.update(chats).set(updates).where(eq(chats.id, chatId));
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ipc/handlers/chat_handlers.ts
Line: 199:213

Comment:
Missing chat existence validation before update. The `getChatSettings` handler verifies the chat exists and throws an error if not found, but `updateChatSettings` silently succeeds even for non-existent chat IDs. This inconsistency could mask bugs.

```suggestion
  createTypedHandler(chatContracts.updateChatSettings, async (_, params) => {
    const { chatId, chatMode, selectedModel } = params;

    // Verify chat exists before updating
    const chat = await db.query.chats.findFirst({
      where: eq(chats.id, chatId),
      columns: { id: true },
    });

    if (!chat) {
      throw new Error("Chat not found");
    }

    // Build the update object dynamically
    const updates: Record<string, unknown> = {};

    if (chatMode !== undefined) {
      updates.chatMode = chatMode;
    }
    if (selectedModel !== undefined) {
      updates.selectedModel = selectedModel;
    }

    await db.update(chats).set(updates).where(eq(chats.id, chatId));
  });
```

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

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

🐛 3 issues in files not directly in the diff

🐛 Keyboard shortcut (Cmd+.) always toggles global chat mode, ignoring per-chat settings (src/hooks/useChatModeToggle.ts:24-33)

The useChatModeToggle hook always calls updateSettings({ selectedChatMode: newMode }) (line 33), which updates the global setting. When a chat is selected with a per-chat mode override, pressing Cmd+. (or Ctrl+.) changes the global mode instead of the per-chat mode. This creates an inconsistent state: the ChatModeSelector reads the per-chat mode via useChatSettings, but the keyboard shortcut modifies the global mode, so the UI won't reflect the shortcut's change.

Root Cause and Impact

The ChatModeSelector (src/components/ChatModeSelector.tsx:61-64) correctly routes mode changes to per-chat settings when selectedChatId is set, but the useChatModeToggle hook was not updated for the per-chat settings feature. It reads settings.selectedChatMode (global) and writes back to global settings at src/hooks/useChatModeToggle.ts:33:

updateSettings({ selectedChatMode: newMode });

Impact: When a user uses the keyboard shortcut to toggle chat mode while a chat is selected, the wrong settings are modified. The global mode changes but the per-chat mode (which the UI reads from) stays the same, making the shortcut appear broken. Worse, it silently modifies global settings as a side effect.


🐛 ChatInput reads global selectedChatMode for UI logic instead of per-chat effective mode (src/components/chat/ChatInput.tsx:176)

After this PR, ChatModeSelector and the streaming backend correctly use per-chat mode overrides, but ChatInput still reads settings?.selectedChatMode (global) for multiple UI-behavior decisions. When a chat has a per-chat mode override that differs from the global mode, ChatInput's UI logic will be wrong.

Affected Code Paths in ChatInput
  1. Send button disabling (src/components/chat/ChatInput.tsx:176): settings?.selectedChatMode !== "local-agent" — if per-chat mode is local-agent but global is build, the send button will be incorrectly disabled when there's an unapproved proposal.

  2. Plan mode fresh chat detection (src/components/chat/ChatInput.tsx:199-211): prevModeRef tracks settings?.selectedChatMode (global). If the user switches a specific chat to plan mode via ChatModeSelector, the global mode isn't changed, so this transition detection never fires.

  3. Plan mode fresh chat creation (src/components/chat/ChatInput.tsx:257): settings?.selectedChatMode === "plan" checks global, so switching a per-chat mode to plan won't trigger the fresh chat flow.

  4. Proposal actions visibility (src/components/chat/ChatInput.tsx:487-488): settings.selectedChatMode !== "ask" && settings.selectedChatMode !== "local-agent" — if per-chat mode is ask but global is build, proposal actions will incorrectly appear.

Impact: The frontend UI behavior (button states, proposal display, plan mode handling) won't match the actual per-chat mode, causing a confusing and broken user experience when per-chat overrides differ from global settings.


⚠️ Creating new chat modifies global selectedChatMode affecting other chats without per-chat overrides (src/components/ChatList.tsx:104)

In ChatList.tsx:104, when creating a new chat, the code calls updateSettings({ selectedChatMode: effectiveDefaultMode }) which modifies the global selectedChatMode setting rather than setting a per-chat mode for the new chat.

Root Cause

With the new per-chat settings feature, the global selectedChatMode serves as a fallback for chats that don't have an explicit per-chat mode set. When ChatList.tsx modifies the global setting on every new chat creation, it can inadvertently change the displayed mode for all other existing chats that rely on the global fallback.

Example scenario:

  1. User sets global default to "build"
  2. User creates Chat #1, explicitly sets it to "ask" (stored as per-chat setting)
  3. User creates Chat #2 - code resets global to "local-agent" (from getEffectiveDefaultChatMode)
  4. Now Chat #1 still shows "ask" (has per-chat override), but any chat without per-chat settings will show "local-agent" instead of the user's intended "build" default

Impact: The global chat mode setting gets overwritten every time a new chat is created, causing unexpected mode changes for chats relying on the global fallback.

View 8 additional findings in Devin Review.

Open in Devin Review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac87ef3864

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +61 to +63
if (selectedChatId) {
updatePerChatMode(newMode);
} else {

Choose a reason for hiding this comment

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

P1 Badge Propagate per-chat mode to all chat-mode consumers

This change writes mode updates to per-chat state (updatePerChatMode) when a chat is selected, but other mode-dependent paths still read/write only global settings (useChatModeToggle updates settings.selectedChatMode, and ChatInput gates behavior off settings.selectedChatMode in its send-disable and plan-transition logic). In a chat whose override differs from global (for example chat override = plan, global = build), Cmd+. toggling and several input behaviors operate on stale global mode, so the UI no longer matches the mode actually used by streaming.

Useful? React with 👍 / 👎.

Comment on lines +1604 to 1607
// Use per-chat aware settings (already defined earlier in this function)
if (
settings.autoApproveChanges &&
settings.selectedChatMode !== "ask"

Choose a reason for hiding this comment

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

P2 Badge Re-read approval settings before auto-applying edits

The auto-apply branch now relies on the settings snapshot created at stream start, because the previous end-of-stream readSettings() refresh was removed here. If a user turns off autoApproveChanges while a long response is streaming, this branch can still auto-apply edits using stale settings, which is a regression from the prior behavior that reloaded settings immediately before processFullResponseActions.

Useful? React with 👍 / 👎.

@github-actions
Copy link
Contributor

🔍 Dyadbot Code Review Summary

✅ No new issues found by consensus review. (7 issue(s) already commented on in prior reviews)

Prior multi-agent review and other reviewers have already identified and commented on the key issues in this PR:

Already Covered File Prior Comment
🟡 MEDIUM src/ipc/handlers/chat_handlers.ts:212 Empty updates guard + chat existence check
🟡 MEDIUM src/hooks/useChatSettings.ts:71-73 isUsingGlobalSettings incorrect during loading
🟡 MEDIUM src/db/schema.ts:81 Inline type duplicates LargeLanguageModel
🟡 MEDIUM src/components/ModelPicker.tsx:155 getModelDisplayName fragile forward reference
🔴 HIGH src/components/ChatModeSelector.tsx:52 selectedChatId gate affects home screen
🔴 HIGH src/components/ChatModeSelector.tsx:63 useChatModeToggle / Cmd+. desync with per-chat mode
🟡 MEDIUM src/ipc/handlers/chat_stream_handlers.ts:268 Token counting / settings consistency

Generated by Dyadbot code review (3 agents, consensus threshold: 2)

@github-actions github-actions bot added the needs-human:review-issue ai agent flagged an issue that requires human review label Feb 13, 2026
@github-actions
Copy link
Contributor

🎭 Playwright Test Results

❌ Some tests failed

OS Passed Failed Flaky Skipped
🍎 macOS 342 5 3 112
🪟 Windows 348 2 9 112

Summary: 690 passed, 7 failed, 12 flaky, 224 skipped

Failed Tests

🍎 macOS

  • free_agent_quota.spec.ts > free agent quota - full flow: mode availability, quota tracking, exceeded banner, switch to build
    • Error: expect(locator).toBeVisible() failed
  • free_agent_quota.spec.ts > free agent quota - quota resets after 24 hours
    • Error: expect(locator).toBeVisible() failed
  • mention_app.spec.ts > mention app (with pro)
    • Error: expect(string).toMatchSnapshot(expected) failed
  • plan_mode.spec.ts > plan mode - accept plan redirects to new chat and saves to disk
    • Error: expect(received).not.toEqual(expected) // deep equality
  • smart_context_deep.spec.ts > smart context deep - mention app should fallback to balanced
    • Error: expect(string).toMatchSnapshot(expected) failed

🪟 Windows

  • mention_app.spec.ts > mention app (with pro)
    • Error: expect(string).toMatchSnapshot(expected) failed
  • plan_mode.spec.ts > plan mode - accept plan redirects to new chat and saves to disk
    • Error: expect(received).not.toEqual(expected) // deep equality

📋 Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/free_agent_quota.spec.ts \
  e2e-tests/mention_app.spec.ts \
  e2e-tests/plan_mode.spec.ts \
  e2e-tests/smart_context_deep.spec.ts

⚠️ Flaky Tests

🍎 macOS

  • capacitor.spec.ts > capacitor upgrade and sync works (passed after 1 retry)
  • context_limit_banner.spec.ts > context limit banner shows 'costs extra' for long context (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed (passed after 1 retry)

🪟 Windows

  • chat_history.spec.ts > should open, navigate, and select from history menu (passed after 1 retry)
  • chat_input.spec.ts > send button disabled during pending proposal (passed after 1 retry)
  • chat_input.spec.ts > send button disabled during pending proposal - reject (passed after 2 retries)
  • concurrent_chat.spec.ts > concurrent chat (passed after 2 retries)
  • context_manage.spec.ts > manage context - exclude paths with smart context (passed after 2 retries)
  • edit_code.spec.ts > edit code edits the right file (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > node.js install flow (passed after 2 retries)
  • template-create-nextjs.spec.ts > create next.js app (passed after 1 retry)

📊 View full report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human:review-issue ai agent flagged an issue that requires human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Isolate the chat mode used for a particular chat

3 participants