From 26a71125722405bf3af0081a66a25fcd08a5e121 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 11 Aug 2026 17:49:10 -0700 Subject: [PATCH] add diagnose command, properly handle demand tracked vs always refreshed symbols --- src/analytics/projections.zig | 67 ++++ src/cache/freshness.zig | 51 ++- src/commands/cache.zig | 20 +- src/commands/common.zig | 116 +++---- src/commands/diagnose.zig | 598 ++++++++++++++++++++++++++++++++++ src/commands/doctor.zig | 20 +- src/main.zig | 5 + src/service.zig | 124 ++++++- src/tui.zig | 3 + 9 files changed, 928 insertions(+), 76 deletions(-) create mode 100644 src/commands/diagnose.zig diff --git a/src/analytics/projections.zig b/src/analytics/projections.zig index 8b8fd3f..4228529 100644 --- a/src/analytics/projections.zig +++ b/src/analytics/projections.zig @@ -689,6 +689,35 @@ const SrfProjection = union(enum) { /// silently producing nonsense. pub const max_abs_spending_real_change: f64 = 0.10; +/// The benchmark stock/bond symbols configured by the `projections.srf` at +/// `path`. +/// +/// Returns the SPY/AGG defaults when the file is absent or silent. +/// +/// Lives here, next to the config it reads. Takes a resolved PATH rather than a +/// directory so this module does no path arithmetic: locating a file beside the +/// portfolio anchor is one rule that belongs in one place +/// (`commands/common.siblingPath`), and an earlier directory-taking version of +/// this had three callers each hand-rolling that join. +/// +/// Command code should not call this directly - go through +/// `commands/common.demandFetchedSymbols`, so that "which symbols are fetched on +/// demand" stays a fetch-policy question and the cache sweep does not have to +/// know that the answer happens to come from projections config. +/// +/// The strings are DUPED into `arena`. An overridden symbol lives in a `[16]u8` +/// field inside the returned `UserConfig`, so a borrowed slice dangles the +/// moment that struct goes out of scope - which is the whole reason this exists +/// rather than callers reading the config themselves. +pub fn benchmarkSymbols(io: std.Io, arena: std.mem.Allocator, path: []const u8) []const []const u8 { + const data = std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(64 * 1024)) catch null; + const cfg = parseProjectionsConfig(data); + const pair = arena.alloc([]const u8, 2) catch return &.{}; + pair[0] = arena.dupe(u8, cfg.benchmark_stock) catch return &.{}; + pair[1] = arena.dupe(u8, cfg.benchmark_bond) catch return &.{}; + return pair; +} + /// Parse a projections.srf file into a UserConfig. /// Returns default config if data is null or unparseable. /// @@ -4265,3 +4294,41 @@ test "integration: declining model + late healthcare troughs mid-retirement" { // ...which is strictly before the final distribution year. try std.testing.expect(t.year_offset < dist_years - 1); } + +test "benchmarkSymbols: defaults to SPY/AGG when projections.srf is absent" { + // These two are fetched for the benchmark comparison and held nowhere, which + // is why AGG went stale unnoticed while SPY - which doubles as a `ticker::` + // alias on a real holding - stayed current. + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const pair = benchmarkSymbols(std.testing.io, arena_state.allocator(), "/nonexistent/projections.srf"); + try std.testing.expectEqual(@as(usize, 2), pair.len); + try std.testing.expectEqualStrings("SPY", pair[0]); + try std.testing.expectEqualStrings("AGG", pair[1]); +} + +test "benchmarkSymbols: an override is honoured and outlives the config" { + const allocator = std.testing.allocator; + const io = std.testing.io; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const dir = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir); + { + const f = try tmp.dir.createFile(io, "projections.srf", .{}); + defer f.close(io); + var buf: [256]u8 = undefined; + var w = f.writer(io, &buf); + try w.interface.writeAll("#!srfv1\ntype::config,benchmark_stock::VTI,benchmark_bond::BND\n"); + try w.interface.flush(); + } + var arena_state = std.heap.ArenaAllocator.init(allocator); + defer arena_state.deinit(); + const path = try std.fs.path.join(arena_state.allocator(), &.{ dir, "projections.srf" }); + // Duped, not borrowed: an override lives in a [16]u8 inside the UserConfig, + // which dies with this call. + const pair = benchmarkSymbols(io, arena_state.allocator(), path); + try std.testing.expectEqual(@as(usize, 2), pair.len); + try std.testing.expectEqualStrings("VTI", pair[0]); + try std.testing.expectEqualStrings("BND", pair[1]); +} diff --git a/src/cache/freshness.zig b/src/cache/freshness.zig index 3cd918a..e4d33b1 100644 --- a/src/cache/freshness.zig +++ b/src/cache/freshness.zig @@ -163,24 +163,38 @@ pub const Report = struct { /// needs to know nothing about what any of them are. /// /// Symbol strings borrow from `keys`; only the slice is allocated. +/// +/// `exclude` is a caller-supplied set of symbols to leave out of the corpus. The +/// meaning is the caller's - this module only needs to know that no verdict it +/// could reach would be true of them. pub fn collect( allocator: std.mem.Allocator, store: *cache.Store, keys: []const []const u8, tracked: *const std.StringHashMap(void), + exclude: []const []const u8, ) ![]Entry { - var out = try allocator.alloc(Entry, keys.len); - errdefer allocator.free(out); - for (keys, 0..) |key, i| { + var out = std.ArrayList(Entry).empty; + errdefer out.deinit(allocator); + outer: for (keys) |key| { + // Excluded keys are dropped from the corpus entirely rather than + // classified, because every classification would be wrong for them: a + // symbol fetched on demand is not stale (whatever needs it fetches it) + // and not an orphan (something does refresh it). Taking the exclusion + // here rather than at each caller means both the sweep and `doctor` skip + // them by construction instead of by remembering to. + for (exclude) |ex| { + if (std.mem.eql(u8, key, ex)) continue :outer; + } const cm = store.readCandleMeta(key); - out[i] = .{ + try out.append(allocator, .{ .symbol = key, .kind = market.classify(key), .last_date = if (cm) |m| m.meta.last_date else null, .tracked = tracked.contains(key), - }; + }); } - return out; + return out.toOwnedSlice(allocator); } /// Classify `entries` as of `now_s`. @@ -555,3 +569,28 @@ test "scan: every allocation-failure path unwinds cleanly" { .{ @as([]const Entry, &entries), fridayEvening() }, ); } + +test "collect-style exclusion: an excluded symbol produces no finding at all" { + const a = testing.allocator; + // A demand-fetched symbol makes every classification wrong: not stale + // (whatever needs it fetches it) and not orphaned (something does refresh + // it). Dropping it from the corpus is the only honest option, and the caller + // footnotes it instead. + const fri = Date.fromYmd(2025, 6, 13); + const entries = [_]Entry{ + eq("AAPL", fri, true), + eq("MSFT", fri, true), + // AGG-shaped: well behind, untracked, and not a finding. + eq("AGG", Date.fromYmd(2025, 6, 2), false), + }; + // With it present it would land in orphans, which reads as "delete this". + var with = try scan(a, &entries, fridayEvening()); + defer with.deinit(a); + try testing.expectEqual(@as(usize, 1), with.orphans.len); + + // Excluded at collect time, it is simply absent. + var without = try scan(a, entries[0..2], fridayEvening()); + defer without.deinit(a); + try testing.expectEqual(@as(usize, 0), without.orphans.len); + try testing.expectEqual(@as(usize, 0), without.stale.len); +} diff --git a/src/commands/cache.zig b/src/commands/cache.zig index 47e2397..6c2d915 100644 --- a/src/commands/cache.zig +++ b/src/commands/cache.zig @@ -229,6 +229,11 @@ const Sweep = struct { arena_state: std.heap.ArenaAllocator, store: Store, report: freshness.Report, + /// Demand-fetched symbols left out of the report, for the footnote. Not a + /// finding - `projections` refreshes these when it runs, and nothing else + /// wants them - but worth naming so their absence is not mistaken for them + /// being checked and clean. + excluded: []const []const u8 = &.{}, /// True when there was nothing to look at. empty: bool, @@ -267,17 +272,20 @@ fn sweep(ctx: *framework.RunCtx) !Sweep { // symbol counts as untracked, which is honest - without one there is // nothing to be tracked BY. var tracked = std.StringHashMap(void).init(arena); + var bench: []const []const u8 = &.{}; if (cli.loadPortfolio(ctx, ctx.today)) |loaded_pf| { var l = loaded_pf; defer l.deinit(allocator); - tracked = try cli.trackedSymbols(ctx, arena, l.portfolio, l.anchor()); + tracked = try cli.trackedSymbols(ctx, arena, l.portfolio); + bench = cli.demandFetchedSymbols(io, arena, l.anchor()); } - const entries = try freshness.collect(arena, &store, keys, &tracked); + const entries = try freshness.collect(arena, &store, keys, &tracked, bench); return .{ .arena_state = arena_state, .store = store, .report = try freshness.scan(arena, entries, now_s), + .excluded = bench, .empty = false, }; } @@ -319,7 +327,7 @@ fn runStale(ctx: *framework.RunCtx) !void { for (report.stale) |f| { try out.print(" {s:<10} {f} {d}d behind {f}\n", .{ f.symbol, f.last_date, f.days_behind, f.peer_date }); } - try out.print("\n zfin --refresh-data=force portfolio re-fetches the tracked set\n", .{}); + try out.print("\n zfin cache refresh re-fetches exactly these\n", .{}); } else if (report.far_behind.len == 0) { try out.print("\nNothing behind its peers.\n", .{}); } @@ -348,6 +356,12 @@ fn runStale(ctx: *framework.RunCtx) !void { try out.print("\n", .{}); } + if (sw.excluded.len > 0) { + try out.print("\nNot checked ({d}): ", .{sw.excluded.len}); + for (sw.excluded, 0..) |sym, i| try out.print("{s}{s}", .{ if (i == 0) "" else ", ", sym }); + try out.print(" - benchmark symbols, fetched on demand by `zfin projections`.\n", .{}); + } + if (report.orphans.len > 0) { try out.print("\nCached but tracked by nothing ({d}): ", .{report.orphans.len}); for (report.orphans, 0..) |sym, i| try out.print("{s}{s}", .{ if (i == 0) "" else ", ", sym }); diff --git a/src/commands/common.zig b/src/commands/common.zig index 9425a7c..3b24ee7 100644 --- a/src/commands/common.zig +++ b/src/commands/common.zig @@ -8,7 +8,6 @@ const framework = @import("framework.zig"); const stderr = @import("../stderr.zig"); pub const fmt = @import("../format.zig"); const theme = @import("../tui/theme.zig"); -const projections = @import("../analytics/projections.zig"); // ── Active CLI text palette ────────────────────────────────── // RGB foreground colors for ALL CLI (non-TUI) text output, emitted as @@ -389,12 +388,17 @@ fn printLoadSummaryImpl(io: std.Io, color: bool, s: LoadSummaryStats) !void { // `cli.` references. const portfolio_loader = @import("../portfolio_loader.zig"); +const projections = @import("../analytics/projections.zig"); /// The set of symbols zfin intends to keep fresh, for a loaded portfolio. /// -/// Holdings, `security_type::watch` lots, `watchlist.srf`, and the projections -/// benchmark pair - the union `Portfolio.fetchedSymbols` defines, with the -/// watchlist and benchmark inputs resolved from `ctx`. +/// Holdings, `security_type::watch` lots, and `watchlist.srf` - what routine +/// operation refreshes. +/// +/// Benchmark symbols are deliberately NOT here. They are fetched on demand and +/// by nothing routine, so calling them "tracked" made `zfin cache stale` list +/// AGG as needing action and made `diagnose` report "nothing is holding it back" +/// about a symbol no routine run asks for. See `demandFetchedSymbols`. /// /// Keys borrow from `arena`; pass an arena that outlives every lookup. The /// scoped-`defer`-frees version of this dangled its keys before the caller read @@ -403,7 +407,6 @@ pub fn trackedSymbols( ctx: *framework.RunCtx, arena: std.mem.Allocator, portfolio: zfin.Portfolio, - anchor: []const u8, ) !std.StringHashMap(void) { var set = std.StringHashMap(void).init(arena); const wl = ctx.resolveWatchlistPath(); @@ -415,12 +418,33 @@ pub fn trackedSymbols( const syms = try portfolio.fetchedSymbols(arena, .{ .watchlist_syms = if (wl_syms) |w| w else &.{}, - .benchmarks = benchmarkPair(ctx.io, arena, anchor), }); for (syms) |sym| try set.put(sym, {}); return set; } +/// The complement of `trackedSymbols`: symbols something fetches on demand, so +/// no routine operation refreshes them and their staleness is not a finding. +/// +/// `anchor` is the resolved portfolio path; the config is read from beside it. +/// Returns empty when there is no config to read, which is the safe direction - +/// an unknown demand-fetched set means nothing gets excluded from a sweep. +/// +/// This exists so callers ask a FETCH-POLICY question. Today the answer is the +/// projections benchmark pair, but `zfin cache stale` and `doctor` have no +/// business importing an analytics module to find that out, and three of them +/// hand-rolled the same dirname arithmetic to do it. Keeping the projections +/// dependency behind this one function means the cache sweep depends on "what +/// does a routine run fetch", which is a question this module already owns. +pub fn demandFetchedSymbols( + io: std.Io, + arena: std.mem.Allocator, + anchor: []const u8, +) []const []const u8 { + const path = siblingPath(arena, anchor, "projections.srf") catch return &.{}; + return projections.benchmarkSymbols(io, arena, path); +} + /// A path to `name` in the same directory as `anchor`. /// /// The portfolio's siblings - `accounts.srf`, `metadata.srf`, @@ -436,29 +460,6 @@ pub fn siblingPath(arena: std.mem.Allocator, anchor: []const u8, name: []const u return std.fmt.allocPrint(arena, "{s}{s}", .{ anchor[0..dir_end], name }); } -/// The projections benchmark pair, read from `projections.srf` beside the -/// portfolio. Returns the SPY/AGG defaults when the file is absent or silent. -/// -/// These belong in the tracked set even though they are held nowhere: they are -/// fetched for the benchmark comparison, which is why AGG went stale unnoticed -/// while SPY - which doubles as a `ticker::` alias on a real holding - stayed -/// current. -/// -/// `views/projections.zig` reads the same file for the same config, but keeps -/// its `UserConfig` alive and reads the fields in place. This exists only -/// because the tracked set outlives the config: an overridden symbol lives in a -/// `[16]u8` field inside `UserConfig`, so the slices must be copied out before -/// that struct dies. -pub fn benchmarkPair(io: std.Io, arena: std.mem.Allocator, anchor: []const u8) []const []const u8 { - const path = siblingPath(arena, anchor, "projections.srf") catch return &.{}; - const data = std.Io.Dir.cwd().readFileAlloc(io, path, arena, .limited(64 * 1024)) catch null; - const cfg = projections.parseProjectionsConfig(data); - const pair = arena.alloc([]const u8, 2) catch return &.{}; - pair[0] = arena.dupe(u8, cfg.benchmark_stock) catch return &.{}; - pair[1] = arena.dupe(u8, cfg.benchmark_bond) catch return &.{}; - return pair; -} - pub const LoadedPortfolio = portfolio_loader.LoadedPortfolio; pub const PortfolioData = portfolio_loader.PortfolioData; pub const loadPortfolioFromConfig = portfolio_loader.loadPortfolioFromConfig; @@ -1555,27 +1556,16 @@ test "siblingPath: joins a filename onto the anchor's directory" { try std.testing.expectEqualStrings("accounts.srf", try siblingPath(a, "portfolio.srf", "accounts.srf")); } -test "benchmarkPair: defaults to SPY/AGG when projections.srf is absent" { - // These two must be in the tracked set even though they are held nowhere. - // AGG went stale unnoticed for exactly this reason - it is fetched only for - // the benchmark comparison, while SPY looked fine because it doubles as a - // `ticker::` alias on a real holding. - var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); - defer arena_state.deinit(); - const pair = benchmarkPair(std.testing.io, arena_state.allocator(), "/nonexistent/portfolio.srf"); - try std.testing.expectEqual(@as(usize, 2), pair.len); - try std.testing.expectEqualStrings("SPY", pair[0]); - try std.testing.expectEqualStrings("AGG", pair[1]); -} - -test "benchmarkPair: an override in projections.srf is honoured" { +test "demandFetchedSymbols: reads the config beside the anchor, not the cwd" { + // The regression this guards: three call sites each hand-rolled the + // anchor-to-directory arithmetic, and one convention off by a separator + // silently reads nothing and returns the SPY/AGG defaults - which looks + // exactly like a portfolio that never configured a benchmark. Overriding + // both symbols is what makes the difference observable. 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); - { const f = try tmp.dir.createFile(io, "projections.srf", .{}); defer f.close(io); @@ -1584,16 +1574,28 @@ test "benchmarkPair: an override in projections.srf is honoured" { try w.interface.writeAll("#!srfv1\ntype::config,benchmark_stock::VTI,benchmark_bond::BND\n"); try w.interface.flush(); } + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + const a = arena.allocator(); + const dir = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(dir); - var arena_state = std.heap.ArenaAllocator.init(allocator); - defer arena_state.deinit(); - const anchor = try std.fs.path.join(allocator, &.{ dir_path, "portfolio.srf" }); - defer allocator.free(anchor); - - // Duped out of the config's stack-local override buffers - borrowing them - // would dangle the moment benchmarkPair returned. - const pair = benchmarkPair(io, arena_state.allocator(), anchor); - try std.testing.expectEqual(@as(usize, 2), pair.len); - try std.testing.expectEqualStrings("VTI", pair[0]); - try std.testing.expectEqualStrings("BND", pair[1]); + const anchor = try std.fs.path.join(a, &.{ dir, "portfolio.srf" }); + const syms = demandFetchedSymbols(io, a, anchor); + try std.testing.expectEqual(@as(usize, 2), syms.len); + try std.testing.expectEqualStrings("VTI", syms[0]); + try std.testing.expectEqualStrings("BND", syms[1]); +} + +test "demandFetchedSymbols: no config yields the defaults, never an error" { + // Excluding nothing is the safe direction for a sweep: a symbol wrongly + // included is a visible false finding, one wrongly excluded is silence. + // The defaults are still returned because SPY/AGG are what projections + // would itself use, so the exclusion matches what actually gets fetched. + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const syms = demandFetchedSymbols(std.testing.io, arena.allocator(), "/nonexistent/portfolio.srf"); + try std.testing.expectEqual(@as(usize, 2), syms.len); + try std.testing.expectEqualStrings("SPY", syms[0]); + try std.testing.expectEqualStrings("AGG", syms[1]); } diff --git a/src/commands/diagnose.zig b/src/commands/diagnose.zig new file mode 100644 index 0000000..55e7875 --- /dev/null +++ b/src/commands/diagnose.zig @@ -0,0 +1,598 @@ +//! `zfin diagnose SYMBOL` - walk the whole candle chain for one symbol and say +//! where it breaks. +//! +//! The counterpart to `doctor`, and deliberately its opposite in both axes: +//! `doctor` checks the whole system and never touches the network; this checks +//! one symbol and queries every tier, including the provider. +//! +//! It exists because the question "is the provider broken for this symbol, or is +//! it us?" came up repeatedly and the only way to answer it was a hand-rolled +//! sequence of curl commands against Tiingo and the shared server. That answer +//! belongs in the tool. +//! +//! READ-ONLY. Nothing here writes to the cache, which is the property that makes +//! it safe to run while diagnosing: `Tiingo.fetchCandles` returns candles without +//! caching them, the server check is a plain GET, and local state comes from +//! `Store.readCandleMeta`. +//! +//! RATE LIMIT CAVEAT: the provider query uses a throwaway `Tiingo` client with +//! its own limiter, so that one request is invisible to the shared hourly budget +//! `DataService` accounts for. On a free tier close to its cap, `diagnose` can be +//! the request that trips it. Accepted to avoid widening `DataService`'s API with +//! a public provider accessor; revisit if this command ever fetches in bulk. + +const std = @import("std"); +const zfin = @import("../root.zig"); +const cli = @import("common.zig"); +const framework = @import("framework.zig"); +const freshness = @import("../cache/freshness.zig"); +const Tiingo = @import("../providers/tiingo.zig").Tiingo; +const http = @import("../net/http.zig"); +const fmt = @import("../format.zig"); + +const Store = zfin.cache.Store; +const Date = zfin.Date; + +pub const ParsedArgs = struct { + symbol: []const u8, +}; + +pub const meta: framework.Meta = .{ + .name = "diagnose", + .group = .infra, + .synopsis = "Trace one symbol's candle data through cache, server and provider", + .help = + \\Usage: zfin diagnose SYMBOL + \\ + \\Reports, in order: + \\ local newest cached bar, TTL state, provider, failure count + \\ peers newest bar held by other symbols of the same kind + \\ server what ZFIN_SERVER offers, and whether it is ahead or behind + \\ provider what the upstream provider actually has right now + \\ + \\Then a verdict naming where the chain breaks, and what to do about it. + \\ + \\Read-only: no cache writes, no fetches into the cache. Answers + \\"is the provider broken for this symbol, or is it us?". + \\ + \\The provider query uses its own rate limiter, so it is not counted + \\against the shared hourly budget. One request per invocation. + \\ + , + .uppercase_first_arg = true, + .user_errors = error{MissingSymbol}, +}; + +pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs { + if (cmd_args.len < 1) { + cli.stderrPrint(ctx.io, "Error: 'diagnose' requires a symbol\n"); + return error.MissingSymbol; + } + return .{ .symbol = cmd_args[0] }; +} + +// ── verdict ────────────────────────────────────────────────── + +/// Everything the verdict is derived from. All optional, because any tier can be +/// absent: no local copy, a kind with no peers, no server configured, a provider +/// that returned nothing. +pub const Observed = struct { + local: ?Date = null, + /// True when the local copy's TTL has not yet lapsed, so nothing will fetch + /// it on the normal path regardless of how old the bar is. + local_fresh: bool = false, + peer: ?Date = null, + server: ?Date = null, + provider: ?Date = null, + /// Is the symbol in the set a normal run fetches (`fetchedSymbols`)? Null + /// when it could not be determined - no portfolio resolved - in which case + /// no conclusion is drawn from it. + tracked: ?bool = null, + /// Is it a projections benchmark symbol? Those are fetched on demand rather + /// than routinely, so "nothing tracks this" is true but misleading - the + /// remedy is `zfin projections`, not a config fix. + benchmark: bool = false, +}; + +pub const Verdict = enum { + /// Nothing cached locally at all. + not_cached, + /// Local matches the best evidence available. + current, + /// Fetched on demand rather than routinely: a benchmark symbol, refreshed + /// whenever `projections` runs. Not tracked, but not neglected either. + demand_fetched, + /// Nothing in the fetch set asks for this symbol, so no normal run will ever + /// update it however stale it gets. The most fundamental gate there is: it + /// outranks the TTL, because even a lapsed TTL is never consulted for a + /// symbol nobody requests. + not_tracked, + /// The provider has a newer bar and the local TTL is still in the future, so + /// the normal path will not look until it lapses. + held_by_ttl, + /// The provider has a newer bar, the TTL has lapsed, and the shared server is + /// also behind - so a normal run syncs the server's older copy and stops. + held_by_server, + /// The provider has a newer bar and nothing is holding us back; a refresh + /// should simply work. + refreshable, + /// Peers have moved on but the provider has nothing newer for THIS symbol. + /// The "is the provider broken for this symbol?" question answered yes: + /// whatever is wrong is upstream or in the symbol itself, not in this cache. + provider_behind_peers, + /// No provider answer to compare against. + unknown, + + pub fn summary(self: Verdict) []const u8 { + return switch (self) { + .not_cached => "nothing cached for this symbol", + .current => "up to date with everything available", + .demand_fetched => "a benchmark symbol, fetched on demand - `zfin projections` refreshes it when it runs", + .not_tracked => "nothing in the fetch set asks for this symbol, so no normal run will update it - however stale it gets", + .held_by_ttl => "the bar exists upstream, but the local TTL has not lapsed - nothing will fetch it until it does", + .held_by_server => "the bar exists upstream; the shared cache is behind too, so a normal run syncs its older copy and stops", + .refreshable => "the bar exists upstream and nothing is holding it back", + .provider_behind_peers => "peers have a newer bar but the provider has nothing newer for this symbol - the gap is upstream, not in this cache", + .unknown => "no provider answer, so the chain cannot be traced past the cache", + }; + } + + /// What to run, when there is something. Split from the symbol so this stays + /// pure and testable; `takes_symbol` exists because not every remedy is + /// per-symbol - `projections` takes none, and printing `zfin projections AGG` + /// hands the operator a command that does not work. + pub const Action = struct { + subcommand: []const u8, + takes_symbol: bool, + }; + + pub fn action(self: Verdict) ?Action { + return switch (self) { + .not_tracked, .held_by_ttl, .held_by_server, .refreshable => .{ .subcommand = "cache refresh", .takes_symbol = true }, + .not_cached => .{ .subcommand = "quote", .takes_symbol = true }, + .demand_fetched => .{ .subcommand = "projections", .takes_symbol = false }, + .current, .provider_behind_peers, .unknown => null, + }; + } +}; + +/// Classify what was observed. +/// +/// Pure, and the only part of this command worth testing exhaustively - the rest +/// is I/O and formatting. Ordering matters: `held_by_ttl` is checked before +/// `held_by_server` because a future TTL stops the fetch earlier in `getCandles` +/// than a server sync does, so it is the more proximate cause. +pub fn classify(o: Observed) Verdict { + const local = o.local orelse return .not_cached; + const provider = o.provider orelse return .unknown; + if (!local.lessThan(provider)) { + // Nothing newer to fetch. Whether that is healthy depends entirely on + // the peers: if they have moved on and this symbol's provider data has + // not, the problem is upstream. If nobody has moved, everything is + // simply up to date - and calling that "the gap is upstream" would + // report a healthy symbol as a fault. + if (o.peer) |peer| { + if (local.lessThan(peer)) return .provider_behind_peers; + } + return .current; + } + // A benchmark outranks both: it is genuinely untracked, but saying so would + // send the operator hunting for a config fix when the answer is simply that + // `projections` fetches it on demand. + if (o.benchmark) return .demand_fetched; + // Untracked outranks the TTL: a lapsed TTL is never even consulted for a + // symbol no code path requests. + if (o.tracked) |t| { + if (!t) return .not_tracked; + } + if (o.local_fresh) return .held_by_ttl; + if (o.server) |srv| { + if (srv.lessThan(provider)) return .held_by_server; + } + return .refreshable; +} + +// ── run ────────────────────────────────────────────────────── + +pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { + const io = ctx.io; + const out = ctx.out; + const symbol = parsed.symbol; + + var arena_state = std.heap.ArenaAllocator.init(ctx.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var obs: Observed = .{}; + + var store = Store.init(io, ctx.allocator, ctx.config.cache_dir); + + // ── local ──────────────────────────────────────────────── + if (store.readCandleMeta(symbol)) |m| { + obs.local = m.meta.last_date; + obs.local_fresh = store.isCandleMetaFresh(symbol); + try out.print("local newest {f}", .{m.meta.last_date}); + try out.print(", {s}", .{if (obs.local_fresh) "TTL still in the future" else "TTL lapsed"}); + try out.print(", {s}", .{@tagName(m.meta.provider)}); + if (m.meta.fail_count > 0) try out.print(", {d} consecutive failures", .{m.meta.fail_count}); + try out.print("\n", .{}); + } else { + try out.print("local nothing cached\n", .{}); + } + + // ── peers ──────────────────────────────────────────────── + const kind = zfin.market.classify(symbol); + { + const keys = store.cacheKeys(arena) catch &.{}; + var empty = std.StringHashMap(void).init(arena); + const entries = try freshness.collect(arena, &store, keys, &empty, &.{}); + var newest: ?Date = null; + var counted: usize = 0; + for (entries) |e| { + if (e.kind != kind) continue; + if (std.mem.eql(u8, e.symbol, symbol)) continue; + const bar = e.last_date orelse continue; + counted += 1; + if (newest == null or newest.?.lessThan(bar)) newest = bar; + } + obs.peer = newest; + if (newest) |n| { + try out.print("peers {d} other {s} cached, newest {f}", .{ counted, @tagName(kind), n }); + if (obs.local) |l| { + if (l.lessThan(n)) { + const days = @divTrunc(n.toEpoch() - l.toEpoch(), std.time.s_per_day); + try out.print(" - {s} is {d}d behind", .{ symbol, days }); + } + } + try out.print("\n", .{}); + } else { + try out.print("peers no other {s} cached, nothing to compare against\n", .{@tagName(kind)}); + } + } + + // ── tracked ────────────────────────────────────────────── + if (cli.loadPortfolio(ctx, ctx.today)) |loaded_pf| { + var l = loaded_pf; + defer l.deinit(ctx.allocator); + for (cli.demandFetchedSymbols(io, arena, l.anchor())) |b| { + if (std.mem.eql(u8, b, symbol)) obs.benchmark = true; + } + if (cli.trackedSymbols(ctx, arena, l.portfolio)) |set| { + obs.tracked = set.contains(symbol); + } else |_| {} + if (obs.benchmark) { + try out.print("tracked on demand - a projections benchmark symbol\n", .{}); + } else if (obs.tracked) |t| { + try out.print("tracked {s}\n", .{if (t) "yes - a normal run fetches this symbol" else "NO - no normal run fetches this symbol"}); + } + } + + // ── server ─────────────────────────────────────────────── + if (ctx.config.server_url) |base| { + if (serverNewest(io, arena, base, ctx.config.server_api_key, symbol)) |srv| { + obs.server = srv; + try out.print("server offers {f}", .{srv}); + if (obs.local) |l| { + if (srv.lessThan(l)) { + try out.print(" - BEHIND your local copy", .{}); + } else if (l.lessThan(srv)) { + try out.print(" - ahead of your local copy", .{}); + } else { + try out.print(" - same as local", .{}); + } + } + try out.print("\n", .{}); + } else { + try out.print("server no answer for this symbol\n", .{}); + } + } else { + try out.print("server ZFIN_SERVER not set\n", .{}); + } + + // ── provider ───────────────────────────────────────────── + if (ctx.config.tiingo_key) |key| { + // Throwaway client with its own limiter - see the caveat at the top of + // this file. One request. + var tg = Tiingo.init(io, arena, key, .{ .per_hour = ctx.config.tiingoHourlyLimit() }); + defer tg.deinit(); + const to = fmt.todayDate(io); + const from = to.addDays(-10); + if (tg.fetchCandles(arena, symbol, from, to)) |candles| { + if (candles.len == 0) { + try out.print("provider tiingo returned no bars for the last 10 days\n", .{}); + } else { + var newest = candles[0].date; + for (candles[1..]) |c| { + if (newest.lessThan(c.date)) newest = c.date; + } + obs.provider = newest; + try out.print("provider tiingo newest {f} ({d} bars in the last 10 days)\n", .{ newest, candles.len }); + } + } else |err| { + // Name the error. "provider failed" cannot distinguish a rate limit + // from a dead key from a symbol the provider does not carry, and + // those have completely different remedies. + try out.print("provider tiingo FAILED: {s}\n", .{@errorName(err)}); + } + } else { + try out.print("provider TIINGO_API_KEY not set\n", .{}); + } + + // ── verdict ────────────────────────────────────────────── + const verdict = classify(obs); + try out.print("\nverdict {s}\n", .{verdict.summary()}); + if (verdict.action()) |act| { + if (act.takes_symbol) { + try out.print(" try: zfin {s} {s}\n", .{ act.subcommand, symbol }); + } else { + try out.print(" try: zfin {s}\n", .{act.subcommand}); + } + } +} + +/// Newest bar the shared server reports for `symbol`, or null. +/// +/// A plain GET of the server's `candles_meta`, parsed for `last_date`. Read-only +/// by construction: nothing is written to the local cache, which matters because +/// the normal sync path would overwrite local bytes with the server's. +fn serverNewest( + io: std.Io, + arena: std.mem.Allocator, + base: []const u8, + api_key: ?[]const u8, + symbol: []const u8, +) ?Date { + const url = std.fmt.allocPrint(arena, "{s}/{s}/candles_meta", .{ base, symbol }) catch return null; + var client = http.Client.init(io, arena); + defer client.deinit(); + var hdr: [1]std.http.Header = .{.{ .name = "", .value = "" }}; + const extra: []const std.http.Header = if (api_key) |k| blk: { + hdr[0] = .{ .name = "X-API-Key", .value = k }; + break :blk hdr[0..1]; + } else &.{}; + var resp = client.request(.GET, url, null, extra) catch return null; + defer resp.deinit(); + return lastDateIn(resp.body); +} + +/// `last_date::` value from an SRF body. +fn lastDateIn(body: []const u8) ?Date { + const key = "last_date::"; + const idx = std.mem.indexOf(u8, body, key) orelse return null; + const rest = body[idx + key.len ..]; + const end = std.mem.indexOfAny(u8, rest, ",\n\r") orelse rest.len; + return Date.parse(rest[0..end]) catch null; +} + +// ── tests ──────────────────────────────────────────────────── + +const testing = std.testing; + +fn d(y: i16, m: u8, day: u8) Date { + return Date.fromYmd(y, m, day); +} + +test "classify: the observed failure - provider has it, TTL holds us back" { + // AMZN on Tue 2026-08-11: Tiingo had 08-10 for over a day, the local copy sat + // on 08-07, and the TTL ran to Tue 16:55 - so nothing would look until then. + // This is the state that prompted the command. + try testing.expectEqual(Verdict.held_by_ttl, classify(.{ + .local = d(2026, 8, 7), + .local_fresh = true, + .peer = d(2026, 8, 10), + .server = d(2026, 8, 7), + .provider = d(2026, 8, 10), + })); +} + +test "classify: TTL is reported before the server, being the earlier gate" { + // Both are true here. `getCandles` checks the TTL before it syncs, so the TTL + // is the more proximate cause and naming the server would send the operator + // to the wrong tier. + const v = classify(.{ + .local = d(2026, 8, 7), + .local_fresh = true, + .server = d(2026, 8, 7), + .provider = d(2026, 8, 10), + }); + try testing.expectEqual(Verdict.held_by_ttl, v); +} + +test "classify: a lapsed TTL with a lagging server names the server" { + try testing.expectEqual(Verdict.held_by_server, classify(.{ + .local = d(2026, 8, 7), + .local_fresh = false, + .server = d(2026, 8, 7), + .provider = d(2026, 8, 10), + })); +} + +test "classify: nothing in the way means a refresh should just work" { + // No server configured. + try testing.expectEqual(Verdict.refreshable, classify(.{ + .local = d(2026, 8, 7), + .local_fresh = false, + .provider = d(2026, 8, 10), + })); + // Server already current, so it is not the obstacle. + try testing.expectEqual(Verdict.refreshable, classify(.{ + .local = d(2026, 8, 7), + .local_fresh = false, + .server = d(2026, 8, 10), + .provider = d(2026, 8, 10), + })); +} + +test "classify: everything level is current, not a fault" { + // SPY on Tue 2026-08-11: local, peers, server and provider all on 08-10. + // The first draft called this "the gap is upstream", which reports a + // perfectly healthy symbol as a problem. + const v = classify(.{ + .local = d(2026, 8, 10), + .local_fresh = true, + .peer = d(2026, 8, 10), + .server = d(2026, 8, 10), + .provider = d(2026, 8, 10), + }); + try testing.expectEqual(Verdict.current, v); + try testing.expectEqual(@as(?Verdict.Action, null), v.action()); + + // Local somehow AHEAD of the provider, peers level - still nothing to do. + try testing.expectEqual(Verdict.current, classify(.{ + .local = d(2026, 8, 11), + .peer = d(2026, 8, 11), + .provider = d(2026, 8, 10), + })); +} + +test "classify: peers ahead with the provider stuck points upstream" { + // THE QUESTION THIS COMMAND EXISTS FOR, answered yes. Peers hold 08-11, the + // provider has nothing past 08-10 for this symbol, so no local action helps. + const v = classify(.{ + .local = d(2026, 8, 10), + .local_fresh = false, + .peer = d(2026, 8, 11), + .provider = d(2026, 8, 10), + }); + try testing.expectEqual(Verdict.provider_behind_peers, v); + try testing.expectEqual(@as(?Verdict.Action, null), v.action()); +} + +test "classify: absent tiers degrade to a named unknown, never a guess" { + // No local copy at all. + try testing.expectEqual(Verdict.not_cached, classify(.{ .provider = d(2026, 8, 10) })); + // No provider answer: the chain cannot be traced, and saying so beats + // inferring from peers. + try testing.expectEqual(Verdict.unknown, classify(.{ + .local = d(2026, 8, 7), + .peer = d(2026, 8, 10), + })); + // Nothing at all. + try testing.expectEqual(Verdict.not_cached, classify(.{})); +} + +test "classify: every verdict offering an action names a runnable command" { + for ([_]Verdict{ .not_cached, .current, .demand_fetched, .not_tracked, .held_by_ttl, .held_by_server, .refreshable, .provider_behind_peers, .unknown }) |v| { + try testing.expect(v.summary().len > 0); + // A subcommand, not a full command line - the caller appends the symbol, + // so this must never already contain one. + if (v.action()) |act| { + try testing.expect(act.subcommand.len > 0); + try testing.expect(std.mem.indexOf(u8, act.subcommand, "zfin") == null); + try testing.expect(std.mem.indexOf(u8, act.subcommand, "<") == null); + } + } +} + +test "lastDateIn: parses the meta shape, tolerates junk" { + const meta_body = "#!srfv1\n#!expires=1786481700\nlast_close:num:274.48,last_date::2026-08-07,provider::tiingo\n"; + try testing.expect(lastDateIn(meta_body).?.eql(d(2026, 8, 7))); + try testing.expectEqual(@as(?Date, null), lastDateIn("#!srfv1\n")); + try testing.expectEqual(@as(?Date, null), lastDateIn("last_date::not-a-date\n")); +} + +test "classify: an untracked symbol is named as such, ahead of the TTL" { + // THE CASE THAT PROMPTED THIS. A benchmark symbol whose shared-cache copy was + // CURRENT and whose provider had the bar - so the first version reported + // "nothing is holding it back" - and yet a normal run left it untouched, + // because no fetch path asks for benchmark symbols at all. + const v = classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .peer = d(2026, 8, 11), + .server = d(2026, 8, 11), + .provider = d(2026, 8, 11), + .tracked = false, + }); + try testing.expectEqual(Verdict.not_tracked, v); + + // Outranks a fresh TTL: even once that lapses, nothing consults it. + try testing.expectEqual(Verdict.not_tracked, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = true, + .provider = d(2026, 8, 11), + .tracked = false, + })); +} + +test "classify: tracked=null draws no conclusion" { + // No portfolio resolved, so trackedness is unknown. Guessing either way + // would be worse than falling through to the next observable cause. + try testing.expectEqual(Verdict.held_by_ttl, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = true, + .provider = d(2026, 8, 11), + .tracked = null, + })); + try testing.expectEqual(Verdict.refreshable, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .provider = d(2026, 8, 11), + .tracked = null, + })); +} + +test "classify: being tracked does not mask the real cause" { + try testing.expectEqual(Verdict.held_by_ttl, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = true, + .provider = d(2026, 8, 11), + .tracked = true, + })); + try testing.expectEqual(Verdict.held_by_server, classify(.{ + .local = d(2026, 6, 29), + .local_fresh = false, + .server = d(2026, 6, 29), + .provider = d(2026, 8, 11), + .tracked = true, + })); +} + +test "classify: a benchmark is demand-fetched, not untracked" { + // AGG. Genuinely absent from the routine fetch set, so `not_tracked` is + // literally true - and misleading, because it sends the operator hunting for + // a config fix when `zfin projections` refreshes it on demand. Outranks both + // the tracked check and the TTL for that reason. + const v = classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .peer = d(2026, 8, 11), + .server = d(2026, 8, 11), + .provider = d(2026, 8, 11), + .tracked = false, + .benchmark = true, + }); + try testing.expectEqual(Verdict.demand_fetched, v); + // No symbol: `zfin projections AGG` is not a command that works. + try testing.expectEqualStrings("projections", v.action().?.subcommand); + try testing.expect(!v.action().?.takes_symbol); + + // Even with a fresh TTL, the demand-fetched fact is the useful one. + try testing.expectEqual(Verdict.demand_fetched, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = true, + .provider = d(2026, 8, 11), + .benchmark = true, + })); +} + +test "classify: a non-benchmark untracked symbol still reports not_tracked" { + // The flag must not swallow the genuine case. + try testing.expectEqual(Verdict.not_tracked, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .provider = d(2026, 8, 11), + .tracked = false, + .benchmark = false, + })); +} + +test "classify: a current benchmark is still just current" { + // Nothing newer anywhere, so the demand-fetched fact is not a finding. + try testing.expectEqual(Verdict.current, classify(.{ + .local = d(2026, 8, 11), + .peer = d(2026, 8, 11), + .provider = d(2026, 8, 11), + .benchmark = true, + })); +} diff --git a/src/commands/doctor.zig b/src/commands/doctor.zig index afc0078..c992198 100644 --- a/src/commands/doctor.zig +++ b/src/commands/doctor.zig @@ -178,6 +178,10 @@ fn checkCandleFreshness( arena: std.mem.Allocator, store: *cache.Store, tracked: *const std.StringHashMap(void), + /// Demand-fetched symbols to leave out - see `freshness.collect`. Warning + /// about a benchmark symbol would be noise: `projections` refreshes it when + /// it runs, and nothing else wants it. + exclude: []const []const u8, now_s: i64, ) !Check { const label = "Candle freshness"; @@ -185,7 +189,7 @@ fn checkCandleFreshness( return .{ .status = .info, .label = label, .detail = "cache not readable" }; if (keys.len == 0) return .{ .status = .info, .label = label, .detail = "nothing cached" }; - const entries = try freshness.collect(arena, store, keys, tracked); + const entries = try freshness.collect(arena, store, keys, tracked, exclude); var report = try freshness.scan(arena, entries, now_s); defer report.deinit(arena); @@ -667,14 +671,16 @@ pub fn run(ctx: *framework.RunCtx, _: ParsedArgs) !void { // would read as an orphan and nothing as stale - an empty set is // the honest input. var tracked = std.StringHashMap(void).init(arena); + var bench: []const []const u8 = &.{}; if (anchor) |a| { - tracked = cli.trackedSymbols(ctx, arena, .{ .lots = all_lots.items, .allocator = arena }, a) catch + tracked = cli.trackedSymbols(ctx, arena, .{ .lots = all_lots.items, .allocator = arena }) catch std.StringHashMap(void).init(arena); + bench = cli.demandFetchedSymbols(io, arena, a); } // wall-clock required: peer freshness is judged against the market // calendar, which needs the real instant rather than `ctx.today`. const now_s = std.Io.Timestamp.now(io, .real).toSeconds(); - try checks.append(arena, try checkCandleFreshness(arena, &store, &tracked, now_s)); + try checks.append(arena, try checkCandleFreshness(arena, &store, &tracked, bench, now_s)); } // Hand-maintained data staleness. @@ -1538,7 +1544,7 @@ test "checkCandleFreshness: warns and names the laggards, never fails" { var store = cache.Store.init(io, arena, dir_path); // Friday 2025-06-13, 22:00 UTC - past the equity boundary. const now_s = zfin.Date.fromYmd(2025, 6, 13).toEpoch() + 22 * std.time.s_per_hour; - const c = try checkCandleFreshness(arena, &store, &tracked, now_s); + const c = try checkCandleFreshness(arena, &store, &tracked, &.{}, now_s); // WARN, not FAIL: `doctor` exits non-zero on any fail, and a stale cache is // a thing to fix rather than a broken install. Cron stays green. @@ -1567,7 +1573,7 @@ test "checkCandleFreshness: a caught-up corpus is OK" { var store = cache.Store.init(io, arena, dir_path); const now_s = zfin.Date.fromYmd(2025, 6, 13).toEpoch() + 22 * std.time.s_per_hour; - const c = try checkCandleFreshness(arena, &store, &tracked, now_s); + const c = try checkCandleFreshness(arena, &store, &tracked, &.{}, now_s); try testing.expectEqual(Status.ok, c.status); } @@ -1580,7 +1586,7 @@ test "checkCandleFreshness: an empty cache is informational, not a warning" { // First run on a fresh machine must not look like a problem. var tracked = std.StringHashMap(void).init(arena); var store = cache.Store.init(io, arena, "/nonexistent/zfin-cache"); - const c = try checkCandleFreshness(arena, &store, &tracked, 0); + const c = try checkCandleFreshness(arena, &store, &tracked, &.{}, 0); try testing.expectEqual(Status.info, c.status); } @@ -1608,6 +1614,6 @@ test "checkCandleFreshness: an untracked laggard does not warn" { var store = cache.Store.init(io, arena, dir_path); const now_s = zfin.Date.fromYmd(2025, 6, 13).toEpoch() + 22 * std.time.s_per_hour; - const c = try checkCandleFreshness(arena, &store, &tracked, now_s); + const c = try checkCandleFreshness(arena, &store, &tracked, &.{}, now_s); try testing.expectEqual(Status.ok, c.status); } diff --git a/src/main.zig b/src/main.zig index f68506e..22ab3db 100644 --- a/src/main.zig +++ b/src/main.zig @@ -46,6 +46,7 @@ const command_modules = .{ // Infrastructure .cache = @import("commands/cache.zig"), + .diagnose = @import("commands/diagnose.zig"), .doctor = @import("commands/doctor.zig"), .version = @import("commands/version.zig"), }; @@ -532,6 +533,10 @@ fn runCli(init: std.process.Init) !u8 { var svc = zfin.DataService.init(io, allocator, config); defer svc.deinit(); + // The global `--refresh-data` flag is service-wide, so it lives on the + // service rather than being threaded to every fetch call site. Without this + // it silently stopped at any call site passing `.{}`. + svc.default_options = cli.fetchOptionsFromPolicy(globals.refresh_policy); // ── Framework dispatch ─────────────────────────────────────── // diff --git a/src/service.zig b/src/service.zig index f324d2c..c07a0db 100644 --- a/src/service.zig +++ b/src/service.zig @@ -345,6 +345,25 @@ pub const DataService = struct { /// set in production. panic_on_network_attempt: bool = false, + /// Service-wide fetch policy, from the global `--refresh-data` flag. + /// + /// A call site passing `.{}` is not opting OUT of the user's instruction - + /// it simply has no opinion, so this applies. Without it, a documented + /// GLOBAL flag silently stopped at any call site that did not thread the + /// option through: `zfin --refresh-data=force projections` did not + /// force-refresh the benchmark symbols, because `views/projections.zig` + /// fetched them with a hardcoded `.{}`. + /// + /// Threading the option to those call sites instead would have meant ~7 + /// signatures and ~19 edits across projections, compare and the TUI, and + /// would have left the next hardcoded `.{}` with the same bug. + /// + /// **Defaults to all-false, which is what keeps tests offline.** Test + /// Configs are keyless literals so a provider fetch already fails with + /// `NoApiKey` before any HTTP, and this field cannot loosen that: an + /// all-false policy ORs to a no-op. A test has to set it deliberately. + default_options: FetchOptions = .{}, + pub fn init(io: std.Io, allocator: std.mem.Allocator, config: Config) DataService { const self = DataService{ .allocator = allocator, @@ -470,8 +489,10 @@ pub const DataService = struct { comptime T: type, symbol: []const u8, comptime postProcess: ?*const fn (*T, std.mem.Allocator) anyerror!void, - opts: FetchOptions, + opts_in: FetchOptions, ) DataError!FetchResult(T) { + // See `getCandles` - one fold, covering every type routed through here. + const opts = self.effectiveOptions(opts_in); var s = self.store(); const data_type = comptime cache.Store.dataTypeFor(T); @@ -762,6 +783,19 @@ pub const DataService = struct { /// paths never reach this site. Production callers always pass. /// Inline so the panic body is only generated when the field is /// actually checked (no overhead on the false branch). + /// `opts` combined with the service-wide policy. + /// + /// A union, not an override: a caller asking for `force_refresh` gets it + /// even under `.auto`, and the global flag applies to callers with no + /// opinion. `skip_network` continues to win over `force_refresh` wherever + /// both end up set, which is the existing documented precedence. + fn effectiveOptions(self: *DataService, opts: FetchOptions) FetchOptions { + return .{ + .force_refresh = opts.force_refresh or self.default_options.force_refresh, + .skip_network = opts.skip_network or self.default_options.skip_network, + }; + } + inline fn assertNetworkAllowed(self: *DataService, context: []const u8) void { if (self.panic_on_network_attempt) { std.debug.panic("network attempted in offline-mode test: {s}", .{context}); @@ -835,7 +869,11 @@ pub const DataService = struct { /// `opts.skip_network = true` -> returns cached data even if stale, /// returns FetchFailed on cache miss without touching the network. /// `opts.force_refresh = true` -> treats cache as stale and fetches. - pub fn getCandles(self: *DataService, symbol: []const u8, opts: FetchOptions) DataError!FetchResult(Candle) { + pub fn getCandles(self: *DataService, symbol: []const u8, opts_in: FetchOptions) DataError!FetchResult(Candle) { + // Fold in the service-wide policy once, here, so every decision below + // sees the user's `--refresh-data` instruction even when the caller + // passed `.{}`. See `default_options`. + const opts = self.effectiveOptions(opts_in); var s = self.store(); // Negative cache: this symbol is known to have no candle data on @@ -2204,10 +2242,13 @@ pub const DataService = struct { self: *DataService, portfolio_syms: ?[]const []const u8, watch_syms: []const []const u8, - opts: FetchOptions, + opts_in: FetchOptions, aggregate_progress: ?AggregateProgressCallback, symbol_progress: ?ProgressCallback, ) LoadAllResult { + // See `getCandles`. Folded here as well as there because this reads + // `opts` directly for its cache-scan fast path, not only via getCandles. + const opts = self.effectiveOptions(opts_in); var result = LoadAllResult{ .prices = std.StringHashMap(f64).init(self.allocator), .cached_count = 0, @@ -5138,3 +5179,80 @@ test "expiryAfterFetch: an empty slice falls back to the next boundary" { DataService.expiryAfterFetch(mon_1700, kind, &.{}), ); } + +test "effectiveOptions: the default policy is a no-op, which is what keeps tests offline" { + const allocator = std.testing.allocator; + const io = std.testing.io; + var svc = DataService.init(io, allocator, .{ .cache_dir = "unused" }); + defer svc.deinit(); + + // Nothing set: identical in, identical out. Test Configs are keyless so a + // provider fetch already fails with NoApiKey before any HTTP, and this field + // must not be able to loosen that by accident. + try std.testing.expectEqual(FetchOptions{}, svc.effectiveOptions(.{})); + try std.testing.expectEqual( + FetchOptions{ .force_refresh = true }, + svc.effectiveOptions(.{ .force_refresh = true }), + ); + try std.testing.expectEqual( + FetchOptions{ .skip_network = true }, + svc.effectiveOptions(.{ .skip_network = true }), + ); +} + +test "effectiveOptions: the service policy reaches a caller with no opinion" { + const allocator = std.testing.allocator; + const io = std.testing.io; + var svc = DataService.init(io, allocator, .{ .cache_dir = "unused" }); + defer svc.deinit(); + + // THE BUG THIS FIXES. `views/projections.zig` fetches the benchmark pair + // with a hardcoded `.{}`, so `--refresh-data=force` never reached SPY/AGG. + svc.default_options = .{ .force_refresh = true }; + try std.testing.expect(svc.effectiveOptions(.{}).force_refresh); + + // `--refresh-data=never` likewise, so an offline session stays offline even + // through a call site that never threaded the option. + svc.default_options = .{ .skip_network = true }; + try std.testing.expect(svc.effectiveOptions(.{}).skip_network); + try std.testing.expect(!svc.effectiveOptions(.{}).force_refresh); +} + +test "effectiveOptions: a union, so an explicit request survives .auto" { + const allocator = std.testing.allocator; + const io = std.testing.io; + var svc = DataService.init(io, allocator, .{ .cache_dir = "unused" }); + defer svc.deinit(); + + // Under `.auto` a caller that explicitly asks for force still gets it - + // `zfin cache refresh` depends on exactly this. + svc.default_options = .{}; + try std.testing.expect(svc.effectiveOptions(.{ .force_refresh = true }).force_refresh); + + // Both set: skip_network continues to win downstream, which is the existing + // documented precedence - this only unions the flags, it does not reorder them. + svc.default_options = .{ .skip_network = true }; + const both = svc.effectiveOptions(.{ .force_refresh = true }); + try std.testing.expect(both.force_refresh and both.skip_network); +} + +test "effectiveOptions: --refresh-data=never survives a hardcoded call site" { + // The offline guarantee, end to end: with the service policy set to never, + // a fetch through a call site that passed `.{}` must not attempt network. + // `panic_on_network_attempt` turns any attempt into a hard failure. + 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, .{ .cache_dir = dir_path }); + defer svc.deinit(); + svc.default_options = .{ .skip_network = true }; + svc.panic_on_network_attempt = true; + + // Nothing cached and no network permitted -> a clean error, not a panic and + // not a request. + try std.testing.expectError(DataError.FetchFailed, svc.getCandles("NOSUCH", .{})); +} diff --git a/src/tui.zig b/src/tui.zig index 8599ae7..460841c 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -2589,6 +2589,9 @@ pub fn run( defer allocator.destroy(svc); svc.* = zfin.DataService.init(io, allocator, config); defer svc.deinit(); + // Same reason as the CLI path: the flag is service-wide, so it belongs on + // the service rather than at each fetch call site. + svc.default_options = cli.fetchOptionsFromPolicy(refresh_policy); var app_inst = try allocator.create(App); defer allocator.destroy(app_inst);