add observation for missing dividend history

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/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