feat: extend dialog/intent templates #317
Conversation
WalkthroughThe changes in this pull request primarily focus on enhancing the functionality of the Changes
Assessment against linked issues
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #317 +/- ##
==========================================
+ Coverage 44.74% 45.57% +0.82%
==========================================
Files 33 33
Lines 3426 3467 +41
==========================================
+ Hits 1533 1580 +47
+ Misses 1893 1887 -6 ☔ View full report in Codecov by Sentry. |
![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 (1)
test/unittests/test_bracket_expansion.py (1)
8-36: Well-structured test with comprehensive coverage!The test effectively validates the
expand_slotsfunction with a practical example, covering all possible combinations of slot values and optional words.Fix indentation in expected_sentences list.
The list uses 14 spaces for indentation instead of the standard 8 spaces.
expected_sentences = ['change brightness to low and color to red', - 'change brightness to low and color to green', + 'change brightness to low and color to green',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
test/unittests/test_bracket_expansion.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
test/unittests/test_bracket_expansion.py
40-40: Local variable templates is assigned to but never used
Remove assignment to unused variable templates
(F841)
🔇 Additional comments (2)
test/unittests/test_bracket_expansion.py (2)
3-6: LGTM! Clean imports and appropriate class renaming.
The imports are minimal and the class renaming from TestBracketExpansion to TestTemplateExpansion better reflects the broader scope of template expansion functionality.
38-114: Excellent test coverage of template expansion patterns!
The test comprehensively covers all syntax features mentioned in the PR objectives:
- Optional words using
[brackets] - Alternatives using
(pipes|or|choices) - Slot placeholders using
{curly_braces} - Nested structures combining all features
- Empty optional sections
- Mid-word affixes
The use of subtests for each template pattern is a good practice for maintainable tests.
Note: The static analysis warning about unused templates variable can be ignored as it's used indirectly through expected_outputs.items().
🧰 Tools
🪛 Ruff (0.8.0)
40-40: Local variable templates is assigned to but never used
Remove assignment to unused variable templates
(F841)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
ovos_utils/dialog.py (1)
95-95: Consider documenting template expansion behaviorThe random selection from expanded templates should be documented to clarify the behavior when using optional syntax (e.g.,
[hello,] how are you).Consider adding a docstring example:
def render(self, template_name, context=None, index=None): """ ... Examples: Template "[hello,] how are you" might expand to either: - "hello how are you" - "how are you" """ovos_utils/file_utils.py (1)
Line range hint
228-244: Document new template syntax in function docstringThe docstring should be updated to include examples of the new optional syntax supported in vocabulary files.
def read_vocab_file(path: str) -> List[List[str]]: """ Read voc file. This reads a .voc file, stripping out empty lines comments and expand - parentheses. It returns each line as a list of all expanded + parentheses and optional syntax. It returns each line as a list of all expanded alternatives. + Supported syntax: + - [optional]: Square brackets for optional words + - (a|b): Parentheses for alternatives + Example: + "[hello,] (good morning|good evening)" expands to: + ["hello good morning", "hello good evening", "good morning", "good evening"] + Args: path (str): path to vocab file. Returns: List of Lists of strings. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
ovos_utils/dialog.py(2 hunks)ovos_utils/file_utils.py(2 hunks)
🔇 Additional comments (3)
ovos_utils/dialog.py (1)
8-8: LGTM: Import change aligns with new template expansion functionality
The change from expand_options to expand_template aligns with the PR objectives to support enhanced template syntax.
ovos_utils/file_utils.py (2)
15-15: LGTM: Import change maintains consistency
The change from expand_options to expand_template maintains consistency with the new template expansion functionality.
244-244: Review case sensitivity in template expansion
The .lower() conversion is applied before template expansion. Verify this doesn't affect any case-sensitive template variables or optional syntax.
✅ Verification successful
Case conversion before template expansion is safe
After examining the implementation of expand_template and its test cases, the .lower() conversion before template expansion is safe because:
-
The template expansion function (
expand_template) operates purely on pattern matching using regex for:- Optional brackets
[optional] - Alternative choices
(a|b) - Slot placeholders
{variable}
- Optional brackets
-
The test cases demonstrate that case sensitivity is preserved for slot variables (e.g.,
{device_name},{zone_name}), which are not affected by the.lower()call since they are processed after template expansion. -
The expansion logic doesn't rely on case-sensitive patterns for its functionality, only on the special characters
[](){}|for syntax.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for case-sensitive variables in vocabulary files
# Search for uppercase variables in .voc files
fd -e voc -x rg -l '\{[A-Z][^}]*\}'
# Search for uppercase content in square brackets
fd -e voc -x rg -l '\[[^]]*[A-Z][^]]*\]'
Length of output: 80
Script:
#!/bin/bash
# Let's check the implementation of expand_template and its usage context
ast-grep --pattern 'def expand_template'
# Also search for any documentation or comments about template expansion
rg -B2 -A2 "expand_template"
# Let's also check for any .voc files in test directories that might show template usage
fd -e voc -p "test"
Length of output: 5150
Script:
#!/bin/bash
# Let's examine the expand_template implementation to understand how it handles case
cat ovos_utils/bracket_expansion.py
# Also check test cases to see if there are any case-sensitive examples
cat test/unittests/test_bracket_expansion.py
Length of output: 15179
pull OpenVoiceOS/ovos-utils#317 into dependencies
pull OpenVoiceOS/ovos-utils#317 into dependencies
* fix:bracket_expansion pull OpenVoiceOS/ovos-utils#317 into dependencies * Update build_tests.yml * failing tests * Update requirements.txt
closes #312
also adds a new util if we want to generate text from .intent + .entity combinations
Summary by CodeRabbit
New Features
expand_templatefunction for enhanced template string processing.expand_slotsfunction to substitute placeholders with slot options.Deprecated Features
expand_templatemethod for sentence expansion tasks.Bug Fixes
expand_templatemethod instead of the deprecatedexpand_options.Testing Improvements
expand_templateandexpand_slots.