Skip to content

Conversation

@aurexav
Copy link

@aurexav aurexav commented Dec 18, 2025

Closes #1176.

Copy link
Collaborator

@joshua-mo-143 joshua-mo-143 left a comment

Choose a reason for hiding this comment

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

simple change then lgtm

Comment on lines 711 to 712
/// The status of the response.
pub status: ResponseStatus,
pub status: Option<ResponseStatus>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please append a message onto the docstring to state that the reason for this being Option is for OpenRouter compatibility. Theoretically this should always be Some when using OpenAI's Responses API

Copy link
Author

Choose a reason for hiding this comment

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

Done

@Sytten
Copy link
Contributor

Sytten commented Dec 27, 2025

This is not a good idea IMO, status is NOT optional for openai.
This means the API are starting to diverge and you should instead copy the struct in the openrouter folder.
You can re-use the fields that are the same exact same. I already started doing that for the reasoning support in openrouter.

@aurexav aurexav closed this Dec 27, 2025
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.

Openrouter‘s response API doesn't contains this field

3 participants