FIX: coerce values for XML tool calls

SamSaffron · SamSaffron · commit 9bff94b146dc · 2025-05-15T15:12:48.000+10:00
diff --git a/lib/completions/endpoints/base.rb b/lib/completions/endpoints/base.rb
@@ -166,6 +166,7 @@ def perform_completion!(
               xml_tool_processor =
                 XmlToolProcessor.new(
                   partial_tool_calls: partial_tool_calls,
+                  tool_definitions: dialect.prompt.tools,
                 ) if xml_tools_enabled? && dialect.prompt.has_tools?
 
               to_strip = xml_tags_to_strip(dialect)
diff --git a/lib/completions/tool_definition.rb b/lib/completions/tool_definition.rb
@@ -165,7 +165,7 @@ def coerce_parameters(params)
         return result if !params.is_a?(Hash)
 
         @parameters.each do |param_def|
-          param_name = param_def.name
+          param_name = param_def.name.to_sym
 
           # Skip if parameter is not provided and not required
           next if !params.key?(param_name) && !param_def.required
diff --git a/lib/completions/xml_tool_processor.rb b/lib/completions/xml_tool_processor.rb
@@ -7,13 +7,14 @@
 module DiscourseAi
   module Completions
     class XmlToolProcessor
-      def initialize(partial_tool_calls: false)
+      def initialize(partial_tool_calls: false, tool_definitions: nil)
         @buffer = +""
         @function_buffer = +""
         @should_cancel = false
         @in_tool = false
         @partial_tool_calls = partial_tool_calls
         @partial_tools = [] if @partial_tool_calls
+        @tool_definitions = tool_definitions
       end
 
       def <<(text)
@@ -71,7 +72,7 @@ def finish
 
         idx = -1
         parse_malformed_xml(@function_buffer).map do |tool|
-          ToolCall.new(
+          new_tool_call(
             id: "tool_#{idx += 1}",
             name: tool[:tool_name],
             parameters: tool[:parameters],
@@ -85,6 +86,13 @@ def should_cancel?
 
       private
 
+      def new_tool_call(id:, name:, parameters:)
+        if tool_def = @tool_definitions&.find { |d| d.name == name }
+          parameters = tool_def.coerce_parameters(parameters)
+        end
+        ToolCall.new(id:, name:, parameters:)
+      end
+
       def add_to_function_buffer(text)
         @function_buffer << text
         detect_partial_tool_calls(@function_buffer, text) if @partial_tool_calls
@@ -119,7 +127,7 @@ def parse_partial_tool_call(buffer)
             current_tool = @partial_tools.last
             if !current_tool || current_tool.name != match[0].strip
               current_tool =
-                ToolCall.new(
+                new_tool_call(
                   id: "tool_#{@partial_tools.length}",
                   name: match[0].strip,
                   parameters: params,
diff --git a/spec/lib/completions/endpoints/open_ai_spec.rb b/spec/lib/completions/endpoints/open_ai_spec.rb
@@ -542,11 +542,24 @@ def request_body(prompt, stream: false, tool_call: false)
         <parameters>
         <location>Sydney</location>
         <unit>c</unit>
+        <is_it_hot>true</is_it_hot>
         </parameters>
         </invoke>
         </function_calls>
       XML
 
+      let(:weather_tool) do
+        {
+          name: "get_weather",
+          description: "get weather",
+          parameters: [
+            { name: "location", type: "string", description: "location", required: true },
+            { name: "unit", type: "string", description: "unit", required: true, enum: %w[c f] },
+            { name: "is_it_hot", type: "boolean", description: "is it hot" },
+          ],
+        }
+      end
+
       it "parses XML tool calls" do
         response = {
           id: "chatcmpl-6sZfAb30Rnv9Q7ufzFwvQsMpjZh8S",
@@ -574,7 +587,7 @@ def request_body(prompt, stream: false, tool_call: false)
         body = nil
         open_ai_mock.stub_raw(response, body_blk: proc { |inner_body| body = inner_body })
 
-        dialect = compliance.dialect(prompt: compliance.generic_prompt(tools: tools))
+        dialect = compliance.dialect(prompt: compliance.generic_prompt(tools: [weather_tool]))
         tool_call = endpoint.perform_completion!(dialect, user)
 
         body_parsed = JSON.parse(body, symbolize_names: true)
@@ -583,7 +596,7 @@ def request_body(prompt, stream: false, tool_call: false)
         expect(body_parsed[:messages][0][:content]).to include("<function_calls>")
 
         expect(tool_call.name).to eq("get_weather")
-        expect(tool_call.parameters).to eq({ location: "Sydney", unit: "c" })
+        expect(tool_call.parameters).to eq({ location: "Sydney", unit: "c", is_it_hot: true })
       end
     end
 
diff --git a/spec/lib/completions/tool_definition_spec.rb b/spec/lib/completions/tool_definition_spec.rb
@@ -132,13 +132,13 @@
       end
 
       it "converts numbers to strings" do
-        result = tool.coerce_parameters({ "name" => 123 })
-        expect(result["name"]).to eq("123")
+        result = tool.coerce_parameters(name: 123)
+        expect(result[:name]).to eq("123")
       end
 
       it "converts booleans to strings" do
-        result = tool.coerce_parameters({ "name" => true })
-        expect(result["name"]).to eq("true")
+        result = tool.coerce_parameters(name: true)
+        expect(result[:name]).to eq("true")
       end
     end
 
@@ -156,18 +156,18 @@
       end
 
       it "converts string numbers to floats" do
-        result = tool.coerce_parameters({ "price" => "42.99" })
-        expect(result["price"]).to eq(42.99)
+        result = tool.coerce_parameters(price: "42.99")
+        expect(result[:price]).to eq(42.99)
       end
 
       it "converts integers to floats" do
-        result = tool.coerce_parameters({ "price" => 42 })
-        expect(result["price"]).to eq(42.0)
+        result = tool.coerce_parameters(price: 42)
+        expect(result[:price]).to eq(42.0)
       end
 
       it "returns nil for invalid number strings" do
-        result = tool.coerce_parameters({ "price" => "not a number" })
-        expect(result["price"]).to be_nil
+        result = tool.coerce_parameters(price: "not a number")
+        expect(result[:price]).to be_nil
       end
     end
 
@@ -190,18 +190,18 @@
       end
 
       it "converts string elements to integers" do
-        result = tool.coerce_parameters({ "numbers" => %w[1 2 3] })
-        expect(result["numbers"]).to eq([1, 2, 3])
+        result = tool.coerce_parameters(numbers: %w[1 2 3])
+        expect(result[:numbers]).to eq([1, 2, 3])
       end
 
       it "parses JSON strings into arrays and converts elements" do
-        result = tool.coerce_parameters({ "numbers" => "[1, 2, 3]" })
-        expect(result["numbers"]).to eq([1, 2, 3])
+        result = tool.coerce_parameters(numbers: "[1, 2, 3]")
+        expect(result[:numbers]).to eq([1, 2, 3])
       end
 
       it "handles mixed type arrays appropriately" do
-        result = tool.coerce_parameters({ "numbers" => [1, "two", 3.5] })
-        expect(result["numbers"]).to eq([1, nil, 3])
+        result = tool.coerce_parameters(numbers: [1, "two", 3.5])
+        expect(result[:numbers]).to eq([1, nil, 3])
       end
     end
 
@@ -231,9 +231,9 @@
       end
 
       it "includes missing required parameters as nil" do
-        result = tool.coerce_parameters({ "optional_param" => "value" })
-        expect(result["required_param"]).to be_nil
-        expect(result["optional_param"]).to eq("value")
+        result = tool.coerce_parameters(optional_param: "value")
+        expect(result[:required_param]).to be_nil
+        expect(result[:optional_param]).to eq("value")
       end
 
       it "skips missing optional parameters" do
@@ -257,16 +257,16 @@
       end
 
       it "preserves true/false values" do
-        result = tool.coerce_parameters({ "flag" => true })
-        expect(result["flag"]).to be true
+        result = tool.coerce_parameters(flag: true)
+        expect(result[:flag]).to be true
       end
 
       it "converts 'true'/'false' strings to booleans" do
-        result = tool.coerce_parameters({ "flag" => "true" })
-        expect(result["flag"]).to be true
+        result = tool.coerce_parameters({ flag: true })
+        expect(result[:flag]).to be true
 
-        result = tool.coerce_parameters({ "flag" => "False" })
-        expect(result["flag"]).to be false
+        result = tool.coerce_parameters({ flag: "False" })
+        expect(result[:flag]).to be false
       end
 
       it "returns nil for invalid boolean strings" do
diff --git a/spec/lib/completions/xml_tool_processor_spec.rb b/spec/lib/completions/xml_tool_processor_spec.rb
@@ -93,10 +93,24 @@
       <parameters>
        <hello>world</hello>
        <test>value</test>
+       <bool>true</bool>
       </parameters>
       </invoke>
     XML
 
+    tool_definition =
+      DiscourseAi::Completions::ToolDefinition.from_hash(
+        name: "hello",
+        description: "hello world",
+        parameters: [
+          { name: "hello", type: "string", description: "hello" },
+          { name: "test", type: "string", description: "test" },
+          { name: "bool", type: "boolean", description: "bool" },
+        ],
+      )
+
+    processor = DiscourseAi::Completions::XmlToolProcessor.new(tool_definitions: [tool_definition])
+
     result = []
     result << (processor << "hello")
     result << (processor << xml)
@@ -109,6 +123,7 @@
         parameters: {
           hello: "world",
           test: "value",
+          bool: true,
         },
       )
     expect(result).to eq([["hello"], [" world"], [tool_call]])
