Skip to content

Normalize OpenCode tool arguments before Qwen rendering - #24

Merged
bingran-you merged 1 commit into
mainfrom
bry/fix-vllm-followup-render
Jul 14, 2026
Merged

Normalize OpenCode tool arguments before Qwen rendering#24
bingran-you merged 1 commit into
mainfrom
bry/fix-vllm-followup-render

Conversation

@bingran-you

Copy link
Copy Markdown
Contributor

What changed

  • Normalizes OpenAI-compatible stringified tool_calls[].function.arguments into JSON objects before forwarding follow-up conversations to the TRL vLLM server.
  • Reuses the same normalization for GRPO prompt-ID reconstruction, eliminating duplicate logic and keeping served/trained tokenization aligned.
  • Fails closed on malformed or non-object tool arguments.
  • Documents the OpenCode-to-Qwen follow-up conversion.

Why

A live red-wine OpenCode canary reached and completed its first tool call, then the TRL server hung while rendering the second model turn. Replaying the exact captured four-message follow-up through the official Qwen3.5 tokenizer reproduced the root cause immediately:

TypeError: Can only get item pairs from a mapping.

OpenCode correctly emits OpenAI chat-completions messages with function arguments serialized as JSON strings. Qwen3.5's chat template iterates those arguments as a mapping. GRPO's offline token reconstruction already converted the string to an object, but the live bridge did not.

A fake TRL backend using the same BenchFlow + OpenCode harness completed both turns, proving the agent loop and tool execution were healthy and isolating the failure to live Qwen chat-template rendering.

Validation

  • 215 package contract tests pass.
  • 39 focused model-bridge and GRPO tests pass.
  • Ruff check/format, Python compilation, and git diff --check pass.
  • Live fake-TRL OpenCode canary completed two model turns, one real sandbox tool call, and verifier execution with no idle timeout.
  • Exact captured follow-up payload has four messages, string tool output, and 10 tools; normalization preserves the original OpenCode message while supplying Qwen a JSON object.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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: d8a9d8f5cc

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

Comment on lines +121 to +122
if not isinstance(arguments, str):
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-mapping tool arguments

When a follow-up message has function.arguments already decoded to a list/null, or the field is missing, this branch treats it as already normalized and forwards it to TRL. Qwen's template is the place that requires a mapping, so those malformed histories still fail during rendering instead of being rejected by the bridge/GRPO normalization; only string values get the object check below. Please allow existing dicts but raise on any other argument value.

Useful? React with 👍 / 👎.

@bingran-you
bingran-you merged commit 0925ea7 into main Jul 14, 2026
1 check passed
@bingran-you
bingran-you deleted the bry/fix-vllm-followup-render branch July 14, 2026 17:38
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.

1 participant