From 1d2cf3280646edba8e7b4dffeaced1843ee408d9 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Mon, 3 Aug 2026 23:09:45 -0700 Subject: [PATCH] migrate to fetchoptions --- AGENTS.md | 2 +- src/PortfolioData.zig | 69 +++++++++++++++++++++++++++------------ src/service.zig | 16 --------- src/tui.zig | 3 +- src/tui/portfolio_tab.zig | 9 +++-- 5 files changed, 57 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5beb6ea..72c6924 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -836,7 +836,7 @@ command. - **Mutual fund detection is heuristic.** `isMutualFund` checks if the symbol is exactly 5 chars ending in 'X'. This skips earnings fetching for mutual funds. It's imperfect but covers the common case. -- **SRF string lifetimes.** When reading SRF records, string fields point into the iterator's internal buffer. If you need strings to outlive the iterator, use a `postProcess` callback to `allocator.dupe()` them (see `dividendPostProcess` in `service.zig`). +- **SRF string lifetimes are handled for you.** `Store.read` parses with a `parse_allocator`, so SRF dupes every owned string into the caller's allocator automatically. **Do not add a `postProcess` callback to dupe strings** - that is not a valid reason for one. `postProcess` exists only for non-trivial post-parse logic, such as recomputing a derived field (see `earningsPostProcess` in `service.zig`, which rebuilds `surprise` from `actual` and `estimate`). - **Buffered stdout.** CLI output uses a single `std.Io.Writer` with a 4096-byte stack buffer, flushed once at the end of `main()`. Don't write to stdout through other means. diff --git a/src/PortfolioData.zig b/src/PortfolioData.zig index 392a350..475227a 100644 --- a/src/PortfolioData.zig +++ b/src/PortfolioData.zig @@ -169,14 +169,15 @@ pub const LoadOptions = struct { /// counts. The TUI uses this to render the /// "Syncing from server... [N/M]" bar before vaxis takes over. aggregate_progress: ?AggregateProgressCallback = null, - /// True forces re-fetch of every symbol regardless of cache - /// TTL; false honors TTLs. Maps to - /// `DataService.LoadAllConfig.force_refresh`. - force_refresh: bool = false, - /// Skip provider fetches and server sync entirely. Returns - /// cached data (even if stale); cache miss treated as failure. - /// Maps to `DataService.LoadAllConfig.skip_network`. - skip_network: bool = false, + /// Cache policy for this load, straight from the invocation's + /// `--refresh-data` setting (build it with + /// `cli.fetchOptionsFromPolicy`). Carried as a whole rather than as + /// two loose bools: callers already hold a `FetchOptions`, so + /// flattening it here meant every call site re-assembled it by hand + /// and a site that forgot silently got network access it was told not + /// to use. `reload` did exactly that, putting a TUI launched with + /// `--refresh-data=never` back online on refresh. + fetch_options: FetchOptions = .{}, /// Watchlist symbols (from a separate `watchlist.srf` file). /// Their prices land in `pd.watchlist_prices`, separate from /// the portfolio summary's allocations. pd internally unions @@ -264,7 +265,7 @@ svc: *DataService, /// only the dividends warm needs this: it runs inside a worker (to keep /// Polygon's 4/min bucket off the first-paint path) and a worker cannot see /// `opts`. Reset on every load/reload. -fetch_opts: FetchOptions = .{}, +fetch_options: FetchOptions = .{}, /// Parsed portfolio file path(s). Arena-owned `[]const u8` /// strings; arena-owned outer slice. `paths[0]` is the anchor @@ -607,15 +608,12 @@ pub fn load( self.classification_map_data = null; // Capture the cache policy for the workers that fetch (dividends). - self.fetch_opts = .{ - .force_refresh = opts.force_refresh, - .skip_network = opts.skip_network, - }; + self.fetch_options = opts.fetch_options; // candles_data lives in candles_arena and survives across // reloads - kept entries are reused, only new symbols hit // the cache. force_refresh wipes it wholesale. - if (opts.force_refresh) { + if (opts.fetch_options.force_refresh) { _ = self.candles_arena.reset(.retain_capacity); self.candles_data = null; } @@ -656,7 +654,7 @@ pub fn load( // Opt-in split adjustment before positions are aggregated, so the // TUI's positions and valuation carry effective shares. No-op // unless `splits_current_through` is set in metadata.srf. - portfolio_loader.enrichLotsSplits(self.svc, gpa, pf.lots, syms, self.paths[0], today, self.fetch_opts); + portfolio_loader.enrichLotsSplits(self.svc, gpa, pf.lots, syms, self.paths[0], today, self.fetch_options); const positions = pf.positions(today, gpa) catch return error.NoAllocations; defer gpa.free(positions); @@ -712,8 +710,8 @@ pub fn load( syms, watch_syms_list.items, .{ - .force_refresh = opts.force_refresh, - .skip_network = opts.skip_network, + .force_refresh = opts.fetch_options.force_refresh, + .skip_network = opts.fetch_options.skip_network, .color = false, // pd doesn't render; caller's progress callback owns UI. }, opts.aggregate_progress, @@ -1029,11 +1027,11 @@ fn dividendsWorker(self: *PortfolioData, delay_ms: usize) void { // `getDividends` would emit a "stale-cached returned (skip_network)" // info line per symbol for no benefit. warm: { - if (self.fetch_opts.skip_network) break :warm; + if (self.fetch_options.skip_network) break :warm; const div_syms = arena_alloc.alloc([]const u8, summary_ref.allocations.len) catch break :warm; for (summary_ref.allocations, div_syms) |alloc, *dst| dst.* = alloc.symbol; self.io.checkCancel() catch return; - self.svc.loadAllDividends(div_syms, self.fetch_opts); + self.svc.loadAllDividends(div_syms, self.fetch_options); } var map = std.StringHashMap([]const Dividend).init(arena_alloc); @@ -1247,6 +1245,37 @@ test "PortfolioData.load: NoPaths error on empty paths slice" { try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{})); } +test "PortfolioData.load: the cache policy is captured for the fetching workers" { + // Regression: `LoadOptions` used to carry `force_refresh`/`skip_network` + // as loose bools, so every call site re-assembled them and a site that + // forgot got network access it had been told not to use. `reload` was + // exactly such a site, putting a `--refresh-data=never` TUI session back + // online on refresh. Carrying the whole `FetchOptions` makes the policy + // impossible to drop silently, and this pins that it reaches the field + // the dividends worker reads. + var svc: DataService = .{ + .allocator = testing.allocator, + .io = testing.io, + .config = .{ .cache_dir = "./.tmp/zfin-pd-test-cache" }, + }; + var pd = PortfolioData.init(.{ + .gpa = testing.allocator, + .io = testing.io, + .svc = &svc, + }); + defer pd.deinit(); + + // Default is network-allowed. + try testing.expect(!pd.fetch_options.skip_network); + + // load() captures the policy before it can fail on paths, so offline + // intent survives even a failed load. + try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{ + .fetch_options = .{ .skip_network = true }, + })); + try testing.expect(pd.fetch_options.skip_network); +} + test "PortfolioData.reload: NoPaths error before any successful load" { var svc: DataService = .{ .allocator = testing.allocator, @@ -1506,7 +1535,7 @@ test "PortfolioData.candles_data: force_refresh resets the candles arena" { // load() with force_refresh=true and empty paths should // wipe candles_data before failing on NoPaths. The // candles_arena is reset, candles_data is nulled. - try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{ .force_refresh = true })); + try testing.expectError(error.NoPaths, pd.load(&.{}, Date.fromYmd(2026, 1, 1), .{ .fetch_options = .{ .force_refresh = true } })); try testing.expect(pd.candles_data == null); } diff --git a/src/service.zig b/src/service.zig index 35c9028..bfbc805 100644 --- a/src/service.zig +++ b/src/service.zig @@ -2092,22 +2092,6 @@ pub const DataService = struct { return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator }; } - /// Read earnings from cache only (no network fetch). See - /// `getCachedCandles` for the allocator contract. - pub fn getCachedEarnings(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(EarningsEvent) { - var s = self.store(); - const result = s.read(allocator, EarningsEvent, symbol, earningsPostProcess, .any) orelse return null; - return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator }; - } - - /// Read options from cache only (no network fetch). See - /// `getCachedCandles` for the allocator contract. - pub fn getCachedOptions(self: *DataService, allocator: std.mem.Allocator, symbol: []const u8) ?FetchResult(OptionsChain) { - var s = self.store(); - const result = s.read(allocator, OptionsChain, symbol, null, .any) orelse return null; - return .{ .data = result.data, .source = .cached, .timestamp = result.timestamp, .allocator = allocator }; - } - // ── Portfolio price loading ────────────────────────────────── /// Status emitted for each symbol during price loading. diff --git a/src/tui.zig b/src/tui.zig index c295949..8599ae7 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -2697,8 +2697,7 @@ pub fn run( .progress = symbol_progress.callback(), .aggregate_progress = aggregate_progress.callback(), .watchlist_syms = watch_syms.items, - .force_refresh = app_inst.fetch_options.force_refresh, - .skip_network = app_inst.fetch_options.skip_network, + .fetch_options = app_inst.fetch_options, }) catch |err| blk: { std.log.scoped(.tui).warn("portfolio load failed: {t}", .{err}); break :blk null; diff --git a/src/tui/portfolio_tab.zig b/src/tui/portfolio_tab.zig index dd4a88a..d1f7e65 100644 --- a/src/tui/portfolio_tab.zig +++ b/src/tui/portfolio_tab.zig @@ -448,6 +448,7 @@ pub const tab = struct { .watchlist_syms = watch_syms.items, .live_quotes = &live, .live_quotes_at_s = live_at_s, + .fetch_options = app.fetch_options, }) catch |err| { app.setStatus("Error refreshing portfolio data"); std.log.scoped(.tui).warn("portfolio.reload: {t}", .{err}); @@ -2281,11 +2282,13 @@ pub fn reloadPortfolioFile(state: *State, app: *App) void { for (wl) |sym| watch_syms.append(app.allocator, sym) catch |err| std.log.debug("watch_syms append failed: {t}", .{err}); } - // pd.reload re-uses captured paths, re-parses, re-fetches - // prices (.force_refresh = false -> honor cache TTLs), and - // spawns fresh workers. + // pd.reload re-uses captured paths, re-parses, re-fetches prices, and + // spawns fresh workers. The invocation's cache policy has to be passed + // through explicitly - omitting it defaults to "network allowed", which + // would put a `--refresh-data=never` session back online. _ = app.portfolio.reload(app.today, .{ .watchlist_syms = watch_syms.items, + .fetch_options = app.fetch_options, }) catch |err| { app.setStatus("Error reloading portfolio file"); std.log.scoped(.tui).warn("portfolio.reload: {t}", .{err});