fix(catalog): bound custom native-id effort lists on gateways - #4499
Conversation
Intersect explicit custom reasoning lists with pinned native metadata whenever the model id itself is capability-backed, including YYLJ/gpt-6-astra. Desktop validates the model id, so none/minimal must not remain on those catalog rows. Full native identity still requires the canonical openai Codex-forward destination. Stored configuration and request-time clamps are unchanged. Refs #3775. Original report by @leonclab. #3804 already bounded the canonical forward case; this is the remaining catalog projection.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7755142d71
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Catalog-advertised efforts are bounded whenever the model id is a pinned native | ||
| // slug. Desktop validates that id, so a gateway such as YYLJ/gpt-6-astra still cannot | ||
| // advertise none/minimal. Full native identity stays behind the alias predicate. | ||
| const nativeEffortSource = hasNativeOpenAiCapabilityMetadata(cm.modelId); |
There was a problem hiding this comment.
Limit native bounding to explicitly declared custom ladders
When a gpt-6-astra custom-model entry omits reasoningEfforts, line 2672 first inherits the provider-derived ladder, but this unconditional native-id check then treats that inherited array as an explicit custom ladder. For example, an openai-apikey custom override that only changes the display name inherits the registry's low–max ladder and now loses the normally synthesized ultra rung because preservePinnedNativeCustomReasoning subsequently preserves the bounded list exactly. This violates the existing omitted-key contract (“leaves the provider-derived ladder in charge”); gate this bound on cm.reasoningEfforts being explicitly present rather than on the merged model alone.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughCustom model rows with pinned native capabilities now receive bounded reasoning ladders on arbitrary gateways. Catalog synchronization preserves those ladders, while full native identity remains restricted to canonical routing conditions. Documentation and integration tests describe and verify the updated behavior. ChangesCustom native capability bounds
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant CustomModel
participant ProviderCatalog
participant NativeMetadata
participant CatalogSync
CustomModel->>ProviderCatalog: submit model id and reasoning efforts
ProviderCatalog->>NativeMetadata: resolve pinned native capability metadata
NativeMetadata-->>ProviderCatalog: return native ladder and default
ProviderCatalog->>CatalogSync: provide bounded custom row
CatalogSync->>CatalogSync: preserve pinned reasoning efforts
CatalogSync-->>CustomModel: expose synchronized catalog row
Merge Risk: ⚪ Minimal · up to The catalog now removes unsupported reasoning efforts for pinned native models on gateways without granting native routing identity. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…epseek-flash-vision Lane I3 of the contributor carry train: the remaining #3775 catalog defect, and the deepseek-flash native-multimodal fix carried from #4467 by jaychou0642-create. #4467 was found by the dispatch-time ownership re-check rather than by the candidate harvest — it was opened after the harvest and before this lane, by the person who filed #4436 — so it is carried with a Co-authored-by trailer instead of being reimplemented. That check exists because lane I1 implemented #4442 fresh while contributor draft #4465 had already proposed it. The #3775 link is an implementation with no source branch. It bounds custom native-id effort lists on gateways, which is what #3804 deliberately left open pending Desktop gateway evidence; the original report is that evidence, since Desktop names gpt-6-astra in the 400 rather than the provider prefix. #4349 and the #4409 ladders do not fix this catalog projection. Cross-platform CI run 34751593123 concluded success on 9f318cb, the exact head merged here, and it covers both links because the lane is cumulative. #4499 carries no ci check of its own under the owner-authorized tip-only CI economy for this batch.
…work lidge-jun#4077 proposed opening the Grok OAuth lane to service_tier priority and correcting the Fast-tier catalog copy. The registry half landed independently through lidge-jun#4431 with a narrower, live-probed scope and no reference to the proposal; the copy correction landed later through lidge-jun#4474 with a trailer. The first half is recorded as an acknowledgement rather than as carried code. Also records the gate's false-positive mode: a description that merely talks about a carry train fails missing_coauthor_credit even with no source author, which lidge-jun#4499 hit. Writing around it is cheaper than loosening the matcher.
Summary
0a4e4bc9bf): Codex Desktop 0.153.4 rejectsminimal/noneon custom models whose model id isgpt-6-astra, including the reported gateway rowYYLJ/gpt-6-astra.openaiCodex-forward custom Astra rows. It deliberately left unproven gateways unbounded and documented that Desktop gateway evidence was still required. The original report is that evidence: Desktop names'gpt-6-astra'in the 400, not the provider prefix.nativeEffortClampis unchanged and still skips routed slugs.reasoningEffortslist with pinned native metadata whenever the model id has native capability metadata, including arbitrary gateways. Full native identity (Responses Lite, multi-agent, context windows, display names) still requires the canonicalopenaiforward destination. Stored configuration is unchanged.Co-authored-bytrailer.[skip ci]; hosted suite proof is the lane tip, fix(provider): treat deepseek-flash as native multimodal #4500.Verification
bun test tests/codex-integration/codex-catalog.test.ts -t 'does not inherit native identity|gateway custom Astra|canonical custom Astra bounds|Astra API registry metadata'^T pass, including the YYLJ reproduction (none/minimalstripped, alias unset, stored config unchanged) and the API Astra row still advertising syntheticultra.bun test tests/codex-integration/codex-catalog.test.ts tests/claude-integration/claude-models-discovery.test.ts^T 352 pass, includingclient_version=0.153.4discovery bounding bothopenai/gpt-6-astraandYYLJ/gpt-6-astrato["low"].bun test tests/codex-integration/codex-catalog-sync-hardening.test.ts tests/codex-integration/codex-convergence-account-selectors.test.ts^T pass.bun run typecheck,bun run structure:check,bun run privacy:scan^T pass.ci.ymlis skipped by design; the lane tip run is the suite proof.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
noneandminimallevels for applicable custom models.Documentation