lobo/zfin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

fix manual_price -> price ratio discriminator

Browse source
This commit is contained in:
Emil Lerch 2026-08-27 07:32:49 -07:00
parent 6dea255377
commit 77b5e38313
Signed by: lobo
GPG key ID: A7B62D657EF764F8
3 changed files with 224 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
src/analytics/valuation.zig
View file

@ -205,16 +205,27 @@ pub const Allocation = struct {
shares: f64,
/// Weighted average cost per share across all lots (cost_basis / shares).
avg_cost: f64,
/// Latest price from API (or manual fallback), before price_ratio adjustment.
/// Latest price from API (or manual fallback). WHETHER
/// `price_ratio` HAS BEEN APPLIED DEPENDS ON `price_ratio` ITSELF -
/// read this together with that field, never alone:
///
/// POSITION-level and RAW. Never feed this into a per-lot
/// calculation: `mergeAllocsBySymbol` folds every ratio variant of a
/// ticker into one row with base-ticker-normalized `shares`, so a
/// lot row multiplying its own raw shares by this price is off by
/// exactly the lot's ratio. Per-lot display sites go through
/// `views/portfolio_sections.zig:effectivePriceFor`. See the "Per-LOT
/// display rows" section of the pricing-model block in
/// `models/portfolio.zig`.
/// - `price_ratio != 1.0` (UNMERGED): this is the EFFECTIVE price,
/// `pos.effectivePrice(raw, is_manual)`, ratio already applied.
/// - `price_ratio == 1.0`: either a plain unratioed position, or a
/// group that `mergeAllocsBySymbol` folded - in which case this
/// is the RAW base-ticker price (`total_mv / norm_shares`) and
/// `shares` are in base-ticker-equivalent units.
///
/// Either way `shares * current_price == market_value` holds, which
/// is what makes the position row self-consistent.
///
/// Never feed this into a per-lot calculation: a lot row that
/// multiplies its own raw shares by this price is off by the lot's
/// ratio in the merged case, and squares it if you "fix" that by
/// keying on `is_manual_price` instead. Per-lot display sites go
/// through `views/portfolio_sections.zig:effectivePriceFor`, which
/// discriminates on `price_ratio`. See the "Per-LOT display rows"
/// section of the pricing-model block in `models/portfolio.zig`.
current_price: f64,
/// Total current value: shares * current_price * price_ratio.
/// May be reduced by adjustForCoveredCalls for ITM sold calls

49
src/models/portfolio.zig
View file

@ -69,27 +69,40 @@ const Split = split.Split;
// ## Per-LOT display rows
//
// `valuation.Allocation` is a POSITION-level view, and its
// `current_price` is the RAW base-ticker price - not any lot's
// effective price. Worse, `valuation.mergeAllocsBySymbol` folds every
// ratio variant of one ticker into a single row whose `shares` are
// normalized into base-ticker-equivalent units. So the allocation is
// self-consistent while telling you nothing directly usable about the
// individual lots underneath it.
// `current_price` is NOT unconditionally raw. Which it is depends on
// whether `mergeAllocsBySymbol` folded the row:
//
// A lot-detail row that multiplies its OWN raw shares by that raw price
// is therefore wrong by exactly the lot's ratio. That shipped: a
// direct-indexing sleeve with `price_ratio:num:0.2387` rendered a
// +$3.39M gain against a real +$433K, and the lot rows under a position
// summed to three times the position's own market value. Three sites had
// independently hand-rolled the same broken expression.
// - UNMERGED (`alloc.price_ratio != 1.0`): the allocation carries the
// lot's own ratio, and `portfolioSummary` already applied it -
// `current_price` is the EFFECTIVE price.
// - MERGED, or plainly unratioed (`alloc.price_ratio == 1.0`):
// `current_price` is the RAW base-ticker price and `shares` are in
// base-ticker-equivalent units.
//
// So a lot-detail row cannot just multiply its own raw shares by
// `current_price`: in the merged case it is wrong by exactly the lot's
// ratio. That shipped - a direct-indexing sleeve with
// `price_ratio:num:0.2387` rendered a +$3.39M gain against a real
// +$433K, and the lot rows under a position summed to three times the
// position's own market value. Three sites had independently
// hand-rolled the same broken expression.
//
// The first fix for that got the OTHER half wrong: it keyed provenance
// on `alloc.is_manual_price`, which is orthogonal to merging, so every
// unmerged live-priced ratio'd lot then had its ratio applied twice.
// That is the single-CIT-lot case this file's `ticker` + `price_ratio`
// docs describe as the primary use, and no test caught it because every
// fixture left `Allocation.price_ratio` at 1.0 - indistinguishable from
// a merged group.
//
// So: per-lot display sites MUST price through
// `views/portfolio_sections.zig:effectivePriceFor(allocations, lot)`, which
// resolves `close_price`, reads provenance off
// `Allocation.is_manual_price`, and applies the ratio via
// `Lot.effectivePrice`. Never read `Allocation.current_price` into a
// per-lot calculation. Current callers: the CLI holdings table, the TUI
// portfolio tab's lot rows, and the shared column-width pass.
// `views/portfolio_sections.zig:effectivePriceFor(allocations, lot)`,
// which resolves `close_price`, discriminates on `price_ratio`, and
// applies the ratio via `Lot.effectivePrice`. Never read
// `Allocation.current_price` into a per-lot calculation, and never
// reintroduce `is_manual_price` as the provenance signal. Current
// callers: the CLI holdings table, the TUI portfolio tab's lot rows,
// and the shared column-width pass.
// ── Share model (split adjustment) ──────────────────────────
//

194
src/views/portfolio_sections.zig
View file

@ -187,30 +187,50 @@ fn gainLossCols(amount: f64) usize {
/// position summed to three times the position's own market value.
///
/// Provenance follows the `is_preadjusted` rule from the pricing-model
/// block in `models/portfolio.zig`:
/// block in `models/portfolio.zig`. The signal is `price_ratio`, and it
/// is answering ONE question: has this allocation been merged?
///
/// - `close_price` is preadjusted (already the lot's own NAV), so a
/// closed lot's ratio is NOT reapplied. Matches how the
/// contributions pipeline values closed lots.
/// - A manual `price::` is preadjusted too. `buildFallbackPrices`
/// stores the raw override and flags the allocation
/// `is_manual_price`, so that flag is the provenance signal here.
/// - Anything else is a candle close: raw, so the ratio applies.
/// - `close_price` short-circuits first. It is the price the lot
/// actually closed at, already in the lot's own terms, so the ratio
/// is NOT reapplied. Matches how the contributions pipeline values
/// closed lots (`effectivePrice(close_price, true)`).
/// - `a.price_ratio != 1.0` means the allocation is UNMERGED and
/// carries this lot's own ratio (`positionsAsOf` groups by
/// `(priceSymbol, price_ratio)` and propagates the ratio through).
/// `portfolioSummary` set its `current_price` to
/// `pos.effectivePrice(raw, is_manual)` - the ratio is ALREADY
/// APPLIED - so applying it again would square it.
/// - `a.price_ratio == 1.0` means either a plain unratioed position
/// (apply 1.0, a no-op) or a MERGED group, where
/// `mergeAllocsBySymbol` normalized shares into base-ticker units,
/// set `current_price` to the raw base price, and reset the ratio
/// to 1.0. Both want the lot's own ratio applied.
///
/// Do NOT use `a.is_manual_price` for this. It looks like a provenance
/// flag and reads like the right answer, but it is orthogonal: a
/// manual `price::` is stored raw in the prices map and flagged, then
/// `portfolioSummary` folds it into `current_price` via
/// `effectivePrice(p, true)` - so by the time it reaches an
/// `Allocation` it has the same "already effective" shape as a live
/// unmerged price. Keying on it applied the ratio twice for every
/// unmerged, live-priced, ratio'd lot - i.e. the single-CIT-lot case
/// that `docs/reference/config/portfolio-srf.md` documents as the
/// primary use of `price_ratio`.
///
/// Known limit, deliberately not handled: a MERGED group that mixes
/// manual-priced and live-priced components gets a blended
/// `current_price` (`total_mv / norm_shares`) that is neither raw nor
/// preadjusted, and a single `is_manual_price` bit cannot describe
/// both. That is an expressiveness gap in `mergeAllocsBySymbol` itself,
/// not something a lot-row accessor can repair. Merged groups whose
/// components share one provenance - the common case, and the only case
/// a ratio'd alias produces - are exact: with every component live off
/// the same raw price `r`, `total_mv / norm_shares` reduces to `r`.
/// preadjusted. That is an expressiveness gap in `mergeAllocsBySymbol`
/// itself, not something a lot-row accessor can repair. Merged groups
/// whose components share one provenance - the common case, and the
/// only case a ratio'd alias produces - are exact: with every
/// component live off the same raw price `r`, `total_mv / norm_shares`
/// reduces to `r`.
pub fn effectivePriceFor(allocations: []const Allocation, lot: Lot) f64 {
if (lot.close_price) |cp| return lot.effectivePrice(cp, true);
for (allocations) |a| {
if (!std.mem.eql(u8, a.symbol, lot.priceSymbol())) continue;
return lot.effectivePrice(a.current_price, a.is_manual_price);
return lot.effectivePrice(a.current_price, a.price_ratio != 1.0);
}
return 0;
}
@ -705,12 +725,44 @@ test "effectivePriceFor: ratio 1.0 passes the raw price straight through" {
try testing.expectEqual(@as(f64, 60), effectivePriceFor(&allocs, lot));
}
test "effectivePriceFor: manual price is preadjusted, so the ratio is NOT reapplied" {
// `buildFallbackPrices` stores the raw `price::` override and flags
// the allocation `is_manual_price`. A manual price is already the
// lot's own NAV by convention, so multiplying by the ratio would
// double-apply it.
var alloc = mkAlloc("ORCX", 100, 18.15, 19.01, 1901, 86);
test "effectivePriceFor: unmerged live price is already effective, ratio NOT reapplied" {
// THE REGRESSION. A single ratio'd lot whose ticker is shared with
// nobody produces an UNMERGED allocation: `positionsAsOf` groups by
// `(priceSymbol, price_ratio)`, so `Allocation.price_ratio` is the
// lot's own 5.0, and `portfolioSummary` already folded that ratio
// into `current_price`. Applying it again squares it.
//
// This is the documented primary use of `price_ratio` - a lone 401k
// CIT lot priced off its retail sibling (see
// docs/reference/config/portfolio-srf.md). Keying provenance on
// `is_manual_price` returned 144.04 * 5.0 = 720.20 here.
const allocs = [_]Allocation{mkAllocUnmerged("VTTHX", 1200, 106.99, 144.04, 5.0, 44_460)};
const lot = Lot{
.symbol = "02315N600",
.ticker = "VTTHX",
.price_ratio = 5.0,
.shares = 1200,
.open_date = Date.fromYmd(2026, 2, 26),
.open_price = 106.99,
};
try testing.expectEqual(@as(f64, 144.04), effectivePriceFor(&allocs, lot));
// And the lot row must reconcile with its own position row.
try testing.expectApproxEqRel(
allocs[0].market_value,
lot.effectiveShares() * effectivePriceFor(&allocs, lot),
1e-9,
);
}
test "effectivePriceFor: unmerged manual price is also already effective" {
// Same unmerged shape, manual `price::` instead of a candle close.
// `buildFallbackPrices` stores the raw override and flags the
// allocation, then `portfolioSummary` folds it in via
// `effectivePrice(p, true)` - so by the time it reaches an
// Allocation it has the same "already effective" shape as the live
// case above. `price_ratio != 1.0` covers both; `is_manual_price` is
// not consulted.
var alloc = mkAllocUnmerged("ORCX", 100, 18.15, 19.01, 5.0, 86);
alloc.is_manual_price = true;
const allocs = [_]Allocation{alloc};
const lot = Lot{
@ -723,6 +775,25 @@ test "effectivePriceFor: manual price is preadjusted, so the ratio is NOT reappl
try testing.expectEqual(@as(f64, 19.01), effectivePriceFor(&allocs, lot));
}
test "effectivePriceFor: a merged group's raw price DOES get the lot's ratio" {
// The counterpart, and why the discriminator can't just be "always
// skip". `mergeAllocsBySymbol` normalizes shares to base-ticker
// units, sets `current_price` to the raw base price, and resets
// `price_ratio` to 1.0 - so the lot's own ratio must be applied.
// `is_manual_price` is false in both this case and the unmerged live
// case above, which is exactly why it cannot discriminate them.
const merged = [_]Allocation{mkAlloc("BENCH", 1929.2, 426.05, 765.91, 1_477_603, 655_655)};
const lot = Lot{
.symbol = "DI-IDX",
.ticker = "BENCH",
.price_ratio = 1.0120921601549708,
.shares = 709.235272,
.open_date = Date.fromYmd(2026, 2, 25),
.open_price = 461.240208,
};
try testing.expectApproxEqRel(@as(f64, 775.1715), effectivePriceFor(&merged, lot), 1e-6);
}
test "effectivePriceFor: close_price wins over the allocation and skips the ratio" {
// A closed lot's `close_price` is the price it actually closed at -
// already in the lot's own units. Matches how the contributions
@ -839,12 +910,66 @@ test "effectivePriceFor: lot rows sum to their merged position's market value" {
try testing.expectApproxEqRel(@as(f64, 655_655.23), lot_gl_total, 1e-6);
}
test "effectivePriceFor: a LONE ratio'd lot's row reconciles with its position" {
// The unmerged half of the same invariant, driven through the real
// pipeline. One ratio'd lot, ticker shared with nobody, so
// `mergeAllocsBySymbol` leaves it alone: `price_ratio` stays 5.0 and
// `current_price` is already effective.
//
// Under the `is_manual_price` discriminator this test failed with
// lot_mv = shares * raw * ratio * ratio - the ratio squared, a 5x
// overstatement of a real position. The merged test above passed
// throughout, which is how the bug shipped.
const raw_price: f64 = 28.808; // retail sibling; institutional NAV = 144.04
const ratio: f64 = 5.0;
var lots = [_]Lot{.{
.symbol = "02315N600",
.ticker = "VTTHX",
.price_ratio = ratio,
.shares = 1200,
.open_date = Date.fromYmd(2026, 2, 26),
.open_price = 106.99,
.account = "Sample 401(k)",
}};
const pf = portfolio_mod.Portfolio{ .lots = &lots, .allocator = testing.allocator };
const as_of = Date.fromYmd(2026, 8, 26);
const positions = try pf.positionsAsOf(testing.allocator, as_of);
defer testing.allocator.free(positions);
try testing.expectEqual(@as(usize, 1), positions.len);
var prices = std.StringHashMap(f64).init(testing.allocator);
defer prices.deinit();
try prices.put("VTTHX", raw_price);
var summary = try valuation.portfolioSummary(as_of, testing.allocator, pf, positions, prices, null);
defer summary.deinit(testing.allocator);
try testing.expectEqual(@as(usize, 1), summary.allocations.len);
const a = summary.allocations[0];
// Unmerged: the allocation KEEPS the lot's ratio, and current_price
// is the effective (institutional) price, not the raw retail one.
try testing.expectEqual(ratio, a.price_ratio);
try testing.expectApproxEqRel(raw_price * ratio, a.current_price, 1e-9);
const eff_price = effectivePriceFor(summary.allocations, lots[0]);
try testing.expectApproxEqRel(raw_price * ratio, eff_price, 1e-9);
try testing.expectApproxEqRel(a.market_value, lots[0].effectiveShares() * eff_price, 1e-9);
// Magnitude pin: 1200 * 144.04 = $172,848, NOT 1200 * 720.20 = $864,240.
try testing.expectApproxEqRel(@as(f64, 172_848.0), lots[0].effectiveShares() * eff_price, 1e-9);
}
// ── computeWidths ─────────────────────────────────────────────
/// Build a minimal Allocation for the width and effective-price tests.
/// `cost_basis` is derived so `unrealized_gain_loss` is consistent (not
/// that computeWidths reads cost_basis, but it keeps the fixture
/// honest).
///
/// `price_ratio` defaults to 1.0, which is the MERGED shape (or a plain
/// unratioed position): `current_price` is the raw base-ticker price.
/// For the unmerged shape use `mkAllocUnmerged` - the distinction is
/// load-bearing, see `effectivePriceFor`.
fn mkAlloc(symbol: []const u8, shares: f64, avg_cost: f64, current_price: f64, market_value: f64, gl: f64) Allocation {
return .{
.symbol = symbol,
@ -860,6 +985,33 @@ fn mkAlloc(symbol: []const u8, shares: f64, avg_cost: f64, current_price: f64, m
};
}
/// An UNMERGED allocation: one ratio'd position that shares its ticker
/// with nobody, so `mergeAllocsBySymbol` never touched it. It keeps its
/// lot's `price_ratio`, and `portfolioSummary` already folded that ratio
/// into `current_price` (`pos.effectivePrice(raw, is_manual)`), so
/// `current_price` here is the EFFECTIVE price, not the raw base price.
///
/// This is the shape every fixture was missing: `mkAlloc` leaves
/// `price_ratio` at 1.0, which is indistinguishable from a merged group,
/// so a whole suite of tests can pass while the unmerged path squares
/// the ratio.
fn mkAllocUnmerged(symbol: []const u8, shares: f64, avg_cost: f64, effective_price: f64, price_ratio: f64, gl: f64) Allocation {
const mv = shares * effective_price;
return .{
.symbol = symbol,
.display_symbol = symbol,
.shares = shares,
.avg_cost = avg_cost,
.current_price = effective_price,
.market_value = mv,
.cost_basis = mv - gl,
.weight = 1.0,
.unrealized_gain_loss = gl,
.unrealized_return = 0,
.price_ratio = price_ratio,
};
}
test "computeWidths: a ratio'd lot's effective price widens the Price column" {
// A ratio'd lot renders its own effective price in the
// otherwise-blank Price cell, and at an 8.6x institutional ratio