diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-13 10:42:56 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-13 10:42:56 +0200 |
| commit | 2d451e35e95a580d0e81c2e1bf7220d879490864 (patch) | |
| tree | 39e2a03c7757db4eacab5582dff94762c04e8126 /docs/superpowers/specs/2026-08-13-rule-builder-design.md | |
| parent | c396853a9290314ab657ac4414a21f26dc1fa6d5 (diff) | |
| download | qtmaildir-2d451e35e95a580d0e81c2e1bf7220d879490864.tar.gz qtmaildir-2d451e35e95a580d0e81c2e1bf7220d879490864.zip | |
docs: record items 75-78 and design the rule builder
The standing backlog reconciliation found four unrecorded entries in the
user's notes, all fallout from item 44's rules dialog now that it is in
daily use: the window forgets its geometry and column widths (75), every
field is free text (76), a rule cannot be previewed against the thread
list (77), and there is no way to build a rule from something visible in
a message (78). Each cause is verified in the code rather than copied
from the note.
Item 76 then went through a brainstorming pass and has a design. The
shape is Thunderbird's filter window, which the user supplied as the
reference: field and operator dropdowns, +/- buttons per row, an all/any
radio, and a separate "but not" block.
The structural point is that Thunderbird owns its filter format and this
project does not. The storage is a notmuch query string shared with
mailctl and executed by the post-new hook, so the builder is a view over
a string rather than a store. That decides the rest: the stored format
is untouched and this stays a single-repo change; a query the builder
cannot represent still opens, saves and runs, in a text mode every rule
carries; and the string is rewritten only when the rows actually
changed, compared against the parsed value rather than tracked with a
dirty flag, which Qt sets during programmatic population.
Measured against the seventeen rules in the live store, sixteen are flat
and one nests an or group inside an and chain, which is what the
exclusion block exists for. The parser is strict by design: it
recognises a query whole or rejects it whole, because a lenient parser
that salvages what it understands is how a not clause gets dropped and a
filter silently widens.
Diffstat (limited to 'docs/superpowers/specs/2026-08-13-rule-builder-design.md')
| -rw-r--r-- | docs/superpowers/specs/2026-08-13-rule-builder-design.md | 368 |
1 files changed, 368 insertions, 0 deletions
diff --git a/docs/superpowers/specs/2026-08-13-rule-builder-design.md b/docs/superpowers/specs/2026-08-13-rule-builder-design.md new file mode 100644 index 0000000..c11ef47 --- /dev/null +++ b/docs/superpowers/specs/2026-08-13-rule-builder-design.md @@ -0,0 +1,368 @@ +# A row builder for tagging rules: design + +Backlog item 76, "Every field in the rules dialog is free text, so a rule is +easy to get wrong". + +**Status:** design approved 2026-08-13, not implemented. + +## The problem this solves + +Item 44 shipped a rules dialog whose query field is a bare `QLineEdit` +(`src/tagrulesdialog.cpp:96`). A rule is written by typing a notmuch query by +hand, which puts the whole burden of the syntax on the user at exactly the +moment they are least likely to catch a mistake, because **notmuch's parser +rejects almost nothing**. `from:((((` parses cleanly and matches nothing. A +mistyped `path:` without its `/**` suffix matches nothing. Neither reports an +error anywhere; the rule simply stops tagging, silently, until somebody notices +mail is not being filed. + +The user asked for the shape Thunderbird uses: dropdowns to build the logic, +`+`/`-` buttons to add and remove conditions, radio buttons for the joining +logic, and typing reduced to the values that genuinely have to be typed. + +## The structural difference from Thunderbird, and why it decides the design + +Thunderbird owns its filter format. Its rows **are** the storage. + +Here they cannot be. The storage is a notmuch query string in +`~/.config/mailrules/rules.json`, shared with the companion `mailctl` project +and executed by the `post-new` hook. The hook runs the query; it knows nothing +about rows and never will. + +So the builder is a **view over a string**, not a store. Everything below +follows from that: + +- Rows compile to a query. That direction is easy. +- A query must parse back into rows. That direction is a parser, and every + existing rule was written by hand. +- A rule the parser cannot represent must still open, still save, and still + run, unchanged. + +## What is being built + +A `RuleQuery` value type that parses and compiles the query string, and a +builder section in `TagRulesDialog` that edits it. **The stored format does not +change.** `TagRule` gains no field, `rules.json` gains no key, and `mailctl` +needs no edit. This is a single-repo change, which is the main thing the design +buys by leaving the query string authoritative. + +## Decisions taken, with the alternatives that were rejected + +### Flat rows plus a separate exclusion block + +Thunderbird offers one exclusive radio: match **all** conditions, or match +**any**. Measured against the user's seventeen real rules, that covers sixteen. +The seventeenth is an `or` group nested inside an `and` chain, which is the +shape a person reaches for when they mean "from any of these senders, but not +when the subject looks like this". + +The builder therefore has two sections: a positive section governed by the +all/any radio, and a **"but not" block** whose rows are always joined `and not`. +That takes the real corpus from 16/17 to 17/17. + +Rejected: flat-only, which would leave a permanent second-class rule that the +user looks at often. + +### The query string is only rewritten when rows actually changed + +Opening a rule, looking at it, and closing must not rewrite the file. A +recompile that is semantically identical but textually different +(`a or b` becoming `(a or b)`) churns a file that a second tool reads. + +The dialog keeps the `RuleQuery` it parsed and compares the current widget state +against it on save. Equal means the stored string is written back byte for byte. + +Rejected: a dirty flag driven by widget `changed` signals. Qt emits those during +programmatic population, so loading a rule into the form would mark it dirty +before the user touched anything, rewriting the file on open. The comparison +approach has no such failure, and it correctly treats an edit that was manually +undone as clean. + +Rejected: storing the rows in the JSON alongside the query. That is a two-repo +format change, obliges `mailctl` to preserve a field it does not use, and makes +the rows authoritative over the query the hook actually runs. + +### The exclusion block is the preferred reading of a trailing negation + +Per-row negation and the exclusion block overlap: `not subject:x` can be a +negated row in the positive section or a row in the block. Both compile +correctly. When parsing, the block wins whenever the negations sit at the end of +an `and` chain, because that is how the user describes these rules in words and +how the original shell hook's comments reasoned about them. + +Consequence, accepted: a query written with per-row negations may come back +displayed as block exclusions. Semantically identical, and nothing is rewritten +unless the rule is edited. + +### Text mode is a toggle on every rule, not an error state + +An unparseable rule opens with the toggle already flipped. There is no special +mode, no disabled builder, and no error. Every rule has both views, which also +gives an escape hatch on rules the builder *can* express but clumsily. + +Rejected: hiding the builder, which reads as something breaking; and showing it +disabled and empty, which is a false affordance. + +## `RuleQuery` + +`src/rulequery.h`, `src/rulequery.cpp`. A plain value type with no widget +dependency, following `TagRules` and `CardLayout`: fully unit-testable without a +UI or a painter. + +```cpp +struct RuleTerm +{ + enum Field { From, To, Cc, Subject, Tag, Folder, Attachment, Date }; + enum Op { Contains, ContainsNot, Is, IsNot, Has, HasNot, Before, After }; + + Field field; + Op op; + QString value; +}; + +struct RuleQuery +{ + enum Join { All, Any }; ///< and / or, over the positive terms only. + + Join join = All; + QList<RuleTerm> terms; ///< Positive section. + QList<RuleTerm> exclusions; ///< The "but not" block, always joined and-not. + + /// False when the query cannot be represented as rows. NOT an error: the + /// rule opens in text mode and saves unchanged. + bool parsed = false; + + static RuleQuery parse(const QString &query); + QString compile() const; +}; + +bool operator==(const RuleQuery &a, const RuleQuery &b); +``` + +`parse` never fails and never throws; it returns a value with `parsed = false`. +`compile` is called only on a parsed value whose rows were edited. + +### Data flow + +Opening a rule: + +``` +TagRule.query ──parse()──> RuleQuery ──> builder widgets + │ + └─ parsed == false ──> text mode, toggle flipped +``` + +Saving: + +``` +rows edited? yes ──compile()──> TagRule.query + no ─────────────> TagRule.query unchanged, byte for byte +``` + +## Grammar + +### What compiles + +| Field | Operators | Compiles to | +|---|---|---| +| From | contains / is | `from:x` / `from:"x"` | +| To | contains / is | `to:x` / `to:"x"` | +| Cc | contains / is | `cc:x` / `cc:"x"` | +| Subject | contains / is | `subject:x` / `subject:"x"` | +| Tag | is | `tag:x` | +| Folder | is | `path:"x/**"` | +| Attachment | has | `attachment:x` | +| Date | before / after | `date:..x` / `date:x..` | + +Every operator has a negated twin (`contains not`, `is not`, `has not`) which +prefixes `not `. Date has none: "not before" is "after". Tag and Folder carry +only is / is not, because "contains" is meaningless for an exact token and an +exact path. + +**Folder appends the `/**` suffix itself.** A `path:` without it matches +nothing, and notmuch does not report that. + +**Quoting.** `is` quotes, `contains` does not. A value containing a space is +quoted regardless, or the query breaks. A value containing a `"` is refused at +the row rather than escaped: notmuch's quoting rules inside a quoted phrase are +not worth modelling for a case that has never occurred. + +### How a query assembles + +Positive terms joined by the radio, then exclusions appended as `and not`: + +``` +join = Any +terms = [ from:vendor.example.org, from:vendor.example.net ] +exclusions = [ subject:receipt, subject:refund ] + + → (from:vendor.example.org or from:vendor.example.net) + and not subject:receipt and not subject:refund +``` + +**Parenthesisation rule:** the positive group is parenthesised when +`join == Any` **and** there is at least one exclusion. Otherwise no parens. + +This is the same binding hazard `CLAUDE.md` already records for the hook's +`tag:new` scope: `tag:new and a or b` binds as `(tag:new and a) or b`. Inside a +rule the same mistake turns a narrow disjunction into a filter that matches +everything. + +### What parses + +A query parses into rows when it is: + +- a flat `and` chain of recognised terms, or +- a flat `or` chain of recognised terms, or +- a parenthesised `or` chain followed by `and not` terms, all flat. + +**An empty query parses to zero rows**, not to a failure. One existing rule has +an empty query and must open in the builder ready to receive a row. + +Everything else sets `parsed = false`: nested parens beyond that one shape, +mixed `and`/`or` without parens, `xor`, an unrecognised prefix (`body:`, `mid:`, +`folder:`), a bare word with no prefix, a `path:` not ending in `/**`. + +### The parser is strict, and that is the safety property + +**Recognise the whole query or reject it whole. Never partially.** + +A lenient parser that salvages the parts it understands is how a `not` clause +gets silently dropped and a filter quietly widens. Strict rejection means the +worst outcome is text mode, never a wrong rule. + +**This parser is not notmuch's parser and must not pretend to be.** It +recognises the shapes this builder emits plus the shapes the existing rules use. +A query it rejects is not invalid: notmuch accepts `from:((((` happily. The +wording in the UI is "can't be shown as rows", never "invalid". + +## Extending this later + +The design is future-proof in one specific dimension, and it is worth being +precise about which. + +**What can never need a redesign:** any query notmuch accepts stays expressible, +saveable and runnable, whether or not the builder understands it. The builder is +not the storage. A rule written years from now with a prefix nobody anticipated +opens in text mode, saves correctly, and runs correctly. No migration, nothing +lost. + +**Three extension points, in ascending cost:** + +| Change | Cost | +|---|---| +| A new field (`body:`, `reply-to:`, `mid:`) | One entry in the enum, the compile switch, the parse table. Additive. | +| A new operator on an existing field | Same, additive. | +| A new query **shape** (nested `or` within `or`, three-level parens, `xor`) | A parser change. | + +Only the third is real work, and hitting it costs a parser extension rather than +a redesign, because storage never depended on the parser. For calibration: the +user's seventeen rules use two shapes, and Thunderbird offers exactly two after +decades. + +**`parsed = false` is a first-class state, not an error path**, and is tested +with real unparseable queries rather than asserted unreachable. + +## The dialog + +The builder replaces the query line edit. Everything else in the form stays. + +``` +Id [ vendor-receipts ] Stage [ 50 ] [x] Applied on every sync + +Match (o) all ( ) any [ ] Edit as text + [From v] [contains v] [vendor.example.org ] [+] [-] + [From v] [contains v] [vendor.example.net ] [+] [-] +But not + [Subject v] [contains v] [receipt ] [+] [-] + [Subject v] [contains v] [refund ] [+] [-] + [+] add exclusion + +Add tags [ vendor, receipts ] +Remove tags [ ] +Note [ ... ] + +Query (from:vendor.example.org or ...) and not subject:receipt [Count matches] +``` + +**The query line stays visible in builder mode, read-only.** It is what ships to +the hook, and watching it update as rows change is what makes the builder +trustworthy rather than a black box. In text mode the same widget becomes +editable: one widget, two states. + +**"Edit as text" is always present.** Flipping it on shows the compiled query, +editable. Flipping it back re-parses: on success the builder repopulates, on +failure the checkbox refuses to clear and says why. + +**The "But not" block appears only when it has rows**, plus an "add exclusion" +affordance. Sixteen of seventeen rules have no exclusions and an empty block on +every rule is noise. + +**Folder rows get a dropdown** populated from the accounts in `Config`, so `/**` +is never typed. A folder present in the file but absent from the config still +displays, as an editable entry, or opening an old rule would silently blank it. + +**Count matches is unchanged.** It already works and is generation-stamped on +`m_ruleCountGeneration` (`src/mainwindow.h:657-664`), which must stay separate +from `m_generation`: bumping the query generation for a count discards any +thread load in flight and blanks the message pane. In text mode it counts what +was typed. + +**Completion**, the other half of item 76, is independent of this design and can +land before or after it: tag names on the add and remove fields, the query +grammar in text mode. It carries its own trap, recorded in `CLAUDE.md`: a +multi-value field must not use `QLineEdit::setCompleter`, because the line edit +overwrites the completer's prefix with the widget's entire text, so the first +tag completes and nothing after it does. Attach with `QCompleter::setWidget` and +drive the prefix by hand. A test using `setText()` passes against that bug, +since `setText` never drives a completer; the keys must be typed. + +## Testing + +`tests/test_rulequery.cpp`, a plain unit test with no widget. + +**The corpus test is the one that matters.** Every real query shape, asserted to +parse, compile back byte-identical, and compare equal after a round trip. That +single test is the whole "the user's file does not churn" guarantee, and it is +what catches a parenthesisation slip on the one nested rule. + +Those queries go in as **generic placeholders**, never the user's real senders +or account names, per the standing rule that nothing personal reaches a commit. +The shapes are what is under test and they survive substitution intact: a +disjunction of eight job-alert senders becomes eight `from:jobs<n>.example.org` +terms and tests exactly the same thing. + +**Rejection tests**, which carry the safety property. Queries that must set +`parsed = false` and must not partially parse: nested `or` inside `or`, mixed +`and`/`or` without parens, `body:foo`, a bare word, a `path:` without `/**`, and +`from:((((`. That last asserts **our** rejection, never a provoked notmuch +failure: `CLAUDE.md` records twice that notmuch accepts it cleanly, and a test +expecting an error there fails against correct code. + +**Compile tests** for every field and operator pair including both negations, +and the parenthesisation rule at its boundary: `join == Any` with zero +exclusions gets no parens, with one exclusion gets parens. + +**A mutation check on the corpus test.** Make `compile()` always parenthesise; if +the corpus test still passes it is not testing what it claims. Per `CLAUDE.md`, a +passing test here proves nothing without one. + +**Dialog-level tests**, added to `test_tagrules`: + +- Open a rule, change nothing, save, assert the stored string is **byte-identical**. +- Open a rule with exclusions, toggle to text and back, assert the builder state survives. +- Open an unparseable rule, assert the toggle starts flipped and refuses to clear. + +Not tested, deliberately: populating the folder dropdown from a live notmuch +index. That needs the fixture database for little value; tests populate it from +`Config`. + +## Out of scope + +**Item 77, previewing a rule's matches in the thread list**, and **item 78, +building a rule from a right-click**. Both touch this dialog and both are +independent of the builder. Item 78 wants this design to land first, so the +created rule arrives in a form that can hold it. + +**Backfill** remains out of scope, as item 44's spec records. Nothing here +changes that. |
