[Fix] Keep conversation commands on the active preset lane#825
[Fix] Keep conversation commands on the active preset lane#825dingyi222666 merged 1 commit intov1-devfrom
Conversation
Keep explicit conversation commands aligned with the active preset lane so switch, list, rollback, and stop operate on the same conversation set users see. Resolve numeric conversation targets through visible list order and let management commands opt into archived conversations when needed. Tests: not run (not requested)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
总体概览该PR对对话管理系统进行了重构,核心改进包括:添加预设车道(preset lane)支持到多个命令中;更新中间件的对话解析逻辑以支持预设选项和存档过滤;简化数字目标解析流程,改为基于显示序列而非存储序列;扩展 变更清单
时间序列图sequenceDiagram
participant User as 用户
participant Cmd as 命令层<br/>(conversation.ts)
participant Mgr as 对话管理中间件<br/>(conversation_manage)
participant Resolve as 路由解析中间件<br/>(resolve_conversation)
participant Service as 对话服务<br/>(conversation.ts)
participant Utils as 工具函数<br/>(completeConversationTarget)
User->>Cmd: 执行带预设选项的命令
activate Cmd
Cmd->>Cmd: 提取presetLane选项
Cmd->>Utils: 调用completeConversationTarget(含presetLane)
activate Utils
Utils->>Service: 调用resolveCommandConversation
activate Service
Service->>Service: 基于displaySeq映射数字目标
Service-->>Utils: 返回规范对话ID
deactivate Service
Utils-->>Cmd: 返回完成的对话目标
deactivate Utils
Cmd->>Mgr: 传递presetLane和includeArchived
deactivate Cmd
activate Mgr
Mgr->>Mgr: 根据presetLane调整缓存键<br/>(active vs all)
Mgr->>Resolve: 触发对话解析
deactivate Mgr
activate Resolve
Resolve->>Resolve: 判断useRoutePresetLane<br/>(基于显式选择)
Resolve-->>Cmd: 返回解析结果
deactivate Resolve
代码审查工作量评估🎯 4 (复杂) | ⏱️ ~45 分钟 可能相关的PR
诗篇
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Code Review
This pull request enhances conversation management by adding preset, archived, and all options to various commands and refactoring conversation resolution logic across several middlewares. Key changes include updating completeConversationTarget to resolve numeric display sequences to canonical IDs and ensuring that fallback logic for current conversations only triggers when no explicit target is provided. Feedback was provided regarding the stop_chat middleware to ensure its fallback behavior is consistent with rollback_chat, preventing accidental stops when an invalid target is specified.
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/middlewares/chat/stop_chat.ts (1)
42-66:⚠️ Potential issue | 🟠 Major仅在未显式指定目标时才回退到当前会话。
这里
conversation == null就会回退到当前活动会话;如果用户已经传了conversationId或targetConversation,但解析失败,这个分支会误停掉当前会话,而不是返回“目标不存在”。同一 PR 里的rollback_chat已经用!hasTarget避免了这个问题,这里也应保持一致。💡 建议修改
- if (conversation == null) { + if (conversation == null && !hasTarget) { conversation = ( await ctx.chatluna.conversation.getCurrentConversation( session, { presetLane: context.options.presetLane, useRoutePresetLane: context.options.presetLane == null } ) ).conversation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/chat/stop_chat.ts` around lines 42 - 66, The current fallback to the active conversation runs whenever conversation == null, which wrongly triggers even if the user explicitly provided a target that failed to resolve; introduce a hasTarget boolean (e.g., based on context.options.conversationId or context.options.targetConversation) and change the logic so the getCurrentConversation/resolveCommandConversation fallback only runs when !hasTarget; if hasTarget and the resolved conversation is null, return/propagate a "target not found" error instead of falling back. Target symbols: conversation variable, ctx.chatluna.conversation.getCurrentConversation, ctx.chatluna.conversation.resolveCommandConversation, and context.options.presetLane / context.options.allPresetLanes.
🧹 Nitpick comments (2)
packages/core/src/middlewares/system/conversation_manage.ts (1)
290-298:includeArchived || undefined模式可以简化
includeArchived: includeArchived || undefined将false转换为undefined。根据getTarget方法的默认参数includeArchived = false和resolveTargetConversation中!options.includeArchived的判断逻辑,undefined和false的行为是相同的。可以考虑直接传递
includeArchived布尔值,简化代码:- includeArchived: includeArchived || undefined, + includeArchived,这个模式在多处使用(archive、restore、export、delete、compress),如果简化可以保持一致性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middlewares/system/conversation_manage.ts` around lines 290 - 298, 当前在 conversation_manage 中通过 includeArchived: includeArchived || undefined 将 false 转换为 undefined,冗余且与 getTarget 的默认参数和 resolveTargetConversation 中对 !options.includeArchived 的判断等价;请在 ctx.chatluna.conversation.deleteConversation 调用(以及其它类似处:archive、restore、export、delete、compress 分支)直接传递布尔值 includeArchived 而不是 includeArchived || undefined,并确保相关调用(例如 resolveTargetConversation / getTarget)仍按布尔值处理以保持行为不变。packages/core/src/commands/conversation.ts (1)
92-96:list命令的-P选项与其他命令不一致
chatluna.list使用大写-P作为preset选项的短选项,而其他命令(如switch、current、archive等)都使用小写-p。这是因为-p已被page选项占用。这种不一致可能会让用户困惑。建议在文档或帮助信息中明确说明这一点,或者考虑将
page选项改为其他短选项(如--page仅使用长选项)以保持-p用于preset的一致性。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/commands/conversation.ts` around lines 92 - 96, The preset short flag is inconsistent: in conversation.ts the list command registers .option('page', '-p <page:number>') and .option('preset', '-P <preset:string>') so preset uses -P while other commands (switch/current/archive) use -p; change the flags to be consistent by removing the short '-p' from the page option (use only the long option 'page' / '--page') and reassign the lowercase '-p' to the preset option (i.e., make .option('preset', '-p <preset:string>')) or alternatively document the exception in the help text; update the option definitions in the list command (page and preset) accordingly so they match the short-option convention used by switch/current/archive.
🤖 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/tests/conversation-resolve.spec.ts`:
- Around line 91-100: The mock middleware builder currently returned only an
after() method which can cause runtime errors if code calls .before(); update
the mock returned object in the middleware arrow (where run is set from fn) to
also implement a before() method that returns this, matching the pattern used in
rollback-chat.spec.ts; ensure the signature and chaining behavior mirror after()
so any request_conversation middleware calling .before() will work without
error.
---
Outside diff comments:
In `@packages/core/src/middlewares/chat/stop_chat.ts`:
- Around line 42-66: The current fallback to the active conversation runs
whenever conversation == null, which wrongly triggers even if the user
explicitly provided a target that failed to resolve; introduce a hasTarget
boolean (e.g., based on context.options.conversationId or
context.options.targetConversation) and change the logic so the
getCurrentConversation/resolveCommandConversation fallback only runs when
!hasTarget; if hasTarget and the resolved conversation is null, return/propagate
a "target not found" error instead of falling back. Target symbols: conversation
variable, ctx.chatluna.conversation.getCurrentConversation,
ctx.chatluna.conversation.resolveCommandConversation, and
context.options.presetLane / context.options.allPresetLanes.
---
Nitpick comments:
In `@packages/core/src/commands/conversation.ts`:
- Around line 92-96: The preset short flag is inconsistent: in conversation.ts
the list command registers .option('page', '-p <page:number>') and
.option('preset', '-P <preset:string>') so preset uses -P while other commands
(switch/current/archive) use -p; change the flags to be consistent by removing
the short '-p' from the page option (use only the long option 'page' / '--page')
and reassign the lowercase '-p' to the preset option (i.e., make
.option('preset', '-p <preset:string>')) or alternatively document the exception
in the help text; update the option definitions in the list command (page and
preset) accordingly so they match the short-option convention used by
switch/current/archive.
In `@packages/core/src/middlewares/system/conversation_manage.ts`:
- Around line 290-298: 当前在 conversation_manage 中通过 includeArchived:
includeArchived || undefined 将 false 转换为 undefined,冗余且与 getTarget 的默认参数和
resolveTargetConversation 中对 !options.includeArchived 的判断等价;请在
ctx.chatluna.conversation.deleteConversation
调用(以及其它类似处:archive、restore、export、delete、compress 分支)直接传递布尔值 includeArchived 而不是
includeArchived || undefined,并确保相关调用(例如 resolveTargetConversation /
getTarget)仍按布尔值处理以保持行为不变。
🪄 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: 8369ff07-8c4f-47ea-a7ce-554c06c0e9f1
⛔ Files ignored due to path filters (2)
packages/core/src/locales/en-US.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.ymlis excluded by!**/*.yml
📒 Files selected for processing (12)
packages/core/src/commands/conversation.tspackages/core/src/middlewares/chat/rollback_chat.tspackages/core/src/middlewares/chat/stop_chat.tspackages/core/src/middlewares/conversation/request_conversation.tspackages/core/src/middlewares/conversation/resolve_conversation.tspackages/core/src/middlewares/system/conversation_manage.tspackages/core/src/services/conversation.tspackages/core/src/utils/conversation.tspackages/core/tests/conversation-resolve.spec.tspackages/core/tests/conversation-service.spec.tspackages/core/tests/conversation-target.spec.tspackages/core/tests/rollback-chat.spec.ts
This pr keeps conversation management commands aligned with the active preset lane, fixes numeric target resolution to follow the visible conversation list, and only includes archived conversations when commands explicitly request them.
New Features
--presetsupport tochatluna.list.--archivedand--alloptions to restore, export, compress, and delete conversation commands.Bug fixes
switch,list,rollback, andstop_chatso they resolve the active conversation from the current preset lane.Other Changes