aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-09 09:31:53 +0200
committerDanilo M. <danix@danix.xyz>2026-08-09 09:31:53 +0200
commit8508ac49d2fe72e362379950b52a64ccd6a6e773 (patch)
tree65137d7a88bdf962acd4ca8bfd4d2b1df73129d5 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
parent69281b218d70737f7a9ce7824f5c0a53d76456e4 (diff)
downloadqtmaildir-8508ac49d2fe72e362379950b52a64ccd6a6e773.tar.gz
qtmaildir-8508ac49d2fe72e362379950b52a64ccd6a6e773.zip
docs: record items 54 and 55 from the user's notes
The backlog is downstream of the user's Obsidian notes and had drifted. Three notes had no entry; two became items and one turned out to be a question with an answer already in the tree. 54, a defect: edits applied before a cron sync still counted as pending. The count is cleared only in the local sync-finished handler, so a run the window did not start left the indicator claiming work that had already shipped, and the exit prompt asked to sync for it. Adjacent to item 49, whose account set goes stale the same way. 55: in a window narrower than the thread view's column widths there is no space left for the splitter's stretch factor to distribute, so the message pane collapses to nothing on first run. The third note asked whether a build can skip the tests. It can: QTMAILDIR_BUILD_TESTS=OFF, which the SlackBuild already passes. Causes verified in the code rather than copied from the notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.md149
1 files changed, 149 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 6beb35b..5d175a5 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
@@ -111,6 +111,8 @@ taking that too literally.
| 51 | Clicking a subject scrolls the list sideways | presentation | XS | open |
| 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | **done** |
| 53 | Message rows still read as a table, not as a conversation | presentation | ? | open, unspecified |
+| 54 | A cron sync carries the edits but the count still says pending | correctness | S | **done** |
+| 55 | In a narrow window the message pane is invisible | presentation | XS | open |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -3238,6 +3240,153 @@ to answer for the strip on thread rows, which must not change.
**Do not start any of these from this description.** It says what is and why,
not what was wanted.
+## 54. A cron sync carries the edits but the count still says pending
+
+**Observed (user, 2026-08-08):** "If I apply some edits in the program, then the
+sync runs from crontab, my edits should go through the sync, instead I still see
+`N changes pending`."
+
+**Cause, verified in code.** The edits really do go out; only the count is
+wrong. A tag edit reaches the notmuch index at edit time, so a `notmuch new`
+fired by cron carries it to the mail store exactly as a local sync would. But
+the pending count is cleared in **one place only**: the success branch of the
+local sync-finished handler, `m_pendingTagEdits.clear()` at
+`src/mainwindow.cpp:1719`, reached from the `MailSync` process this window
+started.
+
+The external path never touches it. `onExternalSyncStateChanged()`
+(`src/mainwindow.cpp:1852`) is driven by `SyncMonitor` watching the lock in
+`/proc/locks`. On `State::Idle` it clears `m_externalSyncBusy`, calls
+`flushHeldEdits()`, and shows "Background sync completed. Press Enter in the
+query bar to refresh." (`src/mainwindow.cpp:1901`). It does not clear
+`m_pendingTagEdits`, does not reset `m_unnettablePendingEdits`, and does not
+drain `m_editedAccounts`. So the indicator keeps counting edits that have
+already shipped, until the user runs a sync from the window.
+
+This is a **defect, not an enhancement**, and it is the same class as item 28:
+the indicator exists to answer "is my work safe to quit on", and here it says no
+when the answer is yes. It also feeds the exit prompt (`pendingEditCount()` at
+`src/mainwindow.cpp:173`), so the user is asked to sync on quit for work that a
+cron run already carried. Adjacent to item 49, which made the account set drive
+which channels a sync runs: `m_editedAccounts` is stale in exactly the same way.
+
+**Approach.** Clear the same three pieces of state on an observed external
+`Idle` that a successful local sync clears. Two things make this harder than
+copying the block, and both must be answered before it is written:
+
+- **The monitor sees a lock, not an outcome.** A cron run that fails releases
+ the lock exactly as a successful one does, and the comment at
+ `src/mainwindow.cpp:1715` records the rule that only a *successful* sync may
+ clear the count. An external run's exit status is not observable from
+ `/proc/locks`. Either the count is cleared optimistically on any external
+ release, or `mailsync.sh` grows a status file the window can read. Ask before
+ choosing: the optimistic version can clear a count whose edits a failed cron
+ run did not carry.
+
+ **Resolved 2026-08-09, and the dilemma was false: the script already writes
+ the outcome.** Every run ends with a
+ `===== RUN END: <ts> status=OK =====` or `status=FAILED mbsync=<n>
+ notmuch=<n>` banner in its log (`assets/mailsync.sh:115-117`), which survives
+ the process that wrote it. `MailSync::lastRunOutcome()` reads it, so neither
+ the optimistic clear nor a change to the sync script was needed. The user
+ chose this over both alternatives.
+- **The held-edit race is already solved for the local path and must hold
+ here.** `flushHeldEdits()` is called on the external `Idle` branch too, and it
+ calls `sendThreadTagChange()`, which writes `m_editedAccounts`
+ **synchronously**, while the pending map is written on the worker's queued
+ reply. The local path handles this by snapshotting `m_editedAccounts` before
+ the flush (`src/mainwindow.cpp:1711`) and subtracting only the snapshot.
+ Anything written here has to do the same, or edits made during the cron run
+ are marked as carried by the sync that did not carry them.
+
+**Constraints.**
+
+- `State::Unknown` must not clear anything. It means `/proc/locks` could not be
+ read, so nothing was observed, and the existing code is careful to distinguish
+ that from `Idle`.
+- The local path must keep working unchanged, including the `m_localSyncHoldsLock`
+ early return at `src/mainwindow.cpp:1874`, which exists so a local sync's own
+ lock release is not mistaken for an external one.
+
+**Verification.** `test_mainwindow` can drive `onExternalSyncStateChanged()`
+directly, which is how the existing external-sync tests work, so this does not
+need a real cron run. Assert on the count and on the exit prompt, and mutate:
+the test must fail if the clear is removed. Item 49's account-set behaviour
+needs its own assertion, since a count that reaches zero while
+`m_editedAccounts` stays full would look correct and still sync the wrong
+channels.
+
+**Built 2026-08-09.** `SyncOutcome` and `MailSync::lastRunOutcome()` parse the
+banner; `Config::syncLog()` supplies the path, defaulting to the script's own
+and overridable through a new `[sync] log` key so a test never reads the
+developer's real log. `onExternalSyncStateChanged()` clears the map, the
+unnettable counter and the account set on `Idle` **and** a definite `OK`, before
+`flushHeldEdits()`, matching the local path's ordering. 13 tests across
+`test_mailsync`, `test_config` and `test_mainwindow`; 15/15 binaries green.
+
+Two notes on the verification, both worth more than the passing count:
+
+- **A timing probe endorsed a tail read that was not happening.** The first
+ version of `lastRunOutcomeReadsATailOfAHugeLog` required the call under
+ 100 ms, and it **passed with the seek deleted**, because reading 10 MB is
+ fast either way. It measured nothing, exactly as CLAUDE.md's rendering-probe
+ entry describes. Replaced with an assertion on content: a marker reachable
+ only from the head of a large file must be invisible to a tail read, plus a
+ guard appending a marker within reach to prove the parser can still find one.
+ That version fails when the seek is removed.
+- **Every fixture was invented, and the first batch was wrong.** They wrote the
+ banner as `RUN END: 2026-08-09 10:20:03`, where the script uses
+ `date -Iseconds` (`assets/mailsync.sh:111`) and so emits
+ `2026-08-09T09:20:33+02:00`. The tests passed anyway, because the parser keys
+ on the `===== RUN END:` prefix and the `status=` token and never looks at the
+ timestamp. Found only by reading the user's real log while waiting for a cron
+ run, not by any test. A fixture invented to match the code tests the code
+ against itself. `lastRunOutcomeReadsABannerTheScriptActuallyWrote` now builds
+ the line by running `date -Iseconds` the way the script does, and the parser
+ was confirmed against the real `~/.local/state/mailsync.log`, which it reads
+ as `Ok`.
+- **The ordering claim is not covered by any test.** With no lock file present
+ `aSyncHoldsTheWriteLock()` is false throughout the suite, so `flushHeldEdits()`
+ is a no-op and moving the clear after it changes no result. The ordering is
+ inherited from the local path rather than independently verified; a test for
+ it needs a held lock, the way item 37's tests stage one.
+
+## 55. In a narrow window the message pane is invisible
+
+**Observed (user, 2026-08-08):** "when opening in a squared window, the left
+pane takes the whole width (right pane almost invisible)".
+
+**Cause, verified in code.** The splitter is built at
+`src/mainwindow.cpp:584` with `setStretchFactor(1, 2)` and **no**
+`setSizes()`, and neither pane carries a minimum width. A stretch factor only
+distributes space left over **after** every child's size hint is satisfied. The
+thread view's hint follows its columns, which are set to fixed widths at
+`src/mainwindow.cpp:557-561` and total roughly 886px (28 + 28 + 130 + 180 +
+520), every one of them `Interactive` with `setStretchLastSection(false)` at
+`src/mainwindow.cpp:513`. In a window narrower than that sum plus a usable
+message pane there is no leftover space at all, so the stretch factor never
+applies and the thread view takes essentially everything.
+
+This only bites on first run. Once a splitter position is saved it is restored
+from `window/splitter` (`src/mainwindow.cpp:120-123`), which is why the window
+behaves after the user has dragged it once. Item 1 built that persistence; it
+does not supply a sane starting point.
+
+**Approach.** Give the message view a minimum width so the splitter cannot
+collapse it, and set an initial `setSizes()` proportional to the window rather
+than letting the hints decide. Sizes only apply when nothing is restored, so it
+must go behind the same "no saved state" check as the geometry.
+
+**Constraints.**
+
+- Must not fight the restore path. A `setSizes()` call that runs after
+ `restoreState()` would throw away the user's saved position, which is item 1.
+- A minimum width on the thread view as well would reintroduce the same problem
+ from the other side. Only the message pane needs one.
+- `resize(1200, 800)` at `src/mainwindow.cpp:206` is the default only when no
+ geometry is saved; the reported case is a user-sized window, so widening the
+ default fixes nothing.
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering