From 698255979fa502ec667a04079b4e6715af28b9a1 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Wed, 19 Aug 2026 13:52:29 -0500 Subject: [PATCH] Treat the GPT-5 family as reasoning-first on Chat Completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GPT-5 family rejects sampling controls: a request with temperature set fails with "'temperature' does not support 0.0 with this model. Only the default (1) value is supported." Callers that set the provider-neutral temperature/top_p options (for example clickhouse-client's ai.temperature setting) get a hard 400 on every gpt-5* model. Omit temperature and top_p for gpt-5-prefixed model IDs with a warning log, exactly as the Anthropic builder already does for the recent Claude models. The reasoning_effort="none" accommodation was also scoped too narrowly to gpt-5.6*: the rest of the family defaults to reasoning too and can burn the entire completion budget on hidden reasoning without producing user-visible text — observed live as gpt-5-mini returning empty output with max_tokens=500. Apply it to the whole gpt-5 family. Cover the omission, the reasoning pin, and the gpt-4.1 passthrough with unit tests. --- .../openai/openai_request_builder.cpp | 46 ++++++++++++++----- tests/unit/openai_client_test.cpp | 42 +++++++++++++++-- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/providers/openai/openai_request_builder.cpp b/src/providers/openai/openai_request_builder.cpp index 224db6f..8395995 100644 --- a/src/providers/openai/openai_request_builder.cpp +++ b/src/providers/openai/openai_request_builder.cpp @@ -4,6 +4,8 @@ #include "ai/openai.h" #include "utils/message_utils.h" +#include + namespace ai { namespace openai { @@ -97,18 +99,34 @@ nlohmann::json OpenAIRequestBuilder::build_request_json( } // Add optional parameters - if (options.temperature) { - request["temperature"] = *options.temperature; - } + // The GPT-5 family is reasoning-first on Chat Completions: it rejects + // sampling controls ("'temperature' does not support 0.0 with this model. + // Only the default (1) value is supported."). Keep accepting the + // provider-neutral options while omitting them for those model IDs, + // mirroring the Anthropic builder. + const bool is_gpt5_family = options.model.starts_with("gpt-5"); + const bool rejects_sampling_parameters = is_gpt5_family; + const auto add_sampling_parameter = [&](const char* key, + const std::optional& value) { + if (!value) { + return; + } + if (rejects_sampling_parameters) { + ai::logger::log_warn( + "Ignoring {} for {} because the model does not support " + "sampling parameters", + key, options.model); + } else { + request[key] = *value; + } + }; + add_sampling_parameter("temperature", options.temperature); + add_sampling_parameter("top_p", options.top_p); if (options.max_tokens) { request["max_completion_tokens"] = *options.max_tokens; } - if (options.top_p) { - request["top_p"] = *options.top_p; - } - if (options.frequency_penalty) { request["frequency_penalty"] = *options.frequency_penalty; } @@ -121,12 +139,18 @@ nlohmann::json OpenAIRequestBuilder::build_request_json( request["seed"] = *options.seed; } - // GPT-5.6 defaults to reasoning in Chat Completions. That mode does not - // support function tools and can consume the full completion budget without - // producing user-visible text. The unified SDK currently exposes Chat - // Completions semantics, so explicitly request non-reasoning output. + // The GPT-5 family defaults to reasoning in Chat Completions. That mode + // can consume the full completion budget on hidden reasoning without + // producing user-visible text (acute with a small max_tokens), and on + // GPT-5.6 it does not support function tools. The unified SDK currently + // exposes Chat Completions semantics, so explicitly request non-reasoning + // output for the whole family. if (options.model.starts_with(models::kGpt56)) { request["reasoning_effort"] = "none"; + } else if (is_gpt5_family) { + // Earlier GPT-5 models reject "none" ("Supported values are: 'minimal', + // 'low', 'medium', and 'high'."); "minimal" is their floor. + request["reasoning_effort"] = "minimal"; } // Add tools if specified diff --git a/tests/unit/openai_client_test.cpp b/tests/unit/openai_client_test.cpp index 891f239..f89090d 100644 --- a/tests/unit/openai_client_test.cpp +++ b/tests/unit/openai_client_test.cpp @@ -150,13 +150,49 @@ TEST_F(OpenAIClientTest, ValidateOptionsValidation) { EXPECT_TRUE(valid_options.is_valid()); } -TEST_F(OpenAIClientTest, Gpt56UsesChatCompletionsCompatibleReasoning) { +TEST_F(OpenAIClientTest, Gpt5FamilyUsesChatCompletionsCompatibleReasoning) { openai::OpenAIRequestBuilder builder; - GenerateOptions options(openai::models::kGpt56Luna, "Use a tool"); + + GenerateOptions gpt56_options(openai::models::kGpt56Luna, "Use a tool"); + EXPECT_EQ(builder.build_request_json(gpt56_options)["reasoning_effort"], + "none"); + + // Earlier GPT-5 models reject "none"; they get the "minimal" floor. + for (const auto* model : + {openai::models::kGpt5Mini, openai::models::kGpt55}) { + GenerateOptions options(model, "Use a tool"); + EXPECT_EQ(builder.build_request_json(options)["reasoning_effort"], + "minimal") + << model; + } +} + +TEST_F(OpenAIClientTest, Gpt5FamilyOmitsUnsupportedSamplingParameters) { + openai::OpenAIRequestBuilder builder; + for (const auto* model : + {openai::models::kGpt56, openai::models::kGpt56Terra, + openai::models::kGpt55, openai::models::kGpt5Mini}) { + GenerateOptions options(model, "Hello"); + options.temperature = 0.0; + options.top_p = 0.8; + + const auto request = builder.build_request_json(options); + + EXPECT_FALSE(request.contains("temperature")) << model; + EXPECT_FALSE(request.contains("top_p")) << model; + } +} + +TEST_F(OpenAIClientTest, NonReasoningModelsKeepSamplingParameters) { + openai::OpenAIRequestBuilder builder; + GenerateOptions options(openai::models::kGpt41, "Hello"); + options.temperature = 0.2; + options.top_p = 0.9; const auto request = builder.build_request_json(options); - EXPECT_EQ(request["reasoning_effort"], "none"); + EXPECT_EQ(request["temperature"], 0.2); + EXPECT_EQ(request["top_p"], 0.9); } // Stream Tests (Basic validation)