Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 383221f841
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "max_input_tokens": 200000, | ||
| "max_output_tokens": 128000 | ||
| "max_output_tokens": 128000, | ||
| "deprecationDate": "19 Feb 2026 00:00:01 GMT" |
There was a problem hiding this comment.
q: thoughts on using YYYY-MM-DD instead like the litellm list has? we have this script sync_models.ts that I think is currently broken but then we could use it to pull dates from there
alternatively we could update the script to convert that date format into what you have here so either way could work
There was a problem hiding this comment.
Sure - any format that the Date api can parse should work, so YYYY-MM-DD should work as is - I will update the config to keep it consistent.
There was a problem hiding this comment.
ok updated the dates - let me look at that script. If its an easy fix I can import those other deprecation dates as well
| "displayName": "GPT-5 mini", | ||
| "reasoning": true, | ||
| "max_input_tokens": 400000, | ||
| "max_input_tokens": 272000, |
There was a problem hiding this comment.
maybe @ibolmo you can confirm this is ok? the sync_models script made this change which pulls from litellm, but it seems like there's a discrepancy with the OpenAI documentation where the context window used to correspond to the max_input_tokens, but with the gpt-5 models it's the combined input and output. so while it says 400,000 context window you infer the max_input_tokens is 272,000 because the max_output_tokens is 128,000. not sure if you've seen issues with the current config we have
There was a problem hiding this comment.
@max-braintrust if you want to revert just these max_input_tokens changes so you can land this change go ahead, we can update again once we get confirmation that it won't affect current functionality
There was a problem hiding this comment.
fwiw, I would agree that 272,000 is likely true. we've had to split firework's max tokens in half on purpsoe for this reason (they don't specify input vs. output max tokens)
There was a problem hiding this comment.
ok that's good to know, it would be nice if we could use the script and litellm as a source of truth so that's good
There was a problem hiding this comment.
agreed. the tough part is that litellm at times is slower than us in updating their registry. thus we have update this manually. i could see a cron/routine job, though, being a watchdog/helper and we can fix our manual updates and/or capture models we didn't think to add
There was a problem hiding this comment.
seems like the sync script is getting mature enough to try to automate this
| "format": "openai", | ||
| "flavor": "chat" | ||
| "flavor": "chat", | ||
| "input_cost_per_mil_tokens": 0, |
There was a problem hiding this comment.
should we keep these? seems odd it would be 0.00
3 participants
Uh oh!
There was an error while loading. Please reload this page.