Support GPT-5.6 max and ultra reasoning levels#209
Open
DVerdeV wants to merge 2 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe change centralizes reasoning-effort values, adds ChangesGPT-5.6 reasoning effort support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ThreadComposer
participant useDesktopState
participant getAvailableModels
participant CodexAppServer
ThreadComposer->>useDesktopState: select model and reasoning effort
useDesktopState->>getAvailableModels: refresh model metadata
getAvailableModels->>CodexAppServer: request model/list
CodexAppServer-->>getAvailableModels: return supported/default efforts
getAvailableModels-->>useDesktopState: return normalized models
useDesktopState-->>ThreadComposer: expose supported options
ThreadComposer->>CodexAppServer: start turn with validated reasoning_effort
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
PR Summary by QodoSupport GPT-5.6 'max' and 'ultra' reasoning levels
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. Fallback picks none
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+5
to
+20
| export const REASONING_EFFORTS = [ | ||
| 'none', | ||
| 'minimal', | ||
| 'low', | ||
| 'medium', | ||
| 'high', | ||
| 'xhigh', | ||
| 'max', | ||
| 'ultra', | ||
| ] as const | ||
|
|
||
| export type ReasoningEffort = (typeof REASONING_EFFORTS)[number] | ||
|
|
||
| export function isReasoningEffort(value: unknown): value is ReasoningEffort { | ||
| return typeof value === 'string' && REASONING_EFFORTS.some((effort) => effort === value) | ||
| } |
There was a problem hiding this comment.
1. Schema effort union stale 🐞 Bug ⚙ Maintainability
The PR adds 'max'/'ultra' to the runtime ReasoningEffort union, but the in-repo app-server schema/types still define ReasoningEffort only through 'xhigh', leaving two conflicting sources of truth. Any code that relies on the schema-generated types (re-exported via appServerDtos) will remain unable to represent these new values without casts/workarounds.
Agent Prompt
### Issue description
The repository now has conflicting definitions of `ReasoningEffort`: runtime/UI types allow `max`/`ultra`, but the schema-generated protocol types still only allow up to `xhigh`. This mismatch will cause friction and incorrect typing anywhere schema types are used.
### Issue Context
`src/api/appServerDtos.ts` re-exports the schema-generated `ReasoningEffort`, so even though the current PR updated runtime normalization to accept new values, the protocol type artifacts remain stale.
### Fix Focus Areas
- documentation/app-server-schemas/typescript/ReasoningEffort.ts[1-8]
- documentation/app-server-schemas/json/v2/ModelListResponse.json[85-96]
- documentation/app-server-schemas/json/v2/TurnStartParams.json[1-200]
- src/api/appServerDtos.ts[1-18]
### Notes
- Regenerate these files from the upstream schema generator if applicable, or update them in-place if this repo is the source of truth.
- Ensure all schema references to `ReasoningEffort` (v1/v2, events, requests) include `max` and `ultra` where appropriate.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
/review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/composables/useDesktopState.test.ts (1)
901-939: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise
maxas well asultra.This test only selects and asserts
ultra;maxis checked only as metadata. Add amaxselection/assertion, or table-drive both new values, so a regression in themaxfiltering or selection path cannot pass unnoticed.🤖 Prompt for 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. In `@src/composables/useDesktopState.test.ts` around lines 901 - 939, Extend the test covering model-specific reasoning levels to exercise both newly supported values, max and ultra, rather than only ultra. Update the selection and assertion flow around setSelectedReasoningEffort and ensure max is explicitly selected and validated, or table-drive equivalent cases so filtering and incompatible-selection behavior are covered for both values.
🤖 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 `@src/composables/useDesktopState.ts`:
- Around line 1661-1696: setAvailableModelMetadata currently replaces cached
reasoning-effort maps with data from the latest model list, allowing placeholder
records to erase known metadata. Update setAvailableModelMetadata to merge new
entries into the existing availableModelReasoningEfforts and
availableModelDefaultReasoningEfforts maps, preserving prior entries and only
updating supported, non-null metadata; continue handling explicit defaults
consistently.
In `@tests/providers-models/gpt-5-6-max-and-ultra-thinking-levels.md`:
- Around line 11-20: Update the responsive verification steps in the GPT-5.6
thinking-level test to specify the exact mobile or tablet device and viewport
dimensions, then explicitly verify all eight options remain visible and readable
in both light and dark themes.
---
Nitpick comments:
In `@src/composables/useDesktopState.test.ts`:
- Around line 901-939: Extend the test covering model-specific reasoning levels
to exercise both newly supported values, max and ultra, rather than only ultra.
Update the selection and assertion flow around setSelectedReasoningEffort and
ensure max is explicitly selected and validated, or table-drive equivalent cases
so filtering and incompatible-selection behavior are covered for both values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 85665583-9911-4933-a9d1-29ee2a798f0d
📒 Files selected for processing (10)
src/App.vuesrc/api/codexGateway.test.tssrc/api/codexGateway.tssrc/components/content/ThreadComposer.vuesrc/composables/useDesktopState.test.tssrc/composables/useDesktopState.tssrc/server/codexAppServerBridge.tssrc/types/codex.tstests/providers-models/gpt-5-6-max-and-ultra-thinking-levels.mdtests/providers-models/index.md
Comment on lines
+1661
to
+1696
| function readSupportedReasoningEffortsForModel(modelId: string): readonly ReasoningEffort[] { | ||
| return availableModelReasoningEfforts.value[modelId.trim()] ?? REASONING_EFFORT_OPTIONS | ||
| } | ||
|
|
||
| function pickReasoningEffortForModel( | ||
| modelId: string, | ||
| preferredEffort: ReasoningEffort | '' = selectedReasoningEffort.value, | ||
| ): ReasoningEffort | '' { | ||
| const normalizedModelId = modelId.trim() | ||
| const supportedEfforts = readSupportedReasoningEffortsForModel(normalizedModelId) | ||
| if (preferredEffort && supportedEfforts.includes(preferredEffort)) return preferredEffort | ||
|
|
||
| const defaultEffort = availableModelDefaultReasoningEfforts.value[normalizedModelId] | ||
| if (defaultEffort && supportedEfforts.includes(defaultEffort)) return defaultEffort | ||
| return supportedEfforts[0] ?? '' | ||
| } | ||
|
|
||
| function ensureReasoningEffortSupportedForModel(modelId: string): void { | ||
| selectedReasoningEffort.value = pickReasoningEffortForModel(modelId) | ||
| } | ||
|
|
||
| function setAvailableModelMetadata(models: AvailableModel[]): void { | ||
| const reasoningEfforts: Record<string, ReasoningEffort[]> = {} | ||
| const defaultReasoningEfforts: Record<string, ReasoningEffort> = {} | ||
| for (const model of models) { | ||
| if (model.supportedReasoningEfforts !== null) { | ||
| reasoningEfforts[model.id] = [...model.supportedReasoningEfforts] | ||
| } | ||
| if (model.defaultReasoningEffort) { | ||
| defaultReasoningEfforts[model.id] = model.defaultReasoningEffort | ||
| } | ||
| } | ||
| availableModelReasoningEfforts.value = reasoningEfforts | ||
| availableModelDefaultReasoningEfforts.value = defaultReasoningEfforts | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Full cache replace on setAvailableModelMetadata can transiently drop known reasoning-effort metadata.
setAvailableModelMetadata (called from refreshModelPreferences, Line 2039) always builds reasoningEfforts/defaultReasoningEfforts from scratch using only the models returned by the current getAvailableModels call, then unconditionally overwrites the reactive maps. When the active thread is provider-backed, getAvailableModels returns only placeholder records (supportedReasoningEfforts: null), so this wipes cached metadata for other models (e.g. Codex-native gpt-5.6-sol). Since readSupportedReasoningEffortsForModel falls back to the full 8-option list when an entry is missing, a model that only supports a subset of efforts would momentarily appear to support all of them again, until the next refresh for that model's context repopulates the cache.
Impact is transient/self-correcting (next selectThread call refreshes it), but it's a genuine cache-invalidation gap.
♻️ Suggested fix: merge instead of replace
function setAvailableModelMetadata(models: AvailableModel[]): void {
- const reasoningEfforts: Record<string, ReasoningEffort[]> = {}
- const defaultReasoningEfforts: Record<string, ReasoningEffort> = {}
+ const reasoningEfforts: Record<string, ReasoningEffort[]> = { ...availableModelReasoningEfforts.value }
+ const defaultReasoningEfforts: Record<string, ReasoningEffort> = { ...availableModelDefaultReasoningEfforts.value }
for (const model of models) {
if (model.supportedReasoningEfforts !== null) {
reasoningEfforts[model.id] = [...model.supportedReasoningEfforts]
}
if (model.defaultReasoningEffort) {
defaultReasoningEfforts[model.id] = model.defaultReasoningEffort
}
}
availableModelReasoningEfforts.value = reasoningEfforts
availableModelDefaultReasoningEfforts.value = defaultReasoningEfforts
}As per coding guidelines, "For startup, thread loading, realtime rendering, routing, API, filesystem, git, or module-loading changes, explicitly check duplicate requests, blocking work, unbounded fanout, large payloads, and cache invalidation risk."
Also applies to: 2033-2039
🤖 Prompt for 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.
In `@src/composables/useDesktopState.ts` around lines 1661 - 1696,
setAvailableModelMetadata currently replaces cached reasoning-effort maps with
data from the latest model list, allowing placeholder records to erase known
metadata. Update setAvailableModelMetadata to merge new entries into the
existing availableModelReasoningEfforts and
availableModelDefaultReasoningEfforts maps, preserving prior entries and only
updating supported, non-null metadata; continue handling explicit defaults
consistently.
Source: Coding guidelines
Comment on lines
+11
to
+20
| #### Steps | ||
| 1. Start a new chat and select `GPT-5.6-Sol` or `GPT-5.6-Terra`. | ||
| 2. Open the Thinking selector in light theme and confirm `Max` and `Ultra` are present. | ||
| 3. Select `Max`, send a prompt, and confirm the turn starts without an invalid reasoning-effort error. | ||
| 4. Select `Ultra`, send a second prompt, and confirm the turn starts without an invalid reasoning-effort error. | ||
| 5. Select `GPT-5.6-Luna` and confirm `Max` is present but `Ultra` is absent. | ||
| 6. Select `GPT-5.5` and confirm both `Max` and `Ultra` are absent. | ||
| 7. Switch from a GPT-5.6 model with `Ultra` selected to GPT-5.5 and confirm Thinking changes to GPT-5.5's default effort. | ||
| 8. Switch to dark theme and repeat the selector visibility checks. | ||
| 9. Reload the page while `Ultra` is configured for GPT-5.6 Sol or Terra and confirm the selector still displays `Ultra`. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the mobile/tablet viewport for the responsive check.
These steps never set a mobile or tablet viewport, so the “all eight options remain visible on mobile” behavior can be skipped while still following the procedure. Add the exact device/viewport used and explicitly verify text readability in both themes.
🧰 Tools
🪛 LanguageTool
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t an invalid reasoning-effort error. 5. Select GPT-5.6-Luna and confirm Max is pre...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...xis present butUltrais absent. 6. SelectGPT-5.5and confirm bothMaxandU...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for 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.
In `@tests/providers-models/gpt-5-6-max-and-ultra-thinking-levels.md` around lines
11 - 20, Update the responsive verification steps in the GPT-5.6 thinking-level
test to specify the exact mobile or tablet device and viewport dimensions, then
explicitly verify all eight options remain visible and readable in both light
and dark themes.
Source: Coding guidelines
Comment on lines
+1665
to
+1676
| function pickReasoningEffortForModel( | ||
| modelId: string, | ||
| preferredEffort: ReasoningEffort | '' = selectedReasoningEffort.value, | ||
| ): ReasoningEffort | '' { | ||
| const normalizedModelId = modelId.trim() | ||
| const supportedEfforts = readSupportedReasoningEffortsForModel(normalizedModelId) | ||
| if (preferredEffort && supportedEfforts.includes(preferredEffort)) return preferredEffort | ||
|
|
||
| const defaultEffort = availableModelDefaultReasoningEfforts.value[normalizedModelId] | ||
| if (defaultEffort && supportedEfforts.includes(defaultEffort)) return defaultEffort | ||
| return supportedEfforts[0] ?? '' | ||
| } |
There was a problem hiding this comment.
1. Fallback picks none 🐞 Bug ≡ Correctness
pickReasoningEffortForModel() falls back to supportedEfforts[0] when there is no preferred or model-default effort; for models without reasoning metadata this becomes 'none' because REASONING_EFFORTS is ordered with 'none' first. refreshModelPreferences() then assigns that value to selectedReasoningEffort, overriding the prior 'medium' default and causing turn/start to send effort: 'none' explicitly.
Agent Prompt
## Issue description
When a model lacks `supportedReasoningEfforts`/`defaultReasoningEffort` metadata, `pickReasoningEffortForModel()` falls back to the first global effort (`'none'`). `refreshModelPreferences()` then writes that into `selectedReasoningEffort`, which makes subsequent `turn/start` calls send an explicit `effort: 'none'`.
## Issue Context
- For models without metadata, `readSupportedReasoningEffortsForModel()` uses `REASONING_EFFORT_OPTIONS` (the global list).
- That list is now backed by `REASONING_EFFORTS`, whose first entry is `'none'`.
- `refreshModelPreferences()` always normalizes `selectedReasoningEffort` via `pickReasoningEffortForModel(...)`, so the fallback value becomes the new selected effort.
## Fix Focus Areas
- src/composables/useDesktopState.ts[1661-1676]
- src/composables/useDesktopState.ts[2023-2095]
### Suggested fix direction
Adjust `pickReasoningEffortForModel()` so that when `preferredEffort` is empty/unspecified and there is no model-default effort available, it returns `''` (meaning “use server/model default”) instead of `supportedEfforts[0]`. Keep the existing `supportedEfforts[0]` fallback for the case where the user had a non-empty `preferredEffort` that is incompatible and the model has no default.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 52e208e |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
maxandultrareasoning levels.supportedReasoningEffortsanddefaultReasoningEffortfrom the existingmodel/listresponse.maxandultrathrough config normalization and turn/collaboration-mode payloads.Why
Codex Mobile previously recognized reasoning levels only through
xhigh, so GPT-5.6's newer values were discarded. Simply adding them globally was also incorrect because support varies by model: GPT-5.6 Sol and Terra supportmax/ultra, Luna supportsmax, and GPT-5.5 supports neither.The selector now follows the capability metadata already returned by Codex instead of maintaining one global menu for every model.
Testing
env -u CODEXUI_MEMORIES pnpm run test:unitpnpm run buildnode dist-cli/index.js --helpLow,Medium,High,Extra high,Max,Ultra.Ultra.Low,Medium,High,Extra high.Ultrato GPT-5.5 selects GPT-5.5'sMediumdefault.Performance
No additional requests or asynchronous work were introduced. The implementation retains reasoning metadata from the existing
model/listresponse and performs bounded lookups over the small model/effort catalogs.Summary by CodeRabbit
New Features
Improvements
Tests