summaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md101
1 files changed, 101 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 5b84b15..bf347c8 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -252,6 +252,8 @@ taking that too literally.
| 178 | Delete and Restore judge a conversation on one message | defect | XS | open, 2026-08-28, split out of item 168 by item 177 and flagged in that spec's "Open, deliberately". `MainWindow::everySelectedRowIsInATrashFolder()` reads `ThreadSummary::firstMessagePath` for any row that is not a message row, which was correct while a thread row MEANT that message and is not correct now that it means the conversation. A conversation is in the trash when ALL of its messages are, so a partly-trashed thread currently answers on whichever message the query returned first: Delete can be hidden on a conversation that still has mail outside the trash, and Restore offered on one that mostly does not. Not data-affecting, both actions are no-ops in the wrong direction, but it is an inconsistency the row-kind rule was supposed to remove. Needs the summary to carry the answer, or the paths of every message, which the digest walk already reads |
| 174 | An external `notmuch new` reaches the index without the pending count noticing | defect | S | open, 2026-08-28, from the notes. Item 54 cleared the count for a sync run by `mailsync.sh`, which is what `SyncMonitor` watches; a bare `notmuch new` (a hand run, or a cron entry that is not the script) takes notmuch's own write lock and touches `/tmp/mbsync.lock` not at all, so nothing observes it. The user's framing is the approach: we own `mailsync.sh` and the whole process |
| 175 | The send countdown says Undo, and cannot be skipped | presentation | XS | open, 2026-08-28, from the notes. Two changes in one control: the button reads Abort, and a second button sends immediately rather than waiting the countdown out |
+| 179 | Undo is one level deep in practice, and there is no Redo | workflow | ? | open, 2026-08-29, from the notes. The `QUndoStack` is real and multi-level; what is missing is a `redo` action (absent from `knownActions()`, never called) and an answer to the stack being CLEARED on every new query (`mainwindow.cpp:3458`), which is what makes a deep stack behave like a shallow one. The clear has a correct reason and cannot simply be removed. Redo re-applies a write to real mail, so item 176's rule binds it too |
+| 180 | The repaint rules are discovered one hole at a time | maintenance | S-L | open, 2026-08-29, from the notes, and a QUESTION rather than a defect. Items 105, 107, 109, 110 and 170 are each one hole in the same surface, all found by hand. Three mechanisms (optimistic repaint, `syncViewMembership()`, revert) agree by documentation rather than by code. Cheapest answer is one invariant test, not a rewrite; the user decides which, and that decides the size |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -1280,3 +1282,102 @@ call the button would make, so skipping is stopping the timer and calling it.
A skip button must not become a default, or the protection is gone for
everyone who learns to press it.
- Which button is the default on Return matters here and is the user's call.
+
+## 179. Undo is one level deep and has no Redo
+
+**Observed (user, from the notes):** "ctrl+Z should work like in any other
+application, keep track of the past N actions and allow to undo them. This
+would also bring in Redo, which would act as the opposite of Undo. As it is
+today it feels hackish and half-implemented."
+
+**Cause, verified in the code.** The stack is real, not a single slot:
+`MainWindow` owns a `QUndoStack` (`mainwindow.h:1434`) and every mutation
+pushes a command onto it, so multi-level undo is already there and works.
+What is missing is the other half of the facility around it:
+
+- **There is no `redo` action at all.** `KeyMap::knownActions()` lists `undo`
+ and nothing else (`keymap.cpp:69`), `Ctrl+Z` is its only binding
+ (`keymap.cpp:201`), and `QUndoStack::redo()` is never called from
+ `mainwindow.cpp`. The `redo()` overrides in the command classes exist only
+ because `QUndoStack` calls them on push; nothing reaches them a second time.
+- **The stack is cleared on every new query** (`mainwindow.cpp:3458`), with a
+ correct reason recorded beside it: the entries invert model updates against
+ rows that are about to be discarded. That is what makes a stack that is N
+ deep behave like one that is one deep, since running a query is the ordinary
+ thing a user does between actions.
+- No `setUndoLimit()` is set anywhere, so the depth is unbounded until a query
+ clears it.
+
+**Approach.** Two separable pieces, and the second is the real work.
+
+1. Add `redo` as an action: `KeyMap::knownActions()`, an icon, a menu entry,
+ and `Ctrl+Shift+Z` as the conventional default. Per CLAUDE.md that is five
+ places, four of them enforced by tests. Small and self-contained.
+2. Decide what survives a query. The clear is not gratuitous, and removing it
+ without an answer reintroduces the half-applied undo it prevents: the
+ database changes and the list does not. The candidate answer is that a
+ command should be able to re-resolve its rows against the current model
+ rather than assuming the ones it was built with are still there, at which
+ point a query no longer invalidates the stack.
+
+**Constraints.**
+- The undo stack is this application's substitute for a confirmation dialog,
+ per CLAUDE.md, so an undo that damages state is worse than the dialog it
+ replaces. Item 176 is exactly that failure and is one commit old.
+- Redo re-applies a write to real mail. It carries the same requirement item
+ 176 established for undo: it must cover what the write actually CHANGED, not
+ what it asked for.
+- `m_awaitingTagConfirmation` holds pointers the stack owns and is cleared
+ beside it (`mainwindow.cpp:3462`). Anything that changes the clear has to
+ keep those two in step or it is a dangling pointer, not a stale row.
+- Sizing the second piece needs the first: `?` until the query-clear question
+ is answered.
+
+## 180. The repaint rules are discovered one hole at a time
+
+**Observed (user, from the notes):** "should we refactor the list UI to be
+responsive so changes are applied immediately instead of waiting for a view
+change to repaint? I feel like we are chasing our own tail with the repaint
+issue, we are effectively plugging holes, every time we have to stumble on a
+view that doesn't refresh when it should, or worse, it refreshes when it
+shouldn't."
+
+**Cause, verified in the code.** This is a maintenance item about a pattern,
+not a defect with a reproduction. The observation is accurate and the history
+is in this backlog: items 105, 107, 109, 110 and 170 are each one hole in the
+same surface, every one found by hand-testing rather than by a test, and each
+fixed in place. The current shape is three separate mechanisms that must agree
+by hand:
+
+- the optimistic REPAINT, `applyTagChange` for a thread and
+ `applyMessageTagChange` for a message, which item 105 established must exist
+ in both scopes and item 107 established must reach loaded replies;
+- the optimistic MEMBERSHIP, `MainWindow::syncViewMembership()`, added by item
+ 170 and called from three funnels (`mainwindow.cpp:5861`, `:7052`, and the
+ deferred path at `:3899`);
+- the revert, `revertPendingTagChange()`, which must cover every scope the
+ other two apply.
+
+CLAUDE.md already carries the rule that keeps them in step ("Every path a
+thread-scoped write travels, a message-scoped one travels too"), which is a
+statement that the coupling is enforced by documentation and review rather
+than by the code.
+
+**Approach.** Not a rewrite. The cheapest thing that would end the pattern is
+a single test that asserts the invariant directly, rather than one test per
+hole: for each scope and each funnel, a write leaves the model, the membership
+and the revert path agreeing. That converts "we stumble on a view that does
+not refresh" from a hand-test finding into a suite failure.
+
+**Constraints.**
+- The deliberate lag documented in CLAUDE.md is NOT a hole and must not be
+ "fixed" by such a test: reading the last unread message of a long
+ conversation leaves the row in place because `applyMessageTagChange` leaves
+ a long thread's summary alone, and judging on a stale union would be wrong
+ in both directions. The invariant has to be written to permit it.
+- Likewise the two deliberate asymmetries: a row is never evicted while it is
+ current, and an automatic write defers eviction where a requested one does
+ not.
+- This needs the user to say whether they want the test or the refactor. The
+ note asks a question rather than reporting a fault, and the answer changes
+ the size from S to L.