Skip to content

Chore: remove response model check and rely on header model for downgrade#12061

Merged
shijie-oai merged 5 commits intomainfrom
shijie/remove-response-model-check
Feb 18, 2026
Merged

Chore: remove response model check and rely on header model for downgrade#12061
shijie-oai merged 5 commits intomainfrom
shijie/remove-response-model-check

Conversation

@shijie-oai
Copy link
Collaborator

@shijie-oai shijie-oai commented Feb 18, 2026

Summary

Ensure that we use the model value from the response header only so that we are guaranteed with the correct slug name. We are no longer checking against the model value from response so that we are less likely to have false positive.

There are two different treatments - for SSE we use the header from the response and for websocket we check top-level events.

Comment on lines +2443 to +2445
let server_model_normalized = server_model.to_ascii_lowercase();
let requested_model_normalized = requested_model.to_ascii_lowercase();
if server_model_normalized == requested_model_normalized {
Copy link
Collaborator Author

@shijie-oai shijie-oai Feb 18, 2026

Choose a reason for hiding this comment

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

We check lowercase slug value just for extra safety.

@shijie-oai shijie-oai marked this pull request as ready for review February 18, 2026 00:35
Comment on lines 198 to 201
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines -187 to -196
fn extract_server_model(value: &Value) -> Option<String> {
value
.get("model")
.and_then(json_value_as_string)
.or_else(|| {
value
.get("headers")
.and_then(header_openai_model_value_from_json)
})
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

internalize this function.

@shijie-oai shijie-oai marked this pull request as ready for review February 18, 2026 00:46
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we unnest this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

Comment on lines +190 to +202
let response_headers_model = self
.response
.as_ref()
.and_then(|response| response.get("headers"))
.and_then(header_openai_model_value_from_json);

match response_headers_model {
Some(model) => Some(model),
None => self
.headers
.as_ref()
.and_then(header_openai_model_value_from_json),
}
Copy link
Collaborator Author

@shijie-oai shijie-oai Feb 18, 2026

Choose a reason for hiding this comment

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

Not sure if this is what unested here really mean here but this should be cleaner.

Copy link
Collaborator

@dylan-hurd-oai dylan-hurd-oai left a comment

Choose a reason for hiding this comment

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

I don't have enough context on this problem to confidently stamp but the general approach LGTM - if we need to urgently block let me know and we can review together

Copy link
Collaborator

@fouad-openai fouad-openai left a comment

Choose a reason for hiding this comment

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

approved

@shijie-oai shijie-oai force-pushed the shijie/remove-response-model-check branch from 0ab4d5b to ab83134 Compare February 18, 2026 01:36
@shijie-oai shijie-oai enabled auto-merge (squash) February 18, 2026 01:49
@shijie-oai shijie-oai merged commit b3a8571 into main Feb 18, 2026
33 checks passed
@shijie-oai shijie-oai deleted the shijie/remove-response-model-check branch February 18, 2026 01:50
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments