Nanogpt custom canonical loader#7538
Conversation
![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: 70f91e4204
ℹ️ 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".
| Ok(models) => { | ||
| let old_count = registry.get_all_models_for_provider(&provider).len(); | ||
| registry.remove_provider(&provider); | ||
|
|
There was a problem hiding this comment.
This branch removes all existing provider entries before validating that the custom loader actually returned any replacements. If the NanoGPT API returns HTTP 200 with an empty data array (for example during a transient backend issue), this path deletes every nano-gpt/* canonical model and writes none back, which silently drops mapping coverage for that provider instead of preserving the models.dev fallback.
Useful? React with 👍 / 👎.
| registry.register(&provider, &model_name, model); | ||
| } | ||
|
|
||
| total_models = total_models - old_count + new_count; |
There was a problem hiding this comment.
The custom-loader phase updates only total_models, but provider metadata is still generated from the original models.dev payload later, so provider_metadata.json can drift from the canonical registry whenever a loader returns a different count. Because catalog views consume model_count from provider metadata, users may see stale model totals even though the underlying canonical models were replaced.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b948f75bc
ℹ️ 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".
| "NANO_GPT_API_KEY" | ||
| ], | ||
| "model_count": 7 | ||
| "model_count": 33 |
There was a problem hiding this comment.
This metadata entry is stale relative to the generated canonical registry in the same commit: canonical_models.json now contains hundreds of nano-gpt/* models (562 entries), but provider_metadata.json still reports model_count: 33, so catalog consumers that read metadata.model_count (for example get_providers_by_format in crates/goose/src/providers/catalog.rs) will under-report NanoGPT coverage even though those models are bundled. Regenerate or update this entry so provider metadata matches the committed canonical model data.
Useful? React with 👍 / 👎.
| actual_ids.sort(); | ||
|
|
||
| let mut expected_ids: Vec<String> = expected_model_ids.iter().map(|s| s.to_string()).collect(); | ||
| expected_ids.sort(); |
There was a problem hiding this comment.
The models shift around in order if their dates get updated on refreshing models.dev.
This just treats it as a set now. (although I suspect this is also going to cause problems when new models come)
|
Close this PR as model.dev has included nanogpt models recently. updated canonical models in this PR #7920 |
4 participants
The data in models.dev is pretty out of date, but nanogpt serves their own model info with similar data.
Adding a way for us to replace model info for specific providers from other sources.