refactor: remove rai.utils module #524
Conversation
feat: add config_path param to model initialization functionality
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/rai_core/rai/messages/artifacts.py (1)
25-42: 🛠️ Refactor suggestion
⚠️ Potential issueConsistent Use of
db_pathin Artifact Storage Functions.
In thestore_artifactsfunction, although adb_pathparameter is accepted and converted to a Path object, the file operations still use the hard-coded string"artifact_database.pkl". For consistency and flexibility, it is recommended to use thedb_pathvariable in all file operations.Below is a suggested diff to address this:
- with open("artifact_database.pkl", "wb") as file: - pickle.dump(artifact_database, file) - with open("artifact_database.pkl", "rb") as file: - artifact_database = pickle.load(file) - if tool_call_id not in artifact_database: - artifact_database[tool_call_id] = artifacts - else: - artifact_database[tool_call_id].extend(artifacts) - with open("artifact_database.pkl", "wb") as file: - pickle.dump(artifact_database, file) + with open(db_path, "wb") as file: + pickle.dump(artifact_database, file) + with open(db_path, "rb") as file: + artifact_database = pickle.load(file) + if tool_call_id not in artifact_database: + artifact_database[tool_call_id] = artifacts + else: + artifact_database[tool_call_id].extend(artifacts) + with open(db_path, "wb") as file: + pickle.dump(artifact_database, file)
🧹 Nitpick comments (6)
src/rai_core/rai/agents/runner.py (5)
29-33: Fix parameter name mismatch in docstringThe docstring describes a parameter named
agent(singular), but the function acceptsagents(plural).- agent: Agent instance implementing stop() method + agents: List of Agent instances implementing stop() method
57-60: Improve string formatting in log messageThe current string continuation is awkwardly formatted. Consider using triple quotes for multiline strings.
- logger.info( - "run_agents is an experimental function. \ - If you believe that your agents are not running properly, \ - please run them separately (in different processes)." - ) + logger.info( + """run_agents is an experimental function. + If you believe that your agents are not running properly, + please run them separately (in different processes).""" + )
78-81: Improve string formatting in log messageSimilar to the previous comment, consider using triple quotes for multiline strings.
- self.logger.info( - f"{self.__class__.__name__}.{self.run.__name__} is an experimental function. \ - If you believe that your agents are not running properly, \ - please run them separately (in different processes)." - ) + self.logger.info( + f"""{self.__class__.__name__}.{self.run.__name__} is an experimental function. + If you believe that your agents are not running properly, + please run them separately (in different processes).""" + )
93-97: Fix parameter name mismatch in docstringSame issue as in the standalone function - the docstring describes
agent(singular) but the method usesagents(plural).- agent: Agent instance implementing stop() method + agents: List of Agent instances implementing stop() method
89-111: Consider reducing code duplicationThe
wait_for_shutdownmethod duplicates the logic of the standalonewait_for_shutdownfunction. Consider refactoring to avoid duplication.You could either:
- Make the class method call the standalone function:
def wait_for_shutdown(self): """Blocks execution until shutdown signal (SIGINT/SIGTERM) is received. Note: Ensures graceful shutdown of the agents and ROS2 nodes on interrupt. Handles both SIGINT (Ctrl+C) and SIGTERM signals. """ wait_for_shutdown(self.agents)
- Or extract the common logic to a private utility function that both can use.
src/rai_core/rai/initialization/model_initialization.py (1)
188-191: Consider adding config_path parameter to get_tracing_callbacksThe
get_tracing_callbacksfunction still uses a hardcoded path viaload_config()without a parameter, unlike the other functions that were updated. Consider adding theconfig_pathparameter for consistency.def get_tracing_callbacks( - override_use_langfuse: bool = False, override_use_langsmith: bool = False + override_use_langfuse: bool = False, override_use_langsmith: bool = False, + config_path: Optional[str] = None ) -> List[BaseCallbackHandler]: - config = load_config() + config = load_config(config_path)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
README.md(1 hunks)docs/debugging_assistant.md(1 hunks)docs/developer_guide.md(1 hunks)docs/developer_guide/tools.md(3 hunks)docs/multimodal_messages.md(1 hunks)docs/tracing.md(1 hunks)examples/agents/react.py(2 hunks)examples/agriculture-demo.py(1 hunks)examples/debugging_assistant.py(1 hunks)examples/manipulation-demo.py(1 hunks)examples/rosbot-xl-demo.py(1 hunks)examples/s2s/conversational.py(1 hunks)src/rai_bench/rai_bench/examples/o3de_test_benchmark.py(1 hunks)src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py(1 hunks)src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py(1 hunks)src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py(1 hunks)src/rai_core/rai/__init__.py(1 hunks)src/rai_core/rai/agents/__init__.py(1 hunks)src/rai_core/rai/agents/langchain/runnables.py(1 hunks)src/rai_core/rai/agents/react_agent.py(1 hunks)src/rai_core/rai/agents/runner.py(1 hunks)src/rai_core/rai/agents/tool_runner.py(1 hunks)src/rai_core/rai/apps/high_level_api.py(1 hunks)src/rai_core/rai/apps/talk_to_docs.py(1 hunks)src/rai_core/rai/cli/rai_cli.py(1 hunks)src/rai_core/rai/communication/hri_connector.py(1 hunks)src/rai_core/rai/communication/ros2/__init__.py(1 hunks)src/rai_core/rai/communication/ros2/ros_logs.py(1 hunks)src/rai_core/rai/initialization/__init__.py(1 hunks)src/rai_core/rai/initialization/model_initialization.py(4 hunks)src/rai_core/rai/messages/__init__.py(2 hunks)src/rai_core/rai/messages/artifacts.py(2 hunks)src/rai_core/rai/messages/multimodal.py(1 hunks)src/rai_core/rai/tools/debugging_assistant.py(0 hunks)src/rai_core/rai/tools/ros2/__init__.py(2 hunks)src/rai_core/rai/tools/ros2/cli.py(2 hunks)src/rai_core/rai/tools/ros2/manipulation/custom.py(1 hunks)src/rai_core/rai/tools/ros2/navigation/nav2.py(1 hunks)src/rai_core/rai/utils/agents.py(0 hunks)src/rai_core/rai/utils/ros_executors.py(0 hunks)src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py(1 hunks)src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py(1 hunks)src/rai_sim/rai_sim/o3de/o3de_bridge.py(1 hunks)tests/communication/test_hri_message.py(1 hunks)
💤 Files with no reviewable changes (3)
- src/rai_core/rai/utils/agents.py
- src/rai_core/rai/tools/debugging_assistant.py
- src/rai_core/rai/utils/ros_executors.py
🧰 Additional context used
🧬 Code Graph Analysis (24)
src/rai_core/rai/apps/talk_to_docs.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_embeddings_model(160-185)
src/rai_core/rai/agents/react_agent.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_tracing_callbacks(188-220)
src/rai_core/rai/communication/hri_connector.py (1)
src/rai_core/rai/messages/multimodal.py (1)
MultimodalMessage(22-57)
examples/rosbot-xl-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_core/rai/agents/langchain/runnables.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_core/rai/communication/ros2/ros_logs.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py (2)
src/rai_core/rai/initialization/model_initialization.py (2)
get_llm_model(124-157)get_llm_model_config_and_vendor(108-121)src/rai_core/rai/agents/conversational_agent.py (1)
create_conversational_agent(52-84)
src/rai_core/rai/tools/ros2/navigation/nav2.py (1)
src/rai_core/rai/messages/artifacts.py (1)
MultimodalArtifact(20-22)
src/rai_bench/rai_bench/examples/o3de_test_benchmark.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_core/rai/communication/ros2/__init__.py (1)
src/rai_core/rai/communication/ros2/context.py (1)
ROS2Context(23-146)
src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
tests/communication/test_hri_message.py (1)
src/rai_core/rai/messages/multimodal.py (1)
MultimodalMessage(22-57)
src/rai_core/rai/apps/high_level_api.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_tracing_callbacks(188-220)
src/rai_sim/rai_sim/o3de/o3de_bridge.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
examples/agriculture-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_core/rai/cli/rai_cli.py (3)
src/rai_core/rai/initialization/model_initialization.py (2)
get_embeddings_model(160-185)get_llm_model(124-157)src/rai_core/rai/messages/multimodal.py (1)
HumanMultimodalMessage(60-81)src/rai_core/rai/messages/conversion.py (1)
preprocess_image(24-48)
src/rai_core/rai/tools/ros2/manipulation/custom.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
examples/manipulation-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py (1)
src/rai_core/rai/messages/multimodal.py (1)
HumanMultimodalMessage(60-81)
src/rai_core/rai/agents/tool_runner.py (2)
src/rai_core/rai/messages/artifacts.py (2)
MultimodalArtifact(20-22)store_artifacts(25-41)src/rai_core/rai/messages/multimodal.py (1)
ToolMultimodalMessage(88-129)
src/rai_core/rai/agents/runner.py (3)
src/rai_core/rai/agents/base.py (1)
BaseAgent(19-29)examples/s2s/conversational.py (2)
stop(144-149)run(83-86)src/rai_core/rai/agents/react_agent.py (2)
stop(78-84)run(53-57)
src/rai_core/rai/initialization/__init__.py (1)
src/rai_core/rai/initialization/model_initialization.py (4)
get_embeddings_model(160-185)get_llm_model(124-157)get_llm_model_config_and_vendor(108-121)get_tracing_callbacks(188-220)
🔇 Additional comments (54)
src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py (1)
28-28: Import path updated correctly.The import for
get_future_resulthas been updated fromrai.utils.ros_asynctorai.communication.ros2.ros_async, aligning with the module restructuring effort.src/rai_core/rai/tools/ros2/manipulation/custom.py (1)
23-23: Import path updated correctly.The import for
get_future_resulthas been updated fromrai.utils.ros_asynctorai.communication.ros2.ros_async, maintaining consistency with the refactoring initiative.src/rai_core/rai/communication/ros2/ros_logs.py (1)
35-35: Import path updated correctly.The import for
get_future_resulthas been updated fromrai.utils.ros_asynctorai.communication.ros2.ros_async, which properly aligns with the module restructuring.src/rai_sim/rai_sim/o3de/o3de_bridge.py (1)
27-27: Import path updated correctly.The import for
get_future_resulthas been updated fromrai.utils.ros_asynctorai.communication.ros2.ros_async, consistent with the module restructuring effort.src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py (1)
23-23: Import path updated correctly.The import for
get_future_resulthas been updated fromrai.utils.ros_asynctorai.communication.ros2.ros_async, properly aligning with the module restructuring effort.src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py (1)
22-22: Import path updated correctly.The import statement has been updated to reflect the new module structure, importing
get_tracing_callbacksfromrai.initializationinstead of the previousrai.utils.model_initialization. This change is consistent with the broader refactoring effort to better organize utility functions.src/rai_core/rai/agents/langchain/runnables.py (1)
26-26: Import path updated correctly.The import statement has been updated to use
rai.initializationinstead ofrai.utils.model_initializationfor theget_llm_modelfunction. This change aligns with the module restructuring to improve code organization without changing functionality.src/rai_core/rai/agents/react_agent.py (1)
27-27: Import path updated correctly.The import statement has been updated to use
rai.initializationinstead ofrai.utils.model_initializationfor theget_tracing_callbacksfunction. This change aligns with the module restructuring effort, preserving functionality while improving code organization.src/rai_core/rai/apps/talk_to_docs.py (1)
33-33: Import path updated correctly.The import statement has been updated to use
rai.initializationinstead ofrai.utils.model_initializationfor theget_embeddings_modelfunction. This change is consistent with the broader refactoring effort to better organize utility functions.examples/rosbot-xl-demo.py (1)
20-20: Import from top-level package correctly updated.The import statement has been updated to import
get_llm_modeldirectly from the top-levelraipackage rather than fromrai.utils.model_initialization. This suggests thatget_llm_modelis now being re-exported from the top-level package, which improves API usability and developer experience.src/rai_core/rai/tools/ros2/navigation/nav2.py (1)
31-31: Import path changed for MultimodalArtifact.The import path for
MultimodalArtifacthas been updated to import fromrai.messagesinstead of the previousrai.messages.multimodallocation. This aligns with the PR objective to restructure utility modules into more appropriate folders.examples/s2s/conversational.py (1)
26-26: Updated import path for get_llm_model.The import for
get_llm_modelnow comes directly fromraiinstead ofrai.utils.model_initialization. This change is consistent with the PR's goal of reorganizing utility functions by functionality.src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py (1)
28-28: Updated import path for HumanMultimodalMessage.The import path for
HumanMultimodalMessagehas been updated to import fromrai.messagesinstead of the previous location inrai.messages.multimodal. This change is part of the broader refactoring effort to reorganize the codebase structure.examples/agriculture-demo.py (1)
20-20: Updated import path for get_llm_model.The import for
get_llm_modelnow comes directly fromraiinstead ofrai.utils.model_initialization. This change is consistent with the PR's refactoring goal to improve organization and usability.src/rai_core/rai/apps/high_level_api.py (1)
21-21: Updated import path for get_llm_model.The import path for
get_llm_modelhas been changed torai.initializationfrom the previousrai.utils.model_initialization. This reflects the reorganization effort where model initialization functionality has been moved to a dedicated module.src/rai_bench/rai_bench/examples/o3de_test_benchmark.py (1)
26-26: Refactor Import for get_llm_model
The import on line 26 has been updated to usefrom rai.initialization import get_llm_modelwhich aligns with the broader refactoring. This ensures consistency across the codebase by consolidating model initialization functions in the new module structure.src/rai_core/rai/communication/hri_connector.py (1)
28-28: Update Import for MultimodalMessage
The import on line 28 now usesfrom rai.messages import MultimodalMessage as RAIMultimodalMessageto leverage the updated public API. This change is consistent with the refactoring that exposedMultimodalMessageat the package level.docs/developer_guide.md (1)
135-135: Update get_llm_model Import Path in Documentation
The documentation now showsfrom rai import get_llm_model, reflecting the refactored module structure. This update improves clarity and ensures that developers are directed to the correct, unified API entry point.examples/manipulation-demo.py (1)
19-19: Refactor get_llm_model Import for Consistency
The modified import on line 19 now usesfrom rai import get_llm_model, which aligns with the restructuring of the public API. This change helps maintain consistency across all example scripts.docs/debugging_assistant.md (1)
25-25: Update Debugging Assistant Launch Command
The launch command on line 25 has been updated tostreamlit run examples/debugging_assistant.py. This change correctly reflects the new location of the ROS 2 Debugging Assistant script following the removal of the old module, thereby ensuring that users run the supported, up-to-date implementation.tests/communication/test_hri_message.py (1)
22-22: Updated Import for MultimodalMessage
The import statement now pullsMultimodalMessagedirectly fromrai.messages(aliased asRAIMultimodalMessage), which aligns with the new module structure and public API adjustments. This change improves consistency across the codebase.README.md (1)
121-122: Corrected Configurator Script Path
The command to run the configurator now correctly points tosrc/rai_core/rai/frontend/configurator.py, reflecting the reorganization of tools and the removal of legacy utilities. This update enhances clarity and usability for developers setting up the project.docs/multimodal_messages.md (1)
20-22: Simplified Import for get_llm_model
Changing the import from a deep path withinrai.utils.model_initializationto the top-levelfrom rai import get_llm_modelstreamlines usage and reinforces the refactoring effort. This makes the API easier to consume for end users and aligns with the broader codebase reorganization.src/rai_core/rai/agents/tool_runner.py (1)
31-31: Updated Artifact and Message Imports
The updated import statement now brings inMultimodalArtifact,ToolMultimodalMessage, andstore_artifactsfrom the consolidatedrai.messagesmodule. This change reflects the refactoring to encapsulate artifact-related utilities and message classes under a single, clearer public API.docs/tracing.md (1)
43-43: Updated Tracing Callbacks Import
Importingget_tracing_callbacksdirectly fromraisimplifies the documentation and aligns with the refactored module structure. This update makes it easier for users to follow the tracing configuration instructions without having to navigate deep module paths.src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py (1)
19-23: Refactored Import Statements Updated Correctly.
The new grouped import fromraifor bothget_llm_modelandget_llm_model_config_and_vendorhelps simplify the dependency graph and aligns with the overall module reorganization. Separating outcreate_conversational_agentmaintains clarity in the API structure.src/rai_core/rai/communication/ros2/__init__.py (1)
19-23: ExposingROS2Contextin the Public API.
By importingROS2Contextfrom.contextand including it in the__all__list, the refactor correctly makes this context manager accessible to external users. This change supports the updated documentation and example usage for ROS 2 integration.src/rai_core/rai/cli/rai_cli.py (1)
27-29: Updated Import Paths for Initialization and Messaging.
The new import statements forget_embeddings_modelandget_llm_modelfromrai.initialization, as well as forHumanMultimodalMessageandpreprocess_imagefromrai.messages, reflect the intended reorganization. This refactoring improves modularity and simplifies the overall import structure.src/rai_core/rai/agents/__init__.py (1)
17-28: Introducing Agent Lifecycle Management Interfaces.
The addition ofAgentRunnerandwait_for_shutdownfromrai.agents.runner—along with their inclusion in the public API (__all__)—properly centralizes agent execution and graceful shutdown functionality. This change is consistent with the broader refactoring objectives and enhances clarity for consumers of the agent framework.src/rai_core/rai/messages/artifacts.py (1)
20-22: New Artifact Type Definition Added.
The introduction of theMultimodalArtifactTypedDict improves type clarity by explicitly defining the expected structure for multimodal artifacts (i.e. lists of base64-encoded images and audio strings). This enhancement aids in static type checking and better documents the artifact schema.examples/agents/react.py (2)
15-16: Import updates correctly reflect the reorganized module structure.The imports now use the new module structure with
AgentRunnerfromrai.agentsandROS2Contextfromrai.communication.ros2, which aligns with the utility module refactoring objectives.
28-29: Agent lifecycle management updated to use the new abstraction.The implementation now uses the
AgentRunnerclass to manage the agent's lifecycle, which centralizes execution and shutdown handling. This is a good improvement over the previous approach that used separateagent.run()andwait_for_shutdown([agent])calls.src/rai_core/rai/__init__.py (2)
15-21: Well-structured public API exports.The explicit imports from submodules create a clear public API for the
raipackage. This approach makes it easier for users to discover and use the key components without having to know their exact location in the submodule hierarchy.
23-30: Comprehensive__all__declaration.The
__all__list properly includes all the imported entities, ensuring they're available when users import from theraipackage. This is a good practice for controlling what gets exported from a module.src/rai_core/rai/tools/ros2/cli.py (2)
19-19: Import expanded to include necessary base classes.The import now includes
BaseToolandBaseToolkitalongside the previously importedtooldecorator, which is necessary for the new toolkit class implementation.
55-65: New toolkit class improves modularity and reusability.The
ROS2CLIToolkitclass encapsulates the existing ROS 2 CLI tool functions, making them easier to use as a cohesive set. This implementation follows theBaseToolkitpattern and provides a clean way to instantiate all ROS 2 CLI tools at once.docs/developer_guide/tools.md (3)
198-199: Documentation imports updated to match refactored code structure.The import statements correctly reflect the new module structure, importing
ROS2Contextfromrai.communication.ros2andAgentRunnerfromrai.
210-211: Example code updated to use the new agent lifecycle management pattern.The documentation now demonstrates the correct usage of the
AgentRunnerclass, which improves consistency between the code and documentation.
225-225: Second example also updated to use the correct import path.The import of
ROS2Contexthas been properly updated to maintain consistency across all examples in the documentation.src/rai_core/rai/tools/ros2/__init__.py (2)
15-23: Well-structured import of ROS2 CLI tools.Good addition of the CLI toolkit and related functions from the
.clisubmodule, providing a comprehensive set of ROS2 CLI tools. This aligns with the refactoring goals of improving organization and usability.
79-90: Proper exposure of ROS2 CLI tools in the module's public API.The additions to
__all__correctly expose the newly imported CLI toolkit and functions, making them accessible to users of this module. This change maintains consistency with the module's existing public API pattern.src/rai_core/rai/initialization/__init__.py (2)
15-20: Appropriate reorganization of model initialization imports.The refactoring improves module organization by consolidating model initialization functions from the previous utils module. These imports align with the PR objective to split utilities into functionally related modules.
22-27: Clear public API definition for initialization module.The
__all__list properly exposes the imported model initialization functions, creating a focused public API for this module. This is consistent with the refactoring goal of improving code organization and usability.examples/debugging_assistant.py (4)
15-20: Well-structured imports using the refactored module paths.The imports correctly use the refactored paths (
raiinstead ofrai.utils), demonstrating the improved usability from the refactoring. Good use of the newly exposedROS2CLIToolkitfrom the tools module.
22-33: Effective agent initialization with caching.Good use of Streamlit's caching mechanism to optimize performance. The agent is properly configured with the complex LLM model and ROS2 CLI tools. The system prompt clearly defines the agent's role as a ROS2 expert.
35-46: Clean Streamlit app configuration and execution.The Streamlit app is well-configured with appropriate page title and icon. The
run_streamlit_appfunction properly initializes the app with the cached agent and sets an informative initial message for users.
49-50: Standard main function guard.Following the Python best practice of using the
if __name__ == "__main__"guard to ensure the app only runs when the script is executed directly.src/rai_core/rai/messages/multimodal.py (1)
16-16: Simplified import list for better clarity.The import statement has been simplified to include only the types actually used in this module. This aligns with the refactoring goal of improving code organization and removing unnecessary dependencies.
src/rai_core/rai/messages/__init__.py (3)
16-16: Updates imported artifact functions correctlyGood job reorganizing artifact-related functionality from
.multimodalto.artifactsand adding new artifact management functions to the public API.
21-21: Export of MultimodalMessage in public APIExcellent addition - exposing
MultimodalMessagedirectly in the public API improves usability and aligns with how it's used in other modules.
30-35: Updated all list for public APIThe
__all__list has been correctly updated to match the new imports, maintaining API consistency.src/rai_core/rai/agents/runner.py (1)
1-116: Great implementation of agent lifecycle managementThis new module effectively centralizes agent lifecycle management, providing both functional and object-oriented interfaces for running agents and handling graceful shutdown. The signal handling implementation is solid and the warning about experimental status is appropriate.
src/rai_core/rai/initialization/model_initialization.py (2)
88-94: Enhanced config loading flexibilityGood enhancement allowing configuration to be loaded from a specified path. The implementation correctly falls back to the default "config.toml" when no path is provided.
111-113: Consistent propagation of config_path parameterGood job consistently propagating the
config_pathparameter through all the model initialization functions and their calls to each other.Also applies to: 127-132, 160-161
|
@coderabbitai full review |
WalkthroughThis update restructures several internal modules, focusing on consolidating public APIs, refactoring imports, and improving agent lifecycle management. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StreamlitApp
participant Agent (ConversationalAgent)
participant ROS2CLIToolkit
participant LLM
User->>StreamlitApp: Submit message via chat input
StreamlitApp->>Agent: Pass message history
Agent->>ROS2CLIToolkit: Query ROS 2 system (as needed)
ROS2CLIToolkit-->>Agent: Return ROS 2 info
Agent->>LLM: Generate response
LLM-->>Agent: Response (may include tool calls)
Agent->>StreamlitApp: Stream response (with tool outputs)
StreamlitApp->>User: Display assistant reply and tool call history
sequenceDiagram
participant Main
participant AgentRunner
participant Agent(s)
participant OS
Main->>AgentRunner: Instantiate with list of agents
AgentRunner->>Agent: Start agent.run() (background)
Note over AgentRunner,Agent: Agents run concurrently
OS-->>AgentRunner: SIGINT/SIGTERM signal
AgentRunner->>Agent: Call stop() on each agent
Agent->>AgentRunner: Confirm stopped
AgentRunner->>Main: Shutdown complete
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
examples/debugging_assistant.py (2)
24-32: Consider adding error handling for model initialization.The current implementation doesn't handle potential exceptions from
get_llm_modelorcreate_conversational_agent. Consider adding try-except blocks to gracefully handle initialization failures, especially since this is a user-facing application.@st.cache_resource def initialize_agent(): + try: llm = get_llm_model(model_type="complex_model", streaming=True) agent = create_conversational_agent( llm, ROS2CLIToolkit().get_tools(), system_prompt="""You are a ROS 2 expert helping a user with their ROS 2 questions. You have access to various tools that allow you to query the ROS 2 system. Be proactive and use the tools to answer questions. Retrieve as much information from the ROS 2 system as possible. """, ) return agent + except Exception as e: + st.error(f"Failed to initialize agent: {str(e)}") + return None
41-47: Add null check for the agent in main function.If the agent initialization fails (especially with the suggested error handling above), the current implementation would raise an exception. Consider adding a null check to handle this case gracefully.
def main(): + agent = initialize_agent() + if agent is None: + st.error("Could not initialize the ROS 2 debugging assistant. Please check your configuration.") + return + run_streamlit_app( - initialize_agent(), + agent, page_title="ROS 2 Debugging Assistant", initial_message="Hi! I am a ROS 2 assistant. How can I help you?", )src/rai_core/rai/agents/runner.py (5)
29-29: Fix docstring parameter name mismatch.The docstring mentions
agentas the parameter, but the function takesagents(plural). This should be updated for consistency.Args: - agent: Agent instance implementing stop() method + agents: List of agent instances implementing stop() method
50-62: Consider adding return type annotation.The
run_agentsfunction is missing a return type annotation. Adding-> Nonewould make the function signature more complete.-def run_agents(agents: List[BaseAgent]): +def run_agents(agents: List[BaseAgent]) -> None:
65-71: Class docstring doesn't match constructor parameters.The class-level docstring is identical to the
run_agentsfunction docstring, but it should describe the class itself rather than just one method. It also doesn't match the actual constructor parameter format.class AgentRunner: - """Runs the agents in the background. - - Args: - agents: List of agent instances - """ + """Manages the lifecycle of multiple agent instances. + + Provides methods to run agents concurrently, wait for shutdown signals, + and ensure graceful shutdown of all agents. + """
89-98: Fix docstring parameter name mismatch in wait_for_shutdown method.Similar to the function version, the method's docstring mentions
agent(singular) while the method operates onself.agents(plural)."""Blocks execution until shutdown signal (SIGINT/SIGTERM) is received. Args: - agent: Agent instance implementing stop() method + (No parameters required as it operates on self.agents) Note: Ensures graceful shutdown of the agent and ROS2 node on interrupt. Handles both SIGINT (Ctrl+C) and SIGTERM signals. """
113-115: Add return type annotation to stop method.The
stopmethod is missing a return type annotation. Adding-> Nonewould make the method signature more complete.- def stop(self): + def stop(self) -> None: for agent in self.agents: agent.stop()src/rai_core/rai/initialization/model_initialization.py (2)
90-94: Add file not found error handling.The current implementation will raise a FileNotFoundError if the config file doesn't exist. Consider adding explicit error handling to provide a more user-friendly message.
def load_config(config_path: Optional[str] = None) -> RAIConfig: - if config_path is None: - with open("config.toml", "rb") as f: - config_dict = tomli.load(f) - else: - with open(config_path, "rb") as f: - config_dict = tomli.load(f) + try: + if config_path is None: + path = "config.toml" + else: + path = config_path + + with open(path, "rb") as f: + config_dict = tomli.load(f) + except FileNotFoundError: + raise FileNotFoundError(f"Config file not found at: {path}. Please ensure the file exists.") + except tomli.TOMLDecodeError: + raise ValueError(f"Invalid TOML format in config file: {path}. Please check the file syntax.")
188-191: Add config_path parameter to get_tracing_callbacks for consistency.Other functions in the module have been updated to accept a
config_pathparameter, butget_tracing_callbacksstill uses the default config path. Consider updating it for consistency.def get_tracing_callbacks( - override_use_langfuse: bool = False, override_use_langsmith: bool = False + override_use_langfuse: bool = False, + override_use_langsmith: bool = False, + config_path: Optional[str] = None ) -> List[BaseCallbackHandler]: - config = load_config() + config = load_config(config_path)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (44)
README.md(1 hunks)docs/debugging_assistant.md(1 hunks)docs/developer_guide.md(1 hunks)docs/developer_guide/tools.md(3 hunks)docs/multimodal_messages.md(1 hunks)docs/tracing.md(1 hunks)examples/agents/react.py(2 hunks)examples/agriculture-demo.py(1 hunks)examples/debugging_assistant.py(1 hunks)examples/manipulation-demo.py(1 hunks)examples/rosbot-xl-demo.py(1 hunks)examples/s2s/conversational.py(1 hunks)src/rai_bench/rai_bench/examples/o3de_test_benchmark.py(1 hunks)src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py(1 hunks)src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py(1 hunks)src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py(1 hunks)src/rai_core/rai/__init__.py(1 hunks)src/rai_core/rai/agents/__init__.py(1 hunks)src/rai_core/rai/agents/langchain/runnables.py(1 hunks)src/rai_core/rai/agents/react_agent.py(1 hunks)src/rai_core/rai/agents/runner.py(1 hunks)src/rai_core/rai/agents/tool_runner.py(1 hunks)src/rai_core/rai/apps/high_level_api.py(1 hunks)src/rai_core/rai/apps/talk_to_docs.py(1 hunks)src/rai_core/rai/cli/rai_cli.py(1 hunks)src/rai_core/rai/communication/hri_connector.py(1 hunks)src/rai_core/rai/communication/ros2/__init__.py(1 hunks)src/rai_core/rai/communication/ros2/ros_logs.py(1 hunks)src/rai_core/rai/initialization/__init__.py(1 hunks)src/rai_core/rai/initialization/model_initialization.py(4 hunks)src/rai_core/rai/messages/__init__.py(2 hunks)src/rai_core/rai/messages/artifacts.py(2 hunks)src/rai_core/rai/messages/multimodal.py(1 hunks)src/rai_core/rai/tools/debugging_assistant.py(0 hunks)src/rai_core/rai/tools/ros2/__init__.py(2 hunks)src/rai_core/rai/tools/ros2/cli.py(2 hunks)src/rai_core/rai/tools/ros2/manipulation/custom.py(1 hunks)src/rai_core/rai/tools/ros2/navigation/nav2.py(1 hunks)src/rai_core/rai/utils/agents.py(0 hunks)src/rai_core/rai/utils/ros_executors.py(0 hunks)src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py(1 hunks)src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py(1 hunks)src/rai_sim/rai_sim/o3de/o3de_bridge.py(1 hunks)tests/communication/test_hri_message.py(1 hunks)
💤 Files with no reviewable changes (3)
- src/rai_core/rai/utils/agents.py
- src/rai_core/rai/utils/ros_executors.py
- src/rai_core/rai/tools/debugging_assistant.py
🧰 Additional context used
🧬 Code Graph Analysis (24)
src/rai_bench/rai_bench/examples/o3de_test_benchmark.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
examples/rosbot-xl-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
examples/manipulation-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_core/rai/apps/high_level_api.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_tracing_callbacks(188-220)
src/rai_core/rai/agents/langchain/runnables.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_core/rai/communication/hri_connector.py (1)
src/rai_core/rai/messages/multimodal.py (1)
MultimodalMessage(22-57)
src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py (1)
src/rai_core/rai/messages/multimodal.py (1)
HumanMultimodalMessage(60-81)
tests/communication/test_hri_message.py (1)
src/rai_core/rai/messages/multimodal.py (1)
MultimodalMessage(22-57)
src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_sim/rai_sim/o3de/o3de_bridge.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_core/rai/communication/ros2/ros_logs.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
src/rai_core/rai/tools/ros2/manipulation/custom.py (1)
src/rai_core/rai/communication/ros2/ros_async.py (1)
get_future_result(21-37)
examples/agriculture-demo.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py (2)
src/rai_core/rai/initialization/model_initialization.py (2)
get_llm_model(124-157)get_llm_model_config_and_vendor(108-121)src/rai_core/rai/agents/conversational_agent.py (1)
create_conversational_agent(52-84)
src/rai_core/rai/cli/rai_cli.py (3)
src/rai_core/rai/initialization/model_initialization.py (2)
get_embeddings_model(160-185)get_llm_model(124-157)src/rai_core/rai/messages/multimodal.py (1)
HumanMultimodalMessage(60-81)src/rai_core/rai/messages/conversion.py (1)
preprocess_image(24-48)
src/rai_core/rai/initialization/__init__.py (1)
src/rai_core/rai/initialization/model_initialization.py (4)
get_embeddings_model(160-185)get_llm_model(124-157)get_llm_model_config_and_vendor(108-121)get_tracing_callbacks(188-220)
src/rai_core/rai/agents/react_agent.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_tracing_callbacks(188-220)
src/rai_core/rai/tools/ros2/navigation/nav2.py (1)
src/rai_core/rai/messages/artifacts.py (1)
MultimodalArtifact(20-22)
examples/s2s/conversational.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_llm_model(124-157)
src/rai_core/rai/tools/ros2/__init__.py (1)
src/rai_core/rai/tools/ros2/cli.py (7)
ROS2CLIToolkit(55-64)ros2_action(68-82)ros2_interface(140-154)ros2_node(104-118)ros2_param(122-136)ros2_service(86-100)ros2_topic(158-183)
examples/agents/react.py (4)
src/rai_core/rai/agents/runner.py (2)
AgentRunner(65-115)run_and_wait_for_shutdown(85-87)src/rai_core/rai/agents/react_agent.py (1)
ReActAgent(30-114)src/rai_core/rai/communication/ros2/connectors/ari_connector.py (1)
ROS2ARIConnector(41-234)src/rai_core/rai/communication/ros2/context.py (1)
ROS2Context(23-146)
src/rai_core/rai/apps/talk_to_docs.py (1)
src/rai_core/rai/initialization/model_initialization.py (1)
get_embeddings_model(160-185)
🔇 Additional comments (54)
src/rai_core/rai/tools/ros2/manipulation/custom.py (1)
23-23: Import path updated correctly.The import path for
get_future_resulthas been successfully updated to match the new module organization, aligning with the refactoring effort to improve code organization by moving ROS2-specific utilities to a more appropriate namespace.src/rai_core/rai/communication/ros2/ros_logs.py (1)
35-35: Import path correctly updated for improved module organization.The import for
get_future_resulthas been properly updated to use the new location in therai.communication.ros2.ros_asyncmodule, consistent with the overall refactoring of utility functions.src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/gdino_tools.py (1)
23-23: Import path correctly updated for ROS2 utilities.The import path for
get_future_resulthas been properly updated to use the new module structure, which better organizes ROS2-specific utilities under the communication namespace.src/rai_extensions/rai_open_set_vision/rai_open_set_vision/tools/segmentation_tools.py (1)
28-28: Import path correctly updated to align with module restructuring.The import path for
get_future_resulthas been successfully updated to the new location inrai.communication.ros2.ros_async, maintaining consistency with the refactoring goal of better organizing utilities.src/rai_sim/rai_sim/o3de/o3de_bridge.py (1)
27-27: Updated Import for Async ROS2 UtilityThe import has been updated from the old module path to the new one (
rai.communication.ros2.ros_async). This change is consistent with the overall refactoring effort to improve module organization and remove the disorganizedrai.utilsmodule. Make sure that all references toget_future_resultin the codebase have been updated accordingly.src/rai_core/rai/tools/ros2/cli.py (2)
19-19: Import statement updated for better organization.The import statement has been expanded to include
BaseToolandBaseToolkitfromlangchain_core.tools, which are now used in the newROS2CLIToolkitclass. This change aligns with the PR's goal of better organizing utility modules.
55-65: Good addition of a toolkit class for ROS2 CLI tools.The new
ROS2CLIToolkitclass properly organizes the existing ROS2 CLI tools into a unified toolkit, making them easier to import and use as a group. This implementation follows theBaseToolkitpattern, which is a good practice for tool collection management in LangChain.This improves the developer experience by:
- Making it easier to import all ROS2 CLI tools at once
- Following LangChain's recommended pattern for tool organization
- Preparing these tools to be used in applications like the new Streamlit-based ROS2 Debugging Assistant
src/rai_core/rai/agents/react_agent.py (1)
27-27: Import path updated as part of utils module refactoring.This change updates the import path for
get_tracing_callbacksto use the new locationrai.initializationinstead of the previousrai.utils.model_initialization. This is consistent with the PR objective of reorganizing utility functions into more appropriate modules.src/rai_bench/rai_bench/examples/o3de_test_benchmark.py (1)
26-26: Import path updated as part of utils module refactoring.The import statement for
get_llm_modelhas been updated to use the new pathrai.initializationinstead of the previousrai.utils.model_initialization. This change is consistent with the broader refactoring effort to reorganize utility functions into more meaningful modules.src/rai_bench/rai_bench/tool_calling_agent_bench/scores_tracing.py (1)
22-22: Import path updated as part of utils module refactoring.The import statement for
get_tracing_callbackshas been updated to use the new pathrai.initializationinstead of the previousrai.utils.model_initialization. This is consistent with the PR's goal of reorganizing utility functions into more appropriate modules.src/rai_core/rai/tools/ros2/navigation/nav2.py (1)
31-31: Import path updated to reflect package reorganization.The import for
MultimodalArtifacthas been updated to use the consolidated path fromrai.messagesinstead of the previous more specific path. This aligns with the broader refactoring where artifact-related classes are now centralized and re-exported through the top-level messages package.src/rai_bench/rai_bench/tool_calling_agent_bench/agent_bench.py (1)
28-28: Import path updated for message class.The import for
HumanMultimodalMessagehas been updated to use the consolidated path fromrai.messagesinstead of the previous path fromrai.messages.multimodal. This is consistent with the broader refactoring effort to simplify imports by providing a unified interface through the top-level messages package.src/rai_core/rai/agents/langchain/runnables.py (1)
26-26: Model initialization import path updated.The import for
get_llm_modelhas been updated to use the new path fromrai.initializationinstead ofrai.utils.model_initialization. This change is part of the broader reorganization effort to improve code structure by moving initialization-related functions out of the general utilities module.examples/manipulation-demo.py (1)
19-19: Import path simplified to use top-level package.The import for
get_llm_modelhas been updated to use the top-levelraipackage instead of the previous path fromrai.utils.model_initialization. This change enhances developer experience by providing easier access to commonly used functions through the main package interface.src/rai_core/rai/apps/talk_to_docs.py (1)
33-33: Update Import forget_embeddings_modelThe import is now correctly refactored to load
get_embeddings_modelfromrai.initializationrather than the deprecatedrai.utils.model_initialization. This change is consistent with the overall reorganization and improves module clarity.examples/agriculture-demo.py (1)
20-20: Refactored Import forget_llm_modelThe import for
get_llm_modelhas been updated to import directly from the top-levelraipackage. This aligns with the new module structure and refactoring objectives ensuring consistency with other example scripts.examples/s2s/conversational.py (1)
26-26: Updated Import forget_llm_modelThe import statement is now updated to directly retrieve
get_llm_modelfrom the top-levelraipackage. This change is in line with the refactoring goal to consolidate model initialization functions and improves the overall code organization.src/rai_core/rai/apps/high_level_api.py (1)
21-21: Corrected Import forget_llm_modelThe import for
get_llm_modelhas been modified to load fromrai.initialization, which is consistent with the broader restructuring across the codebase. This ensures that all modules now reference the function from its new location.src/rai_core/rai/communication/hri_connector.py (1)
27-28: Revised Import forMultimodalMessageThe import for
MultimodalMessagehas been refactored to come fromrai.messages(using the aliasRAIMultimodalMessage) instead ofrai.messages.multimodal. This update aligns with the new module organization and helps maintain a clean and consistent import structure across the codebase.examples/rosbot-xl-demo.py (1)
20-20: Updatedget_llm_modelImport Path
The import forget_llm_modelnow comes from the top-levelraipackage rather than a deep submodule. This change aligns with the overall refactoring effort to simplify and centralize public APIs for model initialization.docs/developer_guide.md (1)
135-135: Simplified Import in Code Example
The change to importget_llm_modeldirectly fromraiin the code snippet improves clarity and consistency with the new project structure. This helps developers avoid deep module paths and reduces maintenance overhead.docs/multimodal_messages.md (1)
20-21: Consistent Import Update forget_llm_model
Updating the import to usefrom rai import get_llm_modelensures that all examples are aligned with the recent restructuring. This makes the public API simpler and more unified.docs/debugging_assistant.md (1)
24-26: Corrected Launch Command for Debugging Assistant
The launch command has been updated to run the new script atexamples/debugging_assistant.py, replacing the outdated path. This documentation change correctly reflects the new project structure and should help developers start the assistant without confusion.src/rai_core/rai/agents/tool_runner.py (1)
31-31: Consolidated Artifact Import
Moving thestore_artifacts(along withMultimodalArtifactandToolMultimodalMessage) import from the oldrai.utils.artifactslocation torai.messagesimproves modularity and clarity. This refactoring centralizes message‐related functionalities, which is in line with the overall cleanup effort. Verify that all usage of the artifact storage function remains compatible.tests/communication/test_hri_message.py (1)
21-22: Updated Import for MultimodalMessageThe import has been changed to pull
MultimodalMessagedirectly fromrai.messages(aliased asRAIMultimodalMessage), which aligns perfectly with the refactoring of the public message APIs.README.md (1)
121-122: Revised Configurator Tool PathThe command now uses the new path
src/rai_core/rai/frontend/configurator.py, reflecting the relocation of the configurator tool. This improves clarity in documentation and ensures users run the correct script.docs/tracing.md (1)
42-44: Updated Import for Tracing CallbacksThe example now imports
get_tracing_callbacksdirectly fromrai, complying with the updated public API exposure. This change simplifies usage and maintains consistency across documentation.src/rai_core/rai/communication/ros2/__init__.py (1)
20-28: Exposing ROS2Context in the Public APIThe addition of the import
from .context import ROS2Contextalong with its inclusion in the__all__list ensures thatROS2Contextis properly exposed as part of the public API. This supports the centralization of ROS 2 context management in the module.src/rai_bench/rai_bench/examples/tool_calling_agent_test_bench.py (1)
19-23: Simplified Model Initialization ImportsThe updated import statement bringing in both
get_llm_modelandget_llm_model_config_and_vendordirectly from the top-levelraipackage is a welcome change. This consolidation is in line with the overall refactoring effort and promotes a cleaner, more intuitive API.src/rai_core/rai/agents/__init__.py (2)
17-17: Import inclusion properly integrates the new AgentRunner and wait_for_shutdown components.The import statement correctly brings in the AgentRunner class and wait_for_shutdown function from the new runner module, supporting the refactoring of utility functions into their appropriate modules.
22-23: Export of new agent lifecycle management components to public API.The addition of AgentRunner and wait_for_shutdown to the all list appropriately exposes these components to users of the package, enabling a more organized approach to agent lifecycle management.
Also applies to: 27-27
src/rai_core/rai/messages/artifacts.py (3)
1-1: Updated copyright year.The copyright year has been updated to 2025.
17-17: Added TypedDict to typing imports.The import statement has been extended to include TypedDict, which is necessary for the MultimodalArtifact type definition.
20-22: Addition of MultimodalArtifact TypedDict.The MultimodalArtifact TypedDict appropriately defines a structure for multimodal content with images and audios lists. The comment on the images field provides useful context about the expected format.
src/rai_core/rai/cli/rai_cli.py (1)
27-28: Updated import paths align with module restructuring.The imports have been properly updated to reflect the new module organization:
- Model initialization functions now from
rai.initializationinstead ofrai.utils.model_initialization- Message-related imports consolidated under
rai.messagesThese changes maintain functionality while improving code organization.
src/rai_core/rai/__init__.py (2)
15-21: Clean public API exposing key components.The top-level imports now explicitly expose important classes and functions from submodules, making them directly accessible from the
raipackage. This improves developer experience by providing a cleaner, more intuitive API.
23-30: Comprehensive all definition.The all list clearly defines the public API of the package, exposing both agent infrastructure (AgentRunner, ReActAgent) and model initialization utilities. This approach makes it easier for developers to discover available functionality.
examples/agents/react.py (2)
15-16: Updated imports reflect module restructuring.The imports have been correctly updated to align with the new module organization:
- AgentRunner from rai.agents instead of wait_for_shutdown from a utils module
- ROS2Context from rai.communication.ros2 module, showing proper domain-specific organization
28-29: Improved agent lifecycle management.The code now uses the AgentRunner class to manage the agent's lifecycle, replacing separate calls to agent.run() and wait_for_shutdown(). This provides a more robust and unified pattern for agent execution and graceful shutdown.
src/rai_core/rai/messages/multimodal.py (1)
16-16: Import statement has been simplifiedThe import statement has been simplified to remove unused types after the refactoring. This is consistent with the broader reorganization mentioned in the PR objectives, where the utils module was split into multiple files for better organization.
docs/developer_guide/tools.md (3)
198-199: Imports correctly updated to reflect new module structureThe imports have been properly updated to reflect the new module structure:
ROS2Contextis now imported fromrai.communication.ros2instead ofrai.utilsAgentRunneris now imported fromraiThis change is aligned with the PR's goal of reorganizing the utils module into more specific locations.
210-211: Agent execution pattern updated to use AgentRunnerThe code has been updated to use the new
AgentRunnerclass instead of directly callingagent.run()followed bywait_for_shutdown([agent]). This pattern improves agent lifecycle management and provides a more centralized approach to handling agent execution and shutdown.
225-225: Import path correctly updated for ROS2ContextThe import of
ROS2Contexthas been properly updated to use the new module path, consistent with the earlier change in this file.src/rai_core/rai/initialization/__init__.py (2)
15-20: Imports focused on model initialization functionalityThe imports have been refactored to focus solely on model initialization functions, removing unrelated functionality like
wait_for_shutdownandROS2Contextwhich were moved to their more appropriate modules. This change aligns well with the PR objective of improving organization by splitting utilities into proper domain-specific modules.
22-27: Exports aligned with new focused responsibilityThe
__all__list has been updated to match the refactored imports, ensuring that only model initialization functions are exported from this module. This maintains API consistency and follows the principle of having each module focus on a specific responsibility.src/rai_core/rai/messages/__init__.py (3)
16-16: Artifact functionality consolidated from appropriate moduleThe import of
MultimodalArtifacthas been updated to come from.artifactsinstead of.multimodal, and two new functions for artifact management (get_stored_artifactsandstore_artifacts) have been added. This change properly consolidates artifact-related functionality into a dedicated module.
21-21: Added MultimodalMessage to exports from multimodal moduleThe
MultimodalMessageclass has been added to the exports from the.multimodalmodule, which is a good practice as it's the base class for the other multimodal message types already being exported.
30-30: Public API expanded with new exportsThe
__all__list has been updated to includeMultimodalMessageand the new artifact management functions (get_stored_artifactsandstore_artifacts), making them part of the public API. This is consistent with the changes to imports and enhances the usability of the messages package.Also applies to: 33-33, 35-35
examples/debugging_assistant.py (2)
15-20: Well-organized imports with clear module structure.The imports showcase the reorganized module structure where utilities have been properly organized into their respective packages. This aligns well with the PR objective of improving the developer experience.
22-33: Good use of Streamlit caching for agent initialization.Caching the agent initialization with
@st.cache_resourceprevents unnecessary recreations of the agent on UI refreshes, which improves performance and resource usage.src/rai_core/rai/tools/ros2/__init__.py (2)
15-23: Good module organization: CLI tools properly exposed.The imports from the
.climodule are well organized, making the ROS2 CLI toolkit and its tools available at the package level. This is in line with the PR's objective of restructuring the utility modules.
79-90: Properly exported classes and functions in all.The update to
__all__properly exposes the new ROS2 CLI toolkit and tool functions, maintaining alphabetical ordering which helps with readability and maintenance.src/rai_core/rai/agents/runner.py (1)
25-48: Good signal handling for graceful shutdown.The
wait_for_shutdownfunction correctly sets up signal handlers for both SIGINT and SIGTERM to ensure graceful shutdown of agents. The use of the Event object and try-finally block ensures proper cleanup.src/rai_core/rai/initialization/model_initialization.py (1)
88-94: Good enhancement for config file path flexibility.The addition of an optional
config_pathparameter toload_configimproves flexibility by allowing users to specify custom config locations. This is a valuable enhancement for different deployment scenarios.
| # Copyright (C) 2025 Robotec.AI | ||
| # |
There was a problem hiding this comment.
Update copyright year.
The copyright notice specifies 2025, but the current year is 2024. This should be corrected to maintain accuracy in the license headers.
-# Copyright (C) 2025 Robotec.AI
+# Copyright (C) 2024 Robotec.AI📝 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.
| # Copyright (C) 2025 Robotec.AI | |
| # | |
| # Copyright (C) 2024 Robotec.AI | |
| # |
| """Blocks execution until shutdown signal (SIGINT/SIGTERM) is received. | ||
|
|
||
| Args: | ||
| agent: Agent instance implementing stop() method | ||
|
|
||
| Note: | ||
| Ensures graceful shutdown of the agent and ROS2 node on interrupt. | ||
| Handles both SIGINT (Ctrl+C) and SIGTERM signals. | ||
| """ | ||
| shutdown_event = Event() | ||
|
|
||
| def signal_handler(signum, frame): | ||
| shutdown_event.set() | ||
|
|
||
| signal.signal(signal.SIGINT, signal_handler) | ||
| signal.signal(signal.SIGTERM, signal_handler) | ||
|
|
||
| try: | ||
| shutdown_event.wait() | ||
| finally: | ||
| for agent in self.agents: | ||
| agent.stop() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Code duplication between function and method implementations.
The wait_for_shutdown method duplicates the code from the standalone function. Consider refactoring to reuse the existing function to reduce duplication.
def wait_for_shutdown(agents: List[BaseAgent]):
"""Blocks execution until shutdown signal (SIGINT/SIGTERM) is received.
Args:
agents: List of agent instances implementing stop() method
Note:
Ensures graceful shutdown of the agent and ROS2 node on interrupt.
Handles both SIGINT (Ctrl+C) and SIGTERM signals.
"""
shutdown_event = Event()
def signal_handler(signum, frame):
shutdown_event.set()
signal.signal(signal.SIGINT, signal_handler)
signal.signal(signal.SIGTERM, signal_handler)
try:
shutdown_event.wait()
finally:
for agent in agents:
agent.stop()
# In the AgentRunner class
def wait_for_shutdown(self):
"""Blocks execution until shutdown signal (SIGINT/SIGTERM) is received.
Note:
Ensures graceful shutdown of the agents and ROS2 nodes on interrupt.
Handles both SIGINT (Ctrl+C) and SIGTERM signals.
"""
- shutdown_event = Event()
-
- def signal_handler(signum, frame):
- shutdown_event.set()
-
- signal.signal(signal.SIGINT, signal_handler)
- signal.signal(signal.SIGTERM, signal_handler)
-
- try:
- shutdown_event.wait()
- finally:
- for agent in self.agents:
- agent.stop()
+ wait_for_shutdown(self.agents)
Purpose
Our utility module became a bag of everything, reducing the DX.
Proposed Changes
Refactored the utils module into various new files which are now properly stored in respective folders.
Issues
#516
Testing
CI
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores
Refactor