diff --git a/TODO.md b/TODO.md index 6676e14..71fe80f 100644 --- a/TODO.md +++ b/TODO.md @@ -600,6 +600,119 @@ Once decisions are made, sweep all four sites + add a regression alignment test per table that mixes a fully-populated row with an em-dash-heavy row and verifies `displayCols` matches. +## CLI dispatch / arg-parsing bugs (found May 2026) + +Found during a post-framework-refactor sanity check of all 20 +commands plus interactive. The framework dispatch itself is +working correctly; these are gaps in command-level behavior or in +the global `--refresh-data` flag's coverage. + +### `--refresh-data=never` doesn't actually skip network calls — priority MEDIUM + +Today `never` and `auto` behave identically: both respect cache +TTLs, neither truly skips network. The help text promises "no +provider calls (offline mode)" but that's aspirational — the +underlying `svc.loadAllPrices` loader has no `skip_network` knob. +See `src/commands/common.zig:280-291` for the documented +approximation. + +Fix: add a `skip_network: bool` option to the loader's options +struct, threaded through to `getCandles` etc. so a stale entry is +served from cache instead of triggering a refetch when the user +explicitly opts in to offline mode. Then map `.never` → `.{ .skip_network = true }` +in `loadPortfolioPrices`. + +Alternative: revise the help text to drop the offline-mode claim +and document `never` as "TTL-respecting" (which is what it +actually does). Less work, less honest. + +### `--refresh-data` ignored by single-symbol commands — priority MEDIUM + +The 7 multi-symbol commands (portfolio, analysis, snapshot, audit, +compare, contributions, projections) honor +`ctx.globals.refresh_policy` because they go through +`cli.loadPortfolioPrices`. The 12 single-symbol commands (perf, +quote, divs, splits, options, earnings, etf, history-symbol-mode, +lookup, plus version/cache which don't fetch) bypass it — they +call `svc.getCandles` etc. directly, which has its own TTL logic +and doesn't read the global flag. + +So `zfin --refresh-data=force perf AAPL` doesn't force a +re-fetch. The help text implies the flag is universal. + +Fix options: +- Thread `refresh_policy` into the per-symbol freshness check + in `service.zig`. Most invasive but most correct. +- Scope the flag's docs to "portfolio commands" and rename it. + Less work, smaller blast radius. +- Add a different flag (`--symbol-refresh`?) for single-symbol + cases. Worst of both — two flags users have to remember. + +### `interactive -s` without a symbol value silently launches the TUI — priority LOW + +`zfin interactive -s` (no value following) doesn't error. The +flag-parsing in `src/tui.zig:2071-2079` checks `if (i + 1 < args.len)` +and silently drops `-s` if no value follows. It also doesn't +validate that the value isn't another flag — `zfin interactive -s --chart 80x24` +would treat `--chart` as the symbol. + +Same issue applies to `--chart` (lines 2080-2086). Both should +error out with a clear "flag requires a value" message and exit 1. + +Fix: rewrite the loop to use the framework's parseArgs convention +— error on missing values, error on flag-shaped values where a +positional value is expected. Or migrate the TUI flag parser to +the framework entirely (see "interactive command isn't framework- +registered" in main.zig's docs for the constraints). + +### `zfin interactive --help` printed help; `--default-keys`/`--default-theme` skip the TUI — priority LOW + +Acknowledged as fixed-with-caveat. The current implementation +intercepts `--help` at the dispatch site (main.zig). The TUI's +`--default-keys` / `--default-theme` print and `return` from +`tui.run` correctly. But: the TUI flag parser's "silently ignore +malformed flag" behavior (above) means +`zfin interactive --default-keys --bogus-flag` is accepted; the +bogus flag is silently dropped. Same fix as above. + +### `projections --vs ` crashes with `FileNotFound` when as-of resolves to imported source — priority MEDIUM + +Repro: `zfin projections --vs 2025-01-01` (any date that resolves +to imported_values rather than a real snapshot) → +`error: FileNotFound` panic with stack trace. + +Root cause: `loadAsOfContext` in `src/commands/projections.zig:760-762` +calls `history.loadSnapshotAt` unconditionally, but the resolution +returned by `resolveAsOfSnapshot` can have `source == .imported` +when the only available data point at that date is from +imported_values, not a real snapshot file. `loadSnapshotAt` then +fails to open a file that doesn't exist. + +`runBands` at line 369 has the same setup but correctly branches: +`if (resolution.?.source == .snapshot) { loadSnapshotAt(...) } else { loadProjectionContextFromImported(...) }`. +`loadAsOfContext` should mirror that branching. + +Pre-existing bug from before the framework refactor. Surfaced +during the sanity check because the new dispatcher correctly +identifies `FileNotFound` as not a user-level error and propagates +with stack trace. Pre-fix the same crash happened, just hidden. + +### `etf ` warns `failed to serialize ETF profile: WriteFailed` — priority LOW + +Every `zfin etf VTI` invocation prints +`warning(cache): VTI: failed to serialize ETF profile: WriteFailed` +to stderr before the foreground output. The ETF profile renders +correctly; just the cache write fails. + +`src/cache/store.zig:209` calls `serializeEtfProfile` which fails +on the `aw.writer.print(...)` call inside it (line 1007). Likely +a Zig 0.16 stdlib quirk in the SRF writer path or a missing +`flush()` somewhere in the writer chain. + +Investigate by replacing the `print` with a manual `print` + +`flush` to see if it's a buffer-not-flushed issue, or by +serializing a known-good fixture in isolation. + ## Low-priority items The following items are acknowledged but not prioritized. Listed here