feat:allow pipelines to return a modified session #296
Conversation
WalkthroughThe pull request introduces an optional Changes
Possibly related PRs
Suggested labels
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
better state management, relates to OpenVoiceOS/ovos-core#643
|
Caution No docstrings were generated. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ovos_plugin_manager/templates/pipeline.py (2)
38-38: Update class docstring to document the new attribute.The
updated_sessionattribute should be documented in the class docstring. Please add:""" Represents an intent handler match result, expected by ovos-core plugins. Attributes: match_type (str): Name of the service that matched the intent. match_data (Optional[Dict]): Additional data provided by the intent match. skill_id (Optional[str]): The skill this handler belongs to. utterance (Optional[str]): The original utterance triggering the intent. + updated_session (Optional[Session]): Modified session to be used after intent handling. """
Line range hint
84-98: Consider documenting session handling in abstract methods.The addition of
updated_sessionenables pipeline stages to modify session state. Consider updating the docstrings of abstract methods inConfidenceMatcherPipelineandPipelineStageMatcherto document how implementing classes should handle session modifications.Example for
match_high:@abc.abstractmethod def match_high(self, utterances: List[str], lang: str, message: Message) -> Optional[IntentHandlerMatch]: """ Match an utterance with high confidence. Args: utterances (List[str]): List of utterances to match. lang (str): The language of the utterances. message (Message): The message containing the utterance. Returns: Optional[IntentHandlerMatch]: The match result or None if no match is found. + If session modifications are needed, set + updated_session in the returned match object. """Also applies to: 156-170
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_plugin_manager/templates/pipeline.py(3 hunks)
🔇 Additional comments (1)
ovos_plugin_manager/templates/pipeline.py (1)
8-8: LGTM!The Session import is correctly placed and necessary for the new functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_plugin_manager/templates/pipeline.py(3 hunks)
🔇 Additional comments (2)
ovos_plugin_manager/templates/pipeline.py (2)
8-8: LGTM!The Session import is correctly placed with other ovos_bus_client imports and properly supports the new session management feature.
38-38: Document the new session management capability.The
updated_sessionattribute enables important session state modifications, but its purpose and usage are not documented. Please update the class docstring to include:
- Purpose of the session modification capability
- When/how the session should be modified
- Lifecycle of the modified session
Please verify the session modification behavior:
better state management, relates to OpenVoiceOS/ovos-core#643
Summary by CodeRabbit