From a14b1ba4bce22a9252520e13768eec3b46457394 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 27 Aug 2026 08:01:50 -0700 Subject: [PATCH] fix/document non-default benchmark stock/bond --- docs/reference/config/projections-srf.md | 31 ++++ src/analytics/projections.zig | 177 ++++++++++++++++++----- src/cache/freshness.zig | 61 +++++++- src/commands/cache.zig | 13 +- src/commands/diagnose.zig | 55 ++++++- src/commands/projections.zig | 4 +- src/models/portfolio.zig | 7 +- src/tui/projections_tab.zig | 15 +- src/views/projections.zig | 11 +- 9 files changed, 315 insertions(+), 59 deletions(-) diff --git a/docs/reference/config/projections-srf.md b/docs/reference/config/projections-srf.md index 6dd797b..4ce841c 100644 --- a/docs/reference/config/projections-srf.md +++ b/docs/reference/config/projections-srf.md @@ -48,8 +48,39 @@ type::event,name::Social Security,start_age:num:70,amount:num:38400 | `spending_change` | num | Signed annual *real* change in spending across the distribution phase, as a whole percent. Negative = declining (e.g. `-2` = -2%/yr, the "spending smile"); positive = rising. Default: absent = flat real spending. Magnitude clamped to 10%/yr. See [Declining spending](#declining-spending-the-smile). | | `survivor_spending_pct` | num | Percent of the couple's joint spending the surviving spouse needs after the first death, for `horizon_age` columns. Default `75` (a 25% reduction). Only applies to a multi-person household with an age gap. See [Planning to an age of death](#planning-to-an-age-of-death). | | `max_accumulation_years` | num | Ceiling (in years) the earliest-retirement search scans when `target_spending` is set. Default `50`, capped at `100`. | +| `benchmark_stock` | str | Symbol for the stock leg of the benchmark-comparison table. Default `SPY`. Max 16 characters; longer values are ignored with a warning. See [Choosing benchmark symbols](#choosing-benchmark-symbols). | +| `benchmark_bond` | str | Symbol for the bond leg of the benchmark-comparison table. Default `AGG`. Same 16-character limit. | | `retirement_target` | num | Annotation on a `horizon`/`horizon_age` line that overrides the earliest-retirement promotion rule. Allowed: `90`, `95`, `99`. | +### Choosing benchmark symbols + +`zfin projections` prints a **Benchmark comparison** table: the stock +leg's trailing returns, the bond leg's, a blend weighted by your actual +equity/fixed-income split, and your portfolio's own weighted return. The +two legs default to `SPY` and `AGG`: + +``` +type::config,benchmark_stock::SPYM +type::config,benchmark_bond::BND +``` + +Either may be set independently; an absent field keeps its default. + +**Prefer a symbol you actually hold.** zfin only keeps candles and +dividends warm for symbols the portfolio holds (plus `watchlist.srf`). +A benchmark held nowhere is deliberately excluded from the routine +`zfin cache refresh` sweep -- it is fetched on demand when `zfin +projections` runs, and `zfin cache stale` reports it under "Not +checked". Its candles therefore stay current, but its **dividends** are +read from cache only and never refreshed, so its total returns drift +toward price-only over time. Pointing `benchmark_stock` at something +already in the portfolio avoids that entirely. + +Note that the benchmark symbols affect only this comparison table. The +**Projected return** figure and the Monte Carlo simulation are computed +from your own holdings' trailing returns and are unaffected by this +setting. + ### Choosing an `expense_ratio` The expense ratio is the annual fund-fee drag on the portfolio. zfin diff --git a/src/analytics/projections.zig b/src/analytics/projections.zig index 36dc34e..9619b59 100644 --- a/src/analytics/projections.zig +++ b/src/analytics/projections.zig @@ -358,32 +358,53 @@ pub const UserConfig = struct { /// `retirement_at` derives its accumulation years directly and /// ignores this cap. max_accumulation_years: u16 = default_max_accumulation_years, - /// Stock benchmark symbol used in the projection's - /// benchmark-comparison table and bands. Defaults to "SPY". + /// Stock benchmark symbol for the projection's benchmark-comparison + /// table and bands, as an inline buffer + length. Read it through + /// `benchmarkStock()`, never directly: `benchmark_stock_len == 0` + /// means "no override, use `default_benchmark_stock`". + /// /// Override via `type::config,benchmark_stock::SYMBOL` in - /// `projections.srf`. The slice points into - /// `benchmark_stock_buf` when overridden, or into a string - /// literal in the binary's read-only data segment for the - /// default - either way, valid for the lifetime of the - /// `UserConfig`. - benchmark_stock: []const u8 = "SPY", - /// Backing buffer for an overridden `benchmark_stock`. Untouched - /// (and unread) when the default is in effect. Sized to fit - /// reasonable ticker lengths. - // SAFETY: only read when `benchmark_stock` points into this buffer - // (i.e. when the user has overridden the default); otherwise the - // backing slice points at a literal and this buffer is unobserved. + /// `projections.srf`. + /// + /// This USED to be a `[]const u8` that pointed into the sibling + /// buffer when overridden. That is a self-referential struct, and + /// `parseProjectionsConfig` returns `UserConfig` BY VALUE - so the + /// returned copy's slice pointed into the dead local's frame. An + /// override therefore arrived downstream as N bytes of whatever the + /// stack had been reused for (observed: NULs), silently blanking the + /// stock leg of the benchmark table. The defaults never broke + /// because string literals live in .rodata, which is why a feature + /// that had probably never worked went unnoticed. + /// + /// Do not reintroduce a stored slice here. The codebase convention + /// is inline-buffer + length + accessor precisely so a value type + /// stays copy-safe - see `LifeEvent.getName` below, plus + /// `models/quote.zig:Quote.name`, `analytics/analysis.zig:Annotation`, + /// `commands/cache.zig:FileInfo.lastDate`, and `tui.zig:ParsedArgs.symbol`. + // SAFETY: paired with `benchmark_stock_len`; only + // `benchmark_stock_buf[0..benchmark_stock_len]` is ever read, and + // `benchmarkStock()` returns the default literal when the length is 0. benchmark_stock_buf: [16]u8 = undefined, - /// Bond benchmark symbol. Same lifetime / override mechanics - /// as `benchmark_stock`. - benchmark_bond: []const u8 = "AGG", - // SAFETY: same override-only read pattern as `benchmark_stock_buf`. + /// Length of the `benchmark_stock` override; 0 = use the default. + benchmark_stock_len: u8 = 0, + /// Bond benchmark symbol. Same buffer + length + accessor mechanics + /// as `benchmark_stock`; read via `benchmarkBond()`. + // SAFETY: paired with `benchmark_bond_len`; same read-only-the-prefix + // invariant as `benchmark_stock_buf`. benchmark_bond_buf: [16]u8 = undefined, + /// Length of the `benchmark_bond` override; 0 = use the default. + benchmark_bond_len: u8 = 0, const max_horizons: usize = 8; const max_persons: usize = 4; pub const max_events: usize = 16; + /// Benchmark symbols used when `projections.srf` declares no + /// override. Public so the fetch-policy layer and tests can name + /// them instead of duplicating the literals. + pub const default_benchmark_stock: []const u8 = "SPY"; + pub const default_benchmark_bond: []const u8 = "AGG"; + /// Errors that can arise when resolving age-based horizons. pub const ResolveError = error{ /// `type::config,horizon_age:num:N` was specified in projections.srf @@ -391,6 +412,26 @@ pub const UserConfig = struct { HorizonAgeWithoutBirthdate, }; + /// The stock benchmark symbol: the user's override, else + /// `default_benchmark_stock`. + /// + /// Derives the slice from `self` at call time, which is what makes + /// `UserConfig` copy-safe. Returning a stored slice into + /// `benchmark_stock_buf` instead is the bug documented on that + /// field - don't. + pub fn benchmarkStock(self: *const UserConfig) []const u8 { + if (self.benchmark_stock_len == 0) return default_benchmark_stock; + return self.benchmark_stock_buf[0..self.benchmark_stock_len]; + } + + /// The bond benchmark symbol: the user's override, else + /// `default_benchmark_bond`. Same copy-safety note as + /// `benchmarkStock`. + pub fn benchmarkBond(self: *const UserConfig) []const u8 { + if (self.benchmark_bond_len == 0) return default_benchmark_bond; + return self.benchmark_bond_buf[0..self.benchmark_bond_len]; + } + pub fn getHorizons(self: *const UserConfig) []const u16 { return self.horizons[0..self.horizon_count]; } @@ -706,16 +747,17 @@ pub const max_abs_spending_real_change: f64 = 0.10; /// demand" stays a fetch-policy question and the cache sweep does not have to /// know that the answer happens to come from projections config. /// -/// The strings are DUPED into `arena`. An overridden symbol lives in a `[16]u8` -/// field inside the returned `UserConfig`, so a borrowed slice dangles the -/// moment that struct goes out of scope - which is the whole reason this exists -/// rather than callers reading the config themselves. +/// The strings are DUPED into `arena` so the result outlives the local +/// `UserConfig` this reads them from. (An overridden symbol lives in an +/// inline `[16]u8` inside that config, so borrowing would tie the +/// caller's lifetime to a function-local; `UserConfig` itself is +/// copy-safe, but a slice into one particular copy of it is not.) pub fn benchmarkSymbols(io: std.Io, arena: std.mem.Allocator, path: []const u8) []const []const u8 { const data = std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(64 * 1024)) catch null; const cfg = parseProjectionsConfig(data); const pair = arena.alloc([]const u8, 2) catch return &.{}; - pair[0] = arena.dupe(u8, cfg.benchmark_stock) catch return &.{}; - pair[1] = arena.dupe(u8, cfg.benchmark_bond) catch return &.{}; + pair[0] = arena.dupe(u8, cfg.benchmarkStock()) catch return &.{}; + pair[1] = arena.dupe(u8, cfg.benchmarkBond()) catch return &.{}; return pair; } @@ -904,10 +946,14 @@ pub fn parseProjectionsConfig(data: ?[]const u8) UserConfig { if (sym.len == 0 or sym.len > config.benchmark_stock_buf.len) { warnUser("projections: benchmark_stock must be 1..{d} chars (got {d}); ignoring record", .{ config.benchmark_stock_buf.len, sym.len }); } else { - // Dupe into our own buffer so the slice - // outlives the SRF iterator's backing data. + // Copy into our own buffer + length so the value + // outlives both the SRF iterator's backing data + // AND this function's frame. Storing a slice into + // the buffer instead would dangle the moment + // `config` is returned by value. Same shape as + // the `.event` arm's name handling below. @memcpy(config.benchmark_stock_buf[0..sym.len], sym); - config.benchmark_stock = config.benchmark_stock_buf[0..sym.len]; + config.benchmark_stock_len = @intCast(sym.len); } } if (c.benchmark_bond) |sym| { @@ -915,7 +961,7 @@ pub fn parseProjectionsConfig(data: ?[]const u8) UserConfig { warnUser("projections: benchmark_bond must be 1..{d} chars (got {d}); ignoring record", .{ config.benchmark_bond_buf.len, sym.len }); } else { @memcpy(config.benchmark_bond_buf[0..sym.len], sym); - config.benchmark_bond = config.benchmark_bond_buf[0..sym.len]; + config.benchmark_bond_len = @intCast(sym.len); } } }, @@ -2867,8 +2913,11 @@ test "parseProjectionsConfig rejects negative return_cap" { test "parseProjectionsConfig benchmark defaults are SPY and AGG" { const config = parseProjectionsConfig(null); - try std.testing.expectEqualStrings("SPY", config.benchmark_stock); - try std.testing.expectEqualStrings("AGG", config.benchmark_bond); + try std.testing.expectEqualStrings("SPY", config.benchmarkStock()); + try std.testing.expectEqualStrings("AGG", config.benchmarkBond()); + // len == 0 is what "no override" means; the buffers stay unread. + try std.testing.expectEqual(@as(u8, 0), config.benchmark_stock_len); + try std.testing.expectEqual(@as(u8, 0), config.benchmark_bond_len); } test "parseProjectionsConfig parses benchmark_stock and benchmark_bond" { @@ -2878,8 +2927,54 @@ test "parseProjectionsConfig parses benchmark_stock and benchmark_bond" { \\type::config,benchmark_bond::BND ; const config = parseProjectionsConfig(data); - try std.testing.expectEqualStrings("VTI", config.benchmark_stock); - try std.testing.expectEqualStrings("BND", config.benchmark_bond); + try std.testing.expectEqualStrings("VTI", config.benchmarkStock()); + try std.testing.expectEqualStrings("BND", config.benchmarkBond()); +} + +test "parseProjectionsConfig: an override survives copying the config by value" { + // THE REGRESSION TEST. `benchmark_stock` used to be a `[]const u8` + // pointing into `benchmark_stock_buf` - a self-reference - while + // `parseProjectionsConfig` returns `UserConfig` BY VALUE. Every copy + // after the first therefore carried a slice into a dead frame. + // + // The pre-existing tests all read the symbol one statement after the + // parse call, in the frame that received the return value, so the + // dead bytes were still intact and all of them passed. This one + // copies the struct, scribbles over the stack, and only then reads - + // which is what production does via `ProjectionContext`. + const data = + \\#!srfv1 + \\type::config,benchmark_stock::VTI + \\type::config,benchmark_bond::BND + ; + + var copies: [4]UserConfig = undefined; + copies[0] = parseProjectionsConfig(data); + // Copy through a chain, the way buildContextFromParts -> + // buildProjectionContext -> ProjectionContext does. + copies[1] = copies[0]; + copies[2] = copies[1]; + copies[3] = copies[2]; + + // Churn the stack that `parseProjectionsConfig` used, so a dangling + // pointer reads garbage rather than stale-but-correct bytes. + stackChurn(); + + for (copies) |c| { + try std.testing.expectEqualStrings("VTI", c.benchmarkStock()); + try std.testing.expectEqualStrings("BND", c.benchmarkBond()); + } +} + +/// Overwrite a chunk of stack so a dangling slice into a returned-by-value +/// struct reads scribble instead of stale-but-intact bytes. `noinline` and +/// the volatile-ish sum keep the optimizer from eliding it. +noinline fn stackChurn() void { + var scratch: [16 * 1024]u8 = undefined; + @memset(&scratch, 0xAA); + var sum: usize = 0; + for (scratch) |b| sum +%= b; + std.mem.doNotOptimizeAway(sum); } test "parseProjectionsConfig partial benchmark override falls back to default" { @@ -2889,8 +2984,9 @@ test "parseProjectionsConfig partial benchmark override falls back to default" { \\type::config,benchmark_stock::QQQ ; const config = parseProjectionsConfig(data); - try std.testing.expectEqualStrings("QQQ", config.benchmark_stock); - try std.testing.expectEqualStrings("AGG", config.benchmark_bond); + try std.testing.expectEqualStrings("QQQ", config.benchmarkStock()); + try std.testing.expectEqualStrings("AGG", config.benchmarkBond()); + try std.testing.expectEqual(@as(u8, 0), config.benchmark_bond_len); } test "parseProjectionsConfig rejects oversized benchmark symbol" { @@ -2900,7 +2996,18 @@ test "parseProjectionsConfig rejects oversized benchmark symbol" { \\type::config,benchmark_stock::ABCDEFGHIJKLMNOPQ ; const config = parseProjectionsConfig(data); - try std.testing.expectEqualStrings("SPY", config.benchmark_stock); + try std.testing.expectEqualStrings("SPY", config.benchmarkStock()); + try std.testing.expectEqual(@as(u8, 0), config.benchmark_stock_len); +} + +test "UserConfig: a 16-char symbol fits exactly (boundary)" { + const data = + \\#!srfv1 + \\type::config,benchmark_stock::ABCDEFGHIJKLMNOP + ; + const config = parseProjectionsConfig(data); + try std.testing.expectEqualStrings("ABCDEFGHIJKLMNOP", config.benchmarkStock()); + try std.testing.expectEqual(@as(u8, 16), config.benchmark_stock_len); } test "parseProjectionsConfig parses both retirement_age and retirement_at" { diff --git a/src/cache/freshness.zig b/src/cache/freshness.zig index c53634e..335c8d6 100644 --- a/src/cache/freshness.zig +++ b/src/cache/freshness.zig @@ -208,15 +208,26 @@ pub fn collect( // and not an orphan (something does refresh it). Taking the exclusion // here rather than at each caller means both the sweep and `doctor` skip // them by construction instead of by remembering to. - for (exclude) |ex| { - if (std.mem.eql(u8, key, ex)) continue :outer; + // + // ...but ONLY if the symbol is not otherwise tracked. A symbol that is + // both held and the configured benchmark is subject to the routine + // refresh like any other holding, so its staleness IS a finding and + // dropping it would hide a real position. This bit the moment someone + // pointed `benchmark_stock` at a symbol they actually hold - which is + // the recommended way to configure it, because a benchmark held nowhere + // never gets its dividends warmed. + const is_tracked = tracked.contains(key); + if (!is_tracked) { + for (exclude) |ex| { + if (std.mem.eql(u8, key, ex)) continue :outer; + } } const cm = store.readCandleMeta(key); try out.append(allocator, .{ .symbol = key, .kind = market.classify(key), .last_date = if (cm) |m| m.meta.last_date else null, - .tracked = tracked.contains(key), + .tracked = is_tracked, }); } return out.toOwnedSlice(allocator); @@ -604,6 +615,50 @@ test "scan: every allocation-failure path unwinds cleanly" { ); } +test "collect: a benchmark that is ALSO held stays in the corpus" { + // The regression. `collect` used to drop every excluded key + // unconditionally, so pointing `benchmark_stock` at a symbol you hold + // made that holding invisible to `cache stale`, `cache refresh` and + // `doctor` - the exact opposite of the intent, since the reason to + // choose a held symbol as your benchmark is that held symbols are the + // only ones whose dividends get warmed. + const a = testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir_path = try tmp.dir.realPathFileAlloc(io, ".", a); + defer a.free(dir_path); + var store = cache.Store.init(io, a, dir_path); + + var tracked = std.StringHashMap(void).init(a); + defer tracked.deinit(); + try tracked.put("HELDBM", {}); // held AND the configured benchmark + try tracked.put("PLAIN", {}); // an ordinary holding + + const keys = [_][]const u8{ "HELDBM", "PLAIN", "PUREBM" }; + const exclude = [_][]const u8{ "HELDBM", "PUREBM" }; // the benchmark pair + + const entries = try collect(a, &store, &keys, &tracked, &exclude); + defer a.free(entries); + + // PUREBM is benchmark-only -> dropped. The other two survive. + try testing.expectEqual(@as(usize, 2), entries.len); + var saw_held = false; + var saw_plain = false; + for (entries) |e| { + if (std.mem.eql(u8, e.symbol, "HELDBM")) { + saw_held = true; + // ...and it is reported as tracked, so a stale finding is a + // real finding rather than an orphan verdict. + try testing.expect(e.tracked); + } + if (std.mem.eql(u8, e.symbol, "PLAIN")) saw_plain = true; + try testing.expect(!std.mem.eql(u8, e.symbol, "PUREBM")); + } + try testing.expect(saw_held); + try testing.expect(saw_plain); +} + test "collect-style exclusion: an excluded symbol produces no finding at all" { const a = testing.allocator; // A demand-fetched symbol makes every classification wrong: not stale diff --git a/src/commands/cache.zig b/src/commands/cache.zig index 212bf9c..9fe61c5 100644 --- a/src/commands/cache.zig +++ b/src/commands/cache.zig @@ -287,11 +287,22 @@ fn sweep(ctx: *framework.RunCtx) !Sweep { } const entries = try freshness.collect(arena, &store, keys, &tracked, bench); + + // Report only the symbols `collect` actually dropped. A benchmark that + // is ALSO held stays in the corpus and gets swept like any other + // holding, so listing it under "Not checked" would be a lie - and the + // recommended way to configure `benchmark_stock` is to point it at + // something you hold. + var excluded = std.ArrayList([]const u8).empty; + for (bench) |sym| { + if (!tracked.contains(sym)) try excluded.append(arena, sym); + } + return .{ .arena_state = arena_state, .store = store, .report = try freshness.scan(arena, entries, now_s), - .excluded = bench, + .excluded = try excluded.toOwnedSlice(arena), .empty = false, }; } diff --git a/src/commands/diagnose.zig b/src/commands/diagnose.zig index 118b484..6f14b42 100644 --- a/src/commands/diagnose.zig +++ b/src/commands/diagnose.zig @@ -193,10 +193,18 @@ pub fn classify(o: Observed) Verdict { } return .current; } - // A benchmark outranks both: it is genuinely untracked, but saying so would - // send the operator hunting for a config fix when the answer is simply that - // `projections` fetches it on demand. - if (o.benchmark) return .demand_fetched; + // A benchmark outranks the TTL: it is genuinely untracked, but saying so + // would send the operator hunting for a config fix when the answer is + // simply that `projections` fetches it on demand. + // + // Unless it is ALSO tracked. A symbol that is both held and the configured + // benchmark is refreshed by every routine run like any other holding, so + // the demand-fetch story would misdirect in the opposite direction - and + // pointing `benchmark_stock` at something you hold is the recommended + // configuration, because a benchmark held nowhere never gets its dividends + // warmed. `null` (no portfolio to ask) stays on the demand-fetched path, + // which is the safe direction: we cannot claim it is tracked. + if (o.benchmark and !(o.tracked orelse false)) return .demand_fetched; // Untracked outranks the TTL: a lapsed TTL is never even consulted for a // symbol no code path requests. if (o.tracked) |t| { @@ -318,7 +326,12 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { if (cli.trackedSymbols(ctx, arena, l.portfolio)) |set| { obs.tracked = set.contains(symbol); } else |_| {} - if (obs.benchmark) { + if (obs.benchmark and (obs.tracked orelse false)) { + // Both held and the configured benchmark. The routine sweep + // covers it, so lead with that - the demand-fetch path is + // merely an additional refresher, not the only one. + try out.print("tracked yes - a normal run fetches this symbol (also a projections benchmark)\n", .{}); + } else if (obs.benchmark) { try out.print("tracked on demand - a projections benchmark symbol\n", .{}); } else if (obs.tracked) |t| { try out.print("tracked {s}\n", .{if (t) "yes - a normal run fetches this symbol" else "NO - no normal run fetches this symbol"}); @@ -769,6 +782,38 @@ test "classify: being tracked does not mask the real cause" { })); } +test "classify: a benchmark that is ALSO held is judged like any holding" { + // Pointing `benchmark_stock` at a symbol you hold is the recommended + // configuration (a benchmark held nowhere never gets its dividends + // warmed). Such a symbol IS refreshed by every routine run, so + // reporting "fetched on demand by projections" would misdirect - and + // it used to, because the benchmark check short-circuited ahead of the + // tracked check. + const v = classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .peer = d(2026, 8, 11), + .server = d(2026, 8, 11), + .provider = d(2026, 8, 11), + .tracked = true, // held + .benchmark = true, // and the configured benchmark + }); + try testing.expect(v != .demand_fetched); + try testing.expect(v != .not_tracked); + + // A benchmark we cannot prove is held stays on the demand-fetched + // path - the safe direction when there is no portfolio to ask. + try testing.expectEqual(Verdict.demand_fetched, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .peer = d(2026, 8, 11), + .server = d(2026, 8, 11), + .provider = d(2026, 8, 11), + .tracked = null, + .benchmark = true, + })); +} + test "classify: a benchmark is demand-fetched, not untracked" { // AGG. Genuinely absent from the routine fetch set, so `not_tracked` is // literally true - and misleading, because it sends the operator hunting for diff --git a/src/commands/projections.zig b/src/commands/projections.zig index 9971535..c2fe542 100644 --- a/src/commands/projections.zig +++ b/src/commands/projections.zig @@ -859,7 +859,7 @@ pub fn runBands( var spy_bufs: [5][16]u8 = undefined; var spy_label_buf: [32]u8 = undefined; const spy_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&spy_label_buf, ctx.config.benchmark_stock, ctx.stock_pct * 100), + view.fmtBenchmarkLabel(&spy_label_buf, ctx.config.benchmarkStock(), ctx.stock_pct * 100), comparison.stock_returns, &spy_bufs, false, @@ -868,7 +868,7 @@ pub fn runBands( var agg_bufs: [5][16]u8 = undefined; var agg_label_buf: [32]u8 = undefined; const agg_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&agg_label_buf, ctx.config.benchmark_bond, ctx.bond_pct * 100), + view.fmtBenchmarkLabel(&agg_label_buf, ctx.config.benchmarkBond(), ctx.bond_pct * 100), comparison.bond_returns, &agg_bufs, false, diff --git a/src/models/portfolio.zig b/src/models/portfolio.zig index 69bc859..57dc835 100644 --- a/src/models/portfolio.zig +++ b/src/models/portfolio.zig @@ -968,8 +968,11 @@ pub const Portfolio = struct { /// `stockSymbols`, which borrows from the portfolio, the inputs here have /// mixed and shorter lifetimes - notably a benchmark override lives in a /// `[16]u8` field inside a stack `UserConfig`, so borrowing it would - /// dangle the moment that config went out of scope. Caller owns the - /// result; free the slices and the outer slice, or use an arena. + /// dangle the moment that config went out of scope. (`UserConfig` is + /// itself copy-safe - it stores buffer + length, not a self-slice - but a + /// slice into one particular copy of it is only as long-lived as that + /// copy, which is exactly why this dupes.) Caller owns the result; free + /// the slices and the outer slice, or use an arena. pub fn fetchedSymbols( self: Portfolio, allocator: std.mem.Allocator, diff --git a/src/tui/projections_tab.zig b/src/tui/projections_tab.zig index fdd02df..70048c6 100644 --- a/src/tui/projections_tab.zig +++ b/src/tui/projections_tab.zig @@ -94,13 +94,10 @@ pub const State = struct { /// disabled state). Distinct from `ctx != null` because failed /// loads still mark loaded. loaded: bool = false, - /// User-tunable inputs to the projection engine (annual - /// contribution, target spending, retirement target percentile, - /// etc.). Driven by annotations on the portfolio file. - config: @import("../analytics/projections.zig").UserConfig = .{}, /// Loaded projection context: bands, withdrawal tables, /// horizon configs, optional overlay actuals. Owned by State; - /// freed via `freeLoaded`. + /// freed via `freeLoaded`. The `UserConfig` travels inside this - + /// there is deliberately no second copy on State. ctx: ?@import("../views/projections.zig").ProjectionContext = null, /// Currently-focused horizon row in the terminal-value table. /// (Reserved for future expansion; not consumed today.) @@ -1142,7 +1139,7 @@ fn buildHeaderSection(state: *State, app: *App, arena: std.mem.Allocator, lines: var spy_bufs: [5][16]u8 = undefined; var spy_label_buf: [32]u8 = undefined; const spy_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&spy_label_buf, config.benchmark_stock, stock_pct * 100), + view.fmtBenchmarkLabel(&spy_label_buf, config.benchmarkStock(), stock_pct * 100), comparison.stock_returns, &spy_bufs, false, @@ -1152,7 +1149,7 @@ fn buildHeaderSection(state: *State, app: *App, arena: std.mem.Allocator, lines: var agg_bufs: [5][16]u8 = undefined; var agg_label_buf: [32]u8 = undefined; const agg_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&agg_label_buf, config.benchmark_bond, pctx.bond_pct * 100), + view.fmtBenchmarkLabel(&agg_label_buf, config.benchmarkBond(), pctx.bond_pct * 100), comparison.bond_returns, &agg_bufs, false, @@ -1892,7 +1889,7 @@ fn buildLines(state: *State, app: *App, arena: std.mem.Allocator) ![]const Style var spy_bufs: [5][16]u8 = undefined; var spy_label_buf: [32]u8 = undefined; const spy_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&spy_label_buf, config.benchmark_stock, stock_pct * 100), + view.fmtBenchmarkLabel(&spy_label_buf, config.benchmarkStock(), stock_pct * 100), comparison.stock_returns, &spy_bufs, false, @@ -1902,7 +1899,7 @@ fn buildLines(state: *State, app: *App, arena: std.mem.Allocator) ![]const Style var agg_bufs: [5][16]u8 = undefined; var agg_label_buf: [32]u8 = undefined; const agg_row = view.buildReturnRow( - view.fmtBenchmarkLabel(&agg_label_buf, config.benchmark_bond, ctx.bond_pct * 100), + view.fmtBenchmarkLabel(&agg_label_buf, config.benchmarkBond(), ctx.bond_pct * 100), comparison.bond_returns, &agg_bufs, false, diff --git a/src/views/projections.zig b/src/views/projections.zig index f054927..c402e53 100644 --- a/src/views/projections.zig +++ b/src/views/projections.zig @@ -831,8 +831,15 @@ fn buildContextFromParts( // Symbols default to SPY/AGG; user can override via // `type::config,benchmark_stock::SYMBOL` and // `type::config,benchmark_bond::SYMBOL` in projections.srf. - const stock_sym = config.benchmark_stock; - const bond_sym = config.benchmark_bond; + // + // Read through the accessors, not the backing buffers: `config` is a + // value copy (this function's local, and it gets copied again into + // the returned `ProjectionContext`), and the accessors derive their + // slice from whichever copy you hold. This is the read that surfaced + // the old self-referential-slice bug - an override arrived here as + // NUL bytes and `getCandles` silently found no such symbol. + const stock_sym = config.benchmarkStock(); + const bond_sym = config.benchmarkBond(); const spy_result = svc.getCandles(stock_sym, .{}) catch null; defer if (spy_result) |r| r.deinit(); const spy_divs = svc.getCachedDividends(alloc, stock_sym);