From 5eb8c042668b1c383ac59ca9be00aa1a5dfcb37a Mon Sep 17 00:00:00 2001 From: Justin Ko Date: Wed, 19 Aug 2026 17:34:13 +0900 Subject: [PATCH] fix: aggregate investment totals across ledger sources --- app/repositories/postgres.py | 2 - app/schemas/v1.py | 24 +++-- docs/data-model.md | 9 ++ docs/interface-contract.md | 15 ++- docs/runtime-flows.md | 6 +- tests/test_portfolio_ledger_aggregates.py | 25 ++++- ..._ledger_aggregates_postgres_integration.py | 95 +++++++++++++++++++ 7 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 tests/test_portfolio_ledger_aggregates_postgres_integration.py diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index ce53690..e5dd12d 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -2223,13 +2223,11 @@ class PostgresPortfolioRepository: COALESCE(SUM(ledger.price) FILTER (WHERE ledger.side = 'profit'), 0) FROM portfolio_ledger AS ledger WHERE ledger.portfolio_id = ANY(%s) - AND ledger.source = 'manual' AND ledger.side IN ('invest', 'withdraw', 'profit') AND NOT EXISTS ( SELECT 1 FROM portfolio_ledger AS reversal WHERE reversal.portfolio_id = ledger.portfolio_id - AND reversal.source = 'manual' AND reversal.side = 'reversal' AND reversal.target_ledger_entry_id = ledger.ledger_entry_id ) diff --git a/app/schemas/v1.py b/app/schemas/v1.py index 8a4f3f4..779e0f3 100644 --- a/app/schemas/v1.py +++ b/app/schemas/v1.py @@ -222,10 +222,22 @@ class ManualLedgerHistoryResponse(BaseModel): class PortfolioReturnSummaryResponse(BaseModel): portfolio_id: str - total_invested: float = 0.0 - total_withdrawn: float = 0.0 - net_contributed_capital: float = 0.0 - cumulative_profit_withdrawn: float = 0.0 + total_invested: float = Field( + default=0.0, + description="총투자원금: source와 무관한 유효 INVEST의 gross 합계", + ) + total_withdrawn: float = Field( + default=0.0, + description="총회수원금: source와 무관한 유효 WITHDRAW 합계", + ) + net_contributed_capital: float = Field( + default=0.0, + description="순투자원금: total_invested - total_withdrawn (PROFIT 제외)", + ) + cumulative_profit_withdrawn: float = Field( + default=0.0, + description="누적 수익 인출액: source와 무관한 유효 PROFIT 합계 (원금 계산 제외)", + ) # --------------------------------------------------------------------------- @@ -273,11 +285,11 @@ class PortfolioItem(BaseModel): profit_buffer: float = 0.0 net_contributed_capital: float = Field( default=0.0, - description="순투자현금: 유효 manual INVEST 합계 - 유효 manual WITHDRAW 합계", + description="순투자원금: source와 무관한 유효 INVEST 합계 - 유효 WITHDRAW 합계 (PROFIT 제외)", ) cumulative_profit_withdrawn: float = Field( default=0.0, - description="누적 수익 인출액: 유효 manual PROFIT 합계", + description="누적 수익 인출액: source와 무관한 유효 PROFIT 합계 (원금 계산 제외)", ) tag_ids: list[str] = Field(default_factory=list) managed_by_daemon_id: str | None = None diff --git a/docs/data-model.md b/docs/data-model.md index db5cef9..0b02afb 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -343,6 +343,15 @@ - SELL: asset_quantity -> cash_balance. - REVERSAL: append-only inverse effect of `target_ledger_entry_id`. +### Return Aggregate Semantics + +- `total_invested`: source와 무관한 유효 `INVEST`의 gross 합계. +- `total_withdrawn`: source와 무관한 유효 `WITHDRAW` 합계. +- `net_contributed_capital`: `total_invested - total_withdrawn`; `PROFIT`는 원금 계산에서 제외한다. +- `cumulative_profit_withdrawn`: source와 무관한 유효 `PROFIT` 합계이며 원금과 별도로 노출한다. +- 동일 portfolio에서 matching `REVERSAL.target_ledger_entry_id`가 존재하면 original과 reversal의 source와 무관하게 original entry를 집계에서 제외한다. +- `buffer_out` 등 다른 accounting side를 `INVEST`로 재해석하지 않으며 existing ledger row를 mutation/backfill하지 않는다. + ### State Rules - Manual writes lock the portfolio row while calculating/appending/updating snapshots to avoid lost updates. diff --git a/docs/interface-contract.md b/docs/interface-contract.md index a7836c6..1889b3a 100644 --- a/docs/interface-contract.md +++ b/docs/interface-contract.md @@ -347,10 +347,13 @@ - Response: HTTP 200 `PortfolioListResponse {"portfolios": [{"portfolio_id": "", "asset_id": "", "symbol": "", "net_contributed_capital": , "cumulative_profit_withdrawn": , ...}], "total": }` - Note: list items are serialized from the server-side `portfolios JOIN assets` result; `symbol` must equal the joined `assets.symbol` for each returned `asset_id`. - Ledger aggregate fields: - - `net_contributed_capital` (**순투자현금**) = 유효 manual `INVEST` 합계 - 유효 manual `WITHDRAW` 합계. - - `cumulative_profit_withdrawn` (**누적 수익 인출액**) = 유효 manual `PROFIT` 합계. 매매상 실현손익 전체가 아니다. - - `REVERSAL.target_ledger_entry_id`로 참조된 `INVEST`, `WITHDRAW`, `PROFIT` 원거래는 유효 집계에서 제외하며 `REVERSAL` 자체는 금액 action으로 합산하지 않는다. - - 목록에 반환될 portfolio ID 집합만 한 번의 grouped aggregate로 조회한다. 해당 유효 manual ledger가 없는 항목은 두 필드 모두 `0.0`이다. + - `total_invested` (**총투자원금**, summary response) = source와 무관한 유효 `INVEST`의 gross 합계. + - `total_withdrawn` (**총회수원금**, summary response) = source와 무관한 유효 `WITHDRAW` 합계. + - `net_contributed_capital` (**순투자원금**) = `total_invested - total_withdrawn`. `PROFIT`는 원금 계산에서 제외한다. + - `cumulative_profit_withdrawn` (**누적 수익 인출액**) = source와 무관한 유효 `PROFIT` 합계. 원금 또는 매매상 실현손익 전체가 아니다. + - 동일 portfolio에서 matching `REVERSAL.target_ledger_entry_id`로 참조된 `INVEST`, `WITHDRAW`, `PROFIT` 원거래는 original과 reversal의 source와 무관하게 제외하며 `REVERSAL` 자체는 금액 action으로 합산하지 않는다. + - `buffer_out` 등 `INVEST`/`WITHDRAW`/`PROFIT` 이외 side는 원금·수익 aggregate에 포함하거나 `INVEST`로 재해석하지 않는다. + - 목록에 반환될 portfolio ID 집합만 한 번의 grouped aggregate로 조회한다. 해당 유효 ledger가 없는 항목은 aggregate field가 `0.0`이다. - 이 필드들은 portfolio-hub가 소유한 ledger 집계값이다. 소비자는 ledger event를 재해석하지 않는다. ### Endpoint: `GET /v1/portfolios/{portfolio_id}/tags` @@ -438,6 +441,9 @@ - Auth: owner-scoped user/session token. - Response: `PortfolioReturnSummaryResponse` with `total_invested`, `total_withdrawn`, `net_contributed_capital`, and `cumulative_profit_withdrawn`. +- Aggregate semantics: `total_invested`는 source-independent valid `INVEST`의 gross 합계, `total_withdrawn`은 valid `WITHDRAW` 합계, `net_contributed_capital`은 두 값의 차이며 `PROFIT`는 원금에서 제외하고 `cumulative_profit_withdrawn`으로만 노출한다. +- Reversal semantics: 동일 portfolio의 matching `REVERSAL.target_ledger_entry_id`는 original과 reversal의 source와 무관하게 original을 제외한다. `buffer_out` 등 다른 side는 집계하지 않는다. +- Consistency: `GET /v1/portfolios`와 동일한 grouped repository aggregate를 portfolio ID 하나에 위임한다. ## Interface: `Tag Management API (BAT-004)` @@ -562,6 +568,7 @@ ## Change Log +- 2026-08-19 KST: Issue #28 — portfolio return aggregate를 manual-only에서 source-independent semantics로 교정; gross `total_invested`, `INVEST-WITHDRAW` net, separate `PROFIT`, cross-source reversal과 non-capital side 제외를 명시. - 2026-07-14 KST: BAT-024 (REQ-056) — added owner-scoped manual ledger API, manual history, and return summary contracts; clarified PROFIT vs WITHDRAW return semantics and service-token separation from `/ledger/trades`. - 2026-05-21 KST: BAT-011 (REQ-029) — ledger path migration: "POST /v1/assets/{asset_id}/ledger/trades" 명세 → "POST /v1/portfolios/{portfolio_id}/ledger/trades"로 교체; auth 섹션 + daemon authorization 갱신; 구 path HTTP 404 명시. - 2026-05-21 KST: BAT-010 (REQ-032) — v1 portfolio surface baseline 회복: "v1 Portfolio API" 섹션 신규 추가 (REQ-025~028 CRUD + REQ-030 tag 서브 라우트 + REQ-031 comment CRUD); POST/DELETE /v1/portfolios/{id}/tags에 disabled 보호(409) 및 service/client 403 명세 추가; GET /v1/portfolios/{id}/tags 신설; ledger path "pending REQ-029" 마킹; Source Discovery/Batch 헤더 갱신. diff --git a/docs/runtime-flows.md b/docs/runtime-flows.md index da94565..7e0185e 100644 --- a/docs/runtime-flows.md +++ b/docs/runtime-flows.md @@ -152,9 +152,9 @@ 2. `service` token: `auth.daemon_id is None` → HTTP 403. `repo.list_portfolios(managed_by_daemon_id=auth.daemon_id, ...)`. 3. `user`/`admin` token: admin with `?user_id=` → filter by that user; else filter by `auth.user_id`. 4. `?include_disabled=false` (default) → `status="enabled"` filter applied. `?include_disabled=true` → no status filter. -5. 필터링된 반환 대상 portfolio ID만 `repo.get_return_summaries(ids)`에 전달한다. repository는 한 번의 grouped query로 유효 manual ledger를 집계한다. -6. `REVERSAL.target_ledger_entry_id`로 참조된 `INVEST`/`WITHDRAW`/`PROFIT`은 제외한다. `REVERSAL` 자체는 금액 action으로 합산하지 않는다. -7. 각 `PortfolioItem`에 순투자현금(`net_contributed_capital`)과 누적 수익 인출액(`cumulative_profit_withdrawn`)을 결합한다. 집계 row가 없으면 두 값은 `0.0`이다. +5. 필터링된 반환 대상 portfolio ID만 `repo.get_return_summaries(ids)`에 전달한다. repository는 한 번의 grouped query로 source와 무관한 유효 ledger를 side별 집계한다. +6. 동일 portfolio에서 matching `REVERSAL.target_ledger_entry_id`로 참조된 `INVEST`/`WITHDRAW`/`PROFIT`은 original과 reversal의 source와 무관하게 제외한다. `REVERSAL` 자체는 금액 action으로 합산하지 않는다. +7. `total_invested`는 유효 `INVEST`의 gross 합계, `total_withdrawn`은 유효 `WITHDRAW` 합계, `net_contributed_capital`은 두 값의 차이다. `PROFIT`는 원금 계산에서 제외하고 `cumulative_profit_withdrawn`으로만 집계한다. 집계 row가 없으면 값은 `0.0`이다. 8. Returns HTTP 200 `PortfolioListResponse`. 단건 `GET /v1/portfolios/{portfolio_id}/returns/summary`도 같은 grouped aggregate 구현을 portfolio ID 하나로 호출하므로 목록과 유효 원장 의미가 일치한다. diff --git a/tests/test_portfolio_ledger_aggregates.py b/tests/test_portfolio_ledger_aggregates.py index 455bd96..58747a3 100644 --- a/tests/test_portfolio_ledger_aggregates.py +++ b/tests/test_portfolio_ledger_aggregates.py @@ -1,9 +1,12 @@ """Repository contract tests for portfolio ledger return aggregates.""" +from pathlib import Path from unittest.mock import MagicMock, patch from app.repositories.postgres import PostgresPortfolioRepository +_ROOT = Path(__file__).parents[1] + def _repo_with_cursor(rows: list[tuple[object, ...]]): repo = PostgresPortfolioRepository.__new__(PostgresPortfolioRepository) @@ -29,7 +32,7 @@ def test_get_return_summaries_empty_ids_skips_database() -> None: repo.ensure_schema.assert_not_called() -def test_get_return_summaries_groups_portfolios_and_excludes_reversed_sources() -> None: +def test_get_return_summaries_groups_all_sources_and_excludes_reversed_entries() -> None: repo, connection, cursor = _repo_with_cursor( [ ("p-001", 500.0, 125.0, 40.0), @@ -60,8 +63,12 @@ def test_get_return_summaries_groups_portfolios_and_excludes_reversed_sources() assert "group by ledger.portfolio_id" in normalized_sql assert "not exists" in normalized_sql assert "reversal.target_ledger_entry_id = ledger.ledger_entry_id" in normalized_sql - assert "ledger.source = 'manual'" in normalized_sql + assert "ledger.source = 'manual'" not in normalized_sql + assert "reversal.source = 'manual'" not in normalized_sql assert "ledger.side in ('invest', 'withdraw', 'profit')" in normalized_sql + assert "sum(ledger.price) filter (where ledger.side = 'invest')" in normalized_sql + assert "sum(ledger.price) filter (where ledger.side = 'withdraw')" in normalized_sql + assert "sum(ledger.price) filter (where ledger.side = 'profit')" in normalized_sql assert params == (["p-001", "p-002"],) @@ -76,4 +83,16 @@ def test_get_return_summary_delegates_to_shared_batch_semantics() -> None: repo.get_return_summaries = MagicMock(return_value={"p-001": expected}) assert repo.get_return_summary("p-001") == expected - repo.get_return_summaries.assert_called_once_with(["p-001"]) \ No newline at end of file + repo.get_return_summaries.assert_called_once_with(["p-001"]) + + +def test_external_contract_documents_all_source_return_semantics() -> None: + contract = (_ROOT / "docs/interface-contract.md").read_text() + + assert "source와 무관한 유효 `INVEST`의 gross 합계" in contract + assert "`net_contributed_capital` (**순투자원금**) = `total_invested - total_withdrawn`" in contract + assert "`PROFIT`는 원금 계산에서 제외" in contract + assert "original과 reversal의 source와 무관하게 제외" in contract + assert "`buffer_out` 등 `INVEST`/`WITHDRAW`/`PROFIT` 이외 side" in contract + assert "유효 manual `INVEST` 합계" not in contract + assert "유효 manual `PROFIT` 합계" not in contract \ No newline at end of file diff --git a/tests/test_portfolio_ledger_aggregates_postgres_integration.py b/tests/test_portfolio_ledger_aggregates_postgres_integration.py new file mode 100644 index 0000000..740e879 --- /dev/null +++ b/tests/test_portfolio_ledger_aggregates_postgres_integration.py @@ -0,0 +1,95 @@ +"""Real PostgreSQL semantics for all-source portfolio return aggregates. + +Run with PORTFOLIOHUB_RUN_LEDGER_AGGREGATE_POSTGRES=1 against a disposable/local +PostgreSQL endpoint. Normal unit-test environments skip this module. +""" + +from __future__ import annotations + +import os +from unittest.mock import MagicMock + +import psycopg +import pytest + +from app.repositories.postgres import PostgresPortfolioRepository + +pytestmark = pytest.mark.skipif( + os.getenv("PORTFOLIOHUB_RUN_LEDGER_AGGREGATE_POSTGRES") != "1", + reason="set PORTFOLIOHUB_RUN_LEDGER_AGGREGATE_POSTGRES=1 to run real PostgreSQL tests", +) + + +def _connect() -> psycopg.Connection: + return psycopg.connect( + host=os.getenv("TIMESCALEDB_HOST", "127.0.0.1"), + port=int(os.getenv("TIMESCALEDB_PORT", "5432")), + dbname=os.getenv("TIMESCALEDB_NAME", "portfolio_hub_migration_test"), + user=os.getenv("TIMESCALEDB_USER", "portfolio_hub_user"), + password=os.getenv("TIMESCALEDB_PASSWORD", "change-me"), + sslmode=os.getenv("TIMESCALEDB_SSLMODE", "prefer"), + ) + + +def test_all_sources_mixed_sides_and_cross_source_reversals() -> None: + connection = _connect() + try: + with connection.cursor() as cursor: + cursor.execute( + """ + CREATE TEMP TABLE portfolio_ledger ( + ledger_entry_id BIGINT PRIMARY KEY, + portfolio_id TEXT NOT NULL, + price DOUBLE PRECISION NOT NULL, + source TEXT NOT NULL, + side TEXT NOT NULL, + target_ledger_entry_id BIGINT + ) + """ + ) + cursor.executemany( + """ + INSERT INTO portfolio_ledger ( + ledger_entry_id, portfolio_id, price, source, side, target_ledger_entry_id + ) VALUES (%s, %s, %s, %s, %s, %s) + """, + [ + (1, "broker-only", 700, "broker", "invest", None), + (2, "mixed", 1000, "manual", "invest", None), + (3, "mixed", 500, "broker", "invest", None), + (4, "mixed", 200, "broker", "withdraw", None), + (5, "mixed", 50, "manual", "profit", None), + # A manual reversal excludes a broker original. + (6, "mixed", 300, "broker", "invest", None), + (7, "mixed", 300, "manual", "reversal", 6), + # A broker reversal excludes a manual original. + (8, "mixed", 100, "manual", "withdraw", None), + (9, "mixed", 100, "broker", "reversal", 8), + # Non-capital sides are never inferred as INVEST. + (10, "mixed", 800_000, "broker", "buffer_out", None), + ], + ) + + repo = PostgresPortfolioRepository.__new__(PostgresPortfolioRepository) + repo.ensure_schema = MagicMock() + repo._connect = MagicMock(return_value=connection) + + summaries = repo.get_return_summaries(["broker-only", "mixed"]) + + assert summaries == { + "broker-only": { + "total_invested": 700.0, + "total_withdrawn": 0.0, + "net_contributed_capital": 700.0, + "cumulative_profit_withdrawn": 0.0, + }, + "mixed": { + "total_invested": 1500.0, + "total_withdrawn": 200.0, + "net_contributed_capital": 1300.0, + "cumulative_profit_withdrawn": 50.0, + }, + } + repo.ensure_schema.assert_called_once_with() + finally: + connection.close()