diff --git a/src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp b/src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp index eb501855a8..f58a47ec6e 100644 --- a/src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp +++ b/src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp @@ -155,6 +155,8 @@ bool Qwen3CoderToolParserImpl::parseUntilStateChange(ToolCalls_t& toolCalls) { case State::InsideFunctionName: { DEFINE_TAG_POSITION_AND_BREAK_IF_NOT_FOUND(Qwen3CoderToolParser::XML_TAG_END); this->currentFunction.name = streamContent.substr(this->lastProcessedPosition, pos - this->lastProcessedPosition); + // Some models quote the tag attribute (); the quotes are delimiters, not part of the name. + trimSurroundingQuotes(this->currentFunction.name); this->lastProcessedPosition = pos + Qwen3CoderToolParser::XML_TAG_END.length(); this->currentState = State::InsideFunction; break; @@ -175,6 +177,8 @@ bool Qwen3CoderToolParserImpl::parseUntilStateChange(ToolCalls_t& toolCalls) { case State::InsideParameterName: { DEFINE_TAG_POSITION_AND_BREAK_IF_NOT_FOUND(Qwen3CoderToolParser::XML_TAG_END); this->currentParameterName = streamContent.substr(this->lastProcessedPosition, pos - this->lastProcessedPosition); + // Some models quote the tag attribute (); the quotes are delimiters, not part of the name. + trimSurroundingQuotes(this->currentParameterName); this->lastProcessedPosition = pos + Qwen3CoderToolParser::XML_TAG_END.length(); this->currentState = State::InsideParameter; break; diff --git a/src/llm/io_processing/utils.cpp b/src/llm/io_processing/utils.cpp index 2100710a27..32a2ad5b38 100644 --- a/src/llm/io_processing/utils.cpp +++ b/src/llm/io_processing/utils.cpp @@ -135,6 +135,21 @@ void trimNewline(std::string& str) { } } +void trimSurroundingQuotes(std::string& str) { + if (str.size() < 2) { + return; + } + const char quote = str.front(); + if ((quote != '"' && quote != '\'') || str.back() != quote) { + return; + } + // A quote in the middle means the character belongs to the value, not to a delimiter pair. + if (str.find(quote, 1) != str.size() - 1) { + return; + } + str = str.substr(1, str.size() - 2); +} + const char* jsonTypeOf(const rapidjson::Value& val) { if (val.IsObject()) return "object"; diff --git a/src/llm/io_processing/utils.hpp b/src/llm/io_processing/utils.hpp index 4568ec0851..ec7d61bbe9 100644 --- a/src/llm/io_processing/utils.hpp +++ b/src/llm/io_processing/utils.hpp @@ -35,6 +35,12 @@ void writeArgumentOfAnyType(const rapidjson::Value& arg, rapidjson::Writer) already yields today. + std::string name = R"("")"; + trimSurroundingQuotes(name); + EXPECT_EQ(name, ""); +} diff --git a/src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp b/src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp index 89527747a2..ac6522bca7 100644 --- a/src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp +++ b/src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp @@ -556,6 +556,249 @@ value2 EXPECT_EQ(parser.getLastProcessedPosition(), input.find("") + std::string("").size()); EXPECT_EQ(content, "\n"); } +// Regression tests for https://github.com/openvinotoolkit/model_server/issues/4487: +// the model sometimes quotes the XML attribute (, ). +// The quotes are delimiters, so they must not end up in the tool name or in the argument +// keys, and the schema-driven type handling keyed on those names must keep working. +// The unquoted path is unchanged; TestJustParserImplUnaryToolCall above is its control. +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplQuotedParameterName) { + const std::string input = R"( + + + +value1 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"value1\"}"); + EXPECT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::Content) << input; + EXPECT_EQ(parser.getLastProcessedPosition(), input.find("") + std::string("").size()); + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplSingleQuotedParameterName) { + const std::string input = R"( + + + +value1 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"value1\"}"); + EXPECT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::Content) << input; + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplQuotedFunctionName) { + const std::string input = R"( + + + +value1 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"value1\"}"); + EXPECT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::Content) << input; + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplQuotedNamesKeepStringEnforcement) { + // Both names are looked up in toolsParametersTypeMap, so quotes left in place would also + // silently disable type handling: arg1 would stay a number instead of the declared string. + const std::string input = R"( + + + +42 + + +7 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_int_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"42\",\"arg2\":7}"); + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplQuotedParameterNameKeepsBooleanNormalization) { + const std::string input = R"( + + + +True + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "bool_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":true}"); + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplMixedQuotingOfParameterNames) { + const std::string input = R"( + + + +value1 + + +42 + + +52.32 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_int_float_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"value1\",\"arg2\":42,\"arg3\":52.32}"); + EXPECT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::Content) << input; + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplTwoToolCallsWithQuotedNames) { + const std::string input = R"( + + + +value1 + + + + + + +data + + +25.2 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 2) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg1\":\"value1\"}"); + EXPECT_EQ(calls[1].name, "string_float_tool"); + EXPECT_EQ(calls[1].arguments, "{\"arg1\":\"data\",\"arg2\":25.2}"); + EXPECT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::Content) << input; + EXPECT_EQ(parser.getLastProcessedPosition(), input.rfind("") + std::string("").size()) << input; + EXPECT_EQ(content, "\n\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplParameterNameWithInnerQuoteKept) { + // Only a clean wrapping pair is a delimiter; a quote between the ends belongs to the name. + const std::string input = R"( + + + +value1 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"arg\\\"1\":\"value1\"}"); + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplParameterNameWithUnmatchedQuoteKept) { + // Deliberately out of scope: a single unpaired quote is not a delimiter pair, and the + // parser cannot tell a dropped closing quote from a name that starts with one. + const std::string input = R"( + + + +value1 + + +)"; + auto content = input; + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + auto callsOpt = parser.parseChunk(content); + ASSERT_TRUE(callsOpt.has_value()); + ToolCalls_t& calls = callsOpt.value(); + auto status = parser.removeToolCallsFromContentIfNeeded(content); + EXPECT_TRUE(status.ok()) << status.string(); + ASSERT_EQ(calls.size(), 1) << input; + EXPECT_EQ(calls[0].name, "string_tool"); + EXPECT_EQ(calls[0].arguments, "{\"\\\"arg1\":\"value1\"}"); + EXPECT_EQ(content, "\n"); +} +TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplStreamStepWithQuotedFunctionName) { + // getCurrentFunctionName() feeds the first streamed delta, so it must already be clean. + ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); + const std::string input = R"( + + + +value1 +)"; + auto content = input; + auto stepResult = parser.parseChunk(content); + ASSERT_FALSE(stepResult.has_value()); + ASSERT_EQ(parser.getCurrentState(), ovms::Qwen3CoderToolParserImpl::State::InsideParameter); + ASSERT_TRUE(parser.getCurrentFunctionName().has_value()); + EXPECT_EQ(parser.getCurrentFunctionName().value(), "string_tool"); +} TEST_F(Qwen3CoderOutputParserTest, TestJustParserImplStreamStepWithMoreThan1StateChange) { ToolCalls_t calls; ovms::Qwen3CoderToolParserImpl parser(toolsParametersTypeMap); @@ -716,6 +959,19 @@ INSTANTIATE_TEST_SUITE_P( return name; }); +TEST_F(Qwen3CoderOutputParserTest, StreamingQuotedFunctionAndParameterNames) { + // End-to-end streaming counterpart of the parser-level quoted-attribute tests: the quotes + // must not leak into the first delta (function name) nor into the argument keys. + auto nameDelta = outputParser->parseChunk(R"()", {}, true, ov::genai::GenerationFinishReason::NONE); + ASSERT_TRUE(nameDelta.has_value()); + const std::string nameJson = ovms::test::deltaToJson(*nameDelta); + EXPECT_NE(nameJson.find(R"("name":"string_tool")"), std::string::npos) << nameJson; + + auto argumentsDelta = outputParser->parseChunk("value1", {}, true, ov::genai::GenerationFinishReason::NONE); + ASSERT_TRUE(argumentsDelta.has_value()); + const std::string argumentsJson = ovms::test::deltaToJson(*argumentsDelta); + EXPECT_NE(argumentsJson.find(R"("arguments":"{\"arg1\":\"value1\"}")"), std::string::npos) << argumentsJson; +} TEST_F(Qwen3CoderOutputParserTest, StreamingSimpleToolCall) { // since unary reuses streaming we don't need to test for partial tool calls // if we don't get closing tag we don't emit tool call