fix: 코드 감사 배치 2 — 포트폴리오 계산 정확성·오류 응답 계약 - #51
Merged
Merged
Conversation
상장폐지 종목은 거래 없이 보유만 유지되지만 그 가치는 total_portfolio_value에 포함된다. 거래 가능 종목의 목표 비중을 이 전체 금액에 곱해 배분하면서, 상장폐지 보유분이 한 번 더 더해져 리밸런싱마다 포트폴리오 총액이 그 가치만큼 부풀었다. 2종목·수수료 0 기준으로 총액 $200 → $300(+50%), 3종목·수수료 2% 기준으로는 수수료까지 $0.10 대신 $1.10(11배)로 계산됐다. 재분배 대상 풀에서 상장폐지 가치를 제외하도록 수정하고, "리밸런싱 전후 총액은 수수료만큼만 감소한다"는 불변식으로 검증하는 테스트 5개를 추가했다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
주석은 forward fill이라고 하지만 실제로는 백테스트 종료 시점 값인 final_value를 중간 날짜에 주입하고 있었다. 한국·미국 혼합 포트폴리오처럼 시장별 휴장일이 어긋나는 경우 중간일마다 미래 값이 새어 들어가 스파이크가 생기고(+36% 뒤 -26% 반전 확인), 여기서 파생되는 Annual_Volatility·Profit_Factor·Positive/Negative_Days 통계가 오염됐다. 종목별 마지막 관측값을 추적하는 진짜 forward fill로 교체했다. 첫 관측 이전 구간은 기존대로 초기 투자금을 사용하며, 데이터가 첫날보다 늦게 시작하는 종목도 동일하게 처리된다. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PortfolioManagerService의 catch-all 3곳이 모든 예외를
{'status':'error','error': str(e)} 딕셔너리로 바꾸고, 엔드포인트가
이를 그대로 반환했다. 그 결과 @handle_portfolio_errors의 4xx/5xx
매핑과 불투명 에러 ID 체계가 전부 무력화됐고, 내부 예외 문자열(경로,
DB 접속 정보 등)이 성공 상태 코드와 함께 클라이언트로 나갔다.
catch-all은 로깅 후 재발생(raise)으로 바꾸고 엔드포인트의 통과
분기를 제거해, 데코레이터가 상태 코드를 결정하도록 되돌렸다.
성공 응답 형태는 변경하지 않았다.
통합 테스트 test_invalid_strategy_returns_error는 과거의 잘못된
계약(200 + status="error")을 고정하고 있어 422 기대로 수정했다.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
죽어 있던 extractErrorMessage는 FastAPI가 보내지 않는
data.message/data.error를 확인하고 있었다. 실제 응답 형식인 detail
(문자열 및 Pydantic 검증 배열)을 처리하도록 다시 구현하고 export해,
훅과 폼이 같은 추출 로직을 공유하게 했다.
에러 표시는 페이지 레벨 Alert 하나로 통일했다. 기존에는 훅이 일반
axios 메시지("Request failed with status code 4xx")를 Alert에 남기고
폼이 같은 에러의 detail을 모달로 띄워, 모달을 닫으면 쓸모없는 문자열만
남았다. 폼 모달은 제출 전 클라이언트 검증 전용으로 축소했다.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P1-06(상장폐지 리밸런싱), P1-09/P2-29(오류 계약), P1-10(equity curve) 완료 처리하고 각각 측정값·검증 내용을 기록. P1-06 수정 중 발견한 후속 항목 2건(P3-27 수수료 축소가 상장폐지 주식 수까지 감소, P3-28 미매칭 현금 유실) 추가. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
백테스트 감사 배치 2의 P1 이슈를 해결해 포트폴리오 계산 정확성(리밸런싱/Equity curve) 을 개선하고, 실패 응답이 HTTP 200으로 반환되던 오류 계약을 정상화하는 PR입니다. FE에서는 FastAPI의 실제 에러 포맷(detail)을 일관되게 노출하도록 에러 메시지 추출/표시를 정리합니다.
Changes:
- [BE] 상장폐지 종목 가치가 리밸런싱 시 거래 가능 자산 풀에 중복 반영되던 문제를 수정
- [BE] 전략 포트폴리오 equity curve 갭 처리에서
final_value가 중간 날짜에 주입되던 문제를 “진짜 forward fill”로 교체 - [BE/FE] catch-all 에러 dict(200 응답) 경로 제거 → 예외 전파 + 데코레이터 매핑(422/404/500)으로 계약 복원, FE는
detail기반 에러 메시지로 표면 통일
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | 감사 백로그 상태 업데이트 및 후속 이슈(P3-27/P3-28) 기록 |
| backtest_fe/src/shared/api/client.ts | extractErrorMessage를 detail 기반으로 재구현해 공용화 |
| backtest_fe/src/shared/api/tests/extractErrorMessage.test.ts | extractErrorMessage 회귀 테스트 추가(MSW로 AxiosError 재현) |
| backtest_fe/src/pages/PortfolioPage.tsx | Alert에서 멀티라인(detail 배열 합친 메시지) 줄바꿈 보존 |
| backtest_fe/src/features/backtest/hooks/usePortfolioBacktest.ts | 훅에서 extractErrorMessage 사용해 실제 detail 메시지 노출 |
| backtest_fe/src/features/backtest/hooks/tests/usePortfolioBacktest.test.ts | 훅 에러 노출 회귀 테스트 추가 |
| backtest_fe/src/features/backtest/components/PortfolioBacktestForm.tsx | 백엔드 에러 모달 중복 표시 제거(페이지 Alert로 단일화) |
| backtest_be_fast/app/services/portfolio/portfolio_rebalancer.py | 상장폐지 가치 제외한 allocatable_pool_value로 목표값 계산 |
| backtest_be_fast/app/services/portfolio_calculator_service.py | final_value 조기 주입 제거, last-seen 기반 forward fill 구현 |
| backtest_be_fast/app/services/portfolio_manager_service.py | 서비스 레벨 catch-all 제거/재-raise로 에러 계약 복원 |
| backtest_be_fast/app/api/v1/endpoints/backtest.py | 에러 dict 통과 분기 제거(예외 전파를 전제로 정리) |
| backtest_be_fast/tests/unit/test_rebalancer_delisted.py | P1-06 리밸런싱 불변식(총액 보존) 회귀 테스트 추가 |
| backtest_be_fast/tests/unit/test_portfolio_calculator_equity_curve.py | equity curve forward fill 회귀 테스트 추가 |
| backtest_be_fast/tests/unit/test_portfolio_backtest_error_contract.py | 에러 계약(500/422/404, 내부 문자열 비노출) 회귀 테스트 추가 |
| backtest_be_fast/tests/integration/test_backtest_api.py | invalid strategy가 200이 아닌 422를 기대하도록 수정 |
Comment on lines
+221
to
+223
| # 상장폐지 종목도 수수료 비례 축소 계수가 다른 자산과 동일하게 적용되어야 함 | ||
| expected_scale = (pre_total - result['commission_cost']) / pre_total | ||
| assert result['updated_shares']['B'] == pytest.approx(shares['B'] * expected_scale) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
배치 1(#50)에 이어, 감사에서 확인된 P1 항목 중 구조 변경이 필요한 것들을 처리했습니다. 전체 백로그는
TODO.md를 참고하세요.리밸런싱 시 자산이 복제되던 문제
상장폐지 종목은 거래 없이 보유만 유지되지만 그 가치는
total_portfolio_value에 포함됩니다. 거래 가능 종목의 목표 비중(합계 1.0으로 재정규화됨)을 이 전체 금액에 곱해 배분하면서, 상장폐지 보유분이 한 번 더 더해지고 있었습니다.측정된 왜곡:
리밸런싱 주기마다 반복되므로 장기 백테스트에서 복리로 누적됩니다. 재분배 대상 풀에서 상장폐지 가치를 제외하도록 수정하고, "리밸런싱 전후 총액은 수수료만큼만 감소한다"는 불변식으로 검증했습니다.
equity curve에 미래 값이 새어 들어가던 문제
주석은 forward fill이라고 하지만 실제로는 백테스트 종료 시점 값(
final_value)을 중간 날짜의 빈 칸에 채우고 있었습니다. 한국·미국 혼합 포트폴리오처럼 시장별 휴장일이 어긋나면 중간일마다 미래 값이 주입돼 스파이크가 생깁니다(+36% 뒤 −26% 반전 확인). 이 값들은daily_returns를 거쳐Annual_Volatility,Profit_Factor,Positive/Negative_Days통계로 그대로 전파됩니다.종목별 마지막 관측값을 추적하는 진짜 forward fill로 교체했습니다. 첫 관측 이전 구간은 기존대로 초기 투자금을 쓰며, 데이터가 첫날보다 늦게 시작하는 종목도 동일하게 처리됩니다. 이 메서드가 사장 코드가 아니라 실제 통계 산출 경로(
portfolio_manager_service.py:422)에 연결돼 있음을 확인했습니다.실패가 HTTP 200으로 반환되던 오류 계약
PortfolioManagerService의 catch-all 3곳이 모든 예외를{'status':'error','error': str(e)}딕셔너리로 바꾸고 엔드포인트가 이를 그대로 반환했습니다. 그 결과:@handle_portfolio_errors의 4xx/5xx 매핑과 불투명 에러 ID 체계가 완전히 무력화catch-all은 로깅 후 재발생으로 바꾸고 엔드포인트의 통과 분기를 제거했습니다. 검증: 일반 예외 → 500(유출 문자열 부재, 불투명 에러 ID 존재), ValidationError → 422, DataNotFoundError → 404, 성공 응답 형태는 불변.
프론트엔드에서는 같은 에러가 두 곳에 다르게 표시되던 문제를 함께 정리했습니다. 훅은 일반 axios 메시지("Request failed with status code 4xx")를 페이지 Alert에, 폼은 실제
detail을 모달에 띄워서 모달을 닫으면 쓸모없는 문자열만 남았습니다. 죽어 있던extractErrorMessage(FastAPI가 보내지 않는data.message/data.error를 확인)를detail기반으로 재구현해 공유하고, 표시는 페이지 Alert 하나로 통일했습니다.검증
모든 수정은 TDD로 진행했습니다 — 실패 테스트를 먼저 쓰고 예상한 틀린 숫자를 확인한 뒤 고쳤습니다.
docker build --target testBE·FE 재현 통과통합 테스트
test_invalid_strategy_returns_error는 과거의 잘못된 계약(200 +status="error")을 고정하고 있어 422 기대로 수정했습니다.이 PR에 없는 것
fix/audit-batch3브랜치에 보관돼 있으며 별도로 진행합니다.리밸런싱 수정 중 발견한 후속 항목 2건(P3-27 수수료 축소가 상장폐지 주식 수까지 감소, P3-28 미매칭 현금 유실)은
TODO.md에 기록했습니다.🤖 Generated with Claude Code