fix(regional-snapshot): stop fabricating snapshot_confidence and valid_until (#3728)#3781
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryFixes two independent defects in
Confidence Score: 4/5Safe to merge; both defect fixes are correct and well-tested, with only minor follow-ups needed. The two core logic changes are sound: undated inputs correctly land in stale[], and valid_until is now derived from the earliest real TTL across fresh inputs with appropriate clamping. The new resolveInputTimestamp export is missing a default parameter that its sister function provides, one inline comment was left describing the old behavior, and a test case claims to verify the 6h cap but does not actually reach that branch. freshness.mjs (missing default parameter on resolveInputTimestamp, stale inline comment); tests/regional-snapshot.test.mjs (cap branch untested). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildPreMeta called] --> B[classifyInputs]
B --> C{Has payload?}
C -- No --> D["missing[]"]
C -- Yes --> E{"Resolve timestamp"}
E -- null --> F["stale[] was fresh[] before fix"]
E -- ts found --> G{ageMin > maxAgeMin?}
G -- Yes --> H["stale[]"]
G -- No --> I["fresh[]"]
I --> J[deriveValidUntil]
J --> K{"fresh[] empty?"}
K -- Yes --> L[valid_until = now]
K -- No --> M["min ts + maxAgeMin across fresh"]
M --> N{earliestExpiry < now?}
N -- Yes --> L
N -- No --> O{earliestExpiry > now+6h?}
O -- Yes --> P["valid_until = now+6h cap"]
O -- No --> Q[valid_until = earliestExpiry]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * @param {Record<string, unknown>} metaPayloads | ||
| * @returns {number | null} | ||
| */ | ||
| export function resolveInputTimestamp(spec, payload, metaPayloads) { |
Contributor
There was a problem hiding this comment.
resolveInputTimestamp export is missing the metaPayloads = {} default that classifyInputs provides. Any caller that passes only two arguments triggers TypeError: Cannot read properties of undefined for specs that have a metaKey. Keeping the signature consistent also prevents a latent crash if this helper is imported and used in a new context without the full three-argument call.
Suggested change
| export function resolveInputTimestamp(spec, payload, metaPayloads) { | |
| export function resolveInputTimestamp(spec, payload, metaPayloads = {}) { |
Comment on lines
+589
to
+601
| it('valid_until is capped at now + 6h even when inputs have long TTLs (#3728)', () => { | ||
| // energy:mix:v1:_all has maxAgeMin=50400 (35d). A fresh fetch today | ||
| // would otherwise advertise validity weeks out — clamp to the 6h cron. | ||
| const allKeys = {}; | ||
| for (const s of FRESHNESS_REGISTRY) allKeys[s.key] = { fetchedAt: Date.now() }; | ||
| const now = Date.now(); | ||
| const { pre } = buildPreMeta(allKeys, '1.0.0', '1.0.0'); | ||
| const cap = now + 6 * 60 * 60 * 1000; | ||
| // valid_until should be near the cap (the tightest input is 15min, | ||
| // relay:oref:history:v1 — that's actually what wins here, not the cap). | ||
| // The cap behavior is exercised below in the "all long-TTL" case. | ||
| assert.ok(pre.valid_until <= cap + 1000, 'valid_until must not exceed now + 6h cap'); | ||
| }); |
Contributor
There was a problem hiding this comment.
The test name says "capped at now + 6h even when inputs have long TTLs", but with all inputs seeded at Date.now(), the winning TTL is relay:oref:history:v1 at 15 minutes — well under the 6h cap. The comment even promises "The cap behavior is exercised below in the 'all long-TTL' case" but that subsequent test never appears, leaving the earliestExpiry > cap branch in deriveValidUntil unexercised.
koala73
added a commit
that referenced
this pull request
May 18, 2026
… on-deploy false STALE (#3781 follow-on) Static analysis identified 5 Redis keys whose payloads carry no timestamp `extractTimestamp` recognises and would therefore classify as STALE on first deploy under #3728's new logic. All five already have a `seed-meta:*` companion written by existing seeders (and tracked by `api/health.js`); we just point the freshness registry at them. - risk:scores:sebuf:stale:v1 → seed-meta:intelligence:risk-scores - intelligence:cross-source-signals:v1 → seed-meta:intelligence:cross-source-signals - energy:mix:v1:_all → seed-meta:economic:owid-energy-mix - supply_chain:transit-summaries:v1 → seed-meta:supply_chain:transit-summaries - relay:oref:history:v1 → seed-meta:relay:oref:history (new write added next to existing envelopeWrite in ais-relay.cjs) Tests lock in the wiring so a later removal trips a unit test, not production.
Merged
7 tasks
koala73
added a commit
that referenced
this pull request
May 18, 2026
…lope ok, tick-consistent valid_until, fix wrong-assertion test, parameterize metaKey resolution Review round-2 fixes for PR #3781: - ais-relay.cjs: gate the seed-meta:relay:oref:history write on envelope write success. Previously the meta write could land alone if the envelope write failed (Upstash 5xx / blip), letting the freshness classifier report FRESH for data that does not exist in Redis. Added a note about the pre-existing asymmetry at the transit-summaries write (~line 7370), which is intentionally out of scope here. - snapshot-meta.mjs: snapshot `Date.now()` once in buildPreMeta and pass it as a 4th arg to deriveValidUntil so the two-step compute is tick-consistent. Removes the need for timing tolerance in tests that assert exact valid_until values. - regional-snapshot.test.mjs: split the misnamed 6h-cap test into (a) "tightest input wins" and (b) "cap binds when all inputs out-TTL the cap". The previous test claimed to exercise the cap but the 15-min input always won. New (b) restricts the sources map to registry entries with maxAgeMin > 6h so the cap is actually binding. - regional-snapshot.test.mjs: parameterize the metaKey-resolution test across all 5 newly-wired keys (risk:scores, cross-source-signals, energy:mix, transit-summaries, relay:oref:history) so removing any single metaKey trips a test.
…ated inputs as stale (#3728) Two independent defects in the regional snapshot meta builder were both fabricating snapshot_confidence and valid_until: 1. classifyInputs() in freshness.mjs defaulted present-but-undated payloads to "fresh" — a stalled seeder that left an old payload without a timestamp would silently inflate snapshot_confidence. Flipped the default to "stale": you cannot prove freshness without a timestamp. 2. buildPreMeta() in snapshot-meta.mjs hardcoded valid_until = now + 6h, ignoring the per-input maxAgeMin in FRESHNESS_REGISTRY. Replaced with min(ts + maxAgeMin*60_000) across fresh inputs, clamped to [now, now + 6h]. When no inputs are fresh, valid_until collapses to now. Updates tests/regional-snapshot.test.mjs (5 new cases) and the pre-existing mobility test that asserted the old undated-as-fresh behavior. Resolves TODO #181 (pending since PR #2940). Closes #3728
… on-deploy false STALE (#3781 follow-on) Static analysis identified 5 Redis keys whose payloads carry no timestamp `extractTimestamp` recognises and would therefore classify as STALE on first deploy under #3728's new logic. All five already have a `seed-meta:*` companion written by existing seeders (and tracked by `api/health.js`); we just point the freshness registry at them. - risk:scores:sebuf:stale:v1 → seed-meta:intelligence:risk-scores - intelligence:cross-source-signals:v1 → seed-meta:intelligence:cross-source-signals - energy:mix:v1:_all → seed-meta:economic:owid-energy-mix - supply_chain:transit-summaries:v1 → seed-meta:supply_chain:transit-summaries - relay:oref:history:v1 → seed-meta:relay:oref:history (new write added next to existing envelopeWrite in ais-relay.cjs) Tests lock in the wiring so a later removal trips a unit test, not production.
…lope ok, tick-consistent valid_until, fix wrong-assertion test, parameterize metaKey resolution Review round-2 fixes for PR #3781: - ais-relay.cjs: gate the seed-meta:relay:oref:history write on envelope write success. Previously the meta write could land alone if the envelope write failed (Upstash 5xx / blip), letting the freshness classifier report FRESH for data that does not exist in Redis. Added a note about the pre-existing asymmetry at the transit-summaries write (~line 7370), which is intentionally out of scope here. - snapshot-meta.mjs: snapshot `Date.now()` once in buildPreMeta and pass it as a 4th arg to deriveValidUntil so the two-step compute is tick-consistent. Removes the need for timing tolerance in tests that assert exact valid_until values. - regional-snapshot.test.mjs: split the misnamed 6h-cap test into (a) "tightest input wins" and (b) "cap binds when all inputs out-TTL the cap". The previous test claimed to exercise the cap but the 15-min input always won. New (b) restricts the sources map to registry entries with maxAgeMin > 6h so the cap is actually binding. - regional-snapshot.test.mjs: parameterize the metaKey-resolution test across all 5 newly-wired keys (risk:scores, cross-source-signals, energy:mix, transit-summaries, relay:oref:history) so removing any single metaKey trips a test.
koala73
force-pushed
the
fix/3728-snapshot-confidence-stale-inputs
branch
from
May 18, 2026 08:13
b6a4d5f to
9d9ac28
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent defects in
scripts/regional-snapshot/were both fabricating snapshot meta, so a stalled upstream seeder still produced a high-confidence snapshot with a 6h validity window.freshness.mjs—classifyInputs()defaulted present-but-undated payloads tofresh[]. A crashed seeder that left an old payload with no timestamp would silently inflatesnapshot_confidence. Flipped tostale[]: you cannot prove freshness without a timestamp.snapshot-meta.mjs—buildPreMeta()hardcodedvalid_until = Date.now() + 6h, ignoring per-inputmaxAgeMininFRESHNESS_REGISTRY. Now derived asmin(ts + maxAgeMin*60_000)acrossfresh[], clamped to[now, now + 6h]. Whenfresh[]is empty,valid_untilcollapses tonow.Closes #3728. Resolves TODO #181 (pending since PR #2940).
Changes
scripts/regional-snapshot/freshness.mjs— flip undated default; export newresolveInputTimestamp()helper forsnapshot-meta.mjs.scripts/regional-snapshot/snapshot-meta.mjs— newderiveValidUntil()consultsFRESHNESS_REGISTRYand clamps to the 6h cron interval.tests/regional-snapshot.test.mjs— 5 new cases underdescribe('snapshot meta').tests/regional-snapshot-mobility.test.mjs— updated the one existing test that asserted the old undated-as-fresh behavior.todos/181-pending-...md→todos/181-complete-...mdwith work log.Pre-merge checklist
FRESHNESS_REGISTRYwithout a top-level timestamp field (fetchedAt/generatedAt/timestamp/updatedAt/lastUpdate/seededAt) or a declaredmetaKey. Spot-check during this PR: seeders for the registry keys all write at least one of those (runSeed()inscripts/_seed-utils.mjsalways emitsseed-meta:*.fetchedAt, and the seeders withoutmetaKeylikeseed-economy,seed-owid-energy-mix,seed-gie-gas-storage,seed-military-flights,seed-cross-source-signalswriteseededAtorfetchedAton the payload). External sources like the sebuf scores (risk:scores:sebuf:stale:v1) are produced outside this repo — if any of them lack a timestamp, they will correctly start showing up instale_inputson first deploy. That is the intended new signal.seed-regional-snapshotsrun after merge for astale_inputsspike. A spike is the bug being detected, not the fix misbehaving.Pre-empt: 5 feeds wired to seed-meta to avoid on-deploy noise
Static analysis (see follow-on commit) identified 5 Redis keys whose
payloads carry no timestamp
extractTimestamprecognises and wouldtherefore classify as STALE on first deploy under the new logic.
All five already have a
seed-meta:*companion written by existingseeders (and tracked by
api/health.js); we just point the freshnessregistry at them.
risk:scores:sebuf:stale:v1→seed-meta:intelligence:risk-scoresintelligence:cross-source-signals:v1→seed-meta:intelligence:cross-source-signalsenergy:mix:v1:_all→seed-meta:economic:owid-energy-mixsupply_chain:transit-summaries:v1→seed-meta:supply_chain:transit-summariesrelay:oref:history:v1→seed-meta:relay:oref:history(new write added next to the existingenvelopeWriteinais-relay.cjs)Test plan
npm run typecheck— cleannpm run linton the four touched files — clean (npx biome lint <files>)npx tsx --test tests/regional-snapshot.test.mjs tests/regional-snapshot-mobility.test.mjs— 125 pass, 0 fail; the 5 new#3728cases and the updated mobility case all passnpm run test:data— 9101 pass, 0 failReview round 2
Reviewer approved with should-fixes — applied in commit
b6a4d5f6f.scripts/ais-relay.cjs(~line 1224) — gated theseed-meta:relay:oref:historywrite onenvelopeWritereturningok. Previously the meta write could land alone if the envelope write failed (Upstash 5xx / network blip), letting the freshness classifier report FRESH for data that does not exist in Redis. Comment notes that the pre-existingtransit-summarieswrite at ~line 7370 has the same shape and is intentionally out of scope (separate PR).scripts/regional-snapshot/snapshot-meta.mjs—buildPreMetanow snapshots a singleDate.now()and passes it as a 4th arg toderiveValidUntil. The two-step (classify → derive) compute is now tick-consistent, so tests can assert exactvalid_untilvalues without timing tolerance.tests/regional-snapshot.test.mjs— split the misnamed 6h-cap test into (a)"valid_until = min(input TTLs) when all inputs are fresh — tightest input wins"and (b)"valid_until is capped at now + 6h when ALL fresh inputs have TTL > 6h". The previous single test claimed to exercise the cap but the 15-minrelay:oref:history:v1input always won, so the cap was never actually exercised. Test (b) restricts the sources map to registry entries withmaxAgeMin > 360minso the cap is the binding constraint.tests/regional-snapshot.test.mjs— parameterized the metaKey-resolution test across all 5 newly-wired keys (risk:scores:sebuf:stale:v1,intelligence:cross-source-signals:v1,energy:mix:v1:_all,supply_chain:transit-summaries:v1,relay:oref:history:v1). Each runs as its ownit()so a failure points at the specific key that regressed.Verification
npm run typecheck— cleannpx tsx --test tests/regional-snapshot.test.mjs tests/regional-snapshot-mobility.test.mjs— 132 pass / 0 fail (was 125; +4 parameterized metaKey cases, +1 split cap case, –1 replaced 6h-cap case = +4 net pass count adjustment)npm run test:data— 9108 pass / 0 failnpx biome lint scripts/ais-relay.cjs scripts/regional-snapshot/snapshot-meta.mjs tests/regional-snapshot.test.mjs— no new diagnostics introduced on changed lines (pre-existing warnings on unrelated lines remain)