From 4933928cf3fef67b28cd74ac69531035d61afe6c Mon Sep 17 00:00:00 2001 From: gadgethd <111318106+gadgethd@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:53:09 +0000 Subject: [PATCH] fix: serve charts during generation refresh --- backend/src/stats/chartSnapshot.test.ts | 3 ++- backend/src/stats/chartSnapshot.ts | 5 ++--- backend/src/stats/statsService.test.ts | 19 +++++++++++++------ backend/src/stats/statsService.ts | 19 +++++++++++++------ 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/backend/src/stats/chartSnapshot.test.ts b/backend/src/stats/chartSnapshot.test.ts index 2809f45..326d3b2 100644 --- a/backend/src/stats/chartSnapshot.test.ts +++ b/backend/src/stats/chartSnapshot.test.ts @@ -72,7 +72,8 @@ test('chart snapshot store pins the schema version and refuses older overwrites' const loaded = await loadStoredChartSnapshot(query, 'ukmesh', 7); assert.equal(loaded?.schema_version, CHART_SNAPSHOT_SCHEMA_VERSION); - assert.deepEqual(calls[0]?.params, ['ukmesh', CHART_SNAPSHOT_SCHEMA_VERSION, 7]); + assert.deepEqual(calls[0]?.params, ['ukmesh', CHART_SNAPSHOT_SCHEMA_VERSION]); + assert.doesNotMatch(calls[0]?.text ?? '', /visibility_generation = \$3/); assert.equal( await saveStoredChartSnapshot(query, 'ukmesh', payload(), 7, 6 * 60 * 60_000, NOW), diff --git a/backend/src/stats/chartSnapshot.ts b/backend/src/stats/chartSnapshot.ts index 79d4279..dfe069f 100644 --- a/backend/src/stats/chartSnapshot.ts +++ b/backend/src/stats/chartSnapshot.ts @@ -91,12 +91,11 @@ export async function loadStoredChartSnapshot( } const result = await query( `SELECT scope_key, schema_version, visibility_generation, generated_at, payload - FROM stats_chart_snapshots + FROM stats_chart_snapshots WHERE scope_key = $1 AND schema_version = $2 - AND visibility_generation = $3 LIMIT 1`, - [scope, CHART_SNAPSHOT_SCHEMA_VERSION, visibilityGeneration], + [scope, CHART_SNAPSHOT_SCHEMA_VERSION], ); return result.rows[0] ?? null; } diff --git a/backend/src/stats/statsService.test.ts b/backend/src/stats/statsService.test.ts index fed1f54..d59bd9c 100644 --- a/backend/src/stats/statsService.test.ts +++ b/backend/src/stats/statsService.test.ts @@ -139,7 +139,7 @@ test('a valid durable chart snapshot serves a cold process without analytical qu assert.equal(saves, 0); }); -test('chart snapshots from an older privacy generation are never served or republished', async () => { +test('a complete older-generation snapshot is served while one scope refresh runs', async () => { const generatedAt = new Date(Date.now() - 60_000).toISOString(); const oldPayload = { snapshot: { @@ -150,6 +150,10 @@ test('chart snapshots from an older privacy generation are never served or repub }, marker: 'old-generation', }; + let resolveRefresh!: (value: ReturnType) => void; + const refresh = new Promise>((resolve) => { + resolveRefresh = resolve; + }); let chartCalls = 0; const repository = { loadChartSnapshot: async () => ({ @@ -162,7 +166,7 @@ test('chart snapshots from an older privacy generation are never served or repub saveChartSnapshot: async () => true, fetchChartsData: async () => { chartCalls += 1; - return emptyChartsData(); + return refresh; }, fetchChannelTraffic: async () => ({ rows: [] }), } as unknown as StatsRepository; @@ -178,13 +182,16 @@ test('chart snapshots from an older privacy generation are never served or repub maskDecodedPathNodes: () => [], }); - const result = await service.getCharts('ukmesh', undefined) as { + const first = await service.getCharts('ukmesh', undefined) as { snapshot: { visibilityGeneration: number }; marker?: string; }; + const second = await service.getCharts('ukmesh', undefined) as typeof first; assert.equal(chartCalls, 1); - assert.equal(result.snapshot.visibilityGeneration, 2); - assert.equal(result.marker, undefined); + assert.equal(first.snapshot.visibilityGeneration, 1); + assert.equal(first.marker, 'old-generation'); + assert.deepEqual(second, first); + resolveRefresh(emptyChartsData()); }); test('a visibility change at chart publication fails closed without caching the result', async () => { @@ -267,7 +274,7 @@ test('expired persisted canonical charts are served while one refresh runs in th assert.equal(chartCalls, 1); resolveRefresh(emptyChartsData()); - await chartsInflight.get('ukmesh:v1'); + await chartsInflight.get('ukmesh'); assert.notEqual(chartsCache.get('ukmesh:v1')?.data, stale); chartsCache.shutdown(); }); diff --git a/backend/src/stats/statsService.ts b/backend/src/stats/statsService.ts index 3653844..95ce588 100644 --- a/backend/src/stats/statsService.ts +++ b/backend/src/stats/statsService.ts @@ -463,7 +463,6 @@ export function createStatsService(deps: StatsServiceDeps) { if ( !row || row.scope_key !== scope - || Number(row.visibility_generation) !== visibilityGeneration ) { return undefined; } @@ -472,7 +471,7 @@ export function createStatsService(deps: StatsServiceDeps) { scope, chartsSnapshotStaleTtlMs, Date.now(), - visibilityGeneration, + undefined, { allowExpired: true }, ); const storedGeneratedAtMs = new Date(row.generated_at).getTime(); @@ -508,7 +507,7 @@ export function createStatsService(deps: StatsServiceDeps) { key: string, visibilityGeneration: number, ): Promise { - const existing = chartsInflight.get(key); + const existing = chartsInflight.get(scope); if (existing) return existing; if (chartsInflight.size >= MAX_UNIQUE_STATS_INFLIGHT) { throw new StatsWorkOverloadedError(); @@ -556,9 +555,9 @@ export function createStatsService(deps: StatsServiceDeps) { chartsCache.set(key, { ts: validated.generatedAtMs, data: validated.payload }); return validated.payload; })().finally(() => { - if (chartsInflight.get(key) === refresh) chartsInflight.delete(key); + if (chartsInflight.get(scope) === refresh) chartsInflight.delete(scope); }); - chartsInflight.set(key, refresh); + chartsInflight.set(scope, refresh); return refresh; } @@ -587,7 +586,15 @@ export function createStatsService(deps: StatsServiceDeps) { // background, while observer-scoped data never crosses this boundary. cached = await loadPersistedCharts(scope, key, visibilityGeneration); } - if (cached && Date.now() - cached.ts <= chartsSnapshotStaleTtlMs) { + const cachedVisibilityGeneration = Number( + (cached?.data as { snapshot?: { visibilityGeneration?: unknown } } | undefined) + ?.snapshot?.visibilityGeneration, + ); + if ( + cached + && cachedVisibilityGeneration === visibilityGeneration + && Date.now() - cached.ts <= chartsSnapshotStaleTtlMs + ) { // A cold process must treat the persisted snapshot's durable max-age as // authoritative. The shorter in-memory cadence is only how often we // check freshness; it must not force a multi-minute rebuild after every