genichinandClaude Opus 4.8
2a19b7f058
BAT-016's initial implementation passed only against mocked repositories and broke on a real database. Two defects fixed: 1. insert_ledger_trade used `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL` but portfolio_ledger is a TimescaleDB hypertable, which cannot have that partial unique index (must include the partitioning column recorded_at). The statement errored against Postgres. Replaced with application-level idempotency: SELECT-before-insert inside the existing `SELECT ... FOR UPDATE` portfolio row lock, which serializes concurrent same-portfolio writes (race-safe). Duplicate returns current authoritative portfolio state (dcy-006 OQ-4 b). 2. `with self._connect() as conn:` closes the shared psycopg3 connection on exit, leaving the reused repository connection unusable. Switched to `with conn.transaction():` (commits without closing). Added tests/test_ledger_consolidation_integration.py — real-Postgres integration tests (new / duplicate / negative-balance) that close the mock-only verification gap. Full suite: 376 passed. SDLC: REQ-036/037 CHG-20260605-03 reconcile OQ-7 (ii) (DB partial unique → application-level idempotency) since the hypertable makes the DB constraint infeasible. REQ-036/037/038 → Implemented; BAT-016 → release-candidate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
64 lines
5.3 KiB
Markdown
64 lines
5.3 KiB
Markdown
# BAT-016 Implementation
|
|
|
|
- Status: implemented
|
|
- Batch: BAT-016
|
|
|
|
## Plan Summary
|
|
- 순서: REQ-036(마이그레이션 + ensure_schema) → REQ-037(repository/도메인/응답/회계) → REQ-038(deprecated)
|
|
- 각 REQ 후 부분 회귀, batch 마지막 통합 회귀를 수행했다.
|
|
|
|
## Changed Files
|
|
- `migrations/versions/20260605_0025_ledger_consolidation.py`
|
|
- `app/repositories/postgres.py`
|
|
- `app/repositories/portfolio_repository.py`
|
|
- `app/domain/models.py`
|
|
- `app/domain/services.py` call-site compatible model usage retained through `LedgerEntry.action` compatibility
|
|
- `app/schemas/v1.py`
|
|
- `app/api/v1_portfolios.py`
|
|
- `app/api/portfolios.py`
|
|
- `tests/test_v1_portfolios_ledger.py`
|
|
- `tests/test_v1.py`
|
|
- `tests/test_daemon_assignment.py`
|
|
- `tests/test_auth.py`
|
|
|
|
## Execution Log
|
|
1. `portfolio_ledger` schema was consolidated to carry trade-write identifiers/state directly: `ledger_entry_id`, `order_id`, `execution_id`, `side`, `executed_at`, `cash_balance_after`, and `asset_quantity_after`.
|
|
2. Legacy `ledger_trades` references were removed from application code; `ensure_schema()` now migrates old `action` data into `side`, drops `action`, and drops `ledger_trades` when present.
|
|
3. `LedgerEntry` became the canonical domain object for recent trades and authoritative ledger-trade writes.
|
|
4. `POST /v1/portfolios/{portfolio_id}/ledger/trades` now writes to `portfolio_ledger`, supports idempotent lookup by broker identifiers, computes cash/security balances, and returns accounting state.
|
|
5. Legacy `/portfolios/{portfolio_id}/buy` and `/portfolios/{portfolio_id}/sell` routes were marked `deprecated=True` and emit `Deprecation: true` on legacy responses/errors.
|
|
6. Regression tests were updated to assert the consolidated model and deprecation header behavior.
|
|
|
|
## Validation
|
|
- Targeted regression:
|
|
- `.venv/bin/python -m pytest -q tests/test_domain_services.py tests/test_v1_portfolios_ledger.py tests/test_daemon_assignment.py tests/test_v1.py`
|
|
- Result: `47 passed in 3.66s`
|
|
- Full suite:
|
|
- `.venv/bin/python -m pytest -q`
|
|
- Result: `373 passed, 3 warnings in 14.78s`
|
|
|
|
## Rework (2026-06-05, post-verification defect fix)
|
|
초기 구현이 mock-only 검증을 통과했으나 실 DB에서 신 path가 깨지는 결함 2건이 verify 단계에서 발견되어 재구현했다.
|
|
|
|
### 결함 1 — hypertable에서 불가능한 `ON CONFLICT` (치명)
|
|
- `insert_ledger_trade`가 `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL DO NOTHING`를 사용했으나, `portfolio_ledger`는 TimescaleDB hypertable(파티션 컬럼 `recorded_at`)이라 해당 partial unique index를 만들 수 없다(`cannot create a unique index without the column "recorded_at"`). 인덱스가 없으니 ON CONFLICT는 실 DB에서 `there is no unique or exclusion constraint matching the ON CONFLICT specification` 에러. 모든 ledger 테스트가 MagicMock repo라 실 SQL이 실행되지 않아 미검출.
|
|
- 수정: ON CONFLICT 제거. 기존 `SELECT ... FOR UPDATE` 락 안에서 **application-level idempotency(SELECT-before-insert)**로 교체. broker write(order_id 존재)만 `(portfolio_id, order_id, execution_id)`로 기존 row 조회 후 있으면 duplicate 처리, 없으면 INSERT. portfolios row lock이 동시 write를 직렬화하므로 race-safe. duplicate는 OQ-4(b)에 따라 현재 권위 portfolio 상태를 반환.
|
|
- 설계 정합: dcy-006 OQ-7 (ii) "partial UNIQUE index"는 hypertable에서 불가 → REQ-036/REQ-037 change-req(CHG-20260605-03)로 application-level idempotency로 재정의.
|
|
|
|
### 결함 2 — 공유 connection close (잠재)
|
|
- `with self._connect() as conn:`는 psycopg3에서 블록 종료 시 connection을 **close**한다. repo는 connection을 재사용(self._connection)하므로 insert_ledger_trade 후 repo가 unusable(다음 호출 `the connection is closed`). 앱은 request당 repo를 새로 만들어 가렸지만 잠재 결함.
|
|
- 수정: `with conn.transaction():`(commit하되 connection 유지)로 교체. repo 다른 메서드들과 connection 수명 일관.
|
|
|
|
### 실 DB 검증 추가
|
|
- mock-only 검증의 맹점을 닫기 위해 `tests/test_ledger_consolidation_integration.py` 신규(실 Postgres 의존, DB 미가용 시 skip). new(회계+portfolio 갱신) / duplicate(재insert 없음 + 현 권위 상태) / 음수 허용을 실 DB에서 검증.
|
|
|
|
## Validation (rework)
|
|
- mock 회귀: `tests/test_v1_portfolios_ledger.py tests/test_daemon_assignment.py tests/test_domain_services.py` → 24 passed
|
|
- 실 DB 통합: `tests/test_ledger_consolidation_integration.py` → 3 passed (실 Postgres)
|
|
- 전체: `.venv/bin/python -m pytest -q` → **376 passed** (기존 373 + 통합 3)
|
|
- 실 DB 직접 검증: 수정 전 `ON CONFLICT ... WHERE order_id IS NOT NULL` → `there is no unique or exclusion constraint matching...` 에러 재현 확인. 수정 후 통합 테스트로 정상 동작 확인.
|
|
|
|
## Remaining Risks
|
|
- alembic 마이그레이션(`20260605_0025`)은 원본 portfolio_ledger 스키마(action/Float 컬럼) 기준으로 작성되어 fresh DB(alembic 경로)에서 1회 정상 적용된다. 단 ensure_schema가 이미 실행된 개발 DB에서는 컬럼 중복으로 충돌 — ensure_schema vs alembic 이중 경로의 기존 패턴. prod 적용은 release staging에서 별도 검증(workspace_purpose 메모리).
|
|
- `ledger_entry_id`는 hypertable 제약상 단독 PRIMARY KEY 불가 → Identity 컬럼(비-PK)으로 유지. REQ-036 AC-1 change-req로 정합화.
|