fix(provider): strip GLM non-standard special tokens in zhipu adapter#5675
fix(provider): strip GLM non-standard special tokens in zhipu adapter#5675NayukiChiba wants to merge 3 commits intoAstrBotDevs:masterfrom
Conversation
GLM models (e.g. glm-4.6v-flash) can leak internal control tokens into the visible reply content: - <None> – model's null-response signal - <|endoftext|> – end-of-sequence token - <|user|>, <|assistant|>, <|system|>, <|observation|> – role tokens Fix by overriding _normalize_content and _parse_openai_completion in ProviderZhipu to apply a regex cleaning pass that removes these tokens before the response is returned to the user. Also corrects a wrong import (star.func_tools -> agent.tool) that was present in the original stub. Closes AstrBotDevs#5556
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 addresses an issue where GLM models, specifically when used via the Zhipu provider, would occasionally include internal control tokens in their output, which are not intended for end-users. The changes introduce robust cleaning logic at multiple points in the response processing pipeline to ensure these non-standard tokens are removed, resulting in cleaner and more user-friendly model outputs. This significantly improves the quality of responses from GLM models. Highlights
Changelog
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.
Code Review
This pull request introduces a fix to strip non-standard special tokens from Zhipu GLM provider responses. The implementation is robust, using a two-pass cleaning approach to handle both regular and streaming responses, which is a thoughtful way to address potential issues with tokens split across chunks. The accompanying tests are comprehensive and well-structured, covering various edge cases. I have one suggestion to improve the test code structure by using a pytest fixture for resource management, which would reduce boilerplate and enhance maintainability. Overall, this is a high-quality contribution.
将 TestParseOpenAICompletionCleaning 中的 provider fixture 从 @pytest.fixture 改为 @pytest_asyncio.fixture,以兼容 asyncio STRICT 模式,同时移除各测试方法中重复的 try...finally 样板代码
There was a problem hiding this comment.
Hey - 我在这里给出一些整体性的反馈:
- 在
_clean_glm_special_tokens中,<None>的正则(re.compile(r"<None>", re.IGNORECASE)) 会匹配到更长的 token 中的子串,比如<NoneThing>;建议通过锚点、单词边界,或显式限定两侧尖括号的方式,只匹配真正需要处理的内容,避免误删仅仅包含该子串的合法内容。 - 在
_parse_openai_completion中的额外清洗逻辑,会用一个新的MessageChain().message(cleaned)覆盖llm_response.result_chain,这会丢弃基础解析器生成的更丰富结构或多段内容(例如工具调用或非文本部分);建议仅原地更新相关消息内容,而不是重建整个链。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `_clean_glm_special_tokens`, the `<None>` regex (`re.compile(r"<None>", re.IGNORECASE)`) will match inside larger tokens like `<NoneThing>`; consider anchoring or using word boundaries/explicit surrounding angle brackets to avoid stripping legitimate content that merely contains that substring.
- The extra cleaning in `_parse_openai_completion` overwrites `llm_response.result_chain` with a new `MessageChain().message(cleaned)`, which will discard any richer structure or multiple segments the base parser produced (e.g. tool calls or non-text parts); consider updating only the relevant message content in-place instead of reconstructing the chain.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- In
_clean_glm_special_tokens, the<None>regex (re.compile(r"<None>", re.IGNORECASE)) will match inside larger tokens like<NoneThing>; consider anchoring or using word boundaries/explicit surrounding angle brackets to avoid stripping legitimate content that merely contains that substring. - The extra cleaning in
_parse_openai_completionoverwritesllm_response.result_chainwith a newMessageChain().message(cleaned), which will discard any richer structure or multiple segments the base parser produced (e.g. tool calls or non-text parts); consider updating only the relevant message content in-place instead of reconstructing the chain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_clean_glm_special_tokens`, the `<None>` regex (`re.compile(r"<None>", re.IGNORECASE)`) will match inside larger tokens like `<NoneThing>`; consider anchoring or using word boundaries/explicit surrounding angle brackets to avoid stripping legitimate content that merely contains that substring.
- The extra cleaning in `_parse_openai_completion` overwrites `llm_response.result_chain` with a new `MessageChain().message(cleaned)`, which will discard any richer structure or multiple segments the base parser produced (e.g. tool calls or non-text parts); consider updating only the relevant message content in-place instead of reconstructing the chain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
我改一下re逻辑,有点太复杂了 |
根据 sourcery-ai review 意见进行以下修复: 1. _GLM_NULL_TOKEN_RE 移除 re.IGNORECASE:GLM 只输出精确的 <None>(大写 N),去掉不必要的大小写不敏感标志使正则更精确, 避免误匹配非 GLM 控制 token 的合法内容。 2. _parse_openai_completion 改用 completion_text setter 原地 更新文本,不再重建整个 result_chain,从而保留 tool call 等非文本组件,防止丢弃基础解析器生成的更丰富结构。 3. 同步更新测试:<none>/<NONE> 不再被清除,断言改为保留原文。
|
加了一个None识别的玩意 |
|
@Soulter 可以merge了 |
GLM models (e.g. glm-4.6v-flash) can leak internal control tokens
into the visible reply content:
<None>– model's null-response signal<|endoftext|>– end-of-sequence token<|user|>,<|assistant|>,<|system|>,<|observation|>– role tokensFix by overriding
_normalize_contentand_parse_openai_completioninProviderZhiputo apply a regex cleaning pass that removes these tokensbefore the response is returned to the user.
Also corrects a wrong import (
star.func_tools→agent.tool) that waspresent in the original stub.
Closes #5556
Summary by Sourcery
从 Zhipu 提供方的响应中剥离 GLM 特有的非标准特殊标记,防止泄露的控制标记出现在用户可见的输出中。
Bug Fixes:
<None>和<|endoftext|>),包括组装后的流式补全结果,确保它们不再出现在助手回复中。Tests:
Original summary in English
Summary by Sourcery
Strip GLM-specific non-standard special tokens from Zhipu provider responses to prevent leaked control markers from appearing in user-visible output.
Bug Fixes:
Tests:
Original summary in English
Summary by Sourcery
从 Zhipu 提供方的响应中剥离 GLM 特有的非标准特殊标记,防止泄露的控制标记出现在用户可见的输出中。
Bug Fixes:
<None>和<|endoftext|>),包括组装后的流式补全结果,确保它们不再出现在助手回复中。Tests:
Original summary in English
Summary by Sourcery
Strip GLM-specific non-standard special tokens from Zhipu provider responses to prevent leaked control markers from appearing in user-visible output.
Bug Fixes:
Tests: