Skip to content

fix(web): keep Pi thinking controls visible before selection - #27397

Open
voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:votlcrash/pi-thinking-control-fallback
Open

voltcrash wants to merge 4 commits into
pingdotgg:mainfrom
voltcrash:votlcrash/pi-thinking-control-fallback

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Pi's thinking picker is blank for non-active models that advertise levels without a default. Show the descriptor name ("Thinking") until a value is resolved, then keep showing the selected level. This preserves Pi's model-specific defaults from #17835 and does not create a selection.

Fixes #27382. This qualifies for the focused obvious-bug exception: it makes an existing clickable control visible, with no change to provider capabilities, defaults, or dispatch.

Verification:

  • The focused regression failed first ("" instead of "Thinking"). All 30 trait-display, glyph, and composer-state tests pass; four cases cover unset, default, explicit, and overridden values.
  • vp run check:changed against upstream main passes formatting, lint, knip, affected typechecks, and focused tests.
  • Real Chromium client with isolated state and a deterministic Pi RPC executable: selecting non-active zai/glm-5.3-flash shows Thinking; the Low/High/Max menu has no selection or Default badge. Selecting High and Max updates the label. No paid turn was needed.

Web and desktop share the picker; mobile uses the same helper for its existing summary. Native Electron/mobile and remote-network passes were not run; connection and provider behavior are unchanged.

Before:

Blank Pi thinking control

After:

Visible Thinking control

Unselected thinking menu · Selected Max

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on xhigh. Sol verdict: approved (no findings).

Created with GPT-6.1 Sol in Codex (T3 Code).

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 30, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 0b52cd6

Macroscope's review found this PR approvable — Reasoning and thinking controls now use their descriptor label when no selected-option label is available. This is a presentation-only fallback; model selections and defaults remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@voltcrash

Copy link
Copy Markdown
Contributor Author

Note

🤖 GPT-6.1 Sol responding on behalf of voltcrash

The current lint and typecheck failures are in unchanged upstream code. I checked these against base 35c1210331a70fd3f460f9ad3425aed7d9a263cf:

  • Lint: the issue-number comment in ClaudeAdapterV2.ts:4218 violates no-low-value-comments.
  • Typecheck: unresolved fakeProvider in PullRequestService.test.ts:2207/2244, an implicit scope type at line 2208, and unresolved encodeScanCache in usageScanCache.test.ts:254. Both test files are unchanged by this PR.

The changed-file checks and focused regressions passed. This two-file PR keeps the Pi display fix separate from those base failures.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 618beab and 971aab2.

Files selected for processing: 1
  • packages/client-runtime/src/modelTraits.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The reasoning trigger now displays its descriptor label when the thinking option has no current label. Tests cover absent selections, a default selection, explicit selections, and an effort descriptor without a selection.

Changes: Pi thinking trigger

Layer / File(s) Summary
Reasoning trigger label fallback
packages/client-runtime/src/modelTraits.ts, packages/client-runtime/src/modelTraits.test.ts
buildTraitsTriggerDisplay uses the descriptor label when the thinking option has no current label. Tests check trigger labels and descriptor string values across selection states.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • pingdotgg/t3code#27382 — This change provides a visible descriptor label when the thinking option has no current label. It does not add server-side default levels or change Pi model discovery.

Suggested reviewers: maria-rcks

Merge Risk: Minimal · previous review 971aa

The change restores the visible Thinking label without altering model selections or provider behavior.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: keeping Pi thinking controls visible before a value is selected.
Description check Passed The description explains the problem, the display-only fix, scope, approval exception, verification results, test coverage, limitations, and UI evidence. It does not use the template headings exactly,…
Linked Issues check Passed Issue #27382 requires a visible Pi thinking control when a model advertises thinking levels but has no current value or default. The whole-PR diff (merge base 7195b69 to head 971aab2) changes `buildTr…
Out of Scope Changes check Passed The whole-PR diff contains two files: the one-expression fallback in modelTraits.ts and the new focused test file modelTraits.test.ts. Both match the linked issue's display defect. The fallback ap…
Full details: Description check

Explanation

The description explains the problem, the display-only fix, scope, approval exception, verification results, test coverage, limitations, and UI evidence. It does not use the template headings exactly, but it provides the required information and is mostly complete.

Full details: Linked Issues check

Explanation

Issue #27382 requires a visible Pi thinking control when a model advertises thinking levels but has no current value or default. The whole-PR diff (merge base 7195b69 to head 971aab2) changes buildTraitsTriggerDisplay in packages/client-runtime/src/modelTraits.ts. When getProviderOptionCurrentLabel returns no label and the descriptor id is thinking, the label now falls back to descriptor.label ("Thinking"). Before this change, the label was empty, so the trigger rendered blank. The fallback changes only the display label. It does not change getDescriptorStringValue, server defaults, capabilities, or dispatch. Resolved defaults and explicit values still produce their option labels. packages/client-runtime/src/modelTraits.test.ts (new file) covers the unset, default-only, explicit, and overridden cases, and asserts that the unset value stays null. The issue's server-side alternative is not required; the issue lists the web placeholder as an acceptable fix. The Chromium check described in the PR, which I did not rerun, reports the same behavior.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff contains two files: the one-expression fallback in modelTraits.ts and the new focused test file modelTraits.test.ts. Both match the linked issue's display defect. The fallback applies to any descriptor with id thinking, not only Pi. That is the same empty-trigger case the issue describes, so this widening is related and does not change provider behavior. Non-thinking descriptors keep the null fallback, and the test asserts that behavior. No server, provider-capability, default-selection, or dispatch files are changed. The PR description's mention of existing glyph and composer-state suites refers to tests that are not part of this diff, and it creates no out-of-scope change.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@voltcrash
voltcrash force-pushed the votlcrash/pi-thinking-control-fallback branch from 0b52cd6 to f72484e Compare October 30, 2026 06:29
@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Oct 30, 2026

maria-rcks commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

merged current github/main (e6e2ab05cceb) without conflicts and pushed 9a93e93dfcc8724a54f76876d03ef9175fe16c75 to the existing contributor branch without force. no source fixes were needed; the pr still changes three files, +43/-3. no actionable review findings remain.

on this head, blacksmith vp run check:changed github/main passed formatting, lint, both knip checks, seven affected typechecks, six focused tests including the composer browser case, and the compiler check. two independent read-only reviewers approved this exact head. all 29 ci contexts are successful, with no pending or failing checks; github reports clean and mergeable.

rechecked the real web client with system chrome and real pi discovery through this vm's local provider proxy: unset thinking has no selected level, high and low update the composer label, and low survives reload. no mock provider or fixture was used. no provider turn was run in this pass. native clients and remote connections were not rerun.

merged head: unset pi thinking is visible without a selected level

earlier takeover verification and before/after evidence

Verified in a real dev environment

reproduced the missing visible thinking label on github/main (7195b699). the current pr head also needed the fallback in ComposerModelEffort, which now resolves its visible label separately from the shared helper. added that small fix in 9550a8ffc189610129a9b661fe5e133dae923214 and pushed it to this pr's existing head.

ran separate installed worktrees with one background dev server at a time, isolated state, repository playwright and system google chrome. created real projects and threads through the client. no mock provider, fixtures, or seeded data.

on the final head:

  • pi's real gpt-6.1-sol shows "Thinking" when unset, with no selected radio and no invented default. selecting high and low updates the label. high survives a completed-thread reload.
  • the active pi luna model still shows its selected "Medium Default". codex still shows its own reasoning/default and service-tier controls.
  • real pi turns completed at low and high. real codex gpt-6.1-sol completed at high through the web composer and server adapter.
  • blacksmith vp run check:changed github/main passed formatting, lint, both knip passes, seven affected typechecks, the five model-trait cases, the existing composer browser test, and compiler coverage. two independent reviews found no issues at this exact head. final ci is green: 29 checks, zero pending or failing.

selected runtime transcript (full command transcript):

pi: model=gpt-6.1-sol thinkingLevel=low stopReason=stop
    FINAL_PI_LOW_VERIFIED
pi: model=gpt-6.1-sol thinkingLevel=high stopReason=stop
    FINAL_PI_HIGH_VERIFIED.
codex: model=gpt-6.1-sol effort=high task_complete
    FINAL_CODEX_HIGH_VERIFIED

matching composer crops, main then final head:

main: pi model has no visible thinking label

final head: unset pi model shows thinking

final head: thinking changes to high and low through the composer

inspected the captures and exercised uploaded video playback and seeking.

Not verified

native electron, ios/android, remote/relay/tunnel connections, other providers, and pi against an external service without this vm's real local proxy were not exercised.

the vm's automatic pi extensions initially caused empty proxy responses. the successful real pi turns used --no-extensions, configured through provider settings; the explicit t3 mcp bridge remained active. a cold pi version probe timed out and recovered after refreshing provider status. the recording includes that cached inventory state. browser/export work also stalled for more than ten minutes under memory pressure before closing the unused browser context; the final provider turns completed within seconds. no performance claim is made.

@voltcrash
voltcrash force-pushed the votlcrash/pi-thinking-control-fallback branch from c3a429b to 618beab Compare October 31, 2026 18:55
@voltcrash
voltcrash force-pushed the votlcrash/pi-thinking-control-fallback branch from 618beab to 971aab2 Compare November 1, 2026 06:34
@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Nov 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pi: thinking control missing for every model except pi's active one (no default level, empty trigger)

3 participants