fix(plugin-list): 删掉 UserFilters 的私有算子表,tab preset 走 spec 单一出口 (#3470)

.changeset/user-filters-tab-preset-operators-single-exit.md

@@ -0,0 +1,9 @@
+---
+'@object-ui/plugin-list': patch
+---
+
+`UserFilters` no longer carries its own operator table when it lowers a `ViewTab.filter` preset into an ObjectQL AST node. The private `specOperatorToAst` was the second hand-kept operator map in this package and it had drifted: it lowered `not_in` — the spec's OWN canonical spelling — and the legacy `nin` to the spaced `'not in'`, a spelling that appears in no spec vocabulary. `isFilterAST` refuses it, so clicking such a tab produced an empty list plus `400 INVALID_FILTER`. Measured against a real backend (published `@objectstack/*@17.0.0-rc.2` + app-showcase, on `showcase_task`): `$filter=[["status","not in",["done"]]]` returned `400 INVALID_FILTER`, while `[["status","not_in",["done"]]]` returned `200` with the same 8 records as the `["status","!=","done"]` baseline.
+
+Lowering is now purely structural — all 19 `VIEW_FILTER_OPERATORS` are already members of the wire's `VALID_AST_OPERATORS`, so nothing needs translating — with the spec's own `normalizeFilterOperator` as the single exit for the legacy spellings stored metadata still carries (`gt`, `eq`, `nin`, `notEquals`, …). That is the same exit the write side (`viewFilterFold`) and the saved-view fold in `@object-ui/core` already use, so the directions cannot drift into two dialects. An operator the spec does not know is passed through verbatim, so a misspelling still earns a loud `400` rather than being coerced into a valid filter.
+
+`before` and `after` are now passed through rather than rewritten to `<` and `>`. That was the one judgement call, and it was settled by measurement rather than assumption: on the same live backend the word and the symbol return identical status and identical record ids, on a `date` field and on a `datetime` field, in both directions — so the rewrite was a no-op and dropping it is a pure fix. The remaining 18 canonical operators were measured the same way and are likewise unchanged in what the server answers; `not_in` is the only one whose answer changes, from `400` to the correct rows. Tab presets given in the legacy already-lowered `filters: triplet[]` shape are untouched, as before.

packages/plugin-list/src/UserFilters.tsx

@@ -10,6 +10,7 @@ import * as React from 'react';
 import { cn, Button, Popover, PopoverContent, PopoverTrigger, LookupValuePicker } from '@object-ui/components';
 import { ChevronDown, X, Plus } from 'lucide-react';
 import type { ListViewSchema } from '@object-ui/types';
+import { normalizeFilterOperator } from '@objectstack/spec/ui';
 import { useSafeFieldLabel, useObjectTranslation } from '@object-ui/i18n';
 
 function useMoreLabel(): string {
@@ -96,31 +97,63 @@ export interface UserFiltersProps {
   onSelectionsChange?: (selections: Record<string, Array<string | number | boolean>>) => void;
 }
 
-/**
- * Map @objectstack/spec ViewFilterRule operators to ObjectQL AST operators.
- * Accepts the canonical vocabulary (`greater_than`, `not_equals`, `before`, …)
- * that the Studio filter builder now writes, plus legacy shorthand spellings
- * (`gt`, `eq`, `nin`) still present in already-stored view metadata.
- */
-function specOperatorToAst(op: string | undefined): string {
-  switch (op) {
-    case undefined: case 'equals': case 'eq': return '=';
-    case 'not_equals': case 'ne': case 'neq': return '!=';
-    case 'greater_than_or_equal': case 'gte': return '>=';
-    case 'less_than_or_equal': case 'lte': return '<=';
-    case 'greater_than': case 'gt': case 'after': return '>';
-    case 'less_than': case 'lt': case 'before': return '<';
-    case 'not_contains': return 'notcontains';
-    case 'starts_with': return 'startswith';
-    case 'not_in': case 'nin': return 'not in';
-    default: return op;
-  }
-}
-
 /**
  * Normalize tab presets to the client shape. Accepts both:
  * - @objectstack/spec ViewTab: `{ name, label, filter: ViewFilterRule[], isDefault }`
  * - legacy client shape: `{ id, label, filters: triplet[], default }`
+ *
+ * **The spec rule → AST lowering is purely structural**, and deliberately owns
+ * NO operator table of its own (#3470). All 19 `VIEW_FILTER_OPERATORS` are
+ * already members of the wire's `VALID_AST_OPERATORS`, so nothing needs
+ * translating; only the legacy spellings stored view metadata still carries
+ * (`gt`, `eq`, `nin`, `notEquals`, …) need folding onto the canonical word, and
+ * that is exactly what the spec's OWN {@link normalizeFilterOperator} does —
+ * the same single exit the WRITE side uses (`app-shell/views/viewFilterFold.ts`)
+ * and the same one `@object-ui/core`'s `viewFilterRuleToNode` uses for a saved
+ * view's `filter` (#3431). One exit, so the directions cannot drift apart.
+ *
+ * The private table this replaced was the second hand-kept operator map in this
+ * package, and it had drifted: it lowered `not_in`/`nin` to the SPACED
+ * `'not in'`, which is in no spec vocabulary — `isFilterAST()` refuses it and
+ * the wire answers `400 INVALID_FILTER`. Measured against a real backend
+ * (published `@objectstack/*@17.0.0-rc.2` + app-showcase, `showcase_task`):
+ * `[["status","not in",["done"]]]` → **400 INVALID_FILTER**;
+ * `[["status","not_in",["done"]]]` → **200, 8 rows** (same rows as the
+ * `["status","!=","done"]` baseline). Every other operator the old table
+ * rewrote was measured to be a no-op on the answer — see below.
+ *
+ * **`before`/`after` are passed through, not mapped** (the one judgement call
+ * this change had to make: the old table rewrote them to `<`/`>`, and both
+ * words are themselves `VALID_AST_OPERATORS` members). Measured on the same
+ * backend, on a `date` field and a `datetime` field, both directions —
+ * identical status AND identical record ids:
+ *
+ *   `["due_date","before","2026-08-01"]`               → 200, 2 rows
+ *   `["due_date","<","2026-08-01"]`                    → 200, the SAME 2 rows
+ *   `["due_date","after","2026-08-01"]`                → 200, 8 rows
+ *   `["due_date",">","2026-08-01"]`                    → 200, the SAME 8 rows
+ *   `["created_at","before","2026-08-01T00:00:00.000Z"]` → 200, 6 rows  (`<` idem)
+ *   `["created_at","after","2026-08-01T00:00:00.000Z"]`  → 200, 3 rows  (`>` idem)
+ *
+ * So dropping that rewrite is a pure fix, not a behaviour change. (The spec
+ * agrees independently: `canonicalAstOperator('before')` is `'<'`.)
+ *
+ * An operator the spec does not know is passed through **verbatim**, so
+ * `isFilterAST()` still refuses it and the server still answers a loud `400`.
+ * A misspelling must never be coerced into a valid operator (AGENTS.md #0.1):
+ * silently reading `bfore` as `before` would return a plausible-looking wrong
+ * record set instead of an error the author can see.
+ *
+ * That applies to a rule which OMITS the operator too, and it is the one place
+ * this is a deliberate behaviour change rather than a pure fix. The deleted
+ * table opened with `case undefined: … return '='`, inventing an equality
+ * predicate for a rule that has no operator at all. `ViewFilterRuleSchema`
+ * REQUIRES `operator` (it is a bare `z.enum`, no default — an operator-less rule
+ * fails `safeParse` with `invalid_value`), so such a rule is off-spec metadata
+ * that publish validation refuses; silently answering it with `field = value`
+ * was a lenient consumer standing in for the contract. It now lowers to
+ * `[field, undefined, value]`, which `isFilterAST()` refuses — the same loud
+ * `400` every other off-spec spelling gets.
  */
 function normalizeTabPresets(tabs: any[]): Array<{ id: string; label: string; filters: any[]; default?: boolean }> {
   return (tabs || [])
@@ -133,7 +166,7 @@ function normalizeTabPresets(tabs: any[]): Array<{ id: string; label: string; fi
         : (Array.isArray(t.filter)
             ? t.filter
                 .filter((r: any) => r && typeof r.field === 'string')
-                .map((r: any) => [r.field, specOperatorToAst(r.operator), r.value])
+                .map((r: any) => [r.field, normalizeFilterOperator(r.operator), r.value])
             : []),
       default: t.default ?? t.isDefault,
     }));

packages/plugin-list/src/__tests__/UserFilters.tabPresetOperators.test.tsx

@@ -0,0 +1,198 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * Tab-preset operator lowering: `ViewTab.filter` → ObjectQL AST (#3470).
+ *
+ * `UserFilters` used to carry its OWN operator table (`specOperatorToAst`) —
+ * the second hand-kept operator map in this package — and it had drifted: it
+ * lowered `not_in`/`nin` to the SPACED `'not in'`, a spelling that appears in
+ * no spec vocabulary. `isFilterAST()` refuses it and the wire answers `400
+ * INVALID_FILTER`, so a tab preset written with the spec's own canonical
+ * spelling produced an empty list plus an error the moment it was clicked.
+ *
+ * The table is gone. Lowering is now purely structural — all 19
+ * `VIEW_FILTER_OPERATORS` are already `VALID_AST_OPERATORS` members — with the
+ * spec's OWN `normalizeFilterOperator` as the single exit, exactly as the write
+ * side (`app-shell/views/viewFilterFold.ts`) and `@object-ui/core`'s saved-view
+ * fold (#3431) do.
+ *
+ * These assertions pin the acceptance fact OFFLINE, using the spec's own
+ * `isFilterAST` — the very predicate that gates the filter server-side — so the
+ * fix cannot regress without a live backend in the loop. The corresponding LIVE
+ * measurement (published `@objectstack/*@17.0.0-rc.2` + app-showcase) is
+ * recorded on `normalizeTabPresets` and in the PR: `'not in'` → 400,
+ * `not_in` → 200 with the 8 rows the `!=` baseline returns, and `before`/`after`
+ * → byte-identical answers to `<`/`>` on both a `date` and a `datetime` field.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi } from 'vitest';
+import { render } from '@testing-library/react';
+import { isFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data';
+import {
+  VIEW_FILTER_OPERATORS,
+  VIEW_FILTER_OPERATOR_ALIASES,
+  ViewFilterRuleSchema,
+  normalizeFilterOperator,
+} from '@objectstack/spec/ui';
+import { UserFilters } from '../UserFilters';
+
+/**
+ * Render a tabs-mode `UserFilters` whose single preset is the given spec
+ * `ViewFilterRule`, and return the AST node it emits.
+ *
+ * The preset is the DEFAULT tab, so `TabFilters`' mount effect emits it without
+ * a click — the same path a user takes when a view opens on its default tab,
+ * and the path that shipped the 400.
+ */
+function emitFor(rule: Record<string, unknown>): unknown[] {
+  const onFilterChange = vi.fn();
+  render(
+    <UserFilters
+      config={{
+        element: 'tabs',
+        tabs: [{ name: 'preset', label: 'Preset', isDefault: true, filter: [rule] }],
+      }}
+      data={[]}
+      onFilterChange={onFilterChange}
+    />,
+  );
+  expect(onFilterChange).toHaveBeenCalledTimes(1);
+  return onFilterChange.mock.calls[0][0];
+}
+
+describe('UserFilters tab presets — the reported defect (#3470)', () => {
+  it('lowers a canonical `not_in` preset to a node isFilterAST ACCEPTS', () => {
+    // BEFORE this fix: emitted `['status', 'not in', ['done']]` — isFilterAST
+    // false, 400 INVALID_FILTER measured against a real backend. This is the
+    // assertion that flips red→green.
+    const node = emitFor({ field: 'status', operator: 'not_in', value: ['done'] });
+    expect(node).toEqual([['status', 'not_in', ['done']]]);
+    expect(
+      isFilterAST(node),
+      `a not_in tab preset produced ${JSON.stringify(node)}, which isFilterAST() rejects — `
+        + 'the server answers 400 INVALID_FILTER and the tab shows an empty list',
+    ).toBe(true);
+  });
+
+  it('folds the legacy `nin` spelling onto the canonical `not_in`', () => {
+    // Stored view metadata carries the shorthand: saveMeta persists the authored
+    // body verbatim, so the spec's own z.preprocess never reaches the row.
+    const node = emitFor({ field: 'status', operator: 'nin', value: ['done'] });
+    expect(node).toEqual([['status', 'not_in', ['done']]]);
+    expect(isFilterAST(node)).toBe(true);
+  });
+
+  it('never emits the spaced `not in`, which no spec vocabulary defines', () => {
+    for (const spelling of ['not_in', 'nin', 'notin', 'notIn']) {
+      const [[, operator]] = emitFor({ field: 'status', operator: spelling, value: ['done'] }) as [
+        [string, string, unknown],
+      ];
+      expect(operator, `authored '${spelling}' lowered to '${operator}'`).not.toBe('not in');
+      expect(operator).toBe('not_in');
+    }
+  });
+});
+
+describe('UserFilters tab presets — lowering is structural, not translated', () => {
+  it('reads both vocabularies from the spec', () => {
+    // Guards every it.each below against silently passing on an empty list.
+    expect(VIEW_FILTER_OPERATORS.length).toBe(19);
+    expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0);
+  });
+
+  it('needs no operator table at all: every view operator is already AST-valid', () => {
+    // This is WHY the private table could be deleted rather than repaired.
+    expect(VIEW_FILTER_OPERATORS.filter((op) => !VALID_AST_OPERATORS.has(op))).toEqual([]);
+  });
+
+  it.each(VIEW_FILTER_OPERATORS)('a `%s` preset survives the isFilterAST gate', (op) => {
+    const value = op === 'in' || op === 'not_in' ? ['a', 'b'] : op === 'between' ? [1, 2] : 'x';
+    const node = emitFor({ field: 'some_field', operator: op, value });
+    expect(
+      isFilterAST(node),
+      `a '${op}' tab preset produced ${JSON.stringify(node)}, which isFilterAST() rejects`,
+    ).toBe(true);
+  });
+
+  it.each(Object.keys(VIEW_FILTER_OPERATOR_ALIASES))(
+    'the legacy spelling `%s` lowers through the spec, not a local table',
+    (alias) => {
+      const [[, operator]] = emitFor({ field: 'f', operator: alias, value: ['a', 'b'] }) as [
+        [string, string, unknown],
+      ];
+      // The single-exit pin: whatever the spec says, verbatim. A second table
+      // reintroduced here fails this the moment it disagrees by one spelling.
+      expect(operator).toBe(normalizeFilterOperator(alias));
+      expect(VALID_AST_OPERATORS.has(operator)).toBe(true);
+    },
+  );
+
+  it('passes `before`/`after` through instead of rewriting them to `<`/`>`', () => {
+    // The one judgement call in #3470, settled by MEASUREMENT rather than
+    // assumption: on the live rc.2 backend the word and the symbol return
+    // identical status and identical record ids, on a `date` field and on a
+    // `datetime` field, in both directions. So the rewrite was a no-op and
+    // dropping it is a pure fix. (`canonicalAstOperator('before') === '<'`
+    // in the spec says the same thing independently.)
+    expect(emitFor({ field: 'due_date', operator: 'before', value: '2026-08-01' }))
+      .toEqual([['due_date', 'before', '2026-08-01']]);
+    expect(emitFor({ field: 'due_date', operator: 'after', value: '2026-08-01' }))
+      .toEqual([['due_date', 'after', '2026-08-01']]);
+  });
+
+  it('passes an unknown spelling through VERBATIM so the server rejects it loudly', () => {
+    // Direction note (honest reverse-verification): this one was GREEN BEFORE
+    // the change too — the deleted table's `default:` branch also passed
+    // unknowns through. It is here to pin the CONTRACT, not to flip: coercing
+    // `bfore` to `before` would answer a plausible-looking wrong record set
+    // instead of the 400 that tells the author their metadata is misspelled.
+    const node = emitFor({ field: 'due_date', operator: 'bfore', value: '2026-08-01' });
+    expect(node).toEqual([['due_date', 'bfore', '2026-08-01']]);
+    expect(isFilterAST(node)).toBe(false);
+  });
+
+  it('refuses to invent `=` for a rule that omits the operator', () => {
+    // The ONE deliberate behaviour change in #3470, and the only assertion here
+    // whose "before" was a passing 200 rather than a 400. The deleted table
+    // opened with `case undefined: … return '='`, so `{field, value}` with no
+    // operator lowered to `['status','=','x']` — accepted by isFilterAST, and
+    // answered by the server as a real equality predicate.
+    //
+    // `ViewFilterRuleSchema` REQUIRES `operator` (bare z.enum, no default), so
+    // that rule is off-spec metadata publish validation refuses. Inventing an
+    // answer for it was a lenient consumer standing in for the contract
+    // (AGENTS.md #0.1); it now earns the same loud 400 as any other off-spec
+    // spelling.
+    expect(ViewFilterRuleSchema.safeParse({ field: 'status', value: 'x' }).success).toBe(false);
+
+    const node = emitFor({ field: 'status', value: 'x' });
+    expect(isFilterAST(node)).toBe(false);
+    // What the old table would have produced, for contrast — accepted, and so
+    // silently answered as `status = 'x'`.
+    expect(isFilterAST([['status', '=', 'x']])).toBe(true);
+  });
+
+  it('leaves an already-lowered legacy `filters` triplet list untouched', () => {
+    // The other accepted tab shape (`{ id, label, filters: triplet[] }`) never
+    // went through the operator table and must not start now.
+    const onFilterChange = vi.fn();
+    render(
+      <UserFilters
+        config={{
+          element: 'tabs',
+          tabs: [{ id: 'active', label: 'Active', default: true, filters: [['status', '=', 'active']] }],
+        } as never}
+        data={[]}
+        onFilterChange={onFilterChange}
+      />,
+    );
+    expect(onFilterChange).toHaveBeenCalledWith([['status', '=', 'active']]);
+  });
+});

packages/plugin-list/src/__tests__/UserFilters.test.tsx

@@ -114,8 +114,12 @@ describe('UserFilters — selection persistence (ADR-0047)', () => {
       />,
     );
 
-    // Restored tab wins over the isDefault tab and emits its preset filter
-    expect(onFilterChange).toHaveBeenCalledWith([['priority', '=', 'urgent']]);
+    // Restored tab wins over the isDefault tab and emits its preset filter.
+    // The operator stays the spec's canonical `equals` — lowering a rule to an
+    // AST node is structural and translates nothing (#3470); `equals` is itself
+    // a `VALID_AST_OPERATORS` member, measured 200 on a live backend. Operator
+    // coverage proper lives in `UserFilters.tabPresetOperators.test.tsx`.
+    expect(onFilterChange).toHaveBeenCalledWith([['priority', 'equals', 'urgent']]);
   });
 
   it('reports tab switches through onSelectionsChange', () => {