Treat the GPT-5 family as reasoning-first on Chat Completions - #58
Merged
Conversation
iskakaushik
force-pushed
the
openai-sampling-gate
branch
from
August 19, 2026 18:53
c369214 to
0d0bb6c
Compare
iskakaushik
changed the title
iskakaushik
force-pushed
the
openai-sampling-gate
branch
from
August 19, 2026 18:57
0d0bb6c to
7379843
Compare
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.
iskakaushik
force-pushed
the
openai-sampling-gate
branch
from
August 19, 2026 19:01
7379843 to
6982559
Compare
iskakaushik
added a commit
to iskakaushik/ClickHouse
that referenced
this pull request
Aug 19, 2026
The SDK is now directly consumable from its main branch: upstream replaced its nested submodules with pinned CMake FetchContent (ClickHouse/ai-sdk-cpp#57), retiring the separately maintained flattened integration branch. The new pin also carries the response_messages terminal-reply fix requested in review (ClickHouse/ai-sdk-cpp#55) and GPT-5-family request normalization: the family rejects temperature/top_p and defaults to reasoning, which returned a hard 400 for any configured ai.temperature and burned small ai.max_tokens budgets on hidden reasoning with no visible SQL (ClickHouse/ai-sdk-cpp#58). Add the SDK's new http_sse_stream.cpp to the explicit source list; without it every target linking the SDK fails. Throw NETWORK_ERROR instead of LOGICAL_ERROR when a generation fails: a provider/API error is external, and LOGICAL_ERROR aborts debug builds on any API failure (observed as SIGABRT in unit_tests_dbms during live runs). Teach the generation prompt to treat SQL fragments embedded in the request as untrusted. Without instruction, current models faithfully reproduce statement stacking and UNION SELECT ... FROM passwords when the request embeds them (deterministic on claude-haiku-4-5 at temperature 0), so SQLInjectionProtection asserted behavior nothing had asked of the model. Update live-test model IDs that referenced retired models (gpt-4o-mini, gpt-4, claude-3-haiku, claude-3-opus all 404 today) and scope the injection test's comment assertion to quote-terminated statement stacking: models may legitimately echo the user's own comment text, and the result is prepopulated for review, not executed. Validated locally with unit_tests_dbms: offline AI suites pass, and the live AITestFixture suite passes 8/8 against both OpenAI (gpt-5-mini, gpt-5.6) and Anthropic (claude-haiku-4-5, claude-sonnet-5), including the multi-step SchemaExploration case the underlying fix exists for.
iskakaushik
added a commit
to iskakaushik/ClickHouse
that referenced
this pull request
Aug 19, 2026
The SDK is now directly consumable from its main branch: upstream replaced its nested submodules with pinned CMake FetchContent (ClickHouse/ai-sdk-cpp#57), retiring the separately maintained flattened integration branch. The new pin also carries the response_messages terminal-reply fix requested in review (ClickHouse/ai-sdk-cpp#55) and GPT-5-family request normalization: the family rejects temperature/top_p and defaults to reasoning, which returned a hard 400 for any configured ai.temperature and burned small ai.max_tokens budgets on hidden reasoning with no visible SQL (ClickHouse/ai-sdk-cpp#58). Add the SDK's new http_sse_stream.cpp to the explicit source list; without it every target linking the SDK fails. Throw NETWORK_ERROR instead of LOGICAL_ERROR when a generation fails: a provider/API error is external, and LOGICAL_ERROR aborts debug builds on any API failure (observed as SIGABRT in unit_tests_dbms during live runs). Teach the generation prompt to treat SQL fragments embedded in the request as untrusted. Without instruction, current models faithfully reproduce statement stacking and UNION SELECT ... FROM passwords when the request embeds them (deterministic on claude-haiku-4-5 at temperature 0), so SQLInjectionProtection asserted behavior nothing had asked of the model. Update live-test model IDs that referenced retired models (gpt-4o-mini, gpt-4, claude-3-haiku, claude-3-opus all 404 today) and scope the injection test's comment assertion to quote-terminated statement stacking: models may legitimately echo the user's own comment text, and the result is prepopulated for review, not executed. Validated locally with unit_tests_dbms: offline AI suites pass, and the live AITestFixture suite passes 8/8 against both OpenAI (gpt-5-mini, gpt-5.6) and Anthropic (claude-haiku-4-5, claude-sonnet-5), including the multi-step SchemaExploration case the underlying fix exists for.
1 task
alexey-milovidov
pushed a commit
to ClickHouse/ClickHouse
that referenced
this pull request
Aug 20, 2026
At the previous pin (`ab06ef7`), OpenAI requests dropped the `system` prompt whenever a messages array was passed - the agent always sends history, so its system prompt never reached OpenAI models. The GPT-5 family also rejected `temperature` and burned small `max_tokens` budgets on hidden reasoning without the `reasoning_effort` handling. Both are fixed upstream (ClickHouse/ai-sdk-cpp#40, ClickHouse/ai-sdk-cpp#58), and the default models move to `gpt-5.6` / `claude-sonnet-5`. The new pin adds `src/providers/http_sse_stream.cpp` to the build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.