[EN] Make shade queries more flexible for more generic requests #2221
Conversation
WalkthroughWalkthroughThe recent updates to the Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedyamllint
Additional comments not posted (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
sentences/en/cover_HassGetState.yaml
Outdated
| @@ -3,33 +3,37 @@ intents: | |||
| HassGetState: | |||
| data: | |||
| - sentences: | |||
| - "is <name> {cover_states:state} [in <area>]" | |||
| - "is <name> {cover_states:state} [[in ](<area>|<floor>)]" | |||
There was a problem hiding this comment.
I don't think "is the shutter closed in upstairs" sounds right.
The best idea would probably be to split this into 2 separate sentences instead of using the alternatives.
Also, here is probably a good place to replace is with (is|are) - e.g. are the bedroom blinds open?
There was a problem hiding this comment.
That's true, but also unavoidable.
Doing a little brainstorming here:
Possible area names:
- Kitchen
- Upstairs
- Outside
- Basement
Possible floor names:
- Upstairs
- First Floor
- Basement
We'd want to match:
- "... in the kitchen"
in <area> - "... upstairs" ->
(<area>|<floor>) - "... outside" ->
(<area>|<floor>) - "... in the basement"
in (<area>|<floor>) - "... on the first floor"
on floor>
Since there is not really any strict alignment of article to area or floor, these can be generalized as:
[[(in|on) ](<area>|<floor>)] or, we can split this to two sentences
[[in ](<area>|<floor>)] and [[on ]<floor>], but that makes the number of refactors to support floors higher. Instead, they can be combined more exclusively as:
[([in ](<area>|<floor>)|[on ]<floor>)]
Either of the two single expansion options I listed are also very general and not specific to covers and something that I can, in another PR, add to _common.yaml so it's easier to add floor support to intents.
There was a problem hiding this comment.
For now, I'm using [[(in|on) ](<area>|<floor>)] because it's more concise, even though it possibly matches less correct grammar. I'm happy to change it to [([in ](<area>|<floor>)|[on ]<floor>)], especially by adding it to a common expansion.
There was a problem hiding this comment.
I can think of stuff like on the patio.
I'd suggest creating a general expansion rule along the lines of in_area_or_floor: ([(in|on) ]<area>|[on ]<floor>).
Then you can use [<in_area_or_floor>] throughout all sentences where it makes sense.
There was a problem hiding this comment.
Oh, I don't know how I missed this one... All the others were updated to cover that use case. I have a draft PR for more general expansion rules. I can add [in_area_or_floor] there. For now I'm using [[<in>] <area_floor>], which expands to [[(in|on|at|of)] (<area>|<floor>)]. #2225
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
tetele
changed the title
sentences/en/cover_HassGetState.yaml
Outdated
| @@ -3,33 +3,37 @@ intents: | |||
| HassGetState: | |||
| data: | |||
| - sentences: | |||
| - "is <name> {cover_states:state} [in <area>]" | |||
| - "is <name> {cover_states:state} [[in ](<area>|<floor>)]" | |||
There was a problem hiding this comment.
I can think of stuff like on the patio.
I'd suggest creating a general expansion rule along the lines of in_area_or_floor: ([(in|on) ]<area>|[on ]<floor>).
Then you can use [<in_area_or_floor>] throughout all sentences where it makes sense.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| - "how many {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]" | ||
| - "how many (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}" |
There was a problem hiding this comment.
These sentences are too long per the linter's recommendations. Consider revising them to improve readability and maintain the width standard.
- "how many {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]"
- "how many (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}"
+ "how many {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]" # Consider splitting this into multiple lines
+ "how many (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}" # Consider splitting this into multiple linesCommittable suggestion was skipped due to low confidence.
Tools
yamllint
[error] 35-35: line too long (110 > 80 characters) (line-length)
[error] 36-36: line too long (98 > 80 characters) (line-length)
| - "(which|what) {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]" | ||
| - "(which|what) (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}" |
There was a problem hiding this comment.
The sentences effectively enhance query flexibility but exceed the recommended line length, potentially impacting readability.
- "(which|what) {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]"
- "(which|what) (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}"
+ "(which|what) {cover_classes:device_class} (is|are) {cover_states:state} [[(in|on) ](<area>|<floor>)]" # Consider splitting this into multiple lines
+ "(which|what) (<area>|<floor>) {cover_classes:device_class} (is|are) {cover_states:state}" # Consider splitting this into multiple linesCommittable suggestion was skipped due to low confidence.
Tools
yamllint
[error] 28-28: line too long (114 > 80 characters) (line-length)
[error] 29-29: line too long (102 > 80 characters) (line-length)
| - "(is|are) any {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]" | ||
| - "(is|are) any (<area>|<floor>) {cover_classes:device_class} {cover_states:state}" |
There was a problem hiding this comment.
The new sentences greatly improve the flexibility of queries. However, both lines exceed the recommended 80 characters, affecting readability.
- "(is|are) any {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]"
- "(is|are) any (<area>|<floor>) {cover_classes:device_class} {cover_states:state}"
+ "(is|are) any {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]" # Consider splitting this into multiple lines
+ "(is|are) any (<area>|<floor>) {cover_classes:device_class} {cover_states:state}" # Consider splitting this into multiple linesCommittable suggestion was skipped due to low confidence.
Tools
yamllint
[error] 14-14: line too long (105 > 80 characters) (line-length)
[error] 15-15: line too long (93 > 80 characters) (line-length)
| - "are all [the] {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]" | ||
| - "are all [the] (<area>|<floor>) {cover_classes:device_class} {cover_states:state}" |
There was a problem hiding this comment.
These sentences also exceed the 80 character limit. Splitting these into multiple lines could enhance readability and maintainability.
- "are all [the] {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]"
- "are all [the] (<area>|<floor>) {cover_classes:device_class} {cover_states:state}"
+ "are all [the] {cover_classes:device_class} {cover_states:state} [[(in|on) ](<area>|<floor>)]" # Consider splitting this into multiple lines
+ "are all [the] (<area>|<floor>) {cover_classes:device_class} {cover_states:state}" # Consider splitting this into multiple linesCommittable suggestion was skipped due to low confidence.
Tools
yamllint
[error] 21-21: line too long (106 > 80 characters) (line-length)
[error] 22-22: line too long (94 > 80 characters) (line-length)
Add coverage for new sentences. These are ones that I try all the time naturally but end up failing.
Summary by CodeRabbit
New Features
HassGetStateintent to allow optional specification of location (area or floor) for more flexible querying of cover states.Bug Fixes