Skip to content

fix(app): keep thinking picker clicks active - #702

Merged
Astro-Han merged 4 commits into
devfrom
codex/fix-model-reasoning-variant
May 17, 2026
Merged

fix(app): keep thinking picker clicks active#702
Astro-Han merged 4 commits into
devfrom
codex/fix-model-reasoning-variant

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

Summary

Fix the prompt model picker so reasoning-effort options inside the nested thinking-level popover can be clicked and applied.

This PR adds stable test selectors for the thinking-level trigger/options and covers the user path with a focused Playwright E2E test.

Why

Closes #701.

The affected session export showed backend requests were still using openai / gpt-5.5 / variant xhigh, so the session had not actually downgraded its model variant. The failure was in the frontend interaction layer: moving focus into the nested thinking-level popover was treated as focus outside by the outer model selector popover. That closed the outer picker before the inner option click could run, so model.variant.set(...) never applied.

Related Issue

Closes #701

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please check that the focus outside guard is scoped tightly to nested picker content and does not weaken normal outside-click dismissal for the model selector.

Risk Notes

Low. The change is limited to prompt model picker dismissal behavior and a focused E2E regression test. No provider, model-routing, persistence, or generated snapshot changes are included.

How To Verify

Focused E2E: passed
PLAYWRIGHT_PORT=3127 PLAYWRIGHT_SERVER_PORT=4127 bun run test:e2e e2e/models/model-picker-thinking.spec.ts

Typecheck: passed
bun run typecheck

Diff check: passed
git diff --check

Manual Electron check: passed
bun run dev:desktop, then opened the prompt model picker, opened the thinking-level submenu, selected 超高, and confirmed the bottom model control and picker row updated to GPT-5.5 超高 / 思考档位 超高.

Screenshots or Recordings

Not attached. This is a transient picker interaction-state fix; the visible path was checked manually in Electron and is covered by the focused E2E test above.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has exactly one type label (bug, enhancement, task, or documentation), at least one primary routing label (app, ui, platform, harness, or ci), and exactly one priority label (P0 to P3), or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 5 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d8730383-5a51-41e2-b0b9-70bf4e89cca2

📥 Commits

Reviewing files that changed from the base of the PR and between 1981523 and 3047f10.

📒 Files selected for processing (3)
  • packages/app/e2e/models/model-picker-thinking.spec.ts
  • packages/app/src/components/prompt-input/model-picker.tsx
  • packages/opencode/test/config/e2e-smoke-tagging.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-model-reasoning-variant

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 and usage tips.

@github-actions github-actions Bot added app Application behavior and product flows ui Design system and user interface labels May 17, 2026
@Astro-Han Astro-Han added bug Something isn't working P2 Medium priority labels May 17, 2026 — with ChatGPT Codex Connector

@github-actions github-actions 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.

Suggested priority: P2 (includes user-path files (packages/app/src/components/prompt-input/model-picker.tsx)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a new E2E test for the 'thinking' model variant selection and adds data attributes to the UI components to support these tests. It also refines the model picker's behavior to prevent the main popover from closing when interacting with nested menus. A review comment suggests moving the isPickerContentTarget helper function outside the component to avoid unnecessary re-creations and improve reusability.

Comment thread packages/app/src/components/prompt-input/model-picker.tsx Outdated
@github-actions

github-actions Bot commented May 17, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 32 -> 24 (-8) 64 -> 72 (+8) 55 -> 54 (-1) 5 -> 4 (-1) 33.3 -> 16.8 (-16.5) 133.4 -> 133.4 (0) 2 -> 2 (0) 0 -> 0 (0) pass
default / long-session-input-lag 48 -> 40 (-8) 64 -> 48 (-16) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 40 -> 40 (0) 48 -> 48 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 24 -> 16 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.7 -> 16.7 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 24 (0) 24 -> 24 (0) 51 -> 50 (-1) 1 -> 0 (-1) 33.4 -> 33.3 (-0.1) 83.3 -> 83.3 (0) 2 -> 2 (0) 0 -> 0 (0) pass
default / terminal-side-panel-open 40 -> 40 (0) 40 -> 48 (+8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 16 -> 16 (0) 16 -> 16 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.7 -> 16.7 (0) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls

@github-actions github-actions Bot added the harness Model harness, prompts, tool descriptions, and session mechanics label May 17, 2026
…ing-variant

# Conflicts:
#	packages/opencode/test/config/e2e-smoke-tagging.test.ts
@Astro-Han
Astro-Han merged commit 8b6916f into dev May 17, 2026
28 checks passed
@Astro-Han
Astro-Han deleted the codex/fix-model-reasoning-variant branch May 17, 2026 16:23
Astro-Han added a commit that referenced this pull request May 17, 2026
Prepare PawWork v2026.5.18 for the stable desktop release.

- Bump the desktop package version to 2026.5.18.
- Scope the diagnostics unreadable-file retention test to POSIX permission semantics so Windows advisory does not fail on chmod behavior that Windows does not enforce the same way.

Verification:
- Focused desktop diagnostics test passed locally: 11 pass / 0 fail.
- Release typecheck passed locally for packages/desktop-electron.
- PR #706 CI passed, including ci, desktop-smoke, e2e-artifacts, CodeQL, dependency-review, label-policy, commit-lint, and title lint.

Release notes:
- Drafted against the merged range since v2026.5.17: #691, #692, #693, #694, #702, and #703.
- Cold-read review completed before merge; wording was tightened to avoid overclaiming diagnostics impact and to keep verification short.
Astro-Han added a commit that referenced this pull request May 18, 2026
Root cause / goal:
- PR #702 fixed the visible model-picker interaction bug where nested thinking options could be clicked and applied.
- The remaining gap was regression coverage for the session re-entry restore path: an existing session should restore the saved model.variant from the latest user message instead of falling back to the default reasoning effort.

Change boundary:
- Added a real Playwright re-entry regression in packages/app/e2e/models/model-picker-thinking.spec.ts.
- Reused the visible nested thinking-picker click path through a small helper.
- Marked the re-entry case @smoke and updated the smoke inventory assertion so PR smoke E2E covers it.
- Aligned opencode workflow contract tests with the action SHAs already present on latest dev after dependency bump commits.
- No product/provider/persistence/routing behavior changed.

Review follow-up:
- Addressed the missing Probe.sessionID test type field.
- Promoted the re-entry regression into PR smoke coverage.
- Verified the CodeRabbit review thread is resolved.

Verification:
- PLAYWRIGHT_PORT=3132 PLAYWRIGHT_SERVER_PORT=4132 bun run --cwd packages/app test:e2e -- e2e/models/model-picker-thinking.spec.ts --grep "session re-entry restores thinking variant" passed.
- PLAYWRIGHT_PORT=3133 PLAYWRIGHT_SERVER_PORT=4133 bun run --cwd packages/app test:e2e -- e2e/models/model-picker-thinking.spec.ts passed.
- PLAYWRIGHT_PORT=3136 PLAYWRIGHT_SERVER_PORT=4136 bun run --cwd packages/app test:e2e -- e2e/models/model-picker-thinking.spec.ts --grep @smoke passed, 2 tests.
- bun test test/github/codeql-workflow.test.ts test/github/pr-routing-triage.test.ts test/config/e2e-smoke-tagging.test.ts passed, 10 tests.
- bun turbo test:ci --filter=opencode passed, 2766 tests.
- git diff --check passed.
- PR CI passed: CodeQL, e2e-artifacts, smoke-macos-arm64, typecheck, lint, unit-app, unit-desktop, unit-opencode, unit-ui-focused, and policy checks.

Residual risk:
- Test-only change. Electron manual verification was not rerun because no visible product code changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows bug Something isn't working harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Reasoning effort cannot be changed from the session model picker

1 participant