From 3d78eaaf492caba5909f3267620848830f00b66b Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 11:47:08 +0100 Subject: [PATCH 01/14] feat(metrics): add filter parsing/validation scaffolding (FT-1966) --- src/core/metrics/filter.test.ts | 82 +++++++++++++++++++++++++ src/core/metrics/filter.ts | 102 ++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 src/core/metrics/filter.test.ts create mode 100644 src/core/metrics/filter.ts diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts new file mode 100644 index 0000000..365d0b9 --- /dev/null +++ b/src/core/metrics/filter.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from 'vitest'; +import { + parseMetricFilters, + validateMetricFilters, + hasActiveMetricFilters, +} from './filter.js'; + +describe('parseMetricFilters', () => { + it('splits comma-separated values and trims them', () => { + const f = parseMetricFilters({ + metricType: 'goal_count, goal_ratio', + goal: '1, purchase', + outlierMethod: 'quantile,stdev', + impactDirection: 'positive', + propertyFilterPath: 'page_name , order', + propertyFilterContains: ' BookingFullDetails ', + }); + expect(f.metricType).toEqual(['goal_count', 'goal_ratio']); + expect(f.goal).toEqual(['1', 'purchase']); + expect(f.outlierMethod).toEqual(['quantile', 'stdev']); + expect(f.impactDirection).toEqual(['positive']); + expect(f.propertyFilterPath).toEqual(['page_name', 'order']); + expect(f.propertyFilterContains).toBe('BookingFullDetails'); + }); + + it('returns undefined for absent / empty values', () => { + const f = parseMetricFilters({}); + expect(f.metricType).toBeUndefined(); + expect(f.goal).toBeUndefined(); + expect(f.outlierLimiting).toBeUndefined(); + expect(f.hasPropertyFilter).toBeUndefined(); + expect(f.cuped).toBeUndefined(); + expect(f.propertyFilterContains).toBeUndefined(); + }); + + it('reads tri-state booleans for outlier limiting and cuped', () => { + expect(parseMetricFilters({ outlierLimiting: true }).outlierLimiting).toBe(true); + expect(parseMetricFilters({ outlierLimiting: false }).outlierLimiting).toBe(false); + expect(parseMetricFilters({ cuped: true }).cuped).toBe(true); + expect(parseMetricFilters({ cuped: false }).cuped).toBe(false); + }); + + it('collapses the property-filter dest pair into a tri-state', () => { + // neither flag: commander leaves propertyFilter at its default true; we ignore it + expect(parseMetricFilters({ propertyFilter: true }).hasPropertyFilter).toBeUndefined(); + // --has-property-filter + expect(parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true }).hasPropertyFilter).toBe(true); + // --no-property-filter + expect(parseMetricFilters({ propertyFilter: false }).hasPropertyFilter).toBe(false); + }); +}); + +describe('hasActiveMetricFilters', () => { + it('is false when nothing is set', () => { + expect(hasActiveMetricFilters(parseMetricFilters({}))).toBe(false); + }); + it('is true when any filter is set, including false booleans', () => { + expect(hasActiveMetricFilters(parseMetricFilters({ metricType: 'goal_count' }))).toBe(true); + expect(hasActiveMetricFilters(parseMetricFilters({ outlierLimiting: false }))).toBe(true); + expect(hasActiveMetricFilters(parseMetricFilters({ propertyFilter: false }))).toBe(true); + expect(hasActiveMetricFilters(parseMetricFilters({ cuped: true }))).toBe(true); + }); +}); + +describe('validateMetricFilters', () => { + it('rejects unknown outlier methods', () => { + const opts = { outlierMethod: 'bogus' }; + expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).toThrow(/outlier.*method/i); + }); + it('rejects unknown impact directions', () => { + const opts = { impactDirection: 'sideways' }; + expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).toThrow(/impact.*direction/i); + }); + it('rejects passing both --has-property-filter and --no-property-filter', () => { + const opts = { hasPropertyFilter: true, propertyFilter: false }; + expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).toThrow(/property-filter/i); + }); + it('accepts valid values', () => { + const opts = { outlierMethod: 'quantile,stdev', impactDirection: 'positive,negative' }; + expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).not.toThrow(); + }); +}); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts new file mode 100644 index 0000000..4f44159 --- /dev/null +++ b/src/core/metrics/filter.ts @@ -0,0 +1,102 @@ +// Client-side filtering for `metrics list`. Every field referenced here is +// already present in the GET /metrics list response, so no extra API calls are +// needed. Server-side equivalents are tracked as a follow-up to FT-1966. + +export interface MetricFilters { + metricType?: string[] | undefined; + goal?: string[] | undefined; + outlierLimiting?: boolean | undefined; // tri-state: undefined = no filter + outlierMethod?: string[] | undefined; + hasPropertyFilter?: boolean | undefined; // tri-state + propertyFilterPath?: string[] | undefined; + propertyFilterContains?: string | undefined; + impactDirection?: string[] | undefined; + cuped?: boolean | undefined; // tri-state +} + +export const OUTLIER_METHODS = ['unlimited', 'quantile', 'stdev', 'fixed'] as const; +export const IMPACT_DIRECTIONS = ['positive', 'negative', 'unknown'] as const; + +function splitCsv(value: unknown): string[] | undefined { + if (typeof value !== 'string') return undefined; + const parts = value + .split(',') + .map((s) => s.trim()) + .filter((s) => s !== ''); + return parts.length > 0 ? parts : undefined; +} + +function trimmedString(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const t = value.trim(); + return t === '' ? undefined : t; +} + +export function parseMetricFilters(options: Record): MetricFilters { + // Property-filter presence is split across two commander dests because the + // requested flag names (--has-property-filter / --no-property-filter) do not + // share a base name. --no-property-filter declared alone defaults + // `propertyFilter` to true, so we only treat an explicit `=== false` as the + // "absent" filter and let --has-property-filter own the "present" filter. + const hasPropertyFilter = + options.hasPropertyFilter === true + ? true + : options.propertyFilter === false + ? false + : undefined; + + return { + metricType: splitCsv(options.metricType), + goal: splitCsv(options.goal), + outlierLimiting: typeof options.outlierLimiting === 'boolean' ? options.outlierLimiting : undefined, + outlierMethod: splitCsv(options.outlierMethod), + hasPropertyFilter, + propertyFilterPath: splitCsv(options.propertyFilterPath), + propertyFilterContains: trimmedString(options.propertyFilterContains), + impactDirection: splitCsv(options.impactDirection), + cuped: typeof options.cuped === 'boolean' ? options.cuped : undefined, + }; +} + +export function hasActiveMetricFilters(f: MetricFilters): boolean { + return ( + f.metricType !== undefined || + f.goal !== undefined || + f.outlierLimiting !== undefined || + f.outlierMethod !== undefined || + f.hasPropertyFilter !== undefined || + f.propertyFilterPath !== undefined || + f.propertyFilterContains !== undefined || + f.impactDirection !== undefined || + f.cuped !== undefined + ); +} + +export function validateMetricFilters( + options: Record, + filters: MetricFilters +): void { + if (options.hasPropertyFilter === true && options.propertyFilter === false) { + throw new Error('Cannot combine --has-property-filter with --no-property-filter.'); + } + + if (filters.outlierMethod) { + const allowed = OUTLIER_METHODS as readonly string[]; + const bad = filters.outlierMethod.filter((m) => !allowed.includes(m.toLowerCase())); + if (bad.length > 0) { + throw new Error( + `Invalid --outlier-method value(s): ${bad.join(', ')}. Must be one of: ${OUTLIER_METHODS.join(', ')}.` + ); + } + } + + if (filters.impactDirection) { + const allowed = IMPACT_DIRECTIONS as readonly string[]; + const bad = filters.impactDirection.filter((d) => !allowed.includes(d.toLowerCase())); + if (bad.length > 0) { + throw new Error( + `Invalid --impact-direction value(s): ${bad.join(', ')}. Must be one of: ${IMPACT_DIRECTIONS.join(', ')}.` + ); + } + } +} From 3d25fb6f387bceb721bcc6a0aca6800e6d80460f Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 11:52:24 +0100 Subject: [PATCH 02/14] feat(metrics): filter list by type, impact direction, goal (FT-1966) --- src/core/metrics/filter.test.ts | 67 +++++++++++++++++++++++++++++++++ src/core/metrics/filter.ts | 39 +++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index 365d0b9..0f884e9 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -3,6 +3,7 @@ import { parseMetricFilters, validateMetricFilters, hasActiveMetricFilters, + filterMetrics, } from './filter.js'; describe('parseMetricFilters', () => { @@ -80,3 +81,69 @@ describe('validateMetricFilters', () => { expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).not.toThrow(); }); }); + +type M = Record; +const metric = (over: M = {}): M => ({ + id: 1, + name: 'm', + type: 'goal_count', + effect: 'positive', + goal_id: 1, + goal: { id: 1, name: 'page_view' }, + denominator_goal_id: null, + denominator_goal: null, + outlier_limit_method: 'unlimited', + denominator_outlier_limit_method: null, + vr_lookback_interval: null, + denominator_vr_lookback_interval: null, + property_filter: null, + denominator_property_filter: null, + ...over, +}); + +describe('filterMetrics - type / impact / goal', () => { + it('returns all metrics when no filters are active', () => { + const data = [metric(), metric({ id: 2 })]; + expect(filterMetrics(data, parseMetricFilters({}))).toHaveLength(2); + }); + + it('filters by metric type (case-insensitive, OR within list)', () => { + const data = [ + metric({ id: 1, type: 'goal_count' }), + metric({ id: 2, type: 'goal_ratio' }), + metric({ id: 3, type: 'custom_sql' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ metricType: 'GOAL_COUNT,goal_ratio' })); + expect(out.map((m) => m.id)).toEqual([1, 2]); + }); + + it('filters by impact direction', () => { + const data = [ + metric({ id: 1, effect: 'positive' }), + metric({ id: 2, effect: 'negative' }), + metric({ id: 3, effect: 'unknown' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ impactDirection: 'negative,unknown' })); + expect(out.map((m) => m.id)).toEqual([2, 3]); + }); + + it('filters by goal numeric id (numerator or denominator)', () => { + const data = [ + metric({ id: 1, goal_id: 1 }), + metric({ id: 2, goal_id: 9, denominator_goal_id: 1 }), + metric({ id: 3, goal_id: 5 }), + ]; + const out = filterMetrics(data, parseMetricFilters({ goal: '1' })); + expect(out.map((m) => m.id)).toEqual([1, 2]); + }); + + it('filters by goal name substring (numerator or denominator)', () => { + const data = [ + metric({ id: 1, goal: { id: 1, name: 'page_view' } }), + metric({ id: 2, goal: { id: 2, name: 'checkout' }, denominator_goal: { id: 3, name: 'page_view_all' } }), + metric({ id: 3, goal: { id: 4, name: 'purchase' } }), + ]; + const out = filterMetrics(data, parseMetricFilters({ goal: 'page_view' })); + expect(out.map((m) => m.id)).toEqual([1, 2]); + }); +}); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index 4f44159..0676c83 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -100,3 +100,42 @@ export function validateMetricFilters( } } } + +type Metric = Record; + +const lc = (v: unknown): string => String(v ?? '').toLowerCase(); + +function byType(m: Metric, f: MetricFilters): boolean { + if (!f.metricType) return true; + const want = f.metricType.map((t) => t.toLowerCase()); + return want.includes(lc(m.type)); +} + +function byImpactDirection(m: Metric, f: MetricFilters): boolean { + if (!f.impactDirection) return true; + const want = f.impactDirection.map((d) => d.toLowerCase()); + return want.includes(lc(m.effect)); +} + +function byGoal(m: Metric, f: MetricFilters): boolean { + if (!f.goal) return true; + const goal = m.goal as Metric | null | undefined; + const denomGoal = m.denominator_goal as Metric | null | undefined; + const ids = [m.goal_id, m.denominator_goal_id] + .filter((v) => v !== null && v !== undefined) + .map((v) => String(v)); + const names = [goal?.name, denomGoal?.name] + .filter((v): v is string => typeof v === 'string') + .map((n) => n.toLowerCase()); + return f.goal.some((token) => { + if (/^\d+$/.test(token)) return ids.includes(token); + const t = token.toLowerCase(); + return names.some((n) => n.includes(t)); + }); +} + +const PREDICATES: Array<(m: Metric, f: MetricFilters) => boolean> = [byType, byImpactDirection, byGoal]; + +export function filterMetrics(metrics: Metric[], f: MetricFilters): Metric[] { + return metrics.filter((m) => PREDICATES.every((p) => p(m, f))); +} From 8037530735322ed2d883a2cc23e0647baf98625f Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 11:55:40 +0100 Subject: [PATCH 03/14] feat(metrics): filter list by outlier limiting/method and CUPED (FT-1966) --- src/core/metrics/filter.test.ts | 51 +++++++++++++++++++++++++++++++++ src/core/metrics/filter.ts | 44 +++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index 0f884e9..0ca24fc 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -147,3 +147,54 @@ describe('filterMetrics - type / impact / goal', () => { expect(out.map((m) => m.id)).toEqual([1, 2]); }); }); + +describe('filterMetrics - outlier / cuped', () => { + it('--outlier-limiting keeps only metrics with limiting (numerator or denominator)', () => { + const data = [ + metric({ id: 1, outlier_limit_method: 'unlimited' }), + metric({ id: 2, outlier_limit_method: 'quantile' }), + metric({ id: 3, outlier_limit_method: 'unlimited', denominator_outlier_limit_method: 'stdev' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ outlierLimiting: true })); + expect(out.map((m) => m.id)).toEqual([2, 3]); + }); + + it('--no-outlier-limiting keeps only metrics without limiting', () => { + const data = [ + metric({ id: 1, outlier_limit_method: 'unlimited' }), + metric({ id: 2, outlier_limit_method: 'fixed' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ outlierLimiting: false })); + expect(out.map((m) => m.id)).toEqual([1]); + }); + + it('filters by outlier method (numerator or denominator, OR within list)', () => { + const data = [ + metric({ id: 1, outlier_limit_method: 'quantile' }), + metric({ id: 2, outlier_limit_method: 'unlimited', denominator_outlier_limit_method: 'fixed' }), + metric({ id: 3, outlier_limit_method: 'stdev' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ outlierMethod: 'quantile,fixed' })); + expect(out.map((m) => m.id)).toEqual([1, 2]); + }); + + it('--cuped keeps only metrics with a lookback interval', () => { + const data = [ + metric({ id: 1, vr_lookback_interval: null }), + metric({ id: 2, vr_lookback_interval: '2w' }), + metric({ id: 3, vr_lookback_interval: null, denominator_vr_lookback_interval: '1w' }), + ]; + const out = filterMetrics(data, parseMetricFilters({ cuped: true })); + expect(out.map((m) => m.id)).toEqual([2, 3]); + }); + + it('--no-cuped keeps only metrics without a lookback interval', () => { + const data = [ + metric({ id: 1, vr_lookback_interval: '4w' }), + metric({ id: 2, vr_lookback_interval: '' }), + metric({ id: 3, vr_lookback_interval: null }), + ]; + const out = filterMetrics(data, parseMetricFilters({ cuped: false })); + expect(out.map((m) => m.id)).toEqual([2, 3]); + }); +}); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index 0676c83..23dd183 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -134,7 +134,49 @@ function byGoal(m: Metric, f: MetricFilters): boolean { }); } -const PREDICATES: Array<(m: Metric, f: MetricFilters) => boolean> = [byType, byImpactDirection, byGoal]; +function isLimited(method: unknown): boolean { + const m = lc(method); + return m !== '' && m !== 'unlimited'; +} + +function metricHasOutlierLimiting(m: Metric): boolean { + return isLimited(m.outlier_limit_method) || isLimited(m.denominator_outlier_limit_method); +} + +function byOutlierLimiting(m: Metric, f: MetricFilters): boolean { + if (f.outlierLimiting === undefined) return true; + return metricHasOutlierLimiting(m) === f.outlierLimiting; +} + +function byOutlierMethod(m: Metric, f: MetricFilters): boolean { + if (!f.outlierMethod) return true; + const want = f.outlierMethod.map((x) => x.toLowerCase()); + const num = lc(m.outlier_limit_method); + const den = lc(m.denominator_outlier_limit_method); + return (num !== '' && want.includes(num)) || (den !== '' && want.includes(den)); +} + +function nonEmpty(value: unknown): boolean { + return value !== null && value !== undefined && String(value).trim() !== ''; +} + +function metricHasCuped(m: Metric): boolean { + return nonEmpty(m.vr_lookback_interval) || nonEmpty(m.denominator_vr_lookback_interval); +} + +function byCuped(m: Metric, f: MetricFilters): boolean { + if (f.cuped === undefined) return true; + return metricHasCuped(m) === f.cuped; +} + +const PREDICATES: Array<(m: Metric, f: MetricFilters) => boolean> = [ + byType, + byImpactDirection, + byGoal, + byOutlierLimiting, + byOutlierMethod, + byCuped, +]; export function filterMetrics(metrics: Metric[], f: MetricFilters): Metric[] { return metrics.filter((m) => PREDICATES.every((p) => p(m, f))); From c3fb3fc4a14211c04303422cf8a5c8dfac38ac92 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 11:59:01 +0100 Subject: [PATCH 04/14] feat(metrics): filter list by property filter presence/path/contents (FT-1966) --- src/core/metrics/filter.test.ts | 53 ++++++++++++++++++ src/core/metrics/filter.ts | 98 +++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index 0ca24fc..050563b 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -198,3 +198,56 @@ describe('filterMetrics - outlier / cuped', () => { expect(out.map((m) => m.id)).toEqual([2, 3]); }); }); + +const PF = JSON.stringify({ + filter: { and: [{ in: [{ value: ['BookingFullDetails'] }, { var: { path: 'page_name' } }] }] }, +}); +const PF2 = JSON.stringify({ filter: { and: [{ '==': [{ var: { path: 'currency' } }, 'USD'] }] } }); + +describe('filterMetrics - property filter', () => { + it('--has-property-filter keeps only metrics with a non-empty filter', () => { + const data = [ + metric({ id: 1, property_filter: null }), + metric({ id: 2, property_filter: '' }), + metric({ id: 3, property_filter: '{}' }), + metric({ id: 4, property_filter: PF }), + metric({ id: 5, property_filter: null, denominator_property_filter: PF2 }), + ]; + const out = filterMetrics(data, parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true })); + expect(out.map((m) => m.id)).toEqual([4, 5]); + }); + + it('--no-property-filter keeps only metrics without a filter', () => { + const data = [ + metric({ id: 1, property_filter: null }), + metric({ id: 2, property_filter: PF }), + ]; + const out = filterMetrics(data, parseMetricFilters({ propertyFilter: false })); + expect(out.map((m) => m.id)).toEqual([1]); + }); + + it('--property-filter-path matches var.path substrings (numerator or denominator)', () => { + const data = [ + metric({ id: 1, property_filter: PF }), // page_name + metric({ id: 2, property_filter: null, denominator_property_filter: PF2 }), // currency + metric({ id: 3, property_filter: null }), + ]; + const out = filterMetrics(data, parseMetricFilters({ propertyFilterPath: 'PAGE,currency' })); + expect(out.map((m) => m.id)).toEqual([1, 2]); + }); + + it('--property-filter-contains matches anywhere in the serialized filter (incl. values)', () => { + const data = [ + metric({ id: 1, property_filter: PF }), // contains BookingFullDetails + metric({ id: 2, property_filter: PF2 }), // contains USD + ]; + const out = filterMetrics(data, parseMetricFilters({ propertyFilterContains: 'bookingfull' })); + expect(out.map((m) => m.id)).toEqual([1]); + }); + + it('handles property_filter supplied as an object, not just a string', () => { + const data = [metric({ id: 1, property_filter: { filter: { var: { path: 'amount' } } } })]; + const out = filterMetrics(data, parseMetricFilters({ propertyFilterPath: 'amount' })); + expect(out.map((m) => m.id)).toEqual([1]); + }); +}); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index 23dd183..6d2fafa 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -169,6 +169,101 @@ function byCuped(m: Metric, f: MetricFilters): boolean { return metricHasCuped(m) === f.cuped; } +function propertyFilterText(raw: unknown): string { + if (raw === null || raw === undefined) return ''; + if (typeof raw === 'string') return raw; + try { + return JSON.stringify(raw); + } catch { + return ''; + } +} + +function hasMeaningfulPropertyFilter(raw: unknown): boolean { + const t = propertyFilterText(raw).trim(); + if (t === '' || t === 'null' || t === '{}') return false; + // A wrapper like {"filter":{}} or {"filter":{"and":[]}} counts as empty. + try { + const parsed = JSON.parse(t) as unknown; + const inner = (parsed as Record | null)?.filter; + if (inner !== undefined) { + const innerText = JSON.stringify(inner); + return innerText !== '{}' && innerText !== '[]' && innerText !== 'null'; + } + } catch { + // not JSON — treat the raw non-empty string as a present filter + } + return true; +} + +function metricHasPropertyFilter(m: Metric): boolean { + return ( + hasMeaningfulPropertyFilter(m.property_filter) || + hasMeaningfulPropertyFilter(m.denominator_property_filter) + ); +} + +function collectVarPaths(node: unknown, out: string[]): void { + if (Array.isArray(node)) { + for (const child of node) collectVarPaths(child, out); + return; + } + if (node !== null && typeof node === 'object') { + for (const [key, value] of Object.entries(node)) { + if (key === 'var') { + if (typeof value === 'string') out.push(value); + else if ( + value !== null && + typeof value === 'object' && + typeof (value as Record).path === 'string' + ) { + out.push((value as Record).path as string); + } + } + collectVarPaths(value, out); + } + } +} + +function extractPropertyFilterPaths(raw: unknown): string[] { + const text = propertyFilterText(raw); + if (text === '') return []; + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return []; + } + const out: string[] = []; + collectVarPaths(parsed, out); + return out; +} + +function byPropertyFilterPresence(m: Metric, f: MetricFilters): boolean { + if (f.hasPropertyFilter === undefined) return true; + return metricHasPropertyFilter(m) === f.hasPropertyFilter; +} + +function byPropertyFilterPath(m: Metric, f: MetricFilters): boolean { + if (!f.propertyFilterPath) return true; + const paths = [ + ...extractPropertyFilterPaths(m.property_filter), + ...extractPropertyFilterPaths(m.denominator_property_filter), + ].map((p) => p.toLowerCase()); + const want = f.propertyFilterPath.map((t) => t.toLowerCase()); + return want.some((t) => paths.some((p) => p.includes(t))); +} + +function byPropertyFilterContains(m: Metric, f: MetricFilters): boolean { + if (!f.propertyFilterContains) return true; + const hay = ( + propertyFilterText(m.property_filter) + + ' ' + + propertyFilterText(m.denominator_property_filter) + ).toLowerCase(); + return hay.includes(f.propertyFilterContains.toLowerCase()); +} + const PREDICATES: Array<(m: Metric, f: MetricFilters) => boolean> = [ byType, byImpactDirection, @@ -176,6 +271,9 @@ const PREDICATES: Array<(m: Metric, f: MetricFilters) => boolean> = [ byOutlierLimiting, byOutlierMethod, byCuped, + byPropertyFilterPresence, + byPropertyFilterPath, + byPropertyFilterContains, ]; export function filterMetrics(metrics: Metric[], f: MetricFilters): Metric[] { From 6e58620bcfbc291430fdd98692b3ba74823502b0 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:03:03 +0100 Subject: [PATCH 05/14] fix(metrics): treat empty property-filter wrappers as no filter (FT-1966) --- src/core/metrics/filter.test.ts | 10 ++++++++++ src/core/metrics/filter.ts | 30 ++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index 050563b..e63d73a 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -250,4 +250,14 @@ describe('filterMetrics - property filter', () => { const out = filterMetrics(data, parseMetricFilters({ propertyFilterPath: 'amount' })); expect(out.map((m) => m.id)).toEqual([1]); }); + + it('treats an empty filter wrapper (empty and/or array) as no property filter', () => { + const data = [ + metric({ id: 1, property_filter: JSON.stringify({ filter: { and: [] } }) }), + metric({ id: 2, property_filter: JSON.stringify({ filter: { or: [] } }) }), + metric({ id: 3, property_filter: PF }), + ]; + const out = filterMetrics(data, parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true })); + expect(out.map((m) => m.id)).toEqual([3]); + }); }); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index 6d2fafa..15ebe02 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -179,21 +179,31 @@ function propertyFilterText(raw: unknown): string { } } +function hasLeafContent(node: unknown): boolean { + if (node === null || node === undefined) return false; + if (Array.isArray(node)) return node.some(hasLeafContent); + if (typeof node === 'object') return Object.values(node).some(hasLeafContent); + return true; // a primitive leaf (string/number/boolean) = real content +} + function hasMeaningfulPropertyFilter(raw: unknown): boolean { const t = propertyFilterText(raw).trim(); - if (t === '' || t === 'null' || t === '{}') return false; - // A wrapper like {"filter":{}} or {"filter":{"and":[]}} counts as empty. + if (t === '' || t === 'null') return false; + let parsed: unknown; try { - const parsed = JSON.parse(t) as unknown; - const inner = (parsed as Record | null)?.filter; - if (inner !== undefined) { - const innerText = JSON.stringify(inner); - return innerText !== '{}' && innerText !== '[]' && innerText !== 'null'; - } + parsed = JSON.parse(t); } catch { - // not JSON — treat the raw non-empty string as a present filter + // Not JSON but a non-empty string — treat the raw value as a present filter. + return true; } - return true; + // Unwrap the {"filter": ...} envelope when present, then check for any leaf. + const inner = + parsed !== null && + typeof parsed === 'object' && + 'filter' in (parsed as Record) + ? (parsed as Record).filter + : parsed; + return hasLeafContent(inner); } function metricHasPropertyFilter(m: Metric): boolean { From db2cef517fe36fbf4972d7665ad33f800a0e56aa Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:06:02 +0100 Subject: [PATCH 06/14] fix(metrics): do not treat bare-array property_filter as present (FT-1966) --- src/core/metrics/filter.test.ts | 2 ++ src/core/metrics/filter.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index e63d73a..059458e 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -256,6 +256,8 @@ describe('filterMetrics - property filter', () => { metric({ id: 1, property_filter: JSON.stringify({ filter: { and: [] } }) }), metric({ id: 2, property_filter: JSON.stringify({ filter: { or: [] } }) }), metric({ id: 3, property_filter: PF }), + metric({ id: 4, property_filter: '[]' }), + metric({ id: 5, property_filter: '[{}]' }), ]; const out = filterMetrics(data, parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true })); expect(out.map((m) => m.id)).toEqual([3]); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index 15ebe02..eb050f1 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -200,6 +200,7 @@ function hasMeaningfulPropertyFilter(raw: unknown): boolean { const inner = parsed !== null && typeof parsed === 'object' && + !Array.isArray(parsed) && 'filter' in (parsed as Record) ? (parsed as Record).filter : parsed; From 538203bf6c76ee49c1c179fa9fcbf14d06e2485d Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:07:41 +0100 Subject: [PATCH 07/14] feat(metrics): add listAllMetrics to page through every metric (FT-1966) --- src/core/metrics/list.ts | 42 +++++++++++++++++++++++++++++ src/core/metrics/metrics.test.ts | 45 +++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/core/metrics/list.ts b/src/core/metrics/list.ts index e199d56..1e1a251 100644 --- a/src/core/metrics/list.ts +++ b/src/core/metrics/list.ts @@ -44,3 +44,45 @@ export async function listMetrics( pagination: { page: params.page, items: params.items, hasMore: data.length >= params.items }, }; } + +const FETCH_ALL_PAGE_SIZE = 200; +const FETCH_ALL_MAX_PAGES = 100; + +/** + * Fetch every metric across all pages, resolving owners/teams a single time. + * Used by `metrics list` when client-side filters are active so filtering sees + * the full set rather than a single page. + */ +export async function listAllMetrics( + client: APIClient, + params: ListMetricsParams, + pageSize: number = FETCH_ALL_PAGE_SIZE +): Promise> { + const ownerIds = params.owners ? await resolveOwnerIds(client, params.owners) : undefined; + const teamIds = params.teams ? await resolveTeamIds(client, params.teams) : undefined; + + const all: unknown[] = []; + let page = 1; + for (; page <= FETCH_ALL_MAX_PAGES; page++) { + const data = await client.listMetrics({ + items: pageSize, + page, + archived: params.archived, + include_drafts: params.include_drafts, + search: params.search, + sort: params.sort, + sort_asc: params.sortAsc, + ids: params.ids, + owners: ownerIds, + teams: teamIds, + review_status: params.reviewStatus, + }); + all.push(...data); + if (data.length < pageSize) break; + } + + return { + data: all, + pagination: { page: 1, items: all.length, hasMore: false }, + }; +} diff --git a/src/core/metrics/metrics.test.ts b/src/core/metrics/metrics.test.ts index ae4bf05..b1fbb45 100644 --- a/src/core/metrics/metrics.test.ts +++ b/src/core/metrics/metrics.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { listMetrics, resolveOwnerIds, resolveTeamIds } from './list.js'; +import { listMetrics, listAllMetrics, resolveOwnerIds, resolveTeamIds } from './list.js'; import { getMetric } from './get.js'; import { createMetric } from './create.js'; import { updateMetric } from './update.js'; @@ -811,3 +811,46 @@ describe('validateMetricFields', () => { ).toThrow(/--value-source-property is required/); }); }); + +describe('listAllMetrics', () => { + it('pages through results until a short page is returned', async () => { + const client = { + listMetrics: vi.fn(), + } as any; + // pageSize 2: page1 full (2), page2 full (2), page3 short (1) -> stop + client.listMetrics + .mockResolvedValueOnce([{ id: 1 }, { id: 2 }]) + .mockResolvedValueOnce([{ id: 3 }, { id: 4 }]) + .mockResolvedValueOnce([{ id: 5 }]); + + const result = await listAllMetrics(client, { items: 100, page: 1 }, 2); + + expect(result.data.map((m: any) => m.id)).toEqual([1, 2, 3, 4, 5]); + expect(client.listMetrics).toHaveBeenCalledTimes(3); + expect(client.listMetrics).toHaveBeenNthCalledWith(1, expect.objectContaining({ items: 2, page: 1 })); + expect(client.listMetrics).toHaveBeenNthCalledWith(3, expect.objectContaining({ items: 2, page: 3 })); + }); + + it('stops after a single page when fewer than pageSize are returned', async () => { + const client = { listMetrics: vi.fn().mockResolvedValue([{ id: 1 }]) } as any; + const result = await listAllMetrics(client, { items: 100, page: 1 }, 2); + expect(result.data).toHaveLength(1); + expect(client.listMetrics).toHaveBeenCalledTimes(1); + }); + + it('resolves owners and teams once, then forwards their IDs on every page', async () => { + const client = { + listMetrics: vi.fn().mockResolvedValue([]), + resolveUsers: vi.fn().mockResolvedValue([{ id: 10, email: 'jane@example.com' }]), + resolveTeams: vi.fn().mockResolvedValue([{ id: 5, name: 'Growth' }]), + } as any; + + await listAllMetrics(client, { items: 100, page: 1, owners: 'jane@example.com', teams: 'Growth' }, 2); + + expect(client.resolveUsers).toHaveBeenCalledTimes(1); + expect(client.resolveTeams).toHaveBeenCalledTimes(1); + expect(client.listMetrics).toHaveBeenCalledWith( + expect.objectContaining({ owners: '10', teams: '5' }) + ); + }); +}); From 8922867a57a664b00d709351fe9934f5169c159c Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:13:03 +0100 Subject: [PATCH 08/14] feat(list): add isClientFiltered footer hook to createListCommand (FT-1966) --- src/lib/utils/list-command.test.ts | 27 +++++++++++++++++++++++++++ src/lib/utils/list-command.ts | 24 +++++++++++++++++------- src/lib/utils/pagination.ts | 5 +++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/lib/utils/list-command.test.ts b/src/lib/utils/list-command.test.ts index 7526e16..9626b81 100644 --- a/src/lib/utils/list-command.test.ts +++ b/src/lib/utils/list-command.test.ts @@ -3,6 +3,7 @@ import { createListCommand } from './list-command.js'; import { getAPIClientFromOptions, getGlobalOptions, printFormatted } from './api-helper.js'; import { resetCommand } from '../../test/helpers/command-reset.js'; import { setTTYOverride } from './stdin.js'; +import { printPaginationFooter } from './pagination.js'; vi.mock('./api-helper.js', async (importOriginal) => { const actual = await importOriginal(); @@ -70,6 +71,32 @@ describe('createListCommand', () => { expect(printFormatted).toHaveBeenCalled(); }); + it('prints a filtered total footer (not the pagination footer) when isClientFiltered returns true', async () => { + const fetch = vi.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]); + const cmd = createListCommand({ + description: 'List items', + fetch, + isClientFiltered: () => true, + }); + resetCommand(cmd); + + await cmd.parseAsync(['node', 'test']); + + const output = consoleSpy.mock.calls.flat().join('\n'); + expect(output).toContain('(filtered)'); + expect(printPaginationFooter).not.toHaveBeenCalled(); + }); + + it('uses the pagination footer when isClientFiltered is absent', async () => { + const fetch = vi.fn().mockResolvedValue([{ id: 1 }]); + const cmd = createListCommand({ description: 'List items', fetch }); + resetCommand(cmd); + + await cmd.parseAsync(['node', 'test']); + + expect(printPaginationFooter).toHaveBeenCalled(); + }); + describe('output behavior under piping', () => { afterEach(() => { setTTYOverride({ stdin: true, stdout: true }); diff --git a/src/lib/utils/list-command.ts b/src/lib/utils/list-command.ts index 2d20e85..5f9971e 100644 --- a/src/lib/utils/list-command.ts +++ b/src/lib/utils/list-command.ts @@ -6,7 +6,7 @@ import { shouldOutputIdsOnly, withErrorHandling, } from './api-helper.js'; -import { addPaginationOptions, printPaginationFooter } from './pagination.js'; +import { addPaginationOptions, printPaginationFooter, printFilteredFooter } from './pagination.js'; import { applyShowExclude } from '../../api-client/entity-summary.js'; import type { APIClient } from '../../api-client/api-client.js'; @@ -16,6 +16,12 @@ export interface ListCommandOptions { fetch: (client: APIClient, options: Record) => Promise; summarizeRow?: (item: Record) => Record; extraOptions?: (cmd: Command) => Command; + /** + * When this returns true for the parsed options, `fetch` is expected to have + * already returned the COMPLETE client-filtered result set, so a total-count + * footer is printed instead of the page-based pagination footer. + */ + isClientFiltered?: (options: Record) => boolean; } export function createListCommand(opts: ListCommandOptions): Command { @@ -65,12 +71,16 @@ export function createListCommand(opts: ListCommandOptions): Command { } printFormatted(data, globalOptions); - printPaginationFooter( - items.length, - options.items, - options.page, - globalOptions.output as string - ); + if (opts.isClientFiltered?.(options)) { + printFilteredFooter(items.length, globalOptions.output as string); + } else { + printPaginationFooter( + items.length, + options.items, + options.page, + globalOptions.output as string + ); + } }) ); diff --git a/src/lib/utils/pagination.ts b/src/lib/utils/pagination.ts index d67be86..bbbfd40 100644 --- a/src/lib/utils/pagination.ts +++ b/src/lib/utils/pagination.ts @@ -20,3 +20,8 @@ export function printPaginationFooter( : `Page ${page} (${count} results).`; console.log(chalk.gray(footer)); } + +export function printFilteredFooter(count: number, outputFormat?: string): void { + if (outputFormat === 'json' || outputFormat === 'yaml') return; + console.log(chalk.gray(`${count} result${count === 1 ? '' : 's'} (filtered).`)); +} From fdb3cb5a317375e43de05366c2c9b979ad28c062 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:18:41 +0100 Subject: [PATCH 09/14] feat(metrics): wire client-side filters into metrics list (FT-1966) --- src/commands/metrics/index.ts | 57 ++++++++++++++++++++++++++-- src/commands/metrics/metrics.test.ts | 55 +++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 4 deletions(-) diff --git a/src/commands/metrics/index.ts b/src/commands/metrics/index.ts index e599f71..b0ee382 100644 --- a/src/commands/metrics/index.ts +++ b/src/commands/metrics/index.ts @@ -14,7 +14,13 @@ import { summarizeMetricRow, } from '../../api-client/entity-summary.js'; import { createListCommand } from '../../lib/utils/list-command.js'; -import { listMetrics as coreListMetrics } from '../../core/metrics/list.js'; +import { listMetrics as coreListMetrics, listAllMetrics } from '../../core/metrics/list.js'; +import { + parseMetricFilters, + validateMetricFilters, + hasActiveMetricFilters, + filterMetrics, +} from '../../core/metrics/filter.js'; import { getMetric } from '../../core/metrics/get.js'; import { createMetric } from '../../core/metrics/create.js'; import { updateMetric } from '../../core/metrics/update.js'; @@ -33,7 +39,7 @@ const listCommand = createListCommand({ description: 'List all metrics', defaultItems: 100, fetch: async (client, options) => { - const result = await coreListMetrics(client, { + const baseParams = { items: options.items as number, page: options.page as number, archived: options.archived as boolean, @@ -45,16 +51,59 @@ const listCommand = createListCommand({ owners: options.owners as string | undefined, teams: options.teams as string | undefined, reviewStatus: options.reviewStatus as string | undefined, - }); + }; + + const filters = parseMetricFilters(options); + if (hasActiveMetricFilters(filters)) { + validateMetricFilters(options, filters); + // Client-side filters need the full set, not a single page. + const all = await listAllMetrics(client, baseParams); + return filterMetrics(all.data as Array>, filters); + } + + const result = await coreListMetrics(client, baseParams); return result.data; }, summarizeRow: summarizeMetricRow, + isClientFiltered: (options) => hasActiveMetricFilters(parseMetricFilters(options)), extraOptions: (cmd) => cmd .option('--include-drafts', 'include draft (non-activated) metrics') .option('--owners ', 'filter by owners (comma-separated IDs, names, or emails)') .option('--teams ', 'filter by teams (comma-separated IDs or names)') - .option('--review-status ', 'filter by review status (pending, approved, none)'), + .option('--review-status ', 'filter by review status (pending, approved, none)') + // Client-side filters (applied across all pages). API-side equivalents are a follow-up. + .option( + '--metric-type ', + 'filter by metric type(s), comma-separated (goal_count, goal_unique_count, goal_time_to_achievement, goal_property, goal_property_unique_count, goal_ratio, goal_retention, goal_activity_period_count, custom_sql)' + ) + .option( + '--goal ', + 'filter by goal name or ID, comma-separated (matches numerator and denominator goals)' + ) + // positive flag declared before negative so the boolean stays tri-state + .option('--outlier-limiting', 'only metrics that have outlier limiting (method other than unlimited)') + .option('--no-outlier-limiting', 'only metrics without outlier limiting') + .option( + '--outlier-method ', + 'filter by outlier limit method(s), comma-separated (unlimited, quantile, stdev, fixed)' + ) + .option('--has-property-filter', 'only metrics that have a goal property filter') + .option('--no-property-filter', 'only metrics without a goal property filter') + .option( + '--property-filter-path ', + 'filter by property path(s) referenced in the property filter, comma-separated (substring match)' + ) + .option( + '--property-filter-contains ', + 'filter by a substring anywhere in the serialized property filter' + ) + .option( + '--impact-direction ', + 'filter by impact direction(s), comma-separated (positive, negative, unknown)' + ) + .option('--cuped', 'only metrics with CUPED (variance reduction) enabled') + .option('--no-cuped', 'only metrics without CUPED'), }); const getCommand = new Command('get') diff --git a/src/commands/metrics/metrics.test.ts b/src/commands/metrics/metrics.test.ts index 90d6474..9a02c39 100644 --- a/src/commands/metrics/metrics.test.ts +++ b/src/commands/metrics/metrics.test.ts @@ -201,6 +201,61 @@ describe('metrics command', () => { expect(printFormatted).toHaveBeenCalled(); }); + const richMetrics = [ + { id: 1, name: 'a', type: 'goal_count', effect: 'positive', goal_id: 1, goal: { id: 1, name: 'page_view' }, outlier_limit_method: 'unlimited', vr_lookback_interval: null, property_filter: null }, + { id: 2, name: 'b', type: 'goal_ratio', effect: 'negative', goal_id: 2, goal: { id: 2, name: 'checkout' }, outlier_limit_method: 'quantile', vr_lookback_interval: '2w', property_filter: '{"filter":{"and":[{"var":{"path":"page_name"}}]}}' }, + { id: 3, name: 'c', type: 'custom_sql', effect: 'unknown', goal_id: 3, goal: { id: 3, name: 'purchase' }, outlier_limit_method: 'unlimited', vr_lookback_interval: null, property_filter: null }, + ]; + + it('filters metrics list by --metric-type (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--metric-type', 'goal_ratio,custom_sql']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([2, 3]); + }); + + it('filters metrics list by --cuped (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--cuped']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([2]); + }); + + it('filters metrics list by --no-cuped (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--no-cuped']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([1, 3]); + }); + + it('filters metrics list by --goal name (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--goal', 'checkout']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([2]); + }); + + it('rejects an invalid --impact-direction value', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + try { + await metricsCommand.parseAsync(['node', 'test', 'list', '--impact-direction', 'sideways']); + throw new Error('Should have thrown'); + } catch (error) { + if (!(error as Error).message.startsWith('process.exit')) throw error; + const errorOutput = consoleErrorSpy.mock.calls.flat().join(' '); + expect(errorOutput).toContain('impact-direction'); + } + }); + + it('does not client-filter (single page) when no new filters are passed', async () => { + await metricsCommand.parseAsync(['node', 'test', 'list']); + expect(mockClient.listMetrics).toHaveBeenCalledWith(defaultListParams); + }); + it('should get metric by id', async () => { await metricsCommand.parseAsync(['node', 'test', 'get', '1']); From 51299e52afe3ec699279574215626e1a46f4638f Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:22:57 +0100 Subject: [PATCH 10/14] test(metrics): cover --has/--no-property-filter wiring and conflict (FT-1966) --- src/commands/metrics/metrics.test.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/commands/metrics/metrics.test.ts b/src/commands/metrics/metrics.test.ts index 9a02c39..dc8ef74 100644 --- a/src/commands/metrics/metrics.test.ts +++ b/src/commands/metrics/metrics.test.ts @@ -231,6 +231,34 @@ describe('metrics command', () => { expect(printed.map((m) => m.id)).toEqual([1, 3]); }); + it('filters metrics list by --has-property-filter (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--has-property-filter']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([2]); + }); + + it('filters metrics list by --no-property-filter (client-side)', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + await metricsCommand.parseAsync(['node', 'test', 'list', '--no-property-filter']); + + const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; + expect(printed.map((m) => m.id)).toEqual([1, 3]); + }); + + it('rejects combining --has-property-filter with --no-property-filter', async () => { + mockClient.listMetrics.mockResolvedValue(richMetrics); + try { + await metricsCommand.parseAsync(['node', 'test', 'list', '--has-property-filter', '--no-property-filter']); + throw new Error('Should have thrown'); + } catch (error) { + if (!(error as Error).message.startsWith('process.exit')) throw error; + const errorOutput = consoleErrorSpy.mock.calls.flat().join(' '); + expect(errorOutput).toContain('property-filter'); + } + }); + it('filters metrics list by --goal name (client-side)', async () => { mockClient.listMetrics.mockResolvedValue(richMetrics); await metricsCommand.parseAsync(['node', 'test', 'list', '--goal', 'checkout']); From b2806fc7de22eefccd33f32328e715f1be4629aa Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:26:57 +0100 Subject: [PATCH 11/14] docs(metrics): document client-side list filters (FT-1966) --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index 6d46a28..84b5ffb 100644 --- a/README.md +++ b/README.md @@ -929,6 +929,17 @@ abs metrics list --owners "jane@example.com" --teams Growth # by name/email abs metrics list --review-status pending abs metrics list --sort name --asc abs metrics list --ids 1,2,3 + +# Client-side filters (applied across all pages) +abs metrics list --metric-type goal_ratio,custom_sql +abs metrics list --goal purchase # name or ID, incl. denominator +abs metrics list --outlier-limiting # has limiting; --no-outlier-limiting for none +abs metrics list --outlier-method quantile,stdev +abs metrics list --has-property-filter # --no-property-filter for none +abs metrics list --property-filter-path page_name +abs metrics list --property-filter-contains BookingFullDetails +abs metrics list --impact-direction positive,negative +abs metrics list --cuped # --no-cuped for none abs metrics get 123 abs metrics create --name "Revenue" --type count abs metrics update 123 --description "Updated" @@ -953,6 +964,22 @@ abs metrics access grant-user 123 --user 1 --role 2 abs metrics access revoke-user 123 --user 1 --role 2 ``` +#### Metric list filters (client-side) + +These filters run client-side: when any is set, the CLI fetches every metric and filters locally. They combine with AND across flags and OR within a comma-separated list. (Server-side equivalents are a planned follow-up.) + +| Filter | Description | +|---|---| +| `--metric-type ` | `goal_count`, `goal_unique_count`, `goal_time_to_achievement`, `goal_property`, `goal_property_unique_count`, `goal_ratio`, `goal_retention`, `goal_activity_period_count`, `custom_sql` | +| `--goal ` | Goal name (substring) or ID; matches numerator and denominator goals | +| `--outlier-limiting` / `--no-outlier-limiting` | Has / does not have outlier limiting (method other than `unlimited`) | +| `--outlier-method ` | `unlimited`, `quantile`, `stdev`, `fixed` | +| `--has-property-filter` / `--no-property-filter` | Has / does not have a goal property filter | +| `--property-filter-path ` | Property path(s) referenced inside the property filter (substring) | +| `--property-filter-contains ` | Substring anywhere in the serialized property filter | +| `--impact-direction ` | `positive`, `negative`, `unknown` | +| `--cuped` / `--no-cuped` | Has / does not have CUPED (variance reduction) enabled | + ### Teams Aliases: `teams`, `team` From 1b85739037621af0593a879b60f967548f4ff666 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 12:33:16 +0100 Subject: [PATCH 12/14] docs(metrics): note pagination flags are inert under client filtering (FT-1966) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 84b5ffb..a4b4aa4 100644 --- a/README.md +++ b/README.md @@ -966,7 +966,7 @@ abs metrics access revoke-user 123 --user 1 --role 2 #### Metric list filters (client-side) -These filters run client-side: when any is set, the CLI fetches every metric and filters locally. They combine with AND across flags and OR within a comma-separated list. (Server-side equivalents are a planned follow-up.) +These filters run client-side: when any is set, the CLI fetches every metric and filters locally. They combine with AND across flags and OR within a comma-separated list. Because the full set is fetched and filtered, `--items`/`--page` do not apply while filtering — every match is shown. (Server-side equivalents are a planned follow-up.) | Filter | Description | |---|---| From bc785d08f430a47872e6d5da5fdac262f87132f0 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 15:32:25 +0100 Subject: [PATCH 13/14] style: apply prettier formatting (FT-1966) --- src/commands/metrics/index.ts | 5 ++- src/commands/metrics/metrics.test.ts | 52 +++++++++++++++++++++++++--- src/core/metrics/filter.test.ts | 41 ++++++++++++++++------ src/core/metrics/filter.ts | 3 +- src/core/metrics/metrics.test.ts | 16 +++++++-- 5 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/commands/metrics/index.ts b/src/commands/metrics/index.ts index b0ee382..5a9e106 100644 --- a/src/commands/metrics/index.ts +++ b/src/commands/metrics/index.ts @@ -82,7 +82,10 @@ const listCommand = createListCommand({ 'filter by goal name or ID, comma-separated (matches numerator and denominator goals)' ) // positive flag declared before negative so the boolean stays tri-state - .option('--outlier-limiting', 'only metrics that have outlier limiting (method other than unlimited)') + .option( + '--outlier-limiting', + 'only metrics that have outlier limiting (method other than unlimited)' + ) .option('--no-outlier-limiting', 'only metrics without outlier limiting') .option( '--outlier-method ', diff --git a/src/commands/metrics/metrics.test.ts b/src/commands/metrics/metrics.test.ts index dc8ef74..f221fc0 100644 --- a/src/commands/metrics/metrics.test.ts +++ b/src/commands/metrics/metrics.test.ts @@ -202,14 +202,50 @@ describe('metrics command', () => { }); const richMetrics = [ - { id: 1, name: 'a', type: 'goal_count', effect: 'positive', goal_id: 1, goal: { id: 1, name: 'page_view' }, outlier_limit_method: 'unlimited', vr_lookback_interval: null, property_filter: null }, - { id: 2, name: 'b', type: 'goal_ratio', effect: 'negative', goal_id: 2, goal: { id: 2, name: 'checkout' }, outlier_limit_method: 'quantile', vr_lookback_interval: '2w', property_filter: '{"filter":{"and":[{"var":{"path":"page_name"}}]}}' }, - { id: 3, name: 'c', type: 'custom_sql', effect: 'unknown', goal_id: 3, goal: { id: 3, name: 'purchase' }, outlier_limit_method: 'unlimited', vr_lookback_interval: null, property_filter: null }, + { + id: 1, + name: 'a', + type: 'goal_count', + effect: 'positive', + goal_id: 1, + goal: { id: 1, name: 'page_view' }, + outlier_limit_method: 'unlimited', + vr_lookback_interval: null, + property_filter: null, + }, + { + id: 2, + name: 'b', + type: 'goal_ratio', + effect: 'negative', + goal_id: 2, + goal: { id: 2, name: 'checkout' }, + outlier_limit_method: 'quantile', + vr_lookback_interval: '2w', + property_filter: '{"filter":{"and":[{"var":{"path":"page_name"}}]}}', + }, + { + id: 3, + name: 'c', + type: 'custom_sql', + effect: 'unknown', + goal_id: 3, + goal: { id: 3, name: 'purchase' }, + outlier_limit_method: 'unlimited', + vr_lookback_interval: null, + property_filter: null, + }, ]; it('filters metrics list by --metric-type (client-side)', async () => { mockClient.listMetrics.mockResolvedValue(richMetrics); - await metricsCommand.parseAsync(['node', 'test', 'list', '--metric-type', 'goal_ratio,custom_sql']); + await metricsCommand.parseAsync([ + 'node', + 'test', + 'list', + '--metric-type', + 'goal_ratio,custom_sql', + ]); const printed = vi.mocked(printFormatted).mock.calls.at(-1)?.[0] as Array<{ id: number }>; expect(printed.map((m) => m.id)).toEqual([2, 3]); @@ -250,7 +286,13 @@ describe('metrics command', () => { it('rejects combining --has-property-filter with --no-property-filter', async () => { mockClient.listMetrics.mockResolvedValue(richMetrics); try { - await metricsCommand.parseAsync(['node', 'test', 'list', '--has-property-filter', '--no-property-filter']); + await metricsCommand.parseAsync([ + 'node', + 'test', + 'list', + '--has-property-filter', + '--no-property-filter', + ]); throw new Error('Should have thrown'); } catch (error) { if (!(error as Error).message.startsWith('process.exit')) throw error; diff --git a/src/core/metrics/filter.test.ts b/src/core/metrics/filter.test.ts index 059458e..f3ad2f0 100644 --- a/src/core/metrics/filter.test.ts +++ b/src/core/metrics/filter.test.ts @@ -45,7 +45,9 @@ describe('parseMetricFilters', () => { // neither flag: commander leaves propertyFilter at its default true; we ignore it expect(parseMetricFilters({ propertyFilter: true }).hasPropertyFilter).toBeUndefined(); // --has-property-filter - expect(parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true }).hasPropertyFilter).toBe(true); + expect( + parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true }).hasPropertyFilter + ).toBe(true); // --no-property-filter expect(parseMetricFilters({ propertyFilter: false }).hasPropertyFilter).toBe(false); }); @@ -70,7 +72,9 @@ describe('validateMetricFilters', () => { }); it('rejects unknown impact directions', () => { const opts = { impactDirection: 'sideways' }; - expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).toThrow(/impact.*direction/i); + expect(() => validateMetricFilters(opts, parseMetricFilters(opts))).toThrow( + /impact.*direction/i + ); }); it('rejects passing both --has-property-filter and --no-property-filter', () => { const opts = { hasPropertyFilter: true, propertyFilter: false }; @@ -140,7 +144,11 @@ describe('filterMetrics - type / impact / goal', () => { it('filters by goal name substring (numerator or denominator)', () => { const data = [ metric({ id: 1, goal: { id: 1, name: 'page_view' } }), - metric({ id: 2, goal: { id: 2, name: 'checkout' }, denominator_goal: { id: 3, name: 'page_view_all' } }), + metric({ + id: 2, + goal: { id: 2, name: 'checkout' }, + denominator_goal: { id: 3, name: 'page_view_all' }, + }), metric({ id: 3, goal: { id: 4, name: 'purchase' } }), ]; const out = filterMetrics(data, parseMetricFilters({ goal: 'page_view' })); @@ -153,7 +161,11 @@ describe('filterMetrics - outlier / cuped', () => { const data = [ metric({ id: 1, outlier_limit_method: 'unlimited' }), metric({ id: 2, outlier_limit_method: 'quantile' }), - metric({ id: 3, outlier_limit_method: 'unlimited', denominator_outlier_limit_method: 'stdev' }), + metric({ + id: 3, + outlier_limit_method: 'unlimited', + denominator_outlier_limit_method: 'stdev', + }), ]; const out = filterMetrics(data, parseMetricFilters({ outlierLimiting: true })); expect(out.map((m) => m.id)).toEqual([2, 3]); @@ -171,7 +183,11 @@ describe('filterMetrics - outlier / cuped', () => { it('filters by outlier method (numerator or denominator, OR within list)', () => { const data = [ metric({ id: 1, outlier_limit_method: 'quantile' }), - metric({ id: 2, outlier_limit_method: 'unlimited', denominator_outlier_limit_method: 'fixed' }), + metric({ + id: 2, + outlier_limit_method: 'unlimited', + denominator_outlier_limit_method: 'fixed', + }), metric({ id: 3, outlier_limit_method: 'stdev' }), ]; const out = filterMetrics(data, parseMetricFilters({ outlierMethod: 'quantile,fixed' })); @@ -213,15 +229,15 @@ describe('filterMetrics - property filter', () => { metric({ id: 4, property_filter: PF }), metric({ id: 5, property_filter: null, denominator_property_filter: PF2 }), ]; - const out = filterMetrics(data, parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true })); + const out = filterMetrics( + data, + parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true }) + ); expect(out.map((m) => m.id)).toEqual([4, 5]); }); it('--no-property-filter keeps only metrics without a filter', () => { - const data = [ - metric({ id: 1, property_filter: null }), - metric({ id: 2, property_filter: PF }), - ]; + const data = [metric({ id: 1, property_filter: null }), metric({ id: 2, property_filter: PF })]; const out = filterMetrics(data, parseMetricFilters({ propertyFilter: false })); expect(out.map((m) => m.id)).toEqual([1]); }); @@ -259,7 +275,10 @@ describe('filterMetrics - property filter', () => { metric({ id: 4, property_filter: '[]' }), metric({ id: 5, property_filter: '[{}]' }), ]; - const out = filterMetrics(data, parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true })); + const out = filterMetrics( + data, + parseMetricFilters({ hasPropertyFilter: true, propertyFilter: true }) + ); expect(out.map((m) => m.id)).toEqual([3]); }); }); diff --git a/src/core/metrics/filter.ts b/src/core/metrics/filter.ts index eb050f1..5d0a690 100644 --- a/src/core/metrics/filter.ts +++ b/src/core/metrics/filter.ts @@ -48,7 +48,8 @@ export function parseMetricFilters(options: Record): MetricFilt return { metricType: splitCsv(options.metricType), goal: splitCsv(options.goal), - outlierLimiting: typeof options.outlierLimiting === 'boolean' ? options.outlierLimiting : undefined, + outlierLimiting: + typeof options.outlierLimiting === 'boolean' ? options.outlierLimiting : undefined, outlierMethod: splitCsv(options.outlierMethod), hasPropertyFilter, propertyFilterPath: splitCsv(options.propertyFilterPath), diff --git a/src/core/metrics/metrics.test.ts b/src/core/metrics/metrics.test.ts index b1fbb45..383c8dc 100644 --- a/src/core/metrics/metrics.test.ts +++ b/src/core/metrics/metrics.test.ts @@ -827,8 +827,14 @@ describe('listAllMetrics', () => { expect(result.data.map((m: any) => m.id)).toEqual([1, 2, 3, 4, 5]); expect(client.listMetrics).toHaveBeenCalledTimes(3); - expect(client.listMetrics).toHaveBeenNthCalledWith(1, expect.objectContaining({ items: 2, page: 1 })); - expect(client.listMetrics).toHaveBeenNthCalledWith(3, expect.objectContaining({ items: 2, page: 3 })); + expect(client.listMetrics).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ items: 2, page: 1 }) + ); + expect(client.listMetrics).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ items: 2, page: 3 }) + ); }); it('stops after a single page when fewer than pageSize are returned', async () => { @@ -845,7 +851,11 @@ describe('listAllMetrics', () => { resolveTeams: vi.fn().mockResolvedValue([{ id: 5, name: 'Growth' }]), } as any; - await listAllMetrics(client, { items: 100, page: 1, owners: 'jane@example.com', teams: 'Growth' }, 2); + await listAllMetrics( + client, + { items: 100, page: 1, owners: 'jane@example.com', teams: 'Growth' }, + 2 + ); expect(client.resolveUsers).toHaveBeenCalledTimes(1); expect(client.resolveTeams).toHaveBeenCalledTimes(1); From 247c07b733639fcf63e5b89f7d2b229ed0e2542b Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 29 May 2026 19:48:11 +0100 Subject: [PATCH 14/14] fix(metrics): warn and report hasMore on listAllMetrics page-cap truncation (FT-1966) --- src/core/metrics/list.ts | 26 +++++++++++++++++++++----- src/core/metrics/metrics.test.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/core/metrics/list.ts b/src/core/metrics/list.ts index 1e1a251..26f6865 100644 --- a/src/core/metrics/list.ts +++ b/src/core/metrics/list.ts @@ -56,14 +56,17 @@ const FETCH_ALL_MAX_PAGES = 100; export async function listAllMetrics( client: APIClient, params: ListMetricsParams, - pageSize: number = FETCH_ALL_PAGE_SIZE + pageSize: number = FETCH_ALL_PAGE_SIZE, + maxPages: number = FETCH_ALL_MAX_PAGES ): Promise> { const ownerIds = params.owners ? await resolveOwnerIds(client, params.owners) : undefined; const teamIds = params.teams ? await resolveTeamIds(client, params.teams) : undefined; const all: unknown[] = []; - let page = 1; - for (; page <= FETCH_ALL_MAX_PAGES; page++) { + // Stays true if we exhaust `maxPages` without seeing a short (final) page, + // meaning there may be more metrics we did not fetch. + let truncated = true; + for (let page = 1; page <= maxPages; page++) { const data = await client.listMetrics({ items: pageSize, page, @@ -78,11 +81,24 @@ export async function listAllMetrics( review_status: params.reviewStatus, }); all.push(...data); - if (data.length < pageSize) break; + if (data.length < pageSize) { + truncated = false; + break; + } + } + + // Don't silently drop metrics: warn (stderr) and report hasMore honestly when + // the page cap is reached. Client-side filtering would otherwise look + // complete while missing matches beyond the cap. + if (truncated) { + console.error( + `Warning: reached the ${maxPages}-page fetch cap (${all.length} metrics); results may be incomplete. ` + + 'Narrow the set with server-side filters (--search, --owners, --teams, --ids) for complete client-side filtering.' + ); } return { data: all, - pagination: { page: 1, items: all.length, hasMore: false }, + pagination: { page: 1, items: all.length, hasMore: truncated }, }; } diff --git a/src/core/metrics/metrics.test.ts b/src/core/metrics/metrics.test.ts index 383c8dc..eb5ffe6 100644 --- a/src/core/metrics/metrics.test.ts +++ b/src/core/metrics/metrics.test.ts @@ -863,4 +863,34 @@ describe('listAllMetrics', () => { expect.objectContaining({ owners: '10', teams: '5' }) ); }); + + it('warns and reports hasMore when the page cap is reached (truncation)', async () => { + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + // Every page is full (pageSize 2), so the loop never breaks early and hits the cap. + const client = { listMetrics: vi.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]) } as any; + + const result = await listAllMetrics(client, { items: 100, page: 1 }, 2, 3); + + expect(client.listMetrics).toHaveBeenCalledTimes(3); // maxPages + expect(result.data).toHaveLength(6); + expect(result.pagination?.hasMore).toBe(true); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining('fetch cap')); + errSpy.mockRestore(); + }); + + it('does not warn and reports hasMore false when a short page ends the fetch', async () => { + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const client = { + listMetrics: vi + .fn() + .mockResolvedValueOnce([{ id: 1 }, { id: 2 }]) + .mockResolvedValueOnce([{ id: 3 }]), + } as any; + + const result = await listAllMetrics(client, { items: 100, page: 1 }, 2, 3); + + expect(result.pagination?.hasMore).toBe(false); + expect(errSpy).not.toHaveBeenCalled(); + errSpy.mockRestore(); + }); });