fix(browser): support GPT-5.6 Sol Pro as a selectable target - #342
fix(browser): support GPT-5.6 Sol Pro as a selectable target#342Learn2Solve wants to merge 2 commits into
Conversation
Requesting Sol's Pro effort was impossible: `gpt-5.6-sol-pro` was rejected by the
variant parser as an unknown variant, and even when the account was already on Pro
the selection guards vetoed it.
Measured against ChatGPT on 2026-07-27, the intelligence picker renders as:
Instant5.5 / Medium / High / Extra High / Pro role=menuitemradio
GPT-5.6 Sol role=menuitem, aria-haspopup=menu
So Pro is an effort radio layered on the active model - the same shape gpt-5.5-pro
already maps to ("Pro") - while "GPT-5.6 Sol" is a submenu opener. With the account
on Sol Pro, the composer pill reads "Pro", which made `canTrustSelectedOption` fail
on `desiredVersion === '5-6' -> return !hasProComposerPill()`, plus three sibling
guards keyed on `desiredModelVariant === 'sol' && hasProComposerPill()`. Those are
correct for plain Sol and exactly backwards once Pro is what was asked for.
Changes:
- add `gpt-5.6-sol-pro` to ModelName, mapping to the browser label "Pro" (same as the
other Pro tiers) and surviving normalizeChatGptModelForBrowser;
- accept the `sol pro` / `sol-pro` / `pro` variants in the GPT-5.6 label parser. Sol is
the only 5.6 model ChatGPT offers a Pro effort for, so a bare `gpt-5.6-pro` is
unambiguous;
- gate the four Pro-pill vetoes on `!wantsPro`, and require the pill rather than forbid
it when the caller asked for Sol Pro;
- the API side has no Pro model id (verified 2026-07-27: /v1/models/gpt-5.6-pro and
/v1/models/gpt-5.6-sol-pro both 404 "does not exist"), so API runs resolve to
gpt-5.6-sol at its highest exposed effort.
Verified end to end against a live ChatGPT Pro account:
before: Unable to find model option matching "GPT-5.6 Sol" in the model switcher.
Available: Instant5.5, Medium, High, Extra High, Pro, GPT-5.6 Sol.
after: Model selection evidence: requested=Pro; resolved=Pro;
strategy=select; verified=yes
Answer: GPT-5.6 Pro,高推理档位。
Test suite unchanged in outcome: 8 pre-existing failures before and after (Azure /
route-plan cases that fail on a clean checkout in this environment too).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b1f0b7c51
ℹ️ 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".
| const BROWSER_MODEL_LABELS: [ModelName, string][] = [ | ||
| // Most specific first (e.g., "gpt-5.2-thinking" before "gpt-5.2") | ||
| // Pro is an effort radio in the same picker, exactly like gpt-5.5-pro below. | ||
| ["gpt-5.6-sol-pro", "Pro"], |
There was a problem hiding this comment.
Preserve the Sol version when selecting Pro
When the account's active model is not already GPT-5.6 Sol, mapping this target to the bare Pro label discards both the desired version and variant before ensureModelSelection runs. The selector consequently sees desiredVersion and desiredModelVariant as null, can click or accept the currently visible Pro effort for another model, and verifies it as a generic Pro selection, silently producing results from a non-Sol model. Pass enough target identity for the picker to navigate to Sol before selecting its Pro effort.
Useful? React with 👍 / 👎.
| "gpt-5.6-sol-pro": { | ||
| model: "gpt-5.6-sol", |
There was a problem hiding this comment.
Route the browser-only alias to the real API model
For an API or auto-selected API run using gpt-5.6-sol-pro, resolveEffectiveModelId uses config.apiModel ?? model, so this model field does not establish the intended API fallback and the CLI stores gpt-5.6-sol-pro as effectiveModelId. run.ts prioritizes that value and sends the nonexistent alias on the wire, producing the 404 described by this change instead of calling gpt-5.6-sol; represent the requested alias separately and set its API model mapping explicitly.
Useful? React with 👍 / 👎.
| export type KnownModelName = | ||
| | "gpt-5.6" | ||
| | "gpt-5.6-sol" | ||
| | "gpt-5.6-sol-pro" |
There was a problem hiding this comment.
Classify Sol Pro as a Pro model
Adding this only to KnownModelName leaves it out of ProModelName and PRO_MODELS, so isProModel("gpt-5.6-sol-pro") remains false. In browser mode, shouldDetachSession therefore runs this long Pro consultation in the foreground rather than the resilient detached worker used for every other Pro target, allowing foreground termination to stop answer capture; add the new target to the Pro classification as well.
Useful? React with 👍 / 👎.
All three findings were real; verified each against the code before changing it.
P1 - "Preserve the Sol version when selecting Pro". Correct, and it reintroduced the
exact silent-wrong-model failure this PR set out to remove: mapping the target to a bare
"Pro" label left desiredVersion and desiredModelVariant null, so the guards never fired
and any active model's Pro effort would verify as success. The label now keeps its full
identity ("GPT-5.6 Sol Pro"), and because the Pro radio's own text carries neither the
version nor the variant, identity is read from the surrounding picker instead:
pickerShowsGpt56Sol() requires the open menu (or the composer signal) to evidence Sol,
and both the version/variant guards and canTrustSelectedOption defer to it for this tier.
P1 - "Route the browser-only alias to the real API model". Correct:
resolveEffectiveModelId returns `apiModel ?? model` and that id goes on the wire, so the
config entry now sets `apiModel: "gpt-5.6-sol"` explicitly rather than relying on the
`model` field.
P2 - "Classify Sol Pro as a Pro model". Correct: added to ProModelName and PRO_MODELS, so
isProModel() is true and shouldDetachSession() runs these long consultations in the
detached worker like every other Pro target. Confirmed live - the run now reports
"Detached session worker".
One more spot the review did not name but the same reasoning reaches:
assertResolvedModelSelection rejected any resolved label containing "pro" for a 5.6
target. That token disqualifies a plain Sol selection and is required for a Sol Pro one,
so it is now compared against what was asked for, and the error message names the tier.
Re-verified live after the changes:
requested=GPT-5.6 Sol Pro; resolved=GPT-5.6 Sol Pro; verified=yes
Answer: GPT-5.6 Pro,Pro 推理档位。
Suite outcome still unchanged: the same 8 pre-existing failures as a clean checkout.
|
Thanks — all three findings were real, and I verified each against the code before changing anything. Pushed in c6b3832. P1 "Preserve the Sol version when selecting Pro" — correct, and worse than cosmetic: it reintroduced the exact silent-wrong-model failure this PR set out to remove. A bare The label now keeps its full identity ( P1 "Route the browser-only alias to the real API model" — correct. P2 "Classify Sol Pro as a Pro model" — correct. Added to One more spot the review did not name, but the same reasoning reaches it: Re-verified live after the changes: Suite outcome unchanged: same 8 pre-existing failures as a clean checkout in this environment (Azure / route-plan cases). |
|
Codex review: found issues before merge. Reviewed July 31, 2026, 5:39 PM ET / 21:39 UTC. ClawSweeper reviewWhat this changesThe branch adds Merge readinessThis PR has credible live proof for its proposed alias, but current Priority: P2 Review scores
Verification
How this fits togetherOracle parses a CLI model request into a ChatGPT browser-picker target, verifies that the picker resolved the intended model, then applies the requested thinking-time effort. API runs use the same model input but resolve through a separate API model-id mapping. flowchart LR
A[CLI model and effort flags] --> B[Model alias parsing]
B --> C[Browser model label]
C --> D[ChatGPT model picker]
D --> E[Selected-model verification]
A --> F[Thinking-time selector]
F --> G[Sol Pro effort]
B --> H[API model fallback]
Decision needed
Why: Both paths can express the same browser effort tier, but choosing a second accepted model name changes the CLI compatibility contract and API fallback semantics beyond a mechanical bug repair. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep Do we have a high-confidence way to reproduce the issue? Yes, source and the merged adjacent PR establish a high-confidence current-main path: request Is this the best way to solve the issue? No, not without maintainer direction: the patch implements a second public spelling for an effort tier that current main already exposes through the documented model-plus-effort interface. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 68b8c51b0ee0. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
Problem
Requesting Sol's Pro effort is currently impossible.
gpt-5.6-sol-prois rejected by the variant parser (Unknown GPT-5.6 browser variant "sol pro". Use gpt-5.6 or gpt-5.6-sol.), and even when the account is already on Sol Pro, selection fails:Note "GPT-5.6 Sol" is listed as available yet reported as not found.
Why
Dumped the live picker DOM (ChatGPT, 2026-07-27):
Instant5.5/Medium/High/Extra High/PromenuitemradioProhasaria-checked=trueGPT-5.6 Solmenuitemaria-haspopup="menu"So Pro is an effort radio layered on the active model - the same shape
gpt-5.5-proalready maps to ("Pro") - whileGPT-5.6 Solis a submenu opener, not a selectable model row.With the account on Sol Pro the composer pill reads
Pro, which trips:canTrustSelectedOption:if (desiredVersion === '5-6') return !hasProComposerPill();desiredModelVariant === 'sol' && hasProComposerPill()Those are right for plain Sol and exactly backwards once Pro is what was asked for.
This is adjacent to #340 (which taught
thinkingTime: heavyto recognise the Pro effort control) but distinct: that fix assumes Sol can already be selected, and the selection step itself is what fails here.Change
gpt-5.6-sol-protoModelName, mapping to browser label"Pro"and survivingnormalizeChatGptModelForBrowsersol pro/sol-pro/proin the GPT-5.6 label parser - Sol is the only 5.6 model with a Pro effort, so baregpt-5.6-prois unambiguous!wantsPro; require the pill instead of forbidding it when Sol Pro was requested/v1/models/gpt-5.6-proand/v1/models/gpt-5.6-sol-proboth 404does not exist, verified 2026-07-27), so API runs resolve togpt-5.6-solat its highest exposed effortVerification
Live ChatGPT Pro account, before vs after:
verified=yesis the point - previously the only way to reach Pro was--browser-model-strategy current, which skipsassertResolvedModelSelectionentirely and silently inherits whatever the account defaults to (it returned GPT-5.3 for me).Tests added to
runOptions,cli/options,cli/browserConfig. One existing assertion changed intentionally:gpt-5.6-prono longer throws, since it now resolves;GPT-5.6 Luna/GPT-5.6 Terrastill do.Suite outcome unchanged: 8 pre-existing failures before and after (Azure / route-plan cases that also fail on a clean checkout in my environment).