Skip to content

Feature: 支持trace上报思考文本放<trace_think>块里#224

Closed
weimch wants to merge 1 commit into
mainfrom
feature/trace_thought_tag
Closed

Feature: 支持trace上报思考文本放<trace_think>块里#224
weimch wants to merge 1 commit into
mainfrom
feature/trace_thought_tag

Conversation

@weimch

@weimch weimch commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
  • 方便区分思考文本和正常文本,同时保持兼容性

- 方便区分思考文本和正常文本,同时保持兼容性
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

发现的问题

🚨 Critical

  • trpc_agent_sdk/telemetry/_trace.py:82tests/telemetry/test_trace.py:53:新增的 _join_parts_with_thought_taggetattr(part, "thought", False) 判断思考段,但现有测试的 _make_part 用的是无 spec 的 MagicMock,未设置 thought 属性。访问 MagicMock 任意未定义属性会返回一个真值子 mock,因此 getattr(part, "thought", False) 返回真值而非 False,导致所有文本段都被包成 <trace_think>...</trace_think>
    • 影响:现有断言会全部失败,例如 tests/telemetry/test_trace.py:230 期望 "hello\nworld",实际得到 "<trace_think>hello</trace_think>\n<trace_think>world</trace_think>";同类断言在 :249:269:370:384:406:471:545 等处同样失败,CI 被阻塞。
    • 修复方向:在 _make_part 显式 part.thought = False(并按用例可传入 thought),或用带 spec=Part 的 mock;同时补充 thought=True 的用例覆盖新分支。

⚠️ Warning

  • tests/telemetry/test_trace.py:53 附近:本 PR 引入了 thought 区分逻辑,但测试既未为既有 _make_part 设置 thought,也未新增任何 thought=True 的用例,新分支实际无正向覆盖。建议补充 thought 段被 <trace_think> 包裹、非 thought 段不包裹的断言,确保导出格式符合下游 zhiyan-llm 解析预期。

总结

存在一个必须修复的 Critical 问题:新辅助函数对无 spec 的 MagicMock part 判定 thought 恒为真,会使现有 telemetry 单测大面积失败并阻塞 CI。修复 _make_part 并补充 thought 分支测试后即可合入。

测试建议

  • 补充 thought=True 的 part 断言输出为 <trace_think>{text}</trace_think>thought=False 输出为原文,并覆盖 trace_runner/trace_cancellation/trace_agent(含 override_messagesuser_content 两条路径)。

result = []
for part in parts:
text = part.text or ""
if getattr(part, "thought", False):

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.

MagicMock 未设置 thought 导致思考段判定恒真

_join_parts_with_thought_taggetattr(part, "thought", False) 判断思考段,但测试 _make_part 用无 spec 的 MagicMock,任意未定义属性返回真值子 mock,导致所有文本段都被包成 <trace_think>...</trace_think>,现有断言大面积失败、CI 阻塞。修复建议:在 _make_part 显式 part.thought = False 或改用 spec=Part 的 mock,并补充 thought=True 用例。

@github-actions

Copy link
Copy Markdown

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


minchangwei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@weimch weimch closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants