Commit 99786f9
fix(spec): the stored-filter conversion's TODO for a null-valued key is true on every block, and no longer says to drop the key (#20709)
Fixes #20662
Clause-②: no (author-shown wording only; no accept or reject moves)
## What changes
The ADR-0087 D2 conversion `page-component-filter-record-to-rule-array`
leaves a record-form filter with a `null`-valued key as stored and
reports it as a TODO, which `os migrate meta --stored` prints. Its
reason said the renderer skips that key, so it "constrains nothing
today", and told the operator to "Drop the key". At the `.objectui-sha`
pin `dd3f7e1be356` that is true only on a block that queries an object.
On a block whose rows are inline, the key selects the rows whose value
is null, so dropping it widens the block.
Following triage's direction (comment 5893989151: one wording true on
both kinds of block, no "drop the key" advice), the reason now:
- states what the key does on each kind of block: skipped where the
block queries an object, so it constrains nothing; matched where its
rows are inline (`data: { provider: 'value' }` or `staticData`), so it
selects the rows whose value is null;
- says that no one rule keeps both;
- names the rule for the rows with no value,
`{"field":"owner_id","operator":"is_null"}` (built from the key), and
says that a filter which leaves the key unconstrained has no rule for
it. It advises neither rewrite. The choice is the author's.
The verdict does not move. The filter is still declined, left
byte-identical and reported as one TODO, on any block. The reason text
does not branch on where the rows come from; the conversion never reads
that.
Round 2 (seat note 5897754447, a claim amendment): the
empty-operator-object reason beside it (`{ amount: {} }`) said the
object "constrains nothing". At the pin the renderer refuses it instead.
Where the block queries an object, `convertFiltersToAST` throws through
`refuseEmptyOperatorMap` (`INVALID_FILTER`, 400). Where the block's rows
are inline, `ValueDataSource.find` answers no rows through
`zeroKeyConditionRefusal`. The reason and its docblock sentence now say
that, say that no rule spells an operator object with no operator, and
keep the renderer's own remedy, dropping the key. The verdict does not
move.
Files:
- `packages/spec/src/conversions/registry.ts`: the reason string in
`recordFilterToRules`, the sentence in its docblock, and the conversion
entry docblock's parenthetical in "What is left exactly as stored" ("the
renderer skips that key today"). That parenthetical is a fourth copy of
the same claim in the same file. It is text only and fixed in place:
same defect, same file under this claim, same gates. Round 2 rewrites
the empty-operator-object declined reason and its docblock sentence,
text only. Round 3 (review 5898951990) drops "a `data` array" from the
null-key reason's inline list: at the pin a bare `data` array reaches no
`ValueDataSource.find`. Round 4 makes the rationale in the conversion
entry docblock's `## Reach` paragraph name the inline sources the filter
reaches at the pin (`data: { provider: 'value' }` or `staticData`), and
adds that a bare `data` array reaches none of them (`object-calendar`
draws it unfiltered; `object-map` / `object-gantt` do not take it as a
record source). Comment text only; the verdict sentence is unchanged.
-
`packages/spec/src/migrations/entries/semantic/18.element-data-source-and-object-block-filter-rule-array.ts`:
the same claim in the D3 entry's `reason`.
`packages/spec/src/migrations/registry.ts` is regenerated by
`gen:migration-registry` and not edited by hand. Round 3 narrows the
inline list in its older sentence ("None of this depends on where a
block's rows come from …") to `data: { provider: 'value' }` or
`staticData`.
-
`packages/spec/src/conversions/page-component-filter-record-to-rule-array.test.ts`:
the `DECLINED_ROWS` comment that restated the null-key claim; the
all-or-nothing test's comment, which named `owner_id: null` for a row
that is `deleted_at: { $null: true }`; and two new pins. A null-valued
key gets the same reason on an inline-row `object-map` and an
object-bound one. That reason names the `is_null` rule for the key, and
the block's door takes that rule; the control is that the door refuses
the stored record. An empty operator object gets the same reason on both
blocks, and that reason names `INVALID_FILTER`, a code in
`StandardErrorCode`.
- `.changeset/20662-null-key-todo-reason.md`: `@objectstack/spec` patch,
covering both reasons.
## Verification record
**Pin reading** (`dd3f7e1be356`, raw source):
- `packages/core/src/utils/filter-converter.ts` `convertFiltersToAST`
skips a key whose value is `null`/`undefined` (`skippedNullKeys`).
- `packages/core/src/adapters/ValueDataSource.ts` `find` sends an object
`$filter` to `matchesFilter`, and its simple-equality arm compares with
`comparandEquals`, which is `value === target`. Its `is_null` arm is
`value === null || value === undefined`. Server-side, `parseFilterAST`
lowers `is_null` to `{ $null: true }`.
- The inline branches of `ObjectMap` / `ObjectCalendar` pass
`useResolvedFilter(schema.filter)` to `new ValueDataSource(...).find`.
`filter-tokens.ts` `resolveContextTokens` returns a `null` value
unchanged.
- The #20305 dev's live probe (report 5893209491) measured that `find`
with `{ owner_id: null }` selects only the null row, and not the `'u1'`
row or the row that has no key.
**Lit, at base `31ed067639`** (the stored-migration pass and
`formatStoredMigrationReport`, the function `os migrate meta --stored`
prints through, over a one-page stub `sys_metadata` holding an
`object-grid` that queries `deal` and an `object-map` with `data: {
provider: 'value' }`, both `filter: { owner_id: null }`):
```text
TODO page-component-filter-record-to-rule-array: {"owner_id":null} left as stored at pages[0].regions[0].components[1].properties.filter — On the `object-map` block `inline`, this filter has the key `owner_id` set to null: the renderer skips a null-valued key, so today it constrains nothing, while an `equals` rule would test for null. Drop the key, or write a rule that tests for null if that is what it should select. Left as stored, it keeps loading unchanged, but it is not the rule-array form its door declares — rewrite it by hand.
```
The `object-grid` line carried the same sentence.
**Dark, at `a51c02fe83`** (same probe, spec rebuilt):
```text
TODO page-component-filter-record-to-rule-array: {"owner_id":null} left as stored at pages[0].regions[0].components[1].properties.filter — On the `object-map` block `inline`, this filter has the key `owner_id` set to null, and what that key selects depends on where the block's rows come from, so no one rule keeps it: where the block queries an object, the renderer skips a null-valued key, so it constrains nothing; where its rows are inline (`data: { provider: 'value' }`, a `data` array or `staticData`), it selects the rows whose `owner_id` is null. Decide which rows it should select: the rows with no `owner_id` value are the rule `{"field":"owner_id","operator":"is_null"}`, and a filter that leaves `owner_id` unconstrained has no rule for it. Left as stored, it keeps loading unchanged, but it is not the rule-array form its door declares — rewrite it by hand.
```
In both runs the row is still `skipped` with two TODOs. The probe file
was temporary and is not in the diff.
**Round 2, empty operator object** (same printer probe, with `filter: {
amount: {} }` on the same two blocks). Lit at `c5eed1b4d3`: "... this
filter has the key `amount` set to an empty operator object, which
constrains nothing — and no rule says "nothing". Drop the key. Left as
stored, ...". Dark at `3fcedfb564`: "... set to an empty operator
object, which names the field and no operator, so no rule spells it. The
renderer does not ignore it today: where the block queries an object, it
refuses the filter (`INVALID_FILTER`, 400); where its rows are inline,
it answers no rows. Drop the key. Left as stored, ...". Both runs: row
`skipped`, two TODOs. Pin reading at `dd3f7e1be356`:
`filter-converter.ts:818` calls `refuseEmptyOperatorMap`, whose
`FilterOperatorError` has `code = 'INVALID_FILTER'` and `httpStatus =
400`; `ValueDataSource.find` answers `[]` when `zeroKeyConditionRefusal`
returns a refusal.
**Round 3, the inline list** (the null-key printer probe, rows `null` /
`u1` / missing). Lit at `3fcedfb564`: "... where its rows are inline
(`data: { provider: 'value' }`, a `data` array or `staticData`), it
selects the rows whose `owner_id` is null. ...". Dark at `a6e54de377`:
"... where its rows are inline (`data: { provider: 'value' }` or
`staticData`), it selects the rows whose `owner_id` is null. ...". Pin
reading at `dd3f7e1be356`: `record-source.ts:303-307` folds `staticData`
to `{ provider: 'value', items }`, and the value branches hand the
resolved filter to `ValueDataSource.find` (`ObjectMap.tsx:950-952`,
`ObjectCalendar.tsx:708-710`, `ObjectTree.tsx:911-913`,
`ObjectGantt.tsx:1009-1010`). A bare `data` array reaches none of them:
`ObjectCalendar.tsx:387-389, 599-600, 647` draws it with no fetch and no
filter, and the `view-data` arm refuses an array
(`record-source.ts:179`). The round-1 Dark quote above is the
`a51c02fe83` reading, before this narrowing.
**Tests and gates, at `6f1396efa2`** (this branch merged with
`origin/main` `671d4c164f` through `scripts/pm/os-regen-merge.sh`; the
delta against main is exactly the five files above). Round 4's
one-comment commit `eaf2d6e6e8` re-ran the conversions and migrations
set (1034 passed), spec typecheck, `check:generated` (15 up to date),
`check:doc-authoring` and `check:issue-citations`, all green; the
derived gate list is unchanged:
- `@objectstack/spec` `local` project: 575 files, 16972 passed, 1 todo.
`typecheck`, including the test layer, passed.
- `repo` project, narrowed to the three files that read the conversion
and migration registries (`conversions-major18-merge`,
`step18-rationale-merge`, `retired-key-migrate-sentence`): 35 passed.
The full `repo` project did not finish inside the foreground cap and is
NOT MEASURED locally; CI runs it.
- `check:generated`: all 15 artifacts are up to date against a spec
rebuilt after the merge.
- `dispatch-gates --commands` derived 87 commands. 84 exited 0,
including `check:migration-registry`, `check:spec-changes`,
`check:upgrade-guide`, `check:docs`, `check:api-surface`,
`check:authorable-surface`, `check:objectui-pin-citations`,
`check:doc-authoring`, `check:nul-bytes` and
`check:adr-0087-registration`. Three exited 3 with PREREQUISITE NOT MET,
because they need a whole-workspace build: `check:dual-build-cjs-loads`,
`check:lean-entry-closure` and `check:type-check-debt`. They are NOT
MEASURED locally. `--ran` reconciles 87 derived: 84 run, 3 NOT-MEASURED,
0 unrun.
- Ablation of the new pin (`scripts/ablation-replace.mjs`, from the
committed state): the anchor `operator: isNull })}` was replaced so the
reason named an `equals`/`null` rule instead. The new test went red:
expected the reason to contain
`{"field":"owner_id","operator":"is_null"}`. The other five null-related
tests stayed green. The file was restored: its blob matches HEAD and
`git diff HEAD` is empty.
- Round-2 ablation of the empty-operator pin, from committed
`bcda701b88`: the anchor "block queries an object, it refuses the filter
(`INVALID_FILTER`, 400); where its rows " was replaced with "block
queries an object, it constrains nothing; where its rows ". The pin went
red: expected the reason to contain `INVALID_FILTER`. The file was
restored: its blob matches HEAD `f1f29e6f4802` and `git diff HEAD` is
empty.
## Acceptance notes
- No test pinned the false clause. The existing rows assert only the
prefix "has the key `owner_id` set to null", so there was nothing to
re-pin. The new pin checks named subjects: the same reason on both
blocks, the `is_null` rule, and that the door takes it. It does not pin
prose. The empty-operator reason was the same: only its prefix was
asserted.
- `packages/spec/CHANGELOG.md`'s 17.5.0 entry carries the old null-key
sentence. It is a released record and is not edited; the corrected text
ships in this PR's changeset.
---
_Generated by [Claude
Code](https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent f05919b commit 99786f9
5 files changed
Lines changed: 112 additions & 21 deletions
File tree
- .changeset
- packages/spec/src
- conversions
- migrations
- entries/semantic
| 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 | + | |
Lines changed: 49 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| 35 | + | |
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| |||
320 | 321 | | |
321 | 322 | | |
322 | 323 | | |
323 | | - | |
| 324 | + | |
| 325 | + | |
324 | 326 | | |
325 | 327 | | |
326 | 328 | | |
| |||
357 | 359 | | |
358 | 360 | | |
359 | 361 | | |
360 | | - | |
| 362 | + | |
361 | 363 | | |
362 | 364 | | |
363 | 365 | | |
| |||
646 | 648 | | |
647 | 649 | | |
648 | 650 | | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
649 | 696 | | |
650 | 697 | | |
651 | 698 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11314 | 11314 | | |
11315 | 11315 | | |
11316 | 11316 | | |
11317 | | - | |
11318 | | - | |
11319 | | - | |
11320 | | - | |
11321 | | - | |
| 11317 | + | |
| 11318 | + | |
| 11319 | + | |
| 11320 | + | |
| 11321 | + | |
| 11322 | + | |
| 11323 | + | |
| 11324 | + | |
| 11325 | + | |
| 11326 | + | |
| 11327 | + | |
| 11328 | + | |
| 11329 | + | |
| 11330 | + | |
| 11331 | + | |
| 11332 | + | |
11322 | 11333 | | |
11323 | 11334 | | |
11324 | 11335 | | |
| |||
11336 | 11347 | | |
11337 | 11348 | | |
11338 | 11349 | | |
| 11350 | + | |
11339 | 11351 | | |
11340 | | - | |
11341 | | - | |
11342 | | - | |
| 11352 | + | |
| 11353 | + | |
| 11354 | + | |
| 11355 | + | |
| 11356 | + | |
| 11357 | + | |
| 11358 | + | |
11343 | 11359 | | |
11344 | 11360 | | |
11345 | 11361 | | |
| |||
11352 | 11368 | | |
11353 | 11369 | | |
11354 | 11370 | | |
11355 | | - | |
11356 | | - | |
| 11371 | + | |
| 11372 | + | |
| 11373 | + | |
| 11374 | + | |
11357 | 11375 | | |
11358 | 11376 | | |
11359 | 11377 | | |
| |||
11525 | 11543 | | |
11526 | 11544 | | |
11527 | 11545 | | |
11528 | | - | |
| 11546 | + | |
| 11547 | + | |
11529 | 11548 | | |
11530 | 11549 | | |
11531 | 11550 | | |
| |||
11566 | 11585 | | |
11567 | 11586 | | |
11568 | 11587 | | |
11569 | | - | |
| 11588 | + | |
| 11589 | + | |
11570 | 11590 | | |
11571 | 11591 | | |
11572 | | - | |
| 11592 | + | |
| 11593 | + | |
| 11594 | + | |
| 11595 | + | |
11573 | 11596 | | |
11574 | 11597 | | |
11575 | 11598 | | |
| |||
Lines changed: 6 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
73 | | - | |
74 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
75 | 78 | | |
76 | 79 | | |
77 | 80 | | |
78 | | - | |
| 81 | + | |
79 | 82 | | |
80 | 83 | | |
81 | 84 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9397 | 9397 | | |
9398 | 9398 | | |
9399 | 9399 | | |
9400 | | - | |
9401 | | - | |
| 9400 | + | |
| 9401 | + | |
| 9402 | + | |
| 9403 | + | |
| 9404 | + | |
9402 | 9405 | | |
9403 | 9406 | | |
9404 | 9407 | | |
9405 | | - | |
| 9408 | + | |
9406 | 9409 | | |
9407 | 9410 | | |
9408 | 9411 | | |
| |||
0 commit comments