Repository navigation
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I checked this by calling Variant.resolve directly in a scratch test. A full CLI run against a capture server failed to start from source on both base and this PR, so this is not verified end to end. On the base, a Bedrock application inference-profile ARN with family: "claude-sonnet" gets the Nova reasoningConfig shape. With this change it gets the Claude shapes for effort, toggle and budget. The catalog id us.anthropic.claude-sonnet-4-5… and a non-Claude ARN produce the same output as before. Variant tests and the core typecheck pass.
Main concerns:
- An ARN carries no version, so ARN-backed Claude models always get
thinking: {type: "adaptive"}on effort variants. Claude 4.5 and older don't use adaptive thinking. The same model by catalog id gets onlyoutput_config. The real-profile check in #53756 used a Sonnet 4.5 profile, so this case is likely common (details inline). - The two new tests are near-copies and could be one.
| const bedrockConverse: Protocol = (model, support) => { | ||
| const id = modelID(model) | ||
| const claude = id.includes("anthropic") | ||
| const claude = id.includes("anthropic") || model.family === "claude" || model.family?.startsWith("claude-") |
There was a problem hiding this comment.
With an opaque ARN, claudeInfo(model) can't parse a version. So manual is false, and effort variants always add thinking: ADAPTIVE_THINKING (I checked: the ARN + claude-sonnet case gives thinking: adaptive + output_config). The same model by catalog id (us.anthropic.claude-sonnet-4-5…) gets only output_config: {effort}, because 4.5 and older don't use adaptive thinking. The real-profile check in #53756 used a Sonnet 4.5 profile, so this path may still be rejected for many users. It's an improvement over reasoningConfig, but please consider adding a test for the older-model case and noting the limitation. The catalog-id-as-key config from the issue still covers those models.
| ]) | ||
| }) | ||
|
|
||
| test("recognizes the bare Claude family for Bedrock inference profile ARNs", () => { |
There was a problem hiding this comment.
This test only differs from the one above in the family value. Could you merge them into one test that loops over ["claude-sonnet", "claude"]? It would be shorter with the same coverage. A non-Claude family on an ARN (e.g. nova) that still gets reasoningConfig would be a useful extra check.
Issue for this PR
Related issue: #53756 (this PR does not close the issue)
Type of change
What does this PR do?
Bedrock application inference-profile ARNs are opaque. This change uses configured family metadata (
claudeorclaude-*) to select Claude request fields instead of Nova'sreasoningConfigfallback.Limitation: family metadata does not identify a model version. For an arbitrary custom model key with an opaque ARN, this PR cannot determine whether Claude requires adaptive or manual thinking. Older model versions may still require explicit variants or using the catalog model ID as the model key, as documented in #53756.
How did you verify your code works?
cd packages/core && bun run test test/variant.test.ts— 14 passed, 0 failed, 60 expectations.cd packages/core && bun typecheck— passed.Screenshots / recordings
N/A — no UI change.
Checklist