zfin/TODO.md

29 KiB
Raw Blame History

Future Work

No work here is blocking — we're in a good state. Items below are ordered roughly by priority within each section. Priority labels (HIGH / MEDIUM / LOW) mark items that deserve explicit ranking; unlabeled items are "someday, if the mood strikes."

Next up: configurable benchmark symbols (low-effort win) and the manual-check accounts mechanism (medium effort, real user value).

Projections: future enhancements

  • Configurable benchmark symbols — priority MEDIUM. Currently hardcoded SPY + AGG. Route through projections.srf as a type::config,benchmark::SYMBOL record (or similar). Low effort.
  • 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 shows the full accumulation_years + horizon span without a visual marker for where accumulation ends and distribution begins. Easier to add to the kitty-graphics chart than the braille one.
  • Goal-seek over distribution horizon for W1 — priority LOW. Today the W1 ("set spending, find date") workflow reports the earliest retirement at each user-configured (horizon, confidence) cell. The philosophically correct version asks "when have I accumulated enough wealth that the projection shows a 95% probability of success withdrawing X per year from retirement until age-of-death?" — i.e. goal-seek across both accumulation_years AND distribution_years simultaneously, anchored to a configured age-of-death. NP-shaped search; not worth optimizing until someone wants it.
  • Per-person retirement_age — priority LOW. V1 of the accumulation-phase spec chose Option A: a single household retirement boundary derived from the oldest configured birthdate. Households where one earner retires significantly earlier than the other would benefit from per-person retirement_age fields on each type::birthdate record, with contributions stopped per-person.
  • Configurable max_accumulation_years — priority LOW. Hardcoded at 50 years. Route through projections.srf if anyone hits the cap.
  • Configurable MIN period selection (currently 3Y/5Y/10Y, exclude 1Y)
  • Multiple spending models: flat (current), decreasing (1-2% real annual decrease, Blanchett "spending smile"). Late-life healthcare better modeled as a life event.
  • Unclassified position handling in allocation split (warn user)
  • Historical projection overlay follow-ups. The base --overlay-actuals overlay shipped (CLI tip + TUI primary surface). Open enhancements:
    • Historical metadata.srf / projections.srf for back-dated runs. Today the overlay re-runs against current classifications and assumptions; for historically faithful what-the-model-said-then output we'd check out the git-tracked versions of those files at the as-of commit and load those instead. Edge case until classifications materially drift.
    • Contribution-attribution overlay. Today's actuals line includes contributions implicitly; the bands assume modeled contributions that may or may not match reality. A "decompose actuals into market return vs contributions" annotation would clarify how much of the trajectory was the model being right vs new money arriving on schedule.
    • Mosaic mode: overlay multiple as-of starting points on one chart ("show me 1Y, 3Y, 5Y, 10Y projections all at once") so the user can see how the projection envelope tightened as data came in.
    • Better composition basis for imported-only as-of. Today the imported-only path uses today's allocations scaled by imported_liquid / today_total_liquid. That's the simplest thing that could work, but it's "today's mix back-dated" — it ignores everything we know about the historical context. Specifically: imported_values.srf already carries an expected_return field per row that the user captured at that date in their source spreadsheet. We could:
      • Use the imported expected_return as a sanity check against the simulation's per-position weighted return (warn or clamp if they diverge wildly — the spreadsheet's number reflects what the user actually saw at the time).
      • Use the imported expected_return to bias the stock/bond split inference: a higher expected return implies a higher historical equity weighting than today's mix probably reflects.
      • Reach further: derive a synthetic stock/bond split from the imported expected_return directly, treating it as a weighted average of SPY and AGG returns at that date and solving for the weights. That gives a per-imported- row composition that's locally faithful instead of one-mix-fits-all. None of these are urgent — the current "today's mix scaled" approximation is documented as such and the bands still render meaningfully — but each would tighten the historical faithfulness one notch. Pick whichever has the highest payoff vs. complexity when this gets revisited.
    • Chart zoom for short-history overlays. With a 50-year projection horizon and only ~10 years of imported actuals, the actuals line is squashed into the first 20% of the chart and the comparison-against-bands story is hard to read. Two design directions:
      • Auto-zoom: when the overlay is on, the chart's x-axis defaults to [as_of, today + N years] (where N is small, e.g. 2x the actuals span) instead of [as_of, as_of + horizon]. The bands beyond `today
        • N` are still computed but clipped from view. The tradeoff: the user loses the long-tail terminal-value context unless they toggle back out.
      • Toggle: a separate keybind (e.g. z for zoom) flips between full-horizon and zoomed views. Default off so the bands tell their full story; user opts in when they want overlay legibility. Auto-zoom is more invasive (changes the default chart semantics for everyone running with overlay-on) but better matches what the user actually wants when they toggle the overlay. Toggle is safer but requires the user to know the feature exists. Probably do auto-zoom but expose a toggle to escape it ("show full horizon").

Export chart as PNG (--export-chart <path>) — priority MEDIUM

z2d already supports PNG export natively. Today the chart-bearing commands (quote, history, projections, plus the equivalent TUI tabs) render to braille (CLI) or Kitty graphics (TUI). Adding a --export-chart <path> flag would land just the chart (not the surrounding text output) as a PNG file at the given path, at full fidelity, regardless of which surface invoked it.

Driver: when reviewing a back-dated projection or a notable price move, capturing the chart as an image (e.g. for a write-up, an email to the household, or a wiki page) is currently a screenshot-and-crop chore. PNG export makes it a one-shot CLI invocation.

Sketch:

  • zfin quote AAPL --export-chart aapl.png → just the price+ Bollinger chart as a PNG, no other output.
  • zfin projections --as-of 1Y --overlay-actuals --export-chart projection.png → the projection-bands chart plus overlay, no other output.
  • The chart code already produces RGB pixel buffers via z2d; replace the transmitPreEncodedImage call (TUI) or the braille text path (CLI) with a Surface.write_png call when the flag is present.

Plumbing: a thin "chart-only render" entry point in each chart module (projection_chart.zig, chart.zig for symbols), called from the relevant command's run() when --export-chart is set. Exits before the rest of the text output renders.

Out of scope for V1: file-format alternatives (SVG, PDF), themed color overrides for export (always uses the active terminal theme), non-chart command output as PNG.

Refactor: trim src/format.zig once Money / Date have absorbed their helpers — priority LOW

src/format.zig is still a ~1700-line grab-bag, but the money- and date-shaped helpers that used to live there have been moved out: money formatting now lives in src/Money.zig (with {f} / whole() / trim() / signed() / padRight(N) / padLeft(N)), and date formatting lives in src/Date.zig (with {f} / padRight(N) / padLeft(N)). What's left in format.zig is the genuinely-format-domain stuff: braille charts, return formatters, allocation notes, signed-percent rendering.

If the file ever grows enough to be annoying again, consider renaming to src/render.zig to better describe what's left, or splitting the braille chart out (it's ~600 lines on its own). Not blocking — file it as cleanup if and when it bites.

Refactor: TUI App struct knows too much about each tab — priority MEDIUM

src/tui.zig's App struct currently has dozens of tab-specific fields scattered across its top level — projections_loaded, projections_disabled, projections_config, projections_ctx, projections_horizon_idx, projections_image_id, projections_image_width, projections_image_height, projections_chart_dirty, projections_chart_visible, projections_events_enabled, projections_value_min, projections_value_max, projections_as_of, projections_as_of_requested, projections_overlay_actuals, plus equivalents for portfolio, history, options, earnings, analysis, perf, and quote tabs.

This couples App to the implementation details of every tab. Touching a single tab's state shape requires editing the central struct, which makes refactors noisy and discourages tab modules from owning their own state cleanly.

Proposal: each tab gets exactly ONE field on App — a struct (or pointer to a struct) defined in that tab's own file. The tab module owns its state shape; App only carries the top-level reference.

Sketch:

// src/tui/projections_tab.zig
pub const State = struct {
    loaded: bool = false,
    disabled: bool = false,
    config: projections.UserConfig = .{},
    ctx: ?ProjectionContext = null,
    horizon_idx: usize = 0,
    chart: ChartState = .{},
    as_of: AsOfState = .{},
    overlay_actuals: bool = false,
    // ...
};

// src/tui.zig
pub const App = struct {
    // ... shared cross-tab state (today, allocator, vx_app, ...) ...
    portfolio: portfolio_tab.State = .{},
    quote: quote_tab.State = .{},
    perf: perf_tab.State = .{},
    history: history_tab.State = .{},
    projections: projections_tab.State = .{},
    options: options_tab.State = .{},
    earnings: earnings_tab.State = .{},
    analysis: analysis_tab.State = .{},
};

After the migration, app.projections_overlay_actuals becomes app.projections.overlay_actuals, etc. Each tab's State struct documents its own invariants without polluting App.

Benefits:

  • Tab modules become genuinely self-contained. Adding new state to a tab only touches that tab's file.
  • App's field count drops from ~80 to ~10 (cross-cutting state
    • 8 tab-state fields).
  • Clearer ownership: when reading App, you can tell at a glance which fields are shared vs tab-private.
  • Easier to reason about lifetimes — each tab's freeLoaded() operates on its own struct rather than reaching into App.
  • Onboarding new contributors: "to add a tab feature, define state in the tab file" instead of "edit two files in lockstep."

Migration approach:

This is a large mechanical refactor (~80+ field renames across all tab files plus tui.zig). Best done as one focused PR:

  1. Define State in each tab file with all current fields.
  2. Add the eight new fields to App; flip them on as defaults (so old app.projections_* accesses temporarily still work if we add accessor shims, but cleaner to just rip the bandaid).
  3. Sweep all app.<tab>_<field> references → app.<tab>.<field>.
  4. Delete the old top-level fields from App.
  5. Verify zig build test is unchanged. The refactor should be strictly behavior-preserving.

Risks: large diff (touches every tab file plus tui.zig), but mechanical — no logic changes, no tests should move. The pre- commit hooks catch any miss instantly.

While we're at it: action handler bodies should also move. The same shape problem shows up in tui.zig's keybind-action dispatch — sort_reverse, toggle_chart, toggle_events, account_filter, and the per-tab branches inside them are ~100 lines of tab-specific logic living in the central event loop. Concretely (line numbers as of writing):

  • sort_reverse (~line 1328) dual-dispatches by active tab: portfolio flips sort direction + calls sortPortfolioAllocations / rebuildPortfolioRows; projections toggles overlay-actuals + reloads data + sets status. None of that body is App-level concern; it's two separate tabs' private state mutations.
  • toggle_chart (~line 1382) flips projections_chart_visible and resets scroll_offset. Pure projections-tab business.
  • toggle_events (~line 1390) flips projections_events_enabled and triggers a reload. Same.
  • account_filter (~line 1366) opens the account picker mode with portfolio-tab-specific cursor positioning.

The cleaner shape: each tab module exposes a handleAction(app, action) bool (or similar) that returns true when it consumed the action. tui.zig's dispatch becomes a thin "ask the active tab if it wants this action; otherwise fall through to global handlers." The body of each case shrinks to a one-liner that dispatches to tab_module.handleAction(self).

This pairs naturally with the State-struct migration above — the tab module's handleAction operates on its own State struct rather than reaching into App. Some keybinds are genuinely cross-cutting (quit, refresh, tab navigation, scroll) and stay in the central handler. The split is "App owns chrome; tab owns content."

Driver: every overlay-actuals-shaped feature added to a tab recently has involved adding 12 fields to App, and the struct keeps growing. Eventually it becomes unreviewable.

Bug: braille charts use raw close, not adj_close — cliff at splits

Reproduction: zfin quote SOXX (or the TUI quote tab). The braille chart drops sharply on 2024-03-07, which is the iShares Semiconductor ETF's 3-for-1 split date:

  • 2024-03-06 close: $689.60
  • 2024-03-07 close: $237.75 (≈ $689.60 / 2.9)

The adj_close column in ~/.cache/zfin/SOXX/candles_daily.srf tracks correctly through the split (~$226 → ~$234), so the provider data is fine. The bug is purely cosmetic: the chart renders the unadjusted close price.

Root cause: computeBrailleChart in src/format.zig:888 indexes data[i].close instead of data[i].adj_close. Lines 901, 902, 904, 905, 935 all use .close.

Independent confirmation: zfin splits SOXX returns 2024-03-07 3:1 from Polygon. So the split data exists in the provider layer (and gets cached as splits.srf once requested), but the charting code path doesn't consult it.

Fix candidates:

A. Switch computeBrailleChart to consume adj_close directly. Simplest. Affects every chart caller (quote, history, projections median band, TUI quote/projections tabs). Cosmetic only — no computation depends on it. The price-axis labels would render adjusted prices, which may surprise users used to seeing the raw last-close. Mitigate with a comment in the chart's right-edge label region or a header note.

B. Pass a flag to computeBrailleChart selecting close vs adj_close. Default to adjusted; let the quote tab show raw. More flexible, marginally more code.

C. Add a chart_close accessor to Candle that returns adj_close if non-zero, else close. Same effect as (A) with a defensive fallback.

D. Apply split adjustments at chart-data prep time using splits.srf. Walk the candle slice with the split history and pre-multiply pre-split closes by the cumulative ratio. More work, but produces a chart-axis dollar value the user expects ("today's last close was $X, the chart starts at $Y from N years ago"). This is what most charting libraries do. Requires plumbing DataService.getSplits into the chart-prep path on every chart caller, OR doing the adjustment once in the service layer alongside candle fetching. Not all callers have a DataService reference today (e.g., runProjection's synthetic median-band candles).

Recommendation: Start with (A) or (C) — single-line fix, gets the cliff out of all charts immediately. (D) is the "correct" fix but a bigger refactor; file as a follow-up if (A)/(C) lands first.

Other affected symbols: Any held position with a split in the last 10 years will have the same artifact. Check NVDA (10:1 split on 2024-06-10) for a louder example.

Priority: LOW. Cosmetic only — analytics already use adj_close correctly via the per-position trailing-returns path. But it's confusing when scanning a chart and seeing a 50% drop that isn't real.

Audit: manual-check accounts mechanism (NYL, Kelly's ESPP, etc.) — priority HIGH

Some accounts/positions can't be reconciled from broker CSVs and need a human-in-the-loop reminder at the audit step. Examples:

  • NY Life — no CSV export at all. Values only live in periodic statements.
  • Kelly's ESPP — accrued payroll-deduction cash doesn't appear in the Fidelity positions CSV until the purchase date hits (typically every 6 months). Between purchases the cash is a real contribution that zfin audit can't see.
  • Future: treat as an open category.

The existing update_cadence::weekly|monthly|quarterly|none field already sort-of covers this, but has two gaps:

  1. It fires off the last git-detected change, not the last human review. For NYL, the value sometimes hasn't changed in months — so git never fires, cadence never trips.
  2. ESPP needs weekly-ish attention while accumulating cash between purchases, but the accrued balance is invisible to the CSV audit.

Options

A. New update_cadence::manual variant — always fires every audit run until silenced. Blunt but zero design work.

B. last_refreshed::YYYY-MM-DD field on accounts.srf — explicit human-review timestamp, decoupled from git-detected changes. Audit compares today - last_refreshed against the cadence. User bumps the field when they check the statement. Probably the most correct fit for NYL.

C. Sticky TODO list — a todos.srf or todo:: field on accounts that audit always surfaces until cleared. General-purpose; also covers "remember to rebalance on 5/15".

ESPP-specific follow-through

ESPP is also a contribution-attribution blind spot. If Kelly's paystub deducts $X/week but the cash lot doesn't reach portfolio.srf until the purchase date, the attribution math is under-counting contributions and over-counting the purchase-week gain. Possible fixes are discussed in the "Contributions diff" TODO below — option C there (per-account cash_is_contribution) would make manually-entered ESPP cash additions count correctly.

In-kind transfer support (type::in_kind) — priority MEDIUM

transaction_log.srf parses type::in_kind records but the contributions matcher always rejects them with "in-kind transfers not yet supported in v1." In-kind movements need per-symbol matching across accounts: an in-kind transfer of 100 VTI shares from Acct A to Acct B shows up as lot_removed on A + new_stock on B (or a rollup_delta share increase if B already had a VTI lot), neither of which can be matched by the current amount-based cash matcher.

Proposed: a second pass in matchTransfers that iterates type::in_kind records and looks for same-symbol matches across lot_removed on from + new_stock/rollup_delta on to within the window. Gated on share-count and open_price sanity so a partial transfer doesn't false-positive against an unrelated edit.

Driver: when the user starts moving positions between accounts directly (e.g. Roth conversion of already-held shares, 401k → rollover IRA in-kind) rather than liquidating and re-buying.

Torn SRF files from server sync (root cause unknown)

Status: Root cause still unidentified. We have mitigations and diagnostics in place that keep torn responses from corrupting the cache, but we don't yet know why responses arrive torn. Until we have a root cause, this is not resolved — it's mitigated.

Mitigations landed so far:

  • syncFromServer (src/service.zig) validates responses via cache.Store.looksCompleteSrf before writeRaw. Torn HTTP bodies (empty, missing #!srfv1 header, or no trailing newline) are rejected with a warn-level log and NOT written to cache.
  • HTTP responses are checked for an ETag sha256 header; on mismatch we retry the request once before giving up and falling back to the provider.
  • Read-path self-heal: on SRF parse failure during read, the cache entry is invalidated so a subsequent refresh can repair without user intervention.
  • Diagnostics: richer error capture around the sync path. So far, HTTP transit is the dominant source of torn responses — but that's an observation, not a root cause.

Remaining work:

  • Identify root cause. Candidates to investigate: proxy/load-balancer behavior, HTTP keepalive reuse, partial reads on the server side, client-side buffer handling. The etag retry tells us whether the problem is per-request or persistent; dig into the diagnostics output when the next occurrence is captured.
  • Once root cause is known, decide whether the current mitigations are sufficient or whether a targeted fix is needed. The mitigations may end up being the whole answer, but we can't conclude that without understanding the underlying cause.

(Content-Length validation was considered and rejected: once the server starts compressing response bodies, Content-Length reflects the compressed byte count, not the decoded payload, so it's not a reliable integrity check.)

Market-aware cache TTL for daily candles

Daily candle TTL is currently 23h45m, but candle data only becomes meaningful after the market close. Investigate keying the cache freshness to ~4:30 PM Eastern rather than a rolling window. This would avoid unnecessary refetches during the trading day and ensure a fetch shortly after close gets fresh data. Probably alleviated by the cron job approach.

On-demand server-side fetch for new symbols

Currently the server's SRF endpoints (/candles, /dividends, etc.) are pure cache reads — they 404 if the data isn't already on disk. New symbols only get populated when added to the portfolio and picked up by the next cron refresh.

Consider: on a cache miss, instead of blocking the HTTP response with a multi-second provider fetch, kick off an async background fetch (or just auto-add the symbol to the portfolio) and return 404 as usual. The next request — or the next cron run — would then have the data. This gives "instant-ish gratification" for new symbols without the downsides of synchronous fetch-on-miss (latency, rate limit contention, unbounded cache growth from arbitrary tickers).

Note that this process doesn't do anything to eliminate all the API keys that are necessary for a fully functioning system. A more aggressive view would be to treat ZFIN_SERVER as a 100% source of record, but that would introduce some opacity to the process as we wait for candles (for example) to populate. This could be solved on the server by spawning a thread to fetch the data, then returning 202 Accepted, which could then be polled client side. Maybe this is a better long term approach?

Low-priority items

The following items are acknowledged but not prioritized. Listed here so they don't get lost; pick up opportunistically.

UX

  • CLI options command UX. The options command auto-expands only the nearest monthly expiration and lists others collapsed. Reconsider the interaction model — e.g. allow specifying an expiration date, showing all monthlies expanded by default, or filtering by strategy (covered calls, spreads).
  • TUI: toggle to last symbol keybind. A single-key toggle that flips between the current symbol and the previously selected one (like cd - in bash or Ctrl+^ in vim). Store last_symbol on App; on symbol change, stash the previous. Useful for eyeball-comparing performance/risk data between two symbols.

Data quality

  • Fix enrich command for international funds. deriveMetadata in src/commands/enrich.zig misclassifies international ETFs:

    1. geo uses Alpha Vantage's Country field, which is the fund issuer's domicile (USA for all US-listed ETFs), not the fund's investment geography. Every US-domiciled international fund gets geo::US.
    2. asset_class short-circuits to "ETF" when asset_type == "ETF", or falls through to a US-market-cap heuristic that always produces "US Large Cap" / "US Mid Cap" / "US Small Cap".

    Known misclassified tickers (all came back as geo::US, asset_class::US Large Cap):

    • FRDM — Freedom 100 Emerging Markets ETF → should be geo::Emerging Markets, asset_class::Emerging Markets
    • HFXI — NYLI FTSE International Equity Currency Neutral ETF → should be geo::International Developed, asset_class::International Developed
    • IDMO — Invesco S&P International Developed Momentum ETF → should be geo::International Developed, asset_class::International Developed
    • IVLU — iShares MSCI International Developed Value Factor ETF → should be geo::International Developed, asset_class::International Developed

    The Alpha Vantage OVERVIEW endpoint doesn't provide fund geography data. Options: use the ETF_PROFILE holdings/country data to infer geography, parse the fund name for keywords ("International", "Emerging", "ex-US"), or accept that enrich is a scaffold and emit a # TODO comment for ETFs instead of silently misclassifying.

Options / valuation

  • Per-account covered call adjustment. adjustForCoveredCalls in valuation.zig operates on portfolio-wide aggregated allocations. It matches sold calls against total underlying shares across all accounts. This is wrong — calls in one account can only cover shares in that same account. Fixing means restructuring portfolioSummary, since Allocation is currently account-agnostic. Low priority — naked calls are rare, and calls are typically in the same account as the underlying.
  • Covered call adjustment O(N*M) loop. adjustForCoveredCalls has a nested loop — for each allocation, it iterates all lots to find matching option contracts. Fine for personal portfolios (<1000 lots). Pre-indexing options by underlying would help if someone had a very large options-heavy portfolio.

Analysis / correctness

  • Analysis account/asset-class total mismatch. The "By Account" and "By Tax Type" sections in the analysis command sum to slightly more than "Asset Class" (~0.6% error). Likely a discrepancy between how the lot-level account loop values cash, CDs, or options vs how the asset-class section computes them via portfolio.totalCash() / totalCdFaceValue(). Per-account values themselves are correct after the price_ratio fix.

Audit

  • Audit large-lot threshold tuning. src/commands/audit.zig uses audit_large_lot_threshold: f64 = 10_000.0 as the cutoff for "surface this new lot for confirmation." Revisit if $10k proves too aggressive (ESPP accruals spam the report) or too permissive (large DRIP confirmations slip past). If runtime tuning becomes necessary, a --large-lot <amount> flag or a global audit_large_lot_threshold field on accounts.srf would be reasonable extensions.

Infra / performance

  • HTTP connection pooling. Parallel server sync in loadAllPrices spawns up to 8 threads, each with its own HTTP connection. Could reuse connections to reduce TCP handshake overhead. Only matters with very large portfolios (100+ symbols) hitting ZFIN_SERVER.
  • Streaming cache deserialization. Cache store reads entire files into memory (readFileAlloc with 50MB limit). For portfolios with 10+ years of daily candles, this could use significant memory. Keep current approach unless memory becomes a real problem.