Make control surfaces reflect server truth (no panel-local optimism) - #29
Merged
Conversation
Per the control-surface-state-pattern decision (Option A): - Switch panel (Rule 1): drop the local @State isOn mirror + onChange reconcile; the power button now binds directly to viewModel.isOn, so a failed service call can no longer leave it stuck disagreeing with the card. - CommitCooldown (Rule 2): replace the pure time-computed isSuppressed with a stored, observed flag driven by a cancellable expiry timer. Window expiry is now observable, so after a failed light/climate slider commit the VM re-reads the model and onChange reverts the slider (~1s). Re-commits debounce by cancelling the prior timer. Duration stays injectable for tests. - Cover: adopt the shared CommitCooldown with a pendingPosition so its position slider reconciles on failure like Light/Climate. Tests: new CommitCooldownTests (immediate suppress, expiry, recommit debounce) plus VM-level tests that brightness/targetTemp/position return the pending value while suppressed and revert to the model after expiry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the multi-line comments introduced in the previous commit to the project's /** ... */ block convention. Single-line // comments untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two fixes from the review of the control-surface-server-truth change: - Cover (finding 2): position reverted to server truth (cover.currentPosition) so the card tile, subtitle, and iconTapped never reflect an unconfirmed value. The slider's optimistic-with-failure-reconcile behaviour moves to a dedicated pending-aware sliderPosition, which CoverControlPanel now observes. - Light/Climate (finding 1): one CommitCooldown gates several pending values, so each commit now clears the others first (resetPending). Otherwise a stale pending from an earlier commit resurfaced and masked server truth when a different property was committed within the shared window. Tests: cover asserts position stays server truth during a pending commit and sliderPosition reconciles on expiry; a new light test locks in the no-stale- pending behaviour. Timing-dependent tests now await CommitCooldown.expiryTask instead of racing a fixed sleep against the scheduler (removes flakiness). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
@State isOnmirror +onChangereconcile; the power button now binds directly toviewModel.isOn. KeptisPressedlocal.isSuppressedwith a stored, observed flag driven by a cancellable expiry timer, so window expiry is observable and re-commits debounce (window measured from the last commit).durationis injectable for tests;expiryTaskis exposed read-only so tests await expiry deterministically.brightness/targetTempre-read server truth and the panel'sonChangereverts the slider. Each commit clears the other pending values so a stale pending can't resurface under the shared cooldown.positionstays server truth (card, subtitle,iconTapped); a dedicated pending-awaresliderPositiondrives the panel slider so it reconciles on failure without ever showing unconfirmed state on the card.Why
Control panels kept panel-local optimistic state that reconciled only via
onChangekeyed on the model — which fires on success but not on failure. A rejected/failed service call left a toggle or slider showing a state the server never accepted, disagreeing with the card behind it. Per the recorded decision (control-surface-state-pattern.md, Option A), control surfaces reflect the@Model(server truth); the UI never shows unconfirmed state. Toggles bind directly; sliders keep local drag state but reconcile on completion including failure.Notes
CommitCooldownTests(immediate suppress, expiry, re-commit debounce) plus VM-level tests thatbrightness/targetTemp/sliderPositionreturn the pending value while suppressed and revert to the model on expiry; a cover test assertspositionstays server truth during a pending commit; a light test locks in the no-stale-pending behaviour.HemeraTests: 376 tests pass.