Skip to content

fix: prevent context compression trigger when saving unchanged settings #7947

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 2 commits into from
Closed

fix: prevent context compression trigger when saving unchanged settings #7947

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83ed754
fix: prevent context compression trigger when saving unchanged settings
roomote Sep 12, 2025
3df1150
fix: prevent unnecessary state updates when settings haven't changed
roomote Sep 13, 2025
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
300 changes: 227 additions & 73 deletions webview-ui/src/components/settings/SettingsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,79 +296,233 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t

const handleSubmit = () => {
if (isSettingValid) {
vscode.postMessage({ type: "language", text: language })
vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly })
vscode.postMessage({
type: "alwaysAllowReadOnlyOutsideWorkspace",
bool: alwaysAllowReadOnlyOutsideWorkspace,
})
vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite })
vscode.postMessage({ type: "alwaysAllowWriteOutsideWorkspace", bool: alwaysAllowWriteOutsideWorkspace })
vscode.postMessage({ type: "alwaysAllowWriteProtected", bool: alwaysAllowWriteProtected })
vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute })
vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser })
vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp })
vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] })
vscode.postMessage({ type: "deniedCommands", commands: deniedCommands ?? [] })
vscode.postMessage({ type: "allowedMaxRequests", value: allowedMaxRequests ?? undefined })
vscode.postMessage({ type: "allowedMaxCost", value: allowedMaxCost ?? undefined })
vscode.postMessage({ type: "autoCondenseContext", bool: autoCondenseContext })
vscode.postMessage({ type: "autoCondenseContextPercent", value: autoCondenseContextPercent })
vscode.postMessage({ type: "browserToolEnabled", bool: browserToolEnabled })
vscode.postMessage({ type: "soundEnabled", bool: soundEnabled })
vscode.postMessage({ type: "ttsEnabled", bool: ttsEnabled })
vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed })
vscode.postMessage({ type: "soundVolume", value: soundVolume })
vscode.postMessage({ type: "diffEnabled", bool: diffEnabled })
vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints })
vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize })
vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost })
vscode.postMessage({ type: "remoteBrowserEnabled", bool: remoteBrowserEnabled })
vscode.postMessage({ type: "fuzzyMatchThreshold", value: fuzzyMatchThreshold ?? 1.0 })
vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs })
vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 })
vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 })
vscode.postMessage({ type: "terminalOutputCharacterLimit", value: terminalOutputCharacterLimit ?? 50000 })
vscode.postMessage({ type: "terminalShellIntegrationTimeout", value: terminalShellIntegrationTimeout })
vscode.postMessage({ type: "terminalShellIntegrationDisabled", bool: terminalShellIntegrationDisabled })
vscode.postMessage({ type: "terminalCommandDelay", value: terminalCommandDelay })
vscode.postMessage({ type: "terminalPowershellCounter", bool: terminalPowershellCounter })
vscode.postMessage({ type: "terminalZshClearEolMark", bool: terminalZshClearEolMark })
vscode.postMessage({ type: "terminalZshOhMy", bool: terminalZshOhMy })
vscode.postMessage({ type: "terminalZshP10k", bool: terminalZshP10k })
vscode.postMessage({ type: "terminalZdotdir", bool: terminalZdotdir })
vscode.postMessage({ type: "terminalCompressProgressBar", bool: terminalCompressProgressBar })
vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled })
vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit })
vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds })
vscode.postMessage({ type: "maxOpenTabsContext", value: maxOpenTabsContext })
vscode.postMessage({ type: "maxWorkspaceFiles", value: maxWorkspaceFiles ?? 200 })
vscode.postMessage({ type: "showRooIgnoredFiles", bool: showRooIgnoredFiles })
vscode.postMessage({ type: "maxReadFileLine", value: maxReadFileLine ?? -1 })
vscode.postMessage({ type: "maxImageFileSize", value: maxImageFileSize ?? 5 })
vscode.postMessage({ type: "maxTotalImageSize", value: maxTotalImageSize ?? 20 })
vscode.postMessage({ type: "maxConcurrentFileReads", value: cachedState.maxConcurrentFileReads ?? 5 })
vscode.postMessage({ type: "includeDiagnosticMessages", bool: includeDiagnosticMessages })
vscode.postMessage({ type: "maxDiagnosticMessages", value: maxDiagnosticMessages ?? 50 })
vscode.postMessage({ type: "currentApiConfigName", text: currentApiConfigName })
vscode.postMessage({ type: "updateExperimental", values: experiments })
vscode.postMessage({ type: "alwaysAllowModeSwitch", bool: alwaysAllowModeSwitch })
vscode.postMessage({ type: "alwaysAllowSubtasks", bool: alwaysAllowSubtasks })
vscode.postMessage({ type: "alwaysAllowFollowupQuestions", bool: alwaysAllowFollowupQuestions })
vscode.postMessage({ type: "alwaysAllowUpdateTodoList", bool: alwaysAllowUpdateTodoList })
vscode.postMessage({ type: "followupAutoApproveTimeoutMs", value: followupAutoApproveTimeoutMs })
vscode.postMessage({ type: "condensingApiConfigId", text: condensingApiConfigId || "" })
vscode.postMessage({ type: "updateCondensingPrompt", text: customCondensingPrompt || "" })
vscode.postMessage({ type: "updateSupportPrompt", values: customSupportPrompts || {} })
vscode.postMessage({ type: "includeTaskHistoryInEnhance", bool: includeTaskHistoryInEnhance ?? true })
vscode.postMessage({ type: "upsertApiConfiguration", text: currentApiConfigName, apiConfiguration })
vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting })
vscode.postMessage({ type: "profileThresholds", values: profileThresholds })
vscode.postMessage({ type: "openRouterImageApiKey", text: openRouterImageApiKey })
vscode.postMessage({
type: "openRouterImageGenerationSelectedModel",
text: openRouterImageGenerationSelectedModel,
})
// Helper function to check if a value has changed
const hasChanged = (cachedValue: any, originalValue: any): boolean => {
Copy link
Contributor Author

@roomote roomote bot Sep 12, 2025

Choose a reason for hiding this comment

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

The fix correctly addresses the issue by only sending messages for changed settings. However, I notice we're missing test coverage for this new hasChanged helper function and the conditional message sending logic. Could we add tests to ensure this critical functionality works as expected?

Copy link
Contributor Author

@roomote roomote bot Sep 12, 2025

Choose a reason for hiding this comment

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

Minor type safety improvement: Consider making the hasChanged function generic with proper TypeScript types instead of using any:

Suggested change
const hasChanged = (cachedValue: any, originalValue: any): boolean => {
const hasChanged = <T,>(cachedValue: T, originalValue: T): boolean => {
// Handle objects and arrays with deep comparison
if (typeof cachedValue === "object" && cachedValue !== null) {
return JSON.stringify(cachedValue) !== JSON.stringify(originalValue)
}
return cachedValue !== originalValue
}

// Handle objects and arrays with deep comparison
if (typeof cachedValue === "object" && cachedValue !== null) {
return JSON.stringify(cachedValue) !== JSON.stringify(originalValue)
Copy link
Contributor Author

@roomote roomote bot Sep 12, 2025

Choose a reason for hiding this comment

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

Is using JSON.stringify for deep comparison intentional here? For large configuration objects, this could be expensive. Have you considered using a more efficient deep equality check library like lodash.isEqual or memoizing the stringified values?

}
return cachedValue !== originalValue
}

// Only send messages for settings that have actually changed
if (hasChanged(language, extensionState.language)) {
Copy link
Contributor Author

@roomote roomote bot Sep 12, 2025

Choose a reason for hiding this comment

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

I notice there's significant repetition in the conditional checks (lines 309-525). Each setting follows the same pattern. Could we refactor this into a more maintainable approach? For example:

Suggested change
if (hasChanged(language, extensionState.language)) {
// Helper to send message only if changed
const sendIfChanged = <T,>(key: keyof ExtensionStateContextType, messageType: string, value: T, messageBuilder: (value: T) => any) => {
if (hasChanged(value, extensionState[key])) {
vscode.postMessage(messageBuilder(value))
}
}
// Usage
sendIfChanged('language', 'language', language, (val) => ({ type: 'language', text: val }))
sendIfChanged('alwaysAllowReadOnly', 'alwaysAllowReadOnly', alwaysAllowReadOnly, (val) => ({ type: 'alwaysAllowReadOnly', bool: val }))

vscode.postMessage({ type: "language", text: language })
}
if (hasChanged(alwaysAllowReadOnly, extensionState.alwaysAllowReadOnly)) {
vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly })
}
if (hasChanged(alwaysAllowReadOnlyOutsideWorkspace, extensionState.alwaysAllowReadOnlyOutsideWorkspace)) {
vscode.postMessage({
type: "alwaysAllowReadOnlyOutsideWorkspace",
bool: alwaysAllowReadOnlyOutsideWorkspace,
})
}
if (hasChanged(alwaysAllowWrite, extensionState.alwaysAllowWrite)) {
vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite })
}
if (hasChanged(alwaysAllowWriteOutsideWorkspace, extensionState.alwaysAllowWriteOutsideWorkspace)) {
vscode.postMessage({ type: "alwaysAllowWriteOutsideWorkspace", bool: alwaysAllowWriteOutsideWorkspace })
}
if (hasChanged(alwaysAllowWriteProtected, extensionState.alwaysAllowWriteProtected)) {
vscode.postMessage({ type: "alwaysAllowWriteProtected", bool: alwaysAllowWriteProtected })
}
if (hasChanged(alwaysAllowExecute, extensionState.alwaysAllowExecute)) {
vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute })
}
if (hasChanged(alwaysAllowBrowser, extensionState.alwaysAllowBrowser)) {
vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser })
}
if (hasChanged(alwaysAllowMcp, extensionState.alwaysAllowMcp)) {
vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp })
}
if (hasChanged(allowedCommands, extensionState.allowedCommands)) {
vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] })
}
if (hasChanged(deniedCommands, extensionState.deniedCommands)) {
vscode.postMessage({ type: "deniedCommands", commands: deniedCommands ?? [] })
}
if (hasChanged(allowedMaxRequests, extensionState.allowedMaxRequests)) {
vscode.postMessage({ type: "allowedMaxRequests", value: allowedMaxRequests ?? undefined })
}
if (hasChanged(allowedMaxCost, extensionState.allowedMaxCost)) {
vscode.postMessage({ type: "allowedMaxCost", value: allowedMaxCost ?? undefined })
}
// Only send autoCondenseContext if it has actually changed
if (hasChanged(autoCondenseContext, extensionState.autoCondenseContext)) {
vscode.postMessage({ type: "autoCondenseContext", bool: autoCondenseContext })
}
// Only send autoCondenseContextPercent if it has actually changed
if (hasChanged(autoCondenseContextPercent, extensionState.autoCondenseContextPercent)) {
vscode.postMessage({ type: "autoCondenseContextPercent", value: autoCondenseContextPercent })
}
if (hasChanged(browserToolEnabled, extensionState.browserToolEnabled)) {
vscode.postMessage({ type: "browserToolEnabled", bool: browserToolEnabled })
}
if (hasChanged(soundEnabled, extensionState.soundEnabled)) {
vscode.postMessage({ type: "soundEnabled", bool: soundEnabled })
}
if (hasChanged(ttsEnabled, extensionState.ttsEnabled)) {
vscode.postMessage({ type: "ttsEnabled", bool: ttsEnabled })
}
if (hasChanged(ttsSpeed, extensionState.ttsSpeed)) {
vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed })
}
if (hasChanged(soundVolume, extensionState.soundVolume)) {
vscode.postMessage({ type: "soundVolume", value: soundVolume })
}
if (hasChanged(diffEnabled, extensionState.diffEnabled)) {
vscode.postMessage({ type: "diffEnabled", bool: diffEnabled })
}
if (hasChanged(enableCheckpoints, extensionState.enableCheckpoints)) {
vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints })
}
if (hasChanged(browserViewportSize, extensionState.browserViewportSize)) {
vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize })
}
if (hasChanged(remoteBrowserHost, extensionState.remoteBrowserHost)) {
vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost })
}
if (hasChanged(remoteBrowserEnabled, extensionState.remoteBrowserEnabled)) {
vscode.postMessage({ type: "remoteBrowserEnabled", bool: remoteBrowserEnabled })
}
if (hasChanged(fuzzyMatchThreshold, extensionState.fuzzyMatchThreshold)) {
vscode.postMessage({ type: "fuzzyMatchThreshold", value: fuzzyMatchThreshold ?? 1.0 })
}
if (hasChanged(writeDelayMs, extensionState.writeDelayMs)) {
vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs })
}
if (hasChanged(screenshotQuality, extensionState.screenshotQuality)) {
vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 })
}
if (hasChanged(terminalOutputLineLimit, extensionState.terminalOutputLineLimit)) {
vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 })
}
if (hasChanged(terminalOutputCharacterLimit, extensionState.terminalOutputCharacterLimit)) {
vscode.postMessage({
type: "terminalOutputCharacterLimit",
value: terminalOutputCharacterLimit ?? 50000,
})
}
if (hasChanged(terminalShellIntegrationTimeout, extensionState.terminalShellIntegrationTimeout)) {
vscode.postMessage({ type: "terminalShellIntegrationTimeout", value: terminalShellIntegrationTimeout })
}
if (hasChanged(terminalShellIntegrationDisabled, extensionState.terminalShellIntegrationDisabled)) {
vscode.postMessage({ type: "terminalShellIntegrationDisabled", bool: terminalShellIntegrationDisabled })
}
if (hasChanged(terminalCommandDelay, extensionState.terminalCommandDelay)) {
vscode.postMessage({ type: "terminalCommandDelay", value: terminalCommandDelay })
}
if (hasChanged(terminalPowershellCounter, extensionState.terminalPowershellCounter)) {
vscode.postMessage({ type: "terminalPowershellCounter", bool: terminalPowershellCounter })
}
if (hasChanged(terminalZshClearEolMark, extensionState.terminalZshClearEolMark)) {
vscode.postMessage({ type: "terminalZshClearEolMark", bool: terminalZshClearEolMark })
}
if (hasChanged(terminalZshOhMy, extensionState.terminalZshOhMy)) {
vscode.postMessage({ type: "terminalZshOhMy", bool: terminalZshOhMy })
}
if (hasChanged(terminalZshP10k, extensionState.terminalZshP10k)) {
vscode.postMessage({ type: "terminalZshP10k", bool: terminalZshP10k })
}
if (hasChanged(terminalZdotdir, extensionState.terminalZdotdir)) {
vscode.postMessage({ type: "terminalZdotdir", bool: terminalZdotdir })
}
if (hasChanged(terminalCompressProgressBar, extensionState.terminalCompressProgressBar)) {
vscode.postMessage({ type: "terminalCompressProgressBar", bool: terminalCompressProgressBar })
}
if (hasChanged(mcpEnabled, extensionState.mcpEnabled)) {
vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled })
}
if (hasChanged(alwaysApproveResubmit, extensionState.alwaysApproveResubmit)) {
vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit })
}
if (hasChanged(requestDelaySeconds, extensionState.requestDelaySeconds)) {
vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds })
}
if (hasChanged(maxOpenTabsContext, extensionState.maxOpenTabsContext)) {
vscode.postMessage({ type: "maxOpenTabsContext", value: maxOpenTabsContext })
}
if (hasChanged(maxWorkspaceFiles, extensionState.maxWorkspaceFiles)) {
vscode.postMessage({ type: "maxWorkspaceFiles", value: maxWorkspaceFiles ?? 200 })
}
if (hasChanged(showRooIgnoredFiles, extensionState.showRooIgnoredFiles)) {
vscode.postMessage({ type: "showRooIgnoredFiles", bool: showRooIgnoredFiles })
}
if (hasChanged(maxReadFileLine, extensionState.maxReadFileLine)) {
vscode.postMessage({ type: "maxReadFileLine", value: maxReadFileLine ?? -1 })
}
if (hasChanged(maxImageFileSize, extensionState.maxImageFileSize)) {
vscode.postMessage({ type: "maxImageFileSize", value: maxImageFileSize ?? 5 })
}
if (hasChanged(maxTotalImageSize, extensionState.maxTotalImageSize)) {
vscode.postMessage({ type: "maxTotalImageSize", value: maxTotalImageSize ?? 20 })
}
if (hasChanged(maxConcurrentFileReads, extensionState.maxConcurrentFileReads)) {
vscode.postMessage({ type: "maxConcurrentFileReads", value: cachedState.maxConcurrentFileReads ?? 5 })
Copy link
Contributor Author

@roomote roomote bot Sep 12, 2025

Choose a reason for hiding this comment

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

I'm using cachedState.maxConcurrentFileReads here but all other lines use the destructured variable. Should this be maxConcurrentFileReads for consistency?

}
if (hasChanged(includeDiagnosticMessages, extensionState.includeDiagnosticMessages)) {
vscode.postMessage({ type: "includeDiagnosticMessages", bool: includeDiagnosticMessages })
}
if (hasChanged(maxDiagnosticMessages, extensionState.maxDiagnosticMessages)) {
vscode.postMessage({ type: "maxDiagnosticMessages", value: maxDiagnosticMessages ?? 50 })
}
if (hasChanged(currentApiConfigName, extensionState.currentApiConfigName)) {
vscode.postMessage({ type: "currentApiConfigName", text: currentApiConfigName })
}
if (hasChanged(experiments, extensionState.experiments)) {
vscode.postMessage({ type: "updateExperimental", values: experiments })
}
if (hasChanged(alwaysAllowModeSwitch, extensionState.alwaysAllowModeSwitch)) {
vscode.postMessage({ type: "alwaysAllowModeSwitch", bool: alwaysAllowModeSwitch })
}
if (hasChanged(alwaysAllowSubtasks, extensionState.alwaysAllowSubtasks)) {
vscode.postMessage({ type: "alwaysAllowSubtasks", bool: alwaysAllowSubtasks })
}
if (hasChanged(alwaysAllowFollowupQuestions, extensionState.alwaysAllowFollowupQuestions)) {
vscode.postMessage({ type: "alwaysAllowFollowupQuestions", bool: alwaysAllowFollowupQuestions })
}
if (hasChanged(alwaysAllowUpdateTodoList, extensionState.alwaysAllowUpdateTodoList)) {
vscode.postMessage({ type: "alwaysAllowUpdateTodoList", bool: alwaysAllowUpdateTodoList })
}
if (hasChanged(followupAutoApproveTimeoutMs, extensionState.followupAutoApproveTimeoutMs)) {
vscode.postMessage({ type: "followupAutoApproveTimeoutMs", value: followupAutoApproveTimeoutMs })
}
if (hasChanged(condensingApiConfigId, extensionState.condensingApiConfigId)) {
vscode.postMessage({ type: "condensingApiConfigId", text: condensingApiConfigId || "" })
}
if (hasChanged(customCondensingPrompt, extensionState.customCondensingPrompt)) {
vscode.postMessage({ type: "updateCondensingPrompt", text: customCondensingPrompt || "" })
}
if (hasChanged(customSupportPrompts, extensionState.customSupportPrompts)) {
vscode.postMessage({ type: "updateSupportPrompt", values: customSupportPrompts || {} })
}
if (hasChanged(includeTaskHistoryInEnhance, extensionState.includeTaskHistoryInEnhance)) {
vscode.postMessage({ type: "includeTaskHistoryInEnhance", bool: includeTaskHistoryInEnhance ?? true })
}
if (hasChanged(apiConfiguration, extensionState.apiConfiguration)) {
vscode.postMessage({ type: "upsertApiConfiguration", text: currentApiConfigName, apiConfiguration })
}
if (hasChanged(telemetrySetting, extensionState.telemetrySetting)) {
vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting })
}
if (hasChanged(profileThresholds, extensionState.profileThresholds)) {
vscode.postMessage({ type: "profileThresholds", values: profileThresholds })
}
if (hasChanged(openRouterImageApiKey, extensionState.openRouterImageApiKey)) {
vscode.postMessage({ type: "openRouterImageApiKey", text: openRouterImageApiKey })
}
if (
hasChanged(
openRouterImageGenerationSelectedModel,
extensionState.openRouterImageGenerationSelectedModel,
)
) {
vscode.postMessage({
type: "openRouterImageGenerationSelectedModel",
text: openRouterImageGenerationSelectedModel,
})
}
setChangeDetected(false)
}
}
Expand Down
Loading