Persist provider-aware model selections#1371
Open
juliusmarminge wants to merge 8 commits intomainfrom
Open
Conversation
- Replace model-only fields with provider/model selections across orchestration - Add projection schema and migration updates for provider-backed snapshots - Update server and web tests to use the new selection shape
|
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 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)
Comment |
github-actions
bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Model picker clears reasoning effort and fast mode
- Preserved existing composerDraft.modelSelection.options in onProviderModelSelect when the provider remains the same, preventing reasoning effort and fast mode settings from being silently cleared on model switch.
Or push these changes by commenting:
@cursor push 569dd7bcab
Preview (569dd7bcab)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -3135,10 +3135,14 @@
return;
}
const resolvedModel = resolveAppModelSelection(provider, customModelsByProvider, model);
+ const existingOptions = composerDraft.modelSelection?.options;
+ const preserveOptions =
+ existingOptions !== undefined && composerDraft.modelSelection?.provider === provider;
const nextModelSelection: ModelSelection = {
provider,
model: resolvedModel,
- };
+ ...(preserveOptions ? { options: existingOptions } : {}),
+ } as ModelSelection;
setComposerDraftModelSelection(activeThread.id, nextModelSelection);
setStickyComposerModelSelection(nextModelSelection);
scheduleComposerFocus();
@@ -3150,6 +3154,7 @@
setComposerDraftModelSelection,
setStickyComposerModelSelection,
customModelsByProvider,
+ composerDraft.modelSelection,
],
);
const setPromptFromTraits = useCallback(![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- Keep draft model options when switching models within the same provider - Decode SQL errors correctly in projection snapshot model selection - Default missing sticky provider to codex during draft migration
- Preserve default provider model selections in the draft state - Update composer draft store expectations for sticky/provider-specific models
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Runtime-mode-set handler may trigger spurious session restarts
- Removed the fallback
?? toProviderModelOptions(thread.modelSelection)so that when no cached model options exist,cachedModelOptionsremainsundefinedand themodelOptionskey is omitted from the options passed toensureSessionForThread, preventing the spurious restart.
- Removed the fallback
- ✅ Fixed: Unreachable fallback chain in plan implementation model selection
- Simplified
nextThreadModelSelectionto assignselectedModelSelectiondirectly, removing the unreachable??fallback chain sinceselectedModelSelectionis always a non-nullModelSelectionfromuseMemo.
- Simplified
Or push these changes by commenting:
@cursor push 14070d6c45
Preview (14070d6c45)
diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -705,9 +705,7 @@
return;
}
const cachedProviderOptions = threadProviderOptions.get(event.payload.threadId);
- const cachedModelOptions =
- threadModelOptions.get(event.payload.threadId) ??
- toProviderModelOptions(thread.modelSelection);
+ const cachedModelOptions = threadModelOptions.get(event.payload.threadId);
yield* ensureSessionForThread(event.payload.threadId, event.occurredAt, {
...(cachedProviderOptions !== undefined
? { providerOptions: cachedProviderOptions }
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -3035,12 +3035,7 @@
text: implementationPrompt,
});
const nextThreadTitle = truncateTitle(buildPlanImplementationThreadTitle(planMarkdown));
- const nextThreadModelSelection: ModelSelection = selectedModelSelection ??
- activeThread.modelSelection ??
- activeProject.defaultModelSelection ?? {
- provider: "codex",
- model: DEFAULT_MODEL_BY_PROVIDER.codex,
- };
+ const nextThreadModelSelection: ModelSelection = selectedModelSelection;
sendInFlightRef.current = true;
beginSendPhase("sending-turn");
Contributor
|
Bugbot Autofix prepared fixes for both issues found in the latest run.
Or push these changes by commenting: Preview (e0f5c02e35)diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -16,6 +16,7 @@
} from "@t3tools/contracts";
import { Cache, Cause, Duration, Effect, Layer, Option, Schema, Stream } from "effect";
import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker";
+import { toProviderModelOptions } from "@t3tools/shared/model";
import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts";
import { GitCore } from "../../git/Services/GitCore.ts";
@@ -81,17 +82,6 @@
right: ProviderModelOptions | undefined,
): boolean => JSON.stringify(left ?? null) === JSON.stringify(right ?? null);
-function toProviderModelOptions(
- modelSelection: ModelSelection | undefined,
-): ProviderModelOptions | undefined {
- if (!modelSelection?.options) {
- return undefined;
- }
- return modelSelection.provider === "codex"
- ? { codex: modelSelection.options }
- : { claudeAgent: modelSelection.options };
-}
-
function isUnknownPendingApprovalRequestError(cause: Cause.Cause<ProviderServiceError>): boolean {
const error = Cause.squash(cause);
if (Schema.is(ProviderAdapterRequestError)(error)) {
diff --git a/apps/server/src/persistence/Layers/ProjectionProjects.ts b/apps/server/src/persistence/Layers/ProjectionProjects.ts
--- a/apps/server/src/persistence/Layers/ProjectionProjects.ts
+++ b/apps/server/src/persistence/Layers/ProjectionProjects.ts
@@ -77,7 +77,7 @@
${row.workspaceRoot},
${row.defaultModelSelection?.provider ?? null},
${row.defaultModelSelection?.model ?? null},
- ${JSON.stringify(row.defaultModelSelection?.options ?? null)},
+ ${row.defaultModelSelection?.options != null ? JSON.stringify(row.defaultModelSelection.options) : null},
${JSON.stringify(row.scripts)},
${row.createdAt},
${row.updatedAt},
diff --git a/apps/server/src/persistence/Layers/ProjectionThreads.ts b/apps/server/src/persistence/Layers/ProjectionThreads.ts
--- a/apps/server/src/persistence/Layers/ProjectionThreads.ts
+++ b/apps/server/src/persistence/Layers/ProjectionThreads.ts
@@ -89,7 +89,7 @@
${row.title},
${row.modelSelection.provider},
${row.modelSelection.model},
- ${JSON.stringify(row.modelSelection.options ?? null)},
+ ${row.modelSelection.options != null ? JSON.stringify(row.modelSelection.options) : null},
${row.runtimeMode},
${row.interactionMode},
${row.branch},
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -28,6 +28,7 @@
getDefaultModel,
normalizeModelSlug,
resolveModelSlugForProvider,
+ toProviderModelOptions,
} from "@t3tools/shared/model";
import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
@@ -211,16 +212,6 @@
return provider === "codex" ? modelOptions?.codex : modelOptions?.claudeAgent;
}
-function toProviderModelOptions(
- modelSelection: ModelSelection | null | undefined,
-): ProviderModelOptions | undefined {
- if (!modelSelection?.options) {
- return undefined;
- }
- return modelSelection.provider === "codex"
- ? { codex: modelSelection.options }
- : { claudeAgent: modelSelection.options };
-}
const COMPOSER_PATH_QUERY_DEBOUNCE_MS = 120;
const SCRIPT_TERMINAL_COLS = 120;
const SCRIPT_TERMINAL_ROWS = 30;
diff --git a/packages/shared/src/model.ts b/packages/shared/src/model.ts
--- a/packages/shared/src/model.ts
+++ b/packages/shared/src/model.ts
@@ -10,7 +10,9 @@
type ClaudeCodeEffort,
type CodexModelOptions,
type CodexReasoningEffort,
+ type ModelSelection,
type ModelSlug,
+ type ProviderModelOptions,
type ProviderReasoningEffort,
type ProviderKind,
} from "@t3tools/contracts";
@@ -266,4 +268,15 @@
return `Ultrathink:\n${trimmed}`;
}
+export function toProviderModelOptions(
+ modelSelection: ModelSelection | null | undefined,
+): ProviderModelOptions | undefined {
+ if (!modelSelection?.options) {
+ return undefined;
+ }
+ return modelSelection.provider === "codex"
+ ? { codex: modelSelection.options }
+ : { claudeAgent: modelSelection.options };
+}
+
export { CLAUDE_CODE_EFFORT_OPTIONS, CODEX_REASONING_EFFORT_OPTIONS }; |
- Keep sticky selection in sync when provider options change - Add regression test for creating the initial sticky snapshot
- Store missing model options as SQL NULL in project and thread projections - Stop rehydrating derived model options when restarting provider sessions - Share model option conversion logic in `packages/shared`
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Redundant double call to
extractModelSelectionOptions- Stored the result of extractModelSelectionOptions in a local variable and reused it for both the truthiness check and the value assignment.
- ✅ Fixed: Model options always injected on session restart fallback
- Changed the fallback to only derive model options from requestedModelSelection (explicit user request) rather than from the thread's persisted modelSelection, preventing silent injection during runtime-mode-triggered restarts.
Or push these changes by commenting:
@cursor push 73c8d110db
Preview (73c8d110db)
diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -249,7 +249,10 @@
const desiredModelSelection = requestedModelSelection ?? thread.modelSelection;
const desiredModel = desiredModelSelection.model;
const desiredModelOptions =
- options?.modelOptions ?? toProviderModelOptions(desiredModelSelection);
+ options?.modelOptions ??
+ (requestedModelSelection !== undefined
+ ? toProviderModelOptions(requestedModelSelection)
+ : undefined);
const effectiveCwd = resolveThreadWorkspaceCwd({
thread,
projects: readModel.projects,
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -643,21 +643,14 @@
);
const selectedPromptEffort = composerProviderState.promptEffort;
const selectedModelOptionsForDispatch = composerProviderState.modelOptionsForDispatch;
- const selectedModelSelection = useMemo<ModelSelection>(
- () => ({
+ const selectedModelSelection = useMemo<ModelSelection>(() => {
+ const options = extractModelSelectionOptions(selectedProvider, selectedModelOptionsForDispatch);
+ return {
provider: selectedProvider,
model: selectedModel,
- ...(extractModelSelectionOptions(selectedProvider, selectedModelOptionsForDispatch)
- ? {
- options: extractModelSelectionOptions(
- selectedProvider,
- selectedModelOptionsForDispatch,
- ),
- }
- : {}),
- }),
- [selectedModel, selectedModelOptionsForDispatch, selectedProvider],
- );
+ ...(options ? { options } : {}),
+ };
+ }, [selectedModel, selectedModelOptionsForDispatch, selectedProvider]);
const providerOptionsForDispatch = useMemo(() => getProviderStartOptions(settings), [settings]);
const selectedModelForPicker = selectedModel;
const modelOptionsByProvider = useMemo(
Contributor
There was a problem hiding this comment.
🟢 Low
The const runtime declared inside createHarness shadows the outer let runtime (line 70), so afterEach reads the uninitialized outer variable and never calls runtime.dispose(). The ManagedRuntime and its scope/workers leak after every test.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts around line 234:
The `const runtime` declared inside `createHarness` shadows the outer `let runtime` (line 70), so `afterEach` reads the uninitialized outer variable and never calls `runtime.dispose()`. The `ManagedRuntime` and its scope/workers leak after every test.
Evidence trail:
apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts lines 70-72 (outer `let runtime`), line 234 (`const runtime` inside createHarness), lines 275-287 (return object without runtime), lines 82-84 (afterEach checking outer runtime)
- Add migration for legacy provider/model payloads - Cover project, thread, and orchestration event records
Comment on lines
+32
to
+50
| yield* sql` | ||
| UPDATE projection_threads | ||
| SET | ||
| provider = CASE | ||
| WHEN lower(model) LIKE '%claude%' THEN 'claudeAgent' | ||
| ELSE 'codex' | ||
| END, | ||
| model_options_json = CASE | ||
| WHEN model_options_json IS NULL THEN NULL | ||
| WHEN json_valid(model_options_json) = 0 THEN model_options_json | ||
| WHEN json_type(model_options_json, '$.codex') IS NOT NULL | ||
| OR json_type(model_options_json, '$.claudeAgent') IS NOT NULL | ||
| THEN CASE | ||
| WHEN lower(model) LIKE '%claude%' THEN json_extract(model_options_json, '$.claudeAgent') | ||
| ELSE json_extract(model_options_json, '$.codex') | ||
| END | ||
| ELSE model_options_json | ||
| END | ||
| `; |
Contributor
There was a problem hiding this comment.
🟡 Medium Migrations/018_CanonicalizeLegacyModelSelections.ts:32
The UPDATE projection_threads statement at line 32 has no WHERE clause, so it unconditionally overwrites the provider column for ALL rows. Migration 016 may have already set provider based on actual session data (provider_name from projection_thread_sessions), and this UPDATE would overwrite those correct values with heuristic-based values derived solely from the model name. Consider adding a WHERE clause such as WHERE provider IS NULL to preserve already-correct data.
Suggested change
| yield* sql` | |
| UPDATE projection_threads | |
| SET | |
| provider = CASE | |
| WHEN lower(model) LIKE '%claude%' THEN 'claudeAgent' | |
| ELSE 'codex' | |
| END, | |
| model_options_json = CASE | |
| WHEN model_options_json IS NULL THEN NULL | |
| WHEN json_valid(model_options_json) = 0 THEN model_options_json | |
| WHEN json_type(model_options_json, '$.codex') IS NOT NULL | |
| OR json_type(model_options_json, '$.claudeAgent') IS NOT NULL | |
| THEN CASE | |
| WHEN lower(model) LIKE '%claude%' THEN json_extract(model_options_json, '$.claudeAgent') | |
| ELSE json_extract(model_options_json, '$.codex') | |
| END | |
| ELSE model_options_json | |
| END | |
| `; | |
| yield* sql` | |
| UPDATE projection_threads | |
| SET | |
| provider = CASE | |
| WHEN lower(model) LIKE '%claude%' THEN 'claudeAgent' | |
| ELSE 'codex' | |
| END, | |
| model_options_json = CASE | |
| WHEN model_options_json IS NULL THEN NULL | |
| WHEN json_valid(model_options_json) = 0 THEN model_options_json | |
| WHEN json_type(model_options_json, '$.codex') IS NOT NULL | |
| OR json_type(model_options_json, '$.claudeAgent') IS NOT NULL | |
| THEN CASE | |
| WHEN lower(model) LIKE '%claude%' THEN json_extract(model_options_json, '$.claudeAgent') | |
| ELSE json_extract(model_options_json, '$.codex') | |
| END | |
| ELSE model_options_json | |
| END | |
| WHERE provider IS NULL | |
| `; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/persistence/Migrations/018_CanonicalizeLegacyModelSelections.ts around lines 32-50:
The `UPDATE projection_threads` statement at line 32 has no `WHERE` clause, so it unconditionally overwrites the `provider` column for ALL rows. Migration 016 may have already set `provider` based on actual session data (`provider_name` from `projection_thread_sessions`), and this UPDATE would overwrite those correct values with heuristic-based values derived solely from the model name. Consider adding a `WHERE` clause such as `WHERE provider IS NULL` to preserve already-correct data.
Evidence trail:
apps/server/src/persistence/Migrations/018_CanonicalizeLegacyModelSelections.ts lines 31-47: UPDATE projection_threads with no WHERE clause, sets provider based on model name heuristic.
apps/server/src/persistence/Migrations/016_ProjectionProviders.ts lines 22-31: UPDATE projection_threads SET provider = COALESCE((SELECT provider_name FROM projection_thread_sessions...), 'codex') WHERE provider IS NULL - correctly sets provider from actual session data.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dead
providerparameter instartProviderSessionclosure- Removed the unused
readonly provider?: ProviderKindfield from the startProviderSession closure's input type, since the body only uses the outerpreferredProvidervariable.
- Removed the unused
Or push these changes by commenting:
@cursor push e7ae3f40f4
Preview (e7ae3f40f4)
diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -260,10 +260,7 @@
.listSessions()
.pipe(Effect.map((sessions) => sessions.find((session) => session.threadId === threadId)));
- const startProviderSession = (input?: {
- readonly resumeCursor?: unknown;
- readonly provider?: ProviderKind;
- }) =>
+ const startProviderSession = (input?: { readonly resumeCursor?: unknown }) =>
providerService.startSession(threadId, {
threadId,
...(preferredProvider ? { provider: preferredProvider } : {}),| ...(preferredProvider ? { provider: preferredProvider } : {}), | ||
| ...(effectiveCwd ? { cwd: effectiveCwd } : {}), | ||
| ...(desiredModel ? { model: desiredModel } : {}), | ||
| ...(options?.modelOptions !== undefined ? { modelOptions: options.modelOptions } : {}), |
Contributor
There was a problem hiding this comment.
Dead provider parameter in startProviderSession closure
Low Severity
The startProviderSession closure declares a provider?: ProviderKind parameter in its input type but the function body never reads input?.provider — it unconditionally uses the outer preferredProvider instead. All callers were updated to stop passing provider, but the type signature still advertises the option. A future developer could reasonably pass provider expecting it to influence session creation, only for it to be silently ignored.
- Replace provider-specific model options with typed modelSelection - Persist and replay selected provider, model, and options end to end - Update adapter, service, and chat composer tests for the new shape
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Broader comparison causes unnecessary claudeAgent session restarts
- Changed sameModelSelectionOptions to compare only left?.options and right?.options instead of the full ModelSelection, so model-name-only changes no longer trigger unnecessary session restarts for claudeAgent.
Or push these changes by commenting:
@cursor push de46c2f9bd
Preview (de46c2f9bd)
diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -78,7 +78,7 @@
const sameModelSelectionOptions = (
left: ModelSelection | undefined,
right: ModelSelection | undefined,
-): boolean => JSON.stringify(left ?? null) === JSON.stringify(right ?? null);
+): boolean => JSON.stringify(left?.options ?? null) === JSON.stringify(right?.options ?? null);
function isUnknownPendingApprovalRequestError(cause: Cause.Cause<ProviderServiceError>): boolean {
const error = Cause.squash(cause);|
|
||
| const sameModelOptions = ( | ||
| left: ProviderModelOptions | undefined, | ||
| right: ProviderModelOptions | undefined, |
Contributor
There was a problem hiding this comment.
Broader comparison causes unnecessary claudeAgent session restarts
Medium Severity
sameModelSelectionOptions compares the entire ModelSelection (provider, model, and options) via JSON.stringify, but shouldRestartForModelSelectionChange is meant to restart claudeAgent sessions only when options change. Since both adapters report sessionModelSwitch: "in-session", shouldRestartForModelChange is always false, making shouldRestartForModelSelectionChange the only path that could trigger a restart for model-name-only changes on claudeAgent — causing unnecessary session restarts instead of using the cheaper in-session setModel call.
Additional Locations (1)
Contributor
There was a problem hiding this comment.
🟡 Medium
asClaudeModelOptions returns undefined when modelOptions only contains fastMode (no effort or thinking), causing fast mode settings to be silently dropped. Similarly, asCodexModelOptions misses fastMode when only reasoningEffort is checked. Consider adding fastMode to the property checks in both helpers.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/chat/composerProviderRegistry.tsx around line 52:
`asClaudeModelOptions` returns `undefined` when `modelOptions` only contains `fastMode` (no `effort` or `thinking`), causing fast mode settings to be silently dropped. Similarly, `asCodexModelOptions` misses `fastMode` when only `reasoningEffort` is checked. Consider adding `fastMode` to the property checks in both helpers.
Evidence trail:
- `apps/web/src/components/chat/composerProviderRegistry.tsx` lines 52-62: `asCodexModelOptions` checks only `"reasoningEffort"`, `asClaudeModelOptions` checks only `"effort" || "thinking"`
- `packages/contracts/src/model.ts` lines 10-21: Both `CodexModelOptions` and `ClaudeModelOptions` include optional `fastMode` field
- `packages/shared/src/model.ts` lines 211-224: `normalizeCodexModelOptions` can produce objects with only `fastMode` when `reasoningEffort` equals default
- `packages/shared/src/model.ts` lines 226-249: `normalizeClaudeModelOptions` similarly can produce objects with only `fastMode`
Comment on lines
+391
to
+399
| const modelForTurn = | ||
| sessionModelSwitch === "unsupported" | ||
| ? input.modelSelection | ||
| ? { | ||
| ...input.modelSelection, | ||
| model: activeSession?.model ?? input.modelSelection.model, | ||
| } | ||
| : undefined | ||
| : input.modelSelection; |
Contributor
There was a problem hiding this comment.
🟡 Medium Layers/ProviderCommandReactor.ts:391
When sessionModelSwitch === "unsupported" and input.modelSelection is undefined, modelForTurn becomes undefined instead of falling back to activeSession?.model. This causes the provider's locked model to be dropped when starting a turn without an explicit model selection, potentially sending an empty model selection to the provider.
- const modelForTurn =
- sessionModelSwitch === "unsupported"
- ? input.modelSelection
- ? {
- ...input.modelSelection,
- model: activeSession?.model ?? input.modelSelection.model,
- }
- : undefined
- : input.modelSelection;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around lines 391-399:
When `sessionModelSwitch === "unsupported"` and `input.modelSelection` is undefined, `modelForTurn` becomes `undefined` instead of falling back to `activeSession?.model`. This causes the provider's locked model to be dropped when starting a turn without an explicit model selection, potentially sending an empty model selection to the provider.
Evidence trail:
apps/server/src/orchestration/Layers/ProviderCommandReactor.ts lines 390-398 at REVIEWED_COMMIT - shows the conditional logic where `modelForTurn` becomes `undefined` when `sessionModelSwitch === "unsupported"` and `input.modelSelection` is falsy, rather than falling back to `activeSession?.model`.
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
1 participant
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
modelfields with provider-awaremodelSelectionrecords across projects, threads, and orchestration flows.Testing
bun fmtbun lintbun typecheckbun run testNote
High Risk
High risk because it changes the persisted schema and core orchestration/provider request flow, including migrations that rewrite existing projection rows and stored event payloads; any mismatch could break session recovery or snapshot decoding.
Overview
Switches orchestration from scalar model strings to provider-aware
ModelSelectionobjects. Commands/events now carrydefaultModelSelection(projects) andmodelSelection(threads/turn starts), including optional per-provideroptions, and tests are updated accordingly.Updates persistence + snapshot/projection plumbing to store provider/model/options explicitly. Projection tables gain new provider/options columns, repositories and
ProjectionSnapshotQuerydecode these intoModelSelection, and new migrations backfill provider and canonicalize legacy rows/event payloads.Adjusts provider session lifecycle to be
modelSelection-driven.ProviderCommandReactorand adapters (CodexAdapter,ClaudeAdapter,ProviderService) now start/restart sessions and send turns usingmodelSelection, persist it in runtime bindings for recovery, and relax model-string validation to allow custom models as long as the provider is explicit.Written by Cursor Bugbot for commit 96d6160. This will update automatically on new commits. Configure here.
Note
Persist provider-aware model selections across threads, projects, and composer drafts
provider,model, andmodelOptionsfields throughout the stack with a single discriminatedModelSelectionobject ({ provider, model, options }) on threads, projects, commands, events, and composer drafts.providercolumns and backfills from session history, 017 adds JSON options columns, and 018 canonicalizes legacy provider/model/options rows and event payloads.provider/model/modelOptionsfields and normalize them to a canonicalmodelSelection, defaulting the provider tocodexwhen only a model string is present.inferProviderForModelfrom shared/src/model.ts; provider is now always explicit in theModelSelectionobject.provider/model/modelOptionsrely on the legacy normalization path; missing or invalid model selections decode tonull/undefinedrather than a best-effort fallback.Macroscope summarized 96d6160.