lobo/zfin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

update TODO

Browse source
This commit is contained in:
Emil Lerch 2026-05-16 12:20:13 -07:00
parent 0d934b4565
commit cd342e3f60
1 changed files with 179 additions and 154 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

333
TODO.md
View file

@ -5,8 +5,9 @@ ordered roughly by priority within each section. Priority labels
(`HIGH` / `MEDIUM` / `LOW`) mark items that deserve explicit (`HIGH` / `MEDIUM` / `LOW`) mark items that deserve explicit
ranking; unlabeled items are "someday, if the mood strikes." ranking; unlabeled items are "someday, if the mood strikes."
**Next up:** configurable benchmark symbols (low-effort win) and the **Next up:** configurable benchmark symbols (low-effort win, exercises
manual-check accounts mechanism (medium effort, real user value). the post-refactor framework on a small feature) and the manual-check
accounts mechanism (medium effort, real user value).
## Projections: future enhancements ## Projections: future enhancements
@ -175,136 +176,6 @@ renaming to `src/render.zig` to better describe what's left, or
splitting the braille chart out (it's ~600 lines on its own). splitting the braille chart out (it's ~600 lines on its own).
Not blocking — file it as cleanup if and when it bites. 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:
```zig
// 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 1–2 fields to `App`, and the
struct keeps growing. Eventually it becomes unreviewable.
## Bug: braille charts use raw `close`, not `adj_close` — cliff at splits ## Bug: braille charts use raw `close`, not `adj_close` — cliff at splits
**Reproduction:** `zfin quote SOXX` (or the TUI quote tab). The **Reproduction:** `zfin quote SOXX` (or the TUI quote tab). The
@ -371,27 +242,38 @@ on 2024-06-10) for a louder example.
But it's confusing when scanning a chart and seeing a 50% drop But it's confusing when scanning a chart and seeing a 50% drop
that isn't real. that isn't real.
## Audit: manual-check accounts mechanism (NYL, Riley's ESPP, etc.) — priority HIGH ## Audit: manual-check accounts mechanism — priority HIGH
Some accounts/positions can't be reconciled from broker CSVs and need a Some accounts/positions can't be reconciled from broker CSVs and need a
human-in-the-loop reminder at the audit step. Examples: human-in-the-loop reminder at the audit step. Two recurring shapes:
- **NY Life** — no CSV export at all. Values only live in periodic - **No-CSV-export accounts** (e.g. some insurance / annuity products)
statements. where values only live in periodic statements. Git can't detect a
- **Riley's ESPP** — accrued payroll-deduction cash doesn't appear in the "change" because nothing changes locally; the user has to log in
Fidelity positions CSV until the purchase date hits (typically every to see the new value.
6 months). Between purchases the cash is a real contribution that - **Payroll-deduction-then-purchase accounts** (e.g. ESPP) where
`zfin audit` can't see. payroll-deducted cash doesn't appear in the broker positions CSV
- Future: treat as an open category. until the purchase date hits (typically every 6 months). Between
purchases the cash is a real contribution that `zfin audit` can't
see.
The existing `update_cadence::weekly|monthly|quarterly|none` field already The existing `update_cadence::weekly|monthly|quarterly|none` field already
sort-of covers this, but has two gaps: sort-of covers this, but has two gaps:
1. It fires off the last *git-detected change*, not the last *human 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 review*. For statement-only accounts, the value sometimes hasn't
git never fires, cadence never trips. changed in months — so git never fires, cadence never trips.
2. ESPP needs weekly-ish attention while accumulating cash between 2. Payroll-deduction accounts need weekly-ish attention while
purchases, but the accrued balance is invisible to the CSV audit. accumulating cash between purchases, but the accrued balance is
invisible to the CSV audit.
Drift symptom seen in practice: several accounts on
`update_cadence::weekly` in `accounts.srf` weren't flagged as overdue
despite no changes in two weeks, because the cadence reads
git-detected change time rather than human-review time. The cadence
values themselves may also be wrong for these accounts — revisit
whether weekly is the right cadence vs. monthly/quarterly given how
rarely they actually change.
### Options ### Options
@ -402,21 +284,164 @@ B. **`last_refreshed::YYYY-MM-DD` field on `accounts.srf`** — explicit
human-review timestamp, decoupled from git-detected changes. Audit human-review timestamp, decoupled from git-detected changes. Audit
compares `today - last_refreshed` against the cadence. User bumps compares `today - last_refreshed` against the cadence. User bumps
the field when they check the statement. Probably the most the field when they check the statement. Probably the most
correct fit for NYL. correct fit for statement-only accounts.
C. **Sticky TODO list** — a `todos.srf` or `todo::` field on accounts C. **Sticky TODO list** — a `todos.srf` or `todo::` field on accounts
that audit always surfaces until cleared. General-purpose; also that audit always surfaces until cleared. General-purpose; also
covers "remember to rebalance on 5/15". covers "remember to rebalance on 5/15".
### ESPP-specific follow-through ### ESPP-style accrual follow-through
ESPP is also a contribution-attribution blind spot. If Riley's paystub Payroll-deduction accounts are also a contribution-attribution blind
deducts $X/week but the cash lot doesn't reach `portfolio.srf` until spot. If a paystub deducts $X/week but the cash lot doesn't reach
the purchase date, the attribution math is under-counting contributions `portfolio.srf` until the purchase date, the attribution math is
and over-counting the purchase-week gain. Possible fixes are discussed under-counting contributions and over-counting the purchase-week
in the "Contributions diff" TODO below — option C there (per-account gain. Possible fixes are discussed in the "Contributions diff" TODO
`cash_is_contribution`) would make manually-entered ESPP below — option C there (per-account `cash_is_contribution`) would
cash additions count correctly. make manually-entered ESPP-style cash additions count correctly.
## Investigate: audit phantom discrepancy on freshly-added lots — priority MEDIUM
**Observed during a weekly audit run:** `zfin audit` flagged a
discrepancy on a newly-added single-symbol lot that, on inspection,
was correctly entered in `portfolio.srf` and matched the brokerage.
The flag was a false positive.
Possible causes (not yet diagnosed):
- Audit logic mishandles lots whose `open_date::` is the audit's
reference date (today) — share count is "new" but no transaction
history yet supports it from the audit's perspective.
- The broker's positions CSV was internally inconsistent the morning
after a high-movement day. Other lots on the same export may
have been affected; only the new lot was noticed because it was new.
- Some interaction with rollup/DRIP classification on the new lot.
Repro: next time a brand-new lot is added on a heavy-movement day,
check whether `zfin audit` flags it without cause. Compare the
audit's reported brokerage figure against the raw CSV value to
isolate audit-logic bug from CSV-input bug.
Workaround for now: visual inspection. If `portfolio.srf` matches
the brokerage UI, ignore the audit flag for this cycle and move on.
## 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"
column populated for SPY/AGG/Benchmark rows but the **Your Portfolio**
row's Week cell is missing (or shows `--`). The 1Y/3Y/5Y/10Y portfolio
columns work fine.
Lead: `src/views/projections.zig` line ~394 calls
`performance.weekReturn` for SPY and AGG candles, but a corresponding
call for the portfolio aggregate doesn't appear to be wired in. The
benchmark week return is composed weighted from SPY+AGG; the
portfolio side needs its own weekly aggregation across held lots.
Probably a small fix — `weekReturn` already exists in
`src/analytics/performance.zig`, just needs to be applied to the
portfolio's value series (or to a synthetic weighted candle from
held positions) and threaded into the view model. Verify the TUI
projections tab has the same gap (`src/tui/projections_tab.zig`).
## Investigate: small dollar-value discrepancy between consecutive `compare` runs
**Symptom:** Last week's `zfin compare 1W` reported a "now" Total
Investable Assets that didn't match this week's `zfin compare 1W`
"then" value by a small but non-zero amount (low single-digit
thousands). The two numbers should be identical — both are reading
the same week-ago history snapshot file.
Candidates to investigate:
- Snapshot file mutated after last week's run (re-snapshot? manual
edit?). Check `git log -- history/<date>-portfolio.srf` and
`git diff` against any prior version if tracked.
- Live cache prices changed between runs and the snapshot path is
somehow falling through to live prices for some symbols. The
snapshot SHOULD be self-contained (shares × snapshotted price),
but verify by re-running `zfin compare <date> <date>` (same date
both ends) and confirming the same number both ways.
- Last week's reported "now" was computed against a working-copy
portfolio that was later edited (reconciliation tweak post-run),
while this week's "then" reads the committed snapshot. Cross-check
against any archived report output from last week.
- `price_ratio` or `adj_close` semantics differing between code paths
(the REPORT.md §2 caveat about commit-side using current prices
for DRIP/rollup deltas is a known inconsistency in attribution —
may or may not apply here).
If the source is the working-copy-vs-snapshot mismatch (third bullet),
that's a workflow issue, not a bug — but worth confirming.
## Investigate: detailed 401(k) contributions data source
Found a more detailed contributions screen on at least one
employer-sponsored 401(k) provider portal — distinct from the
standard positions/holdings view we already pull from. Worth
investigating whether this unlocks better attribution than what
we get from the positions CSV alone, and whether other 401(k)
providers expose similar screens.
Open questions to answer when picking this up:
- Which screen specifically (path / URL within the portal)? Is there
an export option, or is it view-only / scrape territory?
- What fields does it expose (employee pre-tax, employer match,
after-tax / mega-backdoor, by-pay-period dates, per-fund
allocations)?
- Refresh cadence — per-paycheck, daily, on-demand?
- Can it be auto-discovered like the existing audit CSVs, or
is it manual-entry territory?
If the export is structured and recurring, this could feed a
401(k)-specific contributions classifier that bypasses the lot-diff
heuristic for that account, similar to how `cash_is_contribution`
opts ESPP/HSA accounts into cash-based attribution.
Related: ESPP-style accrual blind spot in the "Audit: manual-check
accounts mechanism" section above.
## In-kind transfer support (`type::in_kind`) — priority MEDIUM ## In-kind transfer support (`type::in_kind`) — priority MEDIUM