refactor: Introduce MarketEvent hierarchy (unify Operation/Transaction)

[przemyslawbialon]

Problem

The current domain model has three related smells that all point to the same missing abstraction:

B1. Operation base class is a marker with no contract

# pit38/domain/stock/operations/operation.py
class Operation:
    type = None

Subclasses (Dividend, ServiceFee, StockSplit) have divergent signatures:

• Dividend(date, value) with value: FiatValue
• ServiceFee(date, value) with value: FiatValue
• StockSplit(date, stock, ratio) with ratio: int

No polymorphism. No shared method. isinstance(x, Operation) is the only benefit — a marker interface without interface.

B2. Transaction is not an Operation, but treated as one

# pit38/stock.py:34 — return type
def read_all(...) -> List[Operation | Transaction]:

Transaction is excluded from the hierarchy, yet appears in mixed lists with Operation subclasses. This forces code like:

records = read_all(...)
transactions = [r for r in records if isinstance(r, Transaction)]
operations = [r for r in records if isinstance(r, Operation)]

This is a strong smell: things we treat as a union should have a common parent, or the union itself is wrong.

B3. Two classification mechanisms coexist

• Transaction.action: Action (BUY/SELL)
• Operation.type: StockMarketOperation (class attribute)

Both ultimately describe "what kind of event is this", but live in different namespaces with different shapes. Filtering code has to know both:

# pit38/stock.py (simplified)
filter_transactions → isinstance(Transaction)
filter_stock_splits → op.type == StockMarketOperation.STOCK_SPLIT

Goal

Introduce a unified MarketEvent hierarchy. Every thing we load from a broker CSV is a subclass. Runtime classification happens via isinstance or pattern matching — enums become serialization detail, not primary domain concept.

Proposed hierarchy

MarketEvent (abstract frozen dataclass)
  ├── Trade (action: BUY | SELL, asset: AssetValue, fiat_value: FiatValue)
  ├── Dividend (value: FiatValue)
  ├── Fee (value: FiatValue, description: str)
  └── CorporateAction (abstract)
        └── StockSplit (asset: str, ratio: int)

Trade replaces Transaction. Dividend, Fee, StockSplit replace today's Operation subclasses.

Each concrete class can define a tax_impact() method if that helps, or stay as plain data (with downstream calculators doing the work). Design decision to make during implementation.

Acceptance criteria

• New pit38/domain/market/events/ package (name negotiable — see Naming below)
• MarketEvent abstract base: date: pendulum.DateTime as common attribute
• Trade(MarketEvent) replaces Transaction
• Dividend(MarketEvent), Fee(MarketEvent), StockSplit(CorporateAction(MarketEvent))
• All concrete classes are @dataclass(frozen=True) (enables __hash__, removes copy.deepcopy need in Queue)
• Action enum shrinks to TradeAction(BUY, SELL) — used only by Trade.action
• StockMarketOperation enum is deleted — classification is now type-based
• OperationFactory replaced by constructor-per-class OR a small build_from_row(row, asset_class) dispatcher that returns the right MarketEvent subclass
• pit38/stock.py and pit38/crypto.py filters use isinstance or pattern matching
• ProfitCalculator iterates List[MarketEvent] and dispatches via pattern match
• All 92 existing tests pass (after migration)
• E2E: example CSVs produce identical output

Naming

• MarketEvent vs Event vs Operation — I propose MarketEvent (current Operation name is too overloaded)
• Trade vs Transaction — Trade is more domain-correct ("transaction" is overloaded in finance, includes non-trade events). But refactoring everything named transaction is a lot of churn. Alternative: keep Transaction class name, move it under MarketEvent hierarchy as-is.

Decide during implementation. Both are reasonable.

Scale

Large refactor: 8-12h. Touches:

• Domain hierarchy (new files, renames)
• Loaders (BaseCsvLoader, subclasses, factories)
• Tax calculators (ProfitCalculator, PerStockProfitCalculator, YearlyProfitCalculator)
• Plugins (plugins/stock/revolut/*, plugins/crypto/*)
• CLI (stock.py, crypto.py)
• Tests (widespread — pass-through if signatures kept)

Should be tackled after A1 (Decimal) because they touch the same value types, and after #9 series (BaseCsvLoader) because the refactor builds on the ABC.

Priority

P1 — unblocks cleaner extensibility (forex, options, staking rewards) and removes two design smells at once.

Related

• Resolves B1, B2, B3 from the tech review
• Blocked by: A1 (Decimal refactor) and Consolidate operation and transaction reading from csv #9 series
• Should probably happen on a dedicated milestone "Domain v2" — not squeezed into existing milestones