Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .changeset/row-predicate-record-canon-5330.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
---
'@object-ui/core': minor
'@object-ui/react': minor
---

Row predicates declare a canon: `record.*`. The bare shorthand and `data.*` now
warn once, and are unchanged otherwise.

A row predicate (`visible` / `disabled` / `enabled` on an action renderer, a row
scope, a `record:alert`) has bound the row three ways since objectui#4075 —
`record.status`, bare `status`, and `data.status` — without any of them being
declared the contract. The maintainer ruled that question on 2026-08-20
(objectui#5330, option B), mirroring the objectstack#7917 option-② precedent for
the identical renderer-tolerance shape: **the canon is `record.*`**, and the
other two enter a deprecation window.

The canon states the **server's** accept set, which was this card's first
measurement and turns out to be strictly narrower than the renderer's. Measured
against `@objectstack/formula@17.1.0`, the engine the server evaluates with:

| spelling | server runtime | server authoring oracle |
|---|---|---|
| `record.status` | `{ ok: true, value: true }` | accepted |
| bare `status` | `Unknown variable: status` | refused |
| `data.status` | `Unknown variable: data` | **silently accepted** |

`buildScope({ record })` mounts exactly `['record']` — `data` is never bound and
the row's fields are never flattened to top level. The three-way binding is a
client tolerance with no server counterpart, which is why the warning belongs on
this side.

`data.*` is the dangerous one, and the reason the warning exists. `data` is in
`@objectstack/formula`'s `SCOPE_ROOTS`, so the server's bare-identifier oracle
waves it through — that list is a deliberately generous "never faults" lint
baseline, not the runtime accept set. A `data.*` row predicate therefore passes
every authoring gate the platform has and then binds nothing at runtime: not an
error, a constant `false`. A `visible` that is constantly false is a button that
silently never appears — the objectui#4075 fail-closed signature.

What ships:

- `@object-ui/core` exports `detectNonCanonicalRowSpelling`,
`warnNonCanonicalRowSpelling`, `resetRowPredicateCanonWarnings` and
`ROW_PREDICATE_CANONICAL_ROOT` from a new `evaluator/rowPredicateCanon.ts`,
which carries the canon statement and the measurement.
- Both evaluation tiers report once, in dev: `evalRowPredicate` (core) and
`useCondition` (react, for bags bound by `usePredicateRecordContext`).
- Detection reuses the server's own oracles (`collectCelRootIdentifiers`,
`firstUndeclaredReference`) rather than a regex, so no second dialect
judgement is invented client-side.

**No spelling is removed and no behaviour changes.** Every predicate that
resolved before resolves now — the ruling defers removal behind a stored-metadata
survey, and the warning is what makes that survey possible (ADR-0078: a
tolerance nothing ever reports can never be retired).

The deprecation is scoped to the **runtime record layer**. `data` remains the
canonical root one layer over, in a metadata-editing form (ADR-0089 D3
`CANONICAL_ROOT_BY_LAYER`), and the detector stands down there.
150 changes: 150 additions & 0 deletions packages/core/src/evaluator/__tests__/rowPredicateCanon.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#5330 — the row-predicate spelling CANON (`record.*`) and its Phase-1
* deprecation warning.
*
* These pins are deliberately split in two, because the card's two halves fail
* in opposite directions:
*
* - the CANON pins assert the binding is UNCHANGED. The ruling defers every
* removal behind a stored-metadata survey, so a test that stopped resolving
* the shorthand would be the regression, not the feature.
* - the WARNING pins assert the tolerance is no longer silent — the ADR-0078
* reason a tolerance nothing reports can never be retired.
*
* The `record-alert` renderer's own three-spelling pins landed separately with
* PR #5688 (`plugin-detail/.../record-alert.rowBinding.test.tsx`) and are NOT
* duplicated here; this file pins the shared evaluator tier those renderers sit
* on, plus the detector itself.
*/

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import {
detectNonCanonicalRowSpelling,
resetRowPredicateCanonWarnings,
ROW_PREDICATE_CANONICAL_ROOT,
evalRowPredicate,
} from '../index.js';

const row = { status: 'in_review', amount: 10 };

beforeEach(() => resetRowPredicateCanonWarnings());

describe('[#5330] the canon is `record.*`', () => {
it('names `record` as the one canonical root', () => {
expect(ROW_PREDICATE_CANONICAL_ROOT).toBe('record');
});

it('reports nothing for the canonical spelling', () => {
expect(detectNonCanonicalRowSpelling("record.status == 'in_review'", row, true)).toBeNull();
});
});

describe('[#5330] non-canonical spellings are DETECTED', () => {
it('reports the bare shorthand, and names the canonical rewrite', () => {
expect(detectNonCanonicalRowSpelling("status == 'in_review'", row, true)).toEqual({
kind: 'bare-shorthand',
identifier: 'status',
canonical: 'record.status',
});
});

it('reports a `data.`-rooted predicate on a record surface', () => {
expect(detectNonCanonicalRowSpelling("data.status == 'in_review'", row, true)).toEqual({
kind: 'metadata-layer-root',
identifier: 'data',
canonical: 'record',
});
});
});

/**
* Every case here is a spelling the detector must NOT report. They are the
* whole reason it consults the row and the caller's binding rather than
* pattern-matching the source: a false deprecation warning sends an author to
* rewrite a predicate that was correct.
*/
describe('[#5330] the detector stands down rather than guessing', () => {
it('leaves `data.*` alone when `data` is NOT this row (rowless / metadata-editing layer)', () => {
// ADR-0089 D3: `data` is the CANONICAL root of a metadata-editing form.
// Reporting it there would contradict that layer's own contract.
expect(detectNonCanonicalRowSpelling("data.status == 'in_review'", row, false)).toBeNull();
});

it('leaves a host-scope root alone', () => {
expect(detectNonCanonicalRowSpelling('features.beta == true', row, true)).toBeNull();
});

it('leaves an undeclared identifier alone when it is not a field of THIS row', () => {
// A deployment global this module cannot see is not the #4075 shorthand.
expect(detectNonCanonicalRowSpelling('unknownGlobal == 1', row, true)).toBeNull();
});

it('leaves an unparseable source alone — syntax is another gate’s verdict', () => {
expect(detectNonCanonicalRowSpelling("record.status === 'x'", row, true)).toBeNull();
});
});

describe('[#5330] `evalRowPredicate` — the binding is UNCHANGED (no removal before the survey)', () => {
it('still resolves all three spellings against the row', () => {
expect(evalRowPredicate("record.status == 'in_review'", row)).toBe(true);
expect(evalRowPredicate("status == 'in_review'", row)).toBe(true);
expect(evalRowPredicate("data.status == 'in_review'", row)).toBe(true);
});

it('still tells the three spellings apart on a NON-matching row (not vacuously true)', () => {
const other = { status: 'draft', amount: 1 };
expect(evalRowPredicate("record.status == 'in_review'", other)).toBe(false);
expect(evalRowPredicate("status == 'in_review'", other)).toBe(false);
expect(evalRowPredicate("data.status == 'in_review'", other)).toBe(false);
});
});

describe('[#5330] `evalRowPredicate` — the tolerance is no longer silent', () => {
let warn: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
});
afterEach(() => warn.mockRestore());

const deprecationWarnings = (): string[] =>
warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('DEPRECATED spelling'));

it('warns on the bare shorthand and prescribes `record.status`', () => {
evalRowPredicate("status == 'in_review'", row, { label: 'row action "approve"' });
const msgs = deprecationWarnings();
expect(msgs).toHaveLength(1);
expect(msgs[0]).toContain('record.status');
expect(msgs[0]).toContain('objectui#5330');
expect(msgs[0]).toContain('row action "approve"');
});

it('warns on `data.*` and says the server binds no `data` at all', () => {
evalRowPredicate("data.status == 'in_review'", row);
const msgs = deprecationWarnings();
expect(msgs).toHaveLength(1);
expect(msgs[0]).toContain('metadata-editing-form root');
});

it('stays silent for the canonical spelling', () => {
evalRowPredicate("record.status == 'in_review'", row);
expect(deprecationWarnings()).toHaveLength(0);
});

it('warns ONCE per (label, predicate) — these run on every row of every frame', () => {
for (let i = 0; i < 5; i++) evalRowPredicate("status == 'in_review'", row, { label: 'grid' });
expect(deprecationWarnings()).toHaveLength(1);
});

it('does NOT report a legacy `${…}`-dialect predicate, where `data.*` is the normal spelling', () => {
evalRowPredicate('${data.status === "in_review"}', row);
expect(deprecationWarnings()).toHaveLength(0);
});
});
1 change: 1 addition & 0 deletions packages/core/src/evaluator/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ export * from './ExpressionEvaluator.js';
export * from './predicateInput.js';
export * from './declaredPredicate.js';
export * from './fieldRules.js';
export * from './rowPredicateCanon.js';
export * from './listConditional.js';
export * from './optionRules.js';
export * from './optionLint.js';
Expand Down
34 changes: 32 additions & 2 deletions packages/core/src/evaluator/listConditional.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,7 @@
import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js';
import { ExpressionEvaluator } from './ExpressionEvaluator.js';
import { toPredicateRecord, type FieldContainerLike } from '../utils/predicate-record.js';
import { warnNonCanonicalRowSpelling } from './rowPredicateCanon.js';

/**
* Syntax that only the legacy JS-dialect evaluator understands and that is NOT
Expand DownExpand Up@@ -206,8 +207,27 @@ export interface RowPredicateOptions {
* Evaluate a single boolean predicate against a row record on the canonical CEL
* engine (with a legacy-dialect fallback — see the module note). The row's
* fields are bound three ways so every authoring convention resolves:
* `record.status` (spec/canonical), bare `status` (row-action shorthand), and
* `data.status` (legacy). The optional `scope` (host predicate scope) is bound
* `record.status`, bare `status` (row-action shorthand), and `data.status`.
*
* ⚠️ Those three are NOT peers, and this doc comment used to read as though
* they were. **The canon is `record.*`** (maintainer ruling 2026-08-20 on
* objectui#5330, option B); the other two are client tolerances in a
* deprecation window, kept because stored metadata carries them and warned
* about — from the CEL path below — by `warnNonCanonicalRowSpelling`. The
* server accepts `record.*` and NOTHING else: measured on
* `@objectstack/formula@17.1.0`, `buildScope({ record })` mounts exactly
* `['record']`, so a bare field faults `Unknown variable: status` there and a
* `data.*` predicate faults `Unknown variable: data`. See
* {@link ./rowPredicateCanon.ts} for the full measurement, for why `data.*` is
* the dangerous one (it is silently ACCEPTED by the server's authoring oracle
* and still binds nothing at runtime), and for why the deprecation is scoped to
* this runtime layer rather than declared platform-wide (`data` is the
* canonical root of a metadata-editing form — ADR-0089 D3).
*
* ⛔ No spelling is removed from the binding, and none may be before a
* stored-metadata survey sizes the window — that is part of the same ruling.
*
* The optional `scope` (host predicate scope) is bound
* alongside so `features.*` / `user.*` predicates keep working — but the row is
* the subject: `record` and `data` always name THIS row, on both dialect paths,
* even when the host scope carries keys of those names (objectui#3796).
Expand DownExpand Up@@ -287,6 +307,16 @@ export function evalRowPredicate(
}
}

// CEL path — everything reaching here is CEL, which is what makes this the
// right place for the objectui#5330 spelling warning: in the legacy `${…}`
// dialect above, `data.*` is the NORMAL spelling, so reporting it there would
// be a false positive on every legacy predicate. `data` names THIS row unless
// the caller is `rowless` (then the host scope keeps its own — see the
// option), which is exactly the condition the detector needs.
if (predicateText !== '(expression)') {
warnNonCanonicalRowSpelling(predicateText, rowObj, !opts.rowless, opts.label);
}

// CEL path. The fault-aware `evalCel` costs two evaluations (to tell a fault
// from a genuine `false`), so only pay it when a caller wants the labelled
// fail-closed warning — the formatting hot path takes the single-eval fast
Expand Down
Loading
Loading