fix: emit a thinking-effort flag for grok, which silently dropped it - #5858
Merged
Conversation
This was referenced Sep 2, 2026
…ly dropped it A level saved against a grok provider — a task pin, a pipeline stage, a scheduled task, or a reviewer row — never reached the CLI, and the effort select was hidden, so nothing revealed the setting was inert. `effortLevelsForProvider` had no grok arm and returned null, which short-circuits `resolveCliEffort` and makes `buildEffortArgs` emit nothing. Grok has accepted an effort flag all along: `--reasoning-effort <EFFORT>`, aliased `--effort`, which is exactly what `buildEffortArgs` already emits for every non-codex/cursor vendor. Only the ladder was missing. The ladder is grok's own, read off the CLI rather than guessed — `grok --reasoning-effort bogus` answers `use one of: xhigh, high, medium, low`. There is no `max`/`minimal`, so a level saved against claude or codex clamps to `xhigh` the way an out-of-range level already clamps on agy. - `GROK_EFFORT_LEVELS` + `isGrokProvider` in `server/lib/providerModels.js`, mirrored in `client/src/utils/providers.js`. The predicate is defined inline rather than imported from `grok.js`, which imports providerModels and would cycle. The bare `grok` id is the HTTP API provider and is excluded — no CLI, no flag to carry a level. - `hasEffortFlag` now also recognizes `--reasoning-effort`. Grok's parser accepts a duplicate flag and takes the last one, so without this a user who baked `--reasoning-effort high` into their provider args would get a second injected `--effort` that silently overrode their pin — the opposite of that function's documented "a baked pin wins" contract. Two surfaces picked the ladder up on their own, which is the design working: `REVIEWER_EFFORT_LEVELS` derives from `effortLevelsForProvider`, so grok became an effort-selectable reviewer server-side; its hand-written client mirror in `reviewerPins.js` needed the matching entry. The Effort select and reviewer picker then render grok's tiers with no component change. Tests that had used grok as their canonical "provider with no effort control" now use kimi, which genuinely has none, so each keeps its original point. Closes #5854
atomantic
force-pushed
the
next/issue-5854
branch
from
September 2, 2026 17:15
c3c3399 to
ab35d02
Compare
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
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.
Summary
A thinking-effort level saved against a grok provider never reached the CLI — and the effort select was hidden, so nothing revealed the setting was inert. This affected task pins, pipeline stages, scheduled tasks, and reviewer rows alike.
effortLevelsForProviderhad no grok arm and returnednull, which short-circuitsresolveCliEffortsobuildEffortArgsemits nothing. Grok has accepted an effort flag all along —--reasoning-effort <EFFORT>, aliased--effort, which is exactly whatbuildEffortArgsalready emits for every non-codex/cursor vendor. Only the ladder was missing.The ladder is read off the CLI rather than guessed:
No
max/minimal, so a level saved against claude or codex clamps toxhigh, the same contract agy's narrower ladder already has.GROK_EFFORT_LEVELS+isGrokProviderinserver/lib/providerModels.js, mirrored inclient/src/utils/providers.js. The predicate is defined inline rather than imported fromgrok.js— that module imports providerModels, so importing back would cycle. The baregrokid is the HTTP API provider and is excluded: no CLI, no flag to carry a level.hasEffortFlagnow also matches--reasoning-effort. Grok's parser accepts a duplicate flag and takes the last one, so without this a user who baked--reasoning-effort highinto their provider args got a second injected--effortthat silently overrode their pin — the opposite of that function's documented "a baked pin wins" contract.Two surfaces picked it up on their own
REVIEWER_EFFORT_LEVELSis derived fromeffortLevelsForProvider("a CLI that gains or loses a tier moves both at once"), so grok became an effort-selectable reviewer server-side with no edit. Its hand-written client mirror inreviewerPins.jsneeded the matching entry. The Effort select and reviewer picker then render grok's tiers with no component change — the design working as intended.Tests that had used grok as their canonical "provider with no effort control" now use kimi, which genuinely has none, so each keeps its original point rather than being deleted.
Test plan
server/lib/providerModels.test.js— ladder for thegrok-cli/grok-tuiids, a path-configured command, andGrok.exe;nullfor thegrokAPI provider.resolveCliEffortclampsmax/ultra→xhighandminimal→low.buildEffortArgsemits['--effort', <level>], and emits nothing when--reasoning-effortis already baked in (both separated and joined forms).hasEffortFlaggets the long form in both shapes plus its dangling/valueless cases.client/src/utils/providers.test.js— the server-mirror parity cases now assert the ladder on both implementations, including the clamp.server/lib/cosValidation.test.js— grok as an effort-selectable reviewer:reviewerEffortLevels,EFFORT_SELECTABLE_REVIEWERS, drop-don't-clamp onmax, the settings-scalar fold, andreviewerEffortArgs.server/services/agentTuiSpawning.test.js— grok's TUI gets the flag (it is a root-level flag on the same binary), and an out-of-ladder level clamps.client—EffortSelectandReviewerPickeroffer groklow|medium|high|xhighand nomax.imageGen.multipart(20/20 alone) and five client page suites (190/190 alone), the class tracked in ChiefOfStaff page tests flake under full-suite load by querying an unsettled page mount #5857.Closes #5854