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

Add SystemFingerprint and chatMsg.ToolCallID field #543

Conversation

gburt
Copy link
Contributor

@gburt gburt commented Nov 7, 2023

A few small improvements:

  • capture the new system_fingerprint response field to chat calls
  • on ChatCompletionMessage capture the tool_call_id that needs to be set for role=TOOL prompts
  • fix a typo (ToolChoiche)
  • Add omitempty to ChatCompletionMessage optional fields
  • Add required ToolCall.type field

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #543 (0dbd303) into master (08c167f) will not change coverage.
The diff coverage is n/a.

❗ Current head 0dbd303 differs from pull request most recent head 5e56fad. Consider uploading reports for the commit 5e56fad to get more accurate results

@@           Coverage Diff           @@
##           master     #543   +/-   ##
=======================================
  Coverage   97.88%   97.88%           
=======================================
  Files          20       20           
  Lines         991      991           
=======================================
  Hits          970      970           
  Misses         15       15           
  Partials        6        6           
Files Coverage Δ
chat.go 100.00% <ø> (ø)

@sashabaranov
Copy link
Owner

@gburt thank you for the PR! Seems like there's a conflicting change here

@gburt
Copy link
Contributor Author

gburt commented Nov 8, 2023

@gburt thank you for the PR! Seems like there's a conflicting change here

you're welcome! thanks for the great library

ToolCalls []ToolCall `json:"tool_calls,omitempty"`

// For Role=ASSISTANT prompts this may be set to the tool calls generated by the model, such as function calls.
ToolCalls []ToolCall `json:"tool_calls,omitempty"`
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please point out this field in the documentation?

Copy link
Owner

Choose a reason for hiding this comment

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

also, could we please de-capslock the Role=ASSISTANT and Role=TOOL here? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah good call re: caps (we use SCREAMING enums internally...). Here are the docs for CreateChatRequest.messages.tool_calls:
image

@sashabaranov sashabaranov merged commit e3e065d into sashabaranov:master Nov 9, 2023
1 check passed
maerlyn5 pushed a commit to prassoai/go-openai that referenced this pull request Nov 22, 2023
* fix ToolChoiche typo

* add tool_call_id to ChatCompletionMessage

* add /chat system_fingerprint response field

* check empty ToolCallID JSON marshaling

and add omitempty for tool_call_id

* messages also required; don't omitempty

* add Type to ToolCall, required by the API

* fix test, omitempty for response_format ptr

* fix casing of role values in comments
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