aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
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.md85
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.