fix(ri-exchange): exclude convertible RIs with a non-positive duration from cost math

frontend/src/__tests__/riexchange.test.ts

@@ -64,6 +64,7 @@ import {
   loadReshapeRecommendations,
   loadRIExchange,
   openExchangeModal,
+  renderExcludedRIsNotice,
   renderReshapeStalenessBanner,
   setupRIExchangeHandlers,
 } from '../riexchange';
@@ -1271,6 +1272,44 @@ describe('RI Exchange global filter scoping (issue #871)', () => {
     // Modal is closed synchronously on the chip event.
     expect(modal.classList.contains('hidden')).toBe(true);
   });
+
+  describe('excluded-RI notice lifecycle (issue #1181)', () => {
+    const noticeText = (): string => document.getElementById('ri-exchange-recommendations-excluded')?.textContent ?? '';
+
+    beforeEach(async () => {
+      (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({
+        recommendations: [],
+        excluded_ris: [{ ri_id: 'ri-noterm', reason: 'no term' }],
+      });
+      await loadRIExchange();
+      expect(noticeText()).toContain('ri-noterm');
+    });
+
+    it('clears after a failed reload', async () => {
+      (api.getReshapeRecommendations as jest.Mock).mockRejectedValue(new Error('boom'));
+      await loadReshapeRecommendations();
+      expect(noticeText()).toBe('');
+    });
+
+    it('clears when switching to GCP', async () => {
+      stateMod().getCurrentProvider.mockReturnValue('gcp');
+      await loadRIExchange();
+      expect(noticeText()).toBe('');
+    });
+
+    it('clears when switching to Azure', async () => {
+      stateMod().getCurrentProvider.mockReturnValue('azure');
+      stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']);
+      await loadRIExchange();
+      expect(noticeText()).toBe('');
+    });
+
+    it('clears on an account-resolution error', async () => {
+      stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']);
+      await loadRIExchange();
+      expect(noticeText()).toBe('');
+    });
+  });
 });
 
 describe('exchange history table counts (issue #286)', () => {
@@ -1313,3 +1352,52 @@ describe('exchange history table counts (issue #286)', () => {
     expect(cells).not.toContain('4');
   });
 });
+
+// Issue #1181: RIs left out of reshape analysis must be visible in the UI.
+describe('excluded RIs notice', () => {
+  let listEl: HTMLDivElement;
+
+  beforeEach(() => {
+    const wrapper = document.createElement('div');
+    listEl = document.createElement('div');
+    listEl.id = 'ri-exchange-recommendations-list';
+    wrapper.appendChild(listEl);
+    document.body.appendChild(wrapper);
+  });
+
+  afterEach(() => {
+    document.body.innerHTML = '';
+    jest.resetAllMocks();
+  });
+
+  it('lists the excluded ids as text when excluded_ris is non-empty', async () => {
+    (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({
+      recommendations: [],
+      excluded_ris: [
+        { ri_id: 'ri-0', reason: 'no term' },
+        { ri_id: '<img src=x onerror=alert(1)>', reason: 'no term' },
+      ],
+    });
+
+    await loadReshapeRecommendations();
+
+    const notice = document.getElementById('ri-exchange-recommendations-excluded');
+    expect(notice?.textContent).toContain('2 convertible RIs were left out because AWS returned no term length');
+    expect(notice?.textContent).toContain('ri-0');
+    expect(notice?.textContent).toContain('<img src=x onerror=alert(1)>');
+    expect(notice?.querySelector('img')).toBeNull();
+  });
+
+  it('shows no notice when excluded_ris is empty or absent, and clears a stale one', () => {
+    renderExcludedRIsNotice([{ ri_id: 'ri-0', reason: 'x' }]);
+    expect(document.getElementById('ri-exchange-recommendations-excluded')?.textContent).toContain('1 convertible RI was left out');
+    renderExcludedRIsNotice(undefined);
+    expect(document.getElementById('ri-exchange-recommendations-excluded')?.textContent).toBe('');
+  });
+
+  it('does not render a notice for an empty list on load', async () => {
+    (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], excluded_ris: [] });
+    await loadReshapeRecommendations();
+    expect(document.getElementById('ri-exchange-recommendations-excluded')?.textContent ?? '').toBe('');
+  });
+});

frontend/src/api/index.ts

@@ -193,7 +193,7 @@ export {
 } from './inventory';
 
 // Re-export RI exchange functions
-export type { ReshapeRecommendationsResponse } from './riexchange';
+export type { ReshapeRecommendationsResponse, ExcludedRI } from './riexchange';
 export {
   listConvertibleRIs,
   listExchangeableAzureRIs,

frontend/src/api/riexchange.ts

@@ -61,8 +61,15 @@ export async function getRIUtilization(lookbackDays?: number): Promise<RIUtiliza
  * recs_collected_at carries the raw ISO-8601 timestamp so the banner
  * can display a relative-time label ("last collected 23h ago").
  */
+export interface ExcludedRI {
+  ri_id: string;
+  reason: string;
+}
+
 export interface ReshapeRecommendationsResponse {
   recommendations: ReshapeRecommendation[];
+  /** RIs left out of cost-based analysis (no recommendation); omitted when none. */
+  excluded_ris?: ExcludedRI[];
   recs_staleness?: string;
   recs_collected_at?: string | null;
 }

frontend/src/riexchange.ts

@@ -23,6 +23,7 @@ import type {
   OfferingOption,
   TargetOffering,
   ReshapeRecommendationsResponse,
+  ExcludedRI,
   Provider,
 } from './api';
 import { openModal, closeModal } from './modal';
@@ -115,6 +116,7 @@ export async function loadRIExchange(): Promise<void> {
       if (el) renderEmptyState(el, message);
     }
     renderReshapeStalenessBanner('', null);
+    renderExcludedRIsNotice(undefined);
     const container = document.getElementById('ri-exchange-instances-list');
     if (container) {
       container.innerHTML = `<p class="error">Failed to resolve the selected account: ${escapeHtml((error as Error).message)}</p>`;
@@ -167,6 +169,7 @@ function renderGCPEmptyStates(): void {
   if (history) renderEmptyState(history, "RI Exchange isn't available for GCP.");
 
   renderReshapeStalenessBanner('', null);
+  renderExcludedRIsNotice(undefined);
 }
 
 /**
@@ -181,6 +184,7 @@ function renderReshapeNotApplicable(provider: string): void {
     renderEmptyState(container, `Reshape recommendations are not available for ${providerLabel(provider)}.`);
   }
   renderReshapeStalenessBanner('', null);
+  renderExcludedRIsNotice(undefined);
 }
 
 /**
@@ -533,14 +537,41 @@ export async function loadReshapeRecommendations(generation: number = scopeGener
     currentRecommendations = resp.recommendations ?? [];
     renderRecommendations(container);
     renderReshapeStalenessBanner(resp.recs_staleness, resp.recs_collected_at);
+    renderExcludedRIsNotice(resp.excluded_ris);
   } catch (error) {
     if (generation !== scopeGeneration) return;
     teardownSkeleton(container);
+    renderExcludedRIsNotice(undefined);
     const err = error as Error;
     container.innerHTML = `<p class="error">Failed to load recommendations: ${escapeHtml(err.message)}</p>`;
   }
 }
 
+/**
+ * Render (or clear) the notice listing RIs the backend left out of the
+ * reshape analysis. Sits in a sibling slot before the recommendations list;
+ * ids are inserted as text, never HTML.
+ */
+export function renderExcludedRIsNotice(excluded: ExcludedRI[] | undefined): void {
+  const NOTICE_ID = 'ri-exchange-recommendations-excluded';
+  let notice = document.getElementById(NOTICE_ID);
+  if (!notice) {
+    const listEl = document.getElementById('ri-exchange-recommendations-list');
+    if (!listEl || !listEl.parentElement) return;
+    notice = document.createElement('div');
+    notice.id = NOTICE_ID;
+    listEl.parentElement.insertBefore(notice, listEl);
+  }
+  if (!excluded || excluded.length === 0) {
+    notice.textContent = '';
+    notice.className = '';
+    return;
+  }
+  const n = excluded.length;
+  notice.className = 'freshness-banner warning';
+  notice.textContent = `${n} convertible RI${n === 1 ? ' was' : 's were'} left out because AWS returned no term length: ${excluded.map((e) => e.ri_id).join(', ')}`;
+}
+
 /**
  * Render (or clear) the staleness banner above the reshape-recommendations
  * table. The banner slot is a sibling element with id

internal/api/handler_ri_exchange.go

@@ -1598,42 +1598,49 @@ func parseThresholdParam(params map[string]string) (float64, error) {
 //
 //	monthly = (FixedPrice / hours_per_term + UsagePrice + recurring_hourly) × 730
 //
-// 730 is AWS's canonical hours-per-month constant. Returns zero when
-// Duration is zero (defensive — would otherwise divide by zero).
+// 730 is AWS's canonical hours-per-month constant. Returns an error when
+// Duration is non-positive: a zero cost would read as a free RI and skip
+// reshape's dollar-units pricing gate, so callers must exclude the RI.
 //
 // Used to populate exchange.RIInfo.MonthlyCost so the cross-family
 // dollar-units pre-filter can compare against per-target offering
 // costs computed with the same formula.
-func monthlyCostFromConvertibleRI(ri ec2svc.ConvertibleRI) float64 {
+func monthlyCostFromConvertibleRI(ri ec2svc.ConvertibleRI) (float64, error) {
 	if ri.Duration <= 0 {
-		return 0
+		return 0, fmt.Errorf("convertible RI %s has non-positive duration %d: monthly cost is not computable", ri.ReservedInstanceID, ri.Duration)
 	}
 	hoursPerTerm := float64(ri.Duration) / 3600
-	if hoursPerTerm <= 0 {
-		return 0
-	}
-	return ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730
+	return ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730, nil
 }
 
 // convertToExchangeTypes converts provider-specific types to the exchange package types.
-func convertToExchangeTypes(instances []ec2svc.ConvertibleRI, utilData []recommendations.RIUtilization) ([]exchange.RIInfo, []exchange.UtilizationInfo) {
-	riInfos := make([]exchange.RIInfo, len(instances))
+// RIs whose monthly cost cannot be computed are returned as excluded rather
+// than dropped silently, so the response can surface them.
+func convertToExchangeTypes(instances []ec2svc.ConvertibleRI, utilData []recommendations.RIUtilization) ([]exchange.RIInfo, []exchange.UtilizationInfo, []ExcludedRI) {
+	var excluded []ExcludedRI
+	riInfos := make([]exchange.RIInfo, 0, len(instances))
 	for i := range instances {
 		inst := instances[i]
-		riInfos[i] = exchange.RIInfo{
+		monthlyCost, err := monthlyCostFromConvertibleRI(inst)
+		if err != nil {
+			logging.Warnf("convertToExchangeTypes: excluding RI from reshape: %v", err)
+			excluded = append(excluded, ExcludedRI{RIID: inst.ReservedInstanceID, Reason: "AWS returned no term length (duration), so its monthly cost cannot be computed"})
+			continue
+		}
+		riInfos = append(riInfos, exchange.RIInfo{
 			ID:                  inst.ReservedInstanceID,
 			InstanceType:        inst.InstanceType,
 			InstanceCount:       inst.InstanceCount,
 			OfferingClass:       "convertible",
 			NormalizationFactor: inst.NormalizationFactor,
-			MonthlyCost:         monthlyCostFromConvertibleRI(inst),
+			MonthlyCost:         monthlyCost,
 			CurrencyCode:        inst.CurrencyCode,
 			// Plumb the AWS-reported RI duration straight through —
 			// reshape's term-match guard rejects alternatives whose
 			// TermSeconds differs from the source so a 3y RI never
 			// surfaces as an alternative to a 1y commitment.
 			TermSeconds: inst.Duration,
-		}
+		})
 	}
 
 	utilInfos := make([]exchange.UtilizationInfo, len(utilData))
@@ -1644,7 +1651,7 @@ func convertToExchangeTypes(instances []ec2svc.ConvertibleRI, utilData []recomme
 		}
 	}
 
-	return riInfos, utilInfos
+	return riInfos, utilInfos, excluded
 }
 
 // reshapeRequestParams groups parsed query parameters for getReshapeRecommendations.
@@ -1723,7 +1730,7 @@ func (h *Handler) getReshapeRecommendations(ctx context.Context, req *events.Lam
 		return nil, fmt.Errorf("failed to get RI utilization: %w", err)
 	}
 
-	riInfos, utilInfos := convertToExchangeTypes(instances, utilData)
+	riInfos, utilInfos, excluded := convertToExchangeTypes(instances, utilData)
 	// Cross-family alternatives are sourced from the cached AWS Cost
 	// Explorer purchase recommendations table in Postgres — no per-rec
 	// DescribeReservedInstancesOfferings fan-out, no hand-curated
@@ -1742,7 +1749,7 @@ func (h *Handler) getReshapeRecommendations(ctx context.Context, req *events.Lam
 	lookup := purchaseRecLookupFromStore(h.config, cloudAccountID)
 	recs := exchange.AnalyzeReshapingWithRecs(ctx, riInfos, utilInfos, p.threshold, p.region, currencyCode, lookup)
 
-	resp := &ReshapeRecommendationsResponse{Recommendations: recs}
+	resp := &ReshapeRecommendationsResponse{Recommendations: recs, ExcludedRIs: excluded}
 	h.attachReshapeStaleness(ctx, resp)
 	return resp, nil
 }
@@ -2040,6 +2047,15 @@ type ReshapeRecommendationsResponse struct {
 	RecsCollectedAt *time.Time                       `json:"recs_collected_at,omitempty"`
 	RecsStaleness   string                           `json:"recs_staleness,omitempty"`
 	Recommendations []exchange.ReshapeRecommendation `json:"recommendations"`
+	// ExcludedRIs lists RIs left out of cost-based analysis (and so have no
+	// recommendation) because their monthly cost could not be computed.
+	ExcludedRIs []ExcludedRI `json:"excluded_ris,omitempty"`
+}
+
+// ExcludedRI names a convertible RI omitted from reshape analysis and why.
+type ExcludedRI struct {
+	RIID   string `json:"ri_id"`
+	Reason string `json:"reason"`
 }
 
 // reshapeSoftStaleThreshold is the age at which the reshape recs banner

internal/api/handler_ri_exchange_integration_test.go

@@ -135,7 +135,8 @@ func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) {
 
 	ec2Fake := &fakeReshapeEC2{
 		instances: []ec2svc.ConvertibleRI{
-			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1},
+			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000},
+			{ReservedInstanceID: "ri-noterm", InstanceType: "m5.xlarge", InstanceCount: 1},
 		},
 	}
 	recsFake := &fakeReshapeRecs{
@@ -153,6 +154,8 @@ func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) {
 	rec := body.Recommendations[0]
 	require.Equal(t, "ri-1", rec.SourceRIID)
 	require.Equal(t, "m5.large", rec.TargetInstanceType)
+	require.Len(t, body.ExcludedRIs, 1, "an RI with no term length must be reported, not silently skipped")
+	require.Equal(t, "ri-noterm", body.ExcludedRIs[0].RIID)
 
 	// Same-family m5 stripped; c5 + r5 surface ascending by cost.
 	require.Len(t, rec.AlternativeTargets, 2,
@@ -197,7 +200,7 @@ func TestReshapeRecommendations_Integration_SecondCallHitsCache(t *testing.T) {
 
 	ec2Fake := &fakeReshapeEC2{
 		instances: []ec2svc.ConvertibleRI{
-			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1},
+			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000},
 		},
 	}
 	recsFake := &fakeReshapeRecs{
@@ -233,7 +236,7 @@ func TestReshapeRecommendations_Integration_NoCachedRecsReturnsPrimaryOnly(t *te
 
 	ec2Fake := &fakeReshapeEC2{
 		instances: []ec2svc.ConvertibleRI{
-			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1},
+			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000},
 		},
 	}
 	recsFake := &fakeReshapeRecs{
@@ -334,7 +337,7 @@ func TestReshapeRecommendations_Integration_ScopedAccount_FiltersToAccount(t *te
 
 	ec2Fake := &fakeReshapeEC2{
 		instances: []ec2svc.ConvertibleRI{
-			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1},
+			{ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000},
 		},
 	}
 	recsFake := &fakeReshapeRecs{