From 39c71140da80ac54c35ad112b35a158a15319c1f Mon Sep 17 00:00:00 2001 From: Sayan Shaw Date: Wed, 26 Aug 2026 11:31:18 -0700 Subject: [PATCH 1/2] Fix: apply guidance for user-specified response_format regardless of tool_choice --- .../generative/chat/chat_session.cc | 6 ++- .../generative/chat/onnx_chat_generator.cc | 15 +++--- .../cpp/test/sdk_api/chat_completions_test.cc | 46 +++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc b/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc index 7b8c646be..f816b5dfa 100644 --- a/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc +++ b/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc @@ -464,8 +464,10 @@ void ChatSession::ProcessRequestImpl(const Request& request, Response& response) auto turn_tool_ctx = cached_tool_ctx_; UpdateToolContextForTurn(request, turn_tool_ctx); - bool prev_needs_guidance = cached_tool_ctx_.tool_output && !cached_tool_ctx_.text_output; - bool curr_needs_guidance = turn_tool_ctx.tool_output && !turn_tool_ctx.text_output; + bool prev_has_user_guidance = !cached_tool_ctx_.guidance_type.empty() && !cached_tool_ctx_.guidance_data.empty(); + bool curr_has_user_guidance = !turn_tool_ctx.guidance_type.empty() && !turn_tool_ctx.guidance_data.empty(); + bool prev_needs_guidance = prev_has_user_guidance || (cached_tool_ctx_.tool_output && !cached_tool_ctx_.text_output); + bool curr_needs_guidance = curr_has_user_guidance || (turn_tool_ctx.tool_output && !turn_tool_ctx.text_output); if (prev_needs_guidance != curr_needs_guidance) { // Guidance requirements changed — invalidate. The branch below will rebuild from full history. diff --git a/sdk_v2/cpp/src/inferencing/generative/chat/onnx_chat_generator.cc b/sdk_v2/cpp/src/inferencing/generative/chat/onnx_chat_generator.cc index 6a1aa0662..a14d1b2f7 100644 --- a/sdk_v2/cpp/src/inferencing/generative/chat/onnx_chat_generator.cc +++ b/sdk_v2/cpp/src/inferencing/generative/chat/onnx_chat_generator.cc @@ -387,14 +387,17 @@ std::unique_ptr OnnxChatGenerator::CreateImpl(const std::vect } } - // Guard: Apply guidance only for tool-call-only mode (tool output requested, no text output). Text-only reasoning - // (cot_text_only) cannot use grammar guidance because a completed grammar signals EOS to the ORT GenAI generator — - // making IsDone() return true immediately on the next turn, breaking multi-turn continuous decoding. For - // tool-call-only mode the generator is typically invalidated after a successful call anyway, so this is acceptable. - // Reasoning content for text-only mode is handled via StripReasoningContent post-processing. + // Guard: apply guidance based on its source. + // User-specified guidance (via response_format) is always applied — the user explicitly requested it. + // Auto-generated tool grammar is only applied for tool-call-only mode (tool output requested, no text output). + // Text-only reasoning (cot_text_only) cannot use auto-generated grammar because a completed grammar signals EOS + // to the ORT GenAI generator — making IsDone() return true immediately on the next turn, breaking multi-turn + // continuous decoding. For tool-call-only mode the generator is typically invalidated after a successful call + // anyway, so this is acceptable. + bool user_specified_guidance = !tool_ctx.guidance_type.empty() && !tool_ctx.guidance_data.empty(); bool tool_call_only = tool_ctx.tool_output && !tool_ctx.text_output; - if (!guidance_type.empty() && !guidance_data.empty() && tool_call_only) { + if (!guidance_type.empty() && !guidance_data.empty() && (user_specified_guidance || tool_call_only)) { try { gen_params->SetGuidance(guidance_type.c_str(), guidance_data.c_str()); } catch (const std::runtime_error& e) { diff --git a/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc b/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc index 264fe19b5..7ab1854de 100644 --- a/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc +++ b/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc @@ -270,3 +270,49 @@ TEST_F(WebServiceIntegrationTest, ChatCompletionsWithResponseFormat) { std::string content = response["choices"][0]["message"]["content"].get(); EXPECT_FALSE(content.empty()); } + +TEST_F(WebServiceIntegrationTest, ChatCompletionsWithJsonSchemaGuidance) { + auto client = MakeClient(); + + // json_schema response_format should constrain the output to valid JSON matching the schema. + // This tests that guidance is applied even without tool_choice=required (bug #1042). + json schema = { + {"type", "object"}, + {"properties", { + {"answer", {{"type", "integer"}}}, + }}, + {"required", json::array({"answer"})}, + {"additionalProperties", false}, + }; + + json request_body = { + {"model", model_id()}, + {"messages", json::array({ + {{"role", "user"}, {"content", "What is 2+2? Respond with JSON."}}, + })}, + {"temperature", 0}, + {"max_tokens", 64}, + {"response_format", {{"type", "json_schema"}, {"json_schema", schema}}}, + }; + + auto result = client.Post("/v1/chat/completions", request_body.dump(), "application/json"); + ASSERT_TRUE(result) << "HTTP request failed"; + ASSERT_EQ(result->status, 200) << result->body; + + json response = json::parse(result->body); + ASSERT_TRUE(response.contains("choices")); + std::string content = response["choices"][0]["message"]["content"].get(); + EXPECT_FALSE(content.empty()) << "Model produced no output"; + + // The output should be valid JSON + json parsed; + EXPECT_NO_THROW(parsed = json::parse(content)) << "Output is not valid JSON: " << content; + + // The output should have the "answer" field as an integer + if (parsed.is_object()) { + EXPECT_TRUE(parsed.contains("answer")) << "Missing 'answer' field in: " << content; + if (parsed.contains("answer")) { + EXPECT_TRUE(parsed["answer"].is_number_integer()) << "'answer' is not an integer in: " << content; + } + } +} From 8cb921a767150dee012fd4b02777f27b118d0329 Mon Sep 17 00:00:00 2001 From: Sayan Shaw Date: Thu, 27 Aug 2026 15:08:16 -0700 Subject: [PATCH 2/2] Address Copilot review: tighten test assertions, fix cache invalidation for schema changes --- .../inferencing/generative/chat/chat_session.cc | 6 +++++- sdk_v2/cpp/test/sdk_api/chat_completions_test.cc | 15 ++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc b/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc index f816b5dfa..5e70bdaa9 100644 --- a/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc +++ b/sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc @@ -469,7 +469,11 @@ void ChatSession::ProcessRequestImpl(const Request& request, Response& response) bool prev_needs_guidance = prev_has_user_guidance || (cached_tool_ctx_.tool_output && !cached_tool_ctx_.text_output); bool curr_needs_guidance = curr_has_user_guidance || (turn_tool_ctx.tool_output && !turn_tool_ctx.text_output); - if (prev_needs_guidance != curr_needs_guidance) { + // Guidance (grammar) is baked into the OGA generator at creation time and cannot be changed. + // Rebuild when: guidance requirements changed OR the previous turn had user-specified guidance + // (the finite grammar may have completed, causing IsDone() to return true on the next turn, + // and switching schemas requires a fresh grammar). + if (prev_needs_guidance != curr_needs_guidance || prev_has_user_guidance) { // Guidance requirements changed — invalidate. The branch below will rebuild from full history. cached_generator_.reset(); cached_tool_ctx_ = {}; diff --git a/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc b/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc index 7ab1854de..2eafb47e7 100644 --- a/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc +++ b/sdk_v2/cpp/test/sdk_api/chat_completions_test.cc @@ -304,15 +304,12 @@ TEST_F(WebServiceIntegrationTest, ChatCompletionsWithJsonSchemaGuidance) { std::string content = response["choices"][0]["message"]["content"].get(); EXPECT_FALSE(content.empty()) << "Model produced no output"; - // The output should be valid JSON + // The output should be valid JSON matching the requested schema exactly. json parsed; - EXPECT_NO_THROW(parsed = json::parse(content)) << "Output is not valid JSON: " << content; + ASSERT_NO_THROW(parsed = json::parse(content)) << "Output is not valid JSON: " << content; - // The output should have the "answer" field as an integer - if (parsed.is_object()) { - EXPECT_TRUE(parsed.contains("answer")) << "Missing 'answer' field in: " << content; - if (parsed.contains("answer")) { - EXPECT_TRUE(parsed["answer"].is_number_integer()) << "'answer' is not an integer in: " << content; - } - } + ASSERT_TRUE(parsed.is_object()) << "Output is not an object: " << content; + ASSERT_TRUE(parsed.contains("answer")) << "Missing 'answer' field in: " << content; + EXPECT_TRUE(parsed["answer"].is_number_integer()) << "'answer' is not an integer in: " << content; + EXPECT_EQ(parsed.size(), 1U) << "Output contains additional properties: " << content; }