From c22674edbd3779ca11e2c49e5ecb4b0cb64801bb Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 00:41:40 +0000 Subject: [PATCH] fix(frontend): keep the RI exchange reshape empty-state count current loadRIExchange loads the convertible RIs, reshape recommendations and history in parallel. When the reshape list came back empty before the RI list, its empty state quoted currentRIs from before the load (the previous scope's count, or "none are registered yet" on first load) and nothing rendered it again once the RIs arrived. Track whether currentRIs belongs to the scope being shown. Until it does, or when the RI list failed to load, the empty state says only "No reshape recommendations right now." After the AWS Promise.all, re-render the empty state for the same generation when the reshape load succeeded with no rows; a failed reshape load keeps its error message. Refs #1135 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/riexchange.test.ts | 42 +++++++++++++++++++++++ frontend/src/riexchange.ts | 40 ++++++++++++++++----- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index e9f42d41c..4167aaff0 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -820,6 +820,48 @@ describe('reshape recommendations empty state', () => { expect(recsEl.textContent).toContain('1 convertible RI '); expect(recsEl.textContent).not.toContain('none are registered'); }); + + // Issue #1135: the three loads race. An empty reshape response that lands + // before the RI list must not keep the previous scope's count. + it('re-renders the empty-state count when recommendations resolve before the RI list', async () => { + (api.listConvertibleRIs as jest.Mock).mockResolvedValueOnce([sampleRI]); + await loadRIExchange(); + expect(recsEl.textContent).toContain('All 1 convertible RI '); + + let releaseRIs: (v: unknown) => void = () => undefined; + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseRIs = r; })); + const reload = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + // Recommendations are in, RIs are not: no stale "All 1" claim. + expect(api.getReshapeRecommendations).toHaveBeenCalledTimes(2); + expect(recsEl.textContent).not.toContain('All 1 convertible RI'); + expect(recsEl.textContent).toContain('No reshape recommendations right now.'); + + releaseRIs([sampleRI, { ...sampleRI, reserved_instance_id: 'ri-2' }, { ...sampleRI, reserved_instance_id: 'ri-3' }]); + await reload; + expect(recsEl.textContent).toContain('All 3 convertible RIs meet your utilization threshold'); + }); + + it('does not quote a count from a failed RI load', async () => { + (api.listConvertibleRIs as jest.Mock).mockResolvedValueOnce([sampleRI]); + await loadRIExchange(); + (api.listConvertibleRIs as jest.Mock).mockRejectedValueOnce(new Error('boom')); + await loadRIExchange(); + expect(recsEl.textContent).toContain('No reshape recommendations right now.'); + expect(recsEl.textContent).not.toContain('convertible RI'); + }); + + it('keeps the error from a failed reshape load after the RI list arrives', async () => { + let releaseRIs: (v: unknown) => void = () => undefined; + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseRIs = r; })); + (api.getReshapeRecommendations as jest.Mock).mockRejectedValueOnce(new Error('ce down')); + const reload = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + releaseRIs([sampleRI]); + await reload; + expect(recsEl.querySelector('.error')?.textContent).toContain('Failed to load recommendations: ce down'); + expect(recsEl.textContent).not.toContain('utilization threshold'); + }); }); // Staleness banner tests for issue #150. diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 4b0855f7b..f334d7639 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -35,6 +35,11 @@ import { getCurrentUser } from './state'; // Module state let currentRIs: ConvertibleRI[] = []; +// True once currentRIs holds the convertible-RI list of the scope being +// shown. The reshape empty state only quotes currentRIs.length while this is +// set, so it never prints the previous scope's count, or a count from a list +// that failed to load (issue #1135). +let currentRIsLoaded = false; let currentUtilization: Map = new Map(); let currentRecommendations: ReshapeRecommendation[] = []; const exchangePaymentPattern = /^\d+(?:\.\d+)?$/; @@ -107,6 +112,7 @@ export async function loadRIExchange(): Promise { if (generation !== scopeGeneration) return; utilizationGeneration++; currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRecommendations = []; currentRIAccountID = undefined; @@ -141,11 +147,17 @@ export async function loadRIExchange(): Promise { } // AWS (default / empty provider): full convertible-RI flow. - await Promise.all([ + const [, recsLoaded] = await Promise.all([ loadConvertibleRIs(accountID, generation), loadReshapeRecommendations(generation), loadExchangeHistory(generation), ]); + // The three loads race. When the reshape list came back empty before the + // RI list, its empty state was written without this scope's RI count, so + // render it again now that both are in. A failed reshape load keeps its + // error message (issue #1135). + if (generation !== scopeGeneration) return; + if (recsLoaded && currentRecommendations.length === 0) rerenderReshape(); } /** @@ -155,6 +167,7 @@ export async function loadRIExchange(): Promise { */ function renderGCPEmptyStates(): void { currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRecommendations = []; currentRIAccountID = undefined; @@ -301,6 +314,7 @@ async function loadConvertibleRIs(accountID?: string, generation: number = scope showSkeletonRows(container, 3, 8); currentRIAccountID = accountID; + currentRIsLoaded = false; try { // issue #871: scope to the selected account so the page honours the @@ -308,6 +322,7 @@ async function loadConvertibleRIs(accountID?: string, generation: number = scope const ris = await api.listConvertibleRIs(accountID); if (generation !== scopeGeneration) return; currentRIs = ris; + currentRIsLoaded = true; renderRIsTable(container, accountID); // Load utilization asynchronously (Cost Explorer is slow) utilizationGeneration++; @@ -339,6 +354,7 @@ async function loadExchangeableAzureRIs(accountID: string | undefined, generatio // later provider switch back to AWS starts clean and reshape copy that // reads currentRIs.length isn't skewed by stale AWS rows. currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRIAccountID = undefined; @@ -521,9 +537,11 @@ function appendAzureCell(tr: HTMLTableRowElement, text: string): void { // Reshape recommendations // ────────────────────────────────────────────── -export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { +// Resolves true when this call rendered a fresh reshape list (empty or not), +// false when it was superseded, failed, or had nowhere to render. +export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-recommendations-list'); - if (!container) return; + if (!container) return false; // Issue #344 T3: skeleton rows for the reshape-recommendations table. // 3 rows × 8 cols matches the rendered table shape — see @@ -533,17 +551,19 @@ export async function loadReshapeRecommendations(generation: number = scopeGener try { const resp: ReshapeRecommendationsResponse = await api.getReshapeRecommendations(); - if (generation !== scopeGeneration) return; + if (generation !== scopeGeneration) return false; currentRecommendations = resp.recommendations ?? []; renderRecommendations(container); renderReshapeStalenessBanner(resp.recs_staleness, resp.recs_collected_at); renderExcludedRIsNotice(resp.excluded_ris); + return true; } catch (error) { - if (generation !== scopeGeneration) return; + if (generation !== scopeGeneration) return false; teardownSkeleton(container); renderExcludedRIsNotice(undefined); const err = error as Error; container.innerHTML = `

Failed to load recommendations: ${escapeHtml(err.message)}

`; + return false; } } @@ -1359,9 +1379,13 @@ function renderRecommendations(container: HTMLElement): void { // contains convertibles to be utilized. Prior to this commit the same // message fired regardless of fleet presence, so a brand-new tenant saw // a claim about utilization of a non-existent fleet. - const copy = currentRIs.length === 0 - ? 'Reshape recommendations appear once your accounts have active convertible Reserved Instances — none are registered yet.' - : `All ${currentRIs.length} convertible RI${currentRIs.length === 1 ? '' : 's'} meet your utilization threshold. No reshape needed.`; + // Until this scope's RI list is in (or when it failed to load), say + // nothing about the fleet rather than quote a stale count (#1135). + const copy = !currentRIsLoaded + ? 'No reshape recommendations right now.' + : currentRIs.length === 0 + ? 'Reshape recommendations appear once your accounts have active convertible Reserved Instances — none are registered yet.' + : `All ${currentRIs.length} convertible RI${currentRIs.length === 1 ? '' : 's'} meet your utilization threshold. No reshape needed.`; container.innerHTML = `

${escapeHtml(copy)}

`; return; }