mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
Don't allow model_supports_reasoning_summaries to disable reasoning (#11833)
The `model_supports_reasoning_summaries` config option was originally added so users could enable reasoning for custom models (models that codex doesn't know about). This is how it was documented in the source, but its implementation didn't match. It was implemented such that it can also be used to disable reasoning for models that otherwise support reasoning. This leads to bad behavior for some reasoning models like `gpt-5.3-codex`. Diagnosing this is difficult, and it has led to many support issues. This PR changes the handling of `model_supports_reasoning_summaries` so it matches its original documented behavior. If it is set to false, it is a no-op. That is, it never disables reasoning for models that are known to support reasoning. It can still be used for its intended purpose -- to enable reasoning for unknown models.
This commit is contained in:
@@ -20,8 +20,10 @@ const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective so
|
||||
const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}";
|
||||
|
||||
pub(crate) fn with_config_overrides(mut model: ModelInfo, config: &Config) -> ModelInfo {
|
||||
if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries {
|
||||
model.supports_reasoning_summaries = supports_reasoning_summaries;
|
||||
if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries
|
||||
&& supports_reasoning_summaries
|
||||
{
|
||||
model.supports_reasoning_summaries = true;
|
||||
}
|
||||
if let Some(context_window) = config.model_context_window {
|
||||
model.context_window = Some(context_window);
|
||||
@@ -100,3 +102,46 @@ fn local_personality_messages_for_slug(slug: &str) -> Option<ModelMessages> {
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::test_config;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn reasoning_summaries_override_true_enables_support() {
|
||||
let model = model_info_from_slug("unknown-model");
|
||||
let mut config = test_config();
|
||||
config.model_supports_reasoning_summaries = Some(true);
|
||||
|
||||
let updated = with_config_overrides(model.clone(), &config);
|
||||
let mut expected = model;
|
||||
expected.supports_reasoning_summaries = true;
|
||||
|
||||
assert_eq!(updated, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reasoning_summaries_override_false_does_not_disable_support() {
|
||||
let mut model = model_info_from_slug("unknown-model");
|
||||
model.supports_reasoning_summaries = true;
|
||||
let mut config = test_config();
|
||||
config.model_supports_reasoning_summaries = Some(false);
|
||||
|
||||
let updated = with_config_overrides(model.clone(), &config);
|
||||
|
||||
assert_eq!(updated, model);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reasoning_summaries_override_false_is_noop_when_model_is_false() {
|
||||
let model = model_info_from_slug("unknown-model");
|
||||
let mut config = test_config();
|
||||
config.model_supports_reasoning_summaries = Some(false);
|
||||
|
||||
let updated = with_config_overrides(model.clone(), &config);
|
||||
|
||||
assert_eq!(updated, model);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user