[BG] Add HassGetDate and HassGetTime #2432
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces new YAML files for Bulgarian language support in a home automation context. It includes response templates for two intents: Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 5
🧹 Outside diff range and nitpick comments (1)
tests/bg/homeassistant_HassGetCurrentDate.yaml (1)
3-8: Consider adding more test variations.The test cases cover basic scenarios, but consider adding tests for:
- Formal/informal variations
- Different date formats (e.g., "кой ден сме днес", "кое число сме днес")
- Edge cases (e.g., queries with additional context)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- responses/bg/HassGetCurrentDate.yaml (1 hunks)
- responses/bg/HassGetCurrentTime.yaml (1 hunks)
- sentences/bg/homeassistant_HassGetCurrentDate.yaml (1 hunks)
- sentences/bg/homeassistant_HassGetCurrentTime.yaml (1 hunks)
- tests/bg/homeassistant_HassGetCurrentDate.yaml (1 hunks)
- tests/bg/homeassistant_HassGetCurrentTime.yaml (1 hunks)
🔇 Additional comments (13)
sentences/bg/homeassistant_HassGetCurrentTime.yaml (2)
1-4: LGTM! The YAML structure is correct.The file follows the proper Home Assistant intent YAML format with correct language specification and intent naming convention.
5-7: 🛠️ Refactor suggestionEnhance coverage of Bulgarian time query patterns.
While the current patterns are valid, consider adding more common variations to improve the natural language understanding:
data: - sentences: - "[кажи ми] колко е часа" - - "[кажи ми] час[а]" + - "колко часа е" + - "в колко часа сме" + - "кажи ми времето" + - "колко е часът"Also, consider removing or refining the pattern
"[кажи ми] час[а]"as it might be too ambiguous and could match unrelated queries.Let's check if similar patterns exist in other languages for consistency:
✅ Verification successful
Enhance coverage of Bulgarian time query patterns and align with other languages
Looking at patterns across different languages, the suggestion to add more variations is well justified. Other languages like Polish, Romanian, and French have comprehensive pattern coverage. The current Bulgarian patterns are minimal compared to other languages.
The suggested improvements are good, and we should also add a few more patterns based on the common structure seen in other languages:
data: - sentences: - "[кажи ми] колко е часа" - - "[кажи ми] час[а]" + - "колко часа е" + - "в колко часа сме" + - "кажи ми времето" + - "колко е часът" + - "кое време е" + - "колко показва часовникът"Removing
"[кажи ми] час[а]"is definitely correct as it's too ambiguous - this is confirmed by looking at patterns in other languages where no language uses such a short/ambiguous pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check time query patterns in other languages # Look for files with similar naming pattern in other language folders fd "homeassistant_HassGetCurrentTime.yaml" -x cat {} \;Length of output: 6132
sentences/bg/homeassistant_HassGetCurrentDate.yaml (2)
1-4: LGTM! The language and intent structure is correctly defined.The YAML structure follows the standard Home Assistant intent recognition format with proper indentation and hierarchy.
1-7: Verify the integration with response templates.The intent recognition patterns should align with the response template's capabilities.
Let's verify the existence and structure of the corresponding response template:
✅ Verification successful
Intent patterns and response template are properly integrated
The response template correctly handles the date formatting in Bulgarian with:
- Complete mapping of months in Bulgarian
- Ordinal numbers for days (1-31)
- Proper date format structure:
[day] [month] [year]-таThis aligns well with the intent patterns that ask for the current date in Bulgarian.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the corresponding response template exists and contains date formatting rg -g "responses/bg/*" "HassGetCurrentDate" -A 5Length of output: 409
Script:
#!/bin/bash # Let's get the complete response template content rg -g "responses/bg/*" "HassGetCurrentDate" -A 15Length of output: 1010
Script:
#!/bin/bash # Let's get more lines to see the complete template structure rg -g "responses/bg/*" "HassGetCurrentDate" -A 25Length of output: 1616
Script:
#!/bin/bash # Let's get even more lines to see the complete template structure including the final response format rg -g "responses/bg/*" "HassGetCurrentDate" -A 40Length of output: 2633
Script:
#!/bin/bash # Let's get the complete file content to see the final response format cat responses/bg/HassGetCurrentDate.yamlLength of output: 1482
tests/bg/homeassistant_HassGetCurrentTime.yaml (3)
1-1: LGTM! Correct language declaration.The language is properly declared using the ISO 639-1 code 'bg' for Bulgarian.
2-9: LGTM! Well-structured test configuration.The test configuration follows the proper YAML structure with all required fields.
1-10: Verify consistency with other language implementations.Let's ensure this implementation follows the same patterns as other languages in the Home Assistant intents repository.
✅ Verification successful
Implementation is consistent with other language patterns
The Bulgarian implementation follows the same structure as other language implementations:
- Uses the standard
HassGetCurrentTimeintent name- Contains language-specific test sentences for asking time
- Includes a localized response format similar to other languages (e.g. Romanian "1 și 2 minute", French "01:02")
- Maintains the same YAML structure with
language,tests,sentences,intent, andresponsefields🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check how other languages implement time-related tests echo "Checking time-related test implementations in other languages..." fd -e yaml -e yml . tests/ -x grep -l "HassGetCurrentTime" {} \; | while read -r file; do echo "=== $file ===" cat "$file" doneLength of output: 6601
responses/bg/HassGetCurrentTime.yaml (1)
1-4: LGTM! File structure follows project conventions.The language declaration and YAML structure are properly configured for Bulgarian language support.
tests/bg/homeassistant_HassGetCurrentDate.yaml (2)
1-2: LGTM!The language declaration and test structure follow the correct YAML format.
1-11: Verify integration with response templates.The test file should align with the response templates defined in the responses YAML file.
Let's verify the existence and format of the response template:
#!/bin/bash # Check if the response template file exists and its content fd 'HassGetCurrentDate.yaml' responses/bg/ --exec catresponses/bg/HassGetCurrentDate.yaml (3)
1-5: LGTM!The file structure and intent declaration follow the correct format for Home Assistant intent responses.
6-19: LGTM!The months dictionary correctly maps all 12 months to their proper Bulgarian names with consistent capitalization.
53-53: 🛠️ Refactor suggestionConsider improving the date template robustness.
The current template implementation has several potential issues:
- The "-та" suffix might not be grammatically correct for all years in Bulgarian
- Missing error handling for cases where slots.date or its properties are undefined
- No validation for day numbers that might be out of range for the ordinal dictionary
Consider implementing a more robust template:
- {{ ordinal[slots.date.day] }} {{ months[slots.date.month] }} {{ slots.date.year }}-та + {%- if slots.date %} + {%- set day = slots.date.day | default(1) %} + {%- set month = slots.date.month | default(1) %} + {%- set year = slots.date.year | default(2024) %} + {%- if day in ordinal and month in months -%} + {{ ordinal[day] }} {{ months[month] }} {{ year }}-та + {%- else -%} + Невалидна дата + {%- endif -%} + {%- else -%} + Датата не е налична + {%- endif -%}Let's check if there are any similar date formatting patterns in other language files:
| - sentences: | ||
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | ||
| - "[кажи ми] [коя|каква] дата е [днес]" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding more natural language variations for better user experience.
While the current patterns cover basic queries, consider adding these common variations in Bulgarian:
- Questions starting with "Коя дата сме днес"
- Informal queries like "Датата?"
- Questions about specific parts of the date (e.g., "кой ден сме днес")
data:
- sentences:
- "[кажи ми] [коя е|каква е] [днешната] дата[та]"
- "[кажи ми] [коя|каква] дата е [днес]"
+ - "коя дата сме [днес]"
+ - "датата"
+ - "[кажи ми] кой ден сме [днес]"📝 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.
| - sentences: | |
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | |
| - "[кажи ми] [коя|каква] дата е [днес]" | |
| - sentences: | |
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | |
| - "[кажи ми] [коя|каква] дата е [днес]" | |
| - "коя дата сме [днес]" | |
| - "датата" | |
| - "[кажи ми] кой ден сме [днес]" |
| - "колко е часа" | ||
| - "кажи ми колко е часа" | ||
| - "час" | ||
| - "кажи ми час" | ||
| intent: | ||
| name: HassGetCurrentTime | ||
| response: "01 часа и 02 минути" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage and response format.
While the basic test cases are good, consider the following improvements:
-
Add more test cases to cover:
- Different hour formats (e.g., "13 часа" vs "1 часа")
- Edge cases like midnight ("полунощ") and noon ("обяд")
- Different time formats (24-hour vs 12-hour)
-
The hard-coded response "01 часа и 02 минути" is too specific. Consider using a more generic format that can handle:
- Hours 1-24
- Minutes 0-59
- Proper Bulgarian grammar for different numbers (e.g., "1 час" vs "2 часа")
Here's a suggested expansion of test cases:
tests:
- sentences:
- "колко е часа"
- "кажи ми колко е часа"
- "час"
- "кажи ми час"
- "колко е часът"
- "в колко часа сме"
intent:
name: HassGetCurrentTime
response: "01 часа и 02 минути"
- sentences:
- "колко е часа"
intent:
name: HassGetCurrentTime
response: "13 часа и 00 минути"
- sentences:
- "колко е часа"
intent:
name: HassGetCurrentTime
response: "00 часа и 00 минути"
responses/bg/HassGetCurrentTime.yaml
Outdated
| default: > | ||
| {% set hour_str = '{0:02d}'.format(slots.time.hour) %} | ||
| {% set minute_str = '{0:02d}'.format(slots.time.minute) %} | ||
| {{ hour_str }} часа и {{ minute_str }} минути |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
The review suggestions are valid and should be implemented
After analyzing other language implementations, the review suggestions are well-founded:
- None of the implementations handle missing time slots, making error handling a valid concern
- Several languages (es, pt, ro) implement proper grammatical rules for their time units
- Some implementations (es) handle special cases like noon/midnight
- The Bulgarian implementation needs these improvements for better localization
The suggested improvements in the review are appropriate and align with best practices seen in other language implementations, particularly:
- Error handling for missing slots
- Proper grammar rules for "час"/"часа"
- Special cases for noon/midnight
🔗 Analysis chain
Consider improving time formatting and adding error handling.
A few suggestions to enhance the implementation:
- Add error handling for missing time slots
- Consider Bulgarian grammar rules for hours:
- Use "час" for 1 hour
- Use "часа" for 2-24 hours
- Consider special cases like noon (обяд) and midnight (полунощ)
Here's a suggested improvement:
default: >
+ {%- if not slots.time %}
+ Съжалявам, но не мога да определя часа.
+ {%- else %}
{% set hour_str = '{0:02d}'.format(slots.time.hour) %}
{% set minute_str = '{0:02d}'.format(slots.time.minute) %}
- {{ hour_str }} часа и {{ minute_str }} минути
+ {%- if slots.time.hour == 12 and slots.time.minute == 0 %}
+ обяд
+ {%- elif slots.time.hour == 0 and slots.time.minute == 0 %}
+ полунощ
+ {%- else %}
+ {{ hour_str }} {% if slots.time.hour == 1 %}час{% else %}часа{% endif %} и {{ minute_str }} минути
+ {%- endif %}
+ {%- endif %}Let's verify if there are any similar patterns in other language implementations:
Let me gather more information about error handling patterns in other language implementations.
Let me check the full content of time response files to understand the complete implementation patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check how other languages handle noon/midnight and hour pluralization
rg -g "responses/**/HassGetCurrentTime.yaml" -A 10 "hour.*format"
Length of output: 1556
Script:
#!/bin/bash
# Check for error handling patterns in time responses
rg -g "responses/**/HassGetCurrentTime.yaml" -B 2 -A 5 "if not slots.time"
Length of output: 76
Script:
#!/bin/bash
# Get full content of time response files
rg -g "responses/**/HassGetCurrentTime.yaml" --no-line-number "^" -A 20
Length of output: 21405
| - "кажи ми днешната дата" | ||
| intent: | ||
| name: HassGetCurrentDate | ||
| response: "седемнайсети Септември 2013-та" |
There was a problem hiding this comment.
💡 Codebase verification
Replace hardcoded response with dynamic date formatting - Update needed across all languages
Looking at the test files across all languages, I can see that all of them are using hardcoded dates (specifically September 17th, 2013) in their test responses. This is a systemic pattern that needs to be addressed. The review comment is correct, but the scope should be expanded:
- All language test files need to be updated to use dynamic date formatting
- The suggested format
{% now 'j F Y' %}would need to be adapted for each language's specific date formatting requirements - The Bulgarian (bg) implementation should maintain its ordinal number formatting system (as seen in the response file with
ordinalmapping)
🔗 Analysis chain
Replace hardcoded response with dynamic date formatting.
The current hardcoded response "седемнайсети Септември 2013-та" is problematic:
- It uses a static date from 2013
- It doesn't match the expected behavior of returning the current date
Consider using a reference date format like this:
- response: "седемнайсети Септември 2013-та"
+ response: "{% now 'j F Y' %}"Let's verify if other language test files use dynamic dates:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for date-related test responses in other language files
rg -g 'tests/*/homeassistant_HassGetCurrentDate.yaml' 'response:'
Length of output: 1587
Script:
#!/bin/bash
# Search for date-related test responses in other language files using a more specific pattern
rg -g 'tests/*/homeassistant_HassGetCurrentDate.yaml' -l '.'
# Then read the content of each found file
fd 'homeassistant_HassGetCurrentDate.yaml' tests/ --exec cat {}
Length of output: 5716
responses/bg/HassGetCurrentDate.yaml
Outdated
| 13: 'тринайсети', | ||
| 14: 'четиринайсети', | ||
| 15: 'петнайсети', | ||
| 16: 'шеснтнайсети', |
There was a problem hiding this comment.
Fix typo in Bulgarian ordinal number.
The word 'шеснтнайсети' contains a typo and should be 'шестнайсети' (sixteenth).
- 16: 'шеснтнайсети',
+ 16: 'шестнайсети',📝 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.
| 16: 'шеснтнайсети', | |
| 16: 'шестнайсети', |
|
I'm waiting for the poll in the bulgarian community of Home Assistant, in facebook, to decide if this variant of translation is good enought, because my opinion is different. If we let this asside, codderabbitai offered a good handfull of additional changes, that will improve the translation. I suggest, that the translator will change his contribution and incloode the coderabbitai suggestions. I can also make the changes by myself if the author does not want to make them. Currently my answer for this commit is "No". I'll keep you in informed for my descision. For everyone that is reading this, the dilemma is which is the more correct way of naming the numbers (take 13, lets say) "тринадесет" or "тринайсет". Untill few years the only official way for naming the number 13 was "тринадесет". Now it was accepted to be named "тринайсет" also, but the second one comes from colloquial language. I will keep you posted. |
Change the names of all numbers in the month bigger that 10 with the more proper and widespread variant.
Add more variants to ask for the current date.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
sentences/bg/homeassistant_HassGetCurrentDate.yaml (1)
6-7: Standardize pattern grouping symbols.For consistency, use square brackets
[]instead of parentheses()for optional groups.Apply this diff:
- "[кажи ми] [коя е|каква е] [днешната] дата[та]" - - "[кажи ми] (коя|каква) дата (е|сме) [днес]" + - "[кажи ми] [коя|каква] дата [е|сме] [днес]"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
sentences/bg/homeassistant_HassGetCurrentDate.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Check: build
sentences/bg/homeassistant_HassGetCurrentDate.yaml
[failure] 1-1:
invalid YAML: while scanning a simple key
🪛 yamllint
sentences/bg/homeassistant_HassGetCurrentDate.yaml
[error] 9-9: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (1)
sentences/bg/homeassistant_HassGetCurrentDate.yaml (1)
1-3: LGTM! File header and intent declaration are well-structured.
The language declaration and intent structure follow the expected format.
🧰 Tools
🪛 GitHub Check: build
[failure] 1-1:
invalid YAML: while scanning a simple key
| data: | ||
| - sentences: | ||
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | ||
| - "[кажи ми] (коя|каква) дата (е|сме) [днес]" | ||
| + - "[кажи ми] датата [днес]" | ||
| + - "[кажи ми] кой ден сме [днес]" |
There was a problem hiding this comment.
Fix YAML syntax errors in sentence patterns.
The indentation in lines 8-9 is incorrect, causing YAML parsing errors. Also, there's inconsistent use of grouping symbols.
Apply this diff to fix the syntax and make the patterns consistent:
data:
- sentences:
- "[кажи ми] [коя е|каква е] [днешната] дата[та]"
- "[кажи ми] (коя|каква) дата (е|сме) [днес]"
- + - "[кажи ми] датата [днес]"
- + - "[кажи ми] кой ден сме [днес]"
+ - "[кажи ми] датата [днес]"
+ - "[кажи ми] кой ден сме [днес]"📝 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.
| data: | |
| - sentences: | |
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | |
| - "[кажи ми] (коя|каква) дата (е|сме) [днес]" | |
| + - "[кажи ми] датата [днес]" | |
| + - "[кажи ми] кой ден сме [днес]" | |
| data: | |
| - sentences: | |
| - "[кажи ми] [коя е|каква е] [днешната] дата[та]" | |
| - "[кажи ми] (коя|каква) дата (е|сме) [днес]" | |
| - "[кажи ми] датата [днес]" | |
| - "[кажи ми] кой ден сме [днес]" |
🧰 Tools
🪛 yamllint
[error] 9-9: syntax error: could not find expected ':'
(syntax)
|
@hristo-atanasov , I agree with the changes which you have made 👍 Since you have added the following sentence:
I have extended the response by adding the weekday in it. In that way we will provide more precise answer to the above sentence. |
Nice! I Even think that when asking "[кажи ми] кой ден сме [днес]", people will be more interested in the week day first, and the date will be an additional clarification. What do you think about a response in the other way - "вторник, седемнадесети Септември 2013-та"? |
|
Also I have one more question. This template wi produce a time string like this: "01 часа и 02 минути" Edit: I know I can translate it, but I'm not sure if it is ok to do it, because if some don't use TTS will see just a text "един часа и две минути" ("one a clock and two minutes") instead of "1 часа и 2 минути" or "01 часа и 02 минути". What do you think about this approach? Would it be a better solution? |
Yep, I agree that this sounds even better. I have changed the response in that way.
During my tests I was using Azure TTS and it pronounces the string as expected - "един часа и две минути". I thought that all other TTS services will have the same comportment, but obviously they doesn't 🙂 I have made one change, which I hope will work with all the TTS services. Please test it with Google TTS. |
|
@kyutov Don't worry, I found it by luck. As I have Azure TTS setup already, but it is disabled and I'm currently using Google TTS. I'm communicating with Mike Hansen about this and already sent him a script that transforms years to bulgarian. He also found that he has access to a library, that can generate the same. So in the end we decided, that I'll wait until he interates this library and will write me to acknoleadge me when it is ready. |
Initial attempt to add HassGetDate and HassGetTime
Summary by CodeRabbit
New Features
HassGetCurrentDateandHassGetCurrentTime.Tests
HassGetCurrentDateandHassGetCurrentTime.