diff --git a/lib/completions/dialects/claude_tools.rb b/lib/completions/dialects/claude_tools.rb index 238a25b0c..48ef02327 100644 --- a/lib/completions/dialects/claude_tools.rb +++ b/lib/completions/dialects/claude_tools.rb @@ -9,33 +9,8 @@ def initialize(tools) end def translated_tools - # Transform the raw tools into the required Anthropic Claude API format raw_tools.map do |t| - properties = {} - 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, - }, - } + { name: t.name, description: t.description, input_schema: t.parameters_json_schema } end end diff --git a/lib/completions/dialects/cohere_tools.rb b/lib/completions/dialects/cohere_tools.rb index 7a49c19c5..26eb3bc76 100644 --- a/lib/completions/dialects/cohere_tools.rb +++ b/lib/completions/dialects/cohere_tools.rb @@ -38,27 +38,24 @@ def tool_results(messages) end def translated_tools - raw_tools.map do |t| - tool = t.dup + raw_tools.map do |tool| + defs = {} - tool[:parameter_definitions] = t[:parameters] - .to_a - .reduce({}) do |memo, p| - name = p[:name] - memo[name] = { - description: p[:description], - type: cohere_type(p[:type], p[:item_type]), - required: p[:required], - } + tool.parameters.each do |p| + name = p.name + defs[name] = { + description: p.description, + type: cohere_type(p.type, p.item_type), + required: p.required, + } - memo[name][:default] = p[:default] if p[:default] - memo - end + #defs[name][:default] = p.default if p.default + end { - name: tool[:name] == "search" ? "search_local" : tool[:name], - description: tool[:description], - parameter_definitions: tool[:parameter_definitions], + name: tool.name == "search" ? "search_local" : tool.name, + description: tool.description, + parameter_definitions: defs, } end end @@ -72,6 +69,7 @@ def instructions attr_reader :raw_tools def cohere_type(type, item_type) + type = type.to_s case type when "string" "str" diff --git a/lib/completions/dialects/gemini.rb b/lib/completions/dialects/gemini.rb index 050fabd2c..8ffbf5d7d 100644 --- a/lib/completions/dialects/gemini.rb +++ b/lib/completions/dialects/gemini.rb @@ -47,22 +47,8 @@ def tools translated_tools = prompt.tools.map do |t| - tool = t.slice(:name, :description) - - 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 = { name: t.name, description: t.description } + tool[:parameters] = t.parameters_json_schema if t.parameters tool end diff --git a/lib/completions/dialects/nova_tools.rb b/lib/completions/dialects/nova_tools.rb index 67cff2ead..54d009a62 100644 --- a/lib/completions/dialects/nova_tools.rb +++ b/lib/completions/dialects/nova_tools.rb @@ -17,10 +17,10 @@ def translated_tools @raw_tools.map do |tool| { toolSpec: { - name: tool[:name], - description: tool[:description], + name: tool.name, + description: tool.description, inputSchema: { - json: convert_tool_to_input_schema(tool), + json: tool.parameters_json_schema, }, }, } @@ -51,32 +51,6 @@ def from_raw_tool(raw_message) }, } 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 diff --git a/lib/completions/dialects/ollama_tools.rb b/lib/completions/dialects/ollama_tools.rb index 6f4d669d9..90cdb136b 100644 --- a/lib/completions/dialects/ollama_tools.rb +++ b/lib/completions/dialects/ollama_tools.rb @@ -14,23 +14,15 @@ def instructions end def translated_tools - raw_tools.map do |t| - tool = t.dup - - tool[:parameters] = t[:parameters] - .to_a - .reduce({ type: "object", properties: {}, required: [] }) do |memo, p| - name = p[:name] - 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 } + raw_tools.map do |tool| + { + type: "function", + function: { + name: tool.name, + description: tool.description, + parameters: tool.parameters_json_schema, + }, + } end end diff --git a/lib/completions/dialects/open_ai_tools.rb b/lib/completions/dialects/open_ai_tools.rb index 73b011461..4cdc97e59 100644 --- a/lib/completions/dialects/open_ai_tools.rb +++ b/lib/completions/dialects/open_ai_tools.rb @@ -9,25 +9,15 @@ def initialize(tools) end def translated_tools - raw_tools.map do |t| - tool = t.dup - - tool[:parameters] = t[:parameters] - .to_a - .reduce({ type: "object", properties: {}, required: [] }) do |memo, p| - name = p[:name] - 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 } + raw_tools.map do |tool| + { + type: "function", + function: { + name: tool.name, + description: tool.description, + parameters: tool.parameters_json_schema, + }, + } end end diff --git a/lib/completions/dialects/xml_tools.rb b/lib/completions/dialects/xml_tools.rb index 781c33bbe..7af04df11 100644 --- a/lib/completions/dialects/xml_tools.rb +++ b/lib/completions/dialects/xml_tools.rb @@ -9,33 +9,37 @@ def initialize(tools) end def translated_tools - raw_tools.reduce(+"") do |tools, function| + result = +"" + + raw_tools.each do |tool| parameters = +"" - if function[:parameters].present? - function[:parameters].each do |parameter| + if tool.parameters.present? + tool.parameters.each do |parameter| parameters << <<~PARAMETER - #{parameter[:name]} - #{parameter[:type]} - #{parameter[:description]} - #{parameter[:required]} + #{parameter.name} + #{parameter.type} + #{parameter.description} + #{parameter.required} PARAMETER - if parameter[:enum] - parameters << "#{parameter[:enum].join(",")}\n" + if parameter.item_type + parameters << "#{parameter.item_type}\n" end + parameters << "#{parameter.enum.join(",")}\n" if parameter.enum parameters << "\n" end end - tools << <<~TOOLS + result << <<~TOOLS - #{function[:name]} - #{function[:description]} + #{tool.name} + #{tool.description} #{parameters} TOOLS end + result end def instructions @@ -43,8 +47,7 @@ def instructions @instructions ||= begin - has_arrays = - raw_tools.any? { |tool| tool[:parameters]&.any? { |p| p[:type] == "array" } } + has_arrays = raw_tools.any? { |tool| tool.parameters&.any? { |p| p.type == "array" } } (<<~TEXT).strip #{tool_preamble(include_array_tip: has_arrays)} diff --git a/lib/completions/endpoints/base.rb b/lib/completions/endpoints/base.rb index 0356597e4..bd06b540c 100644 --- a/lib/completions/endpoints/base.rb +++ b/lib/completions/endpoints/base.rb @@ -28,7 +28,7 @@ def endpoint_for(provider_name) 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 @@ 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/prompt.rb b/lib/completions/prompt.rb index 660b1ca5e..24c31b267 100644 --- a/lib/completions/prompt.rb +++ b/lib/completions/prompt.rb @@ -5,8 +5,8 @@ module Completions class Prompt INVALID_TURN = Class.new(StandardError) - attr_reader :messages - attr_accessor :tools, :topic_id, :post_id, :max_pixels, :tool_choice + attr_reader :messages, :tools + attr_accessor :topic_id, :post_id, :max_pixels, :tool_choice def initialize( system_message_text = nil, @@ -37,10 +37,25 @@ def initialize( @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) diff --git a/lib/completions/tool_definition.rb b/lib/completions/tool_definition.rb new file mode 100644 index 000000000..f3a49b4dc --- /dev/null +++ b/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 diff --git a/lib/completions/xml_tool_processor.rb b/lib/completions/xml_tool_processor.rb index d2f4294cc..f20409f7e 100644 --- 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/dialects/chat_gpt_spec.rb b/spec/lib/completions/dialects/chat_gpt_spec.rb index fa03a491c..34c3832fa 100644 --- a/spec/lib/completions/dialects/chat_gpt_spec.rb +++ b/spec/lib/completions/dialects/chat_gpt_spec.rb @@ -99,17 +99,21 @@ it "returns a list of available tools" do open_ai_tool_f = { function: { - description: context.tools.first[:description], - name: context.tools.first[:name], + description: context.tools.first.description, + name: context.tools.first.name, parameters: { properties: - context.tools.first[:parameters].reduce({}) do |memo, p| - memo[p[:name]] = { description: p[:description], type: p[:type] } + context + .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, + memo + end, required: %w[location unit], type: "object", }, diff --git a/spec/lib/completions/dialects/dialect_context.rb b/spec/lib/completions/dialects/dialect_context.rb index 0a9725a77..c48c92d93 100644 --- a/spec/lib/completions/dialects/dialect_context.rb +++ b/spec/lib/completions/dialects/dialect_context.rb @@ -115,6 +115,6 @@ def tools }, ], }, - ] + ].map { |tool| DiscourseAi::Completions::ToolDefinition.from_hash(tool) } end end diff --git a/spec/lib/completions/dialects/gemini_spec.rb b/spec/lib/completions/dialects/gemini_spec.rb index 860fe6535..c9ec34755 100644 --- a/spec/lib/completions/dialects/gemini_spec.rb +++ b/spec/lib/completions/dialects/gemini_spec.rb @@ -108,11 +108,11 @@ 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 @@ }, ], } - expect(context.dialect_tools).to contain_exactly(gemini_tools) end end diff --git a/spec/lib/completions/dialects/nova_spec.rb b/spec/lib/completions/dialects/nova_spec.rb index 36b0fcc71..aafbf6e4c 100644 --- a/spec/lib/completions/dialects/nova_spec.rb +++ b/spec/lib/completions/dialects/nova_spec.rb @@ -84,29 +84,30 @@ dialect = nova_dialect_klass.new(prompt, llm_model) translated = dialect.translate - expect(translated.tool_config).to eq( - { - tools: [ - { - toolSpec: { - name: "get_weather", - description: "Get the weather in a city", - inputSchema: { - json: { - type: "object", - properties: { - "location" => { - type: "string", - required: true, - }, + expected = { + tools: [ + { + toolSpec: { + name: "get_weather", + description: "Get the weather in a city", + inputSchema: { + json: { + type: "object", + properties: { + "location" => { + type: :string, + description: "the city name", }, }, + required: ["location"], }, }, }, - ], - }, - ) + }, + ], + } + + expect(translated.tool_config).to eq(expected) end end diff --git a/spec/lib/completions/dialects/ollama_spec.rb b/spec/lib/completions/dialects/ollama_spec.rb index a8ee35fe4..efd959d1e 100644 --- a/spec/lib/completions/dialects/ollama_spec.rb +++ b/spec/lib/completions/dialects/ollama_spec.rb @@ -5,6 +5,7 @@ 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 @@ 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) - allow(tool).to receive(:translated_tools) - allow(DiscourseAi::Completions::Dialects::OllamaTools).to receive(:new).and_return(tool) - - context.dialect_tools - - expect(DiscourseAi::Completions::Dialects::OllamaTools).to have_received(:new).with( - context.prompt.tools, - ) - expect(tool).to have_received(:translated_tools) + tool = { name: "noop", description: "do nothing" } + messages = [ + { 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) + + expected = [ + { role: "system", content: "a bot" }, + { role: "user", content: "echo away" }, + { + role: "assistant", + 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) - allow(tool).to receive(:translated_tools) - allow(DiscourseAi::Completions::Dialects::XmlTools).to receive(:new).and_return(tool) + tool = { name: "noop", description: "do nothing" } + messages = [ + { 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( - context.prompt.tools, - ) - expect(tool).to have_received(:translated_tools) + # notice, no tool role + expect(roles).to eq(expected) end end end diff --git a/spec/lib/completions/dialects/ollama_tools_spec.rb b/spec/lib/completions/dialects/ollama_tools_spec.rb index a1f9f235d..6c4dd4fa7 100644 --- a/spec/lib/completions/dialects/ollama_tools_spec.rb +++ b/spec/lib/completions/dialects/ollama_tools_spec.rb @@ -5,34 +5,33 @@ 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 = [ - { - name: "github_file_content", - description: "Retrieves the content of specified GitHub files", - parameters: [ - { - name: "repo_name", - description: "The name of the GitHub repository (e.g., 'discourse/discourse')", - type: "string", - required: true, - }, - { - name: "file_paths", - description: "The paths of the files to retrieve within the repository", - type: "array", - item_type: "string", - required: true, - }, - { - name: "branch", - description: "The branch or commit SHA to retrieve the files from (default: 'main')", - type: "string", - required: false, - }, - ], - }, - ] + tool = { + name: "github_file_content", + description: "Retrieves the content of specified GitHub files", + parameters: [ + { + name: "repo_name", + description: "The name of the GitHub repository (e.g., 'discourse/discourse')", + type: "string", + required: true, + }, + { + name: "file_paths", + description: "The paths of the files to retrieve within the repository", + type: "array", + item_type: "string", + required: true, + }, + { + name: "branch", + description: "The branch or commit SHA to retrieve the files from (default: 'main')", + type: "string", + 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 @@ 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], diff --git a/spec/lib/completions/endpoints/nova_spec.rb b/spec/lib/completions/endpoints/nova_spec.rb index d0f9a5389..aa2727f79 100644 --- a/spec/lib/completions/endpoints/nova_spec.rb +++ b/spec/lib/completions/endpoints/nova_spec.rb @@ -196,10 +196,11 @@ def encode_message(message) "inputSchema" => { "json" => { "type" => "object", + "required" => ["timezone"], "properties" => { "timezone" => { "type" => "string", - "required" => true, + "description" => "The timezone", }, }, }, @@ -268,9 +269,10 @@ def encode_message(message) properties: { timezone: { type: "string", - required: true, + description: "The timezone", }, }, + required: ["timezone"], }, }, }, diff --git a/spec/lib/completions/endpoints/open_ai_spec.rb b/spec/lib/completions/endpoints/open_ai_spec.rb index fc3b8b48b..524ff50db 100644 --- 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) Sydney c + true 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("") 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 new file mode 100644 index 000000000..699614fcf --- /dev/null +++ b/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 diff --git a/spec/lib/completions/xml_tool_processor_spec.rb b/spec/lib/completions/xml_tool_processor_spec.rb index 883996b22..99b7163cf 100644 --- a/spec/lib/completions/xml_tool_processor_spec.rb +++ b/spec/lib/completions/xml_tool_processor_spec.rb @@ -93,10 +93,24 @@ world value + true 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]]) diff --git a/spec/lib/personas/persona_spec.rb b/spec/lib/personas/persona_spec.rb index fa81876e3..d3e905680 100644 --- a/spec/lib/personas/persona_spec.rb +++ b/spec/lib/personas/persona_spec.rb @@ -69,11 +69,11 @@ def system_prompt tools = rendered.tools - 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 == "search" }).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