fix(circuit-breaker): evict invalid cached data when shouldCache fails — unblocks BIS/BLS tabs (#2274)

* fix(circuit-breaker): evict invalid cached data when shouldCache predicate fails

Circuit breakers with persistCache=true would serve stale empty data (e.g.
[] or { rates: [] }) indefinitely within the 15-30min TTL window. This
caused the Central Banks and Labor tabs in the Macro Stress panel to never
appear even after the underlying seeders started returning real data.

- circuit-breaker.ts: when shouldCache option is provided, evict cached
  entries that fail the predicate before checking fresh/SWR paths. This
  forces a live fetch instead of serving known-invalid cached data.
- economic/index.ts: add shouldCache guards to blsBreaker (r.length > 0)
  and all three BIS breakers (rates/entries length > 0) so empty responses
  are never written to persistent cache and existing empty entries are
  evicted on next call.

* fix(circuit-breaker): address Greptile P2 comments on PR #2274

- Use local shouldCache variable instead of options.shouldCache directly
  (the default () => true means the condition is always false without an
  explicit predicate — redundant guard removed, local var is cleaner)
- Document the fire-and-forget race window in comment
- Add 3 tests for the new shouldCache eviction path:
  evicts invalid cached data and fetches fresh, skips eviction for valid
  data, and preserves existing behavior when no predicate is provided

src/services/economic/index.ts

@@ -253,7 +253,7 @@ export async function fetchBlsData(): Promise<FredSeries[]> {
       });
     }
     return out;
-  }, [] as FredSeries[]);
+  }, [] as FredSeries[], { shouldCache: (r) => r.length > 0 });
 }
 
 export function getChangeClass(change: number | null): string {
@@ -712,9 +712,9 @@ export async function fetchBisData(): Promise<BisData> {
 
   try {
     const [policy, eer, credit] = await Promise.all([
-      hPolicy?.rates?.length ? Promise.resolve(hPolicy) : bisPolicyBreaker.execute(() => client.getBisPolicyRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisPolicyFallback),
-      hEer?.rates?.length ? Promise.resolve(hEer) : bisEerBreaker.execute(() => client.getBisExchangeRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisEerFallback),
-      hCredit?.entries?.length ? Promise.resolve(hCredit) : bisCreditBreaker.execute(() => client.getBisCredit({}, { signal: AbortSignal.timeout(20_000) }), emptyBisCreditFallback),
+      hPolicy?.rates?.length ? Promise.resolve(hPolicy) : bisPolicyBreaker.execute(() => client.getBisPolicyRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisPolicyFallback, { shouldCache: (r) => (r.rates?.length ?? 0) > 0 }),
+      hEer?.rates?.length ? Promise.resolve(hEer) : bisEerBreaker.execute(() => client.getBisExchangeRates({}, { signal: AbortSignal.timeout(20_000) }), emptyBisEerFallback, { shouldCache: (r) => (r.rates?.length ?? 0) > 0 }),
+      hCredit?.entries?.length ? Promise.resolve(hCredit) : bisCreditBreaker.execute(() => client.getBisCredit({}, { signal: AbortSignal.timeout(20_000) }), emptyBisCreditFallback, { shouldCache: (r) => (r.entries?.length ?? 0) > 0 }),
     ]);
     return {
       policyRates: policy.rates ?? [],

src/utils/circuit-breaker.ts

@@ -330,7 +330,20 @@ export class CircuitBreaker<T> {
       await this.hydratePersistentCache(cacheKey);
     }
 
-    const cachedEntry = this.getCacheEntry(cacheKey);
+    let cachedEntry = this.getCacheEntry(cacheKey);
+
+    // If the cached data fails the shouldCache predicate, evict it and fetch
+    // fresh rather than serving known-invalid data for the full TTL.
+    // The default shouldCache (() => true) never returns false, so this only
+    // fires when an explicit predicate is passed.
+    // deletePersistentCache is fire-and-forget; on the rare case that
+    // hydratePersistentCache runs again before the delete commits, the entry
+    // is evicted once more — safe and self-resolving.
+    if (cachedEntry !== null && !shouldCache(cachedEntry.data as R)) {
+      this.evictCacheKey(cacheKey);
+      if (this.persistEnabled) this.deletePersistentCache(cacheKey);
+      cachedEntry = null;
+    }
 
     if (this.isStateOnCooldown()) {
       console.log(`[${this.name}] Currently unavailable, ${this.getCooldownRemaining()}s remaining`);

tests/circuit-breaker-persistent-stale-ceiling.test.mts

@@ -690,3 +690,97 @@ describe('CircuitBreaker — same name, different ceiling (registry behavior)',
   });
 });
 
+// ============================================================
+// shouldCache eviction — cached invalid data is evicted and refetched
+// ============================================================
+
+describe('CircuitBreaker — shouldCache eviction of invalid cached data', () => {
+  const CIRCUIT_BREAKER_URL = pathToFileURL(
+    resolve(root, 'src/utils/circuit-breaker.ts'),
+  ).href;
+
+  it('evicts cached data that fails shouldCache and fetches fresh', async () => {
+    const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-fresh`);
+    const { createCircuitBreaker, clearAllCircuitBreakers } = mod;
+    clearAllCircuitBreakers();
+
+    try {
+      const breaker = createCircuitBreaker<number[]>({
+        name: 'ShouldCache Evict Test',
+        cacheTtlMs: 60 * 1000,
+      });
+
+      // Populate cache with empty array (simulates a prior failed fetch)
+      breaker.recordSuccess([]);
+      assert.deepEqual(breaker.getCached(), [], 'empty array is cached');
+
+      let fetchCount = 0;
+      const result = await breaker.execute(
+        async () => { fetchCount++; return [1, 2, 3]; },
+        [] as number[],
+        { shouldCache: (r) => r.length > 0 },
+      );
+
+      assert.equal(fetchCount, 1, 'must bypass empty cache and fetch fresh');
+      assert.deepEqual(result, [1, 2, 3], 'fresh data is returned');
+      assert.deepEqual(breaker.getCached(), [1, 2, 3], 'fresh data is now cached');
+    } finally {
+      clearAllCircuitBreakers();
+    }
+  });
+
+  it('serves valid cached data without fetching when shouldCache passes', async () => {
+    const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-skip`);
+    const { createCircuitBreaker, clearAllCircuitBreakers } = mod;
+    clearAllCircuitBreakers();
+
+    try {
+      const breaker = createCircuitBreaker<number[]>({
+        name: 'ShouldCache Skip Test',
+        cacheTtlMs: 60 * 1000,
+      });
+
+      breaker.recordSuccess([1, 2, 3]);
+
+      let fetchCount = 0;
+      const result = await breaker.execute(
+        async () => { fetchCount++; return [4, 5, 6]; },
+        [] as number[],
+        { shouldCache: (r) => r.length > 0 },
+      );
+
+      assert.equal(fetchCount, 0, 'valid cache must not trigger a live fetch');
+      assert.deepEqual(result, [1, 2, 3], 'cached data is returned');
+    } finally {
+      clearAllCircuitBreakers();
+    }
+  });
+
+  it('without shouldCache, empty array is served from cache without fetching', async () => {
+    const mod = await import(`${CIRCUIT_BREAKER_URL}?t=${Date.now()}-evict-no-pred`);
+    const { createCircuitBreaker, clearAllCircuitBreakers } = mod;
+    clearAllCircuitBreakers();
+
+    try {
+      const breaker = createCircuitBreaker<number[]>({
+        name: 'No ShouldCache Test',
+        cacheTtlMs: 60 * 1000,
+      });
+
+      breaker.recordSuccess([]);
+
+      let fetchCount = 0;
+      const result = await breaker.execute(
+        async () => { fetchCount++; return [1, 2, 3]; },
+        [] as number[],
+        // No shouldCache — default is () => true, so empty array is valid
+      );
+
+      assert.equal(fetchCount, 0, 'without shouldCache, cached empty array is served');
+      assert.deepEqual(result, [], 'empty cached data is returned as-is');
+    } finally {
+      clearAllCircuitBreakers();
+    }
+  });
+});
+