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

typesafe models #166

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

Conversation

kalafus
Copy link
Contributor

@kalafus kalafus commented Feb 6, 2024

What

typesafe models

Why

shouldn't have to validate models.

SpeechQuery had a method to validateSpeechModel, pointing out the need to validate Models, and highlighting the lack of validation across all other Query structs.

**Responses should use Strings and never models, as I have observed some endpoints returning strings incompatible with available models -- e.g., if reaching the text-embedding-ada-002 endpoint, response indicates the model used is text-embedding-ada-002-v2. Responses specify that the models are non-optional, so Response models need be strings to guarantee decoding.

Affected Areas

Models and Queries

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@kalafus
Copy link
Contributor Author

kalafus commented Feb 16, 2024

as it is, i can e.g., create a ChatQuery with an AudioSpeechQuery model tts-1, and it won't be kicked back until the server returns an APIError. The API defines which Model is compatible with which Query, so why not adopt Query specific model types as i have done in this pull, and achieve maximum type safety, neatly avoiding such scenarios?

(same isn't possible for Results, as endpoints don't necessarily return the same model name as was sent in the Query)

@nezhyborets
Copy link
Collaborator

Hi @kalafus , please let me know if you're available for the discussion.

I like the idea of breaking models up into categories so that you can only use supported model on the endpoint. I would even think about going further and do as other libs do - break up OpenAI into different clients, something like openai-dotnet does:

Screenshot 2025-02-14 at 22 47 55

Few notes regarding this PR. I think that maybe in a core sense the set/list of available models per category is not an enumeration. It's just a set of models, and we usually interested only in the single value of the set. Whereas enumeration is used, well, for enumerating over possible values. So I'd maybe go we the same organisation, but use struct instead of enum like this:

public struct ChatModel: Codable, Model {
    public static let allCases: [ChatModel] = [.gpt_3_5_turbo_0125, ...]
    
    /// The latest GPT-3.5 Turbo model with higher accuracy at responding in requested formats and a fix for a bug which caused a text encoding issue for non-English language function calls. Returns a maximum of 4,096 output tokens.
    static let gpt_3_5_turbo_0125 = ChatModel(modelId: "gpt-3.5-turbo-0125") // system
    
    ...
    
    let modelId: String
}

protocol Model {
    var modelId: String { get }
}

I also understand the concern about replies. If we don't like string there, we can have something like struct AnyModel: Model. But overall, I agree with your concept and would like to make this change. Thanks!

@nezhyborets
Copy link
Collaborator

nezhyborets commented Feb 14, 2025

Also a thought about categorising. It would be nice to have a reference for the names and number of categories. Currently we have: ChatModel, ImageModel, SpeechModel, AudioTranslationModel, AudioTranscriptionModel, EmbeddingsModel, ModerationsModel.

What do you think about going with this as a reference for our categories?
Screenshot 2025-02-14 at 23 03 06

I worry that having GPTModel, Reasoning Model, GPTRealtime and GPTAudio instead of just ChatModel may be too wordy and granular. Though I'm generally not against being more specific. Usually it's easier to generalize over specifics than to break down something generic. But maybe there is a better suited reference. We need the categories to make sense for our users and don't just be a mapping of whatever reference from OpenAI guides. Maybe if we'd go with breaking up OpenAI into different clients it'd help with this question

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