Skip to content
This repository was archived by the owner on Jun 3, 2026. It is now read-only.

Task 3.3: Implement Tool Result Status Auto-Detection for BedrockModelProvider Implementation#89

Merged
Unshure merged 6 commits into
mainfrom
agent-tasks/3.3
Oct 29, 2025
Merged

Task 3.3: Implement Tool Result Status Auto-Detection for BedrockModelProvider Implementation#89
Unshure merged 6 commits into
mainfrom
agent-tasks/3.3

Conversation

@afarntrog

Copy link
Copy Markdown
Contributor

Overview

Implements dynamic logic to automatically determine whether to include the status field in tool results based on the Bedrock model ID, matching the Python SDK's auto-detection behavior.

Resolves: #24

Changes

Interface and Configuration

  • Added includeToolResultStatus?: 'auto' | boolean to BedrockModelConfig interface
  • Added MODELS_INCLUDE_STATUS constant array with Claude model pattern
  • Default behavior is 'auto' mode which automatically detects based on model ID

Implementation

  • Implemented _shouldIncludeToolResultStatus() private method with:
    • Explicit true/false handling
    • Auto-detection logic checking model ID against patterns
    • Debug logging for auto-detection decisions
  • Updated _formatContentBlock() method to conditionally include status field based on detection result

Testing

  • Added 8 comprehensive unit tests covering:
    • Explicit true configuration (always includes status)
    • Explicit false configuration (never includes status)
    • 'auto' mode with Claude models (includes status)
    • 'auto' mode with non-Claude models (excludes status)
    • undefined (default) with Claude models (includes status)
    • undefined (default) with non-Claude models (excludes status)
    • Debug logging verification for auto-detection
  • All tests passing (45 total in bedrock.test.ts)

Implementation Notes

Model Pattern Matching

  • Uses substring matching with .includes() to detect Claude models
  • Pattern: 'anthropic.claude' matches all Claude model variants
  • Default model (global.anthropic.claude-sonnet-4-5-20250929-v1:0) correctly matches and includes status

Type Safety

  • Implemented conditional return statements to satisfy TypeScript's strict type checking
  • Avoids type assertion by having separate return paths for with/without status

Integration Tests

Integration tests for tool result formatting would require complex multi-turn tool invocation cycles. The comprehensive unit tests verify the formatting logic for all configuration modes and validate that the output matches the Bedrock API specification.

Exit Criteria

  • includeToolResultStatus config option supports 'auto' | boolean
  • ✅ Model pattern constants defined with TSDoc
  • ✅ Auto-detection logic implemented with proper typing
  • ✅ Tool result formatting uses auto-detection conditionally
  • ✅ Debug logging for auto-detection decisions
  • ✅ Unit tests cover all configuration modes (true, false, 'auto', undefined)
  • ✅ TSDoc documentation explains auto-detection behavior
  • ✅ Default value is 'auto' (via ?? 'auto' in the method)
  • ✅ All tests pass (169 passing, pre-existing failures unrelated to changes)
  • ✅ Linting and type checking pass for modified files

- Add includeToolResultStatus config option supporting 'auto' | boolean
- Implement auto-detection logic to check model ID patterns
- Add MODELS_INCLUDE_STATUS constant with Claude model pattern
- Update _formatContentBlock to conditionally include status field
- Add _shouldIncludeToolResultStatus private method with debug logging
- Add 8 comprehensive unit tests covering all configuration modes
- Default behavior is 'auto' mode which detects based on model ID

Resolves: #24
Comment thread src/models/bedrock.ts Outdated
@afarntrog

Copy link
Copy Markdown
Contributor Author

/strands address the comment #89 (comment)

@github-actions github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
Use cleaner single-return pattern with conditional property spreading
instead of separate if/else return statements for better readability.
@github-actions github-actions Bot removed the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
Comment thread src/models/__tests__/bedrock.test.ts Outdated
@afarntrog

Copy link
Copy Markdown
Contributor Author

/strands address the comment #89 (comment)

@github-actions github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
Removed duplicate test case that was testing non-Claude model behavior
in 'auto' mode. This is already covered by the undefined/default tests.
@github-actions github-actions Bot removed the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
Comment thread src/models/bedrock.ts Outdated
Comment thread src/models/bedrock.ts
@afarntrog

Copy link
Copy Markdown
Contributor Author

/strands address the following 2 comments:

  1. Task 3.3: Implement Tool Result Status Auto-Detection for BedrockModelProvider Implementation #89 (comment) and
  2. Task 3.3: Implement Tool Result Status Auto-Detection for BedrockModelProvider Implementation #89 (comment)
    For the second comment you need to do research and be sure to have support backing up your research - such as a link to the documentation

@github-actions github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
- Remove generic @see link from includeToolResultStatus config
- Add specific AWS API reference to MODELS_INCLUDE_STATUS constant
- Document that status field is only supported by Claude models per AWS docs

Reference: https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ToolResultBlock.html
@github-actions github-actions Bot removed the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
@afarntrog afarntrog requested a review from Unshure October 29, 2025 19:15
Comment thread src/models/__tests__/bedrock.test.ts Outdated
Comment thread src/models/bedrock.ts Outdated
@afarntrog

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
- Change console.log to console.debug for auto-detection logging
- Remove redundant 'follows auto logic for Claude models' test in undefined section
- Update debug logging tests to spy on console.debug instead of console.log
- Test count reduced from 44 to 43 tests (all passing)
@github-actions github-actions Bot removed the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
@afarntrog afarntrog requested a review from Unshure October 29, 2025 19:25
Comment thread src/models/__tests__/bedrock.test.ts Outdated
@afarntrog

Copy link
Copy Markdown
Contributor Author

/strands address this comment #89 (comment)

@github-actions github-actions Bot added the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
Remove debug logging test cases as they are overkill.
The console.debug calls still exist in the implementation for debugging purposes.
Test count reduced from 43 to 41 tests (all passing).
@github-actions github-actions Bot removed the strands-running <strands-managed> Whether or not an agent is currently running label Oct 29, 2025
@Unshure Unshure enabled auto-merge October 29, 2025 20:30
@Unshure Unshure disabled auto-merge October 29, 2025 20:34
@Unshure Unshure merged commit e23ecc6 into main Oct 29, 2025
4 of 11 checks passed
@Unshure Unshure deleted the agent-tasks/3.3 branch October 29, 2025 20:39
dbschmigelski pushed a commit to dbschmigelski/sdk-typescript that referenced this pull request Feb 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task 3.3: Implement Tool Result Status Auto-Detection for BedrockModelProvider

3 participants