Skip to content

fix(catalog): bound custom native-id effort lists on gateways - #4499

Merged
lidge-jun merged 2 commits into
devfrom
codex/260913-i3-3775-custom-native-effort-bound
Sep 13, 2026
Merged

fix(catalog): bound custom native-id effort lists on gateways#4499
lidge-jun merged 2 commits into
devfrom
codex/260913-i3-3775-custom-native-effort-bound

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Summary

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/minimal stripped, alias unset, stored config unchanged) and the API Astra row still advertising synthetic ultra.
  • bun test tests/codex-integration/codex-catalog.test.ts tests/claude-integration/claude-models-discovery.test.ts ^T 352 pass, including client_version=0.153.4 discovery bounding both openai/gpt-6-astra and YYLJ/gpt-6-astra to ["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.
  • Local full suite not run. This branch's ci.yml is skipped by design; the lane tip run is the suite proof.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features

    • Custom models with pinned native capabilities now retain compatible reasoning-effort levels across supported gateways.
    • Advertised reasoning levels are intersected with the model’s native capabilities; unsupported levels are removed and defaults are adjusted when needed.
    • Explicitly empty reasoning-effort lists remain empty.
  • Bug Fixes

    • Custom gateway models no longer receive incompatible synthetic reasoning tiers.
    • Desktop validation now excludes unsupported none and minimal levels for applicable custom models.
  • Documentation

    • Updated model catalog and configuration guidance to clarify reasoning-effort behavior and native identity requirements.

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.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 13, 2026 10:09
@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • missing_coauthor_credit — This pull request says it reimplements, supersedes, carries, or rebases another author's pull request, but no Co-authored-by trailer names that author. Prose in a commit body is not read by anything; the trailer is what GitHub counts. Add it to the description or a commit, or obtain attribution-approved. Paths: #4467.

@github-actions github-actions Bot added the bug Something isn't working label Sep 13, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-13T10:13:42.892013Z 7755142 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft September 13, 2026 10:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 lowmax 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 👍 / 👎.

@github-actions github-actions Bot removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 13, 2026
@github-actions
github-actions Bot marked this pull request as ready for review September 13, 2026 10:38
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 01dbf9f4-c95f-4f2f-a928-d2e5dc1906d6

📥 Commits

Reviewing files that changed from the base of the PR and between eb81eaa and 7755142.

📒 Files selected for processing (7)
  • docs-site/src/content/docs/guides/codex-app-models.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • src/codex/catalog/provider-fetch.ts
  • src/codex/catalog/sync.ts
  • structure/catalog.md
  • tests/claude-integration/claude-models-discovery.test.ts
  • tests/codex-integration/codex-catalog.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Custom 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.

Changes

Custom native capability bounds

Layer / File(s) Summary
Reasoning bounds and catalog rules
src/codex/catalog/provider-fetch.ts, docs-site/src/content/docs/guides/codex-app-models.md, docs-site/src/content/docs/reference/configuration/providers.md, structure/catalog.md
Custom rows with pinned native metadata are bounded against their native reasoning ladder, including gateway-hosted ids. Canonical native identity rules remain unchanged.
Catalog synchronization
src/codex/catalog/sync.ts
Catalog derivation preserves pinned reasoning ladders for eligible custom rows in templated and fallback routed paths.
Gateway and identity validation
tests/codex-integration/codex-catalog.test.ts, tests/claude-integration/claude-models-discovery.test.ts
Tests verify sentinel removal, default repair, catalog output, gateway behavior, native identity separation, and unchanged routed-row fields.

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
Loading

Merge Risk: ⚪ Minimal · up to 77551

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: bounding custom native-ID reasoning-effort lists for gateway models. It is concise, specific, and consistent with the pull request objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/260913-i3-3775-custom-native-effort-bound

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun
lidge-jun merged commit b0080b7 into dev Sep 13, 2026
14 of 17 checks passed
lidge-jun added a commit that referenced this pull request Sep 13, 2026
…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.
@lidge-jun
lidge-jun deleted the codex/260913-i3-3775-custom-native-effort-bound branch September 13, 2026 10:51
FacuM pushed a commit to FacuM/opencodex that referenced this pull request Sep 13, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant