From a2b74687c07744a56c3ae6e095a3d2188adc9247 Mon Sep 17 00:00:00 2001 From: Erick Shaffer Date: Sat, 3 Oct 2026 15:18:10 -0600 Subject: [PATCH] fix: avoid double-counting tax in Amazon splits --- docs/bug-fixes.md | 20 ++++++ internal/application/sync/handlers/amazon.go | 8 +++ .../application/sync/handlers/amazon_test.go | 65 +++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/docs/bug-fixes.md b/docs/bug-fixes.md index d8f7830..195a0ed 100644 --- a/docs/bug-fixes.md +++ b/docs/bug-fixes.md @@ -12,6 +12,26 @@ Each bug fix entry should include: ## Bug Fixes +### 2026-10-03: Amazon multi-category splits added allocated tax twice + +**Description:** +For Amazon orders split across multiple categories, each item's allocated cost already included its share of the final charge, including tax. The splitter also read the original order tax and added it again. Its balancing adjustment then hid the overage by reducing the largest category split, so split amounts did not match the item prices in their notes. + +**Test Case:** +`TestAllocatedAmazonOrderSplitsDoNotAddTaxTwice` reproduces the reported $148.84 charge with $141.44 subtotal, $10.70 tax, and items assigned across Clothing and Home & Garden. + +**Root Cause:** +`allocatedAmazonOrder` replaced the order's items with charge-allocated costs but inherited `GetTax()` from the original order. The shared splitter interpreted those final allocated costs as pre-tax subtotals and applied tax again. + +**Fix Applied:** +The allocated Amazon order now returns zero tax to the splitter because its item costs are pro-rata shares of the final charge, which already includes tax, fees, discounts, and other charge adjustments. Amazon data here provides only order-level tax, not per-item tax, so the category amounts are a proportional allocation rather than an exact tax breakdown. + +**Verification:** +- The regression failed before the fix (`Home & Garden` was -$79.22 and Clothing was -$69.62) and passes after it (`-$73.65` and `-$75.19`). +- `go test ./...` passes. + +**Commit:** Not committed. + ### 2026-09-26: Amazon orders misreported, missed, or matched to other orders' charges **Description:** diff --git a/internal/application/sync/handlers/amazon.go b/internal/application/sync/handlers/amazon.go index eaad71b..4d159af 100644 --- a/internal/application/sync/handlers/amazon.go +++ b/internal/application/sync/handlers/amazon.go @@ -538,6 +538,14 @@ func (a *allocatedAmazonOrder) GetItems() []providers.OrderItem { return items } +// GetTax returns zero because GetItems exposes pro-rata shares of the final +// charge, which already includes tax, fees, discounts, and other adjustments. +// Amazon provides only order-level tax here, so the splitter cannot calculate +// exact item-level tax and must not add the aggregate tax a second time. +func (a *allocatedAmazonOrder) GetTax() float64 { + return 0 +} + // allocatedItem represents an item with its allocated cost type allocatedItem struct { name string diff --git a/internal/application/sync/handlers/amazon_test.go b/internal/application/sync/handlers/amazon_test.go index accfdf0..b613e9d 100644 --- a/internal/application/sync/handlers/amazon_test.go +++ b/internal/application/sync/handlers/amazon_test.go @@ -8,8 +8,10 @@ import ( "github.com/eshaffer321/itemize/internal/adapters/providers" amazonprovider "github.com/eshaffer321/itemize/internal/adapters/providers/amazon" + "github.com/eshaffer321/itemize/internal/domain/allocator" "github.com/eshaffer321/itemize/internal/domain/categorizer" "github.com/eshaffer321/itemize/internal/domain/matcher" + "github.com/eshaffer321/itemize/internal/domain/splitter" "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -919,3 +921,66 @@ func TestAllocatedItem(t *testing.T) { assert.Empty(t, item.GetSKU()) assert.Empty(t, item.GetCategory()) } + +type fixedCategorizer struct { + result *categorizer.CategorizationResult +} + +func (c *fixedCategorizer) CategorizeItems(context.Context, []categorizer.Item, []categorizer.Category) (*categorizer.CategorizationResult, error) { + return c.result, nil +} + +func TestAllocatedAmazonOrderSplitsDoNotAddTaxTwice(t *testing.T) { + listPrices := []float64{32.99, 9.98, 13.49, 69.99, 14.99} + baseItems := make([]providers.OrderItem, len(listPrices)) + allocationItems := make([]allocator.Item, len(listPrices)) + for i, price := range listPrices { + name := []string{"item 1", "item 2", "item 3", "sheet set", "item 5"}[i] + baseItems[i] = &mockItem{name: name, price: price} + allocationItems[i] = allocator.Item{Name: name, ListPrice: price} + } + + allocation, err := allocator.Allocate(allocationItems, 148.84) + require.NoError(t, err) + order := &mockAmazonOrder{ + id: "amazon-tax-regression", + total: 148.84, + subtotal: 141.44, + tax: 10.70, + items: baseItems, + } + allocatedOrder := &allocatedAmazonOrder{ + Order: order, + allocations: allocation.Allocations, + baseItems: baseItems, + } + categorizer := &fixedCategorizer{result: &categorizer.CategorizationResult{Categorizations: []categorizer.ItemCategorization{ + {ItemName: "item 1", CategoryID: "clothing", CategoryName: "Clothing"}, + {ItemName: "item 2", CategoryID: "clothing", CategoryName: "Clothing"}, + {ItemName: "item 3", CategoryID: "clothing", CategoryName: "Clothing"}, + {ItemName: "sheet set", CategoryID: "home", CategoryName: "Home & Garden"}, + {ItemName: "item 5", CategoryID: "clothing", CategoryName: "Clothing"}, + }}} + realSplitter := splitter.NewSplitter(categorizer) + + splits, err := realSplitter.CreateSplits( + context.Background(), + allocatedOrder, + &monarch.Transaction{ID: "txn", Amount: -148.84}, + nil, + nil, + ) + require.NoError(t, err) + require.Len(t, splits, 2) + + amountsByCategory := make(map[string]float64, len(splits)) + notesByCategory := make(map[string]string, len(splits)) + for _, split := range splits { + amountsByCategory[split.CategoryID] = split.Amount + notesByCategory[split.CategoryID] = split.Notes + } + assert.Equal(t, -73.65, amountsByCategory["home"]) + assert.Equal(t, -75.19, amountsByCategory["clothing"]) + assert.Equal(t, -148.84, amountsByCategory["home"]+amountsByCategory["clothing"]) + assert.Contains(t, notesByCategory["home"], "$73.65") +}