fix(analytics): alias per-symbol return columns to stop portfolio metrics collapse #84

Merged

gertjan merged 1 commit from fix/portfolio-returns-column-collision into main

2026-07-14 17:56:10 +00:00

gertjan commented

2026-07-14 17:44:23 +00:00

Owner

Summary

_build_portfolio_returns (src/analytics/portfolio.py) gave every per-symbol
weighted-return column the same literal name ("weighted_return") before
outer-joining the per-symbol frames together. That collision produced two
distinct silent failure modes depending on portfolio size:

2 symbols: polars auto-suffixes the second frame's colliding column to
"weighted_return_right" on join. The final
pl.sum_horizontal(["weighted_return"]) then only ever summed the first
symbol's column — the second symbol's returns were silently dropped and
portfolio metrics (Sharpe, drawdown, VaR, alpha/beta, total/annualised
return) collapsed to a single-symbol result with no error or warning.
3+ symbols: the second join collides again, and this time polars
raises polars.exceptions.DuplicateError instead of silently renaming.
PortfolioAnalytics.compute's broad except Exception swallowed that
error and returned all-None metrics with num_bars=0 — again with no
visible error, just an empty-looking analytics response.

Both failure modes affect GET /portfolios/{pid}/analytics
(src/api/routes/portfolios.py:666-766) and the simulation service
(src/api/services/simulation_service.py:209-214), for any portfolio holding
2+ symbols.

Fix

Alias each symbol's weighted-return column uniquely
(f"weighted_return__{symbol}") instead of reusing a shared literal name,
and collect the column names.
Join with how="full", coalesce=True (also incidentally silences the
polars 1.x deprecation warning for how="outer" — confirmed accepted by
this repo's pinned polars 1.41.2).
Sum with pl.sum_horizontal([pl.col(c).fill_null(0.0) for c in weighted_cols])
so a symbol's missing bar on a given date contributes 0 to that date's
portfolio return instead of nulling out the row or being silently ignored.
Upgraded PortfolioAnalytics.compute's broad except Exception to log
with exc_info=True (was previously only interpolating %s into the
message), so a future regression in this class of bug surfaces in logs
instead of quietly returning empty metrics.

Tests

Added tests/unit/test_analytics_portfolio.py (5 new tests), each
independently hand-computed (no dependency on the buggy code under test for
the expected values):

test_two_symbols_second_symbol_contribution_included — N=2, asserts the
second symbol's weighted return is present in the summed series (fails
against the old code: it returns the first symbol's series unchanged).
test_three_symbols_no_exception_and_correct_values — N=3, asserts no
exception and correct per-date portfolio_return (fails against the old
code with DuplicateError).
test_five_symbols_no_exception_and_correct_values — N=5, same shape.
test_three_symbols_full_compute_does_not_raise_and_returns_metrics —
integration-level check that PortfolioAnalytics.compute doesn't swallow
the join error into all-None metrics for N=3.
test_mixed_non_overlapping_dates_missing_contribution_is_zero — two
symbols with disjoint bar-date ranges; asserts each date's return equals
only the symbol that has a bar that day (missing symbol contributes 0, not
null, and the row isn't dropped).

Verified all 5 new tests fail against the pre-fix code exactly as described
above (git stash the fix, rerun — 5 failed: silent-drop assertion for N=2,
DuplicateError for N=3/5, num_bars == 0 for the compute() integration
test, missing-contribution assertion for the mixed-dates case), then pass
after the fix.

Full suite: ruff check src/, mypy src/ (207 files, no issues), and
pytest -m "not integration" (1067 passed, 0 failed) all green.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

## Summary `_build_portfolio_returns` (`src/analytics/portfolio.py`) gave every per-symbol weighted-return column the same literal name (`"weighted_return"`) before outer-joining the per-symbol frames together. That collision produced two distinct silent failure modes depending on portfolio size: - **2 symbols**: polars auto-suffixes the second frame's colliding column to `"weighted_return_right"` on join. The final `pl.sum_horizontal(["weighted_return"])` then only ever summed the first symbol's column — the second symbol's returns were silently dropped and portfolio metrics (Sharpe, drawdown, VaR, alpha/beta, total/annualised return) collapsed to a single-symbol result with no error or warning. - **3+ symbols**: the *second* join collides again, and this time polars raises `polars.exceptions.DuplicateError` instead of silently renaming. `PortfolioAnalytics.compute`'s broad `except Exception` swallowed that error and returned all-`None` metrics with `num_bars=0` — again with no visible error, just an empty-looking analytics response. Both failure modes affect `GET /portfolios/{pid}/analytics` (`src/api/routes/portfolios.py:666-766`) and the simulation service (`src/api/services/simulation_service.py:209-214`), for any portfolio holding 2+ symbols. ## Fix - Alias each symbol's weighted-return column uniquely (`f"weighted_return__{symbol}"`) instead of reusing a shared literal name, and collect the column names. - Join with `how="full", coalesce=True` (also incidentally silences the polars 1.x deprecation warning for `how="outer"` — confirmed accepted by this repo's pinned polars 1.41.2). - Sum with `pl.sum_horizontal([pl.col(c).fill_null(0.0) for c in weighted_cols])` so a symbol's missing bar on a given date contributes 0 to that date's portfolio return instead of nulling out the row or being silently ignored. - Upgraded `PortfolioAnalytics.compute`'s broad `except Exception` to log with `exc_info=True` (was previously only interpolating `%s` into the message), so a future regression in this class of bug surfaces in logs instead of quietly returning empty metrics. ## Tests Added `tests/unit/test_analytics_portfolio.py` (5 new tests), each independently hand-computed (no dependency on the buggy code under test for the expected values): - `test_two_symbols_second_symbol_contribution_included` — N=2, asserts the second symbol's weighted return is present in the summed series (fails against the old code: it returns the first symbol's series unchanged). - `test_three_symbols_no_exception_and_correct_values` — N=3, asserts no exception and correct per-date portfolio_return (fails against the old code with `DuplicateError`). - `test_five_symbols_no_exception_and_correct_values` — N=5, same shape. - `test_three_symbols_full_compute_does_not_raise_and_returns_metrics` — integration-level check that `PortfolioAnalytics.compute` doesn't swallow the join error into all-None metrics for N=3. - `test_mixed_non_overlapping_dates_missing_contribution_is_zero` — two symbols with disjoint bar-date ranges; asserts each date's return equals only the symbol that has a bar that day (missing symbol contributes 0, not null, and the row isn't dropped). Verified all 5 new tests fail against the pre-fix code exactly as described above (`git stash` the fix, rerun — 5 failed: silent-drop assertion for N=2, `DuplicateError` for N=3/5, `num_bars == 0` for the compute() integration test, missing-contribution assertion for the mixed-dates case), then pass after the fix. Full suite: `ruff check src/`, `mypy src/` (207 files, no issues), and `pytest -m "not integration"` (1067 passed, 0 failed) all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

gertjan added 1 commit

2026-07-14 17:44:23 +00:00

fix(analytics): alias per-symbol return columns to stop portfolio metrics collapse

Deploy / check (pull_request) Successful in 3m29s

Details

Deploy / deploy (pull_request) Has been skipped

Details

8cfe25c11e

_build_portfolio_returns named every per-symbol weighted-return column the
literal "weighted_return", then outer-joined the per-symbol frames. With 2
symbols polars silently suffixed the collision to "weighted_return_right"
and sum_horizontal(["weighted_return"]) only summed the first symbol,
silently collapsing portfolio metrics to a single-symbol result. With 3+
symbols the second join raised polars.exceptions.DuplicateError, which
PortfolioAnalytics.compute's broad except swallowed into all-None
metrics/num_bars=0 with no visible error.

Alias each symbol's column uniquely, join with how="full" (also silences
the polars 1.x "outer" deprecation warning), and fill_null(0.0) before
summing so a symbol's missing bar on a date contributes zero instead of
being dropped. Also upgrade compute()'s broad except to log with
exc_info so this class of bug can't hide silently again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>