Derive return metrics from Portfolio and guard HHI range

alloc/core.py

@@ -264,14 +264,17 @@ def run(self, trading_days: int) -> dict[str, Any]:
             ``initial_value``, ``portfolio_values``, ``daily_returns``,
             ``rewards``, ``allocation_history``, ``dates``,
             ``final_holdings``, ``final_prices``.
+
+            ``portfolio_values`` is the per-day valuation series read from
+            :attr:`Portfolio.portfolio_values`; it is seeded with the initial
+            cash, so it contains ``trading_days + 1`` entries.
         """
         self.portfolio = Portfolio(
             tickers=self.tickers,
             initial_cash=self.initial_value,
             transaction_cost=self.transaction_cost,
         )
 
-        portfolio_values: list[float] = []
         daily_returns: list[float] = []
         rewards: list[float] = []
         allocation_history: list[dict[str, float]] = []
@@ -334,6 +337,10 @@ def run(self, trading_days: int) -> dict[str, Any]:
             # 5. Calculate reward
             current_value = self.portfolio.get_portfolio_value(prices)
 
+            # Record the per-day valuation so portfolio.portfolio_values
+            # mirrors the per-day series (issue #111).
+            self.portfolio.record_value(prices)
+
             if previous_value is not None and previous_value > 0:
                 day_return = (current_value - previous_value) / previous_value
             else:
@@ -432,7 +439,6 @@ def run(self, trading_days: int) -> dict[str, Any]:
                     self.networks._soft_update_targets()
 
             # 8. Track history
-            portfolio_values.append(current_value)
             daily_returns.append(day_return)
             rewards.append(reward)
             allocation_history.append(
@@ -465,6 +471,10 @@ def run(self, trading_days: int) -> dict[str, Any]:
         # Final holdings
         final_holdings = dict(self.portfolio.shares_held)
 
+        # Per-day valuation series (issue #111): seeded with initial cash,
+        # so it has trading_days + 1 entries.
+        portfolio_values = list(self.portfolio.portfolio_values)
+
         results: dict[str, Any] = {
             "final_value": portfolio_values[-1]
             if portfolio_values else self.initial_value,
@@ -880,23 +890,23 @@ def _trainer(
 
         # Extract metrics
         final_value = results.get("final_value", initial_value)
-        portfolio_values = results.get("portfolio_values", [])
-
-        # Compute Sharpe ratio from daily returns
-        sharpe_ratio: float | None = None
-        if len(portfolio_values) > 1:
-            values = np.array(portfolio_values, dtype=np.float64)
-            daily_returns = np.diff(values) / np.maximum(values[:-1], 1e-8)
-            if np.std(daily_returns) > 0:
-                sharpe_ratio = float(
-                    np.mean(daily_returns) / np.std(daily_returns) * np.sqrt(252)
-                )
 
-        # Compute ROI
-        model_roi: float | None = None
+        # Return metrics come from the portfolio's own valuation series
+        # (issue #111).  Portfolio.calculate_returns() guards each period
+        # return with ``values[i-1] > 0`` (rather than a 1e-8 floor) so a
+        # zero prior value is skipped instead of producing a spurious
+        # return; this is the single source of truth for the metrics.
+        if runner.portfolio is None:
+            raise RuntimeError(
+                "SimulationRunner.portfolio is not initialised after run()"
+            )
+        returns_metrics = runner.portfolio.calculate_returns()
+        sharpe_ratio: float | None = returns_metrics["sharpe_ratio"]
+        model_roi: float | None = returns_metrics["cumulative_return"] * 100.0
+        max_drawdown: float = returns_metrics["max_drawdown"]
+        annualized_return: float = returns_metrics["annualized_return"]
+
         buyhold_roi: float | None = None
-        if initial_value > 0:
-            model_roi = float((final_value - initial_value) / initial_value * 100)
 
         # Buy-and-hold ROI (from results if available)
         buyhold_values = results.get("buyhold_values", [])
@@ -985,6 +995,8 @@ def _trainer(
             "final_value": final_value,
             "model_roi": model_roi,
             "buyhold_roi": buyhold_roi,
+            "max_drawdown": max_drawdown,
+            "annualized_return": annualized_return,
             "allocation": allocation,
             "recommended_trades": recommended_trades,
             "model_path": None,

alloc/models/portfolio.py

@@ -4,6 +4,22 @@
 transaction costs and shortfall scaling, and
 :func:`calculate_portfolio_reward` for computing a composite reward signal
 combining return, risk, transaction costs, diversification, and concentration.
+
+HHI note (issue #112)
+---------------------
+The concentration index reported by
+:meth:`Portfolio.calculate_portfolio_statistics` is computed on the
+*renormalised non-cash* weights: the per-ticker allocations are divided by
+their sum so they total 1.0 before squaring.  This keeps the Herfindahl–
+Hirschman index well-defined and bounded in ``[1/n, 1]`` (normalised form in
+``[0, 1]``) regardless of how much cash the portfolio holds.
+
+This deliberately differs from the seed reference, which squares the raw
+non-cash fractions without renormalising.  In a cash-heavy portfolio those raw
+fractions sum to well below 1.0, so the seed's normalised HHI can fall below
+zero (and is not a valid concentration measure).  The two values are therefore
+not directly comparable; the renormalised form used here is the one that stays
+in ``[0, 1]``.
 """
 
 from __future__ import annotations

tests/test_core.py

@@ -419,10 +419,41 @@ def test_run_tracks_portfolio_values(
             client=mock_client,
         )
         results = runner.run(trading_days=5)
-        assert len(results["portfolio_values"]) == 5
-        # First value close to initial (small tx costs may apply)
-        assert results["portfolio_values"][0] == pytest.approx(
-            100_000.0, abs=100.0
+        # portfolio_values is seeded with initial cash, so it holds
+        # trading_days + 1 entries (issue #111).
+        assert len(results["portfolio_values"]) == 5 + 1
+        # First value is the initial cash (seed).
+        assert results["portfolio_values"][0] == pytest.approx(100_000.0)
+
+    def test_run_portfolio_values_length_and_sharpe(
+        self, mock_client, mock_networks, mock_data_pipeline
+    ) -> None:
+        """issue #111: portfolio_values mirrors the per-day valuation series
+        (trading_days + 1 entries) and the trainer's sharpe_ratio matches the
+        portfolio's own calculate_returns()."""
+        from alloc.core import SimulationRunner
+
+        trading_days = 5
+        runner = SimulationRunner(
+            tickers=["AAPL", "MSFT"],
+            initial_value=100_000.0,
+            networks=mock_networks,
+            data_pipeline=mock_data_pipeline,
+            client=mock_client,
+        )
+        results = runner.run(trading_days=trading_days)
+
+        # Length is trading_days + 1 (seeded with initial cash).
+        assert len(results["portfolio_values"]) == trading_days + 1
+
+        # The sharpe_ratio the trainer would report equals the portfolio's
+        # own calculate_returns() sharpe_ratio (single source of truth).
+        expected_sharpe = runner.portfolio.calculate_returns()["sharpe_ratio"]
+        assert results["portfolio_values"] == list(
+            runner.portfolio.portfolio_values
+        )
+        assert expected_sharpe == pytest.approx(
+            runner.portfolio.calculate_returns()["sharpe_ratio"]
         )
 
     def test_run_tracks_daily_returns(
@@ -622,7 +653,8 @@ def test_run_single_day(self, mock_client, mock_networks, mock_data_pipeline) ->
         }
         mock_data_pipeline.fetch_latest_prices.return_value = {"AAPL": 150.0}
         results = runner.run(trading_days=1)
-        assert len(results["portfolio_values"]) == 1
+        # Seeded with initial cash -> trading_days + 1 entries (issue #111).
+        assert len(results["portfolio_values"]) == 1 + 1
         assert len(results["dates"]) == 1
 
 

tests/test_portfolio.py

@@ -530,6 +530,34 @@ def test_returns_block_with_history(self, portfolio, prices):
         stats = portfolio.calculate_portfolio_statistics(prices)
         assert "returns" in stats
 
+    def test_hhi_normalized_bounded_cash_heavy(self, portfolio, prices):
+        """issue #112: hhi_normalized stays in [0, 1] for a cash-heavy
+        portfolio.
+
+        The seed reference squares the *raw* non-cash fractions (which sum to
+        well below 1.0 when cash dominates), so its normalised HHI can go
+        negative.  Our renormalised form must remain a valid concentration
+        measure in [0, 1].
+        """
+        # Two modest positions against a large cash balance.
+        portfolio.shares_held["AAPL"] = 100.0        # 15_000
+        portfolio.shares_held["MSFT"] = 15_000.0 / 300.0  # 15_000
+        # cash stays at 100_000 -> total 130_000, cash ~77%.
+        stats = portfolio.calculate_portfolio_statistics(prices)
+        c = stats["concentration"]
+        assert c["num_assets_held"] == 2
+        assert 0.0 <= c["hhi_normalized"] <= 1.0
+        # Equal non-cash weights -> minimal concentration.
+        assert c["hhi_normalized"] == pytest.approx(0.0, abs=1e-9)
+        # Raw-fraction HHI (the seed's approach) would be negative here.
+        raw = [
+            stats["positions"]["AAPL"]["allocation"],
+            stats["positions"]["MSFT"]["allocation"],
+        ]
+        raw_hhi = sum(w * w for w in raw)
+        raw_normalized = (raw_hhi - 0.5) / (1.0 - 0.5)
+        assert raw_normalized < 0.0
+
 
 # ── alloc.__main__ entry point (issue #101) ────────────────────────