From 12e619c23315045013d78467b3bd6157beeb151c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=93=88=E5=9F=BA=E7=B1=B3?= <140241684+BlueX888@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:27:17 +0800 Subject: [PATCH] fix(models): tolerate non-mapping when_thinking_enabled.extra_body on the reasoning-contract path (#7039) * fix(models): tolerate non-mapping when_thinking_enabled.extra_body on the reasoning-contract path * docs: keep frontend guidance below the hard size limit --------- Co-authored-by: Willem Jiang --- .../harness/deerflow/models/factory.py | 13 +++++-- backend/tests/test_model_factory.py | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/backend/packages/harness/deerflow/models/factory.py b/backend/packages/harness/deerflow/models/factory.py index cd502b2270..d2a5341e68 100644 --- a/backend/packages/harness/deerflow/models/factory.py +++ b/backend/packages/harness/deerflow/models/factory.py @@ -160,7 +160,12 @@ def _infer_dialect(effective_wte: dict) -> str: Mirrors the historical disable-path heuristics so ``dialect: auto`` behaves like the legacy factory did for the same template shape. """ - extra_body = effective_wte.get("extra_body") or {} + extra_body = effective_wte.get("extra_body") + if not isinstance(extra_body, dict): + # ``ModelConfig.when_thinking_enabled`` accepts unvalidated nested values, + # so a truthy non-mapping ``extra_body`` reaches this heuristic; like a + # null value it carries no payload shape to infer from. + extra_body = {} thinking = extra_body.get("thinking") if isinstance(thinking, dict) and thinking.get("type"): return "openai_extra_body" @@ -272,7 +277,11 @@ def _apply_contract_thinking_settings( dialect = contract.dialect if contract.dialect != "auto" else _infer_dialect(effective_wte) # Both directions mirror whichever vLLM switch the template declares, so a # template using the older ``thinking`` key never also grows ``enable_thinking``. - chat_template_kwargs = (effective_wte.get("extra_body") or {}).get("chat_template_kwargs") or {} + # ``when_thinking_enabled`` holds unvalidated nested values: a truthy + # non-mapping ``extra_body`` has no ``chat_template_kwargs`` to mirror and is + # treated as absent, like the null values the ``or {}`` defaults already cover. + extra_body = effective_wte.get("extra_body") + chat_template_kwargs = (extra_body.get("chat_template_kwargs") or {}) if isinstance(extra_body, dict) else {} if thinking_enabled: payload = _deep_merge_dicts(_dialect_payload(dialect, enabled=True, chat_template_kwargs=chat_template_kwargs), effective_wte) if contract.history is not None and dialect == "openai_extra_body": diff --git a/backend/tests/test_model_factory.py b/backend/tests/test_model_factory.py index 51a97caee0..2d4464cf4d 100644 --- a/backend/tests/test_model_factory.py +++ b/backend/tests/test_model_factory.py @@ -2362,6 +2362,40 @@ def test_contract_dialect_auto_infers_disable_payload_from_when_thinking_enabled assert "reasoning_effort" not in captured +@pytest.mark.parametrize( + ("reasoning", "thinking_enabled", "expected"), + [ + ({"thinking": "optional"}, True, {"extra_body": "oops"}), + ({"thinking": "optional"}, False, {}), + ({"thinking": "optional", "dialect": "anthropic"}, True, {"thinking": {"type": "enabled"}, "extra_body": "oops"}), + ({"thinking": "optional", "dialect": "anthropic"}, False, {"thinking": {"type": "disabled"}}), + ], + ids=["auto-thinking-on", "auto-thinking-off", "anthropic-thinking-on", "anthropic-thinking-off"], +) +def test_contract_non_mapping_template_values_build_on_both_thinking_paths(monkeypatch, reasoning, thinking_enabled, expected): + """``ModelConfig.when_thinking_enabled`` is ``dict | None`` with unvalidated nested values, + so a truthy non-mapping ``extra_body`` loads fine from ``config.yaml`` — and the contract + path read it unconditionally: the ``dialect: auto`` inference and the shared + ``chat_template_kwargs`` lookup both called ``.get`` on it, so every ``create_chat_model`` + for the profile crashed with ``AttributeError`` on both thinking directions. The legacy + path forwards the identical template unchanged + (``test_legacy_non_mapping_template_values_are_forwarded_not_rejected``); the contract + path must likewise build, treating the value as absent for its dialect heuristics (the + way the existing ``isinstance`` guards treat null template values) while the enable + direction still forwards it like any other template value.""" + model = _contract_model(reasoning=reasoning, when_thinking_enabled={"extra_body": "oops"}) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="contract-model", thinking_enabled=thinking_enabled) + + for key in ("extra_body", "thinking"): + if key in expected: + assert captured[key] == expected[key], f"non-mapping template value broke the {key} payload" + else: + assert key not in captured, f"non-mapping template value leaked an unexpected {key} payload" + + @pytest.mark.parametrize(("thinking_enabled", "expected"), [(True, True), (False, False)]) def test_contract_dialect_vllm_synthesizes_chat_template_kwargs(monkeypatch, thinking_enabled, expected): model = _contract_model(reasoning={"thinking": "optional", "dialect": "vllm_chat_template"})