Repository navigation
fix: close two roadmap items — snapshot retry and reduce motion - #7
Merged
Merged
Conversation
A `session.snapshot` request that failed after an event was swallowed, so the menu bar kept showing the previous state until the next event or the 300 s resync — whichever came first. Observed once, transiently, while herdr was mid-replay. `SnapshotRetry` hands out a short, bounded run of delays (250 ms, 500 ms, 1 s) and then gives up, leaving the resync as the backstop. The budget is a local in `refreshFromSnapshot`, so the event-driven refresh and the resync loop cannot spend each other's attempts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`refreshUI()` already reads `accessibilityDisplayShouldReduceMotion` fresh on every render, but nothing woke the menu bar when the setting itself changed. With agents working and no state change due, turning Reduce Motion on left the spinner running until the next snapshot — up to the 300 s resync away. Subscribing to `accessibilityDisplayOptionsDidChangeNotification` is the missing wake; the reading side needed no change. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Closes the two smallest deferred items in
ROADMAP.md.A failed live snapshot now retries
refreshFromSnapshot()swallowed a failedsession.snapshotand returned,leaving the menu bar on the previous state until the next event or the 300 s
resync.
SnapshotRetry(new, inKelpieCore) hands out a bounded run ofdelays — 250 ms, 500 ms, 1 s — and then gives up, so the resync stays the
backstop rather than being competed with.
The budget is a local in
refreshFromSnapshot, not a stored property: theevent-driven refresh and the resync loop can be in flight at once, and a shared
budget would let them spend each other's attempts. It also means teardown has
nothing extra to reset.
Giving up deliberately does not call
coalescer.didRefresh(). Leaving thewindow open is what makes the next event refresh immediately instead of
debouncing on top of a UI that is already stale.
Reduce Motion is picked up when it changes
refreshUI()already readaccessibilityDisplayShouldReduceMotionfresh onevery render; what was missing was any wake when the setting itself changed.
With agents working and no state change due, turning Reduce Motion on left the
spinner running until the next snapshot. Subscribing to
accessibilityDisplayOptionsDidChangeNotificationsupplies that wake — thereading side is unchanged.
Verification
swift test— 132 tests, all passing, including four newSnapshotRetrytests written before the type existedxcodebuildDebug build of the app target — succeeds, no warningsThe Reduce Motion observer has no automated coverage (
AppCoordinatorhasnone) and has not been exercised at runtime: confirming it means toggling the
system setting against a running build.
🤖 Generated with Claude Code