diff --git a/docs/bug-fixes.md b/docs/bug-fixes.md index 490c180..796c4a2 100644 --- a/docs/bug-fixes.md +++ b/docs/bug-fixes.md @@ -12,6 +12,48 @@ Each bug fix entry should include: ## Bug Fixes +### 2026-07-11: Walmart ledger charge lines did not always match Monarch posting groups + +**Description:** +Two Walmart orders were repeatedly skipped even though their card transactions had posted in Monarch. Walmart's order ledger reported multiple `FINAL_CHARGES`, but Monarch/Plaid posted the same card activity as a combined transaction or as a different grouping. The strict Walmart multi-delivery path required every ledger charge line to have its own Monarch transaction, so it skipped posted orders as if transactions were missing. + +The same sync also processed byte-identical duplicate Walmart order summaries, producing duplicate ledger snapshots and duplicate errors for the same order IDs. + +**Test Case:** +```go +// internal/application/sync/handlers/walmart_test.go: +// TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateTransaction +// Ledger charges $42.16 + $8.99 should match a single Monarch transaction for $51.15. + +// internal/application/sync/handlers/walmart_test.go: +// TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateSubset +// Ledger charges $0.01 + $4.08 + $71.52 should match Monarch transactions $0.01 + $75.60. + +// internal/adapters/providers/walmart/provider_test.go: +// TestDedupeOrderSummariesByID +// Duplicate Walmart order summaries should be collapsed before fetching/processing details. + +// internal/domain/matcher/subset_test.go: +// TestSubsetSummingTo_PrefersExactTotalOverSmallerToleratedSubset +// An exact $9.99 + $0.01 subset must beat a single $9.99 transaction for a $10.00 target. +``` + +**Root Cause:** +The Walmart handler assumed `FINAL_CHARGES` ledger lines were always one-to-one with Monarch/Plaid transactions. In practice, the ledger lines are Walmart-side final charge entries, while the bank feed can post the same total as one card transaction or a different subset grouping. The Walmart purchase-history response can also include duplicate summaries for the same `orderId`. The subset matcher also returned the first valid, smallest subset, which could select a one-cent-off singleton before considering a later exact aggregate. + +**Fix Applied:** +The Walmart multi-delivery path still tries exact per-charge matching first. If that fails, it now falls back to matching Monarch purchase transaction(s) whose absolute amounts sum to the ledger charge total. A single aggregate match is split directly; multiple aggregate matches are consolidated using the ledger charge total, then split. The subset matcher now ranks valid candidates by amount difference before transaction count, so exact totals beat tolerated near-matches. Walmart order summaries are deduped by `orderId` before fetching full order details. + +**Verification:** +- New regression tests fail before the fix and pass after. +- `go test ./internal/application/sync/handlers ./internal/adapters/providers/walmart -run 'TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBack|TestDedupeOrderSummariesByID' -count=1` passes. +- `go test ./...` passes. +- `go build -o itemize ./cmd/itemize/` passes. +- A live read-only Monarch query confirmed the affected transactions were posted, unsplit, and grouped as `$51.15` and `$75.60 + $0.01`, with no hidden `$42.16`, `$8.99`, `$71.52`, or `$4.08` transactions. +- Temp-DB live dry-runs for `200015335172701` and `200015072601480` both completed with `Processed=1 Skipped=0 Errors=0`. The fetch logs showed Walmart still returned 9 order summaries, deduped to 7 fetched orders. + +--- + ### 2026-07-08: Amazon auth recovery leaked amazon-go and ignored positional accounts **Description:** diff --git a/internal/adapters/providers/walmart/provider.go b/internal/adapters/providers/walmart/provider.go index cbbbb5f..73b873e 100644 --- a/internal/adapters/providers/walmart/provider.go +++ b/internal/adapters/providers/walmart/provider.go @@ -64,9 +64,15 @@ func (p *Provider) FetchOrders(ctx context.Context, opts providers.FetchOptions) return nil, fmt.Errorf("failed to fetch walmart orders: %w", err) } + orderSummaries, duplicates := dedupeOrderSummariesByID(resp.Data.OrderHistoryV2.OrderGroups) + for _, orderID := range duplicates { + p.logger.Warn("duplicate order summary returned by Walmart; using first occurrence", + slog.String("order_id", orderID)) + } + // Convert OrderSummary to full Orders var providerOrders []providers.Order - for _, summary := range resp.Data.OrderHistoryV2.OrderGroups { + for _, summary := range orderSummaries { // Check for context cancellation before each order fetch if err := ctx.Err(); err != nil { return providerOrders, fmt.Errorf("cancelled during order fetch: %w", err) @@ -120,6 +126,27 @@ func (p *Provider) FetchOrders(ctx context.Context, opts providers.FetchOptions) return providerOrders, nil } +func dedupeOrderSummariesByID(orderSummaries []walmartclient.OrderSummary) ([]walmartclient.OrderSummary, []string) { + if len(orderSummaries) == 0 { + return nil, nil + } + + deduped := make([]walmartclient.OrderSummary, 0, len(orderSummaries)) + seen := make(map[string]bool, len(orderSummaries)) + var duplicates []string + + for _, summary := range orderSummaries { + if seen[summary.OrderID] { + duplicates = append(duplicates, summary.OrderID) + continue + } + seen[summary.OrderID] = true + deduped = append(deduped, summary) + } + + return deduped, duplicates +} + // GetOrderDetails fetches full details for a specific order func (p *Provider) GetOrderDetails(ctx context.Context, orderID string) (providers.Order, error) { // Note: We need to know if it's in-store or not, which we can't determine from just the ID diff --git a/internal/adapters/providers/walmart/provider_test.go b/internal/adapters/providers/walmart/provider_test.go index 4d24465..47eaed8 100644 --- a/internal/adapters/providers/walmart/provider_test.go +++ b/internal/adapters/providers/walmart/provider_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/eshaffer321/itemize/internal/adapters/providers" + walmartclient "github.com/eshaffer321/walmart-client-go/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,6 +69,21 @@ func TestProvider_FetchOrders_WithMaxOrders(t *testing.T) { // Then verify MaxOrders limits the result } +func TestDedupeOrderSummariesByID(t *testing.T) { + orders := []walmartclient.OrderSummary{ + {OrderID: "duplicate", FulfillmentType: "DFS", ItemCount: 2}, + {OrderID: "unique", FulfillmentType: "IN_STORE", ItemCount: 1}, + {OrderID: "duplicate", FulfillmentType: "DFS", ItemCount: 2}, + } + + deduped, duplicates := dedupeOrderSummariesByID(orders) + + require.Len(t, deduped, 2) + assert.Equal(t, "duplicate", deduped[0].OrderID) + assert.Equal(t, "unique", deduped[1].OrderID) + assert.Equal(t, []string{"duplicate"}, duplicates) +} + // TestProvider_GetOrderDetails tests fetching order details func TestProvider_GetOrderDetails(t *testing.T) { provider := NewProvider(nil, nil) diff --git a/internal/application/sync/handlers/walmart.go b/internal/application/sync/handlers/walmart.go index 121960b..9d7538a 100644 --- a/internal/application/sync/handlers/walmart.go +++ b/internal/application/sync/handlers/walmart.go @@ -10,11 +10,11 @@ import ( "strings" "time" - "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/eshaffer321/itemize/internal/adapters/providers" walmartprovider "github.com/eshaffer321/itemize/internal/adapters/providers/walmart" "github.com/eshaffer321/itemize/internal/domain/categorizer" "github.com/eshaffer321/itemize/internal/domain/matcher" + "github.com/eshaffer321/monarch-go/v2/pkg/monarch" ) // WalmartOrder extends providers.Order with Walmart-specific methods @@ -234,13 +234,25 @@ func (h *WalmartHandler) processMultiDeliveryOrder( } if !multiResult.AllFound { - // Count actual non-nil matches (len(Matches) includes nil entries for index alignment) - foundCount := 0 - for _, match := range multiResult.Matches { - if match != nil { - foundCount++ - } + aggregateResult, aggregateErr := h.processMultiDeliveryAggregateFallback( + ctx, + order, + monarchTxns, + usedTxnIDs, + catCategories, + monarchCategories, + charges, + dryRun, + ) + if aggregateErr != nil { + return nil, aggregateErr } + if aggregateResult != nil { + return aggregateResult, nil + } + + // Count actual non-nil matches (len(Matches) includes nil entries for index alignment) + foundCount := countFoundMatches(multiResult.Matches) result.Skipped = true result.SkipReason = fmt.Sprintf("could not find all transactions: expected %d, found %d", len(charges), foundCount) @@ -279,6 +291,84 @@ func (h *WalmartHandler) processMultiDeliveryOrder( return h.categorizeAndApplySplits(ctx, order, consolidatedTxn, catCategories, monarchCategories, dryRun) } +func (h *WalmartHandler) processMultiDeliveryAggregateFallback( + ctx context.Context, + order WalmartOrder, + monarchTxns []*monarch.Transaction, + usedTxnIDs map[string]bool, + catCategories []categorizer.Category, + monarchCategories []*monarch.TransactionCategory, + charges []float64, + dryRun bool, +) (*ProcessResult, error) { + ledgerTotal := sumCharges(charges) + if ledgerTotal <= 0 { + return nil, nil + } + + matchOrder := &ledgerAmountOrder{ + Order: order, + ledgerAmount: ledgerTotal, + } + + matchedTxns, err := h.matcher.FindSubsetByTotal(matchOrder, monarchTxns, usedTxnIDs) + if err != nil { + h.logDebug("No aggregate transaction fallback match", + "order_id", order.GetID(), + "ledger_total", ledgerTotal, + "error", err) + return nil, nil + } + + for _, txn := range matchedTxns { + usedTxnIDs[txn.ID] = true + } + + if len(matchedTxns) == 1 { + h.logInfo("Matched multi-delivery order by aggregate ledger total", + "order_id", order.GetID(), + "ledger_charge_count", len(charges), + "ledger_total", ledgerTotal, + "transaction_id", matchedTxns[0].ID) + return h.categorizeAndApplySplits(ctx, order, matchedTxns[0], catCategories, monarchCategories, dryRun) + } + + if h.consolidator == nil { + return nil, fmt.Errorf("aggregate match found %d transactions but consolidator is not configured", len(matchedTxns)) + } + + h.logInfo("Matched multi-delivery order by aggregate transaction subset", + "order_id", order.GetID(), + "ledger_charge_count", len(charges), + "ledger_total", ledgerTotal, + "transaction_count", len(matchedTxns)) + + consolidationResult, err := h.consolidator.ConsolidateTransactions(ctx, matchedTxns, matchOrder, dryRun) + if err != nil { + return nil, fmt.Errorf("aggregate fallback consolidation error: %w", err) + } + + return h.categorizeAndApplySplits(ctx, order, consolidationResult.ConsolidatedTransaction, catCategories, monarchCategories, dryRun) +} + +func countFoundMatches(matches []*matcher.MatchResult) int { + foundCount := 0 + for _, match := range matches { + if match != nil { + foundCount++ + } + } + return foundCount +} + +func sumCharges(charges []float64) float64 { + total := 0.0 + for _, charge := range charges { + total += charge + } + return math.Round(total*100) / 100 +} + // categorizeAndApplySplits applies categorization and splits to a transaction func (h *WalmartHandler) categorizeAndApplySplits( ctx context.Context, diff --git a/internal/application/sync/handlers/walmart_test.go b/internal/application/sync/handlers/walmart_test.go index 5bb0ae7..f4e9b1a 100644 --- a/internal/application/sync/handlers/walmart_test.go +++ b/internal/application/sync/handlers/walmart_test.go @@ -8,10 +8,10 @@ import ( "testing" "time" - "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/eshaffer321/itemize/internal/adapters/providers" "github.com/eshaffer321/itemize/internal/domain/categorizer" "github.com/eshaffer321/itemize/internal/domain/matcher" + "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -108,11 +108,15 @@ func (m *walmartTestMonarch) UpdateSplits(ctx context.Context, id string, splits // walmartTestConsolidator implements TransactionConsolidator type walmartTestConsolidator struct { - result *ConsolidationResult - err error + result *ConsolidationResult + err error + receivedTransactions []*monarch.Transaction + receivedOrderTotal float64 } func (m *walmartTestConsolidator) ConsolidateTransactions(ctx context.Context, transactions []*monarch.Transaction, order providers.Order, dryRun bool) (*ConsolidationResult, error) { + m.receivedTransactions = transactions + m.receivedOrderTotal = order.GetTotal() if m.err != nil { return nil, m.err } @@ -266,7 +270,7 @@ func TestWalmartHandler_ProcessOrder_GiftCard_UsesLedgerAmount(t *testing.T) { } txns := []*monarch.Transaction{ - {ID: "txn-gc", Amount: -70.00, Date: walmartToMonarchDate(orderDate)}, // Matches ledger amount + {ID: "txn-gc", Amount: -70.00, Date: walmartToMonarchDate(orderDate)}, // Matches ledger amount {ID: "txn-other", Amount: -100.00, Date: walmartToMonarchDate(orderDate)}, // Would match order total } @@ -366,6 +370,91 @@ func TestWalmartHandler_ProcessOrder_MultiDelivery_Success(t *testing.T) { assert.True(t, usedTxnIDs["txn-2"]) } +func TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateTransaction(t *testing.T) { + splitter := &walmartTestSplitter{categoryID: "groceries", notes: "Groceries"} + monarchClient := &walmartTestMonarch{} + handler := createTestWalmartHandler(t, splitter, nil, monarchClient) + + orderDate := time.Date(2026, 7, 6, 6, 25, 25, 0, time.FixedZone("MDT", -6*60*60)) + order := &walmartTestOrder{ + id: "200015335172701", + date: orderDate, + total: 51.15, + subtotal: 47.00, + tax: 4.15, + items: []providers.OrderItem{&walmartTestItem{name: "Item", price: 47.00, quantity: 1}}, + charges: []float64{42.16, 8.99}, + isMultiDeliver: true, + } + + txns := []*monarch.Transaction{ + {ID: "txn-aggregate", Amount: -51.15, Date: walmartToMonarchDate(orderDate)}, + } + usedTxnIDs := make(map[string]bool) + + result, err := handler.ProcessOrder( + context.Background(), + order, + txns, + usedTxnIDs, + nil, nil, + true, + ) + + require.NoError(t, err) + assert.True(t, result.Processed) + assert.False(t, result.Skipped) + assert.Equal(t, "txn-aggregate", result.Transaction.ID) + assert.True(t, usedTxnIDs["txn-aggregate"]) +} + +func TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateSubset(t *testing.T) { + splitter := &walmartTestSplitter{categoryID: "groceries", notes: "Groceries"} + monarchClient := &walmartTestMonarch{} + consolidator := &walmartTestConsolidator{ + result: &ConsolidationResult{ + ConsolidatedTransaction: &monarch.Transaction{ID: "txn-75-60", Amount: -75.61}, + }, + } + handler := createTestWalmartHandler(t, splitter, consolidator, monarchClient) + + orderDate := time.Date(2026, 7, 1, 8, 37, 37, 0, time.FixedZone("MDT", -6*60*60)) + order := &walmartTestOrder{ + id: "200015072601480", + date: orderDate, + total: 75.61, + subtotal: 70.00, + tax: 5.61, + items: []providers.OrderItem{&walmartTestItem{name: "Item", price: 70.00, quantity: 1}}, + charges: []float64{0.01, 4.08, 71.52}, + isMultiDeliver: true, + } + + txns := []*monarch.Transaction{ + {ID: "txn-75-60", Amount: -75.60, Date: walmartToMonarchDate(orderDate)}, + {ID: "txn-0-01", Amount: -0.01, Date: walmartToMonarchDate(orderDate)}, + } + usedTxnIDs := make(map[string]bool) + + result, err := handler.ProcessOrder( + context.Background(), + order, + txns, + usedTxnIDs, + nil, nil, + true, + ) + + require.NoError(t, err) + assert.True(t, result.Processed) + assert.False(t, result.Skipped) + assert.Equal(t, "txn-75-60", result.Transaction.ID) + assert.True(t, usedTxnIDs["txn-75-60"]) + assert.True(t, usedTxnIDs["txn-0-01"]) + require.Len(t, consolidator.receivedTransactions, 2) + assert.Equal(t, 75.61, consolidator.receivedOrderTotal) +} + func TestWalmartHandler_ProcessOrder_MultiDelivery_MissingTransaction(t *testing.T) { handler := createTestWalmartHandler(t, nil, nil, nil) diff --git a/internal/domain/matcher/subset.go b/internal/domain/matcher/subset.go index 1d0816d..b5eaa8e 100644 --- a/internal/domain/matcher/subset.go +++ b/internal/domain/matcher/subset.go @@ -56,51 +56,61 @@ func (m *Matcher) FindSubsetByTotal( return matches, nil } -// subsetSummingTo returns the smallest subset of txns whose absolute amounts -// sum to target within tolerance, or nil if none exists. +// subsetSummingTo returns the closest subset of txns whose absolute amounts +// sum to target within tolerance, or nil if none exists. When multiple subsets +// are equally close, it prefers the one with fewer transactions. func subsetSummingTo(txns []*monarch.Transaction, target, tolerance float64) []*monarch.Transaction { n := len(txns) if n > 20 { n = 20 // guard; 2^20 is ~1M — still fast, but cap for safety } - // Try subsets in increasing size order so we prefer fewer transactions + var best []*monarch.Transaction + bestDifference := math.Inf(1) + + // Search every valid subset. Iterating by size preserves the fewer-transactions + // tie-breaker while allowing an exact total to beat an earlier tolerated match. for size := 1; size <= n; size++ { - result := subsetOfSize(txns[:n], target, tolerance, 0, size, nil) - if result != nil { - return result + result, difference := closestSubsetOfSize(txns[:n], target, tolerance, 0, size, nil) + if result != nil && difference < bestDifference { + best = result + bestDifference = difference } } - return nil + return best } -// subsetOfSize is a recursive backtracking search for a subset of exactly `size` -// transactions summing to target. -func subsetOfSize( +// closestSubsetOfSize is a recursive backtracking search for the closest valid +// subset of exactly `size` transactions. +func closestSubsetOfSize( txns []*monarch.Transaction, target, tolerance float64, start, remaining int, current []*monarch.Transaction, -) []*monarch.Transaction { +) ([]*monarch.Transaction, float64) { if remaining == 0 { sum := 0.0 for _, t := range current { sum += math.Abs(t.Amount) } - if math.Abs(sum-target) <= tolerance { + difference := math.Abs(sum - target) + if difference <= tolerance { result := make([]*monarch.Transaction, len(current)) copy(result, current) - return result + return result, difference } - return nil + return nil, 0 } + var best []*monarch.Transaction + bestDifference := math.Inf(1) for i := start; i <= len(txns)-remaining; i++ { - found := subsetOfSize(txns, target, tolerance, i+1, remaining-1, + found, difference := closestSubsetOfSize(txns, target, tolerance, i+1, remaining-1, append(current, txns[i])) - if found != nil { - return found + if found != nil && difference < bestDifference { + best = found + bestDifference = difference } } - return nil + return best, bestDifference } diff --git a/internal/domain/matcher/subset_test.go b/internal/domain/matcher/subset_test.go new file mode 100644 index 0000000..b1c5570 --- /dev/null +++ b/internal/domain/matcher/subset_test.go @@ -0,0 +1,21 @@ +package matcher + +import ( + "testing" + + "github.com/eshaffer321/monarch-go/v2/pkg/monarch" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSubsetSummingTo_PrefersExactTotalOverSmallerToleratedSubset(t *testing.T) { + transactions := []*monarch.Transaction{ + {ID: "near", Amount: -9.99}, + {ID: "penny", Amount: -0.01}, + } + + matches := subsetSummingTo(transactions, 10.00, 0.01) + + require.Len(t, matches, 2) + assert.Equal(t, []string{"near", "penny"}, []string{matches[0].ID, matches[1].ID}) +}