ensure snapshots worker is passing the raw price map

This commit is contained in:
Emil Lerch 2026-08-27 11:13:18 -07:00
parent 08497a3270
commit cf76598659
Signed by: lobo
GPG key ID: A7B62D657EF764F8
2 changed files with 120 additions and 9 deletions

View file

@ -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,

View file

@ -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),