Pair transfers from rules.toml

The boolean transfer flag went two commits ago because a one-sided verdict let half a movement vanish and left the report unbalanced. This is what replaces it: a [[transfer]] block names both legs, and only a matched pair is dropped from the report -- both legs together, never one. Legs pair within five days, nearest date first, and a transaction belongs to at most one transfer, so the first definition to claim a leg keeps it, exactly as the first matching rule keeps a tag. The pairing is derived state like the tags: Engine.Link rewrites the whole transfers table from rules.toml, which is why retag re-derives both halves of what that file decides, and why it runs over the whole index rather than a filtered view -- pairing inside one would let a movement count as a transfer in one report and not in another. An unmatched leg is not a transfer and keeps counting, surfaced as a warning instead. Within one currency the amount is the evidence and must be the exact opposite. Across currencies it is not checked at all: there are no rates here, so the two numbers are unrelated and the dates carry the pairing alone. tolerance_pct is the one exception, per definition, for a route where the bank takes a fee and the two statements genuinely disagree. It defaults to zero and belongs on the one definition that charges; a global or default tolerance would loosen every route that does not. The difference it admits is not forgiven -- the pair leaves the report entirely, so a fee hidden inside one would be spending that appears nowhere. Pair.Fee is what left less what arrived, and report.Excluded carries it out per currency alongside the legs. It counts only pairs whose legs are both in view, for the same reason it counts legs and not transfers: half a pair cannot say what the other half received. The screens: - 6 builds a definition against the index as you type, showing the pairs it would form and the legs it would catch but leave unpaired. Six fields need more room than the rule builder's four, so the form sheds its spacing, then its hints, then the borders on unfocused fields. - 7 lists every definition with what it pairs. Two counts, because they mean different things: an unpaired leg is a definition doing something and not finishing it, no pairs at all is dead weight. Tol names the tolerance, blank where amounts must agree. - 3 grows a (transfers) row under TOTAL, and a fees row beneath it, or the report silently disagrees with the account balances. Two things that are not part of transfers but are the same day's work: - ls --uniq lists each account and description once, normalised the way a glob sees them, which is the shape of "what still needs a rule?" -- fifty visits to one shop are one pattern to write, not fifty rows to read. - The rule builder's preview now filters to what the glob matches instead of marking matches in a full list. The count carries the context the rows no longer can: 2 of 7, measured against everything still in view. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-15 19:19:12 +02:00
parent ef6a231d98
commit ebf7770569
16 changed files with 3472 additions and 177 deletions
@@ -8,7 +8,9 @@ package main
 import (
 	"flag"
 	"fmt"
+	"io"
 	"os"
+	"sort"
 	"strings"
 	"text/tabwriter"

@@ -19,6 +21,7 @@ import (
 	"git.petrovv.com/nikola/money/internal/report"
 	"git.petrovv.com/nikola/money/internal/rules"
 	"git.petrovv.com/nikola/money/internal/store"
+	"git.petrovv.com/nikola/money/internal/transfers"
 	"git.petrovv.com/nikola/money/internal/tui"
 )

@@ -27,9 +30,10 @@ const usage = `money - statement-driven personal finance tracker
 usage: money [--root DIR] <command> [flags]

 commands:
-  tui                  browse transactions, build and prune the rules that tag them (default)
+  tui                  browse transactions, build the rules that tag them and the
+                       transfers that pair them across accounts (default)
  import               extract transactions from every statement into the index
-  retag                re-apply rules.toml to everything already imported
+  retag                re-apply rules.toml: retag everything and re-pair transfers
  ls                   list transactions
  report               spending by tag
  accounts             list accounts with balances
@@ -137,24 +141,33 @@ func cmdConfig(root string, source config.RootSource) error {
 	return nil
 }

+// opened is everything a command needs: the index, the account folders on
+// disk, and the two engines rules.toml describes.
+type opened struct {
+	db       *store.DB
+	accounts []*config.Account
+	engine   *rules.Engine
+	links    *transfers.Engine
+}
+
 // open loads the config and index that every command needs.
-func open(root string) (*store.DB, []*config.Account, *rules.Engine, error) {
+func open(root string) (*opened, error) {
 	if _, err := os.Stat(root); err != nil {
-		return nil, nil, nil, fmt.Errorf("data root %s is not readable: %w", root, err)
+		return nil, fmt.Errorf("data root %s is not readable: %w", root, err)
 	}
 	accounts, err := config.LoadAccounts(root)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, err
 	}
 	r, err := config.LoadRules(root)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, err
 	}
 	db, err := store.Open(config.IndexPath(root))
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, err
 	}
-	return db, accounts, rules.New(r), nil
+	return &opened{db: db, accounts: accounts, engine: rules.New(r), links: transfers.New(r)}, nil
 }

 func cmdImport(root string, args []string) error {
@@ -164,17 +177,17 @@ func cmdImport(root string, args []string) error {
 		return err
 	}

-	db, accounts, engine, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
+	defer o.db.Close()

-	if len(accounts) == 0 {
+	if len(o.accounts) == 0 {
 		return fmt.Errorf("no accounts found in %s (an account is a folder containing %s)", root, config.AccountFile)
 	}

-	res, err := importer.Run(root, db, accounts, engine, importer.Options{Force: *force})
+	res, err := importer.Run(root, o.db, o.accounts, o.engine, o.links, importer.Options{Force: *force})
 	if err != nil {
 		return err
 	}
@@ -199,6 +212,7 @@ func cmdImport(root string, args []string) error {

 	parsed, added, skipped := res.Total()
 	fmt.Printf("\n%d new, %d duplicate, %d parsed; %d rows retagged\n", added, skipped, parsed, res.Retagged)
+	reportPairing(res.Paired, res.Unpaired)

 	failures := res.Errs()
 	for _, f := range failures {
@@ -210,49 +224,88 @@ func cmdImport(root string, args []string) error {
 	return nil
 }

+// cmdRetag re-derives everything rules.toml decides: the tags and the transfer
+// pairing. They are one command because they are one file, and leaving half of
+// the derived state stale would be worse than not offering it at all.
 func cmdRetag(root string, _ []string) error {
-	db, _, engine, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
+	defer o.db.Close()

-	n, err := engine.Retag(db)
+	n, err := o.engine.Retag(o.db)
 	if err != nil {
 		return err
 	}
 	fmt.Printf("%d transactions retagged\n", n)
+
+	paired, unpaired, err := o.links.Link(o.db)
+	if err != nil {
+		return err
+	}
+	reportPairing(paired, unpaired)
 	return nil
 }

+// reportPairing prints the state of the transfer pairing. Unmatched legs are
+// sent to stderr: a leg that never found its other side is money that left an
+// account and cannot be shown to have arrived, which is a warning, not a
+// statistic.
+func reportPairing(paired, unpaired int) {
+	if paired == 0 && unpaired == 0 {
+		return
+	}
+	fmt.Printf("%d transfers matched\n", paired)
+	if unpaired > 0 {
+		fmt.Fprintf(os.Stderr, "warning: %d transfer leg(s) have no counterpart\n", unpaired)
+	}
+}
+
 func cmdLs(root string, args []string) error {
 	fs := flag.NewFlagSet("ls", flag.ContinueOnError)
 	account := fs.String("account", "", "only this account slug")
 	month := fs.String("month", "", "only this month (YYYY-MM)")
 	search := fs.String("search", "", "only descriptions containing this text")
-	untagged := fs.Bool("untagged", false, "only transactions no rule tagged")
+	untagged := fs.Bool("untagged", false, "only transactions no rule tagged and no transfer claimed")
 	limit := fs.Int("limit", 0, "maximum rows (0 = no limit)")
 	wide := fs.Bool("wide", false, "also show type and reported balance")
+	uniq := fs.Bool("uniq", false, "one row per distinct account and description")
 	if err := fs.Parse(args); err != nil {
 		return err
 	}
+	if *uniq && *wide {
+		// Both add columns, but a reported balance belongs to one row and
+		// nothing sensible can be printed for a whole group of them.
+		return fmt.Errorf("--uniq and --wide cannot be combined")
+	}

-	db, _, _, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
+	defer o.db.Close()

-	txns, err := db.Transactions(store.Filter{
+	f := store.Filter{
 		AccountSlug: *account,
 		Month:       *month,
 		Search:      *search,
 		Untagged:    *untagged,
 		Limit:       *limit,
-	})
+	}
+	// Deduplicating first would make a limit mean "the distinct descriptions
+	// among the newest N rows", which is not what it says. Under --uniq it caps
+	// what is printed instead.
+	if *uniq {
+		f.Limit = 0
+	}
+	txns, err := o.db.Transactions(f)
 	if err != nil {
 		return err
 	}
+	if *uniq {
+		return printUniq(os.Stdout, txns, *limit)
+	}

 	w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
 	if *wide {
@@ -268,17 +321,67 @@ func cmdLs(root string, args []string) error {
 			}
 			fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
 				t.Date, t.AccountSlug, t.FormatAmount(), t.Currency, balance,
-				t.RuleTag, t.Type, t.Description)
+				t.DisplayTag(), t.Type, t.Description)
 			continue
 		}
 		fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n",
-			t.Date, t.AccountSlug, t.FormatAmount(), t.Currency, t.RuleTag, t.Description)
+			t.Date, t.AccountSlug, t.FormatAmount(), t.Currency, t.DisplayTag(), t.Description)
 	}
 	w.Flush()
 	fmt.Printf("\n%d transactions\n", len(txns))
 	return nil
 }

+// printUniq lists each account and description once, which is the shape of the
+// question "what still needs a rule?" — fifty visits to one shop are one
+// pattern to write, not fifty rows to read.
+//
+// Descriptions are printed as rules.Engine matches them: normalised, since that
+// is the string a glob is actually tested against, so a pattern written from
+// this list behaves the way the list reads. It also means two statements that
+// differ only in spacing or case collapse to the one row they deserve.
+func printUniq(out io.Writer, txns []model.Transaction, limit int) error {
+	type row struct{ account, desc string }
+	seen := map[row]bool{}
+	var rows []row
+	for _, t := range txns {
+		r := row{t.AccountSlug, model.NormalizeDescription(t.Description)}
+		if seen[r] {
+			continue
+		}
+		seen[r] = true
+		rows = append(rows, r)
+	}
+	// Grouped by account and alphabetical within it: the same payee under
+	// slightly different wordings then lands on adjacent lines, where one glob
+	// covering both is easy to see.
+	sort.Slice(rows, func(i, j int) bool {
+		if rows[i].account != rows[j].account {
+			return rows[i].account < rows[j].account
+		}
+		return rows[i].desc < rows[j].desc
+	})
+
+	total := len(rows)
+	if limit > 0 && len(rows) > limit {
+		rows = rows[:limit]
+	}
+
+	w := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0)
+	fmt.Fprintln(w, "ACCOUNT\tDESCRIPTION")
+	for _, r := range rows {
+		fmt.Fprintf(w, "%s\t%s\n", r.account, r.desc)
+	}
+	w.Flush()
+
+	if len(rows) < total {
+		fmt.Fprintf(out, "\n%d of %d distinct descriptions\n", len(rows), total)
+		return nil
+	}
+	fmt.Fprintf(out, "\n%d distinct descriptions\n", total)
+	return nil
+}
+
 func cmdReport(root string, args []string) error {
 	fs := flag.NewFlagSet("report", flag.ContinueOnError)
 	month := fs.String("month", "", "only this month (YYYY-MM)")
@@ -287,13 +390,13 @@ func cmdReport(root string, args []string) error {
 		return err
 	}

-	db, _, _, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
+	defer o.db.Close()

-	txns, err := db.Transactions(store.Filter{Month: *month, AccountSlug: *account})
+	txns, err := o.db.Transactions(store.Filter{Month: *month, AccountSlug: *account})
 	if err != nil {
 		return err
 	}
@@ -312,28 +415,44 @@ func cmdReport(root string, args []string) error {
 			model.FormatMinor(c.Net(), c.Digits))
 	}
 	w.Flush()
+
+	// Say what was held out, or the report silently disagrees with the account
+	// balances by exactly the amount moved between accounts. Both directions
+	// are named: they came off the OUT and IN columns respectively, and an
+	// exchange puts only one of them in this currency.
+	for _, x := range report.Excluded(txns) {
+		line := fmt.Sprintf("\ntransfers excluded: %d legs in %s, %s out, %s in",
+			x.Legs, x.Currency,
+			model.FormatMinor(x.Out, x.Digits), model.FormatMinor(x.In, x.Digits))
+		// A fee went out with the pair that carried it, so unless it is named
+		// here it is money that left the report without ever being spent.
+		if x.Fee != 0 {
+			line += fmt.Sprintf(", %s in fees", model.FormatMinor(x.Fee, x.Digits))
+		}
+		fmt.Println(line)
+	}
 	return nil
 }

 func cmdAccounts(root string, _ []string) error {
-	db, _, _, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
+	defer o.db.Close()

-	accounts, err := db.Accounts()
+	accounts, err := o.db.Accounts()
 	if err != nil {
 		return err
 	}
 	w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
 	fmt.Fprintln(w, "SLUG\tNAME\tBALANCE\tCUR\tTXNS")
 	for _, a := range accounts {
-		bal, err := db.Balance(a.ID)
+		bal, err := o.db.Balance(a.ID)
 		if err != nil {
 			return err
 		}
-		n, err := db.Count(a.ID)
+		n, err := o.db.Count(a.ID)
 		if err != nil {
 			return err
 		}
@@ -344,10 +463,10 @@ func cmdAccounts(root string, _ []string) error {
 }

 func cmdTUI(root string, _ []string) error {
-	db, accounts, engine, err := open(root)
+	o, err := open(root)
 	if err != nil {
 		return err
 	}
-	defer db.Close()
-	return tui.Run(root, db, accounts, engine)
+	defer o.db.Close()
+	return tui.Run(root, o.db, o.accounts, o.engine, o.links)
 }
@@ -0,0 +1,81 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"git.petrovv.com/nikola/money/internal/model"
+)
+
+func txn(account, desc string) model.Transaction {
+	return model.Transaction{AccountSlug: account, Description: desc}
+}
+
+func uniqOutput(t *testing.T, limit int, txns ...model.Transaction) string {
+	t.Helper()
+	var b strings.Builder
+	if err := printUniq(&b, txns, limit); err != nil {
+		t.Fatal(err)
+	}
+	return b.String()
+}
+
+// The point of the listing is one row per pattern still to write, so fifty
+// visits to one shop must not be fifty rows.
+func TestUniqCollapsesRepeats(t *testing.T) {
+	out := uniqOutput(t, 0,
+		txn("checking", "LIDL SOFIA 4412"),
+		txn("checking", "LIDL SOFIA 4412"),
+		txn("checking", "ZARA"),
+	)
+	if n := strings.Count(out, "LIDL SOFIA 4412"); n != 1 {
+		t.Errorf("the shop appears %d times, want once:\n%s", n, out)
+	}
+	if !strings.Contains(out, "2 distinct descriptions") {
+		t.Errorf("count is wrong:\n%s", out)
+	}
+}
+
+// Descriptions are printed as a glob sees them, so two statements differing
+// only in spacing or case are the one pattern they really are -- and what is
+// printed is exactly what a rule written from it will match.
+func TestUniqNormalisesLikeAGlob(t *testing.T) {
+	out := uniqOutput(t, 0,
+		txn("checking", "Lidl  Sofia   4412"),
+		txn("checking", "LIDL SOFIA 4412"),
+	)
+	if !strings.Contains(out, "LIDL SOFIA 4412") {
+		t.Errorf("want the normalised description:\n%s", out)
+	}
+	if !strings.Contains(out, "1 distinct descriptions") {
+		t.Errorf("spacing and case should not make a second row:\n%s", out)
+	}
+}
+
+// The same description on two accounts is two rows: a rule can be scoped to an
+// account, and the two may well want different tags.
+func TestUniqKeepsAccountsApart(t *testing.T) {
+	out := uniqOutput(t, 0,
+		txn("checking", "TRANSFER"),
+		txn("savings", "TRANSFER"),
+	)
+	if !strings.Contains(out, "2 distinct descriptions") {
+		t.Errorf("want a row per account:\n%s", out)
+	}
+}
+
+// The limit caps the rows printed, and says so, rather than silently cutting
+// the list down to a number that looks complete.
+func TestUniqLimitReportsWhatItHeldBack(t *testing.T) {
+	out := uniqOutput(t, 1,
+		txn("checking", "ZARA"),
+		txn("checking", "LIDL"),
+	)
+	if !strings.Contains(out, "1 of 2 distinct descriptions") {
+		t.Errorf("want the total alongside the limit:\n%s", out)
+	}
+	// Alphabetical within an account, so near-identical wordings land together.
+	if !strings.Contains(out, "LIDL") || strings.Contains(out, "ZARA") {
+		t.Errorf("want the first row alphabetically:\n%s", out)
+	}
+}