Edit a rule from the rules screen

A rule could be written and deleted but never changed, so fixing a glob meant
deleting the rule and typing it again -- losing its note, the comments around
it, and its position, which still breaks ties between equally specific rules.
e now opens the selected rule in the builder and enter rewrites it where it
sits. config.ReplaceRule edits rules.toml textually, as deleting does, and
keeps the comments above the rule: they say why it is there, which editing its
glob rarely changes.

The round trip must not lose what the form does not show. The builder has four
fields and a Rule has five, so an edit carries the type pattern through
untouched and says so under the glob; a type-only rule saves without one. The
preview needed the same care in reverse: a working rule's transactions are
tagged, so an untagged-only preview would be empty for it. Its own rows are
added back, and the ones a narrowed glob stops catching stay on screen marked
-- giving one up is the decision being made, and it must not happen silently.

The builder and its list shared one return view, so opening the builder from
the list left esc pointing back into the form. Each screen now remembers its
own way out; transfers had the same trap and the same fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/config/config.go

@@ -135,14 +135,73 @@ func checkTransfer(t Transfer) error {
 // The file is rewritten through a temporary file so a failure part-way cannot
 // leave the user with a truncated config.
 func AppendRule(root string, r Rule) error {
+	if err := checkRule(r); err != nil {
+		return err
+	}
+	return appendBlock(root, formatRule(r))
+}
+
+// checkRule rejects a rule that would match nothing or decide nothing.
+func checkRule(r Rule) error {
 	if r.Match == "" && r.Type == "" {
 		return fmt.Errorf("a rule needs a match or type pattern")
 	}
 	if r.Tag == "" {
 		return fmt.Errorf("a rule needs a tag")
 	}
+	return nil
+}
 
-	return appendBlock(root, formatRule(r))
+// ReplaceRule rewrites the rule at the given position (0-based, as loaded by
+// LoadRules) with r, leaving it where it sits. Position no longer decides
+// precedence, but it still breaks ties, so an edited rule that moved could
+// start losing to — or start beating — an equally specific one it never used to.
+//
+// Like DeleteRules this edits the file textually rather than re-serialising the
+// parsed rules, because comments and formatting are not recoverable from
+// []Rule. Only the rule's own lines are replaced: the comments directly above
+// it say why it is there, which editing its glob rarely changes, so they stay.
+func ReplaceRule(root string, pos int, r Rule) error {
+	if err := checkRule(r); err != nil {
+		return err
+	}
+
+	path := filepath.Join(root, RulesFile)
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		return fmt.Errorf("read %s: %w", path, err)
+	}
+	lines := strings.Split(string(raw), "\n")
+
+	starts := blockStarts(lines)
+	rules, transfers := blocksOf(starts, "rule"), blocksOf(starts, "transfer")
+	if pos < 0 || pos >= len(rules) {
+		return fmt.Errorf("rule %d is out of range; %s holds %d rules", pos+1, path, len(rules))
+	}
+
+	from, to := blockExtent(lines, starts, rules[pos])
+	kept := make([]string, 0, len(lines))
+	kept = append(kept, lines[:from]...)
+	kept = append(kept, strings.Split(strings.TrimRight(formatRule(r), "\n"), "\n")...)
+	kept = append(kept, lines[to:]...)
+	out := strings.Join(kept, "\n")
+
+	// Never write something that will not load again, and never let editing one
+	// rule disturb a neighbour — of either kind, since both live in this file.
+	var check Rules
+	if _, err := toml.Decode(out, &check); err != nil {
+		return fmt.Errorf("editing %s would produce invalid TOML: %w", path, err)
+	}
+	if len(check.Rule) != len(rules) || len(check.Transfer) != len(transfers) {
+		return fmt.Errorf("editing %s would leave %d rules and %d transfers, expected %d and %d",
+			path, len(check.Rule), len(check.Transfer), len(rules), len(transfers))
+	}
+	if check.Rule[pos] != r {
+		return fmt.Errorf("editing %s would leave rule %d as %+v, expected %+v",
+			path, pos+1, check.Rule[pos], r)
+	}
+
+	return writeFileAtomic(root, path, out)
 }
 
 // AppendTransfer adds a transfer definition to the end of rules.toml. Order
@@ -216,6 +275,45 @@ func blockStarts(lines []string) []blockStart {
 	return out
 }
 
+// blocksOf lists where the entries of one table sit among all the blocks, so a
+// position in rules.toml as LoadRules numbers it can be turned into a position
+// in the file.
+func blocksOf(starts []blockStart, table string) []int {
+	var out []int
+	for k, s := range starts {
+		if s.table == table {
+			out = append(out, k)
+		}
+	}
+	return out
+}
+
+// commentPrefix walks back from a block's header over the comment lines it
+// owns. An entry owns the run of comments directly above it, with no blank line
+// in between; anything further up is a heading for what follows.
+func commentPrefix(lines []string, header int) int {
+	i := header
+	for i > 0 && strings.HasPrefix(strings.TrimSpace(lines[i-1]), "#") {
+		i--
+	}
+	return i
+}
+
+// blockExtent returns the k-th block's own lines as a half-open range: from its
+// header to the last line it owns, leaving out the comments introducing the
+// next block and the blank lines between the two, which belong to neither.
+func blockExtent(lines []string, starts []blockStart, k int) (from, to int) {
+	from = starts[k].line
+	to = len(lines)
+	if k+1 < len(starts) {
+		to = commentPrefix(lines, starts[k+1].line)
+	}
+	for to > from+1 && strings.TrimSpace(lines[to-1]) == "" {
+		to--
+	}
+	return from, to
+}
+
 // deleteBlocks removes entries of one table from rules.toml.
 //
 // The file is edited textually rather than re-serialised from the parsed
@@ -241,12 +339,7 @@ func deleteBlocks(root, table string, positions []int) (int, error) {
 
 	starts := blockStarts(lines)
 	// mine[p] is where the p-th entry of this table sits among all the blocks.
-	var mine []int
-	for k, s := range starts {
-		if s.table == table {
-			mine = append(mine, k)
-		}
-	}
+	mine := blocksOf(starts, table)
 	for _, p := range positions {
 		if p < 0 || p >= len(mine) {
 			return 0, fmt.Errorf("%s %d is out of range; %s holds %d %ss",
@@ -254,33 +347,16 @@ func deleteBlocks(root, table string, positions []int) (int, error) {
 		}
 	}
 
-	// An entry owns the run of comment lines directly above it, with no blank
-	// line in between. Anything further up is a heading for what follows.
-	prefix := func(k int) int {
-		i := starts[k].line
-		for i > 0 && strings.HasPrefix(strings.TrimSpace(lines[i-1]), "#") {
-			i--
-		}
-		return i
-	}
-
 	drop := map[int]bool{}
 	for p := range doomed {
 		k := mine[p]
-		// The block runs up to the next block's comment prefix, so a comment
-		// introducing the following one is not swept up with this one.
-		end := len(lines)
-		if k+1 < len(starts) {
-			end = prefix(k + 1)
-		}
-		for i := prefix(k); i < end; i++ {
+		// A rule's own comments document it and go with it; blockExtent leaves
+		// out the blank lines below it, which are the gap between blocks and
+		// part of neither, so the neighbours are not glued together.
+		_, end := blockExtent(lines, starts, k)
+		for i := commentPrefix(lines, starts[k].line); i < end; i++ {
 			drop[i] = true
 		}
-		// Blank lines are the gap between blocks, not part of either; leaving
-		// them avoids gluing the neighbours together.
-		for i := end - 1; i >= starts[k].line && strings.TrimSpace(lines[i]) == ""; i-- {
-			delete(drop, i)
-		}
 	}
 
 	kept := make([]string, 0, len(lines))

internal/config/config_test.go

@@ -673,3 +673,140 @@ func TestTransferToleranceIsBounded(t *testing.T) {
 		t.Error("expected an out-of-range tolerance to be refused on load")
 	}
 }
+
+// Editing rewrites the rule where it sits: the position is what still breaks
+// ties between equally specific rules, so a rule that moved could start beating
+// one it never used to.
+func TestReplaceRuleKeepsPositionAndComments(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, RulesFile)
+	if err := os.WriteFile(path, []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	edited := Rule{Match: "*LIDL SOFIA*", Tag: "groceries", Account: "checking", Note: "just the branch"}
+	if err := ReplaceRule(root, 0, edited); err != nil {
+		t.Fatal(err)
+	}
+
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatalf("the file no longer parses after editing: %v", err)
+	}
+	if len(loaded.Rule) != 3 {
+		t.Fatalf("got %d rules, want the same 3: %+v", len(loaded.Rule), loaded.Rule)
+	}
+	if loaded.Rule[0] != edited {
+		t.Errorf("rule 1 = %+v, want %+v", loaded.Rule[0], edited)
+	}
+	if loaded.Rule[1].Match != "*PAYROLL*" || loaded.Rule[2].Match != "*TO SAVINGS*" {
+		t.Errorf("the rest = %+v, want them where they were", loaded.Rule[1:])
+	}
+
+	// The comment above a rule says why it is there, which editing its glob
+	// does not change.
+	text, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, want := range []string{"# Rules for my accounts.", "# Weekly shop.", "# Moving money to myself."} {
+		if !strings.Contains(string(text), want) {
+			t.Errorf("comment %q was lost:\n%s", want, text)
+		}
+	}
+	if strings.Contains(string(text), `"*LIDL*"`) {
+		t.Errorf("the old pattern is still there:\n%s", text)
+	}
+}
+
+// A rule and the transfer below it share the file, so a replacement has to end
+// where the next block of any kind begins.
+func TestReplaceRuleLeavesTheOtherKindAlone(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, RulesFile)
+	original := `[[rule]]
+match = "*LIDL*"
+tag   = "groceries"
+
+# Moving money to the broker.
+[[transfer]]
+from_account = "nlb"
+from_desc    = "*TO TRADEREPUBLIC*"
+to_account   = "traderepublic"
+to_desc      = "*FROM NLB*"
+
+[[rule]]
+match = "*ZARA*"
+tag   = "clothes"
+`
+	if err := os.WriteFile(path, []byte(original), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := ReplaceRule(root, 0, Rule{Match: "*KAUFLAND*", Tag: "groceries"}); err != nil {
+		t.Fatal(err)
+	}
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded.Transfer) != 1 || loaded.Transfer[0].FromDesc != "*TO TRADEREPUBLIC*" {
+		t.Errorf("transfers = %+v, want the definition untouched", loaded.Transfer)
+	}
+	if len(loaded.Rule) != 2 || loaded.Rule[0].Match != "*KAUFLAND*" || loaded.Rule[1].Match != "*ZARA*" {
+		t.Errorf("rules = %+v, want only the first one rewritten", loaded.Rule)
+	}
+	raw, _ := os.ReadFile(path)
+	if !strings.Contains(string(raw), "# Moving money to the broker.") {
+		t.Errorf("the transfer's comment was lost:\n%s", raw)
+	}
+}
+
+// Editing a rule that sets a type keeps it: the caller decides what the rule
+// becomes, and a pattern left out of the replacement is a pattern removed.
+func TestReplaceRuleWritesEveryPattern(t *testing.T) {
+	root := t.TempDir()
+	body := "[[rule]]\nmatch = \"*SPOTIFY*\"\ntype  = \"CARD_PAYMENT\"\ntag   = \"music\"\n"
+	if err := os.WriteFile(filepath.Join(root, RulesFile), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	kept := Rule{Match: "*SPOTIFY*", Type: "CARD_PAYMENT", Tag: "subscriptions"}
+	if err := ReplaceRule(root, 0, kept); err != nil {
+		t.Fatal(err)
+	}
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if loaded.Rule[0] != kept {
+		t.Errorf("rule = %+v, want %+v", loaded.Rule[0], kept)
+	}
+
+	// A type-only rule is a rule; a rule with neither pattern is not.
+	if err := ReplaceRule(root, 0, Rule{Type: "CARD_PAYMENT", Tag: "cards"}); err != nil {
+		t.Errorf("a type-only rule was refused: %v", err)
+	}
+	if err := ReplaceRule(root, 0, Rule{Tag: "nothing"}); err == nil {
+		t.Error("expected a rule with no pattern to be rejected")
+	}
+	if err := ReplaceRule(root, 0, Rule{Match: "*X*"}); err == nil {
+		t.Error("expected a rule with no tag to be rejected")
+	}
+}
+
+func TestReplaceRuleOutOfRange(t *testing.T) {
+	root := t.TempDir()
+	if err := AppendRule(root, Rule{Match: "*LIDL*", Tag: "groceries"}); err != nil {
+		t.Fatal(err)
+	}
+	for _, pos := range []int{-1, 1, 7} {
+		if err := ReplaceRule(root, pos, Rule{Match: "*X*", Tag: "x"}); err == nil {
+			t.Errorf("position %d: expected an error", pos)
+		}
+	}
+	loaded, _ := LoadRules(root)
+	if len(loaded.Rule) != 1 || loaded.Rule[0].Match != "*LIDL*" {
+		t.Errorf("rules = %+v, want the file untouched", loaded.Rule)
+	}
+}

internal/tui/tui.go

@@ -84,27 +84,41 @@ type Model struct {
 
 	txns []model.Transaction // rows currently shown in txnTable
 
-	// Rule builder: three inputs on the left, and a live preview on the right
-	// of which untagged descriptions the glob would catch.
-	ruleGlob    textinput.Model
-	ruleAccount textinput.Model
-	ruleTag     textinput.Model
-	ruleNote    textinput.Model
-	ruleFocus   int // which of the inputs has the cursor
-	ruleTable   table.Model
-	ruleReturn  view // the view to go back to on esc
-	untagged    []descGroup
-	ruleMatches int // untagged descriptions the current glob matches
+	// Rule builder: four inputs on the left, and a live preview on the right of
+	// which descriptions the glob would catch.
+	ruleGlob      textinput.Model
+	ruleAccount   textinput.Model
+	ruleTag       textinput.Model
+	ruleNote      textinput.Model
+	ruleFocus     int // which of the inputs has the cursor
+	ruleTable     table.Model
+	ruleReturn    view // the view to go back to on esc
+	previewGroups []descGroup
+	ruleMatches   int // previewed descriptions the current glob matches
 	// ruleCandidates is how many were in view before the glob filtered them,
 	// which the count needs: the preview now shows only matches, so the rows on
 	// screen can no longer say what they were chosen out of.
 	ruleCandidates int
+	// ruleDropped counts the descriptions an edit would let go of. It is only
+	// ever non-zero while editing: a new rule has nothing to lose.
+	ruleDropped int
+
+	// Editing an existing rule reuses the builder rather than a second form:
+	// the fields start filled in, and enter rewrites the rule where it sits
+	// instead of appending. ruleEditPos is its file position, which the edit
+	// must not change — that is what still breaks ties between equally specific
+	// rules. ruleEditOrig is the rule as it was, so what the form does not show
+	// (the type pattern) survives the round trip instead of being dropped.
+	ruleEditing  bool
+	ruleEditPos  int
+	ruleEditOrig config.Rule
 
 	// Rule list: every rule with the number of transactions it actually
 	// claims, so dead ones can be found and removed.
-	ruleListTable table.Model
-	ruleUsage     []int
-	confirm       confirmation
+	ruleListTable  table.Model
+	ruleUsage      []int
+	ruleListReturn view // the view to go back to on esc
+	confirm        confirmation
 
 	// Transfer builder: the two sides of a movement on the left, and on the
 	// right the pairs it would form out of what is already imported, together
@@ -129,8 +143,9 @@ type Model struct {
 	previewFeeDigits int
 
 	// Transfer list: every definition with what it currently pairs.
-	transferListTable table.Model
-	transferResult    transfers.Result
+	transferListTable  table.Model
+	transferResult     transfers.Result
+	transferListReturn view // the view to go back to on esc
 
 	filter       store.Filter
 	onlyUntagged bool
@@ -418,25 +433,55 @@ const (
 	ruleFormWidth  = ruleInputWidth + 12
 )
 
-// descGroup is one distinct untagged description and how often it occurs.
-// Grouping matters: a statement holds the same payee dozens of times, and a
-// rule is written against the description, not against individual rows.
+// descGroup is one distinct description the builder previews against, and how
+// often it occurs. Grouping matters: a statement holds the same payee dozens of
+// times, and a rule is written against the description, not against individual
+// rows.
 type descGroup struct {
 	Description string
 	Accounts    map[string]bool
 	Count       int
+	// Claimed marks a description the rule being edited currently tags. Nothing
+	// sets it for a new rule, where every candidate is untagged by definition.
+	Claimed bool
 }
 
-// reloadUntagged rebuilds the alphabetical list of untagged descriptions that
-// the rule builder previews against.
-func (m *Model) reloadUntagged() error {
+// reloadPreviewGroups rebuilds the alphabetical list of descriptions the rule
+// builder previews against: everything still waiting for a rule, plus — when a
+// rule is being edited — what that rule already claims.
+//
+// The addition is what makes editing legible. Those rows are tagged, so none of
+// them is in the untagged list, and the preview for a rule that works perfectly
+// would otherwise be empty. They are also exactly what an edit is judged
+// against: narrowing a glob is a decision about which of them to let go.
+func (m *Model) reloadPreviewGroups() error {
 	txns, err := m.db.Transactions(store.Filter{Untagged: true})
 	if err != nil {
 		return err
 	}
+	claimedFrom := len(txns) // everything appended below is claimed by the edited rule
+	if m.ruleEditing {
+		seen := make(map[int64]bool, len(txns))
+		for _, t := range txns {
+			seen[t.ID] = true
+		}
+		all, err := m.db.Transactions(store.Filter{})
+		if err != nil {
+			return err
+		}
+		for _, t := range all {
+			// An index written before rules.toml last changed can hold a row
+			// that is untagged on disk and claimed by the engine in memory;
+			// counting it twice would overstate the group.
+			if seen[t.ID] || m.engine.MatchIndex(t.AccountSlug, t) != m.ruleEditPos {
+				continue
+			}
+			txns = append(txns, t)
+		}
+	}
 
 	byDesc := map[string]*descGroup{}
-	for _, t := range txns {
+	for i, t := range txns {
 		key := model.NormalizeDescription(t.Description)
 		g, ok := byDesc[key]
 		if !ok {
@@ -445,21 +490,31 @@ func (m *Model) reloadUntagged() error {
 		}
 		g.Accounts[t.AccountSlug] = true
 		g.Count++
+		g.Claimed = g.Claimed || i >= claimedFrom
 	}
 
-	m.untagged = make([]descGroup, 0, len(byDesc))
+	m.previewGroups = make([]descGroup, 0, len(byDesc))
 	for _, g := range byDesc {
-		m.untagged = append(m.untagged, *g)
+		m.previewGroups = append(m.previewGroups, *g)
 	}
-	sort.Slice(m.untagged, func(i, j int) bool {
-		a := model.NormalizeDescription(m.untagged[i].Description)
-		b := model.NormalizeDescription(m.untagged[j].Description)
+	sort.Slice(m.previewGroups, func(i, j int) bool {
+		a := model.NormalizeDescription(m.previewGroups[i].Description)
+		b := model.NormalizeDescription(m.previewGroups[j].Description)
 		if a != b {
 			return a < b
 		}
-		return m.untagged[i].Description < m.untagged[j].Description
+		return m.previewGroups[i].Description < m.previewGroups[j].Description
 	})
 
+	// The column says what is in it, which an edit changes: the rows it claims
+	// are on screen precisely because they are not waiting for a rule.
+	cols := m.ruleTable.Columns()
+	cols[1].Title = "Untagged description"
+	if m.ruleEditing {
+		cols[1].Title = "Description"
+	}
+	m.ruleTable.SetColumns(cols)
+
 	m.refreshRulePreview()
 	return nil
 }
@@ -474,29 +529,40 @@ func (m *Model) reloadUntagged() error {
 // still waiting for a rule — which is the other question this screen answers.
 // Either way the count beside the form is measured against everything in view,
 // so a glob that has narrowed the list to three of forty still says so.
+//
+// A description the rule being edited currently tags is the one exception to
+// the rows going: losing one is the thing worth seeing before saving, so it
+// stays on screen marked “−” instead of vanishing silently with the rest.
 func (m *Model) refreshRulePreview() {
 	var (
 		pattern = strings.TrimSpace(m.ruleGlob.Value())
 		account = strings.TrimSpace(m.ruleAccount.Value())
-		rows    = make([]table.Row, 0, len(m.untagged))
+		rows    = make([]table.Row, 0, len(m.previewGroups))
 	)
-	m.ruleMatches, m.ruleCandidates = 0, 0
+	m.ruleMatches, m.ruleCandidates, m.ruleDropped = 0, 0, 0
 
-	for _, g := range m.untagged {
+	for _, g := range m.previewGroups {
 		// An account filter narrows the preview the same way the saved rule
 		// will narrow its matching.
-		if account != "" && !g.Accounts[account] {
-			continue
+		inAccount := account == "" || g.Accounts[account]
+		if inAccount {
+			m.ruleCandidates++
 		}
-		m.ruleCandidates++
+		matched := inAccount &&
+			(pattern == "" || glob.Match(pattern, model.NormalizeDescription(g.Description)))
 
 		marker := " "
-		if pattern != "" {
-			if !glob.Match(pattern, model.NormalizeDescription(g.Description)) {
-				continue
-			}
+		switch {
+		case matched && pattern != "":
 			marker = "▸"
 			m.ruleMatches++
+		case matched:
+			// No glob yet, so the row is context rather than an answer.
+		case g.Claimed:
+			marker = "−"
+			m.ruleDropped++
+		default:
+			continue
 		}
 		rows = append(rows, table.Row{marker, g.Description, fmt.Sprintf("%d", g.Count)})
 	}
@@ -512,8 +578,9 @@ func (m *Model) refreshRulePreview() {
 	m.ruleTable.SetCursor(cursor)
 }
 
-// saveRule appends the composed rule to rules.toml, reloads the engine and
-// retags, so the effect is visible immediately.
+// saveRule writes the composed rule to rules.toml — appending it, or rewriting
+// the rule being edited in place — then reloads the engine and retags, so the
+// effect is visible immediately.
 func (m *Model) saveRule() error {
 	r := config.Rule{
 		Match:   strings.TrimSpace(m.ruleGlob.Value()),
@@ -521,7 +588,13 @@ func (m *Model) saveRule() error {
 		Tag:     strings.TrimSpace(m.ruleTag.Value()),
 		Note:    strings.TrimSpace(m.ruleNote.Value()),
 	}
-	if r.Match == "" {
+	if m.ruleEditing {
+		// The form has no type field, so an edit carries the rule's type
+		// through untouched rather than quietly dropping a pattern it never
+		// showed the user. It is also why a type rule can have no glob at all.
+		r.Type = m.ruleEditOrig.Type
+	}
+	if r.Match == "" && r.Type == "" {
 		return fmt.Errorf("enter a glob first, e.g. *LIDL*")
 	}
 	if r.Tag == "" {
@@ -531,7 +604,11 @@ func (m *Model) saveRule() error {
 		return fmt.Errorf("no account called %q; leave it blank to apply to every account", r.Account)
 	}
 
-	if err := config.AppendRule(m.root, r); err != nil {
+	if m.ruleEditing {
+		if err := config.ReplaceRule(m.root, m.ruleEditPos, r); err != nil {
+			return err
+		}
+	} else if err := config.AppendRule(m.root, r); err != nil {
 		return err
 	}
 	if err := m.reloadConfig(); err != nil {
@@ -543,7 +620,18 @@ func (m *Model) saveRule() error {
 		return err
 	}
 
-	m.status = fmt.Sprintf("saved rule %s → %s, %d transactions retagged", r.Match, r.Tag, n)
+	if m.ruleEditing {
+		m.status = fmt.Sprintf("rule %d is now %s → %s, %d transactions retagged",
+			m.ruleEditPos+1, rulePattern(r), r.Tag, n)
+		// The account came from the rule rather than from the user, so it goes
+		// with the rest; an appended rule keeps it, since the next one written
+		// is usually for the same account.
+		m.ruleEditing = false
+		m.ruleEditOrig = config.Rule{}
+		m.ruleAccount.SetValue("")
+	} else {
+		m.status = fmt.Sprintf("saved rule %s → %s, %d transactions retagged", r.Match, r.Tag, n)
+	}
 	m.ruleGlob.SetValue("")
 	m.ruleTag.SetValue("")
 	m.ruleNote.SetValue("")
@@ -752,9 +840,16 @@ func (m *Model) deleteRules(positions []int) error {
 
 // openRuleList switches to the rule list, remembering where to return to.
 func (m *Model) openRuleList() {
-	if m.view != viewRuleList {
-		m.ruleReturn = m.view
+	if m.view != viewRuleList && m.view != viewRules {
+		m.ruleListReturn = m.view
 	}
+	m.showRuleList()
+}
+
+// showRuleList switches to the rule list without recording where esc goes.
+// Coming back from the builder must not record it as the way out, or esc from
+// the list would bounce back into the form the user just left.
+func (m *Model) showRuleList() {
 	m.view = viewRuleList
 	m.confirm = confirmNone
 	if err := m.reloadRuleList(); err != nil {
@@ -791,7 +886,7 @@ func (m *Model) updateRuleList(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 	case "q", "ctrl+c":
 		return m, tea.Quit
 	case "esc":
-		m.view = m.ruleReturn
+		m.view = m.ruleListReturn
 		return m, nil
 	case "1":
 		m.view = viewAccounts
@@ -810,6 +905,9 @@ func (m *Model) updateRuleList(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		m.openTransferList()
 		return m, nil
 
+	case "e":
+		return m, m.openRuleEditor(m.ruleListTable.Cursor())
+
 	case "d":
 		i := m.ruleListTable.Cursor()
 		rs := m.engine.Rules()
@@ -842,21 +940,55 @@ func (m *Model) updateRuleList(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 	return m, cmd
 }
 
-// openRuleBuilder switches to the rule builder, remembering where to return to
-// and seeding the account field from whatever account is being browsed.
+// openRuleBuilder switches to the rule builder on a new rule, seeding the
+// account field from whatever account is being browsed.
 func (m *Model) openRuleBuilder() tea.Cmd {
+	// Leaving an edit, the fields still hold that rule. A new rule starting out
+	// as a copy of the one just edited would be saved as a second, almost
+	// identical rule, so the form is emptied on the way out of edit mode.
+	if m.ruleEditing {
+		m.ruleEditing = false
+		m.ruleEditOrig = config.Rule{}
+		for _, in := range m.ruleInputs() {
+			in.SetValue("")
+		}
+	}
+	if m.ruleAccount.Value() == "" && m.filter.AccountSlug != "" {
+		m.ruleAccount.SetValue(m.filter.AccountSlug)
+	}
+	return m.showRuleBuilder()
+}
+
+// openRuleEditor switches to the rule builder on the rule at a file position:
+// the fields start filled in from it, and saving rewrites it where it sits
+// rather than appending a near-duplicate.
+func (m *Model) openRuleEditor(pos int) tea.Cmd {
+	rs := m.engine.Rules()
+	if pos < 0 || pos >= len(rs) {
+		return nil
+	}
+	r := rs[pos]
+	m.ruleEditing, m.ruleEditPos, m.ruleEditOrig = true, pos, r
+	m.ruleGlob.SetValue(r.Match)
+	m.ruleAccount.SetValue(r.Account)
+	m.ruleTag.SetValue(r.Tag)
+	m.ruleNote.SetValue(r.Note)
+	return m.showRuleBuilder()
+}
+
+// showRuleBuilder switches to the builder and loads what the preview needs,
+// remembering where to return to on esc.
+func (m *Model) showRuleBuilder() tea.Cmd {
 	if m.view != viewRules {
 		m.ruleReturn = m.view
 	}
 	m.view = viewRules
-	if m.ruleAccount.Value() == "" && m.filter.AccountSlug != "" {
-		m.ruleAccount.SetValue(m.filter.AccountSlug)
-	}
 	m.setRuleFocus(0)
 	if err := m.reloadSuggestions(); err != nil {
 		m.err = err
 	}
-	if err := m.reloadUntagged(); err != nil {
+	// Last: what the preview lists depends on whether a rule is being edited.
+	if err := m.reloadPreviewGroups(); err != nil {
 		m.err = err
 	}
 	return textinput.Blink
@@ -1092,9 +1224,11 @@ func (m *Model) saveTransfer() error {
 }
 
 // openTransferList switches to the transfer list, remembering where to return.
+// As on the rule list, the builder is not somewhere to return to: it is reached
+// from here, and esc would bounce between the two.
 func (m *Model) openTransferList() {
-	if m.view != viewTransferList {
-		m.transferReturn = m.view
+	if m.view != viewTransferList && m.view != viewTransfers {
+		m.transferListReturn = m.view
 	}
 	m.view = viewTransferList
 	m.confirm = confirmNone
@@ -1231,7 +1365,7 @@ func (m *Model) updateTransferList(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 	case "q", "ctrl+c":
 		return m, tea.Quit
 	case "esc":
-		m.view = m.transferReturn
+		m.view = m.transferListReturn
 		return m, nil
 	case "1":
 		m.view = viewAccounts
@@ -1495,11 +1629,18 @@ func (m *Model) updateRules(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		return m, nil
 	case "enter":
 		m.err = nil
+		editing := m.ruleEditing // saveRule clears it on the way through
 		if err := m.saveRule(); err != nil {
 			m.err = err
 			return m, nil
 		}
-		if err := m.reloadUntagged(); err != nil {
+		if editing {
+			// An edit is a round trip from the rules screen, so it ends there
+			// with the new counts rather than in an empty form.
+			m.showRuleList()
+			return m, nil
+		}
+		if err := m.reloadPreviewGroups(); err != nil {
 			m.err = err
 		}
 		return m, nil
@@ -1843,9 +1984,15 @@ func (m *Model) ruleFormView() string {
 	// Give up the blank lines between fields first (21 lines) and the hints on
 	// unfocused fields second (18), rather than letting the last field run off
 	// the bottom. Below that the box borders are the floor.
+	// An edit carrying a type pattern renders one line more, and two once the
+	// fields are spaced out, so it asks for that much more room before either.
+	typed := 0
+	if m.ruleEditing && m.ruleEditOrig.Type != "" {
+		typed = 1
+	}
 	room := m.height - 6
-	spaced := m.height <= 0 || room >= 25
-	hints := m.height <= 0 || room >= 21
+	spaced := m.height <= 0 || room >= 25+2*typed
+	hints := m.height <= 0 || room >= 21+typed
 
 	field := func(i int, label, help string) string {
 		name := labelStyle.Render("  " + label)
@@ -1867,6 +2014,14 @@ func (m *Model) ruleFormView() string {
 	var b strings.Builder
 	inputs := m.ruleInputs()
 	b.WriteString(field(0, "glob", "vs. the description"))
+	// A type pattern has no field of its own, so an edit carrying one says so
+	// rather than leaving the rule looking broader than it is.
+	if m.ruleEditing && m.ruleEditOrig.Type != "" {
+		b.WriteString(hintStyle.Render("+ type:"+m.ruleEditOrig.Type+" · kept") + "\n")
+		if spaced {
+			b.WriteString("\n")
+		}
+	}
 	b.WriteString(field(1, "account", completionHint(inputs[1], m.ruleFocus == 1, "blank = all accounts")))
 	b.WriteString(field(2, "tag", completionHint(inputs[2], m.ruleFocus == 2, "applied to matches")))
 	b.WriteString(field(3, "note", "why this rule exists"))
@@ -1874,9 +2029,15 @@ func (m *Model) ruleFormView() string {
 	// The count is the whole point of the preview: it says what the rule will
 	// do before it is written to disk.
 	summary := fmt.Sprintf("%d of %d descriptions match", m.ruleMatches, m.ruleCandidates)
-	if strings.TrimSpace(m.ruleGlob.Value()) == "" {
+	if strings.TrimSpace(m.ruleGlob.Value()) == "" && !m.ruleEditing {
 		summary = fmt.Sprintf("%d untagged descriptions", m.ruleCandidates)
 	}
+	// What an edit gives up is not visible in a count of what it keeps. It goes
+	// on its own line because the two together overflow the form's width, and
+	// lipgloss would wrap it mid-phrase.
+	if m.ruleDropped > 0 {
+		summary += fmt.Sprintf("\n%d no longer claimed", m.ruleDropped)
+	}
 	b.WriteString(matchCountStyle.Render(summary))
 
 	return ruleFormStyle.Render(b.String())
@@ -1979,6 +2140,9 @@ func (m *Model) title() string {
 	case viewAccounts:
 		return "money · accounts"
 	case viewRules:
+		if m.ruleEditing {
+			return fmt.Sprintf("money · editing rule %d · rewrites it in rules.toml", m.ruleEditPos+1)
+		}
 		return "money · rule builder · writes to rules.toml"
 	case viewRuleList:
 		unused := len(m.unusedRules())
@@ -2021,12 +2185,17 @@ func (m *Model) help() string {
 	case viewAccounts:
 		return "enter open · 2 transactions · 3 report · 4 new rule · 5 rules · 6 new transfer · 7 transfers · i import · r retag · q quit"
 	case viewRules:
-		return "tab complete/next field · ↑↓ field · ctrl+n/p other completions · pgup/pgdn scroll list · enter save rule · esc back · ctrl+c quit"
+		save := "enter save rule"
+		if m.ruleEditing {
+			save = "enter save changes"
+		}
+		return "tab complete/next field · ↑↓ field · ctrl+n/p other completions · pgup/pgdn scroll list · " +
+			save + " · esc back · ctrl+c quit"
 	case viewRuleList:
 		if m.confirm != confirmNone {
 			return "y confirm · any other key cancels"
 		}
-		return "d delete rule · p prune all unused · r refresh counts · 4 new rule · 7 transfers · 1 accounts · esc back · q quit"
+		return "e edit rule · d delete rule · p prune all unused · r refresh counts · 4 new rule · 7 transfers · 1 accounts · esc back · q quit"
 	case viewTransfers:
 		return "tab complete/next field · ↑↓ field · pgup/pgdn scroll pairs · enter save transfer · esc back · ctrl+c quit"
 	case viewTransferList:

internal/tui/tui_test.go

@@ -1914,3 +1914,213 @@ func TestRuleBuilderPreviewShowsOnlyMatches(t *testing.T) {
 		t.Errorf("summary = %q, want 0 of 4", m.ruleFormView())
 	}
 }
+
+// loadRulesFile puts a rules.toml under the model's root and makes the model
+// read it, as though it had been there all along.
+func loadRulesFile(t *testing.T, m *Model, body string) {
+	t.Helper()
+	if err := os.WriteFile(filepath.Join(m.root, config.RulesFile), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := m.reloadConfig(); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := m.engine.Retag(m.db); err != nil {
+		t.Fatal(err)
+	}
+	if err := m.reload(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+const editableRules = `[[rule]]
+match = "*ZZZ*"
+tag   = "misc"
+
+# The weekly shop.
+[[rule]]
+match = "*LIDL*"
+tag   = "groceries"
+note  = "both branches"
+`
+
+// Editing rewrites the rule where it sits rather than appending a second,
+// almost identical one, and lands back on the list it started from.
+func TestRuleListEditsRuleInPlace(t *testing.T) {
+	m, db, root := newRuleModel(t)
+	loadRulesFile(t, m, editableRules)
+
+	key(t, m, "5")
+	m.ruleListTable.SetCursor(1)
+	key(t, m, "e")
+
+	if m.view != viewRules || !m.ruleEditing {
+		t.Fatalf("e opened view %v (editing %v), want the builder on the rule", m.view, m.ruleEditing)
+	}
+	if m.ruleGlob.Value() != "*LIDL*" || m.ruleTag.Value() != "groceries" || m.ruleNote.Value() != "both branches" {
+		t.Fatalf("form = %q / %q / %q, want the rule filled in",
+			m.ruleGlob.Value(), m.ruleTag.Value(), m.ruleNote.Value())
+	}
+
+	m.ruleTag.SetValue("food")
+	key(t, m, "enter")
+
+	loaded, err := config.LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := config.Rule{Match: "*LIDL*", Tag: "food", Note: "both branches"}
+	if len(loaded.Rule) != 2 {
+		t.Fatalf("rules = %+v, want the same 2: an edit must not append", loaded.Rule)
+	}
+	if loaded.Rule[0].Match != "*ZZZ*" {
+		t.Errorf("rule 1 = %+v, want it where it was", loaded.Rule[0])
+	}
+	if loaded.Rule[1] != want {
+		t.Errorf("rule 2 = %+v, want %+v", loaded.Rule[1], want)
+	}
+
+	// The file is edited textually, so the comment documenting the rule stays.
+	raw, err := os.ReadFile(filepath.Join(root, config.RulesFile))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(raw), "# The weekly shop.") {
+		t.Errorf("the rule's comment was lost:\n%s", raw)
+	}
+
+	// Saving retags immediately, as it does for a new rule.
+	txns, err := db.Transactions(store.Filter{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	tagged := 0
+	for _, txn := range txns {
+		if txn.RuleTag == "food" {
+			tagged++
+		}
+	}
+	if tagged != 3 {
+		t.Errorf("%d transactions tagged food, want 3", tagged)
+	}
+
+	// An edit is a round trip: it ends on the list, with the new counts.
+	if m.view != viewRuleList {
+		t.Fatalf("view = %v after saving, want the rules list", m.view)
+	}
+	if rows := m.ruleListTable.Rows(); len(rows) != 2 || rows[1][4] != "food" {
+		t.Errorf("rows = %v, want the edited tag on the list", rows)
+	}
+	if !strings.Contains(m.status, "rule 2 is now") {
+		t.Errorf("status = %q, want it to name the rule that changed", m.status)
+	}
+}
+
+// The preview lists what the rule already claims, not only what is untagged:
+// for a rule that works those are the same rows, and an empty preview would say
+// nothing about the edit. Narrowing the glob then shows what it gives up.
+func TestRuleEditPreviewsWhatItDrops(t *testing.T) {
+	m, _, _ := newRuleModel(t)
+	loadRulesFile(t, m, editableRules)
+
+	key(t, m, "5")
+	m.ruleListTable.SetCursor(1)
+	key(t, m, "e")
+
+	all, matched := previewRows(m)
+	if len(all) != 2 || len(matched) != 2 {
+		t.Fatalf("preview = %v (matched %v), want the two descriptions the rule claims", all, matched)
+	}
+
+	m.ruleGlob.SetValue("*SOFIA*")
+	m.refreshRulePreview()
+
+	all, matched = previewRows(m)
+	if len(matched) != 1 || matched[0] != "LIDL SOFIA 4412" {
+		t.Errorf("matched = %v, want only the Sofia branch", matched)
+	}
+	// The row it stops claiming stays on screen: losing one is the thing worth
+	// seeing before saving, so it is marked rather than dropped silently.
+	if len(all) != 2 || m.ruleDropped != 1 {
+		t.Fatalf("preview = %v, dropped = %d; want the Varna row kept and counted", all, m.ruleDropped)
+	}
+	var dropped []string
+	for _, row := range m.ruleTable.Rows() {
+		if row[0] == "−" {
+			dropped = append(dropped, row[1])
+		}
+	}
+	if len(dropped) != 1 || dropped[0] != "LIDL VARNA 9911" {
+		t.Errorf("dropped rows = %v, want the Varna branch", dropped)
+	}
+	if !strings.Contains(m.ruleFormView(), "1 no longer claimed") {
+		t.Errorf("summary = %q, want the dropped count", m.ruleFormView())
+	}
+}
+
+// The form has no type field, so an edit carries the pattern through instead of
+// silently widening the rule to every transaction the glob matches.
+func TestRuleEditKeepsTypePattern(t *testing.T) {
+	m, _, root := newRuleModel(t)
+	loadRulesFile(t, m, "[[rule]]\nmatch = \"*ZZZ*\"\ntype  = \"CARD_PAYMENT\"\ntag   = \"misc\"\n")
+
+	key(t, m, "5")
+	key(t, m, "e")
+
+	if !strings.Contains(m.ruleFormView(), "type:CARD_PAYMENT") {
+		t.Errorf("form = %q, want it to name the type it is carrying", m.ruleFormView())
+	}
+
+	m.ruleTag.SetValue("cards")
+	key(t, m, "enter")
+
+	loaded, err := config.LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := config.Rule{Match: "*ZZZ*", Type: "CARD_PAYMENT", Tag: "cards"}
+	if len(loaded.Rule) != 1 || loaded.Rule[0] != want {
+		t.Errorf("rules = %+v, want %+v", loaded.Rule, want)
+	}
+}
+
+// Leaving an edit must not leave the form holding that rule, or the next thing
+// saved would be a near-duplicate of the rule just edited.
+func TestNewRuleAfterAnEditStartsEmpty(t *testing.T) {
+	m, _, _ := newRuleModel(t)
+	loadRulesFile(t, m, editableRules)
+
+	key(t, m, "5")
+	m.ruleListTable.SetCursor(1)
+	key(t, m, "e")
+	key(t, m, "esc")
+	key(t, m, "4")
+
+	if m.ruleEditing {
+		t.Error("4 opened the builder still in edit mode")
+	}
+	for _, in := range m.ruleInputs() {
+		if in.Value() != "" {
+			t.Errorf("form field = %q, want a blank form for a new rule", in.Value())
+		}
+	}
+}
+
+// The builder is reached from the list, so it is not somewhere the list can
+// return to: esc must walk back out, not bounce between the two screens.
+func TestEscFromRuleListDoesNotBounceIntoTheBuilder(t *testing.T) {
+	m, _, _ := newRuleModel(t)
+	loadRulesFile(t, m, editableRules)
+
+	key(t, m, "2")
+	key(t, m, "5")
+	key(t, m, "e")
+	key(t, m, "esc")
+	if m.view != viewRuleList {
+		t.Fatalf("esc from the builder went to %v, want the list it was opened from", m.view)
+	}
+	key(t, m, "esc")
+	if m.view != viewTxns {
+		t.Errorf("esc from the list went to %v, want the transactions it was opened from", m.view)
+	}
+}