feat: auto reasoning effort with bounded limits and per-message model attribution#4527
feat: auto reasoning effort with bounded limits and per-message model attribution#4527xGh0st0X13 wants to merge 2 commits into
Conversation
… attribution Picking a reasoning effort per prompt is manual busywork: cheap edits waste a high-effort turn and hard problems get short-changed. Adding `Auto` as an effort choice lets the existing reviewer model read the prompt and thread context and pick an effort for that turn, clamped between a user-set minimum and maximum so token spend stays predictable. When the reviewer cannot decide the provider default is used, clamped to the same bounds, and the resolved value never leaks `auto` to a provider CLI. Because model and effort can now change between turns in one thread, each assistant message records the selection that produced it and the hover meta row shows it, e.g. `GPT-5.6 Sol Auto (Low)`, so a response can be traced back to the model and effort behind it. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| ): boolean { | ||
| const descriptor = effortDescriptorFrom(descriptors); | ||
| return descriptor ? getProviderOptionCurrentValue(descriptor) === AUTO_EFFORT_VALUE : false; | ||
| } |
There was a problem hiding this comment.
Unused auto-effort helper export
Low Severity
isAutoEffortSelectedInDescriptors is newly exported but has no call sites in production or tests. Nearby code already uses getProviderOptionCurrentValue plus AUTO_EFFORT_VALUE (and isAutoEffortActive for the selections-based check), so this helper is dead surface area.
Reviewed by Cursor Bugbot for commit 3fc14be. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new auto reasoning effort feature with substantial new services, async flows, and UI. Several unresolved review comments identify high and medium severity concurrency bugs in the TurnResponder and reviewer flow that could cause incorrect model attribution or resumed turns after session stop. You can customize Macroscope's approvability policy. Learn more. |
Four correctness problems in the auto-effort path: The reviewer ran inside the reactor's single worker, so one slow review stalled every queued event for every thread for up to 45 seconds. Only the review moves off the queue; the turn resumes as its own work item so session configuration and turn sends stay serialized. `DrainableWorker` grows a `fork` that runs work off the queue while still counting toward `drain()`, keeping tests deterministic. An off-ladder reviewer answer (e.g. `turbo`) was clamped rather than rejected, and clamping reads an unknown effort as above the ceiling — so nonsense selected the maximum effort instead of the documented clamped default. Responders were looked up per thread, so a completion straggling in from a superseded turn was labelled with the newer turn's model. Turn ids do not exist when the reactor records a selection, so ingestion now binds the parked selection to the turn the provider reports starting, and reads are scoped to it. Codex's reasoning-effort reviewer described attachments in the prompt but never materialized the image files, so image-only requests were sized from filenames alone. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| return { | ||
| record: ({ threadId, responder }) => | ||
| update(threadId, (state) => ({ ...state, pending: responder })), |
There was a problem hiding this comment.
🟡 Medium Layers/TurnResponder.ts:44
record overwrites the previous pending responder instead of preserving it, so when two turns start back-to-back on the same thread, the second record call clobbers the first. Because sendTurn is forked and runs asynchronously, the worker can process the second turn-start (calling record again) before the first provider emits turn.started and calls bind. The first bind then attaches the second turn's responder to the first turn, and the second bind finds pending already cleared and is a no-op. The result is swapped or missing model attribution on both assistant messages. The pending responder needs to be tracked per-turn rather than as a single overwritable field.
Also found in 1 other location(s)
apps/server/src/orchestration/Layers/ProviderCommandReactor.ts:1138
turnResponder.recordstores only onependingresponder per thread, butsendTurnis forked immediately afterward, so the worker can process another turn start and overwrite that pending value before the first provider emitsturn.started. With two rapidly queued turns, turn A can therefore bind turn B's model/effort metadata, while turn B gets no attribution. The responder must be correlated to each send/turn rather than overwritten in a per-thread singleton before asynchronoussendTurnbegins.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/TurnResponder.ts around line 44:
`record` overwrites the previous pending responder instead of preserving it, so when two turns start back-to-back on the same thread, the second `record` call clobbers the first. Because `sendTurn` is forked and runs asynchronously, the worker can process the second turn-start (calling `record` again) before the first provider emits `turn.started` and calls `bind`. The first `bind` then attaches the second turn's responder to the first turn, and the second `bind` finds `pending` already cleared and is a no-op. The result is swapped or missing model attribution on both assistant messages. The pending responder needs to be tracked per-turn rather than as a single overwritable field.
Also found in 1 other location(s):
- apps/server/src/orchestration/Layers/ProviderCommandReactor.ts:1138 -- `turnResponder.record` stores only one `pending` responder per thread, but `sendTurn` is forked immediately afterward, so the worker can process another turn start and overwrite that pending value before the first provider emits `turn.started`. With two rapidly queued turns, turn A can therefore bind turn B's model/effort metadata, while turn B gets no attribution. The responder must be correlated to each send/turn rather than overwritten in a per-thread singleton before asynchronous `sendTurn` begins.
|
|
||
| const EMPTY_THREAD_RESPONDERS: ThreadResponders = { pending: undefined, turns: [] }; | ||
|
|
||
| const makeTurnResponder = Effect.gen(function* () { |
There was a problem hiding this comment.
🟡 Medium Layers/TurnResponder.ts:34
When ProviderCommandReactor calls record but the forked turn send fails before the provider starts a turn, the parked pending responder is never cleared. A later get call with no turnId (e.g. a straggler completion from an earlier turn) returns state.pending in preference to the last bound turn's responder, incorrectly labelling a message with a model from a turn that never ran. Consider clearing pending when the send fails, or adding an expiry so stale parked values don't outlive their turn attempt.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/TurnResponder.ts around line 34:
When `ProviderCommandReactor` calls `record` but the forked turn send fails before the provider starts a turn, the parked `pending` responder is never cleared. A later `get` call with no `turnId` (e.g. a straggler completion from an earlier turn) returns `state.pending` in preference to the last bound turn's responder, incorrectly labelling a message with a model from a turn that never ran. Consider clearing `pending` when the send fails, or adding an expiry so stale parked values don't outlive their turn attempt.
There was a problem hiding this comment.
One Effect service convention finding on the new TurnResponder service. See the inline comment.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
This new service defines a standalone TurnResponderShape interface instead of declaring the interface inline in the Context.Service tag. The convention is to inline the interface and refer to the inferred shape as TurnResponder["Service"].
Inline the members directly in the tag declaration and drop TurnResponderShape:
export class TurnResponder extends Context.Service<
TurnResponder,
{
readonly record: (input: {
readonly threadId: ThreadId;
readonly responder: OrchestrationMessageResponder;
}) => Effect.Effect<void>;
readonly bind: (input: {
readonly threadId: ThreadId;
readonly turnId: TurnId;
}) => Effect.Effect<void>;
readonly get: (input: {
readonly threadId: ThreadId;
readonly turnId?: TurnId | undefined;
}) => Effect.Effect<OrchestrationMessageResponder | undefined>;
}
>()("t3/orchestration/Services/TurnResponder") {}Then in Layers/TurnResponder.ts, replace satisfies TurnResponderShape with satisfies TurnResponder["Service"] and drop the type TurnResponderShape import.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2db9e66. Configure here.
| threadId: event.payload.threadId, | ||
| responder: { | ||
| modelSelection: sendTurnRequest.value.modelSelection ?? work.threadModelSelection, | ||
| ...(work.autoEffortModelSelection !== undefined ? { autoEffort: true } : {}), |
There was a problem hiding this comment.
Stop ignored during auto-effort review
High Severity
Forking the auto-effort reviewer off the worker allows thread.session-stop-requested to process concurrently. This means a turn awaiting review can proceed after its session has been stopped, potentially reactivating the session and sending a turn that should have been canceled.
Reviewed by Cursor Bugbot for commit 2db9e66. Configure here.
| -TURN_HISTORY_LIMIT, | ||
| ), | ||
| }, | ||
| ), |
There was a problem hiding this comment.
Pending responder overwritten across turns
Medium Severity
The record function overwrites a single pending responder slot per thread. Because turn processing is asynchronous, a newer turn can record its responder before an older turn's bind operation claims its own. This results in incorrect labeling, where an older turn's messages receive the newer turn's responder, and the newer turn may end up unlabeled.
Reviewed by Cursor Bugbot for commit 2db9e66. Configure here.


What Changed
Auto reasoning effort.
Autobecomes a choice in the composer's effort control, alongside the fixed levels. When it is on, the reviewer model already used for command approvals reads the prompt plus thread context and picks the effort for that turn, clamped between a user-set Minimum and Maximum. The control is a toggle with a submenu holding those two limits; picking a minimum above the current maximum raises the maximum to match (and vice versa) so the range cannot contradict itself. If the reviewer fails or times out, the provider's own default effort is used, clamped to the same bounds.autois never forwarded to a provider CLI.The reviewer runs on the thread's own provider at its cheapest effort, not the globally configured text-generation model, so this works on Cursor, Codex, Claude, and the rest without extra setup.
Per-message model attribution. Since model and effort can now differ between turns in one thread, each assistant message records the selection that produced it, and the existing hover meta row (copy button, timestamp) gains the provider logo, model name, and effort —
GPT-5.6 Sol Auto (Low)when the reviewer picked it, or just the effort when set manually.Where the pieces live:
packages/shared/autoEffort— effort descriptor derivation, bound clamping, and limit reconciliation, shared by web and mobile.packages/shared/messageResponder— turns a recorded selection into display strings using the live provider snapshot.apps/server—TurnResponderholds the resolved selection for the active turn;ProviderCommandReactorrecords it at turn start andProviderRuntimeIngestionattaches it when an assistant message completes.035addsresponder_jsontoprojection_thread_messages; the projection pipeline, snapshot query, and thread reducer carry it to clients.Why
Choosing an effort per prompt is manual busywork with a real cost in both directions: a trivial rename burns a high-effort turn, and a hard design question gets short-changed because the picker was left on low. Letting a cheap reviewer pass make that call per turn removes the fiddling, and the minimum/maximum bounds keep token spend predictable instead of handing the decision over unconditionally — that ceiling is the reason this is safe to leave on.
Attribution is the necessary companion. Once effort (and model) can vary per turn inside one thread, a response on its own no longer tells you what produced it, which makes both trusting a good answer and diagnosing a bad one guesswork.
Note on rollout: assistant messages written before migration
035have no recorded selection, so the hover row simply omits the label for them rather than showing a placeholder. No backfill is possible since the information was never captured.UI Changes
Two surfaces change, both in the composer/timeline:
Auto · <context>with a toggle and a submenu for Minimum/Maximum.GPT-5.6 Sol Auto (Low).I verified both live in an isolated dev environment rather than only in tests: with
Autoenabled on Cursor's GPT-5.6 Sol, a trivial prompt had the reviewer resolveLow, and the hover row renderedGPT-5.6 Sol Auto (Low)(hidden until hover, as before). Happy to attach screenshots/video on request.Checklist
On the first box, being straight with you: this is not small. It is two related features (~2.6k lines) because the attribution row is what makes variable effort legible. If you would consider it in smaller pieces, or not at all, say so and I will split it into the auto-effort change and the attribution change, or close it.
Verification
vp test runover the changed scope — 163 tests passing:packages/shared/{autoEffort,messageResponder},apps/web/src/components/chat/{TraitsPicker,MessagesTimeline,composerProviderState},apps/mobile/src/lib/providerOptions,apps/server/src/orchestration/Layers/{ProviderCommandReactor,ProviderRuntimeIngestion},apps/server/src/textGeneration/TextGeneration.vp check(format + lint) clean on all changed files;typecheckclean for contracts, shared, client-runtime, server, web, and mobile.Note
High Risk
Large cross-cutting change to turn-start orchestration and model selection; auto-effort adds per-turn LLM latency and silent fallback behavior on reviewer failure.
Overview
Adds Auto reasoning effort: composers (web, mobile) expose an Auto toggle with Minimum / Maximum bounds; shared
autoEffortlogic ranks efforts, reconciles conflicting limits, and stripsautobefore provider CLIs see it.On turn start,
ProviderCommandReactorruns a cheapselectReasoningEffortreview on the thread’s own provider (forked off the drainable worker so other events keep moving), clamps to the window, then sends the concrete effort. Reviewer failure or timeout falls back to the clamped provider default.Per-turn attribution parks the resolved
modelSelection(+autoEffort) in newTurnResponder, binds it when the provider reportsturn.started, and attaches it on assistant message completion. Persistence addsresponder_json(migration 035) through projections and the client reducer; the chat timeline shows provider icon, model, and effort (e.g. Auto (High)) in the existing hover meta row.Reviewed by Cursor Bugbot for commit 2db9e66. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add auto reasoning effort selection with per-turn bounds and per-message model attribution
autoreasoning effort mode where a lightweight LLM reviewer (selectReasoningEffort) picks a concrete effort level per turn from a configured ladder, clamped by optional floor and ceiling bounds set in the traits picker.TurnResponderservice that records which model selection and effort produced each assistant turn, binding the responder to the provider turn id before ingestion finalizes the message.responder_json) inprojection_thread_messagesvia migration 035 and threads it through projection snapshots, the thread reducer, andthread.message-sentevents.Autowhen effort was auto-selected.DrainableWorkerwith aforkmethod so auto-effort review can run off the queue without blocking event processing while still being counted toward drain.Macroscope summarized 2db9e66.