diff --git a/src/PortfolioData.zig b/src/PortfolioData.zig index d33fcca..cee772a 100644 --- a/src/PortfolioData.zig +++ b/src/PortfolioData.zig @@ -1045,13 +1045,22 @@ fn snapshotsWorker(self: *PortfolioData, as_of: Date, positions: []const zfin.Po // read. self.io.checkCancel() catch return; const candle_map = self.candles_data orelse return; - const summary_ref = self.summary orelse return; - var prices = std.StringHashMap(f64).init(self.arena.child_allocator); - defer prices.deinit(); - for (summary_ref.allocations) |alloc| { - prices.put(alloc.symbol, alloc.current_price) catch return; - } + // `computeHistoricalSnapshots` wants RAW base-ticker prices - it applies + // each position's `price_ratio` itself. `revalue_base_prices` is exactly + // that map (candle closes, captured in `load` before the fallback + // derivation), and it is what the CLI path passes too, so both paths now + // agree by construction. + // + // This used to rebuild the map from `summary.allocations[].current_price`, + // which is NOT raw: for an unmerged allocation that price already has the + // lot's ratio folded in, so applying `price_ratio` again squared it. The + // historical side comes from raw candles and was correct, so the ratio + // between the two was wrong and every percentage on the TUI's + // `Historical:` line broke for a lone `ticker::` + `price_ratio::` lot. + // See the "Per-LOT display rows" block in `models/portfolio.zig` for the + // same trap in the lot-row renderers. + const prices = self.revalue_base_prices orelse return; self.snapshots_data = zfin.valuation.computeHistoricalSnapshots( as_of, diff --git a/src/analytics/valuation.zig b/src/analytics/valuation.zig index a42cad2..ba7fd12 100644 --- a/src/analytics/valuation.zig +++ b/src/analytics/valuation.zig @@ -708,7 +708,17 @@ fn findPriceAtDate(candles: []const Candle, target: Date) ?f64 { /// Compute historical portfolio snapshots for all standard lookback periods. /// `candle_map` maps symbol -> sorted candle slice. -/// `current_prices` maps symbol -> current price. +/// +/// `current_prices` maps symbol -> **RAW base-ticker price**, on the same +/// footing as the candle closes this reads for the historical side. Pass the +/// candle-close price map (`PortfolioData.revalue_base_prices`, or +/// `portfolio_loader`'s `prices`) - NEVER +/// `summary.allocations[].current_price`, which for an unmerged allocation +/// already has the lot's `price_ratio` folded in and would get it applied a +/// second time below, squaring it. That shipped in the TUI path and silently +/// broke every percentage on the `Historical:` line for a lone `ticker::` + +/// `price_ratio::` lot. +/// /// Only equity positions are considered. pub fn computeHistoricalSnapshots( as_of: Date, @@ -730,8 +740,9 @@ pub fn computeHistoricalSnapshots( const candles = candle_map.get(pos.symbol) orelse continue; const hist_price = findPriceAtDate(candles, target) orelse continue; - // Both prices come from candle history (live API provenance), - // so apply the share-class price_ratio - `is_preadjusted = false`. + // Both sides are RAW base-ticker prices - the historical one from + // candle history, the current one per this function's contract - + // so the share-class ratio applies to both: `is_preadjusted = false`. hist_value += pos.marketValue(hist_price, false); curr_value += pos.marketValue(curr_price, false); count += 1; @@ -756,6 +767,97 @@ fn makeCandle(date: Date, price: f64) Candle { return .{ .date = date, .open = price, .high = price, .low = price, .close = price, .adj_close = price, .volume = 1000 }; } +test "computeHistoricalSnapshots: current_prices must be RAW, ratio applied once" { + // The regression. A lone `ticker::` + `price_ratio::` lot produces an + // UNMERGED allocation whose `current_price` already has the ratio folded + // in. Feeding that in here made `pos.marketValue(.., false)` apply the + // ratio a SECOND time, squaring it - while the historical side, coming + // from raw candles, stayed correct. So the ratio between the two sides + // was wrong and every percentage on the TUI's `Historical:` line broke. + // + // Contract: pass RAW base-ticker prices. Then a pure price move of + // +10% must read as +10% regardless of the share-class ratio. + const a = std.testing.allocator; + const as_of = Date.fromYmd(2024, 6, 3); + const raw_then: f64 = 20.0; + const raw_now: f64 = 22.0; // +10% + const ratio: f64 = 5.0; + + // One month back is the first period in `HistoricalPeriod.all`. + const candles = [_]Candle{ + makeCandle(as_of.subtractMonths(1), raw_then), + makeCandle(as_of, raw_now), + }; + var candle_map = std.StringHashMap([]const Candle).init(a); + defer candle_map.deinit(); + try candle_map.put("VTTHX", &candles); + + const positions = [_]portfolio_mod.Position{.{ + .symbol = "VTTHX", + .shares = 100, + .avg_cost = 90, + .total_cost = 9000, + .open_lots = 1, + .closed_lots = 0, + .realized_gain_loss = 0, + .price_ratio = ratio, + }}; + + var raw_prices = std.StringHashMap(f64).init(a); + defer raw_prices.deinit(); + try raw_prices.put("VTTHX", raw_now); + + const snaps = computeHistoricalSnapshots(as_of, &positions, raw_prices, candle_map); + const m1 = snaps[0]; // 1M + try std.testing.expectEqual(@as(usize, 1), m1.position_count); + // Ratio applied exactly once on each side: 100 * 20 * 5 and 100 * 22 * 5. + try std.testing.expectApproxEqAbs(@as(f64, 10_000), m1.historical_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 11_000), m1.current_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 10.0), m1.changePct(), 1e-9); + + // What the buggy caller did: hand in the EFFECTIVE price instead. + // The ratio gets squared, the percentage inflates wildly, and the + // dollar figures become nonsense. + var effective_prices = std.StringHashMap(f64).init(a); + defer effective_prices.deinit(); + try effective_prices.put("VTTHX", raw_now * ratio); + const bad = computeHistoricalSnapshots(as_of, &positions, effective_prices, candle_map)[0]; + try std.testing.expectApproxEqAbs(@as(f64, 55_000), bad.current_value, 0.01); // 5x too big + try std.testing.expect(bad.changePct() > 400.0); // vs the true +10% +} + +test "computeHistoricalSnapshots: an unratioed position is unaffected either way" { + // Blast-radius guard: the fix must not move the needle for ordinary + // holdings, where raw and effective prices are the same number. + const a = std.testing.allocator; + const as_of = Date.fromYmd(2024, 6, 3); + const candles = [_]Candle{ + makeCandle(as_of.subtractMonths(1), 100), + makeCandle(as_of, 110), + }; + var candle_map = std.StringHashMap([]const Candle).init(a); + defer candle_map.deinit(); + try candle_map.put("ABC", &candles); + + const positions = [_]portfolio_mod.Position{.{ + .symbol = "ABC", + .shares = 10, + .avg_cost = 90, + .total_cost = 900, + .open_lots = 1, + .closed_lots = 0, + .realized_gain_loss = 0, + }}; + var prices = std.StringHashMap(f64).init(a); + defer prices.deinit(); + try prices.put("ABC", 110); + + const m1 = computeHistoricalSnapshots(as_of, &positions, prices, candle_map)[0]; + try std.testing.expectApproxEqAbs(@as(f64, 1000), m1.historical_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 1100), m1.current_value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 10.0), m1.changePct(), 1e-9); +} + test "findPriceAtDate exact match" { const candles = [_]Candle{ makeCandle(Date.fromYmd(2024, 1, 2), 100),