Skip to content

#105 Browse and pick a Home Assistant entity during season-sync setup - #107

Merged
GonzRon merged 4 commits into
masterfrom
claude/sleepy-fermat-h5k5ul
Oct 5, 2026
Merged

GonzRon merged 4 commits into
masterfrom
claude/sleepy-fermat-h5k5ul

Conversation

@GonzRon

@GonzRon GonzRon commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Implements #105 (1.8.0 content, coordinator #104) under the owner rulings of 2026-10-05. Plan: docs/superpowers/plans/2026-10-05-issue-105-home-assistant-entity-picker.md.

What changes

The setup sheet (Link to Home Assistant on an Asset's season card) now opens on a Choose entity row instead of a bare Entity ID field. Tapping it turns the sheet into a searchable list of Home Assistant's input_boolean helpers, showing each one's friendly name over its exact entity ID, sorted by name. A tap picks one and returns to the form. Enter entity ID manually keeps the typed path. Save is held on Link until there is an entity to send: a pick, or non-blank typed text. Save then sends the exact entity ID through the shipped #16 LinkSeasonSync, unchanged. As #16 shipped it, Home Assistant is checked after the link is saved; no extra per-entity request is added.

  • Core (B1): HaEntityList.kt maps a GET /api/states answer to candidates. Ids are checked with isValidEntityId, names bounded at 128 characters with control characters replaced, and duplicates dropped. Failures use Test connection's status map. EntityPicker.kt (pickerRows) filters to the input_boolean domain, searches name and id by case-insensitive substring, sorts by name then id, and stops at 2,000 rows with a truncation flag.
  • Client (B2): HomeAssistantStateClient.listStates makes one foreground GET <base>/api/states through the same exchange as the poll: address rule, permission, home-network gate, headers, timeouts and failure map. It has its own 8 MiB cap; the poll stays at 64 KiB. The answer is parsed on the client's IO context. AppGraph.listHaEntities wires it over the stored connection and token.
  • Sheet (B3): the browser is a mode of the existing sheet, not a separate screen. It has a search field, scope line, loading line, Refresh, and distinct messages for "no helpers at all" and "nothing matches the search". A failed read shows its [SHIPPED][FEATURE] Home Assistant operating-season synchronization #16 message above the last good list and keeps the selection. A read still running is cancelled, which disconnects it, when the browser closes or the owner picks, switches to manual entry or starts another read. Dismissing the whole sheet cancels it too: Cancel, back, a tap outside the sheet, or a swipe.
  • Strings: P105-1…10 (ratified) in values/strings_asset_edit.xml and all nine packs. The translations are drafts until a native speaker reviews them.
  • Docs (B4): docs/home-assistant-season-sync.md (linking section and Limits) and docs/capabilities.md.

Unchanged: schema, backup format, API, MCP, manifest, permissions, the poll, the applier, LinkSeasonSync, and every #16 string.

Rulings applied

  • REST /api/states only; device grouping and the entity/device registries are deferred (Q1). The acceptance bullet about device context is deferred, not met.
  • 8 MiB cap for the list call only (Q2).
  • input_boolean.* only, with manual entry as the escape hatch (Q3).
  • Changing an already-linked Asset's entity is out of scope (Q4).
  • The ten strings, with your wording for P105-8 and P105-9 (Q5).
  • The browser is a mode of the sheet (Q6).

Departures from the plan (recorded in §9a)

  • HaStateMapper.kt's stringOrNull and bounded became internal, so the list mapper uses the same bounding code (no behaviour change).
  • Enter entity ID manually prefills the field with a picked id.
  • The two device rows prove the form's Choose entity row, the manual path, and the browser's fixed parts and Cancel. The list rows, the messages and the pick are proven in the JVM tests, because no Home Assistant answers on the emulator.

An earlier departure, Save staying enabled with nothing chosen, was reversed in review (eaad0cf). Row 16 holds as the plan wrote it.

Review history

Task review (206a00e):

  1. The list's JSON was parsed on the main thread. It is now parsed on the client's IO context.
  2. A superseded read could land late and replace a newer list. It is now cancelled.
  3. The empty-scope flag is computed once per list instead of on every keystroke.

PR review 5416922609 (eaad0cf):

  1. Whole-sheet dismissal bypassed the view model and orphaned a running read. Back, a tap outside the sheet, a swipe and the form's Cancel now call dismiss(), which cancels it.
  2. Save is held on Link until there is an entity to send; Resume is unchanged.

The fix also repaired two shipped #16 device tests (S55, and #78's year-round link). They typed into a field the browse-first sheet no longer shows, so they now open Enter entity ID manually first. The S55 test asserts Save is disabled before an entity is given and enabled after.

Noted and left as is: a key-store or database failure before the read shows no message, as Save already does (none is ratified for it).

Tests and gates

  • New tests:
    • core: HaStatesMapperTest, EntityPickerTest;
    • client: rows 9–10 in HomeAssistantStateClientTest;
    • view model: rows 11–17 in LinkSeasonSyncViewModelTest, plus the superseded-read, sheet-dismissal and Save-waits-for-an-entity cases;
    • connected: rows 18–19 in SeasonSyncScreensTest, plus the two repaired [SHIPPED][FEATURE] Home Assistant operating-season synchronization #16 tests.
  • CI is green on the current head eaad0cf (run 37333710584), including :app:testDebugUnitTest with LocalizationCoverageTest and UiLiteralGuardTest.
  • R6 hygiene greps over the range: no e-mail, home path, serial or stray IP address.
  • Gate before merge, not yet run: SeasonSyncScreensTest, then the connected suite, on emulator-5554 (ANDROID_SERIAL=emulator-5554 ./gradlew :app:connectedDebugAndroidTest). CI does not compile or run androidTest, so this is also the only check of the sheet's dismissal wiring. The real-Home-Assistant proof (§10) stays the controller's release step.

🤖 Generated with Claude Code

https://claude.ai/code/session_0133GBN5r2nyJM4teL6rziSk

claude added 3 commits October 5, 2026 00:22
…d on Q1–Q6 and ten strings

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SsCifH5iWH1JdtAsUcbKnW
…ten strings in ten packs

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SsCifH5iWH1JdtAsUcbKnW
…ded read

- listStates maps up to 8 MiB of JSON on the client's io, not the sheet's
  main thread.
- The browser's read is held as a job; a new read, a pick, manual entry and a
  close cancel it (the client disconnects), so a late answer never lands in a
  browser opened after it. New row-14 case fails without the cancel.
- The empty-scope flag (P105-8) is computed once per list, not per keystroke.
- Plan §9a records the review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0133GBN5r2nyJM4teL6rziSk

@GonzRon GonzRon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head 206a00e32ba85eab9a805289985611a1f42385f3.

CI run 385 is green (build, MCP, schedules, bundle), and the implementation is generally well aligned with #105: exact entity IDs remain canonical, REST discovery reuses #16's transport policy, the 8 MiB cap is isolated to the foreground list call, filtering/search/sort are pure, failed refreshes preserve the last list/selection, and the late-answer cancellation fix is good for the in-sheet paths.

Disposition: two blocking findings before merge

1. BLOCKING — whole-sheet dismissal can orphan an in-flight 8 MiB list request

The cancellation fix covers closeBrowse(), pick, manual entry and starting a new read, but the actual ModalBottomSheet.onDismissRequest bypasses the ViewModel and calls onDone() directly.

That matters because this ViewModel is created with viewModel(key = "season-sync-sheet:$assetId:$opening") under the Asset-detail ViewModelStoreOwner. Removing the conditional sheet from composition does not clear that ViewModel; each reopen increments opening and creates another keyed ViewModel while the old one remains alive until the Asset-detail owner is destroyed.

So this sequence is possible:

  1. open Choose entity → GET /api/states starts;
  2. press Android Back / tap scrim / swipe the sheet away;
  3. the UI disappears, but the old request keeps running (up to the 30 s call bound, potentially reading/parsing 8 MiB);
  4. reopen and start another list read;
  5. repeat → multiple abandoned foreground reads can overlap.

That contradicts the #105 contract that the inventory read is an explicit foreground action and that the sheet's list/read goes away when the sheet closes. Route every whole-sheet dismissal through a ViewModel cancellation method before onDone() (and pin that dismissal path with a test), rather than only cancelling the browser's own Cancel button.

2. BLOCKING — Save is enabled with no entity solely to preserve a stale #16 test

The approved #105 plan changed the form contract to: Save is disabled until a picked entity or a manual ID exists. The implementation instead leaves canSave independent of entity presence, and the execution record says this was done so the old #16 device assertion would remain true.

That test describes the old manual-ID form; it is not a product invariant. With the new browse-first UI, an owner can tap Save while the only entity affordance says Choose entity, and gets P16-49 telling them to enter an entity ID. That is internally inconsistent with the new primary flow.

Please restore the planned predicate for LINK: require chosen != null || entityId.trim().isNotEmpty() (RESUME remains unaffected), and update the old device assertion to the new contract. Invalid typed IDs should still flow through P16-49 exactly as before.

Proof gate before merge

The new/edited androidTest code is not exercised by CI and the PR says the connected classes have not been run. Because #105 changes the actual sheet/navigation surface and acceptance explicitly calls for Android coverage, I would run at least SeasonSyncScreensTest on emulator-5554 before merging, then the planned broader connected gate. The real-Home-Assistant smoke proof can remain the controller/release proof if needed, but the instrumented code should at least compile and run once before merge.

What I checked and am accepting

  • mapHaStatesAnswer: bounded body contract, status map, strict IDs, first-wins duplicate handling, bounded/control-scrubbed friendly names.
  • pickerRows: exact input_boolean domain, name/id substring search, deterministic ordering, 2,000-row cap.
  • HomeAssistantStateClient.listStates: same address/network/host/header/no-redirect policy as #16, separate 8 MiB cap, parsing moved off main.
  • ViewModel selection/manual/refresh semantics: exact ID only; last good list survives failure; a picked entity never retargets by friendly name.
  • Localization: new copy is resource-backed across the shipped packs and existing guards are green.
  • No schema/backup/API/MCP/manifest expansion.

I found no other code blocker in this pass.

…ts for an entity

- Back, scrim, swipe and the form's Cancel go through the model's dismiss(),
  which cancels a list read still running: each opening's model is keyed under
  the asset page and outlives its sheet.
- Link's Save is held until there is an entity to send (a pick or non-blank
  typed text), as row 16 specifies; Resume is unchanged.
- The two shipped device rows that typed into the field (S55, #78's YEAR_ROUND
  link) open Enter entity ID manually first; the S55 row asserts Save held,
  then enabled.
- Plan §9a records the review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0133GBN5r2nyJM4teL6rziSk

GonzRon commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Review 5416922609, finding 2 and the proof gate. Both findings are fixed in eaad0cf; finding 1 is answered on its thread.

2. Save waits for an entity. For Link, canSave now also requires entityToLink.isNotEmpty(), which means a pick or non-blank typed text. Resume is unchanged. A badly shaped typed ID still reaches P16-49 on Save, as before; typingClearsThePickAndManualEntryPrefillsTheField still pins that. New JVM case: linkSaveWaitsForAnEntityAndResumeDoesNot.

Two more connected tests were broken, and the fix covers them. While updating the old Save assertion, I found two #16 device tests that typed into the Entity ID field right after opening the sheet: the S55 test and #78's YEAR_ROUND link test. The browse-first sheet doesn't show that field until Enter entity ID manually is tapped, so both would have failed on emulator-5554. typeEntityId() now opens manual entry first when the field isn't shown. The S55 test asserts Save is disabled before an entity is given and enabled after. Your emulator gate would have caught this, and it's why it matters.

Proof gate: agreed, still open. SeasonSyncScreensTest hasn't been run. This environment can't download the Android SDK, so it can't build the androidTest APK or run an emulator. It needs ANDROID_SERIAL=emulator-5554 ./gradlew :app:connectedDebugAndroidTest (that class first) before merge. Plan §9a records the review, the reversal of the Save deviation, and this gate.


Generated by Claude Code

@GonzRon
GonzRon merged commit 29d5876 into master Oct 5, 2026
8 checks passed
@GonzRon
GonzRon deleted the claude/sleepy-fermat-h5k5ul branch October 7, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants