From cbb90f1b1ad785dddb7dbd7fdea763d69d92f048 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sun, 20 Sep 2026 07:33:36 -0700 Subject: [PATCH] collapse smart earnings into the new framework --- docs/dev/caching-implementation.md | 22 ++- src/service.zig | 245 ++++++++++++++++++++--------- 2 files changed, 192 insertions(+), 75 deletions(-) diff --git a/docs/dev/caching-implementation.md b/docs/dev/caching-implementation.md index 903612e..0f70175 100644 --- a/docs/dev/caching-implementation.md +++ b/docs/dev/caching-implementation.md @@ -303,7 +303,7 @@ matters because `candles_meta.srf` is never created for a symbol that has no candles, so anything keying freshness off the meta file alone would treat such a symbol as perpetually stale and re-fetch it forever. -### `fetchCached` (dividends, splits, options, earnings, ...) +### `fetchCached` (dividends, splits, options, earnings) Everything that is not candles flows through the generic `fetchCached`, which is simpler because each type is a single file: @@ -316,8 +316,22 @@ which is simpler because each type is a single file: 3. Server sync (if configured); a fresh synced entry returns, subject to the same `needsRefresh` check. 4. Provider fetch; on success write with the type's TTL; on - `NotFound` write a negative entry; on transient error return - `FetchFailed` without poisoning the cache. + `NotFound` write a negative entry; on `NoApiKey` return `NoApiKey`; + on any other transient error return `FetchFailed`. Neither of the + last two poisons the cache. + +`NoApiKey` is propagated rather than collapsed into `FetchFailed` +because it is the one distinction callers act on differently: +`commands/earnings.zig` and `tui/earnings_tab.zig` both name the +environment variable to set, which they cannot do from a generic +failure. It also must not negative-cache -- nothing is wrong with the +symbol, and poisoning it would suppress the fetch after the key is +finally configured. + +`getEarnings` is a thin wrapper over this, not a parallel +implementation: it short-circuits mutual funds (no quarterly earnings +exist, so there is nothing to cache or fetch) and then delegates, +supplying `earningsPostProcess` and `earningsNeedsRefreshHook`. #### The two comptime hooks @@ -644,7 +658,7 @@ the upstream provider on every request. | Per-type generic fetch | `fetchCached` - `src/service.zig` | | Fresh-but-incomplete decision | `serveFreshOrDiscard` - `src/service.zig` | | Dividend schedule check | `dividendsNeedRefresh`, `dividendCadenceDays` - `src/service.zig` | -| Earnings smart refresh | `earningsNeedsRefresh` - `src/service.zig` | +| Earnings smart refresh | `earningsNeedsRefresh`, `earningsNeedsRefreshHook` - `src/service.zig` | | Candle fetch + incremental | `getCandles` - `src/service.zig` | | Batch price load | `loadAllPrices` - `src/service.zig` | | Live quotes (uncached) | `loadLiveQuotes`, `getQuote` - `src/service.zig` | diff --git a/src/service.zig b/src/service.zig index 0b245fe..e9d0125 100644 --- a/src/service.zig +++ b/src/service.zig @@ -606,6 +606,17 @@ pub const DataService = struct { s.writeWithSource(T, symbol, retried, data_type.ttl(), sourceHintFor(T)); return .{ .data = retried, .source = .fetched, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator }; } + // A MISSING KEY IS A CONFIGURATION FAULT, not a data fault, + // and it is the one distinction callers act on differently: + // `commands/earnings.zig` and `tui/earnings_tab.zig` both + // name the environment variable to set, which they cannot do + // from a generic FetchFailed. Propagate it rather than + // collapsing it, and do not negative-cache - nothing is + // wrong with the symbol. + if (err == DataError.NoApiKey) { + log.warn("{s}: {s} unavailable: no API key configured", .{ symbol, @tagName(data_type) }); + return DataError.NoApiKey; + } // Only NotFound (provider says "this symbol genuinely has // no data of this type") gets a negative-cache entry. // Transient failures (network, 5xx, auth misconfig, parse @@ -673,6 +684,10 @@ pub const DataService = struct { var cboe = try self.getProvider(Cboe); return cboe.fetchOptionsChain(self.allocator, symbol); }, + EarningsEvent => { + var fmp = try self.getProvider(Fmp); + return fmp.fetchEarnings(self.allocator, symbol); + }, else => @compileError("unsupported type for fetchFromProvider"), }; } @@ -1629,12 +1644,30 @@ pub const DataService = struct { return false; } + /// `earningsNeedsRefresh` bound to the production window, in the shape + /// `fetchCached`'s `needsRefresh` hook takes. + /// + /// The three-argument form stays separate so tests can sweep the + /// window boundary without reaching for the constant; this adapter is + /// what production passes. + fn earningsNeedsRefreshHook(events: []const EarningsEvent, today: Date) bool { + return earningsNeedsRefresh(events, today, earnings_actual_chase_days); + } + /// Fetch earnings history for a symbol. - /// Checks cache first; fetches from FMP if stale/missing. - /// Smart refresh: even if cache is fresh, re-fetches when a *recent* - /// past earnings date (within `earnings_actual_chase_days`) still has - /// no actual yet (results just came out). Older gaps are treated as - /// permanent and honored until TTL -- see `earningsNeedsRefresh`. + /// + /// A thin wrapper over `fetchCached`: the cache / offline / server-sync + /// / provider / negative-cache sequence is entirely generic, and the + /// two earnings-specific parts are supplied as hooks - + /// `earningsPostProcess` rebuilds the derived `surprise` field, and + /// `earningsNeedsRefreshHook` is the smart refresh (re-fetch inside the + /// 30-day TTL once a report date has passed but the actual is still + /// missing; see `earningsNeedsRefresh`). + /// + /// The only thing that cannot be a hook is the mutual-fund skip, which + /// short-circuits before any cache read: a fund has no quarterly + /// earnings at all, so there is nothing to cache, fetch, or + /// negative-cache. /// /// `opts.skip_network = true` -> returns cached data even if stale, /// returns FetchFailed on cache miss without touching the network. @@ -1644,70 +1677,7 @@ pub const DataService = struct { if (market.classify(symbol) == .mutual_fund) { return .{ .data = &.{}, .source = .cached, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator }; } - - var s = self.store(); - const today = fmt.todayDate(self.io); - - if (!opts.force_refresh) { - if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| { - // Re-fetch only when a recent report (within the chase - // window) is still missing its actual; older gaps never - // backfill, so honor the cache. Suppressed under - // skip_network (offline mode never refetches). - const needs_refresh = !opts.skip_network and - earningsNeedsRefresh(cached.data, today, earnings_actual_chase_days); - - if (!needs_refresh) { - log.debug("{s}: earnings fresh in local cache", .{symbol}); - return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator }; - } - // Stale: free cached events and re-fetch below - EarningsEvent.freeSlice(self.allocator, cached.data); - } - } - - if (opts.skip_network) { - // Offline mode: fall back to any cached entry (even stale) before giving up. - if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .any)) |cached| { - log.info("{s}: earnings stale-cached returned (skip_network)", .{symbol}); - return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator }; - } - return DataError.FetchFailed; - } - - // Try server sync before hitting FMP (skipped on force_refresh). - if (!opts.force_refresh and self.syncFromServer(symbol, .earnings)) { - if (s.read(self.allocator, EarningsEvent, symbol, earningsPostProcess, .fresh_only)) |cached| { - log.debug("{s}: earnings synced from server and fresh", .{symbol}); - return .{ .data = cached.data, .source = .cached, .timestamp = cached.timestamp, .allocator = self.allocator }; - } - log.debug("{s}: earnings synced from server but stale, falling through to provider", .{symbol}); - } - - log.debug("{s}: fetching earnings from provider", .{symbol}); - self.assertNetworkAllowed("getEarnings fmp.fetchEarnings"); - var fmp = try self.getProvider(Fmp); - - const fetched = fmp.fetchEarnings(self.allocator, symbol) catch |err| blk: { - if (err == error.RateLimited) { - self.rateLimitBackoff(); - break :blk fmp.fetchEarnings(self.allocator, symbol) catch { - return DataError.FetchFailed; - }; - } - if (isPermanentProviderFailure(err)) { - s.writeNegative(symbol, .earnings); - } - return DataError.FetchFailed; - }; - - // Delegate to the centralized TTL policy (DataType.earnings.ttl) - // so the per-key jitter applies - a bare `.{ .seconds = ... }` - // here would re-expire a whole portfolio in lockstep at the 30d - // boundary. - s.write(EarningsEvent, symbol, fetched, cache.DataType.earnings.ttl()); - - return .{ .data = fetched, .source = .fetched, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator }; + return self.fetchCached(EarningsEvent, symbol, earningsPostProcess, earningsNeedsRefreshHook, opts); } /// Fetch ETF profile for a symbol. Assembles a unified @@ -4895,7 +4865,10 @@ test "fetchCached hook: an overdue distribution discards the fresh entry and ref try std.testing.expectEqual(@as(usize, 6), fresh.data.len); } - try std.testing.expectError(DataError.FetchFailed, svc.getDividends("TEST", .{})); + // NoApiKey, not FetchFailed: reaching the provider at all is the + // proof that the fresh cache was discarded, and the specific error + // is what tells us we got as far as key resolution. + try std.testing.expectError(DataError.NoApiKey, svc.getDividends("TEST", .{})); // A transient failure must not poison the cache - the entry is still // there for the next run, and no negative entry was written. @@ -4924,7 +4897,7 @@ test "fetchCached hook: force_refresh never reaches it" { var store = svc.store(); store.write(Dividend, "TEST", divs[0..], cache.DataType.dividends.ttl()); - try std.testing.expectError(DataError.FetchFailed, svc.getDividends("TEST", .{ .force_refresh = true })); + try std.testing.expectError(DataError.NoApiKey, svc.getDividends("TEST", .{ .force_refresh = true })); } test "fetchCached hook: a symbol with too little history is left to the TTL" { @@ -4988,6 +4961,136 @@ test "fetchCached hook: getOptions passes no hook and is unaffected" { try std.testing.expectEqual(Source.cached, result.source); } +test "fetchCached: a missing API key propagates as NoApiKey, not FetchFailed" { + // Errors carry information. `commands/earnings.zig` and + // `tui/earnings_tab.zig` both switch on NoApiKey specifically so they + // can name the environment variable to set - "Error fetching earnings + // data" sends the user to read source code instead. Collapsing it + // into FetchFailed erases the one distinction any caller acts on. + // + // Also: a missing key must NOT negative-cache. Nothing is wrong with + // the symbol, and poisoning it would suppress the fetch after the key + // is finally configured. + const allocator = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir_path); + + var svc = DataService.init(io, allocator, Config{ .cache_dir = dir_path }); + defer svc.deinit(); + + // Cold cache, no keys configured, for each type routed through + // `fetchCached` that needs one. + try std.testing.expectError(DataError.NoApiKey, svc.getDividends("TEST", .{})); + try std.testing.expectError(DataError.NoApiKey, svc.getSplits("TEST", .{})); + try std.testing.expectError(DataError.NoApiKey, svc.getEarnings("TEST", .{})); + + // No negative entry was written for any of them. + try std.testing.expect(svc.getCachedDividends(allocator, "TEST") == null); +} + +test "getEarnings: the mutual-fund skip short-circuits before any cache or network work" { + // The one earnings-specific branch that could not become a hook. A + // fund has no quarterly earnings at all, so this must return empty + // without reading the cache, syncing, or resolving a provider - and + // in particular without the NoApiKey that a real fetch would raise + // here, since no FMP key is configured. + const allocator = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir_path); + + var svc = DataService.init(io, allocator, Config{ .cache_dir = dir_path }); + defer svc.deinit(); + + svc.panic_on_network_attempt = true; + const result = try svc.getEarnings("VBTLX", .{}); + defer result.deinit(); + try std.testing.expectEqual(@as(usize, 0), result.data.len); + try std.testing.expectEqual(Source.cached, result.source); + + // A non-fund ticker does NOT take the skip: it reaches the generic + // path, where a cold cache under skip_network is FetchFailed. + try std.testing.expectError(DataError.FetchFailed, svc.getEarnings("TESTA", .{ .skip_network = true })); +} + +test "getEarnings: smart refresh survives the move onto fetchCached" { + // The behaviour the refactor had to preserve. A fresh cache holding a + // past report with no actual must re-fetch; the same cache with the + // actual filled in must be served. Before the refactor this logic sat + // in a hand-rolled copy of `fetchCached`; now it is the generic hook, + // and this is what proves the wiring is real. + const allocator = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir_path); + + var svc = DataService.init(io, allocator, Config{ .cache_dir = dir_path }); + defer svc.deinit(); + + const today = fmt.todayDate(io); + var store = svc.store(); + + // A report three days ago with no actual yet -> inside the chase + // window -> must refetch, which with no FMP key surfaces as NoApiKey. + var pending = [_]EarningsEvent{ + .{ .symbol = "TEST", .date = today.addDays(-3), .estimate = 1.5 }, + }; + store.write(EarningsEvent, "TEST", pending[0..], cache.DataType.earnings.ttl()); + try std.testing.expectError(DataError.NoApiKey, svc.getEarnings("TEST", .{})); + + // Same report with the actual posted -> nothing outstanding -> served + // from cache without touching the provider. + var settled = [_]EarningsEvent{ + .{ .symbol = "TEST", .date = today.addDays(-3), .estimate = 1.5, .actual = 1.62 }, + }; + store.write(EarningsEvent, "TEST", settled[0..], cache.DataType.earnings.ttl()); + + svc.panic_on_network_attempt = true; + const result = try svc.getEarnings("TEST", .{}); + defer result.deinit(); + try std.testing.expectEqual(@as(usize, 1), result.data.len); + try std.testing.expectEqual(Source.cached, result.source); + // `earningsPostProcess` still runs through the generic path: surprise + // is derived, not stored. + try std.testing.expect(result.data[0].surprise != null); + try std.testing.expectApproxEqAbs(@as(f64, 0.12), result.data[0].surprise.?, 1e-9); +} + +test "getEarnings: skip_network suppresses the smart refresh" { + // Offline mode must serve the incomplete-but-fresh entry rather than + // discard it and fail. Same rule the dividend hook follows, and it now + // comes from one place instead of two. + const allocator = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir_path = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir_path); + + var svc = DataService.init(io, allocator, Config{ .cache_dir = dir_path }); + defer svc.deinit(); + + const today = fmt.todayDate(io); + var store = svc.store(); + var pending = [_]EarningsEvent{ + .{ .symbol = "TEST", .date = today.addDays(-3), .estimate = 1.5 }, + }; + store.write(EarningsEvent, "TEST", pending[0..], cache.DataType.earnings.ttl()); + + svc.panic_on_network_attempt = true; + const result = try svc.getEarnings("TEST", .{ .skip_network = true }); + defer result.deinit(); + try std.testing.expectEqual(@as(usize, 1), result.data.len); + try std.testing.expectEqual(Source.cached, result.source); +} + test "loadAllDividends: honors skip_network for every symbol, and one miss does not abort the rest" { const allocator = std.testing.allocator; const io = std.testing.io;