diff options
3 files changed, 278 insertions, 51 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 730c722..ba8b99e 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 @@ -5039,3 +5039,65 @@ when the write is moved above the guard. The cold-cache cost measured above was not touched and should not be. Nothing about the timing changed. + +## 90. A saved-query button clears the account selection + +**Observed (user, notes):** "select an account and hit the 'unread' button, the +panel should show unread messages from that account only, instead it refreshes +the 'all accounts' list." + +**Cause, and it is a deliberate line, not an oversight.** +`MainWindow::runSavedQuery()` (`src/mainwindow.cpp:1907`) resets the account box +whenever the saved query names no account: + +```cpp +const int index = saved.account.isEmpty() + ? m_accountBox->findData(QString()) + : m_accountBox->findData(saved.account); +if (index >= 0) + m_accountBox->setCurrentIndex(index); +``` + +The comment above it states the intent: "An unscoped query CLEARS the selection +rather than inheriting whatever was there, which is the defect the rules preview +hit." `runQuery()` (`:2020`) then scopes whatever the box holds, so with the +reset removed the button would inherit the selection and the user would get what +they asked for. + +**So this is a decision, not a bug to fix quietly.** Two behaviours are +defensible and each breaks the other's case: + +- **Inherit.** The account box is a persistent scope and the buttons are + queries within it. This is what the note asks for. The risk is the one the + comment names: a query that already scopes itself, such as a rules preview + with `path:"work/**"`, gets scoped twice and matches nothing. +- **Reset.** A saved query is self-contained and says exactly what it shows. + This is what ships. Its cost is that the account dropdown looks like a filter + and silently is not. + +**Approach.** Ask the user which they want before writing anything. If it is +inherit, the narrow version is to inherit only for hand-written and generated +entries and keep the reset for a preview, since a preview is the one caller that +supplies its own path; the rules preview already forces index 0 itself at +`:1637`, so it does not depend on this line. + +**Constraints.** + +- Whatever is chosen, one rule for every saved-query surface: the buttons, the + menu and the dropdown all reach `runSavedQuery()`, and three behaviours here + would be worse than either one. +- A saved query that DOES name an account must keep overriding the box. That is + not in question and no test should lose it. +- `Account::scopedQuery()` is the only place a scope is applied. Do not add a + second one to make a caller behave. + +**Folded into item 93 on 2026-08-15, not fixed in place.** Explaining this item +to the user produced the reframing in 93: the button that misbehaves here should +never have been a saved query, so `runSavedQuery()` keeps this behaviour and the +filters get their own. The narrow fix considered here, inheriting the selection +for unscoped entries, was rejected because it would also make every unscoped +MENU entry compose, and those are the self-contained destinations that should +keep resetting the account. + +Kept in full because the cause, the comment defending the reset, and the rules +preview that motivated it are all still true of the code. 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 841ee91..9b38d03 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 @@ -154,9 +154,10 @@ taking that too literally. | 85 | Nothing on screen can be searched for by right-clicking it | workflow | M | **done** 2026-08-14, unreleased; see `specs/2026-08-14-search-from-message-design.md`. Split from 78; rebuilt the details dialog as rows | | 86 | A right-click search can replace or narrow, but never exclude | workflow | S | **done** 2026-08-14, unreleased; see `specs/2026-08-14-exclude-from-search-design.md`. Follows 85. The `extend` bool became a `SearchMode` enum across four signatures | | 89 | A sync moves the list under the user's hands, and the auto-sync skips rather than retries | workflow | M | open; from the 2026-08-15 notes pass. Two faults under one complaint, and the larger half is a design question | -| 90 | A saved-query button clears the account selection | workflow | S | open; from the 2026-08-15 notes pass. Cause is a DELIBERATE line; decision needed from the user before any change | +| 90 | A saved-query button clears the account selection | workflow | S | **folded into 93** 2026-08-15. Not fixed in place: the button that misbehaves stops being a saved query at all. See `specs/2026-08-15-builtin-filters-design.md` | | 91 | Double-clicking a thread could open it in its own window | workflow | ? | open, unspecified; the user marked it "(?) UX not sure" | | 92 | Nothing distinguishes a tag written by a rule from one the user applied | information | ? | open, unspecified; the user asked it as a question, and the answer decides whether it is a display item or a format change across two repos | +| 93 | The query buttons are whatever the user pinned, not a designed set of filters | workflow | M | open, specified 2026-08-15; see `specs/2026-08-15-builtin-filters-design.md`. Absorbs item 90. Four built-in filters that compose with the account dropdown | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -545,56 +546,43 @@ work. - Do not restore the pre-0.16.0 behaviour by making the delay negative for the user. `auto_sync_delay_ms` is theirs to set. -## 90. A saved-query button clears the account selection - -**Observed (user, notes):** "select an account and hit the 'unread' button, the -panel should show unread messages from that account only, instead it refreshes -the 'all accounts' list." - -**Cause, and it is a deliberate line, not an oversight.** -`MainWindow::runSavedQuery()` (`src/mainwindow.cpp:1907`) resets the account box -whenever the saved query names no account: - -```cpp -const int index = saved.account.isEmpty() - ? m_accountBox->findData(QString()) - : m_accountBox->findData(saved.account); -if (index >= 0) - m_accountBox->setCurrentIndex(index); -``` - -The comment above it states the intent: "An unscoped query CLEARS the selection -rather than inheriting whatever was there, which is the defect the rules preview -hit." `runQuery()` (`:2020`) then scopes whatever the box holds, so with the -reset removed the button would inherit the selection and the user would get what -they asked for. - -**So this is a decision, not a bug to fix quietly.** Two behaviours are -defensible and each breaks the other's case: - -- **Inherit.** The account box is a persistent scope and the buttons are - queries within it. This is what the note asks for. The risk is the one the - comment names: a query that already scopes itself, such as a rules preview - with `path:"work/**"`, gets scoped twice and matches nothing. -- **Reset.** A saved query is self-contained and says exactly what it shows. - This is what ships. Its cost is that the account dropdown looks like a filter - and silently is not. - -**Approach.** Ask the user which they want before writing anything. If it is -inherit, the narrow version is to inherit only for hand-written and generated -entries and keep the reset for a preview, since a preview is the one caller that -supplies its own path; the rules preview already forces index 0 itself at -`:1637`, so it does not depend on this line. - -**Constraints.** - -- Whatever is chosen, one rule for every saved-query surface: the buttons, the - menu and the dropdown all reach `runSavedQuery()`, and three behaviours here - would be worse than either one. -- A saved query that DOES name an account must keep overriding the box. That is - not in question and no test should lose it. -- `Account::scopedQuery()` is the only place a scope is applied. Do not add a - second one to make a caller behave. +## 93. The query buttons are whatever the user pinned, not a designed set of filters + +**Observed (user, 2026-08-15),** reached by explaining item 90 rather than from +the notes: + +> The queries that show as buttons shouldn't be in the same league as the ones I +> write and store in the "more queries" menu. If I see "Unread" as a button, I +> read it as "filter all my mails and show me only what is not yet read", but +> being a button, in my head it should cooperate with other UI elements. So if a +> dropdown offers to select an account, that same button should work with that +> selection transparently, not fight it. + +**Cause.** Nothing ships as a default. `Config::startupSavedQuery()` falls back +to `m_savedQueries.first()` and a fresh install has an empty query row, so every +button the user has is one they wrote into `[queries]` and that the queries.json +migration pinned (`src/config.cpp:455`). The buttons became "whatever is pinned" +by migration, never by design. The user's own words: "that's the direction I +wanted from the start, we drifted to what is today". + +**Approach.** Four built-in filters, Unread, Inbox, Flagged and Sent, as +generated entries in the closed set `kQueryGenerators` that already exists for +Sent. They compose with the account dropdown; saved queries keep setting the +account from what they stored. The user's own pinned queries are UNPINNED once +the buttons are confirmed working, never deleted, so they fold into the menu and +stay recoverable. + +**Absorbs item 90.** The button that clears the account selection stops being a +saved query at all, so there is nothing left to fix in `runSavedQuery()`. + +**Specified in `specs/2026-08-15-builtin-filters-design.md`.** Three constraints +worth knowing before opening it: a generator must answer PER ACCOUNT rather than +having its all-accounts query wrapped in a scope, or Sent double-scopes and works +only by accident of `path:` being hierarchical; Sent stays `flat` and the other +three do not, so the four match in scope and not in view mode; and changing the +account deliberately runs nothing, since the button is the verb. + +**Size: M.** ## 91. Double-clicking a thread could open it in its own window diff --git a/docs/superpowers/specs/2026-08-15-builtin-filters-design.md b/docs/superpowers/specs/2026-08-15-builtin-filters-design.md new file mode 100644 index 0000000..cd22bab --- /dev/null +++ b/docs/superpowers/specs/2026-08-15-builtin-filters-design.md @@ -0,0 +1,177 @@ +# Built-in filters, separate from saved queries + +**Resolves backlog items 93 and 90.** Item 90 (a saved-query button clears the +account selection) is not fixed in place: the button that misbehaves stops being +a saved query at all. + +Status: **specified, not built.** Written 2026-08-15 from the user's own framing, +recorded verbatim below because it is the whole design. + +## The user's model + +> The queries that show as buttons shouldn't be in the same league as the ones I +> write and store in the "more queries" menu. If I see "Unread" as a button, I +> read it as "filter all my mails and show me only what is not yet read", but +> being a button, in my head it should cooperate with other UI elements. So if a +> dropdown offers to select an account, that same button should work with that +> selection transparently, not fight it. + +Two kinds of thing, and today they share one mechanism: + +- **A filter** narrows whatever the user is already looking at. It composes with + the account dropdown. It never decides scope on its own. +- **A saved query** is a destination. It is self-contained, says exactly what it + shows, and is entitled to set the account, because the user saved it that way. + +## Why this is not a small fix to `runSavedQuery()` + +`MainWindow::runSavedQuery()` (`src/mainwindow.cpp:1907`) resets the account box +whenever the entry names no account, which is what item 90 observes. The obvious +fix, inheriting the selection for unscoped entries, would make the buttons +compose. It would also make every unscoped entry in the MENU compose, and those +are exactly the self-contained destinations that should keep resetting. + +The distinction has to be a property of the entry. It cannot be `pinned`, which +decides button-versus-menu today: making `pinned` change semantics would mean +unpinning a query silently changes what it does. + +## How the buttons became saved queries + +Worth recording, because the drift is the defect. **Nothing ships as a default.** +`Config::startupSavedQuery()` falls back to `m_savedQueries.first()` and a fresh +install has an empty query row. Every button the user has is something they wrote +into `[queries]` in the INI and that the queries.json migration +(`src/config.cpp:455`) pinned: + +```cpp +// Pinned, because these are buttons today. A migration that left +// them unpinned would empty the query row on the first launch +// after an upgrade, which reads as data loss. +query.pinned = true; +``` + +Correct for that migration, and it is how "the buttons" came to be "whatever the +user pinned" rather than a designed set. The user's words: "that's the direction +I wanted from the start, we drifted to what is today". + +## The design + +**Four built-in filters ship: Unread, Inbox, Flagged, Sent.** They are generated +entries, named in `kQueryGenerators` (`src/config.cpp:60`), which is already a +closed set validated on load. Adding a fifth later is one entry in that list. + +**All four compose with the account dropdown.** "All accounts" gives the union +across every account; one account selected gives that account only. Saved queries +are unchanged: they still set the account from what they stored. + +`generated` and its validation already exist for Sent, including the property +this design depends on: + +> Storing the GENERATOR rather than its output keeps both: the query stays live, +> and the entry is an ordinary row the user owns. + +### A generator must answer per account, not only for all + +This is the one non-obvious piece, and getting it wrong produces a query that +works by accident. + +`Config::allSentQuery()` joins every account's `sentQuery()` with ` or `, giving +`path:"a/Sent/**" or path:"b/Sent/**"`. Scoping that with the selected account's +`Account::scopedQuery()` yields: + +``` +path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**") +``` + +That returns the right rows, because `path:` is hierarchical and the `b` half +cannot match inside `a`. It is still wrong to build: it double-scopes, and it +relies on a property of the path syntax rather than on saying what is meant. The +correct query is `path:"a/Sent/**"`, which `Account::sentQuery()` already +returns. + +So a generator needs a per-account entry point beside the all-accounts one. +`Account::sentQuery()` is that entry point for Sent and already exists; +`allSentQuery()` is its join. The other three are `tag:` queries with no path of +their own, so `Account::scopedQuery()` is exactly right for them and no second +entry point is needed. + +Two shapes, one rule: **ask the generator for this filter in this account's +scope, or across all accounts.** Do not compose by wrapping an all-accounts +query. + +### Sent stays flat, the other three do not + +`SavedQuery::flat` lists messages rather than threads, and Sent sets it. This is +correct and must not be unified away: a thread would fold the user's sent message +back into the conversation it belongs to, which is item 63's whole finding. + +The four buttons therefore behave identically in SCOPE and not in view mode. +Flagged by the user as a thing to know before it surprises anyone: "same +behavior for all four" was said about the account, and holds there. + +## Migration: unpin, never delete + +The user's existing pinned queries keep their names, and some will collide with a +built-in filter's. **Unpin them once the new buttons are confirmed working**, at +the user's explicit instruction: + +> when we'll get to testing the new buttons, simply unpin my queries, so they +> fold into the menu and the new buttons will have their real estate + +They fold into the menu, keep working, and are recoverable by pinning them again. +Nothing the user wrote is deleted. Do not "clean up" a shadowed duplicate. + +## Changing the account does NOT run a query + +**Decided by the user, 2026-08-15:** + +> changing the account should not run the query, hitting the button after +> changing the account is what queries + +The dropdown selects scope and nothing else. The button is the verb. Selecting an +account and then clicking Unread is the gesture; selecting an account on its own +changes what the next click will mean and leaves the list alone. + +This is also what the code does today: `m_accountBox` has **no signal connected +to it**, verified 2026-08-15. So nothing has to be built for this, and the thing +to be careful of is not building it by reflex. Do not connect +`currentIndexChanged` to a re-run while making the buttons compose. + +Two reasons this is the right call beyond the user having made it. A re-run on +every dropdown change moves the list under a user who is reading, which is the +class of complaint item 89 is about. And the account combo is a plain +`QComboBox`, so a keyboard user arrowing through it would fire a query per +account passed on the way to the one they wanted. + +## Constraints + +- **One rule across all three surfaces.** Buttons, the "more queries" menu and + the dropdown all reach `runSavedQuery()`. Three behaviours would be worse than + either one. +- **A saved query that names an account keeps overriding the box.** Not in + question; no test should lose it. +- `Account::scopedQuery()` is the only place a scope is applied. Do not add a + second one to make a caller behave. +- **An unknown generator is reported and KEPT**, never dropped + (`src/config.cpp:555`). A filter added in a later build must survive a save + from an older one. +- No `kQueriesFormatVersion` bump. Adding generators to a closed set is not a + breaking change, and queries.json has one implementation, so this is not a + two-repo change the way rules.json would be. + +## Verification + +- Each filter, per account and across all accounts, asserted on the GENERATED + QUERY STRING rather than on a row count. A count test passes against a + double-scoped query, which is the failure this design exists to avoid. +- Sent for one account must be `path:"a/Sent/**"`, with no enclosing + `path:"a/**" and (...)`. This is the assertion that catches composing by + wrapping. +- A saved query with a stored account still sets the box. +- A saved query with no stored account still CLEARS the box, which is the + behaviour item 90 leaves alone. +- Sent stays flat and the other three do not. +- An account whose config sets no `sent` folder must not produce a Sent filter + that matches everything. `folderQuery()` returns empty for an unset folder and + an empty query means "match everything" to notmuch, so this is a real trap and + needs its own assertion. |