From a71c91f6efad8301a96a8488075692bda13ad0b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Sep 2026 02:23:32 +0000 Subject: [PATCH] fix: add unified ledger amount and enforce manual decimal precision Co-authored-by: genichin <4282227+genichin@users.noreply.github.com> --- app/api/v1_portfolios.py | 14 +-- app/repositories/portfolio_repository.py | 11 +- app/repositories/postgres.py | 101 ++++++++++++------ app/schemas/v1.py | 13 +-- docs/data-model.md | 3 + docs/runtime-flows.md | 9 +- ...tfolio_ledger_manual_metadata_migration.py | 41 +++++++ tests/test_v1_portfolios_manual_ledger.py | 36 +++++++ 8 files changed, 173 insertions(+), 55 deletions(-) diff --git a/app/api/v1_portfolios.py b/app/api/v1_portfolios.py index 74e1e58..bd64a37 100644 --- a/app/api/v1_portfolios.py +++ b/app/api/v1_portfolios.py @@ -769,16 +769,18 @@ def _decode_ledger_cursor(cursor: str, *, portfolio_id: str, source: str | None) def _ledger_entry_to_history_item(entry) -> LedgerHistoryItem: + action = (entry.action or entry.side or "").upper() return LedgerHistoryItem( ledger_entry_id=entry.ledger_entry_id, portfolio_id=entry.portfolio_id, source=entry.source or "broker", - action=(entry.action or entry.side or "").upper(), + action=action, recorded_at=entry.recorded_at, executed_at=entry.executed_at, order_id=entry.order_id, execution_id=entry.execution_id, quantity=entry.quantity, + amount=entry.price if action in {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT", "DIVIDEND"} else None, price=entry.price, fee_amount=entry.fee_amount, tax_amount=entry.tax_amount, @@ -825,11 +827,11 @@ def record_manual_ledger_v1( entry = repo.record_manual_ledger( portfolio_id, action=body.action, - amount=float(body.amount) if body.amount is not None else None, - quantity=float(body.quantity) if body.quantity is not None else None, - price=float(body.price) if body.price is not None else None, - fee=float(body.fee), - tax=float(body.tax), + amount=body.amount, + quantity=body.quantity, + price=body.price, + fee=body.fee, + tax=body.tax, executed_at=( datetime( body.executed_at.year, diff --git a/app/repositories/portfolio_repository.py b/app/repositories/portfolio_repository.py index c0eb9d4..e550cec 100644 --- a/app/repositories/portfolio_repository.py +++ b/app/repositories/portfolio_repository.py @@ -3,6 +3,7 @@ from __future__ import annotations from datetime import datetime +from decimal import Decimal from typing import Protocol from app.domain.models import Asset, AuthToken, BrokerCredential, Daemon, LedgerEntry, Portfolio, PortfolioComment, PortfolioTag, StrategySet, Tag, TokenPrincipal, User @@ -320,11 +321,11 @@ class PortfolioRepository(Protocol): portfolio_id: str, *, action: str, - amount: float | None = None, - quantity: float | None = None, - price: float | None = None, - fee: float = 0.0, - tax: float = 0.0, + amount: Decimal | None = None, + quantity: Decimal | None = None, + price: Decimal | None = None, + fee: Decimal = Decimal("0"), + tax: Decimal = Decimal("0"), executed_at: datetime | None = None, target_ledger_entry_id: int | None = None, note: str = "", diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index c12c945..0a46ff0 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -8,6 +8,7 @@ import logging import uuid from dataclasses import dataclass from datetime import datetime, timezone +from decimal import Decimal, InvalidOperation, ROUND_HALF_UP from typing import Any import psycopg @@ -17,6 +18,9 @@ from app.domain.errors import DaemonInactiveError, DaemonInUseError, DaemonNotFo from app.domain.models import Asset, AuthToken, BrokerCredential, Daemon, LedgerEntry, Portfolio, PortfolioComment, PortfolioTag, StrategySet, Tag, TokenPrincipal, User logger = logging.getLogger(__name__) +_LEDGER_NUMERIC_SCALE = 8 +_LEDGER_MAX_INTEGER_DIGITS = 12 +_LEDGER_QUANT = Decimal("0.00000001") _SELECT_USER = """ SELECT @@ -2007,16 +2011,44 @@ class PostgresPortfolioRepository: def _manual_total_value(self, cash: float, quantity: float, price: float, buffer: float) -> float: return float(cash) + float(quantity) * float(price) + float(buffer) + @staticmethod + def _validate_ledger_decimal(value: Decimal, *, field_name: str, allow_zero: bool) -> Decimal: + if value.is_nan() or value.is_infinite(): + raise ValueError(f"{field_name} must be a finite decimal") + if allow_zero: + if value < 0: + raise ValueError(f"{field_name} must be greater than or equal to 0") + elif value <= 0: + raise ValueError(f"{field_name} must be greater than 0") + scale = max(-value.as_tuple().exponent, 0) + if scale > _LEDGER_NUMERIC_SCALE: + raise ValueError(f"{field_name} exceeds NUMERIC(20,8) precision") + digits = len(value.copy_abs().as_tuple().digits) + integer_digits = max(digits - scale, 0) + if value == 0: + integer_digits = 0 + if integer_digits > _LEDGER_MAX_INTEGER_DIGITS: + raise ValueError(f"{field_name} exceeds NUMERIC(20,8) precision") + return value.quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) + + @classmethod + def _coerce_ledger_decimal(cls, value: Any, *, field_name: str, allow_zero: bool) -> Decimal: + try: + decimal_value = Decimal(str(value)) + except (InvalidOperation, ValueError, TypeError): + raise ValueError(f"{field_name} must be a decimal") from None + return cls._validate_ledger_decimal(decimal_value, field_name=field_name, allow_zero=allow_zero) + def record_manual_ledger( self, portfolio_id: str, *, action: str, - amount: float | None = None, - quantity: float | None = None, - price: float | None = None, - fee: float = 0.0, - tax: float = 0.0, + amount: Decimal | None = None, + quantity: Decimal | None = None, + price: Decimal | None = None, + fee: Decimal = Decimal("0"), + tax: Decimal = Decimal("0"), executed_at: datetime | None = None, target_ledger_entry_id: int | None = None, note: str = "", @@ -2036,15 +2068,15 @@ class PostgresPortfolioRepository: if row is None: raise ValueError("포트폴리오를 찾을 수 없습니다") portfolio = self._row_to_portfolio(row) - cash = float(portfolio.cash_balance) - qty = float(portfolio.asset_quantity) - buffer = float(portfolio.profit_buffer) - asset_price = float(portfolio.price) - ledger_price = float(price or 0.0) - ledger_quantity = float(quantity or 0.0) - fee_f = float(fee or 0.0) - tax_f = float(tax or 0.0) - amount_f = float(amount or 0.0) + cash = self._coerce_ledger_decimal(portfolio.cash_balance, field_name="cash_balance", allow_zero=True) + qty = self._coerce_ledger_decimal(portfolio.asset_quantity, field_name="asset_quantity", allow_zero=True) + buffer = self._coerce_ledger_decimal(portfolio.profit_buffer, field_name="profit_buffer", allow_zero=True) + asset_price = self._coerce_ledger_decimal(portfolio.price, field_name="asset_price", allow_zero=True) + ledger_price = self._coerce_ledger_decimal(price or 0, field_name="price", allow_zero=True) + ledger_quantity = self._coerce_ledger_decimal(quantity or 0, field_name="quantity", allow_zero=True) + fee_f = self._coerce_ledger_decimal(fee or 0, field_name="fee", allow_zero=True) + tax_f = self._coerce_ledger_decimal(tax or 0, field_name="tax", allow_zero=True) + amount_f = self._coerce_ledger_decimal(amount or 0, field_name="amount", allow_zero=True) ledger_executed_at = executed_at or datetime.now(timezone.utc) if ledger_executed_at.tzinfo is None: ledger_executed_at = ledger_executed_at.replace(tzinfo=timezone.utc) @@ -2105,7 +2137,7 @@ class PostgresPortfolioRepository: buffer += amount_f ledger_price = amount_f elif normalized == "BUY": - required = ledger_quantity * ledger_price + fee_f + tax_f + required = (ledger_quantity * ledger_price + fee_f + tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) if cash < required: raise ValueError("INSUFFICIENT_CASH") cash -= required @@ -2114,7 +2146,7 @@ class PostgresPortfolioRepository: elif normalized == "SELL": if qty < ledger_quantity: raise ValueError("INSUFFICIENT_QUANTITY") - cash += ledger_quantity * ledger_price - fee_f - tax_f + cash += (ledger_quantity * ledger_price - fee_f - tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) qty -= ledger_quantity asset_price = ledger_price elif normalized == "REVERSAL": @@ -2139,18 +2171,19 @@ class PostgresPortfolioRepository: if cur.fetchone() is not None: raise ValueError("LEDGER_ENTRY_ALREADY_REVERSED") target_action = (reverse_target.action or "").upper() - amount_f = float(reverse_target.price) - ledger_price = float(reverse_target.price) - ledger_quantity = float(reverse_target.quantity) - fee_f = float(reverse_target.fee_amount) - tax_f = float(reverse_target.tax_amount) + amount_f = self._coerce_ledger_decimal(reverse_target.price, field_name="amount", allow_zero=True) + ledger_price = self._coerce_ledger_decimal(reverse_target.price, field_name="price", allow_zero=True) + ledger_quantity = self._coerce_ledger_decimal(reverse_target.quantity, field_name="quantity", allow_zero=True) + fee_f = self._coerce_ledger_decimal(reverse_target.fee_amount, field_name="fee", allow_zero=True) + tax_f = self._coerce_ledger_decimal(reverse_target.tax_amount, field_name="tax", allow_zero=True) if target_action == "BUY": - cash += ledger_quantity * ledger_price + fee_f + tax_f + cash += (ledger_quantity * ledger_price + fee_f + tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) qty -= ledger_quantity elif target_action == "SELL": - if cash < ledger_quantity * ledger_price - fee_f - tax_f: + required_cash = (ledger_quantity * ledger_price - fee_f - tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) + if cash < required_cash: raise ValueError("INSUFFICIENT_CASH") - cash -= ledger_quantity * ledger_price - fee_f - tax_f + cash -= required_cash qty += ledger_quantity elif target_action == "BUFFER_OUT": if cash < amount_f: @@ -2178,7 +2211,7 @@ class PostgresPortfolioRepository: if buffer < 0: raise ValueError("INSUFFICIENT_BUFFER") - total_value = self._manual_total_value(cash, qty, asset_price, buffer) + total_value = (cash + qty * asset_price + buffer).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP) cur.execute( """ UPDATE portfolios @@ -2190,7 +2223,7 @@ class PostgresPortfolioRepository: last_trade_at = NOW() WHERE portfolio_id = %s """, - (cash, qty, asset_price, buffer, portfolio_id), + (float(cash), float(qty), float(asset_price), float(buffer), portfolio_id), ) cur.execute( """ @@ -2208,14 +2241,14 @@ class PostgresPortfolioRepository: ( portfolio_id, normalized.lower(), - float(ledger_price), - float(ledger_quantity), - float(fee_f), - float(tax_f), - float(cash), - float(qty), - float(total_value), - float(buffer), + ledger_price, + ledger_quantity, + fee_f, + tax_f, + cash, + qty, + total_value, + buffer, "manual", note, ledger_executed_at, diff --git a/app/schemas/v1.py b/app/schemas/v1.py index 1b113b6..0f39cb0 100644 --- a/app/schemas/v1.py +++ b/app/schemas/v1.py @@ -157,11 +157,11 @@ class ManualLedgerRequest(BaseModel): model_config = ConfigDict(extra="forbid") action: ManualLedgerAction - amount: Decimal | None = Field(default=None, gt=0) - quantity: Decimal | None = Field(default=None, gt=0) - price: Decimal | None = Field(default=None, gt=0) - fee: Decimal = Field(default=Decimal("0"), ge=0) - tax: Decimal = Field(default=Decimal("0"), ge=0) + amount: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8) + quantity: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8) + price: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8) + fee: Decimal = Field(default=Decimal("0"), ge=0, max_digits=20, decimal_places=8) + tax: Decimal = Field(default=Decimal("0"), ge=0, max_digits=20, decimal_places=8) executed_at: date | None = None target_ledger_entry_id: int | None = None note: str = "" @@ -177,7 +177,7 @@ class ManualLedgerRequest(BaseModel): fee_or_tax_provided = "fee" in provided_fields or "tax" in provided_fields fee_provided = "fee" in provided_fields executed_at_provided = "executed_at" in provided_fields - confirm_duplicate_provided = "confirm_duplicate" in provided_fields and self.confirm_duplicate + confirm_duplicate_provided = "confirm_duplicate" in provided_fields if self.action in amount_actions: if self.amount is None: raise ValueError("amount is required for this action") @@ -255,6 +255,7 @@ class LedgerHistoryItem(BaseModel): order_id: str | None = None execution_id: str | None = None quantity: float | None = None + amount: float | None = None price: float | None = None fee_amount: float | None = None tax_amount: float | None = None diff --git a/docs/data-model.md b/docs/data-model.md index 0b02afb..86177bd 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -341,6 +341,7 @@ - BUFFER_IN: cash_balance -> profit_buffer. - BUY: cash_balance -> asset_quantity. - SELL: asset_quantity -> cash_balance. +- DIVIDEND: external net cash dividend -> profit_buffer only; `tax_amount` is reference metadata and does not affect snapshot math. - REVERSAL: append-only inverse effect of `target_ledger_entry_id`. ### Return Aggregate Semantics @@ -356,6 +357,8 @@ - Manual writes lock the portfolio row while calculating/appending/updating snapshots to avoid lost updates. - Manual writes reject insufficient cash/quantity/buffer with HTTP 409 rather than creating a new negative state. +- DIVIDEND duplicate candidate (`portfolio_id`, `executed_at` UTC date, decimal `amount`, `note`) returns HTTP 409 `POTENTIAL_DUPLICATE_DIVIDEND` unless request confirms with `confirm_duplicate=true`. +- DIVIDEND uses date-only input stored as `executed_at` UTC midnight (`00:00:00Z`). - `total_value_after` for manual entries is the portfolio snapshot value after the mutation and includes cash, asset valuation, and profit buffer. ## Shared Consistency Rules diff --git a/docs/runtime-flows.md b/docs/runtime-flows.md index 7e0185e..31ed0c2 100644 --- a/docs/runtime-flows.md +++ b/docs/runtime-flows.md @@ -280,11 +280,12 @@ 3. Disabled portfolio receives HTTP 409 `PORTFOLIO_DISABLED`. 4. If `managed_by_daemon_id IS NOT NULL` and request lacks `confirm_daemon_managed_manual_write=true`, the API returns HTTP 409 `MANUAL_WRITE_REQUIRES_CONFIRMATION` with daemon id and required confirmation marker. 5. Valid actions dispatch to a single repository transaction that locks the portfolio row (`FOR UPDATE OF p`), calculates the action effect, appends a `portfolio_ledger` row with `source='manual'`, and updates the portfolio snapshot. -6. BUY/SELL require sufficient cash/quantity. BUFFER_OUT/BUFFER_IN/WITHDRAW/PROFIT require sufficient buffer/cash as appropriate. Insufficient state returns HTTP 409 with structured code. +6. BUY/SELL require sufficient cash/quantity. BUFFER_OUT/BUFFER_IN/WITHDRAW/PROFIT require sufficient buffer/cash as appropriate. DIVIDEND requires `amount` and date-only `executed_at`, stores `executed_at` at UTC midnight, and increases only `profit_buffer` (`tax` metadata only). Insufficient state returns HTTP 409 with structured code. 7. REVERSAL requires `target_ledger_entry_id`, rejects duplicate reversal targets, and appends a reversing row instead of mutating/deleting the target. -8. Response includes after-snapshot: `cash_balance_after`, `asset_quantity_after`, `profit_buffer_after`, `total_value_after`. -9. `GET /v1/portfolios/{portfolio_id}/ledger/manual` returns newest manual entries only (`source='manual'`). -10. `GET /v1/portfolios/{portfolio_id}/returns/summary` returns `net_contributed_capital=sum(INVEST)-sum(WITHDRAW)` and `cumulative_profit_withdrawn=sum(PROFIT)` separately. +8. Duplicate DIVIDEND candidate (same portfolio/date/amount/note and original not reversed) returns HTTP 409 `POTENTIAL_DUPLICATE_DIVIDEND` with matched entry id; `confirm_duplicate=true` re-submit records it. +9. Response includes after-snapshot: `cash_balance_after`, `asset_quantity_after`, `profit_buffer_after`, `total_value_after`. +10. `GET /v1/portfolios/{portfolio_id}/ledger/manual` returns newest manual entries only (`source='manual'`). +11. `GET /v1/portfolios/{portfolio_id}/returns/summary` returns `net_contributed_capital=sum(INVEST)-sum(WITHDRAW)` and `cumulative_profit_withdrawn=sum(PROFIT)` separately. ## Flow: `v1 API — POST /auth/service-tokens` diff --git a/tests/test_bat025_portfolio_ledger_manual_metadata_migration.py b/tests/test_bat025_portfolio_ledger_manual_metadata_migration.py index dbaf8e9..4c40209 100644 --- a/tests/test_bat025_portfolio_ledger_manual_metadata_migration.py +++ b/tests/test_bat025_portfolio_ledger_manual_metadata_migration.py @@ -10,6 +10,7 @@ from __future__ import annotations import os from datetime import datetime, timezone +from decimal import Decimal from urllib.parse import unquote, urlparse from uuid import uuid4 @@ -271,3 +272,43 @@ def test_broker_and_manual_ledger_writes_regress_after_real_upgrade( ) assert cur.fetchone() == ("manual", "bat025-user", "synthetic-local-fixture") repo.close() + + +def test_dividend_precision_and_reversal_round_trip_on_real_postgres( + isolated_settings: Settings, +) -> None: + """DIVIDEND min unit persists and REVERSAL removes the same amount exactly.""" + upgrade_to_head(isolated_settings) + repo = _repo(isolated_settings) + portfolio_id = _seed_portfolio(repo, "dividend-precision") + + dividend = repo.record_manual_ledger( + portfolio_id, + action="DIVIDEND", + amount=Decimal("0.00000001"), + tax=Decimal("0.00000001"), + executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=timezone.utc), + note="precision-test", + ) + + assert dividend.price == 0.00000001 + assert dividend.tax_amount == 0.00000001 + assert dividend.profit_buffer_after == 0.00000001 + with repo._connect().cursor() as cur: + cur.execute( + "SELECT price::text, tax_amount::text, profit_buffer_after::text FROM portfolio_ledger WHERE ledger_entry_id = %s", + (dividend.ledger_entry_id,), + ) + assert cur.fetchone() == ("0.00000001", "0.00000001", "0.00000001") + + reversal = repo.record_manual_ledger( + portfolio_id, + action="REVERSAL", + target_ledger_entry_id=dividend.ledger_entry_id, + ) + assert reversal.profit_buffer_after == 0.0 + with repo._connect().cursor() as cur: + cur.execute("SELECT profit_buffer::text FROM portfolios WHERE portfolio_id = %s", (portfolio_id,)) + assert cur.fetchone()[0] == "0" + + repo.close() diff --git a/tests/test_v1_portfolios_manual_ledger.py b/tests/test_v1_portfolios_manual_ledger.py index b42aaf1..5a4ec67 100644 --- a/tests/test_v1_portfolios_manual_ledger.py +++ b/tests/test_v1_portfolios_manual_ledger.py @@ -500,6 +500,30 @@ def test_dividend_requires_executed_at_and_rejects_fee_field() -> None: assert invalid_fee.status_code == 422 +def test_dividend_rejects_sub_minimum_numeric_precision() -> None: + repo = _FakeRepo(_portfolio()) + client = _make_client(repo) + + too_precise = client.post( + f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual", + json={"action": "DIVIDEND", "amount": "0.000000004", "executed_at": "2026-09-01"}, + headers=_auth(), + ) + assert too_precise.status_code == 422 + + +def test_dividend_rejects_amount_exceeding_numeric_20_8() -> None: + repo = _FakeRepo(_portfolio()) + client = _make_client(repo) + + too_large = client.post( + f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual", + json={"action": "DIVIDEND", "amount": "1000000000000.00000000", "executed_at": "2026-09-01"}, + headers=_auth(), + ) + assert too_large.status_code == 422 + + def test_dividend_duplicate_requires_confirmation() -> None: repo = _FakeRepo(_portfolio()) client = _make_client(repo) @@ -571,12 +595,24 @@ def test_unified_ledger_includes_dividend_fields() -> None: item = resp.json()["items"][0] assert item["source"] == "manual" assert item["action"] == "DIVIDEND" + assert item["amount"] == 33.0 + assert item["price"] == 33.0 assert item["executed_at"] == "2026-09-01T00:00:00Z" assert item["tax_amount"] == 3.0 assert item["note"] == "Q3" assert item["profit_buffer_after"] == created.json()["profit_buffer_after"] +def test_unified_ledger_openapi_contains_nullable_amount() -> None: + repo = _FakeRepo(_portfolio()) + client = _make_client(repo) + + response = client.get("/openapi.json") + assert response.status_code == 200 + schemas = response.json()["components"]["schemas"] + assert "amount" in schemas["LedgerHistoryItem"]["properties"] + + def test_reversal_rejects_fee_tax_and_missing_target() -> None: repo = _FakeRepo(_portfolio()) client = _make_client(repo)