feat(tool_parser,reasoning_parser): add Step-3.5 tool and reasoning parsers#1817
feat(tool_parser,reasoning_parser): add Step-3.5 tool and reasoning parsers#1817slin1237 wants to merge 1 commit into
Conversation
…arsers Tool parser handles Step-3.5's XML framing (<tool_call><function=name> <parameter=key>value</parameter></function></tool_call>) with schema-typed value coercion (JSON-first inference fallback) and incremental streaming. Reasoning parser uses <think>/</think> with always_in_reasoning=true (template prefills the think prefix, matching step3). Both register Step-3.5 patterns ahead of the broad step3 match so Step-3.5 IDs are not captured by the step3 parser. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds ChangesStep-3.5 Parser Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
tests
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Step-3.5 models by adding dedicated reasoning and tool parsers. The new Step3p5Parser in reasoning_parser handles reasoning outputs that start immediately inside the thinking state, while the Step3p5Parser in tool_parser parses XML-framed tool calls with schema-based type coercion and JSON-first inference. Extensive integration and unit tests are also added to verify routing, streaming, and edge cases. Feedback on the changes suggests improving integer parsing in the tool parser by attempting to parse as u64 when i64 parsing fails, which prevents large unsigned integers from unnecessarily degrading to strings.
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.
| if let Ok(n) = trimmed.parse::<i64>() { | ||
| return Some(Value::Number(n.into())); | ||
| } | ||
| // vLLM degrades to string on parse failure. | ||
| return Some(Value::String(raw.to_string())); |
There was a problem hiding this comment.
When parsing integer types (such as uint64 or unsigned long), values exceeding i64::MAX will fail to parse as i64 and degrade to a string representation. To better support large unsigned integers, we can attempt to parse as u64 if the i64 parsing fails.
if let Ok(n) = trimmed.parse::<i64>() {
return Some(Value::Number(n.into()));
}
if let Ok(n) = trimmed.parse::<u64>() {
return Some(Value::Number(n.into()));
}
// vLLM degrades to string on parse failure.
return Some(Value::String(raw.to_string()));![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 205643b504
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let current_args = &self.streamed_args_for_tool[self.current_tool_id as usize]; | ||
| if !current_args.is_empty() { | ||
| let open_braces = current_args.matches('{').count(); | ||
| let close_braces = current_args.matches('}').count(); | ||
| if open_braces > close_braces { |
There was a problem hiding this comment.
When a streamed string argument contains a } before </tool_call> (for example code snippets or text like foo }), this raw brace count can make open_braces == close_braces, so the parser skips appending the outer JSON object's closing brace. The emitted argument delta remains malformed (e.g. {"text": "foo }"), and the final unstreamed-args fallback cannot repair it because it compares against compact serde_json output with different spacing; since this parser always opens the argument object in parse_and_stream_parameters and never streams the outer close elsewhere, close it based on parser state rather than counting braces inside serialized values.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Step-3.5 must be registered BEFORE step3: first substring match wins and | ||
| // "step3.5"/"step3p5" both contain "step3". |
There was a problem hiding this comment.
🟡 Nit: This comment is indented to align with the trailing // on the kimi line above, making it visually appear as a continuation of the kimi comment. Moving it to a standalone block comment before line 200 would make the association with the step3.5 registrations clearer.
| // Step-3.5 must be registered BEFORE step3: first substring match wins and | |
| // "step3.5"/"step3p5" both contain "step3". | |
| // Step-3.5 must be registered BEFORE step3: first substring match wins and | |
| // "step3.5"/"step3p5" both contain "step3". |
There was a problem hiding this comment.
Clean, well-structured PR. The Step-3.5 tool and reasoning parsers correctly port the vLLM XML framing, the type coercion logic faithfully mirrors _convert_param_value, and factory routing is correct in both registries (first-substring-match for reasoning, longest-prefix-wins for tool parser). Test coverage is thorough at 36 cases spanning coercion branches, streaming boundary conditions, malformed input, and factory routing disambiguation vs step3.
0 🔴 Important · 1 🟡 Nit · 0 🟣 Pre-existing
|
This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you! |
1 participant
Description
Problem
SMG has no parser for Step-3.5, so its XML-framed tool calls and
<think>reasoning are not extracted.Solution
Add Step-3.5 tool and reasoning parsers, registered so Step-3.5 model IDs route to them ahead of the broad
step3patterns (first substring match wins, andstep3.5/step3p5both containstep3).Changes
crates/tool_parser/src/parsers/step3p5.rs): XML framing —<tool_call><function=name><parameter=key>value</parameter></function></tool_call>— with schema-typed value coercion (string/int/num/bool/objectfamilies, literalnull) and a JSON-first inference fallback when no schema is available; incremental streaming with partial-token hold-back. The single.expect()is on compile-time regex literals (#[expect(reason=…)]).crates/reasoning_parser/src/parsers/step3p5.rs):<think>/</think>viaBaseReasoningParserwithalways_in_reasoning=true(the template prefills the think prefix, matchingstep3).step3p5and routestep3.5/step-3.5/Step-3.5*/stepfun-ai/Step-3.5*to it, before the broadstep3match.Test Plan
cargo +nightly fmt --all— cleancargo clippy -p tool-parser -p reasoning-parser --all-targets -- -D warnings— cleancargo test -p tool-parser -p reasoning-parser— all pass, incl. the newtool_parser_step3p5suite (36 cases: single/parallel/multiple-function blocks, schema coercion + inference branches, streaming across chunk/char/multibyte boundaries and split inside function/parameter tags, malformed/incomplete/invalid input, reset, factory routing step3.5 → step3p5 not step3).Checklist
cargo +nightly fmtpassescargo clippy --all-targets -- -D warningspasses (parser crates; default features)Summary by CodeRabbit
Release Notes