From 3c0d60462e8b6479631f07cb75e273beba4c4048 Mon Sep 17 00:00:00 2001 From: rezaho Date: Sun, 13 Sep 2026 02:27:21 +0200 Subject: [PATCH 1/2] Ask the OpenAI legs for a reasoning summary On the Responses API a reasoning summary is returned only when the request carries `reasoning.summary`, and this builder has only ever set the effort. So every gpt-5 deployment on the OpenAI and Azure legs has been reasoning, and being billed for reasoning tokens, while streaming no reasoning text at all. Measured over eleven live streaming requests: the request as built before this commit drew zero summary parts on both control sends while the deployment billed 148 and 244 reasoning tokens, and every send that asked streamed a heading, a run of text deltas and two done events, all before the reply's first text delta. The field is gated on the same two facts as the prompt-cache fields beside it, and in the same shape: a provider set, a module-level model predicate, and one adapter method that reads the provider the factory stamped with the class-name fallback. The provider gate is not redundant with the model one, because the factory routes every unrecognized provider to this class. The model gate is closed at the gpt-5 generation, which is the whole of what the two provider pages state. The summary rides the `reasoning` object the effort creates and never creates it, so a caller who turned thinking off is never told to show its thinking. A `reasoning_summary` kwarg wins over the gate in both directions, with `False` as the off word, and is added to the parameter allow-list that drops it today. The OAuth leg is untouched: it speaks to the ChatGPT backend, which neither provider page covers, and its summaries already stream. Second, a defect on the read side: `harmonize_response` joined a reasoning item's parts with `str()`, and the wire part is an object, so the first real summary would have put a Python dict repr where the model's own words belong. One small part reader now serves both the summary and the content lists, and plain string parts still read as they did. --- src/marsys/models/adapters/openai.py | 103 +++++- tests/models/test_adapter_streaming.py | 5 +- tests/models/test_azure_openai_leg.py | 31 +- tests/models/test_openai_minimal_effort.py | 6 + tests/models/test_openai_reasoning_summary.py | 320 ++++++++++++++++++ 5 files changed, 452 insertions(+), 13 deletions(-) create mode 100644 tests/models/test_openai_reasoning_summary.py diff --git a/src/marsys/models/adapters/openai.py b/src/marsys/models/adapters/openai.py index 1ae5fa59..6a3e2451 100644 --- a/src/marsys/models/adapters/openai.py +++ b/src/marsys/models/adapters/openai.py @@ -99,6 +99,29 @@ # content. _BREAKPOINT_ITEM_ROLES = frozenset({"system", "developer", "user"}) + +# --- reasoning summaries ------------------------------------------------------------ +# +# The generation documented to serve a provider-authored reasoning summary. Azure's +# reasoning page marks the field on all twenty-one gpt-5.x deployments, carries no such +# row at all for the gpt-6 family, and splits the o-series three-for-six; only the +# gpt-5 generation is written down here because only it is uniformly documented, and +# opening it upward is a measurement rather than a guess. +_REASONING_SUMMARY_GENERATION = 5 + +# The providers whose endpoints were measured to serve the field, on the same reasoning +# as the prompt-cache set above: the factory routes an unrecognized provider to this +# adapter, so a third-party OpenAI-compatible endpoint behind a gpt-5-shaped model name +# would otherwise be asked for a field nobody has asked it for. +_REASONING_SUMMARY_PROVIDERS = frozenset({"openai", "azure"}) + +# The word sent when a served leg says nothing else. Measured against a live Azure +# resource over eleven streaming requests: on one deployment `detailed` came back with a +# summary three times out of three and `auto` twice out of four, at no measurable +# difference in reasoning or output tokens. Arrival is not guaranteed either way, which +# the provider documents, so a request that draws no summary is not a fault. +_DEFAULT_REASONING_SUMMARY = "detailed" + _GENERATION_RE = re.compile(r"^gpt-(\d+)(?:\.(\d+))?") @@ -265,6 +288,47 @@ def supports_explicit_prompt_cache(model_lower: str) -> bool: return generation is not None and generation >= _EXPLICIT_PROMPT_CACHE_MIN_VERSION +def supports_reasoning_summary(model_lower: str) -> bool: + """Whether a model name is one the provider documents as serving a summary. + + Closed at the gpt-5 generation, which is the whole of what the two provider pages + state: every gpt-5.x deployment is marked, the gpt-6 family has no such row, and the + o-series is marked on three of six names this regex cannot match anyway. + + Reads the name the way the caching rule beside it does, and fails in the one + direction that is worth naming here. On Azure the name is an operator-chosen + deployment label, so a deployment renamed away from the model it serves stops being + asked for a summary, and the person watching then sees the generic cue for every + turn, forever, with no error anywhere to read. The pilots' deployments are named + `gpt-5.6-sol` and `gpt-5.6-terra`, which this matches. The other direction is loud + and cheap by comparison: a name shaped like the generation on a leg that does not + serve the field takes a 400 on its first call. + """ + match = _GENERATION_RE.match(model_lower or "") + if not match: + return False + return int(match.group(1)) == _REASONING_SUMMARY_GENERATION + + +def _reasoning_parts_text(parts: List[Any]) -> str: + """A reasoning item's ``summary`` or ``content`` list read as text, one part a line. + + The wire part is an object, ``{"type": "summary_text", "text": ...}`` on a summary + and ``{"type": "reasoning_text", "text": ...}`` on the content list, so rendering a + part with ``str()`` puts a Python dict repr where the model's own words belong. A + plain string part is its own text and stays readable, which is the shape older + fixtures and re-hosted surfaces still hand over. + """ + texts = [] + for part in parts: + if not part: + continue + text = part.get("text") if isinstance(part, dict) else part + if text: + texts.append(str(text)) + return "\n".join(texts) + + def _blocks_with_breakpoint( blocks: List[Any], ) -> Optional[List[Any]]: @@ -667,13 +731,20 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An # Handle OpenAI reasoning (effort-based for all models via Responses API). # An explicit `reasoning_effort` wins; failing that, a caller's thinking budget # selects the bucket, so the one knob this stack exposes reaches this leg too. + # The summary rides this object and never creates it: a request that asks for no + # thinking must not be told to show its thinking, and there is one creation site + # for `reasoning` on this builder. reasoning_effort = kwargs.get("reasoning_effort") if not reasoning_effort: reasoning_effort = thinking_budget_to_effort(kwargs.get("thinking_budget")) if reasoning_effort and reasoning_effort.lower() in ["minimal", "low", "medium", "high"]: - payload["reasoning"] = { + reasoning = { "effort": self._served_effort(reasoning_effort.lower(), model_lower) } + summary = self._reasoning_summary(model_lower, kwargs.get("reasoning_summary")) + if summary: + reasoning["summary"] = summary + payload["reasoning"] = reasoning if kwargs.get("prompt_cache_key") is not None: payload["prompt_cache_key"] = kwargs["prompt_cache_key"] @@ -722,6 +793,7 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An "tool_choice", "parallel_tool_calls", "reasoning_effort", # Converted to reasoning.effort + "reasoning_summary", # Converted to reasoning.summary # Streaming and logging "stream", "stream_options", @@ -770,6 +842,31 @@ def _served_effort(self, effort: str, model_lower: str) -> str: """Allow re-hosted surfaces to override the shared model compatibility rule.""" return served_reasoning_effort(effort, model_lower) + def _reasoning_summary(self, model_lower: str, requested: Any) -> Optional[str]: + """The summary word this request carries, or ``None`` for no ``summary`` field. + + The caller's three states and the gate resolve together here, so the payload + sets the effort and the summary side by side and decides nothing in place. + + An explicit ``reasoning_summary`` wins in both directions, because the gate + exists to protect the default from legs nobody measured and a caller who names a + word knows their own leg: a string is passed through untouched, and ``False`` + sends nothing at all while leaving the effort exactly as it was. Absent, the + gate answers: the provider the factory stamped (falling back to this class's own + name, the way the caching gate beside it does), then the documented generation. + Returns the value rather than a verdict, following ``_served_effort``, so a + re-hosted surface whose table splits inside the generation overrides one method + instead of growing a second conditional in the builder. + """ + if requested is not None: + return requested or None + provider = getattr(self, "provider", None) or self._provider_name() + if provider not in _REASONING_SUMMARY_PROVIDERS: + return None + if not supports_reasoning_summary(model_lower): + return None + return _DEFAULT_REASONING_SUMMARY + def _supports_explicit_prompt_cache(self, model_lower: str) -> bool: """Whether this request may carry the explicit prompt-cache fields. @@ -890,9 +987,9 @@ def harmonize_response( # Prefer summary (key insights) over detailed content if summary: - reasoning_data = "\n".join(str(s) for s in summary if s) + reasoning_data = _reasoning_parts_text(summary) elif content_array: - reasoning_data = "\n".join(str(c) for c in content_array if c) + reasoning_data = _reasoning_parts_text(content_array) else: reasoning_data = None diff --git a/tests/models/test_adapter_streaming.py b/tests/models/test_adapter_streaming.py index 863628b5..5a90f69a 100644 --- a/tests/models/test_adapter_streaming.py +++ b/tests/models/test_adapter_streaming.py @@ -348,7 +348,10 @@ def test_foreign_reasoning_details_are_not_emitted_as_anthropic_blocks(): {"type": "response.completed", "response": { "id": "resp_1", "model": "gpt-test", "output": [ - {"type": "reasoning", "content": [], "summary": ["Weighing options."]}, + # The wire shape: a summary is a list of objects, not of strings, which is + # what the terminal object carries once the request asks for one. + {"type": "reasoning", "content": [], + "summary": [{"type": "summary_text", "text": "Weighing options."}]}, {"type": "message", "role": "assistant", "status": "completed", "content": [{"type": "output_text", "text": "Hello world."}]}, ], diff --git a/tests/models/test_azure_openai_leg.py b/tests/models/test_azure_openai_leg.py index 60b7fa71..af629ce2 100644 --- a/tests/models/test_azure_openai_leg.py +++ b/tests/models/test_azure_openai_leg.py @@ -323,16 +323,19 @@ def test_a_thinking_budget_selects_a_reasoning_effort(budget, expected): def test_the_configured_thinking_budget_reaches_this_leg(): """The only deliberation knob this stack exposes is a token budget. Unmapped, a caller's setting is inert and every call runs at the provider default (`medium`), - which is indistinguishable from the knob working.""" + which is indistinguishable from the knob working. + + The object carries a summary beside the effort because this deployment's generation + is documented to serve one and the request is the only place it can be asked for.""" payload = _azure().format_request_payload(MESSAGES, thinking_budget=32768) - assert payload["reasoning"] == {"effort": "high"} + assert payload["reasoning"] == {"effort": "high", "summary": "detailed"} def test_an_explicit_effort_beats_the_budget(): payload = _azure().format_request_payload( MESSAGES, thinking_budget=32768, reasoning_effort="low" ) - assert payload["reasoning"] == {"effort": "low"} + assert payload["reasoning"] == {"effort": "low", "summary": "detailed"} def test_thinking_off_sends_no_reasoning_block(): @@ -350,31 +353,41 @@ def test_the_smallest_budget_asks_for_an_effort_this_surface_actually_serves(): """`minimal` is a 400 here — the endpoint's own reply lists none / low / medium / high / xhigh / max — so the smallest bucket has to arrive as the smallest this surface has. It must still ask for reasoning: a positive budget is "think a little", - and `none` would answer a question nobody asked.""" + and `none` would answer a question nobody asked. + + The summary rides along for the same reason as above; the two fields are resolved + independently and the effort substitution is what this case is about.""" payload = _azure().format_request_payload(MESSAGES, thinking_budget=512) - assert payload["reasoning"] == {"effort": "low"} + assert payload["reasoning"] == {"effort": "low", "summary": "detailed"} def test_an_explicit_minimal_is_substituted_too(): """The caller who names the effort outright is on the same endpoint as the one who named a budget, and it rejects the value for both of them.""" payload = _azure().format_request_payload(MESSAGES, reasoning_effort="minimal") - assert payload["reasoning"] == {"effort": "low"} + assert payload["reasoning"] == {"effort": "low", "summary": "detailed"} def test_the_first_party_leg_still_sends_minimal(): """The control, and the scope line: `minimal` is served by OpenAI's own endpoint and the substitution above belongs to this re-hosting surface, not to the shared payload builder. A run of this file that changed the first-party leg would be a silent change - to every OpenAI caller in the stack.""" + to every OpenAI caller in the stack. + + Both names carry a summary: the gate is the model generation and the provider, and a + hand-built `OpenAIAdapter` names OpenAI's own endpoint by its class. `gpt-5.6-codex` + is a fixture name here rather than a deployment either provider publishes, kept + because it is the one case that exercises the codex effort exception; it pins the + exception and the new default together on one row.""" payload = OpenAIAdapter( model_name="gpt-5.6", api_key="k", base_url="https://api.openai.com/v1" ).format_request_payload(MESSAGES, thinking_budget=512) - assert payload["reasoning"] == {"effort": "minimal"} + assert payload["reasoning"] == {"effort": "minimal", "summary": "detailed"} codex = OpenAIAdapter( model_name="gpt-5.6-codex", api_key="k", base_url="https://api.openai.com/v1" ).format_request_payload(MESSAGES, thinking_budget=512) - assert codex["reasoning"] == {"effort": "low"} # the pre-existing codex exception + # the pre-existing codex effort exception, beside the summary the generation serves + assert codex["reasoning"] == {"effort": "low", "summary": "detailed"} # --- the meter --------------------------------------------------------------- diff --git a/tests/models/test_openai_minimal_effort.py b/tests/models/test_openai_minimal_effort.py index 78892048..c6f1c6aa 100644 --- a/tests/models/test_openai_minimal_effort.py +++ b/tests/models/test_openai_minimal_effort.py @@ -91,9 +91,15 @@ def post(url, *, json, **kwargs): payload = captured[0] if expected == "minimal" and model_name != "gpt-5": expected = "low" + # The OAuth leg's object exists with or without an effort and always says `auto`. + # The api-key leg's object exists only when an effort resolved, and then carries the + # summary its model generation is documented to serve; every model name here is that + # generation, so the two rows that resolve no effort still send nothing at all. reasoning = {"summary": "auto"} if oauth else {} if expected is not None: reasoning["effort"] = expected + if not oauth: + reasoning["summary"] = "detailed" assert payload.get("reasoning", {}) == reasoning assert payload["prompt_cache_key"] == "install:owner" if oauth: diff --git a/tests/models/test_openai_reasoning_summary.py b/tests/models/test_openai_reasoning_summary.py new file mode 100644 index 00000000..2f444a82 --- /dev/null +++ b/tests/models/test_openai_reasoning_summary.py @@ -0,0 +1,320 @@ +"""The OpenAI-family reasoning-summary contract: who is asked, and who asks back. + +On the Responses API a reasoning summary is returned only when the request carries +``reasoning.summary``. Measured against a live Azure resource over eleven streaming +requests: the request the adapter built before this contract existed drew zero summary +parts on both control sends while the deployment billed 148 and 244 reasoning tokens, +and every send that asked streamed a bold heading, a run of text deltas and two done +events, all before the reply's first text delta. So the model was thinking and being +paid for out loud, and nobody could see it, because nothing asked. + +The field is gated on the same two facts as the prompt-cache fields beside it, and for +the same reasons. The provider: the factory routes every unrecognized provider to this +adapter, so a third-party OpenAI-compatible endpoint would otherwise be handed a field +only OpenAI's and Azure's surfaces have been measured to take. The model: Azure's +reasoning page marks the summary on every gpt-5.x deployment, carries no such row for +the gpt-6 family, and marks three of six o-series names. + +Two rules that are easy to lose and expensive to rediscover, so both are pinned here. +The summary rides the ``reasoning`` object the effort creates and never creates it, so a +caller who turned thinking off is never told to show its thinking. And an explicit +``reasoning_summary`` wins in both directions, because the gate protects the default +from legs nobody measured while a caller who names a word owns the answer. + +No network anywhere in this file. +""" + +import warnings + +import pytest + +from marsys.models.adapters.azure import AsyncAzureOpenAIAdapter, AzureOpenAIAdapter +from marsys.models.adapters.factory import ProviderAdapterFactory +from marsys.models.adapters.openai import ( + AsyncOpenAIAdapter, + OpenAIAdapter, + supports_reasoning_summary, +) +from marsys.models.models import BaseAPIModel + +MESSAGES = [{"role": "user", "content": "hi"}] + +# The deployments the two provider pages publish for the generation the gate serves. +SERVED = [ + "gpt-5", "gpt-5.4-mini", "gpt-5.5", "gpt-5.6", + "gpt-5.6-terra", "gpt-5.6-sol", "gpt-5.6-luna", +] + +# Outside the generation, by three different routes: a newer family with no documented +# row, an older non-reasoning model, two o-series names this generation reader cannot +# match at all, and a name that is not a model. +NOT_SERVED = ["gpt-6-astra", "gpt-4o", "o3-mini", "o4-mini", "not-a-gpt-model"] + +# The four builders that inherit the OpenAI payload builder. The OAuth pair is the +# unchanged control and lives in its own files; it speaks to the ChatGPT backend, hangs +# its `reasoning` object off no effort at all, and this session does not touch it. +INHERITING = [OpenAIAdapter, AsyncOpenAIAdapter, AzureOpenAIAdapter, AsyncAzureOpenAIAdapter] + + +def _make(adapter_type, model_name, provider=None): + adapter = adapter_type( + model_name=model_name, api_key="not-a-real-key", + base_url="https://example.invalid/openai/v1", max_tokens=1024, + ) + if provider is not None: + adapter.provider = provider + return adapter + + +@pytest.fixture(params=INHERITING) +def builder(request): + return request.param + + +# --- the served legs ---------------------------------------------------------------- + + +@pytest.mark.parametrize("provider", ["openai", "azure"]) +@pytest.mark.parametrize("model", SERVED) +@pytest.mark.parametrize( + "configured", [{"reasoning_effort": "medium"}, {"thinking_budget": 8192}], + ids=["effort", "budget"], +) +def test_a_served_leg_asks_for_a_detailed_summary(builder, provider, model, configured): + """Either route to an effort brings the summary with it, and the object carries the + two fields and nothing else.""" + adapter = _make(builder, model, provider=provider) + payload = adapter.format_request_payload(MESSAGES, **configured) + assert payload["reasoning"] == { + "effort": adapter._served_effort( + configured.get("reasoning_effort", "medium"), model.lower() + ), + "summary": "detailed", + } + + +@pytest.mark.parametrize("model", SERVED + NOT_SERVED) +def test_the_generation_predicate_reads_the_two_published_tables(model): + """The rule on its own, away from any adapter: the gpt-5 generation and nothing + else.""" + assert supports_reasoning_summary(model) is (model in SERVED) + + +# --- no effort, no object ----------------------------------------------------------- + + +@pytest.mark.parametrize( + "configured", + [{}, {"thinking_budget": 0}, {"thinking_budget": -1}, {"reasoning_effort": "xhigh"}], + ids=["nothing", "budget-zero", "budget-negative", "unserved-word"], +) +def test_a_request_that_asks_for_no_thinking_is_never_told_to_show_it(builder, configured): + """The summary rides the object the effort creates and never creates it. This stack's + thinking switch is enforced by sending no effort at all, so a summary that could + stand on its own would draw thinking frames for a switch the operator turned off.""" + payload = _make(builder, "gpt-5.6-terra", provider="azure").format_request_payload( + MESSAGES, reasoning_summary="detailed", **configured + ) + assert "reasoning" not in payload + + +# --- outside the gate --------------------------------------------------------------- + + +@pytest.mark.parametrize("provider", ["openai", "azure"]) +@pytest.mark.parametrize("model", NOT_SERVED) +def test_a_model_outside_the_generation_is_asked_for_nothing(builder, provider, model): + """Unknown rather than unsupported, in every case here, and an unsupported value on a + known field is a 400 on this surface rather than a field quietly ignored.""" + adapter = _make(builder, model, provider=provider) + payload = adapter.format_request_payload(MESSAGES, reasoning_effort="medium") + assert payload["reasoning"] == {"effort": adapter._served_effort("medium", model)} + + +@pytest.mark.parametrize("provider", ["groq", "together", "some-new-gateway"]) +def test_an_unknown_provider_routed_to_this_builder_is_asked_for_nothing(provider): + """``factory.py`` hands every unrecognized provider to the OpenAI adapter, so the + model gate alone would let a gpt-5.6-shaped name on a foreign gateway receive a field + that gateway has never been asked for.""" + adapter = ProviderAdapterFactory.create_adapter( + provider=provider, model_name="gpt-5.6-terra", + api_key="not-a-real-key", base_url="https://example.invalid/v1", + ) + assert isinstance(adapter, OpenAIAdapter) + payload = adapter.format_request_payload(MESSAGES, reasoning_effort="medium") + assert payload["reasoning"] == {"effort": "medium"} + + +@pytest.mark.parametrize("provider", ["xai", "groq", "some-new-gateway"]) +def test_a_stamped_provider_beats_the_class_name(builder, provider): + """The stamp outranks class identity, which is the whole reason routing unrecognized + providers to this builder is safe.""" + payload = _make(builder, "gpt-5.6-terra", provider=provider).format_request_payload( + MESSAGES, reasoning_effort="medium" + ) + assert "summary" not in payload["reasoning"] + + +def test_a_hand_built_adapter_is_taken_at_its_class_name(builder): + """Nothing stamped, so the class names the surface: building ``OpenAIAdapter`` by + hand says the request is bound for OpenAI's own endpoint and ``AzureOpenAIAdapter`` + pins `azure`. Both are legs the field was measured on.""" + adapter = _make(builder, "gpt-5.6-terra") + assert getattr(adapter, "provider", None) is None + payload = adapter.format_request_payload(MESSAGES, reasoning_effort="medium") + assert payload["reasoning"] == {"effort": "medium", "summary": "detailed"} + + +# --- the caller's word -------------------------------------------------------------- + + +@pytest.mark.parametrize("word", ["auto", "concise"]) +def test_a_named_word_on_a_served_leg_is_sent_as_given(builder, word): + """The gate decides the default and never rewrites a caller. There is no served + substitute to rewrite a summary word into, the way there is for an effort, so the + caller who names one owns the reply.""" + payload = _make(builder, "gpt-5.6-terra", provider="azure").format_request_payload( + MESSAGES, reasoning_effort="medium", reasoning_summary=word + ) + assert payload["reasoning"] == {"effort": "medium", "summary": word} + + +@pytest.mark.parametrize("model, provider", [("gpt-6-astra", "azure"), ("gpt-5.6-terra", "groq")]) +def test_a_named_word_off_the_gate_is_sent_too(builder, model, provider): + """The other direction. The gate protects the default from legs nobody measured; a + caller who has measured one says so and is believed.""" + adapter = _make(builder, model, provider=provider) + payload = adapter.format_request_payload( + MESSAGES, reasoning_effort="medium", reasoning_summary="detailed" + ) + assert payload["reasoning"] == { + "effort": adapter._served_effort("medium", model), "summary": "detailed", + } + + +def test_the_off_word_sends_no_summary_and_leaves_the_effort_alone(builder): + """``False`` rather than a string: `None` cannot mean off, because the parameter + allow-list reads a `None` value as "not passed", and `none` is already an effort word + on the Azure surface.""" + payload = _make(builder, "gpt-5.6-terra", provider="azure").format_request_payload( + MESSAGES, reasoning_effort="high", reasoning_summary=False + ) + assert payload["reasoning"] == {"effort": "high"} + + +@pytest.mark.parametrize("value", ["auto", "concise", "detailed", False]) +def test_the_summary_kwarg_does_not_warn_as_unknown(builder, value): + """The allow-list drops any parameter it does not name, with a warning, and it + treated this one as unknown until this contract existed. `False` is the case that + matters: the warning fires on any value that is not `None`.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + _make(builder, "gpt-5.6-terra", provider="azure").format_request_payload( + MESSAGES, reasoning_effort="medium", reasoning_summary=value + ) + + +# --- through the model layer -------------------------------------------------------- + + +@pytest.mark.parametrize("asynchronous", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("provider", ["openai", "azure"]) +async def test_the_summary_reaches_the_wire_through_the_model_layer( + monkeypatch, provider, asynchronous +): + """The payload the transport is handed, not the one an adapter was asked for by hand, + and built twice to pin that the request is a function of its input alone.""" + from unittest.mock import AsyncMock, MagicMock + + raw_response = { + "output": [{"type": "message", "content": [{"type": "output_text", "text": "ok"}]}], + "usage": {}, + } + model = BaseAPIModel( + model_name="gpt-5.6-terra", provider=provider, api_key="fake-key", + base_url="https://example.invalid/openai/v1", reasoning_effort="medium", + ) + captured = [] + + def post(url, *, json, **kwargs): + captured.append(json) + response = MagicMock(status_code=200, status=200) + response.raise_for_status.return_value = None + response.json.return_value = raw_response + async_response = MagicMock(status=200) + async_response.raise_for_status.return_value = None + async_response.json = AsyncMock(return_value=raw_response) + response.__aenter__ = AsyncMock(return_value=async_response) + response.__aexit__ = AsyncMock(return_value=False) + return response + + monkeypatch.setattr("marsys.models.adapters.base.requests.post", post) + session = MagicMock() + session.post.side_effect = post + monkeypatch.setattr(model.async_adapter, "_ensure_session", AsyncMock(return_value=session)) + + for _ in range(2): + if asynchronous: + await model.arun(MESSAGES) + else: + model.run(MESSAGES) + + assert len(captured) == 2 + assert captured[0]["reasoning"] == {"effort": "medium", "summary": "detailed"} + assert captured[0] == captured[1] + + +# --- reading the answer back -------------------------------------------------------- + + +def _harmonized(reasoning_item): + raw = { + "id": "resp_1", "model": "gpt-5.6-terra", "created_at": 1, "status": "completed", + "output": [ + reasoning_item, + {"type": "message", "role": "assistant", "status": "completed", + "content": [{"type": "output_text", "text": "ok"}]}, + ], + "usage": {}, + } + return _make(OpenAIAdapter, "gpt-5.6-terra").harmonize_response(raw, request_start_time=0.0) + + +def test_the_summary_parts_read_as_the_models_own_words(): + """The parts arrive as objects, and rendering an object with `str()` writes a Python + dict repr where the words belong. Nobody had seen it because nothing had ever asked + for a summary, so every fixture in the tree fed plain strings.""" + resp = _harmonized({ + "type": "reasoning", "content": [], + "summary": [ + {"type": "summary_text", "text": "**Weighing options**\n\nFirst the crates."}, + {"type": "summary_text", "text": "**Checking the load**\n\nThen the trolley."}, + ], + }) + assert resp.reasoning == ( + "**Weighing options**\n\nFirst the crates.\n" + "**Checking the load**\n\nThen the trolley." + ) + assert "{" not in resp.reasoning + assert "'type'" not in resp.reasoning + + +def test_a_plain_string_part_still_reads_as_it_did(): + """Re-hosted surfaces and older fixtures hand over strings; both shapes are read.""" + resp = _harmonized({"type": "reasoning", "content": [], "summary": ["Weighing options."]}) + assert resp.reasoning == "Weighing options." + + +def test_an_empty_summary_falls_through_to_the_content_blocks(): + """The preference of summary over content is unchanged; the content list carries the + same object shape under a different type name and is read the same way.""" + resp = _harmonized({ + "type": "reasoning", "summary": [], + "content": [{"type": "reasoning_text", "text": "The long form."}], + }) + assert resp.reasoning == "The long form." + + +def test_a_reasoning_item_with_neither_reads_as_nothing(): + resp = _harmonized({"type": "reasoning", "summary": [], "content": []}) + assert resp.reasoning is None From 43ff7535b91f1e19003a290831c8c2b8d1d41f74 Mon Sep 17 00:00:00 2001 From: rezaho Date: Sun, 13 Sep 2026 03:25:05 +0200 Subject: [PATCH 2/2] Read the model generation once for both gates The summary gate parsed the generation with its own copy of the regex read, which the prompt-cache gate above it already did. One module-level reader now answers both, so the two gates differ in their floor and in nothing else. Behaviour is unchanged for every input: the reader returns the same pair the inline parses built, and the suite pins both gates over the served and unserved names. The tests that expected a served effort asked the adapter for it, which let the effort half of those rows agree with whatever the code did. Each row now writes its expected effort out. The accumulator's terminal capture asserted only the response id, so a mangled summary part would have passed it. It reads the rebuilt response back through the harmonizer and asserts the words. --- src/marsys/models/adapters/openai.py | 6 ++-- tests/models/test_adapter_streaming.py | 10 +++++- tests/models/test_azure_openai_leg.py | 7 +++- tests/models/test_openai_reasoning_summary.py | 36 +++++++++++-------- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/marsys/models/adapters/openai.py b/src/marsys/models/adapters/openai.py index 6a3e2451..bc66bf73 100644 --- a/src/marsys/models/adapters/openai.py +++ b/src/marsys/models/adapters/openai.py @@ -304,10 +304,8 @@ def supports_reasoning_summary(model_lower: str) -> bool: and cheap by comparison: a name shaped like the generation on a leg that does not serve the field takes a 400 on its first call. """ - match = _GENERATION_RE.match(model_lower or "") - if not match: - return False - return int(match.group(1)) == _REASONING_SUMMARY_GENERATION + generation = _generation(model_lower) + return generation is not None and generation[0] == _REASONING_SUMMARY_GENERATION def _reasoning_parts_text(parts: List[Any]) -> str: diff --git a/tests/models/test_adapter_streaming.py b/tests/models/test_adapter_streaming.py index 5a90f69a..38896ab8 100644 --- a/tests/models/test_adapter_streaming.py +++ b/tests/models/test_adapter_streaming.py @@ -371,7 +371,15 @@ def test_responses_accumulator_taps_and_captures_the_terminal_object(): ("text_delta", "Hello "), ("text_delta", "world."), ] - assert acc.to_rest_response()["id"] == "resp_1" + rest = acc.to_rest_response() + assert rest["id"] == "resp_1" + # The rebuilt REST shape carries the terminal reasoning item as it arrived, so it is + # read back through the same harmonizer the non-streaming path uses: an accumulator + # that dropped or flattened the summary part would hand over a dict repr here. + harmonized = AsyncOpenAIAdapter( + model_name="gpt-test", api_key="k", base_url="https://api.openai.com/v1", + ).harmonize_response(rest, request_start_time=0.0) + assert harmonized.reasoning == "Weighing options." def test_responses_failed_event_is_terminal(): diff --git a/tests/models/test_azure_openai_leg.py b/tests/models/test_azure_openai_leg.py index af629ce2..1f5ae7cf 100644 --- a/tests/models/test_azure_openai_leg.py +++ b/tests/models/test_azure_openai_leg.py @@ -332,6 +332,8 @@ def test_the_configured_thinking_budget_reaches_this_leg(): def test_an_explicit_effort_beats_the_budget(): + """The object carries a summary beside the effort for the same reason as above: this + deployment's generation is documented to serve one, whichever route set the effort.""" payload = _azure().format_request_payload( MESSAGES, thinking_budget=32768, reasoning_effort="low" ) @@ -363,7 +365,10 @@ def test_the_smallest_budget_asks_for_an_effort_this_surface_actually_serves(): def test_an_explicit_minimal_is_substituted_too(): """The caller who names the effort outright is on the same endpoint as the one who - named a budget, and it rejects the value for both of them.""" + named a budget, and it rejects the value for both of them. + + The summary rides along here too, resolved independently of the substitution this + case is about.""" payload = _azure().format_request_payload(MESSAGES, reasoning_effort="minimal") assert payload["reasoning"] == {"effort": "low", "summary": "detailed"} diff --git a/tests/models/test_openai_reasoning_summary.py b/tests/models/test_openai_reasoning_summary.py index 2f444a82..433a6549 100644 --- a/tests/models/test_openai_reasoning_summary.py +++ b/tests/models/test_openai_reasoning_summary.py @@ -77,20 +77,22 @@ def builder(request): @pytest.mark.parametrize("provider", ["openai", "azure"]) @pytest.mark.parametrize("model", SERVED) @pytest.mark.parametrize( - "configured", [{"reasoning_effort": "medium"}, {"thinking_budget": 8192}], + "configured, served_effort", + [({"reasoning_effort": "medium"}, "medium"), ({"thinking_budget": 8192}, "medium")], ids=["effort", "budget"], ) -def test_a_served_leg_asks_for_a_detailed_summary(builder, provider, model, configured): +def test_a_served_leg_asks_for_a_detailed_summary( + builder, provider, model, configured, served_effort +): """Either route to an effort brings the summary with it, and the object carries the - two fields and nothing else.""" + two fields and nothing else. + + The effort each row expects is written out rather than computed, so a substitution + that went wrong fails here instead of agreeing with itself. Nothing on these rows is + substituted: only `minimal` ever is, on either surface.""" adapter = _make(builder, model, provider=provider) payload = adapter.format_request_payload(MESSAGES, **configured) - assert payload["reasoning"] == { - "effort": adapter._served_effort( - configured.get("reasoning_effort", "medium"), model.lower() - ), - "summary": "detailed", - } + assert payload["reasoning"] == {"effort": served_effort, "summary": "detailed"} @pytest.mark.parametrize("model", SERVED + NOT_SERVED) @@ -125,10 +127,13 @@ def test_a_request_that_asks_for_no_thinking_is_never_told_to_show_it(builder, c @pytest.mark.parametrize("model", NOT_SERVED) def test_a_model_outside_the_generation_is_asked_for_nothing(builder, provider, model): """Unknown rather than unsupported, in every case here, and an unsupported value on a - known field is a 400 on this surface rather than a field quietly ignored.""" + known field is a 400 on this surface rather than a field quietly ignored. + + `medium` is served unchanged by every name and surface on these rows, so the effort is + written out rather than asked of the code under test.""" adapter = _make(builder, model, provider=provider) payload = adapter.format_request_payload(MESSAGES, reasoning_effort="medium") - assert payload["reasoning"] == {"effort": adapter._served_effort("medium", model)} + assert payload["reasoning"] == {"effort": "medium"} @pytest.mark.parametrize("provider", ["groq", "together", "some-new-gateway"]) @@ -182,14 +187,15 @@ def test_a_named_word_on_a_served_leg_is_sent_as_given(builder, word): @pytest.mark.parametrize("model, provider", [("gpt-6-astra", "azure"), ("gpt-5.6-terra", "groq")]) def test_a_named_word_off_the_gate_is_sent_too(builder, model, provider): """The other direction. The gate protects the default from legs nobody measured; a - caller who has measured one says so and is believed.""" + caller who has measured one says so and is believed. + + `medium` again arrives unsubstituted on both rows, so the expected effort is a + literal.""" adapter = _make(builder, model, provider=provider) payload = adapter.format_request_payload( MESSAGES, reasoning_effort="medium", reasoning_summary="detailed" ) - assert payload["reasoning"] == { - "effort": adapter._served_effort("medium", model), "summary": "detailed", - } + assert payload["reasoning"] == {"effort": "medium", "summary": "detailed"} def test_the_off_word_sends_no_summary_and_leaves_the_effort_alone(builder):