From d3f2f15a2da31d00000f8097afeb8748795d86c0 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 23 Apr 2026 05:45:39 -0700 Subject: [PATCH] centralize calculation of effective price and market value --- src/analytics/valuation.zig | 12 ++--- src/commands/audit.zig | 18 +++++--- src/commands/snapshot.zig | 10 +++-- src/models/portfolio.zig | 87 +++++++++++++++++++++++++++++++++++-- 4 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/analytics/valuation.zig b/src/analytics/valuation.zig index e33f10c..6f9967b 100644 --- a/src/analytics/valuation.zig +++ b/src/analytics/valuation.zig @@ -223,11 +223,9 @@ pub fn portfolioSummary( for (positions) |pos| { if (pos.shares <= 0) continue; const raw_price = prices.get(pos.symbol) orelse continue; - // Only apply price_ratio to live/fetched prices. Manual/fallback prices - // (avg_cost) are already in the correct terms for the share class. const is_manual = if (manual_prices) |mp| mp.contains(pos.symbol) else false; - const price = if (is_manual) raw_price else raw_price * pos.price_ratio; - const mv = pos.shares * price; + const price = pos.effectivePrice(raw_price, is_manual); + const mv = pos.marketValue(raw_price, is_manual); total_value += mv; total_cost += pos.total_cost; total_realized += pos.realized_gain_loss; @@ -421,8 +419,10 @@ pub fn computeHistoricalSnapshots( const candles = candle_map.get(pos.symbol) orelse continue; const hist_price = findPriceAtDate(candles, target) orelse continue; - hist_value += pos.shares * hist_price * pos.price_ratio; - curr_value += pos.shares * curr_price * pos.price_ratio; + // Both prices come from candle history (live API provenance), + // so apply the share-class price_ratio — `is_preadjusted = false`. + hist_value += pos.marketValue(hist_price, false); + curr_value += pos.marketValue(curr_price, false); count += 1; } diff --git a/src/commands/audit.zig b/src/commands/audit.zig index 7ecbcf8..28097ad 100644 --- a/src/commands/audit.zig +++ b/src/commands/audit.zig @@ -788,9 +788,15 @@ 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 = price * pos.price_ratio; - pf_value = pos.shares * price * pos.price_ratio; + pf_price = pos.effectivePrice(price, false); + pf_value = pos.marketValue(price, false); try matched_symbols.put(pos.symbol, {}); try matched_symbols.put(pos.lot_symbol, {}); found_stock = true; @@ -861,7 +867,8 @@ pub fn compareAccounts( try matched_symbols.put(pos.symbol, {}); const price = prices.get(pos.symbol) orelse pos.avg_cost; - const mv = pos.shares * price * pos.price_ratio; + // See latent-bug note above — preserves pre-refactor behavior. + const mv = pos.marketValue(price, false); portfolio_total += mv; has_discrepancies = true; @@ -869,7 +876,7 @@ pub fn compareAccounts( .symbol = pos.symbol, .portfolio_shares = pos.shares, .brokerage_shares = null, - .portfolio_price = price * pos.price_ratio, + .portfolio_price = pos.effectivePrice(price, false), .brokerage_price = null, .portfolio_value = mv, .brokerage_value = null, @@ -1255,7 +1262,8 @@ pub fn run(allocator: std.mem.Allocator, svc: *zfin.DataService, portfolio_path: for (portfolio.lots) |lot| { if (lot.price) |p| { if (!prices.contains(lot.priceSymbol())) { - try prices.put(lot.priceSymbol(), p * lot.price_ratio); + // Pre-multiply — see "Pricing model" in models/portfolio.zig. + try prices.put(lot.priceSymbol(), lot.effectivePrice(p, false)); } } } diff --git a/src/commands/snapshot.zig b/src/commands/snapshot.zig index 30a8dbd..5b9328d 100644 --- a/src/commands/snapshot.zig +++ b/src/commands/snapshot.zig @@ -248,7 +248,11 @@ pub fn run( for (portfolio.lots) |lot| { if (lot.price) |p| { if (!prices.contains(lot.priceSymbol())) { - try prices.put(lot.priceSymbol(), p * lot.price_ratio); + // Pre-multiply manual overrides so the shared `prices` + // map holds share-class-correct values — see the + // "Pricing model / caching pre-multiply pattern" note + // in models/portfolio.zig. + try prices.put(lot.priceSymbol(), lot.effectivePrice(p, false)); } } } @@ -675,8 +679,8 @@ fn buildSnapshot( .stock => { const raw_price = prices.get(price_sym) orelse lot.open_price; const is_manual = manual_set.contains(price_sym); - const effective_price = if (is_manual) raw_price else raw_price * lot.price_ratio; - const value = lot.shares * effective_price; + const effective_price = lot.effectivePrice(raw_price, is_manual); + const value = lot.marketValue(raw_price, is_manual); // Candle-native quote_date/quote_stale: `symbol_prices` // already ran `candleCloseOnOrBefore(as_of)` per symbol, diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index 42e6003..c8f6b9d 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -2,6 +2,67 @@ const std = @import("std"); const Date = @import("date.zig").Date; const Candle = @import("candle.zig").Candle; +// ── Pricing model ──────────────────────────────────────────── +// +// How a lot's market value gets computed is non-obvious because several +// independent concerns overlap. Consolidated here so new readers (and +// future-us) don't have to reverse-engineer it from call sites. +// +// ## Inputs +// +// 1. `lot.shares` — signed share count. Negative = short (written +// options, short stock). Absolute value is what multiplies price for +// cost/value; the sign flows through to P&L. +// +// 2. Some "raw price" from one of these sources, in priority order: +// a. Candle close for the target date (live API — retail share +// class). This is the common path. +// b. `lot.price` manual override (`price::` in portfolio.srf). The +// user enters what they see in their brokerage statement, so this +// is in the LOT's share class already — no ratio needed. +// c. `position.avg_cost` fallback when no candle is available and no +// manual override exists. This is in the LOT's share class (user +// paid institutional-class prices to open the lot). +// +// 3. `lot.price_ratio` — share-class conversion factor. Default 1.0 +// for retail-class lots. Example: VTTHX (institutional, $144) holds +// VTHR (retail, $27.78), ratio ≈ 5.185. API gives us the $27.78 +// retail close; we multiply to get the $144 institutional price. +// +// ## The rule +// +// `effective_price = is_preadjusted ? raw_price : raw_price * price_ratio` +// +// Where `is_preadjusted` means "this raw price is already in the lot's +// share-class terms, don't apply the ratio." Sources (2b) and (2c) are +// preadjusted; source (2a) is not. +// +// `market_value = shares * effective_price` +// +// See `Lot.effectivePrice`, `Lot.marketValue`, and the matching methods +// on `Position` for the canonical implementation. All callers in +// snapshot.zig, audit.zig, and valuation.zig route through these — do +// not reintroduce inline `price * price_ratio` expressions. +// +// ## Caching pre-multiply pattern +// +// When manual overrides (2b) get folded into a shared `prices` map +// keyed by symbol, they're PRE-MULTIPLIED by `price_ratio` at insert +// time (see `commands/snapshot.zig:buildSnapshot` and +// `commands/audit.zig`). This normalizes the cached value so later +// readers can treat every entry uniformly as "price in whichever terms +// the lot needs." The `manual_set` (from `buildFallbackPrices`) then +// tells readers which entries are preadjusted. +// +// ## Known limitation (tracked as a follow-up) +// +// `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. + // ── Money-market / stable-NAV classification ──────────────── // // Centralized so that audit.zig, the Fidelity/Schwab parsers, and the @@ -182,8 +243,16 @@ pub const Lot = struct { return self.shares * self.open_price; } - pub fn marketValue(self: Lot, current_price: f64) f64 { - return self.shares * current_price; + /// Apply the share-class `price_ratio` to `raw_price`. See the + /// "Pricing model" block at the top of this file for the full + /// semantics of `is_preadjusted`. + pub fn effectivePrice(self: Lot, raw_price: f64, is_preadjusted: bool) f64 { + return if (is_preadjusted) raw_price else raw_price * self.price_ratio; + } + + /// Market value of the lot at `raw_price`: `shares * effectivePrice`. + pub fn marketValue(self: Lot, raw_price: f64, is_preadjusted: bool) f64 { + return self.shares * self.effectivePrice(raw_price, is_preadjusted); } /// Realized gain/loss for a closed lot: shares * (close_price - open_price). @@ -234,6 +303,18 @@ pub const Position = struct { /// Supporting dual-holding of investor + institutional shares of the same /// ticker would require a different grouping key in positions(). price_ratio: f64 = 1.0, + + /// Apply the share-class `price_ratio` to `raw_price` — the + /// Position-aggregate mirror of `Lot.effectivePrice`. See the + /// "Pricing model" block at the top of this file. + pub fn effectivePrice(self: Position, raw_price: f64, is_preadjusted: bool) f64 { + return if (is_preadjusted) raw_price else raw_price * self.price_ratio; + } + + /// Market value of the position at `raw_price`: `shares * effectivePrice`. + pub fn marketValue(self: Position, raw_price: f64, is_preadjusted: bool) f64 { + return self.shares * self.effectivePrice(raw_price, is_preadjusted); + } }; /// A portfolio is a collection of lots. @@ -619,7 +700,7 @@ test "lot basics" { }; try std.testing.expect(lot.isOpen()); try std.testing.expectApproxEqAbs(@as(f64, 1500.0), lot.costBasis(), 0.01); - try std.testing.expectApproxEqAbs(@as(f64, 2000.0), lot.marketValue(200.0), 0.01); + try std.testing.expectApproxEqAbs(@as(f64, 2000.0), lot.marketValue(200.0, true), 0.01); try std.testing.expectApproxEqAbs(@as(f64, 500.0), lot.unrealizedGainLoss(200.0), 0.01); try std.testing.expect(lot.realizedGainLoss() == null); }