Conversation
…ndling of stop requests
dosubot
bot
added
the
size:L
Summary of ChangesHello @Soulter, 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 significantly enhances user control over active agent sessions by introducing a robust stop mechanism. Users can now interrupt ongoing agent responses via a new command or a dedicated UI button, with the backend ensuring graceful termination and preservation of any partial outputs. Additionally, it resolves a critical issue with file segment persistence in the chat interface by standardizing attachment storage, improving the reliability of multimedia content in conversations. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
dosubot
bot
added
the
feature:chatui
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -151,12 +151,14 @@
`/stop` 命令是内置对话命令的一部分，与 `/reset`、`/his` 等命令一样，可在任何对话平台（Telegram、QQ、微信、Discord 等）中使用。该命令的详细说明已正式收录于官方用户文档（zh/use/command.md 和 en/use/command.md）。
##### WebUI 停止按钮
-在 Dashboard 聊天界面，当 Agent 正在执行任务或模型正在生成回复时，输入框右侧的发送按钮会变为停止按钮（红色方块图标）：
-
-- 停止按钮在 Agent 运行时自动出现，替代发送按钮
+在 Dashboard 聊天界面，当 Agent 正在执行任务或模型正在生成回复时，输入框右侧的发送按钮会变为停止按钮：
+
+- 停止按钮使用 `mdi-stop` 图标，在 `isRunning` 状态（流式生成或对话运行中）时自动出现，替代发送按钮
+- 鼠标悬停时显示提示文本："停止生成"（中文）/ "Stop generating"（英文）
- 点击按钮可中断当前的生成过程
- 中断后，已生成的部分内容会被保留在对话历史中
-- 停止后会通过 API 调用请求中止当前会话的 Agent 任务
+- 停止后会通过 `/api/chat/stop` API 端点请求中止当前会话的 Agent 任务
+- 前端通过状态管理确保错误处理被抑制，避免用户主动停止时显示误报错误
该功能的详细说明已正式收录于官方用户文档（zh/use/webui.md 和 en/use/webui.md 的"聊天"章节）。
@@ -167,27 +169,46 @@
##### 内置 Agent Runner（工具循环 Agent Runner）
对于内置 Agent Runner，停止请求是平滑的（graceful），会保留中断前的部分输出：
-- 使用 `request_agent_stop_all()` 方法，不中断事件传播，允许后续流程（如历史记录保存）继续执行
+- 使用 `request_agent_stop_all()` 方法（`active_event_registry`），不中断事件传播，允许后续流程（如历史记录保存）继续执行
+- 该方法设置 `agent_stop_requested` 标志，返回被请求停止的任务数量
- 系统消息提示："[SYSTEM: User actively interrupted the response generation. Partial output before interruption is preserved.]"
-- Agent 转换为 DONE 状态，并触发 on_agent_done 钩子
-- 对话历史和会话状态得以保留
-- 响应类型标记为 "aborted"
-- Agent 在执行过程中会定期检查 `_stop_requested` 标志，可平滑退出
+- Agent Runner 通过后台监控任务（`_watch_agent_stop_signal`）周期性检查停止信号
+- 检测到停止信号后，调用 `request_stop()` 方法设置内部 `_stop_requested` 标志位
+- Agent 在流式生成或工具调用过程中检查 `_stop_requested` 标志，并在适当时机平滑退出
+- Agent 转换为 DONE 状态，并触发 `on_agent_done` 钩子
+- 对话历史和会话状态得以保留，响应类型标记为 "aborted"
+- `was_aborted()` 方法返回 `true`，表明任务被用户主动中止
##### 第三方 Agent Runner（如 Dify、Coze）
对于第三方 Agent Runner，停止请求会完全中断事件传播：
-- 使用 `stop_all()` 方法，完全停止事件流
+- 使用 `stop_all()` 方法（`active_event_registry`），完全停止事件流
- 不保留中间状态，直接终止任务执行
#### 技术实现要点
-- `request_agent_stop_all()` 方法（active_event_registry）：用于平滑停止，设置停止标志但不中断事件传播
-- `stop_all()` 方法（active_event_registry）：用于硬停止，完全中断事件传播
-- `request_stop()` 方法（工具循环 Agent Runner）：设置 `_stop_requested` 标志位
-- `was_aborted()` 方法（工具循环 Agent Runner）：检查任务是否被用户主动中止
-- Agent 执行过程中通过后台监控任务（`_watch_agent_stop_signal`）检查停止信号
-- Dashboard 通过 `/api/chat/stop` API 端点触发停止请求
+##### 后端实现
+- `request_agent_stop_all()` 方法（`active_event_registry`）：用于平滑停止，设置 `agent_stop_requested` 标志但不中断事件传播，返回被请求停止的任务数量
+- `stop_all()` 方法（`active_event_registry`）：用于硬停止，完全中断事件传播
+- `request_stop()` 方法（`tool_loop_agent_runner`）：设置内部 `_stop_requested` 和 `_aborted` 标志位
+- `was_aborted()` 方法（`tool_loop_agent_runner`）：检查任务是否被用户主动中止
+- Agent 执行过程中通过后台监控任务（`_watch_agent_stop_signal`）周期性检查停止信号
+- `/api/chat/stop` POST 端点（`chat.py`）：接收前端停止请求，验证会话所有权后调用 `request_agent_stop_all()`
+
+##### 历史记录持久化改进
+- `_save_to_history()` 方法新增 `user_aborted` 参数，支持在用户中止时仍保存对话历史
+- 历史保存条件从 `not event.is_stopped()` 改为 `not event.is_stopped() or agent_runner.was_aborted()`
+- 确保即使会话被中止，已生成的部分内容也能正确保存到历史记录中
+- 空响应在用户中止时会被妥善处理，不会导致历史记录丢失
+
+##### 前端实现
+- 状态管理（`useMessages.ts`）：新增 `currentRequestController`、`currentReader`、`currentRunningSessionId` 和 `userStopRequested` 状态
+- `stopMessage()` 函数：调用 `/api/chat/stop` API 端点，取消 SSE reader，中止 fetch 请求
+- 错误处理优化：检查 `userStopRequested` 标志，在用户主动停止时抑制错误提示
+- 停止按钮（`ChatInput.vue`）：根据 `isRunning` 状态动态切换发送/停止按钮，正确清理流式状态和活跃 SSE 连接
+
+##### 附件持久化修复
+- `_create_attachment_from_file()` 方法检查多个候选路径（`attachments_dir` 和 `legacy_img_dir`），确保文件段能够正确持久化和检索
---
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 在
useMessages.stopMessage中，userStopRequested在检查有效的sessionId之前就被设置为true，而当sessionId为 falsy 时你会提前返回，但没有重置该值；建议将这个赋值移动到sessionId检查之后（或在提前返回时重置它），以避免在后续发送中抑制错误日志。 - 在
ChatRoute._create_attachment_from_file中，你现在会同时探测self.attachments_dir和self.legacy_img_dir；请确保在所有运行时场景下，该类上的legacy_img_dir属性总是已定义，或者在使用前加一层保护（例如通过getattr），以避免在没有 legacy 目录时运行导致属性错误。 - 在
run_agent中与 stop-watcher 相关的取消逻辑在多个分支（正常完成、abort 和异常路径）中被重复实现；建议抽取一个小的帮助函数来安全地取消并等待stop_watcher，以减少重复代码和清理逻辑不一致的风险。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- In `useMessages.stopMessage`, `userStopRequested` is set to `true` before you check for a valid `sessionId`, and you early-return without resetting it when `sessionId` is falsy; consider moving that assignment after the `sessionId` check (or resetting it on early return) to avoid suppressing error logging for subsequent sends.
- In `ChatRoute._create_attachment_from_file`, you now probe both `self.attachments_dir` and `self.legacy_img_dir`; please ensure `legacy_img_dir` is always defined on this class in all runtime contexts, or guard its use (e.g., via `getattr`) to avoid attribute errors when running without legacy directories.
- The stop-watcher cancellation logic in `run_agent` is duplicated in multiple branches (normal completion, aborted, and exception paths); consider extracting a small helper to cancel and await `stop_watcher` safely to reduce repetition and the risk of inconsistent cleanup.
## Individual Comments
### Comment 1
<location> `tests/test_tool_loop_agent_runner.py:447-451` </location>
<code_context>
+ assert any(resp.type == "aborted" for resp in rest_responses)
+ assert runner.was_aborted() is True
+
+ final_resp = runner.get_final_llm_resp()
+ assert final_resp is not None
+ assert final_resp.role == "assistant"
+ assert final_resp.completion_text == "partial "
+ assert runner.run_context.messages[-1].role == "assistant"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that hooks are invoked correctly on abort to ensure the new on_agent_done behavior is covered.
This test doesn't verify that `agent_hooks.on_agent_done` is invoked on abort. To cover the new behavior, please:
- Assert that `mock_hooks.on_agent_done` (or your stubbed method) is called exactly once.
- Optionally, assert it is called with the `run_context` and `final_resp` from `runner.get_final_llm_resp()`.
That ensures the abort path exercises hooks and guards against regressions where they’re skipped.
Suggested implementation:
```python
rest_responses = []
async for response in step_iter:
rest_responses.append(response)
assert any(resp.type == "aborted" for resp in rest_responses)
assert runner.was_aborted() is True
# Ensure final response and messages are set correctly on abort
final_resp = runner.get_final_llm_resp()
assert final_resp is not None
assert final_resp.role == "assistant"
assert final_resp.completion_text == "partial "
assert runner.run_context.messages[-1].role == "assistant"
# Ensure hooks are invoked correctly on abort
mock_hooks.on_agent_done.assert_called_once_with(runner.run_context, final_resp)
```
These edits assume:
1. A `mock_hooks` fixture or mock object is already created in this test and passed into the runner so that `on_agent_done` is a `Mock` with `assert_called_once_with` available. If your hooks object is named differently (e.g., `agent_hooks` or `hooks`), update the assertion line accordingly.
2. `on_agent_done`'s signature is `(run_context, final_resp)`. If your implementation uses a different signature (e.g., keyword arguments or additional parameters), adjust the `assert_called_once_with` arguments to match.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进后续的审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
useMessages.stopMessage,userStopRequestedis set totruebefore you check for a validsessionId, and you early-return without resetting it whensessionIdis falsy; consider moving that assignment after thesessionIdcheck (or resetting it on early return) to avoid suppressing error logging for subsequent sends. - In
ChatRoute._create_attachment_from_file, you now probe bothself.attachments_dirandself.legacy_img_dir; please ensurelegacy_img_diris always defined on this class in all runtime contexts, or guard its use (e.g., viagetattr) to avoid attribute errors when running without legacy directories. - The stop-watcher cancellation logic in
run_agentis duplicated in multiple branches (normal completion, aborted, and exception paths); consider extracting a small helper to cancel and awaitstop_watchersafely to reduce repetition and the risk of inconsistent cleanup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `useMessages.stopMessage`, `userStopRequested` is set to `true` before you check for a valid `sessionId`, and you early-return without resetting it when `sessionId` is falsy; consider moving that assignment after the `sessionId` check (or resetting it on early return) to avoid suppressing error logging for subsequent sends.
- In `ChatRoute._create_attachment_from_file`, you now probe both `self.attachments_dir` and `self.legacy_img_dir`; please ensure `legacy_img_dir` is always defined on this class in all runtime contexts, or guard its use (e.g., via `getattr`) to avoid attribute errors when running without legacy directories.
- The stop-watcher cancellation logic in `run_agent` is duplicated in multiple branches (normal completion, aborted, and exception paths); consider extracting a small helper to cancel and await `stop_watcher` safely to reduce repetition and the risk of inconsistent cleanup.
## Individual Comments
### Comment 1
<location> `tests/test_tool_loop_agent_runner.py:447-451` </location>
<code_context>
+ assert any(resp.type == "aborted" for resp in rest_responses)
+ assert runner.was_aborted() is True
+
+ final_resp = runner.get_final_llm_resp()
+ assert final_resp is not None
+ assert final_resp.role == "assistant"
+ assert final_resp.completion_text == "partial "
+ assert runner.run_context.messages[-1].role == "assistant"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that hooks are invoked correctly on abort to ensure the new on_agent_done behavior is covered.
This test doesn't verify that `agent_hooks.on_agent_done` is invoked on abort. To cover the new behavior, please:
- Assert that `mock_hooks.on_agent_done` (or your stubbed method) is called exactly once.
- Optionally, assert it is called with the `run_context` and `final_resp` from `runner.get_final_llm_resp()`.
That ensures the abort path exercises hooks and guards against regressions where they’re skipped.
Suggested implementation:
```python
rest_responses = []
async for response in step_iter:
rest_responses.append(response)
assert any(resp.type == "aborted" for resp in rest_responses)
assert runner.was_aborted() is True
# Ensure final response and messages are set correctly on abort
final_resp = runner.get_final_llm_resp()
assert final_resp is not None
assert final_resp.role == "assistant"
assert final_resp.completion_text == "partial "
assert runner.run_context.messages[-1].role == "assistant"
# Ensure hooks are invoked correctly on abort
mock_hooks.on_agent_done.assert_called_once_with(runner.run_context, final_resp)
```
These edits assume:
1. A `mock_hooks` fixture or mock object is already created in this test and passed into the runner so that `on_agent_done` is a `Mock` with `assert_called_once_with` available. If your hooks object is named differently (e.g., `agent_hooks` or `hooks`), update the assertion line accordingly.
2. `on_agent_done`'s signature is `(run_context, final_resp)`. If your implementation uses a different signature (e.g., keyword arguments or additional parameters), adjust the `assert_called_once_with` arguments to match.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| final_resp = runner.get_final_llm_resp() | ||
| assert final_resp is not None | ||
| assert final_resp.role == "assistant" | ||
| assert final_resp.completion_text == "partial " | ||
| assert runner.run_context.messages[-1].role == "assistant" |
There was a problem hiding this comment.
suggestion (testing): 断言在 abort 时 hooks 会被正确调用，以确保覆盖新的 on_agent_done 行为。
当前这个测试没有验证在 abort 时 agent_hooks.on_agent_done 是否被调用。为了覆盖新的行为，请：
- 断言
mock_hooks.on_agent_done（或你 stub 的方法）恰好被调用一次。 - （可选）断言它是使用来自
runner.get_final_llm_resp()的run_context和final_resp作为参数被调用的。
这样可以确保 abort 路径会执行 hooks，并防止它们被跳过而造成回归。
建议实现：
rest_responses = []
async for response in step_iter:
rest_responses.append(response)
assert any(resp.type == "aborted" for resp in rest_responses)
assert runner.was_aborted() is True
# Ensure final response and messages are set correctly on abort
final_resp = runner.get_final_llm_resp()
assert final_resp is not None
assert final_resp.role == "assistant"
assert final_resp.completion_text == "partial "
assert runner.run_context.messages[-1].role == "assistant"
# Ensure hooks are invoked correctly on abort
mock_hooks.on_agent_done.assert_called_once_with(runner.run_context, final_resp)这些修改基于以下假设：
- 测试中已经创建了一个
mock_hooksfixture 或 mock 对象，并将其传入 runner，从而on_agent_done是一个带有assert_called_once_with方法的Mock。如果你的 hooks 对象名字不同（例如agent_hooks或hooks），请相应地更新断言那一行。 on_agent_done的函数签名为(run_context, final_resp)。如果你的实现使用了不同的签名（例如关键字参数或额外的参数），请相应调整assert_called_once_with的参数以保持一致。
Original comment in English
suggestion (testing): Assert that hooks are invoked correctly on abort to ensure the new on_agent_done behavior is covered.
This test doesn't verify that agent_hooks.on_agent_done is invoked on abort. To cover the new behavior, please:
- Assert that
mock_hooks.on_agent_done(or your stubbed method) is called exactly once. - Optionally, assert it is called with the
run_contextandfinal_respfromrunner.get_final_llm_resp().
That ensures the abort path exercises hooks and guards against regressions where they’re skipped.
Suggested implementation:
rest_responses = []
async for response in step_iter:
rest_responses.append(response)
assert any(resp.type == "aborted" for resp in rest_responses)
assert runner.was_aborted() is True
# Ensure final response and messages are set correctly on abort
final_resp = runner.get_final_llm_resp()
assert final_resp is not None
assert final_resp.role == "assistant"
assert final_resp.completion_text == "partial "
assert runner.run_context.messages[-1].role == "assistant"
# Ensure hooks are invoked correctly on abort
mock_hooks.on_agent_done.assert_called_once_with(runner.run_context, final_resp)These edits assume:
- A
mock_hooksfixture or mock object is already created in this test and passed into the runner so thaton_agent_doneis aMockwithassert_called_once_withavailable. If your hooks object is named differently (e.g.,agent_hooksorhooks), update the assertion line accordingly. on_agent_done's signature is(run_context, final_resp). If your implementation uses a different signature (e.g., keyword arguments or additional parameters), adjust theassert_called_once_witharguments to match.
![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 a new feature allowing users to stop ongoing agent conversations. The changes involve adding a /stop command to the bot's built-in commands, which utilizes a new request_agent_stop_all mechanism in the active_event_registry to signal agents to halt execution without fully terminating the event. The ToolLoopAgentRunner was updated to handle these stop requests gracefully, preserving partial outputs and marking the run as aborted. The conversation history saving logic was adjusted to correctly record these aborted sessions. On the frontend, a 'Stop generating' button was added to the chat input, integrating with a new /chat/stop API endpoint to trigger the agent termination. Additionally, the attachment storage directory was generalized from imgs_dir to attachments_dir, and a minor indentation bug in tool call result handling within the dashboard's chat route was fixed. Review comments highlighted areas for improving code clarity, reducing duplication (especially for task cancellation logic), using constants for system messages, and adding type hints for better maintainability.
| accumulated_parts.append( | ||
| { | ||
| "type": "tool_call", | ||
| "tool_calls": [tool_calls[tc_id]], | ||
| } | ||
| ) | ||
| tool_calls.pop(tc_id, None) |
There was a problem hiding this comment.
The indentation for the accumulated_parts.append and tool_calls.pop lines within the if tc_id in tool_calls: block is incorrect. These lines should be at the same indentation level as tool_calls[tc_id]["finished_ts"] = tcr.get("ts") to be part of the if block. As it stands, they are outside the if block, which might lead to unexpected behavior if tc_id is not in tool_calls.
| accumulated_parts.append( | |
| { | |
| "type": "tool_call", | |
| "tool_calls": [tool_calls[tc_id]], | |
| } | |
| ) | |
| tool_calls.pop(tc_id, None) | |
| if tc_id in tool_calls: | |
| tool_calls[tc_id]["result"] = tcr.get("result") | |
| tool_calls[tc_id]["finished_ts"] = tcr.get("ts") | |
| accumulated_parts.append( | |
| { | |
| "type": "tool_call", | |
| "tool_calls": [tool_calls[tc_id]], | |
| } | |
| ) | |
| tool_calls.pop(tc_id, None) |
| if agent_runner.done() and ( | ||
| not event.is_stopped() or agent_runner.was_aborted() |
There was a problem hiding this comment.
The condition not event.is_stopped() or agent_runner.was_aborted() seems a bit redundant. If agent_runner.was_aborted() is true, it implies the agent stopped, so event.is_stopped() might also be true or irrelevant. It would be clearer to state the conditions explicitly, perhaps if agent_runner.done() and (agent_runner.was_aborted() or not event.is_stopped()) to emphasize that saving happens if it's done and either aborted or not stopped by other means.
# 保存历史记录
if agent_runner.done() and (agent_runner.was_aborted() or not event.is_stopped()):| if not llm_response and not user_aborted: | ||
| return |
There was a problem hiding this comment.
The condition not llm_response and not user_aborted might be simplified. If user_aborted is true, llm_response could still be None if the interruption happened very early. The logic seems to correctly handle this, but the phrasing could be more direct, e.g., if not llm_response and not user_aborted: return implies that if user_aborted is true, we proceed even if llm_response is None, which is then handled by setting llm_response = LLMResponse(...).
| llm_resp = LLMResponse( | ||
| role="assistant", | ||
| completion_text="[SYSTEM: User actively interrupted the response generation. Partial output before interruption is preserved.]", | ||
| ) |
There was a problem hiding this comment.
Similar to the previous comment, the completion_text for an interrupted response is hardcoded here. It should be a constant to avoid duplication and improve maintainability.
| llm_resp = LLMResponse( | |
| role="assistant", | |
| completion_text="[SYSTEM: User actively interrupted the response generation. Partial output before interruption is preserved.]", | |
| ) | |
| llm_resp = llm_resp_result | |
| if llm_resp.role != "assistant": | |
| # Use the same constant for consistency | |
| INTERRUPTED_MESSAGE = "[SYSTEM: User actively interrupted the response generation. Partial output before interruption is preserved.]" | |
| llm_resp = LLMResponse( | |
| role="assistant", | |
| completion_text=INTERRUPTED_MESSAGE, | |
| ) |
| def _should_stop_agent(astr_event) -> bool: | ||
| return astr_event.is_stopped() or bool(astr_event.get_extra("agent_stop_requested")) |
There was a problem hiding this comment.
The function _should_stop_agent checks for astr_event.is_stopped() and astr_event.get_extra("agent_stop_requested"). While this works, it might be more robust to have a single, clear method on AstrMessageEvent that encapsulates all conditions for stopping, making the intent clearer and centralizing the logic.
| def _should_stop_agent(astr_event) -> bool: | |
| return astr_event.is_stopped() or bool(astr_event.get_extra("agent_stop_requested")) | |
| def _should_stop_agent(astr_event) -> bool: | |
| # Consider adding a dedicated method to AstrMessageEvent for clearer intent | |
| return astr_event.is_stopped() or astr_event.is_agent_stop_requested() |
| def __init__(self, message_str, message_obj, platform_meta, session_id) -> None: | ||
| super().__init__(message_str, message_obj, platform_meta, session_id) | ||
| os.makedirs(imgs_dir, exist_ok=True) | ||
| os.makedirs(attachments_dir, exist_ok=True) |
There was a problem hiding this comment.
The os.makedirs call is made in the __init__ method of WebChatMessageEvent. This means that every time a WebChatMessageEvent is instantiated, it attempts to create this directory. While exist_ok=True prevents errors, it's an unnecessary operation that could be moved to a more appropriate place, such as application startup or a singleton initialization, to be called only once.
| self._stop_requested = False | ||
| self._aborted = False |
There was a problem hiding this comment.
These new attributes _stop_requested and _aborted are introduced without being explicitly typed. While Python's dynamic typing allows this, adding type hints would improve readability and maintainability, especially for a class with a complex lifecycle like ToolLoopAgentRunner.
| self._stop_requested = False | |
| self._aborted = False | |
| self.run_context = run_context | |
| self._stop_requested: bool = False | |
| self._aborted: bool = False |
| const currentRequestController = ref<AbortController | null>(null); | ||
| const currentReader = ref<ReadableStreamDefaultReader<Uint8Array> | null>(null); | ||
| const currentRunningSessionId = ref(''); | ||
| const userStopRequested = ref(false); |
There was a problem hiding this comment.
These new ref variables (currentRequestController, currentReader, currentRunningSessionId, userStopRequested) are added to manage the state of ongoing requests and user-initiated stops. While functional, it might be beneficial to group related state variables into a single reactive object or a dedicated class for better organization and encapsulation, especially as the component grows.
| } catch (err) { | ||
| // ignore reader cancel failures | ||
| } |
There was a problem hiding this comment.
The try...catch block around currentReader.value?.cancel() with an empty catch block (// ignore reader cancel failures) can hide important errors. While the intention is to ignore expected CancelledErrors, it might inadvertently suppress other unexpected exceptions during cancellation. It's generally better to log or handle specific exceptions, or at least log the ignored error at a debug level.
try {
await currentReader.value?.cancel();
} catch (err) {
console.debug('Reader cancellation failed (possibly expected):', err);
}| class MockAbortableStreamProvider(MockProvider): | ||
| async def text_chat_stream(self, **kwargs): | ||
| abort_signal = kwargs.get("abort_signal") | ||
| yield LLMResponse( | ||
| role="assistant", | ||
| completion_text="partial ", | ||
| is_chunk=True, | ||
| ) | ||
| if abort_signal and abort_signal.is_set(): | ||
| yield LLMResponse( | ||
| role="assistant", | ||
| completion_text="partial ", | ||
| is_chunk=False, | ||
| ) | ||
| return | ||
| yield LLMResponse( | ||
| role="assistant", | ||
| completion_text="partial final", | ||
| is_chunk=False, | ||
| ) |
There was a problem hiding this comment.
The MockAbortableStreamProvider is a good addition for testing the stop functionality. However, the abort_signal parameter is expected to be a specific type (e.g., asyncio.Event). Adding a type hint for abort_signal in the method signature would improve clarity and help catch potential type-related issues during development.
| class MockAbortableStreamProvider(MockProvider): | |
| async def text_chat_stream(self, **kwargs): | |
| abort_signal = kwargs.get("abort_signal") | |
| yield LLMResponse( | |
| role="assistant", | |
| completion_text="partial ", | |
| is_chunk=True, | |
| ) | |
| if abort_signal and abort_signal.is_set(): | |
| yield LLMResponse( | |
| role="assistant", | |
| completion_text="partial ", | |
| is_chunk=False, | |
| ) | |
| return | |
| yield LLMResponse( | |
| role="assistant", | |
| completion_text="partial final", | |
| is_chunk=False, | |
| ) | |
| class MockAbortableStreamProvider(MockProvider): | |
| async def text_chat_stream(self, abort_signal: asyncio.Event | None = None, **kwargs): |
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
为运行中的 agent 会话添加用户主动停止支持，同时保留部分输出，并修复 Web 聊天中的文件附件持久化问题。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Add user-initiated stop support for running agent sessions while preserving partial outputs and fix file attachment persistence in web chat.
New Features:
Bug Fixes:
Enhancements:
Tests: