From 2a89125977bbcd51f0930de01aeb65ff378c18e7 Mon Sep 17 00:00:00 2001
From: Emil Lerch <emil@lerch.org>
Date: Mon, 18 May 2026 16:18:26 -0700
Subject: [PATCH] be more clear what negative cache means

---
 src/service.zig | 77 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 3 deletions(-)

diff --git a/src/service.zig b/src/service.zig
index 9a0bbb8..e1da597 100644
--- a/src/service.zig
+++ b/src/service.zig
@@ -62,6 +62,25 @@ pub const DataError = error{
     AuthError,
 };
 
+/// Decide whether a provider failure is permanent enough to merit a
+/// negative-cache entry. Negative entries suppress retries until the
+/// next manual `--refresh` / `cache clear`, so writing one is only
+/// safe when we're confident more attempts won't succeed.
+///
+/// Today the only certain-permanent failure is `NotFound`: the symbol
+/// just doesn't have data of this type at this provider. Everything
+/// else (rate limit, network blip, server 5xx, auth, parse error) is
+/// either transient or fixable; recording a negative entry would
+/// silently suppress retries for hours/days.
+///
+/// Rate-limit (`error.RateLimited`) is excluded here because callers
+/// handle it specially (single retry after backoff). Anything that
+/// reaches this classifier and isn't `NotFound` returns false →
+/// caller returns `FetchFailed` without poisoning the cache.
+pub fn isPermanentProviderFailure(err: anyerror) bool {
+    return err == error.NotFound;
+}
+
 /// Re-exported provider types needed by commands via DataService.
 pub const CompanyOverview = alphavantage.CompanyOverview;
 
@@ -314,7 +333,14 @@ pub const DataService = struct {
                 s.write(T, symbol, retried, data_type.ttl());
                 return .{ .data = retried, .source = .fetched, .timestamp = std.Io.Timestamp.now(self.io, .real).toSeconds(), .allocator = self.allocator };
             }
-            s.writeNegative(symbol, data_type);
+            // 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
+            // error) propagate as FetchFailed without poisoning the
+            // cache, so the next call retries naturally.
+            if (isPermanentProviderFailure(err)) {
+                s.writeNegative(symbol, data_type);
+            }
             return DataError.FetchFailed;
         };
 
@@ -561,6 +587,11 @@ pub const DataService = struct {
                 }
                 return DataError.TransientError;
             }
+            // FetchFailed at this point means BOTH Tiingo and Yahoo
+            // returned NotFound (or Yahoo was unavailable on top of
+            // Tiingo NotFound) — symbol genuinely has no candle data
+            // anywhere we look. Negative-cache the result so we don't
+            // keep retrying nonexistent symbols.
             s.writeNegative(symbol, .candles_daily);
             return DataError.FetchFailed;
         };
@@ -634,7 +665,9 @@ pub const DataService = struct {
                     return DataError.FetchFailed;
                 };
             }
-            s.writeNegative(symbol, .earnings);
+            if (isPermanentProviderFailure(err)) {
+                s.writeNegative(symbol, .earnings);
+            }
             return DataError.FetchFailed;
         };
 
@@ -659,7 +692,9 @@ pub const DataService = struct {
                     return DataError.FetchFailed;
                 };
             }
-            s.writeNegative(symbol, .etf_profile);
+            if (isPermanentProviderFailure(err)) {
+                s.writeNegative(symbol, .etf_profile);
+            }
             return DataError.FetchFailed;
         };
 
@@ -1608,6 +1643,42 @@ pub const DataService = struct {
 
 // ── Tests ─────────────────────────────────────────────────────────
 
+test "isPermanentProviderFailure: NotFound is permanent" {
+    try std.testing.expect(isPermanentProviderFailure(error.NotFound));
+}
+
+test "isPermanentProviderFailure: RequestFailed is transient" {
+    try std.testing.expect(!isPermanentProviderFailure(error.RequestFailed));
+}
+
+test "isPermanentProviderFailure: ServerError is transient" {
+    try std.testing.expect(!isPermanentProviderFailure(error.ServerError));
+}
+
+test "isPermanentProviderFailure: Unauthorized is transient" {
+    // Auth misconfigs are user-fixable (set the API key); not a reason
+    // to permanently suppress retries.
+    try std.testing.expect(!isPermanentProviderFailure(error.Unauthorized));
+}
+
+test "isPermanentProviderFailure: InvalidResponse is transient" {
+    // Parse errors are usually a provider format change or one-off
+    // garbage response — retrying later is fine.
+    try std.testing.expect(!isPermanentProviderFailure(error.InvalidResponse));
+}
+
+test "isPermanentProviderFailure: PaymentRequired is transient" {
+    // FMP marks plan-locked symbols with HTTP 402; user can upgrade
+    // their plan or rotate providers, so don't poison the cache.
+    try std.testing.expect(!isPermanentProviderFailure(error.PaymentRequired));
+}
+
+test "isPermanentProviderFailure: RateLimited is transient" {
+    // Rate-limit is the textbook transient case; the caller already
+    // handles it specially with backoff + retry.
+    try std.testing.expect(!isPermanentProviderFailure(error.RateLimited));
+}
+
 test "isMutualFund identifies mutual funds" {
     // Standard mutual fund tickers (5 letters ending in X)
     try std.testing.expect(DataService.isMutualFund("FDSCX"));