update todo
This commit is contained in:
parent
b4db761733
commit
fa728b3f04
1 changed files with 113 additions and 0 deletions
113
TODO.md
113
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 <date>` 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 <SYMBOL>` 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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue