From e00072e9e6a62417a0f65b22547e6dea2fcf100d Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Fri, 19 Jun 2026 03:41:43 +0200 Subject: [PATCH 1/2] fix(backends): merge intrinsic extra_body once The intrinsic OpenAI path passed extra_body explicitly and also let user-supplied extra_body remain in api_params, so model_options containing extra_body raised TypeError before the request was sent. Pop user extra_body before updating api_params and merge it into the intrinsic extra_body, preserving chat_template_kwargs from both sides. Fixes #1241 Assisted-by: Codex Signed-off-by: Vincent Gao --- mellea/backends/openai.py | 13 +++++++ test/backends/test_openai_intrinsics_unit.py | 41 ++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/mellea/backends/openai.py b/mellea/backends/openai.py index 3683a81a3..2a08f6702 100644 --- a/mellea/backends/openai.py +++ b/mellea/backends/openai.py @@ -661,6 +661,7 @@ async def _generate_from_intrinsic( user_api_params = self._make_backend_specific_and_remove( model_options, is_chat_context=True ) + user_extra_body = user_api_params.pop("extra_body", None) api_params.update(user_api_params) # Map THINKING to the correct backend parameter(s). Two mechanisms: @@ -680,6 +681,18 @@ async def _generate_from_intrinsic( else: api_params["reasoning_effort"] = thinking + if user_extra_body is not None: + user_extra_body = dict(user_extra_body) + merged_extra_body = dict(extra_body) + user_ctk = user_extra_body.pop("chat_template_kwargs", None) + merged_extra_body.update(user_extra_body) + if user_ctk is not None: + merged_extra_body["chat_template_kwargs"] = { + **merged_extra_body.get("chat_template_kwargs", {}), + **user_ctk, + } + extra_body = merged_extra_body + # --- call the OpenAI-compatible API -------------------------------- # The rewriter may add instruction messages where 'role' is a default # (e.g. UserMessage with role="user"). exclude_unset would drop it, diff --git a/test/backends/test_openai_intrinsics_unit.py b/test/backends/test_openai_intrinsics_unit.py index 15a9fd6f2..dfda6f723 100644 --- a/test/backends/test_openai_intrinsics_unit.py +++ b/test/backends/test_openai_intrinsics_unit.py @@ -479,6 +479,47 @@ async def test_reasoning_effort_bool_true(): ) +async def test_user_extra_body_merges_into_intrinsic_extra_body(): + """User extra_body merges with intrinsic adapter keys without duplicate kwargs.""" + backend = _make_backend_with_adapter(_SIMPLE_CONFIG) + ctx = _make_context() + mock_create = AsyncMock(return_value=_simple_chat_completion()) + + mock_client = MagicMock() + mock_client.chat.completions.create = mock_create + + with patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ): + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), + ctx, + backend, + strategy=None, + model_options={ + ModelOption.THINKING: True, + "extra_body": { + "guided_json": {"type": "string"}, + "chat_template_kwargs": {"caller_key": "caller-value"}, + }, + }, + ) + await mot.avalue() + + call_kwargs = mock_create.call_args.kwargs + assert "extra_body" in call_kwargs + extra_body = call_kwargs["extra_body"] + assert "documents" in extra_body + assert extra_body["guided_json"] == {"type": "string"} + assert extra_body["chat_template_kwargs"]["adapter_name"] == "answerability" + assert extra_body["chat_template_kwargs"]["enable_thinking"] is True + assert extra_body["chat_template_kwargs"]["caller_key"] == "caller-value" + assert call_kwargs["reasoning_effort"] == "medium" + + async def test_reasoning_effort_bool_false(): """THINKING: False sets chat_template_kwargs.enable_thinking=False; no reasoning_effort.""" backend = _make_backend_with_adapter(_SIMPLE_CONFIG) From a1a58b24a12e6a186b96e9ea05df39d2cb52b7fc Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Mon, 27 Jul 2026 09:30:03 +0200 Subject: [PATCH 2/2] refactor(backends): extract _merge_user_extra_body helper Both the intrinsic and standard chat paths merged user extra_body with duplicated logic; the intrinsic copy had dropped the comments explaining the shallow copy and why a shallow merge is safe. Extract one helper so they stay in sync and the reasoning lives in a single place. Apply it to _generate_from_raw as well, which passed extra_body explicitly while also spreading backend-specific params that can contain it - the same TypeError as #1241 on the completions endpoint. Add tests for the merge helper, for the intrinsic path without THINKING, and for the completions path. Assisted-by: Claude Code Signed-off-by: Vincent Gao --- mellea/backends/openai.py | 72 ++++++++++------- test/backends/test_openai_intrinsics_unit.py | 66 +++++++++++++++ test/backends/test_openai_unit.py | 84 ++++++++++++++++++++ 3 files changed, 195 insertions(+), 27 deletions(-) diff --git a/mellea/backends/openai.py b/mellea/backends/openai.py index 2a08f6702..fe60e6e43 100644 --- a/mellea/backends/openai.py +++ b/mellea/backends/openai.py @@ -433,6 +433,39 @@ def _make_backend_specific_and_remove( return model_opts + @staticmethod + def _merge_user_extra_body( + base: dict[str, Any], user: dict[str, Any] | None + ) -> dict[str, Any]: + """Merges a user-supplied `extra_body` into the one Mellea built. + + Both must end up in a single `extra_body` value; passing two spreads that + each contain one raises `TypeError` at call time. + + Args: + base: the `extra_body` Mellea assembled for this request + user: `extra_body` taken from the caller's model_options, or None + + Returns: + a new dict; `base` and `user` are left unmodified + """ + if user is None: + return base + + # shallow copy so .pop() below doesn't mutate the caller's dict + user = dict(user) + merged = dict(base) + user_ctk = user.pop("chat_template_kwargs", None) + # shallow merge is safe: chat_template_kwargs is the only nested dict key + # Mellea writes into extra_body; it is deep-merged separately below + merged.update(user) + if user_ctk is not None: + merged["chat_template_kwargs"] = { + **merged.get("chat_template_kwargs", {}), + **user_ctk, + } + return merged + async def _generate_from_context( self, action: Component[C] | CBlock, @@ -681,17 +714,7 @@ async def _generate_from_intrinsic( else: api_params["reasoning_effort"] = thinking - if user_extra_body is not None: - user_extra_body = dict(user_extra_body) - merged_extra_body = dict(extra_body) - user_ctk = user_extra_body.pop("chat_template_kwargs", None) - merged_extra_body.update(user_extra_body) - if user_ctk is not None: - merged_extra_body["chat_template_kwargs"] = { - **merged_extra_body.get("chat_template_kwargs", {}), - **user_ctk, - } - extra_body = merged_extra_body + extra_body = self._merge_user_extra_body(extra_body, user_extra_body) # --- call the OpenAI-compatible API -------------------------------- # The rewriter may add instruction messages where 'role' is a default @@ -938,19 +961,9 @@ async def _generate_from_chat_context_standard( ) user_extra_body = backend_specific.pop("extra_body", None) if user_extra_body is not None: - # shallow copy so .pop() below doesn't mutate the caller's dict - user_extra_body = dict(user_extra_body) - eb = dict(extra_params.get("extra_body") or {}) - user_ctk = user_extra_body.pop("chat_template_kwargs", None) - # shallow merge is safe: chat_template_kwargs is the only nested dict - # key Mellea writes into extra_body; it is deep-merged separately below - eb.update(user_extra_body) - if user_ctk is not None: - eb["chat_template_kwargs"] = { - **eb.get("chat_template_kwargs", {}), - **user_ctk, - } - extra_params["extra_body"] = eb + extra_params["extra_body"] = self._merge_user_extra_body( + extra_params.get("extra_body") or {}, user_extra_body + ) chat_response: Coroutine[ Any, Any, ChatCompletion | openai.AsyncStream[ChatCompletionChunk] @@ -1217,15 +1230,20 @@ async def _generate_from_raw( prompts = [self.formatter.print(action) for action in actions] + backend_specific = self._make_backend_specific_and_remove( + model_opts, is_chat_context=False + ) + extra_body = self._merge_user_extra_body( + extra_body, backend_specific.pop("extra_body", None) + ) + try: completion_response: Completion = ( await self._async_client.completions.create( model=self._model_id, prompt=prompts, extra_body=extra_body, - **self._make_backend_specific_and_remove( - model_opts, is_chat_context=False - ), + **backend_specific, ) ) # type: ignore except openai.BadRequestError as e: diff --git a/test/backends/test_openai_intrinsics_unit.py b/test/backends/test_openai_intrinsics_unit.py index dfda6f723..fa3d12dc1 100644 --- a/test/backends/test_openai_intrinsics_unit.py +++ b/test/backends/test_openai_intrinsics_unit.py @@ -11,6 +11,7 @@ import json import pathlib +from copy import deepcopy from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch import pytest @@ -520,6 +521,71 @@ async def test_user_extra_body_merges_into_intrinsic_extra_body(): assert call_kwargs["reasoning_effort"] == "medium" +async def test_user_extra_body_without_thinking(): + """User extra_body merges when THINKING is unset; the #1241 reproduction.""" + backend = _make_backend_with_adapter(_SIMPLE_CONFIG) + ctx = _make_context() + mock_create = AsyncMock(return_value=_simple_chat_completion()) + + mock_client = MagicMock() + mock_client.chat.completions.create = mock_create + + with patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ): + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), + ctx, + backend, + strategy=None, + model_options={"extra_body": {"guided_json": {"type": "string"}}}, + ) + await mot.avalue() + + call_kwargs = mock_create.call_args.kwargs + extra_body = call_kwargs["extra_body"] + assert "documents" in extra_body + assert extra_body["guided_json"] == {"type": "string"} + assert extra_body["chat_template_kwargs"]["adapter_name"] == "answerability" + assert "reasoning_effort" not in call_kwargs + + +async def test_user_extra_body_is_not_mutated(): + """The caller's extra_body dict survives the merge unchanged.""" + backend = _make_backend_with_adapter(_SIMPLE_CONFIG) + ctx = _make_context() + mock_create = AsyncMock(return_value=_simple_chat_completion()) + + mock_client = MagicMock() + mock_client.chat.completions.create = mock_create + + user_extra_body = { + "guided_json": {"type": "string"}, + "chat_template_kwargs": {"caller_key": "caller-value"}, + } + original = deepcopy(user_extra_body) + + with patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ): + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), + ctx, + backend, + strategy=None, + model_options={"extra_body": user_extra_body}, + ) + await mot.avalue() + + assert user_extra_body == original + + async def test_reasoning_effort_bool_false(): """THINKING: False sets chat_template_kwargs.enable_thinking=False; no reasoning_effort.""" backend = _make_backend_with_adapter(_SIMPLE_CONFIG) diff --git a/test/backends/test_openai_unit.py b/test/backends/test_openai_unit.py index 03bd70df0..daa4b10bf 100644 --- a/test/backends/test_openai_unit.py +++ b/test/backends/test_openai_unit.py @@ -4,9 +4,13 @@ _simplify_and_merge, and _make_backend_specific_and_remove. """ +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch + import pytest +from openai.types import Completion from openai.types.chat import ChatCompletion, ChatCompletionChunk, ChatCompletionMessage from openai.types.chat.chat_completion import Choice +from openai.types.completion_choice import CompletionChoice from mellea.backends import ModelOption from mellea.backends.openai import OpenAIBackend @@ -315,5 +319,85 @@ async def test_processing_reasoning_content_takes_precedence_over_reasoning(back assert mot._underlying_value == "answer" +# --- _merge_user_extra_body --- + + +def test_merge_user_extra_body_none_returns_base(backend): + """A missing user extra_body leaves the base untouched.""" + base = {"documents": ["d"]} + assert backend._merge_user_extra_body(base, None) is base + + +def test_merge_user_extra_body_user_keys_win(backend): + """User keys overlay the base, and unrelated base keys survive.""" + merged = backend._merge_user_extra_body( + {"documents": ["d"], "guided_json": {"type": "integer"}}, + {"guided_json": {"type": "string"}}, + ) + assert merged == {"documents": ["d"], "guided_json": {"type": "string"}} + + +def test_merge_user_extra_body_deep_merges_chat_template_kwargs(backend): + """chat_template_kwargs merges key-wise rather than being replaced wholesale.""" + merged = backend._merge_user_extra_body( + {"chat_template_kwargs": {"adapter_name": "answerability"}}, + {"chat_template_kwargs": {"caller_key": "caller-value"}}, + ) + assert merged["chat_template_kwargs"] == { + "adapter_name": "answerability", + "caller_key": "caller-value", + } + + +def test_merge_user_extra_body_does_not_mutate_inputs(backend): + """Neither argument is modified; .pop() operates on a copy.""" + base = {"chat_template_kwargs": {"adapter_name": "answerability"}} + user = {"chat_template_kwargs": {"caller_key": "caller-value"}} + backend._merge_user_extra_body(base, user) + assert base == {"chat_template_kwargs": {"adapter_name": "answerability"}} + assert user == {"chat_template_kwargs": {"caller_key": "caller-value"}} + + +async def test_generate_from_raw_merges_user_extra_body(backend): + """The completions path passes one extra_body, not two spreads (#1241).""" + import pydantic + + from mellea.core.base import CBlock + from mellea.stdlib.context import ChatContext + + class Answer(pydantic.BaseModel): + value: int + + mock_create = AsyncMock( + return_value=Completion( + id="raw-test", + created=0, + model="fake", + object="text_completion", + choices=[CompletionChoice(index=0, finish_reason="stop", text="ok")], + ) + ) + mock_client = MagicMock() + mock_client.completions.create = mock_create + + with patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ): + await backend._generate_from_raw( + [CBlock(value="what is 1+1?")], + ChatContext(), + format=Answer, + model_options={"extra_body": {"caller_key": "caller-value"}}, + ) + + call_kwargs = mock_create.call_args.kwargs + extra_body = call_kwargs["extra_body"] + assert extra_body["caller_key"] == "caller-value" + assert "guided_json" in extra_body or "structured_outputs" in extra_body + + if __name__ == "__main__": pytest.main([__file__, "-v"])