Merge branch 'rule-builder': a row builder for the tagging rules

Item 76 replaces the four free-text fields with a row builder: field and
operator dropdowns per condition, +/- to add and remove them, a match
all/any choice, and a separate "but not" block. The stored format does
not change, so mailctl needs no edit. The query string stays
authoritative and remains visible, and a rule the builder cannot
represent opens in a text mode every rule carries.

Along the way, items 75, 77 and 80, and a data-loss defect released in
0.16.0 (item 79): opening the dialog and pressing Save destroyed the
first rule with nothing edited. That one damaged a real rule in the
user's own file, which was repaired by hand.

Four defects in this work were found by hand rather than by the suite,
and each is recorded where it was missed: a lost note, a one-way text
mode toggle, a geometry save on a path neither button takes, and a rule
list squeezed to one row by a long rule. Two Qt traps and one about the
user's compositor went into CLAUDE.md.

3 files changed, 222 insertions, 16 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 1216542..eec544e 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
@@ -131,10 +131,12 @@ taking that too literally.
 | 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 |
-| 75 | The tagging rules window forgets its size and its column widths | persistence | S | open; follows item 44 |
-| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | open; design approved 2026-08-13, see `specs/2026-08-13-rule-builder-design.md` |
-| 77 | No way to see what a rule would collect, in the thread list | workflow | S | open; the dialog counts matches, it cannot show them |
+| 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13 on `rule-builder`, unreleased. The window-kind question is left open, see below |
+| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13 on `rule-builder`, unreleased. See `specs/2026-08-13-rule-builder-design.md` |
+| 77 | No way to see what a rule would collect, in the thread list | workflow | S | **done** 2026-08-13 on `rule-builder`, unreleased |
 | 78 | No way to build a rule from something visible in a message | workflow | M | open; wants 76 first, so the created rule lands in a form that can hold it |
+| 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13 on `rule-builder`, unreleased. Follows item 76 |
+| 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand |
 
 Sizes are rough: XS under an hour, S a sitting, M a session.
 
@@ -4763,6 +4765,55 @@ the test asserts on the saved value rather than on the resulting frame.
 
 **Size: S.**
 
+**Done 2026-08-13** on `rule-builder`, unreleased, for the COLUMN WIDTHS.
+`saveGeometry()` and the list header's `saveState()` go to `tagrules/geometry`
+and `tagrules/header` in `uistate.conf`, written on `done(int)` so they survive
+Cancel as well as Save; `resize(760, 520)` stays as the first-run fallback.
+
+**The window SIZE does not come back, and that half of the item cannot be
+fixed here.** The user's desktop is Hyprland, a tiling compositor. It tiles the
+window to fill its slot, so the size dragged is the tile's; `saveGeometry`
+records `frameGeometry` and `normalGeometry` and `restoreGeometry` restores the
+NORMAL one, which stays at whatever `resize()` last set. Decoded from the real
+state file after a hand test: frame 2248x806, normal 760x664. The code restores
+760 faithfully and the window still opens tiled.
+
+Three wrong diagnoses were tried and each was disproved by a probe rather than
+by argument: that `restoreGeometry` rejected the blob as off-screen (it returns
+true on the real display; the negative y is the DP-1 origin), that the layout
+overrode a geometry set before the first show (a `showEvent` restore produced
+the identical size), and that the offscreen test could tell the two apart (it
+returns the same frame for both, so the mutation survived).
+
+Nothing worth building remains unless the user wants the dialog to open at a
+remembered size when floated, which needs a Hyprland window rule rather than
+code here.
+
+**The approach above was wrong on one point, and a test caught it.** It said to
+drop the `resizeColumnToContents` calls "once a saved header state exists",
+which fixes the restore and leaves the original defect standing: with no saved
+state, a width the user had just dragged was still discarded by the next add or
+delete. The rule shipped instead is that each column is auto-sized ONCE, on its
+first fill, after which its width belongs to the user however it was set. Two
+flags, because the count column is filled later by a reply from the worker.
+
+**It then shipped broken once more, and the test that covered it passed.** The
+save was written in `closeEvent`, and the test asserted with `close()`. Neither
+button goes anywhere near either: Cancel calls `reject()`, Save calls
+`accept()`, and only the window manager's X button sends a `QCloseEvent`. So
+the size was kept for the one route out of three that the buttons never take,
+and the user found it in one try by resizing and pressing Cancel. The save now
+overrides `done(int)`, which both buttons funnel through and `close()` reaches,
+and the test asserts all three routes rather than trusting one to stand for the
+others. A second trap sits underneath: `close()` on a widget that was never
+shown returns early without reaching `done()`, so that leg of the test has to
+`show()` first or it proves nothing.
+
+The **popup or primary window** question was put to the user and deliberately
+not taken: it stays a `QDialog`. Reopening it needs the unsaved-edit story that
+being modal currently sidesteps, and that is its own decision rather than part
+of this item.
+
 ## 76. Every field in the rules dialog is free text, so a rule is easy to get wrong
 
 **Observed.** A rule is written by typing into four line edits, and the user
@@ -4840,6 +4891,18 @@ means the preview discards whatever thread load was in flight.
 
 **Size: S.**
 
+**Done 2026-08-13** on `rule-builder`, unreleased. A **Preview in list**
+button emits `previewRequested(query)`; `MainWindow::onRulePreviewRequested`
+clears the account selector, puts the query in the bar and runs it, then
+raises itself. The dialog stays open, which is the point.
+
+Both constraints above became assertions, and BOTH mutations were needed: a
+test that emitted the hook's `tag:new and (...)` wrapping fails, and one that
+skips the account reset fails. The second only bites once the test config
+actually has an account to select, since the default empty config leaves the
+selector on "All accounts" already and the assertion passed against the
+mutation until that was fixed.
+
 ## 78. No way to build a rule from something visible in a message
 
 **Observed.** The user would like to select an address or another piece of a
@@ -4866,6 +4929,98 @@ in CLAUDE.md.
 
 **Size: M.**
 
+## 79. Opening the rules dialog and saving destroys the first rule
+
+**Observed.** Open Tagging rules, press Save, change nothing. The first rule
+in the list loses its query and its tags. It then vanishes entirely the next
+time anything reads the file, because a rule with an empty query is dropped
+as malformed on load.
+
+Found while building item 76, by a test written to catch a different problem.
+Reproduced against the released tag rather than the branch, with a throwaway
+worktree at 9585674 and a two-rule fixture: after constructing the dialog and
+calling its save path, the store held one rule instead of two.
+
+**Cause.** `TagRulesDialog::onSelectionChanged` blocks signals for `m_note`
+only. Two lines later, `m_enabled->setChecked(rule.enabled)` emits `toggled`,
+which is connected to `applyEditsToCurrentRule()`. That handler writes every
+field of the current rule from the widgets, and it runs BEFORE
+`m_query->setText(rule.query)` has filled the query widget, so it writes the
+previous rule's text. On the first open there is no previous rule and the
+widgets are empty, so rule 0 gets an empty query and an empty tag list.
+`TagRules::load` then drops it (`src/tagrules.cpp:150`).
+
+The existing comment above the `QSignalBlocker` shows the hazard was known for
+`m_note` and simply not extended to `m_enabled`. A blocker per widget is the
+wrong shape: the whole load needs one guard.
+
+**Fix.** Raise `m_reloading` for the duration of `onSelectionChanged` and
+restore it afterwards, replacing the single-widget blocker. `m_reloading`
+already exists for exactly this class of problem and
+`applyEditsToCurrentRule()` already honours it. Also take the rule by value
+rather than by const reference: the reference points into `m_working`, which
+the handler mutates, so it could be read back half overwritten.
+
+Fixed on the `rule-builder` branch as part of item 76, with
+`switchingRulesDoesNotLeakRowsBetweenThem` in `test_tagrules` as the
+regression test. It fails against the unfixed code.
+
+**Damage in the field, and the repair.** The live rules file had exactly one
+casualty: the account rule sitting first in the list, with its `query`, its
+`add` and its `note` all empty while every sibling account rule was intact.
+
+**The note was missed on the first pass of the repair**, because the shell
+backup was read for the tagging command and the note comes from the comment
+block ABOVE it, which the migration had given to all five account rules
+alike. The user spotted the gap. `applyEditsToCurrentRule()` writes every
+field, so every field is equally exposed: repair work here must check the
+whole rule, not the fields that first drew attention. Restored from the four
+siblings, which carry byte-identical notes.
+Restored from `post-new.shell-backup`, which item 44's migration kept, and
+verified by loading the file through mailctl's own reader: 17 rules, correct
+scoping. The rule had stopped tagging, but only one message had arrived in the
+meantime (14968 of 14969 in that account still carried the tag); it was tagged
+by hand and the account is now complete.
+
+**Constraints.** The user chose to leave the fix on the branch rather than cut
+a patch release, so 0.16.0 in the field still has it. Do not open that dialog
+in a released build.
+
+**Size: XS** for the fix. The reproduction and the field repair were the work.
+
+## 80. A rule with many conditions squeezes the rule list to one visible row
+
+**Observed.** A rule with eight From conditions left the rule list showing
+about one and a half rows, with the second rule half cut off under the first.
+Reported with a screenshot; the builder filled the window and the list it sits
+under kept almost nothing.
+
+**Cause.** `m_list` was added to the dialog's `QVBoxLayout` with stretch 1
+(`src/tagrulesdialog.cpp:109`) and the form below it with none, which looks
+like the list wins. It does not: a stretch factor only distributes space ABOVE
+each widget's minimum, and the form's minimum grows with every condition row,
+so each row came directly out of the list. Measured on the builder's size hint:
+120px with one row, 414px with eight.
+
+**Approach.** Done 2026-08-13. A `QSplitter` divides the list from the editor,
+so the balance is the user's and is saved to `uistate.conf` beside the column
+widths, and the condition rows sit in a `QScrollArea` capped at 190px so the
+editor cannot grow without bound whatever the splitter is set to. The scroll
+area rather than the builder is what text mode hides, since hiding the inner
+widget would leave an empty frame.
+
+**Constraints.** Three measures were tried before one distinguished the bug
+from the fix, and two passed against broken code: the dialog's
+`minimumSizeHint` does not track form rows and read 580 either way, and a
+`qMin` against the scroll area's own size hint read small whether or not the
+cap was set, because an uncapped `maximumHeight` is `QWIDGETSIZE_MAX`. The
+assertions that survive mutation are the editor pane's minimum inside the
+splitter, and the cap read directly. A row's size hint is also invalid until
+the event loop has run, so the test needs `processEvents` after selecting a
+rule or it measures one row's height twice.
+
+**Size: XS.**
+
 ## Deferred, unsized, or split out
 
 Items noted while triaging but not part of the original list. Same numbering
diff --git a/docs/superpowers/plans/2026-08-13-rule-builder.md b/docs/superpowers/plans/2026-08-13-rule-builder.md
index fb39cca..96fddc6 100644
--- a/docs/superpowers/plans/2026-08-13-rule-builder.md
+++ b/docs/superpowers/plans/2026-08-13-rule-builder.md
@@ -376,9 +376,20 @@ bool needsQuotes(const RuleTerm &term)
 {
     if (term.field == RuleTerm::Folder)
         return true;
-    if (term.op == RuleTerm::Is || term.op == RuleTerm::IsNot)
+    if (term.value.contains(QLatin1Char(' ')))
         return true;
-    return term.value.contains(QLatin1Char(' '));
+    // Is/IsNot means an exact phrase, and only the free-text fields need
+    // quotes to express one. A tag or an attachment name is a single bare
+    // token to notmuch, which reads `tag:inbox` and `tag:"inbox"` identically
+    // (both count 5322 against the live index). Quoting them would therefore
+    // change the stored string without changing what it matches, and this
+    // type's whole contract is that an unedited rule compiles back byte for
+    // byte.
+    if (term.op == RuleTerm::Is || term.op == RuleTerm::IsNot) {
+        return term.field == RuleTerm::From || term.field == RuleTerm::To
+               || term.field == RuleTerm::Cc || term.field == RuleTerm::Subject;
+    }
+    return false;
 }
 
 QString compileTerm(const RuleTerm &term)
@@ -793,6 +804,9 @@ bool parseTerm(const QString &token, RuleTerm *out)
         return !value.isEmpty();
     }
 
+    // Tag and Attachment compile unquoted (see needsQuotes in Task 2), so
+    // their operator must not be inferred from the quoting: reading a quoted
+    // tag back as Is would compile it unquoted and change the stored string.
     if (field == RuleTerm::Attachment)
         out->op = RuleTerm::Has;
     else if (field == RuleTerm::Tag)
@@ -1099,6 +1113,11 @@ void TestRuleQuery::anUnrepresentableQueryRejectsWhole()
         QStringLiteral("date:2026-01-01..2026-02-01"),  // two-sided range
         QStringLiteral("from:a.example.org xor subject:x"),
     };
+    // NOT in this list: `from:((((`. It parses, as a From row whose value is
+    // the literal text `((((`, and round-trips byte for byte. That is exactly
+    // what the query means to notmuch, which treats the parens as characters
+    // to search for rather than as grouping, so the row tells the truth and
+    // rejecting it would buy nothing. See the test below.
 
     for (const QString &query : unrepresentable) {
         const RuleQuery q = RuleQuery::parse(query);
@@ -1112,11 +1131,19 @@ void TestRuleQuery::anUnrepresentableQueryRejectsWhole()
 
 void TestRuleQuery::aMalformedQueryIsRejectedNotDiagnosed()
 {
-    // notmuch accepts `from:((((` cleanly and matches nothing, so there is no
-    // failure to observe and a test asserting one fails against correct code.
-    // The assertion is on OUR rejection only.
+    // notmuch accepts `from:((((` cleanly and matches nothing: the parens are
+    // characters it searches for, not grouping. So there is no failure to
+    // observe, and a test asserting one fails against correct code.
+    //
+    // This parser accepts it too, as a From row whose value is that literal
+    // text, which is what the query actually means. What must hold is the
+    // round trip, not a rejection: displaying it as a row and compiling it
+    // back must not alter the stored string.
     const RuleQuery q = RuleQuery::parse(QStringLiteral("from:(((("));
-    QVERIFY(!q.parsed);
+    QVERIFY(q.parsed);
+    QCOMPARE(q.terms.size(), 1);
+    QCOMPARE(q.terms.at(0).value, QStringLiteral("(((("));
+    QCOMPARE(q.compile(), QStringLiteral("from:(((("));
 }
 ```
 
diff --git a/docs/superpowers/specs/2026-08-13-rule-builder-design.md b/docs/superpowers/specs/2026-08-13-rule-builder-design.md
index c11ef47..4d6c001 100644
--- a/docs/superpowers/specs/2026-08-13-rule-builder-design.md
+++ b/docs/superpowers/specs/2026-08-13-rule-builder-design.md
@@ -221,7 +221,12 @@ 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 `/**`.
+`folder:`), a bare word with no prefix, a `path:` not ending in `/**`, a
+two-sided `date:` range, a trailing operator, an unterminated quote.
+
+A parenthesis **inside a value** is not a shape at all: `from:((((` is a From
+term whose value happens to contain parens, and it parses and round-trips like
+any other. Only a parenthesis in grouping position is a shape question.
 
 ### The parser is strict, and that is the safety property
 
@@ -270,7 +275,7 @@ 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
+Match    (o) all   ( ) any
    [From    v] [contains v] [vendor.example.org  ] [+] [-]
    [From    v] [contains v] [vendor.example.net  ] [+] [-]
 But not
@@ -282,9 +287,22 @@ Add tags     [ vendor, receipts    ]
 Remove tags  [                     ]
 Note         [ ...                 ]
 
-Query    (from:vendor.example.org or ...) and not subject:receipt  [Count matches]
+Query    (from:vendor.example.org or ...) and not subject:receipt
+                                                   [ ] Edit as text
 ```
 
+**The "Edit as text" toggle belongs to the QUERY row, not to the match row.**
+An earlier draft of this sketch put it beside the all/any radios, which is
+where it reads best and is also wrong: switching to text mode hides the
+builder, and a checkbox living inside the builder disappears with it, leaving
+no way back except closing the dialog. That shipped and a hand test found it
+within minutes. The query row is visible in both modes, so a toggle there is
+always reachable.
+
+The test for this must assert **reachability**, not the checked state. A
+hidden checkbox reports its state perfectly well, so a state assertion passes
+against the broken layout.
+
 **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
@@ -334,10 +352,16 @@ 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.
+`and`/`or` without parens, `body:foo`, a bare word, a `path:` without `/**`, a
+two-sided `date:` range, and a trailing operator.
+
+**`from:((((` is not among them, and the reason is worth stating.** notmuch
+treats those parens as characters to search for rather than as grouping, so the
+query is meaningful, matches nothing, and reports no error. This parser accepts
+it as a From row whose value is that literal text, which is what it means. The
+assertion there is the **round trip**, never a rejection and never a provoked
+notmuch failure: `CLAUDE.md` records twice that notmuch accepts it cleanly, and
+a test expecting an error 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