Skip to content

Commit e5d764c

Browse files
authored
Merge pull request RooCodeInc#1476 from RooVetGit/sliding_window_logging
Fix logic for default modelMaxTokens for thinking models
2 parents 2a65db6 + da286c4 commit e5d764c

File tree

5 files changed

+147
-6
lines changed

5 files changed

+147
-6
lines changed

src/core/Cline.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1124,9 +1124,12 @@ export class Cline {
11241124

11251125
const totalTokens = tokensIn + tokensOut + cacheWrites + cacheReads
11261126

1127+
// Default max tokens value for thinking models when no specific value is set
1128+
const DEFAULT_THINKING_MODEL_MAX_TOKENS = 16_384
1129+
11271130
const modelInfo = this.api.getModel().info
11281131
const maxTokens = modelInfo.thinking
1129-
? this.apiConfiguration.modelMaxTokens || modelInfo.maxTokens
1132+
? this.apiConfiguration.modelMaxTokens || DEFAULT_THINKING_MODEL_MAX_TOKENS
11301133
: modelInfo.maxTokens
11311134
const contextWindow = modelInfo.contextWindow
11321135
const trimmedMessages = await truncateConversationIfNeeded({

webview-ui/package-lock.json

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/__tests__/getMaxTokensForModel.test.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getMaxTokensForModel } from "@/utils/model-utils"
1+
import { DEFAULT_THINKING_MODEL_MAX_TOKENS, getMaxTokensForModel } from "@/utils/model-utils"
22

33
describe("getMaxTokensForModel utility from model-utils", () => {
44
test("should return maxTokens from modelInfo when thinking is false", () => {
@@ -29,7 +29,7 @@ describe("getMaxTokensForModel utility from model-utils", () => {
2929
expect(result).toBe(4096)
3030
})
3131

32-
test("should fallback to modelInfo.maxTokens when thinking is true but apiConfig.modelMaxTokens is not defined", () => {
32+
test("should fallback to DEFAULT_THINKING_MODEL_MAX_TOKENS when thinking is true but apiConfig.modelMaxTokens is not defined", () => {
3333
const modelInfo = {
3434
maxTokens: 2048,
3535
thinking: true,
@@ -38,7 +38,7 @@ describe("getMaxTokensForModel utility from model-utils", () => {
3838
const apiConfig = {}
3939

4040
const result = getMaxTokensForModel(modelInfo, apiConfig)
41-
expect(result).toBe(2048)
41+
expect(result).toBe(DEFAULT_THINKING_MODEL_MAX_TOKENS)
4242
})
4343

4444
test("should handle undefined inputs gracefully", () => {
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* @fileoverview Tests for token and model utility functions
3+
*/
4+
5+
import {
6+
getMaxTokensForModel,
7+
calculateTokenDistribution,
8+
ModelInfo,
9+
ApiConfig,
10+
DEFAULT_THINKING_MODEL_MAX_TOKENS,
11+
} from "../model-utils"
12+
13+
describe("Model utility functions", () => {
14+
describe("getMaxTokensForModel", () => {
15+
/**
16+
* Testing the specific fix in commit cc79178f:
17+
* For thinking models, use apiConfig.modelMaxTokens if available,
18+
* otherwise fall back to 16_384 (not modelInfo.maxTokens)
19+
*/
20+
21+
it("should return apiConfig.modelMaxTokens for thinking models when provided", () => {
22+
const modelInfo: ModelInfo = {
23+
thinking: true,
24+
maxTokens: 8000,
25+
}
26+
27+
const apiConfig: ApiConfig = {
28+
modelMaxTokens: 4000,
29+
}
30+
31+
expect(getMaxTokensForModel(modelInfo, apiConfig)).toBe(4000)
32+
})
33+
34+
it("should return 16_384 for thinking models when modelMaxTokens not provided", () => {
35+
const modelInfo: ModelInfo = {
36+
thinking: true,
37+
maxTokens: 8000,
38+
}
39+
40+
const apiConfig: ApiConfig = {}
41+
42+
// This tests the specific fix: now using DEFAULT_THINKING_MODEL_MAX_TOKENS instead of falling back to modelInfo.maxTokens
43+
expect(getMaxTokensForModel(modelInfo, apiConfig)).toBe(DEFAULT_THINKING_MODEL_MAX_TOKENS)
44+
})
45+
46+
it("should return 16_384 for thinking models when apiConfig is undefined", () => {
47+
const modelInfo: ModelInfo = {
48+
thinking: true,
49+
maxTokens: 8000,
50+
}
51+
52+
expect(getMaxTokensForModel(modelInfo, undefined)).toBe(DEFAULT_THINKING_MODEL_MAX_TOKENS)
53+
})
54+
55+
it("should return modelInfo.maxTokens for non-thinking models", () => {
56+
const modelInfo: ModelInfo = {
57+
thinking: false,
58+
maxTokens: 8000,
59+
}
60+
61+
const apiConfig: ApiConfig = {
62+
modelMaxTokens: 4000,
63+
}
64+
65+
expect(getMaxTokensForModel(modelInfo, apiConfig)).toBe(8000)
66+
})
67+
68+
it("should return undefined for non-thinking models with undefined maxTokens", () => {
69+
const modelInfo: ModelInfo = {
70+
thinking: false,
71+
}
72+
73+
const apiConfig: ApiConfig = {
74+
modelMaxTokens: 4000,
75+
}
76+
77+
expect(getMaxTokensForModel(modelInfo, apiConfig)).toBeUndefined()
78+
})
79+
80+
it("should return undefined when modelInfo is undefined", () => {
81+
const apiConfig: ApiConfig = {
82+
modelMaxTokens: 4000,
83+
}
84+
85+
expect(getMaxTokensForModel(undefined, apiConfig)).toBeUndefined()
86+
})
87+
})
88+
89+
describe("calculateTokenDistribution", () => {
90+
it("should calculate token distribution correctly", () => {
91+
const contextWindow = 10000
92+
const contextTokens = 5000
93+
const maxTokens = 2000
94+
95+
const result = calculateTokenDistribution(contextWindow, contextTokens, maxTokens)
96+
97+
expect(result.reservedForOutput).toBe(maxTokens)
98+
expect(result.availableSize).toBe(3000) // 10000 - 5000 - 2000
99+
100+
// Percentages should sum to 100%
101+
expect(Math.round(result.currentPercent + result.reservedPercent + result.availablePercent)).toBe(100)
102+
})
103+
104+
it("should default to 20% of context window when maxTokens not provided", () => {
105+
const contextWindow = 10000
106+
const contextTokens = 5000
107+
108+
const result = calculateTokenDistribution(contextWindow, contextTokens)
109+
110+
expect(result.reservedForOutput).toBe(2000) // 20% of 10000
111+
expect(result.availableSize).toBe(3000) // 10000 - 5000 - 2000
112+
})
113+
114+
it("should handle negative or zero inputs by using positive fallbacks", () => {
115+
const result = calculateTokenDistribution(-1000, -500)
116+
117+
expect(result.currentPercent).toBe(0)
118+
expect(result.reservedPercent).toBe(0)
119+
expect(result.availablePercent).toBe(0)
120+
expect(result.reservedForOutput).toBe(0) // With negative inputs, both context window and tokens become 0, so 20% of 0 is 0
121+
expect(result.availableSize).toBe(0)
122+
})
123+
124+
it("should handle zero total tokens without division by zero errors", () => {
125+
const result = calculateTokenDistribution(0, 0, 0)
126+
127+
expect(result.currentPercent).toBe(0)
128+
expect(result.reservedPercent).toBe(0)
129+
expect(result.availablePercent).toBe(0)
130+
expect(result.reservedForOutput).toBe(0)
131+
expect(result.availableSize).toBe(0)
132+
})
133+
})
134+
})

webview-ui/src/utils/model-utils.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
* Utility functions for working with language models and tokens
33
*/
44

5+
/**
6+
* Default maximum tokens for thinking-capable models when no specific value is provided
7+
*/
8+
export const DEFAULT_THINKING_MODEL_MAX_TOKENS = 16_384
9+
510
/**
611
* Model information interface with properties used in token calculations
712
*/
@@ -70,7 +75,7 @@ export const getMaxTokensForModel = (
7075
apiConfig: ApiConfig | undefined,
7176
): number | undefined => {
7277
if (modelInfo?.thinking) {
73-
return apiConfig?.modelMaxTokens || modelInfo?.maxTokens
78+
return apiConfig?.modelMaxTokens || DEFAULT_THINKING_MODEL_MAX_TOKENS
7479
}
7580
return modelInfo?.maxTokens
7681
}

0 commit comments

Comments
 (0)