DEV: use a proper object for tool definition (#1337)

* DEV: use a proper object for tool definition

This moves away from using a loose hash to define tools, which
is error prone.

Instead given a proper object we will also be able to coerce the
return values to match tool definition correctly

* fix xml tools

* fix anthropic tools

* fix specs... a few more to go

* specs are passing

* FIX: coerce values for XML tool calls

* Update spec/lib/completions/tool_definition_spec.rb

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

lib/completions/dialects/claude_tools.rb

@@ -9,33 +9,8 @@ module DiscourseAi
         end
 
         def translated_tools
-          # Transform the raw tools into the required Anthropic Claude API format
           raw_tools.map do |t|
-            properties = {}
+            { name: t.name, description: t.description, input_schema: t.parameters_json_schema }
-            required = []
-
-            if t[:parameters]
-              properties = {}
-
-              t[:parameters].each do |param|
-                mapped = { type: param[:type], description: param[:description] }
-                mapped[:items] = { type: param[:item_type] } if param[:item_type]
-                mapped[:enum] = param[:enum] if param[:enum]
-                properties[param[:name]] = mapped
-              end
-              required =
-                t[:parameters].select { |param| param[:required] }.map { |param| param[:name] }
-            end
-
-            {
-              name: t[:name],
-              description: t[:description],
-              input_schema: {
-                type: "object",
-                properties: properties,
-                required: required,
-              },
-            }
           end
         end
 

lib/completions/dialects/cohere_tools.rb

@@ -38,27 +38,24 @@ module DiscourseAi
         end
 
         def translated_tools
-          raw_tools.map do |t|
+          raw_tools.map do |tool|
-            tool = t.dup
+            defs = {}
 
-            tool[:parameter_definitions] = t[:parameters]
+            tool.parameters.each do |p|
-              .to_a
+              name = p.name
-              .reduce({}) do |memo, p|
+              defs[name] = {
-                name = p[:name]
+                description: p.description,
-                memo[name] = {
+                type: cohere_type(p.type, p.item_type),
-                  description: p[:description],
+                required: p.required,
-                  type: cohere_type(p[:type], p[:item_type]),
-                  required: p[:required],
               }
 
-                memo[name][:default] = p[:default] if p[:default]
+              #defs[name][:default] = p.default if p.default
-                memo
             end
 
             {
-              name: tool[:name] == "search" ? "search_local" : tool[:name],
+              name: tool.name == "search" ? "search_local" : tool.name,
-              description: tool[:description],
+              description: tool.description,
-              parameter_definitions: tool[:parameter_definitions],
+              parameter_definitions: defs,
             }
           end
         end
@@ -72,6 +69,7 @@ module DiscourseAi
         attr_reader :raw_tools
 
         def cohere_type(type, item_type)
+          type = type.to_s
           case type
           when "string"
             "str"

lib/completions/dialects/gemini.rb

@@ -47,22 +47,8 @@ module DiscourseAi
 
           translated_tools =
             prompt.tools.map do |t|
-              tool = t.slice(:name, :description)
+              tool = { name: t.name, description: t.description }
-
+              tool[:parameters] = t.parameters_json_schema if t.parameters
-              if t[:parameters]
-                tool[:parameters] = t[:parameters].reduce(
-                  { type: "object", required: [], properties: {} },
-                ) do |memo, p|
-                  name = p[:name]
-                  memo[:required] << name if p[:required]
-
-                  memo[:properties][name] = p.except(:name, :required, :item_type)
-
-                  memo[:properties][name][:items] = { type: p[:item_type] } if p[:item_type]
-                  memo
-                end
-              end
-
               tool
             end
 

lib/completions/dialects/nova_tools.rb

@@ -17,10 +17,10 @@ module DiscourseAi
               @raw_tools.map do |tool|
                 {
                   toolSpec: {
-                    name: tool[:name],
+                    name: tool.name,
-                    description: tool[:description],
+                    description: tool.description,
                     inputSchema: {
-                      json: convert_tool_to_input_schema(tool),
+                      json: tool.parameters_json_schema,
                     },
                   },
                 }
@@ -51,32 +51,6 @@ module DiscourseAi
             },
           }
         end
-
-        private
-
-        def convert_tool_to_input_schema(tool)
-          tool = tool.transform_keys(&:to_sym)
-          properties = {}
-          tool[:parameters].each do |param|
-            schema = {}
-            type = param[:type]
-            type = "string" if !%w[string number boolean integer array].include?(type)
-
-            schema[:type] = type
-
-            if enum = param[:enum]
-              schema[:enum] = enum
-            end
-
-            schema[:items] = { type: param[:item_type] } if type == "array"
-
-            schema[:required] = true if param[:required]
-
-            properties[param[:name]] = schema
-          end
-
-          { type: "object", properties: properties }
-        end
       end
     end
   end

lib/completions/dialects/ollama_tools.rb

@@ -14,23 +14,15 @@ module DiscourseAi
         end
 
         def translated_tools
-          raw_tools.map do |t|
+          raw_tools.map do |tool|
-            tool = t.dup
+            {
-
+              type: "function",
-            tool[:parameters] = t[:parameters]
+              function: {
-              .to_a
+                name: tool.name,
-              .reduce({ type: "object", properties: {}, required: [] }) do |memo, p|
+                description: tool.description,
-                name = p[:name]
+                parameters: tool.parameters_json_schema,
-                memo[:required] << name if p[:required]
+              },
-
+            }
-                except = %i[name required item_type]
-                except << :enum if p[:enum].blank?
-
-                memo[:properties][name] = p.except(*except)
-                memo
-              end
-
-            { type: "function", function: tool }
           end
         end
 

lib/completions/dialects/open_ai_tools.rb

@@ -9,25 +9,15 @@ module DiscourseAi
         end
 
         def translated_tools
-          raw_tools.map do |t|
+          raw_tools.map do |tool|
-            tool = t.dup
+            {
-
+              type: "function",
-            tool[:parameters] = t[:parameters]
+              function: {
-              .to_a
+                name: tool.name,
-              .reduce({ type: "object", properties: {}, required: [] }) do |memo, p|
+                description: tool.description,
-                name = p[:name]
+                parameters: tool.parameters_json_schema,
-                memo[:required] << name if p[:required]
+              },
-
+            }
-                except = %i[name required item_type]
-                except << :enum if p[:enum].blank?
-
-                memo[:properties][name] = p.except(*except)
-
-                memo[:properties][name][:items] = { type: p[:item_type] } if p[:item_type]
-                memo
-              end
-
-            { type: "function", function: tool }
           end
         end
 

lib/completions/dialects/xml_tools.rb

@@ -9,33 +9,37 @@ module DiscourseAi
         end
 
         def translated_tools
-          raw_tools.reduce(+"") do |tools, function|
+          result = +""
+
+          raw_tools.each do |tool|
             parameters = +""
-            if function[:parameters].present?
+            if tool.parameters.present?
-              function[:parameters].each do |parameter|
+              tool.parameters.each do |parameter|
                 parameters << <<~PARAMETER
                   <parameter>
-                  <name>#{parameter[:name]}</name>
+                  <name>#{parameter.name}</name>
-                  <type>#{parameter[:type]}</type>
+                  <type>#{parameter.type}</type>
-                  <description>#{parameter[:description]}</description>
+                  <description>#{parameter.description}</description>
-                  <required>#{parameter[:required]}</required>
+                  <required>#{parameter.required}</required>
                 PARAMETER
-                if parameter[:enum]
+                if parameter.item_type
-                  parameters << "<options>#{parameter[:enum].join(",")}</options>\n"
+                  parameters << "<array_item_type>#{parameter.item_type}</array_item_type>\n"
                 end
+                parameters << "<options>#{parameter.enum.join(",")}</options>\n" if parameter.enum
                 parameters << "</parameter>\n"
               end
             end
 
-            tools << <<~TOOLS
+            result << <<~TOOLS
               <tool_description>
-              <tool_name>#{function[:name]}</tool_name>
+              <tool_name>#{tool.name}</tool_name>
-              <description>#{function[:description]}</description>
+              <description>#{tool.description}</description>
               <parameters>
               #{parameters}</parameters>
               </tool_description>
             TOOLS
           end
+          result
         end
 
         def instructions
@@ -43,8 +47,7 @@ module DiscourseAi
 
           @instructions ||=
             begin
-              has_arrays =
+              has_arrays = raw_tools.any? { |tool| tool.parameters&.any? { |p| p.type == "array" } }
-                raw_tools.any? { |tool| tool[:parameters]&.any? { |p| p[:type] == "array" } }
 
               (<<~TEXT).strip
               #{tool_preamble(include_array_tip: has_arrays)}

lib/completions/endpoints/base.rb

@@ -28,7 +28,7 @@ module DiscourseAi
               DiscourseAi::Completions::Endpoints::OpenRouter,
             ]
 
-            endpoints << DiscourseAi::Completions::Endpoints::Ollama if Rails.env.development?
+            endpoints << DiscourseAi::Completions::Endpoints::Ollama if !Rails.env.production?
 
             if Rails.env.test? || Rails.env.development?
               endpoints << DiscourseAi::Completions::Endpoints::Fake
@@ -166,6 +166,7 @@ module DiscourseAi
               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)

lib/completions/prompt.rb

@@ -5,8 +5,8 @@ module DiscourseAi
     class Prompt
       INVALID_TURN = Class.new(StandardError)
 
-      attr_reader :messages
+      attr_reader :messages, :tools
-      attr_accessor :tools, :topic_id, :post_id, :max_pixels, :tool_choice
+      attr_accessor :topic_id, :post_id, :max_pixels, :tool_choice
 
       def initialize(
         system_message_text = nil,
@@ -37,10 +37,25 @@ module DiscourseAi
         @messages.each { |message| validate_message(message) }
         @messages.each_cons(2) { |last_turn, new_turn| validate_turn(last_turn, new_turn) }
 
-        @tools = tools
+        self.tools = tools
         @tool_choice = tool_choice
       end
 
+      def tools=(tools)
+        raise ArgumentError, "tools must be an array" if !tools.is_a?(Array) && !tools.nil?
+
+        @tools =
+          tools.map do |tool|
+            if tool.is_a?(Hash)
+              ToolDefinition.from_hash(tool)
+            elsif tool.is_a?(ToolDefinition)
+              tool
+            else
+              raise ArgumentError, "tool must be a hash or a ToolDefinition was #{tool.class}"
+            end
+          end
+      end
+
       # this new api tries to create symmetry between responses and prompts
       # this means anything we get back from the model via endpoint can be easily appended
       def push_model_response(response)

lib/completions/tool_definition.rb

@@ -0,0 +1,252 @@
+# frozen_string_literal: true
+
+module DiscourseAi
+  module Completions
+    class ToolDefinition
+      class ParameterDefinition
+        ALLOWED_TYPES = %i[string boolean integer array number].freeze
+        ALLOWED_KEYS = %i[name description type required enum item_type].freeze
+
+        attr_reader :name, :description, :type, :required, :enum, :item_type
+
+        def self.from_hash(hash)
+          extra_keys = hash.keys - ALLOWED_KEYS
+          if !extra_keys.empty?
+            raise ArgumentError, "Unexpected keys in parameter definition: #{extra_keys}"
+          end
+
+          new(
+            name: hash[:name],
+            description: hash[:description],
+            type: hash[:type],
+            required: hash[:required],
+            enum: hash[:enum],
+            item_type: hash[:item_type],
+          )
+        end
+
+        def initialize(name:, description:, type:, required: false, enum: nil, item_type: nil)
+          raise ArgumentError, "name must be a string" if !name.is_a?(String) || name.empty?
+
+          if !description.is_a?(String) || description.empty?
+            raise ArgumentError, "description must be a string"
+          end
+
+          type_sym = type.to_sym
+
+          if !ALLOWED_TYPES.include?(type_sym)
+            raise ArgumentError, "type must be one of: #{ALLOWED_TYPES.join(", ")}"
+          end
+
+          # Validate enum if provided
+          if enum
+            raise ArgumentError, "enum must be an array" if !enum.is_a?(Array)
+
+            # Validate enum entries match the specified type
+            enum.each do |value|
+              case type_sym
+              when :string
+                if !value.is_a?(String)
+                  raise ArgumentError, "enum values must be strings for type 'string'"
+                end
+              when :boolean
+                if ![true, false].include?(value)
+                  raise ArgumentError, "enum values must be booleans for type 'boolean'"
+                end
+              when :integer
+                if !value.is_a?(Integer)
+                  raise ArgumentError, "enum values must be integers for type 'integer'"
+                end
+              when :number
+                if !value.is_a?(Numeric)
+                  raise ArgumentError, "enum values must be numbers for type 'number'"
+                end
+              when :array
+                if !value.is_a?(Array)
+                  raise ArgumentError, "enum values must be arrays for type 'array'"
+                end
+              end
+            end
+          end
+
+          if item_type && type_sym != :array
+            raise ArgumentError, "item_type can only be specified for array type"
+          end
+
+          if item_type
+            if !ALLOWED_TYPES.include?(item_type.to_sym)
+              raise ArgumentError, "item type must be one of: #{ALLOWED_TYPES.join(", ")}"
+            end
+          end
+
+          @name = name
+          @description = description
+          @type = type_sym
+          @required = !!required
+          @enum = enum
+          @item_type = item_type ? item_type.to_sym : nil
+        end
+
+        def to_h
+          result = { name: @name, description: @description, type: @type, required: @required }
+          result[:enum] = @enum if @enum
+          result[:item_type] = @item_type if @item_type
+          result
+        end
+      end
+
+      def parameters_json_schema
+        properties = {}
+        required = []
+
+        result = { type: "object", properties: properties, required: required }
+
+        parameters.each do |param|
+          name = param.name
+          required << name if param.required
+          properties[name] = { type: param.type, description: param.description }
+          properties[name][:items] = { type: param.item_type } if param.item_type
+          properties[name][:enum] = param.enum if param.enum
+        end
+
+        result
+      end
+
+      attr_reader :name, :description, :parameters
+
+      def self.from_hash(hash)
+        allowed_keys = %i[name description parameters]
+        extra_keys = hash.keys - allowed_keys
+        if !extra_keys.empty?
+          raise ArgumentError, "Unexpected keys in tool definition: #{extra_keys}"
+        end
+
+        params = hash[:parameters] || []
+        parameter_objects =
+          params.map do |param|
+            if param.is_a?(Hash)
+              ParameterDefinition.from_hash(param)
+            else
+              param
+            end
+          end
+
+        new(name: hash[:name], description: hash[:description], parameters: parameter_objects)
+      end
+
+      def initialize(name:, description:, parameters: [])
+        raise ArgumentError, "name must be a string" if !name.is_a?(String) || name.empty?
+
+        if !description.is_a?(String) || description.empty?
+          raise ArgumentError, "description must be a string"
+        end
+
+        raise ArgumentError, "parameters must be an array" if !parameters.is_a?(Array)
+
+        # Check for duplicated parameter names
+        param_names = parameters.map { |p| p.name }
+        duplicates = param_names.select { |param_name| param_names.count(param_name) > 1 }.uniq
+        if !duplicates.empty?
+          raise ArgumentError, "Duplicate parameter names found: #{duplicates.join(", ")}"
+        end
+
+        @name = name
+        @description = description
+        @parameters = parameters
+      end
+
+      def to_h
+        { name: @name, description: @description, parameters: @parameters.map(&:to_h) }
+      end
+
+      def coerce_parameters(params)
+        result = {}
+
+        return result if !params.is_a?(Hash)
+
+        @parameters.each do |param_def|
+          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
+
+          # Handle required but missing parameters
+          if !params.key?(param_name) && param_def.required
+            result[param_name] = nil
+            next
+          end
+
+          value = params[param_name]
+
+          # For array type, handle item coercion
+          if param_def.type == :array
+            result[param_name] = coerce_array_value(value, param_def.item_type)
+          else
+            result[param_name] = coerce_single_value(value, param_def.type)
+          end
+        end
+
+        result
+      end
+
+      private
+
+      def coerce_array_value(value, item_type)
+        # Handle non-array input by attempting to parse JSON strings
+        if !value.is_a?(Array)
+          if value.is_a?(String)
+            begin
+              parsed = JSON.parse(value)
+              value = parsed.is_a?(Array) ? parsed : nil
+            rescue JSON::ParserError
+              return nil
+            end
+          else
+            return nil
+          end
+        end
+
+        # No item type specified, return the array as is
+        return value if !item_type
+
+        # Coerce each item in the array
+        value.map { |item| coerce_single_value(item, item_type) }
+      end
+
+      def coerce_single_value(value, type)
+        result = nil
+
+        case type
+        when :string
+          result = value.to_s
+        when :integer
+          if value.is_a?(Integer)
+            result = value
+          elsif value.is_a?(Float)
+            result = value.to_i
+          elsif value.is_a?(String) && value.match?(/\A-?\d+\z/)
+            result = value.to_i
+          end
+        when :number
+          if value.is_a?(Numeric)
+            result = value.to_f
+          elsif value.is_a?(String) && value.match?(/\A-?\d+(\.\d+)?\z/)
+            result = value.to_f
+          end
+        when :boolean
+          if value == true || value == false
+            result = value
+          elsif value.is_a?(String)
+            if value.downcase == "true"
+              result = true
+            elsif value.downcase == "false"
+              result = false
+            end
+          end
+        end
+
+        result
+      end
+    end
+  end
+end

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 @@ module DiscourseAi
 
         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 @@ module DiscourseAi
 
       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 @@ module DiscourseAi
             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,

spec/lib/completions/dialects/chat_gpt_spec.rb

@@ -99,14 +99,18 @@ RSpec.describe DiscourseAi::Completions::Dialects::ChatGpt do
     it "returns a list of available tools" do
       open_ai_tool_f = {
         function: {
-          description: context.tools.first[:description],
+          description: context.tools.first.description,
-          name: context.tools.first[:name],
+          name: context.tools.first.name,
           parameters: {
             properties:
-              context.tools.first[:parameters].reduce({}) do |memo, p|
+              context
-                memo[p[:name]] = { description: p[:description], type: p[:type] }
+                .tools
+                .first
+                .parameters
+                .reduce({}) do |memo, p|
+                  memo[p.name] = { description: p.description, type: p.type }
 
-                memo[p[:name]][:enum] = p[:enum] if p[:enum]
+                  memo[p.name][:enum] = p.enum if p.enum
 
                   memo
                 end,

spec/lib/completions/dialects/dialect_context.rb

@@ -115,6 +115,6 @@ class DialectContext
           },
         ],
       },
-    ]
+    ].map { |tool| DiscourseAi::Completions::ToolDefinition.from_hash(tool) }
   end
 end

spec/lib/completions/dialects/gemini_spec.rb

@@ -108,11 +108,11 @@ RSpec.describe DiscourseAi::Completions::Dialects::Gemini do
               required: %w[location unit],
               properties: {
                 "location" => {
-                  type: "string",
+                  type: :string,
                   description: "the city name",
                 },
                 "unit" => {
-                  type: "string",
+                  type: :string,
                   description: "the unit of measurement celcius c or fahrenheit f",
                   enum: %w[c f],
                 },
@@ -121,7 +121,6 @@ RSpec.describe DiscourseAi::Completions::Dialects::Gemini do
           },
         ],
       }
-
       expect(context.dialect_tools).to contain_exactly(gemini_tools)
     end
   end

spec/lib/completions/dialects/nova_spec.rb

@@ -84,8 +84,7 @@ RSpec.describe DiscourseAi::Completions::Dialects::Nova do
         dialect = nova_dialect_klass.new(prompt, llm_model)
         translated = dialect.translate
 
-        expect(translated.tool_config).to eq(
+        expected = {
-          {
           tools: [
             {
               toolSpec: {
@@ -96,17 +95,19 @@ RSpec.describe DiscourseAi::Completions::Dialects::Nova do
                     type: "object",
                     properties: {
                       "location" => {
-                          type: "string",
+                        type: :string,
-                          required: true,
+                        description: "the city name",
                       },
                     },
+                    required: ["location"],
                   },
                 },
               },
             },
           ],
-          },
+        }
-        )
+
+        expect(translated.tool_config).to eq(expected)
       end
     end
 

spec/lib/completions/dialects/ollama_spec.rb

@@ -5,6 +5,7 @@ require_relative "dialect_context"
 RSpec.describe DiscourseAi::Completions::Dialects::Ollama do
   fab!(:model) { Fabricate(:ollama_model) }
   let(:context) { DialectContext.new(described_class, model) }
+  let(:dialect_class) { DiscourseAi::Completions::Dialects::Dialect.dialect_for(model) }
 
   describe "#translate" do
     context "when native tool support is enabled" do
@@ -59,35 +60,49 @@ RSpec.describe DiscourseAi::Completions::Dialects::Ollama do
   describe "#tools" do
     context "when native tools are enabled" do
       it "returns the translated tools from the OllamaTools class" do
-        tool = instance_double(DiscourseAi::Completions::Dialects::OllamaTools)
+        model.update!(provider_params: { enable_native_tool: true })
 
-        allow(model).to receive(:lookup_custom_param).with("enable_native_tool").and_return(true)
+        tool = { name: "noop", description: "do nothing" }
-        allow(tool).to receive(:translated_tools)
+        messages = [
-        allow(DiscourseAi::Completions::Dialects::OllamaTools).to receive(:new).and_return(tool)
+          { type: :user, content: "echo away" },
+          { type: :tool_call, content: "{}", name: "noop" },
+          { type: :tool, content: "{}", name: "noop" },
+        ]
+        prompt = DiscourseAi::Completions::Prompt.new("a bot", tools: [tool], messages: messages)
+        dialect = dialect_class.new(prompt, model)
 
-        context.dialect_tools
+        expected = [
-
+          { role: "system", content: "a bot" },
-        expect(DiscourseAi::Completions::Dialects::OllamaTools).to have_received(:new).with(
+          { role: "user", content: "echo away" },
-          context.prompt.tools,
+          {
-        )
+            role: "assistant",
-        expect(tool).to have_received(:translated_tools)
+            content: nil,
+            tool_calls: [{ type: "function", function: { name: "noop" } }],
+          },
+          { role: "tool", content: "{}", name: "noop" },
+        ]
+        expect(dialect.translate).to eq(expected)
       end
     end
 
     context "when native tools are disabled" do
       it "returns the translated tools from the XmlTools class" do
-        tool = instance_double(DiscourseAi::Completions::Dialects::XmlTools)
+        model.update!(provider_params: { enable_native_tool: false })
 
-        allow(model).to receive(:lookup_custom_param).with("enable_native_tool").and_return(false)
+        tool = { name: "noop", description: "do nothing" }
-        allow(tool).to receive(:translated_tools)
+        messages = [
-        allow(DiscourseAi::Completions::Dialects::XmlTools).to receive(:new).and_return(tool)
+          { type: :user, content: "echo away" },
+          { type: :tool_call, content: "{}", name: "noop" },
+          { type: :tool, content: "{}", name: "noop" },
+        ]
+        prompt = DiscourseAi::Completions::Prompt.new("a bot", tools: [tool], messages: messages)
+        dialect = dialect_class.new(prompt, model)
 
-        context.dialect_tools
+        expected = %w[system user assistant user]
+        roles = dialect.translate.map { |x| x[:role] }
 
-        expect(DiscourseAi::Completions::Dialects::XmlTools).to have_received(:new).with(
+        # notice, no tool role
-          context.prompt.tools,
+        expect(roles).to eq(expected)
-        )
-        expect(tool).to have_received(:translated_tools)
       end
     end
   end

spec/lib/completions/dialects/ollama_tools_spec.rb

@@ -5,8 +5,7 @@ require_relative "dialect_context"
 RSpec.describe DiscourseAi::Completions::Dialects::OllamaTools do
   describe "#translated_tools" do
     it "translates a tool from our generic format to the Ollama format" do
-      tools = [
+      tool = {
-        {
         name: "github_file_content",
         description: "Retrieves the content of specified GitHub files",
         parameters: [
@@ -30,9 +29,9 @@ RSpec.describe DiscourseAi::Completions::Dialects::OllamaTools do
             required: false,
           },
         ],
-        },
+      }
-      ]
 
+      tools = [DiscourseAi::Completions::ToolDefinition.from_hash(tool)]
       ollama_tools = described_class.new(tools)
 
       translated_tools = ollama_tools.translated_tools
@@ -49,16 +48,19 @@ RSpec.describe DiscourseAi::Completions::Dialects::OllamaTools do
                 properties: {
                   "repo_name" => {
                     description: "The name of the GitHub repository (e.g., 'discourse/discourse')",
-                    type: "string",
+                    type: :string,
                   },
                   "file_paths" => {
                     description: "The paths of the files to retrieve within the repository",
-                    type: "array",
+                    type: :array,
+                    items: {
+                      type: :string,
+                    },
                   },
                   "branch" => {
                     description:
                       "The branch or commit SHA to retrieve the files from (default: 'main')",
-                    type: "string",
+                    type: :string,
                   },
                 },
                 required: %w[repo_name file_paths],

spec/lib/completions/endpoints/nova_spec.rb

@@ -196,10 +196,11 @@ RSpec.describe DiscourseAi::Completions::Endpoints::AwsBedrock do
               "inputSchema" => {
                 "json" => {
                   "type" => "object",
+                  "required" => ["timezone"],
                   "properties" => {
                     "timezone" => {
                       "type" => "string",
-                      "required" => true,
+                      "description" => "The timezone",
                     },
                   },
                 },
@@ -268,9 +269,10 @@ RSpec.describe DiscourseAi::Completions::Endpoints::AwsBedrock do
                   properties: {
                     timezone: {
                       type: "string",
-                      required: true,
+                      description: "The timezone",
                     },
                   },
+                  required: ["timezone"],
                 },
               },
             },

spec/lib/completions/endpoints/open_ai_spec.rb

@@ -542,11 +542,24 @@ RSpec.describe DiscourseAi::Completions::Endpoints::OpenAi do
         <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 @@ RSpec.describe DiscourseAi::Completions::Endpoints::OpenAi do
         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 @@ RSpec.describe DiscourseAi::Completions::Endpoints::OpenAi do
         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
 

spec/lib/completions/tool_definition_spec.rb

@@ -0,0 +1,338 @@
+# frozen_string_literal: true
+
+RSpec.describe DiscourseAi::Completions::ToolDefinition do
+  # Test case 1: Basic tool definition creation
+  describe "#initialize" do
+    it "creates a tool with name, description and parameters" do
+      param =
+        DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+          name: "query",
+          description: "The search query",
+          type: :string,
+          required: true,
+        )
+
+      tool =
+        described_class.new(
+          name: "search_engine",
+          description: "Searches the web for information",
+          parameters: [param],
+        )
+
+      expect(tool.name).to eq("search_engine")
+      expect(tool.description).to eq("Searches the web for information")
+      expect(tool.parameters.size).to eq(1)
+      expect(tool.parameters.first.name).to eq("query")
+    end
+  end
+
+  # Test case 2: Creating tool from hash
+  describe ".from_hash" do
+    it "creates a tool from a hash representation" do
+      hash = {
+        name: "calculator",
+        description: "Performs math operations",
+        parameters: [
+          {
+            name: "expression",
+            description: "Math expression to evaluate",
+            type: "string",
+            required: true,
+          },
+        ],
+      }
+
+      tool = described_class.from_hash(hash)
+
+      expect(tool.name).to eq("calculator")
+      expect(tool.description).to eq("Performs math operations")
+      expect(tool.parameters.size).to eq(1)
+      expect(tool.parameters.first.name).to eq("expression")
+      expect(tool.parameters.first.type).to eq(:string)
+    end
+
+    it "rejects a hash with extra keys" do
+      hash = {
+        name: "calculator",
+        description: "Performs math operations",
+        parameters: [],
+        extra_key: "should not be here",
+      }
+
+      expect { described_class.from_hash(hash) }.to raise_error(ArgumentError, /Unexpected keys/)
+    end
+  end
+
+  # Test case 3: Parameter with enum validation
+  describe DiscourseAi::Completions::ToolDefinition::ParameterDefinition do
+    context "with enum values" do
+      it "accepts valid enum values matching the type" do
+        param =
+          described_class.new(
+            name: "operation",
+            description: "Math operation to perform",
+            type: :string,
+            enum: %w[add subtract multiply divide],
+          )
+
+        expect(param.enum).to eq(%w[add subtract multiply divide])
+      end
+
+      it "rejects enum values that don't match the specified type" do
+        expect {
+          described_class.new(
+            name: "operation",
+            description: "Math operation to perform",
+            type: :integer,
+            enum: %w[add subtract], # String values for integer type
+          )
+        }.to raise_error(ArgumentError, /enum values must be integers/)
+      end
+    end
+
+    context "with item_type specification" do
+      it "only allows item_type for array type parameters" do
+        expect {
+          described_class.new(
+            name: "colors",
+            description: "List of colors",
+            type: :array,
+            item_type: :string,
+          )
+        }.not_to raise_error
+
+        expect {
+          described_class.new(
+            name: "color",
+            description: "A single color",
+            type: :string,
+            item_type: :string,
+          )
+        }.to raise_error(ArgumentError, /item_type can only be specified for array type/)
+      end
+    end
+  end
+
+  # Test case 4: Coercing string parameters
+  describe "#coerce_parameters" do
+    context "with string parameters" do
+      let(:tool) do
+        param =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "name",
+            description: "User's name",
+            type: :string,
+          )
+
+        described_class.new(
+          name: "greeting",
+          description: "Generates a greeting",
+          parameters: [param],
+        )
+      end
+
+      it "converts numbers to strings" do
+        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")
+      end
+    end
+
+    # Test case 5: Coercing number parameters
+    context "with number parameters" do
+      let(:tool) do
+        param =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "price",
+            description: "Item price",
+            type: :number,
+          )
+
+        described_class.new(name: "store", description: "Store operations", parameters: [param])
+      end
+
+      it "converts string numbers to floats" do
+        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)
+      end
+
+      it "returns nil for invalid number strings" do
+        result = tool.coerce_parameters(price: "not a number")
+        expect(result[:price]).to be_nil
+      end
+    end
+
+    # Test case 6: Coercing array parameters with item types
+    context "with array parameters and item types" do
+      let(:tool) do
+        param =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "numbers",
+            description: "List of numeric values",
+            type: :array,
+            item_type: :integer,
+          )
+
+        described_class.new(
+          name: "stats",
+          description: "Statistical operations",
+          parameters: [param],
+        )
+      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])
+      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])
+      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])
+      end
+    end
+
+    # Test case 7: Required parameters
+    context "with required and optional parameters" do
+      let(:tool) do
+        param1 =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "required_param",
+            description: "This is required",
+            type: :string,
+            required: true,
+          )
+
+        param2 =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "optional_param",
+            description: "This is optional",
+            type: :string,
+          )
+
+        described_class.new(
+          name: "test_tool",
+          description: "Test tool",
+          parameters: [param1, param2],
+        )
+      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")
+      end
+
+      it "skips missing optional parameters" do
+        result = tool.coerce_parameters({})
+        expect(result[:required_param]).to be_nil
+        expect(result.key?("optional_param")).to be false
+      end
+    end
+
+    # Test case 8: Boolean parameter coercion
+    context "with boolean parameters" do
+      let(:tool) do
+        param =
+          DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+            name: "flag",
+            description: "Boolean flag",
+            type: :boolean,
+          )
+
+        described_class.new(name: "feature", description: "Feature toggle", parameters: [param])
+      end
+
+      it "preserves true/false values" do
+        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: "False" })
+        expect(result[:flag]).to be false
+      end
+
+      it "returns nil for invalid boolean strings" do
+        result = tool.coerce_parameters({ "flag" => "not a boolean" })
+        expect(result["flag"]).to be_nil
+      end
+    end
+  end
+
+  # Test case 9: Duplicate parameter validation
+  describe "duplicate parameter validation" do
+    it "rejects tool definitions with duplicate parameter names" do
+      param1 =
+        DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+          name: "query",
+          description: "Search query",
+          type: :string,
+        )
+
+      param2 =
+        DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+          name: "query", # Same name as param1
+          description: "Another parameter",
+          type: :string,
+        )
+
+      expect {
+        described_class.new(
+          name: "search",
+          description: "Search tool",
+          parameters: [param1, param2],
+        )
+      }.to raise_error(ArgumentError, /Duplicate parameter names/)
+    end
+  end
+
+  # Test case 10: Serialization to hash
+  describe "#to_h" do
+    it "serializes the tool to a hash with all properties" do
+      param =
+        DiscourseAi::Completions::ToolDefinition::ParameterDefinition.new(
+          name: "colors",
+          description: "List of colors",
+          type: :array,
+          item_type: :string,
+          required: true,
+        )
+
+      tool =
+        described_class.new(
+          name: "palette",
+          description: "Color palette generator",
+          parameters: [param],
+        )
+
+      hash = tool.to_h
+
+      expect(hash[:name]).to eq("palette")
+      expect(hash[:description]).to eq("Color palette generator")
+      expect(hash[:parameters].size).to eq(1)
+
+      param_hash = hash[:parameters].first
+      expect(param_hash[:name]).to eq("colors")
+      expect(param_hash[:type]).to eq(:array)
+      expect(param_hash[:item_type]).to eq(:string)
+      expect(param_hash[:required]).to eq(true)
+    end
+  end
+end

spec/lib/completions/xml_tool_processor_spec.rb

@@ -93,10 +93,24 @@ RSpec.describe DiscourseAi::Completions::XmlToolProcessor do
       <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 @@ RSpec.describe DiscourseAi::Completions::XmlToolProcessor do
         parameters: {
           hello: "world",
           test: "value",
+          bool: true,
         },
       )
     expect(result).to eq([["hello"], [" world"], [tool_call]])

spec/lib/personas/persona_spec.rb

@@ -69,11 +69,11 @@ RSpec.describe DiscourseAi::Personas::Persona do
 
     tools = rendered.tools
 
-    expect(tools.find { |t| t[:name] == "search" }).to be_present
+    expect(tools.find { |t| t.name == "search" }).to be_present
-    expect(tools.find { |t| t[:name] == "tags" }).to be_present
+    expect(tools.find { |t| t.name == "tags" }).to be_present
 
     # needs to be configured so it is not available
-    expect(tools.find { |t| t[:name] == "image" }).to be_nil
+    expect(tools.find { |t| t.name == "image" }).to be_nil
   end
 
   it "can parse string that are wrapped in quotes" do