watchlist, benchmark symbols, and portfolio symbols refreshed in all the places

src/PortfolioData.zig

@@ -715,30 +715,31 @@ pub fn load(
 
     // ── Compute watchlist symbols ────────────────────────────
     //
-    // Union of caller-supplied watchlist syms (typically from
+    // Union of caller-supplied watchlist syms (typically from a separate
-    // a separate `watchlist.srf` file) and portfolio's own
+    // `watchlist.srf` file) and portfolio's own `.watch` lots, with held
-    // `.watch` lots, with held symbols (already in `syms`)
+    // symbols (already in `syms`) excluded so we never double-fetch.
-    // excluded so we never double-fetch.
+    //
-    var watchlist_set = std.StringHashMap(void).init(gpa);
+    // Shared with the CLI rather than reimplemented here. This was the third
-    defer watchlist_set.deinit();
+    // hand-rolled copy of the same set logic, and the copies had drifted: the
+    // CLI's omitted `watchlist.srf` entirely, so a watchlist-only symbol was
+    // displayed from whatever the cache held and never fetched. One
+    // implementation means that class of divergence cannot recur.
+    //
+    // Also now order-stable - watch lots then watchlist entries. The previous
+    // HashMap-iteration build made the "[5/28] Loading X" progress order vary
+    // between runs for no reason.
+    const watch_syms_list = pf.extraPriceSymbols(gpa, syms, opts.watchlist_syms) catch return error.OutOfMemory;
+    defer gpa.free(watch_syms_list);
+
+    // Lookup sets for splitting the unified price map below. Derived FROM the
+    // two symbol lists rather than rebuilt from the lots, so they cannot
+    // disagree with what was actually fetched.
     var portfolio_set = std.StringHashMap(void).init(gpa);
     defer portfolio_set.deinit();
     for (syms) |s| portfolio_set.put(s, {}) catch return error.OutOfMemory;
-    for (opts.watchlist_syms) |sym| {
+    var watchlist_set = std.StringHashMap(void).init(gpa);
-        if (!portfolio_set.contains(sym)) watchlist_set.put(sym, {}) catch return error.OutOfMemory;
+    defer watchlist_set.deinit();
-    }
+    for (watch_syms_list) |s| watchlist_set.put(s, {}) catch return error.OutOfMemory;
-    for (pf.lots) |lot| {
-        if (lot.security_type == .watch) {
-            const sym = lot.priceSymbol();
-            if (!portfolio_set.contains(sym)) watchlist_set.put(sym, {}) catch return error.OutOfMemory;
-        }
-    }
-    var watch_syms_list: std.ArrayList([]const u8) = .empty;
-    defer watch_syms_list.deinit(gpa);
-    {
-        var it = watchlist_set.keyIterator();
-        while (it.next()) |k| watch_syms_list.append(gpa, k.*) catch return error.OutOfMemory;
-    }
 
     // ── Fetch prices ──────────────────────────────────────────
     //
@@ -762,7 +763,7 @@ pub fn load(
 
     var load_all = self.svc.loadAllPrices(
         syms,
-        watch_syms_list.items,
+        watch_syms_list,
         opts.fetch_options,
         opts.aggregate_progress,
         sym_cb,

src/cache/store.zig

@@ -1903,7 +1903,18 @@ pub fn deserializePortfolio(allocator: std.mem.Allocator, data: []const u8) !Por
     var skipped: usize = 0;
     while (try it.next()) |fields| {
         const line = it.state.line;
-        var lot = fields.to(Lot, .{}) catch {
+        // `strings_to_numbers` because these are HUMAN-EDITED files, which is
+        // exactly the case srf's default strict coercion is not for - its own
+        // doc says "if you want to use this for human-edited files, turn this
+        // on". Strict mode assumes the writer was a machine, so a numeric
+        // field spelled with a string separator (`close_price::200.00` instead
+        // of `close_price:num:200.00`) reaches an unchecked `val.?.number` and
+        // takes the whole process down. One such typo was enough to panic every
+        // `zfin portfolio` run.
+        //
+        // The `catch` below still handles genuinely unparseable values; this
+        // only stops a hand-typed separator from being fatal.
+        var lot = fields.to(Lot, .{ .strings_to_numbers = true }) catch {
             std.log.warn("portfolio: could not parse record at line {d}", .{line});
             skipped += 1;
             continue;
@@ -3720,3 +3731,33 @@ test "appendRaw atomicity: concurrent readers see either pre- or post-append, ne
     try testing.expect(total > 0);
     try testing.expectEqual(@as(u32, 0), bad);
 }
+
+test "deserializePortfolio: a hand-typed string separator on a numeric field is not fatal" {
+    // THE CRASH THIS GUARDS. A numeric field spelled with a string separator -
+    // `close_price::200.00` instead of `close_price:num:200.00`, one character -
+    // panicked with "access of union field 'number' while field 'string' is
+    // active", because srf's default strict coercion is built for
+    // machine-written cache files and reaches an unchecked `val.?.number`.
+    // These files are hand-maintained, so leniency is the documented answer.
+    const data =
+        "#!srfv1\n" ++
+        "symbol::AAPL,shares:num:100,open_date::2024-01-15,open_price:num:140.00,close_date::2024-06-03,close_price::200.00\n";
+    var p = try deserializePortfolio(std.testing.allocator, data);
+    defer p.deinit();
+    try std.testing.expectEqual(@as(usize, 1), p.lots.len);
+    // Parsed, not skipped, and to the right value.
+    try std.testing.expectApproxEqAbs(@as(f64, 200.00), p.lots[0].close_price.?, 0.001);
+}
+
+test "deserializePortfolio: underscore digit separators parse (they always did)" {
+    // Recorded because they look suspicious and are not the bug: Zig's
+    // std.fmt.parseFloat accepts `_` natively, so `shares:num:1_234_567` has
+    // always been fine. Chased this before finding the real cause.
+    const data =
+        "#!srfv1\n" ++
+        "security_type::illiquid,symbol::Sample Asset,shares:num:1_234_567,open_date::2024-01-15,open_price:num:1.00\n";
+    var p = try deserializePortfolio(std.testing.allocator, data);
+    defer p.deinit();
+    try std.testing.expectEqual(@as(usize, 1), p.lots.len);
+    try std.testing.expectApproxEqAbs(@as(f64, 1_234_567), p.lots[0].shares, 0.5);
+}

src/commands/portfolio.zig

@@ -126,23 +126,35 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
 
     var fail_count: usize = 0;
 
-    // Also collect watch symbols that need fetching
+    // Loaded BEFORE the fetch below, not after, and that ordering is the
-    var watch_syms: std.ArrayList([]const u8) = .empty;
+    // fix for a real bug: these symbols used to be read only for display
-    defer watch_syms.deinit(allocator);
+    // and priced from `getCachedLastClose`, so nothing on any CLI path ever
-    {
+    // put them in the fetch set. A watchlist-only symbol therefore went
-        var seen = std.StringHashMap(void).init(allocator);
+    // arbitrarily stale - SPCX sat 39 days out of date - while the TUI,
-        defer seen.deinit();
+    // which does pass `watchlist_syms` into its load, showed it current.
-        for (syms) |s| try seen.put(s, {});
+    //
-        for (portfolio.lots) |lot| {
+    // Lifetime is unchanged: these slices must outlive the `display` call
-            if (lot.security_type == .watch and !seen.contains(lot.priceSymbol())) {
+    // at the end of `run`, because `watch_list` and `watch_prices`' keys
-                try seen.put(lot.priceSymbol(), {});
+    // borrow them. Freeing earlier rendered them as freed-memory garbage.
-                try watch_syms.append(allocator, lot.priceSymbol());
+    const wl_syms: ?[][]const u8 = if (watchlist_path) |wl_path|
-            }
+        cli.loadWatchlist(io, allocator, wl_path)
-        }
+    else
-    }
+        null;
+    defer cli.freeWatchlist(allocator, wl_syms);
+
+    // Symbols to price that aren't stock positions: `security_type::watch`
+    // lots in the portfolio file, plus everything in `watchlist.srf`. The set
+    // logic is a Portfolio method so it is testable - `run` needs a live
+    // context, so anything embedded here can only be covered by hand.
+    const watch_syms = try portfolio.extraPriceSymbols(
+        allocator,
+        syms,
+        if (wl_syms) |l| l else &.{},
+    );
+    defer allocator.free(watch_syms);
 
     // All symbols to fetch (stock positions + watch)
-    const all_syms_count = syms.len + watch_syms.items.len;
+    const all_syms_count = syms.len + watch_syms.len;
 
     if (all_syms_count > 0) {
         // Use consolidated parallel loader
@@ -150,7 +162,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
             io,
             svc,
             syms,
-            watch_syms.items,
+            watch_syms,
             ctx.globals.refresh_policy,
             color,
         );
@@ -183,21 +195,11 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
 
     // Separate watchlist file (backward compat). Loaded here at the run
     // scope - NOT inside the collection block below - so its symbol
-    // strings outlive the `display` call at the end of `run`.
-    // `watch_list` (and `watch_prices`' keys) borrow these slices;
-    // freeing them before display - the previous behavior, where the
-    // load + `defer freeWatchlist` lived inside the collection block -
-    // rendered the file's watch symbols as freed-memory garbage on the
-    // CLI. The TUI is unaffected: it keeps watchlist symbols in its
-    // long-lived arena.
-    const wl_syms: ?[][]const u8 = if (watchlist_path) |wl_path|
-        cli.loadWatchlist(io, allocator, wl_path)
-    else
-        null;
-    defer cli.freeWatchlist(allocator, wl_syms);
-
     // Collect watch symbols and their prices for display.
     // Includes watch lots from portfolio + symbols from the watchlist file.
+    // `wl_syms` was loaded above, before the fetch, so these symbols are
+    // now in the fetch set rather than being priced from a cache nothing
+    // refreshes.
     var watch_list: std.ArrayList([]const u8) = .empty;
     defer watch_list.deinit(allocator);
     var watch_prices = std.StringHashMap(f64).init(allocator);

src/models/portfolio.zig

@@ -908,6 +908,144 @@ pub const Portfolio = struct {
         }
         return result.toOwnedSlice(allocator);
     }
+
+    /// Every symbol zfin fetches candles for: holdings, watch lots, the
+    /// separate `watchlist.srf`, and the benchmark pair.
+    ///
+    /// This exists because "what do we keep fresh?" had **five** disjoint
+    /// answers, and things fell through the gaps between them. Holdings came
+    /// from `Portfolio.stockSymbols`, watch lots from a hand-rolled loop in
+    /// each caller, `watchlist.srf` from `cli.loadWatchlist` (TUI only - the
+    /// CLI loaded it for display and priced it from cache, so a
+    /// watchlist-only symbol went arbitrarily stale), and the benchmark pair
+    /// from two hardcoded `getCandles(sym, .{})` calls on a lazy path that
+    /// only ran when someone opened projections. Observed consequences: SPCX
+    /// sat 39 days out of date while sitting in `watchlist.srf`, and AGG was
+    /// unreachable by `--refresh-data=force` entirely.
+    ///
+    /// One answer, shared by the CLI, the TUI and zfin-server, so a symbol
+    /// cannot be tracked by one and invisible to another.
+    ///
+    /// **Every returned string is duplicated into `allocator`.** Unlike
+    /// `stockSymbols`, which borrows from the portfolio, the inputs here have
+    /// mixed and shorter lifetimes - notably a benchmark override lives in a
+    /// `[16]u8` field inside a stack `UserConfig`, so borrowing it would
+    /// dangle the moment that config went out of scope. Caller owns the
+    /// result; free the slices and the outer slice, or use an arena.
+    pub fn fetchedSymbols(
+        self: Portfolio,
+        allocator: std.mem.Allocator,
+        opts: struct {
+            /// Symbols from a separate `watchlist.srf`.
+            watchlist_syms: []const []const u8 = &.{},
+            /// Benchmark symbols (e.g. the projections stock/bond pair).
+            /// Passed as plain strings so this stays free of any dependency
+            /// on the projections config.
+            benchmarks: []const []const u8 = &.{},
+        },
+    ) ![][]const u8 {
+        var seen = std.StringHashMap(void).init(allocator);
+        defer seen.deinit();
+
+        var result = std.ArrayList([]const u8).empty;
+        errdefer {
+            for (result.items) |s| allocator.free(s);
+            result.deinit(allocator);
+        }
+
+        // Owns nothing until the dupe succeeds, so `seen` keys borrow from
+        // `result` and stay valid for the whole build.
+        const add = struct {
+            fn f(
+                a: std.mem.Allocator,
+                set: *std.StringHashMap(void),
+                list: *std.ArrayList([]const u8),
+                sym: []const u8,
+            ) !void {
+                if (sym.len == 0) return;
+                if (set.contains(sym)) return;
+                const owned = try a.dupe(u8, sym);
+                // The errdefer is scoped to the append and no further, on purpose.
+                // Left armed across the `set.put` below it would double-free:
+                // `list` already owns `owned` by then, and the caller's errdefer
+                // frees everything in `list`. An allocation-failure test caught
+                // exactly that as a segfault.
+                {
+                    errdefer a.free(owned);
+                    try list.append(a, owned);
+                }
+                try set.put(owned, {});
+            }
+        }.f;
+
+        // Holdings. Skips options, CDs, cash, and manual-price-only lots -
+        // see `stockSymbols` for why each is excluded.
+        const held = try self.stockSymbols(allocator);
+        defer allocator.free(held);
+        for (held) |s| try add(allocator, &seen, &result, s);
+
+        // `security_type::watch` lots inside the portfolio file.
+        for (self.lots) |lot| {
+            if (lot.security_type != .watch) continue;
+            try add(allocator, &seen, &result, lot.priceSymbol());
+        }
+
+        for (opts.watchlist_syms) |s| try add(allocator, &seen, &result, s);
+        for (opts.benchmarks) |s| try add(allocator, &seen, &result, s);
+
+        return result.toOwnedSlice(allocator);
+    }
+
+    /// Symbols to price that are NOT stock positions: `security_type::watch` lots
+    /// in the portfolio file, plus every entry from a separate `watchlist.srf`,
+    /// excluding anything already in `held`.
+    ///
+    /// Separate from `fetchedSymbols` because the price loader takes holdings and
+    /// extras as two slices - it derives progress totals from the two counts - so a
+    /// single flat union does not fit there.
+    ///
+    /// It lives here rather than inline in the command for a testability reason
+    /// that bit once already: `commands/portfolio.zig`'s `run` needs a live
+    /// `RunCtx`, a `DataService` and the network, so its tests only ever exercise
+    /// `display`. Set logic embedded in `run` is untestable by construction, and
+    /// the version that was embedded there had a bug - it never included
+    /// `watchlist.srf` at all, leaving SPCX 39 days stale.
+    ///
+    /// Returned slices BORROW from `portfolio` and `watchlist_syms`; only the outer
+    /// slice is owned by the caller.
+    pub fn extraPriceSymbols(
+        self: Portfolio,
+        allocator: std.mem.Allocator,
+        held: []const []const u8,
+        watchlist_syms: []const []const u8,
+    ) ![][]const u8 {
+        var seen = std.StringHashMap(void).init(allocator);
+        defer seen.deinit();
+        for (held) |s| try seen.put(s, {});
+
+        var out = std.ArrayList([]const u8).empty;
+        errdefer out.deinit(allocator);
+
+        for (self.lots) |lot| {
+            if (lot.security_type != .watch) continue;
+            const sym = lot.priceSymbol();
+            if (sym.len == 0 or seen.contains(sym)) continue;
+            try seen.put(sym, {});
+            try out.append(allocator, sym);
+        }
+        for (watchlist_syms) |sym| {
+            if (sym.len == 0 or seen.contains(sym)) continue;
+            try seen.put(sym, {});
+            try out.append(allocator, sym);
+        }
+        return out.toOwnedSlice(allocator);
+    }
+
+    /// Free a `fetchedSymbols` result.
+    pub fn freeFetchedSymbols(allocator: std.mem.Allocator, syms: [][]const u8) void {
+        for (syms) |s| allocator.free(s);
+        allocator.free(syms);
+    }
 };
 
 /// Check if a string looks like a CUSIP (9 alphanumeric characters).
@@ -1299,9 +1437,9 @@ test "positions separates lots with different price_ratio" {
 
     var lots = [_]Lot{
         // Direct SPY holding, price_ratio = 1.0 (default)
-        .{ .symbol = "SPY", .shares = 717.34, .open_date = Date.fromYmd(2025, 2, 25), .open_price = 461.24, .account = "Tax Loss" },
+        .{ .symbol = "SPY", .shares = 100.0, .open_date = Date.fromYmd(2025, 2, 25), .open_price = 400.00, .account = "Sample Account" },
         // Institutional S&P 500 CIT, uses SPY as ticker with a ratio
-        .{ .symbol = "NON40OR52", .shares = 5070.866, .open_date = Date.fromYmd(2026, 2, 26), .open_price = 97.24, .ticker = "SPY", .price_ratio = 0.2381, .account = "Fidelity Riley 401(k)" },
+        .{ .symbol = "NON40OR52", .shares = 5000.0, .open_date = Date.fromYmd(2026, 2, 26), .open_price = 90.00, .ticker = "SPY", .price_ratio = 0.25, .account = "Fidelity Riley 401(k)" },
     };
 
     var portfolio = Portfolio{ .lots = &lots, .allocator = allocator };
@@ -1316,12 +1454,12 @@ test "positions separates lots with different price_ratio" {
     for (pos) |p| {
         if (p.price_ratio == 1.0) {
             found_direct = true;
-            try std.testing.expectApproxEqAbs(@as(f64, 717.34), p.shares, 0.01);
+            try std.testing.expectApproxEqAbs(@as(f64, 100.0), p.shares, 0.01);
             try std.testing.expectEqualStrings("SPY", p.lot_symbol);
         } else {
             found_institutional = true;
-            try std.testing.expectApproxEqAbs(@as(f64, 5070.866), p.shares, 0.01);
+            try std.testing.expectApproxEqAbs(@as(f64, 5000.0), p.shares, 0.01);
-            try std.testing.expectApproxEqAbs(@as(f64, 0.2381), p.price_ratio, 0.0001);
+            try std.testing.expectApproxEqAbs(@as(f64, 0.25), p.price_ratio, 0.0001);
             try std.testing.expectEqualStrings("NON40OR52", p.lot_symbol);
         }
     }
@@ -1752,3 +1890,226 @@ test "positionsAsOf reflects split_factor: effective shares, invariant basis, ef
     // (the whole point - raw 100 * 120 would undercount 10x).
     try std.testing.expectApproxEqAbs(@as(f64, 120000), positions[0].marketValue(120.0, false), 0.01);
 }
+
+// ── fetchedSymbols ───────────────────────────────────────────
+
+/// Build a Portfolio from lots for the union tests. Lots borrow from the
+/// caller; `fetchedSymbols` dupes everything it keeps, so that is safe.
+fn testPortfolio(lots: []Lot) Portfolio {
+    return .{ .lots = lots, .allocator = std.testing.allocator };
+}
+
+test "fetchedSymbols: unions all four sources and dedups across them" {
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+        // A ticker alias: the price symbol is what gets fetched, which is
+        // why SPY stayed fresh while AGG did not.
+        .{ .symbol = "NON40OR52", .ticker = "SPY", .shares = 5, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 90, .security_type = .stock },
+        .{ .symbol = "QTUM", .shares = 0, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 0, .security_type = .watch },
+        // Excluded by stockSymbols: manual price, no ticker alias.
+        .{ .symbol = "ORCBI", .shares = 3, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 10, .price = 11, .security_type = .stock },
+        // Excluded: not a stock or watch lot.
+        .{ .symbol = "CASHX", .shares = 1, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 1, .security_type = .cash },
+    };
+    const wl = [_][]const u8{ "SPCX", "AMZN" }; // AMZN duplicates a holding
+    const bm = [_][]const u8{ "SPY", "AGG" }; // SPY duplicates the alias above
+
+    const syms = try testPortfolio(&lots).fetchedSymbols(a, .{
+        .watchlist_syms = &wl,
+        .benchmarks = &bm,
+    });
+    defer Portfolio.freeFetchedSymbols(a, syms);
+
+    // AMZN, SPY, QTUM, SPCX, AGG - five distinct, no duplicates.
+    try std.testing.expectEqual(@as(usize, 5), syms.len);
+    for ([_][]const u8{ "AMZN", "SPY", "QTUM", "SPCX", "AGG" }) |want| {
+        var found = false;
+        for (syms) |s| if (std.mem.eql(u8, s, want)) {
+            found = true;
+        };
+        try std.testing.expect(found);
+    }
+    // Manual-price-only and cash lots stay out.
+    for (syms) |s| {
+        try std.testing.expect(!std.mem.eql(u8, s, "ORCBI"));
+        try std.testing.expect(!std.mem.eql(u8, s, "CASHX"));
+    }
+}
+
+test "fetchedSymbols: a watchlist-only symbol is included" {
+    // THE SPCX REGRESSION. It sat in watchlist.srf 39 days out of date
+    // because no CLI path ever put it in the fetch set - the CLI loaded
+    // the file for display and priced it from cache.
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+    };
+    const wl = [_][]const u8{"SPCX"};
+    const syms = try testPortfolio(&lots).fetchedSymbols(a, .{ .watchlist_syms = &wl });
+    defer Portfolio.freeFetchedSymbols(a, syms);
+
+    var found = false;
+    for (syms) |s| if (std.mem.eql(u8, s, "SPCX")) {
+        found = true;
+    };
+    try std.testing.expect(found);
+}
+
+test "fetchedSymbols: a benchmark symbol held nowhere is still included" {
+    // THE AGG REGRESSION. AGG is not held and not watched - it is the bond
+    // half of the benchmark comparison, fetched only from a lazy
+    // projections path with hardcoded default FetchOptions, so
+    // `--refresh-data=force` could never reach it.
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+    };
+    const bm = [_][]const u8{ "SPY", "AGG" };
+    const syms = try testPortfolio(&lots).fetchedSymbols(a, .{ .benchmarks = &bm });
+    defer Portfolio.freeFetchedSymbols(a, syms);
+
+    try std.testing.expectEqual(@as(usize, 3), syms.len);
+    var found_agg = false;
+    for (syms) |s| if (std.mem.eql(u8, s, "AGG")) {
+        found_agg = true;
+    };
+    try std.testing.expect(found_agg);
+}
+
+test "fetchedSymbols: empty and blank inputs produce no entries" {
+    const a = std.testing.allocator;
+    var lots = [_]Lot{};
+    const wl = [_][]const u8{""}; // blank line in watchlist.srf
+    const syms = try testPortfolio(&lots).fetchedSymbols(a, .{ .watchlist_syms = &wl });
+    defer Portfolio.freeFetchedSymbols(a, syms);
+    try std.testing.expectEqual(@as(usize, 0), syms.len);
+}
+
+test "fetchedSymbols: result outlives a stack-allocated benchmark override" {
+    // A projections override lives in a [16]u8 INSIDE the UserConfig
+    // struct, so borrowing it would dangle as soon as that config went out
+    // of scope. This is why the union dupes rather than borrows.
+    const a = std.testing.allocator;
+    var lots = [_]Lot{};
+    var syms: [][]const u8 = undefined;
+    {
+        var buf: [16]u8 = undefined;
+        @memcpy(buf[0..4], "VBIL");
+        const bm = [_][]const u8{buf[0..4]};
+        syms = try testPortfolio(&lots).fetchedSymbols(a, .{ .benchmarks = &bm });
+        @memset(&buf, 0xAA); // scribble over the source
+    }
+    defer Portfolio.freeFetchedSymbols(a, syms);
+    try std.testing.expectEqual(@as(usize, 1), syms.len);
+    try std.testing.expectEqualStrings("VBIL", syms[0]);
+}
+
+test "watchSymbols: watchlist.srf entries are included, holdings excluded" {
+    // THE SPCX BUG, at the layer where it actually lived. The version embedded
+    // in `commands/portfolio.zig` never looked at watchlist.srf at all, so a
+    // watchlist-only symbol was displayed from whatever the cache happened to
+    // hold - 39 days old, in SPCX's case - and never fetched.
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+        .{ .symbol = "QTUM", .shares = 0, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 0, .security_type = .watch },
+    };
+    const held = [_][]const u8{"AMZN"};
+    const wl = [_][]const u8{ "SPCX", "QTUM", "AMZN", "" };
+
+    const out = try testPortfolio(&lots).extraPriceSymbols(a, &held, &wl);
+    defer a.free(out);
+
+    // QTUM once (watch lot, deduped against the watchlist), SPCX from the
+    // file. AMZN is held so it belongs to the other slice, and the blank
+    // line is dropped.
+    try std.testing.expectEqual(@as(usize, 2), out.len);
+    try std.testing.expectEqualStrings("QTUM", out[0]);
+    try std.testing.expectEqualStrings("SPCX", out[1]);
+}
+
+test "watchSymbols: a ticker alias on a watch lot is priced by its alias" {
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "NON40OR52", .ticker = "SPY", .shares = 0, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 0, .security_type = .watch },
+    };
+    const out = try testPortfolio(&lots).extraPriceSymbols(a, &.{}, &.{});
+    defer a.free(out);
+    try std.testing.expectEqual(@as(usize, 1), out.len);
+    try std.testing.expectEqualStrings("SPY", out[0]);
+}
+
+test "watchSymbols: no watch lots and no watchlist yields an empty set" {
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+    };
+    const held = [_][]const u8{"AMZN"};
+    const out = try testPortfolio(&lots).extraPriceSymbols(a, &held, &.{});
+    defer a.free(out);
+    try std.testing.expectEqual(@as(usize, 0), out.len);
+}
+
+/// OOM-path wrapper for `checkAllAllocationFailures`.
+fn fetchedSymbolsOom(a: std.mem.Allocator, lots: []Lot, wl: []const []const u8, bm: []const []const u8) !void {
+    const syms = try (Portfolio{ .lots = lots, .allocator = a }).fetchedSymbols(a, .{
+        .watchlist_syms = wl,
+        .benchmarks = bm,
+    });
+    Portfolio.freeFetchedSymbols(a, syms);
+}
+
+fn watchSymbolsOom(a: std.mem.Allocator, lots: []Lot, held: []const []const u8, wl: []const []const u8) !void {
+    const out = try (Portfolio{ .lots = lots, .allocator = a }).extraPriceSymbols(a, held, wl);
+    a.free(out);
+}
+
+test "fetchedSymbols/watchSymbols: every allocation-failure path unwinds cleanly" {
+    // Covers the errdefer arms, which are otherwise unreachable: a partial
+    // build must free the strings it already duped, and the inner arm must
+    // free a dupe whose append then failed.
+    var lots = [_]Lot{
+        .{ .symbol = "AMZN", .shares = 10, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 200, .security_type = .stock },
+        .{ .symbol = "QTUM", .shares = 0, .open_date = Date.fromYmd(2026, 1, 2), .open_price = 0, .security_type = .watch },
+    };
+    const wl = [_][]const u8{"SPCX"};
+    const bm = [_][]const u8{ "SPY", "AGG" };
+    const held = [_][]const u8{"AMZN"};
+
+    try std.testing.checkAllAllocationFailures(
+        std.testing.allocator,
+        fetchedSymbolsOom,
+        .{ &lots, @as([]const []const u8, &wl), @as([]const []const u8, &bm) },
+    );
+    try std.testing.checkAllAllocationFailures(
+        std.testing.allocator,
+        watchSymbolsOom,
+        .{ &lots, @as([]const []const u8, &held), @as([]const []const u8, &wl) },
+    );
+}
+
+test "extraPriceSymbols: order is stable - watch lots first, then the watchlist file" {
+    // `PortfolioData.load` used to build this through a StringHashMap, so
+    // iteration order - and therefore the "[5/28] Loading X" progress order -
+    // varied run to run for no reason. Callers may now rely on the order.
+    const a = std.testing.allocator;
+    var lots = [_]Lot{
+        .{ .symbol = "AAPL", .shares = 10, .open_date = Date.fromYmd(2024, 1, 2), .open_price = 100, .security_type = .stock },
+        .{ .symbol = "TSLA", .shares = 0, .open_date = Date.fromYmd(2024, 1, 2), .open_price = 0, .security_type = .watch },
+        .{ .symbol = "NVDA", .shares = 0, .open_date = Date.fromYmd(2024, 1, 2), .open_price = 0, .security_type = .watch },
+    };
+    const held = [_][]const u8{"AAPL"};
+    const wl = [_][]const u8{ "MSFT", "QTUM" };
+
+    // Run twice: a hash-order build would be free to differ between calls.
+    for (0..2) |_| {
+        const out = try testPortfolio(&lots).extraPriceSymbols(a, &held, &wl);
+        defer a.free(out);
+        try std.testing.expectEqual(@as(usize, 4), out.len);
+        try std.testing.expectEqualStrings("TSLA", out[0]);
+        try std.testing.expectEqualStrings("NVDA", out[1]);
+        try std.testing.expectEqualStrings("MSFT", out[2]);
+        try std.testing.expectEqualStrings("QTUM", out[3]);
+    }
+}

src/tui/portfolio_tab.zig

@@ -420,10 +420,9 @@ pub const tab = struct {
             for (wl) |sym| watch_syms.append(a, sym) catch |err| std.log.debug("watch_syms append: {t}", .{err});
         }
 
-        // Symbols to quote live = watchlist + held stock symbols. Held
+        // Symbols to quote live = held + everything on either watch source.
-        // symbols are duped into the arena (see above).
+        // Held symbols are duped into the arena (see above).
         var quote_syms: std.ArrayList([]const u8) = .empty;
-        for (watch_syms.items) |sym| quote_syms.append(a, sym) catch |err| std.log.debug("quote_syms append: {t}", .{err});
         if (app.portfolio.file) |pf| {
             if (pf.stockSymbols(app.allocator)) |hs| {
                 defer app.allocator.free(hs); // outer slice; strings duped into arena
@@ -431,6 +430,18 @@ pub const tab = struct {
                     const dup = a.dupe(u8, sym) catch continue;
                     quote_syms.append(a, dup) catch |err| std.log.debug("quote_syms append: {t}", .{err});
                 }
+                // The same union the price load uses, so live quotes and
+                // candles cover the same set. Previously this took only
+                // `watchlist.srf` and skipped the portfolio's own `.watch`
+                // lots, so those rows silently fell back to the prior close
+                // while watchlist rows updated intraday - the same
+                // two-sources-of-watch-symbols split that left a
+                // watchlist-only symbol unfetched on the CLI, mirrored.
+                // Borrowed, not duped: these point into `pf.lots` and
+                // `app.watchlist`, both stable across this call.
+                if (pf.extraPriceSymbols(a, hs, watch_syms.items)) |extra| {
+                    for (extra) |sym| quote_syms.append(a, sym) catch |err| std.log.debug("quote_syms append: {t}", .{err});
+                } else |err| std.log.debug("extraPriceSymbols for live quotes: {t}", .{err});
             } else |err| std.log.debug("stockSymbols for live quotes: {t}", .{err});
         }