Fix text extraction for responses with reasoning content#33
Merged
sixlive merged 8 commits intoprism-php:mainfrom Oct 16, 2025
Merged
Fix text extraction for responses with reasoning content#33sixlive merged 8 commits intoprism-php:mainfrom
sixlive merged 8 commits intoprism-php:mainfrom
Conversation
Collaborator
|
I'd rather leave Bedrock recently announced closer feature parity with the Anthropic API, so will re-review whether that separation is still needed more widely separately. Will wait until #34 is merged with removal of responseMessages and rebase this then. |
- Fix ConverseTextHandler error message that incorrectly referenced 'Anthropic' - Standardize exception types to use PrismException instead of generic Exception - Add consistent provider prefixes to error messages for better debugging - Ensure all error messages correctly identify their respective handlers Fixes: - ConverseTextHandler: 'Anthropic: unknown finish reason' → 'Converse: unknown finish reason' - AnthropicMessageMap: Use PrismException with 'Anthropic:' prefix for consistency - ConverseMessageMap: Use PrismException with 'Converse:' prefix for consistency
This commit introduces a new trait `ExtractsText` to handle the extraction of text content from the Converse API response. The `ConverseTextHandler` is updated to utilize this trait, replacing the previous hardcoded path with a more robust extraction method. This ensures that the text content is correctly parsed, even when nested within reasoning content. A new test case is added to verify the functionality with reasoning content.
08ba230 to
6106fbe
Compare
ChrisB-TL
approved these changes
Oct 3, 2025
Contributor
|
@ChrisB-TL #34 has been merged so this may be ready. I am also trying to use the OpenAI models in Bedrock like |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #32.
Problem
The
ConverseTextHandlerwas hardcoded to extract text fromcontent[0].text, which fails when AWS Bedrock returns responses with reasoning content where the actual text is atcontent[1].text.Solution
data_get($data, 'output.message.content.0.text', '')with flexible$this->extractText($data)method.The handler now correctly processes both standard responses and responses with reasoning content. :)