feat: support persona custom error reply message with fallback#5547
feat: support persona custom error reply message with fallback#5547zouyonghe merged 2 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello, 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 enhances the system's error handling capabilities by allowing users to define custom error messages for each persona. This feature provides a more user-friendly experience by replacing generic system errors with specific, pre-configured messages when an agent's LLM request encounters an issue. The changes span across the core agent logic, database persistence, API, and the WebUI, ensuring a complete and configurable solution. 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
|
There was a problem hiding this comment.
Hey - 我发现了 2 个问题,并给出了一些整体性的反馈:
- 从当前会话 / persona 中解析
persona.custom_error_message的逻辑在astr_main_agent._ensure_persona_and_skills和agent_sub_stages/third_party.process中被重复实现;建议抽取到一个共享的 helper(例如放到persona_manager上或一个小的工具模块中),这样 persona 解析和裁剪的行为可以保持一致,并且在每个 event 上只执行一次。 - 从
event的 extras 中读取和规范化persona_custom_error_message目前在多个地方实现(ToolLoopAgentRunner._get_persona_custom_error_message、astr_agent_run_util中的_get_persona_custom_error_message,以及internal.py中的内联逻辑);将其统一收敛到一个通用的 helper,可以减少重复代码,并更容易在不同错误路径上保持一致的回退行为。 - 用于部分 persona 更新的
NOT_GIVEN哨兵值,现在同时存在于 DB 层(sqlite.update_persona)和PersonaManager.update_persona中;将这个哨兵集中到一个共享模块/类型中会更清晰,避免调用方和实现层不小心混用不同的哨兵对象或语义。
给 AI Agents 的提示词
Please address the comments from this code review:
## Overall Comments
- 从当前会话 / persona 中解析 `persona.custom_error_message` 的逻辑在 `astr_main_agent._ensure_persona_and_skills` 和 `agent_sub_stages/third_party.process` 中被重复实现;建议抽取到一个共享的 helper(例如放到 `persona_manager` 上或一个小的工具模块中),这样 persona 解析和裁剪的行为可以保持一致,并且在每个 event 上只执行一次。
- 从 `event` 的 extras 中读取和规范化 `persona_custom_error_message` 目前在多个地方实现(`ToolLoopAgentRunner._get_persona_custom_error_message`、`astr_agent_run_util` 中的 `_get_persona_custom_error_message`,以及 `internal.py` 中的内联逻辑);将其统一收敛到一个通用的 helper,可以减少重复代码,并更容易在不同错误路径上保持一致的回退行为。
- 用于部分 persona 更新的 `NOT_GIVEN` 哨兵值,现在同时存在于 DB 层(`sqlite.update_persona`)和 `PersonaManager.update_persona` 中;将这个哨兵集中到一个共享模块/类型中会更清晰,避免调用方和实现层不小心混用不同的哨兵对象或语义。
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/runners/tool_loop_agent_runner.py" line_range="81-90" />
<code_context>
class ToolLoopAgentRunner(BaseAgentRunner[TContext]):
+ def _get_persona_custom_error_message(self) -> str | None:
+ """Read persona-level custom error message from event extras when available."""
+ try:
+ event = getattr(self.run_context.context, "event", None)
+ if event is None or not hasattr(event, "get_extra"):
+ return None
+ raw_message = event.get_extra("persona_custom_error_message")
+ if not isinstance(raw_message, str):
+ return None
+ return raw_message.strip() or None
+ except Exception:
+ return None
+
</code_context>
<issue_to_address>
**suggestion:** 考虑将 persona 自定义错误消息的提取逻辑进行集中管理,以避免重复和行为偏移。
用于读取/规范化 `persona_custom_error_message` 的逻辑现在在这里、`astr_agent_run_util._get_persona_custom_error_message`、`third_party.process`、`internal.process` 以及 `_ensure_persona_and_skills` 中都有实现,并且在错误处理和类型检查上存在细微差异。为了保持行为一致(裁剪、`None` 与空字符串的处理、异常处理行为)并便于后续演进,建议抽取一个共享的 helper 或小型模块级工具函数,并在这些调用点统一复用。
建议实现如下:
```python
def _extract_persona_custom_error_message(event) -> str | None:
"""Shared helper to extract a normalized persona custom error message from an event.
Normalization rules:
- If the event is None or does not expose `get_extra`, return None.
- If the extra is not a string, return None.
- Trim whitespace; if the result is empty, return None.
- Swallow all exceptions to avoid breaking caller flows on bad extras.
"""
try:
if event is None or not hasattr(event, "get_extra"):
return None
raw_message = event.get_extra("persona_custom_error_message")
if not isinstance(raw_message, str):
return None
message = raw_message.strip()
return message or None
except Exception:
return None
class ToolLoopAgentRunner(BaseAgentRunner[TContext]):
```
```python
def _get_persona_custom_error_message(self) -> str | None:
"""Read persona-level custom error message from event extras when available."""
event = getattr(self.run_context.context, "event", None)
return _extract_persona_custom_error_message(event)
```
为了在整个项目中真正实现“单一共享 helper”的目标并避免行为偏移:
1. 将 `_extract_persona_custom_error_message` 移动到一个共享模块(例如 `astrbot/core/agent/persona_utils.py` 或类似位置),避免与该 runner 强绑定。
2. 用该共享 helper 替换以下位置中现有的自定义错误消息解析逻辑:
- `astr_agent_run_util._get_persona_custom_error_message`
- `third_party.process`
- `internal.process`
- `_ensure_persona_and_skills`
确保它们在裁剪、`None` 与空字符串处理,以及异常处理语义上保持一致。
3. 调整这些模块中的 import,改为引用共享 helper,而不是在本地重新实现这段逻辑。
</issue_to_address>
### Comment 2
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/third_party.py" line_range="118" />
<code_context>
provider_settings=cfg,
)
+ custom_error_message = None
+ if persona:
+ raw_custom_error_message = persona.get("custom_error_message")
</code_context>
<issue_to_address>
**issue (complexity):** 建议将 persona 相关的自定义错误消息解析提取到一个单独的异步 helper 方法中,以让 `process` 函数更专注于编排逻辑。
你可以保持当前的新行为不变,但通过将 persona / 错误消息解析抽取到一个小的 helper 中来降低复杂度。这样可以让 `process` 专注于流程编排,同时将分支逻辑、await 调用和错误处理封装起来。
例如,在 `ThirdPartyAgentSubStage` 内:
```python
class ThirdPartyAgentSubStage(Stage):
...
async def _resolve_persona_custom_error_message(
self, event: MessageEvent
) -> str | None:
try:
conversation_persona_id = None
conv_mgr = self.ctx.plugin_manager.context.conversation_manager
curr_cid = await conv_mgr.get_curr_conversation_id(event.unified_msg_origin)
if curr_cid:
conv = await conv_mgr.get_conversation(event.unified_msg_origin, curr_cid)
if conv:
conversation_persona_id = conv.persona_id
(
_,
persona,
_,
_,
) = await self.ctx.plugin_manager.context.persona_manager.resolve_selected_persona(
umo=event.unified_msg_origin,
conversation_persona_id=conversation_persona_id,
platform_name=event.get_platform_name(),
provider_settings=self.conf["provider_settings"],
)
if not persona:
return None
raw_msg = persona.get("custom_error_message")
if isinstance(raw_msg, str):
return raw_msg.strip() or None
return None
except Exception as e:
logger.debug("Failed to resolve persona custom error message: %s", e)
return None
```
然后可以简化 `process`(或主要处理方法):
```python
# before your hook call
custom_error_message = await self._resolve_persona_custom_error_message(event)
event.set_extra("persona_custom_error_message", custom_error_message)
# later when creating the stream
event.set_result(
MessageEventResult()
.set_result_content_type(ResultContentType.STREAMING_RESULT)
.set_async_stream(
run_third_party_agent(
runner,
stream_to_general=False,
custom_error_message=custom_error_message,
),
),
)
```
这样既保留了全部功能,也保留了仅用于调试的日志输出,同时把紧耦合的解析逻辑从主控制流中抽离出来。另外,它也为你提供了一个集中位置,可以在需要时与任何 dashboard 端的规范化逻辑进行复用/对齐。
</issue_to_address>请帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The logic for resolving
persona.custom_error_messagefrom the current conversation/persona is duplicated inastr_main_agent._ensure_persona_and_skillsandagent_sub_stages/third_party.process; consider extracting a shared helper (e.g. onpersona_manageror a small utility) so persona resolution and trimming behavior stay consistent and only run once per event. - Reading and normalizing the
persona_custom_error_messagefromeventextras is implemented in multiple places (ToolLoopAgentRunner._get_persona_custom_error_message,_get_persona_custom_error_messageinastr_agent_run_util, and inline ininternal.py); consolidating this into a single shared helper would reduce duplication and make it easier to keep the fallback behavior uniform across error paths. - The use of a
NOT_GIVENsentinel for partial persona updates now exists in both the DB layer (sqlite.update_persona) andPersonaManager.update_persona; it might be clearer to centralize this sentinel in a shared module/type so that callers and implementations cannot accidentally mix different sentinel objects or semantics.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for resolving `persona.custom_error_message` from the current conversation/persona is duplicated in `astr_main_agent._ensure_persona_and_skills` and `agent_sub_stages/third_party.process`; consider extracting a shared helper (e.g. on `persona_manager` or a small utility) so persona resolution and trimming behavior stay consistent and only run once per event.
- Reading and normalizing the `persona_custom_error_message` from `event` extras is implemented in multiple places (`ToolLoopAgentRunner._get_persona_custom_error_message`, `_get_persona_custom_error_message` in `astr_agent_run_util`, and inline in `internal.py`); consolidating this into a single shared helper would reduce duplication and make it easier to keep the fallback behavior uniform across error paths.
- The use of a `NOT_GIVEN` sentinel for partial persona updates now exists in both the DB layer (`sqlite.update_persona`) and `PersonaManager.update_persona`; it might be clearer to centralize this sentinel in a shared module/type so that callers and implementations cannot accidentally mix different sentinel objects or semantics.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/runners/tool_loop_agent_runner.py" line_range="81-90" />
<code_context>
class ToolLoopAgentRunner(BaseAgentRunner[TContext]):
+ def _get_persona_custom_error_message(self) -> str | None:
+ """Read persona-level custom error message from event extras when available."""
+ try:
+ event = getattr(self.run_context.context, "event", None)
+ if event is None or not hasattr(event, "get_extra"):
+ return None
+ raw_message = event.get_extra("persona_custom_error_message")
+ if not isinstance(raw_message, str):
+ return None
+ return raw_message.strip() or None
+ except Exception:
+ return None
+
</code_context>
<issue_to_address>
**suggestion:** Consider centralizing persona custom error message extraction to avoid duplication and drift.
This logic for reading/normalizing `persona_custom_error_message` is now duplicated here, in `astr_agent_run_util._get_persona_custom_error_message`, `third_party.process`, `internal.process`, and `_ensure_persona_and_skills`, with subtle differences in error handling and type checks. To keep behavior consistent (trimming, `None` vs empty string, exception behavior) and easier to evolve, please extract a single shared helper or small module-level utility and reuse it across these call sites.
Suggested implementation:
```python
def _extract_persona_custom_error_message(event) -> str | None:
"""Shared helper to extract a normalized persona custom error message from an event.
Normalization rules:
- If the event is None or does not expose `get_extra`, return None.
- If the extra is not a string, return None.
- Trim whitespace; if the result is empty, return None.
- Swallow all exceptions to avoid breaking caller flows on bad extras.
"""
try:
if event is None or not hasattr(event, "get_extra"):
return None
raw_message = event.get_extra("persona_custom_error_message")
if not isinstance(raw_message, str):
return None
message = raw_message.strip()
return message or None
except Exception:
return None
class ToolLoopAgentRunner(BaseAgentRunner[TContext]):
```
```python
def _get_persona_custom_error_message(self) -> str | None:
"""Read persona-level custom error message from event extras when available."""
event = getattr(self.run_context.context, "event", None)
return _extract_persona_custom_error_message(event)
```
To fully realize the “single shared helper” goal project-wide and avoid behavior drift:
1. Move `_extract_persona_custom_error_message` into a shared module (e.g. `astrbot/core/agent/persona_utils.py` or similar) so it’s not tied to this runner.
2. Replace the existing custom-error-message parsing logic in:
- `astr_agent_run_util._get_persona_custom_error_message`
- `third_party.process`
- `internal.process`
- `_ensure_persona_and_skills`
with calls to this shared helper, ensuring they all follow the same trimming, `None` vs empty-string, and exception-handling semantics.
3. Adjust imports in those modules to pull in the shared helper instead of re-implementing the logic locally.
</issue_to_address>
### Comment 2
<location path="astrbot/core/pipeline/process_stage/method/agent_sub_stages/third_party.py" line_range="118" />
<code_context>
provider_settings=cfg,
)
+ custom_error_message = None
+ if persona:
+ raw_custom_error_message = persona.get("custom_error_message")
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the persona-specific custom error message resolution into a dedicated async helper method to keep the `process` function focused on orchestration logic.
You can keep the new behavior but reduce complexity by extracting the persona/error-message resolution into a small helper. That keeps `process` focused on orchestration while encapsulating the branching, awaits, and error handling.
For example, inside `ThirdPartyAgentSubStage`:
```python
class ThirdPartyAgentSubStage(Stage):
...
async def _resolve_persona_custom_error_message(
self, event: MessageEvent
) -> str | None:
try:
conversation_persona_id = None
conv_mgr = self.ctx.plugin_manager.context.conversation_manager
curr_cid = await conv_mgr.get_curr_conversation_id(event.unified_msg_origin)
if curr_cid:
conv = await conv_mgr.get_conversation(event.unified_msg_origin, curr_cid)
if conv:
conversation_persona_id = conv.persona_id
(
_,
persona,
_,
_,
) = await self.ctx.plugin_manager.context.persona_manager.resolve_selected_persona(
umo=event.unified_msg_origin,
conversation_persona_id=conversation_persona_id,
platform_name=event.get_platform_name(),
provider_settings=self.conf["provider_settings"],
)
if not persona:
return None
raw_msg = persona.get("custom_error_message")
if isinstance(raw_msg, str):
return raw_msg.strip() or None
return None
except Exception as e:
logger.debug("Failed to resolve persona custom error message: %s", e)
return None
```
Then `process` (or the main handler method) can be simplified:
```python
# before your hook call
custom_error_message = await self._resolve_persona_custom_error_message(event)
event.set_extra("persona_custom_error_message", custom_error_message)
# later when creating the stream
event.set_result(
MessageEventResult()
.set_result_content_type(ResultContentType.STREAMING_RESULT)
.set_async_stream(
run_third_party_agent(
runner,
stream_to_general=False,
custom_error_message=custom_error_message,
),
),
)
```
This keeps all functionality, preserves the debug-only logging, but removes the tightly-coupled resolution block from the main control flow. It also gives you a single place to reuse/align with any dashboard-side normalization logic if needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
astrbot/core/pipeline/process_stage/method/agent_sub_stages/third_party.py
Outdated
Show resolved
Hide resolved
| custom_error_message = None | ||
| try: | ||
| conversation_persona_id = None | ||
| conv_mgr = self.ctx.plugin_manager.context.conversation_manager | ||
| curr_cid = await conv_mgr.get_curr_conversation_id(event.unified_msg_origin) | ||
| if curr_cid: | ||
| conv = await conv_mgr.get_conversation( | ||
| event.unified_msg_origin, curr_cid | ||
| ) | ||
| if conv: | ||
| conversation_persona_id = conv.persona_id | ||
| ( | ||
| _, | ||
| persona, | ||
| _, | ||
| _, | ||
| ) = await self.ctx.plugin_manager.context.persona_manager.resolve_selected_persona( | ||
| umo=event.unified_msg_origin, | ||
| conversation_persona_id=conversation_persona_id, | ||
| platform_name=event.get_platform_name(), | ||
| provider_settings=self.conf["provider_settings"], | ||
| ) | ||
| if persona: | ||
| raw_custom_error_message = persona.get("custom_error_message") | ||
| if isinstance(raw_custom_error_message, str): | ||
| custom_error_message = raw_custom_error_message.strip() or None | ||
| except Exception as e: | ||
| logger.debug("Failed to resolve persona custom error message: %s", e) | ||
| event.set_extra("persona_custom_error_message", custom_error_message) |
astrbot/core/astr_agent_run_util.py
Outdated
| def _get_persona_custom_error_message(astr_event) -> str | None: | ||
| raw_message = astr_event.get_extra("persona_custom_error_message") | ||
| if not isinstance(raw_message, str): | ||
| return None | ||
| return raw_message.strip() or None | ||
|
|
There was a problem hiding this comment.
这个函数的健壮性不足。如果 astr_event 没有 get_extra 方法,或者 get_extra 抛出异常,它可能会失败。在 astrbot/core/agent/runners/tool_loop_agent_runner.py 中有一个更健壮的类似实现。这里也存在代码重复。建议通过添加 try...except 块来使此函数更安全。理想情况下,此逻辑应位于单个共享的工具函数中以避免重复,但这可能需要进一步重构以避免循环依赖。
def _get_persona_custom_error_message(astr_event) -> str | None:
try:
raw_message = astr_event.get_extra("persona_custom_error_message")
if not isinstance(raw_message, str):
return None
return raw_message.strip() or None
except Exception:
return None
astrbot/core/astr_agent_run_util.py
Outdated
| custom_error_message = _get_persona_custom_error_message(astr_event) | ||
| if custom_error_message: | ||
| err_msg = custom_error_message | ||
| else: | ||
| err_msg = ( | ||
| f"\n\nAstrBot 请求失败。\n错误类型: {type(e).__name__}\n错误信息: " | ||
| f"{e!s}\n\n请在平台日志查看和分享错误详情。\n" | ||
| ) |
| custom_error_message = event.get_extra("persona_custom_error_message") | ||
| if isinstance(custom_error_message, str): | ||
| custom_error_message = custom_error_message.strip() or None | ||
| else: | ||
| custom_error_message = None |
There was a problem hiding this comment.
此处清理 custom_error_message 的逻辑是多余的。存储在 event.get_extra("persona_custom_error_message") 中的值在设置之前已经被清理(剥离空格并验证为字符串或 None)。您可以通过在使用前仅确保类型正确来简化此部分。
| custom_error_message = event.get_extra("persona_custom_error_message") | |
| if isinstance(custom_error_message, str): | |
| custom_error_message = custom_error_message.strip() or None | |
| else: | |
| custom_error_message = None | |
| custom_error_message = event.get_extra("persona_custom_error_message") | |
| if not isinstance(custom_error_message, str): | |
| custom_error_message = None |
| err_msg = custom_error_message | ||
| if not err_msg: | ||
| err_msg = ( | ||
| f"\nAstrBot 请求失败。\n错误类型: {type(e).__name__}\n" | ||
| f"错误信息: {e!s}\n\n请在平台日志查看和分享错误详情。\n" | ||
| ) |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我已经审查了你的更改,看起来非常不错!
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的审查。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
Validation
由 Sourcery 提供的摘要
添加“按人格配置”的可定制错误回退消息,并贯通后端、持久化层和仪表盘 UI,使其在可用时在代理的各类错误路径中生效。
新功能:
增强内容:
Original summary in English
Summary by Sourcery
添加可在 persona 级别配置的错误回退消息,这些消息会被持久化存储,通过 API 暴露,并在各类智能体错误路径中复用。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Add persona-level configurable error fallback messages that are persisted, exposed via APIs, and used across agent error paths.
New Features:
Enhancements: