Skip to content

fix: coerce string follow_up to array in ask_followup_question tool#12234

Draft
roomote-v0[bot] wants to merge 2 commits intomainfrom
fix/coerce-follow-up-string-to-array
Draft

fix: coerce string follow_up to array in ask_followup_question tool#12234
roomote-v0[bot] wants to merge 2 commits intomainfrom
fix/coerce-follow-up-string-to-array

Conversation

@roomote-v0
Copy link
Copy Markdown
Contributor

@roomote-v0 roomote-v0 Bot commented Apr 30, 2026

Related GitHub Issue

Closes: #12233

Description

This PR attempts to address Issue #12233. Feedback and guidance are welcome.

Some models (e.g. Qwen3.6 35B-A3B) output the follow_up parameter of ask_followup_question as a string instead of the required array format. This causes the tool call to fail with a missing parameter error.

Changes:

  1. Added coerceFollowUp() helper in AskFollowupQuestionTool.ts that normalizes follow_up from various formats into the expected Suggestion[]:

    • Already an array: returned as-is
    • A JSON string that parses to an array: parsed and returned
    • A plain string: wrapped as [{ text: value }]
    • Anything else (null, undefined, number): returns undefined so callers can error
  2. Updated AskFollowupQuestionTool.execute() to apply coercion before the array check.

  3. Updated NativeToolCallParser.ts in both the partial streaming handler (createPartialToolUse) and finalization handler (finalizeStreamingToolCall) to coerce string follow_up values to arrays.

  4. Added/updated tests covering:

    • Plain string coercion (wrapped as single suggestion)
    • JSON string array coercion (parsed into proper array)
    • Non-coercible values (null, undefined, numbers) still error
    • Unit tests for the coerceFollowUp helper function itself
    • NativeToolCallParser finalization with string follow_up

Test Procedure

  • Run cd src && npx vitest run core/tools/__tests__/askFollowupQuestionTool.spec.ts
  • All 22 tests pass, covering existing behavior and new coercion paths
  • Lint and type checks pass across the full monorepo

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue.
  • Scope: Changes are focused on the linked issue.
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and updated tests have been added to cover the changes.
  • Documentation Impact: No documentation updates required.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

Additional Notes

This fix is backwards-compatible. Models that already produce arrays are unaffected. Models that produce strings will now have their output gracefully coerced into the expected format.

Interactively review PR in Roo Code Cloud

Some models (e.g. Qwen3.6 35B-A3B) output the follow_up parameter as a
string instead of the required array format. This adds a coerceFollowUp
helper that normalizes:
- JSON strings that parse to arrays
- Plain strings (wrapped as single suggestion)
- Existing arrays (passed through unchanged)

Applied in AskFollowupQuestionTool.execute() and both partial/finalize
handlers in NativeToolCallParser.

Closes #12233
@lyjxxxx
Copy link
Copy Markdown

lyjxxxx commented Apr 30, 2026

@roomote After this change, model can call ask_followup_question correctly. However, when I choose an answer, my model gives an error: Chat template error: Runtime error: Cannot convert value of type Optional to Jinja Value

@roomote-v0
Copy link
Copy Markdown
Contributor Author

roomote-v0 Bot commented Apr 30, 2026

Fixaroo Clock   See task

Fixed the Jinja template error by ensuring all coerced follow_up suggestions include an explicit mode: null field, matching the tool schema's required: ["text", "mode"] contract. All local checks passed (lint, type-check, 23/23 tests).

View commit | Revert commit

The tool schema requires mode on each suggestion item (required: ["text", "mode"]).
When follow_up was coerced from a string, the mode field was missing entirely.
Some model Jinja templates (e.g. Qwen via llama.cpp) fail with
"Cannot convert value of type Optional<Any> to Jinja Value" when a required
field is absent from tool call arguments in conversation history.

Now all coercion paths (coerceFollowUp helper + NativeToolCallParser partial
and finalization handlers) normalize every suggestion to include mode: null
when not explicitly set.
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.

[BUG]

2 participants