fix(frontend): update savings charts in place, drop stale trend data

frontend/src/__tests__/chart-upsert.test.ts

@@ -0,0 +1,76 @@
+/**
+ * Issue #747: charts are updated in place instead of destroyed and rebuilt,
+ * so the entry animation doesn't replay on every filter or range change.
+ */
+jest.mock('chart.js', () => {
+  const MockChart = jest.fn().mockImplementation((canvas: HTMLCanvasElement, config: { type: string; data: unknown; options?: unknown }) => ({
+    canvas,
+    config: { type: config.type },
+    data: config.data,
+    options: config.options,
+    update: jest.fn(),
+    destroy: jest.fn(),
+  }));
+  return { Chart: MockChart, registerables: [] };
+});
+
+import { Chart, type ChartConfiguration } from 'chart.js';
+import { upsertChart } from '../modules/chart-utils';
+
+type MockInstance = { canvas: HTMLCanvasElement; data: unknown; options: unknown; update: jest.Mock; destroy: jest.Mock };
+
+const config = (value: number, type: 'line' | 'bar' = 'line'): ChartConfiguration => ({
+  type,
+  data: { datasets: [{ label: 'x', data: [value] }] },
+  options: { responsive: true },
+});
+
+describe('upsertChart', () => {
+  beforeEach(() => (Chart as unknown as jest.Mock).mockClear());
+
+  test('creates a chart on first render', () => {
+    const canvas = document.createElement('canvas');
+    const chart = upsertChart(null, canvas, config(1));
+    expect(Chart).toHaveBeenCalledTimes(1);
+    expect((chart as unknown as MockInstance).canvas).toBe(canvas);
+  });
+
+  test('updates an existing chart in place without animation', () => {
+    const canvas = document.createElement('canvas');
+    const first = upsertChart(null, canvas, config(1));
+    const next = config(2);
+    const second = upsertChart(first, canvas, next);
+
+    expect(second).toBe(first);
+    expect(Chart).toHaveBeenCalledTimes(1);
+    const inst = second as unknown as MockInstance;
+    expect(inst.data).toBe(next.data);
+    expect(inst.options).toBe(next.options);
+    expect(inst.update).toHaveBeenCalledWith('none');
+    expect(inst.destroy).not.toHaveBeenCalled();
+  });
+
+  test('recreates the chart when its canvas was replaced', () => {
+    const first = upsertChart(null, document.createElement('canvas'), config(1));
+    const fresh = document.createElement('canvas');
+    const second = upsertChart(first, fresh, config(2));
+
+    expect((first as unknown as MockInstance).destroy).toHaveBeenCalledTimes(1);
+    expect(second).not.toBe(first);
+    expect((second as unknown as MockInstance).canvas).toBe(fresh);
+    expect(Chart).toHaveBeenCalledTimes(2);
+  });
+
+  // #749's Purchases series toggle switches between a line and a bar chart;
+  // Chart.js can't change the chart type of an instance in place.
+  test('recreates the chart when the chart type changes', () => {
+    const canvas = document.createElement('canvas');
+    const first = upsertChart(null, canvas, config(1, 'bar'));
+    const second = upsertChart(first, canvas, config(2, 'line'));
+
+    expect((first as unknown as MockInstance).destroy).toHaveBeenCalledTimes(1);
+    expect((first as unknown as MockInstance).update).not.toHaveBeenCalled();
+    expect(second).not.toBe(first);
+    expect(Chart).toHaveBeenCalledTimes(2);
+  });
+});

frontend/src/__tests__/dashboard.test.ts

@@ -1058,6 +1058,84 @@ describe('Dashboard Module', () => {
       (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
     });
 
+    // #747: a slower, older response must not overwrite a newer one.
+    describe('stale-response guard (#747)', () => {
+      type Deferred = { resolve: (v: unknown) => void; reject: (e: unknown) => void };
+      const deferAnalytics = (): Deferred => {
+        const d = {} as Deferred;
+        (api.getSavingsAnalytics as jest.Mock).mockImplementationOnce(() => new Promise((resolve, reject) => {
+          d.resolve = resolve; d.reject = reject;
+        }));
+        return d;
+      };
+      const plotted = (): number[] => {
+        const calls = (Chart as unknown as jest.Mock).mock.calls;
+        const cfg = calls[calls.length - 1]?.[1] as { data: { datasets: { data: { y: number }[] }[] } } | undefined;
+        return cfg?.data.datasets[0]?.data.map((p) => p.y) ?? [];
+      };
+      // Two points: one point is the sparse-data state (#188), which draws no chart.
+      const point = (y: number) => ({ data_points: [
+        { timestamp: '2025-12-31T00:00:00Z', cumulative_savings: 0 },
+        { timestamp: '2026-01-01T00:00:00Z', cumulative_savings: y },
+      ] });
+
+      test('an older response landing last is ignored', async () => {
+        const older = deferAnalytics();
+        const newer = deferAnalytics();
+        const first = loadSavingsTrendChart();
+        const second = loadSavingsTrendChart();
+
+        newer.resolve(point(200));
+        await second;
+        older.resolve(point(100));
+        await first;
+
+        expect(Chart).toHaveBeenCalledTimes(1);
+        expect(plotted()).toEqual([0, 200]);
+      });
+
+      test('an older request failing last does not replace the newer chart with an error', async () => {
+        const older = deferAnalytics();
+        const newer = deferAnalytics();
+        const first = loadSavingsTrendChart();
+        const second = loadSavingsTrendChart();
+
+        newer.resolve(point(200));
+        await second;
+        older.reject(new Error('503'));
+        await first;
+
+        expect(document.getElementById('savings-trend-chart')?.classList.contains('hidden')).toBe(false);
+        expect(document.getElementById('savings-trend-empty')?.classList.contains('hidden')).toBe(true);
+      });
+    });
+
+    // #747: a range change updates the existing chart instead of rebuilding it.
+    test('a second load updates the trend chart in place without animation (#747)', async () => {
+      const update = jest.fn();
+      const destroy = jest.fn();
+      (Chart as unknown as jest.Mock).mockImplementationOnce((canvas: HTMLCanvasElement, config: { type: string }) => ({ canvas, config, update, destroy }));
+      (api.getSavingsAnalytics as jest.Mock)
+        .mockResolvedValueOnce({ data_points: [
+          { timestamp: '2025-12-31T00:00:00Z', cumulative_savings: 0 },
+          { timestamp: '2026-01-01T00:00:00Z', cumulative_savings: 100 },
+        ] })
+        .mockResolvedValueOnce({ data_points: [
+          { timestamp: '2025-12-31T00:00:00Z', cumulative_savings: 0 },
+          { timestamp: '2026-01-01T00:00:00Z', cumulative_savings: 300 },
+        ] });
+
+      await loadSavingsTrendChart();
+      await loadSavingsTrendChart();
+
+      expect(Chart).toHaveBeenCalledTimes(1);
+      expect(destroy).not.toHaveBeenCalled();
+      expect(update).toHaveBeenCalledWith('none');
+      // The trend chart is module state; drop it so later tests start fresh.
+      (api.getSavingsAnalytics as jest.Mock).mockResolvedValueOnce({ data_points: [] });
+      await loadSavingsTrendChart();
+    });
+
     // #744: summary label + table for the trend chart.
     test('describes the plotted series and lists every point as a table (#744)', async () => {
       (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [
@@ -1837,6 +1915,24 @@ describe('Dashboard Module', () => {
 
     // renderSavingsByService DOM behaviour tests (now driven by recommendation fixtures).
     describe('DOM behaviour', () => {
+      // #747: re-rendering with new data updates the chart in place.
+      test('re-rendering updates the existing chart instead of rebuilding it (#747)', () => {
+        buildDOM();
+        const update = jest.fn();
+        const destroy = jest.fn();
+        (Chart as unknown as jest.Mock).mockImplementationOnce((canvas: HTMLCanvasElement, config: { type: string }) => ({ canvas, config, update, destroy }));
+        renderSavingsByService([rec('ec2', 100)]);
+        renderSavingsByService([rec('ec2', 100), rec('rds', 50)]);
+
+        expect(Chart).toHaveBeenCalledTimes(1);
+        expect(destroy).not.toHaveBeenCalled();
+        expect(update).toHaveBeenCalledWith('none');
+
+        // The empty state still tears the chart down.
+        renderSavingsByService([]);
+        expect(destroy).toHaveBeenCalledTimes(1);
+      });
+
       // #744: the role="img" canvas gets a value summary and a table.
       describe('text alternative (#744)', () => {
         const tableRows = (): string[][] =>

frontend/src/dashboard.ts

@@ -18,7 +18,7 @@ import { buildFirstRunEmptyState, isScopeFiltered } from './first-run-empty-stat
 import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/skeleton';
 import { buildKpiTile } from './lib/kpi-tile';
 import { scopeUpcomingToProvider, upcomingEmptyMessage, UPCOMING_ACCOUNT_SCOPE_NOTE } from './lib/upcoming-scope';
-import { formatTrendAxisTick, formatTrendTooltipTitle, formatSparseTrendMessage, MIN_TREND_POINTS } from './modules/chart-utils';
+import { formatTrendAxisTick, formatTrendTooltipTitle, formatSparseTrendMessage, MIN_TREND_POINTS, upsertChart } from './modules/chart-utils';
 import { setChartSummary, renderChartTable, removeChartTable } from './lib/chart-a11y';
 // Re-export for backward compatibility (tests and other callers import from dashboard.ts).
 export { formatTrendAxisTick } from './modules/chart-utils';
@@ -30,6 +30,8 @@ Chart.register(...registerables);
 // the per-service savings chart instance below.
 let savingsTrendChart: Chart | null = null;
 let savingsTrendRange: '7' | '30' | '90' | 'all' = '90';
+// Request generation for loadSavingsTrendChart; see the stale guard there (#747).
+let savingsTrendGeneration = 0;
 
 // Chart instance for the per-service savings-range bar chart (issue #765).
 let savingsByServiceChart: Chart | null = null;
@@ -881,12 +883,6 @@ export function renderSavingsByService(
   const section = document.getElementById('savings-by-service-section');
   if (!canvas) return;
 
-  // Destroy existing chart before rebuilding.
-  if (savingsByServiceChart) {
-    savingsByServiceChart.destroy();
-    savingsByServiceChart = null;
-  }
-
   const stats = computeServiceStatsFromRecs(recs);
   const heading = section?.querySelector('h3');
 
@@ -933,6 +929,8 @@ export function renderSavingsByService(
 
   if (positive.length === 0) {
     if (heading) heading.textContent = 'Potential savings range per service';
+    savingsByServiceChart?.destroy();
+    savingsByServiceChart = null;
     canvas.classList.add('hidden');
     removeChartTable(SAVINGS_BY_SERVICE_TABLE_ID);
     if (emptyEl) {
@@ -988,7 +986,8 @@ export function renderSavingsByService(
       formatCurrency(e.upside), formatCurrency(e.visibleTotal)]),
   });
 
-  savingsByServiceChart = new Chart(canvas, {
+  // #747: update in place so a filter change doesn't replay the entry animation.
+  savingsByServiceChart = upsertChart(savingsByServiceChart, canvas, {
     type: 'bar',
     data: {
       labels,
@@ -1102,6 +1101,9 @@ export async function loadSavingsTrendChart(): Promise<void> {
   const canvas = document.getElementById('savings-trend-chart') as HTMLCanvasElement | null;
   const empty = document.getElementById('savings-trend-empty');
   if (!canvas) return;
+  // #747: quick range or filter changes overlap requests; only the latest
+  // may paint, or an older response landing last would show the wrong range.
+  const generation = ++savingsTrendGeneration;
 
   const now = new Date();
   const nowMs = now.getTime();
@@ -1135,6 +1137,7 @@ export async function loadSavingsTrendChart(): Promise<void> {
       ...(provider ? { provider } : {}),
       ...(accountIDs.length > 0 ? { account_ids: accountIDs } : {}),
     });
+    if (generation !== savingsTrendGeneration) return;
 
     const points = data.data_points ?? [];
 
@@ -1198,8 +1201,6 @@ export async function loadSavingsTrendChart(): Promise<void> {
     canvas.classList.remove('hidden');
     empty?.classList.add('hidden');
 
-    if (savingsTrendChart) savingsTrendChart.destroy();
-
     // #744: summarize the plotted series and list every point as text.
     const rangeText = isAllRange ? 'the last year' : `the last ${days} days`;
     const latest = chartData[chartData.length - 1]!;
@@ -1213,7 +1214,8 @@ export async function loadSavingsTrendChart(): Promise<void> {
       rows: chartData.map(p => [formatTrendAxisTick(p.x, interval), formatCurrency(p.y)]),
     });
 
-    savingsTrendChart = new Chart(canvas, {
+    // #747: update in place so a range click doesn't replay the entry animation.
+    savingsTrendChart = upsertChart(savingsTrendChart, canvas, {
       type: 'line',
       data: {
         datasets: [{
@@ -1263,6 +1265,7 @@ export async function loadSavingsTrendChart(): Promise<void> {
       },
     });
   } catch (err) {
+    if (generation !== savingsTrendGeneration) return;
     // Analytics is Postgres-backed now, but the endpoint still guards with
     // a 503 when the analytics client isn't wired. Don't break the rest of
     // the dashboard — hide the widget and fall back to a neutral message.

frontend/src/modules/chart-utils.ts

@@ -2,6 +2,7 @@
  * Shared chart utility helpers used by multiple chart modules.
  */
 
+import { Chart, type ChartConfiguration } from 'chart.js';
 import { formatCurrency } from '../utils';
 
 /**
@@ -48,3 +49,28 @@ export function formatTrendTooltipTitle(tsMs: number, intervalHint: 'hourly' | '
     }
     return d.toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' });
 }
+
+/**
+ * Create a chart, or update an existing one in place (issue #747).
+ *
+ * Destroying and recreating a chart on every filter or range change replays
+ * Chart.js's grow-from-zero entry animation each time. When `existing` is
+ * still drawing on `canvas`, swap in the new data and options and redraw
+ * without animation; otherwise (first render, the canvas was replaced, or the
+ * chart type changed, e.g. the Purchases chart's line/bar series toggle from
+ * #749) destroy any stale instance and create a fresh one.
+ */
+export function upsertChart(existing: Chart | null, canvas: HTMLCanvasElement, config: ChartConfiguration): Chart {
+    if (existing && existing.canvas === canvas && chartType(existing) === config.type) {
+        existing.data = config.data;
+        existing.options = config.options ?? {};
+        existing.update('none');
+        return existing;
+    }
+    existing?.destroy();
+    return new Chart(canvas, config);
+}
+
+function chartType(chart: Chart): string | undefined {
+    return 'type' in chart.config ? chart.config.type : undefined;
+}

frontend/src/modules/savings-history.ts

@@ -5,7 +5,7 @@
 import { Chart, registerables } from 'chart.js';
 import { getSavingsAnalytics, type SavingsAnalyticsResponse, type SavingsDataPoint } from '../api';
 import * as state from '../state';
-import { formatTrendAxisTick } from './chart-utils';
+import { formatTrendAxisTick, upsertChart } from './chart-utils';
 import { formatCompactCurrency, formatCurrency } from '../utils';
 import { setChartSummary, renderChartTable, removeChartTable } from '../lib/chart-a11y';
 
@@ -401,10 +401,6 @@ function renderSavingsChart(
     const axisMinMs = periodStart ? periodStart.getTime() : nowMs - 7 * 86400_000;
     const axisMaxMs = periodEnd ? periodEnd.getTime() : nowMs;
 
-    if (savingsChart) {
-        savingsChart.destroy();
-    }
-
     const label = cumulative ? 'Cumulative Savings' : 'Period Savings';
     const dataset = cumulative
         ? {
@@ -449,7 +445,10 @@ function renderSavingsChart(
         ]),
     });
 
-    savingsChart = new Chart(ctx, {
+    // #747: update in place so a period or unit change doesn't replay the
+    // entry animation. Switching the series (#749) changes the chart type,
+    // which upsertChart handles by recreating the chart.
+    savingsChart = upsertChart(savingsChart, ctx, {
         type: cumulative ? 'line' : 'bar',
         data: {
             datasets: [dataset],

frontend/src/trends.ts

@@ -41,7 +41,11 @@ const CHART_ID = 'trends-savings-chart';
 
 const PROVIDER_LABELS: Record<string, string> = { aws: 'AWS', azure: 'Azure', gcp: 'GCP' };
 
-let trendsChart: Chart | null = null;
+// Typed as the bar chart it is: assigning Chart<'bar'> to the default
+// Chart<keyof ChartTypeRegistry> trips a Chart.js variance check whose
+// result depends on ts-loader's check order (it broke the webpack build
+// once chart-utils gained upsertChart, #747).
+let trendsChart: Chart<'bar', (number | null)[], string> | null = null;
 // Each load takes a ticket; a response that lands after a newer request was
 // issued is dropped so a slow reply can't overwrite the current scope.
 let loadSeq = 0;