Skip to content

fix(wecomai_event): 修复消息链提取纯文本时的空白处理#8563

Merged
RC-CHN merged 3 commits into
AstrBotDevs:masterfrom
NayukiChiba:fix/8474-wecomai-stream-newline
Jun 4, 2026
Merged

fix(wecomai_event): 修复消息链提取纯文本时的空白处理#8563
RC-CHN merged 3 commits into
AstrBotDevs:masterfrom
NayukiChiba:fix/8474-wecomai-stream-newline

Conversation

@NayukiChiba
Copy link
Copy Markdown
Contributor

@NayukiChiba NayukiChiba commented Jun 3, 2026

Modifications / 改动点

  • 增加了strip_result参数以控制是否需要去除首尾空白
  • 如果是非流式输出,保持为默认为True,表示保留换行,进行strip,丢掉\n
  • 如果是流式,表示不需要strip,跳过strip,保留\n
  • 流式输出时保留换行等格式字符
  • 更新相关调用以适应新参数

close #8474

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

test session starts 
platform win32 -- Python 3.12.12, pytest-9.0.3, pluggy-1.6.0
rootdir: D:\Nayey\Code\NayukiChiba\Astrbot
configfile: pyproject.toml
plugins: anyio-4.13.0, asyncio-1.4.0, cov-7.1.0
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 24 items                                                                                        

tests\unit\test_wecomai_event.py ........................                                           [100%]
24 passed in 2.13s


Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

- 增加了strip_result参数以控制是否去除首尾空白
- 流式输出时保留换行等格式字符
- 更新相关调用以适应新参数
- 测试 _extract_plain_text_from_chain 方法在流式和非流式场景下的行为
- 确保流式输出时换行符等格式字符能够正确保留
- 覆盖了不同输入场景的测试用例
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. labels Jun 3, 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:

  • Consider making strip_result a keyword-only argument in _extract_plain_text_from_chain (e.g., def _extract_plain_text_from_chain(message_chain: MessageChain | None, *, strip_result: bool = True)) to avoid accidental positional misuse and make its semantics clearer at call sites.
  • The new tests around _extract_plain_text_from_chain are quite granular and repetitive; you could reduce duplication and improve readability by parametrizing similar cases (e.g., whitespace/newline/tab behaviors) with pytest.mark.parametrize instead of separate test functions.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider making `strip_result` a keyword-only argument in `_extract_plain_text_from_chain` (e.g., `def _extract_plain_text_from_chain(message_chain: MessageChain | None, *, strip_result: bool = True)`) to avoid accidental positional misuse and make its semantics clearer at call sites.
- The new tests around `_extract_plain_text_from_chain` are quite granular and repetitive; you could reduce duplication and improve readability by parametrizing similar cases (e.g., whitespace/newline/tab behaviors) with `pytest.mark.parametrize` instead of separate test functions.

## Individual Comments

### Comment 1
<location path="tests/unit/test_wecomai_event.py" line_range="15-24" />
<code_context>
+
+# ============================================================
+# strip_result 参数类型安全测试
+# ============================================================
+
+
+def test_strip_result_param_is_explicit():
+    """strip_result 是显式关键字参数,不影响位置参数调用"""
+    chain = MessageChain([Plain("测试")])
+    # 不传 strip_result,使用默认值 True
+    result1 = WecomAIBotMessageEvent._extract_plain_text_from_chain(chain)
+    # 显式传 strip_result=True
+    result2 = WecomAIBotMessageEvent._extract_plain_text_from_chain(chain, True)
+    assert result1 == result2 == "测试"
+
+
+def test_strip_result_false_prevents_whitespace_loss():
+    """验证 strip_result 参数机制:False 时保留空白,True 时去除"""
+    text = "  内容  "
+    chain = MessageChain([Plain(text)])
+
+    stripped = WecomAIBotMessageEvent._extract_plain_text_from_chain(chain, True)
+    notStripped = WecomAIBotMessageEvent._extract_plain_text_from_chain(chain, False)
+
+    assert stripped == "内容"
+    assert notStripped == "  内容  "
+    assert stripped != notStripped
</code_context>
<issue_to_address>
**issue (testing):** Add end-to-end tests around `send_streaming` / `enqueue_stream_plain` to ensure they actually use `strip_result=False` and preserve newlines in real flows.

Current tests validate `_extract_plain_text_from_chain` directly, but the original regression was in the streaming callers (`send_streaming` / `enqueue_stream_plain`) where `strip_result=False` is passed. If that argument is later removed or changed at the call sites, these tests could still pass.

Please add at least one async test for each of `send_streaming` and `enqueue_stream_plain` that:
- sends chunked content with `\n` (and optionally `\t`) through the real streaming path,
- inspects the final text or mocked downstream payload,
- asserts that newlines and formatting characters are preserved.

That will protect the actual streaming behavior, not just the helper function.
</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.

Comment thread tests/unit/test_wecomai_event.py Outdated
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 introduces a strip_result parameter to _extract_plain_text_from_chain in wecomai_event.py to allow preserving formatting and newlines during streaming, and adds comprehensive unit tests to verify this behavior. The feedback suggests renaming a variable in the tests to comply with PEP 8 naming conventions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/unit/test_wecomai_event.py Outdated
- 移除测试文件 test_wecomai_event.py,包含多个针对 _extract_plain_text_from_chain 方法的测试用例
- 测试用例涵盖了流式和非流式场景下的文本提取行为
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 4, 2026
@RC-CHN RC-CHN merged commit 9a648eb into AstrBotDevs:master Jun 4, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]流式输出时,企微机器人显示不会换行

2 participants