Skip to content

feat: add Claude Opus 4.8 support across Anthropic, Bedrock, and Vertex providers#386

Merged
edelauna merged 9 commits into
Zoo-Code-Org:mainfrom
vandre-sales:feat/bedrock-claude-opus-4-8
May 29, 2026
Merged

feat: add Claude Opus 4.8 support across Anthropic, Bedrock, and Vertex providers#386
edelauna merged 9 commits into
Zoo-Code-Org:mainfrom
vandre-sales:feat/bedrock-claude-opus-4-8

Conversation

@vandre-sales
Copy link
Copy Markdown
Contributor

@vandre-sales vandre-sales commented May 28, 2026

Summary

Adds full support for Claude Opus 4.8 (claude-opus-4-8 / anthropic.claude-opus-4-8) across the Anthropic, AWS Bedrock, and Google Vertex providers. Opus 4.8 was released by Anthropic on May 28, 2026 as the successor to Opus 4.7.

This builds directly on the adaptive-thinking compatibility layer introduced for Opus 4.7 in #316. Per Anthropic's official migration guide, there are no breaking API changes between 4.7 and 4.8 — 4.8 inherits the same adaptive-thinking contract, the same removal of sampling parameters, the same native 1M context window, and the same prompt-cache behaviour. The change is therefore small and surgical.

Note on #316: This PR is a superset of #316 — because Opus 4.7 is not yet merged into main, the diff necessarily includes the 4.7 work it depends on (the isGen47Model detection branch). If #316 merges first, this PR will rebase down to the 4.8-only delta. Maintainers may prefer to review/merge this PR in place of #316, or merge #316 first and then this. Happy to adjust to whichever workflow you prefer.

What changed

Model registry (packages/types)

  • anthropic.ts — registered claude-opus-4-8: 1M context, 128k max output, supportsReasoningBudget, supportsReasoningBinary, supportsTemperature: false (mirrors 4.7).
  • bedrock.ts — registered anthropic.claude-opus-4-8 with prompt-cache points, cachableFields, and 1M tiered pricing; added it to BEDROCK_1M_CONTEXT_MODEL_IDS and BEDROCK_GLOBAL_INFERENCE_MODEL_IDS.
  • vertex.ts — registered claude-opus-4-8 with the same shape; added it to VERTEX_1M_CONTEXT_MODEL_IDS.

Provider logic (src/api/providers)

  • anthropic.ts — added claude-opus-4-8 to both prompt-caching switch statements so it gets the same handling as 4.7 (native 1M context, no context-1m-2025-08-07 beta header required).
  • bedrock.ts — renamed isGen47ModelisAdaptiveThinkingModel and expanded the pattern to match opus-4-7, opus-4-8, sonnet-4-7, and sonnet-4-8. The rename describes the actual capability (adaptive thinking) rather than a single generation, making future Claude versions easier to plug in. The branch still emits thinking: { type: "adaptive", display: "summarized" } + output_config: { effort: "xhigh" } and omits temperature.
  • openai.ts — comment-only update mentioning 4.8; no logic change (already honors the supportsTemperature: false flag).

Tests

  • anthropic.spec.ts — 5 new cases mirroring 4.7 (1M-beta-header guard, adaptive thinking ON/OFF, custom maxTokens, getModel info).
  • anthropic-vertex.spec.ts — 1M context tier pricing for Vertex Opus 4.8.
  • shared/api.spec.tsgetModelMaxOutputTokens hybrid-token handling on 4.8.
  • bedrock.spec.ts — new Claude 4.7+ adaptive thinking (Opus 4.7 / Opus 4.8) describe block with 5 cases covering:
    • adaptive thinking + xhigh effort for 4.7
    • adaptive thinking + xhigh effort for 4.8
    • thinking + temperature omitted when reasoning is disabled
    • regression guard: 4.6 still uses budget_tokens + temperature
    • cross-region prefix detection (us.anthropic.claude-opus-4-8)

This also closes the unit-test coverage gap that existed for the Bedrock adaptive-thinking branch in #316.

Fast Mode — out of scope

Opus 4.8 introduces a "fast mode" (2.5x faster, $10/$50 per 1M tokens). Per Anthropic docs it is not available on the Claude Platform on AWS / Bedrock and direct-API access is gated behind a waitlist, so it is intentionally excluded from this PR.

Other 4.8 features (no client change needed)

The following 4.8 additions are server-side optimizations that the existing client pipeline benefits from transparently — no code change required:

  • Mid-conversation role: "system" messages
  • refusal content block + documented stop_details
  • Lower prompt-cache minimum (1,024 tokens)

Backward compatibility

✅ 100% backward compatible. The adaptive-thinking branch only activates for opus-4-7, opus-4-8, sonnet-4-7, and sonnet-4-8. Claude 4.6 and earlier continue to use the budget_tokens thinking format and still send temperature (covered by a new regression test).

Pricing note

⚠️ Pricing for 4.8 is set to match 4.7 ($5/$25 per 1M tokens regular tier, $10/$37.5 for the 1M tier). Please verify against the AWS Bedrock pricing page before merging.

Testing

  • 235 unit tests pass, 0 failures
  • ✅ Type-check clean on both packages/types and src
  • Validated live end-to-end: built the VSIX, installed it, and ran a real Bedrock conversation using global.anthropic.claude-opus-4-8 (Global Inference Profile). Adaptive thinking active with xhigh effort, no 400 errors on temperature or thinking.type.enabled.
  • ✅ Regression-verified that global.anthropic.claude-opus-4-7 still works after the isGen47ModelisAdaptiveThinkingModel rename.

References

Summary by CodeRabbit

  • New Features

    • Added Claude Opus 4.8 (Anthropic/Bedrock/Vertex) and Claude Opus 4.7 to Bedrock; expanded model registries, 1M-context support, prompt-caching, and reasoning capability flags.
    • Implemented adaptive-thinking handling for Opus 4.7+ and updated behavior to omit temperature where required.
  • Tests

    • Added/extended tests for Opus 4.7/4.8: context windows, pricing, prompt-caching, adaptive thinking, reasoning modes, and E2E Bedrock smoke.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds claude-opus-4-8 model metadata across Anthropic, Bedrock, and Vertex; integrates the model into Anthropic prompt-caching and Vertex 1M-context paths; extends Bedrock to detect Opus/Sonnet 4.7+ and send adaptive-thinking payloads (omitting temperature); and adds tests covering model metadata, request construction, and token-handling.

Changes

Multi-Provider Model Support and Bedrock Adaptive Thinking

Layer / File(s) Summary
Model registries (Anthropic, Bedrock, Vertex)
packages/types/src/providers/anthropic.ts, packages/types/src/providers/bedrock.ts, packages/types/src/providers/vertex.ts
Added claude-opus-4-8 entries with token/context limits, pricing, capability flags, and updated 1M-context/Bedrock lists and extended-context tier pricing.
Anthropic handler integration & Vertex test
src/api/providers/anthropic.ts, src/api/providers/__tests__/anthropic-vertex.spec.ts
Integrated claude-opus-4-8 into Anthropic prompt-caching/beta-header flow; added Vertex getModel test asserting 1M-context beta behavior and supportsTemperature: false.
Anthropic createMessage/getModel tests
src/api/providers/__tests__/anthropic.spec.ts
Added createMessage and getModel tests for claude-opus-4-8 covering 1M-context header handling, adaptive-thinking vs omitted thinking, and modelMaxTokens preservation.
Bedrock adaptive-thinking support & tests
src/api/providers/bedrock.ts, src/api/providers/__tests__/bedrock.spec.ts
Expanded Bedrock thinking type to include adaptive; detect Opus/Sonnet 4.7/4.8 and build adaptive-thinking payloads (display: "summarized", output_config.effort: "xhigh") while omitting temperature; preserved legacy budget_tokens behavior for older models; added tests including regressions and cross-region handling.
Bedrock e2e tests (Opus 4.8)
apps/vscode-e2e/src/suite/providers/bedrock.test.ts
Added an end-to-end Bedrock smoke test for Opus 4.8 that runs in mock vs live modes and verifies request headers and completion.
Shared tests & docs clarifications
src/api/providers/openai.ts, src/shared/__tests__/api.spec.ts
Clarified OpenAI temperature omission comment to require supportsTemperature === false to omit; added Anthropic hybrid-token unit test for claude-opus-4-8.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant AnthropicHandler
  participant BedrockHandler
  participant VertexHandler
  Client->>AnthropicHandler: createMessage(claude-opus-4-8)
  AnthropicHandler->>AnthropicHandler: Apply ephemeral cache preprocessing
  AnthropicHandler->>AnthropicHandler: Add prompt-caching-2024-07-31 beta
  AnthropicHandler-->>Client: request built
  Client->>BedrockHandler: createMessage(anthropic.claude-opus-4-8, enableReasoning)
  BedrockHandler->>BedrockHandler: Detect adaptive-thinking model
  BedrockHandler->>BedrockHandler: thinking type: adaptive, output_config.effort: xhigh
  BedrockHandler->>BedrockHandler: Omit temperature
  BedrockHandler-->>Client: InvokeModel request
  Client->>VertexHandler: getModel(claude-opus-4-8, vertex1MContext)
  VertexHandler->>VertexHandler: Apply 1M-context beta header
  VertexHandler-->>Client: ModelInfo with 1M context window
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • Zoo-Code-Org/Zoo-Code#233: Related to OpenAI temperature handling and tests; aligns with the comment/behavior clarification here.

Suggested reviewers

  • taltas
  • hannesrudolph

Poem

🐰 I nibble on a fresh new spec,

Opus hops in, so spry and deft.
Bedrock, Anthropic, Vertex align,
A rabbit stamps the changelog fine. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is comprehensive and well-structured, covering the summary, what changed across each component, testing validation, backward compatibility, pricing notes, and references. However, the critical 'Related GitHub Issue' section (required by template) is missing—no issue number is linked in the 'Closes: #' field. Add the 'Related GitHub Issue' section with 'Closes: #<issue_number>' to link this PR to its corresponding approved GitHub Issue as required by the template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the main change: adding Claude Opus 4.8 support across three providers (Anthropic, Bedrock, Vertex). It is concise, specific, and clearly describes the primary feature being added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/api/providers/bedrock.ts`:
- Around line 448-452: The completePrompt flow still unconditionally injects
temperature into the Bedrock inferenceConfig causing 400s for adaptive-thinking
models; update the completePrompt implementation (the inferenceConfig
construction in the completePrompt function) to guard temperature the same way
createMessage does by checking isAdaptiveThinkingModel and omitting temperature
when true, using modelConfig.temperature or this.options.modelTemperature only
when not adaptive-thinking; ensure the same symbol names (completePrompt,
inferenceConfig, isAdaptiveThinkingModel, modelConfig) are used so behavior
matches createMessage.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 943f0c9b-4c4b-4eb2-a01c-9403cec98c02

📥 Commits

Reviewing files that changed from the base of the PR and between 6470431 and dfcfff8.

📒 Files selected for processing (10)
  • packages/types/src/providers/anthropic.ts
  • packages/types/src/providers/bedrock.ts
  • packages/types/src/providers/vertex.ts
  • src/api/providers/__tests__/anthropic-vertex.spec.ts
  • src/api/providers/__tests__/anthropic.spec.ts
  • src/api/providers/__tests__/bedrock.spec.ts
  • src/api/providers/anthropic.ts
  • src/api/providers/bedrock.ts
  • src/api/providers/openai.ts
  • src/shared/__tests__/api.spec.ts

Comment thread src/api/providers/bedrock.ts
@vandre-sales vandre-sales force-pushed the feat/bedrock-claude-opus-4-8 branch from dfcfff8 to e2d630a Compare May 28, 2026 23:00
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

vandre-sales added a commit to vandre-sales/Zoo-Code that referenced this pull request May 28, 2026
…g models

Addresses CodeRabbit review on Zoo-Code-Org#386. completePrompt was unconditionally
sending temperature in its inferenceConfig, which causes a 400 error for
Claude Opus/Sonnet 4.7 and 4.8 (sampling parameters were removed by
Anthropic for these models). createMessage already guarded this, but the
non-stream path did not.

- Extract the adaptive-thinking detection into a private
  isAdaptiveThinkingModel(modelId) method (parseBaseModelId-aware, so
  cross-region/global prefixes are handled).
- Reuse it in both createMessage and completePrompt so the two request
  paths stay consistent.
- Add two regression tests: completePrompt omits temperature for opus-4-8
  and still sends it for opus-4-6.

64 bedrock tests pass, check-types clean.
@edelauna
Copy link
Copy Markdown
Contributor

Nice! Thank you for your contribution. I recently added bedrock to our e2e test harness would you be able to add a smoke test there for 4.8?

@vandre-sales
Copy link
Copy Markdown
Contributor Author

Good catch by @coderabbitai — fixed in 2bd49d74f.

completePrompt was indeed sending temperature unconditionally, which would 400 on the adaptive-thinking models (Opus/Sonnet 4.7 and 4.8). createMessage already guarded it, but the non-stream path didn't.

What changed:

  • Extracted the detection into a private isAdaptiveThinkingModel(modelId) method that runs parseBaseModelId first, so cross-region/global prefixes (us. / eu. / global.) are handled correctly.
  • Reused it in both createMessage and completePrompt so the two request paths stay in sync (single source of truth instead of a duplicated inline check).
  • Added two regression tests on the non-stream path: completePrompt omits temperature for opus-4-8 and still sends it for opus-4-6.

Validation: 64 bedrock tests pass, check-types clean across all packages.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Tip

For best results, initiate chat on the files or code changes.

@vandre-sales Great work on the refactor! Extracting isAdaptiveThinkingModel as a shared private method is the right call — it ensures both paths stay consistent and removes the risk of them drifting out of sync again in the future. The cross-region prefix handling via parseBaseModelId in one place is a nice touch as well.

🐇 ✨


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

vandre-sales and others added 7 commits May 28, 2026 20:27
Adds Claude Opus 4.7 to the Bedrock native model registry with:
- Full ModelInfo (maxTokens, contextWindow, pricing, cache config)
- supportsReasoningBudget: true (enables thinking budget in UI)
- cachableFields for multi-point prompt caching
- 1M context tier pricing
- Global Inference support

Without this entry, custom model usage falls back to guessModelInfoFromId()
which lacks supportsReasoningBudget and cachableFields, causing "too many
tokens" errors during parallel file injection (no cache = tokens accumulate).

Note: Pricing estimated based on claude-opus-4-6-v1. To be verified against
Bedrock console pricing page before merge.
…rature

Claude Opus/Sonnet 4.7 introduced breaking API changes:
- temperature/top_p/top_k removed (causes 400 error)
- thinking.type 'enabled' + budget_tokens removed (causes 400 error)
- New thinking.type 'adaptive' with output_config.effort levels
- New display: 'summarized' option to surface thinking content

Changes:
- Detect Gen 4.7+ models via baseModelId.includes('opus-4-7' | 'sonnet-4-7')
- Omit temperature from inferenceConfig for 4.7+ models
- Use thinking: { type: 'adaptive', display: 'summarized' } for 4.7+
- Set output_config.effort: 'xhigh' (highest level for coding/agentic tasks)
- Maintain full backward compatibility with 4.6 and earlier models
- Expanded BedrockAdditionalModelFields interface to support both formats

References:
- Claude 4.7 release notes (Apr 16, 2026)
- effort levels: low | medium | high | xhigh | max
…gistries

- Register claude-opus-4-8 in anthropicModels with 1M context, 128k output,
  supportsReasoningBudget, supportsReasoningBinary, supportsTemperature: false
  (mirrors 4.7 - no breaking API changes per the official migration guide).
- Register anthropic.claude-opus-4-8 in bedrockModels with cache points,
  cachableFields, and 1M context tier pricing.
- Register claude-opus-4-8 in vertexModels with the same shape.
- Add anthropic.claude-opus-4-8 to BEDROCK_1M_CONTEXT_MODEL_IDS and
  BEDROCK_GLOBAL_INFERENCE_MODEL_IDS.
- Add claude-opus-4-8 to VERTEX_1M_CONTEXT_MODEL_IDS.
… detection)

- Anthropic provider: add claude-opus-4-8 to both prompt-caching switch
  statements so it gets the same handling as 4.7 (native 1M context, no beta
  header required).
- Bedrock provider: rename isGen47Model -> isAdaptiveThinkingModel and expand
  the pattern to match opus-4-7, opus-4-8, sonnet-4-7, sonnet-4-8. 4.8 inherits
  the same adaptive-thinking + temperature-rejection contract from 4.7 with no
  breaking API changes.
- OpenAI-compatible provider: update comment to mention 4.8 alongside 4.7; no
  logic change (already honors the supportsTemperature: false flag).

The rename describes the capability (adaptive thinking) rather than a specific
generation, making future Claude versions easier to support.
- anthropic.spec.ts: 5 cases mirroring 4.7 (1M-beta-header guard, adaptive
  thinking ON/OFF, custom maxTokens, getModel info).
- anthropic-vertex.spec.ts: 1M context tier pricing for Vertex Opus 4.8.
- shared/api.spec.ts: getModelMaxOutputTokens hybrid-token handling on 4.8.
- bedrock.spec.ts: new 'Claude 4.7+ adaptive thinking' block with 5 cases
  covering 4.7 + 4.8 adaptive thinking, reasoning-off behaviour, a 4.6
  regression guard (budget_tokens + temperature), and cross-region prefix
  detection (us.anthropic.claude-opus-4-8).

235 unit tests pass, 0 type errors. Validated live end-to-end via Bedrock
Global Inference (global.anthropic.claude-opus-4-8).
…g models

Addresses CodeRabbit review on Zoo-Code-Org#386. completePrompt was unconditionally
sending temperature in its inferenceConfig, which causes a 400 error for
Claude Opus/Sonnet 4.7 and 4.8 (sampling parameters were removed by
Anthropic for these models). createMessage already guarded this, but the
non-stream path did not.

- Extract the adaptive-thinking detection into a private
  isAdaptiveThinkingModel(modelId) method (parseBaseModelId-aware, so
  cross-region/global prefixes are handled).
- Reuse it in both createMessage and completePrompt so the two request
  paths stay consistent.
- Add two regression tests: completePrompt omits temperature for opus-4-8
  and still sends it for opus-4-6.

64 bedrock tests pass, check-types clean.
Addresses @edelauna's review request on Zoo-Code-Org#386 to cover 4.8 in the new
Bedrock e2e harness.

Mirrors the existing user-agent smoke test but re-points the provider at
us.anthropic.claude-opus-4-8. Since 4.8 is an adaptive-thinking model, this
exercises the request path that omits temperature (and sends thinking.type
"adaptive" when reasoning is enabled), proving a Bedrock round-trip
completes without a 400. Runs against the binary-event-stream mock server in
CI and against real AWS when BEDROCK_LIVE_E2E=true. The original 4.7-era
test is left untouched; model id is overridable via BEDROCK_OPUS_48_MODEL_ID.

check-types clean (tsconfig.esm.json).
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 98.58156% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/api/providers/bedrock.ts 93.10% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@vandre-sales vandre-sales force-pushed the feat/bedrock-claude-opus-4-8 branch from 2bd49d7 to 506bc9a Compare May 28, 2026 23:31
@vandre-sales
Copy link
Copy Markdown
Contributor Author

Done — added in 506bc9a72. Thanks for setting up the Bedrock e2e harness, @edelauna; it made this straightforward.

I rebased onto latest main to pick up your harness, then added a second test to apps/vscode-e2e/src/suite/providers/bedrock.test.ts that mirrors your user-agent smoke test but re-points the provider at us.anthropic.claude-opus-4-8. Since 4.8 is an adaptive-thinking model, this exercises the request path that omits temperature (and would send thinking.type: "adaptive" when reasoning is enabled), proving a Bedrock round-trip completes without a 400.

Details, keeping it consistent with your harness:

  • Runs against your binary-event-stream mock server in CI and against real AWS when BEDROCK_LIVE_E2E=true — same dual-mode setup as your test.
  • Model id is overridable via BEDROCK_OPUS_48_MODEL_ID (defaults to us.anthropic.claude-opus-4-8, the cross-region profile).
  • Your original test is left untouched.

I kept it as a parity/connectivity smoke test to match the harness's intent. If you'd prefer a deeper assertion — e.g. extending the mock server to capture the request body and asserting thinking: "adaptive" + absence of temperature on the wire — I'm happy to follow up; just let me know which direction you'd like.

check-types clean across all packages (tsconfig.esm.json).

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/api/providers/__tests__/bedrock.spec.ts (1)

76-82: 💤 Low value

Consider consistent naming: "ZooCode" instead of "Zoo Code".

The title uses "Zoo Code" (two words), but the assertion on line 90 checks for "ZooCode#" (one word). Consider using "ZooCode" throughout for consistency with the actual user-agent marker.

📝 Suggested title adjustment
-		it("should identify itself as Zoo Code in the AWS client app id", () => {
+		it("should identify itself as ZooCode in the AWS client app id", () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/providers/__tests__/bedrock.spec.ts` around lines 76 - 82, Update the
test title to use the same "ZooCode" spelling as the assertion: change the
it(...) description that currently reads "should identify itself as Zoo Code in
the AWS client app id" to use "ZooCode" (one word) so it matches the expectation
checked by expect.stringMatching(/^ZooCode#/), ensuring consistency between the
test description and the mockBedrockRuntimeClient assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/api/providers/__tests__/bedrock.spec.ts`:
- Around line 76-82: Update the test title to use the same "ZooCode" spelling as
the assertion: change the it(...) description that currently reads "should
identify itself as Zoo Code in the AWS client app id" to use "ZooCode" (one
word) so it matches the expectation checked by
expect.stringMatching(/^ZooCode#/), ensuring consistency between the test
description and the mockBedrockRuntimeClient assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b941adf2-e8c0-46a7-9c5e-f744d3e8de00

📥 Commits

Reviewing files that changed from the base of the PR and between 2bd49d7 and 506bc9a.

📒 Files selected for processing (11)
  • apps/vscode-e2e/src/suite/providers/bedrock.test.ts
  • packages/types/src/providers/anthropic.ts
  • packages/types/src/providers/bedrock.ts
  • packages/types/src/providers/vertex.ts
  • src/api/providers/__tests__/anthropic-vertex.spec.ts
  • src/api/providers/__tests__/anthropic.spec.ts
  • src/api/providers/__tests__/bedrock.spec.ts
  • src/api/providers/anthropic.ts
  • src/api/providers/bedrock.ts
  • src/api/providers/openai.ts
  • src/shared/__tests__/api.spec.ts
✅ Files skipped from review due to trivial changes (1)
  • src/api/providers/openai.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/api/providers/anthropic.ts
  • src/api/providers/tests/anthropic-vertex.spec.ts
  • src/shared/tests/api.spec.ts
  • packages/types/src/providers/anthropic.ts
  • src/api/providers/tests/anthropic.spec.ts
  • packages/types/src/providers/bedrock.ts
  • packages/types/src/providers/vertex.ts
  • src/api/providers/bedrock.ts

Codecov flagged the sonnet-4-7 and sonnet-4-8 branches of
isAdaptiveThinkingModel as uncovered — they have no Bedrock registry entry
yet (future-proof guards), so no existing test reached them.

Add a focused unit test that calls the private method directly (same pattern
the suite already uses for parseBaseModelId / getPrefixForRegion), covering:
- all four positive patterns: opus-4-7, opus-4-8, sonnet-4-7, sonnet-4-8
- cross-region / global prefixes (us./eu./global.) via parseBaseModelId
- negative cases: opus-4-6, sonnet-4-6, claude-3-5, nova

Brings patch coverage to 100%. 68 bedrock tests pass, check-types clean.
Copy link
Copy Markdown
Contributor

@edelauna edelauna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for adding the e2e tests 🔥

outputPrice: 25.0, // $25 per million output tokens (≤200K context)
cacheWritesPrice: 6.25, // $6.25 per million tokens
cacheReadsPrice: 0.5, // $0.50 per million tokens
supportsReasoningBudget: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing supportsReasoningBinary: true — the Anthropic provider entry has it (anthropic.ts:126). Without it, anthropic-vertex.ts resolves thinking via getModelParams → getAnthropicReasoning and emits { type: "enabled", budget_tokens: N }. Anthropic docs confirm type: "enabled" is not supported on Opus 4.7+ (returns 400).

Adding the flag alone won't fully fix it — anthropic-vertex.ts also needs to call getAnthropicProviderReasoning directly (like anthropic.ts:65) instead of relying on reasoning from getModelParams.

contextWindow: 200_000, // Default 200K, extendable to 1M with beta flag 'context-1m-2025-08-07'
supportsImages: true,
supportsPromptCache: true,
supportsReasoningBudget: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing supportsTemperature: false — without it ApiOptions.tsx:787 still shows the temperature slider for this model even though temperature is silently stripped by isAdaptiveThinkingModel.

contextWindow: 200_000, // Default 200K, extendable to 1M with beta flag 'context-1m-2025-08-07'
supportsImages: true,
supportsPromptCache: true,
supportsReasoningBudget: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for Opus 4.8 — supportsTemperature: false should be included here too.

@edelauna edelauna added this pull request to the merge queue May 29, 2026
Merged via the queue into Zoo-Code-Org:main with commit 5d0d6e7 May 29, 2026
10 checks passed
@edelauna edelauna mentioned this pull request May 29, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants