Add OpenAI-compatible model provider - #167
Conversation
github-actions
Bot
added
workshop/frontend
|
I've completed a thorough review. The change is well-structured, well-tested, and consistent with the codebase's capability/routing conventions. I found no actionable defects. Summary of what I verified:
LGTM! |
|
Confirmed: for the OpenAI provider ( The review is complete. Let me summarize my findings. The PR is well-structured and thoroughly tested. My analysis of all 14 changed files:
I found no actionable defects. The change is consistent with the codebase's capability/routing conventions and the stated "direct mode only, unavailable in gateway mode" design. LGTM! |
|
|
||
| // Validate and normalize fields owned by the OpenAI-compatible provider. | ||
| export function normalizeAiModelConfig(config: AiModelConfig): AiModelConfig { | ||
| if (config.provider !== "openai-compatible") return config; |
There was a problem hiding this comment.
Why do we need to validate openai-compatible configs but not other types of configs?
A lot of this function is redundantly validating things that capnweb-validate will already have checked for us. Can it be simplified? Or maybe it can be removed entirely? Is any of this validation actually important?
There was a problem hiding this comment.
There's a lot of user input here because there's not much we can infer nor do I think its worth the complexity to do so - to keep things simple I want to support but leave as an advanced escape hatch
Cap’n Web covers the wire shape, but not the semantic checks or URL normalization. We still need the conditional fields, URL checks, integer limits, and outputLimit < contextWindow. I’ll remove anything that only repeats the generated validator.
|
|
||
| // Pick an example model to show in the custom-model placeholders for the given provider. | ||
| function exampleModel(provider: AiModelProvider): { modelId: string, name: string } { | ||
| if (provider === 'openai-compatible') { |
There was a problem hiding this comment.
This is sort of inconsistent with how ollama is handled. ollama also has no suggested model, which means it falls back to FALLBACK_EXAMPLE_MODEL, which is actually currently set specifically to be a good example for ollama.
Seems like we should extend FALLBACK_EXAMPLE_MODEL to be a full on map from provider name to an exmaple model?
| }; | ||
| } | ||
|
|
||
| const OPENAI_COMPATIBLE_PROFILES = { |
There was a problem hiding this comment.
Pi seems to have some of its own code to guess at capabilities based on known hostnames, although apparently it doesn't have great defaults for hostnames that aren't in the list (it defaults to supportsDeveloperRole: true which several people complained about). It seems like it would be nice to leverage Pi's list though, rather than always assume conservative settings. Can we figure out how to do that?
There was a problem hiding this comment.
We actually already are using this but our conservative profile pins seven fields that generic endpoints commonly reject (which really just helps models without host entries).
If we're okay with it, we can remove compat and trust Pi completely. Known hosts improve, but unknown hosts receive optimistic OpenAI defaults.
| maxOutputTokens, | ||
| }; | ||
| } | ||
| let model = SUGGESTED_MODELS[config.provider][config.model]; |
There was a problem hiding this comment.
Ugh I'm realizing this whole function is kinda broken, predating this change. It doesn't work for models not in SUGGESTED_MODELS!
@AshishKumar4 Can you please figure out a better approach here? Does pi-agent-core give us any new options for figuring out a model's token limits? If not maybe we could pull from models.dev?
0fda37d to
8d92708
Compare
|
Thank you for taking the time to contribute. This pull request was automatically closed because:
Please update the pull request to meet these automatic checks, then reopen it. Passing these checks does not guarantee acceptance; maintainers still determine whether a change is obviously correct and trivially verifiable. If an exception is appropriate, a maintainer can apply the |
| @@ -0,0 +1,616 @@ | |||
| // Generated by scripts/build-openai-compatible-models.mjs. Do not edit. | |||
There was a problem hiding this comment.
These don't have me particularly excited, but the alternative is pulling in the full pi catalog with is hundreds of thousands of entries long.
This is built as a smaller subset with only the necessary info directly from Pi at build time
|
I don't think #184 was intended to close this; silly github. @ndisidore are you still working on this? |
3 participants
Adds beta support for user-configured OpenAI-compatible Chat Completions providers (
openai-compatiblein config, OpenAI-compatible in the UI).This first version is intentionally minimal:
/chat/completionsendpoints are normalized automatically.conservativecompatibility profile, usingsystemandmax_tokenswhile disabling optional features generic endpoints may not support. We can look at expanding this in the futureShould unlock the following services:
Screenshots