fix(tool_parser): ignore <|END_ACTION|> inside Cohere JSON strings by santhreal · Pull Request #1941 · smg-project/smg

santhreal · 2026-07-19T03:32:28Z

Description

Problem

Solution

Scan for END_ACTION only outside JSON string literals (with escape awareness), matching how sibling parsers treat delimiter markers inside payloads.

Changes

crates/tool_parser/src/parsers/cohere.rs: string-aware END_ACTION locator for complete and streaming paths
crates/tool_parser/tests/tool_parser_cohere.rs: regression for a marker inside a string param

Test Plan

Before (pristine main): input
<|START_ACTION|>{"tool_name": "echo", "parameters": {"text": "say <|END_ACTION|> please"}}<|END_ACTION|>
yields 0 tools (JSON cut mid-string).

After: 1 tool named echo with text == "say <|END_ACTION|> please".

cargo +nightly fmt --all -- --check
cargo clippy -p tool-parser --all-targets -- -D warnings
cargo test -p tool-parser --test tool_parser_cohere

Checklist

cargo +nightly fmt passes
cargo clippy --all-targets on tool-parser with -D warnings passes
(Optional) Documentation updated
(Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

Bug Fixes
- Improved tool-call parsing when the action-end marker appears inside JSON string values.
- Prevented streaming responses from terminating prematurely in this scenario.
Tests
- Added coverage confirming marker text is preserved correctly in tool-call arguments.

A parameter value that mentions the end marker truncated the action JSON at the first match. Scan for END_ACTION only outside string literals. Signed-off-by: santhreal <64453045+santhreal@users.noreply.github.com>

coderabbitai · 2026-07-19T03:32:47Z

Warning

Review limit reached

@santhreal, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6bb37ae1-0b75-40d0-9b2a-139ea0f005f0

📥 Commits

Reviewing files that changed from the base of the PR and between 1f881c4 and ab57c19.

📒 Files selected for processing (2)

crates/tool_parser/src/parsers/cohere.rs
crates/tool_parser/tests/tool_parser_cohere.rs

📝 Walkthrough

Walkthrough

Cohere action parsing now detects END_ACTION only outside JSON strings in buffered and incremental flows. An integration test verifies embedded marker text remains part of a tool argument.

Changes

Cohere action parsing

Layer / File(s)	Summary
Outside-string marker detection `crates/tool_parser/src/parsers/cohere.rs`	A scanner tracks quoted and escaped JSON content so `END_ACTION` inside strings is not treated as the terminator.
Incremental integration and regression coverage `crates/tool_parser/src/parsers/cohere.rs`, `crates/tool_parser/tests/tool_parser_cohere.rs`	Incremental parsing uses the scanner, and a test verifies embedded marker text is preserved in decoded arguments.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: slin1237, catherinesue

Poem

I’m a bunny guarding JSON tight,
Keeping markers safe in strings tonight.
The parser hops past false ends,
While tests protect its furry friends.
One tool call lands just right!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: Cohere parser handling of <\|END_ACTION\|> inside JSON strings.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

gemini-code-assist

Code Review

This pull request updates the Cohere tool parser to correctly handle and ignore END_ACTION markers when they appear inside JSON string parameters. It introduces a helper function find_end_action_outside_strings to parse the JSON structure and skip occurrences within strings, and adds a corresponding integration test. The review feedback suggests a performance optimization in the helper function to check the first byte of the needle before calling starts_with.

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.

gemini-code-assist · 2026-07-19T03:33:44Z

+            if bytes[i..].starts_with(needle) {
+                return Some(i);
+            }


To avoid calling starts_with on every single character outside of JSON strings, we can first check if the current byte matches the first byte of the needle (using needle.first()). This improves performance by avoiding slice comparison overhead for non-matching characters.

Suggested change

if bytes[i..].starts_with(needle) {

return Some(i);

}

if Some(&b) == needle.first() && bytes[i..].starts_with(needle) {

return Some(i);

}

Thanks. Added the needle.first() guard in 3e1c3a1.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f881c4434

ℹ️ 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".

chatgpt-codex-connector · 2026-07-19T03:35:08Z

+            if in_string {
+                if b == b'\\' {
+                    escape = true;
+                    i += 1;
+                    continue;
+                }
+                if b == b'"' {
+                    in_string = false;
+                }
+                i += 1;
+                continue;


Recover at delimiters after malformed quoted JSON

When model output has an unterminated JSON string before a real <|END_ACTION|> delimiter, in_string never clears, so this loop ignores that delimiter. parse_complete then leaves the entire action block in normal_text, while incremental parsing stays in InAction and can swallow all later chunks/actions; before this change the delimiter was still recognized and the malformed action was discarded. The existing malformed-JSON coverage shows malformed action output is an expected parser input.

Useful? React with 👍 / 👎.

Thanks. Valid marker-in-string cases still use the string-aware scan; if quotes never close we now fall back to a plain END_ACTION search so malformed actions still terminate (3e1c3a1).

Keep string-aware scanning for valid marker-in-string payloads, but fall back to a plain delimiter search when quotes never close so malformed actions still terminate. Signed-off-by: santhreal <64453045+santhreal@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e1c3a1ef3

ℹ️ 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".

chatgpt-codex-connector · 2026-07-19T03:41:12Z

+        if in_string {
+            return s.find(END_ACTION);


Select the real delimiter after malformed strings

When malformed output has an unterminated JSON string containing <|END_ACTION|> before the actual closing delimiter, this fallback returns the first marker—the one still inside the string—rather than the later real delimiter. The parser then consumes that marker, drops the malformed action, and leaves the real delimiter in normal text (or disrupts subsequent streaming state), so the documented malformed-JSON recovery does not work for the same marker-in-string case this change introduces.

Useful? React with 👍 / 👎.

Thanks. Fallback now uses rfind so an earlier marker inside the unclosed string is skipped in favor of the last delimiter (2ce7c0a), with a regression test.

An earlier marker can sit inside the unclosed string; the real close is usually last. Signed-off-by: santhreal <64453045+santhreal@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ce7c0a725

ℹ️ 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".

chatgpt-codex-connector · 2026-07-19T04:01:40Z

+        if in_string {
+            // Prefer the last marker: an earlier one may sit inside the
+            // unclosed string before the real closing delimiter.
+            return s.rfind(END_ACTION);


Preserve markers inside unfinished streamed strings

When a valid string containing <|END_ACTION|> arrives before its closing quote and the real delimiter (a normal streaming chunk boundary), in_string remains true and this fallback returns that in-string marker. parse_incremental then drains it and switches to Text, so the rest of the JSON is emitted as normal text rather than completing the tool call once later chunks arrive. The fallback must not run for an incomplete stream without a confirmed closing delimiter.

Useful? React with 👍 / 👎.

Thanks. Agreed — removed the find/rfind fallback entirely in $(git rev-parse --short HEAD). Unclosed quotes now return None so streaming can wait; only a delimiter outside strings completes the action.

Correction: that commit is ab57c19.

chatgpt-codex-connector · 2026-07-19T04:01:40Z

+        if in_string {
+            // Prefer the last marker: an earlier one may sit inside the
+            // unclosed string before the real closing delimiter.
+            return s.rfind(END_ACTION);


Do not consume subsequent actions during malformed recovery

For a malformed first action with an unclosed string followed by a valid second <|START_ACTION|>...<|END_ACTION|> block, rfind selects the second action's closing marker. extract_action_json consequently hands both blocks to one JSON parse, which fails, and consumes the valid second block instead of allowing the loop to parse it. This regresses recovery for valid actions following malformed model output.

Useful? React with 👍 / 👎.

Thanks. Same fix: no rfind recovery, so a malformed first action cannot consume a later valid block ($(git rev-parse --short HEAD)).

Correction: that commit is ab57c19.

String-aware scan only. A fallback terminates mid-stream when a marker sits inside an unfinished string, and rfind can swallow a following action. Unclosed quotes wait for more input instead. Signed-off-by: santhreal <64453045+santhreal@users.noreply.github.com>

santhreal requested review from CatherineSue and slin1237 as code owners July 19, 2026 03:32

github-actions Bot added tests Test changes tool-parser Tool/function call parser changes labels Jul 19, 2026

gemini-code-assist Bot reviewed Jul 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

coderabbitai Bot approved these changes Jul 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

fix(tool_parser): prefer last END_ACTION when recovering unclosed JSON

2ce7c0a

An earlier marker can sit inside the unclosed string; the real close is usually last. Signed-off-by: santhreal <64453045+santhreal@users.noreply.github.com>

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

slin1237 approved these changes Jul 19, 2026

View reviewed changes

slin1237 merged commit 3051e76 into smg-project:main Jul 19, 2026
78 of 81 checks passed

ai-jz mentioned this pull request Sep 9, 2026

fix(tool_parser): preserve Qwen XML streamed arguments #2490

Merged

2 of 4 tasks

Conversation

santhreal commented Jul 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Changes

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

santhreal Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

santhreal commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading