- Notifications
You must be signed in to change notification settings - Fork6.9k
Open
Conversation
- Add _strip_thinking_tokens() helper to remove thinking tags from text- Apply to non-streaming _get_content_and_tool_calls() method- Apply to all streaming methods (stream_chat, astream_chat)- Add Literal import that was missing from upstream- Handles Amazon Nova models that include thinking tokens in response text- Add 7 unit tests for the strip functionFixesrun-llama#20489
…erse- Add _strip_thinking_tokens() static method to remove thinking tags- Re-introduce accidentally removed generic_utils imports- Apply fix to all streaming and non-streaming methods- Bump version to 0.12.10- Update tests to use actual method instead of duplicating logicFixesrun-llama#20489
- Add BEDROCK_STRUCTURED_OUTPUT_SUPPORTED_MODELS tuple with supported models- Add is_bedrock_structured_output_supported() function- Implement structured_predict() and astructured_predict() methods- Fall back to function calling for unsupported models- Add 4 unit tests for model support detectionFixesrun-llama#20703
7fbac0f to53f233fCompareAstraBert requested changesFeb 20, 2026
Member
AstraBert left a comment
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.
Before review, strip_thinking_tokens should be removed
Comment on lines +389 to +398
| @staticmethod | ||
| def _strip_thinking_tokens(text: str) -> str: | ||
| """ | ||
| Remove <thinking>...</thinking> tokens from text. | ||
| Some models (e.g., Amazon Nova) include thinking tokens in the response | ||
| text even when thinking is disabled. This helper removes them. | ||
| """ | ||
| return re.sub(r"<thinking>.*?</thinking>", "", text, flags=re.DOTALL).strip() | ||
Member
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.
These are unrelated changes, you prob need to rebase your PR on main instead of basing it off of the same branch where you proposed the strip thinking tokens changes
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 freeto 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
AWS Bedrock launched native structured outputs in February 2026 via the
outputConfig.textFormatparameter. This feature adds native support for structured outputs to thellama-index-llms-bedrock-converseintegration, replacing the function calling workaround for supported models.Changes:
BEDROCK_STRUCTURED_OUTPUT_SUPPORTED_MODELStuple with supported models (Claude 4.5+, Amazon Nova, DeepSeek, Mistral)is_bedrock_structured_output_supported()function with region prefix supportstructured_predict()andastructured_predict()methodsSupported Models:
Fixes#20703
New Package?
Version Bump?
Type of Change
How Has This Been Tested?
Suggested Checklist:
uv run make format; uv run make lintto appease the lint gods