From 3d8369150396582f47d6f3351efac6b76072b3ea Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 06:09:56 +0000 Subject: [PATCH 1/2] fix(plugin-list): fold the operator in convertFilterGroupToAST so a canonical row still filters A list view whose stored filter used the spec's canonical operator spelling queried with NO filter at all and returned every record, while the filter panel showed the condition applied. Nothing errored. The function read the row's operator RAW against VALUELESS_FILTER_BUILDER_OPERATORS (the FilterBuilder's six camelCase dropdown ids), while the completeness test it falls through to does fold, through the spec's normalizeFilterOperator, to decide arity. Two halves of one predicate, two vocabularies: a row spelled is_null missed the value-less short-circuit, landed on scalar, had its empty value read as an unfinished row, and was dropped. Both raw reads now fold, and so do the isEmpty/isNotEmpty arms that resolve to a null comparison ahead of mapOperator - leaving those on literal ids would have repaired is_null and left is_empty emitting a different node from its twin, which is the defect rather than a fix for it. The exported set's membership is unchanged, deliberately: app-shell's fold documents its own value-less set as that set PLUS the canonical spellings only that layer sees, and widening the export would make that compensation redundant by side effect. Same shape as the sibling repair at the builder's value-input gate. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .../9359-list-ast-valueless-canonical-fold.md | 50 +++ packages/plugin-list/src/ListView.tsx | 86 ++++- ...FilterGroupToAST.canonicalSpelling.test.ts | 303 ++++++++++++++++++ 3 files changed, 434 insertions(+), 5 deletions(-) create mode 100644 .changeset/9359-list-ast-valueless-canonical-fold.md create mode 100644 packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts diff --git a/.changeset/9359-list-ast-valueless-canonical-fold.md b/.changeset/9359-list-ast-valueless-canonical-fold.md new file mode 100644 index 0000000000..5f1fc62810 --- /dev/null +++ b/.changeset/9359-list-ast-valueless-canonical-fold.md @@ -0,0 +1,50 @@ +--- +'@object-ui/plugin-list': patch +--- + +fix(plugin-list): `convertFilterGroupToAST` folds the operator, so a saved view spelled canonically queries the filter it shows (objectui#9359) + +NOT cosmetic, and not a new defect: a list view whose stored filter used the +spec's canonical operator spelling QUERIED WITH NO FILTER AT ALL and returned +every record, while the filter panel showed the condition applied. Nothing +errored and the result set looked plausible. + +`convertFilterGroupToAST` read the row's operator RAW, against +`VALUELESS_FILTER_BUILDER_OPERATORS` — the FilterBuilder's six camelCase +dropdown ids. The completeness test it falls through to +(`isFilterValueComplete`) DOES fold, through the spec's +`normalizeFilterOperator`, to decide arity. So the two halves of one predicate +spoke different vocabularies: a row spelled `is_null` missed the value-less +short-circuit, landed on `scalar`, had its `value: ''` read as an unfinished +row, and was dropped. Measured through the real converter on one `text` column: + + isNull (dropdown id) -> ["title","isnull",null] + is_null (canonical) -> [] <- the defect + equals + value "acme" -> ["title","=","acme"] <- control, fires + +The canonical spelling is not exotic — it is what `foldFilterGroupToSpecRules` +persists when the user saves the panel's group as a view, and what any +spec-side producer emits. So a saved view could PERSIST correctly and still +QUERY as though it had no filter. + +This is the same failure objectui#4744 repaired for the dropdown's own +spellings, reached by the other vocabulary. Both raw reads in this function now +fold, as do the two `isEmpty` / `isNotEmpty` arms that resolve to a null +comparison ahead of `mapOperator` — leaving those on literal ids would have +fixed `is_null` and left `is_empty` emitting a different node from its twin, +which is this defect rather than a repair of it. All four canonical spellings +now emit exactly what their camelCase twins emit. + +**The exported set is unchanged, deliberately.** It states a fact about what the +BUILDER'S DROPDOWN draws, and `app-shell`'s `foldFilterGroupToSpecRules` +documents its own value-less set as that set PLUS the canonical spellings only +that layer sees. Widening the export would have made that layer's deliberate +compensation redundant by side effect, in a file nobody is editing. The defect +was a reader that forgot to normalize its input, so the reader is what was +repaired — the same shape the sibling repair used at the builder's value-input +gate (objectui#9302). No published type or export changes, and no stored +operator is rewritten: converting is not migrating. + +Which operator vocabulary should WIN is a separate, still-open question and is +not decided here. The `contains` / `icontains` boundary is untouched and pinned: +the fold this reader routes through maps neither onto the other. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index dfbbd7a5ca..251749481b 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -31,7 +31,7 @@ import { useObjectLabel, useSafeFieldLabel, createSafeTranslation, useDisplayLoc // objectui's keyed `{ key, defaultValue, params }` ref — that vocabulary lives // on the FLAT `schema.ariaLabel` and is resolved by `SchemaRenderer` instead // (objectui#5134). -import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; +import { resolveI18nLabel as resolveInlineI18nLabel, normalizeFilterOperator } from '@objectstack/spec/ui'; import { usePermissions } from '@object-ui/permissions'; /** @@ -300,6 +300,63 @@ export interface ListViewProps { // Helper to convert FilterBuilder group to ObjectStack AST. // Accepts both the FilterBuilder vocabulary (camelCase) and the // @objectstack/spec ViewFilterRule vocabulary (snake_case). + +/** + * The shared value-less set, re-keyed by the spelling + * `normalizeFilterOperator` folds each member to — the lookup table + * `convertFilterGroupToAST` reads (objectui#9359). + * + * DERIVED, never a second literal: a hand-kept canonical copy beside the + * exported set is exactly how the two would come to disagree, and the + * disagreement is invisible — a filter the panel shows and the query does not + * carry. + * + * Why it exists here instead of the export being widened: the exported set + * states a fact about what the BUILDER'S DROPDOWN draws, and its members are + * that dropdown's own camelCase ids. Two other layers read it — this function + * (what the live grid QUERIES) and `app-shell`'s `foldFilterGroupToSpecRules` + * (what a saved view PERSISTS, already documented as this set PLUS the + * canonical spellings only that layer sees). Folding the canonical spellings + * INTO the export would make that layer's deliberate compensation redundant by + * side effect, in a file nobody is editing. The defect was never a set missing + * members; it was a reader that forgot to normalize its input, so the reader is + * what is repaired. Same shape the sibling repair used at the builder's own + * value-input gate (objectui#9302). + * + * `exists` / `notExists` fold to themselves — the spec's vocabulary has no + * existence operator and `VIEW_FILTER_OPERATOR_ALIASES` deliberately has no row + * for either — so this set is the same SIZE as the one it derives from. + */ +const VALUELESS_FILTER_BUILDER_OPERATORS_CANONICAL: ReadonlySet = new Set( + [...VALUELESS_FILTER_BUILDER_OPERATORS].map(op => String(normalizeFilterOperator(op))), +); + +/** + * Is this row COMPLETE without a value — asked of whichever spelling the row + * actually carries (objectui#9359). + * + * The two halves of one predicate used to speak different vocabularies. The + * value-less short-circuit did a raw `has()` on the exported set's camelCase + * ids, while the completeness test it falls through to + * (`isFilterValueComplete`) DOES fold, through this same + * `normalizeFilterOperator`, to decide arity. So a row spelled `is_null` — the + * spec's canonical form, which is what `foldFilterGroupToSpecRules` persists + * and what any spec-side producer emits — missed the short-circuit, landed on + * `scalar`, had its `value: ''` read as an unfinished row and was DROPPED. The + * function returned `[]`, the grid queried with no filter at all, and every + * record came back while the panel showed a filter applied. Silent. + * + * That is the same failure objectui#4744 repaired for the dropdown's own + * spellings — recorded in the exported set's docblock — reached by the other + * vocabulary. Folding here is one more site joining a fold this file already + * performs (`mapOperator` already matches case- and underscore-insensitively, + * and `isFilterValueComplete` folds through the spec's map) rather than a new + * dialect. + */ +function isValuelessFilterOperator(operator: string): boolean { + return VALUELESS_FILTER_BUILDER_OPERATORS_CANONICAL.has(String(normalizeFilterOperator(operator))); +} + /** * Filter-builder / view operator → filter-AST operator. * @@ -591,7 +648,14 @@ export function convertFilterGroupToAST(group: FilterGroup): any[] { // `value: ''` by `addCondition`, and left that way because the operator // dropdown preserves `value` — was dropped as unfinished. The grid then // applied NO filter while the panel showed one. - if (VALUELESS_FILTER_BUILDER_OPERATORS.has(c.operator)) return true; + // + // Read through `isValuelessFilterOperator`, which folds the row's + // spelling before the lookup (objectui#9359): the set's members are the + // dropdown's camelCase ids, so a stored `is_null` — the canonical form a + // saved view carries — used to miss this short-circuit entirely and be + // dropped by the completeness test below, which folds. Same silent + // outcome as the #4744 defect, reached by the other vocabulary. + if (isValuelessFilterOperator(c.operator)) return true; // Skip incomplete rows (no value entered yet). Emitting `[field, op, '']` // would be a silently-wrong filter (matches only empty) rather than // "no filter", excluding all rows. Matches groupToCondition in @@ -609,8 +673,14 @@ export function convertFilterGroupToAST(group: FilterGroup): any[] { return isFilterValueComplete(c.operator, c.value); }) .map(c => { - if (c.operator === 'isEmpty') return [c.field, '=', null]; - if (c.operator === 'isNotEmpty') return [c.field, '!=', null]; + // Folded, not compared raw (objectui#9359). These two arms resolve to a + // null comparison BEFORE `mapOperator` is consulted, so leaving them on + // literal camelCase ids would have made the repair below reach `is_null` + // and not `is_empty` — trading one spelling-dependent answer for another, + // which is the defect this card is about rather than a fix for it. + const canonicalOperator = String(normalizeFilterOperator(c.operator)); + if (canonicalOperator === 'is_empty') return [c.field, '=', null]; + if (canonicalOperator === 'is_not_empty') return [c.field, '!=', null]; // A value-less row's third slot is emitted as `null` rather than as // whatever `c.value` still holds: the operator dropdown PRESERVES the // previous operator's value, so an `Is null` row can carry a leftover @@ -619,7 +689,13 @@ export function convertFilterGroupToAST(group: FilterGroup): any[] { // for `isnull`/`isnotnull` — it emits `{ [field]: { $null: true|false } }` // — so `null` is inert on the wire and keeps the emission a function of // the operator alone. Same shape the `isEmpty` arms above already use. - if (VALUELESS_FILTER_BUILDER_OPERATORS.has(c.operator)) { + // The same fold as the short-circuit above (objectui#9359): a row kept + // BECAUSE it is value-less must also be EMITTED as value-less, or the + // canonical spelling would carry its stale `value` into the third slot + // while the camelCase one carried `null` — one operator, two nodes. + // `mapOperator` already collapses case and underscores, so `is_null` + // lands on the same `isnull` its dropdown twin does. + if (isValuelessFilterOperator(c.operator)) { return [c.field, mapOperator(c.operator), null]; } return [c.field, mapOperator(c.operator), c.value]; diff --git a/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts new file mode 100644 index 0000000000..f6f2443fda --- /dev/null +++ b/packages/plugin-list/src/__tests__/convertFilterGroupToAST.canonicalSpelling.test.ts @@ -0,0 +1,303 @@ +/** + * 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. + */ + +/** + * `convertFilterGroupToAST` reads the operator through the same fold the rest + * of the predicate already uses, so ONE value-less operator emits ONE node + * whichever spelling the stored row carries (objectui#9359). + * + * ## The defect, in one sentence + * + * The two halves of one predicate spoke different vocabularies. The value-less + * short-circuit did a raw `has()` against `VALUELESS_FILTER_BUILDER_OPERATORS` + * — six camelCase dropdown ids — while the completeness test it falls through + * to (`isFilterValueComplete`) DOES fold, through the spec's + * `normalizeFilterOperator`, to decide arity. So a row spelled `is_null` — the + * spec's canonical form, which is exactly what `foldFilterGroupToSpecRules` + * persists into a saved view — missed the short-circuit, landed on `scalar`, + * had its `value: ''` read as an unfinished row, and was DROPPED. The function + * then returned `[]`: no filter at all. The live grid returned every record + * while the panel showed a filter applied. Nothing errored. + * + * This is the same failure the exported set was created to prevent, recorded + * verbatim in its own docblock — *"the live-grid copy listed only + * `isEmpty`/`isNotEmpty` … the grid read that as an unfinished row, dropped it, + * and applied NO filter at all while the panel showed one. Silent, and every + * record came back."* objectui#4744 repaired it for the dropdown's spellings + * and left the canonical ones on the old path. + * + * ## What the repair deliberately does NOT do (the ruling on objectui#9302, + * which this card is covered by) + * + * - ⛔ the EXPORTED set is not widened. Its stated job is *which rows the + * builder leaves value-less* — a fact about that dropdown's own six ids — + * and `app-shell`'s `foldFilterGroupToSpecRules` documents its own + * `VALUELESS_FILTER_OPERATORS` as that set PLUS the canonical spellings + * only that layer sees. Widening the export would make another layer's + * deliberate compensation redundant by SIDE EFFECT, in a file nobody is + * editing. Pinned below, and pinned identically by the sibling repair in + * `packages/components`. + * - ⛔ no row's stored `operator` is rewritten. This function converts; it + * does not migrate. + * - ⛔ no second spelling joins the toolbar's dropdown. + * + * ⛔ Out of scope and untouched: WHICH operator vocabulary wins (decision card + * objectui#9306). This is a defect at one reader regardless of how that lands. + * + * ⛔ And the fold does not cross the `contains` / `icontains` boundary — + * objectui#7379 holds that is "a semantic boundary, not two spellings of one + * thing", and `VIEW_FILTER_OPERATOR_ALIASES` has no row for either. Pinned by + * the boundary guard at the bottom. + * + * ## DIRECTION, predicted before the first run + * + * On the unmodified tree the four CANONICAL rows (`is_null`, `is_not_null`, + * `is_empty`, `is_not_empty`) are RED — each emits `[]` where a real node is + * required. Everything else is GREEN in both directions and is carried for a + * named reason rather than for coverage: + * + * - the six DROPDOWN ids emit their node today and must keep emitting the + * SAME node — the over-reach guard. A repair that folded only one + * direction, or that replaced the set's members with canonical spellings, + * moves these; + * - `equals` with a value is the FIRING CONTROL, in the same run. Without it + * "every row emits something" and "the converter is dead" read alike; + * - `exists` / `notExists` have no canonical twin — the spec's vocabulary has + * no existence operator and its alias table deliberately has no row for + * one — so the fold returns them verbatim. They pin that routing the + * lookup through a fold did not drop the two members with nothing to fold + * to. (They are `OPT_IN_OPERATORS` and this toolbar offers neither; the + * emission is unreachable in the product and pinned here so a future + * decision to offer them lands as a red test, exactly as + * `convertFilterGroupToAST.test.ts` already pins it.) + */ +import { describe, it, expect } from 'vitest'; +import { normalizeFilterOperator } from '@objectstack/spec/ui'; +import { isFilterAST } from '@objectstack/spec/data'; +import { VALUELESS_FILTER_BUILDER_OPERATORS } from '@object-ui/components'; +import type { FilterGroup } from '@object-ui/components'; +import { convertFilterGroupToAST } from '../ListView'; + +/** + * One row, in the state the panel actually holds: `addCondition` seeds + * `{ operator: 'equals', value: '' }` and the operator dropdown updates + * `operator` ALONE, so a value-less row keeps the `''` seed. A stored view + * folded by `foldFilterGroupToSpecRules` carries no `value` key at all for + * these operators, so both shapes are swept below. + */ +const emit = (operator: string, value: unknown = '') => + convertFilterGroupToAST({ + id: 'root', + logic: 'and', + conditions: [{ id: 'c1', field: 'title', operator, value }], + } as unknown as FilterGroup); + +/** + * The card's measurement table, turned into a pin. + * + * `dialect` is load-bearing for reading a failure, not decoration: + * - `dropdown` — one of the builder's own camelCase ids, i.e. a literal + * member of the exported set. Emits its node today and after: over-reach + * guard; + * - `canonical` — `@objectstack/spec`'s spelling of the SAME operator, which + * is what a saved view stores. `[]` today (the defect), the node after: + * the firing cases; + * - `control` — really does take a value, and has one. + */ +const ROWS: ReadonlyArray<{ + operator: string; + value?: unknown; + emitted: unknown[]; + dialect: 'dropdown' | 'canonical' | 'control'; +}> = [ + { operator: 'isNull', emitted: ['title', 'isnull', null], dialect: 'dropdown' }, + { operator: 'is_null', emitted: ['title', 'isnull', null], dialect: 'canonical' }, + { operator: 'isNotNull', emitted: ['title', 'isnotnull', null], dialect: 'dropdown' }, + { operator: 'is_not_null', emitted: ['title', 'isnotnull', null], dialect: 'canonical' }, + // `isEmpty` / `isNotEmpty` are resolved to a null comparison BEFORE + // `mapOperator` is consulted, so their canonical twins must land on the same + // arm — otherwise the repair would trade one spelling-dependent answer for + // another, which is the defect this card is about. + { operator: 'isEmpty', emitted: ['title', '=', null], dialect: 'dropdown' }, + { operator: 'is_empty', emitted: ['title', '=', null], dialect: 'canonical' }, + { operator: 'isNotEmpty', emitted: ['title', '!=', null], dialect: 'dropdown' }, + { operator: 'is_not_empty', emitted: ['title', '!=', null], dialect: 'canonical' }, + // No canonical twin exists for these two. + { operator: 'exists', emitted: ['title', 'exists', null], dialect: 'dropdown' }, + { operator: 'notExists', emitted: ['title', 'notExists', null], dialect: 'dropdown' }, + // THE FIRING CONTROL, in the same run as the rest. + { operator: 'equals', value: 'acme', emitted: ['title', '=', 'acme'], dialect: 'control' }, +]; + +describe('objectui#9359 — one operator, one emitted node, whichever spelling it arrives in', () => { + it.each(ROWS)('$dialect `$operator` emits $emitted', ({ operator, value, emitted }) => { + expect( + emit(operator, value ?? ''), + `a "${operator}" row emitted nothing or the wrong node, so the live grid queries ` + + 'without it: the panel shows a filter applied and every record comes back', + ).toEqual(emitted); + }); + + it.each(ROWS.filter((r) => r.dialect === 'canonical'))( + 'canonical `$operator` emits the SAME node as its dropdown twin', + ({ operator, emitted }) => { + // The acceptance criterion stated directly: two spellings of one operator + // are one filter. Asserted against the twin's own live emission rather + // than against the literal above, so the two can never drift apart in + // this file while agreeing in the product (or the reverse). + const twin = ROWS.find((r) => r.dialect === 'dropdown' && r.emitted[1] === emitted[1]); + expect(twin, `no dropdown twin listed for ${operator}`).toBeDefined(); + expect(emit(operator)).toEqual(emit(twin!.operator)); + }, + ); + + it.each(['is_null', 'is_not_null', 'is_empty', 'is_not_empty'])( + 'a stored `%s` rule with NO value key at all is still a filter', + (operator) => { + // What `foldFilterGroupToSpecRules` actually persists: it writes `value` + // only when the operator takes one, so the rule read back from storage + // has no `value` property. `undefined` is the other shape the same row + // reaches this function in, and it must not be read as unfinished either. + const node = convertFilterGroupToAST({ + id: 'root', + logic: 'and', + conditions: [{ id: 'c1', field: 'title', operator }], + } as unknown as FilterGroup); + expect(node.length, `a stored "${operator}" rule emitted nothing`).toBeGreaterThan(0); + }, + ); + + it.each(['is_null', 'is_not_null', 'is_empty', 'is_not_empty', 'isNull', 'isEmpty'])( + '`%s` survives isFilterAST, the gate that decides if the filter is parsed at all', + (operator) => { + // A non-empty node the AST gate rejects is no better than `[]` — worse, + // since driver-sql drops the whole filter without erroring. Emitting a + // node is only half the repair. + const node = emit(operator); + expect(node.length, `${operator} emitted no node`).toBeGreaterThan(0); + expect( + isFilterAST(node), + `${JSON.stringify(node)} is rejected by isFilterAST(); the filter reaches the wire ` + + 'in a spelling the server will not compile', + ).toBe(true); + }, + ); + + it('emits the same node whatever stale value a canonical row still carries', () => { + // The operator dropdown preserves `value`, so a row stored as `is_null` can + // hold text typed under the previous operator — invisible, because no value + // input is drawn. The emission must be a function of the operator alone. + expect(emit('is_null', 'typed under equals')).toEqual(['title', 'isnull', null]); + }); + + it('keeps a canonical value-less row alongside a complete one', () => { + const both = convertFilterGroupToAST({ + id: 'root', + logic: 'and', + conditions: [ + { id: 'c1', field: 'stage', operator: 'equals', value: 'won' }, + { id: 'c2', field: 'closed_at', operator: 'is_null', value: '' }, + ], + } as unknown as FilterGroup); + expect(both).toEqual(['and', ['stage', '=', 'won'], ['closed_at', 'isnull', null]]); + expect(isFilterAST(both)).toBe(true); + }); +}); + +describe('objectui#9359 — the instrument can actually fire', () => { + it('every canonical case is outside the exported set and folds onto a member', () => { + // The instrument standard. A table built only on spellings the raw `has()` + // already matched would be green before ANY repair and would measure + // nothing at all. + const canonical = ROWS.filter((r) => r.dialect === 'canonical'); + expect(canonical.length).toBeGreaterThanOrEqual(4); + const foldedMembers = new Set( + [...VALUELESS_FILTER_BUILDER_OPERATORS].map((op) => String(normalizeFilterOperator(op))), + ); + for (const row of canonical) { + // Not a literal member — so the raw lookup could not have matched it… + expect(VALUELESS_FILTER_BUILDER_OPERATORS.has(row.operator)).toBe(false); + // …and it folds ONTO a member, which is what makes the repair reach it. + expect(foldedMembers.has(String(normalizeFilterOperator(row.operator)))).toBe(true); + } + // …and the control is in neither, which is why it keeps needing a value. + expect(VALUELESS_FILTER_BUILDER_OPERATORS.has('equals')).toBe(false); + expect(normalizeFilterOperator('equals')).toBe('equals'); + }); + + it('the fold adds no members — it only re-keys the six', () => { + // `exists` / `notExists` fold to themselves, so the derived set is the same + // SIZE as the one it derives from. A fold that collapsed two members onto + // one would silently shrink the set the gate consults. + const folded = new Set( + [...VALUELESS_FILTER_BUILDER_OPERATORS].map((op) => String(normalizeFilterOperator(op))), + ); + expect(folded.size).toBe(VALUELESS_FILTER_BUILDER_OPERATORS.size); + }); +}); + +describe('objectui#9359 — ⛔ the repair moves nothing but this reader', () => { + it('the EXPORTED set keeps its dropdown-only membership', () => { + // Acceptance criterion 4, and the ruling's whole point: two other layers + // read this set and one of them already compensates for the canonical + // spellings. Widening it would make that layer's deliberate half redundant + // by side effect. Green in both directions by construction — it fails only + // for a repair that widened the export instead of folding at the reader. + expect([...VALUELESS_FILTER_BUILDER_OPERATORS].sort()).toEqual([ + 'exists', + 'isEmpty', + 'isNotEmpty', + 'isNotNull', + 'isNull', + 'notExists', + ]); + }); + + it('an operator nothing knows still needs a value', () => { + // The default must stay "takes a value". A fold that swallowed unknown + // spellings into the value-less set would emit a filter for a row the user + // never finished — the objectui#4744 failure, inverted. + expect(normalizeFilterOperator('totally_unknown')).toBe('totally_unknown'); + expect(emit('totally_unknown', '')).toEqual([]); + expect(emit('totally_unknown', 'v')).toEqual(['title', 'totally_unknown', 'v']); + }); + + it('a value-taking row with no value is still dropped, in both vocabularies', () => { + // The other half of the same gate: folding must not make every row look + // finished. `not_in` is the canonical spelling of a value-taking operator. + expect(emit('equals', '')).toEqual([]); + expect(emit('not_in', [])).toEqual([]); + expect(emit('between', ['2024-01-01', ''])).toEqual([]); + }); +}); + +describe('objectui#9359 — ⛔ the fold does not cross the `contains` boundary', () => { + it('`contains` and `icontains` are not folded onto each other', () => { + // objectui#7379: these "must never be folded onto" each other — "That is a + // semantic boundary, not two spellings of one thing." + // `VIEW_FILTER_OPERATOR_ALIASES` has no row for either. This reader now + // routes a lookup through that fold, so pin that the boundary still stands. + // A seat that folds those two has broken a ruling, not fixed a bug. Green + // in both directions by construction — a boundary guard, not a control. + expect(normalizeFilterOperator('contains')).toBe('contains'); + expect(normalizeFilterOperator('icontains')).toBe('icontains'); + expect(normalizeFilterOperator('contains')).not.toBe(normalizeFilterOperator('icontains')); + // …and neither is value-less under either spelling, so the gate this card + // repairs never had an opinion about them. + const folded = new Set( + [...VALUELESS_FILTER_BUILDER_OPERATORS].map((op) => String(normalizeFilterOperator(op))), + ); + for (const op of ['contains', 'icontains', 'containsCaseInsensitive']) { + expect(VALUELESS_FILTER_BUILDER_OPERATORS.has(op)).toBe(false); + expect(folded.has(String(normalizeFilterOperator(op)))).toBe(false); + } + // And they still reach the wire as distinct AST operators from this reader. + expect(emit('contains', 'ac')).toEqual(['title', 'contains', 'ac']); + expect(emit('icontains', 'ac')).toEqual(['title', 'icontains', 'ac']); + }); +}); From e77b2e8f84b36ce8a4e29cdb8579e380855d196a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 16:51:53 +0000 Subject: [PATCH 2/2] docs(changeset): correct the reachability narration on the value-less fold The changeset's headline and one paragraph claimed that a SAVED view spelled canonically "could persist correctly and still query as though it had no filter". Measured in a worktree at this branch's head, that route does not reach this reader. A stored ViewFilterRule[] arrives as ListView's schema.filter, which buildEffectiveFilter passes as its BASE-FILTER argument to @object-ui/core's mergeFilterNodes/toFilterNode; toFilterNode lowers it through viewFilterRuleToNode, which already normalizes the operator. Measured: { field: closed_at, operator: is_null, value: '' } lowers to [["closed_at","is_null",""]] and isFilterAST accepts it. The panel group that convertFilterGroupToAST converts is empty on that load. Census over all 7652 tracked files (whole-file slurp, not line-anchored): convertFilterGroupToAST has exactly one production caller (buildEffectiveFilter); currentFilters has exactly one setter call site (the FilterBuilder panel's onChange) and one initialiser (the initialFilters prop); initialFilters is passed at exactly one production site, from readListFilterState - a per-browser localStorage cache written from that same panel. Both producers carry the dropdown's camelCase ids. The defect itself is untouched and remains real: the comment above the function declares it accepts BOTH the FilterBuilder vocabulary and the @objectstack/spec ViewFilterRule vocabulary, and it dropped a COMPLETE row of the second, emitting no filter rather than an error. The measured isNull/is_null/control block, the exported-set paragraph, the contains boundary note and the frontmatter are unchanged. No code changes. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt --- .../9359-list-ast-valueless-canonical-fold.md | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/.changeset/9359-list-ast-valueless-canonical-fold.md b/.changeset/9359-list-ast-valueless-canonical-fold.md index 5f1fc62810..9119107a77 100644 --- a/.changeset/9359-list-ast-valueless-canonical-fold.md +++ b/.changeset/9359-list-ast-valueless-canonical-fold.md @@ -2,12 +2,12 @@ '@object-ui/plugin-list': patch --- -fix(plugin-list): `convertFilterGroupToAST` folds the operator, so a saved view spelled canonically queries the filter it shows (objectui#9359) +fix(plugin-list): `convertFilterGroupToAST` folds the operator, so a canonical value-less row emits its node instead of being dropped (objectui#9359) -NOT cosmetic, and not a new defect: a list view whose stored filter used the -spec's canonical operator spelling QUERIED WITH NO FILTER AT ALL and returned -every record, while the filter panel showed the condition applied. Nothing -errored and the result set looked plausible. +NOT cosmetic, and not a new defect: a filter row carrying the spec's canonical +operator spelling QUERIED WITH NO FILTER AT ALL and returned every record, +while the filter panel showed the condition applied. Nothing errored and the +result set looked plausible. `convertFilterGroupToAST` read the row's operator RAW, against `VALUELESS_FILTER_BUILDER_OPERATORS` — the FilterBuilder's six camelCase @@ -22,10 +22,22 @@ row, and was dropped. Measured through the real converter on one `text` column: is_null (canonical) -> [] <- the defect equals + value "acme" -> ["title","=","acme"] <- control, fires -The canonical spelling is not exotic — it is what `foldFilterGroupToSpecRules` -persists when the user saves the panel's group as a view, and what any -spec-side producer emits. So a saved view could PERSIST correctly and still -QUERY as though it had no filter. +WHO REACHES THIS READER — measured, not assumed. Its one production caller is +`buildEffectiveFilter`, and the argument it converts is the list toolbar's own +FilterBuilder group: live in the session, or restored per browser by +`writeListFilterState`. Both carry the dropdown's camelCase ids, so the +canonical spelling has no measured producer into this reader today. A SAVED +view does NOT arrive here — its stored `ViewFilterRule[]` travels +`schema.filter` into the base-filter argument of that same call and is lowered +by `@object-ui/core`'s `toFilterNode` / `viewFilterRuleToNode`, which already +folds: `{ field: 'closed_at', operator: 'is_null', value: '' }` lowers to +`["closed_at","is_null",""]`, accepted by `isFilterAST`. So what is repaired is +not a live saved-view outage. It is a reader that contradicted its OWN declared +contract — the comment above it states it accepts both the FilterBuilder +vocabulary and the `@objectstack/spec` `ViewFilterRule` vocabulary, and it +dropped a COMPLETE row of the second one, emitting no filter rather than an +error. A reader that drops a complete row is a defect whether or not today's +saved-view path happens to pre-fold. This is the same failure objectui#4744 repaired for the dropdown's own spellings, reached by the other vocabulary. Both raw reads in this function now