remove extra pre-multiply on manual snapshot price data

src/analytics/valuation.zig

@@ -1152,6 +1152,96 @@ test "computeDayChange: positions without candles are counted but excluded" {
     try std.testing.expectApproxEqAbs(@as(f64, 100), dc.change, 0.01);
 }
 
+test "buildFallbackPrices: a pre-inserted manual price loses its flag" {
+    // Why pre-multiplying a manual override into the shared map is unsound,
+    // demonstrated on the pure function rather than asserted in prose.
+    //
+    // The gap-fill is guarded by `if (!prices.contains(sym))`, so an entry
+    // someone else already inserted is skipped - and therefore never lands
+    // in `manual_price_set`. Downstream `effectivePrice(price, is_manual)`
+    // then sees `is_manual == false` and applies `price_ratio` to a value
+    // that already had it folded in, squaring it.
+    //
+    // `commands/snapshot.zig` did exactly that until the pre-multiply loop
+    // was deleted; `commands/audit.zig` still does, and cannot stop until
+    // the reconcile path grows a preadjusted set. See "The pre-multiply
+    // anti-pattern" in models/portfolio.zig.
+    const a = std.testing.allocator;
+    const ratio: f64 = 5.0;
+    const typed: f64 = 144.04; // what the user reads off the statement
+    const lots = [_]portfolio_mod.Lot{.{
+        .symbol = "02315N600",
+        .ticker = "VTTHX",
+        .price_ratio = ratio,
+        .price = typed,
+        .shares = 100,
+        .open_date = Date.fromYmd(2026, 2, 26),
+        .open_price = 106.99,
+    }};
+    const positions = [_]portfolio_mod.Position{dcPos("VTTHX", 100, ratio)};
+
+    // CORRECT: hand it an empty map. The override goes in untouched and the
+    // symbol is flagged, so the ratio is skipped and the value is exact.
+    {
+        var prices = std.StringHashMap(f64).init(a);
+        defer prices.deinit();
+        var manual = try buildFallbackPrices(a, &lots, &positions, &prices);
+        defer manual.deinit();
+
+        try std.testing.expect(manual.contains("VTTHX"));
+        const p = prices.get("VTTHX").?;
+        try std.testing.expectApproxEqAbs(typed, p, 0.001); // stored RAW
+        const value = positions[0].marketValue(p, manual.contains("VTTHX"));
+        try std.testing.expectApproxEqAbs(@as(f64, 14_404.0), value, 0.01);
+    }
+
+    // BROKEN: pre-multiply the same override in first. The gap-fill skips
+    // it, the flag never appears, and the ratio lands twice.
+    {
+        var prices = std.StringHashMap(f64).init(a);
+        defer prices.deinit();
+        try prices.put("VTTHX", typed * ratio); // the pre-multiply
+        var manual = try buildFallbackPrices(a, &lots, &positions, &prices);
+        defer manual.deinit();
+
+        try std.testing.expect(!manual.contains("VTTHX")); // flag lost
+        const p = prices.get("VTTHX").?;
+        const value = positions[0].marketValue(p, manual.contains("VTTHX"));
+        // shares * (p * ratio) * ratio = 100 * 144.04 * 25 = 360,100.
+        // The error factor is the ratio SQUARED - 25x here, not 5x.
+        try std.testing.expectApproxEqAbs(@as(f64, 360_100.0), value, 0.01);
+    }
+}
+
+test "buildFallbackPrices: a candle price is never overridden or flagged" {
+    // The guard's legitimate purpose: a live close already in the map wins
+    // over a manual override, and must NOT be flagged preadjusted - candle
+    // closes are raw and do need the ratio.
+    const a = std.testing.allocator;
+    const lots = [_]portfolio_mod.Lot{.{
+        .symbol = "02315N600",
+        .ticker = "VTTHX",
+        .price_ratio = 5.0,
+        .price = 999.0, // stale override, should lose
+        .shares = 100,
+        .open_date = Date.fromYmd(2026, 2, 26),
+        .open_price = 106.99,
+    }};
+    const positions = [_]portfolio_mod.Position{dcPos("VTTHX", 100, 5.0)};
+
+    var prices = std.StringHashMap(f64).init(a);
+    defer prices.deinit();
+    try prices.put("VTTHX", 28.808); // raw candle close
+    var manual = try buildFallbackPrices(a, &lots, &positions, &prices);
+    defer manual.deinit();
+
+    try std.testing.expect(!manual.contains("VTTHX"));
+    try std.testing.expectApproxEqAbs(@as(f64, 28.808), prices.get("VTTHX").?, 0.001);
+    // Ratio applied once: 100 * 28.808 * 5 = 14,404.
+    const value = positions[0].marketValue(prices.get("VTTHX").?, false);
+    try std.testing.expectApproxEqAbs(@as(f64, 14_404.0), value, 0.01);
+}
+
 test "computeDayChange: a hand-priced holding is IMPOSSIBLE, not a gap" {
     // The headline behaviour. A 529 fund the user prices by hand on Saturdays
     // has no candle history and never will: the only price zfin will ever

src/commands/audit.zig

@@ -175,7 +175,16 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         for (portfolio.lots) |lot| {
             if (lot.price) |p| {
                 if (!prices.contains(lot.priceSymbol())) {
-                    // Pre-multiply - see "Pricing model" in models/portfolio.zig.
+                    // KNOWN GAP, see "The pre-multiply anti-pattern" in
+                    // models/portfolio.zig. Pre-multiplying here is unsound
+                    // - `resolvePositionValue` applies the ratio again - so
+                    // a manual price on a lot with a non-unit `price_ratio`
+                    // reads high by that ratio SQUARED (25x at ratio 5, not
+                    // 5x). It stays for now because the
+                    // reconcile path has no preadjusted-set to thread
+                    // through; snapshot.zig fixed its copy by deleting the
+                    // pre-multiply and letting `buildFallbackPrices` flag
+                    // the symbol instead, which this path cannot do yet.
                     try prices.put(lot.priceSymbol(), lot.effectivePrice(p, false));
                 }
             }

src/commands/snapshot.zig

@@ -294,26 +294,29 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         }
     }
 
-    // Build the flat-price map that `portfolioSummary` expects, plus
-    // apply manual `price::` overrides from portfolio.srf (which win
-    // over the candle lookup).
+    // Build the flat-price map that `portfolioSummary` expects: RAW
+    // base-ticker closes, keyed by symbol.
+    //
+    // Manual `price::` overrides are deliberately NOT applied here.
+    // `captureSnapshot` runs `buildFallbackPrices`, which gap-fills them
+    // correctly - it stores the raw value AND records the symbol in its
+    // `manual_set`, so the downstream `lot.effectivePrice(raw, is_manual)`
+    // knows to skip the share-class ratio (a hand-typed price is already in
+    // the lot's own share class).
+    //
+    // This used to pre-multiply overrides into the map here, which was wrong
+    // twice over. The map is keyed by SYMBOL while `price_ratio` is per LOT,
+    // so folding one lot's ratio in poisons every other lot sharing that
+    // ticker. And `buildFallbackPrices` gap-fills only when the symbol is
+    // absent, so the pre-inserted entry was skipped and never flagged -
+    // leaving `is_manual` false downstream and the ratio applied a second
+    // time, squaring it.
     var prices = std.StringHashMap(f64).init(allocator);
     defer prices.deinit();
     var sp_it = symbol_prices.iterator();
     while (sp_it.next()) |entry| {
         try prices.put(entry.key_ptr.*, entry.value_ptr.close);
     }
-    for (portfolio.lots) |lot| {
-        if (lot.price) |p| {
-            if (!prices.contains(lot.priceSymbol())) {
-                // 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));
-            }
-        }
-    }
 
     // Derive output path.
     var as_of_buf: [10]u8 = undefined;

src/models/portfolio.zig

@@ -46,15 +46,36 @@ const Split = split.Split;
 // snapshot.zig, audit/, and valuation.zig route through these - do
 // not reintroduce inline `price * price_ratio` expressions.
 //
-// ## Caching pre-multiply pattern
+// ## The pre-multiply anti-pattern (do not add new instances)
 //
-// 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/`). 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.
+// A tempting shortcut is to fold `price_ratio` into a shared `prices` map
+// at insert time - "pre-multiplying" a manual override - so later readers
+// can treat every entry uniformly. It is unsound, for two reasons:
+//
+//   1. The map is keyed by SYMBOL while `price_ratio` is per LOT. Folding
+//      one lot's ratio into a symbol-keyed entry corrupts every other lot
+//      sharing that ticker at a different ratio - which is exactly the
+//      `ticker::` aliasing this whole mechanism exists to support.
+//   2. It collides with `buildFallbackPrices`, whose first pass gap-fills
+//      only `if (!prices.contains(sym))`. A pre-inserted entry is skipped,
+//      so it never lands in `manual_set`, so the downstream
+//      `effectivePrice(raw, is_manual)` sees `is_manual == false` and
+//      applies the ratio a SECOND time, squaring it.
+//
+// The correct shape is raw value + a companion "preadjusted" set:
+// `buildFallbackPrices` stores the override untouched and records the
+// symbol in `manual_set`; readers pass that through as `is_preadjusted`.
+// `commands/snapshot.zig` does this (it used to pre-multiply, and squared
+// the ratio for any manual-priced lot with a non-unit one).
+//
+// KNOWN GAP: `commands/audit.zig` still pre-multiplies, because the
+// reconcile path has no `manual_set` equivalent -
+// `reconcile/common.zig:resolvePositionValue` applies the ratio to
+// whatever is in the map with no way to opt out. Fixing it properly means
+// threading preadjusted-ness through the reconcile signatures. Until then
+// a manual `price::` combined with a non-unit `price_ratio` reads high by
+// the ratio in `zfin audit` only. Do not "fix" it by dividing the ratio
+// out at insert - that just moves the corruption onto sibling lots.
 //
 // ## avg_cost fallback
 //