feat: 添加失败时保存本轮记录的配置项

[Blueteemo]

Summary by Sourcery

Add a configurable option to persist agent run history even when the run fails or returns empty output.

New Features:

• Introduce the save_failed_agent_history configuration flag for agent runners to control whether failed runs are saved to conversation history.

Enhancements:

• Extend history-saving logic to optionally append a synthetic failure message so failed agent runs can still be inspected for debugging.
• Expose the new save_failed_agent_history option in default config and provider templates, including UI metadata and conditions.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The new await self._save_to_history(...) calls appear to have lost one level of indentation compared to the surrounding if blocks; please re-indent them so they are clearly scoped inside the corresponding conditionals.
• In _save_to_history, consider working on a copy of all_messages before appending the synthetic failure Message so you don't mutate the original message list that may still be used elsewhere.
• The new logger.info in the failure path may be too noisy for normal operation; consider using debug or adding more context (e.g., conversation or request id) to make the log more actionable.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `await self._save_to_history(...)` calls appear to have lost one level of indentation compared to the surrounding `if` blocks; please re-indent them so they are clearly scoped inside the corresponding conditionals.
- In `_save_to_history`, consider working on a copy of `all_messages` before appending the synthetic failure `Message` so you don't mutate the original message list that may still be used elsewhere.
- The new `logger.info` in the failure path may be too noisy for normal operation; consider using `debug` or adding more context (e.g., conversation or request id) to make the log more actionable.

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="297-303" />
<code_context>
                         if agent_runner.done() and (
                             not event.is_stopped() or agent_runner.was_aborted()
                         ):
-                            await self._save_to_history(
-                                event,
-                                req,
-                                agent_runner.get_final_llm_resp(),
-                                agent_runner.run_context.messages,
-                                agent_runner.stats,
-                                user_aborted=agent_runner.was_aborted(),
-                            )
+await self._save_to_history(
</code_context>
<issue_to_address>
**issue (bug_risk):** The `await self._save_to_history` call has lost its indentation, which will break syntax and control flow.

This call needs to remain indented inside the `if agent_runner.done()` block. At column 0 it’s no longer part of the `if` body and will raise a syntax error.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request introduces a new configuration option, save_failed_agent_history, which allows the system to save conversation history even when an agent run fails (e.g., empty LLM response). This is primarily intended for debugging purposes when using the local agent runner. The changes include updates to the default configuration, schema definitions, and the internal history-saving logic to handle failed runs by appending a placeholder message. I have no feedback to provide.

[dosubot]

Related Documentation

1 document(s) may need updating based on files changed in this PR:

AstrBotTeam's Space

pr4697的改动

View Suggested Changes

@@ -403,6 +403,25 @@
 - 消息格式为两行输出，第一行显示工具名称，第二行显示截断的返回结果
 
 工具分配可通过 UI 或配置文件完成。相关配置项说明已在配置文件注释中明确。
+  
+  **配置项：save_failed_agent_history**
+  
+  该配置项用于在 Agent 运行失败时保存对话历史，便于调试和问题排查：
+  
+  - **类型**：布尔值（Boolean）
+  - **默认值**：`false`
+  - **描述**：失败时保存本轮记录
+  - **前置条件**：仅在 `agent_runner_type` 设置为 `"local"`（使用内置 Agent Runner）时生效
+  - **提示**：启用后，当 Agent 本轮运行失败时（如模型返回空输出），也会将本轮记录保存到会话历史中，包括用户输入、工具调用记录和失败提示。
+  - **行为**：
+    - 当 LLM 响应为空且无工具调用结果时，通常不会保存对话历史
+    - 启用该选项后，即使响应为空，系统仍会保存对话历史
+    - 自动附加失败提示消息："[Agent run failed. History saved for debugging.]"
+    - 历史记录包含用户输入、工具调用记录和失败提示
+  - **使用场景**：
+    - 调试 Agent 失败场景，保留失败上下文便于排查问题
+    - 分析模型返回空输出的原因
+    - 追踪长时间运行的工具调用失败情况
 
 #### 在插件中注册 SubAgent（PR #5765）
 

[Accept] [Decline]

^{How did I do? Any feedback?}  

[Blueteemo]

@sourcery-ai review

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The hardcoded placeholder assistant message ("[Agent run failed. History saved for debugging.]") would be easier to maintain and localize if extracted into a constant and/or built from available failure context (e.g., error details or stats) instead of a fixed English string inlined here.
• Consider whether failed runs that have non-empty tool_calls_result but no completion_text should also get a placeholder message or some other marker, to distinguish silent model failures from intentional tool-only responses when save_failed_agent_history is enabled.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The hardcoded placeholder assistant message (`"[Agent run failed. History saved for debugging.]"`) would be easier to maintain and localize if extracted into a constant and/or built from available failure context (e.g., error details or stats) instead of a fixed English string inlined here.
- Consider whether failed runs that have non-empty `tool_calls_result` but no `completion_text` should also get a placeholder message or some other marker, to distinguish silent model failures from intentional tool-only responses when `save_failed_agent_history` is enabled.

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="454-458" />
<code_context>
             logger.debug("LLM 响应为空，不保存记录。")
             return

+        if save_failed_history and not llm_response.completion_text:
+            logger.debug(
+                "Saving failed agent history as save_failed_agent_history is enabled."
+            )
+            messages_to_save = all_messages + [
+                Message(
+                    role="assistant",
</code_context>
<issue_to_address>
**issue (bug_risk):** Failure marker is added even when there are tool results or when the user aborted, which changes existing behavior.

This change alters prior behavior: cases with empty `llm_response.completion_text` but with tool call results or a user abort used to be saved without a failure marker. Now, any `save_failed_history=True` + `not llm_response.completion_text` adds the failure message, even when tools succeeded or the user intentionally aborted. This may incorrectly mark successful tool-only runs or user-aborted runs as failures. Please narrow the condition (e.g., also check `not req.tool_calls_result` and `not user_aborted`) or otherwise distinguish model failures from tool-only responses and user-initiated aborts.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Blueteemo]

@sourcery-ai review

[SourceryAI]

Hey - I've found 1 issue, and left some high level feedback:

• The condition to detect an empty/failed run (not llm_response.completion_text and not req.tool_calls_result and not user_aborted) is duplicated and slightly inverted in two branches; consider extracting this into a clearly named boolean (e.g. is_failed_run) to avoid divergence and improve readability.
• When save_failed_agent_history is enabled, the synthetic assistant message is hard-coded and English-only; you may want to either localize it to match the rest of the UI or include more structured failure context (e.g. error type or reason) so it’s more useful for debugging.
• Currently the synthetic failure message is added after all messages and then the first system message is skipped in the loop; if downstream consumers rely on the last message being a real model response, consider documenting or adjusting this behavior to avoid surprising them (e.g. by marking the synthetic message with a specific role/metadata).

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The condition to detect an empty/failed run (`not llm_response.completion_text and not req.tool_calls_result and not user_aborted`) is duplicated and slightly inverted in two branches; consider extracting this into a clearly named boolean (e.g. `is_failed_run`) to avoid divergence and improve readability.
- When `save_failed_agent_history` is enabled, the synthetic assistant message is hard-coded and English-only; you may want to either localize it to match the rest of the UI or include more structured failure context (e.g. error type or reason) so it’s more useful for debugging.
- Currently the synthetic failure message is added after all messages and then the first system message is skipped in the loop; if downstream consumers rely on the last message being a real model response, consider documenting or adjusting this behavior to avoid surprising them (e.g. by marking the synthetic message with a specific role/metadata).

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="454" />
<code_context>
             logger.debug("LLM 响应为空，不保存记录。")
             return

+        if (
+            save_failed_history
+            and not llm_response.completion_text
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing the failed-run condition into a single predicate and reworking the save_failed_history branching to avoid duplicated logic and inverted conditions.

You can reduce complexity by centralizing the “failed run” predicate and simplifying the branching around `save_failed_history`. Right now you repeat the same multi-part condition twice with different `save_failed_history` polarity and thread that into two separate flows (`return` vs. `messages_to_save`), which makes future changes harder.

A localized refactor inside `_save_to_history` keeps behavior but removes duplication and makes intent clearer:

```python
async def _save_to_history(
    self,
    event: AstrMessageEvent,
    req: ProviderRequest,
    llm_response: LLMResponse | None,
    all_messages: list[Message],
    runner_stats: AgentStats | None,
    user_aborted: bool = False,
    save_failed_history: bool = False,
) -> None:
    if not req or not req.conversation:
        return

    if not llm_response and not user_aborted:
        return

    if llm_response and llm_response.role != "assistant":
        if not user_aborted:
            return
        llm_response = LLMResponse(
            role="assistant",
            completion_text=llm_response.completion_text or "",
        )
    elif llm_response is None:
        llm_response = LLMResponse(role="assistant", completion_text="")

    # Centralize "failed run" definition
    is_failed = (
        not llm_response.completion_text
        and not req.tool_calls_result
        and not user_aborted
    )

    # Same early-return behavior when we *don’t* save failed history
    if is_failed and not save_failed_history:
        logger.debug("LLM 响应为空，不保存记录。")
        return

    messages_to_save = all_messages
    if is_failed and save_failed_history:
        logger.debug(
            "Saving failed agent history as save_failed_agent_history is enabled."
        )
        messages_to_save = messages_to_save + [
            Message(
                role="assistant",
                content="[Agent run failed. History saved for debugging.]",
            )
        ]

    message_to_save: list[dict] = []
    skipped_initial_system = False
    for message in messages_to_save:
        if message.role == "system" and not skipped_initial_system:
            skipped_initial_system = True
            continue
        if message.role in ["assistant", "user"] and message._no_save:
            continue
        message_to_save.append(message.model_dump())

    ...
```

This keeps:

- The original early-return semantics when `save_failed_history` is `False`.
- The new behavior of appending a failure marker when `save_failed_history` is `True`.

But it removes:

- The duplicated 3-part predicate.
- The “inverted” condition that’s currently split across the early-return and `messages_to_save` assignment.
</issue_to_address>

---

Hi @Blueteemo! 👋

Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀

Install the sourcery-ai bot to get automatic code reviews on every pull request ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The _save_to_history logic repeats the same long condition twice (for the early return and for constructing messages_to_save); consider extracting this into a named boolean (e.g. is_empty_failed_run) to reduce duplication and make the behavior easier to reason about.
• The synthetic failure Message content is currently a hardcoded English string in an otherwise Chinese-labeled feature; consider either localizing it or allowing the failure message to be configured so that saved histories are consistent with the rest of the system.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `_save_to_history` logic repeats the same long condition twice (for the early return and for constructing `messages_to_save`); consider extracting this into a named boolean (e.g. `is_empty_failed_run`) to reduce duplication and make the behavior easier to reason about.
- The synthetic failure `Message` content is currently a hardcoded English string in an otherwise Chinese-labeled feature; consider either localizing it or allowing the failure message to be configured so that saved histories are consistent with the rest of the system.

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="446-449" />
<code_context>
             not llm_response.completion_text
             and not req.tool_calls_result
             and not user_aborted
+            and not save_failed_history
         ):
             logger.debug("LLM 响应为空，不保存记录。")
</code_context>
<issue_to_address>
**suggestion:** Consider restructuring the empty-response conditions to avoid duplicating the same predicate twice.

The predicate `not llm_response.completion_text and not req.tool_calls_result and not user_aborted` is now split between the guard (with `and not save_failed_history`) and the subsequent `if save_failed_history` block. This duplication will be fragile if the definition of a "failed" run changes. Consider computing a `failed_run` boolean once and reusing it, or inverting the branches so `save_failed_history` is the main path and the guard is the fallback, to keep the logic DRY and easier to maintain.
</issue_to_address>

### Comment 2
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/internal.py" line_range="463-468" />
<code_context>
+            logger.debug(
+                "Saving failed agent history as save_failed_agent_history is enabled."
+            )
+            messages_to_save = all_messages + [
+                Message(
+                    role="assistant",
+                    content="[Agent run failed. History saved for debugging.]",
+                )
+            ]
+        else:
</code_context>
<issue_to_address>
**suggestion:** Using a plain-text sentinel message for failed runs may be brittle for downstream consumers.

The synthetic `assistant` message is helpful for debugging but indistinguishable from real model output. If downstream code treats all assistant messages as model-generated, this could cause confusion or require ad-hoc filtering. Please either add explicit metadata/flags so this message can be reliably identified, or use a dedicated role/marker consistent with existing conventions so it can be handled and localized cleanly.

```suggestion
            messages_to_save = all_messages + [
                Message(
                    role="system",
                    content="[Agent run failed. History saved for debugging.]",
                )
            ]
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Blueteemo]

Sourcery 的建议都是代码质量/重构类型，不影响功能实现。当前实现已满足 issue #7620（失败时保存本轮记录）的需求。

建议的代码重构（如提取失败条件为变量）虽然更优雅，但会增加 review 复杂度，当前实现已足够清晰可维护。

功能已正确实现，暂不进一步重构，等待开发者审核合并。

[Blueteemo]

@sourcery-ai review

[sourcery-ai]

Hey - I've left some high level feedback:

• The synthetic assistant message [Agent run failed. History saved for debugging.] is hard-coded and English-only in an otherwise Chinese-configured system; consider making this text configurable or localized, or marking the failure via structured metadata instead of a user-visible message.
• When save_failed_agent_history is enabled, runs aborted by the user are still excluded by the and not user_aborted condition; double-check whether aborted runs should be treated as a kind of failure to be saved under this flag or if a separate flag/branch is more appropriate.
• The logger.info call for each failed-agent history save may be noisy in high-traffic environments; consider downgrading this to debug or adding more context (e.g., conversation or run identifiers) to make the log more actionable.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The synthetic assistant message `[Agent run failed. History saved for debugging.]` is hard-coded and English-only in an otherwise Chinese-configured system; consider making this text configurable or localized, or marking the failure via structured metadata instead of a user-visible message.
- When `save_failed_agent_history` is enabled, runs aborted by the user are still excluded by the `and not user_aborted` condition; double-check whether aborted runs should be treated as a kind of failure to be saved under this flag or if a separate flag/branch is more appropriate.
- The `logger.info` call for each failed-agent history save may be noisy in high-traffic environments; consider downgrading this to `debug` or adding more context (e.g., conversation or run identifiers) to make the log more actionable.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}