From 41f3e950b2e683dd39ec936b6a017cc2543329a5 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Thu, 4 Jun 2026 22:36:20 -0700 Subject: [PATCH 1/6] feat(mcp): lean-by-default fields + currency hoisting for query_transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents rarely set the optional fields param, so query_transactions returned all ~22 fields per row by default — most unused. Make the MCP default a compact projection (core,category); pass fields=all for the full struct. This is MCP-only: the REST API shares service.ParseFields and keeps returning full objects when ?fields= is omitted. Also hoist iso_currency_code to a single top-level field when every returned row shares one currency (the common single-currency household case), with a safe per-row fallback the moment a response mixes currencies. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/mcp/currency_hoist_test.go | 117 ++++++++++++++++++ .../mcp/response_shapes_integration_test.go | 48 ++++++- internal/mcp/server.go | 2 +- internal/mcp/tools.go | 86 ++++++++++++- 4 files changed, 245 insertions(+), 8 deletions(-) create mode 100644 internal/mcp/currency_hoist_test.go diff --git a/internal/mcp/currency_hoist_test.go b/internal/mcp/currency_hoist_test.go new file mode 100644 index 00000000..f28f7a7e --- /dev/null +++ b/internal/mcp/currency_hoist_test.go @@ -0,0 +1,117 @@ +//go:build !lite + +package mcp + +import "testing" + +func strptr(s string) *string { return &s } + +func TestHoistUniformCurrency(t *testing.T) { + tests := []struct { + name string + rows []map[string]any + wantCur string + wantOK bool + wantStrip bool // per-row iso_currency_code removed on success + }{ + { + name: "all USD via *string hoists and strips", + rows: []map[string]any{{"iso_currency_code": strptr("USD")}, {"iso_currency_code": strptr("USD")}}, + wantCur: "USD", + wantOK: true, + wantStrip: true, + }, + { + name: "all USD via plain string hoists", + rows: []map[string]any{{"iso_currency_code": "USD"}, {"iso_currency_code": "USD"}}, + wantCur: "USD", + wantOK: true, + }, + { + name: "mixed currencies stays per-row", + rows: []map[string]any{{"iso_currency_code": strptr("USD")}, {"iso_currency_code": strptr("EUR")}}, + wantOK: false, + }, + { + name: "a nil currency disables hoisting", + rows: []map[string]any{{"iso_currency_code": strptr("USD")}, {"iso_currency_code": (*string)(nil)}}, + wantOK: false, + }, + { + name: "an empty-string currency disables hoisting", + rows: []map[string]any{{"iso_currency_code": "USD"}, {"iso_currency_code": ""}}, + wantOK: false, + }, + { + name: "field absent (not in projection) disables hoisting", + rows: []map[string]any{{"amount": 1.0}, {"amount": 2.0}}, + wantOK: false, + }, + { + name: "empty list disables hoisting", + rows: []map[string]any{}, + wantOK: false, + }, + { + name: "single row hoists", + rows: []map[string]any{{"iso_currency_code": strptr("GBP")}}, + wantCur: "GBP", + wantOK: true, + wantStrip: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cur, ok := hoistUniformCurrency(tt.rows) + if ok != tt.wantOK { + t.Fatalf("ok = %v, want %v", ok, tt.wantOK) + } + if ok && cur != tt.wantCur { + t.Errorf("cur = %q, want %q", cur, tt.wantCur) + } + if tt.wantStrip { + for i, r := range tt.rows { + if _, present := r["iso_currency_code"]; present { + t.Errorf("row %d still carries iso_currency_code after successful hoist", i) + } + } + } + if !tt.wantOK { + // Non-hoist outcomes must leave rows untouched. + for i, r := range tt.rows { + if _, hadCur := r["iso_currency_code"]; !hadCur { + continue + } + if _, present := r["iso_currency_code"]; !present { + t.Errorf("row %d lost iso_currency_code despite no hoist", i) + } + } + } + }) + } +} + +func TestCurrencyString(t *testing.T) { + tests := []struct { + name string + in any + wantStr string + wantOK bool + }{ + {"plain string", "USD", "USD", true}, + {"empty string", "", "", false}, + {"non-nil pointer", strptr("EUR"), "EUR", true}, + {"nil pointer", (*string)(nil), "", false}, + {"empty pointer", strptr(""), "", false}, + {"wrong type", 42, "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, ok := currencyString(tt.in) + if ok != tt.wantOK || s != tt.wantStr { + t.Errorf("currencyString(%v) = (%q, %v), want (%q, %v)", tt.in, s, ok, tt.wantStr, tt.wantOK) + } + }) + } +} diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index 3962d107..1c36516f 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -461,8 +461,9 @@ func TestTransactionSummaryResponseShape(t *testing.T) { } // TestMerchantSummaryResponseShape pins {merchants, totals, filters} + row fields. -// TestQueryTransactionsResponseShape pins category as an object (not a slug -// string) and the wrapper envelope. +// TestQueryTransactionsResponseShape pins the lean-by-default projection +// (core,category), category as an object (not a slug string), the wrapper +// envelope, and top-level currency hoisting when rows share one currency. func TestQueryTransactionsResponseShape(t *testing.T) { f := seedFixtures(t) res, _, err := f.svc.handleQueryTransactions(f.ctx, nil, queryTransactionsInput{ @@ -477,8 +478,14 @@ func TestQueryTransactionsResponseShape(t *testing.T) { t.Fatal("expected at least one transaction") } txn := asObject(t, "query_transactions.transactions[0]", txns[0]) + // Lean default = core,category. id is always included; fields outside the + // default projection (account_id, provider_merchant_name, timestamps, + // metadata) must be absent unless explicitly requested. requireKeys(t, "query_transactions.transactions[0]", txn, - "id", "account_id", "amount", "date", "provider_name", "category", + "id", "amount", "date", "provider_name", "category", + ) + requireAbsent(t, "query_transactions.transactions[0]", txn, + "account_id", "provider_merchant_name", "created_at", "updated_at", "metadata", "short_id", ) switch v := txn["category"].(type) { case map[string]any: @@ -489,6 +496,41 @@ func TestQueryTransactionsResponseShape(t *testing.T) { default: t.Errorf("category must be object or null, got %T (%v)", v, v) } + // Currency hoisting: the seeded rows are all USD, so iso_currency_code is + // emitted once at the top level and dropped from each row. (Written to + // tolerate the per-row fallback too, in case fixtures ever go multi-currency.) + if cur, hoisted := out["iso_currency_code"]; hoisted { + if s, _ := cur.(string); s == "" { + t.Errorf("top-level iso_currency_code must be a non-empty string, got %v", cur) + } + requireAbsent(t, "query_transactions.transactions[0]", txn, "iso_currency_code") + } else { + requireKeys(t, "query_transactions.transactions[0]", txn, "iso_currency_code") + } +} + +// TestQueryTransactionsFieldsAll pins that fields=all restores the full +// per-row struct (the pre-lean-default contract) and suppresses currency +// hoisting — full fidelity is full fidelity. +func TestQueryTransactionsFieldsAll(t *testing.T) { + f := seedFixtures(t) + res, _, err := f.svc.handleQueryTransactions(f.ctx, nil, queryTransactionsInput{ + Limit: 10, + Fields: "all", + }) + out := decodeToolResult[map[string]any](t, "query_transactions(all)", res, err) + requireKeys(t, "query_transactions(all)", out, "transactions", "has_more", "limit") + // fields=all means no projection map, so no top-level hoisting. + requireAbsent(t, "query_transactions(all)", out, "iso_currency_code") + txns := asArray(t, "query_transactions(all).transactions", out["transactions"]) + if len(txns) == 0 { + t.Fatal("expected at least one transaction") + } + txn := asObject(t, "query_transactions(all).transactions[0]", txns[0]) + requireKeys(t, "query_transactions(all).transactions[0]", txn, + "id", "account_id", "amount", "date", "provider_name", + "iso_currency_code", "provider_merchant_name", "category", "created_at", "metadata", + ) } // TestPreviewRuleResponseShape pins `sample_matches` (not `sample`) + sample diff --git a/internal/mcp/server.go b/internal/mcp/server.go index bea99578..f1960141 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -426,7 +426,7 @@ func (s *MCPServer) buildToolRegistry() { // --- Query + aggregate --- makeToolDefLogged(ToolSpec{ Name: "query_transactions", Title: "Query Transactions", Classification: ToolRead, - Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see breadbox://categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Use the fields parameter to request only the fields you need (e.g., fields=core,category) to significantly reduce response size.", + Description: "Query bank transactions with optional filters and cursor-based pagination. Amounts: positive = money out (debit), negative = money in (credit). Dates: YYYY-MM-DD, start_date inclusive, end_date exclusive. Filter by category_slug (see breadbox://categories for the slug list); parent slugs include all children. Results ordered by date desc by default. Pagination: pass next_cursor from response. Responses are lean by default — a compact field set (core,category) is returned unless you pass fields=all or an explicit field/alias list. When every row shares one currency, iso_currency_code is returned once at the top level instead of on each row; otherwise each row carries its own.", }, s.handleQueryTransactions, s), makeToolDefLogged(ToolSpec{ Name: "count_transactions", Title: "Count Transactions", Classification: ToolRead, diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index e514ee2e..67ba7eea 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -35,7 +35,7 @@ type queryTransactionsInput struct { Cursor string `json:"cursor,omitempty" jsonschema:"Pagination cursor from previous result"` SortBy string `json:"sort_by,omitempty" jsonschema:"Sort: date (default), amount, provider_name"` SortOrder string `json:"sort_order,omitempty" jsonschema:"Sort direction: desc (default) or asc"` - Fields string `json:"fields,omitempty" jsonschema:"Comma-separated list of fields to include in response. Aliases: minimal (provider_name,amount,date), core (id,date,amount,provider_name,iso_currency_code), category (category,provider_category_primary,provider_category_detailed), timestamps (created_at,updated_at,datetime,authorized_datetime). Default: all fields. id is always included."` + Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Aliases: minimal (provider_name,amount,date), core (id,date,amount,provider_name,iso_currency_code), category (category,provider_category_primary,provider_category_detailed), timestamps (created_at,updated_at,datetime,authorized_datetime). Default when omitted: core,category (a compact projection). Pass fields=all for every field. id is always included."` } type countTransactionsInput struct { @@ -120,7 +120,21 @@ func (s *MCPServer) handleQueryTransactions(_ context.Context, _ *mcpsdk.CallToo return errorResult(err), nil, nil } - fieldSet, err := service.ParseFields(input.Fields) + // Lean-by-default: an omitted fields param returns a compact projection + // (core identity + category) rather than all ~22 fields. Agents rarely set + // fields and the full row is mostly unused, so the default response is the + // cheap one. Pass fields=all for the complete struct (the legacy default), + // or any explicit field/alias list to widen or narrow. This default is + // MCP-only — the REST API (which shares service.ParseFields) keeps + // returning full objects when ?fields= is omitted. + fieldsRaw := input.Fields + switch fieldsRaw { + case "": + fieldsRaw = defaultTransactionFields + case "all": + fieldsRaw = "" // ParseFields("") → nil → full struct + } + fieldSet, err := service.ParseFields(fieldsRaw) if err != nil { return errorResult(err), nil, nil } @@ -142,17 +156,81 @@ func (s *MCPServer) handleQueryTransactions(_ context.Context, _ *mcpsdk.CallToo for i, t := range result.Transactions { filtered[i] = service.FilterTransactionFields(t, fieldSet) } - return jsonResult(map[string]any{ + resp := map[string]any{ "transactions": filtered, "next_cursor": result.NextCursor, "has_more": result.HasMore, "limit": result.Limit, - }) + } + // Currency hoisting: when every returned row shares one currency (the + // overwhelmingly common single-currency household case), emit it once + // at the top level and drop the per-row copies. A response that mixes + // currencies (or carries a null one) falls back to per-row. Agents read + // the top-level iso_currency_code when present, else the per-row field. + if cur, ok := hoistUniformCurrency(filtered); ok { + resp["iso_currency_code"] = cur + } + return jsonResult(resp) } return jsonResult(result) } +// defaultTransactionFields is the compact projection query_transactions returns +// when the caller omits fields. core = id,date,amount,provider_name, +// iso_currency_code; category = the resolved category + provider category +// hints. Callers widen with fields=all or any explicit alias/field list. +const defaultTransactionFields = "core,category" + +// hoistUniformCurrency strips the per-row iso_currency_code from a projected +// transaction list when every row shares one non-empty currency, returning the +// common code so the caller can emit it once at the top level. It returns +// ("", false) — leaving the rows untouched — when the list is empty, when any +// row omits the field (it wasn't in the projection), or when currencies differ +// or are null. This is the single-currency optimization: one top-level +// iso_currency_code instead of N identical copies, with a safe per-row fallback +// the moment a response actually mixes currencies. +func hoistUniformCurrency(rows []map[string]any) (string, bool) { + if len(rows) == 0 { + return "", false + } + common := "" + for _, r := range rows { + v, present := r["iso_currency_code"] + if !present { + return "", false + } + c, ok := currencyString(v) + if !ok { + return "", false + } + if common == "" { + common = c + } else if c != common { + return "", false + } + } + for _, r := range rows { + delete(r, "iso_currency_code") + } + return common, true +} + +// currencyString coerces a projected iso_currency_code value (string or +// *string, per FilterTransactionFields) to a non-empty currency code. +func currencyString(v any) (string, bool) { + switch s := v.(type) { + case string: + return s, s != "" + case *string: + if s == nil || *s == "" { + return "", false + } + return *s, true + } + return "", false +} + func (s *MCPServer) handleCountTransactions(_ context.Context, _ *mcpsdk.CallToolRequest, input countTransactionsInput) (*mcpsdk.CallToolResult, any, error) { ctx := context.Background() params := service.TransactionCountParams{ From 64f23222f96fdf8a22b8d6794cee959bfcee9a5e Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Thu, 4 Jun 2026 22:42:34 -0700 Subject: [PATCH 2/6] feat(mcp): lean-by-default field projection for list_series and list_transaction_rules Extend the field-projection mechanism (previously transaction-only) to two heavy read tools. fields.go's parser is generalized into parseFieldsWith so each entity supplies its own valid-field/alias maps; ParseFields stays a stable wrapper so the REST API is untouched. - list_series: lean "overview" default (identity + renewal prediction) drops the verbose detection_signals blob; fields=all or get_series for full detail. - list_transaction_rules: lean "summary" default drops the conditions/actions trees; fields=all restores the full definition. Both are MCP-tool-only; the breadbox://rules resource still returns full, and the tool<->resource parity test now asserts that invariant via fields=all. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcp/response_shapes_integration_test.go | 37 ++- internal/mcp/server.go | 4 +- internal/mcp/tools_reads.go | 29 +++ internal/mcp/tools_series.go | 25 ++ internal/service/fields.go | 21 +- internal/service/fields_entities.go | 235 ++++++++++++++++++ internal/service/fields_entities_test.go | 126 ++++++++++ 7 files changed, 468 insertions(+), 9 deletions(-) create mode 100644 internal/service/fields_entities.go create mode 100644 internal/service/fields_entities_test.go diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index 1c36516f..c0eabf47 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -533,6 +533,35 @@ func TestQueryTransactionsFieldsAll(t *testing.T) { ) } +// TestListTransactionRulesLeanDefault pins that the rule roster is lean by +// default (summary projection — no conditions/actions trees) and that +// fields=all restores the full definition. +func TestListTransactionRulesLeanDefault(t *testing.T) { + f := seedFixtures(t) + + // Default (no fields) → summary projection. + res, _, err := f.svc.handleListTransactionRules(f.ctx, nil, listTransactionRulesInput{}) + out := decodeToolResult[map[string]any](t, "list_transaction_rules", res, err) + requireKeys(t, "list_transaction_rules", out, "rules") + rules := asArray(t, "list_transaction_rules.rules", out["rules"]) + if len(rules) == 0 { + t.Fatal("expected at least the seeded rule") + } + rule := asObject(t, "list_transaction_rules.rules[0]", rules[0]) + requireKeys(t, "list_transaction_rules.rules[0]", rule, "id", "name", "enabled", "priority", "trigger", "hit_count") + requireAbsent(t, "list_transaction_rules.rules[0]", rule, "conditions", "actions", "created_at", "short_id") + + // fields=all → full definition restored. + resAll, _, errAll := f.svc.handleListTransactionRules(f.ctx, nil, listTransactionRulesInput{Fields: "all"}) + outAll := decodeToolResult[map[string]any](t, "list_transaction_rules(all)", resAll, errAll) + rulesAll := asArray(t, "list_transaction_rules(all).rules", outAll["rules"]) + if len(rulesAll) == 0 { + t.Fatal("expected at least the seeded rule") + } + ruleAll := asObject(t, "list_transaction_rules(all).rules[0]", rulesAll[0]) + requireKeys(t, "list_transaction_rules(all).rules[0]", ruleAll, "id", "name", "conditions", "actions", "created_at") +} + // TestPreviewRuleResponseShape pins `sample_matches` (not `sample`) + sample // row fields (transaction_id, not id). func TestPreviewRuleResponseShape(t *testing.T) { @@ -1045,8 +1074,14 @@ func TestReferenceMirrorTools_ParityWithResources(t *testing.T) { name: "list_transaction_rules <-> breadbox://rules", envelopeKey: "", // both surfaces return the same {rules, has_more, total} object toolFn: func() (*mcpsdk.CallToolResult, any, error) { + // list_transaction_rules is lean-by-default (summary projection, + // no conditions/actions trees). The resource has no fields knob + // and returns the full payload, so parity is asserted against + // the tool's full mode (fields=all) — that's the invariant that + // must hold: same service call, same full shape. return f.svc.handleListTransactionRules(f.ctx, nil, listTransactionRulesInput{ - Limit: rulesResourceLimit, + Limit: rulesResourceLimit, + Fields: "all", }) }, resourceFn: func() (*mcpsdk.ReadResourceResult, error) { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index f1960141..23543cdf 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -368,7 +368,7 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleGetSyncStatus, s), makeToolDefLogged(ToolSpec{ Name: "list_transaction_rules", Title: "List Rules", Classification: ToolRead, - Description: "List transaction rules with their conditions, actions, and pipeline stage. Mirror of breadbox://rules. Filter by category_slug, enabled, or search by name. Read this before authoring new rules to avoid duplicates.", + Description: "List transaction rules. Filter by category_slug, enabled, or search by name. Read this before authoring new rules to avoid duplicates. Lean by default: returns a summary projection (name, enabled, priority, trigger, category, hit_count) without the conditions/actions trees — pass fields=all to inspect or audit full rule definitions. Mirror of breadbox://rules (which always returns full).", }, s.handleListTransactionRules, s), makeToolDefLogged(ToolSpec{ Name: "list_workflows", Title: "List Workflows", Classification: ToolRead, @@ -376,7 +376,7 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleListWorkflows, s), makeToolDefLogged(ToolSpec{ Name: "list_series", Title: "List Recurring Series", Classification: ToolRead, - Description: "List detected recurring series (subscriptions, bills, loans). Optional status filter (active|candidate|paused|cancelled). Each row carries cadence, expected_amount + iso_currency_code (never sum across currencies), next_expected_date, occurrence_count, confidence (auto|confirmed|rejected), and detection_signals — the raw evidence the detector used (interval_cv, amount_branch, merchant_key_is_fallback). Read status=candidate to find series awaiting a confirm/reject verdict; calibrate from the signals before deciding.", + Description: "List detected recurring series (subscriptions, bills, loans). Optional status filter (active|candidate|paused|cancelled). Lean by default: each row carries identity + renewal prediction (cadence, expected_amount + iso_currency_code — never sum across currencies, next_expected_date, occurrence_count, confidence), but NOT the verbose detection_signals evidence. Read status=candidate to find series awaiting a confirm/reject verdict; pass fields=all (or use get_series for one series) to see detection_signals — interval_cv, amount_branch, merchant_key_is_fallback — before deciding.", }, s.handleListSeries, s), makeToolDefLogged(ToolSpec{ Name: "get_series", Title: "Get Recurring Series", Classification: ToolRead, diff --git a/internal/mcp/tools_reads.go b/internal/mcp/tools_reads.go index bbbf2ced..9d0246da 100644 --- a/internal/mcp/tools_reads.go +++ b/internal/mcp/tools_reads.go @@ -47,6 +47,7 @@ type listTransactionRulesInput struct { SearchMode string `json:"search_mode,omitempty" jsonschema:"Search mode: contains (default), words, fuzzy"` Limit int `json:"limit,omitempty" jsonschema:"Max results (default 50, max 500)"` Cursor string `json:"cursor,omitempty" jsonschema:"Pagination cursor from previous result"` + Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Alias: summary (name,enabled,priority,trigger,category,hit_count,last_hit_at; the default — omits the conditions and actions trees). Default when omitted: summary. Pass fields=all for every field including the full conditions/actions. id is always included."` } // --- Handlers --- @@ -120,9 +121,37 @@ func (s *MCPServer) handleListTransactionRules(_ context.Context, _ *mcpsdk.Call return errorResult(err), nil, nil } + // Lean-by-default: the rule roster omits the conditions/actions trees (the + // heavy, deeply-nested part) unless the caller asks. Pass fields=all to + // inspect or audit full rule definitions. + fieldsRaw := input.Fields + switch fieldsRaw { + case "": + fieldsRaw = service.DefaultRuleFields + case "all": + fieldsRaw = "" // ParseRuleFields("") → nil → full struct + } + fieldSet, err := service.ParseRuleFields(fieldsRaw) + if err != nil { + return errorResult(err), nil, nil + } + result, err := s.svc.ListTransactionRules(ctx, params) if err != nil { return errorResult(err), nil, nil } + + if fieldSet != nil { + projected := make([]map[string]any, len(result.Rules)) + for i, r := range result.Rules { + projected[i] = service.FilterRuleFields(r, fieldSet) + } + return jsonResult(map[string]any{ + "rules": projected, + "next_cursor": result.NextCursor, + "has_more": result.HasMore, + "total": result.Total, + }) + } return jsonResult(result) } diff --git a/internal/mcp/tools_series.go b/internal/mcp/tools_series.go index ac1857ca..014fcb16 100644 --- a/internal/mcp/tools_series.go +++ b/internal/mcp/tools_series.go @@ -14,6 +14,7 @@ import ( type listSeriesInput struct { Status string `json:"status,omitempty" jsonschema:"Optional status filter: active, candidate, paused, or cancelled."` + Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Aliases: minimal (name,status,type,cadence), overview (identity + renewal prediction; the default — omits the verbose detection_signals blob). Default when omitted: overview. Pass fields=all for every field including detection_signals. Use get_series for a single series' full detail. id is always included."` } type getSeriesInput struct { @@ -46,10 +47,34 @@ func (s *MCPServer) handleListSeries(_ context.Context, _ *mcpsdk.CallToolReques if input.Status != "" { status = &input.Status } + // Lean-by-default: list_series returns the overview projection (identity + + // renewal prediction) unless the caller asks for more. The verbose + // detection_signals blob is excluded by default — fetch a single series' + // full detail via get_series, or pass fields=all here. + fieldsRaw := input.Fields + switch fieldsRaw { + case "": + fieldsRaw = service.DefaultSeriesFields + case "all": + fieldsRaw = "" // ParseSeriesFields("") → nil → full struct + } + fieldSet, err := service.ParseSeriesFields(fieldsRaw) + if err != nil { + return errorResult(err), nil, nil + } + series, err := s.svc.ListSeries(ctx, status) if err != nil { return errorResult(err), nil, nil } + + if fieldSet != nil { + projected := make([]map[string]any, len(series)) + for i, sr := range series { + projected[i] = service.FilterSeriesFields(sr, fieldSet) + } + return jsonResult(map[string]any{"series": projected}) + } return jsonResult(map[string]any{"series": series}) } diff --git a/internal/service/fields.go b/internal/service/fields.go index 59afdbbe..4a0b89ea 100644 --- a/internal/service/fields.go +++ b/internal/service/fields.go @@ -44,9 +44,18 @@ var fieldAliases = map[string][]string{ "timestamps": {"created_at", "updated_at", "datetime", "authorized_datetime"}, } -// ParseFields parses and validates the fields query parameter. +// ParseFields parses and validates the transaction fields query parameter. // Returns nil if no field selection (return all fields). func ParseFields(raw string) (map[string]bool, error) { + return parseFieldsWith(raw, validFields, fieldAliases) +} + +// parseFieldsWith is the generic field-selection parser shared by every +// entity's Parse*Fields wrapper. valid is the set of selectable JSON field +// names; aliases expand shorthand names to groups. It returns nil for an empty +// selection (caller should return the full struct). id and short_id are always +// included so a row is always identifiable. +func parseFieldsWith(raw string, valid map[string]bool, aliases map[string][]string) (map[string]bool, error) { if raw == "" { return nil, nil } @@ -57,24 +66,24 @@ func ParseFields(raw string) (map[string]bool, error) { if f == "" { continue } - if expanded, ok := fieldAliases[f]; ok { + if expanded, ok := aliases[f]; ok { for _, ef := range expanded { fields[ef] = true } continue } - if !validFields[f] { + if !valid[f] { unknown = append(unknown, f) continue } fields[f] = true } if len(unknown) > 0 { - validList := make([]string, 0, len(validFields)+len(fieldAliases)) - for k := range validFields { + validList := make([]string, 0, len(valid)+len(aliases)) + for k := range valid { validList = append(validList, k) } - for k := range fieldAliases { + for k := range aliases { validList = append(validList, k) } sort.Strings(validList) diff --git a/internal/service/fields_entities.go b/internal/service/fields_entities.go new file mode 100644 index 00000000..69cad8d7 --- /dev/null +++ b/internal/service/fields_entities.go @@ -0,0 +1,235 @@ +//go:build !lite + +package service + +// Field projection for recurring series and transaction rules, mirroring the +// transaction projection in fields.go. The heavy, rarely-needed payloads — +// a series' detection_signals JSON blob, a rule's conditions/actions trees — +// are excluded from each entity's lean default alias and only returned when +// the caller asks (fields=all, or names them explicitly). + +// --- Recurring series --- + +var seriesValidFields = map[string]bool{ + "id": true, + "short_id": true, + "user_id": true, + "name": true, + "merchant_key": true, + "cadence": true, + "expected_day": true, + "expected_amount": true, + "amount_tolerance": true, + "iso_currency_code": true, + "category_id": true, + "status": true, + "type": true, + "detection_source": true, + "confidence": true, + "confirmed_by_type": true, + "last_amount": true, + "last_seen_date": true, + "next_expected_date": true, + "renewal_health": true, + "days_until_renewal": true, + "occurrence_count": true, + "detection_signals": true, + "tags": true, + "created_at": true, + "updated_at": true, +} + +var seriesFieldAliases = map[string][]string{ + // minimal: just enough to recognize a series in a list. + "minimal": {"name", "status", "type", "cadence"}, + // overview: identity + lifecycle + renewal prediction — the useful default + // for list_series. Deliberately omits detection_signals (verbose, only + // needed for the get_series detail view), merchant_key/category_id internals, + // detection_source, confirmed_by_type, tolerances, and timestamps. + "overview": { + "name", "status", "type", "cadence", "confidence", + "expected_amount", "iso_currency_code", "expected_day", + "last_amount", "last_seen_date", "next_expected_date", + "renewal_health", "days_until_renewal", "occurrence_count", "tags", + }, +} + +// DefaultSeriesFields is the lean projection list_series returns when the caller +// omits fields. +const DefaultSeriesFields = "overview" + +// ParseSeriesFields parses the fields selector for recurring series. +func ParseSeriesFields(raw string) (map[string]bool, error) { + return parseFieldsWith(raw, seriesValidFields, seriesFieldAliases) +} + +// FilterSeriesFields projects a SeriesResponse down to the requested fields. +// Returns nil when fields is nil (caller uses the full struct). +func FilterSeriesFields(s SeriesResponse, fields map[string]bool) map[string]any { + if fields == nil { + return nil + } + m := make(map[string]any, len(fields)) + for key, want := range fields { + if !want { + continue + } + switch key { + case "id": + m["id"] = s.ID + case "short_id": + m["short_id"] = s.ShortID + case "user_id": + m["user_id"] = s.UserID + case "name": + m["name"] = s.Name + case "merchant_key": + m["merchant_key"] = s.MerchantKey + case "cadence": + m["cadence"] = s.Cadence + case "expected_day": + m["expected_day"] = s.ExpectedDay + case "expected_amount": + m["expected_amount"] = s.ExpectedAmount + case "amount_tolerance": + m["amount_tolerance"] = s.AmountTolerance + case "iso_currency_code": + m["iso_currency_code"] = s.IsoCurrencyCode + case "category_id": + m["category_id"] = s.CategoryID + case "status": + m["status"] = s.Status + case "type": + m["type"] = s.Type + case "detection_source": + m["detection_source"] = s.DetectionSource + case "confidence": + m["confidence"] = s.Confidence + case "confirmed_by_type": + m["confirmed_by_type"] = s.ConfirmedByType + case "last_amount": + m["last_amount"] = s.LastAmount + case "last_seen_date": + m["last_seen_date"] = s.LastSeenDate + case "next_expected_date": + m["next_expected_date"] = s.NextExpectedDate + case "renewal_health": + m["renewal_health"] = s.RenewalHealth + case "days_until_renewal": + m["days_until_renewal"] = s.DaysUntilRenewal + case "occurrence_count": + m["occurrence_count"] = s.OccurrenceCount + case "detection_signals": + m["detection_signals"] = s.DetectionSignals + case "tags": + m["tags"] = s.Tags + case "created_at": + m["created_at"] = s.CreatedAt + case "updated_at": + m["updated_at"] = s.UpdatedAt + } + } + return m +} + +// --- Transaction rules --- + +var ruleValidFields = map[string]bool{ + "id": true, + "short_id": true, + "name": true, + "conditions": true, + "actions": true, + "trigger": true, + "category_slug": true, + "category_display_name": true, + "category_icon": true, + "category_color": true, + "priority": true, + "enabled": true, + "expires_at": true, + "created_by_type": true, + "created_by_id": true, + "created_by_name": true, + "hit_count": true, + "last_hit_at": true, + "created_at": true, + "updated_at": true, +} + +var ruleFieldAliases = map[string][]string{ + // summary: roster view — what a rule is, whether it's on, how it's firing — + // without the conditions and actions trees (the heavy, deeply-nested part + // only needed when inspecting or editing a specific rule). + "summary": { + "name", "enabled", "priority", "trigger", + "category_slug", "category_display_name", + "created_by_type", "hit_count", "last_hit_at", + }, +} + +// DefaultRuleFields is the lean projection list_transaction_rules returns when +// the caller omits fields. +const DefaultRuleFields = "summary" + +// ParseRuleFields parses the fields selector for transaction rules. +func ParseRuleFields(raw string) (map[string]bool, error) { + return parseFieldsWith(raw, ruleValidFields, ruleFieldAliases) +} + +// FilterRuleFields projects a TransactionRuleResponse down to the requested +// fields. Returns nil when fields is nil (caller uses the full struct). +func FilterRuleFields(r TransactionRuleResponse, fields map[string]bool) map[string]any { + if fields == nil { + return nil + } + m := make(map[string]any, len(fields)) + for key, want := range fields { + if !want { + continue + } + switch key { + case "id": + m["id"] = r.ID + case "short_id": + m["short_id"] = r.ShortID + case "name": + m["name"] = r.Name + case "conditions": + m["conditions"] = r.Conditions + case "actions": + m["actions"] = r.Actions + case "trigger": + m["trigger"] = r.Trigger + case "category_slug": + m["category_slug"] = r.CategorySlug + case "category_display_name": + m["category_display_name"] = r.CategoryName + case "category_icon": + m["category_icon"] = r.CategoryIcon + case "category_color": + m["category_color"] = r.CategoryColor + case "priority": + m["priority"] = r.Priority + case "enabled": + m["enabled"] = r.Enabled + case "expires_at": + m["expires_at"] = r.ExpiresAt + case "created_by_type": + m["created_by_type"] = r.CreatedByType + case "created_by_id": + m["created_by_id"] = r.CreatedByID + case "created_by_name": + m["created_by_name"] = r.CreatedByName + case "hit_count": + m["hit_count"] = r.HitCount + case "last_hit_at": + m["last_hit_at"] = r.LastHitAt + case "created_at": + m["created_at"] = r.CreatedAt + case "updated_at": + m["updated_at"] = r.UpdatedAt + } + } + return m +} diff --git a/internal/service/fields_entities_test.go b/internal/service/fields_entities_test.go new file mode 100644 index 00000000..0e05089d --- /dev/null +++ b/internal/service/fields_entities_test.go @@ -0,0 +1,126 @@ +//go:build !lite + +package service + +import ( + "encoding/json" + "testing" +) + +func TestParseSeriesFields(t *testing.T) { + // Empty → nil (full struct). + f, err := ParseSeriesFields("") + if err != nil { + t.Fatalf("empty: %v", err) + } + if f != nil { + t.Errorf("empty selection should be nil, got %v", f) + } + + // overview alias excludes detection_signals; id/short_id always present. + f, err = ParseSeriesFields("overview") + if err != nil { + t.Fatalf("overview: %v", err) + } + if f["detection_signals"] { + t.Error("overview must not include detection_signals") + } + for _, want := range []string{"id", "short_id", "name", "status", "next_expected_date"} { + if !f[want] { + t.Errorf("overview missing %q", want) + } + } + + // Unknown field rejected. + if _, err := ParseSeriesFields("bogus_series_field"); err == nil { + t.Error("expected error for unknown field") + } +} + +func TestFilterSeriesFields_DropsDetectionSignals(t *testing.T) { + s := SeriesResponse{ + ID: "uuid", + ShortID: "abc123", + Name: "Netflix", + Status: "active", + DetectionSignals: json.RawMessage(`{"interval_cv":0.02}`), + } + f, err := ParseSeriesFields(DefaultSeriesFields) + if err != nil { + t.Fatal(err) + } + m := FilterSeriesFields(s, f) + if _, present := m["detection_signals"]; present { + t.Error("default projection must omit detection_signals") + } + if m["name"] != "Netflix" { + t.Errorf("name = %v, want Netflix", m["name"]) + } + if m["id"] != "uuid" || m["short_id"] != "abc123" { + t.Errorf("id/short_id must always be present, got id=%v short_id=%v", m["id"], m["short_id"]) + } + + // fields=all path → nil set → full struct (caller uses struct directly). + all, err := ParseSeriesFields("") + if err != nil { + t.Fatal(err) + } + if FilterSeriesFields(s, all) != nil { + t.Error("nil field set must return nil (full struct)") + } +} + +func TestParseRuleFields(t *testing.T) { + f, err := ParseRuleFields("summary") + if err != nil { + t.Fatalf("summary: %v", err) + } + if f["conditions"] || f["actions"] { + t.Error("summary must not include conditions/actions trees") + } + for _, want := range []string{"id", "short_id", "name", "enabled", "priority", "hit_count"} { + if !f[want] { + t.Errorf("summary missing %q", want) + } + } +} + +func TestFilterRuleFields_DropsTrees(t *testing.T) { + r := TransactionRuleResponse{ + ID: "uuid", + ShortID: "rul456", + Name: "Groceries", + Enabled: true, + Priority: 10, + Conditions: Condition{Field: "provider_name", Op: "contains", Value: "Whole Foods"}, + Actions: []RuleAction{{Type: "set_category"}}, + HitCount: 42, + } + f, err := ParseRuleFields(DefaultRuleFields) + if err != nil { + t.Fatal(err) + } + m := FilterRuleFields(r, f) + if _, present := m["conditions"]; present { + t.Error("default projection must omit conditions") + } + if _, present := m["actions"]; present { + t.Error("default projection must omit actions") + } + if m["name"] != "Groceries" || m["hit_count"] != 42 { + t.Errorf("summary fields wrong: name=%v hit_count=%v", m["name"], m["hit_count"]) + } + + // Explicit conditions request includes the tree. + withCond, err := ParseRuleFields("summary,conditions,actions") + if err != nil { + t.Fatal(err) + } + m2 := FilterRuleFields(r, withCond) + if _, present := m2["conditions"]; !present { + t.Error("explicit conditions request must include the tree") + } + if _, present := m2["actions"]; !present { + t.Error("explicit actions request must include the tree") + } +} From 188037ac77182fd7bd04587fed0319d48b36d002 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Thu, 4 Jun 2026 22:45:14 -0700 Subject: [PATCH 3/6] feat(mcp): per-response byte cap with operator override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a safety ceiling on any single MCP tool response, enforced at the jsonResult chokepoint on the post-compaction wire bytes. Over the cap, the tool returns an actionable RESPONSE_TOO_LARGE error (narrow filters / lower limit / paginate / use fields) instead of dumping a context-blowing payload — the pattern GitHub's MCP server uses. Default 100 KB (~25K tokens). Override via BREADBOX_MCP_MAX_RESPONSE_BYTES (env, consistent with config precedence); set to 0 to disable. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/mcp/response_cap_test.go | 95 +++++++++++++++++++++++++++++++ internal/mcp/tools.go | 35 ++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 internal/mcp/response_cap_test.go diff --git a/internal/mcp/response_cap_test.go b/internal/mcp/response_cap_test.go new file mode 100644 index 00000000..c249abd0 --- /dev/null +++ b/internal/mcp/response_cap_test.go @@ -0,0 +1,95 @@ +//go:build !lite + +package mcp + +import ( + "encoding/json" + "strings" + "testing" + + mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// bigPayload builds a value whose JSON encoding exceeds n bytes. +func bigPayload(n int) map[string]any { + return map[string]any{"blob": strings.Repeat("x", n)} +} + +func TestJSONResult_ResponseCap(t *testing.T) { + orig := maxResponseBytes + t.Cleanup(func() { maxResponseBytes = orig }) + + // Cap at 1 KB; a ~5 KB payload must be rejected with RESPONSE_TOO_LARGE. + maxResponseBytes = 1000 + res, _, err := jsonResult(bigPayload(5000)) + if err != nil { + t.Fatalf("jsonResult returned a transport error: %v", err) + } + if !res.IsError { + t.Fatal("expected IsError for oversized response") + } + var payload map[string]string + if uerr := json.Unmarshal([]byte(textOf(t, res)), &payload); uerr != nil { + t.Fatalf("decode error payload: %v", uerr) + } + if payload["code"] != "RESPONSE_TOO_LARGE" { + t.Errorf("code = %q, want RESPONSE_TOO_LARGE", payload["code"]) + } + + // A small payload under the cap passes. + res, _, _ = jsonResult(map[string]any{"ok": true}) + if res.IsError { + t.Errorf("small payload should not error: %s", textOf(t, res)) + } +} + +func TestJSONResult_CapDisabledWhenZero(t *testing.T) { + orig := maxResponseBytes + t.Cleanup(func() { maxResponseBytes = orig }) + + maxResponseBytes = 0 // disabled + res, _, _ := jsonResult(bigPayload(50_000)) + if res.IsError { + t.Errorf("cap=0 must disable the limit, got error: %s", textOf(t, res)) + } +} + +func TestResolveMaxResponseBytes(t *testing.T) { + t.Setenv("BREADBOX_MCP_MAX_RESPONSE_BYTES", "") + if got := resolveMaxResponseBytes(); got != defaultMaxResponseBytes { + t.Errorf("empty env → %d, want default %d", got, defaultMaxResponseBytes) + } + + t.Setenv("BREADBOX_MCP_MAX_RESPONSE_BYTES", "5000") + if got := resolveMaxResponseBytes(); got != 5000 { + t.Errorf("env=5000 → %d, want 5000", got) + } + + t.Setenv("BREADBOX_MCP_MAX_RESPONSE_BYTES", "0") + if got := resolveMaxResponseBytes(); got != 0 { + t.Errorf("env=0 → %d, want 0 (disabled)", got) + } + + t.Setenv("BREADBOX_MCP_MAX_RESPONSE_BYTES", "garbage") + if got := resolveMaxResponseBytes(); got != defaultMaxResponseBytes { + t.Errorf("invalid env → %d, want default %d", got, defaultMaxResponseBytes) + } + + t.Setenv("BREADBOX_MCP_MAX_RESPONSE_BYTES", "-10") + if got := resolveMaxResponseBytes(); got != defaultMaxResponseBytes { + t.Errorf("negative env → %d, want default %d (rejected)", got, defaultMaxResponseBytes) + } +} + +// textOf extracts the first text content block from a tool result. +func textOf(t *testing.T, res *mcpsdk.CallToolResult) string { + t.Helper() + if res == nil || len(res.Content) == 0 { + t.Fatal("tool result has no content") + } + tc, ok := res.Content[0].(*mcpsdk.TextContent) + if !ok { + t.Fatalf("expected TextContent, got %T", res.Content[0]) + } + return tc.Text +} diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 67ba7eea..ceebfb40 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "os" "strconv" "breadbox/internal/service" @@ -688,6 +689,29 @@ func parseConditions(m map[string]any) (service.Condition, error) { return c, nil } +// defaultMaxResponseBytes caps a single MCP tool response as a safety net +// against pathological payloads (e.g. query_transactions with fields=all and +// limit=500). Bytes are a proxy for tokens (~4 bytes/token), so ~100 KB ≈ +// 25K tokens — the same order as the cap GitHub's MCP server enforces. When a +// response exceeds the cap the tool returns a RESPONSE_TOO_LARGE error telling +// the caller to narrow the query, rather than dumping or silently truncating. +const defaultMaxResponseBytes = 100_000 + +// maxResponseBytes is the active response cap, resolved once at process start. +// Override with BREADBOX_MCP_MAX_RESPONSE_BYTES (operator env knob, consistent +// with Breadbox's env → app_config → default precedence); set it to 0 to +// disable the cap entirely. +var maxResponseBytes = resolveMaxResponseBytes() + +func resolveMaxResponseBytes() int { + if v := os.Getenv("BREADBOX_MCP_MAX_RESPONSE_BYTES"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n >= 0 { + return n + } + } + return defaultMaxResponseBytes +} + func jsonResult(v any) (*mcpsdk.CallToolResult, any, error) { data, err := json.Marshal(v) if err != nil { @@ -699,6 +723,17 @@ func jsonResult(v any) (*mcpsdk.CallToolResult, any, error) { // Operates directly on JSON bytes to avoid unmarshal→remarshal overhead. data = compactIDsBytes(data) + // Safety cap: refuse to emit a response larger than the configured byte + // ceiling. Better to return an actionable error than to blow the caller's + // context with a payload they didn't expect. Checked on the post-compaction + // wire bytes (what actually ships). + if maxResponseBytes > 0 && len(data) > maxResponseBytes { + return errorResultWithCode("RESPONSE_TOO_LARGE", fmt.Sprintf( + "response is %d bytes, over the %d-byte cap. Narrow the result: add filters, lower limit, paginate via next_cursor, or use the fields parameter to request fewer fields. Operators can raise or disable the cap via BREADBOX_MCP_MAX_RESPONSE_BYTES.", + len(data), maxResponseBytes, + )), nil, nil + } + // StructuredContent is the post-2025-06-18 spec slot for typed tool // output. We populate it alongside the TextContent block so: // - clients on the new spec get the structured payload they can From e16cf33403276b5f6c1d354a33b6631adf18b1a0 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Thu, 4 Jun 2026 23:09:38 -0700 Subject: [PATCH 4/6] docs(mcp): document lean-by-default projections, currency hoisting, response cap Keep the canonical MCP tools reference in sync with the token-optimization changes: query_transactions / list_series / list_transaction_rules lean defaults + fields=all, top-level currency hoisting, and the per-response byte cap with its BREADBOX_MCP_MAX_RESPONSE_BYTES override. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp-tools-reference.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 3d6dd038..e8ff70b5 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -10,6 +10,12 @@ Tools are classified as **Read** or **Write**: - **Write tools** require the MCP mode to be set to `read_write` (configurable in the admin dashboard under MCP Settings) - Individual tools can be disabled from the admin dashboard +## Response size + +Token-heavy read tools are **lean by default** — they return a compact field projection and let you opt into more via the `fields` parameter (`fields=all` for the full payload). See `query_transactions`, `list_series`, and `list_transaction_rules`. + +Every tool response is also subject to a **byte cap** (default ~100 KB ≈ 25K tokens). A response over the cap returns a `RESPONSE_TOO_LARGE` error asking you to narrow the query (add filters, lower `limit`, paginate via `next_cursor`, or use `fields`) rather than emitting an oversized payload. Operators can raise or disable the cap via the `BREADBOX_MCP_MAX_RESPONSE_BYTES` environment variable (`0` disables it). + ## Sessions Before using any tools, agents should call `create_session` to establish a session. This provides the agent with dataset context and server instructions. @@ -22,6 +28,8 @@ Before using any tools, agents should call `create_session` to establish a sessi Query transactions with composable filters and cursor pagination. +**Lean by default.** When `fields` is omitted the response is a compact projection (`core,category`) rather than all ~22 fields — pass `fields=all` for the full row, or any explicit alias/field list. When every returned row shares one currency, `iso_currency_code` is emitted once at the top level of the envelope and dropped from each row (per-row fallback when a page mixes currencies). This default is MCP-only; the REST API (`GET /api/v1/transactions`) still returns full objects when `?fields=` is omitted. + | Parameter | Type | Description | |-----------|------|-------------| | `account_id` | string | Filter by account ID | @@ -39,7 +47,7 @@ Query transactions with composable filters and cursor pagination. | `any_tag` | array | OR filter — must have at least one slug. | | `sort_by` | string | `date` (default), `amount`, `provider_name` | | `sort_order` | string | `desc` (default), `asc` | -| `fields` | string | Field selection. Aliases: `minimal`, `core`, `category`, `timestamps` | +| `fields` | string | Field selection. Aliases: `minimal`, `core`, `category`, `timestamps`. Omitted → `core,category` (lean default); `all` → every field. `id` always included. | | `cursor` | string | Pagination cursor | | `limit` | int | Results per page (default 50, max 500) | @@ -251,7 +259,7 @@ Admin-only tag CRUD. Agents typically don't need these — `add_transaction_tag` ### list_series (Read) -List detected recurring series. Optional `status` filter (`active` | `candidate` | `paused` | `cancelled`). Each row carries `type` (`subscription` | `bill` | `loan` | `other` — inferred from category, set via `set_series_type`), `cadence`, `expected_amount` + `iso_currency_code` (never sum across currencies), `next_expected_date`, `occurrence_count`, `confidence` (`auto` | `confirmed` | `rejected`), and `detection_signals` — the raw evidence the detector used. Active series also carry a derived `renewal_health` (`active` | `due_soon` | `overdue` | `stale` | `unknown`) and signed `days_until_renewal` (negative = overdue) so you can answer "what renews soon" and "what looks cancelled" without re-deriving cadence math — `stale` means a full cadence cycle elapsed past the expected charge. Read `status=candidate` to find series awaiting a verdict. +List detected recurring series. Optional `status` filter (`active` | `candidate` | `paused` | `cancelled`). **Lean by default** (`fields` omitted → `overview` projection): each row carries `type` (`subscription` | `bill` | `loan` | `other` — inferred from category, set via `set_series_type`), `cadence`, `expected_amount` + `iso_currency_code` (never sum across currencies), `next_expected_date`, `occurrence_count`, and `confidence` (`auto` | `confirmed` | `rejected`). Active series also carry a derived `renewal_health` (`active` | `due_soon` | `overdue` | `stale` | `unknown`) and signed `days_until_renewal` (negative = overdue) so you can answer "what renews soon" and "what looks cancelled" without re-deriving cadence math — `stale` means a full cadence cycle elapsed past the expected charge. The verbose `detection_signals` evidence is **omitted** from the lean list — pass `fields=all`, or use `get_series` for one series' full detail. Read `status=candidate` to find series awaiting a verdict. ### get_series (Read) @@ -324,13 +332,16 @@ Create a rule that fires during sync. Actions compose (`set_category` + `add_tag ### list_transaction_rules (Read) -List rules with optional filters. Returns actions, priority, hit_count, last_hit_at. +List rules with optional filters and cursor pagination. **Lean by default** (`fields` omitted → `summary` projection): each row carries `name`, `enabled`, `priority`, `trigger`, `category_slug` / `category_display_name`, `hit_count`, `last_hit_at`, `created_by_type` — the roster view, **without** the `conditions` / `actions` trees. Pass `fields=all` to inspect or audit full rule definitions. Mirror of `breadbox://rules` (which always returns full). | Parameter | Type | Description | |-----------|------|-------------| | `search` | string | Substring / words / fuzzy search on rule name | | `category_slug` | string | Filter by target category | | `enabled` | bool | Filter by enabled status | +| `fields` | string | Field selection. Alias: `summary` (default). `all` → full definition incl. `conditions`/`actions`. | +| `cursor` | string | Pagination cursor | +| `limit` | int | Results per page (default 50, max 500) | ### update_transaction_rule (Write) From 0806384ba3167dfb260b4110419b960af532c266 Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Sat, 13 Jun 2026 16:27:11 -0700 Subject: [PATCH 5/6] feat(mcp,agent): query_transaction_rules + working sidecar shell/grep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds query_transaction_rules — the filterable/sortable analogue of query_transactions for the rule set — and fixes the agent sidecar so its Bash/Grep analysis tools actually work inside the container. query_transaction_rules: filter by category_slug / enabled / trigger / creator_type / search / min_hit_count / only_unused; sort by priority (default) | hit_count | last_hit_at | created_at | name. Lean-by-default summary projection + response cap, shared with list_transaction_rules. Cursor pagination is emitted only for the default priority sort; an explicit sort_by returns a single top-N page. Lets a reviewer audit coverage and prune dead rules without dumping the whole roster (which overflowed the tool-result cap in prod). Sidecar shell: the Alpine runtime image shipped no bash and no ripgrep and left SHELL unset, so an agent that tried to grep a large saved tool-result hit "No suitable shell found" / vendored-rg ENOENT and was left unable to do file-level analysis. Install bash + ripgrep, set SHELL=/bin/bash and USE_BUILTIN_RIPGREP=0 (the SDK's bundled rg isn't embedded in the Bun --compile output), with defensive defaults in the sidecar itself for non-Docker (per-user worktree) installs. Also fixes the routine-review prompt's reference to the phantom merchant_summary tool (no merchant grouping exists; the small queue is already in context) and points rule-coverage analysis at the new tool. Co-Authored-By: Claude Opus 4.8 --- Dockerfile | 17 ++- agent/sidecar/index.ts | 17 +++ docs/mcp-tools-reference.md | 20 ++++ internal/admin/settings_agents.go | 1 + .../mcp/response_shapes_integration_test.go | 25 ++++ internal/mcp/server.go | 6 +- internal/mcp/server_test.go | 1 + internal/mcp/tools_reads.go | 81 +++++++++++-- internal/service/rules.go | 105 ++++++++++------ internal/service/rules_integration_test.go | 86 +++++++++++++ internal/service/types.go | 113 ++++++++++-------- prompts/agents/strategy-routine-review.md | 3 +- 12 files changed, 377 insertions(+), 98 deletions(-) diff --git a/Dockerfile b/Dockerfile index 622b387a..f1e5cba3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -70,7 +70,22 @@ FROM alpine:3.21 # Without nodejs the spawn fires ENOENT on an unhandled error handler # inside the SDK; the async iterator ends silently with zero messages # and the run is misclassified as a 0-cost, 0-turn "success". -RUN apk --no-cache add ca-certificates tzdata postgresql16-client libstdc++ libgcc nodejs +# bash + ripgrep: the agent's own analysis tools (Bash, Grep) need a POSIX +# shell and ripgrep. Alpine ships neither (only busybox sh, no rg), so an +# agent that tried to grep a large saved tool-result hit +# "No suitable shell found" / "spawn .../vendor/ripgrep/.../rg ENOENT" and +# was left unable to do file-level analysis. We install both and point the +# SDK at the system rg via USE_BUILTIN_RIPGREP=0 (its bundled copy is not +# embedded in the Bun --compile output). See agent/sidecar/index.ts. +RUN apk --no-cache add ca-certificates tzdata postgresql16-client libstdc++ libgcc nodejs bash ripgrep + +# SHELL: the Claude Agent SDK's Bash tool requires SHELL to point at a POSIX +# shell; the container env doesn't set it otherwise. USE_BUILTIN_RIPGREP=0: +# use the system ripgrep we installed instead of the SDK's vendored binary, +# which isn't present in the sidecar bundle. Both are also defaulted +# defensively in the sidecar itself for non-Docker installs. +ENV SHELL=/bin/bash \ + USE_BUILTIN_RIPGREP=0 WORKDIR /app COPY --from=builder /breadbox /app/breadbox diff --git a/agent/sidecar/index.ts b/agent/sidecar/index.ts index e5ee4857..a44d0bef 100644 --- a/agent/sidecar/index.ts +++ b/agent/sidecar/index.ts @@ -93,6 +93,23 @@ function configureRuntimeEnv(spec: JobSpec): void { // when set; we set both so the cli.js init path can pick whichever // it consults first. process.env.CLAUDE_CONFIG_DIR = scratch; + + // Analysis tooling: the SDK's Bash tool aborts with "No suitable shell + // found" when SHELL is unset, and its Grep tool spawns a vendored ripgrep + // that isn't embedded in our Bun --compile bundle (ENOENT). The Docker + // image sets both env vars and installs bash + ripgrep; here we default + // them defensively so non-Docker installs (the per-user worktree binary at + // ~/.breadbox/agent-bin) also get a working shell + system rg. Only set + // when unset so an operator override always wins. + if (!process.env.SHELL) { + process.env.SHELL = existsSync("/bin/bash") ? "/bin/bash" : "/bin/sh"; + } + // Prefer the system ripgrep (on PATH) over the SDK's bundled copy, which is + // absent from the compiled sidecar. Harmless where a system rg is present; + // where it isn't, Grep was already broken via the missing vendored binary. + if (process.env.USE_BUILTIN_RIPGREP === undefined) { + process.env.USE_BUILTIN_RIPGREP = "0"; + } } async function main() { diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 07a28e55..22755d6a 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -343,6 +343,26 @@ List rules with optional filters and cursor pagination. **Lean by default** (`fi | `cursor` | string | Pagination cursor | | `limit` | int | Results per page (default 50, max 500) | +### query_transaction_rules (Read) + +The filterable/sortable analogue of `query_transactions`, for the rule set. Same lean-by-default `summary` projection as `list_transaction_rules`, but adds trigger / creator / hit-count filters and sorting so you can ask targeted questions — "which rules never fire?", "highest-impact rules for groceries", "agent-created rules" — instead of dumping the whole roster. To check whether **one** merchant is already covered before creating a rule, prefer `find_matching_rules`; use this when you want a filtered *slice* of rules or coverage analytics. + +| Parameter | Type | Description | +|-----------|------|-------------| +| `category_slug` | string | Filter to rules whose `set_category` action targets this slug | +| `enabled` | bool | Filter by enabled status | +| `trigger` | string | `on_create` \| `on_change` (alias `on_update`) \| `always` | +| `creator_type` | string | `user` \| `agent` \| `system` | +| `search` | string | Substring / words / fuzzy search on rule name | +| `search_mode` | string | `contains` (default) \| `words` \| `fuzzy` | +| `min_hit_count` | int | Only rules with `hit_count >= n` (surfaces high-impact rules) | +| `only_unused` | bool | Only rules that have never fired (`hit_count = 0`) — dead/over-specific rules worth pruning | +| `sort_by` | string | `priority` (default, pipeline order) \| `hit_count` \| `last_hit_at` \| `created_at` \| `name` | +| `sort_order` | string | `asc` \| `desc` (per-column default otherwise) | +| `fields` | string | Field selection. Alias: `summary` (default). `all` → full definition. | +| `cursor` | string | Pagination cursor. Only valid with the default `priority` sort; an explicit `sort_by` returns a single top-N page with no `next_cursor`. | +| `limit` | int | Results per page (default 50, max 500) | + ### update_transaction_rule (Write) Every field is optional; omit to leave unchanged. Pass `conditions={}` to explicitly switch to match-all; pass `actions=[...]` to replace the entire action set; pass `expires_at=""` to clear expiry. Pass `stage` (`baseline` | `standard` | `refinement` | `override`) to re-slot a rule into a canonical pipeline stage without guessing a raw `priority`. If both `stage` and `priority` are supplied, `priority` wins. diff --git a/internal/admin/settings_agents.go b/internal/admin/settings_agents.go index fdcbe34c..625fc148 100644 --- a/internal/admin/settings_agents.go +++ b/internal/admin/settings_agents.go @@ -60,6 +60,7 @@ func AgentsSettingsHandler(svc *service.Service, mcpServer *breadboxmcp.MCPServe "update_transactions": "Tags", "list_annotations": "Tags", "list_transaction_rules": "Rules", + "query_transaction_rules": "Rules", "create_transaction_rule": "Rules", "update_transaction_rule": "Rules", "delete_transaction_rule": "Rules", diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index c0eabf47..fa223683 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -562,6 +562,31 @@ func TestListTransactionRulesLeanDefault(t *testing.T) { requireKeys(t, "list_transaction_rules(all).rules[0]", ruleAll, "id", "name", "conditions", "actions", "created_at") } +// TestQueryTransactionRulesShape pins that query_transaction_rules shares the +// lean-by-default projection and accepts the richer filter/sort knobs. +func TestQueryTransactionRulesShape(t *testing.T) { + f := seedFixtures(t) + + // Default (no fields) → summary projection, same as the list mirror. + res, _, err := f.svc.handleQueryTransactionRules(f.ctx, nil, queryTransactionRulesInput{}) + out := decodeToolResult[map[string]any](t, "query_transaction_rules", res, err) + requireKeys(t, "query_transaction_rules", out, "rules") + rules := asArray(t, "query_transaction_rules.rules", out["rules"]) + if len(rules) == 0 { + t.Fatal("expected at least the seeded rule") + } + rule := asObject(t, "query_transaction_rules.rules[0]", rules[0]) + requireKeys(t, "query_transaction_rules.rules[0]", rule, "id", "name", "enabled", "priority", "trigger", "hit_count") + requireAbsent(t, "query_transaction_rules.rules[0]", rule, "conditions", "actions", "created_at", "short_id") + + // An explicit sort is accepted and suppresses the cursor (single top-N page). + resSorted, _, errSorted := f.svc.handleQueryTransactionRules(f.ctx, nil, queryTransactionRulesInput{SortBy: "hit_count"}) + outSorted := decodeToolResult[map[string]any](t, "query_transaction_rules(sorted)", resSorted, errSorted) + if nc, ok := outSorted["next_cursor"].(string); ok && nc != "" { + t.Errorf("explicit sort_by must not emit next_cursor, got %q", nc) + } +} + // TestPreviewRuleResponseShape pins `sample_matches` (not `sample`) + sample // row fields (transaction_id, not id). func TestPreviewRuleResponseShape(t *testing.T) { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 544caea3..1f236297 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -368,8 +368,12 @@ func (s *MCPServer) buildToolRegistry() { }, s.handleGetSyncStatus, s), makeToolDefLogged(ToolSpec{ Name: "list_transaction_rules", Title: "List Rules", Classification: ToolRead, - Description: "List transaction rules. Filter by category_slug, enabled, or search by name. Read this before authoring new rules to avoid duplicates. Lean by default: returns a summary projection (name, enabled, priority, trigger, category, hit_count) without the conditions/actions trees — pass fields=all to inspect or audit full rule definitions. Mirror of breadbox://rules (which always returns full).", + Description: "List transaction rules (the roster). Filter by category_slug, enabled, or search by name. Lean by default: returns a summary projection (name, enabled, priority, trigger, category, hit_count) without the conditions/actions trees — pass fields=all to inspect or audit full rule definitions. Mirror of breadbox://rules. For richer analysis — filter by trigger/creator/hit-count or sort by impact — use query_transaction_rules; to check whether one specific merchant is already covered, use find_matching_rules.", }, s.handleListTransactionRules, s), + makeToolDefLogged(ToolSpec{ + Name: "query_transaction_rules", Title: "Query Rules", Classification: ToolRead, + Description: "Query and analyze the rule set — the rules analogue of query_transactions. Filter by category_slug, enabled, trigger (on_create|on_change|always), creator_type (user|agent|system), name search, min_hit_count, or only_unused (rules that have never fired). Sort by priority (default, pipeline order), hit_count, last_hit_at, created_at, or name. Lean by default (summary projection: name, enabled, priority, trigger, category, hit_count, last_hit_at — no conditions/actions trees); pass fields=all for the full definitions. Use this to audit coverage and prune dead rules (only_unused=true) without dumping the whole roster. To check coverage for ONE merchant before creating a rule, prefer find_matching_rules. Cursor pagination applies only to the default priority sort; an explicit sort_by returns a single top-N page (raise limit, max 500).", + }, s.handleQueryTransactionRules, s), makeToolDefLogged(ToolSpec{ Name: "list_workflows", Title: "List Workflows", Classification: ToolRead, Description: "List the household's automation layer: the `workflows` it has enabled (each carries name, slug, trigger sync|schedule|manual, schedule_cron, tool_scope, the source `preset` it was instantiated from, plus last_run_status + last_run_at), and the full catalog of available `presets` it could enable (slug, name, category, description, tool_scope, trigger, default schedule_cron, and whether it's already enabled). Read this to see what runs automatically before suggesting new rules or reports — an existing workflow may already cover the task. Enabling/configuring workflows is an admin-UI action (the /workflows gallery), not an MCP write.", diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index 4882f0f6..c86beb52 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -142,6 +142,7 @@ func TestToolRegistryScopeContract(t *testing.T) { "list_tags", "get_sync_status", "list_transaction_rules", + "query_transaction_rules", "list_series", "get_series", "explain_series_candidates", diff --git a/internal/mcp/tools_reads.go b/internal/mcp/tools_reads.go index 9d0246da..70235dd5 100644 --- a/internal/mcp/tools_reads.go +++ b/internal/mcp/tools_reads.go @@ -50,6 +50,28 @@ type listTransactionRulesInput struct { Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Alias: summary (name,enabled,priority,trigger,category,hit_count,last_hit_at; the default — omits the conditions and actions trees). Default when omitted: summary. Pass fields=all for every field including the full conditions/actions. id is always included."` } +// queryTransactionRulesInput is the rich, filterable/sortable query over the +// rule set — the rules analogue of query_transactions. It is lean-by-default +// (summary projection) like list_transaction_rules, but adds trigger / creator +// / hit-count filters and sorting so an agent can ask targeted questions ("which +// rules never fire?", "highest-impact rules for groceries", "agent-created +// rules") instead of dumping the whole roster and scanning it. +type queryTransactionRulesInput struct { + CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter to rules whose set_category action targets this category slug."` + Enabled *bool `json:"enabled,omitempty" jsonschema:"Filter by enabled status (true=enabled only, false=disabled only)."` + Trigger string `json:"trigger,omitempty" jsonschema:"Filter by firing trigger: on_create, on_change (alias on_update), always."` + CreatorType string `json:"creator_type,omitempty" jsonschema:"Filter by who created the rule: user, agent, system."` + Search string `json:"search,omitempty" jsonschema:"Search by rule name. Comma-separated values are ORed."` + SearchMode string `json:"search_mode,omitempty" jsonschema:"Search mode: contains (default), words, fuzzy"` + MinHitCount *int `json:"min_hit_count,omitempty" jsonschema:"Filter to rules whose hit_count is >= this value — surfaces high-impact rules. Ignored when only_unused=true."` + OnlyUnused *bool `json:"only_unused,omitempty" jsonschema:"When true, return only rules that have never fired (hit_count=0) — dead or over-specific rules worth pruning."` + SortBy string `json:"sort_by,omitempty" jsonschema:"Sort: priority (default, pipeline execution order), hit_count, last_hit_at, created_at, name."` + SortOrder string `json:"sort_order,omitempty" jsonschema:"Sort direction: asc or desc. Default per column (desc for hit_count/last_hit_at/created_at, asc for priority/name)."` + Limit int `json:"limit,omitempty" jsonschema:"Max results (default 50, max 500)"` + Cursor string `json:"cursor,omitempty" jsonschema:"Pagination cursor from previous result. Only valid with the default priority sort; an explicit sort_by returns a single top-N page with no next_cursor."` + Fields string `json:"fields,omitempty" jsonschema:"Comma-separated fields to include, to cut response size. Alias: summary (name,enabled,priority,trigger,category,hit_count,last_hit_at; the default — omits the conditions and actions trees). Default when omitted: summary. Pass fields=all for every field including the full conditions/actions. id is always included."` +} + // --- Handlers --- func (s *MCPServer) handleListAccounts(_ context.Context, _ *mcpsdk.CallToolRequest, input listAccountsInput) (*mcpsdk.CallToolResult, any, error) { @@ -124,14 +146,42 @@ func (s *MCPServer) handleListTransactionRules(_ context.Context, _ *mcpsdk.Call // Lean-by-default: the rule roster omits the conditions/actions trees (the // heavy, deeply-nested part) unless the caller asks. Pass fields=all to // inspect or audit full rule definitions. - fieldsRaw := input.Fields - switch fieldsRaw { - case "": - fieldsRaw = service.DefaultRuleFields - case "all": - fieldsRaw = "" // ParseRuleFields("") → nil → full struct + fieldSet, err := parseRuleFieldsDefault(input.Fields) + if err != nil { + return errorResult(err), nil, nil + } + + result, err := s.svc.ListTransactionRules(ctx, params) + if err != nil { + return errorResult(err), nil, nil + } + return ruleListResult(result, fieldSet) +} + +func (s *MCPServer) handleQueryTransactionRules(_ context.Context, _ *mcpsdk.CallToolRequest, input queryTransactionRulesInput) (*mcpsdk.CallToolResult, any, error) { + ctx := context.Background() + + params := service.TransactionRuleListParams{ + Limit: input.Limit, + Cursor: input.Cursor, + CategorySlug: optStr(input.CategorySlug), + Enabled: input.Enabled, + Trigger: optStr(input.Trigger), + CreatorType: optStr(input.CreatorType), + Search: optStr(input.Search), + MinHitCount: input.MinHitCount, + SortBy: input.SortBy, + SortDir: input.SortOrder, + } + if input.OnlyUnused != nil { + params.OnlyUnused = *input.OnlyUnused } - fieldSet, err := service.ParseRuleFields(fieldsRaw) + var err error + if params.SearchMode, err = parseSearchMode(input.SearchMode); err != nil { + return errorResult(err), nil, nil + } + + fieldSet, err := parseRuleFieldsDefault(input.Fields) if err != nil { return errorResult(err), nil, nil } @@ -140,7 +190,24 @@ func (s *MCPServer) handleListTransactionRules(_ context.Context, _ *mcpsdk.Call if err != nil { return errorResult(err), nil, nil } + return ruleListResult(result, fieldSet) +} + +// parseRuleFieldsDefault resolves the shared lean-by-default fields contract for +// the rule list/query tools: "" → summary projection, "all" → full struct. +func parseRuleFieldsDefault(raw string) (map[string]bool, error) { + switch raw { + case "": + raw = service.DefaultRuleFields + case "all": + raw = "" // ParseRuleFields("") → nil → full struct + } + return service.ParseRuleFields(raw) +} +// ruleListResult shapes a rule list result, applying the field projection when +// one is requested. Shared by list_transaction_rules and query_transaction_rules. +func ruleListResult(result *service.TransactionRuleListResult, fieldSet map[string]bool) (*mcpsdk.CallToolResult, any, error) { if fieldSet != nil { projected := make([]map[string]any, len(result.Rules)) for i, r := range result.Rules { diff --git a/internal/service/rules.go b/internal/service/rules.go index f15e28b9..7b913d60 100644 --- a/internal/service/rules.go +++ b/internal/service/rules.go @@ -862,6 +862,30 @@ func (s *Service) ListTransactionRules(ctx context.Context, params TransactionRu argN++ } + if params.Trigger != nil && *params.Trigger != "" { + // Normalize the on_update alias to on_change for filtering, but also + // match rows still stored under the legacy value so neither spelling + // hides the other. + trig := *params.Trigger + if trig == "on_change" || trig == "on_update" { + whereClauses = append(whereClauses, fmt.Sprintf("tr.trigger IN ($%d, $%d)", argN, argN+1)) + args = append(args, "on_change", "on_update") + argN += 2 + } else { + whereClauses = append(whereClauses, fmt.Sprintf("tr.trigger = $%d", argN)) + args = append(args, trig) + argN++ + } + } + + if params.OnlyUnused { + whereClauses = append(whereClauses, "tr.hit_count = 0") + } else if params.MinHitCount != nil { + whereClauses = append(whereClauses, fmt.Sprintf("tr.hit_count >= $%d", argN)) + args = append(args, *params.MinHitCount) + argN++ + } + if params.Search != nil && *params.Search != "" { mode := "" if params.SearchMode != nil { @@ -945,8 +969,13 @@ func (s *Service) ListTransactionRules(ctx context.Context, params TransactionRu }, nil } - // Cursor-based pagination (API/MCP) - if params.Cursor != "" { + // Cursor-based pagination (API/MCP). The cursor key is (created_at, id), so + // it is only correct for the default created-at-descending ordering. When + // the caller asks for an explicit sort (hit_count, last_hit_at, name, + // priority), we return a single top-N page and emit no next_cursor rather + // than paginate against a key that doesn't match the ORDER BY. + cursorEligible := params.SortBy == "" + if cursorEligible && params.Cursor != "" { cursorTime, cursorIDStr, err := decodeTimestampCursor(params.Cursor) if err != nil { return nil, ErrInvalidCursor @@ -996,7 +1025,7 @@ func (s *Service) ListTransactionRules(ctx context.Context, params TransactionRu rules := s.convertRuleRows(ctx, scanned) var nextCursor string - if hasMore && len(scanned) > 0 { + if cursorEligible && hasMore && len(scanned) > 0 { last := scanned[len(scanned)-1] nextCursor = encodeTimestampCursor(last.createdAt.Time.UTC(), formatUUID(last.id)) } @@ -1540,13 +1569,13 @@ func (s *Service) ApplyAllRulesRetroactively(ctx context.Context) (map[string]in // Per transaction: fold rules to determine net action intents, mirroring // the sync resolver. Category: last-writer-wins. Tags: net-diff. type txnIntent struct { - txnID pgtype.UUID - catID pgtype.UUID - catSlug string - catRule *compiledRule - tagAdds map[string]*compiledRule // slug → first rule that added it - tagRemoves map[string]*compiledRule - matchingRules []*compiledRule // every rule whose condition matched (for rule_applied) + txnID pgtype.UUID + catID pgtype.UUID + catSlug string + catRule *compiledRule + tagAdds map[string]*compiledRule // slug → first rule that added it + tagRemoves map[string]*compiledRule + matchingRules []*compiledRule // every rule whose condition matched (for rule_applied) } var intents []txnIntent rowCount := 0 @@ -1741,8 +1770,8 @@ type RulePreviewMatch struct { // RulePreviewResult contains the results of a rule preview/dry-run. type RulePreviewResult struct { - MatchCount int64 `json:"match_count"` - TotalScanned int64 `json:"total_scanned"` + MatchCount int64 `json:"match_count"` + TotalScanned int64 `json:"total_scanned"` SampleMatches []RulePreviewMatch `json:"sample_matches"` } @@ -1973,10 +2002,10 @@ func (s *Service) previewRuleInternal(ctx context.Context, excludeRuleID *pgtype for rows.Next() { rowCount++ var ( - id pgtype.UUID - tctx TransactionContext - date pgtype.Date - catSlug string + id pgtype.UUID + tctx TransactionContext + date pgtype.Date + catSlug string ) if err := rows.Scan(&id, &tctx.Name, &tctx.MerchantName, &tctx.Amount, &tctx.CategoryPrimary, &tctx.CategoryDetailed, @@ -2045,22 +2074,22 @@ func (s *Service) BatchIncrementHitCounts(ctx context.Context, hits map[string]i // is populated at response time by looking up the set_category action's slug // (no JOINed category columns). type ruleRow struct { - id pgtype.UUID - shortID string - name string - conditions []byte // NULL -> nil -> match-all - actions []byte - trigger string - priority int32 - enabled bool - expiresAt pgtype.Timestamptz - createdByType string - createdByID pgtype.Text - createdByName string - hitCount int32 - lastHitAt pgtype.Timestamptz - createdAt pgtype.Timestamptz - updatedAt pgtype.Timestamptz + id pgtype.UUID + shortID string + name string + conditions []byte // NULL -> nil -> match-all + actions []byte + trigger string + priority int32 + enabled bool + expiresAt pgtype.Timestamptz + createdByType string + createdByID pgtype.Text + createdByName string + hitCount int32 + lastHitAt pgtype.Timestamptz + createdAt pgtype.Timestamptz + updatedAt pgtype.Timestamptz } // scanDest returns a slice of pointers for use with rows.Scan. @@ -2456,12 +2485,12 @@ type RuleApplicationRow struct { // RuleStats contains aggregate stats about a rule's impact. type RuleStats struct { - TotalApplications int64 `json:"total_applications"` - UniqueTransactions int64 `json:"unique_transactions"` - FirstAppliedAt string `json:"first_applied_at,omitempty"` - LastAppliedAt string `json:"last_applied_at,omitempty"` - SyncApplications int64 `json:"sync_applications"` - RetroApplications int64 `json:"retro_applications"` + TotalApplications int64 `json:"total_applications"` + UniqueTransactions int64 `json:"unique_transactions"` + FirstAppliedAt string `json:"first_applied_at,omitempty"` + LastAppliedAt string `json:"last_applied_at,omitempty"` + SyncApplications int64 `json:"sync_applications"` + RetroApplications int64 `json:"retro_applications"` } // GetRuleStats returns aggregate stats about a rule's applications, sourced diff --git a/internal/service/rules_integration_test.go b/internal/service/rules_integration_test.go index db37aa9c..7f6a2f8f 100644 --- a/internal/service/rules_integration_test.go +++ b/internal/service/rules_integration_test.go @@ -358,6 +358,92 @@ func TestListTransactionRules_FilterByCategory(t *testing.T) { } } +// TestQueryTransactionRules_FiltersAndSort exercises the query_transaction_rules +// knobs added on top of the base list: trigger filter, hit-count filters +// (min_hit_count / only_unused), explicit sort, and the rule that an explicit +// sort suppresses cursor pagination (the cursor key is (created_at,id), valid +// only for the default ordering). +func TestQueryTransactionRules_FiltersAndSort(t *testing.T) { + svc, queries, pool := newService(t) + ctx := context.Background() + cat := testutil.MustCreateCategory(t, queries, "food_and_drink", "Food & Drink") + + // Three rules with distinct triggers + hit counts. + mk := func(name, trigger string, hits int) string { + r, err := svc.CreateTransactionRule(ctx, service.CreateTransactionRuleParams{ + Name: name, + CategorySlug: cat.Slug, + Trigger: trigger, + Conditions: service.Condition{Field: "provider_name", Op: "eq", Value: name}, + }) + if err != nil { + t.Fatalf("create %q: %v", name, err) + } + if hits > 0 { + if _, err := pool.Exec(ctx, "UPDATE transaction_rules SET hit_count = $1 WHERE id = $2", hits, r.ID); err != nil { + t.Fatalf("bump hit_count for %q: %v", name, err) + } + } + return r.ID + } + mk("alpha", "on_create", 0) // never fired + mk("bravo", "on_change", 5) // mid + mk("charlie", "always", 20) // top impact + + // trigger filter: on_change matches the on_change rule (and the on_update alias). + trig := "on_change" + got, err := svc.ListTransactionRules(ctx, service.TransactionRuleListParams{Trigger: &trig}) + if err != nil { + t.Fatalf("trigger filter: %v", err) + } + if got.Total != 1 || len(got.Rules) != 1 || got.Rules[0].Name != "bravo" { + t.Errorf("trigger=on_change: want [bravo], got total=%d %+v", got.Total, ruleNames(got.Rules)) + } + + // only_unused: just the never-fired rule. + got, err = svc.ListTransactionRules(ctx, service.TransactionRuleListParams{OnlyUnused: true}) + if err != nil { + t.Fatalf("only_unused: %v", err) + } + if got.Total != 1 || got.Rules[0].Name != "alpha" { + t.Errorf("only_unused: want [alpha], got total=%d %+v", got.Total, ruleNames(got.Rules)) + } + + // min_hit_count >= 5 excludes the never-fired rule. + minHits := 5 + got, err = svc.ListTransactionRules(ctx, service.TransactionRuleListParams{MinHitCount: &minHits}) + if err != nil { + t.Fatalf("min_hit_count: %v", err) + } + if got.Total != 2 { + t.Errorf("min_hit_count=5: want 2, got %d %+v", got.Total, ruleNames(got.Rules)) + } + + // sort_by=hit_count desc → charlie(20), bravo(5), alpha(0); and no cursor on + // an explicit sort even when there's more than a page. + got, err = svc.ListTransactionRules(ctx, service.TransactionRuleListParams{SortBy: "hit_count", SortDir: "desc", Limit: 2}) + if err != nil { + t.Fatalf("sort_by hit_count: %v", err) + } + if len(got.Rules) != 2 || got.Rules[0].Name != "charlie" || got.Rules[1].Name != "bravo" { + t.Errorf("sort_by=hit_count desc: want [charlie bravo], got %+v", ruleNames(got.Rules)) + } + if !got.HasMore { + t.Errorf("sort_by=hit_count limit=2: expected HasMore with 3 rules") + } + if got.NextCursor != "" { + t.Errorf("explicit sort must not emit a next_cursor, got %q", got.NextCursor) + } +} + +func ruleNames(rules []service.TransactionRuleResponse) []string { + out := make([]string, len(rules)) + for i, r := range rules { + out[i] = r.Name + } + return out +} + func TestListTransactionRules_FilterBySearch(t *testing.T) { svc, queries, _ := newService(t) cat := testutil.MustCreateCategory(t, queries, "food_and_drink", "Food & Drink") diff --git a/internal/service/types.go b/internal/service/types.go index eb2acea5..882c3ef4 100644 --- a/internal/service/types.go +++ b/internal/service/types.go @@ -40,31 +40,31 @@ type TransactionCategoryInfo struct { } type TransactionResponse struct { - ID string `json:"id"` - ShortID string `json:"short_id"` - AccountID *string `json:"account_id"` - AccountName *string `json:"account_name"` - UserName *string `json:"user_name"` - AttributedUserID *string `json:"attributed_user_id,omitempty"` - AttributedUserName *string `json:"attributed_user_name,omitempty"` - EffectiveUserID *string `json:"effective_user_id,omitempty"` - Amount float64 `json:"amount"` - IsoCurrencyCode *string `json:"iso_currency_code"` - Date string `json:"date"` - AuthorizedDate *string `json:"authorized_date"` - Datetime *string `json:"datetime"` - AuthorizedDatetime *string `json:"authorized_datetime"` + ID string `json:"id"` + ShortID string `json:"short_id"` + AccountID *string `json:"account_id"` + AccountName *string `json:"account_name"` + UserName *string `json:"user_name"` + AttributedUserID *string `json:"attributed_user_id,omitempty"` + AttributedUserName *string `json:"attributed_user_name,omitempty"` + EffectiveUserID *string `json:"effective_user_id,omitempty"` + Amount float64 `json:"amount"` + IsoCurrencyCode *string `json:"iso_currency_code"` + Date string `json:"date"` + AuthorizedDate *string `json:"authorized_date"` + Datetime *string `json:"datetime"` + AuthorizedDatetime *string `json:"authorized_datetime"` ProviderName string `json:"provider_name"` ProviderMerchantName *string `json:"provider_merchant_name"` Category *TransactionCategoryInfo `json:"category"` - CategoryOverride string `json:"category_override"` + CategoryOverride string `json:"category_override"` ProviderCategoryPrimary *string `json:"provider_category_primary"` ProviderCategoryDetailed *string `json:"provider_category_detailed"` ProviderCategoryConfidence *string `json:"provider_category_confidence"` ProviderPaymentChannel *string `json:"provider_payment_channel"` - Pending bool `json:"pending"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` + Pending bool `json:"pending"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` // Tags attached to this transaction (slug list). Empty slice when none are // attached. Populated by ListTransactions / GetTransaction. @@ -88,17 +88,17 @@ type TransactionListResult struct { } type TransactionListParams struct { - Cursor string + Cursor string // Offset enables random-access pagination (LIMIT/OFFSET) — used by the // admin page-numbered pagination. Cursor pagination remains the default // for external REST clients; when Offset > 0 the service ignores Cursor. - Offset int - Limit int - StartDate *time.Time - EndDate *time.Time - AccountID *string - UserID *string - CategorySlug *string + Offset int + Limit int + StartDate *time.Time + EndDate *time.Time + AccountID *string + UserID *string + CategorySlug *string // Multi-select variants. When non-empty they take precedence over the // singular `AccountID` / `CategorySlug` fields above and produce an OR // match across every value in the list (parent categories still include @@ -122,11 +122,11 @@ type TransactionListParams struct { } type TransactionCountParams struct { - StartDate *time.Time - EndDate *time.Time - AccountID *string - UserID *string - CategorySlug *string + StartDate *time.Time + EndDate *time.Time + AccountID *string + UserID *string + CategorySlug *string // Multi-select variants — see TransactionListParams. AccountIDs []string CategorySlugs []string @@ -398,13 +398,13 @@ type AdminAccountDetail struct { // accounts (always single-element today). type AccountDetailResponse struct { AccountResponse - DisplayName *string `json:"display_name"` - Excluded bool `json:"excluded"` - Provider string `json:"provider,omitempty"` - UserName string `json:"connection_user_name,omitempty"` - ConnectionShortID string `json:"connection_short_id,omitempty"` - Balances []AccountBalance `json:"balances"` - RecentTransactions []TransactionResponse `json:"recent_transactions"` + DisplayName *string `json:"display_name"` + Excluded bool `json:"excluded"` + Provider string `json:"provider,omitempty"` + UserName string `json:"connection_user_name,omitempty"` + ConnectionShortID string `json:"connection_short_id,omitempty"` + Balances []AccountBalance `json:"balances"` + RecentTransactions []TransactionResponse `json:"recent_transactions"` } // AccountBalance represents a balance in a single currency. Today every @@ -495,7 +495,7 @@ type ActivityEntry struct { CategoryColor *string `json:"category_color,omitempty"` CategoryIcon *string `json:"category_icon,omitempty"` RuleName string `json:"rule_name,omitempty"` - RuleID string `json:"rule_id,omitempty"` + RuleID string `json:"rule_id,omitempty"` // RuleShortID is the rule's 8-char short_id used to build the // /rules/ link target on rule_applied timeline rows. The // rule detail route accepts either UUID or short_id, but the @@ -509,8 +509,8 @@ type ActivityEntry struct { // "category" it renders the category chip from CategoryName + // CategoryColor + CategoryIcon. ActionField string `json:"action_field,omitempty"` - CommentID string `json:"comment_id,omitempty"` - TagSlug string `json:"tag_slug,omitempty"` // for tag_added / tag_removed entries + CommentID string `json:"comment_id,omitempty"` + TagSlug string `json:"tag_slug,omitempty"` // for tag_added / tag_removed entries // TagDisplayName, TagColor and TagIcon drive the rendered tag-chip on // tag_added / tag_removed timeline rows. Empty/nil when the tag no @@ -578,9 +578,9 @@ type TransactionRuleResponse struct { Name string `json:"name"` // Conditions may be a zero-value Condition{} to mean "match all transactions" // (stored as NULL in the DB). - Conditions Condition `json:"conditions"` - Actions []RuleAction `json:"actions"` - Trigger string `json:"trigger"` + Conditions Condition `json:"conditions"` + Actions []RuleAction `json:"actions"` + Trigger string `json:"trigger"` // CategorySlug/CategoryName/CategoryIcon/CategoryColor are derived from the // first set_category action in Actions (kept for admin UI convenience). // Category info is no longer a denormalized column on transaction_rules — @@ -610,11 +610,24 @@ type TransactionRuleListParams struct { // CreatorType filters by the rule's creator. Accepted values: // "user", "agent", "system". Other values (or nil) leave the // dimension unfiltered. Used by the admin list page's "Type" - // filter; the public REST API does not expose this knob. + // filter and the query_transaction_rules MCP tool. CreatorType *string - // SortBy drives the ORDER BY clause for the offset-paginated path (admin UI). - // Accepted values: "created_at" (default), "hit_count", "last_hit_at", "priority", "name". - // Ignored by the cursor-paginated path (API), which must stay stable on (date, id). + // Trigger filters by the rule's firing trigger. Accepted values: + // "on_create", "on_change" (alias "on_update"), "always". nil leaves + // the dimension unfiltered. + Trigger *string + // MinHitCount filters to rules whose hit_count is >= this value. Use to + // surface high-impact rules. nil leaves the dimension unfiltered. + MinHitCount *int + // OnlyUnused filters to rules that have never fired (hit_count = 0). Use + // to surface dead/over-specific rules worth pruning. Ignored when false. + OnlyUnused bool + // SortBy drives the ORDER BY clause. Accepted values: "priority" (default), + // "created_at", "hit_count", "last_hit_at", "name". Honored by BOTH the + // offset-paginated path (admin UI) and the cursor-paginated path (API/MCP). + // Cursor pagination is only emitted for the default ordering — see + // ListTransactionRules; an explicit non-default SortBy returns a single + // top-N page with no next_cursor. SortBy string // SortDir is "asc" or "desc". Empty → per-column default (desc for most, asc for name/priority). SortDir string @@ -733,8 +746,8 @@ type MerchantSummaryRow struct { } type MerchantSummaryResult struct { - Merchants []MerchantSummaryRow `json:"merchants"` - Totals MerchantSummaryTotals `json:"totals"` + Merchants []MerchantSummaryRow `json:"merchants"` + Totals MerchantSummaryTotals `json:"totals"` Filters MerchantSummaryFilters `json:"filters"` } diff --git a/prompts/agents/strategy-routine-review.md b/prompts/agents/strategy-routine-review.md index 9ba83ebe..972eeae9 100644 --- a/prompts/agents/strategy-routine-review.md +++ b/prompts/agents/strategy-routine-review.md @@ -24,7 +24,7 @@ Clear the `needs-review` backlog with care. Create rules for new recurring patte ``` c. When uncertain, skip — LEAVE the `needs-review` tag on the transaction. The tag stays, the transaction stays in the queue for next time. Do NOT silently remove the tag without a category decision. -5. After reviewing, check if any new merchants appeared 2+ times (use `merchant_summary` if needed). For each candidate merchant, call `find_matching_rules(merchant="")` FIRST. If it returns a rule that already sets the category, the merchant is covered — do NOT create another rule. Only draft a rule where coverage is missing, then `preview_rule` it before creating. +5. After reviewing, scan the backlog you already fetched in step 2 for any merchant that appears 2+ times — a recurring pattern worth a rule. (The queue is small, so the names are already in front of you; no extra summary call is needed.) For each candidate merchant, call `find_matching_rules(merchant="")` FIRST. If it returns a rule that already sets the category, the merchant is covered — do NOT create another rule. Only draft a rule where coverage is missing, then `preview_rule` it before creating. 6. Submit a brief report. ## Rules in routine mode @@ -39,4 +39,5 @@ Clear the `needs-review` backlog with care. Create rules for new recurring patte - There are fewer items, so take time on each one - Prefer `contains` over exact match for merchant name rules - To avoid duplicates, use `find_matching_rules(merchant=...)` per candidate merchant — a targeted "is this already covered?" check. Do NOT dump the entire rule set with `list_transaction_rules` and scan it by hand; with hundreds of rules that wastes the run and still misses near-duplicates. `find_matching_rules(transaction_id=...)` is the equivalent check anchored to a specific row when you want every condition field evaluated. +- When you do want a filtered slice of the rule set rather than a single-merchant check — e.g. "what already targets `food_and_drink_groceries`?" or "which of my rules never fire?" — use `query_transaction_rules` with filters (`category_slug`, `creator_type`, `only_unused=true`) and a sort (`sort_by=hit_count`), not a full roster dump. It is lean-by-default and bounded, so it stays cheap. - Record your reasoning on non-obvious categorizations via the note on the `tags_to_remove` entry and/or the `comment` in the `update_transactions` compound op From 57b97dd8e26e1f5a20ca9c1c001a147f55b1932f Mon Sep 17 00:00:00 2001 From: Ricardo Canales Date: Sat, 13 Jun 2026 16:34:28 -0700 Subject: [PATCH 6/6] feat(mcp): restore merchant_summary tool (re-expose GetMerchantSummary) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six agent prompts (merchant-analysis, strategy-spending-report, strategy-rule-foundation, strategy-anomaly-detection, strategy-large-charge-sentinel, review-depth-thorough) and docs/mcp-tools-reference.md all reference a merchant_summary MCP tool that wasn't registered. The service method (GetMerchantSummary) and the REST endpoint (/transactions/merchants) already exist — only the MCP wiring was missing — so every prompt that told an agent to "use merchant_summary" hit an unknown tool and the agent improvised. Re-register the tool: per-merchant aggregation (count, total, avg, first/last date) over a window, with min_count / spending_only / category_slug / account_id / user_id / amount-range / search (fuzzy) filters. It's the merchant-grouped companion to transaction_summary (which groups by category/time, not merchant), backed by the same service method the REST endpoint uses. The admin tool-label map already listed merchant_summary; docs already documented it. Co-Authored-By: Claude Opus 4.8 --- docs/mcp-tools-reference.md | 17 +++--- .../mcp/response_shapes_integration_test.go | 30 +++++++++++ internal/mcp/server.go | 4 ++ internal/mcp/server_test.go | 1 + internal/mcp/tools.go | 53 ++++++++++++++++++- 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 22755d6a..90a358b6 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -70,16 +70,19 @@ Aggregated spending totals. Default date range: 30 days. ### merchant_summary (Read) -Merchant-level statistics. Default date range: 90 days. +Aggregate spend per **merchant** over a window (default last 90 days) — the merchant-grouped companion to `transaction_summary` (which groups by category/time, not merchant). Each row: `merchant`, `transaction_count`, `total_amount`, `avg_amount`, `first_date`, `last_date`, `iso_currency_code`. Amounts follow the convention positive = money out (debit); never summed across currencies. Capped at 500 merchants. | Parameter | Type | Description | |-----------|------|-------------| -| `min_count` | int | Minimum transaction count (use 2 for recurring, 3 for subscriptions) | -| `spending_only` | bool | Exclude credits/refunds | -| `search` | string | Search merchant names | -| `exclude_search` | string | Exclude matching merchants | - -Plus same date/account/user filters as `transaction_summary`. +| `start_date` / `end_date` | string | Window (YYYY-MM-DD). Defaults: 90 days ago → tomorrow | +| `min_count` | int | Minimum transaction count (use 2 for recurring, 3+ for subscriptions) | +| `spending_only` | bool | Only money-out amounts; exclude credits/refunds | +| `category_slug` | string | Filter by category before aggregating (parent includes children) | +| `account_id` / `user_id` | string | Filter by account or household member | +| `min_amount` / `max_amount` | float | Amount range (positive=debit, negative=credit) | +| `search` | string | Search merchant names (min 2 chars). Comma-separated values ORed | +| `search_mode` | string | `contains` (default) \| `words` \| `fuzzy` | +| `exclude_search` | string | Exclude merchants matching this text | --- diff --git a/internal/mcp/response_shapes_integration_test.go b/internal/mcp/response_shapes_integration_test.go index fa223683..e551db6f 100644 --- a/internal/mcp/response_shapes_integration_test.go +++ b/internal/mcp/response_shapes_integration_test.go @@ -587,6 +587,36 @@ func TestQueryTransactionRulesShape(t *testing.T) { } } +// TestMerchantSummaryShape pins the merchant_summary tool: a merchants array of +// per-merchant aggregates plus a totals block. seedFixtures creates two +// "Whole Foods" rows, so a window covering them yields a merchant with count 2. +func TestMerchantSummaryShape(t *testing.T) { + f := seedFixtures(t) + + res, _, err := f.svc.handleMerchantSummary(f.ctx, nil, merchantSummaryInput{ + StartDate: "2026-04-01", + EndDate: "2026-05-01", + }) + out := decodeToolResult[map[string]any](t, "merchant_summary", res, err) + requireKeys(t, "merchant_summary", out, "merchants", "totals") + merchants := asArray(t, "merchant_summary.merchants", out["merchants"]) + if len(merchants) == 0 { + t.Fatal("expected at least one merchant in window") + } + row := asObject(t, "merchant_summary.merchants[0]", merchants[0]) + requireKeys(t, "merchant_summary.merchants[0]", row, + "merchant", "transaction_count", "total_amount", "avg_amount", "first_date", "last_date") + + // min_count=3 excludes the count-2 Whole Foods merchant → empty result. + resHi, _, errHi := f.svc.handleMerchantSummary(f.ctx, nil, merchantSummaryInput{ + StartDate: "2026-04-01", EndDate: "2026-05-01", MinCount: 3, + }) + outHi := decodeToolResult[map[string]any](t, "merchant_summary(min_count=3)", resHi, errHi) + if got := asArray(t, "merchant_summary(min_count=3).merchants", outHi["merchants"]); len(got) != 0 { + t.Errorf("min_count=3: expected no merchants, got %d", len(got)) + } +} + // TestPreviewRuleResponseShape pins `sample_matches` (not `sample`) + sample // row fields (transaction_id, not id). func TestPreviewRuleResponseShape(t *testing.T) { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 1f236297..e1fa4de4 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -440,6 +440,10 @@ func (s *MCPServer) buildToolRegistry() { Name: "transaction_summary", Title: "Spending Summary", Classification: ToolRead, Description: "Get aggregated transaction totals grouped by category and/or time period. Replaces the need to paginate through thousands of individual transactions for spending analysis. Amounts follow the convention: positive = money out (debit), negative = money in (credit). Only includes non-deleted, non-pending transactions by default.", }, s.handleTransactionSummary, s), + makeToolDefLogged(ToolSpec{ + Name: "merchant_summary", Title: "Merchant Summary", Classification: ToolRead, + Description: "Aggregate spend per MERCHANT over a window (default last 90 days) — the merchant-grouped companion to transaction_summary, which groups by category/time but not merchant. Each row: merchant, transaction_count, total_amount, avg_amount, first_date, last_date. Amounts follow the convention positive = money out (debit), negative = money in (credit); never summed across currencies. Use min_count=2 to surface recurring merchants, min_count=3+ for subscriptions; spending_only=true to exclude credits/refunds in a spending report; first_date to spot newly-appearing merchants. Filter by category_slug, account_id, user_id, amount range, or search (fuzzy-tolerant for bank-feed name variants). Capped at 500 merchants.", + }, s.handleMerchantSummary, s), // --- Apply review decisions --- // update_transactions is the universal write for review work. It diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index c86beb52..2a88cadb 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -132,6 +132,7 @@ func TestToolRegistryScopeContract(t *testing.T) { "query_transactions", "count_transactions", "transaction_summary", + "merchant_summary", "list_annotations", "preview_rule", "find_matching_rules", diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index eb0c6469..d6c4deef 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -311,6 +311,56 @@ func (s *MCPServer) handleTransactionSummary(_ context.Context, _ *mcpsdk.CallTo return jsonResult(result) } +type merchantSummaryInput struct { + StartDate string `json:"start_date,omitempty" jsonschema:"Start date (YYYY-MM-DD) inclusive. Defaults to 90 days ago."` + EndDate string `json:"end_date,omitempty" jsonschema:"End date (YYYY-MM-DD) exclusive. Defaults to tomorrow."` + AccountID string `json:"account_id,omitempty" jsonschema:"Filter by account ID"` + UserID string `json:"user_id,omitempty" jsonschema:"Filter by user ID (family member)"` + CategorySlug string `json:"category_slug,omitempty" jsonschema:"Filter by category slug before aggregating (parent slug includes children)"` + MinCount int `json:"min_count,omitempty" jsonschema:"Minimum transaction count for a merchant to appear (default 1). Use 2 to surface recurring charges, 3+ for subscriptions."` + SpendingOnly *bool `json:"spending_only,omitempty" jsonschema:"Only count money-out (positive) amounts; exclude credits/refunds. Use for spending reports."` + MinAmount *float64 `json:"min_amount,omitempty" jsonschema:"Minimum transaction amount (positive=debit, negative=credit)"` + MaxAmount *float64 `json:"max_amount,omitempty" jsonschema:"Maximum transaction amount"` + Search string `json:"search,omitempty" jsonschema:"Search merchant names (min 2 chars). Comma-separated values are ORed."` + SearchMode string `json:"search_mode,omitempty" jsonschema:"Search mode: contains (default), words, fuzzy (typo-tolerant — good for bank-feed name variants)"` + ExcludeSearch string `json:"exclude_search,omitempty" jsonschema:"Exclude merchants whose name matches this text (min 2 chars)"` +} + +// handleMerchantSummary aggregates spend per merchant over a window. Backs the +// same GetMerchantSummary the REST /transactions/merchants endpoint uses. +func (s *MCPServer) handleMerchantSummary(_ context.Context, _ *mcpsdk.CallToolRequest, input merchantSummaryInput) (*mcpsdk.CallToolResult, any, error) { + ctx := context.Background() + + params := service.MerchantSummaryParams{ + AccountID: optStr(input.AccountID), + UserID: optStr(input.UserID), + CategorySlug: optStr(input.CategorySlug), + MinCount: input.MinCount, + MinAmount: input.MinAmount, + MaxAmount: input.MaxAmount, + Search: optStr(input.Search), + ExcludeSearch: optStr(input.ExcludeSearch), + } + + var err error + if params.StartDate, params.EndDate, err = parseDateRange(input.StartDate, input.EndDate); err != nil { + return errorResult(err), nil, nil + } + if params.SearchMode, err = parseSearchMode(input.SearchMode); err != nil { + return errorResult(err), nil, nil + } + if input.SpendingOnly != nil && *input.SpendingOnly { + params.SpendingOnly = true + } + + result, err := s.svc.GetMerchantSummary(ctx, params) + if err != nil { + return errorResult(err), nil, nil + } + + return jsonResult(result) +} + // --- Transaction Rules --- type createTransactionRuleInput struct { @@ -839,7 +889,7 @@ func shortIDPrefix(key string) (string, bool) { // avoiding the unmarshal→walk→remarshal cycle. It scans the byte stream and // collapses each object's own id/short_id pair: // -// {"id":"","short_id":""} → {"id":""} +// {"id":"","short_id":""} → {"id":""} // // Only the bare "short_id" key triggers the rewrite; the "short_id" key is // then dropped. If the sibling "id" field is missing or "short_id" is null, @@ -1134,4 +1184,3 @@ func scanJSONString(data []byte, pos int) (string, int) { } return s, end } -