diff --git a/src/commands/contributions.zig b/src/commands/contributions.zig index c062b34..02b673b 100644 --- a/src/commands/contributions.zig +++ b/src/commands/contributions.zig @@ -1482,9 +1482,14 @@ fn detectEdits( const unit_value: f64 = blk: { if (rep_lot.security_type == .stock) { - if (prices.get(rep_lot.priceSymbol())) |p| break :blk p * rep_lot.price_ratio; - if (rep_lot.price) |p| break :blk p * rep_lot.price_ratio; - break :blk rep_lot.open_price * rep_lot.price_ratio; + // `prices.get` is the raw retail-class API price → ratio applies. + // `lot.price` (manual override) and `lot.open_price` are both + // in the lot's own share-class terms (preadjusted) → ratio + // must NOT be applied. See the "Pricing model" doc-block in + // models/portfolio.zig. + if (prices.get(rep_lot.priceSymbol())) |p| break :blk rep_lot.effectivePrice(p, false); + if (rep_lot.price) |p| break :blk rep_lot.effectivePrice(p, true); + break :blk rep_lot.effectivePrice(rep_lot.open_price, true); } break :blk 1.0; }; @@ -1631,11 +1636,17 @@ fn computeReport( } else base_kind; // Determine unit_value for stocks: prefer current cached price; // fall back to manual price::; fall back to open_price. + // + // `prices.get` is the raw retail-class API price → ratio applies. + // `lot.price` (manual override) and `lot.open_price` are both + // in the lot's own share-class terms (preadjusted) → ratio + // must NOT be applied. See the "Pricing model" doc-block in + // models/portfolio.zig. const unit_value: f64 = blk: { if (lot.security_type == .stock) { - if (prices.get(lot.priceSymbol())) |p| break :blk p * lot.price_ratio; - if (lot.price) |p| break :blk p * lot.price_ratio; - break :blk lot.open_price * lot.price_ratio; + if (prices.get(lot.priceSymbol())) |p| break :blk lot.effectivePrice(p, false); + if (lot.price) |p| break :blk lot.effectivePrice(p, true); + break :blk lot.effectivePrice(lot.open_price, true); } // cash/cd: 1:1 with shares break :blk 1.0; @@ -1699,8 +1710,13 @@ fn computeReport( // For fresh stock lots: value at open_price (that's literally the // money that went in). For cash: shares == dollars. For CDs: // face = shares × open_price. + // + // open_price is in the lot's own share-class terms (preadjusted), + // so route it through effectivePrice with is_preadjusted=true to + // avoid double-applying price_ratio. See the "Pricing model" + // doc-block in models/portfolio.zig. const unit_value: f64 = switch (lot.security_type) { - .stock => lot.open_price * lot.price_ratio, + .stock => lot.effectivePrice(lot.open_price, true), .cash => 1.0, .cd => lot.open_price, .option => lot.open_price * lot.multiplier, @@ -3024,6 +3040,177 @@ test "computeReport: manual-priced lot (price:: no ticker) uses manual price" { try std.testing.expectApproxEqAbs(@as(f64, 711.96), report.changes[0].value(), 0.5); } +// ── price_ratio regression tests ───────────────────────────── +// +// `lot.open_price` and `lot.price` (manual override) are both in the +// LOT's own share-class terms — i.e. preadjusted. Multiplying either +// by `lot.price_ratio` would double-apply the ratio. Only API-fetched +// prices from `prices.get(...)` (retail share class) need the ratio. +// See the "Pricing model" doc-block at the top of `models/portfolio.zig`. +// +// These four tests cover each of the three unit_value branches in +// `computeReport`'s new-lot and same-key-share-delta paths. Without +// the fix, the new-lot, manual-price, and open-price fallback tests +// inflate value by `price_ratio`× while the live-price test stays +// correct. + +test "computeReport: new institutional stock lot uses preadjusted open_price (no double-ratio)" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + // No live price — exercises the open_price fallback branch in + // the new-lot path. open_price is in the LOT's institutional + // share class, so the ratio must NOT be applied. + + const before = [_]Lot{}; + const after = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 39.249, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.new_stock, report.changes[0].kind); + // 39.249 × 140.92 = 5,531.18 (institutional value). + // Buggy version produces 39.249 × 140.92 × 6.6139 ≈ 36,581. + try std.testing.expectApproxEqAbs(@as(f64, 5531.18), report.changes[0].value(), 0.5); +} + +test "computeReport: same-key share delta with no live price uses preadjusted open_price" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + // No price in the map — open_price fallback should fire. + + const before = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 100, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 110, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.rollup_delta, report.changes[0].kind); + // 10 × 140.92 = 1,409.20 (institutional value of the share delta). + // Buggy version produces 10 × 140.92 × 6.6139 ≈ 9,322. + try std.testing.expectApproxEqAbs(@as(f64, 1409.20), report.changes[0].value(), 0.5); +} + +test "computeReport: same-key share delta with manual price:: uses preadjusted manual price" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + // No price in the map — manual `price::` override should fire. + // Manual prices are entered in the LOT's share class (what the + // user sees on their statement), so they're preadjusted. + + const before = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 100, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price = 145.00, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 110, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price = 145.00, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.rollup_delta, report.changes[0].kind); + // 10 × 145.00 = 1,450.00. Buggy version: 10 × 145.00 × 6.6139 ≈ 9,590. + try std.testing.expectApproxEqAbs(@as(f64, 1450.00), report.changes[0].value(), 0.5); +} + +test "computeReport: same-key share delta with live price applies ratio" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const allocator = arena_state.allocator(); + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + // Live retail-class price — ratio MUST be applied to convert + // to institutional NAV. This is the only branch that's correct + // in the buggy code; lock it in so the fix doesn't regress. + try prices.put("VTTVX", 21.30); + + const before = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 100, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + const after = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 110, + .open_date = Date.fromYmd(2026, 2, 26), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + + const report = try computeReport(allocator, &before, &after, &prices, Date.fromYmd(2026, 4, 18), .{}); + + try std.testing.expectEqual(@as(usize, 1), report.changes.len); + try std.testing.expectEqual(ChangeKind.rollup_delta, report.changes[0].kind); + // 10 × 21.30 × 6.6139 = 1,408.76 (institutional value). + try std.testing.expectApproxEqAbs(@as(f64, 1408.76), report.changes[0].value(), 0.5); +} + test "computeReport: maturity_date change on same CD is flagged" { var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index 6de0fab..b61c2ef 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -596,8 +596,14 @@ pub const Portfolio = struct { defer allocator.free(acct_positions); for (acct_positions) |pos| { - const price = prices.get(pos.symbol) orelse pos.avg_cost; - total += pos.shares * price * pos.price_ratio; + // Live API price is in the retail share class → ratio applies + // (is_preadjusted=false). avg_cost fallback is in the lot's own + // share-class terms → ratio must NOT be applied + // (is_preadjusted=true). See the "Pricing model" doc-block above. + total += if (prices.get(pos.symbol)) |p| + pos.marketValue(p, false) + else + pos.marketValue(pos.avg_cost, true); } total += self.nonStockValueForAccount(as_of, account_name); @@ -1357,6 +1363,38 @@ test "totalForAccount" { try std.testing.expectApproxEqAbs(@as(f64, 44500.0), total, 0.01); } +test "totalForAccount: institutional lot missing from prices map uses preadjusted avg_cost" { + // Regression test for the price_ratio double-application bug in + // Portfolio.totalForAccount. When a position misses the prices + // map, the avg_cost fallback is in the LOT's share-class terms + // (preadjusted) — multiplying by price_ratio would inflate the + // value by the ratio. See the "Pricing model" doc-block above. + const allocator = std.testing.allocator; + + var lots = [_]Lot{ + .{ + .symbol = "02315N402", + .ticker = "VTTVX", + .shares = 100, + .open_date = Date.fromYmd(2024, 1, 1), + .open_price = 140.92, + .price_ratio = 6.6139, + .account = "Sample 401(k)", + }, + }; + + const portfolio = Portfolio{ .lots = &lots, .allocator = allocator }; + + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + // Empty prices map — avg_cost (= 140.92, institutional) fallback fires. + + // Correct: 100 × 140.92 = 14,092 (institutional value). + // Buggy: 100 × 140.92 × 6.6139 ≈ 93,213. + const total = portfolio.totalForAccount(Date.fromYmd(2026, 5, 8), allocator, "Sample 401(k)", prices); + try std.testing.expectApproxEqAbs(@as(f64, 14092.0), total, 0.5); +} + // ── Money-market predicate tests ───────────────────────────── test "isMoneyMarketSymbol: known Schwab and Fidelity tickers" {