comments and additional tests

This commit is contained in:
Emil Lerch 2026-08-28 11:41:53 -07:00
parent 3fc1be8073
commit 0964da08d5
Signed by: lobo
GPG key ID: A7B62D657EF764F8
3 changed files with 74 additions and 5 deletions

View file

@ -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

20
src/cache/store.zig vendored
View file

@ -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

View file

@ -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,