Fix on/off-only light overlay; add hasOverlay flag; harden VM cache - #23
Merged
Conversation
On/off-only lights (supported_color_modes == ["onoff"]) have no brightness or color controls, so: - LightCardViewModel.supportedModes returns [] for onoff-only lights - new isDimmable gates the medium-tile brightness fill in LightCard - new hasOverlay (== !supportedModes.isEmpty) makes makeOverlayView return nil, so the card body presents no dead brightness panel — icon toggle remains Add EntityCardViewModel.hasOverlay capability flag (default false) so handleCardTap branches on it instead of building an overlay view just to nil-check it. Override to true in Cover/Switch/Climate/Automation VMs (every VM whose makeOverlayView returns non-nil); Light drives it from supportedModes. Guard ViewModelFactory.makeViewModel(forEntityId:) with a domain-agnostic existence check so a cached VM whose backing @model was deleted mid-session is evicted rather than returned. Entities are never individually deleted during sync (missing ones are marked unavailable; full deletion only happens in wipeLocalData, which discards the whole factory), so there is no per-entity deletion chokepoint to hook — hence the lookup-time guard (Option B) over an unused evict method (Option A). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the project comment-style convention for the comments added in the previous commit; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
makeViewModel(forEntityId:) re-validates a cached VM before returning it, to avoid handing back a VM whose backing @model was deleted. The probe ran on every cached lookup — a hot path (CuratedHomeViewModel.tiles(from:), CuratedHomeView.tileView(for:), AreaDetailViewModel per-entity) re-run on each render — and fanned out across all registrations, building a throwaway VM per domain (up to 8 SwiftData fetches + one allocation per lookup). Add a cheap `entityExists` closure and a `domain` to ViewModelFactory .Registration, plus a domain index on the factory. The probe now derives the domain from the entity id ("<domain>.<object>") and consults only the owning registration — one fetch, no allocation. Correctness is unchanged: deleted id evicts and returns nil; live id returns the same cached instance. Add a domain-targeting test (a spy registration whose probe must never be called for a light lookup) alongside the existing eviction / same-instance tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
adborbas
force-pushed
the
fix/light-onoff-overlay
branch
from
July 16, 2026 20:42
ede770b to
bec944b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
supported_color_modes == ["onoff"]) no longer get a non-functional brightness slider or fill:LightCardViewModel.supportedModesreturns[]for onoff-only lights.isDimmablegates the medium-tileCardFillOverlay.hasOverlay(!supportedModes.isEmpty) makesmakeOverlayViewreturnnil, so tapping the card body presents no dead panel — the icon toggle remains the control.hasOverlaycapability flag added toEntityCardViewModel(defaultfalse).handleCardTapnow branches on it instead of building an overlay view just to nil-check it. Overridden totruein Cover/Switch/Climate/Automation VMs (every VM whosemakeOverlayViewreturns non-nil); Light derives it fromsupportedModes.ViewModelFactorystale-VM guard:makeViewModel(forEntityId:)re-confirms the backing@Modelstill exists before returning a cached VM. The probe is domain-targeted (via the id's<domain>.<object>prefix) and allocation-free — one cheap fetch against the owning registration rather than a fan-out that built a throwaway VM per domain.Why
"onoff"color mode means the light has no brightness; the old panel showed a 0–255 slider that did nothing and the medium tile drew an always-empty fill.handleCardTapconstructed and discarded an overlayAnyViewon every card tap solely to test!= nil.tiles(from:),tileView(for:)), fanning out across all registrations — up to 8 SwiftData fetches plus an allocation per lookup. It is now one targeted fetch.Notes
wipeLocalData, which discards the whole factory), so there is no per-entity deletion chokepoint to hook.supportedModes/isDimmable/hasOverlayfor lights,hasOverlayfor Switch/Automation (true) and Scene (false), andViewModelFactorycache eviction / same-instance / domain-targeting. FullHemeraTestssuite green (381 tests).