-
Notifications
You must be signed in to change notification settings - Fork 273
fix: strip stray |CHAT2API| markers in tool call parsing#147
fix: strip stray |CHAT2API| markers in tool call parsing #147Biogasbottle wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df9eb06cd
i️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1 Badge Preserve empty parsed content in stream flush
Using parsed.content || this.buffer treats an intentionally empty cleaned result as falsy and falls back to the raw buffered payload. When the parser removes a malformed/unsupported tool block and no user-visible text remains, flush() emits the original |CHAT2API|... block back to clients, so the marker-leak bug still reproduces at end-of-stream. Use a nullish check (parsed.content ?? this.buffer) or equivalent to preserve valid empty strings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2 Badge Avoid dropping fenced code when cleaning invalid tool blocks
This cleanup is computed from parseable, which is content after stripFencedCodeBlocks(). In the toolCalls.length === 0 path, responses that contain an invalid tool block plus any unrelated triple-backtick code lose the entire fenced section, which is a regression from previous behavior where non-tool content was preserved. Clean against the original content (or otherwise reinsert fenced segments) so only the matched tool block is removed.
Useful? React with 👍 / 👎.
Problem
When using DeepSeek with managed XML tool calling (opencode client), |CHAT2API| markers leak into the response content, causing downstream tool execution to fail.
Root Cause
In both managedXml.parse() and ToolStreamParser.flush(), the parsed XML blocks were not being stripped from the content when:
The cleanup logic only ran when oolCalls.length > 0, leaving raw |CHAT2API| XML in the response text.
Fix
awMatches.reduce(...) to strip matched blocks even when oolCalls.length === 0
Additional