diff --git a/.changeset/view-filter-rule-value-shaped-by-operator.md b/.changeset/view-filter-rule-value-shaped-by-operator.md new file mode 100644 index 0000000000..23a285700c --- /dev/null +++ b/.changeset/view-filter-rule-value-shaped-by-operator.md @@ -0,0 +1,80 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): a view filter rule's `value` must have the shape its OPERATOR can execute (#6227) + + + +`ViewFilterRuleSchema.value` was declared +`string | number | boolean | null | (string | number)[]` with **no coupling to +`operator`**, so every operator accepted every shape. A set operator carrying a +scalar — `{ field: 'stage', operator: 'not_in', value: 'won' }` — was a +spec-valid view filter rule. It published cleanly, and then failed when someone +opened the view. + +That made the failure two-stage. #5869 / PR #6209 had already closed the runtime +half: `assertListComparandShapes` refuses the lowered `{ stage: { $nin: 'won' } }` +with a named `400 INVALID_FILTER` (a `500 DATABASE_ERROR` before it). Correct +refusal, wrong moment — by then the author is long gone, and the view had been +sitting in the store looking valid. The authoring surface now refuses the same +shapes at publish time, so the feedback reaches the person who can act on it. + +**The tightening mirrors the runtime gate exactly — three constraints, one for +one, and deliberately nothing more:** + +| operator | `value` must be | why | +|---|---|---| +| `in` / `not_in` (and the `nin` / `notIn` / `notin` spellings) | an array, any length | lowers to `$in` / `$nin` | +| `between` | exactly `[min, max]` | lowers to `$between` | +| everything else | unchanged | the query path does not judge them | + +It goes no further on purpose. #5685 already ruled on the opposite error — a +schema stricter than the runtime "in ways the runtime deliberately allows" was +found to be the wrong side and was widened to match — so these all still parse: + +- `in: []` — an empty list is a declared predicate ("matches nothing" / + "matches everything"), and both drivers say so. +- `equals: ['a', 'b']` — lowers to a deep-equality comparand every backend answers. +- `contains: 5` — no backend refuses it. +- `is_empty: ''` — the null predicates take their direction from the operator + **name**; `convertComparison` ignores the value position, and the ObjectUI + client deliberately sends a truthy placeholder there. + +The refusal names the operator, the field, the shape received and the shape to +write: + +``` +Operator "not_in" on field "stage" requires an ARRAY of values. Received a +string ("won"). "not_in" tests membership of a list — write ["won"] for a single +value, or use "not_equals" to compare against it. An empty list [] is allowed and +is a real predicate. This is refused at authoring time because the query path +refuses it too (400 INVALID_FILTER, #5869). +``` + +**Migration.** A filter rule whose operator is `in`, `not_in` or `between` and +whose `value` is not an array of the right arity now fails to parse; `os validate` +and `os lint` report each one by path. Wrap a single value in a list +(`value: 'won'` → `value: ['won']`) or complete the range's second bound. + +Two checks are worth doing where they look unnecessary. A rule reading +`operator: 'in', value: ''` is an **unfinished** row, not a filter — decide what +it was meant to select rather than mechanically rewriting it to `[""]`, which is +a real and different predicate. And a view that already carried one of these +shapes **was never returning filtered rows**: it answered `400 INVALID_FILTER` on +render, so re-check what the view is supposed to show rather than assuming the +old result set was correct. + +**Metadata at rest is not rewritten, and there is no D2 conversion.** The read +path does not re-validate stored rows, so no stored view becomes unreadable; what +changes is that re-saving one is refused at the write gate, naming `value`. A +conversion was considered and rejected: this shape was never written by any +first-party producer (measured — every `in` / `not_in` rule across this repo, +`objectui` and `cloud` already carries an array) and has never executed, so +coercing it at load would be the platform guessing intent rather than replaying a +rename — and it cannot guess honestly, since `between: 5` has no defensible +second bound. + +Two operator vocabularies are now exported — +`VIEW_FILTER_LIST_VALUE_OPERATORS` and `VIEW_FILTER_PAIR_VALUE_OPERATORS` — so a +producer can ask the question the schema asks instead of keeping its own copy. diff --git a/content/docs/references/ui/view.mdx b/content/docs/references/ui/view.mdx index 3754b24c35..db5904389f 100644 --- a/content/docs/references/ui/view.mdx +++ b/content/docs/references/ui/view.mdx @@ -829,7 +829,7 @@ View filter rule | :--- | :--- | :--- | :--- | | **field** | `string` | ✅ | Field name to filter on | | **operator** | `Enum<'equals' \| 'not_equals' \| 'contains' \| 'not_contains' \| 'starts_with' \| 'ends_with' \| 'greater_than' \| 'less_than' \| 'greater_than_or_equal' \| … +10 more>` | ✅ | Filter operator | -| **value** | `string \| number \| boolean \| null \| (string \| number)[]` | optional | Filter value | +| **value** | `string \| number \| boolean \| null \| (string \| number)[]` | optional | Filter value. The accepted SHAPE depends on the operator: `in` / `not_in` take an array (any length, including []), `between` takes exactly [min, max], every other operator takes a scalar. The unary operators (is_empty / is_not_empty / is_null / is_not_null) take their direction from the operator name and ignore this key. | ### Allowed Values: `ViewFilterRule.operator` diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 74e69c8409..637b64efe7 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -422,6 +422,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674 - **`import-run-automations-declared-default-corrected`** — `api.ImportRequest runAutomations — the declared default of the key on BOTH import bodies, POST /api/v1/data/:object/import (ImportRequest) and its async twin POST /api/v1/data/:object/import/jobs (CreateImportJobRequest, which IS the same schema object). It was declared default(false) and described as "off by default for bulk"; it is now default(true), which is what the server has always done` → an explicit runAutomations: false on any import request that is meant to load rows without firing triggers/hooks. That spelling is unchanged and has always been the only one the server read — what changes is that omitting the key now DECLARES what it already DID. Callers who want automations on need write nothing - Why not automatic: A DECLARATION corrected to match a runtime that did not move — the inverse of a behaviour flip, and registered here for the reason protocol 12's `rest-requireauth-default-flip` and this major's `action-descriptor-resume-authority-default-flip` are: whether a given import was meant to fire triggers is a judgment no transform can make, so the prescription is a TODO rather than a rewrite. The server decides in import-prepare.ts with `body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has since #2922 — automations always ran on import historically (the engine ignored the flag entirely before then), so opt-out was made the explicit act, matching platform convention. The schema said the opposite in both machine-readable and human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s JSON Schema, and the describe prose in the published reference tables for both defs. ⚠️ Nothing in this repo reconciled the two and NO deployed caller changes behaviour: no request path parses an import body through this schema — the route reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog entry, a declaration and not a parse. That is exactly why this needed a ruling rather than a docs edit: the divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. A client or SDK that validated its request through the published schema materialised `runAutomations: false` from the declared default and sent it explicitly, and the server honoured it — so the same request body produced opposite behaviour depending on whether the caller validated before sending, with the validating caller silently losing its triggers. Nothing rejected it, nothing warned, and the reference page told an author the wrong thing in the other direction. There is deliberately NO schema tombstone and no D2 conversion: no key is removed, and an HTTP request body is neither authored nor persisted — the same disposition `notification-list-cursor-retired` (#6361) takes for the sibling default on this major, and `batch-options-validate-only-retired` before it. The declared move itself is recorded mechanically, per key, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: the spec follows the runtime). ADR-0049 / ADR-0078. - Done when: Every import request of yours that must NOT fire triggers sends `runAutomations: false` explicitly, rather than omitting the key and trusting the old declared default. The check is worth doing precisely where it looks unnecessary: if you build the body by parsing it through `ImportRequestSchema` (or the published JSON Schema) and then send the PARSED object, your bulk loads were running with automations OFF and will now run with them ON — that is the only class whose behaviour changes, and it changes toward what an unvalidated caller always got. ⚠️ Behaviour on the wire is deliberately UNCHANGED and should be verified as such: a body that omits `runAutomations` fired triggers before this change and fires them after, and `runAutomations: false` turns them off before and after. Nothing starts being refused — the route never validated this body against the schema and does not begin to. `dryRun` is unaffected and still runs NO automations whatever the flag says (#6037). +- **`view-filter-rule-value-shaped-by-operator`** — `ui.ViewFilterRule value — the third key of a view filter rule, on every carrier of ViewFilterRuleSchema: ListView.filter, a list view tab filter, Page.filterBy, a related-list component filter and a lookup picker filter. It accepted any declared scalar or array for EVERY operator; the accepted shape is now decided by the rule operator — in / not_in require an array, between requires exactly two bounds, and every other operator is unchanged` → an ARRAY for in / not_in (a single value becomes a one-element list: value: "won" becomes value: ["won"]), and a two-element [min, max] array for between. The empty list [] stays legal for in / not_in and keeps its meaning. Nothing else moves: a scalar operator carrying an array, a string operator carrying a number, and a unary operator carrying an ignored value all still parse + - Why not automatic: A publish-time gate catching up to a query-time one, not a new rule. #5869 / PR #6209 closed the RUNTIME half: `assertListComparandShapes` (@objectstack/objectql, filter-comparand-shape.ts) refuses a lowered `{ stage: { $nin: "won" } }` with a named 400 INVALID_FILTER, and before that it was a 500. The authoring surface stayed silent, so the failure was two-stage: the view published cleanly and only broke when someone opened it. That file names this very schema as the reachable authoring source of the defect. The tightening MIRRORS that gate exactly — three constraints, one for one — and deliberately goes no further, because #5685 already ruled on the opposite error: a schema stricter than the runtime "in ways the runtime deliberately allows" was the WRONG side and was widened to match. So `in: []` is still accepted (a declared predicate both drivers implement), `equals: ["a","b"]` is still accepted (it lowers to a deep-equality comparand), and `is_empty: ""` is still accepted (the null predicates take their direction from the operator NAME — convertComparison ignores the value position, and the ObjectUI client deliberately sends a truthy placeholder there). ⚠️ Metadata AT REST is deliberately NOT rewritten, and there is no D2 conversion. A D2 entry replays a shape the platform once WROTE and renamed; this shape was never written by any first-party producer (every in / not_in rule in this repo, in objectui and in the cloud repo already carries an array — measured) and has never EXECUTED, since it 400s on first render today. Coercing it at load would be the platform guessing intent rather than replaying a rename, and it cannot guess honestly: value: "" would become the predicate [""] (a real filter on the empty string) rather than the "not filled in yet" a console row means, and between: 5 has no defensible second bound at all. The read path does not re-validate stored rows (applyConversionsToStoredItem never validates, by its own contract), so no stored view becomes unreadable; what changes is that RE-SAVING such a view is refused at the write gate naming `value`, instead of storing a filter that 400s. ADR-0049 / ADR-0078 / ADR-0112. + - Done when: Grep your authored views, pages and related-list components for a filter rule whose operator is in, not_in or between (including the alias spellings nin / notIn / notin) and whose value is not an array of the right arity, then wrap or complete it. `os validate` / `os lint` now report each one by path with the operator, the received shape and the corrected shape, so the sweep is mechanical rather than by eye. Two checks are worth doing where it looks unnecessary: a rule reading `operator: "in", value: ""` is an UNFINISHED row, not a filter — decide what it was meant to select rather than mechanically rewriting it to [""], which is a real and different predicate. And a view that already carried one of these shapes was never returning filtered rows: it answered 400 INVALID_FILTER on render (#5869), so re-check what the view is supposed to show rather than assuming the old result set was correct. --- diff --git a/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts index bdcd7f8ca1..6912c988d0 100644 --- a/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts +++ b/packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts @@ -19,7 +19,13 @@ * actually validates against. */ import { describe, it, expect } from 'vitest'; -import { ViewMetadataSchema, VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui'; +import { + ViewMetadataSchema, + VIEW_FILTER_OPERATORS, + VIEW_FILTER_OPERATOR_ALIASES, + VIEW_FILTER_LIST_VALUE_OPERATORS, + VIEW_FILTER_PAIR_VALUE_OPERATORS, +} from '@objectstack/spec/ui'; import { graftNormalizedOperators } from './protocol.js'; /** Graft through the real spec schema, the way `saveMetaItem` does. */ @@ -31,6 +37,30 @@ function graftThroughSchema(authored: unknown): unknown { return graftNormalizedOperators(authored, parsed.data); } +/** + * A `value` whose SHAPE the given canonical operator can carry (#6227). + * + * This suite's subject is the ALIAS FOLD, and the fold is only observable on a + * rule that PARSES. Since #6227 the spec couples `value` to `operator` — the + * three aliases that fold to `not_in` (`nin` / `notin` / `notIn`) need an array, + * and a range needs two bounds — so a single hard-coded scalar would be refused + * for a reason that has nothing to do with what is being tested. + * + * Read from the spec's own exported vocabularies rather than a local list, so a + * list-valued operator added later cannot silently reintroduce the breakage: + * that is the same "one declared vocabulary, not N dialects" rule the alias + * table itself exists to serve. + */ +function valueFor(canonicalOperator: string): unknown { + if ((VIEW_FILTER_LIST_VALUE_OPERATORS as readonly string[]).includes(canonicalOperator)) { + return ['x']; + } + if ((VIEW_FILTER_PAIR_VALUE_OPERATORS as readonly string[]).includes(canonicalOperator)) { + return ['x', 'y']; + } + return 'x'; +} + /** Flattened runtime view overlay — the shape a console personalization PUT sends. */ const view = (filter: unknown, extra: Record = {}) => ({ name: 'showcase_task.open', @@ -53,9 +83,9 @@ describe('graftNormalizedOperators — through the real view metadata schema', ( it('canonicalizes every alias the spec still folds', () => { const canonical = new Set(VIEW_FILTER_OPERATORS); const stillLegacy: string[] = []; - for (const alias of Object.keys(VIEW_FILTER_OPERATOR_ALIASES)) { + for (const [alias, foldsTo] of Object.entries(VIEW_FILTER_OPERATOR_ALIASES)) { const out = graftThroughSchema( - view([{ field: 'status', operator: alias, value: 'x' }]), + view([{ field: 'status', operator: alias, value: valueFor(foldsTo) }]), ) as { filter: Array<{ operator: string }> }; if (!canonical.has(out.filter[0].operator)) stillLegacy.push(alias); } diff --git a/packages/spec/api-surface/ui.json b/packages/spec/api-surface/ui.json index d53ff887f8..19cb6137ef 100644 --- a/packages/spec/api-surface/ui.json +++ b/packages/spec/api-surface/ui.json @@ -327,8 +327,10 @@ "UserFiltersParsed (type)", "UserFiltersSchema (const)", "VIEW_CONSOLE_ROW_DECORATIONS (const)", + "VIEW_FILTER_LIST_VALUE_OPERATORS (const)", "VIEW_FILTER_OPERATORS (const)", "VIEW_FILTER_OPERATOR_ALIASES (const)", + "VIEW_FILTER_PAIR_VALUE_OPERATORS (const)", "VIEW_METADATA_BRANCHES (const)", "VIEW_METADATA_MEMBERS (const)", "VIEW_WRITE_PATH_IDENTITY_KEYS (const)", diff --git a/packages/spec/export-origins/ui.json b/packages/spec/export-origins/ui.json index 9f93d415b1..bf187ecf2e 100644 --- a/packages/spec/export-origins/ui.json +++ b/packages/spec/export-origins/ui.json @@ -327,8 +327,10 @@ "UserFiltersParsed": "src/ui/view.zod.ts#UserFiltersParsed (type)", "UserFiltersSchema": "src/ui/view.zod.ts#UserFiltersSchema (const)", "VIEW_CONSOLE_ROW_DECORATIONS": "src/ui/view.zod.ts#VIEW_CONSOLE_ROW_DECORATIONS (const)", + "VIEW_FILTER_LIST_VALUE_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_LIST_VALUE_OPERATORS (const)", "VIEW_FILTER_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_OPERATORS (const)", "VIEW_FILTER_OPERATOR_ALIASES": "src/ui/view.zod.ts#VIEW_FILTER_OPERATOR_ALIASES (const)", + "VIEW_FILTER_PAIR_VALUE_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_PAIR_VALUE_OPERATORS (const)", "VIEW_METADATA_BRANCHES": "src/ui/view.zod.ts#VIEW_METADATA_BRANCHES (const)", "VIEW_METADATA_MEMBERS": "src/ui/view.zod.ts#VIEW_METADATA_MEMBERS (const)", "VIEW_WRITE_PATH_IDENTITY_KEYS": "src/ui/view.zod.ts#VIEW_WRITE_PATH_IDENTITY_KEYS (const)", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 4c415ca978..137ae02af5 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -765,6 +765,13 @@ "migrationId": "import-run-automations-declared-default-corrected", "toMajor": 17, "rationale": "A DECLARATION corrected to match a runtime that did not move — the inverse of a behaviour flip, and registered here for the reason protocol 12's `rest-requireauth-default-flip` and this major's `action-descriptor-resume-authority-default-flip` are: whether a given import was meant to fire triggers is a judgment no transform can make, so the prescription is a TODO rather than a rewrite. The server decides in import-prepare.ts with `body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has since #2922 — automations always ran on import historically (the engine ignored the flag entirely before then), so opt-out was made the explicit act, matching platform convention. The schema said the opposite in both machine-readable and human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s JSON Schema, and the describe prose in the published reference tables for both defs. ⚠️ Nothing in this repo reconciled the two and NO deployed caller changes behaviour: no request path parses an import body through this schema — the route reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog entry, a declaration and not a parse. That is exactly why this needed a ruling rather than a docs edit: the divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. A client or SDK that validated its request through the published schema materialised `runAutomations: false` from the declared default and sent it explicitly, and the server honoured it — so the same request body produced opposite behaviour depending on whether the caller validated before sending, with the validating caller silently losing its triggers. Nothing rejected it, nothing warned, and the reference page told an author the wrong thing in the other direction. There is deliberately NO schema tombstone and no D2 conversion: no key is removed, and an HTTP request body is neither authored nor persisted — the same disposition `notification-list-cursor-retired` (#6361) takes for the sibling default on this major, and `batch-options-validate-only-retired` before it. The declared move itself is recorded mechanically, per key, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: the spec follows the runtime). ADR-0049 / ADR-0078." + }, + { + "surface": "ui.ViewFilterRule value — the third key of a view filter rule, on every carrier of ViewFilterRuleSchema: ListView.filter, a list view tab filter, Page.filterBy, a related-list component filter and a lookup picker filter. It accepted any declared scalar or array for EVERY operator; the accepted shape is now decided by the rule operator — in / not_in require an array, between requires exactly two bounds, and every other operator is unchanged", + "replacement": "an ARRAY for in / not_in (a single value becomes a one-element list: value: \"won\" becomes value: [\"won\"]), and a two-element [min, max] array for between. The empty list [] stays legal for in / not_in and keeps its meaning. Nothing else moves: a scalar operator carrying an array, a string operator carrying a number, and a unary operator carrying an ignored value all still parse", + "migrationId": "view-filter-rule-value-shaped-by-operator", + "toMajor": 17, + "rationale": "A publish-time gate catching up to a query-time one, not a new rule. #5869 / PR #6209 closed the RUNTIME half: `assertListComparandShapes` (@objectstack/objectql, filter-comparand-shape.ts) refuses a lowered `{ stage: { $nin: \"won\" } }` with a named 400 INVALID_FILTER, and before that it was a 500. The authoring surface stayed silent, so the failure was two-stage: the view published cleanly and only broke when someone opened it. That file names this very schema as the reachable authoring source of the defect. The tightening MIRRORS that gate exactly — three constraints, one for one — and deliberately goes no further, because #5685 already ruled on the opposite error: a schema stricter than the runtime \"in ways the runtime deliberately allows\" was the WRONG side and was widened to match. So `in: []` is still accepted (a declared predicate both drivers implement), `equals: [\"a\",\"b\"]` is still accepted (it lowers to a deep-equality comparand), and `is_empty: \"\"` is still accepted (the null predicates take their direction from the operator NAME — convertComparison ignores the value position, and the ObjectUI client deliberately sends a truthy placeholder there). ⚠️ Metadata AT REST is deliberately NOT rewritten, and there is no D2 conversion. A D2 entry replays a shape the platform once WROTE and renamed; this shape was never written by any first-party producer (every in / not_in rule in this repo, in objectui and in the cloud repo already carries an array — measured) and has never EXECUTED, since it 400s on first render today. Coercing it at load would be the platform guessing intent rather than replaying a rename, and it cannot guess honestly: value: \"\" would become the predicate [\"\"] (a real filter on the empty string) rather than the \"not filled in yet\" a console row means, and between: 5 has no defensible second bound at all. The read path does not re-validate stored rows (applyConversionsToStoredItem never validates, by its own contract), so no stored view becomes unreadable; what changes is that RE-SAVING such a view is refused at the write gate naming `value`, instead of storing a filter that 400s. ADR-0049 / ADR-0078 / ADR-0112." } ], "removed": [] @@ -1589,6 +1596,13 @@ "migrationId": "import-run-automations-declared-default-corrected", "toMajor": 17, "rationale": "A DECLARATION corrected to match a runtime that did not move — the inverse of a behaviour flip, and registered here for the reason protocol 12's `rest-requireauth-default-flip` and this major's `action-descriptor-resume-authority-default-flip` are: whether a given import was meant to fire triggers is a judgment no transform can make, so the prescription is a TODO rather than a rewrite. The server decides in import-prepare.ts with `body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has since #2922 — automations always ran on import historically (the engine ignored the flag entirely before then), so opt-out was made the explicit act, matching platform convention. The schema said the opposite in both machine-readable and human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s JSON Schema, and the describe prose in the published reference tables for both defs. ⚠️ Nothing in this repo reconciled the two and NO deployed caller changes behaviour: no request path parses an import body through this schema — the route reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog entry, a declaration and not a parse. That is exactly why this needed a ruling rather than a docs edit: the divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. A client or SDK that validated its request through the published schema materialised `runAutomations: false` from the declared default and sent it explicitly, and the server honoured it — so the same request body produced opposite behaviour depending on whether the caller validated before sending, with the validating caller silently losing its triggers. Nothing rejected it, nothing warned, and the reference page told an author the wrong thing in the other direction. There is deliberately NO schema tombstone and no D2 conversion: no key is removed, and an HTTP request body is neither authored nor persisted — the same disposition `notification-list-cursor-retired` (#6361) takes for the sibling default on this major, and `batch-options-validate-only-retired` before it. The declared move itself is recorded mechanically, per key, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: the spec follows the runtime). ADR-0049 / ADR-0078." + }, + { + "surface": "ui.ViewFilterRule value — the third key of a view filter rule, on every carrier of ViewFilterRuleSchema: ListView.filter, a list view tab filter, Page.filterBy, a related-list component filter and a lookup picker filter. It accepted any declared scalar or array for EVERY operator; the accepted shape is now decided by the rule operator — in / not_in require an array, between requires exactly two bounds, and every other operator is unchanged", + "replacement": "an ARRAY for in / not_in (a single value becomes a one-element list: value: \"won\" becomes value: [\"won\"]), and a two-element [min, max] array for between. The empty list [] stays legal for in / not_in and keeps its meaning. Nothing else moves: a scalar operator carrying an array, a string operator carrying a number, and a unary operator carrying an ignored value all still parse", + "migrationId": "view-filter-rule-value-shaped-by-operator", + "toMajor": 17, + "rationale": "A publish-time gate catching up to a query-time one, not a new rule. #5869 / PR #6209 closed the RUNTIME half: `assertListComparandShapes` (@objectstack/objectql, filter-comparand-shape.ts) refuses a lowered `{ stage: { $nin: \"won\" } }` with a named 400 INVALID_FILTER, and before that it was a 500. The authoring surface stayed silent, so the failure was two-stage: the view published cleanly and only broke when someone opened it. That file names this very schema as the reachable authoring source of the defect. The tightening MIRRORS that gate exactly — three constraints, one for one — and deliberately goes no further, because #5685 already ruled on the opposite error: a schema stricter than the runtime \"in ways the runtime deliberately allows\" was the WRONG side and was widened to match. So `in: []` is still accepted (a declared predicate both drivers implement), `equals: [\"a\",\"b\"]` is still accepted (it lowers to a deep-equality comparand), and `is_empty: \"\"` is still accepted (the null predicates take their direction from the operator NAME — convertComparison ignores the value position, and the ObjectUI client deliberately sends a truthy placeholder there). ⚠️ Metadata AT REST is deliberately NOT rewritten, and there is no D2 conversion. A D2 entry replays a shape the platform once WROTE and renamed; this shape was never written by any first-party producer (every in / not_in rule in this repo, in objectui and in the cloud repo already carries an array — measured) and has never EXECUTED, since it 400s on first render today. Coercing it at load would be the platform guessing intent rather than replaying a rename, and it cannot guess honestly: value: \"\" would become the predicate [\"\"] (a real filter on the empty string) rather than the \"not filled in yet\" a console row means, and between: 5 has no defensible second bound at all. The read path does not re-validate stored rows (applyConversionsToStoredItem never validates, by its own contract), so no stored view becomes unreadable; what changes is that RE-SAVING such a view is refused at the write gate naming `value`, instead of storing a filter that 400s. ADR-0049 / ADR-0078 / ADR-0112." } ], "removed": [] diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 2f99cff0a0..3172b890af 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -3193,6 +3193,66 @@ const step17: MigrationStep = { + 'schema and does not begin to. `dryRun` is unaffected and still runs NO automations ' + 'whatever the flag says (#6037).', }, + { + id: 'view-filter-rule-value-shaped-by-operator', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span (see the note on the entry above). + surface: + 'ui.ViewFilterRule value — the third key of a view filter rule, on every carrier of ' + + 'ViewFilterRuleSchema: ListView.filter, a list view tab filter, Page.filterBy, a ' + + 'related-list component filter and a lookup picker filter. It accepted any declared ' + + 'scalar or array for EVERY operator; the accepted shape is now decided by the rule ' + + 'operator — in / not_in require an array, between requires exactly two bounds, and ' + + 'every other operator is unchanged', + replacement: + 'an ARRAY for in / not_in (a single value becomes a one-element list: value: "won" ' + + 'becomes value: ["won"]), and a two-element [min, max] array for between. The empty ' + + 'list [] stays legal for in / not_in and keeps its meaning. Nothing else moves: a ' + + 'scalar operator carrying an array, a string operator carrying a number, and a unary ' + + 'operator carrying an ignored value all still parse', + reason: + 'A publish-time gate catching up to a query-time one, not a new rule. #5869 / PR ' + + '#6209 closed the RUNTIME half: `assertListComparandShapes` ' + + '(@objectstack/objectql, filter-comparand-shape.ts) refuses a lowered ' + + '`{ stage: { $nin: "won" } }` with a named 400 INVALID_FILTER, and before that it ' + + 'was a 500. The authoring surface stayed silent, so the failure was two-stage: the ' + + 'view published cleanly and only broke when someone opened it. That file names this ' + + 'very schema as the reachable authoring source of the defect. The tightening MIRRORS ' + + 'that gate exactly — three constraints, one for one — and deliberately goes no ' + + 'further, because #5685 already ruled on the opposite error: a schema stricter than ' + + 'the runtime "in ways the runtime deliberately allows" was the WRONG side and was ' + + 'widened to match. So `in: []` is still accepted (a declared predicate both drivers ' + + 'implement), `equals: ["a","b"]` is still accepted (it lowers to a deep-equality ' + + 'comparand), and `is_empty: ""` is still accepted (the null predicates take their ' + + 'direction from the operator NAME — convertComparison ignores the value position, ' + + 'and the ObjectUI client deliberately sends a truthy placeholder there). ' + + '⚠️ Metadata AT REST is deliberately NOT rewritten, and there is no D2 conversion. ' + + 'A D2 entry replays a shape the platform once WROTE and renamed; this shape was ' + + 'never written by any first-party producer (every in / not_in rule in this repo, in ' + + 'objectui and in the cloud repo already carries an array — measured) and has never ' + + 'EXECUTED, since it 400s on first render today. Coercing it at load would be the ' + + 'platform guessing intent rather than replaying a rename, and it cannot guess ' + + 'honestly: value: "" would become the predicate [""] (a real filter on the empty ' + + 'string) rather than the "not filled in yet" a console row means, and between: 5 has ' + + 'no defensible second bound at all. The read path does not re-validate stored rows ' + + '(applyConversionsToStoredItem never validates, by its own contract), so no stored ' + + 'view becomes unreadable; what changes is that RE-SAVING such a view is refused at ' + + 'the write gate naming `value`, instead of storing a filter that 400s. ' + + 'ADR-0049 / ADR-0078 / ADR-0112.', + acceptanceCriteria: + 'Grep your authored views, pages and related-list components for a filter rule whose ' + + 'operator is in, not_in or between (including the alias spellings nin / notIn / ' + + 'notin) and whose value is not an array of the right arity, then wrap or complete ' + + 'it. `os validate` / `os lint` now report each one by path with the operator, the ' + + 'received shape and the corrected shape, so the sweep is mechanical rather than by ' + + 'eye. Two checks are worth doing where it looks unnecessary: a rule reading ' + + '`operator: "in", value: ""` is an UNFINISHED row, not a filter — decide what it was ' + + 'meant to select rather than mechanically rewriting it to [""], which is a real and ' + + 'different predicate. And a view that already carried one of these shapes was never ' + + 'returning filtered rows: it answered 400 INVALID_FILTER on render (#5869), so ' + + 're-check what the view is supposed to show rather than assuming the old result set ' + + 'was correct.', + }, ], }; diff --git a/packages/spec/src/ui/view-filter-rule-value-shape.test.ts b/packages/spec/src/ui/view-filter-rule-value-shape.test.ts new file mode 100644 index 0000000000..71b768cb71 --- /dev/null +++ b/packages/spec/src/ui/view-filter-rule-value-shape.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6227] `ViewFilterRuleSchema.value` is shaped by the rule's OPERATOR. + * + * The defect these pins close was a TWO-STAGE failure, not a missing rule: + * `{ field: 'stage', operator: 'not_in', value: 'won' }` published cleanly and + * then answered a named 400 `INVALID_FILTER` at query time (#5869 / PR #6209 + * closed that runtime half). The author was gone by then. These pins assert the + * publish-time half now refuses the same three shapes the runtime refuses — + * `$in`/`$nin` must be arrays, `$between` must be a 2-array — and, just as + * importantly, that it refuses NOTHING ELSE (#5685: a schema stricter than the + * runtime is the wrong side). + * + * Every rejection pin asserts the issue PATH and the message's leading sentence, + * not merely that a throw happened: a bare `.toThrow()` cannot tell "refused for + * the right reason at the right key" from "refused because the value union + * rejected the type", and those are different defects (#6142). + */ + +import { describe, expect, it } from 'vitest'; +import { + VIEW_FILTER_LIST_VALUE_OPERATORS, + VIEW_FILTER_OPERATORS, + VIEW_FILTER_PAIR_VALUE_OPERATORS, + ViewFilterRuleSchema, +} from './view.zod'; + +/** Parse helper — the authored object form, exactly as a view carries it. */ +const parse = (rule: Record) => ViewFilterRuleSchema.safeParse(rule); + +/** The single `value`-path issue a shape refusal must produce. */ +function valueIssue(result: ReturnType) { + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + const issues = result.error.issues.filter((i) => i.path.join('.') === 'value'); + expect(issues).toHaveLength(1); + return issues[0]!; +} + +describe('#6227 — the reported shape is refused at authoring time', () => { + it('refuses the card example: a set operator carrying a scalar', () => { + const result = parse({ field: 'stage', operator: 'not_in', value: 'won' }); + const issue = valueIssue(result); + + expect(issue.code).toBe('custom'); + expect(issue.path).toEqual(['value']); + // Leading sentence kept verbatim from the runtime's `nonListComparandError` + // so one condition keeps one wording across both moments (#5240). + expect(issue.message).toContain( + 'Operator "not_in" on field "stage" requires an ARRAY of values.', + ); + // The refusal must carry what the author has to DO, not just what is wrong. + expect(issue.message).toContain('Received a string ("won")'); + expect(issue.message).toContain('write ["won"] for a single value'); + expect(issue.message).toContain('or use "not_equals" to compare against it'); + // And it must say the empty list is NOT what is being refused. + expect(issue.message).toContain('An empty list [] is allowed'); + }); + + it('names the runtime twin, so the two moments are traceable to one rule', () => { + const issue = valueIssue(parse({ field: 'stage', operator: 'in', value: 'won' })); + expect(issue.message).toContain('400 INVALID_FILTER, #5869'); + }); + + it('refuses through an ALIAS spelling too — the fold runs before the check', () => { + // `nin` / `notIn` are stored spellings; `normalizeFilterOperator` folds them + // to `not_in` pre-check, so the refusal names the CANONICAL operator. + for (const alias of ['nin', 'notIn', 'notin']) { + const issue = valueIssue(parse({ field: 'tags', operator: alias, value: 'x' })); + expect(issue.message).toContain('Operator "not_in"'); + } + }); + + it('refuses a malformed range with the range wording, not the list wording', () => { + const issue = valueIssue(parse({ field: 'age', operator: 'between', value: 5 })); + expect(issue.message).toContain( + 'Operator "between" on field "age" requires a [min, max] value array.', + ); + expect(issue.message).toContain('A range needs exactly two bounds, in order.'); + expect(issue.message).not.toContain('tests membership of a list'); + }); +}); + +describe('#6227 — the three constraints mirror the runtime gate exactly', () => { + it.each([ + ['in', 'scalar string', 'won'], + ['in', 'scalar number', 5], + ['in', 'boolean', true], + ['in', 'null', null], + ['in', 'omitted', undefined], + ['not_in', 'scalar string', 'won'], + ['not_in', 'omitted', undefined], + ])('refuses %s + %s', (operator, _label, value) => { + const rule: Record = { field: 'f', operator }; + if (value !== undefined) rule.value = value; + const issue = valueIssue(parse(rule)); + expect(issue.message).toContain('requires an ARRAY of values'); + }); + + it.each([ + ['scalar', 5], + ['one-element array', [1]], + ['three-element array', [1, 2, 3]], + ['empty array', []], + ['null', null], + ])('refuses between + %s', (_label, value) => { + const issue = valueIssue(parse({ field: 'f', operator: 'between', value })); + expect(issue.message).toContain('requires a [min, max] value array'); + }); + + it('refuses between with NO value', () => { + const issue = valueIssue(parse({ field: 'f', operator: 'between' })); + expect(issue.message).toContain('Received no value ((omitted))'); + }); +}); + +describe('#6227 — what stays accepted (the #5685 side: never stricter than the runtime)', () => { + it.each([ + ['in + array', { field: 'f', operator: 'in', value: ['a', 'b'] }], + ['not_in + array', { field: 'f', operator: 'not_in', value: ['a'] }], + ['in + mixed member types', { field: 'f', operator: 'in', value: ['a', 2] }], + // An empty list is a DECLARED predicate — "matches nothing" / "matches + // everything" — and the runtime gate accepts it in as many words. + ['in + empty array', { field: 'f', operator: 'in', value: [] }], + ['not_in + empty array', { field: 'f', operator: 'not_in', value: [] }], + ['between + pair', { field: 'f', operator: 'between', value: [1, 2] }], + ['between + ISO date pair', { field: 'd', operator: 'between', value: ['2024-01-01', '2024-12-31'] }], + // A scalar operator carrying an array lowers to a deep-equality comparand. + ['equals + array', { field: 'f', operator: 'equals', value: ['a', 'b'] }], + ['not_equals + array', { field: 'f', operator: 'not_equals', value: ['a'] }], + // A string operator carrying a number: no backend refuses it. + ['contains + number', { field: 'f', operator: 'contains', value: 5 }], + ['starts_with + number', { field: 'f', operator: 'starts_with', value: 5 }], + // Ordering operators take a scalar of any declared type (#5685 widened these). + ['greater_than + ISO string', { field: 'd', operator: 'greater_than', value: '2026-01-01' }], + ['before + string', { field: 'd', operator: 'before', value: '2026-01-01' }], + ['after + string', { field: 'd', operator: 'after', value: '2026-01-01' }], + // Ordering operators carrying an array are not this check's business either. + ['greater_than + array', { field: 'f', operator: 'greater_than', value: [1, 2] }], + // Alias spellings with a CONFORMING value keep parsing. + ['nin alias + array', { field: 'f', operator: 'nin', value: ['a'] }], + ['notIn alias + array', { field: 'f', operator: 'notIn', value: ['a'] }], + ])('accepts %s', (_label, rule) => { + const result = parse(rule as Record); + expect(result.success).toBe(true); + }); + + it.each(['is_empty', 'is_not_empty', 'is_null', 'is_not_null'])( + 'accepts the unary operator %s with OR without a value', + (operator) => { + // `convertComparison` maps these to `{ $null: true|false }` and ignores the + // value position; the ObjectUI client sends a truthy PLACEHOLDER for both + // `isnull` and `isnotnull`. Refusing a value here would break a live + // first-party producer to enforce nothing. + expect(parse({ field: 'f', operator }).success).toBe(true); + expect(parse({ field: 'f', operator, value: '' }).success).toBe(true); + expect(parse({ field: 'f', operator, value: true }).success).toBe(true); + expect(parse({ field: 'f', operator, value: ['x'] }).success).toBe(true); + }, + ); + + it('leaves every operator outside the two shaped vocabularies unjudged', () => { + const shaped = new Set([ + ...VIEW_FILTER_LIST_VALUE_OPERATORS, + ...VIEW_FILTER_PAIR_VALUE_OPERATORS, + ]); + for (const operator of VIEW_FILTER_OPERATORS) { + if (shaped.has(operator)) continue; + // Both a scalar and an array parse for every unshaped operator. + expect(parse({ field: 'f', operator, value: 'x' }).success).toBe(true); + expect(parse({ field: 'f', operator, value: ['x'] }).success).toBe(true); + } + }); +}); + +describe('#6227 — the exported vocabularies are the ones the check reads', () => { + it('declares exactly the operators that lower to $in / $nin', () => { + expect([...VIEW_FILTER_LIST_VALUE_OPERATORS]).toEqual(['in', 'not_in']); + }); + + it('declares exactly the operator that lowers to $between', () => { + expect([...VIEW_FILTER_PAIR_VALUE_OPERATORS]).toEqual(['between']); + }); + + it('keeps both vocabularies inside the canonical operator enum', () => { + for (const operator of [ + ...VIEW_FILTER_LIST_VALUE_OPERATORS, + ...VIEW_FILTER_PAIR_VALUE_OPERATORS, + ]) { + expect(VIEW_FILTER_OPERATORS).toContain(operator); + } + }); +}); + +describe('#6227 — the refinement does not disturb the schema around it', () => { + it('still folds alias operators to canonical on a CONFORMING rule', () => { + const parsed = ViewFilterRuleSchema.parse({ field: 'tags', operator: 'nin', value: ['a'] }); + expect(parsed.operator).toBe('not_in'); + }); + + it('still rejects an unknown key by name (the .strict() posture survives)', () => { + const result = parse({ field: 'f', operator: 'in', value: ['a'], nope: 1 }); + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + expect(result.error.issues.some((i) => /nope/.test(i.message))).toBe(true); + }); + + it('still rejects an unknown OPERATOR at the operator path, not the value path', () => { + const result = parse({ field: 'f', operator: 'sideways', value: 'x' }); + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + expect(result.error.issues.some((i) => i.path.join('.') === 'operator')).toBe(true); + }); + + it('reports the VALUE defect at the value path even when the type union also fails', () => { + // A nested array fails the declared `value` union on TYPE grounds. The rule + // is still refused, and the union's own issue is what names it — the + // refinement must not swallow or duplicate that. + const result = parse({ field: 'f', operator: 'in', value: [['a']] }); + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + expect(result.error.issues.every((i) => i.path[0] === 'value')).toBe(true); + }); +}); diff --git a/packages/spec/src/ui/view.zod.ts b/packages/spec/src/ui/view.zod.ts index afa68f516a..759d06b00f 100644 --- a/packages/spec/src/ui/view.zod.ts +++ b/packages/spec/src/ui/view.zod.ts @@ -191,6 +191,35 @@ export const VIEW_FILTER_OPERATORS = [ export type ViewFilterOperator = (typeof VIEW_FILTER_OPERATORS)[number]; +/** + * The operators whose `value` is a LIST rather than a scalar (#6227). + * + * These are the authoring spellings that lower to `$in` / `$nin` + * (`AST_OPERATOR_MAP`, `data/filter.zod.ts`), which + * {@link https://github.com/objectstack-ai/objectstack/issues/5869 | the runtime + * gate} requires to be arrays. Exported so a producer can ask the question the + * schema asks instead of hard-coding its own list: `@object-ui`'s filter builder + * decides `isMultiOperator` from a local `["in", "notIn"]` literal + * (`components/src/custom/filter-builder.tsx`), a second dialect of exactly this + * fact that is already one spelling adrift — `notIn` is an alias, not the + * canonical member. One declared vocabulary, same reasoning as + * {@link VIEW_FILTER_OPERATOR_ALIASES}. + */ +export const VIEW_FILTER_LIST_VALUE_OPERATORS = [ + 'in', 'not_in', +] as const satisfies readonly ViewFilterOperator[]; + +/** + * The operators whose `value` is a two-element `[min, max]` array (#6227). + * + * Separate from {@link VIEW_FILTER_LIST_VALUE_OPERATORS} because the check is + * different in kind: membership takes ANY arity (`[]` included), a range takes + * exactly two bounds. + */ +export const VIEW_FILTER_PAIR_VALUE_OPERATORS = [ + 'between', +] as const satisfies readonly ViewFilterOperator[]; + /** * Legacy operator spellings normalized to the canonical vocabulary above. * @@ -346,6 +375,158 @@ export function stripViewConsoleDecorations(body: unknown): unknown { return stripRowDecorations(body, false, 0); } +/** `string` / `number` / `an array of 3` / `null` … — the word the refusal uses. */ +function describeFilterValue(value: unknown): string { + if (value === null) return 'null'; + if (value === undefined) return 'no value'; + if (Array.isArray(value)) return `an array of ${value.length}`; + return `a ${typeof value}`; +} + +/** + * A short, bounded rendering of the offending value. + * + * Bounded for the reason the runtime twin's `shapePreview` is: the value can be + * arbitrarily large, and the message is for a human reading a refusal, not a + * dump. + */ +function previewFilterValue(value: unknown): string { + if (value === undefined) return '(omitted)'; + let text: string; + try { + text = JSON.stringify(value) ?? String(value); + } catch { + text = String(value); + } + return text.length > 40 ? `${text.slice(0, 39)}…` : text; +} + +/** + * [#6227] `value` must have the shape the rule's OPERATOR can execute. + * + * ## The two-stage failure this closes + * + * `{ field: 'stage', operator: 'not_in', value: 'won' }` — a set operator with a + * scalar comparand — was a spec-VALID `ViewFilterRule`: `value` declared + * `string | number | boolean | null | (string | number)[]` with no coupling to + * `operator`, so every operator accepted every shape. The view published cleanly + * and then failed at QUERY time, where #5869 / PR #6209 had already closed the + * runtime half: `assertListComparandShapes` (`@objectstack/objectql`, + * `filter-comparand-shape.ts`) refuses the lowered `{ stage: { $nin: 'won' } }` + * with a named 400 `INVALID_FILTER`. Correct refusal, wrong moment — the author + * is gone by then, and before #6209 the same shape was a 500. That file's own + * module docblock names this schema as the reachable authoring source of the + * defect. + * + * ## Why this mirrors the runtime gate EXACTLY, and refuses to go further + * + * The checks below are `assertListComparandShapes`' three constraints, one for + * one: `$in`/`$nin` must be an array, `$between` must be a 2-array. Nothing else + * is judged here, deliberately — #5685 already ruled on the opposite error, where + * `FieldOperatorsSchema` declared `$gt` as `number | Date | FieldReference` while + * every first-party producer put an ISO STRING there; the schema was ruled the + * wrong side and widened to match the runtime. A publish-time gate refusing more + * than the query path refuses would re-create that mismatch pointing the other + * way, and would reject stored metadata that executes correctly today. + * Specifically NOT refused, because the runtime does not refuse them: + * + * - **`in: []` / `not_in: []`.** An empty list is a legitimate declared predicate + * — "matches nothing" / "matches everything" — and the runtime gate says so in + * as many words. Arity is not this check's business for membership; only "is it + * a list at all". + * - **A scalar operator carrying an array** (`equals: ['a','b']`). `equals` + * lowers to a bare `{ field: value }` deep-equality comparand + * (`convertComparison`), which every backend answers. + * - **A string operator carrying a number** (`contains: 5`). Lowers to + * `$contains: 5`; no backend refuses it. + * - **A unary operator carrying a value** (`is_empty: ''`). The null predicates + * take their direction from the operator NAME — `convertComparison` maps them + * to `{ $null: true|false }` and ignores the value position entirely — and the + * ObjectUI client deliberately sends a truthy PLACEHOLDER value for both + * `isnull` and `isnotnull`. Refusing it would break a live first-party producer + * to enforce nothing. + * + * ## Why `superRefine` and not `z.discriminatedUnion` (measured, not assumed) + * + * 1. **`z.discriminatedUnion` cannot read this discriminator — it does not + * construct.** `operator` is `z.preprocess(normalizeFilterOperator, z.enum(…))` + * — the alias fold that lets a stored `notIn` / `nin` / `gt` parse. Zod 4 + * extracts a discriminator's literal values from the option's own def, and a + * preprocess wrapper hides them: building the union throws + * `Invalid discriminated union option at index "0"` before any parse happens. + * The alias fold is load-bearing ({@link VIEW_FILTER_OPERATOR_ALIASES} exists + * for stored metadata) and is not negotiable to buy a union. + * 2. **A refinement adds no JSON-Schema structure.** Measured with + * `z.toJSONSchema` before and after: byte-identical output. `ui/ViewFilterRule` + * is a PUBLISHED def whose authorable key set is a ratchet of exactly three + * entries (`authorable-surface/ui.json`). A union fans that one def into N + * branches re-declaring the same three keys per branch — the phantom + * liveness-worklist inflation #7042 measured and refused for + * `ViewContainerWireSchema` one screen down — and ObjectUI's SchemaForm would + * stop rendering the single operator dropdown it renders today. + * 3. **Error quality points at the defect.** A refinement emits ONE issue at path + * `['value']` naming the operator, the received shape and the expected one. A + * union emits every branch's failure and leads with the discriminator, i.e. it + * blames `operator` for a defect that is in `value`. + * + * In Zod 4 a refinement lives INSIDE the schema rather than wrapping it in a + * `ZodEffects`, so `.shape` and the `ZodObject` class survive (measured) and + * every carrier — `z.array(ViewFilterRuleSchema)` on `ListView.filter`, a tab + * filter, `Page.filterBy`, a related-list filter and a lookup picker filter — + * keeps working untouched. + * + * ## The wording is the runtime's wording (#5240) + * + * The leading sentence is kept verbatim from `nonListComparandError` / + * `malformedRangeComparandError` so one condition keeps one wording across the + * two moments it can be reported. The TAIL deliberately differs: the runtime's + * closing fact is "the filter was NOT applied", which is false here — nothing + * ran, the metadata is being refused — so this one prescribes the fix instead. + */ +function checkViewFilterRuleValueShape( + rule: { field?: unknown; operator?: unknown; value?: unknown }, + ctx: z.RefinementCtx, +): void { + // `operator` is read POST-parse, so it is already folded to canonical by + // `normalizeFilterOperator`: a stored `notIn` is `not_in` here, and this check + // never has to know the alias table. + const operator = rule.operator as ViewFilterOperator; + const value = rule.value; + const field = typeof rule.field === 'string' ? rule.field : ''; + + const isList = (VIEW_FILTER_LIST_VALUE_OPERATORS as readonly string[]).includes(operator); + const isPair = (VIEW_FILTER_PAIR_VALUE_OPERATORS as readonly string[]).includes(operator); + + if (isList) { + if (Array.isArray(value)) return; + ctx.addIssue({ + code: 'custom', + path: ['value'], + message: + `Operator "${operator}" on field "${field}" requires an ARRAY of values. ` + + `Received ${describeFilterValue(value)} (${previewFilterValue(value)}). ` + + `"${operator}" tests membership of a list — write ` + + `${value === undefined ? '["…"]' : previewFilterValue([value])} for a single value, ` + + `or use ${operator === 'in' ? '"equals"' : '"not_equals"'} to compare against it. ` + + `An empty list [] is allowed and is a real predicate. This is refused at authoring ` + + `time because the query path refuses it too (400 INVALID_FILTER, #5869).`, + }); + return; + } + + if (!isPair) return; + if (Array.isArray(value) && value.length === 2) return; + ctx.addIssue({ + code: 'custom', + path: ['value'], + message: + `Operator "${operator}" on field "${field}" requires a [min, max] value array. ` + + `Received ${describeFilterValue(value)} (${previewFilterValue(value)}). ` + + `A range needs exactly two bounds, in order. This is refused at authoring time ` + + `because the query path refuses it too (400 INVALID_FILTER, #5869).`, + }); +} + /** * View Filter Rule Schema * Standardized filter condition used in list views, tabs, and page-level filters. @@ -411,10 +592,20 @@ export const ViewFilterRuleSchema = lazySchema(() => strictObject({ */ operator: z.preprocess(normalizeFilterOperator, z.enum(VIEW_FILTER_OPERATORS)) .describe('Filter operator'), - /** Filter value (optional for unary operators like is_empty, is_null) */ + /** + * Filter value (optional for unary operators like is_empty, is_null). + * + * The accepted SHAPE is coupled to `operator` by + * {@link checkViewFilterRuleValueShape} (#6227). + */ value: z.union([z.string(), z.number(), z.boolean(), z.null(), z.array(z.union([z.string(), z.number()]))]) - .optional().describe('Filter value'), -}).describe('View filter rule')); + .optional().describe( + 'Filter value. The accepted SHAPE depends on the operator: `in` / `not_in` take an ' + + 'array (any length, including []), `between` takes exactly [min, max], every other ' + + 'operator takes a scalar. The unary operators (is_empty / is_not_empty / is_null / ' + + 'is_not_null) take their direction from the operator name and ignore this key.', + ), +}).superRefine(checkViewFilterRuleValueShape).describe('View filter rule')); export type ViewFilterRule = z.input; /** Post-parse shape of {@link ViewFilterRule} — defaults applied, transforms run (ADR-0122). */