Skip to content

feat: Allow scheduling model switches during execution #8335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

feat: Allow scheduling model switches during execution #8335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .review/pr-8274
Open in desktop
Submodule pr-8274 added at e46929
1 change: 1 addition & 0 deletions tmp/pr-8287-Roo-Code
Open in desktop
Submodule pr-8287-Roo-Code added at 88a473
66 changes: 55 additions & 11 deletions webview-ui/src/components/chat/ApiConfigSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ interface ApiConfigSelectorProps {
listApiConfigMeta: Array<{ id: string; name: string; modelId?: string }>
pinnedApiConfigs?: Record<string, boolean>
togglePinnedApiConfig: (id: string) => void
scheduledConfigId?: string
onScheduleChange?: (configId: string | undefined) => void
}

export const ApiConfigSelector = ({
Expand All @@ -32,6 +34,8 @@ export const ApiConfigSelector = ({
listApiConfigMeta,
pinnedApiConfigs,
togglePinnedApiConfig,
scheduledConfigId,
onScheduleChange,
}: ApiConfigSelectorProps) => {
const { t } = useAppTranslation()
const [open, setOpen] = useState(false)
Expand Down Expand Up @@ -73,11 +77,23 @@ export const ApiConfigSelector = ({

const handleSelect = useCallback(
(configId: string) => {
onChange(configId)
if (disabled && onScheduleChange) {
Copy link
Contributor Author

@roomote roomote bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: UX edge case. If a user selects the current model while disabled (no schedule yet), we end up “scheduling” the same model. Prefer treating this as a no-op (or cancel any existing schedule). This improves clarity and avoids a clock icon that implies a change that won’t happen.

Suggested change
if (disabled && onScheduleChange) {
if (disabled && onScheduleChange) {
// When disabled, schedule the change instead of applying immediately
if (configId === value) {
// Prevent scheduling the current model
onScheduleChange(undefined)
setOpen(false)
setSearchValue("")
return
}
if (scheduledConfigId === configId) {
// If clicking the same config, cancel the scheduled change
onScheduleChange(undefined)
} else {
// Schedule the new config
onScheduleChange(configId)
}

Copy link

@mikhailartamonov mikhailartamonov Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (disabled && onScheduleChange) {
if (disabled && onScheduleChange) {

// When disabled, schedule the change instead of applying immediately
if (scheduledConfigId === configId) {
// If clicking the same config, cancel the scheduled change
onScheduleChange(undefined)
} else {
// Schedule the new config
onScheduleChange(configId)
}
} else {
// Apply immediately when not disabled
onChange(configId)
Copy link
Contributor Author

@roomote roomote bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When disabled is true but onScheduleChange is undefined, selections will still call onChange(configId), applying the switch immediately during execution. That breaks the scheduling-only behavior. Consider guarding this path.

Suggested change
onChange(configId)
} else {
// Apply immediately when not disabled; but if disabled without scheduler, ignore
if (disabled) {
return
}
onChange(configId)
}

}
setOpen(false)
setSearchValue("")
},
[onChange],
[disabled, onChange, onScheduleChange, scheduledConfigId],
)

const handleEditClick = useCallback(() => {
Expand All @@ -88,6 +104,7 @@ export const ApiConfigSelector = ({
const renderConfigItem = useCallback(
(config: { id: string; name: string; modelId?: string }, isPinned: boolean) => {
const isCurrentConfig = config.id === value
const isScheduledConfig = config.id === scheduledConfigId

return (
<div
Expand All @@ -98,6 +115,7 @@ export const ApiConfigSelector = ({
"hover:bg-vscode-list-hoverBackground",
isCurrentConfig &&
"bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground",
isScheduledConfig && !isCurrentConfig && "border-l-2 border-vscode-focusBorder",
)}>
<div className="flex-1 min-w-0 flex items-center gap-1 overflow-hidden">
<span className="flex-shrink-0">{config.name}</span>
Expand All @@ -112,11 +130,18 @@ export const ApiConfigSelector = ({
)}
</div>
<div className="flex items-center gap-1">
{isCurrentConfig && (
{isCurrentConfig && !isScheduledConfig && (
<div className="size-5 p-1 flex items-center justify-center">
<span className="codicon codicon-check text-xs" />
</div>
)}
{isScheduledConfig && (
<StandardTooltip content={disabled ? t("chat:scheduledSwitch") : t("chat:nextModel")}>
<div className="size-5 p-1 flex items-center justify-center">
<span className="codicon codicon-clock text-xs text-vscode-focusBorder" />
</div>
</StandardTooltip>
)}
<StandardTooltip content={isPinned ? t("chat:unpin") : t("chat:pin")}>
<Button
variant="ghost"
Expand All @@ -138,25 +163,40 @@ export const ApiConfigSelector = ({
</div>
)
},
[value, handleSelect, t, togglePinnedApiConfig],
[value, scheduledConfigId, handleSelect, t, togglePinnedApiConfig, disabled],
)

// Get the scheduled config's display name
const scheduledConfigName = useMemo(() => {
if (!scheduledConfigId) return null
const config = listApiConfigMeta.find((c) => c.id === scheduledConfigId)
return config?.name || null
}, [scheduledConfigId, listApiConfigMeta])

return (
<Popover open={open} onOpenChange={setOpen} data-testid="api-config-selector-root">
<StandardTooltip content={title}>
<StandardTooltip
content={scheduledConfigName && disabled ? `${title} (Scheduled: ${scheduledConfigName})` : title}>
<PopoverTrigger
disabled={disabled}
disabled={false} // Always allow opening the dropdown
Copy link
Contributor Author

@roomote roomote bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Accessibility. The trigger is intentionally kept enabled (disabled={false}) to allow scheduling while execution is in progress, but it lacks aria-disabled. Adding aria-disabled preserves semantics for assistive tech while retaining interactivity.

Suggested change
disabled={false} // Always allow opening the dropdown
disabled={false} aria-disabled={disabled} // Always allow opening the dropdown

data-testid="dropdown-trigger"
className={cn(
"min-w-0 inline-flex items-center relative whitespace-nowrap px-1.5 py-1 text-xs",
"bg-transparent border border-[rgba(255,255,255,0.08)] rounded-md text-vscode-foreground",
"bg-transparent border rounded-md text-vscode-foreground",
"transition-all duration-150 focus:outline-none focus-visible:ring-1 focus-visible:ring-vscode-focusBorder focus-visible:ring-inset",
disabled
? "opacity-50 cursor-not-allowed"
: "opacity-90 hover:opacity-100 hover:bg-[rgba(255,255,255,0.03)] hover:border-[rgba(255,255,255,0.15)] cursor-pointer",
? scheduledConfigId
? "border-vscode-focusBorder opacity-70 cursor-pointer hover:opacity-90"
: "border-[rgba(255,255,255,0.08)] opacity-50 cursor-pointer hover:opacity-70"
: "border-[rgba(255,255,255,0.08)] opacity-90 hover:opacity-100 hover:bg-[rgba(255,255,255,0.03)] hover:border-[rgba(255,255,255,0.15)] cursor-pointer",
triggerClassName,
)}>
<span className="truncate">{displayName}</span>
<span className="truncate flex items-center gap-1">
{displayName}
{scheduledConfigId && disabled && (
<span className="codicon codicon-clock text-xs text-vscode-focusBorder" />
)}
</span>
</PopoverTrigger>
</StandardTooltip>
<PopoverContent
Expand Down Expand Up @@ -188,7 +228,11 @@ export const ApiConfigSelector = ({
) : (
<div className="p-3 border-b border-vscode-dropdown-border">
<p className="text-xs text-vscode-descriptionForeground m-0">
{t("prompts:apiConfiguration.select")}
{disabled && scheduledConfigId
? t("prompts:apiConfiguration.selectToCancel")
: disabled
? t("prompts:apiConfiguration.selectToSchedule")
: t("prompts:apiConfiguration.select")}
</p>
</div>
)}
Expand Down
21 changes: 21 additions & 0 deletions webview-ui/src/components/chat/ChatTextArea.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
{
inputValue,
setInputValue,
sendingDisabled,
selectApiConfigDisabled,
placeholderText,
selectedImages,
Expand Down Expand Up @@ -91,6 +92,9 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
cloudUserInfo,
} = useExtensionState()

// State for scheduled model switch
const [scheduledConfigId, setScheduledConfigId] = useState<string | undefined>(undefined)

// Find the ID and display text for the currently selected API configuration.
const { currentConfigId, displayName } = useMemo(() => {
const currentConfig = listApiConfigMeta?.find((config) => config.name === currentApiConfigName)
Expand Down Expand Up @@ -907,6 +911,21 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
vscode.postMessage({ type: "loadApiConfigurationById", text: value })
}, [])

// Handle scheduled config change
const handleScheduledConfigChange = useCallback((configId: string | undefined) => {
setScheduledConfigId(configId)
// We'll apply the change when sendingDisabled becomes false
}, [])

// Effect to apply scheduled config change when sending is re-enabled
useEffect(() => {
if (!sendingDisabled && scheduledConfigId) {
// Apply the scheduled change
handleApiConfigChange(scheduledConfigId)
setScheduledConfigId(undefined)
}
}, [sendingDisabled, scheduledConfigId, handleApiConfigChange])

return (
<div
className={cn(
Expand Down Expand Up @@ -1231,6 +1250,8 @@ export const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
listApiConfigMeta={listApiConfigMeta || []}
pinnedApiConfigs={pinnedApiConfigs}
togglePinnedApiConfig={togglePinnedApiConfig}
scheduledConfigId={scheduledConfigId}
onScheduleChange={handleScheduledConfigChange}
/>
<AutoApproveDropdown triggerClassName="min-w-[28px] text-ellipsis overflow-hidden flex-shrink" />
</div>
Expand Down
Loading
Loading