Fix thread snooze and give snoozed threads their own sidebar shelf - #328
Conversation
displayEquivalent(to:) gates sidebar array rewrites, and the thread-perf port never taught it about snoozedUntil/snoozedAt — so a thread.snoozed upsert differed only in fields the gate ignores and was swallowed as an activity-only bump. Snoozing a thread did nothing in the UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Snoozed threads used to ride the settled disclosure, which misread "waiting until later" as "done". They now get a dedicated Snoozed shelf per project — soonest wake first, wake-all on hover, and a status label that says when the thread comes back. The header summary and meter gain a matching snoozed band so the counts agree with the rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sidebar-snooze UIProbe scenario drives the full flow — right-click, Snooze page, preset commit, reclassification into the Snoozed disclosure, wake — and is what reproduced the swallowed snooze upsert. The live E2E protocol flow now also round-trips thread.snooze/thread.unsnooze through a real server and asserts the shell upsert carries the wake time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The macAppUpdate advisory test exercises a host that reports "none", but HostApplicationDescriptor only admitted "sparkle", so mobile typecheck failed on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SergeSerb2
left a comment
SergeSerb2
left a comment
There was a problem hiding this comment.
The snoozed shelf implementation is generally coherent, but the UI probe’s expected row-count calculation does not account for snoozed rows after their disclosure is opened. This can make the probe report false failures for valid snooze behavior. The diff is complete.
SergeCode auto-review · model=codex/gpt-5.6-luna · head=64751c529c01
SergeSerb2
left a comment
SergeSerb2
left a comment
There was a problem hiding this comment.
The sidebar classification, disclosure wiring, and wake-all behavior look consistent. I found no blocking correctness or security issues in the reviewed diff. The new snooze flow would benefit from unit coverage for projection/summary classification and wake ordering.
Could not anchor
- info
apps/mac/Sources/SergeCodeMac/UI/Shell/SidebarPresentation.swift— Please consider adding focused tests coveringgroupThreadsandSidebarProjectSummarywhen threads are snoozed, expired, or mixed with settled threads. This protects the new active/snoozed/settled partition and its total/count invariants.
SergeCode auto-review · model=codex/gpt-5.6-luna · head=64751c529c01
SergeSerb2
left a comment
SergeSerb2
left a comment
There was a problem hiding this comment.
The new shelf wiring is coherent, but the projection currently moves attention/error threads into Snoozed, contradicting the row-label behavior and potentially hiding actionable failures. Add an attention-priority guard (and regression coverage) before merging.
SergeCode auto-review · model=codex/gpt-5.6-luna · head=64751c529c01
SergeSerb2
left a comment
SergeSerb2
left a comment
There was a problem hiding this comment.
The snoozed shelf classification is structurally sound, but the PR does not provide a time-based invalidation path for snoozes to leave the shelf automatically. The implementation explicitly relies on Date() during projection while noting that timer expiry emits no event, so a snoozed thread can remain displayed until an unrelated model/view update occurs. Please add a lightweight refresh/timer tied to the nearest snooze expiry and cover it with a focused test.
SergeCode auto-review · model=codex/gpt-5.6-luna · head=a60202e4467e
| // snoozedUntil passes (timer wakes emit no event) or the server | ||
| // clears it on activity, and parking it among the settled rows | ||
| // misread as "done" what is really "waiting". | ||
| if ThreadInboxSemantics.isSnoozed(item.thread, now: now) { |
There was a problem hiding this comment.
important: isSnoozed(..., now: now) is only evaluated when the sidebar recomputes, but snooze expiry itself does not mutate the model or emit an event (as acknowledged by the comment above). Consequently, after snoozedUntil passes, the thread can remain in the Snoozed shelf indefinitely until another unrelated update causes a render. Add a scheduled invalidation/refresh at the next snooze expiry (or equivalent observable clock), and add a test proving the thread moves back to active as time advances.
1 participant
What changed
Bug fix — snoozing a thread did nothing.
ChatThread.displayEquivalent(to:)gates sidebar array rewrites so activity-onlyupdatedAtbumps don't invalidate every row. The thread-perf port (#317) addedsnoozedUntil/snoozedAtto the model but never taught the gate about them, so athread.snoozedshell upsert — which differs only in those fields plusupdatedAt— was judged display-equivalent and silently dropped. The Snooze menu action reached the server, persisted, and then visibly did nothing until an app restart. Fixed by comparing the snooze fields indisplayEquivalent.Feature — dedicated "Snoozed" disclosure. Snoozed threads used to ride the per-project "Settled" disclosure, which misread "waiting until later" as "done". Each project section now has its own Snoozed shelf: soonest wake first, a wake-all hover action (mirroring settled's archive-all), "Snoozed until …" status labels, and a matching snoozed band in the project header summary/meter so counts agree with rows.
Also: widened
HostApplicationDescriptor.updateCapabilityto"sparkle" | "none"— the runtime guard and its test already assumed "none" exists, and the narrow literal failed repo-wide typecheck.How it was verified
SERGECODE_UI_PROBE_SCENARIO=sidebar-snoozeprobe drives the real UI against the mock backend: right-click a thread row → Snooze submenu page → first preset → assertssnoozedUntillands, the row leaves the active split for the Snoozed disclosure, the disclosure reveals, and waking returns the row. This scenario reproduced the swallowed upsert before the fix and passes after.SERGECODE_LIVE_E2E=1, real server sidecar):liveEndToEndProtocolFlownow round-tripsthread.snooze/thread.unsnoozeover the wire and asserts the shell upsert carries (then clears) the wake time. Passed.displayEquivalentmust flag snooze-only diffs; a snooze-only upsert must land inmodel.threads; snoozed threads split into their own bucket ordered by soonest wake.pnpm run verify --allgreen (typecheck, lint, 737-test mac suite, server + TS suites, lint:mobile).Scope note: mobile still groups snoozed threads with settled; this PR changes the macOS sidebar only.
🤖 Generated with Claude Code