Skip to content

feat(pipeline): 预回应表情功能 - 外层包装 + 自动撤回#7009

Open
Sclock wants to merge 15 commits intoAstrBotDevs:masterfrom
Sclock:fix/pre-ack-emoji
Open

feat(pipeline): 预回应表情功能 - 外层包装 + 自动撤回#7009
Sclock wants to merge 15 commits intoAstrBotDevs:masterfrom
Sclock:fix/pre-ack-emoji

Conversation

@Sclock
Copy link
Copy Markdown

@Sclock Sclock commented Mar 26, 2026

Summary

  • 新增 PreAckEmojiManager 类,在 PipelineScheduler.execute() 中作为前后包装运行,完全位于洋葱模型外部
  • 为 Telegram、飞书、Discord 三个平台实现 remove_react() 方法,支持表情撤回
  • 配置新增 auto_remove 字段和 Discord 平台默认值,三个平台可独立配置
  • PreProcessStage 移除旧的表情逻辑,统一由 PreAckEmojiManager 管理
  • 修复洋葱模型 did_yield bug:防止 generator stage yield 后外层循环重复执行后续阶段
  • 使用 EmojiRef dataclass 解耦飞书 reaction_id 的存储,避免 event 上的副作用

Test Plan

  • 13 个表情配置逻辑单元测试(开关、空列表、平台过滤、唤醒守卫、auto_remove、错误处理、Lark reaction_id 流程)
  • 5 个流水线执行次数测试(洋葱后普通阶段、多层嵌套、未挂起生成器)
  • 全部 18 个测试通过

🤖 Generated with Claude Code

Summary by Sourcery

Add a centralized pre-acknowledgement emoji manager around the pipeline scheduler to send and optionally auto-remove typing reactions, and strengthen pipeline execution correctness for onion-style stages.

New Features:

  • Introduce PreAckEmojiManager and EmojiRef to manage pre-acknowledgement emojis before and after pipeline execution outside the onion model.
  • Add platform-native emoji removal support for Lark, Telegram, and Discord events, including optional auto-removal after processing controlled via per-platform config.

Bug Fixes:

  • Fix onion-model pipeline execution so generator stages and their following stages execute exactly once, even with nested or non-yielding generator stages.

Enhancements:

  • Refactor pre-ack emoji handling out of the preprocess stage into a dedicated manager to avoid side effects on events and better encapsulate platform-specific behavior.

Documentation:

  • Extend configuration metadata and i18n descriptions to document per-platform pre-ack emoji auto-remove options.

Tests:

  • Add unit tests covering pre-ack emoji enabling, filtering, auto-removal behavior, error handling, and Lark reaction_id handling across supported platforms.
  • Add pipeline execution tests to verify correct call counts for normal, onion, nested, and non-yielding generator stages.

AstrBot Maintainer and others added 10 commits March 26, 2026 19:56
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…onfig

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sStage emoji code

PreAckEmojiManager now wraps pipeline execution in scheduler.execute(),
completely outside the onion model. Old emoji logic removed from PreProcessStage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- PreAckEmojiManager now returns EmojiRef(emoji, reaction_id) instead of str
- Lark react() returns reaction_id instead of storing on event as side-effect
- remove_react() accepts optional reaction_id parameter across all platforms
- Discord remove_react adds hasattr guard for consistency with react()
- Added 2 new tests for Lark reaction_id flow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@auto-assign auto-assign bot requested review from advent259141 and anka-afk March 26, 2026 12:58
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Mar 26, 2026
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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!

此拉取请求引入了预回应表情功能,允许机器人在接收到命令时自动添加表情,并在处理完成后根据配置自动撤回。核心改动包括引入一个独立的表情管理器,实现了对 Telegram、飞书和 Discord 平台的表情撤回支持,并提供了细粒度的配置选项。此外,还修复了流水线洋葱模型中的一个执行 bug,并优化了飞书 reaction_id 的存储方式,以提升系统的健壮性和可扩展性。

Highlights

  • 新增预回应表情管理器: 引入 PreAckEmojiManager 类,在 PipelineScheduler.execute() 外部运行,负责在流水线执行前后处理表情。
  • 支持表情自动撤回: 为 Telegram、飞书、Discord 三个平台实现了 remove_react() 方法,支持在处理完成后自动撤回预回应表情。
  • 新增配置选项: 在配置中新增 auto_remove 字段,允许为每个平台独立配置是否自动撤回表情,并为 Discord 平台设置了默认值。
  • 重构表情处理逻辑: 将旧的表情逻辑从 PreProcessStage 移除,统一由新的 PreAckEmojiManager 进行管理,提高了模块化和可维护性。
  • 修复洋葱模型 bug: 修复了洋葱模型中 did_yield 的 bug,确保生成器阶段 yield 后,外层循环不会重复执行后续阶段。
  • 解耦飞书 reaction_id: 使用 EmojiRef dataclass 存储飞书的 reaction_id,避免了直接在事件对象上产生副作用,提高了数据管理的清晰度。
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@dosubot dosubot bot added area:core The bug / feature is about astrbot's core, backend area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. labels Mar 26, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • The new behavior where event.react may return a reaction_id (used by PreAckEmojiManager) conflicts with existing -> None type hints and base-class expectations (e.g., LarkEvent.react and AstrMessageEvent.react), so consider updating the method signatures and/or base interface to explicitly allow and document a reaction_id return value (and optional emoji where applicable) to keep the contract consistent across platforms.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new behavior where `event.react` may return a `reaction_id` (used by `PreAckEmojiManager`) conflicts with existing `-> None` type hints and base-class expectations (e.g., `LarkEvent.react` and `AstrMessageEvent.react`), so consider updating the method signatures and/or base interface to explicitly allow and document a `reaction_id` return value (and optional emoji where applicable) to keep the contract consistent across platforms.

## Individual Comments

### Comment 1
<location path="astrbot/core/platform/sources/lark/lark_event.py" line_range="560" />
<code_context>
 async def react(self, emoji: str) -> None:
</code_context>
<issue_to_address>
**issue:** Return type of `react` no longer matches its annotation and base-class contract.

This still returns a `reaction_id` (or `None`) despite the `-> None` annotation and the base `AstrMessageEvent.react` contract documenting fire-and-forget behavior. Either update the annotation and base contract to match the new behavior, or keep `react` as `-> None` and expose a separate method to obtain the `reaction_id`, to avoid confusing readers and breaking type checking.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the pre-acknowledgement emoji functionality into a new PreAckEmojiManager, enabling the addition and conditional removal of emojis for Telegram, Lark, and Discord platforms. This involves updating configuration with an auto_remove option, implementing remove_react methods in platform-specific event classes, and integrating the manager into the PipelineScheduler's execution flow. New unit tests for the emoji manager and pipeline execution have been added. A critical issue was identified in the _process_stages method, where generator stages could cause subsequent pipeline stages to execute multiple times, and a fix has been suggested to prevent this unintended behavior.

active_event_registry.register(event)
emoji = await self.pre_ack_emoji_mgr.add_emoji(event)
try:
await self._process_stages(event)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

PR 描述中提到修复了洋葱模型重复执行后续阶段的 bug,并且添加了对应的单元测试 (test_pipeline_execution.py)。但我在 _process_stages 方法中没有看到对应的修复。

目前的实现中,当一个 generator stage (i) 执行完毕后(async for 循环结束),外层 for 循环会继续执行 i+1,导致 i+1 以及之后的所有阶段被再次执行。这会导致 test_pipeline_execution.py 中的测试失败。

为了修复这个问题,需要在 async for 循环之后判断是否真的发生了 yield。如果发生了,就应该 break 外层循环,因为后续阶段已经在递归调用 _process_stages(event, i + 1) 中处理过了。

建议将 _process_stages 方法修改如下:

    async def _process_stages(self, event: AstrMessageEvent, from_stage=0) -> None:
        """依次执行各个阶段

        Args:
            event (AstrMessageEvent): 事件对象
            from_stage (int): 从第几个阶段开始执行, 默认从0开始

        """
        for i in range(from_stage, len(self.stages)):
            stage = self.stages[i]  # 获取当前要执行的阶段
            coroutine = stage.process(event)

            if isinstance(coroutine, AsyncGenerator):
                did_yield = False
                async for _ in coroutine:
                    did_yield = True
                    # 此处是前置处理完成后的暂停点(yield), 下面开始执行后续阶段
                    if event.is_stopped():
                        logger.debug(
                            f"阶段 {stage.__class__.__name__} 已终止事件传播。",
                        )
                        break

                    # 递归调用, 处理所有后续阶段
                    await self._process_stages(event, i + 1)

                    # 此处是后续所有阶段处理完毕后返回的点, 执行后置处理
                    if event.is_stopped():
                        logger.debug(
                            f"阶段 {stage.__class__.__name__} 已终止事件传播。",
                        )
                        break
                
                if did_yield:
                    break
            else:
                # 如果返回的是普通协程(不含yield的async函数), 则不进入下一层(基线条件)
                # 简单地等待它执行完成, 然后继续执行下一个阶段
                await coroutine

                if event.is_stopped():
                    logger.debug(f"阶段 {stage.__class__.__name__} 已终止事件传播。")
                    break

这个修改引入了 did_yield 标志位,确保只有在 generator stage 真正 yield 并进入内层逻辑后,才会中断外层循环,从而避免了重复执行的问题。对于没有 yield 的 generator stage,其行为保持不变,会继续执行后续 stage.

    async def _process_stages(self, event: AstrMessageEvent, from_stage=0) -> None:
        """依次执行各个阶段

        Args:
            event (AstrMessageEvent): 事件对象
            from_stage (int): 从第几个阶段开始执行, 默认从0开始

        """
        for i in range(from_stage, len(self.stages)):
            stage = self.stages[i]  # 获取当前要执行的阶段
            coroutine = stage.process(event)

            if isinstance(coroutine, AsyncGenerator):
                did_yield = False
                async for _ in coroutine:
                    did_yield = True
                    # 此处是前置处理完成后的暂停点(yield), 下面开始执行后续阶段
                    if event.is_stopped():
                        logger.debug(
                            f"阶段 {stage.__class__.__name__} 已终止事件传播。",
                        )
                        break

                    # 递归调用, 处理所有后续阶段
                    await self._process_stages(event, i + 1)

                    # 此处是后续所有阶段处理完毕后返回的点, 执行后置处理
                    if event.is_stopped():
                        logger.debug(
                            f"阶段 {stage.__class__.__name__} 已终止事件传播。",
                        )
                        break
                
                if did_yield:
                    break
            else:
                # 如果返回的是普通协程(不含yield的async函数), 则不进入下一层(基线条件)
                # 简单地等待它执行完成, 然后继续执行下一个阶段
                await coroutine

                if event.is_stopped():
                    logger.debug(f"阶段 {stage.__class__.__name__} 已终止事件传播。")
                    break

AstrBot Maintainer and others added 5 commits March 26, 2026 21:13
Address Sourcery review: Lark react() returns reaction_id but type hint
was -> None. Updated base class and Lark override to -> str | None.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Root cause: add_emoji runs before _process_stages, so
is_at_or_wake_command has not been set by WakingCheckStage yet.
For Lark and Telegram this was always False, preventing emoji.
Non-wake messages will have their emoji auto-removed in finally
block after WakingCheckStage stops the event.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a generator stage yields, _process_stages recursively handles all
subsequent stages. Without the did_yield break, the outer for-loop
continued iterating and re-executed those stages. The did_yield flag
breaks the outer loop after a yielding generator completes, while
non-yielding generators (return before yield) continue normally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant