fix(frontend/recs): enforce one-variant-per-cell radio selection (closes #224)

After PR #195 (issue #188) the recommendations refresh fans out across
2 terms × 3 payments per `(provider, account, service, region,
resource_type, engine)` cell — up to 6 alternative rec rows per
physical resource. They're alternatives, not additions, but the
checkboxes had no mutual-exclusion logic, producing wrong purchase
intent in two ways:

1. **Manual checking**: a user could check `1yr/all-upfront` AND
   `3yr/no-upfront` for the same EC2 m5.large in the same account.
   Both fed into the purchase plan → double commitment for one
   resource.

2. **`select-all` was the destructive form**: every visible row got
   added, so 3 cells × 6 variants = 18 commitments instead of the
   3 the user expected when clicking "buy what's recommended". 6× the
   intended spend per cell.

# What changes

- New `cellKey(rec)` helper returns the
  `(provider, cloud_account_id, service, region, resource_type, engine)`
  prefix (same as the scheduler ID encoding from #189, minus the
  `(term, payment)` suffix). Recs sharing this key are alternatives
  for the same physical resource.

- Per-row checkbox change handler now enforces radio behaviour: on
  check, scan the visible-recs list for any sibling in the same cell
  that's already selected, deselect it FIRST, then add the new rec.
  Cells are independent — selecting in cell X doesn't touch cell Y's
  selection.

- `select-all` handler rewritten: clear current selection, then call
  new `pickBestVariantPerCell(recs)` which groups by cell and picks
  the variant with the highest **effective monthly savings**:
  `effective = savings - (upfront_cost / (term * 12))`. Amortizing
  the upfront over the commitment term means a 3yr/all-upfront with
  a huge lump-sum doesn't beat a 1yr/no-upfront just on raw
  `savings`. Sibling issue #223 will replace this tiebreaker with
  "matches resolved GlobalConfig.DefaultTerm + DefaultPayment" when
  it lands; until then, amortized savings is the right deterministic
  default.

# Out of scope (deliberate)

- **Native `<input type="radio" name="cell-X">` markup**. Per the
  issue: "Designer call — if cell-grouping (sibling issue #226)
  lands first, radios become visually correct." Stayed with
  checkboxes-with-radio-behaviour for this PR; markup switch waits
  for cell-grouping visual.
- **Default-select per cell from settings** (sibling #223).

# Tests

4 new tests in `frontend/src/__tests__/recommendations.test.ts`
inside the `'Issue #224: one-variant-per-cell radio selection'`
describe block:

  (a) Manual toggle within a cell — checking variant B with variant
      A already selected: A is removed, B is added. Sibling A was
      not also added; B was not also removed.
  (b) Cross-cell independence — selecting in cell X must NOT remove
      cell Y's existing selection.
  (c) `select-all` collapses 18 → 3: 3 cells × 6 variants. After
      click, exactly 3 add calls; clearSelectedRecommendations was
      called first to drop stale state.
  (d) Tiebreaker pin — single cell with 3 variants whose
      `(savings, upfront_cost, term)` produce known amortized values
      ($200, $300, $400). The middle variant wins ($400 effective)
      despite the high-upfront variant having $1200 raw savings,
      proving the amortization is actually computed.

`tsc --noEmit` clean. `npx jest --testPathPatterns="recommendations"`
exit 0. `npm run build` (webpack production) exit 0.

Author: cristim

frontend/src/__tests__/recommendations.test.ts

@@ -2023,3 +2023,165 @@ describe('Issue #132: bulk-buy collapses SP plan types into one bucket', () => {
     expect(sectionTitles.some((t) => t.includes('AWS / ec2'))).toBe(true);
   });
 });
+
+// Issue #224: at most one (term, payment) variant per physical-resource
+// cell can be selected at any time. After PR #195's per-cell fan-out (2
+// terms × 3 payments per cell), naive selection produces wrong purchase
+// intent — manual checking lets the user accumulate sibling commitments,
+// and `select-all` over-commits 6×. Cell = `(provider, account, service,
+// region, resource_type, engine)`. The fix lives in two places: the
+// per-row checkbox change handler (deselect any in-cell sibling on check)
+// and the select-all handler (group by cell, pick highest-effective per).
+describe('Issue #224: one-variant-per-cell radio selection', () => {
+  beforeEach(() => {
+    document.body.replaceChildren();
+    const recsTab = document.createElement('div');
+    recsTab.id = 'recommendations-tab';
+    recsTab.className = 'tab-content active';
+    const summary = document.createElement('div');
+    summary.id = 'recommendations-summary';
+    const list = document.createElement('div');
+    list.id = 'recommendations-list';
+    recsTab.appendChild(summary);
+    recsTab.appendChild(list);
+    document.body.appendChild(recsTab);
+    const purchaseModal = document.createElement('div');
+    purchaseModal.id = 'purchase-modal';
+    purchaseModal.className = 'hidden';
+    document.body.appendChild(purchaseModal);
+    jest.clearAllMocks();
+    (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
+    (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set());
+  });
+
+  // (a) Manual toggle: two variants of the same cell. Checking variant B
+  // when variant A is already selected must remove A first, leaving only B.
+  test('(a) checking variant B in same cell deselects sibling variant A', async () => {
+    const recs = [
+      { id: 'cellA-1y-allup',  provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 1, savings: 100, upfront_cost: 500 },
+      { id: 'cellA-3y-noup',   provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 3, savings: 200, upfront_cost: 0 },
+    ];
+    (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] });
+    // Pretend variant A is already selected (the "user previously checked it" state).
+    (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['cellA-1y-allup']));
+    await loadRecommendations();
+
+    // Tick variant B in the same cell.
+    const cbs = Array.from(document.querySelectorAll<HTMLInputElement>('tbody input[data-rec-id]'));
+    const variantB = cbs.find((cb) => cb.dataset['recId'] === 'cellA-3y-noup');
+    expect(variantB).toBeDefined();
+    variantB!.checked = true;
+    variantB!.dispatchEvent(new Event('change'));
+
+    // The handler must have removed sibling A AND added B.
+    const removed = (state.removeSelectedRecommendation as jest.Mock).mock.calls.map((c) => c[0]);
+    const added = (state.addSelectedRecommendation as jest.Mock).mock.calls.map((c) => c[0]);
+    expect(removed).toContain('cellA-1y-allup');
+    expect(added).toContain('cellA-3y-noup');
+    // Sanity: B was not also removed, A was not also added.
+    expect(removed).not.toContain('cellA-3y-noup');
+    expect(added).not.toContain('cellA-1y-allup');
+  });
+
+  // (b) Cross-cell independence: selecting a rec in cell X must not
+  // affect cell Y's selection state. The radio enforcement is per-cell,
+  // not global.
+  test('(b) selecting in cell X does not affect cell Y selections', async () => {
+    const recs = [
+      { id: 'cellX-1y',  provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 1, savings: 100, upfront_cost: 500 },
+      { id: 'cellY-1y',  provider: 'aws', cloud_account_id: 'acct-1', service: 'rds', resource_type: 'db.r5.large', region: 'us-east-1', engine: 'mysql', count: 1, term: 1, savings: 200, upfront_cost: 1000 },
+    ];
+    (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] });
+    // Pretend cellY is already selected.
+    (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['cellY-1y']));
+    await loadRecommendations();
+
+    // Tick cellX.
+    const cbs = Array.from(document.querySelectorAll<HTMLInputElement>('tbody input[data-rec-id]'));
+    const cellX = cbs.find((cb) => cb.dataset['recId'] === 'cellX-1y');
+    expect(cellX).toBeDefined();
+    cellX!.checked = true;
+    cellX!.dispatchEvent(new Event('change'));
+
+    // cellY must NOT have been removed — cells are independent.
+    const removed = (state.removeSelectedRecommendation as jest.Mock).mock.calls.map((c) => c[0]);
+    expect(removed).not.toContain('cellY-1y');
+    const added = (state.addSelectedRecommendation as jest.Mock).mock.calls.map((c) => c[0]);
+    expect(added).toContain('cellX-1y');
+  });
+
+  // (c) Select-all collapses to one-per-cell. Three distinct cells × six
+  // variants each = 18 recs. After select-all, exactly 3 should be added,
+  // not 18 (one per cell). This is the headline money-impact fix.
+  test('(c) select-all picks exactly one variant per cell (3 cells × 6 variants → 3 selected)', async () => {
+    const cells = [
+      { account: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '' },
+      { account: 'acct-1', service: 'ec2', resource_type: 'm5.large',  region: 'us-east-1', engine: '' },
+      { account: 'acct-1', service: 'rds', resource_type: 'db.r5.large', region: 'eu-west-1', engine: 'mysql' },
+    ];
+    const variants = [
+      { term: 1, payment: 'all-upfront' },
+      { term: 1, payment: 'partial-upfront' },
+      { term: 1, payment: 'no-upfront' },
+      { term: 3, payment: 'all-upfront' },
+      { term: 3, payment: 'partial-upfront' },
+      { term: 3, payment: 'no-upfront' },
+    ];
+    const recs: Array<Record<string, unknown>> = [];
+    let i = 0;
+    for (const c of cells) {
+      for (const v of variants) {
+        recs.push({
+          id: `c${i++}`,
+          provider: 'aws', cloud_account_id: c.account, service: c.service,
+          resource_type: c.resource_type, region: c.region, engine: c.engine,
+          count: 1, term: v.term, savings: 100 + i, upfront_cost: 500,
+        });
+      }
+    }
+    expect(recs).toHaveLength(18);
+
+    (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] });
+    await loadRecommendations();
+
+    const selectAll = document.getElementById('select-all-recs') as HTMLInputElement;
+    expect(selectAll).not.toBeNull();
+    selectAll.checked = true;
+    selectAll.dispatchEvent(new Event('change'));
+
+    // Exactly 3 add calls — one per cell.
+    expect((state.addSelectedRecommendation as jest.Mock).mock.calls).toHaveLength(3);
+    // And clearSelectedRecommendations was called first to drop any stale state.
+    expect(state.clearSelectedRecommendations).toHaveBeenCalled();
+  });
+
+  // (d) Tiebreaker: when multiple variants share a cell, select-all picks
+  // the variant with the highest EFFECTIVE monthly savings (amortizing
+  // upfront over term * 12 months) — NOT the highest raw `savings`.
+  // Concrete example: a 3yr/all-upfront with $36000 upfront + $1200/mo
+  // headline savings has effective = 1200 - 36000/36 = $200. A 1yr/no-upfront
+  // with $0 upfront + $300/mo headline savings has effective = $300. The
+  // 1yr/no-upfront wins despite the 3yr's higher raw `savings`.
+  test('(d) select-all picks highest-effective-savings (amortized) per cell', async () => {
+    const recs = [
+      // 3yr/all-upfront — high raw savings ($1200/mo) but huge upfront drags effective to $200/mo.
+      { id: 'big-upfront', provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 3, savings: 1200, upfront_cost: 36000 },
+      // 1yr/no-upfront — lower raw ($300/mo) but $0 upfront → effective stays at $300/mo.
+      { id: 'no-upfront',  provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 1, savings: 300,  upfront_cost: 0 },
+      // 3yr/partial-upfront — middle of the road ($600/mo savings, $7200 upfront → effective = 600 - 7200/36 = $400).
+      { id: 'middle',      provider: 'aws', cloud_account_id: 'acct-1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', engine: '', count: 1, term: 3, savings: 600,  upfront_cost: 7200 },
+    ];
+    (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] });
+    await loadRecommendations();
+
+    const selectAll = document.getElementById('select-all-recs') as HTMLInputElement;
+    selectAll.checked = true;
+    selectAll.dispatchEvent(new Event('change'));
+
+    // Exactly one add call (single cell, one variant picked).
+    const addCalls = (state.addSelectedRecommendation as jest.Mock).mock.calls;
+    expect(addCalls).toHaveLength(1);
+    // The "middle" variant has the highest effective ($400/mo > $300 > $200) — picked.
+    expect(addCalls[0]![0]).toBe('middle');
+  });
+});

frontend/src/recommendations.ts

@@ -142,6 +142,55 @@ const SORTABLE_STRING_COLUMNS: Record<string, (r: LocalRecommendation) => string
   region: (r) => r.region ?? '',
 };
 
+// cellKey identifies the physical-resource cell a rec belongs to.
+// After PR #195's per-(term, payment) fan-out, a single physical resource
+// produces up to 6 alternative rec rows (2 terms × 3 payments). The
+// `(provider, cloud_account_id, service, region, resource_type, engine)`
+// prefix is the cell — recs sharing this prefix are alternatives, not
+// additions. Same prefix the scheduler ID encoding uses
+// (scheduler.go:856-858, PR #189) but without the (term, payment) suffix.
+//
+// Used by issue #224's radio enforcement: at most one variant per cell
+// can be selected at any time.
+function cellKey(rec: LocalRecommendation): string {
+  return `${rec.provider}|${rec.cloud_account_id ?? ''}|${rec.service}|${rec.region}|${rec.resource_type}|${rec.engine ?? ''}`;
+}
+
+// pickBestVariantPerCell collapses a list of recs to one rec per cell,
+// choosing the variant with the highest effective monthly savings.
+//
+// Effective savings amortizes the upfront cost across the term:
+//   effective = savings - (upfront_cost / (term * 12))
+//
+// Two `(savings, upfront_cost, term)` tuples that look identical on raw
+// `savings` can score very differently on the amortized number — e.g. a
+// 3yr all-upfront commitment with a large lump-sum upfront has a much
+// lower effective monthly savings than a no-upfront variant with the
+// same headline savings. Picking by amortization picks the variant
+// that's actually best for the operator's wallet over the term.
+//
+// Used by issue #224's `select-all` handler. Sibling issue #223 will
+// replace this tiebreaker with "matches resolved GlobalConfig.DefaultTerm
+// + DefaultPayment" once that lands; until then, amortized savings is
+// the right deterministic default.
+function pickBestVariantPerCell(recs: readonly LocalRecommendation[]): LocalRecommendation[] {
+  const byCell = new Map<string, LocalRecommendation>();
+  const effective = (r: LocalRecommendation): number => {
+    // Defensive clamp: if a rec arrives with term=0 from a malformed
+    // fixture, treat it as 1yr so the division doesn't blow up.
+    const monthsInTerm = Math.max(1, r.term * 12);
+    return r.savings - (r.upfront_cost / monthsInTerm);
+  };
+  for (const r of recs) {
+    const k = cellKey(r);
+    const existing = byCell.get(k);
+    if (!existing || effective(r) > effective(existing)) {
+      byCell.set(k, r);
+    }
+  }
+  return Array.from(byCell.values());
+}
+
 const SORT_HEADER_LABELS: Record<string, string> = {
   provider: 'Provider',
   account: 'Account',
@@ -1930,7 +1979,16 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void {
   if (selectAllCheckbox) {
     selectAllCheckbox.addEventListener('change', () => {
       if (selectAllCheckbox.checked) {
-        recommendations.forEach((r) => state.addSelectedRecommendation(r.id));
+        // Issue #224: select-all picks ONE variant per cell (highest-effective-
+        // savings) rather than every visible row. After PR #195's per-(term,
+        // payment) fan-out, naive "select every row" produces 6× the intended
+        // commitments per resource — wrong purchase intent. Clear current
+        // selection first so a stale choice from a different filter context
+        // doesn't bleed through.
+        state.clearSelectedRecommendations();
+        for (const r of pickBestVariantPerCell(recommendations)) {
+          state.addSelectedRecommendation(r.id);
+        }
       } else {
         state.clearSelectedRecommendations();
       }
@@ -1942,11 +2000,26 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void {
   // changes so a stale selection from a previous filter is a no-op
   // once the user narrows, rather than pointing at whichever rec
   // happens to occupy the old index position.
+  //
+  // Issue #224: enforce one-variant-per-cell radio behaviour on check.
+  // When the user checks a variant, deselect any other variant of the
+  // same cell that's already selected — a single physical resource
+  // can only carry one (term, payment) commitment at a time.
   container.querySelectorAll<HTMLInputElement>('input[data-rec-id]').forEach(cb => {
     cb.addEventListener('change', () => {
       const id = cb.dataset['recId'] || '';
       if (!id) return;
       if (cb.checked) {
+        const newRec = recommendations.find((r) => r.id === id);
+        if (newRec) {
+          const newCell = cellKey(newRec);
+          const selected = state.getSelectedRecommendationIDs();
+          for (const r of recommendations) {
+            if (r.id !== id && selected.has(r.id) && cellKey(r) === newCell) {
+              state.removeSelectedRecommendation(r.id);
+            }
+          }
+        }
         state.addSelectedRecommendation(id);
       } else {
         state.removeSelectedRecommendation(id);