[TheGreatAxios]

Implements INTR-662: ports the Corbits Responses adapter (streaming and buffered parsing, tool calls, usage, media, encrypted reasoning replay) with quirks validation and composition hooks, registers openai-responses and openai-compatible-responses, adds the optional ProviderAdapter.isStreamTerminal predicate consumed by the harness, and grounds discovery with 54 live fixtures across ten first-party models plus matrix rows and catalog offerings baked from that evidence. Follow-up surface (built-in tools, async calling, shell) tracked in INTR-703.

[linear]

INTR-662

[alexanderguy]

This is an agentic review. I ran it with the pull-request-review skill and checked each finding with a second pass. gmake all exits 0 on 5f20958bb.

I left most items as inline comments. These items have no line in the diff:

Should fix

• The new plugin keys are not in modelProviderPlugins (packages/types/src/catalog.ts:5). The admin UI (tenant-model-providers.tsx:204,218) and catalog.test.ts:15-16 use that list, so an operator cannot pick the Responses plugin there. Could we add both keys and update the test?

Commit messages

• fa678be67 also fixes the README model list. That fix belongs in 60d744180, which adds the list.
• The bodies of 22af6a475 and 60d744180 describe the tests. Could they give the reason for the change?

Questions

• Why add a second provider, "OpenAI Responses", and not a plugin choice on "OpenAI Direct"? The two providers have the same base URL and the same models, so an operator must pick between two catalog entries for one account.
• The reasoning fixtures have empty reasoning summaries. Is that intended? If yes, the fixtures do not show that summary text reaches inference.thinking.

[TheGreatAxios]

Replying to the review-level items — the inline threads are answered individually.

Plugin keys: both keys added to modelProviderPlugins and the test workaround replaced with a custom-adapter example, so the admin picker lists them.

History: rewritten. The README model-list hunk moved into the builders commit, both test-describing bodies now explain why, not what, and every fix in this round sits in its owning commit.

Second provider vs plugin choice: the wire is a deployment property, not a model property — plugin lives on the provider, so one OpenAI Direct entry cannot serve both wires. An operator whose account serves both genuinely has two deployments. Plugin is also the factory dispatch key, so merging would remove the only signal distinguishing them. openai-compatible-responses stays preset-less for now; operators can already register any plugin key via the API.

Empty reasoning summaries: intended consequence — the old captures never requested a summary, so the server returned null. Recaptured with summary requested: 9/10 models now return live summary text (gpt-6.1-terra still returns empty — recorded as is), which grounds the thinking path.

[alexanderguy]

This is an agentic re-review at 28c0cc6f0. I ran it with the pull-request-review skill. A second pass tried to disprove each finding. gmake all exits 0.

Thank you for the fixes. I checked each item from the last round against the code, and all of them are resolved. Three new items remain, and a few small wording points. They are inline.

[alexanderguy]

This is an agentic re-review at 32c396e08. I ran it with the pull-request-review skill. Two more passes tried to disprove each finding. gmake all exits 0 at the head, and CI passes.

Thank you for the fixes. I checked each item from the last round against the code, and all of them are resolved. The history rewrite caused two new problems, and a few small items remain. One item is inline.

Should fix

• Commits 720ac4334 through 39cad1364 do not type-check. In these commits, openai-responses.ts imports filenameForDocumentMime from ./openai and also declares a local function with the same name. gmake build fails at 720ac4334 and at 39cad1364:

  openai-responses.ts(34,3): error TS2440: Import declaration conflicts with local declaration of 'filenameForDocumentMime'.
  openai-responses.ts(260,13): error TS2554: Expected 1 arguments, but got 2.
  

  The head builds because a1a663e70 deletes the local function. Could that deletion move into 720ac4334? Then a git bisect does not stop on a build error.

• a1a663e70 "Match Chat wire encoding for tool errors" changes the adapter by +306/−15 lines and its tests by +188/−4. Only the toolResultText change and the test "error tool results use the shared Chat wire encoding" match the subject. The rest of the commit contains:

  • the url_citation and web_search_call parsing
  • the code_interpreter_call JSON and SSE handling
  • the null error / incomplete_details decoding and the failed-status handling
  • six tests for that code
  • the deletion of the local filenameForDocumentMime

  In the last round, this code was in "Add Responses web search and code execution". Now 32c396e08 with that subject does not touch the adapter, but its body still says "Parse citations and code execution events in both JSON and SSE." Could we move the parsing hunks and the six tests back into 32c396e08, and the deletion into 720ac4334? Then each message matches its diff.

Small items

• The body of 8165b24ff says "Fifty-four live exchanges". That commit adds 55 exchanges in 45 sessions. Could we correct the number?

• The PR description does not match the branch in three places:

  1. It says that the adapter has "composition hooks". The adapter has no hook API. ResponsesQuirks holds only data, and its comment rejects function-valued fields.
  2. It says "54 live fixtures". The branch has 49 sessions with 59 exchanges under packages/inference-discovery-openai/sessions/openai-responses.
  3. It does not say that the PR adds web search and code execution.

  Could we update it?

[alexanderguy]

The comment above says that turn 2 replays the reasoning item in the adapter's shape. But this line copies the server's summary, and the adapter always sends summary: [] (packages/inference/src/providers/openai-responses.ts:352). The fixtures match today only because turn 1 asks for no summary. The plugin test (openai-responses.plugin.test.ts:405-446) uses summary: [] on both sides, so it cannot tell the two forms apart.

In production, the catalog requests summary: "detailed" for every Responses offering. So the server returns summary text, and the adapter replays []. Could we send summary: [] here? Then the code matches the comment, and the ?? read-site default goes away.

[alexanderguy]

One more point about this line. My request in the last round to add reasoning: { summary: "detailed" } to the catalog opened a gap here.

In production, the adapter now replays reasoning items that had summary text, and it sends them with summary: [] (packages/inference/src/providers/openai-responses.ts:352). The multi-turn fixture does not cover this case. Turn 1 sends no reasoning, store or include, so its reasoning item comes back with an empty summary, and turn 2 never removes any text. The plugin test has the same limit: its input summary is also empty, so it cannot tell a copy of the summary from [].

Two details that can matter if you recapture:

• This line copies the server's summary, so a recapture with summary text would record the copy and not [].
• gpt-5-mini has returned empty summaries on its short turns so far.

How do you want to close this? If a capture is not practical, a comment at the adapter line that states the assumption also works for me.