Record the terminal assistant reply in response_messages - #55
Merged
Conversation
GenerateResult::response_messages documents that it includes the assistant's response so callers can continue the conversation, but the multi-step coordinator only appended assistant turns on the tool-call feedback path. A run whose last step finished with stop (or any non-tool_calls reason) returned the step-1 assistant/tool exchange without the final answer. This regressed in the multi-step history fix (f9d8b40) and survived the coordinator refactor (09c423c); flagged by review on ClickHouse/ClickHouse#113959. Append the terminal step's assistant text to the accumulator on the non-tool_calls exits, and cover the contract with unit tests for the multi-step coordinator (which previously had none).
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.
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.
GenerateResult::response_messages documents that it includes the
assistant's response so callers can continue the conversation, but
the multi-step coordinator only appended assistant turns on the
tool-call feedback path. A run whose last step finished with stop
(or any non-tool_calls reason) returned the step-1 assistant/tool
exchange without the final answer. This regressed in the multi-step
history fix (f9d8b40) and survived the coordinator refactor
(09c423c); flagged by review on ClickHouse/ClickHouse#113959.
Append the terminal step's assistant text to the accumulator on the
non-tool_calls exits, and cover the contract with unit tests for the
multi-step coordinator (which previously had none).