Skip to content
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

llms/googleai: Support deeply nested tool definitions #1136

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

goodenough227
Copy link

Fix Multi-level Conversion for OpenAI Tool Structure

Problem

The current tool structure conversion logic only processes the first level of OpenAI tool definitions, resulting in information loss when handling deeply nested tool structures. This affects the integrity and correctness of complex tool definitions.

Improvements

  • Refactored tool structure conversion logic by introducing an iterative processing mechanism
  • Implemented complete traversal and conversion of nested structures
  • Added schemaWorkItem struct to manage the conversion process
  • Utilized queue-based approach for multi-level property conversion

Technical Details

  • Introduced schemaWorkItem type to track conversion progress
  • Implemented work queue (workQueue) for level-by-level processing
  • Enhanced handling of various properties:
    • Object type properties
    • Array type items
    • Required fields
    • Description information
    • Enum values

Impact

This improvement ensures that complex tool definitions maintain complete structural information during the conversion process, enhancing compatibility with the OpenAI API.

Testing

  • Added relevant unit tests
  • Verified correct conversion of multi-level nested structures

Copy link
Owner

@tmc tmc left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, can you address my nit.

@@ -433,6 +434,112 @@ func testTools(t *testing.T, llm llms.Model) {
assert.NotZero(t, resp.Choices[0].GenerationInfo["output_tokens"])
}

func testToolsWithDeeplyNestedToolStructures(t *testing.T, llm llms.Model) {
t.Helper()
t.Parallel()
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think t.Parallel calls make sense in helpers, the test function should have this responsibility.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the reminder, I have deleted the "t.Parallel()" code segment.

@tmc tmc changed the title fix: Fixed the issue where only the first layer was processed when co… llms/googleai: Support deeply nested tool definitions Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants