Conversation
The per-Session latch counter existed so a bounded bell burst could replay when a second alarm track latched behind an already-ringing one. The bell is gone and the alarm treatment keys its one finite burst on the episode, which the speech and push sinks already treat as the summons: a second track latching inside an episode enriches it and never re-delivers. The counter had no reader left, yet still crossed the host->webview wire, sat in every Activity literal, and was compared by alertStatesEqual — so an enriching latch emitted a state change nothing acted on. Latching now only opens an episode when no track is ringing. The two counter tests become episode-identity tests: a second track latching mid-episode keeps the episode id, and re-latching after all tracks clear starts a new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting the ring counter took the last transient field out of three persistence fixtures, so nothing failed if `toPersistedAlertState` started writing an episode to disk. Each fixture carries an episode again, and the projection now takes the live `AlertState`, which is the type its callers actually hand it — the persisted arm stays only because a record read back off disk may itself carry live fields an older build wrote. `latchRing` no longer latches anything; it opens a delivery episode, and nothing closes one — `getState` masks it while no track rings. Renamed `openEpisode` and pointed its comment at that mask instead of `releaseRing`. Two episode assertions used `episode?.id`, which passes against a null episode; both now require the episode first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
fc733a6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a8e1e7ac.mouseterm.pages.dev |
| Branch Preview URL: | https://alert-drop-ringseq.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark the PR ready when you want the full review. Two notes, both on comments the diff adds; the counter deletion itself traces clean.
openEpisode's doc attributes the behavior to the wrong mechanism. Masking in getState is what hides a stale entry.episode after every track has cleared. What makes a second latch enrich the same summons is the !this.hasActiveRing(entry) guard on the line below, which the comment never names. A later reader who changes the masking would think they had changed the enrichment rule, or could drop the guard believing the masking covers it — and this is the method docs/specs/alert.md now points at as the source of truth for the rule.
toPersistedAlertState's widened parameter does not check anything new. AlertState was already assignable to PersistedAlertState — status and todo are the same types, a required notification satisfies the optional one, and the extra live fields are allowed on a non-fresh object. That is why getActivity(id) type-checked against the old narrow signature, and a union with a subtype collapses to the supertype, so AlertState | PersistedAlertState is the same parameter type the compiler saw before. The one thing it does change is to drop the excess-property check the narrow type applied to inline object literals — the exact shape the comment above it warns about. No current caller passes one, so nothing regresses today; the point is that "the projection is honest at the boundary" in the description reads as new enforcement, and the honesty here is documentary.
| * Open a delivery episode when the first track latches. Nothing closes one: | ||
| * `getState` masks the episode to `null` while no track rings, so a track | ||
| * latching behind an already-ringing one enriches that same summons. |
There was a problem hiding this comment.
| * Open a delivery episode when the first track latches. Nothing closes one: | |
| * `getState` masks the episode to `null` while no track rings, so a track | |
| * latching behind an already-ringing one enriches that same summons. | |
| * Open a delivery episode only when no track is ringing yet, so a track | |
| * latching behind an already-ringing one enriches that same summons. Nothing | |
| * closes one: `getState` masks a stale episode to `null` while no track rings. |
| * build wrote, and re-projecting is what strips them. | ||
| */ | ||
| export function toPersistedAlertState(state: PersistedAlertState): PersistedAlertState { | ||
| export function toPersistedAlertState(state: AlertState | PersistedAlertState): PersistedAlertState { |
There was a problem hiding this comment.
AlertState already satisfied the old PersistedAlertState parameter, so this union collapses to the same type the compiler saw before — see the review body.
2 participants
Stacked on #716.
ringSeqwas a per-latch counter that existed so the retired bell's bounded animation could replay when a second track latched behind an already-ringing one. Every renderer now keys on the alertepisode(or the Workspace union'sringingSince), which the speech and push sinks already treat as the summons. The counter is deleted end to end:AlertState, the entry,alertStatesEqual, the cold seed, the wire, and every test fixture.latchRingno longer latched anything and is nowopenEpisode.a second track latching mid-episode keeps the episode idandre-latching after all tracks clear starts a new episode(covers BEL spam on a latched track).session-save.test.tsand the VS Codesession-state.test.tscarry an episode, and a mutation that copies it through fails both.toPersistedAlertStatetakes the live state type, so the projection is honest at the boundary.alert.mdPublic State reads "additional track latches join the episode"; the two counter rationale paragraphs are gone.Repo-wide grep for
ringSeq: zero hits.🤖 Generated with Claude Code