refactor(frontend): extract parseNumericFilter to shared column-filter lib

frontend/src/__tests__/column-filters.test.ts

@@ -0,0 +1,133 @@
+/**
+ * Unit tests for the shared column-filter lib (issue #166).
+ *
+ * parseNumericFilter is the core utility extracted from recommendations.ts.
+ * applyColumnFilters is the generic version that any tab can use.
+ * recommendations.ts continues to re-export ParsedNumericFilter for backward
+ * compat with existing consumers.
+ */
+import { parseNumericFilter, applyColumnFilters } from '../lib/column-filters';
+
+// ---------------------------------------------------------------------------
+// parseNumericFilter
+// ---------------------------------------------------------------------------
+
+describe('parseNumericFilter (lib)', () => {
+  const accept = (expr: string, n: number): boolean => {
+    const r = parseNumericFilter(expr);
+    if (!r.ok) throw new Error(`unexpected parse failure for "${expr}": ${r.error}`);
+    return r.predicate(n);
+  };
+
+  test('empty / blank returns match-all', () => {
+    const r = parseNumericFilter('');
+    expect(r.ok).toBe(true);
+    if (r.ok) expect(r.predicate(0)).toBe(true);
+    expect(parseNumericFilter('   ').ok).toBe(true);
+  });
+
+  test('plain number: exact equality', () => {
+    expect(accept('42', 42)).toBe(true);
+    expect(accept('42', 43)).toBe(false);
+    expect(accept('-5', -5)).toBe(true);
+    expect(accept('3.14', 3.14)).toBe(true);
+    expect(accept('3.14', 3.15)).toBe(false);
+  });
+
+  test('comparators >, >=, <, <=', () => {
+    expect(accept('>10', 11)).toBe(true);
+    expect(accept('>10', 10)).toBe(false);
+    expect(accept('>=10', 10)).toBe(true);
+    expect(accept('<5', 4)).toBe(true);
+    expect(accept('<5', 5)).toBe(false);
+    expect(accept('<=5', 5)).toBe(true);
+  });
+
+  test('inclusive range X..Y (order-independent)', () => {
+    expect(accept('10..20', 10)).toBe(true);
+    expect(accept('10..20', 20)).toBe(true);
+    expect(accept('10..20', 15)).toBe(true);
+    expect(accept('10..20', 9)).toBe(false);
+    expect(accept('20..10', 15)).toBe(true);
+  });
+
+  test('comma-separated terms OR together', () => {
+    expect(accept('5, >100', 5)).toBe(true);
+    expect(accept('5, >100', 150)).toBe(true);
+    expect(accept('5, >100', 50)).toBe(false);
+  });
+
+  test('invalid expression returns ok:false', () => {
+    const r1 = parseNumericFilter('>>5');
+    expect(r1.ok).toBe(false);
+    if (!r1.ok) expect(r1.error).toMatch(/Invalid filter term/);
+    expect(parseNumericFilter('not-a-number').ok).toBe(false);
+    expect(parseNumericFilter('1..').ok).toBe(false);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// applyColumnFilters (generic)
+// ---------------------------------------------------------------------------
+
+type Row = { id: string; service: string; savings: number };
+type Col = 'service' | 'savings';
+
+const rows: Row[] = [
+  { id: 'a', service: 'ec2', savings: 10 },
+  { id: 'b', service: 'rds', savings: 200 },
+  { id: 'c', service: 'ec2', savings: 500 },
+];
+
+const extractors = {
+  categorical: (r: Row, col: Col) => (col === 'service' ? r.service : String(r.savings)),
+  numeric: (r: Row, _col: Col) => r.savings,
+};
+
+describe('applyColumnFilters (lib)', () => {
+  test('empty filters returns a clone of the input', () => {
+    const out = applyColumnFilters<Row, Col>(rows, {}, extractors);
+    expect(out).toEqual(rows);
+    expect(out).not.toBe(rows);
+  });
+
+  test('categorical set filter narrows by membership', () => {
+    const out = applyColumnFilters<Row, Col>(
+      rows,
+      { service: { kind: 'set', values: ['ec2'] } },
+      extractors,
+    );
+    expect(out.map((r) => r.id)).toEqual(['a', 'c']);
+  });
+
+  test('numeric expr filter narrows by predicate', () => {
+    const out = applyColumnFilters<Row, Col>(
+      rows,
+      { savings: { kind: 'expr', expr: '>100' } },
+      extractors,
+    );
+    expect(out.map((r) => r.id)).toEqual(['b', 'c']);
+  });
+
+  test('multiple filters AND together', () => {
+    const out = applyColumnFilters<Row, Col>(
+      rows,
+      {
+        service: { kind: 'set', values: ['ec2'] },
+        savings: { kind: 'expr', expr: '>100' },
+      },
+      extractors,
+    );
+    expect(out.map((r) => r.id)).toEqual(['c']);
+  });
+
+  test('broken numeric expr is skipped (not treated as match-none)', () => {
+    const out = applyColumnFilters<Row, Col>(
+      rows,
+      { savings: { kind: 'expr', expr: '>>invalid' } },
+      extractors,
+    );
+    // Parse fails -> filter is skipped -> all rows pass
+    expect(out).toHaveLength(rows.length);
+  });
+});

frontend/src/lib/column-filters.ts

@@ -0,0 +1,127 @@
+/**
+ * Shared column-filter primitives (issue #166).
+ *
+ * Extracted from recommendations.ts so Plans, History, and RI Exchange
+ * can reuse the same numeric-filter parser without copy-pasting it.
+ *
+ * Each consuming tab owns its own column-id type and filter record (keeping
+ * column ids type-safe per tab), but the low-level parser and the generic
+ * apply pipeline live here.
+ */
+
+// ---------------------------------------------------------------------------
+// Numeric filter parser
+// ---------------------------------------------------------------------------
+
+export type ParsedNumericFilter =
+  | { ok: true; predicate: (n: number) => boolean }
+  | { ok: false; error: string };
+
+const MATCH_ALL: ParsedNumericFilter = { ok: true, predicate: () => true };
+
+/**
+ * Parse a numeric filter expression such as ">= 100", "< 50", "10..20",
+ * or a comma-separated OR list of those. Returns a predicate function on
+ * success, or an error object on parse failure so callers can surface inline
+ * validation messages.
+ *
+ * Supported syntax (case-insensitive, whitespace-tolerant):
+ *   - `>=N` / `<=N` / `>N` / `<N` — comparison
+ *   - `N..M` — inclusive range (order-independent)
+ *   - `N` — exact match
+ *   - Comma-separated terms are OR-combined
+ *   - Empty string / blank — match all
+ */
+export function parseNumericFilter(expr: string): ParsedNumericFilter {
+  if (!expr || expr.trim() === '') return MATCH_ALL;
+  const terms = expr.split(',').map((t) => t.trim()).filter((t) => t !== '');
+  if (terms.length === 0) return MATCH_ALL;
+
+  const predicates: Array<(n: number) => boolean> = [];
+  for (const term of terms) {
+    // Order matters: ">=" / "<=" must be checked before ">" / "<".
+    let p: ((n: number) => boolean) | null = null;
+    let m: RegExpMatchArray | null;
+    if ((m = term.match(/^>=\s*(-?\d+(?:\.\d+)?)$/))) {
+      const v = Number(m[1]);
+      p = (n) => n >= v;
+    } else if ((m = term.match(/^<=\s*(-?\d+(?:\.\d+)?)$/))) {
+      const v = Number(m[1]);
+      p = (n) => n <= v;
+    } else if ((m = term.match(/^>\s*(-?\d+(?:\.\d+)?)$/))) {
+      const v = Number(m[1]);
+      p = (n) => n > v;
+    } else if ((m = term.match(/^<\s*(-?\d+(?:\.\d+)?)$/))) {
+      const v = Number(m[1]);
+      p = (n) => n < v;
+    } else if ((m = term.match(/^(-?\d+(?:\.\d+)?)\s*\.\.\s*(-?\d+(?:\.\d+)?)$/))) {
+      const lo = Number(m[1]);
+      const hi = Number(m[2]);
+      const min = Math.min(lo, hi);
+      const max = Math.max(lo, hi);
+      p = (n) => n >= min && n <= max;
+    } else if ((m = term.match(/^(-?\d+(?:\.\d+)?)$/))) {
+      const v = Number(m[1]);
+      p = (n) => n === v;
+    }
+    if (p === null) {
+      return { ok: false, error: `Invalid filter term: "${term}"` };
+    }
+    predicates.push(p);
+  }
+  // OR across terms
+  return {
+    ok: true,
+    predicate: (n) => predicates.some((p) => p(n)),
+  };
+}
+
+// ---------------------------------------------------------------------------
+// Generic filter types
+// ---------------------------------------------------------------------------
+
+export type ColumnFilterKind =
+  | { kind: 'set'; values: string[] }
+  | { kind: 'expr'; expr: string };
+
+/**
+ * Apply a column-filter record to a list of rows. Each column maps to either
+ * a categorical set-filter or a numeric expression filter. All active filters
+ * are ANDed together.
+ *
+ * @param rows - The full list of rows to filter.
+ * @param filters - A partial record mapping column ids to their active filter.
+ * @param cellExtractors - Per-column functions that return the raw cell value
+ *   for a given row. Categorical columns return a string; numeric columns
+ *   return a number.
+ *
+ * Returns a new array containing only rows that pass all active filters.
+ * Broken numeric expressions (parse failure) are skipped so the UI can show
+ * an inline validation error without forcing the user to clear the field first.
+ */
+export function applyColumnFilters<TRow, TColumnId extends string>(
+  rows: readonly TRow[],
+  filters: Partial<Record<TColumnId, ColumnFilterKind>>,
+  cellExtractors: {
+    categorical: (row: TRow, col: TColumnId) => string;
+    numeric: (row: TRow, col: TColumnId) => number;
+  },
+): TRow[] {
+  const entries = Object.entries(filters) as Array<[TColumnId, ColumnFilterKind]>;
+  if (entries.length === 0) return [...rows];
+
+  return rows.filter((row) => {
+    for (const [col, filter] of entries) {
+      if (filter.kind === 'set') {
+        const cellValue = cellExtractors.categorical(row, col);
+        if (!filter.values.includes(cellValue)) return false;
+      } else {
+        const parsed = parseNumericFilter(filter.expr);
+        if (!parsed.ok) continue; // ignore broken expressions; UI shows the error
+        const cellNum = cellExtractors.numeric(row, col);
+        if (!parsed.predicate(cellNum)) return false;
+      }
+    }
+    return true;
+  });
+}

frontend/src/recommendations.ts

@@ -22,6 +22,11 @@ import type { RecommendationsResponse, LocalRecommendation, RecommendationsSumma
 import { openModal } from './modal';
 import { showSkeletonRows, teardownSkeleton } from './lib/skeleton';
 import { canAccess } from './permissions';
+import { parseNumericFilter, applyColumnFilters as applyColumnFiltersLib } from './lib/column-filters';
+// Re-export the shared primitives so existing consumers that import from
+// recommendations.ts keep working without import-path churn (issue #166).
+export { parseNumericFilter } from './lib/column-filters';
+export type { ParsedNumericFilter } from './lib/column-filters';
 
 // Module state for current purchase modal recommendations
 let currentPurchaseRecommendations: LocalRecommendation[] = [];
@@ -1305,101 +1310,38 @@ function sortIndicator(column: string, active: string, direction: 'asc' | 'desc'
 // via groupsInSortOrder() supersedes the flat-list sort. The same
 // SORTABLE_NUMERIC_COLUMNS / SORTABLE_STRING_COLUMNS maps are reused there.
 
-// Numeric filter expression parser. Grammar:
-//   - empty/whitespace      -> match-all
-//   - "42"                  -> equals
-//   - ">X" / "<X" / ">=X" / "<=X" -> comparator
-//   - "X..Y"                -> inclusive range (X and Y both numbers)
-//   - comma-separated       -> OR of any of the above
-// Returns a discriminated union so callers can render parse errors
-// inline without type-narrowing gymnastics. Whitespace inside terms is
-// trimmed; whitespace between terms is allowed.
-export type ParsedNumericFilter =
-  | { ok: true; predicate: (n: number) => boolean }
-  | { ok: false; error: string };
-
-const MATCH_ALL: ParsedNumericFilter = { ok: true, predicate: () => true };
-
-export function parseNumericFilter(expr: string): ParsedNumericFilter {
-  if (!expr || expr.trim() === '') return MATCH_ALL;
-  const terms = expr.split(',').map((t) => t.trim()).filter((t) => t !== '');
-  if (terms.length === 0) return MATCH_ALL;
-
-  const predicates: Array<(n: number) => boolean> = [];
-  for (const term of terms) {
-    // Order matters: ">=" / "<=" must be checked before ">" / "<".
-    let p: ((n: number) => boolean) | null = null;
-    let m: RegExpMatchArray | null;
-    if ((m = term.match(/^>=\s*(-?\d+(?:\.\d+)?)$/))) {
-      const v = Number(m[1]);
-      p = (n) => n >= v;
-    } else if ((m = term.match(/^<=\s*(-?\d+(?:\.\d+)?)$/))) {
-      const v = Number(m[1]);
-      p = (n) => n <= v;
-    } else if ((m = term.match(/^>\s*(-?\d+(?:\.\d+)?)$/))) {
-      const v = Number(m[1]);
-      p = (n) => n > v;
-    } else if ((m = term.match(/^<\s*(-?\d+(?:\.\d+)?)$/))) {
-      const v = Number(m[1]);
-      p = (n) => n < v;
-    } else if ((m = term.match(/^(-?\d+(?:\.\d+)?)\s*\.\.\s*(-?\d+(?:\.\d+)?)$/))) {
-      const lo = Number(m[1]);
-      const hi = Number(m[2]);
-      const min = Math.min(lo, hi);
-      const max = Math.max(lo, hi);
-      p = (n) => n >= min && n <= max;
-    } else if ((m = term.match(/^(-?\d+(?:\.\d+)?)$/))) {
-      const v = Number(m[1]);
-      p = (n) => n === v;
-    }
-    if (p === null) {
-      return { ok: false, error: `Invalid filter term: "${term}"` };
-    }
-    predicates.push(p);
-  }
-  // OR across terms
-  return {
-    ok: true,
-    predicate: (n) => predicates.some((p) => p(n)),
-  };
-}
+// parseNumericFilter and ParsedNumericFilter are now in lib/column-filters.ts
+// (issue #166 extraction). Imported at the top of this file and re-exported
+// so existing consumers that import from recommendations.ts keep working.
 
-// Apply the per-column filters to a rec list. ANDs all column filters
-// together. Categorical: row passes iff its column value (string-form,
-// empty/null mapped to "") is in `values`. Numeric: row passes iff
-// parseNumericFilter(expr).predicate accepts the value (skipped if
+// Apply the per-column filters to a rec list. Routes through the shared
+// generic applyColumnFilters from lib/column-filters (issue #166/#570).
+//
+// ANDs all column filters together. Categorical: row passes iff its column
+// value (string-form, empty/null mapped to "") is in `values`. Numeric: row
+// passes iff parseNumericFilter(expr).predicate accepts the value (skipped if
 // parse failed — the popover's inline error tells the user).
 //
 // Account uses cloud_account_id for matching; Term uses String(r.term).
 // All other categorical columns compare on the underlying string field.
+//
+// Issue #484: numeric predicates compare against the rounded display value so
+// exact-match ("123.45") works for rows whose raw value rounds to the typed
+// value, and ">N" / "<N" / "N..M" all behave consistently with what the user
+// sees in the cell.
 export function applyColumnFilters(
   recs: readonly LocalRecommendation[],
   filters: state.RecommendationsColumnFilters,
 ): LocalRecommendation[] {
-  const entries = Object.entries(filters) as Array<
-    [state.RecommendationsColumnId, state.RecommendationsColumnFilter]
-  >;
-  if (entries.length === 0) return [...recs];
-
-  return recs.filter((r) => {
-    for (const [col, f] of entries) {
-      if (f.kind === 'set') {
-        const cellRaw = categoricalCellValue(r, col);
-        if (!f.values.includes(cellRaw)) return false;
-      } else {
-        const parsed = parseNumericFilter(f.expr);
-        if (!parsed.ok) continue; // ignore broken expressions; UI shows the error
-        // Issue #484: compare against the rounded display value so exact-match
-        // ("123.45") works for rows whose raw value rounds to the typed value,
-        // and ">N" / "<N" / "N..M" all behave consistently with what the user
-        // sees in the cell. NaN passes through roundForDisplay unchanged.
-        const period = state.getCostPeriod();
-        const cellNum = roundForDisplay(numericCellValue(r, col), displayPrecision(col, period));
-        if (!parsed.predicate(cellNum)) return false;
-      }
-    }
-    return true;
-  });
+  const period = state.getCostPeriod();
+  return applyColumnFiltersLib<LocalRecommendation, state.RecommendationsColumnId>(
+    recs,
+    filters,
+    {
+      categorical: categoricalCellValue,
+      numeric: (r, col) => roundForDisplay(numericCellValue(r, col), displayPrecision(col, period)),
+    },
+  );
 }
 
 function categoricalCellValue(r: LocalRecommendation, col: state.RecommendationsColumnId): string {