Skip to content

tax: Switch money and asset amounts from float to Decimal #61

@przemyslawbialon

Description

@przemyslawbialon

Problem

All monetary and asset amounts use Python float:

  • pit38/domain/currency_exchange_service/currencies.py:35FiatValue.amount: float
  • pit38/domain/transactions/asset.py:3AssetValue.amount: float
  • NBP exchange rate multiplication — float
  • FIFO queue quantity tracking — float

This is a tax tool. 19% calculations. Sums of hundreds of transactions per broker per year, potentially five years of carry-forward. float (IEEE 754) means:

>>> 0.1 + 0.2
0.30000000000000004

Accumulation errors compound. round(..., 2) in FiatValue.__add__ masks single-operation imprecision but does not fix accumulation. The EPSILON = 0.0000001 in per_stock_calculator.py:13 is explicit evidence of float fragility — the FIFO matching would fail without it.

Real risk: a user with 500 transactions across a year could be mis-taxed by grosze/złoty. One PR #52 (PLN rounding) makes the visible number cleaner but does not fix the underlying accumulation in intermediate calculations.

Goal

Switch all monetary and asset amounts to decimal.Decimal. Integer arithmetic would be even safer (amounts as integer grosze), but Decimal is pragmatic and Python-native.

Acceptance criteria

  • FiatValue.amount: Decimal, AssetValue.amount: Decimal
  • CsvLoader._parse_fiat/_parse_asset uses Decimal(row["..."]) not float(row["..."])
  • NBP exchange rate provider returns Decimal (currently float)
  • Exchanger.exchange() preserves Decimal through multiplication
  • Queue operations in per_stock_calculator.py work with Decimal
  • Drop the EPSILON hack — Decimal equality is exact
  • FiatValue.__add__/__mul__ drop the round(..., 2) (no longer needed, Decimal is exact)
  • All 92 existing tests pass
  • New test: sum of many small amounts gives exact result (would fail with float)
  • E2E verification: example CSVs produce identical tax output as before (integer PLN match, intermediate may differ by fractional grosze — that's the point)

Implementation sketch

from decimal import Decimal, getcontext

getcontext().prec = 28  # default, plenty for tax calculations

@dataclass(frozen=True)
class FiatValue:
    amount: Decimal
    currency: Currency
    
    def __add__(self, other: "FiatValue") -> "FiatValue":
        if self.currency != other.currency:
            raise InvalidCurrencyException(...)
        return FiatValue(self.amount + other.amount, self.currency)
    
    # No round() anywhere — Decimal is exact

CSV parsing: Decimal(row["fiat_value"]) handles both "1234.56" and "1234" correctly.

Scale

  • ~10 files touched (anywhere float(...) is used for money/amounts)
  • Medium-size refactor: 6-10h
  • Breaking change for any consumer of FiatValue.amount/AssetValue.amount as float — audit CLI output formatting

Priority

P0 for correctness. This is the single most important change in the domain model.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    correctnessTax-correctness impact (high priority)enhancementNew feature or requesttaxTax calculation logic or rules

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions