diff --git a/docs/dev/caching-implementation.md b/docs/dev/caching-implementation.md index f683662..83c5070 100644 --- a/docs/dev/caching-implementation.md +++ b/docs/dev/caching-implementation.md @@ -463,6 +463,23 @@ touched while `ZFIN_SERVER` is unreachable is poisoned until re-hit the network for every legitimately candle-less symbol on every run. It belongs with the deferred routing work. +### The label is not sticky, and must not become sticky + +`external` describes where a series came from *now*, not a permanent +property of the symbol. If a provider ever does serve it, the bars in +the cache genuinely came from that provider, and `applyTiingoCoverage` +overwriting the label with `.tiingo` / `.yahoo` is the correct outcome: +the provenance changed, and the symbol has rejoined the ordinary +provider path under its own steam. That is a good day, not a bug. + +So do **not** add a guard preserving `.external` across a successful +fetch, and any future routing built on the variant must keep that escape +hatch open. Making it a one-way door recreates the latch bug that keying +routing off `provider == .yahoo` already produced - see [the +`tiingo_retry_after_s` rationale](#getcandles-single-symbol) - except +worse, because the pinned symbol would be pinned to a tier that only one +machine can populate. + ### Adding a `CandleProvider` variant is a coordinated deployment Not a backward-compatible change. `provider` has no default, so it is diff --git a/src/cache/store.zig b/src/cache/store.zig index e5626c7..879102e 100644 --- a/src/cache/store.zig +++ b/src/cache/store.zig @@ -1550,11 +1550,27 @@ pub const Store = struct { /// motivating case is a unitized trust with no ticker and no /// CUSIP, whose daily unit values come from a plan /// recordkeeper's feed; Tiingo, Yahoo and TwelveData all 404 - /// it. So zfin can never fetch or restate such a series - /// itself. It arrives either by `ZFIN_SERVER` sync (the client + /// it. So zfin cannot fetch or restate such a series itself, + /// and it arrives either by `ZFIN_SERVER` sync (the client /// case) or by direct population of the cache directory (the /// server case). /// + /// **This is a statement about the present, not a permanent + /// property of the symbol - and the label is deliberately not + /// sticky.** If a provider ever does serve the symbol, the + /// bars in the cache genuinely came from that provider, so + /// `applyTiingoCoverage` overwriting this label with `.tiingo` + /// or `.yahoo` is *correct*: the provenance changed, and the + /// symbol has rejoined the ordinary provider path. Do not add + /// a guard that preserves `.external` across a successful + /// fetch. Doing so would make it a one-way door, which is the + /// exact shape of the latch bug that keying routing off + /// `provider == .yahoo` already produced (see + /// `tiingo_retry_after_s`) - a symbol that gained coverage + /// would be pinned to the external regime forever. Any future + /// routing built on this variant must preserve that escape + /// hatch. + /// /// **A negative-cache entry on such a symbol is unrecoverable**, /// not merely inconvenient. `writeNegative` overwrites /// `candles_daily.srf` with a marker, negative entries never diff --git a/src/service.zig b/src/service.zig index 53a2a6d..8e82d98 100644 --- a/src/service.zig +++ b/src/service.zig @@ -4379,9 +4379,10 @@ test "getCandles offline never escalates a stale adjustment basis" { // The label is pure provenance and must stay behavior-free, with one // exception: it vetoes the negative-cache write, because for a series // no provider carries, that marker is unrecoverable rather than merely -// sticky. These tests pin both halves - the label changes nothing about -// how a cache is served, and it does change whether a unanimous 404 -// gets remembered. +// sticky. These tests pin all three halves - the label changes nothing +// about how a cache is served, it does change whether a unanimous 404 +// gets remembered, and it is itself overwritten the moment a provider +// starts serving the symbol. test "shouldNegativeCache refuses an external-provider symbol" { // Looped over every variant so a fifth one has to come here and @@ -4403,6 +4404,41 @@ test "shouldNegativeCache refuses an external-provider symbol" { try std.testing.expect(DataService.shouldNegativeCache(null)); } +test "a successful provider fetch relabels an external symbol (the label is not sticky)" { + // `external` records where the bars came from NOW, not a permanent + // property of the symbol. A provider serving it means the cached + // bars genuinely came from that provider, so relabelling is the + // honest outcome - and it is the good outcome, because the symbol + // has rejoined the ordinary provider path under its own steam. + // + // This test exists to stop a well-meaning future guard that + // preserves `.external` across a successful fetch. That would make + // the label a one-way door: a symbol that gained coverage would be + // pinned to a tier only one machine can populate, which is the + // latch bug that routing off `provider == .yahoo` already caused. + const was_external = cache.Store.CandleMeta{ + .last_close = 25, + .last_date = Date.fromYmd(2026, 8, 14), + .provider = .external, + }; + + // Tiingo started carrying it. + const via_tiingo = DataService.applyTiingoCoverage(was_external, "XTRN", 1_787_000_000, .tiingo, .covered); + try std.testing.expectEqual(cache.Store.CandleProvider.tiingo, via_tiingo.provider); + + // Yahoo did, with Tiingo still disclaiming it. Provenance follows + // whoever actually answered, same as for any other symbol. + const via_yahoo = DataService.applyTiingoCoverage(was_external, "XTRN", 1_787_000_000, .yahoo, .not_found); + try std.testing.expectEqual(cache.Store.CandleProvider.yahoo, via_yahoo.provider); + + // And once relabelled, the negative-cache veto no longer applies - + // the symbol is an ordinary provider-sourced series again, so a + // later unanimous 404 is a real verdict worth remembering. + try std.testing.expect(DataService.shouldNegativeCache(via_tiingo)); + try std.testing.expect(DataService.shouldNegativeCache(via_yahoo)); + try std.testing.expect(!DataService.shouldNegativeCache(was_external)); +} + test "getCandles serves an external-provider cache without touching the network" { // `provider::external` must be inert on the serve path. In // particular it must not fall into the `.twelvedata` carve-out,