fix(opportunities): cost-column sort toggle + expand/collapse persistence on filter change

frontend/src/__tests__/recommendations.test.ts

@@ -5170,6 +5170,38 @@ describe('Issues #225 + #226: cell grouping with savings range and collapse/expa
       const variantRows = document.querySelectorAll('.rec-variant-row');
       expect(variantRows.length).toBe(2);
     });
+
+    // QA 4.13: expand/collapse state must be reset when the data source
+    // reloads due to a provider/account Global filter change. Before the fix,
+    // stale expandedCells keys from the prior provider desynchronised the
+    // Expand-All button label and left newly-shown groups collapsed.
+    test('QA 4.13: expand state clears on loadRecommendations (provider filter change)', async () => {
+      const recs = multiVariantRecs();
+      (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs });
+      (state.getRecommendations as jest.Mock).mockReturnValue(recs);
+
+      // 1. Load and expand all groups.
+      await loadRecommendations();
+      const expandAllBtn = document.querySelector<HTMLButtonElement>('.expand-all-toggle');
+      expect(expandAllBtn).not.toBeNull();
+      expandAllBtn!.click();
+
+      // Confirm we are now in "Collapse all" state.
+      expect(expandAllBtn!.textContent).toMatch(/Collapse all/);
+
+      // 2. Simulate a provider filter change by calling loadRecommendations()
+      //    again (this is exactly what the subscribeProvider callback invokes).
+      await loadRecommendations();
+
+      // 3. Expand state must have been cleared: button label reverts to
+      //    "Expand all" and no variant rows are visible (groups are collapsed).
+      const expandAllBtn2 = document.querySelector<HTMLButtonElement>('.expand-all-toggle');
+      expect(expandAllBtn2).not.toBeNull();
+      expect(expandAllBtn2!.textContent).toMatch(/Expand all/);
+
+      const variantRows = document.querySelectorAll('.rec-variant-row');
+      expect(variantRows.length).toBe(0);
+    });
   });
 });
 
@@ -6478,6 +6510,70 @@ describe('Issue #494: deterministic group sort on multi-variant cells', () => {
       expect(first.length).toBe(3);
     }
   });
+
+  // -------------------------------------------------------------------------
+  // QA 4.13 - Monthly Cost: zero-cost (all-upfront) variants must not prevent
+  // sort-direction toggle from reordering rows.
+  //
+  // Before the fix, cells with at least one all-upfront variant (monthly_cost=0)
+  // all scored 0 via Math.min(...finite), tying every such cell so the direction
+  // multiplier had nothing to act on and subsequent sort clicks were no-ops.
+  // The fix uses the minimum NON-ZERO recurring cost, falling back to 0 only
+  // when all finite values are 0 (pure all-upfront cell).
+  // -------------------------------------------------------------------------
+  test('Monthly Cost: mixed cell (all-upfront + no-upfront) sorts by non-zero recurring cost, not 0', async () => {
+    // cellLow: all-upfront (monthly=0) + no-upfront (monthly=20) -> score = 20
+    const cellLow = multiVariantCell({
+      resourceType: 'low-mixed', payment1y: 'all-upfront', payment3y: 'no-upfront',
+      upfront1y: 500, upfront3y: 0, monthly1y: 0, monthly3y: 20,
+    });
+    // cellHigh: all-upfront (monthly=0) + no-upfront (monthly=80) -> score = 80
+    const cellHigh = multiVariantCell({
+      resourceType: 'high-mixed', payment1y: 'all-upfront', payment3y: 'no-upfront',
+      upfront1y: 500, upfront3y: 0, monthly1y: 0, monthly3y: 80,
+    });
+    const recs = [...cellHigh, ...cellLow]; // wrong insertion order
+
+    // Ascending: cellLow (score=20) must precede cellHigh (score=80).
+    setupTestFixture(recs, { column: 'monthly_cost', direction: 'asc' });
+    await loadRecommendations();
+    const ascOrder = renderedCellOrder();
+    expect(indexOrFail(ascOrder, 'low-mixed'))
+      .toBeLessThan(indexOrFail(ascOrder, 'high-mixed'));
+
+    // Descending: cellHigh (score=80) must now precede cellLow (score=20).
+    // This is the toggle that was broken before the fix -- both cells scored 0
+    // so direction-flip was a no-op.
+    setupTestFixture(recs, { column: 'monthly_cost', direction: 'desc' });
+    await loadRecommendations();
+    const descOrder = renderedCellOrder();
+    expect(indexOrFail(descOrder, 'high-mixed'))
+      .toBeLessThan(indexOrFail(descOrder, 'low-mixed'));
+
+    // The two directions must produce OPPOSITE orderings (the toggle is live).
+    expect(ascOrder).not.toEqual(descOrder);
+  });
+
+  test('Monthly Cost: pure all-upfront cell scores 0 (not POSITIVE_INFINITY) and sorts before all-null cells', async () => {
+    // cellPureAllUpfront: both variants have monthly_cost=0 (all-upfront) -> score=0
+    const cellPureAllUpfront = multiVariantCell({
+      resourceType: 'pure-upfront', payment1y: 'all-upfront', payment3y: 'all-upfront',
+      upfront1y: 600, upfront3y: 1800, monthly1y: 0, monthly3y: 0,
+    });
+    // cellAllNull: both variants have monthly_cost=null -> score=POSITIVE_INFINITY
+    const cellAllNull = multiVariantCell({
+      resourceType: 'pure-null', payment1y: 'all-upfront', payment3y: 'all-upfront',
+      upfront1y: 1000, upfront3y: 3000, monthly1y: null, monthly3y: null,
+    });
+    const recs = [...cellAllNull, ...cellPureAllUpfront];
+
+    // Ascending: pure-upfront (score=0) must come before all-null (score=+Inf).
+    setupTestFixture(recs, { column: 'monthly_cost', direction: 'asc' });
+    await loadRecommendations();
+    const order = renderedCellOrder();
+    expect(indexOrFail(order, 'pure-upfront'))
+      .toBeLessThan(indexOrFail(order, 'pure-null'));
+  });
 });
 
 // Helper used by the issue-#479/#480/#481/#482/#483/#484 describes below to

frontend/src/recommendations.ts

@@ -62,15 +62,20 @@ export function getAccountName(accountId: string): string {
 
 // issues #225 + #226: expand/collapse state for cell grouping.
 // Contains the cellKey strings of cells the user has explicitly expanded.
-// Cleared on page load / full refresh; survives per-column filter/sort re-renders.
+// Cleared on every loadRecommendations() entry (page load, tab switch back
+// to Opportunities, provider/account Global filter change, manual refresh,
+// lookback change, stale auto-refresh — see resetExpandedCells() call at
+// the top of loadRecommendations). Survives per-column filter/sort/period
+// re-renders, which go through rerenderRecommendations() instead.
 const expandedCells = new Set<string>();
 // Last computed group keys for the visible filtered set — used by the
 // Expand-All button handler to populate expandedCells without re-computing.
 let lastVisibleGroupKeys: string[] = [];
 
 // issue #135: expand/collapse state for SP plan-type group rows.
 // Contains the spGroupKey strings the user has explicitly expanded.
-// Cleared together with expandedCells on page load / full refresh.
+// Cleared together with expandedCells on every loadRecommendations() entry
+// (see resetExpandedCells); survives column-filter/sort/period re-renders.
 const expandedSpGroups = new Set<string>();
 
 // #272 (CR follow-up): cache of the most-recent API-derived summary so
@@ -462,6 +467,13 @@ export async function loadRecommendations(): Promise<void> {
   // the module default ("savings desc"). Idempotent on subsequent reloads.
   readSortFromUrl();
 
+  // QA 4.13 (expand/collapse desync): clear expand state whenever the data
+  // source is reloaded due to a provider/account Global filter change.
+  // Column-filter and sort re-renders go through rerenderRecommendations()
+  // instead, so they intentionally do NOT reach this path -- preserving the
+  // "expand survives column-filter/sort" behavior documented near line 63.
+  resetExpandedCells();
+
   // Issue #344 T3: skeleton rows for the recommendations table so the
   // panel reads as "loading" instead of staying blank while the
   // (potentially multi-second) Promise.all resolves. 5 rows ≈ above-
@@ -1187,14 +1199,25 @@ function cellScoreFor(
   }
   if (column === 'monthly_cost') {
     const period = state.getCostPeriod();
-    const finite: number[] = [];
+    // #494 best-case framing: prefer the smallest NON-ZERO recurring cost so
+    // that all-upfront variants (monthly_cost=0, which is a real value not a
+    // null sentinel) do not tie every cell at 0 and make direction-toggling a
+    // no-op. Tiers:
+    //   1. any non-zero finite value exists  -> Math.min of those
+    //   2. all finite values are 0 (pure all-upfront cell) -> 0
+    //   3. no finite value at all (all null)  -> POSITIVE_INFINITY (sink to bottom)
+    const nonZero: number[] = [];
+    const anyFinite: number[] = [];
     for (const v of variants) {
-      const scaled = scaleCost(v.monthly_cost, period);
-      if (scaled != null) finite.push(scaled);
+      const s = scaleCost(v.monthly_cost, period);
+      if (s != null) {
+        anyFinite.push(s);
+        if (s > 0) nonZero.push(s);
+      }
     }
-    // Best-case framing: lowest recurring cost wins. All-null cells sink to
-    // the bottom via the existing POSITIVE_INFINITY sentinel logic.
-    return finite.length === 0 ? Number.POSITIVE_INFINITY : Math.min(...finite);
+    if (nonZero.length) return Math.min(...nonZero);
+    if (anyFinite.length) return 0;
+    return Number.POSITIVE_INFINITY;
   }
   if (column === 'effective_savings_pct') {
     const finite: number[] = [];