fix(wecombot): strip chain-of-thought blocks, rotate stream per round, deliver outbox media#2321
Open
MIKAZE3 wants to merge 8 commits into
Open
fix(wecombot): strip chain-of-thought blocks, rotate stream per round, deliver outbox media#2321MIKAZE3 wants to merge 8 commits into
MIKAZE3 wants to merge 8 commits into
Conversation
…, deliver outbox media The WeCom AI Bot adapter was leaking <think></think> tags in two ways: (1) the LiteLLM requester only stripped the legacy CRETIRE_REASONING_BEGINk marker, missing the public <think> convention used by MiniMax-M3 and similar providers; (2) the WeCom AI Bot stream channel was being closed as soon as the first LLM round finalised, so the second-round push (after a tool call) silently fell back to a markdown reply_text that WeCom does not render as a collapsible "thinking" block. This change fixes both: * LiteLLMRequester._process_thinking_content now strips both <think>…</think> and the legacy marker, with a stateful _ThinkStripState that handles tags split across streaming chunks. LocalAgentRunner._StreamAccumulator exposes the same strip as a final-pass safety net and the second-round accumulator is no longer seeded with the first round's content (which made every intermediate round repeat the opening line in the user reply). * WecomBotWsClient.push_stream_chunk now rotates the stream_id on is_final=True instead of tearing down the stream channel. The new stream_id lets the next LLM round open a fresh streaming message that WeCom renders as its own "thinking" block instead of a raw <think> chunk. * WecomBotMessageConverter.yiri2target no longer drops Image / Voice / File components -- it returns a list of per-component items, and the adapter routes media through a new upload_media (3-step aibot_upload_init / aibot_upload_chunk / aibot_upload_finish protocol) + reply_image / reply_file / reply_voice path. This unblocks the /workspace/outbox/<query_id>/ auto-delivery hook that was silently broken because Image components were dropped before reaching the SDK. Tests cover public-think stripping, stream rotation across tool rounds, and the upload-media / reply-media protocol.
dosubot
Bot
added
the
size:XL
Contributor
|
All contributors have signed the CLA. ✅ 所有贡献者均已签署 CLA。 |
dosubot
Bot
added
eh: Improve
Author
|
I have read the CLA Document and I hereby sign the CLA |
RockChinQ
added a commit
to langbot-app/cla
that referenced
this pull request
Jul 4, 2026
Author
|
抱歉，忘记补图了 修复思维链没删除干净 |
asyncpg returns JSON / JSONB columns as raw strings by default, which breaks every LangBot code path that expects a dict (mcp server loading, bot adapter config, pipeline config, etc.) when running on PostgreSQL. The asyncpg dialect's built-in on_connect hook should install the codecs, but it does not fire reliably on the SQLAlchemy 2.0 async engine path. Monkey-patch AsyncAdapt_asyncpg_dbapi.connect so that each new connection registers json / jsonb codecs eagerly via the wrapper's run_async hook.
… CoT This adds two new misc toggles under the pipeline output config and the plumbing they need end-to-end: * `remove-empty-think-round` (`zh_Hans` 删除无文本输出的思维链): when the LLM emits only chain-of-thought + tool calls in a round (no user-visible text), drop the round's `MessageChunk` from the stream so the user doesn't see an empty bubble. The tool-call loop still runs because `req_messages.append(final_msg)` is unconditional. * `keep-first-think-only` (`zh_Hans` 除了第一条消息的思维链): only effective when `remove-think` is on. Round 1 still streams into WeCom and renders as a discrete thinking block; rounds 2+ fall back to `reply_text` after their stream entry is popped by the SDK, so subsequent LLM rounds no longer allocate a new stream_id and no longer render as thinking blocks. This restores the pre-bb89fe7f fallback behaviour but only for rounds 2+, restoring the 'single thinking block + plain text messages afterwards' UX callers had asked for. Implementation notes: * The new `output.yaml` fields are rendered automatically by the pipeline form (no front-end change needed). * `WecomBotWsClient.push_stream_chunk` now keeps an end-of-round counter per `msg_id` and consults a caller-controlled `fallback_after_round` flag (default 0, meaning the bb89fe7 rotate-per-round behaviour is preserved). When the counter crosses the threshold the stream entry is dropped so the next push fails to find it and returns `False`; wecombot.py already falls through to `reply_text` in that case. * `respback.py` and `pipelinemgr.py` stash the pipeline config on the platform event (`_langbot_pipeline_config`) so the platform adapter can read the misc toggles without growing the adapter interface. The adapter pulls the toggles and toggles the SDK's `fallback_after_round` flag accordingly. * `localagent.py` resolves `remove-empty-think-round` the same way it already resolves `remove-think` (defensive nested `.get` so pipelines missing `output.misc` still parse). The dead-code `first_content` assignment removed by this commit was unused since the bb89fe7 rebase-accumulator fix.
…l-agent runner Builds on 592cffb which wired the toggle plumbing end-to-end. That commit left two correctness gaps in ``LocalAgentRunner.run`` that this change closes: * ``remove_empty_think_round`` only fired on the *post-round* ``final_msg`` yield. The streamed *final chunk* (yielded earlier in the first-round streaming loop, and in the tool-loop streaming block) bypassed the guard entirely, so a chain-of-thought-only round that streamed to completion would still flash an empty bubble at the user before the tool-call loop kicked in. The fix adds a ``msg.is_final and remove_empty_think_round and not (chunk.content or '').strip()`` guard at both yield sites, and keeps intermediate 8-chunk flushes untouched so the typing indicator still works. * The pre-loop ``_skip_emit`` condition required ``tool_calls`` to be present, which meant a round that returned neither text nor tool calls was still forwarded to the user as a blank line. The new condition drops the ``tool_calls`` gate and simply checks for visible text. The same guard is added to the non-streaming tool-loop path so subsequent rounds honour the toggle too. The tool-call loop is unaffected: ``req_messages.append(final_msg)`` runs unconditionally below, and the accumulator still produces ``final_msg`` for the next LLM call to consume. * ``keep-first-think-only`` is now honoured in the runner. The first LLM round (before any tool-call loop iteration) gets a round-aware ``_strip_think_first_round = remove_think and not keep_first_think_only`` value passed to the pre-loop ``_StreamAccumulator`` and the ``_invoke_*_fallback`` helpers. Subsequent rounds keep the raw ``remove_think`` value so per-tool-call micro-thoughts are still hidden. The toggle is a no-op when ``remove-think`` is off. Tests: 20 new unit tests pin the new behaviour and lock the guard shapes against future regressions (intermediate-chunk emission, ``msg.is_final`` gating, both pre-loop and tool-loop guard occurrences, and the end-to-end ``_StreamAccumulator`` empty-final scenario).
…y is on `WecomBotWsClient.push_stream_chunk` used ``round_idx > fallback_after_round`` to decide whether to drop the per-msg_id stream entry at end-of-round. With ``fallback_after_round=1`` (the value the WecomBot adapter sets when the ``keep-first-think-only`` misc toggle is enabled) this meant round 1's final chunk **rotated** the entry to a new stream_id instead of dropping it, and round 2's first push found the fresh entry, sent via the stream channel, and WeCom rendered it as a second streaming bubble (with the "thinking" indicator) instead of falling through to ``reply_text``. The off-by-one is in the comparison itself: the comment a few lines above says "``fallback_after_round == N`` for N >= 1: drop the stream entry once the round counter has crossed N", and the intent is for round N's final to be the last stream chunk — the very next push (round N + 1) must fail to find a stream_id so the caller falls back to the plain markdown ``reply_text`` path. That requires the drop to fire at ``round_idx == N``, i.e. ``round_idx >= N`` (>=), not ``round_idx > N`` (>). Switching ``>`` to ``>=`` makes the toggle behave as advertised: * ``fallback_after_round=0`` — unchanged. The condition is still short-circuited, so all rounds keep rotating the stream_id (the pre-bb89fe7f rotate-per-round behaviour is preserved). * ``fallback_after_round=1`` — round 1 final drops the entry; round 2's first push returns False and the WecomBot adapter falls back to ``reply_text`` for the rest of the turn, so WeCom only shows the "thinking" block on round 1. * ``fallback_after_round=2`` — round 1 still rotates; round 2 final drops; round 3+ falls back to ``reply_text``. Tests: new ``test_wecombot_round_fallback.py`` covers the three ``fallback_after_round`` regimes (1, 2, 0) and pins down both the drop point and the subsequent push failure. The pre-existing ``test_wecombot_stream_rotation.py`` already covers the ``fallback_after_round=0`` regression surface; it continues to pass without changes.
Author
|
更新添加： |
Member
|
感谢 PR，但有一些问题： respback.py / pipelinemgr.py 里通过 query.message_event._langbot_pipeline_config = query.pipeline_config 动态给事件对象塞私有字段，然后在 wecombot.py 里读取这个字段来决定 fallback_after_round。这不是一个稳定的接口契约，而是 pipeline → platform adapter 的隐式 side-channel： src/langbot/pkg/pipeline/respback/respback.py:57 和 src/langbot/pkg/pipeline/pipelinemgr.py:171 写私有字段，失败还被 except Exception: pass 静默吞掉。 这个设计会让后续维护很脆：事件对象变了、Pydantic 行为变了、或者别的输出阶段绕过这段写入，企业微信的流式行为就会静默退化。建议不要通过给 event 动态挂私有属性传配置。要么把 remove-think / keep-first-think-only 这类输出策略完整收敛在 provider / pipeline 层处理，要么显式扩展 reply_message_chunk 的参数或 chunk metadata，让 adapter 通过明确接口拿到它真正需要的输出策略。 此 PR 有更改多个功能，可以分开多个 PR 提交，逐个 review。 |
The ``remove-empty-think-round`` ``keep-first-think-only`` toggles are independent of the upstream sandbox-attachment delivery path, but they share the same wrapper code path that was failing in practice: ``ResponseWrapper._append_outbound_attachments`` only fired on a ``role='assistant'`` final message. When the agent did not produce a final assistant text message (which happens often — e.g. when the LLM immediately issues another tool call after seeing the tool result), outbox files were never delivered. This change splits the responsibility in two and moves the collect trigger to the tool-result branch so every sandbox tool call can ship its files independently of what the LLM does next. Wrapper stage (``ResponseWrapper.process``): * ``_collect_outbound_components`` is the new shared helper. It returns ``(components, service_available)`` so callers can tell whether the collect was a no-op because the service was not configured, vs. because the outbox was actually empty. * A new ``role='tool'`` branch in ``process`` collects and appends attachments every time a tool result reaches the pipeline. It deliberately bypasses the ``_sandbox_outbound_collected`` at-most-once flag because the terminal-assistant branch keeps that semantics for the legacy is_final path. * The legacy ``_append_outbound_attachments`` still sets ``_sandbox_outbound_collected`` — but only when the service was actually consulted, so an absent service does not poison the at-most-once gate on a query that later gains a service. Box service (``BoxService.collect_outbound_attachments``): * The host path now quarantines each file it hands out by ``os.replace``-ing it into ``<host_dir>/.sent/<rel>``. Future collects see the same key on the next walk and skip it, so a multi-tool agent that emits one image per tool round gets one image per round and never receives the same file twice. * ``os.walk`` is replaced with a manual ``os.scandir`` stack so the walk does not descend into ``.sent`` subtrees that appear mid-iteration as a side effect of the move. * Mid-directory quarantine keys on the full relative path (``a/1.jpg`` vs ``b/1.jpg``) so two identically-named files in different subdirectories are not confused. * ``finalize_outbound_attachments`` is a new explicit cleanup entry point that wipes the per-query outbox plus the quarantine. ``ChatHandler`` calls it once streaming completes so the next turn that reuses this ``query_id`` (counter resets across restarts) does not inherit stale attachments. * Exec fallback keeps the read-and-clear semantics (cross container rename is not atomic). The host path is the common case and is fully multi-collect-safe. Tests: * ``tests/unit_tests/box/test_box_outbox_lifecycle.py`` — exercises the host quarantine contract directly (covers single-shot, subdir disambiguation, inter-collect isolation, ``.sent/`` survival, oversize skip, finalize). * ``tests/unit_tests/pipeline/test_wrapper_tool_attachments.py`` — exercises the new ``role='tool'`` branch end-to-end through a constructed ``pipeline_query.Query``. * Two pre-existing tests in ``test_box_service.py`` (``test_outbound_reads_host_and_quarantines``, ``test_outbound_quarantine_survives_second_empty_collect``) were rewritten to assert the new quarantine layout instead of the old read-and-clear behaviour. One obsolete test (``test_outbound_empty_clears_stale_host_dir``) was deleted because the behaviour it asserted no longer exists.
dosubot
Bot
added
size:XXL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(wecombot, persistence): strip chain-of-thought, rotate stream, deliver outbox media, register asyncpg JSON codecs
本 PR 合并两个互不依赖但同时被合并进来的修复：
fix(wecombot)（commitbb89fe7f，对应 issue #2305）：修复企业微信智能机器人（AI Bot）适配器在生产环境长期未暴露的三个连锁 bug。fix(persistence)（commite7bad185）：修复 LangBot 在 PostgreSQL 后端下，asyncpg 引擎未注册 JSON / JSONB codec 导致所有 dict 类型列以裸str返回的回归。概述 / Overview
修复 1 —
fix(wecombot)（issue #2305）Bug 1 — 思维链
<think>…</think>漏剥。当底层 LLM（MiniMax-M3 等 OpenAI 兼容服务）使用公开的<think>…</think>协议把 CoT 文本混在流式content字段返回时，LiteLLMRequester._process_thinking_content只剥离了历史遗留的私有标记CRETIRE_REASONING_BEGINk…ENDk，完全没有识别公开的<think>标签。结果是整段思维链裸漏给企业微信用户。lark-radar/lark-radar#28也报告了相同现象。Bug 2 — 多轮 LLM 思维框丢失 + 内容重复。每个 tool 调用后的下一轮 LLM 流式输出都带前一轮的"开场白"前缀——
LocalAgentRunner._StreamAccumulator在第二轮实例化时被错误地注入了第一轮的first_content，导致accumulated_content每次累加都从零前缀重发。同时WecomBotWsClient.push_stream_chunk在is_final=True时立即pop掉_stream_ids[msg_id]关闭流通道——第二轮 LLM 的 push 走到key = self._stream_ids.get(msg_id) = None，返回False，触发 fallback 走reply_text整段 markdown 回复，企业微信不再把后续消息渲染为可折叠的"思考框"。Bug 3 —
/workspace/outbox/<query_id>/自动投递图片链路整体失效。WecomBotMessageConverter.yiri2target长期写成content = ''.join(msg.text for msg in message_chain if type(msg) is Plain)——完全丢掉Image/Voice/File组件。即便box_service.collect_outbound_attachments正确把/workspace/outbox/<query_id>/foo.png转成Image(base64=…)组件挂到 reply_chain，到了 wecombot 适配器层仍被清空。同时 wecom SDKws_client.py根本没有upload_media/reply_image/reply_file/reply_voice的实现——即便yiri2target不丢，wecom 端也没有发送图片的协议路径。修复 2 —
fix(persistence)asyncpg JSON/JSONB codecBug 4 — asyncpg 引擎下 JSON 列以裸
str返回。asyncpg 默认不为json/jsonb列注册 codec，从 PostgreSQL 读出的dict/list全部退化成str，进而导致：jsonb列）解析失败，工具加载不全；jsonb列）解析失败，机器人无法启动；jsonb列）解析失败，整条流水线直接异常。asyncpg 自身支持
set_type_codec，SQLAlchemy 的dialects.postgresql.asyncpg也提供了on_connecthook，但SQLAlchemy 2.0 async 引擎路径下 hook 不会可靠触发——需要在每次新连接上手动set_type_codec。本 PR 通过 monkey-patchAsyncAdapt_asyncpg_dbapi.connect，把 codec 注册函数挂到 SQLAlchemy 包装连接的run_async钩子上，保证每个新建的连接都被立即注册json/jsonbcodec。改动详情
修复 1 (
fix(wecombot))_process_thinking_content用re.sub剥<think>…</think>+ 剥CRETIRE_REASONING_BEGINk…ENDk（与 legacy 并行）；新增_ThinkStripState跨 chunk 状态机；invoke_llm_stream接入状态机_StreamAccumulator不再注入initial_content=first_content；_StreamAccumulator.add/final_message走LiteLLMRequester._strip_think兜底；修output.get('misc', '')的AttributeErrorpush_stream_chunk(is_final=True)轮换stream_id而非 pop；新增upload_media（3 步aibot_upload_init→aibot_upload_chunk× N →aibot_upload_finish）+reply_image/reply_file/reply_voiceWecomBotMessageConverter.yiri2target返回结构化 list，不丢Image/Voice/File；reply_message/reply_message_chunk拆解组件：plain 走 stream、media 走upload_media+reply_*修复 2 (
fix(persistence))_register_json_codecs()（json+jsonb注册为format='text'+json.loads解码 /json.dumps编码）；新增_patch_asyncpg_dialect()monkey-patchAsyncAdapt_asyncpg_dbapi.connect，把 codec 注册挂到 wrapper 的run_async钩子上；模块导入时自动_patch_asyncpg_dialect()测试结果 / Test results
修复 1
tests/unit_tests -q：601/606 通过 / 5 失败（全部预先存在的 Windows 路径 / env 大小写问题，已用git stash验证 baseline 同样失败）修复 2
bot_adapters.config（jsonb）→ 返回dict而非str；修改后再 select，回写 round-trip 正常tests/unit_tests内；建议在tests/integration_tests内补充端到端用例）on_connecthook 在 SQLite / MySQL 路径上不会被调用已知限制 / Known limitations
修复 1
3 步 upload 协议的
aibot_upload_init/aibot_upload_chunk/aibot_upload_finish命令字段名没有直接来自官方文档的完整示例（fetch 文档时该小节被截断），是按官方 PyPI SDK 描述 + 命名约定推断。如果 wecom 端 errcode 不接受，Image 会静默失败，文本流式仍然正常——日志会带 server errcode 方便定位。后续可基于 errcode 校准命令名。修复 2
AsyncAdapt_asyncpg_dbapi.connect的 monkey-patch 只在新建连接时生效。LangBot 启动早期已经建立的连接（极少数）不受影响，需要让它们自然回收并重建。生产环境下一个完整的 bot 重启即可。on_connecthook 在某些 SQLAlchemy 版本下会被跳过的根因未提交到 SQLAlchemy 上游；如果上游后续修复了，本 patch 可以安全移除（codec 注册不会双触发）。更改前后对比 / Before vs After
修改前 / Before
Bug 1 — 思维链裸漏
1. 思维链裸漏 — 机器人在企业微信里的回复整段被
<think></think>包裹：Bug 2 — 多轮回复重复开场白
2. 多轮回复重复开场白 — 用户问一个问题，机器人发出 4 条消息，每条都带同一个开场白：
好的，我来查一下指定时间段内三台目标主机的相关指标。先并行拉取数据。好的，我来查一下指定时间段内三台目标主机的相关指标。先并行拉取数据。数据已获取，我用 Python 计算每台机器过去三小时的统计指标（max / avg / 当前值）。好的，我来查一下指定时间段内三台目标主机的相关指标。先并行拉取数据。 exec 在本环境暂时不可用，我直接基于拉取到的三段 CSV 数据手算统计量并尝试调用绘图工具出图。好的，我来查一下指定时间段内三台目标主机的相关指标。先并行拉取数据。好的，绘图工具已激活。我先把三台机器的指标数据合并成一个 CSV 写到工作区，再用脚本画趋势图。 数据已获取并解析。这里先用 exec 把合并后的 CSV 写出来，再用 plot.py 画折线趋势图。Bug 3 — outbox 图片静默丢
3. outbox 图片静默丢 — 用户让机器人把
/workspace/outbox/<qid>/test_send.png发过来，机器人信誓旦旦"系统应当已经自动投递给你了"，但用户什么都没收到：实际没有任何图片出现在企业微信会话里。
Bug 4 — PG 后端启动崩溃
（asyncpg 返回的
row["config"]已经是str—— 上层又json.loads一个str，由于内容是嵌套 JSON 字符串而非 JSON literal，导致解析失败。）修改后 / After
Bug 1 — 思维链转可折叠"思考框"
1. 思维链转可折叠"思考框" — 用户看不到思维链原文；后续 tool 调用步骤的"思考"在 wecom 端单独渲染为可折叠的"思考框"，
<think>标签不再裸漏。Bug 2 — 多轮回复去重
2. 多轮回复去重 — 同一 LLM tool 循环里，每条 LLM 终局消息独立：
好的，我来查一下指定时间段内三台目标主机的相关指标。先并行拉取数据。数据已获取，我用 Python 计算每台机器过去三小时的统计指标（max / avg / 当前值）。exec 在本环境暂时不可用，我直接基于拉取到的三段 CSV 数据手算统计量并尝试调用绘图工具出图。好的，绘图工具已激活。我先把三台机器的指标数据合并成一个 CSV 写到工作区，再用脚本画趋势图。数据已获取并解析。这里先用 exec 把合并后的 CSV 写出来，再用 plot.py 画折线趋势图。每条都是 wecom AI Bot 流式消息（"思考框"），不再是 markdown 整段。
Bug 3 — outbox 自动投递图片
3. outbox 自动投递图片 — LLM 把 PNG 写到
/workspace/outbox/<query_id>/→ wecombot 适配器把 Image 组件 base64 解码 →upload_media（3 步 init / chunk / finish）拿到media_id→reply_image(req_id, media_id)推给 wecom 端 → 用户直接在企业微信会话里看到图片消息卡片。Bug 4 — PG 后端启动 / 读取正常
row["config"]拿到的是dict，pipeline_config.py/bot_adapter_config.py/mcp_server.py的所有json.loads(...)包裹可以保留作 defensive check，但不再因为TypeError崩溃。相关 issue / Related issues
<think></think>检查清单 / Checklist
PR 作者完成 / For PR author
项目维护者完成 / For project maintainer