Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jun 7, 2025

Description

Fixes #4343
Fixes #4201

This PR adds support for AWS Bedrock Extended Thinking (reasoning mode) for Claude 3.7/4 models, with improved error handling and retry logic.

Changes Made

1. Added Extended Thinking Support

  • Updated model definitions to include supportsReasoningBudget: true for:
    • Claude 3.7 Sonnet (anthropic.claude-3-7-sonnet-20250219-v1:0)
    • Claude 4 Sonnet (anthropic.claude-sonnet-4-20250514-v1:0)
    • Claude 4 Opus (anthropic.claude-opus-4-20250514-v1:0)

2. Implemented Reasoning Configuration

  • Extended thinking is disabled by default (as per AWS requirements)
  • Only enabled when user explicitly sets enableReasoningEffort: true
  • Added additionalModelRequestFields with thinking configuration when enabled
  • Uses correct anthropic_version: "bedrock-20250514"
  • Removes incompatible parameters (temperature, topP) when thinking is enabled

3. Enhanced Stream Processing

  • Added support for new event types:
    • thinking content blocks
    • thinking_delta events
    • signature_delta events
  • Properly yields reasoning chunks with type: "reasoning"

4. Improved Error Handling

  • Added retry logic with exponential backoff (up to 3 retries)
  • Added specific error type for thinking-related errors
  • Improved generic error message from "Unknown Error" to more descriptive text
  • Errors now properly parsed and displayed to users

5. Updated UI

  • Added ThinkingBudget component to Bedrock settings
  • Shows "Enable Reasoning Mode" toggle for supported models
  • Allows configuration of max thinking tokens (1024-80% of max output tokens)

Testing

  • All existing tests pass
  • Added comprehensive test suite for reasoning functionality
  • Manual testing completed:
    • Verified reasoning is disabled by default
    • Verified reasoning only enabled when explicitly requested
    • Tested error handling and retry logic
    • Confirmed UI shows reasoning toggle only for supported models

Verification of Acceptance Criteria

  • "Enable reasoning" option now available for Bedrock Claude models
  • Better error handling with retry logic (addresses @hannesrudolph's concern)
  • Improved error parsing and messages
  • Support for Claude 3.7/4 extended thinking via AWS Bedrock
  • Reasoning is opt-in only (disabled by default)

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • Documentation updated (if needed)
  • No breaking changes
  • Tests added for new functionality

Additional Context

This implementation follows the technical requirements where extended thinking/reasoning is disabled by default and only enabled when explicitly requested by the user. This maintains backward compatibility while adding the new functionality requested in the issues.


Important

Adds AWS Bedrock Extended Thinking support for Claude models with new configurations, error handling, and tests.

  • Behavior:
    • Adds support for AWS Bedrock Extended Thinking for Claude 3.7/4 models in bedrock.ts.
    • Extended thinking is disabled by default; enabled with enableReasoningEffort: true.
    • Removes incompatible parameters (temperature, topP) when thinking is enabled.
  • Stream Processing:
    • Handles new event types: thinking, thinking_delta, signature_delta in bedrock.ts.
    • Yields reasoning chunks with type: "reasoning".
  • Error Handling:
    • Adds retry logic with exponential backoff in createMessage() in bedrock.ts.
    • Introduces specific error type for thinking-related errors.
    • Improves error messages and parsing.
  • Testing:
    • Adds bedrock-reasoning.spec.ts for testing reasoning functionality.
    • Tests include default behavior, enabled reasoning, unsupported models, and event handling.

This description was created by Ellipsis for 13e272c. You can customize this summary. It will automatically update as commits are pushed.

@hannesrudolph hannesrudolph requested review from cte, jr and mrubens as code owners June 7, 2025 18:30
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 7, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 7, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

I can't actually test if this actually works but, I left some suggestions based on how the Anthropic provider works.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Draft / In Progress] in Roo Code Roadmap Jun 7, 2025
@daniel-lxs daniel-lxs marked this pull request as draft June 7, 2025 22:15
- Refactor Bedrock provider for clarity and type safety.
- Extract constants and helper methods.
- Update documentation.
- Rename and update reasoning tests to Vitest.
- Ensure all tests pass.
@hannesrudolph hannesrudolph moved this from PR [Draft / In Progress] to PR [Needs Prelim Review] in Roo Code Roadmap Jun 8, 2025
@hannesrudolph hannesrudolph marked this pull request as ready for review June 8, 2025 03:18
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. enhancement New feature or request and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 8, 2025
@hannesrudolph
Copy link
Collaborator Author

Review Issues Addressed ✅

Fixed the remaining type safety issue:

  • Added StreamChunk interface with proper type: 'reasoning' | 'text' typing
  • Updated all generator methods to use Generator<StreamChunk, void, unknown> instead of any

All other review feedback was already implemented. Tests pass, no lint errors.

- Add StreamChunk interface with proper type definitions
- Update all generator methods to use Generator<StreamChunk, void, unknown>
- Addresses review feedback for better type safety
- Replace custom StreamChunk interface with ApiStreamChunk import
- Update all generator method signatures to use ApiStreamChunk
- Fixes type compatibility issues with yield* delegation
- Resolves CI test failures related to type mismatches
@hannesrudolph
Copy link
Collaborator Author

Type Safety Fix Applied ✅

Fixed the CI test failures by resolving type compatibility issues:

  • Replaced custom StreamChunk interface with proper ApiStreamChunk import
  • Updated all generator method signatures to use ApiStreamChunk type
  • Resolved yield* delegation type mismatches between generator methods

All local tests pass and TypeScript compilation is clean. The CI failures should now be resolved.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jun 9, 2025
…bility

- Replace multiple handler methods with unified processStreamEvent()
- Simplify main stream processing loop from ~30 lines to 1 line
- Maintain all existing functionality and test coverage
- Address PR reviewer feedback for improved code organization

Addresses: Stream processing consolidation suggestion from PR review
@dosubot dosubot bot removed the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 9, 2025
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 9, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jun 9, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 9, 2025
@hannesrudolph
Copy link
Collaborator Author

this implementation is no good.

hannesrudolph added a commit that referenced this pull request Jun 11, 2025
- Refactor Bedrock provider for clarity and type safety.
- Extract constants and helper methods.
- Update documentation.
- Rename and update reasoning tests to Vitest.
- Ensure all tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

"Enable reasoning" option not available AWS Bedrock does not show or support Sonnet and Opus 4 Extended Thinking

3 participants