diff --git a/CHANGELOG.md b/CHANGELOG.md index c7aba100a3..c60b6a5917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -375,6 +375,34 @@ This release closes that milestone with **181 merged pull requests**. directory such as `assets` also slipped through path validation and raised `IsADirectoryError`. Non-text content is now recorded as no previous text, and a directory path is a validation error instead of a crash. ([#5893]) +- **models:** `when_thinking_enabled` and `when_thinking_disabled` no longer + replace a profile's whole `extra_body` on the legacy (no `reasoning:` block) + path. Both templates were applied with a shallow `dict.update`, so a profile + carrying `extra_body: {tool_stream: true}` beside + `when_thinking_enabled.extra_body.thinking` / `when_thinking_disabled` + templates — the shape of most `extra_body`-based examples in + `config.example.yaml` — lost `tool_stream` in both directions, while the + synthesized disable payloads and the contract path already deep-merged. Both + legacy templates now deep-merge the same way. The merge semantics are: keys + are never removed, a template can only add or override, nested mappings + inherit the profile's other keys, and template values win on conflicts. The + merge also keeps the template's vLLM switch authoritative across its two + spellings: when a profile spells the switch differently from its template + (`chat_template_kwargs.enable_thinking: false` beside the legacy + `thinking: true` alias), the template's value is mirrored onto the profile's + spelling, so `VllmChatModel` and plain OpenAI-compatible classes alike send + the template's intent under whichever key the server reads — on both the + legacy and the contract path, in both directions. Non-mapping template + values are forwarded unchanged. Migration note: because keys are never + removed, a `when_thinking_disabled` template can no longer clear a key the + profile's base `extra_body` sets — a base + `extra_body.thinking: {type: enabled, budget_tokens: 4096}` now reaches the + provider as `{type: disabled, budget_tokens: 4096}` when thinking is off, + which Anthropic-style APIs reject. Enable-only keys such as `budget_tokens` + belong in `when_thinking_enabled`, not in the base `extra_body`; the + synthesized disable payloads and the contract path already behaved this + way. Templates are also deep-copied as they are merged, so constructor + kwargs never alias the cached profile. ([#5894]) - **projects:** The conversation-files view no longer shows an empty heading for a member thread that has no title yet. A thread's `display_name` is `null` on the wire until title generation has run (or if it never does), but @@ -6174,6 +6202,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#5881]: https://github.com/bytedance/deer-flow/pull/5881 [#5884]: https://github.com/bytedance/deer-flow/pull/5884 [#5893]: https://github.com/bytedance/deer-flow/pull/5893 +[#5894]: https://github.com/bytedance/deer-flow/pull/5894 [#5900]: https://github.com/bytedance/deer-flow/pull/5900 [#5928]: https://github.com/bytedance/deer-flow/pull/5928 [#5934]: https://github.com/bytedance/deer-flow/pull/5934 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index f9abc10d58..5db871fe57 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -363,6 +363,22 @@ 成员会抛出 `UnicodeDecodeError`,文件既不会被删除也不会被覆盖。像 `assets` 这样的裸支持目录 也能通过路径校验并抛出 `IsADirectoryError`。现在非文本内容会记录为“无原有文本”,目录路径 则作为校验错误返回,而不是崩溃。([#5893]) +- **模型:** 在旧路径(没有 `reasoning:` 块)上,`when_thinking_enabled` 与 `when_thinking_disabled` + 不再整体替换模型档案的 `extra_body`。两个模板此前都用浅层 `dict.update` 套用,因此档案里与 + `when_thinking_enabled.extra_body.thinking` / `when_thinking_disabled` 模板并列的 + `extra_body: {tool_stream: true}`(`config.example.yaml` 里大多数基于 `extra_body` 的示例都是 + 这种写法)在开与关两个方向都会丢失 `tool_stream`,而合成的禁用载荷和契约路径早已是深度合并。 + 现在两个旧路径模板同样深度合并。合并语义为:永不删除键,模板只能新增或覆盖,嵌套映射会继承 + 档案里的其他键,冲突时以模板值为准。合并还会让模板的 vLLM 开关在两种拼写之间保持权威:当档案 + 与模板对开关的拼写不同(`chat_template_kwargs.enable_thinking: false` 并列旧别名 + `thinking: true`)时,模板的值会镜像到档案的拼写上,因此 `VllmChatModel` 与普通的 OpenAI 兼容 + 类都会在服务端实际读取的那个键上发送模板的意图——旧路径与契约路径、开与关两个方向均如此。 + 非映射类型的模板值按原样转发。迁移提示:由于键永不删除,`when_thinking_disabled` 模板不再能 + 清除档案基础 `extra_body` 里设置的键——基础 `extra_body.thinking: {type: enabled, budget_tokens: 4096}` + 在关闭思考时会以 `{type: disabled, budget_tokens: 4096}` 发到 provider,Anthropic 风格的 API 会拒绝。 + `budget_tokens` 之类仅在开启时有意义的键应放在 `when_thinking_enabled` 里,而不是基础 `extra_body`; + 合成的禁用载荷与契约路径此前已是这一行为。模板在合并时会被深拷贝,因此构造参数不会与缓存的 + 档案共享对象。([#5894]) - **项目:** 会话文件视图不再为尚无标题的成员会话显示空标题。会话的 `display_name` 在标题 生成运行之前(或从未运行时)在接口上为 `null`,但文件分组类型将其声明为必填字符串并原样 渲染,因此在首次回复之前上传的文件会挂在一行空白之下。这类分组现在显示为“未命名”,与 @@ -5243,6 +5259,7 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子 [#5881]: https://github.com/bytedance/deer-flow/pull/5881 [#5884]: https://github.com/bytedance/deer-flow/pull/5884 [#5893]: https://github.com/bytedance/deer-flow/pull/5893 +[#5894]: https://github.com/bytedance/deer-flow/pull/5894 [#5900]: https://github.com/bytedance/deer-flow/pull/5900 [#5928]: https://github.com/bytedance/deer-flow/pull/5928 [#5934]: https://github.com/bytedance/deer-flow/pull/5934 diff --git a/backend/packages/harness/deerflow/models/AGENTS.md b/backend/packages/harness/deerflow/models/AGENTS.md index cbaa3337f0..92d3c5f7e6 100644 --- a/backend/packages/harness/deerflow/models/AGENTS.md +++ b/backend/packages/harness/deerflow/models/AGENTS.md @@ -9,7 +9,7 @@ fast-path permit check from queue insertion, or an older caller can be overtaken while handing off to the wait queue. - `create_chat_model(name, thinking_enabled)` instantiates LLM from config via reflection -- Supports `thinking_enabled` flag with per-model `when_thinking_enabled` overrides +- Supports `thinking_enabled` flag with per-model `when_thinking_enabled` / `when_thinking_disabled` overrides; on both the legacy and the contract path every template and every synthesized `extra_body` payload goes through `_merge_thinking_payload` (the native-Anthropic disable assigns the top-level `thinking` mapping outright — a tagged union keyed by `type`): keys are never removed (a template can only add or override; nested mappings inherit the profile's other keys), template wins on conflicts, so a template's `extra_body` never clobbers sibling keys such as `extra_body.tool_stream`. Corollary: a disable template cannot clear a key the profile sets, so enable-only keys such as `extra_body.thinking.budget_tokens` belong in `when_thinking_enabled`, never in the base `extra_body`. Templates are deep-copied as they are merged, so constructor kwargs never alias the cached `ModelConfig`. The vLLM switch has two spellings (`_VLLM_THINKING_SWITCHES`: legacy `thinking` alias, `enable_thinking`); when a profile spells it differently from the payload, the payload's value is mirrored onto the profile's spelling rather than deleting or canonicalizing it — `VllmChatModel` collapses the pair to `enable_thinking`, other OpenAI-compatible classes send both, and either way the server reads the template's intent. Do not canonicalize `thinking` → `enable_thinking` in the factory: chat templates differ on which key they honor, and only `VllmChatModel` opts into that remap - Supports vLLM-style thinking toggles via `when_thinking_enabled.extra_body.chat_template_kwargs.enable_thinking` for Qwen reasoning models, while normalizing legacy `thinking` configs for backward compatibility - A per-request `reasoning_effort` kwarg (the regular, non-bootstrap lead-agent build passes it even when `None`) is popped from `kwargs` and layered like `model_overrides`: a non-`None` value replaces the profile's, and the thinking transforms applied afterwards (`when_thinking_enabled`, `when_thinking_disabled`, the `extra_body.thinking` disable path) still decide the final value. Never let a key reach the constructor through both `kwargs` and the profile settings — Python raises `got multiple values for keyword argument` and the lead agent cannot be built for that model. Codex checks the requested level itself. Pinned by `tests/test_model_factory.py` and `tests/test_lead_agent_model_resolution.py` - Supports `supports_vision` flag for image understanding models diff --git a/backend/packages/harness/deerflow/models/factory.py b/backend/packages/harness/deerflow/models/factory.py index 14f8297262..4f4ae514c8 100644 --- a/backend/packages/harness/deerflow/models/factory.py +++ b/backend/packages/harness/deerflow/models/factory.py @@ -1,3 +1,4 @@ +import copy import logging from langchain.chat_models import BaseChatModel @@ -46,19 +47,26 @@ def _deep_merge_dicts(base: dict | None, override: dict) -> dict: for key, value in override.items(): if isinstance(value, dict) and isinstance(merged.get(key), dict): merged[key] = _deep_merge_dicts(merged[key], value) + elif isinstance(value, dict): + # Copy, don't alias: *override* is usually a template straight from + # the cached ``ModelConfig`` (see ``_merge_settings``). + merged[key] = copy.deepcopy(value) else: merged[key] = value return merged +# The two spellings of the vLLM/Qwen chat-template thinking switch: the legacy +# ``thinking`` alias DeerFlow documented first and ``enable_thinking``, which +# vLLM's Qwen reasoning parser reads. ``VllmChatModel`` maps the alias onto +# ``enable_thinking`` just before sending; other OpenAI-compatible classes +# forward both keys as written. +_VLLM_THINKING_SWITCHES = ("thinking", "enable_thinking") + + def _vllm_disable_chat_template_kwargs(chat_template_kwargs: dict) -> dict: """Build the disable payload for vLLM/Qwen chat template kwargs.""" - disable_kwargs: dict[str, bool] = {} - if "thinking" in chat_template_kwargs: - disable_kwargs["thinking"] = False - if "enable_thinking" in chat_template_kwargs: - disable_kwargs["enable_thinking"] = False - return disable_kwargs + return {key: False for key in _VLLM_THINKING_SWITCHES if key in chat_template_kwargs} def _merge_settings(settings: dict, payload: dict) -> None: @@ -71,10 +79,64 @@ def _merge_settings(settings: dict, payload: dict) -> None: for key, value in payload.items(): if isinstance(value, dict) and isinstance(settings.get(key), dict): settings[key] = _deep_merge_dicts(settings[key], value) + elif isinstance(value, dict): + # Never hand the constructor (or a later in-place edit here) the + # template's own mapping: payloads come straight from the cached + # ``ModelConfig``, and a shared reference would leak one call's + # adjustments into every later ``create_chat_model`` for that profile. + settings[key] = copy.deepcopy(value) else: settings[key] = value +def _chat_template_kwargs(settings: dict) -> dict | None: + """``settings["extra_body"]["chat_template_kwargs"]`` when both levels are mappings, else ``None``.""" + extra_body = settings.get("extra_body") + if not isinstance(extra_body, dict): + return None + chat_template_kwargs = extra_body.get("chat_template_kwargs") + return chat_template_kwargs if isinstance(chat_template_kwargs, dict) else None + + +def _merge_thinking_payload(settings: dict, payload: dict) -> None: + """Deep-merge a thinking on/off *payload*, keeping its vLLM switch authoritative. + + Keys are never removed: a template can only add or override, so a profile's + unrelated ``extra_body`` / ``chat_template_kwargs`` entries survive in both + directions. The one thing a plain deep merge gets wrong is the vLLM switch, + which has two spellings (see ``_VLLM_THINKING_SWITCHES``): a profile that + spells it differently from the payload (base ``enable_thinking: false``, + template ``thinking: true``) would keep both keys with opposite values, and + whichever key the server reads might be the profile's. So after merging, the + value the payload declares is mirrored onto the *other* spelling when the + profile carries it. Both keys then agree: ``VllmChatModel`` collapses them to + ``enable_thinking``, and a plain OpenAI-compatible class sends both, so the + server sees the payload's intent under whichever name it honors. No spelling + is invented (a template using only ``thinking`` never grows + ``enable_thinking``), and a payload that spells both itself is left as + written. Non-mapping template values are forwarded unchanged, as before. + + Deep-merging also means a template cannot *remove* a key by omitting it: + a base ``extra_body.thinking.budget_tokens`` survives a disable template + that only sets ``thinking.type: disabled``. Enable-only keys therefore + belong in ``when_thinking_enabled``, not in the profile's base settings. + """ + _merge_settings(settings, payload) + payload_kwargs = _chat_template_kwargs(payload) + merged_kwargs = _chat_template_kwargs(settings) + if payload_kwargs is None or merged_kwargs is None: + return + declared = {key: payload_kwargs[key] for key in _VLLM_THINKING_SWITCHES if key in payload_kwargs} + if len(declared) != 1: + return + value = next(iter(declared.values())) + # In-place is safe: ``_merge_settings`` / ``_deep_merge_dicts`` copy every + # template mapping they store, so ``merged_kwargs`` is never the config's own dict. + for key in _VLLM_THINKING_SWITCHES: + if key not in declared and key in merged_kwargs: + merged_kwargs[key] = value + + def _set_dotted_setting(settings: dict, path: str, value: object) -> None: """Write *value* at a dotted *path* (``reasoning_effort`` or ``extra_body.thinking.effort``).""" head, _, rest = path.partition(".") @@ -102,7 +164,7 @@ def _infer_dialect(effective_wte: dict) -> str: if isinstance(thinking, dict) and thinking.get("type"): return "openai_extra_body" chat_template_kwargs = extra_body.get("chat_template_kwargs") or {} - if isinstance(chat_template_kwargs, dict) and ("thinking" in chat_template_kwargs or "enable_thinking" in chat_template_kwargs): + if isinstance(chat_template_kwargs, dict) and any(key in chat_template_kwargs for key in _VLLM_THINKING_SWITCHES): return "vllm_chat_template" native_thinking = effective_wte.get("thinking") if isinstance(native_thinking, dict) and native_thinking.get("type"): @@ -114,7 +176,7 @@ def _infer_dialect(effective_wte: dict) -> str: def _vllm_toggle_chat_template_kwargs(chat_template_kwargs: dict, *, enabled: bool) -> dict: """Mirror whichever vLLM/Qwen switch the template declares; default to ``enable_thinking``.""" - toggled = {key: enabled for key in ("thinking", "enable_thinking") if key in chat_template_kwargs} + toggled = {key: enabled for key in _VLLM_THINKING_SWITCHES if key in chat_template_kwargs} return toggled or {"enable_thinking": enabled} @@ -143,31 +205,37 @@ def _apply_legacy_thinking_settings( ) -> None: """Historical thinking/effort payload path for profiles without a ``reasoning:`` contract. - Kept byte-for-byte so existing configurations retain their behavior (issue - #5073 acceptance criterion), including the synthesized ``reasoning_effort=minimal`` - on the OpenAI-compatible disable path that the contract path drops. + Kept as-is so existing configurations retain their behavior (issue #5073 + acceptance criterion), including the synthesized ``reasoning_effort=minimal`` + on the OpenAI-compatible disable path that the contract path drops. The one + deliberate change is how the operator's templates are applied: both + ``when_thinking_enabled`` and ``when_thinking_disabled`` used to be + ``dict.update``-ed onto the settings, so a template's ``extra_body`` replaced + the profile's whole ``extra_body`` and silently dropped sibling keys such as + GLM's ``tool_stream``. Both now go through ``_merge_thinking_payload`` like + the synthesized ``extra_body`` disable payloads and the contract path: keys + are never removed (so enable-only keys such as ``budget_tokens`` belong in + ``when_thinking_enabled``, not in the base ``extra_body``), template values + win on conflicts, and the template's vLLM switch stays authoritative across + its two spellings. The native-Anthropic disable still assigns + ``settings["thinking"]`` outright: that mapping is a tagged union keyed by + ``type``, and nothing else lives beside it at the top level. """ if requested_reasoning_effort is not None and not is_codex_model: settings["reasoning_effort"] = requested_reasoning_effort if thinking_enabled and has_thinking_settings and effective_wte: - settings.update(effective_wte) + _merge_thinking_payload(settings, effective_wte) if not thinking_enabled: if model_config.when_thinking_disabled is not None: - # User-provided disable settings take full precedence - settings.update(model_config.when_thinking_disabled) + # User-provided disable settings win on conflicts; profile keys survive. + _merge_thinking_payload(settings, model_config.when_thinking_disabled) elif has_thinking_settings and effective_wte.get("extra_body", {}).get("thinking", {}).get("type"): # OpenAI-compatible gateway: thinking is nested under extra_body - settings["extra_body"] = _deep_merge_dicts( - settings.get("extra_body"), - {"thinking": {"type": "disabled"}}, - ) + _merge_thinking_payload(settings, {"extra_body": {"thinking": {"type": "disabled"}}}) settings["reasoning_effort"] = "minimal" elif has_thinking_settings and (disable_chat_template_kwargs := _vllm_disable_chat_template_kwargs(effective_wte.get("extra_body", {}).get("chat_template_kwargs") or {})): # vLLM uses chat template kwargs to switch thinking on/off. - settings["extra_body"] = _deep_merge_dicts( - settings.get("extra_body"), - {"chat_template_kwargs": disable_chat_template_kwargs}, - ) + _merge_thinking_payload(settings, {"extra_body": {"chat_template_kwargs": disable_chat_template_kwargs}}) elif has_thinking_settings and effective_wte.get("thinking", {}).get("type"): # Native langchain_anthropic: thinking is a direct constructor parameter settings["thinking"] = {"type": "disabled"} @@ -208,11 +276,11 @@ def _apply_contract_thinking_settings( 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": payload = _deep_merge_dicts(payload, {"extra_body": {"thinking": {"clear_thinking": contract.history == "clear"}}}) - _merge_settings(settings, payload) + _merge_thinking_payload(settings, payload) elif model_config.when_thinking_disabled is not None: - _merge_settings(settings, model_config.when_thinking_disabled) + _merge_thinking_payload(settings, model_config.when_thinking_disabled) else: - _merge_settings(settings, _dialect_payload(dialect, enabled=False, chat_template_kwargs=chat_template_kwargs)) + _merge_thinking_payload(settings, _dialect_payload(dialect, enabled=False, chat_template_kwargs=chat_template_kwargs)) if contract.effort is None or contract.effort.path != "reasoning_effort": # A custom path is the only effort wire format for this contract. # Drop a generic key supplied by model_overrides as well as stale @@ -448,7 +516,9 @@ def create_chat_model(name: str | None = None, thinking_enabled: bool = False, * requested_reasoning_effort = resolved_reasoning.reasoning_effort if contract.source == "legacy": # Profiles without a ``reasoning:`` block keep the historical payload - # path byte-for-byte, driven by the caller's raw thinking flag. + # path, driven by the caller's raw thinking flag; the only change from + # the pre-contract factory is that its templates deep-merge (see the + # function docstring) instead of replacing whole settings. _apply_legacy_thinking_settings( model_settings_from_config, model_config, diff --git a/backend/tests/test_model_factory.py b/backend/tests/test_model_factory.py index 3f44fd9080..2db148ee1f 100644 --- a/backend/tests/test_model_factory.py +++ b/backend/tests/test_model_factory.py @@ -2,6 +2,8 @@ from __future__ import annotations +import copy + import pytest from langchain.chat_models import BaseChatModel from langchain_core.messages import HumanMessage @@ -285,6 +287,291 @@ def test_thinking_enabled_merges_when_thinking_enabled_settings(monkeypatch): assert FakeChatModel.captured_kwargs.get("max_tokens") == 16000 +def _legacy_model_with_base_extra_body(when_thinking_enabled: dict, extra_body: dict, when_thinking_disabled: dict | None = None) -> ModelConfig: + """A profile without ``reasoning:`` whose base ``extra_body`` carries keys the templates must not clobber.""" + return ModelConfig( + name="legacy-glm", + display_name="legacy-glm", + description=None, + use="langchain_openai:ChatOpenAI", + model="legacy-glm", + supports_thinking=True, + extra_body=extra_body, + when_thinking_enabled=when_thinking_enabled, + when_thinking_disabled=when_thinking_disabled, + ) + + +@pytest.mark.parametrize("thinking_enabled", [True, False], ids=["thinking-on", "thinking-off"]) +def test_legacy_when_thinking_disabled_template_deep_merges_like_the_enable_template(monkeypatch, thinking_enabled): + """Most ``extra_body``-based profiles in ``config.example.yaml`` declare *both* templates. + An operator-provided ``when_thinking_disabled`` was still ``dict.update``-ed, so fixing only + the enable branch would have left thinking-off dropping ``tool_stream`` while thinking-on + kept it. The same profile must not behave differently depending on the toggle.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"thinking": {"type": "enabled"}}}, + when_thinking_disabled={"extra_body": {"thinking": {"type": "disabled"}}}, + extra_body={"tool_stream": True}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=thinking_enabled) + + expected_type = "enabled" if thinking_enabled else "disabled" + assert captured["extra_body"] == {"tool_stream": True, "thinking": {"type": expected_type}} + + +@pytest.mark.parametrize( + "templates", + [ + {"when_thinking_disabled": {"extra_body": {"thinking": {"type": "disabled"}}}}, + {"when_thinking_enabled": {"extra_body": {"thinking": {"type": "enabled"}}}}, + ], + ids=["operator-disable-template", "synthesized-disable"], +) +def test_disable_cannot_clear_enable_only_keys_kept_in_the_base_extra_body(monkeypatch, templates): + """Stated migration rule: keys are never removed, so a base + ``extra_body.thinking: {type: enabled, budget_tokens: N}`` reaches the provider as + ``{type: disabled, budget_tokens: N}`` when thinking is off. The synthesized disable payload + (and the contract path) already behaved this way on ``main``; the operator-provided legacy + disable template now does too. Enable-only keys belong in ``when_thinking_enabled`` — the + changelog, ``models/AGENTS.md`` and ``configuration.mdx`` say so, and this test keeps the + behaviour deliberate rather than accidental.""" + model = ModelConfig( + name="legacy-anthropic-proxy", + display_name="legacy-anthropic-proxy", + description=None, + use="langchain_openai:ChatOpenAI", + model="claude", + supports_thinking=True, + extra_body={"thinking": {"type": "enabled", "budget_tokens": 4096}}, + **templates, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-anthropic-proxy", thinking_enabled=False) + + assert captured["extra_body"]["thinking"] == {"type": "disabled", "budget_tokens": 4096} + + +def _mapping_ids(value: object) -> set[int]: + """Identities of every mapping reachable from *value*, for alias checks.""" + if not isinstance(value, dict): + return set() + ids = {id(value)} + for nested in value.values(): + ids |= _mapping_ids(nested) + return ids + + +@pytest.mark.parametrize( + "base_extra_body, template_extra_body", + [ + ({"top_k": 20}, {"chat_template_kwargs": {"thinking": True}}), + (None, {"chat_template_kwargs": {"thinking": True}}), + ({"top_k": 20}, {"thinking": {"type": "enabled", "options": {"budget_tokens": 1024}}}), + ], + ids=["template-adds-a-nested-mapping", "profile-has-no-extra_body", "template-adds-two-levels"], +) +def test_thinking_templates_are_copied_not_aliased_into_constructor_kwargs(monkeypatch, base_extra_body, template_extra_body): + """Wherever the base lacked a key, the merge used to store the template's own mapping by + reference, so the constructor kwargs (and any later in-place adjustment, such as the vLLM + switch mirroring) pointed at the cached ``ModelConfig``. Every mapping a template + contributes, at every depth, must be a copy, and the profile must be unchanged after the + build.""" + template = {"extra_body": template_extra_body} + model = _legacy_model_with_base_extra_body(when_thinking_enabled=template, extra_body=base_extra_body) + pristine = copy.deepcopy(model.when_thinking_enabled) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"] == {**(base_extra_body or {}), **template_extra_body} + assert _mapping_ids(captured["extra_body"]) & _mapping_ids(model.when_thinking_enabled) == set() + assert model.when_thinking_enabled == pristine + + +def test_legacy_templates_only_add_or_override_nested_keys_never_remove_them(monkeypatch): + """The flip side of deep-merging: a template cannot drop a base key by omitting it. + OpenRouter-style ``reasoning: {exclude: true}`` in the profile survives a template that + only sets ``reasoning.effort`` — the operator must clear a key explicitly if that is meant.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"reasoning": {"effort": "high"}}}, + extra_body={"reasoning": {"exclude": True}}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"] == {"reasoning": {"exclude": True, "effort": "high"}} + + +@pytest.mark.parametrize( + "when_thinking_enabled, expected_extra_body", + [ + ({"extra_body": "oops"}, "oops"), + ({"extra_body": {"chat_template_kwargs": True}}, {"chat_template_kwargs": True}), + ], + ids=["extra_body-not-a-mapping", "chat_template_kwargs-not-a-mapping"], +) +def test_legacy_non_mapping_template_values_are_forwarded_not_rejected(monkeypatch, when_thinking_enabled, expected_extra_body): + """``ModelConfig.when_thinking_enabled`` is ``dict | None`` with unvalidated nested values, + so a non-mapping ``extra_body`` or ``chat_template_kwargs`` is reachable from ``config.yaml``. + The factory used to forward it unchanged; the vLLM-switch bookkeeping must not turn that + into an ``AttributeError`` / ``TypeError`` inside ``create_chat_model``.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled=when_thinking_enabled, + extra_body={"chat_template_kwargs": {"enable_thinking": False}}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"] == expected_extra_body + + +@pytest.mark.parametrize("thinking_enabled", [True, False], ids=["thinking-on", "thinking-off"]) +def test_legacy_when_thinking_enabled_deep_merges_into_the_base_extra_body(monkeypatch, thinking_enabled): + """The legacy enable path used ``dict.update``, so ``when_thinking_enabled.extra_body`` + replaced the profile's whole ``extra_body`` and dropped sibling keys such as GLM's + ``tool_stream`` — but only when thinking was ON; the disable path already deep-merged. + Both directions must keep the operator's keys.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"thinking": {"type": "enabled"}}}, + extra_body={"tool_stream": True}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=thinking_enabled) + + expected_type = "enabled" if thinking_enabled else "disabled" + assert captured["extra_body"] == {"tool_stream": True, "thinking": {"type": expected_type}} + + +def test_legacy_when_thinking_enabled_merges_nested_mappings_recursively(monkeypatch): + """A vLLM template that only sets ``chat_template_kwargs.enable_thinking`` must keep the + profile's other chat-template kwargs, not replace the nested mapping.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"chat_template_kwargs": {"enable_thinking": True}}}, + extra_body={"tool_stream": True, "chat_template_kwargs": {"preserve_thinking": True}}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"] == { + "tool_stream": True, + "chat_template_kwargs": {"preserve_thinking": True, "enable_thinking": True}, + } + + +def _effective_vllm_switch(captured: dict) -> dict: + """What vLLM receives: the provider normalizes the legacy ``thinking`` alias just before sending.""" + from deerflow.models.vllm_provider import _normalize_vllm_chat_template_kwargs + + payload = {"extra_body": dict(captured["extra_body"])} + _normalize_vllm_chat_template_kwargs(payload) + return payload["extra_body"]["chat_template_kwargs"] + + +def _vllm_switch_profile(*, base_key: str, template_key: str, base_value: bool, reasoning: dict | None, use: str = "deerflow.models.vllm_provider:VllmChatModel") -> ModelConfig: + kwargs: dict = dict( + name="qwen", + display_name="qwen", + description=None, + use=use, + model="qwen", + supports_thinking=True, + extra_body={"chat_template_kwargs": {base_key: base_value, "preserve_thinking": True}}, + when_thinking_enabled={"extra_body": {"chat_template_kwargs": {template_key: True}}}, + ) + if reasoning is not None: + kwargs["reasoning"] = reasoning + return ModelConfig(**kwargs) + + +@pytest.mark.parametrize("reasoning", [None, {"thinking": "optional", "dialect": "vllm_chat_template"}], ids=["legacy", "contract"]) +@pytest.mark.parametrize("base_key, template_key", [("enable_thinking", "thinking"), ("thinking", "enable_thinking")], ids=["base-enable_thinking/template-thinking", "base-thinking/template-enable_thinking"]) +@pytest.mark.parametrize("thinking_enabled", [True, False], ids=["thinking-on", "thinking-off"]) +def test_vllm_switch_spelled_by_the_template_wins_over_the_profile_alias(monkeypatch, reasoning, base_key, template_key, thinking_enabled): + """vLLM's toggle has two spellings (legacy ``thinking``, current ``enable_thinking``) and the + provider maps the alias only when ``enable_thinking`` is absent. A profile that spells the + switch differently from its template must not be able to pin the switch after the merge: + the template's (or synthesized) spelling decides, in both directions, on both paths.""" + from deerflow.models.vllm_provider import VllmChatModel + + model = _vllm_switch_profile(base_key=base_key, template_key=template_key, base_value=not thinking_enabled, reasoning=reasoning) + captured: dict = {} + # Resolve the REAL provider class, so a future class-gated merge is exercised as vLLM. + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(VllmChatModel, captured)) + + factory_module.create_chat_model(name="qwen", thinking_enabled=thinking_enabled) + + effective = _effective_vllm_switch(captured) + assert effective["enable_thinking"] is thinking_enabled + assert effective["preserve_thinking"] is True # unrelated chat-template kwargs still survive + + +@pytest.mark.parametrize("reasoning", [None, {"thinking": "optional", "dialect": "vllm_chat_template"}], ids=["legacy", "contract"]) +@pytest.mark.parametrize("base_key, template_key", [("enable_thinking", "thinking"), ("thinking", "enable_thinking")], ids=["base-enable_thinking/template-thinking", "base-thinking/template-enable_thinking"]) +@pytest.mark.parametrize("thinking_enabled", [True, False], ids=["thinking-on", "thinking-off"]) +def test_vllm_switch_is_mirrored_onto_the_profile_spelling_for_classes_that_do_not_normalize(monkeypatch, reasoning, base_key, template_key, thinking_enabled): + """Only ``VllmChatModel`` maps the ``thinking`` alias onto ``enable_thinking``; a plain + ``langchain_openai:ChatOpenAI`` profile pointed at vLLM sends ``chat_template_kwargs`` raw, and + the server reads whichever key its chat template honors. Deleting the profile's spelling would + remove the only key such a server reads (a base ``enable_thinking: false`` beside a template + ``thinking: false`` would leave Qwen3 defaulting to *on*). So the template's value is mirrored + onto the profile's spelling instead: both keys carry the template's intent, raw.""" + from langchain_openai import ChatOpenAI + + model = _vllm_switch_profile(base_key=base_key, template_key=template_key, base_value=not thinking_enabled, reasoning=reasoning, use="langchain_openai:ChatOpenAI") + captured: dict = {} + # Resolve the REAL ChatOpenAI (not a stand-in), so a merge gated on the vLLM class would fail here. + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(ChatOpenAI, captured)) + + factory_module.create_chat_model(name="qwen", thinking_enabled=thinking_enabled) + + raw = captured["extra_body"]["chat_template_kwargs"] # no normalization: what ChatOpenAI puts on the wire + assert raw == {base_key: thinking_enabled, template_key: thinking_enabled, "preserve_thinking": True} + + +def test_vllm_switch_mirroring_never_invents_the_other_spelling(monkeypatch): + """A template that spells the switch one way, on a profile that does not spell it at all, + must not grow the second key: ``VllmChatModel`` would ignore it, but a chat template that + honors only ``thinking`` would suddenly see an ``enable_thinking`` it never asked for.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"chat_template_kwargs": {"thinking": True}}}, + extra_body={"chat_template_kwargs": {"preserve_thinking": True}}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"]["chat_template_kwargs"] == {"preserve_thinking": True, "thinking": True} + + +def test_legacy_when_thinking_enabled_template_still_wins_on_conflicts(monkeypatch): + """Deep-merging must not weaken precedence: a key set in both places takes the template's value.""" + model = _legacy_model_with_base_extra_body( + when_thinking_enabled={"extra_body": {"thinking": {"type": "enabled", "budget_tokens": 4096}}}, + extra_body={"thinking": {"type": "disabled", "budget_tokens": 1}, "tool_stream": True}, + ) + captured: dict = {} + _patch_factory(monkeypatch, _make_app_config([model]), model_class=_capturing_class(FakeChatModel, captured)) + + factory_module.create_chat_model(name="legacy-glm", thinking_enabled=True) + + assert captured["extra_body"] == {"tool_stream": True, "thinking": {"type": "enabled", "budget_tokens": 4096}} + + # --------------------------------------------------------------------------- # thinking_enabled=False — disable logic # --------------------------------------------------------------------------- diff --git a/docs/plans/2026-09-23-reasoning-capability-contract.md b/docs/plans/2026-09-23-reasoning-capability-contract.md index c191cee4a9..f48967f255 100644 --- a/docs/plans/2026-09-23-reasoning-capability-contract.md +++ b/docs/plans/2026-09-23-reasoning-capability-contract.md @@ -109,11 +109,13 @@ of adjustment codes for logging. `create_chat_model` resolves the contract, resolves the request, and then branches on `contract.source`: -- **legacy** keeps the pre-existing code path byte-for-byte: the +- **legacy** keeps the pre-existing code path: the `when_thinking_enabled` guard, the inferred disable payloads, the hard-coded `reasoning_effort=minimal` on the OpenAI-compatible disable path, and the `supports_reasoning_effort=False` strip. Existing profiles therefore behave - exactly as before. + as before, with one later correction (#5894): the operator's + `when_thinking_enabled` / `when_thinking_disabled` templates deep-merge into + the profile's settings instead of replacing whole keys such as `extra_body`. - **contract** applies `when_thinking_enabled` / `thinking` / `when_thinking_disabled` when present and otherwise synthesizes the on/off payload from the dialect. `dialect: auto` infers the dialect from `when_thinking_enabled` the same way diff --git a/frontend/src/content/en/application/configuration.mdx b/frontend/src/content/en/application/configuration.mdx index b015ac2e57..ab37618baf 100644 --- a/frontend/src/content/en/application/configuration.mdx +++ b/frontend/src/content/en/application/configuration.mdx @@ -180,6 +180,7 @@ models: - When changing `effort.path`, remove old `reasoning_effort` keys from the profile and thinking templates. A declared contract rejects these conflicting keys at startup. The chat UI also drops a remembered provider-specific effort when switching to a legacy model that does not advertise it. - `default` also applies to callers that never choose an effort — summarization, title generation, and subagents — so pick a level you are happy to pay for on every background call (`high` above, with `max` left selectable in the composer). - `dialect: auto` (the default) infers the on/off payload from `when_thinking_enabled`, so existing profiles can add `reasoning` without rewriting their templates. Profiles without a `reasoning` block behave exactly as before. +- `when_thinking_enabled` and `when_thinking_disabled` are deep-merged into the profile's settings: a template can add or override keys, never remove them, so the profile's other `extra_body` keys (for example `tool_stream`) survive both toggles. Put enable-only keys such as `extra_body.thinking.budget_tokens` in `when_thinking_enabled`, not in the profile's base `extra_body` — a disable template cannot clear them, and Anthropic-style APIs reject them beside `type: disabled`. - Contradictory profiles (for example `required` together with `when_thinking_disabled`) fail at startup. ### Sandbox diff --git a/frontend/src/content/zh/application/configuration.mdx b/frontend/src/content/zh/application/configuration.mdx index cd2c42b2e5..2834676bd4 100644 --- a/frontend/src/content/zh/application/configuration.mdx +++ b/frontend/src/content/zh/application/configuration.mdx @@ -165,6 +165,7 @@ models: - 修改 `effort.path` 时,请移除 profile 和 thinking 模板中旧的 `reasoning_effort` 字段;契约会在启动时拒绝这类冲突配置。切换到未声明该取值的旧模型时,聊天界面也会丢弃记住的 provider 专属 effort 档位。 - `default` 同样作用于从不指定 effort 的调用方——总结、标题生成和子代理——所以请选一个你愿意为每次后台调用付费的档位(上例用 `high`,`max` 仍可在输入框里手动选择)。 - `dialect: auto`(默认值)会根据 `when_thinking_enabled` 推断开关 payload,因此已有配置可以直接加上 `reasoning` 而无需改写模板。未声明 `reasoning` 块的配置行为完全不变。 +- `when_thinking_enabled` 与 `when_thinking_disabled` 会深度合并到 profile 的设置中:模板只能新增或覆盖键,不能删除键,因此 profile 里其他的 `extra_body` 键(例如 `tool_stream`)在开关两个方向都会保留。像 `extra_body.thinking.budget_tokens` 这类仅在开启时有意义的键请放进 `when_thinking_enabled`,不要放在 profile 的基础 `extra_body` 里——禁用模板无法清除它们,而 Anthropic 风格的 API 会拒绝与 `type: disabled` 并列的这些键。 - 自相矛盾的配置(例如 `required` 同时配置了 `when_thinking_disabled`)会在启动时报错。 ### 沙箱