From a9b5b8fe198251b2cb834948cd2bd4ef39d42c90 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Wed, 24 Jun 2026 14:53:16 -0700 Subject: [PATCH] projections: suppress output of percentage data when accumulation mode exists --- TODO.md | 12 --- src/commands/projections.zig | 13 ++- src/tui/projections_tab.zig | 168 ++++++++++++++++++++++++++++------- src/views/projections.zig | 40 +++++++++ 4 files changed, 189 insertions(+), 44 deletions(-) diff --git a/TODO.md b/TODO.md index be5c8b4..a2f84d2 100644 --- a/TODO.md +++ b/TODO.md @@ -10,18 +10,6 @@ ranking; unlabeled items are "someday, if the mood strikes." - **Configurable return cap per position — priority MEDIUM.** Default: none; cap outliers like NVDA. Should route through `projections.srf` cleanly. -- **Accumulation-mode SWR rate column is misleading — priority LOW.** - When `retirement_age`/`retirement_at` is configured, the "Safe - Withdrawal" table's % column divides the SWR amount by the - CURRENT portfolio value, not the post-accumulation portfolio - value. The dollar amount is correct (it's the safe spending in - retirement, given the projected accumulation), but the % rate - comes out absurdly high (e.g., 22% of today's portfolio). The - Accumulation phase block already shows the median portfolio at - retirement, so the user can compute the real rate themselves — - but the SWR table's rate column should ideally divide by the - median post-accumulation value, or be suppressed when accumulation - is active. Decide which. - **Chart vertical line at retirement boundary — priority LOW.** The accumulation-phase spec called this "mandatory" but it was explicitly deferred during implementation. The chart currently diff --git a/src/commands/projections.zig b/src/commands/projections.zig index 6945dd7..73b0bef 100644 --- a/src/commands/projections.zig +++ b/src/commands/projections.zig @@ -790,11 +790,20 @@ pub fn runBands( // Header row try out.print("{s}\n", .{try view.buildHeaderRow(va, horizons, view.withdrawal_col_width)}); - // Withdrawal rows + // Withdrawal rows. When an accumulation phase is active the + // per-row % rate is suppressed (it would divide today's-dollars + // retirement spending by today's portfolio); a footnote explains + // and points at the Accumulation phase block. + const swr_rate_note = view.swrRateNote(ctx.retirement.accumulation_years); for (confidence_levels, 0..) |conf, ci| { const wr_rows = try view.buildWithdrawalRows(va, conf, horizons, ctx.data.withdrawals, ci); try out.print("{s}\n", .{wr_rows.amount.text}); - try cli.printFg(out, color, cli.CLR_MUTED, "{s}\n", .{wr_rows.rate.text}); + if (swr_rate_note == null) { + try cli.printFg(out, color, cli.CLR_MUTED, "{s}\n", .{wr_rows.rate.text}); + } + } + if (swr_rate_note) |note| { + try cli.printFg(out, color, cli.CLR_MUTED, " {s}\n", .{note}); } // Life events summary — both as-of and live modes resolve ages diff --git a/src/tui/projections_tab.zig b/src/tui/projections_tab.zig index 8d18b5e..5f4ee00 100644 --- a/src/tui/projections_tab.zig +++ b/src/tui/projections_tab.zig @@ -1223,6 +1223,28 @@ fn buildFooterSection(app: *App, arena: std.mem.Allocator, lines: *std.ArrayList } // Safe withdrawal table + try appendSwrTable(lines, arena, th, pctx); + + // Life events summary + try appendEventSummary(lines, app.today, arena, th, pctx); +} + +/// Append the "Safe Withdrawal" table (header + per-confidence +/// amount/rate rows) to a styled-lines list. Shared by the chart and +/// no-chart render paths so they can't drift. +/// +/// During an accumulation phase the per-row % rate is suppressed and +/// replaced with a single footnote: dividing today's-dollars +/// retirement spending by today's portfolio yields a misleading rate. +/// See `view.swrRateNote`. +fn appendSwrTable( + lines: *std.ArrayListUnmanaged(StyledLine), + arena: std.mem.Allocator, + th: theme.Theme, + pctx: view.ProjectionContext, +) !void { + const horizons = pctx.config.getHorizons(); + try lines.append(arena, .{ .text = "", .style = th.contentStyle() }); try lines.append(arena, .{ .text = " Safe Withdrawal (FIRECalc historical simulation)", @@ -1236,21 +1258,27 @@ fn buildFooterSection(app: *App, arena: std.mem.Allocator, lines: *std.ArrayList }); const cached_wr = pctx.data.withdrawals; - const confidence_levels = config.getConfidenceLevels(); + const confidence_levels = pctx.config.getConfidenceLevels(); + const rate_note = view.swrRateNote(pctx.retirement.accumulation_years); for (confidence_levels, 0..) |conf, ci| { const rows = try view.buildWithdrawalRows(arena, conf, horizons, cached_wr, ci); try lines.append(arena, .{ .text = try std.fmt.allocPrint(arena, " {s}", .{rows.amount.text}), .style = th.contentStyle(), }); + if (rate_note == null) { + try lines.append(arena, .{ + .text = try std.fmt.allocPrint(arena, " {s}", .{rows.rate.text}), + .style = th.mutedStyle(), + }); + } + } + if (rate_note) |note| { try lines.append(arena, .{ - .text = try std.fmt.allocPrint(arena, " {s}", .{rows.rate.text}), + .text = try std.fmt.allocPrint(arena, " {s}", .{note}), .style = th.mutedStyle(), }); } - - // Life events summary - try appendEventSummary(lines, app.today, arena, th, pctx); } fn appendEventSummary(lines: *std.ArrayListUnmanaged(StyledLine), as_of: zfin.Date, arena: std.mem.Allocator, th: theme.Theme, pctx: view.ProjectionContext) !void { @@ -2065,31 +2093,7 @@ fn buildLines(state: *State, app: *App, arena: std.mem.Allocator) ![]const Style } // Safe withdrawal table - try lines.append(arena, .{ .text = "", .style = th.contentStyle() }); - try lines.append(arena, .{ - .text = " Safe Withdrawal (FIRECalc historical simulation)", - .style = th.headerStyle(), - }); - try lines.append(arena, .{ .text = "", .style = th.contentStyle() }); - - try lines.append(arena, .{ - .text = try std.fmt.allocPrint(arena, " {s}", .{try view.buildHeaderRow(arena, horizons, view.withdrawal_col_width)}), - .style = th.headerStyle(), - }); - - const cached_wr = ctx.data.withdrawals; - const confidence_levels = config.getConfidenceLevels(); - for (confidence_levels, 0..) |conf, ci| { - const rows = try view.buildWithdrawalRows(arena, conf, horizons, cached_wr, ci); - try lines.append(arena, .{ - .text = try std.fmt.allocPrint(arena, " {s}", .{rows.amount.text}), - .style = th.contentStyle(), - }); - try lines.append(arena, .{ - .text = try std.fmt.allocPrint(arena, " {s}", .{rows.rate.text}), - .style = th.mutedStyle(), - }); - } + try appendSwrTable(&lines, arena, th, ctx); // Life events summary (at the bottom) try appendEventSummary(&lines, app.today, arena, th, ctx); @@ -2196,3 +2200,107 @@ test "formatOverlayUnavailable: respects rebound as-of-input key" { const msg = try formatOverlayUnavailable(&buf, "ctrl+d"); try testing.expectEqualStrings("Overlay only available with --as-of (press ctrl+d to set)", msg); } + +/// Build a `ProjectionContext` for the SWR-table tests. Runs the real +/// (network-free) historical simulation via `view.buildProjectionContext` +/// so `data.withdrawals` is populated. `retirement_at == null` yields a +/// distribution-only context (accumulation_years == 0); a future date +/// yields an accumulation phase. +fn buildSwrTestCtx( + arena: std.mem.Allocator, + retirement_at: ?zfin.Date, + annual_contribution: f64, + as_of: zfin.Date, +) !view.ProjectionContext { + const benchmark = @import("../analytics/benchmark.zig"); + const projections = @import("../analytics/projections.zig"); + var config: projections.UserConfig = .{}; + config.retirement_at = retirement_at; + config.annual_contribution = annual_contribution; + const comparison: benchmark.BenchmarkComparison = .{ + .stock_returns = .{}, + .bond_returns = .{}, + .benchmark_returns = .{}, + .portfolio_returns = .{}, + .conservative_return = 0.07, + .stock_pct = 0.75, + .bond_pct = 0.25, + }; + return view.buildProjectionContext(arena, config, comparison, 0.75, 0.25, 1_000_000, &.{}, as_of); +} + +/// Tallies of the line kinds `appendSwrTable` emits, used by the SWR +/// table tests. Each emitted line is exactly one kind: +/// - `header`: the "Safe Withdrawal (...)" title line. +/// - `amount`: a "NN% safe withdrawal $X $Y ..." row. +/// - `rate`: a bare numeric percentage row (rendered only when +/// there is no accumulation phase). +/// - `footnote`: the "% rate omitted during accumulation ..." note. +const SwrLineCounts = struct { + headers: usize = 0, + amounts: usize = 0, + rates: usize = 0, + footnotes: usize = 0, +}; + +fn classifySwrLines(lines: []const StyledLine) SwrLineCounts { + var c: SwrLineCounts = .{}; + for (lines) |line| { + if (std.mem.indexOf(u8, line.text, "Safe Withdrawal") != null) { + // Title line ("Safe Withdrawal (FIRECalc ...)"). + c.headers += 1; + } else if (std.mem.indexOf(u8, line.text, "omitted") != null) { + // Suppression footnote. + c.footnotes += 1; + } else if (std.mem.indexOf(u8, line.text, "safe withdrawal") != null) { + // Amount row: the "NN% safe withdrawal" label carries a + // '%', so it must be matched before the bare-'%' rate case. + c.amounts += 1; + } else if (std.mem.indexOfScalar(u8, line.text, '%') != null) { + // Numeric percentage row. + c.rates += 1; + } + } + return c; +} + +test "appendSwrTable: distribution-only renders a rate row per confidence, no footnote" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + const ctx = try buildSwrTestCtx(a, null, 0, zfin.Date.fromYmd(2026, 5, 12)); + try testing.expectEqual(@as(u16, 0), ctx.retirement.accumulation_years); + + var lines: std.ArrayListUnmanaged(StyledLine) = .empty; + try appendSwrTable(&lines, a, theme.default_theme, ctx); + + const n_conf = ctx.config.getConfidenceLevels().len; + const counts = classifySwrLines(lines.items); + try testing.expectEqual(@as(usize, 1), counts.headers); + try testing.expectEqual(@as(usize, 0), counts.footnotes); + // One amount row and one rate row per confidence level. + try testing.expectEqual(n_conf, counts.amounts); + try testing.expectEqual(n_conf, counts.rates); +} + +test "appendSwrTable: accumulation suppresses rate rows and adds one footnote" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + // Retire 10 years out with contributions -> non-zero accumulation. + const ctx = try buildSwrTestCtx(a, zfin.Date.fromYmd(2036, 7, 1), 50_000, zfin.Date.fromYmd(2026, 7, 1)); + try testing.expect(ctx.retirement.accumulation_years > 0); + + var lines: std.ArrayListUnmanaged(StyledLine) = .empty; + try appendSwrTable(&lines, a, theme.default_theme, ctx); + + const n_conf = ctx.config.getConfidenceLevels().len; + const counts = classifySwrLines(lines.items); + // Amounts still render; the per-row rate is gone, replaced by a + // single footnote. + try testing.expectEqual(n_conf, counts.amounts); + try testing.expectEqual(@as(usize, 0), counts.rates); + try testing.expectEqual(@as(usize, 1), counts.footnotes); +} diff --git a/src/views/projections.zig b/src/views/projections.zig index 6a509dd..f086389 100644 --- a/src/views/projections.zig +++ b/src/views/projections.zig @@ -1067,6 +1067,34 @@ pub fn buildWithdrawalRows( }; } +/// Footnote shown beneath the Safe Withdrawal table when the +/// projection includes an accumulation phase. Returns `null` (render +/// the rate rows normally) when `accumulation_years == 0`. +/// +/// Why the rate is suppressed during accumulation: `WithdrawalResult. +/// withdrawal_rate` is `annual_amount / initial_value`, and +/// `initial_value` is the *current* portfolio. With an accumulation +/// phase the `annual_amount` is the safe spending sized for the +/// projected *post-accumulation* portfolio, so dividing it by today's +/// (much smaller) portfolio yields an absurdly high "rate" (e.g. 22% +/// of today's value). That is the bug this note resolves. +/// +/// We suppress rather than "divide by the median post-accumulation +/// value" because the units don't line up: `annual_amount` is in +/// today's dollars (spending is CPI-adjusted from today; see +/// `searchSafeWithdrawal`), while the bands' `median_at_retirement` +/// is nominal future dollars. A naive `annual_amount / +/// median_at_retirement` would be off by the accumulation-period +/// inflation factor, misleading in the opposite direction. A +/// unit-correct rate needs the boundary inflation factor threaded +/// through the simulation, which isn't worth it for a cosmetic +/// column. The "Accumulation phase" block already reports the +/// projected portfolio at retirement, so we point the reader there. +pub fn swrRateNote(accumulation_years: u16) ?[]const u8 { + if (accumulation_years == 0) return null; + return "% rate omitted during accumulation (would divide retirement spending by today's portfolio). See Accumulation phase above."; +} + /// Build a percentile row (p10/p50/p90) across horizons. pub fn buildPercentileRow( arena: std.mem.Allocator, @@ -1634,6 +1662,18 @@ test "buildWithdrawalRows produces amount and rate" { try std.testing.expect(rows.rate.style == .muted); } +test "swrRateNote: null without accumulation, present with accumulation" { + // Distribution-only (already retired): the rate is a correct + // withdrawal rate against the current portfolio, so render it. + try std.testing.expect(swrRateNote(0) == null); + + // With an accumulation phase the rate would divide today's-dollars + // retirement spending by today's portfolio; suppress and explain. + const note = swrRateNote(12) orelse return error.TestUnexpectedNull; + try std.testing.expect(std.mem.indexOf(u8, note, "omitted") != null); + try std.testing.expect(std.mem.indexOf(u8, note, "Accumulation phase") != null); +} + test "buildPercentileRow extracts correct percentile" { const allocator = std.testing.allocator; var arena = std.heap.ArenaAllocator.init(allocator);