Try the most specific rule first, not the topmost
File order decided precedence, so a narrow rule had to be written above the broad one it carves an exception out of -- an ordering constraint the file cannot show and the user has to remember. *NIKOLA* below *NIK* silently matched nothing, and a catch-all * could only ever be the last line. Engine.New now sorts once and MatchIndex walks that order: most literal characters first, then fewest *, then account-scoped over unscoped. Literals are what a rule commits to and a * is what it gives up, so a bare * is tried last wherever it sits. The sort is stable, so equally specific rules keep file order and the earlier one wins -- which is all position decides now, and why AppendRule can keep appending without displacing a rule written by hand. The two orders must not be confused: Rules(), Usage and MatchIndex still speak in file positions, because that is what the rules screen numbers and what DeleteRules deletes by. A shadowed rule still reports zero usage, but a zero no longer says anything about where the rule sits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+85
-15
@@ -1,26 +1,94 @@
|
||||
// Package rules applies the ordered glob rules from rules.toml to
|
||||
// transactions, deciding their tag. It is the only thing that decides a tag,
|
||||
// so rule_tag is derived state and Retag can rewrite it wholesale at any time.
|
||||
// Package rules applies the glob rules from rules.toml to transactions,
|
||||
// deciding their tag. The most specific rule that fits wins, so a general rule
|
||||
// and the narrower one carving an exception out of it can be written in either
|
||||
// order. It is the only thing that decides a tag, so rule_tag is derived state
|
||||
// and Retag can rewrite it wholesale at any time.
|
||||
package rules
|
||||
|
||||
import (
|
||||
"sort"
|
||||
|
||||
"git.petrovv.com/nikola/money/internal/config"
|
||||
"git.petrovv.com/nikola/money/internal/glob"
|
||||
"git.petrovv.com/nikola/money/internal/model"
|
||||
"git.petrovv.com/nikola/money/internal/store"
|
||||
)
|
||||
|
||||
// Engine evaluates rules in file order; the first match wins.
|
||||
// Engine evaluates rules most specific first; the first match wins.
|
||||
type Engine struct {
|
||||
// rules is file order, which is what Rules and Usage report and what the
|
||||
// rules screen numbers and deletes by. Evaluation does not use it.
|
||||
rules []config.Rule
|
||||
// order indexes into rules, most specific first. Specificity decides
|
||||
// precedence rather than position, so "*NIKOLA*" claims what it names
|
||||
// wherever it sits relative to the "*NIK*" that would otherwise swallow it.
|
||||
order []int
|
||||
}
|
||||
|
||||
// New builds an engine from the parsed rules file.
|
||||
// New builds an engine from the parsed rules file, working out once which rule
|
||||
// is tried before which.
|
||||
func New(r *config.Rules) *Engine {
|
||||
return &Engine{rules: r.Rule}
|
||||
e := &Engine{rules: r.Rule, order: make([]int, len(r.Rule))}
|
||||
for i := range e.order {
|
||||
e.order[i] = i
|
||||
}
|
||||
// Stable, so rules of equal specificity keep file order between them and
|
||||
// the earlier one still wins.
|
||||
sort.SliceStable(e.order, func(a, b int) bool {
|
||||
return moreSpecific(e.rules[e.order[a]], e.rules[e.order[b]])
|
||||
})
|
||||
return e
|
||||
}
|
||||
|
||||
// Rules returns the ordered rules, as loaded from rules.toml.
|
||||
// moreSpecific reports whether a is tried before b.
|
||||
//
|
||||
// The literal characters a rule spells out are the evidence: they are what it
|
||||
// commits to, and what it hands to a '*' is what it gives up. So "*NIKOLA*"
|
||||
// beats "*NIK*", and a bare "*" sits last of all — the catch-all can be written
|
||||
// anywhere in the file and still catch only what nothing else wanted.
|
||||
//
|
||||
// Two rules spelling out the same amount are separated by what else they pin
|
||||
// down: fewer '*' first, since "NIKOLA" also fixes both ends where "*NIKOLA*"
|
||||
// does not, and then an account-scoped rule over one that applies everywhere.
|
||||
// Rules that tie on all three are left to file order by the stable sort.
|
||||
func moreSpecific(a, b config.Rule) bool {
|
||||
if x, y := literals(a), literals(b); x != y {
|
||||
return x > y
|
||||
}
|
||||
if x, y := stars(a), stars(b); x != y {
|
||||
return x < y
|
||||
}
|
||||
return a.Account != "" && b.Account == ""
|
||||
}
|
||||
|
||||
// literals counts the characters a rule pins down exactly, across every pattern
|
||||
// it sets: a rule with both match and type has to satisfy both, so both count.
|
||||
// '?' is not one of them — it fixes a length, not a character.
|
||||
func literals(r config.Rule) int {
|
||||
n := 0
|
||||
for _, c := range r.Match + r.Type {
|
||||
if c != '*' && c != '?' {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// stars counts the unbounded wildcards, the only ones that let a pattern match
|
||||
// a run of any length.
|
||||
func stars(r config.Rule) int {
|
||||
n := 0
|
||||
for _, c := range r.Match + r.Type {
|
||||
if c == '*' {
|
||||
n++
|
||||
}
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// Rules returns the rules in file order, as loaded from rules.toml. That is the
|
||||
// order the rules screen shows and deletes by; it is no longer the order they
|
||||
// are tried in.
|
||||
func (e *Engine) Rules() []config.Rule { return e.rules }
|
||||
|
||||
// Match returns the first rule matching a transaction on the given account, or
|
||||
@@ -32,19 +100,21 @@ func (e *Engine) Match(accountSlug string, t model.Transaction) *config.Rule {
|
||||
return nil
|
||||
}
|
||||
|
||||
// MatchIndex returns the position of the first rule matching a transaction, or
|
||||
// MatchIndex returns the file position of the rule claiming a transaction, or
|
||||
// -1 if none does. Every pattern a rule sets must match: a rule with both
|
||||
// match and type is an "and", not an "or".
|
||||
//
|
||||
// The position matters as well as the rule: because the first match wins, a
|
||||
// rule that is fully shadowed by an earlier one never applies to anything, and
|
||||
// only the index reveals that.
|
||||
// Rules are tried most specific first, so the answer is the narrowest rule that
|
||||
// fits and not merely the topmost. The returned index is still the position in
|
||||
// rules.toml, because that is what the caller can point the user at — and it is
|
||||
// what reveals a rule that is fully shadowed by a more specific one and so
|
||||
// never applies to anything.
|
||||
func (e *Engine) MatchIndex(accountSlug string, t model.Transaction) int {
|
||||
var (
|
||||
description = model.NormalizeDescription(t.Description)
|
||||
kind = model.NormalizeDescription(t.Type)
|
||||
)
|
||||
for i := range e.rules {
|
||||
for _, i := range e.order {
|
||||
r := &e.rules[i]
|
||||
if r.Account != "" && r.Account != accountSlug {
|
||||
continue
|
||||
@@ -60,9 +130,9 @@ func (e *Engine) MatchIndex(accountSlug string, t model.Transaction) int {
|
||||
return -1
|
||||
}
|
||||
|
||||
// Usage counts how many transactions each rule actually claims. A rule with a
|
||||
// count of zero is dead: either nothing matches it, or an earlier rule takes
|
||||
// everything it would have caught.
|
||||
// Usage counts how many transactions each rule actually claims, indexed by file
|
||||
// position. A rule with a count of zero is dead: either nothing matches it, or
|
||||
// a more specific rule takes everything it would have caught.
|
||||
func (e *Engine) Usage(txns []model.Transaction) []int {
|
||||
counts := make([]int, len(e.rules))
|
||||
for _, t := range txns {
|
||||
|
||||
@@ -108,35 +108,72 @@ func TestRetagRewritesEveryTag(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFirstMatchWins(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*LIDL EXPRESS*", Tag: "snacks"},
|
||||
{Match: "*LIDL*", Tag: "groceries"},
|
||||
}})
|
||||
if tag := engine.Apply("checking", "CARD LIDL EXPRESS 12"); tag != "snacks" {
|
||||
t.Errorf("tag = %q, want snacks (earlier rule must win)", tag)
|
||||
// The narrower rule claims what it names wherever it sits in the file: the
|
||||
// catch-all above it must not swallow the exception written below.
|
||||
func TestMoreSpecificRuleWins(t *testing.T) {
|
||||
both := [][]config.Rule{
|
||||
{{Match: "NIK*", Tag: "misc"}, {Match: "NIKOLA*", Tag: "family"}},
|
||||
{{Match: "NIKOLA*", Tag: "family"}, {Match: "NIK*", Tag: "misc"}},
|
||||
}
|
||||
if tag := engine.Apply("checking", "CARD LIDL 12"); tag != "groceries" {
|
||||
t.Errorf("tag = %q, want groceries", tag)
|
||||
for _, rs := range both {
|
||||
engine := New(&config.Rules{Rule: rs})
|
||||
if tag := engine.Apply("checking", "NIKOLA PETROV"); tag != "family" {
|
||||
t.Errorf("%q first: tag = %q, want family", rs[0].Match, tag)
|
||||
}
|
||||
if tag := engine.Apply("checking", "NIKI TODOROV"); tag != "misc" {
|
||||
t.Errorf("%q first: tag = %q, want misc", rs[0].Match, tag)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Specificity is measured on what the pattern spells out, so the wildcards
|
||||
// around a literal do not buy it precedence, and a bare "*" is always last.
|
||||
func TestSpecificityOrder(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*", Tag: "other"},
|
||||
{Match: "*LIDL*", Tag: "groceries"},
|
||||
{Match: "*LIDL EXPRESS*", Tag: "snacks"},
|
||||
}})
|
||||
for _, c := range []struct{ desc, want string }{
|
||||
{"CARD LIDL EXPRESS 12", "snacks"},
|
||||
{"CARD LIDL 12", "groceries"},
|
||||
{"SOMETHING ELSE", "other"},
|
||||
} {
|
||||
if tag := engine.Apply("checking", c.desc); tag != c.want {
|
||||
t.Errorf("%q: tag = %q, want %q", c.desc, tag, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Two rules that are equally specific are still decided by the file: the
|
||||
// earlier one wins, which is what makes appending a rule safe.
|
||||
func TestEqualSpecificityKeepsFileOrder(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*ACME*", Tag: "first"},
|
||||
{Match: "*ACME*", Tag: "second"},
|
||||
}})
|
||||
if tag := engine.Apply("checking", "ACME LTD"); tag != "first" {
|
||||
t.Errorf("tag = %q, want first", tag)
|
||||
}
|
||||
if tag := engine.Apply("checking", "SOMETHING ELSE"); tag != "" {
|
||||
t.Errorf("tag = %q, want empty for an unmatched description", tag)
|
||||
}
|
||||
}
|
||||
|
||||
// A rule can be dead two ways: nothing matches it, or an earlier rule already
|
||||
// claimed everything it would have caught. Usage must report both as zero.
|
||||
func TestUsageCountsFirstMatchOnly(t *testing.T) {
|
||||
// A rule can be dead two ways: nothing matches it, or a rule that beats it
|
||||
// already claimed everything it would have caught. Usage must report both as
|
||||
// zero, indexed by file position however the rules are ordered for matching.
|
||||
func TestUsageCountsWinnersOnly(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*LIDL*", Tag: "groceries"}, // claims both LIDL rows
|
||||
{Match: "*LIDL SOFIA*", Tag: "shadowed"}, // fully shadowed by the above
|
||||
{Match: "*LIDL SOFIA*", Tag: "groceries"}, // the narrower of the two
|
||||
{Match: "*LIDL*", Tag: "shadowed"}, // every LIDL row here is Sofia
|
||||
{Match: "*NOTHING MATCHES ME*", Tag: "no"}, // matches nothing at all
|
||||
{Match: "*PAYROLL*", Tag: "salary"}, // claims one row
|
||||
}})
|
||||
|
||||
txns := []model.Transaction{
|
||||
{AccountSlug: "checking", Description: "LIDL SOFIA 4412"},
|
||||
{AccountSlug: "checking", Description: "LIDL VARNA 9911"},
|
||||
{AccountSlug: "checking", Description: "LIDL SOFIA 9911"},
|
||||
{AccountSlug: "checking", Description: "ACME PAYROLL"},
|
||||
{AccountSlug: "checking", Description: "UNMATCHED SHOP"},
|
||||
}
|
||||
@@ -153,27 +190,31 @@ func TestUsageCountsFirstMatchOnly(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The index is the rule's position in rules.toml, not its position in the order
|
||||
// it was tried in — that is the number the rules screen shows and deletes by.
|
||||
func TestMatchIndex(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*LIDL EXPRESS*", Tag: "snacks"},
|
||||
{Match: "*LIDL*", Tag: "groceries"},
|
||||
{Match: "*LIDL EXPRESS*", Tag: "snacks"},
|
||||
}})
|
||||
|
||||
if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL EXPRESS 1"}); got != 0 {
|
||||
t.Errorf("index = %d, want 0", got)
|
||||
}
|
||||
if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL 1"}); got != 1 {
|
||||
if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL EXPRESS 1"}); got != 1 {
|
||||
t.Errorf("index = %d, want 1", got)
|
||||
}
|
||||
if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL 1"}); got != 0 {
|
||||
t.Errorf("index = %d, want 0", got)
|
||||
}
|
||||
if got := engine.MatchIndex("checking", model.Transaction{Description: "OTHER"}); got != -1 {
|
||||
t.Errorf("index = %d, want -1 for no match", got)
|
||||
}
|
||||
}
|
||||
|
||||
// Naming an account is itself a narrowing, so the scoped rule beats the
|
||||
// identical unscoped one even when the unscoped one is written first.
|
||||
func TestAccountScopedRule(t *testing.T) {
|
||||
engine := New(&config.Rules{Rule: []config.Rule{
|
||||
{Match: "*TRANSFER*", Tag: "transfer", Account: "savings"},
|
||||
{Match: "*TRANSFER*", Tag: "misc"},
|
||||
{Match: "*TRANSFER*", Tag: "transfer", Account: "savings"},
|
||||
}})
|
||||
|
||||
if tag := engine.Apply("savings", "TRANSFER FROM CHECKING"); tag != "transfer" {
|
||||
|
||||
Reference in New Issue
Block a user