Skip to content

server/python_tools: add python tool parsing logic#10453

Closed
ParthSareen wants to merge 2 commits intomainfrom
parth/python-function-parsing
Closed

server/python_tools: add python tool parsing logic#10453
ParthSareen wants to merge 2 commits intomainfrom
parth/python-function-parsing

Conversation

@ParthSareen
Copy link
Member

@ParthSareen ParthSareen commented Apr 29, 2025

This PR introduces a small python parsing package for supporting python based tool calls.

Only kwargs and empty python functions are currently supported.

The parsing logic can be a bit hard to figure out so feedback to make clearer is appreciated!

@ParthSareen ParthSareen requested a review from Copilot April 29, 2025 00:11
Copy link

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 pull request adds Python tool parsing logic to the server package with comprehensive unit tests for both function calls and value parsing.

  • Implements parsing of Python function calls with keyword arguments and handles multiple function calls in a single string.
  • Adds dedicated tests to cover various argument types and edge cases for both function and value parsing.

Reviewed Changes

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

File Description
server/python_tools_test.go Introduces tests covering a range of scenarios from valid calls to error cases.
server/python_tools.go Implements functions to parse Python values and function call strings.

@ParthSareen ParthSareen force-pushed the parth/python-function-parsing branch from 769e995 to 82f0e0f Compare April 29, 2025 00:13
@ParthSareen ParthSareen marked this pull request as ready for review April 29, 2025 00:13
@ParthSareen ParthSareen requested a review from jmorganca April 29, 2025 00:15
},
{
name: "whitespace handling",
input: "get_current_weather( location = \"San Francisco\" , temp = 72 )",
Copy link
Member

Choose a reason for hiding this comment

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

Should we add more tests with edge cases such as text before/after, etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in this case as the outer function (in next PR) is going to be handling that. Will have tests for varying messages there.

for _, match := range matches {
name := s[match[2]:match[3]]
args := s[match[4]:match[5]]
arguments := make(api.ToolCallFunctionArguments)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
arguments := make(api.ToolCallFunctionArguments)
var arguments api.ToolCallFunctionArguments

@ParthSareen ParthSareen force-pushed the parth/python-function-parsing branch from 8abc5eb to 9ecbcc4 Compare April 30, 2025 22:22
@ParthSareen ParthSareen changed the title server: add python tool parsing logic server/python_tools: add python tool parsing logic May 2, 2025
@ParthSareen ParthSareen force-pushed the parth/python-function-parsing branch from 9ecbcc4 to 611d3a1 Compare May 2, 2025 23:24
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.

3 participants