Skip to content

fix(frontend): color the per-service savings chart by layer #1335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
IAmJSD merged 1 commit into from
Oct 24, 2026
Merged

fix(frontend): color the per-service savings chart by layer #1335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 35 additions & 128 deletions frontend/src/__tests__/dashboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1426,12 +1426,11 @@ describe('Dashboard Module', () => {
// Import the public helpers from dashboard. The module is already
// loaded above via the jest.mock chain, so we can import directly.
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs, darkenHexColor, parseHexColor } = require('../dashboard') as {
const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs, SAVINGS_LAYER_COLORS } = require('../dashboard') as {
renderSavingsByService: (recs: unknown[], byService?: Record<string, { potential_savings: number; current_savings: number }>, filterDesc?: string) => void;
computeServiceStats: (dataPoints: unknown[]) => Map<string, { min: number; max: number; sum: number; count: number; samples: number[] }>;
computeServiceStatsFromRecs: (recs: unknown[]) => Map<string, { min: number; max: number; sum: number; count: number; samples: number[]; minLabel?: string; maxLabel?: string }>;
darkenHexColor: (hex: string, factor?: number) => string;
parseHexColor: (hex: string) => { r: number; g: number; b: number };
SAVINGS_LAYER_COLORS: { current: string; lowestOption: string; upside: string };
};

function buildDOM(): void {
Expand Down Expand Up @@ -1703,98 +1702,56 @@ describe('Dashboard Module', () => {
expect(totalLine).toMatch(/\$400/);
});

describe('stable per-service colours and matching legend (#700)', () => {
describe('one colour per layer, matching the legend (#189)', () => {
type ChartCfg = {
data: { labels: string[]; datasets: { label: string; backgroundColor: string[] }[] };
options: { plugins: { legend: { labels: { generateLabels?: (c: unknown) => { text: string; fillStyle: string }[] } } } };
data: { labels: string[]; datasets: { label: string; backgroundColor: unknown }[] };
options: {
plugins: { legend: { labels: { generateLabels?: unknown } } };
scales: { x: { title?: { display?: boolean } } };
};
};
const lastCfg = (): ChartCfg => {
const calls = (Chart as unknown as jest.Mock).mock.calls;
return calls[calls.length - 1]?.[1] as ChartCfg;
};
const colourOf = (cfg: ChartCfg, svc: string): string => {
const ds = cfg.data.datasets.find((d) => d.label === 'Lowest option');
return ds?.backgroundColor[cfg.data.labels.indexOf(svc)] ?? '';
};

test('a service keeps its colour when its rank changes between renders', () => {
test('each layer is one colour on every bar, so the default legend swatch matches it', () => {
buildDOM();
renderSavingsByService([rec('ec2', 500), rec('rds', 300), rec('elasticache', 100)]);
const before = lastCfg();
expect(before.data.labels[0]).toBe('ec2');
const ec2Before = colourOf(before, 'ec2');
const rdsBefore = colourOf(before, 'rds');
const cacheBefore = colourOf(before, 'elasticache');

// Data refresh: rds overtakes ec2, elasticache jumps to rank 2.
renderSavingsByService([rec('ec2', 120), rec('rds', 900), rec('elasticache', 400)]);
const after = lastCfg();
expect(after.data.labels[0]).toBe('rds');
expect(colourOf(after, 'ec2')).toBe(ec2Before);
expect(colourOf(after, 'rds')).toBe(rdsBefore);
expect(colourOf(after, 'elasticache')).toBe(cacheBefore);
expect(new Set([ec2Before, rdsBefore, cacheBefore]).size).toBe(3);
});

test('every known service slug has a distinct colour per family, twins share one', () => {
const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string };
const families: string[][] = [
['compute', 'ec2', 'vm'],
['relational-db', 'rds', 'sql', 'cloudsql'],
['cache', 'elasticache', 'redis', 'memorystore'],
['savingsplans', 'savings-plans', 'savings-plans-compute', 'savings-plans-ec2instance', 'savings-plans-sagemaker', 'savings-plans-database'],
['search', 'opensearch'],
['data-warehouse', 'redshift'],
['memorydb'],
];
const colours = families.map((slugs) => {
const set = new Set(slugs.map((x) => serviceBarColor(x)));
expect([...set]).toHaveLength(1);
return [...set][0];
renderSavingsByService(
[rec('ec2', 100), rec('ec2', 500, 3), rec('rds', 300), rec('elasticache', 80)],
{ ec2: { potential_savings: 500, current_savings: 50 } },
);
const cfg = lastCfg();
const colours = Object.fromEntries(cfg.data.datasets.map((d) => [d.label, d.backgroundColor]));
expect(colours).toEqual({
'Current / Committed': SAVINGS_LAYER_COLORS.current,
'Lowest option': SAVINGS_LAYER_COLORS.lowestOption,
Upside: SAVINGS_LAYER_COLORS.upside,
});
expect(new Set(colours).size).toBe(families.length);
// A single string (not a per-bar array) is what Chart.js's default
// legend draws, so swatch and bars cannot diverge.
for (const d of cfg.data.datasets) expect(typeof d.backgroundColor).toBe('string');
expect(cfg.options.plugins.legend.labels.generateLabels).toBeUndefined();
});

test('unknown slugs never reuse a known family colour', () => {
const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string };
// Fallback colours must not reuse a known family's colour.
const known = new Set(['compute', 'relational-db', 'cache', 'savingsplans', 'search', 'data-warehouse', 'memorydb'].map((x) => serviceBarColor(x)));
for (const unknown of ['alpha', 'zeta-service', 'nosuch', 'x1', 'foo-bar']) {
expect(known.has(serviceBarColor(unknown))).toBe(false);
}
test('the three layers use three distinct colours', () => {
expect(new Set(Object.values(SAVINGS_LAYER_COLORS)).size).toBe(3);
});

test('an unknown slug gets a deterministic colour from the name hash', () => {
const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string };
expect(serviceBarColor('mystery-service')).toBe(serviceBarColor('mystery-service'));
expect(serviceBarColor('Mystery-Service')).toBe(serviceBarColor('mystery-service'));
// The hash must actually vary: a fixed slot for every unknown slug is a bug.
const seen = new Set(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].map((x) => serviceBarColor(`svc-${x}`)));
expect(seen.size).toBeGreaterThan(1);
});

test('legend swatches carry a border so the pale Upside swatch stays visible', () => {
test('colours do not change when a service changes rank', () => {
buildDOM();
renderSavingsByService([rec('ec2', 500)]);
const cfg = lastCfg();
const items = cfg.options.plugins.legend.labels.generateLabels?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) as unknown as { strokeStyle: string; lineWidth: number }[];
for (const item of items) {
expect(item.lineWidth).toBeGreaterThanOrEqual(1);
expect(item.strokeStyle).toBe('#546e7a');
}
renderSavingsByService([rec('ec2', 500), rec('rds', 300)]);
const before = lastCfg().data.datasets.map((d) => d.backgroundColor);
renderSavingsByService([rec('ec2', 120), rec('rds', 900)]);
const after = lastCfg();
expect(after.data.labels[0]).toBe('rds');
expect(after.data.datasets.map((d) => d.backgroundColor)).toEqual(before);
});

test('legend swatches use neutral tones matching the three bar layers, not the first bar colour', () => {
test('drops the redundant "Service" x-axis title', () => {
buildDOM();
renderSavingsByService([rec('ec2', 500), rec('rds', 300)]);
const cfg = lastCfg();
const gen = cfg.options.plugins.legend.labels.generateLabels;
expect(typeof gen).toBe('function');
const items = gen?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) ?? [];
expect(items.map((i) => i.text)).toEqual(['Current / Committed', 'Lowest option', 'Upside']);
const barColours = cfg.data.datasets.flatMap((d) => d.backgroundColor);
for (const item of items) expect(barColours).not.toContain(item.fillStyle);
expect(new Set(items.map((i) => i.fillStyle)).size).toBe(3);
renderSavingsByService([rec('ec2', 500)]);
expect(lastCfg().options.scales.x.title?.display).not.toBe(true);
});
});

Expand Down Expand Up @@ -1851,26 +1808,6 @@ describe('Dashboard Module', () => {
expect(lowestDs?.data[0]).toBe(0);
});

test('current underlay uses a DARKER variant of each service base hue', () => {
buildDOM();
renderSavingsByService(
[rec('ec2', 500)],
{ ec2: { potential_savings: 500, current_savings: 200 } },
);
const chartCtor = Chart as unknown as jest.Mock;
const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1];
const datasets = (lastCall?.[1] as {
data: { datasets: { label: string; backgroundColor: string[] }[] };
}).data.datasets;
const lowestOptionColor = datasets.find((d) => d.label === 'Lowest option')?.backgroundColor[0] as string;
const currentColor = datasets.find((d) => d.label === 'Current / Committed')?.backgroundColor[0] as string;
// The current colour is the darkened form of the base (lowest-option) colour.
expect(currentColor).toBe(darkenHexColor(lowestOptionColor));
// And it is genuinely darker: each channel sum is lower.
const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; };
expect(sum(currentColor)).toBeLessThan(sum(lowestOptionColor));
});

test('current layer defaults to 0 for a service absent from byService', () => {
buildDOM();
renderSavingsByService([rec('rds', 300)], {}); // no rds entry
Expand Down Expand Up @@ -2087,36 +2024,6 @@ describe('Dashboard Module', () => {
expect(lastChartData().labels[0]).toBe('rds');
});
});

// Issue #908: colour-derivation helpers for the current-savings underlay.
describe('colour helpers (issue #908)', () => {
test('parseHexColor parses #rrggbb', () => {
expect(parseHexColor('#1a73e8')).toEqual({ r: 26, g: 115, b: 232 });
});

test('parseHexColor tolerates a missing leading hash', () => {
expect(parseHexColor('34a853')).toEqual({ r: 52, g: 168, b: 83 });
});

test('parseHexColor falls back to the default blue for malformed input', () => {
expect(parseHexColor('not-a-color')).toEqual({ r: 26, g: 115, b: 232 });
});

test('darkenHexColor returns a strictly darker same-hue colour', () => {
const base = '#34a853';
const darker = darkenHexColor(base);
expect(darker).toMatch(/^#[0-9a-f]{6}$/);
const sum = (hex: string): number => { const c = parseHexColor(hex); return c.r + c.g + c.b; };
expect(sum(darker)).toBeLessThan(sum(base));
// 30% reduction by default (factor 0.7): green channel 168 -> ~118.
expect(parseHexColor(darker).g).toBe(Math.round(168 * 0.7));
});

test('darkenHexColor honours an explicit factor', () => {
// factor 0.5 halves each channel.
expect(darkenHexColor('#646464', 0.5)).toBe('#323232');
});
});
});

// H-3 regression: absent API coverage/count fields must render '--', not '$0'/'80%'/'0'.
Expand Down
129 changes: 17 additions & 112 deletions frontend/src/dashboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,88 +31,17 @@ let savingsByServiceChart: Chart | null = null;
// Maximum number of services to show in the bar chart before truncating.
const SAVINGS_BY_SERVICE_MAX = 10;

// Palette for per-service bars. A service's colour is fixed by its name (see
// serviceBarColor), never by its rank, so it survives data refreshes (#700).
const SERVICE_BAR_COLORS = [
'#1a73e8', // blue
'#34a853', // green
'#fbbc04', // yellow
'#ea4335', // red
'#9c27b0', // purple
'#00bcd4', // cyan
'#ff5722', // deep-orange
'#607d8b', // blue-grey
'#795548', // brown
'#e91e63', // pink
];

// One palette slot per service family, keyed on the slugs the app emits
// (purchase mapServiceSlug, plan-service-ids.ts). A canonical slug and its
// legacy twin share a slot, so the same service is one colour whichever
// spelling a provider returns. Slots 0-6 are reserved for these families.
const KNOWN_SERVICE_COLOR_INDEX: Readonly<Record<string, number>> = {
compute: 0, ec2: 0, vm: 0,
'relational-db': 1, rds: 1, sql: 1, cloudsql: 1,
cache: 2, elasticache: 2, redis: 2, memorystore: 2,
savingsplans: 3, 'savings-plans': 3, 'savings-plans-compute': 3,
'savings-plans-ec2instance': 3, 'savings-plans-sagemaker': 3, 'savings-plans-database': 3,
search: 4, opensearch: 4,
'data-warehouse': 5, redshift: 5,
memorydb: 6,
};
const KNOWN_SERVICE_SLOTS = 7;

/** Stable colour for a service name: identical on every render, regardless of rank. */
export function serviceBarColor(service: string): string {
const key = service.toLowerCase();
let idx = KNOWN_SERVICE_COLOR_INDEX[key];
if (idx === undefined) {
// Unknown slugs hash into the spare slots; two unknown slugs may collide.
let h = 0;
for (let i = 0; i < key.length; i++) h = (h * 31 + key.charCodeAt(i)) >>> 0;
idx = KNOWN_SERVICE_SLOTS + (h % (SERVICE_BAR_COLORS.length - KNOWN_SERVICE_SLOTS));
}
return SERVICE_BAR_COLORS[idx] ?? '#1a73e8';
}

// The legend names the three stacked layers, which every service shares, so
// its swatches are neutral tones that mirror the bar shading (dark / solid /
// 35% alpha) instead of the first bar's service colour.
const LEGEND_SWATCH_COLORS = ['#37474f', '#78909c', 'rgba(120,144,156,0.35)'];
// Border keeps the pale Upside swatch at >= 3:1 against the white card.
const LEGEND_SWATCH_BORDER = '#546e7a';

/**
* Parse a #rrggbb hex string into its r/g/b components. Falls back to the
* default service bar blue (#1a73e8) for malformed input so the chart never
* renders a NaN colour. Exported for unit testing.
*/
export function parseHexColor(hex: string): { r: number; g: number; b: number } {
const m = /^#?([0-9a-fA-F]{6})$/.exec(hex.trim());
const clean = m ? m[1]! : '1a73e8';
return {
r: parseInt(clean.substring(0, 2), 16),
g: parseInt(clean.substring(2, 4), 16),
b: parseInt(clean.substring(4, 6), 16),
};
}

/**
* Derive a darker shade of a base #rrggbb colour by reducing its perceived
* lightness. Used for the current-savings underlay so it reads as the
* "already-realized" portion beneath the lighter potential-range bar of the
* same hue (issue #908). The factor (default 0.7 → ~30% darker) is applied
* multiplicatively per channel so the hue is preserved; the result is
* programmatic, not hardcoded per service. Exported for unit testing.
*/
export function darkenHexColor(hex: string, factor = 0.7): string {
const { r, g, b } = parseHexColor(hex);
const scale = (c: number): string => {
const v = Math.max(0, Math.min(255, Math.round(c * factor)));
return v.toString(16).padStart(2, '0');
};
return `#${scale(r)}${scale(g)}${scale(b)}`;
}
// One colour per stacked layer, the same on every bar, so the legend swatches
// are exactly the colours drawn (#189). The x axis already names each service,
// so hue encodes the layer, not the service: no colour can change with rank
// (#700) and the layers differ by hue rather than by shades of one (#744).
// Slots 1-3 of the validated categorical palette, CVD-distinct in every pair.
// Upside (aqua) sits below 3:1 on white; the tooltip carries its value.
export const SAVINGS_LAYER_COLORS = {
current: '#2a78d6', // blue
lowestOption: '#eb6834', // orange
upside: '#1baf7a', // aqua
} as const;

// In-memory index of the currently-rendered upcoming purchases, keyed by
// execution_id. The "View Details" affordance renders from this — the
Expand Down Expand Up @@ -985,21 +914,6 @@ export function renderSavingsByService(

const totalSavings = positive.reduce((acc, e) => acc + e.visibleTotal, 0);

// Colour: one stable base colour per service name, then derive lighter/darker shades.
const bgColors = visible.map(e => serviceBarColor(e.svc));

// Current / Committed: darkest shade (30% darker than base).
const currentColors = bgColors.map(c => darkenHexColor(c));

// Lowest option: the base (solid) colour.
const lowestOptionColors = bgColors;

// Upside: same hue at 35% opacity for the variability band.
const upsideColors = bgColors.map(c => {
const { r, g, b } = parseHexColor(c);
return `rgba(${r},${g},${b},0.35)`;
});

savingsByServiceChart = new Chart(canvas, {
type: 'bar',
data: {
Expand All @@ -1011,21 +925,21 @@ export function renderSavingsByService(
{
label: 'Current / Committed',
data: currentData,
backgroundColor: currentColors,
backgroundColor: SAVINGS_LAYER_COLORS.current,
borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 4, bottomRight: 4 },
stack: 'savings',
},
{
label: 'Lowest option',
data: lowestOptionData,
backgroundColor: lowestOptionColors,
backgroundColor: SAVINGS_LAYER_COLORS.lowestOption,
borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 0, bottomRight: 0 },
stack: 'savings',
},
{
label: 'Upside',
data: upsideData,
backgroundColor: upsideColors,
backgroundColor: SAVINGS_LAYER_COLORS.upside,
borderRadius: { topLeft: 4, topRight: 4, bottomLeft: 0, bottomRight: 0 },
stack: 'savings',
},
Expand All @@ -1038,17 +952,8 @@ export function renderSavingsByService(
legend: {
display: true,
position: 'top',
labels: {
boxWidth: 12,
generateLabels: (chart) => chart.data.datasets.map((ds, i) => ({
text: ds.label ?? '',
fillStyle: LEGEND_SWATCH_COLORS[i] ?? LEGEND_SWATCH_COLORS[1] ?? '#78909c',
strokeStyle: LEGEND_SWATCH_BORDER,
lineWidth: 1,
hidden: !chart.isDatasetVisible(i),
datasetIndex: i,
})),
},
// Default swatches: each one is its dataset's single bar colour.
labels: { boxWidth: 12 },
},
tooltip: {
callbacks: {
Expand Down Expand Up @@ -1088,7 +993,7 @@ export function renderSavingsByService(
scales: {
x: {
stacked: true,
title: { display: true, text: 'Service' },
// No axis title: the tick labels are the service names (#189).
// Keep every service label on narrow screens (rotate instead of skipping).
ticks: { autoSkip: false, maxRotation: 60 },
},
Expand Down
Loading
Loading