Commit cc6dfd9
fix(spec): refuse an absent value on a value-taking view filter operator at authoring time (#19861)
Fixes #19751
Clause-②: no (narrowing)
## What changes
`checkViewFilterRuleValueShape` (the value-shape refinement of
`ViewFilterRuleSchema`, `packages/spec/src/ui/view.zod.ts`) now refuses
a rule with NO `value` on every operator that takes one. Its scalar arm
returned early on `value === undefined` for every operator, so `{ field:
'name', operator: 'icontains' }` parsed green, while the key's published
description says every operator outside `in` / `not_in` / `between` and
the four unary operators takes a scalar, and the query path refuses the
lowered rule with `400 INVALID_FILTER`.
- The four unary operators (`is_empty`, `is_not_empty`, `is_null`,
`is_not_null`) are answered first and stay valueless, with or without a
value.
- `in` / `not_in` / `between` keep their own arms, which already refused
an absent value.
- The key stays `.optional()` on the shape; the coupling lives in the
refinement, like the other arms.
- The `value` `.describe()` is unchanged (it already declares this
contract), so no generated reference page moves.
- The code comment above the scalar arm, which named an absent value as
a carve-out "the query path itself makes", now says the opposite and
why. The docblock's "mirrors the query path" list and its
runtime-wording section name the new arm.
Refusal text, one issue at the rule's `value` path:
> Filter comparand for operator "icontains" on field "name" is
undefined. The rule carries no value, and "icontains" compares the field
against one — write the value to compare against, or, if the rule means
the field has no value, use an operator that takes none ("is_empty" /
"is_not_empty" / "is_null" / "is_not_null"), which reads its direction
from its name. This is refused at authoring time because the query path
refuses it too (400 INVALID_FILTER).
The leading sentence is the runtime's undefined-comparand sentence
("Filter comparand at PATH is undefined.") with the location named in
the view vocabulary: operator and field, the same substitution the list
and range arms already make. A view rule has no `where` path, and the
`$` spelling in that path is not one a view author can write. The unary
operator names in the tail come from the schema's own
`VIEW_FILTER_VALUELESS_OPERATORS`.
## Producer reading (step 1): objectui at the pinned `.objectui-sha`
`87af769e9a3ee28ace099fdd653d3ebd79fe82e2`
Read with `git show SHA:PATH` from a local clone at that sha, not from a
working tree.
**Does the console ever save a value-taking rule with no `value`? No.**
| writer | file at the pinned sha | what happens to a half-filled row |
|---|---|---|
| `foldFilterGroupToSpecRules`, the one fold every view-filter writer
shares | `packages/app-shell/src/views/viewFilterFold.ts` | a row whose
operator takes a value is dropped when `isFilterValueComplete(operator,
value)` is false (`if (takesValue && isMissingValue(...)) continue`) |
| `isFilterValueComplete` |
`packages/components/src/custom/filter-builder.tsx` | false for `value
== null` (also `''`, `[]`, a half-filled pair), so an absent value is
always incomplete |
| `FilterBuilderField` / `FilterBuilderWidget`: the `filter-builder`
widget that `view.form.ts` names for `filter` and `page.form.ts` for
`filterBy`, plus the per-tab filter editor |
`packages/app-shell/src/views/metadata-admin/widgets.tsx` | calls the
fold on every change; the runtime `ViewConfigPanel` hosts the same
inspector (`ViewConfigPanel.tsx`, `ViewVariantInspector`) |
| list toolbar | `packages/app-shell/src/views/ObjectView.tsx` | no
automatic write at all (its docblock: "There is deliberately NO
persistViewFilter"); explicit saves go through the fold |
| drill-down "Save as view", `foldUrlFilterTriplesToSpecRules` |
`packages/app-shell/src/views/ObjectDataPage.tsx` |
`ViewFilterRuleSchema.safeParse` per rule, refused rules dropped; the
URL triples (`drillUrlFilters.ts`, `parseUrlFilterTriples`) skip an
empty param and always carry a value |
One edge, stated rather than hidden: `handleViewConfigSave`
(`ObjectView.tsx`) persists the config draft whole. A view whose STORED
body already carries such a rule (hand-authored, or written by another
tool) and is re-saved through the panel without its filter being touched
now gets the refusal at save. That view already fails every query today
(next table).
**Does anything drop a valueless row between storage and the query?
No.**
| layer | file | reading |
|---|---|---|
| console lowering: `viewFilterRuleToNode`, behind `toFilterNode` /
`mergeFilterNodes` (plugin-list `buildEffectiveFilter`, plugin-view
`ObjectView`, `ObjectGrid`, `RelatedList`, `LineItemsPanel`) | objectui
`packages/core/src/utils/filter-converter.ts` | a rule without `value`
lowers to the 2-tuple `[field, operator]` and nothing skips it; its own
comment records the runtime throwing `INVALID_FILTER` / 400 for
`['name','icontains']` |
| REST lookup-picker route: `lowerViewFilterRule` | this repo,
`packages/rest/src/view-filter-rule-lowering.ts` | the same 2-tuple; the
module forwards and never drops |
| query normalizer | this repo,
`packages/metadata-protocol/src/protocol.ts` | `isFilterAST`, then
`parseFilterAST`, which throws |
Measured on this tree's spec source (4112752): `isFilterAST(['and',
['name','equals'], ['status','equals','open']])` is true, and
`parseFilterAST` of it throws `INVALID_FILTER` / 400, "Filter comparand
at where.$and[0].name is undefined". One valueless rule fails the WHOLE
view's query, its good rules included.
So no working flow saves or executes this shape, and refusing it at save
breaks nothing that works today.
## Today's behaviour for the whole class (step 2)
Measured at `origin/main` 4112752 by script. The operator list is
`VIEW_FILTER_OPERATORS` read at runtime; the unary set was derived by
behaviour from the schema's own scalar arm (an array is refused on every
non-list, non-range operator outside the private valueless set).
| operators | `ViewFilterRuleSchema`, `value` omitted, before this
change | `parseFilterAST([field, op])` |
|---|---|---|
| `equals`, `not_equals`, `contains`, `not_contains`, `icontains`,
`starts_with`, `ends_with`, `greater_than`, `less_than`,
`greater_than_or_equal`, `less_than_or_equal`, `before`, `after` (13) |
**ACCEPT** | throws `INVALID_FILTER` / 400, "Filter comparand at
where.name (or where.name.$op) is undefined" |
| `in`, `not_in` | refused by the list arm | throws, "requires an ARRAY
of values" |
| `between` | refused by the range arm | throws, "requires a [min, max]
value array" |
| `is_empty`, `is_not_empty`, `is_null`, `is_not_null` | accept | `{
"$null": true }` / `{ "$null": false }` |
After this change the 13 are refused. The other rows are unchanged.
## ADR-0087 reading (step 4)
- This narrows a published accept set. The repo's rule for that during
the launch window is in the header of
`scripts/check-changeset-no-major.mjs`: the level does not carry
breaking-ness, and "the mandatory information carriers for breaking-ness
in the meantime are the **BREAKING** banner the author writes in the
changeset body and the ADR-0087 migration-ledger disposition".
`scripts/check-adr-0087-registration.mjs` then requires a disposition on
the declared-breaking changeset.
- The disposition is `registered`, not `not-required`. The author has a
hand prescription (write the value, switch to a unary operator, or
delete an unfinished row), and `no-migration-prescription` is refused
for a body that carries one. None of the other categories fits: the
package publishes, no existing entry covers absence, and the surface is
a schema, not a runtime interface or a type surface.
- Precedents: `view-filter-rule-scalar-operator-array-refused` (the
sibling arm of this same check) and
`filter-preset-ordering-comparand-refused` (a shape that never executed
usefully) both registered a semantic entry under protocol major 18.
- Added:
`packages/spec/src/migrations/entries/semantic/18.view-filter-rule-absent-value-refused.ts`.
`packages/spec/src/migrations/registry.ts` was regenerated by `pnpm
--filter @objectstack/spec gen:migration-registry` and not hand-edited;
`check:migration-registry` is green. No D2 conversion: there is no value
to infer.
- `check-adr-0087-registration --base origin/main` reads the changeset
as `[BREAKING+clause-②-narrowing] registered
view-filter-rule-absent-value-refused (new here)`.
- `spec-changes.json` and `docs/protocol-upgrade-guide.md` did not move.
The protocol-18 step stays inert until the protocol major reaches 18,
and `check:spec-changes` / `check:upgrade-guide` are green without
regeneration.
## Changeset (step 7)
`.changeset/19751-view-filter-rule-absent-value-refused.md`, `minor` on
`@objectstack/spec`. `files[]` ships `dist` and `src/**/*.zod.ts`, and
both carry the refinement. Its summary is the user-visible change: a
stored view filter rule with no value on a value-taking operator is now
refused at save instead of failing every query. It carries the BREAKING
banner, a FROM/TO block, `Clause-②: no (narrowing)` and the registered
disposition marker. One sentence names that it reverses the carve-out
the still-pending #19514 changeset records (an omitted value "still
parses", an absent comparand "is left unjudged"), so the two entries do
not contradict each other in the compiled CHANGELOG. The #19514 file
itself is not edited.
Level: `minor`. An accept-set narrowing declared `(narrowing)` is
BREAKING (AGENTS.md, Post-Task Checklist step 3), and during the launch
window a breaking change ships as `minor`: the header of
`scripts/check-changeset-no-major.mjs` says "During the launch window we
ship breaking changes as `minor`", and that the BREAKING banner and the
ADR-0087 disposition carry the break, not the level. Both precedents
above shipped `minor` with the same banner. The first round graded this
`patch`; the at-tier contract review (record 5808364674) failed that,
and the patch-round commit ab0104d changes the frontmatter to `minor`
and rewrites the banner sentence to state the convention ("Shipped as
`minor` under the repo's launch-window convention for accept-set
narrowings"). That commit moves no package file.
The PR's `Clause-②: no (narrowing)` line is the claim's, copied
verbatim, and matches the changeset's line. With the arm present, the
level axis of `check-changeset-no-major.mjs` judges the level instead of
standing down. Measured offline with `--event` on this body, it refuses
the first round's `patch` head 22a14a1 (exit 1) and passes `minor` at
ab0104d (exit 0).
## Fixtures, examples and pins (step 5)
- An AST scan of every tracked `.ts` / `.tsx` / `.mts` / `.js` / `.mjs`
/ `.json` outside `content/docs/references/` (1,739 files mention
`operator`) found 311 object literals with `field` plus a string-literal
value-taking operator (aliases folded). 20 of them have no `value` key,
and none is a view filter rule in a shipped example or seed:
- 7 are QA assertions (`expectedValue`, a different schema) in
`examples/app-showcase/qa/platform-smoke.test.json`;
- 5 are QA assertions in `packages/core/src/qa/runner.test.ts` and
`packages/spec/src/qa/testing.test.ts`;
- 1 is a skill trigger condition in
`packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts`;
- 4 are a structural walk with no schema in
`packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts`;
- 3 are in `view-filter-rule-value-shape.test.ts`.
Markdown (`.md` / `.mdx`) has no match. No fixture was an authoring
mistake, so no fixture was edited.
- Pins that pinned the removed carve-out and moved with it:
- `packages/spec/src/ui/view-filter-rule-value-shape.test.ts`: `equals +
omitted` and `greater_than + omitted`, from accepted to refused.
- `packages/spec/src/data/filter-icontains-parse-door.test.ts`: "ABSENCE
is left unjudged" now asserts that absence is refused once, in the
absent-value arm's words and never in the conformance table's. 1 parent 7536721 commit cc6dfd9
6 files changed
Lines changed: 355 additions & 20 deletions
File tree
- .changeset
- packages/spec/src
- data
- migrations
- entries/semantic
- ui
Lines changed: 44 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
Lines changed: 12 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
201 | 201 | | |
202 | 202 | | |
203 | 203 | | |
204 | | - | |
| 204 | + | |
205 | 205 | | |
206 | 206 | | |
207 | 207 | | |
208 | | - | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
209 | 219 | | |
210 | 220 | | |
211 | 221 | | |
| |||
Lines changed: 59 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13639 | 13639 | | |
13640 | 13640 | | |
13641 | 13641 | | |
| 13642 | + | |
| 13643 | + | |
| 13644 | + | |
| 13645 | + | |
| 13646 | + | |
| 13647 | + | |
| 13648 | + | |
| 13649 | + | |
| 13650 | + | |
| 13651 | + | |
| 13652 | + | |
| 13653 | + | |
| 13654 | + | |
| 13655 | + | |
| 13656 | + | |
| 13657 | + | |
| 13658 | + | |
| 13659 | + | |
| 13660 | + | |
| 13661 | + | |
| 13662 | + | |
| 13663 | + | |
| 13664 | + | |
| 13665 | + | |
| 13666 | + | |
| 13667 | + | |
| 13668 | + | |
| 13669 | + | |
| 13670 | + | |
| 13671 | + | |
| 13672 | + | |
| 13673 | + | |
| 13674 | + | |
| 13675 | + | |
| 13676 | + | |
| 13677 | + | |
| 13678 | + | |
| 13679 | + | |
| 13680 | + | |
| 13681 | + | |
| 13682 | + | |
| 13683 | + | |
| 13684 | + | |
| 13685 | + | |
| 13686 | + | |
| 13687 | + | |
| 13688 | + | |
| 13689 | + | |
| 13690 | + | |
| 13691 | + | |
| 13692 | + | |
| 13693 | + | |
| 13694 | + | |
| 13695 | + | |
| 13696 | + | |
13642 | 13697 | | |
13643 | 13698 | | |
13644 | 13699 | | |
| |||
0 commit comments