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

.changeset/20662-null-key-todo-reason.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): the stored-filter conversion's TODO for a null-valued key is true on every block, and no longer tells the operator to drop the key
+
+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` lists. The TODO's reason used to say the renderer skips that key, so it "constrains nothing", and to "Drop the key". That holds only where the block queries an object. Where the block's rows are inline (`data: { provider: 'value' }` or `staticData`), the objectui version this repository pins matches the key against the rows and selects the rows whose value is null, so following the advice there widened what the block shows.
+
+The reason now states both behaviours, says no one rule keeps both, and leaves the choice to the operator. For a stored `{ owner_id: null }` it names the rule `{"field":"owner_id","operator":"is_null"}` for the rows with no `owner_id` value, and says that a filter leaving `owner_id` unconstrained has no rule for it. The protocol-18 migration entry `element-data-source-and-object-block-filter-rule-array` says the same.
+
+The TODO for a key set to an empty operator object (`{ amount: {} }`) also said it "constrains nothing". The renderer refuses it instead: where the block queries an object it refuses the filter with `INVALID_FILTER` (400), and where the block's rows are inline it shows no rows. The reason now says that, and keeps its advice to drop the key, which is the renderer's own remedy.
+
+Nothing else changes. Both filters are still left exactly as stored and still reported as a TODO, on any block. No schema, conversion verdict or exit code moves.
+
+Clause-②: no

packages/spec/src/conversions/page-component-filter-record-to-rule-array.test.ts

@@ -32,6 +32,7 @@
 
 import { describe, expect, it } from 'vitest';
 
+import { StandardErrorCode } from '../api/errors.zod.js';
 import {
   FILTER_OPERATORS,
   VALID_AST_OPERATORS,
@@ -320,7 +321,8 @@ describe('§2 what has no lossless rule spelling is left byte-identical', () =>
   // The third column is what the site's TODO must say, or `null` for the one
   // row that is not a legacy form at all (see the last row).
   const DECLINED_ROWS: ReadonlyArray<readonly [string, unknown, string | null]> = [
-    // The renderer at the pin skips a null key (constrains nothing); a rule would test IS NULL.
+    // At the pin a null key constrains nothing where the block queries an object and selects
+    // the null rows where its rows are inline — no one rule keeps both.
     ['a null value', { owner_id: null }, 'has the key `owner_id` set to null'],
     ['a null value beside a mappable key', { stage: 'open', owner_id: null }, 'has the key `owner_id` set to null'],
     // Direction lives in the VALUE — not in the one operator table.
@@ -357,7 +359,7 @@ describe('§2 what has no lossless rule spelling is left byte-identical', () =>
   });
 
   it('all-or-nothing: a declined key keeps the mappable keys beside it from converting', () => {
-    // Converting `stage` alone would drop `owner_id: null` from an AND list — a wider filter.
+    // Converting `stage` alone would drop `deleted_at: { $null: true }` from an AND list — a wider filter.
     const { value } = gridFilter({ stage: 'open', deleted_at: { $null: true } });
     expect(value).toEqual({ stage: 'open', deleted_at: { $null: true } });
   });
@@ -646,6 +648,51 @@ describe('§8 the TODO channel — every site left as stored is reported (ruling
     expect(inline.todos[0]!.reason).toBe(bound.todos[0]!.reason);
   });
 
+  it('a null-valued key is a TODO in the same words on an inline-row node and an object-bound one, naming an `is_null` rule its door takes', () => {
+    // At the objectui pin the key constrains nothing where the block queries an
+    // object and selects the rows whose value is null where its rows are inline,
+    // so no one rule keeps both: the one reason has to be true on either block.
+    const inline = convert(
+      pageWith({ type: 'object-map', properties: { staticData: [], filter: { owner_id: null } } }),
+    );
+    const bound = convert(
+      pageWith({ type: 'object-map', properties: { objectName: 'deal', filter: { owner_id: null } } }),
+    );
+    expect((componentOf(inline.stack).properties as Dict).filter).toEqual({ owner_id: null });
+    expect(inline.todos).toHaveLength(1);
+    expect(inline.todos[0]!.reason).toBe(bound.todos[0]!.reason);
+    const rule = { field: 'owner_id', operator: 'is_null' };
+    expect(inline.todos[0]!.reason).toContain(JSON.stringify(rule));
+    // The rule it names is one the block's door takes.
+    const door = ComponentPropsMap['object-map'] as unknown as {
+      safeParse: (v: unknown) => { error?: { issues: Array<{ path: PropertyKey[] }> } };
+    };
+    const atFilter = (v: unknown): number =>
+      door.safeParse(v).error?.issues.filter((i) => i.path[0] === 'filter').length ?? 0;
+    expect(atFilter({ objectName: 'deal', filter: [rule] })).toBe(0);
+    // Control: the door really judges this key — the stored record is refused there.
+    expect(atFilter({ objectName: 'deal', filter: { owner_id: null } })).toBeGreaterThan(0);
+  });
+
+  it('an empty operator object is a TODO in the same words on an inline-row node and an object-bound one, naming the refusal the renderer answers', () => {
+    // At the objectui pin the renderer refuses `{ amount: {} }` rather than
+    // ignoring it — `INVALID_FILTER` where the block queries an object, no rows
+    // where its rows are inline — and the one reason says so on either block.
+    const inline = convert(
+      pageWith({ type: 'object-map', properties: { staticData: [], filter: { amount: {} } } }),
+    );
+    const bound = convert(
+      pageWith({ type: 'object-map', properties: { objectName: 'deal', filter: { amount: {} } } }),
+    );
+    expect((componentOf(inline.stack).properties as Dict).filter).toEqual({ amount: {} });
+    expect(inline.todos).toHaveLength(1);
+    expect(inline.todos[0]!.reason).toBe(bound.todos[0]!.reason);
+    const code = 'INVALID_FILTER';
+    // The code it names is one the platform declares.
+    expect(StandardErrorCode.options).toContain(code);
+    expect(inline.todos[0]!.reason).toContain(`\`${code}\``);
+  });
+
   it('names the block by its type, and by its `id` when it has one', () => {
     const { todos } = convert(
       pageWith({ type: 'object-kanban', id: 'pipeline_board', properties: { objectName: 'deal', filter: { $or: [] } } }),

packages/spec/src/conversions/registry.ts

@@ -11314,11 +11314,22 @@ function dollarKeysReason(keys: readonly string[]): string {
  * — `$and` / `$or` / `$not` above all — is not a field, so its record is left
  * alone; that is the ruled boundary, and flattening a combinator into the AND
  * list is exactly the silent selection change it excludes. A `null` value is
- * declined too, and not for a schema reason: the renderer at the
- * `.objectui-sha` pin (`convertFiltersToAST`) SKIPS a record key whose value is
- * null, so that key constrains nothing today, while an `equals null` rule would
- * test IS NULL. An empty operator object is declined for the same reason — it
- * constrains nothing, and no rule says "nothing".
+ * declined too, and not for a schema reason: at the `.objectui-sha` pin the key
+ * selects different rows on different blocks, so no one rule keeps it. Where a
+ * block queries an object, `convertFiltersToAST` SKIPS a record key whose value
+ * is null, so the key constrains nothing; where a block's rows are inline,
+ * `ValueDataSource.find` matches the record through `comparandEquals`, so the
+ * key selects the rows whose value is null. This entry never reads where a
+ * block's rows come from, so its reason states both and advises neither
+ * rewrite: it names the `is_null` rule for the rows with no value, and leaves
+ * which rows the filter should select to the author. An empty operator object
+ * is declined for a different reason: it names a field and no operator, so no
+ * rule spells it. At the same pin the renderer refuses it rather than ignoring
+ * it — where a block queries an object, `convertFiltersToAST` throws through
+ * `refuseEmptyOperatorMap` (`INVALID_FILTER`, 400); where a block's rows are
+ * inline, `ValueDataSource.find` answers no rows through
+ * `zeroKeyConditionRefusal`. Its reason says both and keeps the renderer's own
+ * remedy, dropping the key.
  *
  * Every top-level `$` key is judged before any field key, so the reason names
  * the combinator even when a field key beside it would decline as well. The
@@ -11336,10 +11347,15 @@ function recordFilterToRules(record: Record<string, unknown>): FilterMapping {
       continue;
     }
     if (value === null) {
+      const isNull = 'is_null' satisfies ViewFilterOperator;
       return {
-        declined: `has the key \`${field}\` 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',
+        declined: `has the key \`${field}\` 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\' }` or `staticData`), it selects the rows whose '
+          + `\`${field}\` is null. Decide which rows it should select: the rows with no \`${field}\` `
+          + `value are the rule \`${JSON.stringify({ field, operator: isNull })}\`, and a filter that `
+          + `leaves \`${field}\` unconstrained has no rule for it`,
       };
     }
     if (!isRecordForm(value)) {
@@ -11352,8 +11368,10 @@ function recordFilterToRules(record: Record<string, unknown>): FilterMapping {
     const operators = Object.entries(value);
     if (operators.length === 0) {
       return {
-        declined: `has the key \`${field}\` set to an empty operator object, which constrains `
-          + 'nothing — and no rule says "nothing". Drop the key',
+        declined: `has the key \`${field}\` 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',
       };
     }
     for (const [op, comparand] of operators) {
@@ -11525,7 +11543,8 @@ function describeBlock(component: Dict): string {
  *
  * A record carrying `$and` / `$or` / `$not` (or any top-level `$` key), an AST
  * `and` / `or` group, an operator the rule vocabulary does not spell (`$null`,
- * `$exists`, `like`, …), a `null` value (the renderer skips that key today),
+ * `$exists`, `like`, …), a `null` value (skipped where a block queries an
+ * object, matched where its rows are inline — no one rule keeps both),
  * an array or object comparand in equality position, and any rule the door
  * would refuse. All-or-nothing per filter: converting part of an AND-list
  * widens it. ⛔ A combinator is never flattened into the AND list — for `$or`
@@ -11566,10 +11585,14 @@ function describeBlock(component: Dict): string {
  * `staticData`) is rewritten exactly as a block that queries an object:
  * measured at the `.objectui-sha` pin `dd3f7e1be356`, the renderers that match
  * inline rows in memory (`object-map`, `object-tree`, `object-calendar`,
- * `object-gantt`, through `ValueDataSource.find`) lower a rule array through
+ * `object-gantt`, through `ValueDataSource.find`) take those rows from
+ * `data: { provider: 'value' }` or `staticData`, lower a rule array through
  * the grid's own sink before matching, and select the same rows for it as for
  * the stored form — every mapped operator, against a control where the
- * lowering is absent and the rule array selects none.
+ * lowering is absent and the rule array selects none. A bare `data` array
+ * reaches none of them: `object-calendar` draws it as pre-fetched rows with no
+ * filter applied, and `object-map` / `object-gantt` do not take it as a record
+ * source.
  *
  * ## Why `retiredFromLoadPath`
  *

packages/spec/src/migrations/entries/semantic/18.element-data-source-and-object-block-filter-rule-array.ts

@@ -70,12 +70,15 @@ export const entry: SemanticMigration = {
     + 'path: an author writing the record form is still refused at the `filter` door. ⚠️ A '
     + 'filter carrying `$and` / `$or` / `$not` is left exactly as stored — the rule array '
     + 'only ANDs, and flattening a combinator changes which rows the page selects — and so is '
-    + 'any filter with a part that has no lossless rule spelling: a `null` value (the renderer '
-    + 'skips that key, so it constrains nothing today, where a rule would test IS NULL), an '
+    + 'any filter with a part that has no lossless rule spelling: a `null` value (where a block '
+    + 'queries an object the renderer skips that key, so it constrains nothing, and where its '
+    + 'rows are inline it selects the rows whose value is null — no one rule keeps both, so the '
+    + 'TODO names the `is_null` rule for the rows with no value and leaves which rows to select '
+    + 'to the author), an '
     + 'operator such as `$null` / `$exists` or an AST `like`, an array or object comparand in '
     + 'equality position, or an AST `and` / `or` group. None of this depends on where a '
     + 'block\'s rows come from: a filter on a component whose rows are inline (`data: { '
-    + 'provider: \'value\' }`, a `data` array, or `staticData`) — the binding\'s included — is '
+    + 'provider: \'value\' }` or `staticData`) — the binding\'s included — is '
     + 'rewritten or left exactly as it would be on a block that queries an object, because the '
     + '`object-map`, `object-tree`, `object-calendar` and `object-gantt` blocks of the objectui '
     + 'version this release pins match a rule array against those rows and select the rows the '

packages/spec/src/migrations/registry.ts

@@ -9397,12 +9397,15 @@ const step18: MigrationStep = {
         + 'path: an author writing the record form is still refused at the `filter` door. ⚠️ A '
         + 'filter carrying `$and` / `$or` / `$not` is left exactly as stored — the rule array '
         + 'only ANDs, and flattening a combinator changes which rows the page selects — and so is '
-        + 'any filter with a part that has no lossless rule spelling: a `null` value (the renderer '
-        + 'skips that key, so it constrains nothing today, where a rule would test IS NULL), an '
+        + 'any filter with a part that has no lossless rule spelling: a `null` value (where a block '
+        + 'queries an object the renderer skips that key, so it constrains nothing, and where its '
+        + 'rows are inline it selects the rows whose value is null — no one rule keeps both, so the '
+        + 'TODO names the `is_null` rule for the rows with no value and leaves which rows to select '
+        + 'to the author), an '
         + 'operator such as `$null` / `$exists` or an AST `like`, an array or object comparand in '
         + 'equality position, or an AST `and` / `or` group. None of this depends on where a '
         + 'block\'s rows come from: a filter on a component whose rows are inline (`data: { '
-        + 'provider: \'value\' }`, a `data` array, or `staticData`) — the binding\'s included — is '
+        + 'provider: \'value\' }` or `staticData`) — the binding\'s included — is '
         + 'rewritten or left exactly as it would be on a block that queries an object, because the '
         + '`object-map`, `object-tree`, `object-calendar` and `object-gantt` blocks of the objectui '
         + 'version this release pins match a rule array against those rows and select the rows the '