Sync model names and costs via LiteLLM's registry#227
Sync model names and costs via LiteLLM's registry#227Ankur Goyal (ankrgyl) merged 9 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Ankur Goyal (ankrgyl)
changed the title
Ankur Goyal (ankrgyl)
requested review from
Hurshal Patel (choochootrain),
Olmo Maldonado (ibolmo) and
Manu Goyal (manugoyal)
There was a problem hiding this comment.
mostly have nits but the more I think on it.. what if the problem could be simpler?
what if we used git diff:
converted = getLiteLLMModels()
models = translateToBraintrust(converted) // all of them
tempFile = saveTempfile(models)
// maybe exclude the ones we do not yet have coverage
diff = gitDiff(tempFile, './mode_list.json')
if (!diff) {
throw new Error(diff);
}maybe gron
▶ diff <(gron two.json) <(gron two-b.json)
3c3
< json.contact.email = "mail@tomnomnom.com";
---
> json.contact.email = "contact@tomnomnom.com";
| async function fetchRemoteModels(url: string): Promise<LiteLLMModelList> { | ||
| return new Promise((resolve, reject) => { | ||
| https | ||
| .get(url, (res) => { |
There was a problem hiding this comment.
curious why the use of https module vs. fetch?
There was a problem hiding this comment.
AI generated :)
| "tsup": "^8.4.0", | ||
| "typescript": "5.5.4", | ||
| "vite-tsconfig-paths": "^4.3.2", | ||
| "vitest": "^2.1.9" |
There was a problem hiding this comment.
should we add a few scripts in package.json or use/update Makefile? will we break CI/CD if we find discrepancies?
There was a problem hiding this comment.
I think we should eventually but not yet
| supports_reasoning: z.boolean().optional(), | ||
| supports_web_search: z.boolean().optional(), | ||
| search_context_cost_per_query: searchContextCostPerQuerySchema, | ||
| deprecation_date: z.string().optional(), // YYYY-MM-DD |
There was a problem hiding this comment.
would be nice if there was a published/created date. I recently rearranged our Gemini models to help users to select the most recent model and to have them clustered together (esp. that we vertex ai may have more than just gemini, for example).
vs.
(in my PR)
There was a problem hiding this comment.
agree, unfortunately the litellm registry does not have this as far as i can tell
| import { ModelSchema, ModelSpec } from "../schema/models"; | ||
|
|
||
| // Zod schema for individual model details | ||
| const searchContextCostPerQuerySchema = z |
There was a problem hiding this comment.
#nit would be nice to move lite related things to a litellm.ts with explicit interface (would help with readability/maintainability) esp. that you can reduce the mention of LiteLLM in the variable name, for example.
There was a problem hiding this comment.
maybe. i think it's also worth optimizing for ease of AI-maintainability of this file
| } | ||
| } | ||
|
|
||
| function translateLiteLLMToBraintrust( |
There was a problem hiding this comment.
#nit toBraintrustModelName
There was a problem hiding this comment.
updated
| } | ||
|
|
||
| if (provider === "gemini") { | ||
| if (modelName.startsWith("gemini/gemini-gemma-")) { |
There was a problem hiding this comment.
would be nice to add this to the schema to try to detect if litellm changes the json under us
i.e. assert that the models may start with xai etc.
There was a problem hiding this comment.
that will just show up in the discrepancies
| } | ||
|
|
||
| async function findMissingCommand(argv: any) { | ||
| try { |
There was a problem hiding this comment.
#nit
- could catch in the main()?
- would be nice to break up the function
- would be nice if the function signature was explicit i.e. call the function with the actual arguments
There was a problem hiding this comment.
eh i think i'd rather just keep to the AI generated code
|
|
||
| if (allPresentProviders.length > 0) { | ||
| console.log("\n--- Providers with All Models Present ---"); | ||
| for (const provider of allPresentProviders) { |
There was a problem hiding this comment.
unnecesary if statement?
There was a problem hiding this comment.
it prevents the console.log
| const modelDetail = remoteModels[remoteModelName]; | ||
|
|
||
| if (argv.provider) { | ||
| const lowerArgProvider = argv.provider.toLowerCase(); |
There was a problem hiding this comment.
#nit "provider" (and lowerArgProvider) are a bit vague/indirect
may be more readable if it we wrote the code as searchProvider
There was a problem hiding this comment.
I think i prefer to keep as is for literal argument parsing
| const modelProvider = modelDetail.litellm_provider?.toLowerCase(); | ||
| const modelNameProviderPart = remoteModelName | ||
| .split("/")[0] | ||
| .toLowerCase(); |
There was a problem hiding this comment.
same for remoteModelName might as well call that litellmModel
There was a problem hiding this comment.
i'd prefer to keep as is
This is computed by running ``` cd packages/proxy npx tsx scripts/sync_models.ts check-prices --write ```
2 participants
Our friends at https://github.com/BerriAI/litellm maintain a great registry of model costs which we can reuse. This script helps us automatically keep things in sync.
To facilitate this, I moved the models into their own file. Note that this change does not include the application of any changes from this script. That's in a separate PR so we can clearly see what they are.