diff options
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 114 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-12-tagging-rules-design.md | 341 |
2 files changed, 432 insertions, 23 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 0a1f4dc..c0c1c1e 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 @@ -100,7 +100,7 @@ taking that too literally. | 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | **done** | | 42 | "Syncing..." says nothing about what is being synced | feedback | S | **done** | | 43 | No "Mark all read" for the current view | workflow | S | **done** | -| 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified | +| 44 | No way to manage the filters applied at sync time | workflow | M | open, specified 2026-08-12; see `specs/2026-08-12-tagging-rules-design.md`. Spans this repo and `mailctl` | | 45 | Two Sync buttons, and only one of them works properly | correctness | S | **done** | | 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** | | 47 | The query bar looks unfinished, and cannot be cleared by mouse | presentation | XS | **done** | @@ -117,7 +117,7 @@ taking that too literally. | 58 | `message_zoom` documents a 0.5 to 3.0 range and enforces none of it | correctness | XS | **done** | | 59 | Archive and Mark all read shipped with the same icon | presentation | XS | **done** | | 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done**; already fixed by 5487d58, see below | -| 61 | `test_mainwindow` fails intermittently, about 1 run in 20 | testing | S | open; predates the card list, reproduced on f72dba9 | +| 61 | `test_mainwindow` fails intermittently, about 1 run in 20 | testing | S | open; cause established 2026-08-11 (the user's cron sync holds the mbsync lock), fix is item 38's seam applied suite-wide | | 62 | No config option for the date format on a card | presentation | XS | **done** 2026-08-11 | | 63 | No way to see sent mail, and no filter for it | workflow | M | **done** 2026-08-11; see `specs/2026-08-11-sent-mail-design.md` | | 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 | @@ -130,6 +130,7 @@ taking that too literally. | 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | | 72 | No khard/khal integration | workflow | ? | open, unspecified; the user places it after send, so v2 at the earliest | | 73 | This backlog is past four thousand lines | maintenance | S | open | +| 74 | "Searching..." keeps claiming a query is running while rows are already arriving | feedback | XS | open; cause measured 2026-08-11, the delay itself is the cold page cache and is not fixable here | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -3009,15 +3010,35 @@ So the item is not "expose the existing filters in the UI". It is one of: a change to what this application is: v1 is read-and-organize over an index someone else fills. -**Next step: ask the user which, and where the rules live today.** The -dry-run request is the strongest hint that they have something in mind that -already runs, since a dry run only makes sense against rules that exist. - -**Constraint if it is built here:** `CLAUDE.md` records that this project does -no network protocol work at all and that fetching is external. A filter engine -that rewrites the Maildir would not violate that literally, but it would put -qtmaildir in the business of moving mail, which is a decision to take -deliberately rather than by implementing a dialog. +**Answered and specified 2026-08-12.** It is the first option. The rules exist, +in the notmuch `post-new` hook inside the user's Maildir: hand-written +`notmuch tag` lines, each scoped to `tag:new`, tag-only by design. They carry +substantial reasoning in shell comments about which senders each rule +deliberately excludes. + +The design is `specs/2026-08-12-tagging-rules-design.md`. In short: the rules +move to `~/.config/mailrules/rules.json`, a tool-neutral store both qtmaildir +and `mailctl` read, with unknown fields preserved across a write by either tool +so neither owns the format. `post-new` becomes a Python loop over that file, +living in the mailctl repository. A rule stores no scope, so the same rule +serves the hook (scoped `tag:new`), a dry run (whole corpus, counts only) and a +future backfill. qtmaildir gets `TagRules` plus a management dialog; mailctl +gets read-only `rules list|show|dry-run`. + +**The constraint below was considered and is not triggered.** No rule engine is +built in qtmaildir and nothing rewrites the Maildir: the tagging still happens +in the notmuch hook, and qtmaildir edits the rule file and counts matches. + +**Backfill is deliberately out of v1**, per the user's decision, and is the one +piece that will force a revision to `CLAUDE.md`'s "no destructive-action +confirmation, undo instead" rule, which the user has said is due for revision +anyway. A rule that is safe against arrivals is not safe unscoped. + +**Constraint if a rule engine were ever built here:** `CLAUDE.md` records that +this project does no network protocol work at all and that fetching is external. +A filter engine that rewrites the Maildir would not violate that literally, but +it would put qtmaildir in the business of moving mail, which is a decision to +take deliberately rather than by implementing a dialog. ## 45. Two Sync buttons on the main window @@ -4038,18 +4059,20 @@ suite passed 15/15 immediately after failing twice on the same binary. What distinguished the failing runs was other work happening on the machine at the time, which points at timing rather than at leaked state between tests. -**Cause: not established.** Both tests are timer- and QProcess-driven, and the -obvious hypothesis is that a `QTimer` or a `runExternalSync` step is being given -a deadline that a loaded machine misses. That was not confirmed: synthetic CPU -load across four cores did not reproduce it in 8 runs, so the trigger is -narrower than "the machine is busy". Do not fix this from the hypothesis alone. - -**Approach.** Run the binary in a loop with `-v2` until it fails and keep the -full log, rather than re-running until it passes. The two assertions are about -state that should be set synchronously by the time they run, so the question to -answer first is which step is actually asynchronous: if `sendThreadTagChange` -reaches the worker on a queued connection, the test may be asserting before it -lands, and the fix is a `QTRY_COMPARE` rather than a longer timeout. +**Cause: established 2026-08-11, and it is the user's own cron sync.** The +trigger is another process holding the mbsync lock while the suite runs, not +machine load: measured 0 failures in 30 runs with no lock held, and 30 failures +in 30 runs with one held. Reproduce deterministically with +`flock /tmp/mbsync.lock -c 'sleep 60'` in one shell and the suite in another. +This supersedes the earlier "not established" reading and the load hypothesis, +which synthetic CPU load had already failed to confirm. + +**Approach.** Item 38 already built the seam: `MainWindow::setLocksPathForTesting` +is a static hook that points the lock check at a path the test controls, so a +test that sets it cannot see the user's real sync. The fix is giving the rest of +the suite that same seam, most likely from a fixture or an init hook rather than +per-test, so a newly added test gets it without having to remember. One of item +71's tests already uses the seam, so there is a worked example to copy. **Constraint.** A flaky test is worse than a missing one, because it teaches everyone to re-run the suite instead of reading it. This one already cost a @@ -4584,6 +4607,51 @@ to stay findable under its number. **Size: S**, and it is bookkeeping, so it competes with real work rather than blocking it. +## 74. The first query after boot sits on "Searching..." for seconds + +**Observed (user, 2026-08-11):** the first start of the day takes noticeably +longer to show its default view, and the delay happens while the status bar +reads "Searching...". + +**Cause: the notmuch index paging in from disk, not this application's code.** +Measured by instrumenting `main()`, the `MainWindow` constructor and +`NotmuchWorker::runQuery` behind an environment variable, then running the same +`tag:inbox` query (4444 threads) warm and again after evicting the index from +the page cache with `posix_fadvise(POSIX_FADV_DONTNEED)`: + +| phase | warm | cold | +|---|---|---| +| `notmuch_query_search_threads` returns | 0 ms | 411 ms | +| first batch of 200 reaches the model | 10 ms | 642 ms | +| walk complete, all 4444 threads | 154 ms | 5714 ms | + +A 37x difference over the identical code path. The index measured 1.1 GB. The +in-process startup costs nothing by comparison: `QApplication` in 20 ms, the +whole `MainWindow` constructor in ~120 ms, and the window is shown and +interactive at ~196 ms in both the warm and the cold run. + +**There is nothing to fix in the query path**, and the measurement exists mainly +so this is not re-investigated. Two things it did establish that are worth +keeping. The default startup view is whichever saved query `startup_query` names, +defaulting to `Unread`, so a user with an empty unread view never sees this at +all and a user whose default is Inbox always does. And batching already works: +rows land from 642 ms cold, long before the 5714 ms finish. + +**The one real defect it exposed is the status bar.** "Searching..." is set once +in `runQuery` and cleared only on `queryFinished`, so it keeps claiming the +query is running for the full 5.7 s while rows are visibly arriving behind it. +That makes a slow query read as a frozen one. The fix is to update the text per +batch with the count so far rather than holding one string, which changes no +timing and only stops the bar from lying. + +**The user declined this on 2026-08-11**, having asked for the explanation +rather than a change. Recorded as open because the status bar is still +inaccurate, not because anything is expected to happen. + +**Size: XS** for the status bar. The cold-cache cost itself is not addressable +here and should not be attempted: prefaulting 1.1 GB at startup to make one +query look fast is a worse trade than the wait. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/docs/superpowers/specs/2026-08-12-tagging-rules-design.md b/docs/superpowers/specs/2026-08-12-tagging-rules-design.md new file mode 100644 index 0000000..1d32f02 --- /dev/null +++ b/docs/superpowers/specs/2026-08-12-tagging-rules-design.md @@ -0,0 +1,341 @@ +# Shared tagging rules: design + +Backlog item 44, "No way to manage the filters applied at sync time". + +**Status:** design approved 2026-08-12, not implemented. + +## The problem this solves + +The user asked for a way to "manage filters to be applied when syncing (view +existing, edit, delete, create new, copy as new, dry-run)". The backlog recorded +the item as unspecified for a week because nothing in qtmaildir applies rules at +sync time, and the item could not be planned until it was known whether such +rules existed anywhere. + +They do. They live in the notmuch `post-new` hook inside the user's Maildir, as +a sequence of hand-written `notmuch tag` commands, each scoped to `tag:new` so +it applies to newly indexed mail only. The hook is tag-only by design: it never +removes `inbox` or `unread`, so nothing is archived or marked read unattended. + +Those rules are invisible from every tool. They are also where a substantial +amount of hard-won reasoning lives, in comments recording which senders were +deliberately excluded from a rule and why. That reasoning is readable only by +opening the hook in an editor, and it is exactly the context a person needs when +deciding whether a rule is still correct. + +## What is being built + +A rule store both tools read, in a format neither owns. + +``` + ~/.config/mailrules/rules.json + | + +---------------+---------------+--------------+ + | | | | + post-new qtmaildir qtmaildir mailctl rules + (mailctl) TagRules dialog list/show/ + | read+write dry-run dry-run + | | | | + scope: atomic via read-only + tag:new QSaveFile NotmuchWorker + | (generation- + notmuch tag stamped) + per rule + | + -new consumer + (hardcoded, + only if all + rules ran) +``` + +The constraint that shaped this: qtmaildir is the main consumer of the mail +system, but mailctl must keep working through it. A format with one +implementation is not neutral, it is qtmaildir's format that mailctl is invited +to parse. Both readers are therefore written together, and the format carries an +explicit provision for fields one tool does not understand. + +## The file + +`$XDG_CONFIG_HOME/mailrules/rules.json`, falling back to +`~/.config/mailrules/rules.json`. Neither tool hardcodes a home directory, and +the location is under neither project's own config directory. + +```json +{ + "version": 1, + "rules": [ + { + "id": "account-work", + "stage": 10, + "enabled": true, + "add": ["account-work"], + "remove": [], + "query": "path:\"work-account/**\"", + "note": "Account tags use the maildir spelling, not the account keys another tool shows. Must run before topic rules." + }, + { + "id": "notify-forge", + "stage": 50, + "enabled": true, + "add": ["notify/forge"], + "remove": [], + "query": "from:notifications@example.com", + "note": "All repositories, not one project." + } + ] +} +``` + +JSON, and not the INI the item originally named. Both tools parse JSON with no +new dependency: `QJsonDocument` in Qt, `json` in the Python standard library, +and mailctl is deliberately stdlib-only. INI was rejected on a concrete hazard +rather than taste: tag names in use contain `/` (`notify/forge`, +`mailing-list/*`, `shopping/*`), and `QSettings` treats `/` in a section name as +a group separator. `CLAUDE.md` already records that trap costing this project +once, over the `[general]` section. + +The cost of JSON is that the file cannot carry loose comments. This is +acceptable because every comment in the current hook is attached to a specific +rule, and `note` gives it a home that a UI can display. The reasoning stops +being visible only to someone reading shell. + +### Fields + +| Field | Meaning | +|---|---| +| `version` | Format version, currently `1`. A reader finding a higher number refuses the file rather than guessing at semantics. | +| `id` | Stable unique handle, `[a-z0-9-]`. What a UI selects and a diff tracks. | +| `stage` | Integer, ascending. Ties broken by array order. | +| `enabled` | A rule switched off without losing its note. | +| `add` / `remove` | Tags, as separate arrays. | +| `query` | notmuch query. Carries no scope and no `tag:new`. | +| `note` | Why this rule is shaped the way it is. | + +**`id` is never the tag name.** Tags contain `/`, and a tag can be renamed while +the rule stays the same rule. + +**`add` and `remove` are arrays, not `+tag`/`-tag` strings.** Every current rule +only adds, but the format should not need a version bump the first time a rule +wants a removal, and parsing sigils off strings invites the class of bug where a +tag legitimately beginning with a sigil character is mangled. + +**Unknown keys are preserved across a write by either tool.** A reader stashes +every field it does not recognise and a writer merges them back. Without this, +one qtmaildir save silently strips whatever a newer mailctl wrote, and the file +is neutral in name only. + +## Scope belongs to the runner, not the rule + +A rule is a `(add, remove, query, stage)` tuple that knows nothing about which +mail it applies to. Whoever runs it supplies the scope. + +| Runner | Scope | Effect | +|---|---|---| +| `post-new` | `tag:new` | Tags newly indexed mail. | +| Dry run | none | Counts against the whole corpus. | +| Backfill (future) | none | Applies to the whole corpus. | +| A future timer or hook | its own | Not designed here. | + +This is why stored queries omit `tag:new`. A query that carried its own scope +would have to be stripped back down for a dry run to be useful, and a +hand-written rule that forgot the scope would silently retag the entire archive. + +It also settles where the `tag:new` consumer lives. `notmuch tag -new -- tag:new` +is not a tagging rule, it is the `post-new` runner's own cleanup, and it is +hardcoded there rather than stored as data. It cannot be deleted, disabled or +reordered by either tool because it is not a rule. The failure mode if it went +missing justifies putting it out of reach: `tag:new` would accumulate, and every +rule would begin matching the whole backlog instead of new arrivals. + +Rules genuinely untied to `tag:new` already exist as manual work today: +retroactively applying a newly written rule, and re-running a corrected rule +over history after a mistake. Both are the same rule at a different scope, which +is what this split makes expressible. + +## The runner + +Today's hook is a sequence of hand-written `notmuch tag` lines. It becomes a +loop over `rules.json`. + +**Python, not shell.** The hook must parse JSON. Shell would mean either a new +`jq` dependency on a path that runs every ten minutes, or hand-parsing JSON in +`sh`. Python 3 is already required by mailctl and needs no imports beyond the +standard library here. + +**It lives in the mailctl repository, installed to `~/bin/`.** The hook is a +mail-organization concern and mailctl is the mail-organization tool. Note the +asymmetry with `mailsync.sh`, which lives in qtmaildir precisely because +qtmaildir runs it as a subprocess and depends on its behaviour; here the +relationship is reversed, since qtmaildir never invokes the hook and `notmuch +new` does. + +``` +mailctl repo + mailrules.py load/save/validate/stage-order, unknown-key preservation + post-new reads rules.json, applies with tag:new scope, then -new + mailctl.py mailctl rules list|show|dry-run + +qtmaildir repo + src/tagrules.* the same format, read/write/validate in C++ + (dialog) view, edit, create, delete, copy-as-new, dry-run +``` + +### Algorithm, and the properties that must survive any edit + +1. Load `rules.json`. **On any error, missing file, bad JSON or failed + validation, log to stderr and exit non-zero WITHOUT running the `-new` + consumer.** +2. Sort enabled rules by `stage`, then by array position. +3. For each rule, run + `notmuch tag <+add...> <-remove...> -- 'tag:new and (<query>)'`. +4. **Only after every rule succeeded**, run `notmuch tag -new -- tag:new`. + +**Step 1 is the critical safety property.** If the consumer runs while the rules +did not, `tag:new` is cleared from mail that was never tagged, and that mail can +never be tagged by these rules again. The failure is silent, permanent, and +invisible until someone notices a gap months later. A rules file that fails to +load must leave `tag:new` in place so the next successful run catches up. + +**The query is parenthesised in step 3, and this is not cosmetic.** Several +current rules are a disjunction of senders. Without the parentheses, +`tag:new and a or b` binds as `(tag:new and a) or b`, and the rule matches every +message in the corpus that satisfies `b` rather than only new arrivals. + +**Step 4 makes the run idempotent.** A failure partway through leaves `tag:new` +set, so re-running is safe and completes the work. This is also what makes the +migration verifiable: old and new hooks can be run against the same `tag:new` +set and compared. + +## qtmaildir + +**`TagRules` (`src/tagrules.h`, `src/tagrules.cpp`)**, a plain value type with +no widget dependency, following `Config` and `KeyMap`: it parses, validates, +collects warnings rather than throwing, and is unit-testable without a UI. +`QJsonDocument` to parse and serialize, `QSaveFile` to write. + +```cpp +struct TagRule { + QString id, query, note; + QStringList add, remove; + int stage = 50; + bool enabled = true; + QJsonObject unknown; // fields this version does not know, preserved on save +}; +``` + +The `unknown` member is the neutrality guarantee made concrete. + +**Validation degrades, it does not refuse.** Following `Config`'s existing +pattern: a duplicate id, an empty query, or a rule with neither `add` nor +`remove` drops that rule and records a warning the dialog shows. A file that +will not parse leaves the list empty with one warning. qtmaildir must never fail +to open because of this file. + +**Dry-run runs on the worker.** `NotmuchWorker` owns the only +`notmuch_database_t*` and notmuch permits one open handle per process, so +counting cannot happen on the UI thread even if it were fast. It is not fast: +item 74 measured a 4444-thread query at 5.7 seconds against a cold page cache, +and a dry run counts every rule. The counting slot is generation-stamped like +every other query so a superseded result is discarded. + +Each rule shows two numbers: + +- **Corpus count**, what the rule matches across all mail. This is the number + that answers "is this rule still doing anything" and "would this rule bury + real correspondence", which is what the comments in the current hook spend + most of their words reasoning about. +- **Pending count**, what it would tag on the next sync. Usually zero between + syncs. + +**The dialog** lists rules in stage order, each showing its tags, query, note, +enabled state and counts. Edit, create, delete and copy-as-new operate on the +list; Save writes atomically. Rules are a property of the mail system rather +than of an account, so the account dropdown does not scope them. + +## mailctl + +Read-only in this version: + +``` +mailctl rules list # id, stage, tags, enabled, note +mailctl rules show <id> # one rule, full query and note +mailctl rules dry-run [<id>] # corpus counts, all rules or one +``` + +**No `rules edit`, `add` or `delete`.** mailctl's safety model is that reads are +free and mutations are gated, and a rule edit is a mutation whose blast radius is +every future sync. What that gate should be is a real design question and it is +not needed yet: qtmaildir has the editor, and the file has a text editor. Writing +is fully designed and implemented in `mailrules.py`, because the hook's library +and qtmaildir must agree on atomic-write and unknown-key semantics, but no CLI +surface exposes it. + +`dry-run` reuses the existing `run_notmuch` and counting paths. **Its numbers +must equal qtmaildir's for the same rule**, and checking that once by hand after +migration is the cheap cross-check that the two parsers agree. + +## Concurrency + +Atomic writes, no locking. Every writer does write-to-temp then `rename()`, +which is atomic within a filesystem, so a reader always sees a complete file, +old or new. A truncated read by the hook is therefore impossible, which is the +failure that would actually hurt. + +Last writer wins on a genuine collision, silently. Accepted: this is a +single-user setup, agents run only when asked, and simultaneous edits from two +tools are rare enough to be corrected by eye. Locking was rejected because a +stale lock would block the hook, which is the one thing that must not be +blocked. + +## Migration + +The existing rules are converted once, preserving each comment as the rule's +`note`. + +**The live hook is not touched until the user has reviewed the conversion.** The +new hook is staged alongside as `post-new.new` and swapped by the user. + +**Equivalence is proved, not assumed.** For each rule, `notmuch count` the old +query and the new `tag:new and (query)` construction against the full corpus, +unscoped, and compare rule by rule. Identical counts are the evidence that the +conversion changed nothing. + +The conversion table names real senders and is therefore built and kept locally, +outside version control, per the project rule against personal details in +committed documentation. Every example in this document uses placeholders. + +## Testing + +**qtmaildir, `tests/test_tagrules.cpp`:** parse; round-trip preserving unknown +fields; each validation warning; stage ordering including ties resolved by array +position; and JSON escaping of queries containing quotes, which is not +hypothetical since path-scoped queries are written `path:"account/**"`. + +**mailctl:** plain-assert tests for `mailrules.py` matching the existing +`test_mailctl.py` style, covering the same load, save and ordering semantics. + +**Cross-tool:** the dry-run counts from both tools compared once by hand after +migration. + +The risk here is in the file format and the runner's failure modes, not in +painting. `CLAUDE.md`'s warnings about rendering probes apply to the dialog if +it grows custom painting, which this design does not call for. + +## Out of scope for v1 + +**Backfill: applying a rule to existing mail.** The counts are shown; the button +is not built. This is wanted and is the natural next step, since "apply this new +rule to the backlog" is currently a hand-run `notmuch tag`. + +Two things must be settled before it lands. A rule that is safe against arrivals +is not automatically safe against the whole corpus: a rule removing `inbox`, run +unscoped, would archive years of mail in one action. And `CLAUDE.md` currently +states that this project has no destructive-action confirmation and uses undo +instead, which does not cover a bulk tag write over thousands of messages. **The +user has stated that rule is due for revision**; backfill is the change that +forces it. + +**Rule edits from mailctl**, pending the gating decision above. + +**`post-insert` and timer-driven rules.** The scope split makes them +expressible, and nothing is designed for them here. |