[Fix] preserve chat turn boundaries and queue ordering#845
Conversation
Mark ChatLuna user messages explicitly so round splitting and interleaved think processing keep the intended turn boundaries across adapters. Also sort queued messages by timestamp and tighten token fallback handling to keep queued input and token counts stable. Refs: none
Walkthrough将 ChatLuna 用户消息显式标记并在多处使用该标记作为轮次边界；改进模型 token 计数的回退启发式；修复代理地址未定义访问；消息延迟队列改为带时间戳并按时间排序合并；ConversationService 注入 ConversationRuntime 并在运行时标记 HumanMessage。 Changes
Sequence Diagram(s)sequenceDiagram
participant ConvRuntime as ConversationRuntime
participant ConvService as ConversationService
participant ConvRuntimeCreate as Runtime.Create
participant HumanMsg as HumanMessage
participant Marker as markChatLunaUserMessage
participant RoundDetector as buildConversationRounds
ConvService->>ConvRuntimeCreate: 请求创建 HumanMessage
ConvRuntimeCreate->>HumanMsg: 返回 HumanMessage
HumanMsg->>Marker: markChatLunaUserMessage(HumanMessage)
Marker-->>HumanMsg: 写入 response_metadata.chatluna.source='user'
ConvService->>RoundDetector: buildConversationRounds(messages)
RoundDetector->>RoundDetector: 扫描 isChatLunaUserMessage()
alt 存在 ChatLuna 标记
RoundDetector->>RoundDetector: 在标记处开启新轮次
else 不存在
RoundDetector->>RoundDetector: 回退到 message.type==='human'
end
RoundDetector-->>ConvService: 返回轮次列表
sequenceDiagram
participant Middleware as MessageDelay
participant Queue as Queue[{message,timestamp}]
participant Sorter as 排序逻辑
participant Merger as mergeMessages
participant Handler as Turn 处理上下文
Middleware->>Queue: enqueue({message, timestamp})
Middleware->>Queue: enqueue({message, timestamp})
Middleware->>Queue: enqueue({message, timestamp})
Middleware->>Sorter: 按 session.timestamp 排序队列
Sorter-->>Merger: 提取 Messages 顺序列表
Merger->>Handler: mergeMessages(按时间顺序)
Handler-->>Middleware: 继续或停止处理
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟 Possibly related PRs
开发者之诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements to message handling and conversation management. Key changes include the implementation of a metadata-based approach to identify user messages for defining conversation rounds, ensuring chronological message ordering in the message delay middleware using timestamps, and refactoring the ConversationService for better dependency management. Additionally, a character-based token counting heuristic was added as a fallback. Feedback was provided regarding the new model prefix check in the token counting logic, noting that it is overly restrictive and redundant given existing error handling, and should be removed to allow broader model support.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeeb147e95
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/services/conversation_runtime.ts (1)
425-455:⚠️ Potential issue | 🟠 Major
appendPendingMessage传入的HumanMessage缺少markChatLunaUserMessage标记，破坏 plugin 模式下的轮次分割。
createPendingMessage（message_delay.ts）创建的 HumanMessage 未被标记，appendPendingMessage也未在入口处补标。这些未标记消息被推入messageQueue，在legacy-executor中通过queue.drain()提取并作为human-update事件添加到task.messages，最终进入会话历史。关键问题在
buildConversationRounds的轮次检测逻辑：当会话中存在任何被标记的用户消息时（hasUser === true），轮次边界改由isChatLunaUserMessage判定。未标记的 pending 消息无法满足此条件，会被合并到前一轮，违反本 PR 的轮次边界修复目标。建议在
appendPendingMessage入口处补标：修复方案
async appendPendingMessage( conversationId: string, message: HumanMessage, chatMode?: string ): Promise<boolean> { if (chatMode != null && chatMode !== 'plugin') { return false } + markChatLunaUserMessage(message) + const activeRequest = this.activeByConversation.get(conversationId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/services/conversation_runtime.ts` around lines 425 - 455, appendPendingMessage is pushing unmarked HumanMessage instances into activeRequest.messageQueue which later causes buildConversationRounds to mis-detect round boundaries (it relies on isChatLunaUserMessage). Fix by ensuring any pending HumanMessage passed into appendPendingMessage is marked with markChatLunaUserMessage before being queued: inside appendPendingMessage (and anywhere createPendingMessage is used) call markChatLunaUserMessage(message) for messages intended as user messages, so messageQueue entries will satisfy isChatLunaUserMessage during legacy-executor queue.drain processing and preserve correct round splitting in buildConversationRounds.
🧹 Nitpick comments (4)
packages/core/src/llm-core/prompt/chat_history.ts (1)
124-153: LGTM!
hasUser一次性判定全局是否存在被标记的 ChatLuna 用户消息，再据此切换轮次边界判定方式，对老的未标记历史消息也保留了'human'的回退路径，平滑兼容升级前的会话。可选优化：相同的“hasUser → 标记/回退 human”模式同时存在于
packages/core/src/llm-core/platform/model.ts与packages/shared-adapter/src/utils.ts中。三处复用且各自约 5–10 行，未来可以考虑抽出一个共享的轮次起点判定 helper（例如isRoundStart(message, hasUser)）以避免行为漂移；当前提交无需调整。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/prompt/chat_history.ts` around lines 124 - 153, Extract the duplicated "hasUser → check start by isChatLunaUserMessage or fallback to 'human'" logic into a shared helper isRoundStart(message, hasUser) and replace the inline checks inside buildConversationRounds (function buildConversationRounds) as well as the similar blocks in model.ts and utils.ts; ensure isRoundStart accepts a BaseMessage and the boolean hasUser, performs the exact same condition currently used, and is imported/used in each location to keep behavior identical and avoid drift.packages/core/src/middlewares/chat/message_delay.ts (1)
30-33: 按session.timestamp排序合并，符合 PR 目标。对
QueuedMessage增加时间戳并在合并前按时间排序，能修复并发投递下中间件到达顺序与发送顺序不一致的问题。ECMAScript 2019 起Array.prototype.sort已要求稳定排序，因此相同timestamp的消息会保留入队顺序，这正是期望的回退行为。可选优化：
head.messages.sort(...)会原地修改数组，由于紧接着在 Line 299 将head.messages = []覆盖，行为没有问题；如果以后想避免对中间状态的就地修改，可以改成[...head.messages].sort(...)。不影响当前正确性。Also applies to: 113-116, 294-298
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/message_delay.ts` around lines 30 - 33, Add a timestamp to the queued messages and ensure merging sorts by session.timestamp before concatenation: update the QueuedMessage interface to include timestamp, populate timestamp when enqueuing messages, and change the merge logic that currently calls head.messages.sort(...) to sort by m.session.timestamp (or the new timestamp field) so merged batches preserve original send order under concurrency; note Array.prototype.sort is stable so equal timestamps will keep enqueue order (or use [...head.messages].sort(...) if you want to avoid in-place modification).packages/core/src/utils/langchain.ts (1)
9-29: 标记/识别 helper 实现合理。通过
response_metadata.chatluna.source显式标记 ChatLuna 来源的 user 消息，保留原有response_metadata与chatluna字段。给跨适配器（DeepSeek 等）的轮次切分提供了稳定锚点，符合#843的修复方向。♻️ 可选：LangChain 的
BaseMessage.response_metadata默认就是{}，可去掉外层的?? {}略微简化export function markChatLunaUserMessage(msg: BaseMessage) { - msg.response_metadata = { - ...(msg.response_metadata ?? {}), - chatluna: { - ...((msg.response_metadata?.chatluna as ChatLunaMessageMeta) ?? {}), - source: 'user' - } + msg.response_metadata.chatluna = { + ...((msg.response_metadata.chatluna as ChatLunaMessageMeta) ?? {}), + source: 'user' } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/utils/langchain.ts` around lines 9 - 29, The implementation is fine but can be slightly simplified: in markChatLunaUserMessage, remove the unnecessary fallback ...(msg.response_metadata ?? {}) since LangChain's BaseMessage.response_metadata defaults to {} and instead spread msg.response_metadata directly; keep the rest of the logic (setting response_metadata.chatluna.source = 'user') and retain isChatLunaUserMessage as-is (it can continue to cast response_metadata?.chatluna to ChatLunaMessageMeta).packages/core/src/llm-core/platform/model.ts (1)
804-828: 启发式 + 前缀白名单短路 tiktoken，作为#844的根因修复合理。基于 ASCII / 非 ASCII 估算（0.25 vs 2/3）比
text.length / 4对中日韩文本估算更准；前缀白名单避免非 OpenAI 模型误进 tiktoken 加载路径，这其实比仅在 tiktoken loader 内做.length守卫更彻底。一点小提醒：
'o1'、'o3'、'o4'三个前缀过短，理论上会匹配到非 OpenAI 的模型名（例如某些自命名的o1-xxx）。如果担心误匹配可以收紧成'o1-'、'o3-'、'o4-'，并在裸名（如'o1'自身）情况下再单独 includes 判断；不影响正确性，权衡是否要做。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/platform/model.ts` around lines 804 - 828, In getNumTokens, the whitelist prefixes ['o1','o3','o4'] are too short and may false-match non-OpenAI model names; tighten them to 'o1-','o3-','o4-' and also add a separate check to allow the bare names 'o1','o3','o4' (e.g., modelName === 'o1') so you still accept the official short model IDs but avoid accidental prefix matches; update the prefix array used in the .some(...) check and add an OR condition that checks for exact equality against the bare names using the existing modelName variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/services/conversation.ts`:
- Around line 1123-1124: 把 runtime.withConversationSync 的调用格式改为符合 Prettier
的期望：运行 pnpm format 或在 conversation.ts 中调整该调用（在
this.runtime.withConversationSync(...) 处）使换行/缩进符合项目的 Prettier
配置，确保参数列表和括号位置与其它调用一致并通过格式化检查（解决 CodeFactor 报警）。
---
Outside diff comments:
In `@packages/core/src/services/conversation_runtime.ts`:
- Around line 425-455: appendPendingMessage is pushing unmarked HumanMessage
instances into activeRequest.messageQueue which later causes
buildConversationRounds to mis-detect round boundaries (it relies on
isChatLunaUserMessage). Fix by ensuring any pending HumanMessage passed into
appendPendingMessage is marked with markChatLunaUserMessage before being queued:
inside appendPendingMessage (and anywhere createPendingMessage is used) call
markChatLunaUserMessage(message) for messages intended as user messages, so
messageQueue entries will satisfy isChatLunaUserMessage during legacy-executor
queue.drain processing and preserve correct round splitting in
buildConversationRounds.
---
Nitpick comments:
In `@packages/core/src/llm-core/platform/model.ts`:
- Around line 804-828: In getNumTokens, the whitelist prefixes ['o1','o3','o4']
are too short and may false-match non-OpenAI model names; tighten them to
'o1-','o3-','o4-' and also add a separate check to allow the bare names
'o1','o3','o4' (e.g., modelName === 'o1') so you still accept the official short
model IDs but avoid accidental prefix matches; update the prefix array used in
the .some(...) check and add an OR condition that checks for exact equality
against the bare names using the existing modelName variable.
In `@packages/core/src/llm-core/prompt/chat_history.ts`:
- Around line 124-153: Extract the duplicated "hasUser → check start by
isChatLunaUserMessage or fallback to 'human'" logic into a shared helper
isRoundStart(message, hasUser) and replace the inline checks inside
buildConversationRounds (function buildConversationRounds) as well as the
similar blocks in model.ts and utils.ts; ensure isRoundStart accepts a
BaseMessage and the boolean hasUser, performs the exact same condition currently
used, and is imported/used in each location to keep behavior identical and avoid
drift.
In `@packages/core/src/middlewares/chat/message_delay.ts`:
- Around line 30-33: Add a timestamp to the queued messages and ensure merging
sorts by session.timestamp before concatenation: update the QueuedMessage
interface to include timestamp, populate timestamp when enqueuing messages, and
change the merge logic that currently calls head.messages.sort(...) to sort by
m.session.timestamp (or the new timestamp field) so merged batches preserve
original send order under concurrency; note Array.prototype.sort is stable so
equal timestamps will keep enqueue order (or use [...head.messages].sort(...) if
you want to avoid in-place modification).
In `@packages/core/src/utils/langchain.ts`:
- Around line 9-29: The implementation is fine but can be slightly simplified:
in markChatLunaUserMessage, remove the unnecessary fallback
...(msg.response_metadata ?? {}) since LangChain's BaseMessage.response_metadata
defaults to {} and instead spread msg.response_metadata directly; keep the rest
of the logic (setting response_metadata.chatluna.source = 'user') and retain
isChatLunaUserMessage as-is (it can continue to cast response_metadata?.chatluna
to ChatLunaMessageMeta).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 65632e91-178f-4521-a23d-18650b09b010
📒 Files selected for processing (12)
packages/core/src/llm-core/platform/model.tspackages/core/src/llm-core/prompt/chat_history.tspackages/core/src/llm-core/utils/tiktoken.tspackages/core/src/middlewares/chat/message_delay.tspackages/core/src/services/chat.tspackages/core/src/services/conversation.tspackages/core/src/services/conversation_runtime.tspackages/core/src/services/conversation_types.tspackages/core/src/utils/langchain.tspackages/core/tests/conversation-resolve.spec.tspackages/core/tests/helpers.tspackages/shared-adapter/src/utils.ts
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/llm-core/prompt/chat_history.ts (1)
120-151: 轮次边界判定逻辑正确，与代码库其他位置保持一致。
isChatLunaUserMessage(message) || message.getType() === 'human'这一双重判定既能识别新流程中由markChatLunaUserMessage标记的用户消息，又能向后兼容未标记的旧 human 消息，与packages/core/src/llm-core/platform/model.ts中buildConversationRounds的实现一致，符合本 PR 修复#843的目标。JSDoc 也已同步更新。可选优化（不阻塞合并）：本文件的
buildConversationRounds与model.ts中的同名函数逻辑完全相同，未来可考虑提取到utils/langchain.ts等共享位置，避免两处分支判定漂移。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/llm-core/prompt/chat_history.ts` around lines 120 - 151, The buildConversationRounds implementation is correct and should keep the dual predicate isChatLunaUserMessage(message) || message.getType() === 'human' to remain backward-compatible and consistent with the other buildConversationRounds; no behavioral change required. As an optional (non-blocking) refactor, extract the duplicate logic from buildConversationRounds in this file and the one in model.ts into a single shared utility (e.g., utils function) and replace both implementations with calls to that shared function to prevent future drift; update imports and remove the duplicated function after extraction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/llm-core/prompt/chat_history.ts`:
- Around line 120-151: The buildConversationRounds implementation is correct and
should keep the dual predicate isChatLunaUserMessage(message) ||
message.getType() === 'human' to remain backward-compatible and consistent with
the other buildConversationRounds; no behavioral change required. As an optional
(non-blocking) refactor, extract the duplicate logic from
buildConversationRounds in this file and the one in model.ts into a single
shared utility (e.g., utils function) and replace both implementations with
calls to that shared function to prevent future drift; update imports and remove
the duplicated function after extraction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fcb7e9b4-1470-468a-a853-9c563489ff92
📒 Files selected for processing (3)
packages/core/src/llm-core/platform/model.tspackages/core/src/llm-core/prompt/chat_history.tspackages/core/src/services/conversation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/llm-core/platform/model.ts
- packages/core/src/services/conversation.ts
1 participant
This pr fixes conversation turn detection across ChatLuna and adapter flows, keeps delayed message batches merged in send order, and avoids the undefined
globalProxyAddress.lengthaccess during encoder loading.New Features
None.
Bug fixes
globalProxyAddressbefore reading.lengthin the tiktoken encoder loader.Other Changes
ConversationRuntimeintoConversationServicedirectly instead of resolving it back throughctx.chatluna.