zed icon indicating copy to clipboard operation
zed copied to clipboard

Claude 3.7 on Bedrock seems to have incorrect values

Open chasewalden opened this issue 7 months ago • 4 comments

Summary

While reading through crates/bedrock/src/models.rs, I noticed that some of the values appear to be wrong.

Facing the same issues due to #30714 and thought to start looking at the code to see if maybe I'd be able to open a PR myself.

While going through the bedrock crate, I came across what appears to be incorrect values for some of the Anthropic Claude models.

Max Tokens: Should be 200k, but in the code the "Thinking" variant is missing and ends up using the default 128k value.

https://github.com/zed-industries/zed/blob/ad4645c59bbcd56a612e59225a56b6e500e018eb/crates/bedrock/src/models.rs#L218-L222

Max Output Tokens: Should be 64k based on Anthropic's Docs, but is set to 128k.

https://github.com/zed-industries/zed/blob/ad4645c59bbcd56a612e59225a56b6e500e018eb/crates/bedrock/src/models.rs#L234-L235


On a possibly related note, it appears that only the "Thinking" variant of 3.7 Sonnet is available to be selected in the Assistant Panel.

Image

Zed Version and System Specs

Zed: v0.187.5 (Zed) OS: macOS 15.1.1 Memory: 18 GiB Architecture: aarch64

chasewalden avatar May 22 '25 21:05 chasewalden

@5herlocked, sorry for the mention. Since you raised #30714 and appear to be the one who added this in #25583, maybe you'd know if this looks correct, or if I'm misinterpreting the code here.

chasewalden avatar May 22 '25 21:05 chasewalden

Also, it looks like the first branch shadows the second (since claude-3-7-sonnet-thinking starts with claude-3-7-sonnet), but this might be a separate ticket.

https://github.com/zed-industries/zed/blob/24a108d87634a3dc47f301e9b5f79912b78cd084/crates/bedrock/src/models.rs#L104-L107

chasewalden avatar May 22 '25 21:05 chasewalden

Yeah thanks for raising this,

I have a branch that fixes a whole host of these issues. But I'm waiting for the Claude 4 changes to roll out before refactoring the whole provider to work with the whole thinking paradigm that we're moving towards.

The output token numbers have always been a little fuzzy to find, I'll have to run through the bedrock docs but i suspect you're right.

However for Sonnet thinking, https://docs.aws.amazon.com/bedrock/latest/userguide/model-parameters-anthropic-claude-37.html states that there's a 128K output mode, so I just assume users have access to it. In my stupidity however, I completely forget to pass the extended output length header so I suppose it didn't matter anyway.

5herlocked avatar May 22 '25 21:05 5herlocked

Also, it looks like the first branch shadows the second (since claude-3-7-sonnet-thinking starts with claude-3-7-sonnet), but this might be a separate ticket.

So there's not really a bug there, but it's more embedded in the way id() is used in the Language Model Provider things to de-dupe model values.

See https://github.com/zed-industries/zed/blob/ad4645c59bbcd56a612e59225a56b6e500e018eb/crates/language_models/src/provider/bedrock.rs#L296 which causes all the models with the same id to be merged into one -- causing Sonnet Thinking to override regular Sonnet.

The solution is here -- https://github.com/5herlocked/zed/commit/20cf870893c8176d94a3d9b11317fcd5de7394ec

5herlocked avatar May 22 '25 21:05 5herlocked

@5herlocked seems like this is maybe fixed - ok to close?

morgankrey avatar Nov 11 '25 17:11 morgankrey

This is fixed yeah @morgankrey ty!

5herlocked avatar Nov 11 '25 17:11 5herlocked