Ask which list a Siri reminder should go on - #5818
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Add multi-list disambiguation coverage, distinguish same-named list choices, and provide the documentation PR number.
Pull request overview
Updates Siri reminder creation to auto-select the only available list, show an error when none are available, or ask the user to choose when multiple lists exist.
Changes:
- Adds list-resolution states and disambiguation.
- Adds localized prompt text.
- Updates list-query and intent tests.
File summaries
| File | Reviewed changes | Final findings |
|---|---|---|
Tests/App/Utilities/AppIntentSchemas/RemindersSchemaSupportTests.swift |
Tests list-resolution outcomes. | Moderate: Add coverage for the multi-list intent disambiguation flow. |
Tests/App/Utilities/AppIntentSchemas/ReminderListSchemaEntityQueryTests.swift |
Tests list query behavior and ordering. | — |
Tests/App/Utilities/AppIntentSchemas/CreateReminderSchemaIntentTests.swift |
Updates reminder creation coverage. | — |
Sources/Shared/Resources/Swiftgen/Strings.swift |
Adds the generated localization accessor. | — |
Sources/App/Utilities/AppIntentSchemas/RemindersSchemaSupport.swift |
Exposes list-resolution logic. | — |
Sources/App/Utilities/AppIntentSchemas/ReminderListSchemaEntityQuery.swift |
Provides accessible list ordering. | — |
Sources/App/Utilities/AppIntentSchemas/ReminderListResolution.swift |
Models available-list resolution states. | — |
Sources/App/Utilities/AppIntentSchemas/CreateReminderSchemaIntent.swift |
Resolves or prompts for the target list. | Moderate: Add intent-level coverage for disambiguation. Same-named lists need distinguishable server or entity context. |
Sources/App/Resources/en.lproj/Localizable.strings |
Adds the disambiguation prompt. | Nit: Add the missing companion documentation PR number. |
Review details
Suppressed comments (4)
Sources/App/Resources/en.lproj/Localizable.strings:262
- The required companion documentation PR is still blank in the description (
home-assistant/companion.home-assistant#), even though this adds user-facing Siri behavior. Please add the documentation PR number before merging.
"app_intents.reminders.create.which_list" = "Which list?";
Sources/App/Utilities/AppIntentSchemas/CreateReminderSchemaIntent.swift:61
- The new multi-list path is not exercised by the intent tests:
RemindersSchemaSupportTestsonly verifies that.choiceis constructed, while noCreateReminderSchemaIntentTestsinvocation reachesrequestDisambiguationwith multiple lists. Add an intent-level test that drives this branch and verifies the selected list is used (or otherwise covers the AppIntents disambiguation request), so a broken dialog/parameter interaction is caught before release.
case let .choice(lists):
return try await $list.requestDisambiguation(
among: lists,
dialog: IntentDialog(.init(
"app_intents.reminders.create.which_list",
Sources/App/Utilities/AppIntentSchemas/CreateReminderSchemaIntent.swift:60
- When two exposed servers contain lists with the same name, every candidate here has the same display title because
ReminderListSchemaEntity.displayRepresentationuses onlyname. Siri will show indistinguishable choices, so the user cannot reliably select the intended list/server; include server or entity context in the candidates' display representation before requesting disambiguation.
return try await $list.requestDisambiguation(
among: lists,
dialog: IntentDialog(.init(
Tests/App/Utilities/AppIntentSchemas/RemindersSchemaSupportTests.swift:36
- These tests verify only that
listResolution()returns.choice; no test executesCreateReminderSchemaIntent.perform()with multiple exposed lists, so the new$list.requestDisambiguationcall and dialog are unverified. Add coverage for this branch, or an injectable seam around the system disambiguation API.
guard case let .choice(lists) = RemindersSchemaSupport.listResolution() else {
return XCTFail("expected a choice between the lists")
}
XCTAssertEqual(lists.map(\.entityId), ["todo.shopping", "todo.work"])
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
b0b6bb9 to
215e642
Compare
…-ask-which-list # Conflicts: # Sources/App/Resources/en.lproj/Localizable.strings # Sources/App/Utilities/AppIntentSchemas/ReminderListSchemaEntityQuery.swift # Tests/App/Utilities/AppIntentSchemas/RemindersSchemaSupportTests.swift
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5818 +/- ##
==========================================
+ Coverage 48.51% 48.71% +0.20%
==========================================
Files 1185 1190 +5
Lines 78770 79041 +271
==========================================
+ Hits 38214 38504 +290
+ Misses 40556 40537 -19
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Test coverage of this pull request is below 90%
The unit tests run 82.22% of the lines this pull request
adds or changes (37 of 45 coverable lines).
The per-file breakdown, and the changed lines no test runs, are in the
job summary. Adding tests for those lines and pushing dismisses this
review automatically.
Lines that carry no executable code, and files the unit test targets do not
build, are not counted. If the new code genuinely cannot be unit tested, a
maintainer can dismiss this review.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
AI Policy
Select exactly one option that describes AI usage in this contribution:
Summary
When a reminder is created through Siri without naming a list, the app used to silently add it to the first to-do list it found. Now, if more than one list is exposed to Siri, Siri asks which list to use. With a single list it is used without asking, and with none the existing error is shown.
Screenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes