Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions docs/docs/integrations/openai.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,11 +434,13 @@ thinking setting is not silently dropped when a call also activates an
intrinsic adapter (which writes its own `chat_template_kwargs.adapter_name`).

Toggling thinking per call is also possible via `model_options={"extra_body":
...}` on the call itself, but not via `model_options={"extra_body": ...}` at
**construction** time — that older pattern is not deep-merged and can be
silently overwritten by an unrelated per-call `extra_body` on some other
call in the same session ([#1539](https://github.com/generative-computing/mellea/issues/1539)).
Use `default_extra_body` for anything you want to persist across every call.
...}` on the call itself. The older pattern of setting a persistent default
via `model_options={"extra_body": ...}` at **construction** time also works —
`chat_template_kwargs` in a construction-time `model_options["extra_body"]` is
deep-merged with any per-call `extra_body`, so a call that passes its own,
unrelated per-call `extra_body` does not drop it. Prefer `default_extra_body`
regardless — it is the dedicated mechanism for values you want to persist
across every call.

Note also that switching `enable_thinking` mid-session changes the rendered
chat-template prefix on servers like vLLM, which can invalidate that
Expand Down
54 changes: 52 additions & 2 deletions mellea/backends/model_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,57 @@ def remove_special_keys(model_options: dict[str, Any]) -> dict[str, Any]:
new_options[k] = v
return new_options

@staticmethod
def _merge_extra_body(
base: dict[str, Any], overwrite: dict[str, Any]
) -> dict[str, Any]:
"""Merge two `extra_body` dicts, deep-merging their `chat_template_kwargs`.

Every other key is a flat overwrite, matching `merge_model_options`. If
either side's `chat_template_kwargs` is present but not a dict, the
deep-merge is skipped and `overwrite`'s value wins when present, else
`base`'s.

Args:
base (dict[str, Any]): Lower-precedence `extra_body` dict.
overwrite (dict[str, Any]): Higher-precedence `extra_body` dict.

Returns:
dict[str, Any]: A new merged `extra_body` dict.
"""
merged = dict(base)
base_ctk = merged.pop("chat_template_kwargs", None)

overwrite = dict(overwrite)
overwrite_ctk = overwrite.pop("chat_template_kwargs", None)

merged.update(overwrite)

base_ctk_ok = base_ctk is None or isinstance(base_ctk, dict)
overwrite_ctk_ok = overwrite_ctk is None or isinstance(overwrite_ctk, dict)

if base_ctk_ok and overwrite_ctk_ok:
if base_ctk is not None or overwrite_ctk is not None:
merged["chat_template_kwargs"] = {
**(base_ctk or {}),
**(overwrite_ctk or {}),
}
elif overwrite_ctk is not None:
merged["chat_template_kwargs"] = overwrite_ctk
elif base_ctk is not None:
merged["chat_template_kwargs"] = base_ctk
return merged

@staticmethod
def merge_model_options(
persistent_opts: dict[str, Any], overwrite_opts: dict[str, Any] | None
) -> dict[str, Any]:
"""Merge two model-options dicts, with `overwrite_opts` taking precedence on conflicts.

Creates a new dict that contains all keys and values from persistent opts and overwrite opts.
If there are duplicate keys, overwrite opts key value pairs will be used.
If there are duplicate keys, overwrite opts key value pairs will be used, except for
`extra_body`: when both sides have a dict there, their `chat_template_kwargs` sub-dicts
are deep-merged (see `_merge_extra_body`) instead of one replacing the other.

Args:
persistent_opts (dict[str, Any]): Base model options (lower precedence).
Expand All @@ -242,5 +285,12 @@ def merge_model_options(

if overwrite_opts is not None:
for k, v in overwrite_opts.items():
new_options[k] = v
if (
k == "extra_body"
and isinstance(v, dict)
and isinstance(new_options.get(k), dict)
):
new_options[k] = ModelOption._merge_extra_body(new_options[k], v)
else:
new_options[k] = v
Comment on lines +288 to +295

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For openai backends at least, I believe this causes construction-time enable_thinking to silently override per-call ModelOption.THINKING.

return new_options
83 changes: 83 additions & 0 deletions test/backends/test_model_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,88 @@ def test_model_option_merge():
assert expected_opts == processed_model_opts, "merged dict did not match expected"


def test_model_option_merge_extra_body_unrelated_key_preserves_default():
"""An unrelated per-call extra_body must not clobber a backend-level default."""
default_model_opts = {
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
}
overwrite_opts = {"extra_body": {"some_unrelated_field": 123}}

processed_model_opts = ModelOption.merge_model_options(
default_model_opts, overwrite_opts
)
assert processed_model_opts["extra_body"] == {
"chat_template_kwargs": {"enable_thinking": False},
"some_unrelated_field": 123,
}


def test_model_option_merge_extra_body_deep_merges_chat_template_kwargs():
"""A per-call chat_template_kwargs key must merge with, not replace, the default's."""
default_model_opts = {
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
}
overwrite_opts = {"extra_body": {"chat_template_kwargs": {"adapter_name": "foo"}}}

processed_model_opts = ModelOption.merge_model_options(
default_model_opts, overwrite_opts
)
assert processed_model_opts["extra_body"] == {
"chat_template_kwargs": {"enable_thinking": False, "adapter_name": "foo"}
}


def test_model_option_merge_extra_body_overwrite_wins_on_conflict():
"""Within chat_template_kwargs, the overwrite value wins on an actual key conflict."""
default_model_opts = {
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
}
overwrite_opts = {"extra_body": {"chat_template_kwargs": {"enable_thinking": True}}}

processed_model_opts = ModelOption.merge_model_options(
default_model_opts, overwrite_opts
)
assert processed_model_opts["extra_body"] == {
"chat_template_kwargs": {"enable_thinking": True}
}


def test_model_option_merge_extra_body_does_not_mutate_inputs():
"""Neither `base` nor `overwrite` dicts (nor their nested dicts) are modified."""
base = {"chat_template_kwargs": {"enable_thinking": False}}
overwrite = {"chat_template_kwargs": {"adapter_name": "answerability"}}

ModelOption._merge_extra_body(base, overwrite)

assert base == {"chat_template_kwargs": {"enable_thinking": False}}
assert overwrite == {"chat_template_kwargs": {"adapter_name": "answerability"}}


def test_model_option_merge_extra_body_non_dict_chat_template_kwargs_falls_back():
"""A malformed non-dict `chat_template_kwargs` on the base side must not raise."""
default_model_opts = {"extra_body": {"chat_template_kwargs": "not-a-dict"}}
overwrite_opts = {"extra_body": {"chat_template_kwargs": {"enable_thinking": True}}}

processed_model_opts = ModelOption.merge_model_options(
default_model_opts, overwrite_opts
)
assert processed_model_opts["extra_body"] == {
"chat_template_kwargs": {"enable_thinking": True}
}


def test_model_option_merge_extra_body_overwrite_non_dict_chat_template_kwargs_wins():
"""A malformed non-dict `chat_template_kwargs` on the overwrite side must not raise."""
default_model_opts = {
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
}
overwrite_opts = {"extra_body": {"chat_template_kwargs": "not-a-dict"}}

processed_model_opts = ModelOption.merge_model_options(
default_model_opts, overwrite_opts
)
assert processed_model_opts["extra_body"] == {"chat_template_kwargs": "not-a-dict"}


if __name__ == "__main__":
pytest.main([__file__])
39 changes: 39 additions & 0 deletions test/backends/test_openai_intrinsics_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,45 @@ async def test_chat_template_kwargs_set():
assert extra_body["chat_template_kwargs"]["adapter_name"] == "answerability"


async def test_construction_time_extra_body_default_survives_intrinsic_call():
"""A construction-time extra_body default must survive an intrinsic call
that supplies its own unrelated per-call extra_body, alongside the
adapter's own chat_template_kwargs.adapter_name write."""
backend = _make_backend_with_adapter(
_SIMPLE_CONFIG,
model_options={
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
},
)
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": {"some_unrelated_field": 123}},
)
await mot.avalue()

mock_create.assert_called_once()
extra_body = mock_create.call_args.kwargs.get("extra_body", {})

assert extra_body["some_unrelated_field"] == 123
assert extra_body["chat_template_kwargs"]["enable_thinking"] is False
assert extra_body["chat_template_kwargs"]["adapter_name"] == "answerability"


async def test_result_processor_applied():
"""Full answerability config: likelihood + nest transforms produce the expected JSON."""
backend = _make_backend_with_adapter(_ANSWERABILITY_CONFIG)
Expand Down
16 changes: 16 additions & 0 deletions test/backends/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,19 @@ def test_resolve_model_options_unrelated_keys_are_preserved():
call_options={"call_only": 3},
)
assert resolved == {"backend_only": 1, "helper_only": 2, "call_only": 3}


def test_resolve_model_options_extra_body_default_survives_unrelated_call_extra_body():
"""A backend-level extra_body default must survive an unrelated per-call
extra_body (e.g. an intrinsic/adapter routing call)."""
resolved = resolve_model_options(
backend_defaults={
"extra_body": {"chat_template_kwargs": {"enable_thinking": False}}
},
remap={},
call_options={"extra_body": {"some_unrelated_field": 123}},
)
assert resolved["extra_body"] == {
"chat_template_kwargs": {"enable_thinking": False},
"some_unrelated_field": 123,
}
Loading