-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add configurable context window limit for Qwen CLI provider #7600
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest" | ||
| import { QwenCodeHandler } from "../qwen-code" | ||
| import { qwenCodeDefaultModelId, qwenCodeModels } from "@roo-code/types" | ||
|
|
||
| // Mock fs module | ||
| vi.mock("node:fs", () => ({ | ||
| promises: { | ||
| readFile: vi.fn(), | ||
| writeFile: vi.fn(), | ||
| }, | ||
| })) | ||
|
|
||
| // Mock fetch | ||
| global.fetch = vi.fn() | ||
|
|
||
| describe("QwenCodeHandler", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| describe("getModel", () => { | ||
| it("should return default model when no model is specified", () => { | ||
| const handler = new QwenCodeHandler({}) | ||
| const { id, info } = handler.getModel() | ||
|
|
||
| expect(id).toBe(qwenCodeDefaultModelId) | ||
| expect(info).toEqual(qwenCodeModels[qwenCodeDefaultModelId]) | ||
| }) | ||
|
|
||
| it("should return specified model when valid model is provided", () => { | ||
| const testModelId = "qwen3-coder-flash" | ||
| const handler = new QwenCodeHandler({ | ||
| apiModelId: testModelId, | ||
| }) | ||
| const { id, info } = handler.getModel() | ||
|
|
||
| expect(id).toBe(testModelId) | ||
| expect(info).toEqual(qwenCodeModels[testModelId]) | ||
| }) | ||
|
|
||
| it("should use default context window when qwenCodeMaxContextWindow is not set", () => { | ||
| const handler = new QwenCodeHandler({}) | ||
| const { info } = handler.getModel() | ||
|
|
||
| expect(info.contextWindow).toBe(1_000_000) // Default context window | ||
| }) | ||
|
|
||
| it("should limit context window when qwenCodeMaxContextWindow is set", () => { | ||
| const maxContextWindow = 256_000 | ||
| const handler = new QwenCodeHandler({ | ||
| qwenCodeMaxContextWindow: maxContextWindow, | ||
| }) | ||
| const { info } = handler.getModel() | ||
|
|
||
| expect(info.contextWindow).toBe(maxContextWindow) | ||
| }) | ||
|
|
||
| it("should use original context window when qwenCodeMaxContextWindow is larger", () => { | ||
| const handler = new QwenCodeHandler({ | ||
| qwenCodeMaxContextWindow: 2_000_000, // Larger than default | ||
| }) | ||
| const { info } = handler.getModel() | ||
|
|
||
| expect(info.contextWindow).toBe(1_000_000) // Should not exceed original | ||
| }) | ||
|
|
||
| it("should ignore qwenCodeMaxContextWindow when it's 0 or negative", () => { | ||
| const handler1 = new QwenCodeHandler({ | ||
| qwenCodeMaxContextWindow: 0, | ||
| }) | ||
| const { info: info1 } = handler1.getModel() | ||
|
|
||
| expect(info1.contextWindow).toBe(1_000_000) // Should use default | ||
|
|
||
| const handler2 = new QwenCodeHandler({ | ||
| qwenCodeMaxContextWindow: -1, | ||
| }) | ||
| const { info: info2 } = handler2.getModel() | ||
|
|
||
| expect(info2.contextWindow).toBe(1_000_000) // Should use default | ||
| }) | ||
|
|
||
| it("should apply context window limit to different models", () => { | ||
| const maxContextWindow = 200_000 | ||
| const handler = new QwenCodeHandler({ | ||
| apiModelId: "qwen3-coder-flash", | ||
| qwenCodeMaxContextWindow: maxContextWindow, | ||
| }) | ||
| const { id, info } = handler.getModel() | ||
|
|
||
| expect(id).toBe("qwen3-coder-flash") | ||
| expect(info.contextWindow).toBe(maxContextWindow) | ||
| // Other properties should remain unchanged | ||
| expect(info.maxTokens).toBe(qwenCodeModels["qwen3-coder-flash"].maxTokens) | ||
| expect(info.description).toBe(qwenCodeModels["qwen3-coder-flash"].description) | ||
| }) | ||
| }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great test coverage for the |
||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,6 +30,7 @@ interface QwenOAuthCredentials { | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| interface QwenCodeHandlerOptions extends ApiHandlerOptions { | ||||||||||||||||||||||||
| qwenCodeOauthPath?: string | ||||||||||||||||||||||||
| qwenCodeMaxContextWindow?: number | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function getQwenCachedCredentialPath(customPath?: string): string { | ||||||||||||||||||||||||
|
|
@@ -286,7 +287,17 @@ export class QwenCodeHandler extends BaseProvider implements SingleCompletionHan | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| override getModel(): { id: string; info: ModelInfo } { | ||||||||||||||||||||||||
| const id = this.options.apiModelId ?? qwenCodeDefaultModelId | ||||||||||||||||||||||||
| const info = qwenCodeModels[id as keyof typeof qwenCodeModels] || qwenCodeModels[qwenCodeDefaultModelId] | ||||||||||||||||||||||||
| let info: ModelInfo = | ||||||||||||||||||||||||
| qwenCodeModels[id as keyof typeof qwenCodeModels] || qwenCodeModels[qwenCodeDefaultModelId] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Apply custom context window limit if configured | ||||||||||||||||||||||||
| if (this.options.qwenCodeMaxContextWindow && this.options.qwenCodeMaxContextWindow > 0) { | ||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a debug or info log here when the context window is being limited? It could help users understand when and why their context is being capped:
Suggested change
|
||||||||||||||||||||||||
| info = { | ||||||||||||||||||||||||
| ...info, | ||||||||||||||||||||||||
| contextWindow: Math.min(info.contextWindow, this.options.qwenCodeMaxContextWindow), | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return { id, info } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the naming pattern intentional? I noticed AWS Bedrock uses
awsModelContextWindowfor a similar purpose. The "Max" prefix here does make the limiting behavior clearer, but we might want to consider consistency across providers. What do you think?