diff --git a/include/ninfer/types.h b/include/ninfer/types.h index a2e8b75481..b7294229f2 100644 --- a/include/ninfer/types.h +++ b/include/ninfer/types.h @@ -328,11 +328,12 @@ tool_call_parse_fallback_reason_name(ToolCallParseFallbackReason reason) noexcep } struct ToolCallParseDiagnostics { - bool marker_seen = false; - std::uint32_t structured_call_count = 0; - std::uint32_t empty_arguments_omitted = 0; - std::uint32_t schema_mismatch_arguments = 0; - ToolCallParseFallbackReason fallback_reason = ToolCallParseFallbackReason::None; + bool marker_seen = false; + std::uint32_t structured_call_count = 0; + std::uint32_t empty_arguments_omitted = 0; + std::uint32_t schema_mismatch_arguments = 0; + std::uint32_t duplicate_parameters_repaired = 0; + ToolCallParseFallbackReason fallback_reason = ToolCallParseFallbackReason::None; [[nodiscard]] friend constexpr bool operator==(const ToolCallParseDiagnostics&, const ToolCallParseDiagnostics&) noexcept = default; diff --git a/src/models/qwen3_5/frontend/tool_call_parser.cpp b/src/models/qwen3_5/frontend/tool_call_parser.cpp index 124281d417..a1826ccefc 100644 --- a/src/models/qwen3_5/frontend/tool_call_parser.cpp +++ b/src/models/qwen3_5/frontend/tool_call_parser.cpp @@ -413,7 +413,11 @@ class QwenToolRegionParser { const Contract& contract) : text_(text), max_name_length_(max_name_length), contract_(contract) {} - FallbackReason parse(std::vector& calls) const { + [[nodiscard]] std::uint32_t duplicate_parameters_repaired() const noexcept { + return duplicate_parameters_repaired_; + } + + FallbackReason parse(std::vector& calls) { std::size_t pos = 0; for (;;) { skip_format_whitespace(text_, pos); @@ -439,7 +443,7 @@ class QwenToolRegionParser { return true; } - FallbackReason parse_tool_call(std::size_t& pos, RawToolCall& call) const { + FallbackReason parse_tool_call(std::size_t& pos, RawToolCall& call) { if (!consume(pos, kToolOpen)) { return FallbackReason::MalformedStructure; } skip_format_whitespace(text_, pos); const FallbackReason failure = parse_function(pos, call); @@ -448,7 +452,7 @@ class QwenToolRegionParser { return consume(pos, kToolClose) ? FallbackReason::None : FallbackReason::MalformedStructure; } - FallbackReason parse_function(std::size_t& pos, RawToolCall& call) const { + FallbackReason parse_function(std::size_t& pos, RawToolCall& call) { if (!consume(pos, kFunctionOpen)) { return FallbackReason::MalformedStructure; } const std::size_t name_begin = pos; const std::size_t name_end = text_.find('>', name_begin); @@ -473,7 +477,7 @@ class QwenToolRegionParser { } } - FallbackReason parse_parameter(std::size_t& pos, RawToolCall& call) const { + FallbackReason parse_parameter(std::size_t& pos, RawToolCall& call) { if (!consume(pos, kParamOpen)) { return FallbackReason::MalformedStructure; } const std::size_t name_begin = pos; const std::size_t name_end = text_.find('>', name_begin); @@ -481,18 +485,24 @@ class QwenToolRegionParser { return FallbackReason::MalformedStructure; } const std::string_view name = text_.substr(name_begin, name_end - name_begin); - if (std::any_of(call.parameters.begin(), call.parameters.end(), - [&](const RawParameter& existing) { return existing.name == name; })) { - return FallbackReason::DuplicateParameter; - } + const auto existing = std::find_if(call.parameters.begin(), call.parameters.end(), + [&](const RawParameter& p) { return p.name == name; }); const std::size_t value_begin = name_end + 1; std::size_t value_end = 0; if (!find_parameter_close(value_begin, value_end)) { return FallbackReason::MalformedStructure; } - call.parameters.push_back(RawParameter{ - .name = name, .value = text_.substr(value_begin, value_end - value_begin)}); + const std::string_view value = text_.substr(value_begin, value_end - value_begin); + + // Last occurrence wins, as it would in JSON object syntax, rather than discarding + // an otherwise well-formed call. + if (existing != call.parameters.end()) { + existing->value = value; + ++duplicate_parameters_repaired_; + } else { + call.parameters.push_back(RawParameter{.name = name, .value = value}); + } pos = value_end + kParamClose.size(); return FallbackReason::None; } @@ -538,6 +548,7 @@ class QwenToolRegionParser { std::string_view text_; std::size_t max_name_length_; const Contract& contract_; + std::uint32_t duplicate_parameters_repaired_ = 0; }; GeneratedToolCall normalize_raw_tool_call(const RawToolCall& raw, const Contract& contract, @@ -607,7 +618,7 @@ ParsedToolCallOutput parse_qwen_tool_call_output(const std::string& text, std::vector raw_calls; const std::string_view tool_region = std::string_view(text).substr(first); - const QwenToolRegionParser parser(tool_region, max_tool_name_length, contract); + QwenToolRegionParser parser(tool_region, max_tool_name_length, contract); const FallbackReason failure = parser.parse(raw_calls); if (failure != FallbackReason::None) { out.diagnostics.fallback_reason = failure; @@ -619,6 +630,7 @@ ParsedToolCallOutput parse_qwen_tool_call_output(const std::string& text, out.tool_calls.push_back(normalize_raw_tool_call(raw, contract, out.diagnostics)); } + out.diagnostics.duplicate_parameters_repaired = parser.duplicate_parameters_repaired(); out.diagnostics.structured_call_count = static_cast(out.tool_calls.size()); out.is_tool_call_response = true; return out; diff --git a/src/serve/operational_log.cpp b/src/serve/operational_log.cpp index 75b7d2655a..59b2816eba 100644 --- a/src/serve/operational_log.cpp +++ b/src/serve/operational_log.cpp @@ -298,10 +298,23 @@ std::optional render_tool_call_fallback(const RequestLogConte reason == ninfer::ToolCallParseFallbackReason::None) { return std::nullopt; } + // The reason alone names the verdict, not the markup that earned it. + constexpr std::size_t kMarkupSnippetBytes = 240; + std::string snippet; + if (const std::size_t marker = outcome.text.find(""); + marker != std::string::npos) { + snippet = outcome.text.substr(marker, kMarkupSnippetBytes); + if (outcome.text.size() - marker > kMarkupSnippetBytes) { snippet += "..."; } + for (char& byte : snippet) { + if (byte == '\n' || byte == '\r' || byte == '\t') { byte = ' '; } + } + } + return OperationalRecord{ .severity = OperationalSeverity::Warning, .message = "req#" + std::to_string(context.id) + " tool markup returned as text | " + - pretty_code(ninfer::tool_call_parse_fallback_reason_name(reason)), + pretty_code(ninfer::tool_call_parse_fallback_reason_name(reason)) + + (snippet.empty() ? std::string{} : " | " + snippet), }; } diff --git a/src/serve/request_log.cpp b/src/serve/request_log.cpp index 0b3ece5795..5b4124a553 100644 --- a/src/serve/request_log.cpp +++ b/src/serve/request_log.cpp @@ -89,6 +89,7 @@ Json tool_call_parse_json(const ninfer::ToolCallParseDiagnostics& diagnostics) { {"structured_call_count", diagnostics.structured_call_count}, {"empty_arguments_omitted", diagnostics.empty_arguments_omitted}, {"schema_mismatch_arguments", diagnostics.schema_mismatch_arguments}, + {"duplicate_parameters_repaired", diagnostics.duplicate_parameters_repaired}, {"fallback_reason", ninfer::tool_call_parse_fallback_reason_name(diagnostics.fallback_reason)}}; } diff --git a/tests/test_tool_call_parser.cpp b/tests/test_tool_call_parser.cpp index 06c47f9b40..ad6231edd5 100644 --- a/tests/test_tool_call_parser.cpp +++ b/tests/test_tool_call_parser.cpp @@ -563,11 +563,6 @@ int test_strict_structure_and_active_tool_set() { ninfer::ToolCallParseFallbackReason::MalformedStructure, "missing parameter close was repaired"); - const std::string duplicate = tool_call("configure", {{"value", "first"}, {"value", "second"}}); - failures += - check_rejected(duplicate, contract, ninfer::ToolCallParseFallbackReason::DuplicateParameter, - "duplicate parameter was silently overwritten"); - const std::string unknown_tool = tool_call("other", {{"value", "x"}}); failures += check_rejected(unknown_tool, contract, ninfer::ToolCallParseFallbackReason::UndeclaredTool, @@ -735,8 +730,31 @@ int test_incremental_embedded_parameter_markup() { } // namespace +int test_duplicate_parameter_keeps_last_value() { + int failures = 0; + const fi::ToolCallOutputContract contract = + contract_for("configure", Json{{"value", Json{{"type", "string"}}}}); + const std::string duplicate = tool_call("configure", {{"value", "first"}, {"value", "second"}}); + const auto parsed = fi::parse_qwen_tool_call_output(duplicate, 64, contract); + + failures += check(parsed.is_tool_call_response, "duplicate parameter still fell back to text"); + failures += check(parsed.content.empty(), "duplicate parameter left prose behind"); + failures += check(parsed.tool_calls.size() == 1, "duplicate parameter did not yield one call"); + if (parsed.tool_calls.size() == 1) { + failures += check(parsed.tool_calls.front().arguments_json == R"({"value":"second"})", + "duplicate parameter did not keep the last value"); + } + failures += check(parsed.diagnostics.fallback_reason == + ninfer::ToolCallParseFallbackReason::None, + "duplicate parameter still reported a fallback reason"); + failures += check(parsed.diagnostics.duplicate_parameters_repaired == 1, + "duplicate parameter repair was not recorded in diagnostics"); + return failures; +} + int main() { int failures = 0; + failures += test_duplicate_parameter_keeps_last_value(); failures += test_basic_legacy_parsing(); failures += test_multiple_calls(); failures += test_declared_strings_preserve_text();