fix(grok): surface per-model reasoning effort in the composer - #5403
Closed
ahmed-besic wants to merge 3 commits into
Closed
fix(grok): surface per-model reasoning effort in the composer#5403ahmed-besic wants to merge 3 commits into
ahmed-besic wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Contributor
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. |
This was referenced Aug 5, 2026
2 tasks
Grok models already advertise reasoning effort menus in ACP model _meta, but T3 always published empty capabilities and never applied a selected effort. Map each model's reasoningEfforts into option descriptors, pass _meta.reasoningEffort through session/set_model on start and send, and cover discovery/apply with focused tests.
ahmed-besic
force-pushed
the
fix/grok-reasoning-effort
branch
from
August 12, 2026 09:03
9e2fb80 to
c8d471b
Compare
Author
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7d734a9. Configure here.
Author
|
Fixed the small steering issue and added a regression test. Typecheck and focused Grok tests are passing. |
Member
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Duplicates trusted Grok reasoning controls in #6386. |
netpro2k
added a commit
to netpro2k/t3code
that referenced
this pull request
Aug 24, 2026
Intent: Keep Grok 4.6 usable from T3: plan-mode turns must stay reviewable over ACP, advertised reasoning levels and user-invocable skills must show in the composer, and the T3 runtime-mode control must actually drive Grok instead of ~/.grok/config.toml. Behavior: - Handle both x.ai/exit_plan_mode and _x.ai/exit_plan_mode, including wrapped payloads and null planContent. - Emit the existing proposed-plan card from planContent, or fall back to the last session plan.md body when Grok races the plan-file write. - Reply abandoned with a capture message so Grok unblocks without implementing in the same turn. - Detect enter_plan_mode and promote writes to ~/.grok/sessions/.../plan.md onto the same card. Ignore workspace plan.md files, dedupe identical markdown per turn, and clear fallback state on settle or a new non-steer turn. - Leave implement and request-changes to the existing proposed-plan follow-up; do not auto-approve plans even in full-access mode. - Map each Grok model's ACP _meta.reasoningEfforts into the existing Reasoning select, preserving labels and descriptions and exposing the applied effort as currentValue. Keep one default badge and do not invent a menu when metadata is absent. - Apply reasoning effort through session/set_model _meta.reasoningEffort, including effort-only changes. Do not carry an effort across model switches unless explicitly selected, skip set_model when nothing changes, and defer turn-time mutation until sendTurn validation succeeds. - Honor the selected effort in Grok-backed title, commit, PR, and branch-name generation. - Read user-invocable skills from grok inspect --json into the provider snapshot; use those names for both $ and /, skip bundled helpers, and leave both catalogs empty on inspect failure without failing provider discovery. - Supervised spawns `--permission-mode default` so the thread asks even if the Grok CLI config is always-approve or auto. - Auto-accept edits spawns `--permission-mode acceptEdits`. - Auto spawns `--permission-mode auto` and sends session/new (and session/load) `_meta.autoMode: true`. - Full access spawns `grok agent --always-approve stdio` and sends `_meta.yoloMode: true`. - Probe and text-generation ACP processes omit a T3 mode and keep `grok agent stdio`. - Always allow this session falls back to allow_once when Grok omits allow_always, then auto-approves later prompts in that session. - Auto still escalates risky calls to T3; it is not always-approve. Design constraints: - Keep Grok-specific ACP, inspect, and permission dialects at the adapter/provider boundary. Do not change contracts or other providers. - Reuse turn.proposed.completed, the existing plan card, and generic composer optionDescriptors / skills / slashCommands. Add no Grok-only approval prompt or new composer UI. - Do not spawn-bind --reasoning-effort or require a new thread to change effort; Grok applies _meta in place. - Do not map T3 Auto to yolo / bypassPermissions. That is Full access. - Do not restart mid-thread on a runtime-mode change (separate hang: upstream pingdotgg#6517). Integration: - Grok adapter, xAI ACP extension helpers, ACP mock, ACP session runtime (set_model and session/new|/load _meta), Grok spawn args, Grok model/provider snapshot, inspect catalog parser, text generation, focused tests, internals provider docs, install.md, and permission-modes.md. - Web, desktop, and mobile consume existing proposed-plan events, generic snapshot fields, and the existing runtime-mode picker with no client changes. Verification: - vp test run apps/server/src/provider/acp/GrokAcpSupport.test.ts apps/server/src/provider/Layers/GrokProvider.test.ts apps/server/src/provider/Drivers/GrokSkills.test.ts apps/server/src/provider/Layers/GrokAdapter.test.ts apps/server/src/provider/acp/XAiAcpExtension.test.ts - Runtime-mode slice: 57 passed (GrokAcpSupport, GrokAdapter, GrokProvider). - Earlier combined Grok slice: 64 passed including skills and xAI extension. Rebase notes: - Conflict hotspots are GrokAdapter plan handlers, sendTurn settlement, handleRequestPermission, buildGrokAcpSpawnInput, XAiAcpExtension.ts, GrokProvider discovered-model capabilities, GrokDriver inspect cwd, and AcpSessionRuntime setSessionModel plus session/new _meta. - Upstream pingdotgg#4514 (plan), pingdotgg#5403/pingdotgg#6386/pingdotgg#6887 (reasoning), pingdotgg#4109 (skills), pingdotgg#6502/pingdotgg#6626 (Always allow and spawn permission-mode). Drop this patch only when main handles both plan-exit spellings with live/fallback plan capture, maps ACP reasoningEfforts and effort-only set_model, publishes grok inspect skills to $ and /, forwards all four T3 modes onto Grok argv with autoMode/yoloMode on session setup, Supervised overrides config.toml, and Always allow does not cancel when allow_always is missing.
netpro2k
added a commit
to netpro2k/t3code
that referenced
this pull request
Aug 25, 2026
Intent: Keep Grok 4.6 usable from T3: plan-mode turns must stay reviewable over ACP, advertised reasoning levels and user-invocable skills must show in the composer, and the T3 runtime-mode control must actually drive Grok instead of ~/.grok/config.toml. Behavior: - Handle both x.ai/exit_plan_mode and _x.ai/exit_plan_mode, including wrapped payloads and null planContent. - Emit the existing proposed-plan card from planContent, or fall back to the last session plan.md body when Grok races the plan-file write. - Reply abandoned with a capture message so Grok unblocks without implementing in the same turn. - Detect enter_plan_mode and promote writes to ~/.grok/sessions/.../plan.md onto the same card. Ignore workspace plan.md files, dedupe identical markdown per turn, and clear fallback state on settle or a new non-steer turn. - Leave implement and request-changes to the existing proposed-plan follow-up; do not auto-approve plans even in full-access mode. - Map each Grok model's ACP _meta.reasoningEfforts into the existing Reasoning select, preserving labels and descriptions and exposing the applied effort as currentValue. Keep one default badge and do not invent a menu when metadata is absent. - Apply reasoning effort through session/set_model _meta.reasoningEffort, including effort-only changes. Do not carry an effort across model switches unless explicitly selected, skip set_model when nothing changes, and defer turn-time mutation until sendTurn validation succeeds. - Honor the selected effort in Grok-backed title, commit, PR, and branch-name generation. - Read user-invocable skills from grok inspect --json into the provider snapshot; use those names for both $ and /, skip bundled helpers, and leave both catalogs empty on inspect failure without failing provider discovery. - Supervised spawns `--permission-mode default` so the thread asks even if the Grok CLI config is always-approve or auto. - Auto-accept edits spawns `--permission-mode acceptEdits`. - Auto spawns `--permission-mode auto` and sends session/new (and session/load) `_meta.autoMode: true`. - Full access spawns `grok agent --always-approve stdio` and sends `_meta.yoloMode: true`. - Probe and text-generation ACP processes omit a T3 mode and keep `grok agent stdio`. - Always allow this session falls back to allow_once when Grok omits allow_always, then auto-approves later prompts in that session. - Auto still escalates risky calls to T3; it is not always-approve. Design constraints: - Keep Grok-specific ACP, inspect, and permission dialects at the adapter/provider boundary. Do not change contracts or other providers. - Reuse turn.proposed.completed, the existing plan card, and generic composer optionDescriptors / skills / slashCommands. Add no Grok-only approval prompt or new composer UI. - Do not spawn-bind --reasoning-effort or require a new thread to change effort; Grok applies _meta in place. - Do not map T3 Auto to yolo / bypassPermissions. That is Full access. - Do not restart mid-thread on a runtime-mode change (separate hang: upstream pingdotgg#6517). Integration: - Grok adapter, xAI ACP extension helpers, ACP mock, ACP session runtime (set_model and session/new|/load _meta), Grok spawn args, Grok model/provider snapshot, inspect catalog parser, text generation, focused tests, internals provider docs, install.md, and permission-modes.md. - Web, desktop, and mobile consume existing proposed-plan events, generic snapshot fields, and the existing runtime-mode picker with no client changes. Verification: - vp test run apps/server/src/provider/acp/GrokAcpSupport.test.ts apps/server/src/provider/Layers/GrokProvider.test.ts apps/server/src/provider/Drivers/GrokSkills.test.ts apps/server/src/provider/Layers/GrokAdapter.test.ts apps/server/src/provider/acp/XAiAcpExtension.test.ts - Runtime-mode slice: 57 passed (GrokAcpSupport, GrokAdapter, GrokProvider). - Earlier combined Grok slice: 64 passed including skills and xAI extension. Rebase notes: - Conflict hotspots are GrokAdapter plan handlers, sendTurn settlement, handleRequestPermission, buildGrokAcpSpawnInput, XAiAcpExtension.ts, GrokProvider discovered-model capabilities, GrokDriver inspect cwd, and AcpSessionRuntime setSessionModel plus session/new _meta. - Upstream pingdotgg#4514 (plan), pingdotgg#5403/pingdotgg#6386/pingdotgg#6887 (reasoning), pingdotgg#4109 (skills), pingdotgg#6502/pingdotgg#6626 (Always allow and spawn permission-mode). Drop this patch only when main handles both plan-exit spellings with live/fallback plan capture, maps ACP reasoningEfforts and effort-only set_model, publishes grok inspect skills to $ and /, forwards all four T3 modes onto Grok argv with autoMode/yoloMode on session setup, Supervised overrides config.toml, and Always allow does not cancel when allow_always is missing.
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
3 participants
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.
What Changed
Grok already advertises per-model reasoning effort menus over ACP (
models.availableModels[]._meta.reasoningEfforts), but T3 published every Grok model with empty capabilities and never applied a selected effort. The composer therefore had no Reasoning control for Grok, unlike Claude/Codex.This maps each model's advertised menu into
optionDescriptors(reasoningEffort), passes_meta.reasoningEffortthroughsession/set_modelon session start and each turn (including mid-thread effort-only changes), and wires the same selection into Grok text generation.Why
Grok Build supports Low/Medium/High (and other per-model menus for custom models). Without reading
_metaand applying effort onsession/set_model, T3 always ran at the CLI default and users could not change it from the UI.Related open work: #5160 takes a spawn-flag approach and blocks mid-thread changes. This PR is narrower: discovery + apply via the existing ACP
session/set_model_metapath, which Grok applies in place without restarting the process.UI Changes
Uses the existing Traits picker — no new UI components. Once Grok ACP discovery succeeds, models with
reasoningEffortsshow a Reasoning select; models with different menus keep their own options.Live-verified against
grok agent stdioand the T3 web UI over Tailscale.Validation
vp test run src/provider/acp/GrokAcpSupport.test.ts src/provider/Layers/GrokProvider.test.ts— 17 passedsession/set_modelwith_meta.reasoningEffortapplies the overrideChecklist
Made by Grok Build via the Grok Build harness while working on T3 Code.
Note
Medium Risk
Touches Grok ACP model selection and session/set_model payloads, so a bad
_metaor skip-on-steer bug could send the wrong effort or extra set_model calls. Scope is Grok-only and covered by unit/adapter tests.Overview
Grok models that advertise
reasoningEffortsin ACP_metanow get a Reasoning select in the composer, and the chosen value is applied in-session instead of always using the CLI default.Discovery maps each model's
_meta.reasoningEffortsintooptionDescriptors.applyGrokAcpModelSelectionnow sends_meta.reasoningEffortonsession/set_modelwhen effort is selected (including effort-only changes), and ACPsetSessionModelforwards that_meta. Session start, turns, and text generation all passmodelSelection.options. Steering an in-flight prompt skips effort updates because Grok cannot apply them while a prompt is active.Reviewed by Cursor Bugbot for commit 1754184. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add per-model reasoning effort support to Grok adapter and ACP session model
reasoningEffortas a per-model Reasoning select descriptor built from session model state_meta.reasoningEfforts, replacing the previousEMPTY_CAPABILITIESfor discovered Grok modelsapplyGrokAcpModelSelectionandAcpSessionRuntime.setSessionModelto forwardoptionswith_meta.reasoningEffortinsession/set_modelrequests, issuing the request even when only the effort changes (model id unchanged)steeringTurnIdis set) to avoid redundantsession/set_modelcalls while a prompt is activemodelSelection.optionsthrough toapplyGrokAcpModelSelectionso reasoning effort is applied before promptingsession/set_modelis now invoked when areasoningEffortselection is present even if the model id is unchanged; existing callers withoutoptionsare unaffectedMacroscope summarized 1754184.