fix(app-shell): bridge Is null to the spec's $null instead of erasing the dataset filter

.changeset/9363-dataset-filter-isnull-erases.md

@@ -0,0 +1,29 @@
+---
+'@object-ui/app-shell': patch
+---
+
+Stop the Studio dataset-filter bridge from ERASING a stored filter when the author
+picks `Is null` (objectui#9363).
+
+`groupToCondition` had no mapping for `isNull` / `isNotNull`, so those rows fell
+through to the unmapped-operator drop. A dropped last row makes the function return
+`undefined`, and the dataset inspector commits on every change — so an author with a
+working `dataset.filter` (or a `measure.filter`) who opened the filter popover and
+switched the single condition's operator to **Is null** committed `undefined`, and the
+persisted filter was destroyed. Nothing errored and the panel still showed the
+condition. `Is null` is an ordinary entry in that menu, not an opt-in one.
+
+Both directions now bridge the spec's `$null` predicate: `isNull` serializes to
+`{ field: { $null: true } }` and `isNotNull` to `{ $null: false }`, and a stored
+`$null` reads back as the operator the author picked instead of degrading the whole
+filter to "edit it in the Source tab".
+
+`$null` stays distinct from `$exists`: `isEmpty` / `isNotEmpty` are unchanged, because
+the dropdown offers both pairs as their own rows and the spec's filter vocabulary
+carries both predicates.
+
+Operators this bridge still does not map are still dropped rather than emitted in a
+spelling that means something else — that behaviour is deliberate and is now pinned
+alongside the fix, together with the list of operators the menu offers and this bridge
+cannot store (`notContains`, `between`, `startsWith`, `endsWith`), so the next unmapped
+addition fails a test instead of erasing a filter.

packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.nullOperators-9363.test.ts

@@ -0,0 +1,235 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `Is null` must not ERASE the stored dataset filter (objectui#9363).
+ *
+ * ## The mechanism, and why it is data loss rather than a wrong result set
+ *
+ * `groupToCondition` returns `undefined` when no row survives serialization,
+ * and the Studio dataset inspector commits on EVERY change — the filter popover
+ * mounts the shared `FilterBuilder` and hands each emitted group straight to
+ * `onCommit(groupToCondition(g))`, which lands as `onPatch({ filter })` and is
+ * spread over the draft. So an author with a working `dataset.filter` who opens
+ * the inspector and switches the single condition's operator to **Is null**
+ * commits `undefined`: the persisted key is destroyed, nothing errors, and the
+ * panel still shows the condition.
+ *
+ * The inspector passes no `extraOperators`, so `isNull` / `isNotNull` are
+ * ordinary entries in the default menu — not opt-in ones. The parity block at
+ * the bottom of this file reads that offering from the builder's own exported
+ * bucket function rather than restating it.
+ *
+ * ## The distinction this file exists to keep
+ *
+ * The `continue` these two operators fell into is a DELIBERATE decision for
+ * operators the bridge does not map: dropping beats emitting a filter that
+ * means something else. That behaviour is kept, and pinned below, because a
+ * repair that made the fallback stop dropping everything it cannot map would
+ * emit wrong filters — a worse defect than this one.
+ *
+ * `isNull` / `isNotNull` are a different case and that is the whole repair: the
+ * dialect CAN express them (the spec's own `$null`, asserted below against
+ * `FILTER_OPERATORS` rather than assumed), the builder draws them as COMPLETE
+ * rows with no value input, and this inspector offers them. The drop was an
+ * unhandled operator falling into the fallback's path, not the fallback doing
+ * its job. Both readings are pinned so they stay distinguishable.
+ *
+ * ## Red-first
+ *
+ * Before the repair, the two `$null` expectations fail with `undefined` while
+ * the `equals` control in the same run passes — a table of all-`undefined`
+ * answers and a dead function are otherwise indistinguishable.
+ */
+import { describe, it, expect } from 'vitest';
+import { FILTER_OPERATORS, FieldOperatorsSchema } from '@objectstack/spec/data';
+import {
+  FILTER_BUILDER_OPERATORS,
+  VALUELESS_FILTER_BUILDER_OPERATORS,
+  operatorsForFieldType,
+} from '@object-ui/components';
+import { groupToCondition, conditionToGroup } from './datasetFilterCondition';
+import type { BuilderGroup } from './datasetFilterCondition';
+
+/** One condition row, as the builder emits it. */
+const row = (operator: string, value: unknown = ''): BuilderGroup => ({
+  id: 'g',
+  logic: 'and',
+  conditions: [{ id: 'c1', field: 'closed_at', operator, value }],
+});
+
+describe('groupToCondition — the null predicates this inspector offers (objectui#9363)', () => {
+  it('CONTROL: a mapped operator still serializes, so an empty answer below is about that operator', () => {
+    expect(groupToCondition(row('equals', 'acme'))).toEqual({ closed_at: { $eq: 'acme' } });
+  });
+
+  it('isNull serializes to the dialect\'s null predicate instead of vanishing', () => {
+    expect(
+      groupToCondition(row('isNull')),
+      'an `Is null` row serialized to nothing; committing that ERASES dataset.filter',
+    ).toEqual({ closed_at: { $null: true } });
+  });
+
+  it('isNotNull serializes to the same predicate negated', () => {
+    expect(groupToCondition(row('isNotNull'))).toEqual({ closed_at: { $null: false } });
+  });
+
+  it('keeps a null row alongside a complete one instead of dropping either', () => {
+    expect(groupToCondition({
+      id: 'g',
+      logic: 'and',
+      conditions: [
+        { id: 'c1', field: 'stage', operator: 'equals', value: 'won' },
+        { id: 'c2', field: 'closed_at', operator: 'isNull', value: '' },
+      ],
+    })).toEqual({ $and: [{ stage: { $eq: 'won' } }, { closed_at: { $null: true } }] });
+  });
+
+  it('THE DEFECT: switching the only row of a stored filter to Is null no longer commits `undefined`', () => {
+    // The exact author gesture: a dataset that already has a filter, opened in
+    // the inspector, one operator change. `undefined` here is not "unchanged" —
+    // it is what the host spreads over the draft as `{ filter: undefined }`,
+    // the same patch shape `objectChangePatch` uses to CLEAR the filter.
+    const stored = { stage: { $eq: 'won' } };
+    const { group, representable } = conditionToGroup(stored);
+    expect(representable).toBe(true);
+    const edited: BuilderGroup = {
+      ...group,
+      conditions: [{ ...group.conditions[0], operator: 'isNull', value: '' }],
+    };
+    expect(
+      groupToCondition(edited),
+      'the commit for this gesture was `undefined`, which erases the stored filter',
+    ).toEqual({ stage: { $null: true } });
+  });
+
+  it('leaves the $exists pair exactly as it was', () => {
+    expect(groupToCondition(row('isEmpty'))).toEqual({ closed_at: { $exists: false } });
+    expect(groupToCondition(row('isNotEmpty'))).toEqual({ closed_at: { $exists: true } });
+  });
+
+  it('still drops an operator it does not map, rather than emitting a wrong filter', () => {
+    // Deliberate, and kept: see this file's header. These four are OFFERED by
+    // the menu and dropped, which erases the same way — tracked as its own
+    // finding, not widened here on the way past.
+    expect(groupToCondition(row('notContains', 'a'))).toBeUndefined();
+    expect(groupToCondition(row('between', [1, 5]))).toBeUndefined();
+    expect(groupToCondition(row('startsWith', 'a'))).toBeUndefined();
+    expect(groupToCondition(row('endsWith', 'a'))).toBeUndefined();
+  });
+
+  it('an empty group is still `undefined` — that is the author CLEARING the filter', () => {
+    expect(groupToCondition({ id: 'g', logic: 'and', conditions: [] })).toBeUndefined();
+  });
+});
+
+describe('the emitted token is the spec\'s, not a local invention (objectui#9363)', () => {
+  it('`$null` is a member of the spec\'s filter operator vocabulary', () => {
+    expect(FILTER_OPERATORS).toContain('$null');
+    // Negative control: membership is a real reading, not a list that contains
+    // everything. A plausible-looking spelling this bridge could have invented
+    // is NOT in it.
+    expect(FILTER_OPERATORS).not.toContain('$isNull');
+  });
+
+  it('the spec\'s field-operator door accepts the boolean comparand and refuses a wrong one', () => {
+    expect(FieldOperatorsSchema.safeParse({ $null: true }).success).toBe(true);
+    expect(FieldOperatorsSchema.safeParse({ $null: false }).success).toBe(true);
+    // Negative control: this door judges the VALUE, so a string comparand is
+    // refused — without this leg the assertion above would pass for a schema
+    // that accepts anything.
+    expect(FieldOperatorsSchema.safeParse({ $null: 'yes' }).success).toBe(false);
+  });
+});
+
+describe('conditionToGroup — the read half round-trips the new shape (objectui#9363)', () => {
+  it('reads a stored $null back as the operator the author picked', () => {
+    expect(conditionToGroup({ closed_at: { $null: true } })).toEqual({
+      group: { id: 'g', logic: 'and', conditions: [{ id: 'c0', field: 'closed_at', operator: 'isNull', value: '' }] },
+      representable: true,
+    });
+    expect(conditionToGroup({ closed_at: { $null: false } }).group.conditions[0].operator)
+      .toBe('isNotNull');
+  });
+
+  it('round-trips condition → group → condition', () => {
+    for (const c of [{ closed_at: { $null: true } }, { closed_at: { $null: false } }]) {
+      const { group, representable } = conditionToGroup(c);
+      expect(representable, `${JSON.stringify(c)} fell back to the source editor`).toBe(true);
+      expect(groupToCondition(group)).toEqual(c);
+    }
+  });
+});
+
+/**
+ * Offered ⇄ expressible parity for THIS inspector.
+ *
+ * The direction that broke: every guard in the repo sweeps spec → objectui,
+ * asking whether an operator an author may DECLARE can be rendered. None asks
+ * whether an operator this dropdown OFFERS can be stored by the consumer that
+ * mounted it — and that is the direction where an unmapped operator becomes
+ * silent data loss rather than a rendering gap.
+ *
+ * The offering is read from the builder's own bucket function with NO
+ * `extraOperators`, which is exactly what `DatasetFilterField` passes, so a
+ * future opt-in granted at that call site has to come through here.
+ */
+const PROBE_FIELD_TYPES: ReadonlyArray<string | undefined> = [
+  undefined, 'text', 'a_type_this_builder_has_never_heard_of', 'number', 'currency',
+  'percent', 'rating', 'boolean', 'date', 'datetime', 'time', 'select', 'status',
+  'lookup', 'master_detail', 'user',
+];
+
+function offeredAcrossBuckets(extra: readonly string[]): string[] {
+  const ids = new Set<string>();
+  for (const type of PROBE_FIELD_TYPES) for (const op of operatorsForFieldType(type, extra)) ids.add(op.value);
+  return [...ids].sort();
+}
+
+/** What the dataset inspector's filter popover offers. */
+const OFFERED = offeredAcrossBuckets([]);
+
+/**
+ * Offered, and deliberately NOT expressible by this bridge today.
+ *
+ * Each one drops on commit, and a drop of the last surviving row erases the
+ * stored filter — the same mechanism objectui#9363 fixed for the null pair.
+ * They are listed rather than fixed here because mapping them is a separate
+ * decision per operator (`between` needs a both-bounds-present rule before it
+ * can be emitted at all), and a blanket "stop dropping" would emit filters that
+ * mean something else. Mapping one is what makes this list shrink — and this
+ * assertion go red until it is updated.
+ */
+const DECLARED_UNEXPRESSIBLE = ['between', 'endsWith', 'notContains', 'startsWith'];
+
+/** A value that keeps a row from being dropped as INCOMPLETE, per operator. */
+function probeValue(operator: string): unknown {
+  if (VALUELESS_FILTER_BUILDER_OPERATORS.has(operator)) return '';
+  if (operator === 'in' || operator === 'notIn') return ['a', 'b'];
+  if (operator === 'between') return [1, 5];
+  return 'x';
+}
+
+describe('every operator this inspector OFFERS is either expressible or declared (objectui#9363)', () => {
+  it('the probe types cover every bucket, so the offering below is the whole dropdown', () => {
+    // Granting every id as an opt-in yields the full drawable vocabulary only
+    // if the probe list reaches every bucket. Without this, a bucket added
+    // later would silently shrink what the partition below is asserted over.
+    expect(offeredAcrossBuckets(FILTER_BUILDER_OPERATORS)).toEqual([...FILTER_BUILDER_OPERATORS].sort());
+  });
+
+  it('partitions the offering exactly — no operator is silently unhandled', () => {
+    const expressible: string[] = [];
+    const dropped: string[] = [];
+    for (const operator of OFFERED) {
+      (groupToCondition(row(operator, probeValue(operator))) === undefined ? dropped : expressible)
+        .push(operator);
+    }
+    expect(dropped.sort()).toEqual(DECLARED_UNEXPRESSIBLE);
+    // The other half of the equality: every remaining offered id serializes.
+    expect(expressible.sort()).toEqual(OFFERED.filter((o) => !DECLARED_UNEXPRESSIBLE.includes(o)));
+    // And the null pair is on the expressible side — the card's defect, stated
+    // as a fact about the offering rather than about two literals.
+    expect(expressible).toContain('isNull');
+    expect(expressible).toContain('isNotNull');
+  });
+});

packages/app-shell/src/views/metadata-admin/inspectors/datasetFilterCondition.ts

@@ -11,6 +11,11 @@
  * (nested groups, `$or`, multi-operator objects, unmapped operators) is reported
  * as NOT representable so the caller can fall back to the source editor instead
  * of silently corrupting the author's filter.
+ *
+ * The value-less operators are the exception to "field op value": the builder
+ * draws no input for them, so the row is complete without one. Both pairs the
+ * spec's vocabulary carries — `$exists` (is empty) and `$null` (is null) — are
+ * bridged here, in {@link VALUELESS_TO_MONGO}.
  */
 
 /** FilterBuilder camelCase operator → FilterCondition Mongo operator. */
@@ -26,6 +31,32 @@ const MONGO_TO_OP: Record<string, string> = {
   $contains: 'contains', $in: 'in', $nin: 'notIn',
 };
 
+/**
+ * Value-less builder operators, and the predicate each one lowers to.
+ *
+ * A row carrying one of these is COMPLETE without a value — the builder draws
+ * no input for it — so they are matched ahead of the value-completeness check
+ * in {@link groupToCondition}, not after it.
+ *
+ * `isNull` / `isNotNull` are not a spelling of `isEmpty` / `isNotEmpty`. The
+ * dropdown offers both pairs as their own rows and the spec's filter vocabulary
+ * carries both `$null` and `$exists`, so they stay distinct in both directions;
+ * collapsing them would draw two labels for one wire predicate and rewrite the
+ * author's choice when the filter is read back.
+ *
+ * objectui#9363: the null pair was missing here, so an `Is null` row — an
+ * ordinary entry in this inspector's menu, drawn as a finished row — fell
+ * through to the unmapped-operator `continue` below and was dropped. Dropping
+ * the last surviving row makes this function return `undefined`, and the
+ * inspector commits that as `{ filter: undefined }`, the same patch shape used
+ * to CLEAR the filter. So picking the entry erased the author's stored filter,
+ * with no error and the condition still on screen.
+ */
+const VALUELESS_TO_MONGO: Record<string, Record<string, boolean>> = {
+  isEmpty: { $exists: false }, isNotEmpty: { $exists: true },
+  isNull: { $null: true }, isNotNull: { $null: false },
+};
+
 export interface BuilderCondition { id?: string; field: string; operator: string; value?: unknown }
 export interface BuilderGroup { id?: string; logic: 'and' | 'or'; conditions: BuilderCondition[] }
 
@@ -53,10 +84,17 @@ export function groupToCondition(group: BuilderGroup | undefined): FilterConditi
   const conds = (group?.conditions ?? []).filter((c) => c && c.field);
   const parts: FilterCondition[] = [];
   for (const c of conds) {
-    if (c.operator === 'isEmpty') { parts.push({ [c.field]: { $exists: false } }); continue; }
-    if (c.operator === 'isNotEmpty') { parts.push({ [c.field]: { $exists: true } }); continue; }
+    const valueless = VALUELESS_TO_MONGO[c.operator];
+    if (valueless) { parts.push({ [c.field]: { ...valueless } }); continue; }
     const mop = OP_TO_MONGO[c.operator];
-    if (!mop) continue; // unmapped (e.g. notContains/between) — drop rather than emit a bad filter
+    // Still dropped rather than emitted in a spelling that means something
+    // else. ⚠️ The drop is not free: it is what erases the stored filter when
+    // no other row survives (see VALUELESS_TO_MONGO), and this menu offers
+    // `notContains` / `between` / `startsWith` / `endsWith`, none of which this
+    // table maps. Mapping one is a per-operator decision — `between` needs a
+    // both-bounds-present rule before it can be emitted at all — so they are
+    // declared, and pinned, in `datasetFilterCondition.nullOperators-9363`.
+    if (!mop) continue;
     // Skip incomplete rows (no value typed yet) — emitting `{field:{$op:''}}` would
     // be a silently-wrong filter (matches only empty), not "no filter".
     const v = c.value;
@@ -96,6 +134,13 @@ export function conditionToGroup(cond: FilterCondition | undefined | null): { gr
       const mop = opKeys[0];
       if (mop === '$exists') {
         conditions.push({ id: `c${i}`, field, operator: v.$exists ? 'isNotEmpty' : 'isEmpty', value: '' });
+      } else if (mop === '$null') {
+        // The inverse of the write half: `$null: false` is "is not null", so
+        // the boolean picks the operator rather than becoming the row's value.
+        // Without this arm a filter this bridge now WRITES would read back as
+        // non-representable, sending the author to the Source tab for a row the
+        // builder can draw.
+        conditions.push({ id: `c${i}`, field, operator: v.$null ? 'isNull' : 'isNotNull', value: '' });
       } else {
         const op = MONGO_TO_OP[mop];
         if (!op) return { group: empty, representable: false };