feat(feishu): add /model command for per-chat model switching#2149
feat(feishu): add /model command for per-chat model switching#2149yuanchenglu wants to merge 2 commits into
Conversation
… 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 恢复全局默认。
There was a problem hiding this comment.
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.
| // Use per-chat model if set, fall back to bridge-level default. | ||
| // / 优先使用 per-chat 模型(/model 命令设置),否则用桥接级别的默认模型。 | ||
| const effectiveModel = existing?.model || config.model; |
There was a problem hiding this comment.
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:
- It wipes out the
modelproperty from the store as soon as a new thread is created, causing subsequent turns to fall back to the global default model. - 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;| 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() | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
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()
});
}| if (replyMessage) { | ||
| await replyMessage({ | ||
| path: { message_id: replyToMessageId }, | ||
| data: body | ||
| }); | ||
| } else { | ||
| await createMessage({ | ||
| params: { receive_id_type: "chat_id" }, | ||
| data: { ...body, receive_id: chatId } | ||
| }); | ||
| } |
There was a problem hiding this comment.
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 }
});
}There was a problem hiding this comment.
💡 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; |
There was a problem hiding this comment.
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 👍 / 👎.
| await replyMessage({ | ||
| path: { message_id: replyToMessageId }, | ||
| data: body | ||
| }); |
There was a problem hiding this comment.
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 👍 / 👎.
| 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 } | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
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_MODELenvironment variable ordefault_text_modelinconfig.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.mjs—commandAction()Added a
"model"case that parses/model <name>into{ kind: "set_model", modelName }.index.mjs—setChatModel()New function that stores or clears the per-chat model in the thread store:
/model <name>→ stores the model name inthreadStore.data.chats[chatId].model/model defaultor/modelwith no args → clears the stored model, falling back to the bridge-level defaultindex.mjs—ensureThread()andrunPrompt()Both now check for a per-chat model before using
config.model:ensureThread(): when creating a new runtime thread, readsexisting?.model || config.modelrunPrompt(): readsstate?.model || config.modelfor each turn submission, so a/modelchange takes effect on the very next message (independent of the thread-creation model)Usage
Resolution priority
Testing
Manually verified by sending
/modelcommands 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 incomingmessageIdsosendTextcan use the Feishu reply API to keep bot messages inside the same topic thread./modelcommand:commandActioninlib.mjsroutes the new command tosetChatModel, which stores or clearsmodelin the per-chat thread-store record;ensureThreadandrunPromptboth readstate?.model || config.modelto resolve the effective model per turn.setChatdrops the model field:ensureThread(new thread creation) andresumeThreadcallsetChatwith a literal object that omitsmodel, 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.handleIncomingMessagenow persistsreplyToMessageIdin the store;sendTextresolves this and uses the Feishu reply API when available, falling back tocreateMessagewhen 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
/modelis issued before the chat's first thread is created, or after a/resume. BothensureThreadandresumeThreadcallsetChatwith a hardcoded object that lacks themodelfield, overwriting whatpatchChatstored. 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
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} ✓"Comments Outside Diff (3)
integrations/feishu-bridge/src/index.mjs, line 268-275 (link)setChatreplaces the entire chat record, so when a new thread is created, themodelfield that was stored bysetChatModelis silently dropped. Back inrunPrompt,state?.modelis thenundefinedand the call falls back toconfig.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). Themodelfield needs to be carried into the newstateobject.integrations/feishu-bridge/src/index.mjs, line 501-515 (link)/resumesilently clears the per-chat modelresumeThreadcallssetChatwith a literal object that has nomodelfield, overwriting whateverpatchChatpreviously stored. Any user who runs/model deepseek-v4-prothen/resume <id>will lose the override without any warning. The existing model should be read from the store and carried across.integrations/feishu-bridge/src/lib.mjs, line 351-354 (link)/modelcommand is not listed inhelpText(), so users who type/helpwill have no way to discover the new feature.Reviews (1): Last reviewed commit: "feat(feishu): add /model command for per..." | Re-trigger Greptile