From 03b07bc07efd611116735cb38373c17caab8887a Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sat, 16 May 2026 13:22:38 -0700 Subject: [PATCH] add horizon/event metric and resolve todo --- TODO.md | 41 --------------------------- src/commands/compare.zig | 5 ++-- src/commands/projections.zig | 55 +++++++++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 44 deletions(-) diff --git a/TODO.md b/TODO.md index efc6f15..6130333 100644 --- a/TODO.md +++ b/TODO.md @@ -548,47 +548,6 @@ gain. Possible fixes are discussed in the "Contributions diff" TODO below — option C there (per-account `cash_is_contribution`) would make manually-entered ESPP-style cash additions count correctly. -## Investigate: `compare --projections` block disagrees with standalone `projections` - -**Observed during a weekly review:** The embedded projections block -from `zfin compare --projections 1W --commit-before HEAD` shows -projected-return / SWR numbers that don't line up with what -`zfin projections` (standalone, no flags) and/or -`zfin projections --as-of 1W` produce for the same dates. - -The shape of the disagreement seen in practice was small but -consistent — a fraction-of-a-percent drift on projected return and -a few-thousand-dollar drift on safe-withdrawal dollar amounts — -across "then" and "now" sides of the same compare run. - -The "now" side should match `zfin projections`, and the "then" side -should match `zfin projections --as-of 1W`. Confirm which side(s) -disagree on next investigation pass. - -Candidates to investigate: - -- Different snapshot resolution between the two code paths. `compare` - with `1W` snaps to the nearest-earlier history file; standalone - `projections --as-of 1W` may resolve to the same file or a different - one. Check the muted "(requested X; nearest snapshot: Y)" notes from - both runs. -- Different valuation rules between paths. REPORT.md §2 already - documents that the commit-side pipeline values DRIP/rollup share - deltas using **current** cache prices, not snapshot prices — could - bleed into the embedded projections block if it's reading - commit-side state. -- `metadata.srf` / `projections.srf` loaded as-of-today vs. as-of-then. - REPORT.md §4 already calls this out as a known limitation; verify - whether both paths apply it consistently. -- The `--commit-before HEAD` flag affecting the projections block in - `compare` but not standalone `projections` (which doesn't take that - flag). - -Cross-check: run `zfin projections --vs 1W` and compare its compact -output against the `compare --projections` embedded block. If those -agree, the discrepancy is between `--vs` and standalone `projections`. -If they disagree, the bug is in `compare`'s embedding path specifically. - ## Investigate: missing portfolio "Week" return in `zfin projections` **Symptom:** The `zfin projections` CLI benchmark table has a "Week" diff --git a/src/commands/compare.zig b/src/commands/compare.zig index cc1769b..40ea3fd 100644 --- a/src/commands/compare.zig +++ b/src/commands/compare.zig @@ -368,7 +368,7 @@ pub fn run( break :blk null; }; if (projections_result) |r| { - projections_block = .{ .then = r.then, .now = r.now }; + projections_block = .{ .then = r.then, .now = r.now, .events_enabled = r.events_enabled }; } } @@ -473,6 +473,7 @@ const RenderArgs = struct { const ProjectionsBlock = struct { then: projections.KeyMetrics, now: projections.KeyMetrics, + events_enabled: bool, }; /// Build the view from two holdings maps + totals, then render. @@ -633,7 +634,7 @@ fn renderCompare(out: *std.Io.Writer, color: bool, cv: view.CompareView, proj: ? // so the "headline" numbers cluster together at the top. if (proj) |p| { try out.print("\n", .{}); - try projections.renderKeyComparisonRows(out, color, p.then, p.now); + try projections.renderKeyComparisonRows(out, color, p.then, p.now, p.events_enabled); } try out.print("\n", .{}); diff --git a/src/commands/projections.zig b/src/commands/projections.zig index 11712ed..979c547 100644 --- a/src/commands/projections.zig +++ b/src/commands/projections.zig @@ -441,6 +441,11 @@ pub const KeyMetrics = struct { /// `swr_99 / total_value`. Rendered as a percent alongside the /// dollar amount for comparison sanity. swr_99_rate: f64, + /// Longest configured horizon, in years. Used for the "this is + /// at the {N}-year horizon" caption above the comparison block + /// so the reader doesn't have to remember which horizon + /// `swr_99` refers to. Comes from `ctx.config.getHorizons()`. + horizon_years: u16, }; fn extractKeyMetrics(ctx: view.ProjectionContext) KeyMetrics { @@ -452,6 +457,7 @@ fn extractKeyMetrics(ctx: view.ProjectionContext) KeyMetrics { .projected_return = ctx.comparison.conservative_return, .swr_99 = swr.annual_amount, .swr_99_rate = rate, + .horizon_years = horizons[longest], }; } @@ -558,7 +564,7 @@ pub fn runCompare( } try out.print("\n", .{}); - try renderKeyComparisonRows(out, color, result.then, result.now); + try renderKeyComparisonRows(out, color, result.then, result.now, result.events_enabled); try cli.printFg(out, color, cli.CLR_MUTED, "\nFor the full benchmark + SWR tables run `zfin projections --as-of {s}` and `zfin projections{s}`.\n", .{ then_str, @@ -582,6 +588,11 @@ pub const KeyComparisonResult = struct { resolution: AsOfResolution, /// Resolution of the "now" snapshot. Null when now is live. now_resolution: ?AsOfResolution, + /// Whether simulated lifecycle events (RMDs, lump-sum + /// withdrawals, Social Security) were included in the + /// projection. Captured here so the comparison-row caption + /// can tell the reader what assumptions are baked in. + events_enabled: bool, retained_then: history.LoadedSnapshot, retained_now: ?history.LoadedSnapshot, retained_allocator: std.mem.Allocator, @@ -593,6 +604,27 @@ pub const KeyComparisonResult = struct { } }; +/// Compute the "then" vs "now" key metrics for `--vs` and the +/// `compare --projections` embedded block. +/// +/// Output-equivalence invariant: this function MUST produce +/// identical `KeyMetrics` to what standalone `projections` / +/// `projections --as-of` produce for the same dates. Both paths +/// resolve the same way: +/// +/// - `then` (snapshot): `loadAsOfContext` → +/// `view.loadProjectionContextAsOf(...)` is the same call +/// standalone `projections --as-of` makes at line ~110. +/// - `now` (live): the `cli.loadPortfolio` → +/// `cli.buildPortfolioData` → `view.loadProjectionContext` +/// pipeline below mirrors standalone `projections` (no flags) +/// at lines ~167-202. +/// +/// If you change inputs to either of these loaders, change them +/// in BOTH places. A drift between this path and standalone +/// projections will show as `compare --projections` reporting +/// different numbers from what `projections --as-of ` and +/// `projections` print, which has bitten us before. pub fn computeKeyComparison( io: std.Io, allocator: std.mem.Allocator, @@ -649,6 +681,7 @@ pub fn computeKeyComparison( .now = extractKeyMetrics(now_ctx), .resolution = then_resolution, .now_resolution = now_resolution, + .events_enabled = events_enabled, .retained_then = then_snap, .retained_now = now_snap, .retained_allocator = allocator, @@ -705,6 +738,7 @@ pub fn computeKeyComparison( .now = extractKeyMetrics(now_ctx), .resolution = then_resolution, .now_resolution = null, + .events_enabled = events_enabled, .retained_then = then_snap, .retained_now = null, .retained_allocator = allocator, @@ -714,12 +748,31 @@ pub fn computeKeyComparison( /// Render the three comparison rows (projected return, SWR @99%, SWR /// rate). Shared between `projections --vs` and any other caller that /// wants to embed the same block (e.g. `compare --projections`). +/// Render the three "then → now" comparison rows (projected return, +/// SWR @99% dollars, SWR @99% rate) for the `--vs` and +/// `compare --projections` outputs. +/// +/// The leading caption ("{N}-year horizon, lifecycle events +/// {included|excluded}") tells the reader which assumptions are +/// baked in: lots of numbers crowd the report, and "Safe +/// withdrawal @99%" alone is ambiguous about which horizon's SWR +/// is being shown (the TUI's full table renders both 30-year and +/// 43-year columns, but this compact view only shows the longest +/// horizon). Surface the context so a reader scanning the +/// numbers doesn't have to remember. pub fn renderKeyComparisonRows( out: *std.Io.Writer, color: bool, then: KeyMetrics, now: KeyMetrics, + events_enabled: bool, ) !void { + // `then` and `now` are computed against the same projections.srf + // (REPORT.md §4 — the "then" side reuses today's config), so + // their horizons agree. Use whichever side is convenient. + const events_label: []const u8 = if (events_enabled) "included" else "excluded"; + try cli.printFg(out, color, cli.CLR_MUTED, " ({d}-year horizon, lifecycle events {s})\n", .{ now.horizon_years, events_label }); + try renderCompareRowPct(out, color, "Projected return:", then.projected_return, now.projected_return); try renderCompareRowMoney(out, color, "Safe withdrawal @99%:", then.swr_99, now.swr_99); try renderCompareRowPct(out, color, " (as % of total)", then.swr_99_rate, now.swr_99_rate);