Skip to content

DEV: use a proper object for tool definition #1337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 15, 2025
Merged

Conversation

SamSaffron
Copy link
Member

@SamSaffron SamSaffron commented May 14, 2025

This commit introduces a comprehensive refactoring of the AI tools system by adding a proper object-oriented interface for tool definitions. Instead of working with raw hash structures, tools are now represented by a proper ToolDefinition class with validation, parameter coercion, and schema generation capabilities.

Key changes:

  • Add new ToolDefinition class with nested ParameterDefinition class

    • Strong validation for parameter types, names, and values
    • Type coercion for parameters (string, boolean, number, array, etc.)
    • JSON schema generation via parameters_json_schema method
  • Refactor all dialect adapters to use the new class:

    • Replace direct hash manipulation with proper object methods
    • Simplify code by delegating schema generation to the ToolDefinition class
    • Consistent parameter handling across all LLM providers
  • Improve the XmlToolProcessor to use tool definitions for parameter coercion

    • Properly handle type conversions for parameters in XML tool calls
    • Convert "true"/"false" strings to actual boolean values
  • Change Ollama environment check to enable in both development and test

  • Update tests to work with the new object-oriented interface

This change significantly improves code maintainability, type safety, and error handling while reducing duplicated parameter handling code across different LLM integrations.

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
@SamSaffron SamSaffron marked this pull request as ready for review May 15, 2025 05:15
@nattsw nattsw requested a review from Copilot May 15, 2025 06:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the AI tools system to use an object‐oriented interface for tool definitions, improving type safety, maintainability, and consistency across LLM integrations. Key changes include:

  • Introduction of a new ToolDefinition class with nested ParameterDefinition, including validation, parameter coercion, and JSON schema generation.
  • Updates to dialect adapters and XML tool processing that replace raw hash manipulations with object methods.
  • Revision of tests and endpoint handlers to work with the new object‐oriented interface.

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/lib/completions/tool_definition_spec.rb Adds comprehensive tests for the new ToolDefinition and ParameterDefinition classes.
spec/lib/completions/endpoints/open_ai_spec.rb Updates tests to reflect proper boolean parameter coercion and inclusion of new parameters.
spec/lib/completions/endpoints/nova_spec.rb Adjusts JSON schema requirements, using description and required keys consistently.
spec/lib/completions/dialects/* Updates multiple dialects (Ollama, Gemini, ChatGPT, Nova, Cohere, Claude) to translate tools using the new object interface.
lib/completions/xml_tool_processor.rb Refactors tool call creation to utilize tool definitions for parameter coercion.
lib/completions/tool_definition.rb Introduces the object-oriented implementation for tool definitions and parameter coercion logic.
lib/completions/prompt.rb Modifies the tools setter to convert tool hashes to ToolDefinition instances.
lib/completions/endpoints/base.rb Adjusts environment checks to enable the Ollama endpoint appropriately.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@SamSaffron SamSaffron merged commit 2c64594 into main May 15, 2025
6 checks passed
@SamSaffron SamSaffron deleted the fix-research-params branch May 15, 2025 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants