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

fix: function call error due to nil properties (429) #431

Conversation

vvatanabe
Copy link
Collaborator

@vvatanabe vvatanabe commented Jul 8, 2023

Describe the change
This PR solves issue #429.
PR to reproduce the problem: #430

An error occurs when using the Function Call feature. The relevant error message is as follows:

Completion error: error, status code: 400, message: Invalid schema for function 'func_name': None is not of type 'object'

This issue arises when the map in the Properties field of jsonschema.Definition is set to nil.

Describe your solution
If the map in the Properties field of jsonschema.Definition is set to nil, initialize it to an empty map. Also, as jsonschema.Definition has a recursive structure that can contain jsonschema.Definition as a child, perform the nil check and initialization recursively.

Tests
I have tested that the Properties field is initialized with an empty map in all of the following cases.

  • Empty Definition
  • Definition properties set
  • Nested Definition properties
  • Complex nested Definition
  • Array type Definition

Additional context
Issue: #429

@vvatanabe
Copy link
Collaborator Author

It appears that the Function Call of the OpenAI API tolerates an empty object in the properties field but does not tolerate nil.

However, I question whether there really is a use case where the properties field in the root Definition is left empty. If there is no realistic use case for this, it might be better to re-add omitempty to the properties field.

Copy link
Owner

@sashabaranov sashabaranov left a comment

Choose a reason for hiding this comment

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

Good catch, thank you!

@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Merging #431 (6179155) into master (619ad71) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #431   +/-   ##
=======================================
  Coverage   97.02%   97.02%           
=======================================
  Files          17       17           
  Lines         705      705           
=======================================
  Hits          684      684           
  Misses         15       15           
  Partials        6        6           

see 4 files with indirect coverage changes

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