diff options
Diffstat (limited to 'docs/superpowers/specs')
| -rw-r--r-- | docs/superpowers/specs/2026-08-15-builtin-filters-design.md | 177 |
1 files changed, 177 insertions, 0 deletions
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. |
