add observation for missing dividend history

src/PortfolioData.zig

@@ -87,6 +87,7 @@ const Dividend = zfin.Dividend;
 const AccountMap = zfin.analysis.AccountMap;
 const ClassificationMap = zfin.classification.ClassificationMap;
 const DataService = zfin.DataService;
+const FetchOptions = zfin.FetchOptions;
 
 // ── Public types ──────────────────────────────────────────────
 
@@ -258,6 +259,13 @@ candles_arena: ArenaAllocator,
 io: std.Io,
 svc: *DataService,
 
+/// Cache policy for the current load. Candles and splits are warmed
+/// synchronously inside `load()`, where `LoadOptions` is still in scope, so
+/// only the dividends warm needs this: it runs inside a worker (to keep
+/// Polygon's 4/min bucket off the first-paint path) and a worker cannot see
+/// `opts`. Reset on every load/reload.
+fetch_opts: FetchOptions = .{},
+
 /// Parsed portfolio file path(s). Arena-owned `[]const u8`
 /// strings; arena-owned outer slice. `paths[0]` is the anchor
 /// for sibling-file derivation. Empty before the first load.
@@ -598,6 +606,12 @@ pub fn load(
     self.account_map_data = null;
     self.classification_map_data = null;
 
+    // Capture the cache policy for the workers that fetch (dividends).
+    self.fetch_opts = .{
+        .force_refresh = opts.force_refresh,
+        .skip_network = opts.skip_network,
+    };
+
     // candles_data lives in candles_arena and survives across
     // reloads - kept entries are reused, only new symbols hit
     // the cache. force_refresh wipes it wholesale.
@@ -642,7 +656,7 @@ pub fn load(
     // Opt-in split adjustment before positions are aggregated, so the
     // TUI's positions and valuation carry effective shares. No-op
     // unless `splits_current_through` is set in metadata.srf.
-    portfolio_loader.enrichLotsSplits(self.svc, gpa, pf.lots, syms, self.paths[0], today);
+    portfolio_loader.enrichLotsSplits(self.svc, gpa, pf.lots, syms, self.paths[0], today, self.fetch_opts);
 
     const positions = pf.positions(today, gpa) catch return error.NoAllocations;
     defer gpa.free(positions);
@@ -988,10 +1002,40 @@ fn snapshotsWorker(self: *PortfolioData, as_of: Date, positions: []const zfin.Po
     );
 }
 
+/// Warm the dividend cache, then read it into the map.
+///
+/// The warm lives here rather than in `load()` on purpose. Polygon serves
+/// dividends from a 4/min bucket, so a cold cache with no `ZFIN_SERVER`
+/// could take minutes - unacceptable in front of a first paint. Inside the
+/// worker, that cost only lands when a consumer calls `dividends()` and
+/// blocks on the future, which today is the review tab alone. `load()`
+/// still initiates it, matching every other datum.
+///
+/// Unconditional: unlike split adjustment, dividends have no per-symbol
+/// opt-in to gate on.
 fn dividendsWorker(self: *PortfolioData, delay_ms: usize) void {
     self.io.sleep(.fromMilliseconds(@intCast(delay_ms)), .real) catch return;
     const summary_ref = self.summary orelse return;
     const arena_alloc = self.allocator();
+
+    // Warm first: `getCachedDividends` below cannot populate the cache, so
+    // without this the map silently omits every symbol never fetched by a
+    // per-symbol command. The warm is best-effort - on allocation failure
+    // we skip it and still read whatever is already cached, because a
+    // failed optimization must not cost us the data we already have.
+    //
+    // Skipped entirely under `skip_network`: there is nothing to warm, the
+    // read loop below already covers the cache, and going through
+    // `getDividends` would emit a "stale-cached returned (skip_network)"
+    // info line per symbol for no benefit.
+    warm: {
+        if (self.fetch_opts.skip_network) break :warm;
+        const div_syms = arena_alloc.alloc([]const u8, summary_ref.allocations.len) catch break :warm;
+        for (summary_ref.allocations, div_syms) |alloc, *dst| dst.* = alloc.symbol;
+        self.io.checkCancel() catch return;
+        self.svc.loadAllDividends(div_syms, self.fetch_opts);
+    }
+
     var map = std.StringHashMap([]const Dividend).init(arena_alloc);
     for (summary_ref.allocations) |alloc| {
         self.io.checkCancel() catch return;

src/analytics/observations.zig

@@ -357,6 +357,11 @@ pub const default_checks = [_]Check{
         .label = "Tiny position",
         .run = checkTinyPosition,
     },
+    .{
+        .name = "dividend_data_missing",
+        .label = "Dividend data",
+        .run = checkDividendData,
+    },
     .{
         .name = "drift",
         .label = "Drift since last view",
@@ -731,6 +736,59 @@ fn checkTinyPosition(ctx: CheckCtx) CheckResult {
     return .pass;
 }
 
+/// Holdings whose trailing returns are price-only because no dividend
+/// history was available for them.
+///
+/// This is a data-quality check, not a portfolio-risk one: the numbers in
+/// the table are wrong (understated by roughly the yield), and for an
+/// income-oriented holding that is most of the return. It exists because
+/// the failure is otherwise completely silent - a missing dividend cache
+/// entry renders as a perfectly plausible total-return figure.
+///
+/// Only fires on genuine absence. A holding present in the dividend map
+/// with zero records pays no dividends, which is a correct answer and gets
+/// no finding. See `ReviewRow.dividends_missing`.
+fn checkDividendData(ctx: CheckCtx) CheckResult {
+    if (ctx.rows.len == 0) return .pass;
+
+    var findings = std.ArrayList(Observation).empty;
+    errdefer {
+        for (findings.items) |o| {
+            ctx.allocator.free(o.kind);
+            ctx.allocator.free(o.target);
+            ctx.allocator.free(o.text);
+        }
+        findings.deinit(ctx.allocator);
+    }
+
+    for (ctx.rows) |row| {
+        if (!row.dividends_missing) continue;
+        const text = std.fmt.allocPrint(ctx.allocator, "{s} has no dividend history - its returns are price-only and understate total return. Check POLYGON_API_KEY and `zfin divs {s}`", .{
+            row.symbol,
+            row.symbol,
+        }) catch return errResult(ctx.allocator, "alloc failed");
+        const target = ctx.allocator.dupe(u8, row.symbol) catch {
+            ctx.allocator.free(text);
+            return errResult(ctx.allocator, "alloc failed");
+        };
+        const kind = ctx.allocator.dupe(u8, "dividend_data_missing") catch {
+            ctx.allocator.free(text);
+            ctx.allocator.free(target);
+            return errResult(ctx.allocator, "alloc failed");
+        };
+        findings.append(ctx.allocator, .{
+            .severity = .warn,
+            .kind = kind,
+            .target = target,
+            .text = text,
+        }) catch return errResult(ctx.allocator, "alloc failed");
+    }
+
+    if (findings.items.len == 0) return .pass;
+    const slice = findings.toOwnedSlice(ctx.allocator) catch return errResult(ctx.allocator, "alloc failed");
+    return .{ .warn = slice };
+}
+
 /// Drift since last view. Currently a placeholder - returns `skipped`
 /// until temporal observations ship in a follow-up. The forward-compat
 /// slot in the status grid stays visible (rendered as ➖) so users
@@ -1058,6 +1116,59 @@ test "checkTinyPosition: positions below thresholds flag" {
     }
 }
 
+test "checkDividendData: warns only on genuine absence, not on a holding that pays nothing" {
+    var rows = [_]review_view.ReviewRow{
+        makeRow("AAA", "X", 0.50),
+        makeRow("BBB", "Y", 0.50),
+    };
+
+    // Neither row is missing data. BBB may well pay no dividends at all -
+    // that is a real answer and must stay silent, which is the entire point
+    // of tracking absence separately from emptiness.
+    {
+        const result = checkDividendData(.{
+            .allocator = testing.allocator,
+            .rows = &rows,
+            .totals = emptyTotals(),
+        });
+        defer freeResult(testing.allocator, result);
+        try testing.expect(result == .pass);
+    }
+
+    // BBB's dividend fetch never landed: its returns are price-only.
+    rows[1].dividends_missing = true;
+    {
+        const result = checkDividendData(.{
+            .allocator = testing.allocator,
+            .rows = &rows,
+            .totals = emptyTotals(),
+        });
+        defer freeResult(testing.allocator, result);
+        switch (result) {
+            .warn => |obs| {
+                try testing.expectEqual(@as(usize, 1), obs.len);
+                try testing.expectEqualStrings("BBB", obs[0].target);
+                try testing.expectEqualStrings("dividend_data_missing", obs[0].kind);
+                // The text must say which way the number is wrong, not just
+                // that something is missing.
+                try testing.expect(std.mem.indexOf(u8, obs[0].text, "price-only") != null);
+                try testing.expect(std.mem.indexOf(u8, obs[0].text, "understate") != null);
+            },
+            else => return error.TestUnexpectedResult,
+        }
+    }
+}
+
+test "checkDividendData: no rows is a pass" {
+    const result = checkDividendData(.{
+        .allocator = testing.allocator,
+        .rows = &.{},
+        .totals = emptyTotals(),
+    });
+    defer freeResult(testing.allocator, result);
+    try testing.expect(result == .pass);
+}
+
 test "checkDrift: returns skipped (placeholder)" {
     const ctx: CheckCtx = .{
         .allocator = testing.allocator,

src/commands/audit/hygiene.zig

@@ -1346,7 +1346,7 @@ pub fn runHygieneCheck(
     // split. Cache-only detection; silent when everything is handled.
     if (portfolio.stockSymbols(allocator)) |split_syms| {
         defer allocator.free(split_syms);
-        const unhandled = cli.findUnhandledSplits(svc, allocator, portfolio.lots, split_syms, portfolio_path, as_of);
+        const unhandled = cli.findUnhandledSplits(svc, allocator, portfolio.lots, split_syms, portfolio_path, as_of, cli.fetchOptionsFromPolicy(refresh));
         defer allocator.free(unhandled);
         if (unhandled.len > 0) {
             try out.print("\n", .{});

src/commands/common.zig

@@ -428,7 +428,7 @@ pub fn loadPortfolio(ctx: *framework.RunCtx, as_of: zfin.Date) ?LoadedPortfolio
     // `splits_current_through` in metadata.srf. No-op otherwise, so
     // every existing portfolio behaves exactly as before.
     if (ctx.svc) |svc| {
-        portfolio_loader.applySplitAdjustment(svc, ctx.allocator, &loaded, as_of);
+        portfolio_loader.applySplitAdjustment(svc, ctx.allocator, &loaded, as_of, fetchOptionsFromPolicy(ctx.globals.refresh_policy));
     }
 
     return loaded;

src/commands/review.zig

@@ -191,9 +191,12 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     var acct_map_opt: ?zfin.analysis.AccountMap = svc.loadAccountMap(allocator, anchor_path);
     defer if (acct_map_opt) |*am| am.deinit();
 
-    // Per-symbol cached dividends so total-return windows include
-    // dividend reinvestment when available. Cached-only - no
-    // network - to keep the command fast on large portfolios.
+    // Per-symbol dividends so total-return windows include dividend
+    // reinvestment. Warmed first: `getCachedDividends` cannot populate the
+    // cache, so reading it alone silently omitted every symbol the user had
+    // never inspected with `divs`/`perf` - and an omitted symbol degrades to
+    // a price-only return with no indication. Honors --refresh-data, so
+    // `never` still reads cache only.
     var dividend_map = std.StringHashMap([]const zfin.Dividend).init(allocator);
     defer {
         var it = dividend_map.iterator();
@@ -202,6 +205,15 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
         }
         dividend_map.deinit();
     }
+    {
+        var div_syms = try std.ArrayList([]const u8).initCapacity(
+            allocator,
+            pf_data.summary.allocations.len,
+        );
+        defer div_syms.deinit(allocator);
+        for (pf_data.summary.allocations) |a| div_syms.appendAssumeCapacity(a.symbol);
+        svc.loadAllDividends(div_syms.items, cli.fetchOptionsFromPolicy(ctx.globals.refresh_policy));
+    }
     for (pf_data.summary.allocations) |a| {
         if (svc.getCachedDividends(allocator, a.symbol)) |divs| {
             try dividend_map.put(a.symbol, divs.data);

src/commands/snapshot.zig

@@ -336,7 +336,7 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void {
     }
 
     // Build and render the snapshot.
-    var snap = try captureSnapshot(io, allocator, &portfolio, portfolio_path, svc, prices, symbol_prices, syms, as_of, qdates, now_s);
+    var snap = try captureSnapshot(io, allocator, &portfolio, portfolio_path, svc, prices, symbol_prices, syms, as_of, qdates, now_s, cli.fetchOptionsFromPolicy(ctx.globals.refresh_policy));
     defer snap.deinit(allocator);
 
     const rendered = try renderSnapshot(allocator, snap);
@@ -678,6 +678,7 @@ fn captureSnapshot(
     as_of: Date,
     qdates: QuoteDates,
     now_s: i64,
+    fetch_opts: zfin.FetchOptions,
 ) !Snapshot {
     // Use `positionsAsOf(as_of)` rather than `positions()` so historical
     // backfills correctly count lots that were held on `as_of`
@@ -688,7 +689,7 @@ fn captureSnapshot(
     // Effective shares flow into the summary (via positionsAsOf) and
     // into each per-lot `value` (via marketValue); the stored `.shares`
     // field stays RAW - a snapshot is a frozen historical record.
-    portfolio_loader.enrichLotsSplits(svc, allocator, portfolio.lots, syms, portfolio_path, as_of);
+    portfolio_loader.enrichLotsSplits(svc, allocator, portfolio.lots, syms, portfolio_path, as_of, fetch_opts);
 
     const positions = try portfolio.positionsAsOf(allocator, as_of);
     defer allocator.free(positions);

src/portfolio_loader.zig

@@ -550,6 +550,11 @@ pub fn buildPortfolioData(
 /// cutovers and the fetched split corpus. A no-op (and skips the corpus
 /// fetch entirely) when no symbol has opted in.
 ///
+/// `opts` is the caller's cache policy and must be threaded from the
+/// invocation's `--refresh-data` setting. It used to be hardcoded to
+/// defaults here, which silently fetched splits over the network even under
+/// `--refresh-data=never`.
+///
 /// This mutates only `lots[].split_factor`; callers that hold an
 /// already-computed positions slice must recompute it afterward (the
 /// CLI wrapper `applySplitAdjustment` does this). Used directly by the
@@ -565,12 +570,13 @@ pub fn enrichLotsSplits(
     syms: []const []const u8,
     anchor_path: []const u8,
     as_of: zfin.Date,
+    opts: zfin.FetchOptions,
 ) void {
     var cutovers = svc.loadSplitsCutovers(allocator, anchor_path);
     defer freeCutovers(allocator, &cutovers);
     if (cutovers.count() == 0) return; // nothing opted in -> skip the corpus fetch
 
-    var corpus = svc.loadAllSplits(allocator, syms, .{});
+    var corpus = svc.loadAllSplits(allocator, syms, opts);
     defer {
         var it = corpus.valueIterator();
         while (it.next()) |v| allocator.free(v.*);
@@ -599,14 +605,24 @@ pub const SplitNudge = struct {
 };
 
 /// Collect every held stock symbol that has NOT opted into split
-/// adjustment yet has a split after a lot's purchase date, in the CACHED
-/// split data (no network). One entry per symbol (first qualifying
-/// split). Powers the `audit` hygiene "unhandled stock splits" section.
+/// adjustment yet has a split after a lot's purchase date. One entry per
+/// symbol (first qualifying split). Powers the `audit` hygiene "unhandled
+/// stock splits" section.
 ///
 /// Caller owns the returned slice (`allocator.free` it); each `symbol`
 /// borrows from `lots`, so use the result before the portfolio is freed.
-/// Cheap: cache-only reads. Returns an empty slice on any allocation
-/// failure (hygiene is best-effort).
+/// Returns an empty slice on any allocation failure (hygiene is
+/// best-effort).
+///
+/// `opts` is the caller's cache policy. This used to hardcode
+/// `skip_network`, which made the check cheap but meant flagless `zfin
+/// audit` - which never warms the split cache, since it returns into
+/// hygiene before `applySplitAdjustment` runs - reported findings from
+/// whatever stale data happened to be on disk, and logged a
+/// "stale-cached returned (skip_network)" line per symbol while doing it.
+/// A hygiene check that tells you your data is stale instead of
+/// refreshing it is the wrong trade; pass the invocation's real policy so
+/// `--refresh-data=never` still gets cache-only behavior on request.
 pub fn findUnhandledSplits(
     svc: *zfin.DataService,
     allocator: std.mem.Allocator,
@@ -614,11 +630,12 @@ pub fn findUnhandledSplits(
     syms: []const []const u8,
     anchor_path: []const u8,
     as_of: zfin.Date,
+    opts: zfin.FetchOptions,
 ) []SplitNudge {
     var cutovers = svc.loadSplitsCutovers(allocator, anchor_path);
     defer freeCutovers(allocator, &cutovers);
 
-    var corpus = svc.loadAllSplits(allocator, syms, .{ .skip_network = true });
+    var corpus = svc.loadAllSplits(allocator, syms, opts);
     defer {
         var it = corpus.valueIterator();
         while (it.next()) |v| allocator.free(v.*);
@@ -666,8 +683,9 @@ pub fn applySplitAdjustment(
     allocator: std.mem.Allocator,
     loaded: *LoadedPortfolio,
     as_of: zfin.Date,
+    opts: zfin.FetchOptions,
 ) void {
-    enrichLotsSplits(svc, allocator, loaded.portfolio.lots, loaded.syms, loaded.anchor(), as_of);
+    enrichLotsSplits(svc, allocator, loaded.portfolio.lots, loaded.syms, loaded.anchor(), as_of, opts);
 
     // Positions were aggregated from raw lots at load time; recompute
     // from the (now possibly enriched) lots. On failure, keep the
@@ -979,7 +997,7 @@ test "applySplitAdjustment: end-to-end enriches loaded positions from seeded cac
     // Before enrichment: raw 100 shares.
     try testing.expectApproxEqAbs(@as(f64, 100), loaded.positions[0].shares, 0.001);
 
-    applySplitAdjustment(&svc, allocator, &loaded, zfin.Date.fromYmd(2026, 1, 1));
+    applySplitAdjustment(&svc, allocator, &loaded, zfin.Date.fromYmd(2026, 1, 1), .{ .skip_network = true });
 
     // After: 100 * 10 = 1000 effective shares; factor stamped on the lot;
     // cost basis stays invariant (100 * 40 = 4000).
@@ -1009,13 +1027,52 @@ test "applySplitAdjustment: no cutover is a no-op (raw shares preserved)" {
         return error.TestUnexpectedResult;
     defer loaded.deinit(allocator);
 
-    applySplitAdjustment(&svc, allocator, &loaded, zfin.Date.fromYmd(2026, 1, 1));
+    applySplitAdjustment(&svc, allocator, &loaded, zfin.Date.fromYmd(2026, 1, 1), .{ .skip_network = true });
 
     // Opt-in off -> factor stays 1.0, shares stay raw.
     try testing.expectApproxEqAbs(@as(f64, 1.0), loaded.portfolio.lots[0].split_factor, 0.001);
     try testing.expectApproxEqAbs(@as(f64, 100), loaded.positions[0].shares, 0.001);
 }
 
+test "enrichLotsSplits: skip_network is honored even when the split cache is stale" {
+    // Regression: this used to hardcode default FetchOptions, so
+    // `--refresh-data=never` still hit the network for splits. A FRESH cache
+    // cannot catch that (fetchCached short-circuits before any network), so
+    // the seeded entry is deliberately expired to force the decision point.
+    const allocator = testing.allocator;
+    const io = testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+
+    var path_buf: [std.fs.max_path_bytes]u8 = undefined;
+    const dir = try seedSplitFixture(io, &tmp, &path_buf, "symbol::NVDA,splits_current_through::2024-01-01\n");
+
+    // Re-seed the same split with a long-expired TTL.
+    {
+        var store = zfin.cache.Store.init(io, allocator, dir);
+        var splits = [_]zfin.Split{.{ .date = zfin.Date.fromYmd(2024, 6, 10), .numerator = 10, .denominator = 1 }};
+        store.write(zfin.Split, "NVDA", splits[0..], .{ .seconds = -1_000_000 });
+    }
+
+    var svc = zfin.DataService.init(io, allocator, .{ .cache_dir = dir });
+    defer svc.deinit();
+
+    const pf_path = try std.fs.path.join(allocator, &.{ dir, "zfintest_split_pf.srf" });
+    defer allocator.free(pf_path);
+    const paths = try allocator.dupe([]const u8, &.{pf_path});
+    defer allocator.free(paths);
+    var loaded = loadPortfolioFromPaths(io, allocator, paths, zfin.Date.fromYmd(2026, 1, 1)) orelse
+        return error.TestUnexpectedResult;
+    defer loaded.deinit(allocator);
+
+    // Any provider or server call from here on is a policy violation.
+    svc.panic_on_network_attempt = true;
+    applySplitAdjustment(&svc, allocator, &loaded, zfin.Date.fromYmd(2026, 1, 1), .{ .skip_network = true });
+
+    // Offline still applies the stale split it already had: 100 -> 1000.
+    try testing.expectApproxEqAbs(@as(f64, 1000), loaded.positions[0].shares, 0.001);
+}
+
 test "findUnhandledSplits: flags post-purchase splits for un-opted-in symbols only" {
     const allocator = testing.allocator;
     const io = testing.io;
@@ -1037,7 +1094,7 @@ test "findUnhandledSplits: flags post-purchase splits for un-opted-in symbols on
     const syms = [_][]const u8{"NVDA"};
 
     // Opt-in off + a split after purchase -> one finding.
-    const found = findUnhandledSplits(&svc, allocator, &lots, &syms, anchor, zfin.Date.fromYmd(2026, 1, 1));
+    const found = findUnhandledSplits(&svc, allocator, &lots, &syms, anchor, zfin.Date.fromYmd(2026, 1, 1), .{ .skip_network = true });
     defer allocator.free(found);
     try testing.expectEqual(@as(usize, 1), found.len);
     try testing.expectEqualStrings("NVDA", found[0].symbol);
@@ -1047,7 +1104,7 @@ test "findUnhandledSplits: flags post-purchase splits for un-opted-in symbols on
     var lots_after = [_]zfin.Lot{
         .{ .symbol = "NVDA", .shares = 5, .open_date = zfin.Date.fromYmd(2025, 1, 1), .open_price = 120 },
     };
-    const none = findUnhandledSplits(&svc, allocator, &lots_after, &syms, anchor, zfin.Date.fromYmd(2026, 1, 1));
+    const none = findUnhandledSplits(&svc, allocator, &lots_after, &syms, anchor, zfin.Date.fromYmd(2026, 1, 1), .{ .skip_network = true });
     defer allocator.free(none);
     try testing.expectEqual(@as(usize, 0), none.len);
 }

src/service.zig

@@ -510,7 +510,8 @@ pub const DataService = struct {
             if (err == error.RateLimited) {
                 // Wait and retry once
                 self.rateLimitBackoff();
-                const retried = self.fetchFromProvider(T, symbol) catch {
+                const retried = self.fetchFromProvider(T, symbol) catch |retry_err| {
+                    log.warn("{s}: {s} fetch failed after rate-limit retry: {t}", .{ symbol, @tagName(data_type), retry_err });
                     return DataError.FetchFailed;
                 };
                 s.writeWithSource(T, symbol, retried, data_type.ttl(), sourceHintFor(T));
@@ -521,8 +522,18 @@ pub const DataService = struct {
             // Transient failures (network, 5xx, auth misconfig, parse
             // error) propagate as FetchFailed without poisoning the
             // cache, so the next call retries naturally.
+            //
+            // Log the provider's own error either way: the typed return
+            // collapses to FetchFailed, so this line is the only place the
+            // distinction between RateLimited, Unauthorized and NotFound
+            // survives. Callers that swallow failures per symbol (see
+            // `loadAllDividends`) depend on it.
             if (isPermanentProviderFailure(err)) {
+                // The normal "this symbol has no data of this type" outcome.
+                log.info("{s}: {s} unavailable: {t}", .{ symbol, @tagName(data_type), err });
                 s.writeNegative(symbol, data_type);
+            } else {
+                log.warn("{s}: {s} fetch failed: {t}", .{ symbol, @tagName(data_type), err });
             }
             return DataError.FetchFailed;
         };
@@ -3238,6 +3249,44 @@ pub const DataService = struct {
         }
         return corpus;
     }
+
+    /// Warm the dividend cache for each symbol. Warm-only: nothing is
+    /// returned, because the sole consumer (`PortfolioData`'s dividends
+    /// worker) reads the cache immediately afterward. That is the whole
+    /// difference from `loadAllSplits`, whose corpus feeds `enrichSplits`
+    /// inline.
+    ///
+    /// Why this exists: dividends were the only per-symbol data type with
+    /// a portfolio-wide reader and no portfolio-wide writer. Candles are
+    /// warmed by `loadAllPrices` and splits by `loadAllSplits`, but nothing
+    /// warmed dividends, so `getCachedDividends` read a cache that only
+    /// per-symbol commands (`divs`, `perf`) had ever populated. The visible
+    /// symptom was `views/review.zig` silently reporting price-only
+    /// trailing returns for every symbol the user had never inspected
+    /// individually.
+    ///
+    /// Sequential on purpose. The expensive phase is the provider, and
+    /// Polygon serves both dividends and splits from one 4/min bucket, so
+    /// concurrency cannot speed that up; the server-sync phase is a
+    /// sub-second serial cost for a normal portfolio. Rate limiting needs
+    /// no wiring here - it lives in the provider, so every `getDividends`
+    /// call is already throttled.
+    ///
+    /// Failures are swallowed per symbol: a missing dividend history
+    /// degrades a total return to price-only, which is not worth failing
+    /// a whole portfolio load over. `fetchCached` logs the provider's own
+    /// error (rate limit vs auth vs no-such-data) before collapsing it to
+    /// `FetchFailed`, so a swallowed failure is still diagnosable.
+    pub fn loadAllDividends(
+        self: *DataService,
+        syms: []const []const u8,
+        opts: FetchOptions,
+    ) void {
+        for (syms) |sym| {
+            const fr = self.getDividends(sym, opts) catch continue;
+            fr.deinit();
+        }
+    }
 };
 
 // ── Tests ─────────────────────────────────────────────────────────
@@ -3642,6 +3691,58 @@ test "fetchCached offline mode returns stale-cached data" {
     try std.testing.expectEqual(Source.cached, result.source);
 }
 
+test "loadAllDividends: honors skip_network for every symbol, and one miss does not abort the rest" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const config = Config{ .cache_dir = dir_path };
+    var svc = DataService.init(io, allocator, config);
+    defer svc.deinit();
+
+    // TSTB is cached; TSTA is absent. TSTA comes first, so if a per-symbol
+    // failure aborted the loop, TSTB would never be reached.
+    var divs = [_]Dividend{
+        .{ .ex_date = Date.fromYmd(2026, 3, 15), .amount = 0.50, .type = .regular },
+    };
+    var store = svc.store();
+    store.write(Dividend, "TSTB", divs[0..], cache.DataType.dividends.ttl());
+
+    // The whole loop must stay offline, not just the first symbol.
+    svc.panic_on_network_attempt = true;
+    svc.loadAllDividends(&.{ "TSTA", "TSTB" }, .{ .skip_network = true });
+
+    // The cached symbol survives the pass.
+    const b = svc.getCachedDividends(allocator, "TSTB") orelse return error.TestUnexpectedResult;
+    defer b.deinit();
+    try std.testing.expectEqual(@as(usize, 1), b.data.len);
+
+    // The absent symbol stays absent - a warm must not leave a negative or
+    // empty entry behind that would mask a later real fetch.
+    try std.testing.expect(svc.getCachedDividends(allocator, "TSTA") == null);
+}
+
+test "loadAllDividends: empty symbol list is a no-op" {
+    const allocator = std.testing.allocator;
+    const io = std.testing.io;
+    var tmp = std.testing.tmpDir(.{});
+    defer tmp.cleanup();
+    const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator);
+    defer allocator.free(dir_path);
+
+    const config = Config{ .cache_dir = dir_path };
+    var svc = DataService.init(io, allocator, config);
+    defer svc.deinit();
+
+    // No symbols means no fetches, so this must hold even with network
+    // otherwise allowed.
+    svc.panic_on_network_attempt = true;
+    svc.loadAllDividends(&.{}, .{});
+}
+
 test "getQuote offline mode returns FetchFailed (quotes never cached)" {
     const allocator = std.testing.allocator;
     const io = std.testing.io;

src/views/review.zig

@@ -108,6 +108,17 @@ pub const ReviewRow = struct {
     sharpe_10y: ?f64,
     /// 5Y max drawdown (positive decimal, e.g. 0.30 = 30%).
     maxdd_5y: ?f64,
+    /// True when a dividend map was supplied but had no entry for this
+    /// symbol, so the trailing returns above fell back to the
+    /// adj_close-only path and understate total return by roughly the
+    /// yield.
+    ///
+    /// Deliberately distinct from "present with zero records", which is a
+    /// real answer (the holding pays nothing) and needs no warning. Absence
+    /// means the fetch never succeeded. Collapsing the two is what let a
+    /// cold dividend cache masquerade as a correct total return for as long
+    /// as the review table has existed.
+    dividends_missing: bool = false,
 };
 
 /// Bottom totals row.
@@ -204,10 +215,20 @@ pub fn buildReview(
 
     for (summary.allocations) |a| {
         const candles = candle_map.get(a.symbol) orelse &.{};
-        const dividends: ?[]const zfin.Dividend = if (dividend_map) |dm|
-            (dm.get(a.symbol) orelse null)
-        else
-            null;
+
+        // Absent from the map and present-but-empty are different facts and
+        // must not collapse: the first means the fetch failed and the
+        // returns below are price-only; the second means the holding pays
+        // nothing, which is a correct and unremarkable answer.
+        var dividends: ?[]const zfin.Dividend = null;
+        var dividends_missing = false;
+        if (dividend_map) |dm| {
+            if (dm.get(a.symbol)) |d| {
+                dividends = d;
+            } else {
+                dividends_missing = true;
+            }
+        }
 
         const bucket = bucketForSymbol(a.symbol, classifications);
         const tax_pct = computeTaxPct(a.symbol, portfolio, account_map, as_of);
@@ -229,6 +250,7 @@ pub fn buildReview(
             .sharpe_3y = if (tr_risk.three_year) |m| m.sharpe else null,
             .sharpe_10y = if (tr_risk.ten_year) |m| m.sharpe else null,
             .maxdd_5y = if (tr_risk.five_year) |m| m.max_drawdown else null,
+            .dividends_missing = dividends_missing,
         });
 
         try positions.append(allocator, .{
@@ -1186,6 +1208,111 @@ test "returnIntent: signs map to gain/loss colors" {
     try testing.expectEqual(format.StyleIntent.normal, returnIntent(0.0));
 }
 
+test "buildReview: dividends_missing separates a failed fetch from a holding that pays nothing" {
+    // Regression: this used to be `dm.get(sym) orelse null`, which collapsed
+    // "never fetched" into "pays nothing". Both took the adj_close-only
+    // path, so a cold dividend cache rendered as a plausible total return
+    // with no indication anywhere that the number was price-only.
+    const D = zfin.Date;
+    const Lot = @import("../models/portfolio.zig").Lot;
+
+    var allocs = [_]valuation.Allocation{
+        .{
+            .symbol = "AAA",
+            .display_symbol = "AAA",
+            .shares = 10,
+            .avg_cost = 10,
+            .current_price = 12,
+            .market_value = 120,
+            .cost_basis = 100,
+            .weight = 0.5,
+            .unrealized_gain_loss = 20,
+            .unrealized_return = 0.2,
+            .account = "Sample Brokerage",
+        },
+        .{
+            .symbol = "BBB",
+            .display_symbol = "BBB",
+            .shares = 10,
+            .avg_cost = 10,
+            .current_price = 12,
+            .market_value = 120,
+            .cost_basis = 100,
+            .weight = 0.5,
+            .unrealized_gain_loss = 20,
+            .unrealized_return = 0.2,
+            .account = "Sample Brokerage",
+        },
+    };
+    const summary: valuation.PortfolioSummary = .{
+        .total_value = 240,
+        .total_cost = 200,
+        .unrealized_gain_loss = 40,
+        .unrealized_return = 0.2,
+        .realized_gain_loss = 0,
+        .allocations = allocs[0..],
+    };
+    var lots = [_]Lot{
+        .{ .symbol = "AAA", .shares = 10, .open_date = D.fromYmd(2022, 1, 10), .open_price = 10, .account = "Sample Brokerage" },
+        .{ .symbol = "BBB", .shares = 10, .open_date = D.fromYmd(2022, 1, 10), .open_price = 10, .account = "Sample Brokerage" },
+    };
+    const portfolio: zfin.Portfolio = .{ .lots = lots[0..], .allocator = testing.allocator };
+
+    var candle_map = std.StringHashMap([]const zfin.Candle).init(testing.allocator);
+    defer candle_map.deinit();
+    const cm: classification.ClassificationMap = .{ .entries = &.{}, .allocator = testing.allocator };
+
+    // AAA: present with zero records -> genuinely pays nothing.
+    // BBB: absent entirely -> the fetch never landed.
+    var dividend_map = std.StringHashMap([]const zfin.Dividend).init(testing.allocator);
+    defer dividend_map.deinit();
+    try dividend_map.put("AAA", &.{});
+
+    var view = try buildReview(
+        testing.allocator,
+        std.testing.io,
+        summary,
+        &candle_map,
+        &dividend_map,
+        portfolio,
+        cm,
+        null,
+        D.fromYmd(2026, 6, 4),
+        "test_portfolio.srf",
+    );
+    defer view.deinit(testing.allocator);
+
+    var saw_aaa = false;
+    var saw_bbb = false;
+    for (view.rows) |r| {
+        if (std.mem.eql(u8, r.symbol, "AAA")) {
+            saw_aaa = true;
+            try testing.expect(!r.dividends_missing);
+        } else if (std.mem.eql(u8, r.symbol, "BBB")) {
+            saw_bbb = true;
+            try testing.expect(r.dividends_missing);
+        }
+    }
+    try testing.expect(saw_aaa and saw_bbb);
+
+    // A null map means dividends were never requested at all. That is not a
+    // per-symbol failure, so it must not flag every holding.
+    var view2 = try buildReview(
+        testing.allocator,
+        std.testing.io,
+        summary,
+        &candle_map,
+        null,
+        portfolio,
+        cm,
+        null,
+        D.fromYmd(2026, 6, 4),
+        "test_portfolio.srf",
+    );
+    defer view2.deinit(testing.allocator);
+    for (view2.rows) |r| try testing.expect(!r.dividends_missing);
+}
+
 test "buildReview: end-to-end with testing allocator (leak check)" {
     // Allocator detects leaks: every allocation must be freed for the
     // test to pass. Exercises the full buildReview lifecycle including