Skip to content

Fix for #967: Resolve ResponsesModel union to plain model name in URL path - #968

Open
lzandman wants to merge 1 commit into
openai:mainfrom
lzandman:leon/fix-responsemodel-bug
Open

Fix for #967: Resolve ResponsesModel union to plain model name in URL path#968
lzandman wants to merge 1 commit into
openai:mainfrom
lzandman:leon/fix-responsemodel-bug

Conversation

@lzandman

@lzandman lzandman commented Sep 3, 2026

Copy link
Copy Markdown

Summary

Fixes #967 — a regression of #371.

PrepareRequest.modelNameOrNull() reflectively calls model() on the request params, unwraps an Optional, and then calls toString() on whatever remains. For the Responses API, model() returns Optional<ResponsesModel>, a union wrapper whose toString() is a debug representation. Against a deployment-based Azure base URL that string leaked straight into the path:

# before
POST .../openai/deployments/ResponsesModel%7Bstring=gpt-5.4%7D/responses?api-version=...

# after
POST .../openai/deployments/gpt-5.4/responses?api-version=...

The gateway rejected the malformed segment with a 404.

Changes

  • modelNameOrNull() now unwraps the Optional first, then resolves a ResponsesModel through its Visitor instead of toString(): the string variant is used as-is, and the ChatModel / ResponsesOnlyModel variants use their enum toString(), which already yields the wire value. An unknown variant returns null.
  • Added PrepareRequestTest cases for ResponseCreateParams built with a plain string, a ChatModel, and a ResponsesOnlyModel.

@lzandman
lzandman requested a review from a team as a code owner September 3, 2026 14:25
…in URL path

`PrepareRequest.modelNameOrNull()` called `toString()` on whatever `model()`
returned, so a `ResponsesModel` leaked its debug form into the Azure deployment
path (`ResponsesModel{string=gpt-5.4}` instead of `gpt-5.4`). Resolve the union
through its visitor and add tests for the string, chat, and responses-only
variants.
@lzandman
lzandman force-pushed the leon/fix-responsemodel-bug branch from 04f9b11 to dddcdec Compare September 3, 2026 14:30

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correct boundary. Resolving the ResponsesModel union before path construction avoids leaking its debug representation into model-dependent URLs, while unknown variants fail closed to null rather than inventing a path segment. The string, chat-model and responses-only tests cover the wire-name variants that matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression of #371 — ResponsesModel.toString() leaks into Azure deployment URL path via PrepareRequest.modelNameOrNull()

2 participants