feat(server): advertise reasoning-effort ladders on the raw /v1/models list - #853
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe ChangesReasoning effort metadata
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Catalog as Catalog metadata
participant Server as /v1/models
participant GrokBuild as Grok Build
Catalog->>Server: Provide native reasoning tiers and default
Server->>Server: Build reasoning_efforts metadata
Server->>GrokBuild: Return model entries with tiers and defaults
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/grok-build.md`:
- Around line 51-58: Clarify the documentation around reasoningEfforts and
modelReasoningEfforts to state that these fields describe the proxy-configured
ladder for routed provider entries. Explicitly distinguish this metadata from
native upstream reasoning support, including native entries that do not
advertise these fields, and note that adapters may emulate or map effort levels
to provider-specific fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 42ef54dc-1741-4a47-b468-c514c64f4e3b
📒 Files selected for processing (3)
docs-site/src/content/docs/guides/grok-build.mdsrc/server/index.tstests/grok-models-effort-list.test.ts
![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: 108bd4b81a
ℹ️ 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".
|
Thanks for the reviews — all three comments addressed in
Full suite green after the changes: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/grok-build.md`:
- Around line 54-61: Update the reasoning metadata documentation near the routed
model description to separately state that native GPT-5.6 entries preserve and
expose their pinned upstream reasoning ladders. Keep routed entries described as
exposing proxy-configured provider tiers that adapters may map or emulate,
maintaining the distinction between native and routed behavior.
In `@tests/grok-models-effort-list.test.ts`:
- Around line 61-66: Extend the native gpt-5.6-sol assertions in the existing
test to verify its complete reasoning_efforts list matches the pinned GPT-5.6
ladder, including max and ultra, while preserving the supports_reasoning_effort
and medium default assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11b8d5e9-ffd9-47ee-9081-98691ccc8b89
📒 Files selected for processing (4)
docs-site/src/content/docs/guides/grok-build.mdsrc/codex/catalog.tssrc/server/index.tstests/grok-models-effort-list.test.ts
…s list
Grok Build discovers models through the plain OpenAI list shape at
GET /v1/models (no client_version). Its model picker only enables the
/effort control for entries that advertise the ladder in the Grok model
catalog shape (supports_reasoning_effort + reasoning_efforts[] with
value/label/default options), and --effort is ignored for models that do
not advertise it.
The raw list previously emitted only id/object/created/owned_by, so
routed models appeared without effort controls even though the same
tiers were already configured (reasoningEfforts /
modelReasoningEfforts) and shipped on the Codex catalog branch
(?client_version) and honored end-to-end by the chat-completions
inbound path.
Mirror the configured ladder onto raw list entries: entries with a
non-empty reasoningEfforts list now also carry
supports_reasoning_effort, reasoning_effort (the configured default,
falling back to the first tier when the configured default is not in
the visible list), and reasoning_efforts as {value, label, default}
options. Entries with an empty tier list are unchanged. Extra fields
are ignored by plain OpenAI clients.
…ers too Address review feedback on the reasoning-effort advertisement: - Codex review (P2): the raw list now resolves the default with the same canonical fallback as the Codex catalog resolver in src/codex/catalog/effort.ts (configured default, then medium, then high, then the first tier) instead of always falling back to the first tier, so both branches advertise the same default for a model without an explicit modelDefaultReasoningEfforts entry. - Codex review (P2): native rows are decorated too, using the canonical native ladder from nativeReasoningEfforts (upstream snapshot, with max+ultra restored for the gpt-5.6 family), so auto-registered ocx-gpt-* native models also get a working /effort control in Grok Build. - CodeRabbit (docs): the Grok Build guide now states that the mirrored tiers describe the proxy-configured routed ladder, not native upstream reasoning support, and that adapters may emulate reasoning or map levels onto provider-specific fields. The shared grokEffortFields helper builds the Grok catalog shape for both row kinds; nativeReasoningEfforts is re-exported through the codex/catalog facade. Tests cover the native advertisement, the medium-first canonical fallback with and without a configured default, and the empty-tier omission.
Address the follow-up CodeRabbit review: - Document that native GPT-5.6 rows expose their pinned upstream reasoning ladders, separately from routed entries whose tiers come from proxy provider configuration and may be mapped or emulated by adapters. - Assert the complete gpt-5.6-sol ladder (low through ultra), not only the support flag and medium default, so max/ultra cannot regress silently.
ceb5fe7 to
0938eaf
Compare
|
Addressed the two follow-up CodeRabbit findings in the latest branch:
The branch is rebased onto current |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/grok-models-effort-list.test.ts`:
- Around line 109-121: Extend the invalid-default fallback coverage in the test
around the existing k3 case with a model whose reasoning tiers are ordered
["low", "max"] and whose configured default is invalid. Assert that
reasoning_effort is "low" and that only the low option is marked default,
exercising the efforts[0] fallback branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2847fe5b-4a7f-4e6b-be2a-568f7f9a5795
📒 Files selected for processing (4)
docs-site/src/content/docs/guides/grok-build.mdsrc/codex/catalog.tssrc/server/index.tstests/grok-models-effort-list.test.ts
Address the latest CodeRabbit coverage request by exercising the final branch of the canonical configured -> medium -> high -> first fallback. Use a custom routed model with only low/max tiers so provider registry wire-map self-healing does not add a high tier, then assert low is the sole default option.
|
Addressed the latest CodeRabbit coverage request in |
…t ads gpt-5.6-sol pins default_reasoning_level=low in the Codex catalog path, but the raw list always fell through to medium. Pass the upstream native default into grokEffortFields so both discovery surfaces advertise the same default.
[shipping-github] Verdict: changes-requestedPR: UsefulnessUseful and in scope: it makes configured routed reasoning ladders, and pinned native ladders, discoverable by Grok Build through the plain OpenAI Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
GateHard blocker: approve/execute the two Bottom lineThe implementation is useful, focused, locally validated, and has no remaining confirmed bug or security blocker. I cannot give an |
|
Pre-merge review result: not merge-ready yet. @Wibias's verification covered the native-default fix, but one independently reproduced mismatch remains:
Otherwise the shape is good: the |
|
Addressed the remaining native-ladder mismatch in
Verification: focused catalog/server tests 122/122, typecheck, GUI lint, full suite 6869 pass / 8 skip / 0 fail, privacy scan, and docs build (151 pages) all passed. |
|
The latest The run otherwise completed with 6873 passing tests, and the PR's Could a maintainer please rerun the failed Windows job? |
|
[shipping-github] Addressed feedback Verified on |
|
Thanks @n3wr1ch — merging this. Why it helps: raw Ship it. |
3 participants
Summary
GET /v1/models(noclient_version), and its model picker only enables/effort(and accepts--effort) for entries that advertise the Grok catalog shape:supports_reasoning_effortplusreasoning_effortsoptions.id,object,created, andowned_by, so routed models appeared in Grok Build without an effort control even when their ladder was already configured and available on the Codex catalog branch. The chat-completions path already honorsreasoning_effort; model discovery was the missing link.medium,high, then the first tier. Entries with an empty tier list remain unchanged.ultra, while GPT-5.6 Luna intentionally ends atmax, matching OpenAI Codex's canonicalmodels.jsoncatalog.Verification
bun test ./tests/grok-models-effort-list.test.ts ./tests/codex-catalog.test.ts— 122 passed, 0 failed. The five focused raw-list tests cover routed metadata, empty-tier omission, canonical default fallbacks, and exact Sol/Terra/Luna native ladders and defaults.bun run typecheck— passed.bun run lint:gui— passed.bun run test— 6869 passed, 8 skipped, 0 failed across 479 files.bun run privacy:scan— passed.cd docs-site && bun install --frozen-lockfile && bun run build— passed (151 pages built).low,high,max, defaultmax) is returned byGET /v1/modelsin Grok's expected shape. Sendingreasoning_effortmanually already produced distinct upstream reasoning-token counts (13 atlow, 74 athigh), confirming the backend path works and only discovery metadata was missing.Checklist
devand keeps the implementation focused.