fix(browser): accept GPT-5.6 Sol with an independent Pro pill - #320
fix(browser): accept GPT-5.6 Sol with an independent Pro pill#320jung0han wants to merge 4 commits into
Conversation
![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: 1dea652bdb
ℹ️ 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 allowsIndependentProPill = desiredVersion === '5-6' && desiredModelVariant === 'sol'; | ||
| return !wantsPro && !allowsIndependentProPill && hasProComposerPill(); |
There was a problem hiding this comment.
When a user selects base GPT-5.6 Sol without --browser-thinking-time pro, this exception makes an already-active independent Pro composer pill count as compatible, so activeSelectionMatchesTarget() can return success and no later reset occurs because ensureThinkingTime only runs when config.thinkingTime is set. In sessions where ChatGPT carries over a previous Pro effort, oracle --engine browser --model gpt-5.6-sol will therefore submit at Pro effort instead of the requested base Sol/default effort; the allowance needs to be conditioned on an explicit Pro thinking-time request or otherwise clear the pill.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ab15e7d. Sol/Pro coexistence now requires the explicit thinkingTime === "pro" signal passed from browser config; base Sol with a carried-over Pro pill fails closed. Added regression coverage for both paths and reran a signed-in Sol+Pro smoke.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs changes before merge. Reviewed July 31, 2026, 5:40 PM ET / 21:40 UTC. ClawSweeper reviewWhat this changesThe PR changes Oracle’s ChatGPT browser model matcher to recognize GPT-5.6 Sol when Pro is presented as a separate composer effort control, with focused matcher regressions. Merge readiness⛔ Blocked by patch quality or review findings - 5 items remain This PR remains necessary for the Sol-plus-separate-effort UI, but the current head still has the previously reported P1 compatibility regression: a base GPT-5.6 Sol request can accept a carried-over Pro pill without an explicit Pro effort request. The branch should not merge until that success path is gated on requested effort or the inherited pill is cleared. Priority: P1 Review scores
Verification
How this fits togetherOracle’s browser engine inspects the signed-in ChatGPT composer to select and verify the requested model before it optionally applies a requested thinking effort. The matcher’s result determines whether Oracle proceeds to prompt submission using the existing composer state. flowchart LR
Request[Browser run request] --> Model[Requested GPT-5.6 Sol model]
Composer[ChatGPT composer state] --> Match[Model selection matcher]
Model --> Match
Match --> Verify[Verified selected model]
Effort[Optional requested thinking effort] --> EffortStep[Effort selector]
Verify --> EffortStep
EffortStep --> Submit[Prompt submission]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Pass the requested thinking-time intent into the Sol matcher and accept an independent Pro pill only for an explicit Pro request; otherwise reject or clear that carried-over effort before submitting. Do we have a high-confidence way to reproduce the issue? Yes, by source: request base GPT-5.6 Sol with no configured thinking time while the composer retains an independent Pro pill; the PR returns model success and the later effort step is skipped. Is this the best way to solve the issue? No. Separating Sol model evidence from the independent effort control is appropriate, but the current patch must condition that exception on requested Pro intent or reset the pill for base-Sol requests. 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 justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (10 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
@codex review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
ab15e7d to
aa1efd3
Compare
|
@clawsweeper re-review Author update:
Please re-review head aa1efd3. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦞 diamond lobster
|
@clawsweeper re-review Author update for head
This directly removes the persisted/inherited public-contract concern from the previous review. Please review the new head. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
removed
the
merge-risk: 🚨 compatibility
clawsweeper
Bot
removed
status: 👀 ready for maintainer look
jung0han
changed the title
|
@clawsweeper re-review Author update for head
Please review the new minimal head and ignore the superseded public-contract discussion. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
rating: 🦪 silver shellfish
1 participant
Summary
GPT-5.6 Sol Promodel label without pill evidence remains rejectedReproduction
With GPT-5.6 Sol selected and the independent Pro pill active, current
mainrejects the model before submission:The model matcher currently treats the visible Pro effort as evidence that the selected model is not Sol. This patch keeps Pro rows excluded as model candidates, but stops using an independently observed effort pill to invalidate confirmed Sol version evidence.
Scope
mainVerification
Verified from a clean
v0.16.0worktree with only this PR's two-file diff applied:pnpm checkpnpm vitest run: 144 files passed, 18 skipped; 1,559 tests passed, 43 skippedpnpm buildLive signed-in Chrome smoke using a clean Homebrew Oracle 0.16.0 install plus only the generated
modelSelection.jsfrom this PR:The canonical Chrome profile reported active Pro model tabs during the smoke. No
Answer nowinteraction was used.This fixes only the Sol-versus-independent-effort false negative. It does not add Pro selection or address response-capture/
Answer nowbehavior.