feat: Add Converse component and improve output extraction#9822
feat: Add Converse component and improve output extraction#9822Cristhianzl wants to merge 7 commits into
Conversation
…_text function ♻️ (events.py): refactor _extract_output_text function to handle various input formats and lengths more gracefully ✨ (amazon_bedrock_converse.py): add Amazon Bedrock Converse component for improved conversation handling with modern Converse API 📝 (amazon_bedrock_model.py): update description to recommend Amazon Bedrock Converse for better compatibility and features
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds robust parsing to _extract_output_text for varied output shapes. Introduces a new Amazon Bedrock Converse component with configurable AWS/auth and generation params, including error handling and optional streaming. Expands unit tests for _extract_output_text (with duplicated blocks). Updates legacy Amazon Bedrock component description only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Component as AmazonBedrockConverseComponent
participant Importer as Dynamic Import
participant Model as ChatBedrockConverse
User->>Component: build_model()
Component->>Importer: import ChatBedrockConverse
alt package missing
Importer-->>Component: ImportError
Component-->>User: Raises helpful ImportError
else package available
Component->>Component: Assemble init_params (model, region, creds, endpoint, gen params)
Component->>Component: Aggregate additional_model_request_fields (if provided)
Component->>Model: new ChatBedrockConverse(**init_params)
alt init error
Model-->>Component: Exception (validation/Converse issue/other)
Component-->>User: Raise mapped, user-friendly error
else success
Model-->>Component: instance
Component-->>User: return model
end
end
sequenceDiagram
autonumber
participant Caller
participant Func as _extract_output_text
Caller->>Func: output
alt output is string
Func-->>Caller: output
else output is list
loop items
alt item is string
Func->>Func: append text
else item is dict with text
alt text present and not None
Func->>Func: append item.text
else
Func->>Func: skip/append ""
end
else item is tool_use / partial_json / index-only
Func->>Func: skip
else
Func->>Func: skip
end
end
Func-->>Caller: concatenated text (or "")
else other shapes
Func-->>Caller: ""
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (46.89%) is below the target coverage (55.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #9822 +/- ##
==========================================
- Coverage 22.86% 21.65% -1.22%
==========================================
Files 1086 1074 -12
Lines 39743 39671 -72
Branches 5420 5414 -6
==========================================
- Hits 9089 8590 -499
- Misses 30499 30937 +438
+ Partials 155 144 -11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/tests/unit/components/agents/test_agent_events.py (1)
548-733: Add missing unit tests for AmazonBedrockConverseComponentComponent present at src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py but no test found — add src/backend/tests/unit/components/amazon/test_amazon_bedrock_converse.py. Minimal skeleton:
import pytest from src.backend.tests.base import ComponentTestBaseWithoutClient from lfx.components.amazon.amazon_bedrock_converse import AmazonBedrockConverseComponent class TestAmazonBedrockConverse(ComponentTestBaseWithoutClient): @pytest.fixture def component_class(self): return AmazonBedrockConverseComponent @pytest.fixture def default_kwargs(self): return { "model_id": "anthropic.claude-3-haiku-20240307-v1:0", "region_name": "us-east-1", "aws_access_key_id": "dummy", "aws_secret_access_key": "dummy", } @pytest.fixture def file_names_mapping(self): return {} def test_build_model_initializes(self, component_class, default_kwargs, monkeypatch): import types class DummyModel: ... monkeypatch.setattr("lfx.components.amazon.amazon_bedrock_converse.boto3", types.SimpleNamespace(Session=lambda **_: object())) monkeypatch.setattr("lfx.components.amazon.amazon_bedrock_converse.ChatBedrockConverse", lambda **_: DummyModel()) comp = component_class(**default_kwargs) model = comp.build_model() assert isinstance(model, DummyModel)
🧹 Nitpick comments (9)
src/lfx/src/lfx/base/agents/events.py (2)
114-129: Coerce collected text to str to avoid TypeErrors during joinitem["text"] can be non-str (e.g., numbers); casting prevents runtime errors when joining.
Apply this diff:
- for item in output: + for item in output: if isinstance(item, str): text_parts.append(item) elif isinstance(item, dict): - if "text" in item and item["text"] is not None: - text_parts.append(item["text"]) + if "text" in item and item["text"] is not None: + text_parts.append(str(item["text"])) # Skip tool_use, index-only, and partial_json items elif ( item.get("type") == "tool_use" or "partial_json" in item or ("index" in item and len(item) == 1) ): continue return "".join(text_parts)
95-95: Accept tuples as well as lists for output sequencesSome providers yield tuples; supporting (list | tuple) keeps behavior consistent.
Apply this diff:
- if isinstance(output, list): + if isinstance(output, (list, tuple)):src/backend/tests/unit/components/agents/test_agent_events.py (2)
9-9: Avoid importing a private helper in tests_importing _extract_output_text creates tight coupling to internals. Prefer re-exporting it without underscore (e.g., extract_output_text) from events.py or testing behavior via public handlers.
548-733: Parametrize to reduce repetition and speed up testsSeveral cases differ only by input/expected. Parametrize to keep tests concise.
Example:
import pytest @pytest.mark.parametrize( "inp,expected", [ ("simple text output", "simple text output"), ([], ""), (["text content"], "text content"), ([{"text": "message content"}], "message content"), ([{"type": "tool_use"}], ""), ([{"partial_json": "{}"}], ""), ], ) def test_extract_output_text_parametrized(inp, expected): assert _extract_output_text(inp) == expectedsrc/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py (5)
121-127: Helpful dependency error; consider adding boto3 presence hint as well.If boto3 is missing, the constructor will likely fail later. Optional: pre-check and raise a clearer message, consistent with the legacy component.
try: from langchain_aws.chat_models.bedrock_converse import ChatBedrockConverse except ImportError as e: msg = "langchain_aws is not installed. Please install it with `pip install langchain_aws`." raise ImportError(msg) from e + try: + import boto3 # noqa: F401 + except ImportError as e: + raise ImportError("boto3 is not installed. Please install it with `pip install boto3`.") from e
158-174: Check kwarg name:additional_model_request_fieldsvsadditional_model_fields/model_kwargs.The wrong kwarg name will break initialization. Also, merging dicts is shallow; nested keys (e.g., inferenceConfig) may be overwritten unexpectedly.
If docs confirm the kwarg should be
additional_model_fields, apply:- if additional_model_request_fields: - init_params["additional_model_request_fields"] = additional_model_request_fields + if additional_model_request_fields: + init_params["additional_model_fields"] = additional_model_request_fieldsOptionally deep-merge dicts to avoid clobbering sibling keys.
175-195: Improve error mapping for constructor kwarg issues.Handle “unexpected keyword argument” explicitly to aid users tuning params.
except Exception as e: # Provide helpful error message with fallback suggestions error_details = str(e) if "validation error" in error_details.lower(): msg = ( f"ChatBedrockConverse validation error: {error_details}. " f"This may be due to incompatible parameters for model '{self.model_id}'. " f"Consider adjusting the model parameters or trying the legacy Amazon Bedrock component." ) + elif "unexpected keyword argument" in error_details.lower(): + msg = ( + f"Initialization error: {error_details}. One or more parameters are not supported " + f"by the installed langchain_aws version. Review the ChatBedrockConverse constructor " + f"and remove/rename unsupported kwargs." + ) elif "converse api" in error_details.lower(): msg = ( f"Converse API error: {error_details}. " f"The model '{self.model_id}' may not support the Converse API. " f"Try using the legacy Amazon Bedrock component instead." ) else: msg = f"Could not initialize ChatBedrockConverse: {error_details}"
69-74: Endpoint override is helpful; consider clarifying when it’s needed.Add note that most users should leave it blank to use the managed Bedrock endpoint for the selected region.
121-197: Optional: add a lightweight unit test to lock constructor args.Mock ChatBedrockConverse and assert it receives expected kwargs for a few input combinations (default creds, profile, explicit keys, disable_streaming, additional fields).
I can draft a pytest using mocker to assert the init kwargs without hitting AWS. Want me to open a follow-up PR with tests?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/tests/unit/components/agents/test_agent_events.py(2 hunks)src/lfx/src/lfx/base/agents/events.py(1 hunks)src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py(1 hunks)src/lfx/src/lfx/components/amazon/amazon_bedrock_model.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/backend/tests/unit/components/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/tests/unit/components/**/*.py: Mirror the component directory structure for unit tests in src/backend/tests/unit/components/
Use ComponentTestBaseWithClient or ComponentTestBaseWithoutClient as base classes for component unit tests
Provide file_names_mapping for backward compatibility in component tests
Create comprehensive unit tests for all new components
Files:
src/backend/tests/unit/components/agents/test_agent_events.py
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/tests/unit/components/agents/test_agent_events.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/components/agents/test_agent_events.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/unit/components/agents/test_agent_events.py
src/backend/**/components/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/unit/components/agents/test_agent_events.py
🧬 Code graph analysis (2)
src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py (1)
src/lfx/src/lfx/components/amazon/amazon_bedrock_model.py (1)
build_model(83-128)
src/backend/tests/unit/components/agents/test_agent_events.py (1)
src/lfx/src/lfx/base/agents/events.py (1)
_extract_output_text(88-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Test Starter Templates
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Update Starter Projects
🔇 Additional comments (8)
src/lfx/src/lfx/components/amazon/amazon_bedrock_model.py (1)
10-14: LGTM: clearer legacy guidance in descriptionGood call-out steering users to the Converse component; no behavioral changes introduced.
src/lfx/src/lfx/base/agents/events.py (1)
93-131: Robust multi-format extraction looks correctSkips tool_use/partial_json/index-only safely and concatenates valid text parts; graceful empty-string fallback.
src/backend/tests/unit/components/agents/test_agent_events.py (1)
548-733: Excellent coverage for _extract_output_text across real-world shapesThorough cases for strings, mixed segments, tool_use, partial_json, and index-only.
src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py (5)
8-18: Good new component surface and clear description.Naming, description, and beta flag look consistent with the legacy component and clarify migration.
62-68: Region default is fine; confirm AWS_REGIONS content matches Bedrock-supported regions.If the list includes regions where Bedrock Converse isn’t available, filter or annotate accordingly.
Would you like me to scan the constants and cross-check against AWS docs?
20-27: Incorrect — casing is consistent: AWS_MODEL_IDs
AWS_MODEL_IDs is defined at src/lfx/src/lfx/base/models/aws_constants.py:105 and is imported/used as AWS_MODEL_IDs in src/lfx/src/lfx/components/amazon/amazon_bedrock_model.py and src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py, so no import-time error from casing.
Also: amazon_bedrock_converse.py imports from langflow.base.models.aws_constants (not lfx.base) — confirm that import path is intentional.Likely an incorrect or invalid review comment.
19-27: Default model ID exists — no action required."anthropic.claude-3-5-sonnet-20241022-v2:0" is included in AWS_MODELS_DETAILED (src/lfx/src/lfx/base/models/aws_constants.py) and matches the default in src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py.
105-111: Incorrect — ChatBedrockConverse already acceptsdisable_streamingLangChain docs list
disable_streaming: bool | Literal['tool_calling']as a constructor parameter, so the suggested mapping tostreamingis unnecessary; keep the currentdisable_streamingusage.Likely an incorrect or invalid review comment.
| SecretStrInput( | ||
| name="aws_access_key_id", | ||
| display_name="AWS Access Key ID", | ||
| info="The access key for your AWS account. " | ||
| "Usually set in Python code as the environment variable 'AWS_ACCESS_KEY_ID'.", | ||
| value="AWS_ACCESS_KEY_ID", | ||
| required=True, | ||
| ), | ||
| SecretStrInput( | ||
| name="aws_secret_access_key", | ||
| display_name="AWS Secret Access Key", | ||
| info="The secret key for your AWS account. " | ||
| "Usually set in Python code as the environment variable 'AWS_SECRET_ACCESS_KEY'.", | ||
| value="AWS_SECRET_ACCESS_KEY", | ||
| required=True, | ||
| ), |
There was a problem hiding this comment.
Do not require or persist long‑lived AWS secrets; rely on the default provider chain.
Requiring static keys and storing them (default load_from_db=True) is a security risk and breaks common AWS setups (env vars, IAM roles, SSO/profile). Make these optional, advanced, and non-persistent.
Apply this diff:
- SecretStrInput(
+ SecretStrInput(
name="aws_access_key_id",
display_name="AWS Access Key ID",
- info="The access key for your AWS account. "
- "Usually set in Python code as the environment variable 'AWS_ACCESS_KEY_ID'.",
- value="AWS_ACCESS_KEY_ID",
- required=True,
+ info="Optional. Prefer the AWS default credential chain (env vars, EC2/ECS/SSO/profile).",
+ required=False,
+ advanced=True,
+ load_from_db=False,
),
- SecretStrInput(
+ SecretStrInput(
name="aws_secret_access_key",
display_name="AWS Secret Access Key",
- info="The secret key for your AWS account. "
- "Usually set in Python code as the environment variable 'AWS_SECRET_ACCESS_KEY'.",
- value="AWS_SECRET_ACCESS_KEY",
- required=True,
+ info="Optional. Only provide if not using the default credential chain.",
+ required=False,
+ advanced=True,
+ load_from_db=False,
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SecretStrInput( | |
| name="aws_access_key_id", | |
| display_name="AWS Access Key ID", | |
| info="The access key for your AWS account. " | |
| "Usually set in Python code as the environment variable 'AWS_ACCESS_KEY_ID'.", | |
| value="AWS_ACCESS_KEY_ID", | |
| required=True, | |
| ), | |
| SecretStrInput( | |
| name="aws_secret_access_key", | |
| display_name="AWS Secret Access Key", | |
| info="The secret key for your AWS account. " | |
| "Usually set in Python code as the environment variable 'AWS_SECRET_ACCESS_KEY'.", | |
| value="AWS_SECRET_ACCESS_KEY", | |
| required=True, | |
| ), | |
| SecretStrInput( | |
| name="aws_access_key_id", | |
| display_name="AWS Access Key ID", | |
| info="Optional. Prefer the AWS default credential chain (env vars, EC2/ECS/SSO/profile).", | |
| required=False, | |
| advanced=True, | |
| load_from_db=False, | |
| ), | |
| SecretStrInput( | |
| name="aws_secret_access_key", | |
| display_name="AWS Secret Access Key", | |
| info="Optional. Only provide if not using the default credential chain.", | |
| required=False, | |
| advanced=True, | |
| load_from_db=False, | |
| ), |
🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py around lines
28-43, the AWS access key and secret are currently required and persisted;
change them to optional, non-persistent advanced inputs: set required=False,
mark them as advanced (advanced=True), and disable persistence/load from DB
(load_from_db=False). Also remove or clear the hardcoded default values (use
None or an empty string) so the code relies on the default AWS provider chain
(env vars, IAM roles, SSO/profile) unless a user explicitly supplies them in
advanced settings.
| IntInput( | ||
| name="top_k", | ||
| display_name="Top K", | ||
| value=250, | ||
| info="Limits the number of highest probability vocabulary tokens to consider. " | ||
| "Note: Not all models support top_k. Use 'Additional Model Fields' for manual configuration if needed.", | ||
| advanced=True, | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
top_k input is never applied; user setting is silently ignored.
Either remove the input or wire it into the init params to avoid confusion.
Apply this minimal wiring (pending API confirmation; see separate verification comment):
if hasattr(self, "top_p") and self.top_p is not None:
init_params["top_p"] = self.top_p
+ if hasattr(self, "top_k") and self.top_k is not None:
+ init_params["top_k"] = self.top_kCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py around lines 97
to 104, the IntInput "top_k" is declared but never used so user changes are
ignored; either remove this input or wire it through to the model
invocation/init params where the request payload or client call is constructed
(i.e., add top_k to the parameters dict passed to the Bedrock converse/generate
method, converting to the expected key name and type), and guard it with the
model/API support check; if the API does not support top_k, remove the input to
avoid confusion.
| # Add AWS credentials if provided | ||
| if self.aws_access_key_id: | ||
| init_params["aws_access_key_id"] = self.aws_access_key_id | ||
| if self.aws_secret_access_key: | ||
| init_params["aws_secret_access_key"] = self.aws_secret_access_key | ||
| if self.aws_session_token: | ||
| init_params["aws_session_token"] = self.aws_session_token | ||
| if self.credentials_profile_name: | ||
| init_params["credentials_profile_name"] = self.credentials_profile_name | ||
| if self.endpoint_url: | ||
| init_params["endpoint_url"] = self.endpoint_url | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate credential combinations (all-or-none, and mutually exclusive with profile).
Prevents confusing AWS auth resolution and partial-key errors.
- # Add AWS credentials if provided
+ # Validate credentials: either both static keys, or profile, or neither (default chain)
+ if bool(self.aws_access_key_id) ^ bool(self.aws_secret_access_key):
+ raise ValueError(
+ "Provide both aws_access_key_id and aws_secret_access_key together, "
+ "or leave both blank to use the AWS default credential chain."
+ )
+ if self.credentials_profile_name and (self.aws_access_key_id or self.aws_secret_access_key or self.aws_session_token):
+ raise ValueError("Use either credentials_profile_name or explicit keys, not both.")
+ # Add AWS credentials if provided
if self.aws_access_key_id:
init_params["aws_access_key_id"] = self.aws_access_key_id
if self.aws_secret_access_key:
init_params["aws_secret_access_key"] = self.aws_secret_access_key📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Add AWS credentials if provided | |
| if self.aws_access_key_id: | |
| init_params["aws_access_key_id"] = self.aws_access_key_id | |
| if self.aws_secret_access_key: | |
| init_params["aws_secret_access_key"] = self.aws_secret_access_key | |
| if self.aws_session_token: | |
| init_params["aws_session_token"] = self.aws_session_token | |
| if self.credentials_profile_name: | |
| init_params["credentials_profile_name"] = self.credentials_profile_name | |
| if self.endpoint_url: | |
| init_params["endpoint_url"] = self.endpoint_url | |
| # Validate credentials: either both static keys, or profile, or neither (default chain) | |
| if bool(self.aws_access_key_id) ^ bool(self.aws_secret_access_key): | |
| raise ValueError( | |
| "Provide both aws_access_key_id and aws_secret_access_key together, " | |
| "or leave both blank to use the AWS default credential chain." | |
| ) | |
| if self.credentials_profile_name and (self.aws_access_key_id or self.aws_secret_access_key or self.aws_session_token): | |
| raise ValueError("Use either credentials_profile_name or explicit keys, not both.") | |
| # Add AWS credentials if provided | |
| if self.aws_access_key_id: | |
| init_params["aws_access_key_id"] = self.aws_access_key_id | |
| if self.aws_secret_access_key: | |
| init_params["aws_secret_access_key"] = self.aws_secret_access_key | |
| if self.aws_session_token: | |
| init_params["aws_session_token"] = self.aws_session_token | |
| if self.credentials_profile_name: | |
| init_params["credentials_profile_name"] = self.credentials_profile_name | |
| if self.endpoint_url: | |
| init_params["endpoint_url"] = self.endpoint_url |
🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py around lines 134
to 145, validate AWS credential combinations before populating init_params:
ensure that credentials_profile_name is mutually exclusive with any of
aws_access_key_id/aws_secret_access_key/aws_session_token (raise an error if
both profile and any key are provided), and enforce that if either
aws_access_key_id or aws_secret_access_key is provided the other must be present
(aws_session_token may be optional), otherwise raise a clear ValueError; perform
these checks early and only then add the validated keys/profile to init_params.
| # Add model parameters directly as supported by ChatBedrockConverse | ||
| if hasattr(self, "temperature") and self.temperature is not None: | ||
| init_params["temperature"] = self.temperature | ||
| if hasattr(self, "max_tokens") and self.max_tokens is not None: | ||
| init_params["max_tokens"] = self.max_tokens | ||
| if hasattr(self, "top_p") and self.top_p is not None: | ||
| init_params["top_p"] = self.top_p | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm parameter names for generation controls with ChatBedrockConverse.
temperature, max_tokens, top_p (and top_k if wired) must match the expected constructor args; otherwise you’ll hit “unexpected keyword argument.”
🌐 Web query:
Does `ChatBedrockConverse` accept `temperature`, `max_tokens`, `top_p`, and `top_k` directly as constructor kwargs? If not, what are the correct names?
💡 Result:
Short answer: No — while temperature and max_tokens are shown as top-level kwargs in the LangChain docs/example, top_p and top_k should be passed via model_kwargs. Also note Bedrock’s native name for the sampling-length param is often max_tokens_to_sample (provider-specific) even though LangChain examples may use max_tokens. Sources: LangChain ChatBedrockConverse docs (example showing temperature / max_tokens). [1] Community/Bedrock examples showing top_p, top_k and max_tokens_to_sample in model_kwargs. [2][3]
Sources
- LangChain ChatBedrockConverse docs/example. [1]
- LangChain/GitHub and community Bedrock examples showing top_p/top_k in model_kwargs and Bedrock’s max_tokens_to_sample usage. [2][3]
Don't pass top_p/top_k as top-level kwargs — move them into model_kwargs and verify the token param name.
ChatBedrockConverse accepts temperature and max_tokens as top-level kwargs, but top_p/top_k must be supplied via model_kwargs; Bedrock may expect max_tokens_to_sample instead of max_tokens — update src/lfx/src/lfx/components/amazon/amazon_bedrock_converse.py (lines 146–153).
…omplete(url_component.to_toolkit()) to handle asynchronous operation 🐛 (agent.py): Change CurrentDateComponent(**self.get_base_args()).to_toolkit() to await CurrentDateComponent(**self.get_base_args()).to_toolkit() to handle asynchronous operation 🐛 (component.py): Change def to_toolkit(self) -> list[Tool]: to async def to_toolkit(self) -> list[Tool]: to handle asynchronous operation and await for _get_tools() if it's a coroutine function.
…d names to snake_case for better compatibility with schema field names.
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
| assert sse_client._connection_params["headers"] == test_headers | ||
|
|
||
|
|
||
| class TestFieldNameConversion: |
There was a problem hiding this comment.
Should we have this change in in this PR?
github-actions
Bot
added
enhancement
|
github-actions
Bot
added
enhancement
|
please follow up this PR: #9901 |
2 participants
This pull request introduces significant improvements to Amazon Bedrock model support and output text extraction, with a focus on compatibility, robustness, and expanded testing. The most notable changes are the addition of a new Amazon Bedrock Converse component, a major rewrite of the
_extract_output_textfunction to handle diverse response formats gracefully, and comprehensive unit tests to ensure reliability across edge cases and backward compatibility.Amazon Bedrock Model Components:
AmazonBedrockConverseComponentinamazon_bedrock_converse.py, supporting the modern Converse API for improved conversation handling, more flexible model parameters, and robust error handling for unsupported models or parameters.AmazonBedrockComponentdescription to recommend the new Converse component for most users, clarifying its intended use and compatibility.Output Text Extraction Improvements:
_extract_output_textfunction inevents.pyto handle a wide variety of output formats, including ChatBedrockConverse responses, Anthropic-style outputs, and backward compatibility scenarios. The function now gracefully handles index-only items, tool use, partial JSON, mixed types, and multiple text segments, returning an empty string for non-text items.Testing Enhancements:
_extract_output_textintest_agent_events.py, covering all major formats, edge cases, and backward compatibility scenarios, including specific cases that previously caused errors with ChatBedrockConverse._extract_output_textfunction in the test file to enable these new tests.Summary by CodeRabbit