update TODO
This commit is contained in:
parent
3ff42591ad
commit
3c0e8a3f4d
GPG key ID:
A7B62D657EF764F8
1 changed files with 179 additions and 154 deletions
333
TODO.md
333
TODO.md
|
|
@ -5,8 +5,9 @@ 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).
|
||||
**Next up:** configurable benchmark symbols (low-effort win, exercises
|
||||
the post-refactor framework on a small feature) and the manual-check
|
||||
accounts mechanism (medium effort, real user value).
|
||||
|
||||
## 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).
|
||||
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
|
||||
|
||||
**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
|
||||
that isn't real.
|
||||
|
||||
## Audit: manual-check accounts mechanism (NYL, Kelly'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
|
||||
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
|
||||
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.
|
||||
- **No-CSV-export accounts** (e.g. some insurance / annuity products)
|
||||
where values only live in periodic statements. Git can't detect a
|
||||
"change" because nothing changes locally; the user has to log in
|
||||
to see the new value.
|
||||
- **Payroll-deduction-then-purchase accounts** (e.g. ESPP) where
|
||||
payroll-deducted cash doesn't appear in the broker 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.
|
||||
|
||||
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.
|
||||
review*. For statement-only accounts, the value sometimes hasn't
|
||||
changed in months — so git never fires, cadence never trips.
|
||||
2. Payroll-deduction accounts need weekly-ish attention while
|
||||
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
|
||||
|
||||
|
|
@ -402,21 +284,164 @@ 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.
|
||||
correct fit for statement-only accounts.
|
||||
|
||||
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-style accrual 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.
|
||||
Payroll-deduction accounts are also a contribution-attribution blind
|
||||
spot. If a 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-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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue