skip price_ratio on avg_cost fallback

This commit is contained in:
Emil Lerch 2026-04-23 06:55:27 -07:00
parent 72632344ee
commit 6ec6bd634b
2 changed files with 135 additions and 20 deletions

View file

@ -477,6 +477,36 @@ pub const SchwabAccountComparison = struct {
has_discrepancy: bool, 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. /// Compare Schwab summary against portfolio.srf account totals.
pub fn compareSchwabSummary( pub fn compareSchwabSummary(
allocator: std.mem.Allocator, allocator: std.mem.Allocator,
@ -788,15 +818,9 @@ pub fn compareAccounts(
!std.mem.eql(u8, pos.lot_symbol, bp.symbol)) !std.mem.eql(u8, pos.lot_symbol, bp.symbol))
continue; continue;
pf_shares = pos.shares; pf_shares = pos.shares;
// NOTE: always-false for is_preadjusted preserves const v = resolvePositionValue(pos, prices);
// pre-refactor audit behavior. This is the latent pf_price = v.price;
// bug called out in the "Pricing model" block in pf_value = v.value;
// 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);
try matched_symbols.put(pos.symbol, {}); try matched_symbols.put(pos.symbol, {});
try matched_symbols.put(pos.lot_symbol, {}); try matched_symbols.put(pos.lot_symbol, {});
found_stock = true; found_stock = true;
@ -866,9 +890,8 @@ pub fn compareAccounts(
try matched_symbols.put(pos.symbol, {}); try matched_symbols.put(pos.symbol, {});
const price = prices.get(pos.symbol) orelse pos.avg_cost; const v = resolvePositionValue(pos, prices);
// See latent-bug note above preserves pre-refactor behavior. const mv = v.value;
const mv = pos.marketValue(price, false);
portfolio_total += mv; portfolio_total += mv;
has_discrepancies = true; has_discrepancies = true;
@ -876,7 +899,7 @@ pub fn compareAccounts(
.symbol = pos.symbol, .symbol = pos.symbol,
.portfolio_shares = pos.shares, .portfolio_shares = pos.shares,
.brokerage_shares = null, .brokerage_shares = null,
.portfolio_price = pos.effectivePrice(price, false), .portfolio_price = v.price,
.brokerage_price = null, .brokerage_price = null,
.portfolio_value = mv, .portfolio_value = mv,
.brokerage_value = null, .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.expectApproxEqAbs(@as(f64, 8271.12), parsed.positions[1].current_value.?, 0.01);
try std.testing.expect(parsed.positions[1].quantity == null); 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);
}

View file

@ -54,14 +54,15 @@ const Candle = @import("candle.zig").Candle;
// the lot needs." The `manual_set` (from `buildFallbackPrices`) then // the lot needs." The `manual_set` (from `buildFallbackPrices`) then
// tells readers which entries are preadjusted. // 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 a symbol has no live price AND no manual override, callers fall
// when its raw price came from `pos.avg_cost` which IS preadjusted // back to `position.avg_cost` (the weighted average lot open-price).
// by the rules above. That mismatch is a latent bug, preserved here // That value is already in the lot's share-class terms the user paid
// with a TODO to maintain the pre-refactor audit behavior. Fix is a // institutional-class prices to open the lot so `is_preadjusted = true`.
// one-line change once we're ready to verify the audit report // Both snapshot and audit honor this: snapshot via `buildFallbackPrices`
// deltas. // + `manual_set`, audit via inline `prices.get(sym) orelse avg_cost`
// with a matching `is_preadjusted` flag per branch.
// Money-market / stable-NAV classification // Money-market / stable-NAV classification
// //