fix(middleware): handle string-serialized options in ClarificationMiddleware (#1997)

* fix(middleware): handle string-serialized options in ClarificationMiddleware (#1995)

Some models (e.g. Qwen3-Max) serialize array tool parameters as JSON
strings instead of native arrays. Add defensive type checking in
_format_clarification_message() to deserialize string options before
iteration, preventing per-character rendering.

* fix(middleware): normalize options after JSON deserialization

Address Copilot review feedback:
- Add post-deserialization normalization so options is always a list
  (handles json.loads returning a scalar string, dict, or None)
- Add test for JSON-encoded scalar string ("development")
- Fix test_json_string_with_mixed_types to use actual mixed types
This commit is contained in:
Xinmin Zeng 2026-04-08 21:04:20 +08:00 committed by GitHub
parent 5350b2fb24
commit ad6d934a5f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 135 additions and 0 deletions

View File

@ -1,5 +1,6 @@
"""Middleware for intercepting clarification requests and presenting them to the user.""" """Middleware for intercepting clarification requests and presenting them to the user."""
import json
import logging import logging
from collections.abc import Callable from collections.abc import Callable
from typing import override from typing import override
@ -60,6 +61,20 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]):
context = args.get("context") context = args.get("context")
options = args.get("options", []) options = args.get("options", [])
# Some models (e.g. Qwen3-Max) serialize array parameters as JSON strings
# instead of native arrays. Deserialize and normalize so `options`
# is always a list for the rendering logic below.
if isinstance(options, str):
try:
options = json.loads(options)
except (json.JSONDecodeError, TypeError):
options = [options]
if options is None:
options = []
elif not isinstance(options, list):
options = [options]
# Type-specific icons # Type-specific icons
type_icons = { type_icons = {
"missing_info": "", "missing_info": "",

View File

@ -0,0 +1,120 @@
"""Tests for ClarificationMiddleware, focusing on options type coercion."""
import json
import pytest
from deerflow.agents.middlewares.clarification_middleware import ClarificationMiddleware
@pytest.fixture
def middleware():
return ClarificationMiddleware()
class TestFormatClarificationMessage:
"""Tests for _format_clarification_message options handling."""
def test_options_as_native_list(self, middleware):
"""Normal case: options is already a list."""
args = {
"question": "Which env?",
"clarification_type": "approach_choice",
"options": ["dev", "staging", "prod"],
}
result = middleware._format_clarification_message(args)
assert "1. dev" in result
assert "2. staging" in result
assert "3. prod" in result
def test_options_as_json_string(self, middleware):
"""Bug case (#1995): model serializes options as a JSON string."""
args = {
"question": "Which env?",
"clarification_type": "approach_choice",
"options": json.dumps(["dev", "staging", "prod"]),
}
result = middleware._format_clarification_message(args)
assert "1. dev" in result
assert "2. staging" in result
assert "3. prod" in result
# Must NOT contain per-character output
assert "1. [" not in result
assert '2. "' not in result
def test_options_as_json_string_scalar(self, middleware):
"""JSON string decoding to a non-list scalar is treated as one option."""
args = {
"question": "Which env?",
"clarification_type": "approach_choice",
"options": json.dumps("development"),
}
result = middleware._format_clarification_message(args)
assert "1. development" in result
# Must be a single option, not per-character iteration.
assert "2." not in result
def test_options_as_plain_string(self, middleware):
"""Edge case: options is a non-JSON string, treated as single option."""
args = {
"question": "Which env?",
"clarification_type": "approach_choice",
"options": "just one option",
}
result = middleware._format_clarification_message(args)
assert "1. just one option" in result
def test_options_none(self, middleware):
"""Options is None — no options section rendered."""
args = {
"question": "Tell me more",
"clarification_type": "missing_info",
"options": None,
}
result = middleware._format_clarification_message(args)
assert "1." not in result
def test_options_empty_list(self, middleware):
"""Options is an empty list — no options section rendered."""
args = {
"question": "Tell me more",
"clarification_type": "missing_info",
"options": [],
}
result = middleware._format_clarification_message(args)
assert "1." not in result
def test_options_missing(self, middleware):
"""Options key is absent — defaults to empty list."""
args = {
"question": "Tell me more",
"clarification_type": "missing_info",
}
result = middleware._format_clarification_message(args)
assert "1." not in result
def test_context_included(self, middleware):
"""Context is rendered before the question."""
args = {
"question": "Which env?",
"clarification_type": "approach_choice",
"context": "Need target env for config",
"options": ["dev", "prod"],
}
result = middleware._format_clarification_message(args)
assert "Need target env for config" in result
assert "Which env?" in result
assert "1. dev" in result
def test_json_string_with_mixed_types(self, middleware):
"""JSON string containing non-string elements still works."""
args = {
"question": "Pick one",
"clarification_type": "approach_choice",
"options": json.dumps(["Option A", 2, True, None]),
}
result = middleware._format_clarification_message(args)
assert "1. Option A" in result
assert "2. 2" in result
assert "3. True" in result
assert "4. None" in result