-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: map claude-sonnet-4-5 alias to full snapshot name claude-sonnet-4-5-20250929 #8546
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -288,5 +288,55 @@ describe("AnthropicHandler", () => { | |
| expect(model.info.inputPrice).toBe(6.0) | ||
| expect(model.info.outputPrice).toBe(22.5) | ||
| }) | ||
|
|
||
| it("should map claude-sonnet-4-5 to claude-sonnet-4-5-20250929 for API calls", async () => { | ||
|
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. [P2] Tests claim coverage for countTokens mapping in the PR description, but there is no test exercising AnthropicHandler.countTokens. Please add a unit test that sets apiModelId='claude-sonnet-4-5', invokes countTokens, and verifies the SDK is called with model 'claude-sonnet-4-5-20250929'. You will need to extend the @anthropic-ai/sdk mock with messages.countTokens. |
||
| const handler = new AnthropicHandler({ | ||
| apiKey: "test-api-key", | ||
| apiModelId: "claude-sonnet-4-5", | ||
| }) | ||
|
|
||
| // Test createMessage | ||
| const stream = handler.createMessage("Test system", [{ role: "user", content: "Test message" }]) | ||
|
|
||
| // Consume the stream to trigger the API call | ||
| for await (const chunk of stream) { | ||
| // Just consume the stream | ||
| } | ||
|
|
||
| // Verify the API was called with the full snapshot name | ||
| expect(mockCreate).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stream: true, | ||
| }), | ||
| expect.any(Object), | ||
| ) | ||
|
|
||
| // Clear mock for next test | ||
| mockCreate.mockClear() | ||
|
|
||
| // Test completePrompt | ||
| await handler.completePrompt("Test prompt") | ||
|
|
||
| // Verify the API was called with the full snapshot name | ||
| expect(mockCreate).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stream: false, | ||
| }), | ||
| ) | ||
| }) | ||
|
|
||
| it("should handle claude-sonnet-4-5-20250929 model directly", () => { | ||
| const handler = new AnthropicHandler({ | ||
| apiKey: "test-api-key", | ||
| apiModelId: "claude-sonnet-4-5-20250929", | ||
| }) | ||
| const model = handler.getModel() | ||
| expect(model.id).toBe("claude-sonnet-4-5-20250929") | ||
| expect(model.info.maxTokens).toBe(64000) | ||
| expect(model.info.contextWindow).toBe(200000) | ||
| expect(model.info.supportsReasoningBudget).toBe(true) | ||
| }) | ||
| }) | ||
| }) | ||
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.
[P3] Duplicated model metadata for 'claude-sonnet-4-5' and 'claude-sonnet-4-5-20250929' increases maintenance overhead and the risk of drift. Consider defining a single source of truth (e.g., keep the snapshot entry and alias the short ID at the UI layer or via a mapping table) to avoid pricing/context discrepancies in future updates.