Skip to content

Commit aae255d

Browse files
authored
fix(export): exclude max tokens field for models that don't support it (#8464)
1 parent a84f7ef commit aae255d

File tree

2 files changed

+208
-0
lines changed

2 files changed

+208
-0
lines changed

src/core/config/ProviderSettingsManager.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
import { TelemetryService } from "@roo-code/telemetry"
1616

1717
import { Mode, modes } from "../../shared/modes"
18+
import { buildApiHandler } from "../../api"
1819

1920
// Type-safe model migrations mapping
2021
type ModelMigrations = {
@@ -527,6 +528,31 @@ export class ProviderSettingsManager {
527528
for (const name in configs) {
528529
// Avoid leaking properties from other providers.
529530
configs[name] = discriminatedProviderSettingsWithIdSchema.parse(configs[name])
531+
532+
// If it has no apiProvider, skip filtering
533+
if (!configs[name].apiProvider) {
534+
continue
535+
}
536+
537+
// Try to build an API handler to get model information
538+
try {
539+
const apiHandler = buildApiHandler(configs[name])
540+
const modelInfo = apiHandler.getModel().info
541+
542+
// Check if the model supports reasoning budgets
543+
const supportsReasoningBudget =
544+
modelInfo.supportsReasoningBudget || modelInfo.requiredReasoningBudget
545+
546+
// If the model doesn't support reasoning budgets, remove the token fields
547+
if (!supportsReasoningBudget) {
548+
delete configs[name].modelMaxTokens
549+
delete configs[name].modelMaxThinkingTokens
550+
}
551+
} catch (error) {
552+
// If we can't build the API handler or get model info, skip filtering
553+
// to avoid accidental data loss from incomplete configurations
554+
console.warn(`Skipping token field filtering for config '${name}': ${error}`)
555+
}
530556
}
531557
return profiles
532558
})

src/core/config/__tests__/importExport.spec.ts

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ import { safeWriteJson } from "../../../utils/safeWriteJson"
1717
import type { Mock } from "vitest"
1818

1919
vi.mock("vscode", () => ({
20+
workspace: {
21+
getConfiguration: vi.fn().mockReturnValue({
22+
get: vi.fn(),
23+
}),
24+
},
2025
window: {
2126
showOpenDialog: vi.fn(),
2227
showSaveDialog: vi.fn(),
@@ -58,6 +63,45 @@ vi.mock("os", () => ({
5863

5964
vi.mock("../../../utils/safeWriteJson")
6065

66+
// Mock buildApiHandler to avoid issues with provider instantiation in tests
67+
vi.mock("../../../api", () => ({
68+
buildApiHandler: vi.fn().mockImplementation((config) => {
69+
// Return different model info based on the provider and model
70+
const getModelInfo = () => {
71+
if (config.apiProvider === "claude-code") {
72+
return {
73+
id: config.apiModelId || "claude-sonnet-4-5",
74+
info: {
75+
supportsReasoningBudget: false,
76+
requiredReasoningBudget: false,
77+
},
78+
}
79+
}
80+
if (config.apiProvider === "anthropic" && config.apiModelId === "claude-3-5-sonnet-20241022") {
81+
return {
82+
id: "claude-3-5-sonnet-20241022",
83+
info: {
84+
supportsReasoningBudget: true,
85+
requiredReasoningBudget: true,
86+
},
87+
}
88+
}
89+
// Default fallback
90+
return {
91+
id: config.apiModelId || "claude-sonnet-4-5",
92+
info: {
93+
supportsReasoningBudget: false,
94+
requiredReasoningBudget: false,
95+
},
96+
}
97+
}
98+
99+
return {
100+
getModel: vi.fn().mockReturnValue(getModelInfo()),
101+
}
102+
}),
103+
}))
104+
61105
describe("importExport", () => {
62106
let mockProviderSettingsManager: ReturnType<typeof vi.mocked<ProviderSettingsManager>>
63107
let mockContextProxy: ReturnType<typeof vi.mocked<ContextProxy>>
@@ -436,6 +480,71 @@ describe("importExport", () => {
436480

437481
showErrorMessageSpy.mockRestore()
438482
})
483+
484+
it("should handle import when reasoning budget fields are missing from config", async () => {
485+
// This test verifies that import works correctly when reasoning budget fields are not present
486+
// Using claude-code provider which doesn't support reasoning budgets
487+
488+
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])
489+
490+
const mockFileContent = JSON.stringify({
491+
providerProfiles: {
492+
currentApiConfigName: "claude-code-provider",
493+
apiConfigs: {
494+
"claude-code-provider": {
495+
apiProvider: "claude-code" as ProviderName,
496+
apiModelId: "claude-3-5-sonnet-20241022",
497+
id: "claude-code-id",
498+
apiKey: "test-key",
499+
// No modelMaxTokens or modelMaxThinkingTokens fields
500+
},
501+
},
502+
},
503+
globalSettings: { mode: "code", autoApprovalEnabled: true },
504+
})
505+
506+
;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
507+
508+
const previousProviderProfiles = {
509+
currentApiConfigName: "default",
510+
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
511+
}
512+
513+
mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles)
514+
mockProviderSettingsManager.listConfig.mockResolvedValue([
515+
{ name: "claude-code-provider", id: "claude-code-id", apiProvider: "claude-code" as ProviderName },
516+
{ name: "default", id: "default-id", apiProvider: "anthropic" as ProviderName },
517+
])
518+
519+
mockContextProxy.export.mockResolvedValue({ mode: "code" })
520+
521+
const result = await importSettings({
522+
providerSettingsManager: mockProviderSettingsManager,
523+
contextProxy: mockContextProxy,
524+
customModesManager: mockCustomModesManager,
525+
})
526+
527+
expect(result.success).toBe(true)
528+
expect(fs.readFile).toHaveBeenCalledWith("/mock/path/settings.json", "utf-8")
529+
expect(mockProviderSettingsManager.export).toHaveBeenCalled()
530+
531+
expect(mockProviderSettingsManager.import).toHaveBeenCalledWith({
532+
currentApiConfigName: "claude-code-provider",
533+
apiConfigs: {
534+
default: { apiProvider: "anthropic" as ProviderName, id: "default-id" },
535+
"claude-code-provider": {
536+
apiProvider: "claude-code" as ProviderName,
537+
apiModelId: "claude-3-5-sonnet-20241022",
538+
apiKey: "test-key",
539+
id: "claude-code-id",
540+
},
541+
},
542+
modeApiConfigs: {},
543+
})
544+
545+
expect(mockContextProxy.setValues).toHaveBeenCalledWith({ mode: "code", autoApprovalEnabled: true })
546+
expect(mockContextProxy.setValue).toHaveBeenCalledWith("currentApiConfigName", "claude-code-provider")
547+
})
439548
})
440549

441550
describe("exportSettings", () => {
@@ -1608,5 +1717,78 @@ describe("importExport", () => {
16081717
"https://custom-api.example.com/v1",
16091718
)
16101719
})
1720+
1721+
it.each([
1722+
{
1723+
testCase: "supportsReasoningBudget is false",
1724+
providerName: "claude-code-provider",
1725+
modelId: "claude-sonnet-4-5",
1726+
providerId: "claude-code-id",
1727+
},
1728+
{
1729+
testCase: "requiredReasoningBudget is false",
1730+
providerName: "claude-code-provider-2",
1731+
modelId: "claude-sonnet-4-5",
1732+
providerId: "claude-code-id-2",
1733+
},
1734+
{
1735+
testCase: "both supportsReasoningBudget and requiredReasoningBudget are false",
1736+
providerName: "claude-code-provider-3",
1737+
modelId: "claude-3-5-haiku-20241022",
1738+
providerId: "claude-code-id-3",
1739+
},
1740+
])(
1741+
"should exclude modelMaxTokens and modelMaxThinkingTokens when $testCase",
1742+
async ({ providerName, modelId, providerId }) => {
1743+
// This test verifies that token fields are excluded when model doesn't support reasoning budget
1744+
// Using claude-code provider which has supportsReasoningBudget: false and requiredReasoningBudget: false
1745+
1746+
;(vscode.window.showSaveDialog as Mock).mockResolvedValue({
1747+
fsPath: "/mock/path/roo-code-settings.json",
1748+
})
1749+
1750+
// Use a real ProviderSettingsManager instance to test the actual filtering logic
1751+
const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext)
1752+
1753+
// Wait for initialization to complete
1754+
await realProviderSettingsManager.initialize()
1755+
1756+
// Save a claude-code provider config with token fields
1757+
await realProviderSettingsManager.saveConfig(providerName, {
1758+
apiProvider: "claude-code" as ProviderName,
1759+
apiModelId: modelId,
1760+
id: providerId,
1761+
apiKey: "test-key",
1762+
modelMaxTokens: 4096, // This should be removed during export
1763+
modelMaxThinkingTokens: 2048, // This should be removed during export
1764+
})
1765+
1766+
// Set this as the current provider
1767+
await realProviderSettingsManager.activateProfile({ name: providerName })
1768+
1769+
const mockGlobalSettings = {
1770+
mode: "code",
1771+
autoApprovalEnabled: true,
1772+
}
1773+
1774+
mockContextProxy.export.mockResolvedValue(mockGlobalSettings)
1775+
;(fs.mkdir as Mock).mockResolvedValue(undefined)
1776+
1777+
await exportSettings({
1778+
providerSettingsManager: realProviderSettingsManager,
1779+
contextProxy: mockContextProxy,
1780+
})
1781+
1782+
// Get the exported data
1783+
const exportedData = (safeWriteJson as Mock).mock.calls[0][1]
1784+
1785+
// Verify that token fields were excluded because reasoning budget is not supported/required
1786+
const provider = exportedData.providerProfiles.apiConfigs[providerName]
1787+
expect(provider).toBeDefined()
1788+
expect(provider.apiModelId).toBe(modelId)
1789+
expect("modelMaxTokens" in provider).toBe(false) // Should be excluded
1790+
expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded
1791+
},
1792+
)
16111793
})
16121794
})

0 commit comments

Comments
 (0)