-
Notifications
You must be signed in to change notification settings - Fork 364
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
New content format #122
New content format #122
Conversation
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
…to new-content-format
I'd love to hear some feedback about the |
Addressing the issue about decoding errors, can't it be the case with partial json receiving, the issue which was just recently solved here? Additionally, what do you think about this OpenAI Chat Chunk Object? As it says in docs, it seems to be returning chat completion now, that is why it is just a string. I may be wrong, but it seems intuitive to me that if we are about to receive mixed content, it should be done by streaming methods instead of completions. 🤔 |
Absolutely I missed that! I'll take another spin to check if this happens again |
@rawnly I must really highlight your amazing work here, thank you, for your contribution and that you care about the project! ✨ There is a lot going on in code, will take me some time to get into it, what can tell as of now, we definitely need to break down everything that is going on inside ChatQuery.swift into separate files as OpenAI have a lot of things encapsulated in one another in their Chat API. And it is best to preserve everything that is already there as it is, just "deprecate" it upon release. ps. Do you still have decoding errors after the fix? |
looks like it's fixed now 🚀 |
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Btw one thing i noticed is that new models kind of ignore the |
Yes, totally agree, that what I was suggesting, here:
|
Signed-off-by: rawnly <[email protected]>
Now it should be more clear where things are and it should be easier to manage new models too |
Regarding tools etc, what do you think about smth like Tool(
type: .function,
value: .function(
.init( // `Tool.Function`
name: "get_weather",
description: "Get the weather"
)
)
)
|
nop it's the one from here https://platform.openai.com/docs/api-reference/chat/create#chat-create-tools |
Got you, yeah, cool. There is one additional field for parameters which function accepts 🤔 |
let me push this code so you can look at the model code :) |
still missing tests Signed-off-by: rawnly <[email protected]>
case functionCall = "function_call" | ||
} | ||
|
||
public init(role: Role, content codable: Codable? = nil ,name: String? = nil, functionCall: ChatFunctionCall? = nil) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about typealiasing this Codable?
to something like StringOrChatContent
(as a suggestion, maybe you'll come up with something better), because I am testing it now and it is not very clear what to pass there 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely nice idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe also throwing an error if passing the wrong codable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that would be neat 🔥
what are you thinking about? |
I have two things in mind:
Why I think about it, is because OpenAI documentation for chats is clearly not very straightforward and doesn't cover most of the cases, so devs who will be just figuring it out may have a lot of struggle with these settings and properties. What do you think? |
@rawnly, hey, we are on our way to make a release tag today, do you have some work in progress or is it okay if I push a couple of things here? ✨ |
Go! I don't have anything wip atm |
makes sense
you mean something like a builder? |
yep, sort of 👍 And as of the builder, I think it will be a good addition in the next patch may I be missing something? what do you think, have you encountered any issues in scope of this PR?🤔 |
lgtm can you test the tools in a real request? I didn't have the time and now i'm not at home 👀 |
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
Signed-off-by: rawnly <[email protected]>
SonarCloud Quality Gate failed. 0 Bugs No Coverage information Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Current problem is that according to the api spec Below a stream response returned from the api: {"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"role":"assistant","content":null,"tool_calls":[{"index":0,"id":"call_S1iPLro8pNoUH2WOqYvQt6aD","type":"function","function":{"name":"add_to_calendar","arguments":""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\n"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" "}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"start"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"202"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"3"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"-"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"11"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"-"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"20"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"T"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"Z"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\",\n"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" "}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"title"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"F"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"eder"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"ico"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"'s"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" Birthday"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\",\n"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" "}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"end"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" \""}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"202"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"3"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"-"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"11"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"-"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"21"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"T"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":":"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"00"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"Z"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"\"\n"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"}"}}]},"finish_reason":null}]}
{"id":"chatcmpl-8MYjk8CJDQT2OhPpeaaTuUAoEFeBI","object":"chat.completion.chunk","created":1700387436,"model":"gpt-4-1106-preview","system_fingerprint":"fp_a24b4d720c","choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]}
[DONE] |
First what comes to mind with this problem is trying to decode chunk and in case of decoding error to decode a completion result. I wonder if it is actually possible to ask gpt for what kind of result we want. There is |
I'm using this PR in a project and I found that if you pass a single instance of ChatContent, rather than array of [ChatContent], it will just set the content to nil and fail only at request time with a cryptic "content and role must not be nil" message. The ergonomics of this seem problematic to me, since the swift type checker is unable to help. Maybe at least allowing a single instance of ChatContent to be passed, and then converted to an array, would be good. Otherwise maybe best to not offer an initializer that accepts |
self.content = content | ||
self.name = name | ||
self.functionCall = functionCall | ||
self.toolCalls = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this initializer is a trap for vision requests - you'll get an invalid request error from sending an empty array in toolCalls
(which isn't supported for this model yet). You need to use the other initializer with explicit toolCalls: nil
.
Would it be safe to default to nil here?
|
||
import Foundation | ||
|
||
public struct Message: Codable, Equatable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty generic name for a type - I found it to be a problem in my project that already had its own Message
type defined, and OpenAI.Message
didn't work because the OpenAI
module also contains a class called OpenAI
(see this discussion on this potential flaw in Swift)
The only thing that worked for me was renaming the type in my own project so Message
was available for this package to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I think it would be a good idea to use some naming pattern, to avoid confusions, though this would mean that all projects using this package would need to update namings.
} | ||
|
||
public struct ToolCall: Codable, Equatable { | ||
public let index: Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting decoding errors because index
is not present in the response.
▿ DecodingError
▿ keyNotFound : 2 elements
- .0 : CodingKeys(stringValue: "index", intValue: nil)
▿ .1 : Context
▿ codingPath : 5 elements
- 0 : CodingKeys(stringValue: "choices", intValue: nil)
▿ 1 : _JSONKey(stringValue: "Index 0", intValue: 0)
▿ rep : Rep
- index : 0
- 2 : CodingKeys(stringValue: "message", intValue: nil)
- 3 : CodingKeys(stringValue: "tool_calls", intValue: nil)
▿ 4 : _JSONKey(stringValue: "Index 0", intValue: 0)
▿ rep : Rep
- index : 0
- debugDescription : "No value associated with key CodingKeys(stringValue: \"index\", intValue: nil) (\"index\")."
- underlyingError : nil
Hi Guys. Do you have any updates about this PR? |
What
Includes changes from #115
content
ofChatQuery
is nowChatContent
.Link to the api spec
Missing
Some tests.
tools
/tool_calls
/content_filter
.Problems
One thing I noticed is that even tho the available format for the content is
Array<ChatContent>/String
the response received from the api will always be a string.I tested this branch on a real application and sometimes I have decoding errors, even tho analyzing the response I really can't tell what's wrong.
Affected Areas
Chat related models