Ask the OpenAI legs for a reasoning summary - #64
Merged
Merged
Conversation
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.
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.
rezaho
force-pushed
the
reasoning-summary
branch
from
September 13, 2026 21:05
34d0d61 to
43ff753
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On the Responses API a reasoning summary is returned only when the request carries
reasoning.summary. This builder has only ever setreasoning.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. Everything downstream of the request was already built: the stream accumulator mapsresponse.reasoning_summary_text.deltato a thinking delta, and the consumer turns that into the live trace a person watches. The pipe was built and empty.The evidence
Eleven live streaming requests against an Azure OpenAI resource, on
gpt-5.6-terraandgpt-5.6-sol, 2026-09-12.The control rung is the request exactly as this builder made it before this branch: streaming on, no summary field. It produced zero summary parts on both runs while the deployment billed 148 and 244 reasoning tokens. Every rung that asked produced
response.reasoning_summary_part.added, then 81 to 113response.reasoning_summary_text.delta, then the two done events, all of it before the reply's firstresponse.output_text.delta. Every rung answered HTTP 200; none was refused. Reasoning tokens ran 155 to 316 with a summary against 148 to 244 without, and output tokens overlapped, so asking costs nothing measurable.detailedcame back with a summary three times out of three on terra;autotwice out of four on terra and twice out of two on sol. Hencedetailedas the default. Arrival is not guaranteed either way, which Azure documents in as many words ("Even when enabled, reasoning summaries are not guaranteed to be generated for every step/request"), so a request that draws no summary is not a fault.The two provider pages, both read 2026-09-13:
gpt-5.6-terra,gpt-5.6-solandgpt-5.6-luna; the o-series is marked on three of six; the GPT-6 table carries no such row at all.autodescribed as the most detailed summarizer available for a model, which is a promise about the ceiling rather than about arrival.The gate
Two facts, in the shape the prompt-cache fields beside them already use.
The provider,
_REASONING_SUMMARY_PROVIDERS = {"openai", "azure"}, because the factory routes every unrecognized provider to this class, so a third-party OpenAI-compatible endpoint behind a gpt-5-shaped model name would otherwise be handed a field nobody has asked it for. The model,supports_reasoning_summary(model_lower), closed at the gpt-5 generation because that is the whole of what the two pages state; opening it upward to a gpt-6 deployment is a measurement, not a guess. Both are read by one adapter method,_reasoning_summary(model_lower, requested), which returns the word rather than a verdict, following_served_effortnext door, so a re-hosted surface whose table splits inside the generation overrides one method instead of growing a second conditional in the builder.The predicate's docstring names the direction that bites on Azure: the model name there is an operator-chosen deployment label, so a deployment renamed away from the model it serves silently stops being asked and the person watching sees the generic cue forever with no error to read.
The default and the kwarg
The summary rides the
reasoningobject the effort creates and never creates it. A caller who sends no effort is asking for no thinking, and must not be told to show thinking it was not asked to do; there is also exactly one creation site for that object on this builder, and a second would be two paths for one field.reasoning_summaryis the kwarg, and it wins over the gate in both directions: a string is passed through untouched on any leg, andFalsesends no summary while leaving the resolved effort exactly as it was.Falserather than a string off word, because the parameter allow-list reads aNonevalue as "not passed", andnoneis already an effort word on the Azure surface. The kwarg is added to that allow-list besidereasoning_effort, which is the line that drops it today.Two comparables land on the same shape. The Vercel AI SDK's OpenAI Responses provider defaults
reasoningSummaryto'detailed'whenever a reasoning effort is set, withreasoningSummary: nullto omit it. OpenAI's own Codex CLI exposesmodel_reasoning_summarywithauto | concise | detailed | nonebesidemodel_reasoning_effort. Both ask by default or by one setting, and both keep an off word.The OAuth leg is untouched
OpenAIOAuthAdapterkeeps its hard-wired{"summary": "auto"}and its always-present reasoning object. It speaks to the ChatGPT backend, which neither provider page covers and which nobody has probed, and its summaries already stream through the same accumulator mapping. The two legs' existence rules also differ: the OAuth object exists with no effort, this one does not, so a shared helper would need a flag to carry both. Its tests are unchanged.The harmonizer
A real defect on the read side, found on the way in and fixed here because this branch is what makes it reachable.
harmonize_responsejoined a reasoning item's parts withstr(), and the wire part is an object,{"type": "summary_text", "text": ...}. The first real summary would therefore have put a Python dict repr where the model's own words belong. One small part reader now serves both thesummaryand thecontentlists, plain string parts still read as they did, and the streaming fixture takes the wire shape so the path is pinned end to end.Tests
tests/models/test_openai_reasoning_summary.pyis new: the served generation over the four inheriting adapters and both providers, by either route to an effort; the predicate on its own; the no-effort cases including an explicit summary that is still not sent; the models outside the generation; unknown providers through the factory and a stamped provider beating the class name; the three override words and the off word; the allow-list underwarnings.simplefilter("error"); the payload throughBaseAPIModel.runandarunwith a captured transport, built twice and compared; and the harmonizer on object parts, string parts and content blocks.Rewritten in place, none deleted and none skipped: six expected reasoning objects in
tests/models/test_azure_openai_leg.py, the api-key expectation intests/models/test_openai_minimal_effort.py, and the streaming fixture's summary part intests/models/test_adapter_streaming.py. Nothing added here carries thenetworkmarker or reaches a provider.The accumulator's own case reads the rebuilt terminal response back through
harmonize_responseand asserts the model's words, so a summary part mangled on the way through fails there and not only in the end-to-end case, and the new file writes each row's expected effort out instead of asking_served_effortfor it.tests/modelsis green at 1114 passed, 39 skipped. The rest of the offline suite is unchanged; its three Windows-only failures (test_doc_generation,test_storage::test_invalid_keys_rejected,test_ndjson_reader::test_reader_opens_under_writer_lock_windows) fail identically at the base commit and are unrelated to this branch.Rebasing against #62 and #63
This branch is based on
mainand stacks on neither.#62 (
prompt-cache-diagnostics) does editsrc/marsys/models/adapters/openai.py, contrary to what this note said before: six hunks, at base lines 68, 549, 584, 628, 806 and 815. None of them meets a line this branch touches and the nearest gap is ten lines, so there is no overlap.git merge-tree --write-tree HEAD origin/prompt-cache-diagnosticsmerges clean in either order.#63 (
tool-search-namespaces, stacked on #62) does conflict, and the conflict is now the harmless kind. #63 introduces_generation(model_lower) -> (major, minor) | Noneas the one generation reader and rewritessupports_explicit_prompt_cacheonto it; this branch carries the same reader, same signature, same body, and reads it from both gates.git merge-tree --write-tree HEAD origin/tool-search-namespacesconflicts in that one file, in three places:_generation, only the second paragraph of the docstring, one wording against the other. The signature and the body merge identically. Keep either paragraph._GENERATION_RE: Group deferred tools into namespaces and keep the provider's search items #63's hosted-tool-search block against nothing on this side. Keep Group deferred tools into namespaces and keep the provider's search items #63's._blocks_with_breakpoint: this branch'ssupports_reasoning_summaryand_reasoning_parts_textagainst nothing on that side. Keep this branch's.supports_explicit_prompt_cache's new two-line body merges with no conflict, because both branches wrote the same two lines, and so does theTupleadded to the typing import. The fold this note used to owe is therefore already done on both sides: whichever branch lands second deletes its duplicate copy of_generationand nothing else changes.What this does not do
No change to the error classifier, which still has no 400 arm for the
openaiandazurebranch; a leg that refused this field would classify every call as unknown and not retryable. That gap predates this branch and wants a fix of its own. No change to which lanes ask for a summary on the consuming side, no per-slot setting, and no cost knob.