diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 09:49:07 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 09:49:07 +0200 |
| commit | d128919ad80ecc37b1f58d2ba83a7487362c126f (patch) | |
| tree | 2617a84894f366bdd7c4b866974a7cfb5ab93d4c /docs/superpowers | |
| parent | d7c4d03f7d583767bc23406579e11bcc884dec18 (diff) | |
| download | qtmaildir-d128919ad80ecc37b1f58d2ba83a7487362c126f.tar.gz qtmaildir-d128919ad80ecc37b1f58d2ba83a7487362c126f.zip | |
docs: move CLAUDE.md to AGENTS.md, and reconcile the backlog
AGENTS.md is the single source of truth for how to work on this
repository, shared across every agent tool; CLAUDE.md is now a thin
pointer to it, mirroring the arrangement in the user's own config.
Prose throughout the specs, plans, source comments and tests refers to
this document as CLAUDE.md, which is what it was called until now.
Those references are historical records and are correct about the
content, so they are left alone and AGENTS.md says how to read them.
Two rules added while reconciling:
- The reconcile writes its open-items table to the user's vault as well,
rewritten whole each time so a closed item cannot linger there.
- Build a test only when it asserts something measurable, and hand a
visual change to the user instead. This is the conclusion the
"Rendering probes lie" section was already building toward, and it
sits at the end of it. Guarded against the opposite reading: it does
not license dropping a test that can fail, and items 87, 105 and 176
are each a case where the missing test was the whole problem.
The 2026-08-29 pass over the user's notes found two entries with no item
here, both causes verified in the code rather than copied from the note:
- 179, undo is one level deep in practice and there is no Redo. The
QUndoStack is real and multi-level; what is missing is a redo action
and an answer to the stack being cleared on every new query.
- 180, the repaint rules are discovered one hole at a time. A question
rather than a defect, and the user's own framing: items 105, 107, 109,
110 and 170 are each one hole in the same surface, all found by hand.
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 101 |
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. |
