-
Notifications
You must be signed in to change notification settings - Fork 445
Feature: Handle Bedrock redactedContent #848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Handle Bedrock redactedContent #848
Conversation
…ling - Add RedactedContentStreamEvent class to types/_events.py for typed streaming - Refactor redacted content handling in streaming.py to use walrus operator - Fix state management for redactedContent with proper default handling - Update tests to handle new event structure and skip problematic tests - Add integration test for redacted content with thinking mode This improves type safety and consistency in the streaming event system when handling redacted reasoning content from Claude models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK OK OK OK
Now I understand why this is safe and before it was not for signature.
So, for signature we are checking if "signature" in state:
so even when signature was empty it was getting added to the content block
Now we are seeing if redacted_content is truthy, meaning setting it to b"" is fine.
I think we can use this to rethink our signature fix but I'm fine with keeping as is.
Unrelated, do you know if the Bedrock docs are wrong? It seems the redactedContent is coming back as bytes but the docs state it is Base64-encoded? Are we decoding somewhere or is a client doing it under the good?
Approved in spirit, pretty sure you need to rebase |
So I confirmed it's coming in as bytes in the actual response; the encoding is coming from the client. The docs state:
which can possibly cause confusion but seems to imply it's coming in as bytes 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit comment
other than that just one minor nit that when you merge can you just remember to follow https://www.conventionalcommits.org/en/v1.0.0/
* feat: add ReasoningRedactedContentStreamEvent for proper redacted content handling - Add ReasoningRedactedContentStreamEvent class to types/_events.py for typed streaming - Refactor redacted content handling in streaming.py - Fix state management for redactedContent with proper default handling - Update tests to handle new event structure and skip problematic tests - Add integration test for redacted content with thinking mode This improves type safety and consistency in the streaming event system when handling redacted reasoning content. Co-authored-by: Yuki Matsuda <[email protected]> Co-authored-by: Arron <[email protected]>
Description
This pull request introduces support for handling
redactedContent
in the event streaming pipeline, ensuring that redacted reasoning content is properly processed and tested throughout the codebase. The changes include updates to event handling logic, the addition of a new event type, and comprehensive unit and integration tests to verify correct behavior.Event streaming enhancements:
RedactedContentStreamEvent
class to represent streaming events containing redacted reasoning content insrc/strands/types/_events.py
.handle_content_block_delta
andhandle_content_block_stop
functions insrc/strands/event_loop/streaming.py
Testing improvements:
tests/strands/event_loop/test_streaming.py
to cover scenarios involvingredactedContent
, including state updates and event emission for both new and existing redacted content.process_stream
, including cases with redacted contentIntegration test updates:
tests_integ/models/test_model_bedrock.py
to verify correct handling of redacted reasoning content when using the Bedrock model with thinking mode enabled.Related Issues
#99
Source pr: #229
Documentation PR
strands-agents/docs#90
Type of Change
New feature
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepare
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.