ux(recommendations): persist column filters across page reload (closes #163)

frontend/src/__tests__/recommendations.test.ts

@@ -1,7 +1,7 @@
 /**
  * Recommendations module tests
  */
-import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline } from '../recommendations';
+import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline, loadColumnFilters, saveColumnFilters, resetColumnFiltersState } from '../recommendations';
 import type { CostPeriod } from '../state';
 
 // Mock the api module
@@ -6678,3 +6678,184 @@ describe('renderUsageSparkline (issue #239)', () => {
     expect(pairs[6]).toMatch(/^56\.0,/);
   });
 });
+
+// ============================================================================
+// Issue #163: Column filter localStorage persistence
+// ============================================================================
+
+describe('Column filters localStorage persistence (issue #163)', () => {
+  beforeEach(() => {
+    resetColumnFiltersState();
+    // localStorageMock is reset by jest.clearAllMocks() in the global beforeEach
+  });
+
+  // --- loadColumnFilters ---
+
+  describe('loadColumnFilters', () => {
+    test('returns empty object when localStorage key is absent', () => {
+      // Default mock: getItem returns null
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+
+    test('returns empty object on JSON parse error', () => {
+      localStorageMock.getItem.mockReturnValue('{not valid json}');
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+
+    test('returns empty object when schemaVersion is wrong', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({ schemaVersion: 99, filters: { region: { kind: 'set', values: ['us-east-1'] } } }),
+      );
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+
+    test('returns empty object when filters is not an object', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({ schemaVersion: 1, filters: 'not-an-object' }),
+      );
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+
+    test('restores a categorical (set) filter correctly', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({
+          schemaVersion: 1,
+          filters: { region: { kind: 'set', values: ['us-east-1', 'eu-west-1'] } },
+        }),
+      );
+      const result = loadColumnFilters();
+      expect(result.region).toEqual({ kind: 'set', values: ['us-east-1', 'eu-west-1'] });
+      expect(Object.keys(result)).toHaveLength(1);
+    });
+
+    test('restores a numeric (expr) filter correctly', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({
+          schemaVersion: 1,
+          filters: { savings: { kind: 'expr', expr: '>100' } },
+        }),
+      );
+      const result = loadColumnFilters();
+      expect(result.savings).toEqual({ kind: 'expr', expr: '>100' });
+    });
+
+    test('silently drops unknown column keys', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({
+          schemaVersion: 1,
+          filters: {
+            region: { kind: 'set', values: ['us-east-1'] },
+            future_unknown_column: { kind: 'set', values: ['foo'] },
+          },
+        }),
+      );
+      const result = loadColumnFilters();
+      expect(result.region).toBeDefined();
+      expect((result as Record<string, unknown>)['future_unknown_column']).toBeUndefined();
+      expect(Object.keys(result)).toHaveLength(1);
+    });
+
+    test('silently drops filters with malformed shape', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({
+          schemaVersion: 1,
+          filters: {
+            region: { kind: 'set', values: 'not-an-array' },
+            savings: { kind: 'expr', expr: 42 },
+            count: { kind: 'unknown-kind', data: 'x' },
+          },
+        }),
+      );
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+
+    test('silently drops expr filter with empty string expr', () => {
+      localStorageMock.getItem.mockReturnValue(
+        JSON.stringify({
+          schemaVersion: 1,
+          filters: { savings: { kind: 'expr', expr: '' } },
+        }),
+      );
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+  });
+
+  // --- saveColumnFilters ---
+
+  describe('saveColumnFilters', () => {
+    test('writes correct JSON shape to localStorage', () => {
+      saveColumnFilters({ region: { kind: 'set', values: ['us-east-1'] } });
+      expect(localStorageMock.setItem).toHaveBeenCalledWith(
+        'cudly.recs.columnFilters.v1',
+        expect.stringContaining('"schemaVersion":1'),
+      );
+      const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string;
+      const parsed = JSON.parse(callArg);
+      expect(parsed.schemaVersion).toBe(1);
+      expect(parsed.filters.region).toEqual({ kind: 'set', values: ['us-east-1'] });
+    });
+
+    test('writes empty filters object when no filters active', () => {
+      saveColumnFilters({});
+      const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string;
+      const parsed = JSON.parse(callArg);
+      expect(parsed.filters).toEqual({});
+    });
+
+    test('round-trips a numeric expr filter', () => {
+      saveColumnFilters({ savings: { kind: 'expr', expr: '>500' } });
+      const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string;
+      const payload = JSON.parse(callArg);
+      expect(payload.filters.savings).toEqual({ kind: 'expr', expr: '>500' });
+    });
+  });
+
+  // --- round-trip and guard tests ---
+
+  describe('persistence round-trip', () => {
+    test('save then load restores same filter state', () => {
+      const filters = {
+        region: { kind: 'set' as const, values: ['ap-southeast-1'] },
+        savings: { kind: 'expr' as const, expr: '>200' },
+      };
+      const store = new Map<string, string>();
+      localStorageMock.getItem.mockImplementation((k: string) => store.get(k) ?? null);
+      localStorageMock.setItem.mockImplementation((k: string, v: string) => { store.set(k, v); });
+      saveColumnFilters(filters);
+      const restored = loadColumnFilters();
+      expect(restored.region).toEqual({ kind: 'set', values: ['ap-southeast-1'] });
+      expect(restored.savings).toEqual({ kind: 'expr', expr: '>200' });
+      expect(Object.keys(restored)).toHaveLength(2);
+    });
+
+    test('stored key with unknown column is silently dropped, no error', () => {
+      const store = new Map<string, string>([
+        ['cudly.recs.columnFilters.v1', JSON.stringify({
+          schemaVersion: 1,
+          filters: {
+            region: { kind: 'set', values: ['us-east-1'] },
+            deleted_account_col: { kind: 'set', values: ['123456789012'] },
+          },
+        })],
+      ]);
+      localStorageMock.getItem.mockImplementation((k: string) => store.get(k) ?? null);
+      expect(() => loadColumnFilters()).not.toThrow();
+      const result = loadColumnFilters();
+      expect(result.region).toBeDefined();
+      expect((result as Record<string, unknown>)['deleted_account_col']).toBeUndefined();
+    });
+
+    test('corrupted JSON in localStorage falls back to empty object, no error', () => {
+      localStorageMock.getItem.mockReturnValue('}{invalid json][');
+      expect(() => loadColumnFilters()).not.toThrow();
+      const result = loadColumnFilters();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
+  });
+});

frontend/src/recommendations.ts

@@ -1842,6 +1842,7 @@ function buildPopoverContent(
       const expr = input!.value.trim();
       if (expr === '') {
         state.setRecommendationsColumnFilter(column, null);
+        saveColumnFilters(state.getRecommendationsColumnFilters());
         errorEl!.textContent = '';
         rerenderRecommendations();
         return;
@@ -1853,6 +1854,7 @@ function buildPopoverContent(
       }
       errorEl!.textContent = '';
       state.setRecommendationsColumnFilter(column, { kind: 'expr', expr });
+      saveColumnFilters(state.getRecommendationsColumnFilters());
       rerenderRecommendations();
     };
     input.addEventListener('blur', commit);
@@ -1955,6 +1957,7 @@ function buildPopoverContent(
       } else {
         state.setRecommendationsColumnFilter(column, { kind: 'set', values: selected });
       }
+      saveColumnFilters(state.getRecommendationsColumnFilters());
       updateAllTriState();
       updateSPTriState();
       rerenderRecommendations();
@@ -1973,6 +1976,7 @@ function buildPopoverContent(
       } else {
         state.setRecommendationsColumnFilter(column, { kind: 'set', values: [] });
       }
+      saveColumnFilters(state.getRecommendationsColumnFilters());
       updateAllTriState();
       updateSPTriState();
       rerenderRecommendations();
@@ -2016,6 +2020,7 @@ function buildPopoverContent(
     if (input) {
       // Numeric column: Clear drops the expression entirely (no filter).
       state.setRecommendationsColumnFilter(column, null);
+      saveColumnFilters(state.getRecommendationsColumnFilters());
       input.value = '';
       if (errorEl) errorEl.textContent = '';
       rerenderRecommendations();
@@ -2454,6 +2459,7 @@ function renderFilterStatusBar(loadedCount: number, visibleCount: number): void
       badge.className = 'clear-filters';
       badge.addEventListener('click', () => {
         state.clearAllRecommendationsColumnFilters();
+        saveColumnFilters(state.getRecommendationsColumnFilters());
         rerenderRecommendations();
       });
       bar.insertBefore(badge, live);
@@ -3100,6 +3106,79 @@ export function saveColumnVisibility(hidden: ReadonlySet<state.RecommendationsCo
   }
 }
 
+// ---------------------------------------------------------------------------
+// Column filters — localStorage persistence (issue #163)
+// ---------------------------------------------------------------------------
+
+const COLUMN_FILTERS_LS_KEY = 'cudly.recs.columnFilters.v1';
+const COLUMN_FILTERS_SCHEMA_VERSION = 1;
+
+// Full set of valid column ids, used as an allowlist when loading from
+// localStorage so stale or hand-edited keys are silently dropped.
+const VALID_COLUMN_IDS = new Set<state.RecommendationsColumnId>([
+  'provider', 'account', 'service', 'resource_type', 'region',
+  'count', 'term', 'payment', 'savings', 'upfront_cost',
+  'monthly_cost', 'on_demand_monthly', 'effective_savings_pct',
+]);
+
+interface ColumnFiltersSchema {
+  schemaVersion: number;
+  filters: Record<string, state.RecommendationsColumnFilter>;
+}
+
+/** Load column filter state from localStorage. Returns empty object on any error. Exported for tests. */
+export function loadColumnFilters(): state.RecommendationsColumnFilters {
+  try {
+    const raw = localStorage.getItem(COLUMN_FILTERS_LS_KEY);
+    if (!raw) return {};
+    const parsed = JSON.parse(raw) as Partial<ColumnFiltersSchema>;
+    if (parsed.schemaVersion !== COLUMN_FILTERS_SCHEMA_VERSION) return {};
+    if (!parsed.filters || typeof parsed.filters !== 'object' || Array.isArray(parsed.filters)) return {};
+    const result: state.RecommendationsColumnFilters = {};
+    for (const [key, value] of Object.entries(parsed.filters)) {
+      // Drop unknown column ids (stale after a column rename or deletion).
+      if (!VALID_COLUMN_IDS.has(key as state.RecommendationsColumnId)) continue;
+      // Validate filter shape: must be kind:'set' with string[] or kind:'expr' with string.
+      if (value && value.kind === 'set' && Array.isArray(value.values)
+          && value.values.every((v) => typeof v === 'string')) {
+        result[key as state.RecommendationsColumnId] = { kind: 'set', values: value.values };
+      } else if (value && value.kind === 'expr' && typeof value.expr === 'string' && value.expr !== '') {
+        result[key as state.RecommendationsColumnId] = { kind: 'expr', expr: value.expr };
+      }
+      // Anything else (malformed, hand-edited) is silently dropped.
+    }
+    return result;
+  } catch {
+    return {};
+  }
+}
+
+/** Persist column filter state to localStorage. Exported for tests. */
+export function saveColumnFilters(filters: state.RecommendationsColumnFilters): void {
+  try {
+    const payload: ColumnFiltersSchema = {
+      schemaVersion: COLUMN_FILTERS_SCHEMA_VERSION,
+      filters: filters as Record<string, state.RecommendationsColumnFilter>,
+    };
+    localStorage.setItem(COLUMN_FILTERS_LS_KEY, JSON.stringify(payload));
+  } catch {
+    // Private-browsing / quota-exceeded — non-fatal.
+  }
+}
+
+// Seed flag: set to true once column filters are loaded from localStorage
+// on first render so subsequent renders don't overwrite in-session changes.
+let columnFiltersSeeded = false;
+
+/**
+ * Reset column-filter seeded state. Exported for tests only — not part of
+ * the public API. Call in beforeEach to ensure tests don't share seeding state.
+ */
+export function resetColumnFiltersState(): void {
+  columnFiltersSeeded = false;
+  state.clearAllRecommendationsColumnFilters();
+}
+
 // Seed flag: set to true once column visibility is loaded from localStorage
 // on first render so subsequent renders don't overwrite in-session toggles.
 let columnVisibilitySeeded = false;
@@ -3859,6 +3938,20 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void {
   const container = document.getElementById('recommendations-list');
   if (!container) return;
 
+  // Seed column filters from localStorage on the first render (issue #163).
+  // columnFiltersSeeded stays true for the rest of the session so in-session
+  // changes are not overwritten on subsequent rerenders.
+  if (!columnFiltersSeeded) {
+    const persisted = loadColumnFilters();
+    for (const [col, filter] of Object.entries(persisted)) {
+      state.setRecommendationsColumnFilter(
+        col as state.RecommendationsColumnId,
+        filter,
+      );
+    }
+    columnFiltersSeeded = true;
+  }
+
   // Seed column visibility from localStorage on the first render.
   // columnVisibilitySeeded stays true for the rest of the session so
   // in-session toggles (via the "Columns ▾" popover) are not overwritten.