diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 85 |
1 files changed, 85 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 27b4e28..b3cda60 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -9374,3 +9374,88 @@ assertion after it means something, and mutation-checked by removing the call. Note the failing runs take 45s and the passing one 295ms: the `QTRY` timeouts expire when the refresh never comes, which reads as a slow test rather than a missing feature. + +## 182. An edit made during a sync is announced twice and never says it is waiting + +**Done 2026-08-29**, unreleased, on `thread-row-identity`. + +**Observed (user, by hand, 2026-08-29):** a thread of 9 messages with 5 unread, +marked read while a sync was running. The status bar showed "<subject>: mark as +read"; at the end of the sync it showed the 5 messages reported again. "Double +reporting before/after the sync." + +**Cause, verified in the code.** Not a double write, and the mail is correct. +It is one action reported twice because the FIRST report is the wrong one. + +A sync holds notmuch's exclusive write lock, and the worker's read-write open +blocks on it rather than failing, so an edit made during a sync is HELD and +sent when the lock frees. All three hold branches say so, in a label chosen +deliberately: + +- `sendThreadTagChange()` (`mainwindow.cpp:7122`) +- `sendMessageTagChange()` (`:5937`) +- `sendMove()` (`:6722`) + +Each sets `m_statusLabel` directly to "A sync is running; your change will be +applied when it finishes.", with a comment stating that this is NOT transient +because it describes state lasting until the sync ends, and that a message +which expired would leave the user with rows showing a tag the database has not +got and no explanation of why. + +That label never survives. Every caller announces the action itself immediately +afterwards, through `showTransientStatus()`, which overwrites it: + +- `markThreadsRead()` (`:3641`), the path in the report +- the tag-action funnel (`:5846`) +- `deleteSelection()` (`:6104`) + +So the user is told "mark as read", which claims the write happened, and the +hold is never mentioned. When `flushHeldEdits()` later reports "N held changes +sent now that the sync has finished" (`:4580`), that reads as a second report +of the same action rather than as its completion. + +The two counts differ (threads against messages) because the two messages count +different things, which is why the report describes 5 messages where the first +message named the thread. + +**Approach.** The caller must not overwrite a hold that the funnel has just +explained. `aSyncHoldsTheWriteLock()` is already a const predicate, so a caller +can ask it directly and skip its own announcement; no funnel signature changes. + +The flush message stays. It is the only signal that held work actually landed, +and its absence was item 106, silent data loss on this exact path. + +**Constraints.** + +- The hold label must stay NON-transient, for the reason its own comment gives. + A fix that makes it transient reintroduces the unexplained state it exists to + prevent. +- Three call sites, and a fourth path (`sendMove`) whose callers must be checked + rather than assumed: the same defect is one caller away in each. +- The announcement is what stands in for the confirmation dialog this project + rules out, per the comment at `:5842`: it tells the user when something larger + than they meant has just happened. Suppressing it entirely during a sync would + cost that, so what replaces it has to name the action AND the hold, not drop + the action. +- The two messages counting different things (threads against messages) is a + separate readability question and is NOT part of this item. + +**Built as `MainWindow::announceAction()`**, which the three sites call in +place of `showTransientStatus()`. It asks `aSyncHoldsTheWriteLock()` and, when +one does, sets a non-transient label reading "<action>, waiting for the running +sync to finish". The action is still named for the reason the constraint above +gives: that announcement is what stands in for the confirmation dialog this +project rules out, so the hold is ADDED to it rather than replacing it. The +flush message at the end of the sync is untouched, and now reads as the +completion of the first message rather than a repeat of it. + +The test drives `toggle_unread`, the route the user took, after putting the +window into the state the sync monitor puts it in. It asserts BOTH halves: the +text mentions the sync, and it still says what is waiting. Asserting only the +first would pass against an announcement that dropped the action entirely, +which is the thing the constraint forbids. Mutation-checked by forcing the +non-held branch, which fails with the exact text the user reported. + +The string is translated, since a user-facing string that misses the Italian +translation ships as English inside an otherwise Italian UI: `lupdate` found it +with no context warnings and `lrelease` reports 552 finished, 0 unfinished. |
