You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[medium] [possible mistake]packages/core/src/sync/providers/nano-gpt.ts:272,288 - Check:resolveNanoGptBaseModel only strips the :thinking suffix and only resolves canonical base models for IDs that carry an org prefix (plus the zai-org/, TEE/, and explicit alias cases). Why:providers/nano-gpt/provider.toml documents three NanoGPT model-ID variants — :thinking, legacy -thinking, and Anthropic :<number> budgets — but the code only strips :thinking. Existing catalog entries such as claude-haiku-4-5-20251001-thinking and claude-sonnet-4-5-20250929-thinking (legacy -thinking) have matching models/anthropic/*.toml metadata, as do unprefixed IDs like gemini-2.5-pro (models/google/gemini-2.5-pro.toml) and qwen3.5-27b (models/alibaba/qwen3.5-27b.toml). For every one of these, resolveCanonicalBaseModel returns undefined because there is no / or the suffix is not stripped, so the sync writes a full inline definition instead of using base_model. That conflicts with the AGENTS.md "New models" blocker: "Must use base_model when a models/ metadata entry exists." Action: Either extend resolveNanoGptBaseModel to strip legacy -thinking and :<number> suffixes and to discover canonical metadata for unprefixed IDs (e.g., by probing known provider directories), or confirm that the live ?detailed=true API always returns org-prefixed, :thinking-only IDs for models with matching models/ entries so the legacy/unprefixed existing files are removed as stale rather than rewritten inline.
[low] [possible mistake] PR body — Check: Citations for the NanoGPT catalog API and the reasoning_efforts field. Why: The sync consumes reasoning_efforts as the verified source for reasoning_options and depends on the ?detailed=true endpoint shape, but the PR body does not link to the API reference or model documentation that proves the field exists and maps to the documented reasoning_effort request values (none, minimal, low, medium, high, xhigh). providers/nano-gpt/provider.toml already cites the docs, but the PR body itself states nothing about what each citation supports, so the reasoning-option mapping cannot be reviewed from the PR alone. Action: Add an "Evidence" section to the PR body citing the NanoGPT API reference for the detailed=true catalog response and the reasoning_efforts field, stating exactly what each link proves.
canonical lookup now normalizes legacy -thinking and Anthropic :<number> budget suffixes, then deterministically probes canonical metadata for unprefixed IDs
regression coverage includes the cited Claude, Gemini, and Qwen examples
generated-output verification confirms those entries now serialize with base_model
the PR body now includes an Evidence section linking the detailed catalog, live reasoning_efforts source, reasoning-effort semantics, and suffix documentation
The validation/SDK workflow is green; the refreshed automated review is still running.
[high] [violation]packages/core/src/sync/providers/nano-gpt.ts:237 - Check: Provider models must use base_model when matching models/ metadata exists. Why:TEE/ handling only rewrites deepseek/qwen/glm. Other TEE IDs (e.g. TEE/kimi-k2.5, TEE/gpt-oss-120b, TEE/gemma-4-31b-it) fall through still prefixed with TEE/, and if (direct !== undefined || normalized.includes("/")) return direct then skips the unprefixed resolver. Canonical entries such as moonshotai/kimi-k2.5, openai/gpt-oss-120b, and google/gemma-4-31b-it are never linked, so sync keeps full duplicated provider-agnostic fields. Action: After stripping TEE/, resolve the remainder through the same alias/canonical/unprefixed path used for bare IDs (not only deepseek/qwen/glm), and add tests for those TEE models.
[high] [violation]packages/core/src/sync/providers/nano-gpt.ts:96 - Check:base_model must point at the matching models/<provider>/<model>.toml entry. Why:"xiaomi/mimo-v2.5-pro-ultraspeed": "xiaomi/mimo-v2.5-pro" forces the wrong canonical model even though models/xiaomi/mimo-v2.5-pro-ultraspeed.toml already exists (different name, dates, weights). Without the alias, resolveCanonicalBaseModel("xiaomi/mimo-v2.5-pro-ultraspeed") would select the correct metadata. Action: Remove this alias (or retarget it to xiaomi/mimo-v2.5-pro-ultraspeed) and cover it in the base-model tests.
[medium] [violation]packages/core/src/sync/providers/nano-gpt.ts:226 - Check: Variant model IDs should factor against canonical metadata after stripping NanoGPT suffixes. Why: Suffix cleanup only removes a trailing :\d+ and one final :thinking/-thinking. Catalog IDs such as anthropic/claude-opus-4.6:thinking:low (and :medium/:high/:max) keep the extra effort suffix, so canonical resolution misses anthropic/claude-opus-4-6 and emits full inline copies. Action: Strip chained NanoGPT variant suffixes (at least :thinking:<effort> and repeated thinking markers) before alias/canonical lookup, with tests for those IDs.
[high] [violation]packages/core/src/sync/providers/nano-gpt.ts:146 - Check: Sync must preserve hand-authored provider fields when the API is not authoritative (sync.md). Why:preserveExistingOverrides is only true when existing.base_model === baseModel. On first factoring of the many full NanoGPT TOMLs (no base_model yet), API-absent fields fall through to undefined and inherit canonical metadata instead of keeping local overrides. That drops provider-specific values such as reasoning, tool_call, structured_output, limits, and modalities whenever the detailed payload omits them—e.g. providers/nano-gpt/models/anthropic/claude-sonnet-4.6.toml (reasoning = false, limit.output = 128_000, structured_output = true) against models/anthropic/claude-sonnet-4-6.toml (reasoning = true, output = 64_000, no structured_output). Action: When the source does not authoritatively set a field, fall back to existing for any local model (not only already-factored ones)—same pattern as DeepInfra/LLM Gateway. Keep the stricter “don’t copy full-definition junk into a new factor” behavior only for fields you intentionally want to re-inherit (e.g. description), and add a first-factor test that API-silent reasoning/limit.output/structured_output retain the existing provider overrides.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This supersedes #2891 with the sync implementation separated from generated catalog data, addressing the automated review feedback.
reasoning_effortsrather than inferring controls from IDslimit.inputfrom the source context lengthbase_modelmetadataprovidersfieldSafety and compatibility
Evidence
/api/v1/models?detailed=truecatalog and its detailed pricing, limits, and capability fields.reasoning_effortsarrays consumed by this sync.none,minimal,low,medium,high, andxhigh.base_modellookup.Verification
bun test packages/core/test/sync.test.ts— 50 passingbun validate