Skip to content

Commit 0728875

Browse files
elianivaelianiva
committed
fix(export): exclude max tokens field for models that don't support it
1 parent 13534cc commit 0728875

File tree

2 files changed

+281
-0
lines changed

2 files changed

+281
-0
lines changed

‎src/core/config/ProviderSettingsManager.ts‎

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

1818
import { Mode, modes } from "../../shared/modes"
19+
import { buildApiHandler } from "../../api"
1920

2021
// Type-safe model migrations mapping
2122
type ModelMigrations = {
@@ -528,6 +529,25 @@ export class ProviderSettingsManager {
528529
for (const name in configs) {
529530
// Avoid leaking properties from other providers.
530531
configs[name] = discriminatedProviderSettingsWithIdSchema.parse(configs[name])
532+
533+
// If it has no apiProvider, skip filtering
534+
if (!configs[name].apiProvider) {
535+
continue
536+
}
537+
538+
// Try to build an API handler to get model information
539+
const apiHandler = buildApiHandler(configs[name])
540+
const modelInfo = apiHandler.getModel().info
541+
542+
// Check if the model supports reasoning budgets
543+
const supportsThinkingBudget =
544+
modelInfo.supportsReasoningBudget || modelInfo.requiredReasoningBudget
545+
546+
// If the model doesn't support reasoning budgets, remove the token fields
547+
if (!supportsThinkingBudget) {
548+
delete configs[name].modelMaxTokens
549+
delete configs[name].modelMaxThinkingTokens
550+
}
531551
}
532552
return profiles
533553
})

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

Lines changed: 261 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,157 @@ describe("importExport", () => {
16081717
"https://custom-api.example.com/v1",
16091718
)
16101719
})
1720+
1721+
it("should exclude modelMaxTokens and modelMaxThinkingTokens when supportsReasoningBudget is false", async () => {
1722+
// This test verifies that token fields are excluded when model doesn't support reasoning budget
1723+
// Using claude-code provider which has supportsReasoningBudget: false
1724+
1725+
;(vscode.window.showSaveDialog as Mock).mockResolvedValue({
1726+
fsPath: "/mock/path/roo-code-settings.json",
1727+
})
1728+
1729+
// Use a real ProviderSettingsManager instance to test the actual filtering logic
1730+
const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext)
1731+
1732+
// Wait for initialization to complete
1733+
await realProviderSettingsManager.initialize()
1734+
1735+
// Save a claude-code provider config with token fields
1736+
await realProviderSettingsManager.saveConfig("claude-code-provider", {
1737+
apiProvider: "claude-code" as ProviderName,
1738+
apiModelId: "claude-sonnet-4-5",
1739+
id: "claude-code-id",
1740+
modelMaxTokens: 4096, // This should be removed during export
1741+
modelMaxThinkingTokens: 2048, // This should be removed during export
1742+
})
1743+
1744+
// Set this as the current provider
1745+
await realProviderSettingsManager.activateProfile({ name: "claude-code-provider" })
1746+
1747+
const mockGlobalSettings = {
1748+
mode: "code",
1749+
autoApprovalEnabled: true,
1750+
}
1751+
1752+
mockContextProxy.export.mockResolvedValue(mockGlobalSettings)
1753+
;(fs.mkdir as Mock).mockResolvedValue(undefined)
1754+
1755+
await exportSettings({
1756+
providerSettingsManager: realProviderSettingsManager,
1757+
contextProxy: mockContextProxy,
1758+
})
1759+
1760+
// Get the exported data
1761+
const exportedData = (safeWriteJson as Mock).mock.calls[0][1]
1762+
1763+
// Verify that token fields were excluded because supportsReasoningBudget is false
1764+
const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider"]
1765+
expect(provider).toBeDefined()
1766+
expect(provider.apiModelId).toBe("claude-sonnet-4-5")
1767+
expect("modelMaxTokens" in provider).toBe(false) // Should be excluded
1768+
expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded
1769+
})
1770+
1771+
it("should exclude modelMaxTokens and modelMaxThinkingTokens when requiredReasoningBudget is false", async () => {
1772+
// This test verifies that token fields are excluded when model doesn't require reasoning budget
1773+
// Using claude-code provider which has requiredReasoningBudget: false
1774+
1775+
;(vscode.window.showSaveDialog as Mock).mockResolvedValue({
1776+
fsPath: "/mock/path/roo-code-settings.json",
1777+
})
1778+
1779+
// Use a real ProviderSettingsManager instance to test the actual filtering logic
1780+
const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext)
1781+
1782+
// Wait for initialization to complete
1783+
await realProviderSettingsManager.initialize()
1784+
1785+
// Save a claude-code provider config with token fields
1786+
await realProviderSettingsManager.saveConfig("claude-code-provider-2", {
1787+
apiProvider: "claude-code" as ProviderName,
1788+
apiModelId: "claude-sonnet-4-5",
1789+
id: "claude-code-id-2",
1790+
apiKey: "test-key",
1791+
modelMaxTokens: 4096, // This should be removed during export
1792+
modelMaxThinkingTokens: 2048, // This should be removed during export
1793+
})
1794+
1795+
// Set this as the current provider
1796+
await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-2" })
1797+
1798+
const mockGlobalSettings = {
1799+
mode: "code",
1800+
autoApprovalEnabled: true,
1801+
}
1802+
1803+
mockContextProxy.export.mockResolvedValue(mockGlobalSettings)
1804+
;(fs.mkdir as Mock).mockResolvedValue(undefined)
1805+
1806+
await exportSettings({
1807+
providerSettingsManager: realProviderSettingsManager,
1808+
contextProxy: mockContextProxy,
1809+
})
1810+
1811+
// Get the exported data
1812+
const exportedData = (safeWriteJson as Mock).mock.calls[0][1]
1813+
1814+
// Verify that token fields were excluded because requiredReasoningBudget is false
1815+
const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-2"]
1816+
expect(provider).toBeDefined()
1817+
expect(provider.apiModelId).toBe("claude-sonnet-4-5")
1818+
expect("modelMaxTokens" in provider).toBe(false) // Should be excluded
1819+
expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded
1820+
})
1821+
1822+
it("should exclude modelMaxTokens and modelMaxThinkingTokens when both supportsReasoningBudget and requiredReasoningBudget are false", async () => {
1823+
// This test verifies that token fields are excluded when model has both reasoning budget flags set to false
1824+
// Using claude-code provider which has both flags set to false
1825+
1826+
;(vscode.window.showSaveDialog as Mock).mockResolvedValue({
1827+
fsPath: "/mock/path/roo-code-settings.json",
1828+
})
1829+
1830+
// Use a real ProviderSettingsManager instance to test the actual filtering logic
1831+
const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext)
1832+
1833+
// Wait for initialization to complete
1834+
await realProviderSettingsManager.initialize()
1835+
1836+
// Save a claude-code provider config with token fields
1837+
await realProviderSettingsManager.saveConfig("claude-code-provider-3", {
1838+
apiProvider: "claude-code" as ProviderName,
1839+
apiModelId: "claude-3-5-haiku-20241022", // Use a different model ID
1840+
id: "claude-code-id-3",
1841+
apiKey: "test-key",
1842+
modelMaxTokens: 4096, // This should be removed during export
1843+
modelMaxThinkingTokens: 2048, // This should be removed during export
1844+
})
1845+
1846+
// Set this as the current provider
1847+
await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-3" })
1848+
1849+
const mockGlobalSettings = {
1850+
mode: "code",
1851+
autoApprovalEnabled: true,
1852+
}
1853+
1854+
mockContextProxy.export.mockResolvedValue(mockGlobalSettings)
1855+
;(fs.mkdir as Mock).mockResolvedValue(undefined)
1856+
1857+
await exportSettings({
1858+
providerSettingsManager: realProviderSettingsManager,
1859+
contextProxy: mockContextProxy,
1860+
})
1861+
1862+
// Get the exported data
1863+
const exportedData = (safeWriteJson as Mock).mock.calls[0][1]
1864+
1865+
// Verify that token fields were excluded because both reasoning budget flags are false
1866+
const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-3"]
1867+
expect(provider).toBeDefined()
1868+
expect(provider.apiModelId).toBe("claude-3-5-haiku-20241022")
1869+
expect("modelMaxTokens" in provider).toBe(false) // Should be excluded
1870+
expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded
1871+
})
16111872
})
16121873
})

0 commit comments

Comments
 (0)