feat(cohorts): let a criterion say "never did this event"

apps/start/src/components/cohort/cohort-criteria-builder.tsx

@@ -273,15 +273,19 @@ function EventCriteriaItem({
         <label className="mb-1 block text-sm font-medium">Frequency</label>
         <div className="flex gap-2">
           <DropdownMenuComposed
-            onChange={(operator) =>
+            onChange={(operator) => {
+              const count = criteria.frequency?.count ?? 1;
               onChange({
                 ...criteria,
                 frequency: {
-                  ...(criteria.frequency ?? { count: 1 }),
                   operator,
+                  // "At least 0" matches everyone and the server rejects it,
+                  // so a count of 0 only survives the operators that mean
+                  // "never".
+                  count: operator === 'gte' && count === 0 ? 1 : count,
                 },
-              })
-            }
+              });
+            }}
             items={[
               { value: 'gte', label: 'At least' },
               { value: 'eq', label: 'Exactly' },
@@ -297,17 +301,27 @@ function EventCriteriaItem({
           </DropdownMenuComposed>
           <input
             type="number"
-            min="1"
+            min="0"
             value={criteria.frequency?.count ?? 1}
-            onChange={(e) =>
+            onChange={(e) => {
+              // Explicit NaN check, not `|| 1`: 0 is falsy, so the fallback
+              // rewrote a typed 0 back to 1 and "never did this event" could
+              // not be entered.
+              const parsed = Number.parseInt(e.target.value, 10);
+              const operator = criteria.frequency?.operator ?? 'gte';
               onChange({
                 ...criteria,
                 frequency: {
-                  operator: criteria.frequency?.operator ?? 'gte',
-                  count: Number.parseInt(e.target.value) || 1,
+                  operator,
+                  // Same "At least 0 matches everyone" clamp as the
+                  // operator-change handler above.
+                  count:
+                    Number.isNaN(parsed) || (operator === 'gte' && parsed === 0)
+                      ? 1
+                      : parsed,
                 },
-              })
-            }
+              });
+            }}
             className="w-20 rounded border px-2 py-1 text-sm"
           />
           <span className="flex items-center text-sm text-muted-foreground">

packages/db/src/services/cohort.service.test.ts

@@ -1,6 +1,13 @@
-import type { EventCriteria } from '@openpanel/validation';
+import type {
+  EventBasedCohortDefinition,
+  EventCriteria,
+} from '@openpanel/validation';
 import { describe, expect, it } from 'vitest';
-import { buildEventCriteriaQuery } from './cohort.service';
+import { TABLE_NAMES } from '../clickhouse/client';
+import {
+  buildEventBasedCohortQuery,
+  buildEventCriteriaQuery,
+} from './cohort.service';
 
 const PROJECT_ID = 'test-cohort-timeframe';
 
@@ -88,3 +95,168 @@ describe('buildEventCriteriaQuery timeframe escaping', () => {
     ).toContain('event_date >= toDate(now() - INTERVAL 30 DAY)');
   });
 });
+
+const LAST_30_DAYS = {
+  type: 'relative',
+  value: '30d',
+} satisfies EventCriteria['timeframe'];
+
+function frequencyCriteria(
+  frequency: EventCriteria['frequency'],
+  filters: EventCriteria['filters'] = []
+): EventCriteria {
+  return {
+    name: 'subscription_started',
+    filters,
+    timeframe: LAST_30_DAYS,
+    frequency,
+  };
+}
+
+// Everything the outer profile scan sees, i.e. the query with the NOT IN
+// subquery cut out.
+function outsideExclusion(sql: string): string {
+  const open = sql.indexOf('NOT IN (');
+  if (open === -1) {
+    return sql;
+  }
+  return sql.slice(0, open) + sql.slice(sql.lastIndexOf(')') + 1);
+}
+
+describe('buildEventCriteriaQuery zero frequency', () => {
+  it.each(['eq', 'lte'] as const)(
+    'excludes anyone who did the event when the count is 0 (%s)',
+    (operator) => {
+      const sql = buildEventCriteriaQuery(
+        PROJECT_ID,
+        frequencyCriteria({ operator, count: 0 })
+      );
+
+      // The summary MV has no row for zero occurrences, so a HAVING can never
+      // match here — the query has to be inverted.
+      expect(sql).not.toContain('HAVING');
+      expect(sql).toContain('SELECT DISTINCT id AS profile_id');
+      expect(sql).toContain(`FROM ${TABLE_NAMES.profiles}`);
+      expect(sql).toContain('NOT IN (');
+      expect(sql).toContain(`FROM ${TABLE_NAMES.event_profile_summary_mv}`);
+      expect(sql).toContain("name = 'subscription_started'");
+    }
+  );
+
+  it('gives eq 0 and lte 0 the same query', () => {
+    expect(
+      buildEventCriteriaQuery(
+        PROJECT_ID,
+        frequencyCriteria({ operator: 'eq', count: 0 })
+      )
+    ).toBe(
+      buildEventCriteriaQuery(
+        PROJECT_ID,
+        frequencyCriteria({ operator: 'lte', count: 0 })
+      )
+    );
+  });
+
+  it('keeps the timeframe inside the exclusion, not on the profile scan', () => {
+    const sql = buildEventCriteriaQuery(
+      PROJECT_ID,
+      frequencyCriteria({ operator: 'eq', count: 0 })
+    );
+
+    // "Never did X in the last 30 days" still includes someone who did X 60
+    // days ago — which only holds while the date bound sits in the subquery.
+    expect(sql).toContain('event_date >= toDate(now() - INTERVAL 30 DAY)');
+    expect(outsideExclusion(sql)).not.toContain('event_date');
+    expect(outsideExclusion(sql)).toContain(
+      `project_id = '${PROJECT_ID}'`
+    );
+  });
+
+  it('excludes on the matching property row when the criterion has property filters', () => {
+    const sql = buildEventCriteriaQuery(
+      PROJECT_ID,
+      frequencyCriteria({ operator: 'eq', count: 0 }, [
+        { name: 'properties.plan', operator: 'is', value: ['pro'] },
+      ])
+    );
+
+    // Read as "has no matching (event, property) row": someone who did the
+    // event with plan = free is a member.
+    expect(sql).not.toContain('HAVING');
+    expect(sql).toContain(
+      `FROM ${TABLE_NAMES.event_property_profile_summary_mv}`
+    );
+    expect(sql).toContain("property_key = 'plan'");
+    expect(outsideExclusion(sql)).not.toContain('property_key');
+  });
+
+  it('leaves gte 0 on the ordinary path (zFrequency rejects it upstream)', () => {
+    const sql = buildEventCriteriaQuery(
+      PROJECT_ID,
+      frequencyCriteria({ operator: 'gte', count: 0 })
+    );
+
+    expect(sql).toContain('HAVING countMerge(event_count) >= 0');
+    expect(sql).not.toContain('NOT IN');
+  });
+
+  it.each([
+    ['gte', 1, '>= 1'],
+    ['eq', 2, '= 2'],
+    ['lte', 3, '<= 3'],
+  ] as const)(
+    'still groups and filters for positive counts (%s %i)',
+    (operator, count, expected) => {
+      const sql = buildEventCriteriaQuery(
+        PROJECT_ID,
+        frequencyCriteria({ operator, count })
+      );
+
+      expect(sql).toContain('GROUP BY profile_id');
+      expect(sql).toContain(`HAVING countMerge(event_count) ${expected}`);
+      expect(sql).not.toContain(`FROM ${TABLE_NAMES.profiles}`);
+    }
+  );
+});
+
+describe('buildEventBasedCohortQuery with a zero-count criterion', () => {
+  const definition = {
+    type: 'event',
+    criteria: {
+      operator: 'and',
+      events: [
+        {
+          name: 'signup',
+          filters: [],
+          timeframe: LAST_30_DAYS,
+          frequency: { operator: 'gte', count: 1 },
+        },
+        frequencyCriteria({ operator: 'eq', count: 0 }),
+      ],
+    },
+  } satisfies EventBasedCohortDefinition;
+
+  it('intersects two sets of profile_id', () => {
+    const sql = buildEventBasedCohortQuery(PROJECT_ID, definition);
+    const [signedUp, neverSubscribed] = sql.split(' INTERSECT ');
+
+    expect(neverSubscribed).toBeDefined();
+    // Both operands have to be a bare set of profile_id for the INTERSECT to
+    // mean anything: same column name, one row per profile, no LIMIT or
+    // ORDER BY of their own.
+    expect(signedUp).toContain('SELECT profile_id');
+    expect(neverSubscribed).toContain('SELECT DISTINCT id AS profile_id');
+    expect(sql).not.toContain('ORDER BY');
+    expect(sql).not.toContain('LIMIT');
+  });
+
+  it('unions them under "or"', () => {
+    const sql = buildEventBasedCohortQuery(PROJECT_ID, {
+      ...definition,
+      criteria: { ...definition.criteria, operator: 'or' },
+    });
+
+    expect(sql).toContain(' UNION DISTINCT ');
+    expect(sql).not.toContain(' INTERSECT ');
+  });
+});