Allow sentence metadata in responses #2108
Conversation
|
@witold-gren Can you try this out and let me know if it works for you? I still need to modify Home Assistant core for this to work in Assist, but I want to make sure it's correct here first. |
|
I can confirm that now it works properly on intents scripts (this repository). Thanks for this improvement @synesthesiam |
|
@synesthesiam any update? 😀 |
📝 WalkthroughWalkthroughThe changes introduce a new optional field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Validator
participant Renderer
participant Template
User->>Validator: Submit sentence for validation
Validator->>Validator: Validate sentence with metadata
Validator->>Renderer: Call render_response with metadata
Renderer->>Template: Render template with context including metadata
Template-->>Renderer: Return rendered response
Renderer-->>User: Send rendered response
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 (
|
![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 and nitpick comments (3)
shared/__init__.py (1)
294-294: LGTM! Consider adding type hints forresult.intent_metadata.The addition of
metadatato the context dictionary enhances the flexibility of response rendering by allowing intent metadata to be used in templates. This change aligns well with the PR objectives.To improve type safety, consider adding a type hint for
result.intent_metadatain theRecognizeResultclass. This would provide better clarity on the expected structure of the metadata.script/intentfest/validate.py (2)
170-170: LGTM! Consider adding a comment for clarity.The addition of the
metadatafield toSENTENCE_SCHEMAis well-implemented and aligns with the PR objectives. It's correctly defined as optional and uses a flexible validation approach.Consider adding a brief comment explaining the purpose and expected structure of the
metadatafield for future maintainers. For example:# metadata: Optional dictionary for language-specific data (e.g., non-inflected forms of nouns) vol.Optional("metadata"): {str: match_anything},
601-601: LGTM! Consider enhancing mock metadata for robust testing.The addition of
metadataas aMagicMock()object in the template rendering context is a good approach for flexible testing. It allows templates to use metadata without requiring specific metadata to be defined, maintaining backward compatibility.For more robust testing, consider creating a mock metadata object with a predefined structure that represents expected metadata fields. This could help catch issues where templates rely on specific metadata structure. For example:
"metadata": MagicMock(spec_set={"base_form": str, "gender": str}),This approach would still allow for flexible testing while providing some structure to catch potential issues in templates that rely on specific metadata fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- script/intentfest/validate.py (3 hunks)
- shared/init.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
shared/__init__.py (6)
Pattern
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern
*/**(html|markdown|md): do not comment on HTML used for icons
Pattern
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (2)
script/intentfest/validate.py (2)
11-11: LGTM! Appropriate import for mock object.The import of
MagicMockfromunittest.mockis correctly placed and necessary for the changes in the template rendering context. This is a standard approach for creating mock objects in Python testing scenarios.
Line range hint
1-604: Overall assessment: Well-implemented changes with minor suggestions for improvement.The changes in this file successfully introduce support for sentence metadata in the validation and testing process, aligning well with the PR objectives. The implementation maintains backward compatibility and allows for flexible testing. The suggestions provided aim to improve code clarity and the robustness of testing.
Key points:
- The
metadatafield is correctly added to theSENTENCE_SCHEMA.- The
MagicMockimport and usage for metadata in the template rendering context is appropriate.- Consider adding comments for clarity and enhancing the mock metadata structure for more robust testing.
These changes provide a solid foundation for incorporating sentence metadata in the Home Assistant intents framework.
Sentence metadata was added to hassil in version 1.6.0:
This PR exposes that metadata to response rendering as a
metadataobject:This allows sentence authors to pass language-specific data to response templates, such as the non-inflected/base forms of nouns that are part of the matched sentence (device class, etc.).
Summary by CodeRabbit
New Features
metadatafield in the validation process for enhanced contextual information.Bug Fixes
metadatakey.