add horizon/event metric and resolve todo
This commit is contained in:
parent
adf33b9e21
commit
03b07bc07e
3 changed files with 57 additions and 44 deletions
41
TODO.md
41
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"
|
||||
|
|
|
|||
|
|
@ -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", .{});
|
||||
|
|
|
|||
|
|
@ -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 <DATE>` 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);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue