[codex] Treat max as a first-class reasoning effort#30467
Conversation
| Self::Medium => "medium", | ||
| Self::High => "high", | ||
| Self::XHigh => "xhigh", | ||
| Self::Max => "max", |
There was a problem hiding this comment.
Mark Max as official reasoning effort instead of treating it as a string value from the backend.
fcoury-oai
left a comment
There was a problem hiding this comment.
My OCD welcomes this PR. Code looks great, very straightforward.
Codex found one nit that's completely optional.
Approved! 👍
| Medium, | ||
| High, | ||
| XHigh, | ||
| Max, |
There was a problem hiding this comment.
Nitpick found by Codex:
Now that max has a first-class variant, could we also change reasoning_effort_for_request() in core/src/client.rs to map Ultra to ReasoningEffort::Max instead of Custom("max") (and update its test)? Leaving it unchanged conflicts with Custom’s “unknown effort” contract and means internal request objects still classify a known effort as custom, even though the wire value is the same.
Comment by me: apparently this happens in core/src/client.rs lines 170-175:
fn reasoning_effort_for_request(effort: ReasoningEffortConfig) -> ReasoningEffortConfig {
match effort {
ReasoningEffortConfig::Ultra => ReasoningEffortConfig::Custom("max".to_string()),
effort => effort,
}
}This suggestion is a non-blocker of course.
Why
The Bedrock GPT-5.6 catalog advertises
max, but Codex treated it as an opaque custom effort. That made the reasoning picker render it as lowercasemaxwhile known efforts use productized labels.Making
maxa known effort aligns catalog data, parsing, and UI presentation without changing themaxwire value or persisted representation.What changed
ReasoningEffort::Maxparsing and serialization.Maxin the TUI.futurevalue.Before
After