diff --git a/src/commands/audit.zig b/src/commands/audit.zig index 28097ad..e7b926e 100644 --- a/src/commands/audit.zig +++ b/src/commands/audit.zig @@ -477,6 +477,36 @@ pub const SchwabAccountComparison = struct { has_discrepancy: bool, }; +/// Resolved position value for audit display: effective per-share price +/// and total market value, with correct `price_ratio` handling based on +/// the price's provenance. +/// +/// Two sources feed `prices`: +/// 1. Live candle close — NOT preadjusted for the lot's share class, +/// so `price_ratio` must be applied. +/// 2. `pos.avg_cost` fallback — already in the lot's share-class +/// terms (user paid institutional-class prices to open the lot), +/// so `price_ratio` must be skipped. +/// +/// See the "Pricing model" block in `models/portfolio.zig` for the full +/// treatment. This helper is the audit-side mirror of the snapshot +/// side's `buildFallbackPrices` + `manual_set` pair. +const ResolvedValue = struct { price: f64, value: f64 }; + +fn resolvePositionValue(pos: zfin.Position, prices: std.StringHashMap(f64)) ResolvedValue { + if (prices.get(pos.symbol)) |live| { + return .{ + .price = pos.effectivePrice(live, false), + .value = pos.marketValue(live, false), + }; + } + // Fallback: avg_cost. Already preadjusted. + return .{ + .price = pos.effectivePrice(pos.avg_cost, true), + .value = pos.marketValue(pos.avg_cost, true), + }; +} + /// Compare Schwab summary against portfolio.srf account totals. pub fn compareSchwabSummary( allocator: std.mem.Allocator, @@ -788,15 +818,9 @@ pub fn compareAccounts( !std.mem.eql(u8, pos.lot_symbol, bp.symbol)) continue; pf_shares = pos.shares; - // NOTE: always-false for is_preadjusted preserves - // pre-refactor audit behavior. This is the latent - // bug called out in the "Pricing model" block in - // models/portfolio.zig: when `price` came from - // `pos.avg_cost`, it's already preadjusted and - // multiplying by price_ratio here is wrong. - const price = prices.get(pos.symbol) orelse pos.avg_cost; - pf_price = pos.effectivePrice(price, false); - pf_value = pos.marketValue(price, false); + const v = resolvePositionValue(pos, prices); + pf_price = v.price; + pf_value = v.value; try matched_symbols.put(pos.symbol, {}); try matched_symbols.put(pos.lot_symbol, {}); found_stock = true; @@ -866,9 +890,8 @@ pub fn compareAccounts( try matched_symbols.put(pos.symbol, {}); - const price = prices.get(pos.symbol) orelse pos.avg_cost; - // See latent-bug note above — preserves pre-refactor behavior. - const mv = pos.marketValue(price, false); + const v = resolvePositionValue(pos, prices); + const mv = v.value; portfolio_total += mv; has_discrepancies = true; @@ -876,7 +899,7 @@ pub fn compareAccounts( .symbol = pos.symbol, .portfolio_shares = pos.shares, .brokerage_shares = null, - .portfolio_price = pos.effectivePrice(price, false), + .portfolio_price = v.price, .brokerage_price = null, .portfolio_value = mv, .brokerage_value = null, @@ -1591,3 +1614,94 @@ test "parseSchwabCsv basic" { try std.testing.expectApproxEqAbs(@as(f64, 8271.12), parsed.positions[1].current_value.?, 0.01); try std.testing.expect(parsed.positions[1].quantity == null); } + +// ── resolvePositionValue ────────────────────────────────────── +// +// Pins the audit-side price-provenance rule: live-from-cache prices +// get price_ratio applied; avg_cost-fallback prices do not. This +// closes the latent bug where institutional-share-class positions +// (price_ratio != 1.0) that missed the cache would have their value +// over-reported by the ratio factor. + +test "resolvePositionValue: live cache hit applies price_ratio" { + const allocator = std.testing.allocator; + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + try prices.put("VTTHX", 27.78); // retail-class close + + const pos: zfin.Position = .{ + .symbol = "VTTHX", + .lot_symbol = "VTTHX", + .shares = 100, + .avg_cost = 106.18, + .total_cost = 10618, + .open_lots = 1, + .closed_lots = 0, + .realized_gain_loss = 0, + .account = "401k", + .price_ratio = 5.185, + }; + + const v = resolvePositionValue(pos, prices); + // price_ratio applied: 27.78 * 5.185 = 144.04 + try std.testing.expectApproxEqAbs(@as(f64, 144.04), v.price, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 14403.93), v.value, 0.01); +} + +test "resolvePositionValue: avg_cost fallback skips price_ratio" { + const allocator = std.testing.allocator; + // Empty prices map — simulate cache miss for VTTHX. + var prices = std.StringHashMap(f64).init(allocator); + defer prices.deinit(); + + const pos: zfin.Position = .{ + .symbol = "VTTHX", + .lot_symbol = "VTTHX", + .shares = 100, + .avg_cost = 106.18, // already institutional-class terms + .total_cost = 10618, + .open_lots = 1, + .closed_lots = 0, + .realized_gain_loss = 0, + .account = "401k", + .price_ratio = 5.185, + }; + + const v = resolvePositionValue(pos, prices); + // Pre-fix behavior would have multiplied: 106.18 * 5.185 = 550.55. + // Correct behavior: avg_cost is already in lot share-class terms. + try std.testing.expectApproxEqAbs(@as(f64, 106.18), v.price, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 10618.0), v.value, 0.01); +} + +test "resolvePositionValue: ratio-1.0 position unaffected by provenance" { + // Sanity: when price_ratio == 1.0, the bug never fired. Both paths + // should give the same answer. + const allocator = std.testing.allocator; + var prices_hit = std.StringHashMap(f64).init(allocator); + defer prices_hit.deinit(); + try prices_hit.put("AAPL", 200.0); + + var prices_miss = std.StringHashMap(f64).init(allocator); + defer prices_miss.deinit(); + + const pos: zfin.Position = .{ + .symbol = "AAPL", + .lot_symbol = "AAPL", + .shares = 10, + .avg_cost = 150.0, + .total_cost = 1500, + .open_lots = 1, + .closed_lots = 0, + .realized_gain_loss = 0, + .account = "Roth", + }; + + const hit = resolvePositionValue(pos, prices_hit); + const miss = resolvePositionValue(pos, prices_miss); + + try std.testing.expectApproxEqAbs(@as(f64, 200.0), hit.price, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 2000.0), hit.value, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 150.0), miss.price, 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 1500.0), miss.value, 0.01); +} diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index c8f6b9d..9f933f1 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -54,14 +54,15 @@ const Candle = @import("candle.zig").Candle; // the lot needs." The `manual_set` (from `buildFallbackPrices`) then // tells readers which entries are preadjusted. // -// ## Known limitation (tracked as a follow-up) +// ## avg_cost fallback // -// `audit.zig` unconditionally passes `is_preadjusted = false` even -// when its raw price came from `pos.avg_cost` — which IS preadjusted -// by the rules above. That mismatch is a latent bug, preserved here -// with a TODO to maintain the pre-refactor audit behavior. Fix is a -// one-line change once we're ready to verify the audit report -// deltas. +// When a symbol has no live price AND no manual override, callers fall +// back to `position.avg_cost` (the weighted average lot open-price). +// That value is already in the lot's share-class terms — the user paid +// institutional-class prices to open the lot — so `is_preadjusted = true`. +// Both snapshot and audit honor this: snapshot via `buildFallbackPrices` +// + `manual_set`, audit via inline `prices.get(sym) orelse avg_cost` +// with a matching `is_preadjusted` flag per branch. // ── Money-market / stable-NAV classification ──────────────── //