Skip to content

feat(feishu): add /model command for per-chat model switching#2149

Open
yuanchenglu wants to merge 2 commits into
Hmbown:mainfrom
yuanchenglu:fix/feishu-perchat-model
Open

feat(feishu): add /model command for per-chat model switching#2149
yuanchenglu wants to merge 2 commits into
Hmbown:mainfrom
yuanchenglu:fix/feishu-perchat-model

Conversation

@yuanchenglu
Copy link
Copy Markdown

@yuanchenglu yuanchenglu commented May 26, 2026

Summary

Add a /model <name> command to the Feishu bridge, letting users switch the model for a specific chat (group or DM) without affecting other chats or restarting the bridge.

Motivation

The bridge previously only supported a single global model set via the DEEPSEEK_MODEL environment variable or default_text_model in config.toml. Users who wanted a different model for a particular Feishu group had no way to change it — they would need to restart the entire bridge with a different env var, which is impractical and disruptive for multi-group deployments.

Implementation

Three changes, all in integrations/feishu-bridge/src/:

lib.mjscommandAction()

Added a "model" case that parses /model <name> into { kind: "set_model", modelName }.

index.mjssetChatModel()

New function that stores or clears the per-chat model in the thread store:

  • /model <name> → stores the model name in threadStore.data.chats[chatId].model
  • /model default or /model with no args → clears the stored model, falling back to the bridge-level default

index.mjsensureThread() and runPrompt()

Both now check for a per-chat model before using config.model:

  • ensureThread(): when creating a new runtime thread, reads existing?.model || config.model
  • runPrompt(): reads state?.model || config.model for each turn submission, so a /model change takes effect on the very next message (independent of the thread-creation model)

Usage

/model deepseek-v4-flash      — switch this chat to Flash
/model deepseek-v4-pro        — switch this chat to Pro
/model default                — reset to bridge default (config.model)

Resolution priority

global default (config.model) < per-chat /model override

Testing

Manually verified by sending /model commands in a Feishu group and confirming the model switch is persisted in the thread store and used for subsequent turns.

Greptile Summary

This PR adds a /model <name> command to the Feishu bridge for per-chat model switching, and bundles a second change that stores the incoming messageId so sendText can use the Feishu reply API to keep bot messages inside the same topic thread.

  • /model command: commandAction in lib.mjs routes the new command to setChatModel, which stores or clears model in the per-chat thread-store record; ensureThread and runPrompt both read state?.model || config.model to resolve the effective model per turn.
  • setChat drops the model field: ensureThread (new thread creation) and resumeThread call setChat with a literal object that omits model, silently erasing any override the user set with /model. The override is correctly used during thread creation but is immediately lost, so every subsequent turn falls back to the global default.
  • Reply-in-thread feature: handleIncomingMessage now persists replyToMessageId in the store; sendText resolves this and uses the Feishu reply API when available, falling back to createMessage when the SDK doesn't expose it — but there is no fallback if the reply API call itself throws at runtime.

Confidence Score: 3/5

The core feature of this PR — per-chat model switching — does not reliably persist across turns due to how thread state is written.

The central use case described in the PR (model override taking effect on every subsequent message) breaks in the most common scenario: when /model is issued before the chat's first thread is created, or after a /resume. Both ensureThread and resumeThread call setChat with a hardcoded object that lacks the model field, overwriting what patchChat stored. The override is picked up for thread creation but immediately evicted, so all actual turns run against the global default. The fix is small but the feature is largely non-functional without it.

integrations/feishu-bridge/src/index.mjs — the setChat calls in ensureThread and resumeThread need to carry the existing model field forward.

Important Files Changed

Filename Overview
integrations/feishu-bridge/src/index.mjs Adds /model command handler (setChatModel), per-chat model resolution in ensureThread and runPrompt, and a reply-in-thread feature via replyToMessageId; two defects where setChat in ensureThread and resumeThread overwrites the stored model, and no fallback when the reply API throws.
integrations/feishu-bridge/src/lib.mjs Adds 'model' case to commandAction returning {kind:'set_model', modelName} and exposes parentId/rootId/threadId in incomingIdentity; helpText is not updated to document the new command.

Sequence Diagram

sequenceDiagram
    participant U as User (Feishu)
    participant B as Bridge (index.mjs)
    participant S as ThreadStore
    participant R as Runtime API

    Note over U,R: /model command flow
    U->>B: /model deepseek-v4-pro
    B->>S: "patchChat(chatId, {model})"
    B->>U: Per-chat model set

    Note over U,R: First message after /model (bug path)
    U->>B: Hello
    B->>S: "getChat → {model:deepseek-v4-pro}"
    B->>R: "POST /v1/threads {model:deepseek-v4-pro} ✓"
    R-->>B: "{id: thread-123}"
    B->>S: "setChat({threadId, lastSeq, activeTurnId}) — model lost ⚠️"
    B->>R: "POST turns {model: config.model} — falls back ⚠️"
    R-->>B: turn events
    B->>U: response

    Note over U,R: /model after thread exists (working path)
    U->>B: /model deepseek-v4-pro
    B->>S: patchChat — merges into existing record ✓
    U->>B: Hello again
    B->>S: "getChat → {threadId, model:deepseek-v4-pro}"
    Note right of B: ensureThread returns existing ✓
    B->>R: "POST turns {model:deepseek-v4-pro} ✓"
Loading

Comments Outside Diff (3)

  1. integrations/feishu-bridge/src/index.mjs, line 268-275 (link)

    P1 Per-chat model lost on thread creation

    setChat replaces the entire chat record, so when a new thread is created, the model field that was stored by setChatModel is silently dropped. Back in runPrompt, state?.model is then undefined and the call falls back to config.model, making the override ineffective for every turn after the first thread is initialized. The same scenario occurs when the user sends /new (forced new thread). The model field needs to be carried into the new state object.

    Fix in Codex Fix in Claude Code Fix in Cursor

  2. integrations/feishu-bridge/src/index.mjs, line 501-515 (link)

    P1 /resume silently clears the per-chat model

    resumeThread calls setChat with a literal object that has no model field, overwriting whatever patchChat previously stored. Any user who runs /model deepseek-v4-pro then /resume <id> will lose the override without any warning. The existing model should be read from the store and carried across.

    Fix in Codex Fix in Claude Code Fix in Cursor

  3. integrations/feishu-bridge/src/lib.mjs, line 351-354 (link)

    P2 The /model command is not listed in helpText(), so users who type /help will have no way to discover the new feature.

    Fix in Codex Fix in Claude Code Fix in Cursor

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "feat(feishu): add /model command for per..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

… topics

When running in a Feishu thread-enabled group (话题群), every bot
response — status messages, approval prompts, streaming progress,
turn results — was sent via the Lark SDK's `create` API which spawns
a new standalone topic.  The user sees a cluttered group with orphan
topics for each intermediate bot message.

Root cause: `sendText()` only called `client.im.message.create()`
with a bare `chat_id`, never passing any reply context.  The Feishu
`reply` API was completely unused.

Fix (two changes, one site each):

1. **lib.mjs — incomingIdentity()**: expose `parentId`, `rootId`,
   `threadId` from the raw Feishu message event so callers can
   determine thread context.  (Not consumed directly yet, but
   available for future use.)

2. **index.mjs**:
   - `handleIncomingMessage()`: store the latest incoming
     `messageId` as `replyToMessageId` in the per-chat thread store.
   - `sendText()`: look up `replyToMessageId` from the thread store;
     when present, call `client.im.message.reply()` instead of
     `create()`.  This keeps ALL bot responses nested under the
     original user message inside the same topic.

No config changes needed.  New chats automatically start using the
reply path; existing chats without a `replyToMessageId` in the store
fall back to the old `create` behaviour.

/ 修复飞书话题群中 bot 消息新建独立话题的问题。所有回复改为使用 reply API
/ 在原话题内嵌套回复,而非通过 create API 创建新话题。
The bridge only supported a single global model (DEEPSEEK_MODEL env /
default_text_model in config.toml).  Users who wanted a different model
for a particular Feishu group had to restart the bridge with a different
env var — impractical and disruptive.

This commit adds per-chat model switching so users in a group chat can
run "/model <name>" to switch the model for all future threads and
turns in that chat, without affecting other chats.

Changes:

- **lib.mjs — commandAction()**: handle "model" command → { kind:
  "set_model", modelName }.

- **index.mjs**:
  - setChatModel(chatId, modelName): store/clear per-chat model in the
    thread store.  "/model default" resets to the bridge-level default.
  - ensureThread(): read per-chat model from store when creating a
    new runtime thread; fall back to config.model.
  - runPrompt(): read per-chat model for each turn submission
    (independent of the thread-creation model), so a /model change
    takes effect on the very next message.

Usage:
  /model deepseek-v4-flash      — switch this chat to Flash
  /model deepseek-v4-pro        — switch this chat to Pro
  /model default                — reset to bridge default

Resolution priority (per-chat): global default < per-chat /model.

/ 新增 /model 命令,支持按飞书群设置独立模型。
/ 群内输入 /model <name> 切换,/model default 恢复全局默认。
Copy link
Copy Markdown
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 introduces per-chat model configuration (via a new /model command) and thread-based replies for the Feishu bridge integration, ensuring bot responses stay within the same topic thread. The review feedback highlights three critical issues: a state overwrite bug in ensureThread that wipes out the configured model and reply IDs when a new thread is created, an opportunity to simplify the message ID caching logic using patchChat, and a robustness issue where sendText should fall back to direct messaging if the reply API fails.

Comment on lines +249 to +251
// Use per-chat model if set, fall back to bridge-level default.
// / 优先使用 per-chat 模型(/model 命令设置),否则用桥接级别的默认模型。
const effectiveModel = existing?.model || config.model;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

State Overwrite Bug in ensureThread

While effectiveModel correctly retrieves the per-chat model from existing?.model, the subsequent call to threadStore.setChat(chatId, state) at line 274 (which is outside this diff hunk) completely overwrites the chat state with only threadId, lastSeq, activeTurnId, and updatedAt.

This causes two major issues:

  1. It wipes out the model property from the store as soon as a new thread is created, causing subsequent turns to fall back to the global default model.
  2. It also wipes out replyToMessageId, breaking the threading/reply functionality.

Recommended Fix:
Update ensureThread (line 274) and resumeThread (line 508) to use patchChat instead of setChat to preserve existing properties like model and replyToMessageId:

const state = await threadStore.patchChat(chatId, {
  threadId: thread.id,
  lastSeq: 0,
  activeTurnId: null,
  updatedAt: new Date().toISOString()
});
return state;

Comment on lines +153 to +169
if (identity.messageId) {
const existing = await threadStore.getChat(identity.chatId);
if (existing) {
await threadStore.patchChat(identity.chatId, {
replyToMessageId: identity.messageId,
updatedAt: new Date().toISOString()
});
} else {
await threadStore.setChat(identity.chatId, {
replyToMessageId: identity.messageId,
threadId: null,
lastSeq: 0,
activeTurnId: null,
updatedAt: new Date().toISOString()
});
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

We can simplify the logic for storing the incoming message ID by using patchChat directly. Since patchChat handles merging with the existing state (or initializing an empty object if none exists), we don't need to check if the chat exists or duplicate the updatedAt timestamp and other fields.

  if (identity.messageId) {
    await threadStore.patchChat(identity.chatId, {
      replyToMessageId: identity.messageId,
      updatedAt: new Date().toISOString()
    });
  }

Comment on lines +609 to +619
if (replyMessage) {
await replyMessage({
path: { message_id: replyToMessageId },
data: body
});
} else {
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If replyMessage fails (e.g., due to an expired/deleted message ID or permission changes), the entire sendText call will throw an error and fail to deliver the bot's response. Adding a try-catch block to fall back to createMessage ensures that the message is still delivered to the chat even if threading fails.

    let sent = false;
    if (replyMessage) {
      try {
        await replyMessage({ 
          path: { message_id: replyToMessageId },
          data: body
        });
        sent = true;
      } catch (error) {
        console.error(`Failed to reply to message ${replyToMessageId}, falling back to direct message:`, error);
      }
    }
    if (!sent) {
      await createMessage({
        params: { receive_id_type: "chat_id" },
        data: { ...body, receive_id: chatId }
      });
    }

Copy link
Copy Markdown

@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: 82499e1c20

ℹ️ 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".

// Use per-chat model for this turn (may differ from the thread's
// creation model if the user ran /model after the thread was created).
// / 使用 per-chat 模型执行本轮对话(如果用户在创建线程后切换过模型)。
const effectiveModel = state?.model || config.model;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve per-chat model when computing turn model

runPrompt() derives effectiveModel from state?.model, but when a chat has no thread yet ensureThread() returns a freshly built state object without model and also overwrites stored chat data via setChat(). In that path, /model <name> set just before the first prompt (or before /new) is ignored for the turn submission (model falls back to config.model) and the override is dropped for later turns, so the new feature does not reliably work.

Useful? React with 👍 / 👎.

Comment on lines +610 to +613
await replyMessage({
path: { message_id: replyToMessageId },
data: body
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Fall back to create API when reply send fails

After this change, sendText() always uses message.reply whenever replyToMessageId is present, but any reply-specific API error now aborts delivery because there is no fallback to message.create. This can drop bot responses in chats/messages where reply semantics are unsupported even though normal send is still possible; preserving a fallback keeps behavior compatible with the previous implementation.

Useful? React with 👍 / 👎.

Comment on lines 604 to 621
for (const chunk of splitMessage(text, config.maxReplyChars)) {
await createMessage({
params: { receive_id_type: "chat_id" },
data: {
receive_id: chatId,
msg_type: "text",
content: JSON.stringify({ text: chunk })
}
});
const body = {
msg_type: "text",
content: JSON.stringify({ text: chunk })
};
if (replyMessage) {
await replyMessage({
path: { message_id: replyToMessageId },
data: body
});
} else {
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
});
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 No fallback when replyMessage API call throws

When replyMessage is resolved (i.e., the SDK exposes the reply method and replyToMessageId is set), any API-level error — expired message ID, revoked permission, deleted message — throws and propagates up with no fallback to createMessage. The previous code always used createMessage which can't fail for this reason. A try/catch around the replyMessage call that falls back to createMessage would preserve the original reliability.

Fix in Codex Fix in Claude Code Fix in Cursor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant