Skip to content

Commit e41642d

Browse files
feat(studio): the formula editor inserts a field by label and reads a CEL syntax fault in plain words (#27901)
domain:ui#3 · os-bill · session_01Utp6CaJoinvGQSbHeDaWBe <!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg4i9JiKjEjwtuVzd4gKy9hB)_ Fixes #27837 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 公式编辑器加了「插入字段」(按字段标签选,插入 `record.字段名`);CEL 语法错误先给一句白话,再用等宽字体原样显示引擎的错误和 ^ 位置标记;打字时新出现的报错会自动滚到可见处。 - 为什么改:RC 走查里公式框只有裸文本框、报错是 "Unexpected token: EOF … ^" 这类技术话,且报错行落在 Remove 按钮那一栏下面看不见(浏览器实测复现)。 - 风险与回滚:只动 Studio 的 CEL 编辑器与 studio 文案(en/zh);生产者 `@objectstack/formula` 的 message 不变,存盘门禁与客户端校验读到的文字不变;回滚即 revert 本 PR。 - 先看:`CelPredicateField.tsx`(插入字段与滚动)、`celAuthoring.ts`(按 refusal code 加白话首句)、`ObjectFieldInspector.tsx`(传入带标签的字段清单)。 - 两分钟验证:Studio 打开任一对象的公式字段,点「插入字段」选一个字段,框里出现 `record.字段名`;再输入 `record.amount *`,下方先出白话一句,再出带 ^ 的引擎原文,且这一行在 Remove 按钮上方可见。 - 需要你做的:无,席位落地。 ## What changes The formula field's **Formula (CEL)** editor is the shared `CelPredicateField`. It gains: 1. **Insert field.** With the new optional `fieldOptions` prop (`{ name, label }[]`), the editor shows an **Insert field** picker: a searchable list of fields by label (machine name beside it), matching on either. Choosing one inserts the CEL reference at the caret, replacing any selection: `record.` plus the name in `record` scope, the bare name otherwise. The caret lands after the insert. `ObjectFieldInspector` passes the object's other fields (not the formula field itself, which may not reference itself), labelled through the same `useSafeFieldLabel` resolver the canvas and grid use; a field with no label shows its name. No other host passes `fieldOptions`, so no other editor shows a picker. 2. **A syntax fault in plain words.** `lintCelPredicate` now reads the refusal `code` and `params.detail` that `@objectstack/formula`'s `validateExpression` already returns (its `expression-refusal.ts` vocabulary exists so a localized surface keys its own sentence to the code). For the catch-all CEL code `invalid-cel` the lint issue carries `plain: { key, detail }`, and the editor leads with `engine.celLint.invalidCel` (a plain sentence) followed by the engine's `detail` (its fault plus the source excerpt with the `^` mark) in a monospace block, so the mark lines up under the spot. `message` is unchanged, so the Save gate, `clientValidation` and the server-verdict dedupe read the producer's words as before. Every other refusal (an `unknown-field` with its did-you-mean, `cel-unknown-function`, `cel-method-call`, `cel-template-brace`, …) keeps its message: those already carry a prescription for their fault. 3. **The finding stays in view.** When the editor has focus and its findings change, the findings list is scrolled into view (`block: 'nearest'`), so in a scrolling inspector a new error line no longer lands below the visible area where the **Remove** footer sits. A finding present on mount does not scroll the panel. Studio i18n (en, zh) gains `perm.cel.insertField`, `perm.cel.searchFields`, `perm.cel.noMatchingFields` and `engine.celLint.invalidCel`. Root changeset: `@object-ui/app-shell` patch, `@object-ui/i18n` patch. ## Tests - `CelPredicateField.formulaAuthoring-27837.test.tsx` (11 tests): the lint against the REAL `@objectstack/formula` (`record.estimated_hours *` → `invalid-cel` with `plain`; `record.estimated_hour * 2` → `unknown-field` keeps its message, no `plain`; an inherited property name as a code gets no lead); the editor renders the plain key then the excerpt in a `pre` with the `^`; Insert field lists by label, inserts `record.estimated_hours` at the caret with the caret after it, replaces a selection, searches by label, inserts the bare name in flattened scope, and is absent without `fieldOptions` or when read-only; findings scroll into view under a focused editor and not on mount. - `ObjectFieldInspector.formulaFieldPicker-27837.test.tsx` (1 test): the real inspector offers the siblings `name`, `estimated_hours`, `hourly_rate` (not `est_cost` itself) and choosing **Estimated Hours** writes `expression: 'record.estimated_hours'`. <details><summary>Ablations (one per behaviour, each restored to HEAD by the tool)</summary> All through `node scripts/ablation-replace.mjs … -- bash scripts/pm/os-verify-lock.sh -c "pnpm --dir ui exec vitest run (the two new files)"`; app-shell tests import `src`, so no dist build is involved. - **plain**: delete the `'invalid-cel'` row of `PLAIN_LEAD_CODES` → anchor x1→x0, blob 98f36c1→61e831add2c1 → `Tests 4 failed | 7 passed (11)` (the two plain tests, and the two scroll tests, which wait on the plain line) → restored, blob == HEAD. - **picker**: insert the bare name in `record` scope → anchor x1→x0 → `Tests 3 failed | 8 passed (11)` (both caret-insert tests and the inspector test) → restored. - **scroll**: delete the `scrollIntoView` call → anchor x1→x0 → `Tests 1 failed | 10 passed (11)` → restored. </details> ## Acceptance notes - **The footer overlap, measured once in a browser before any layout change** (as the card asked): a scratch Vite bundle of the real `StudioInspector` + hosted `InspectorShell` (footer `InspectorRemoveButton`) + `CelPredicateField`, real Tailwind theme, Chromium at 1280×720, a filler above the editor so it sits at the bottom of the scroll area, then typing `record.estimated_hours *`. Without the scroll (control leg, `scrollIntoView` stubbed out): scroll viewport 68–622, error line 627–720, `elementFromPoint` at the error line = the Remove footer (`div.border-t.px-4.py-2.5`), so the line is hidden under the footer. With this PR: error line 529–622, inside the viewport, `elementFromPoint` = the error line. In both legs the footer does NOT overlay the content box (footer top 622 = viewport bottom 622), which agrees with #27830's code read: the shell is not wrong; the line simply lands below the visible area. The probe was not committed. - **The producer seam, named.** The plain sentence is one per refusal code. `@objectstack/formula` collapses every CEL parse fault into `invalid-cel` with the engine's text as `detail`; cel-js itself knows the fault's code (`unexpected_token`, `unterminated_string`, …) and its range, and `EvalError` in `packages/formula/src/types.ts` declares `pos?: { start, end }` ("Source position when known") that `classifyError` never fills. A per-fault plain sentence ("the formula ends too early", "a quote is not closed") and a "at character N" would need the formula package to carry cel-js's parse code and range into the refusal params. That is a change to a published package outside this card's file surface: filed by the seat as #27904 (`domain:engine`), not made here. - `invalid-cel` also covers type faults (`1 + 'a'` → "no such overload: int + string"), so the plain sentence is worded for both: something is missing, extra, out of place, or doesn't fit the values beside it. - `ui/AGENTS.md` #2 (docs): no hand-written page under `content/docs/` describes the Studio formula editor (only release notes mention it, which are release-owned), so no doc page changed. - The picker is offered only on the formula editor. The field-rule editors (`visibleWhen` / `readonlyWhen` / `requiredWhen`) share the component and could pass `fieldOptions` too; not in this card. ## Verification All local readings below are on head `261cf0a4` (the branch with `origin/main` merged in, no conflict). - `pnpm --filter @object-ui/app-shell run type-check` and `pnpm --filter @object-ui/i18n run type-check`, after `pnpm --filter '@object-ui/app-shell^...' build`: lock VERDICT command-exit 0. - Affected tests: every `ui/` test file that names `CelPredicateField`, `celAuthoring`, `ObjectFieldInspector` or `lintCelPredicate` (100 files, the two new ones included) plus `packages/i18n/`: `Test Files 191 passed (191)`, `Tests 2422 passed | 13 skipped (2435)`. The whole `ui/` suite is left to CI's Test UI. - `pnpm --dir ui check:new-line-citations`: `0 new citation(s)`. <details><summary>Derived gates: 34 families, each exit captured before any pipe; dispatch-gates --ran reconciles 34 run, 0 NOT-MEASURED</summary> ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:pm-changeset-deadline-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` </details> Every other family CI runs (lint, type-check lanes, Test UI, Build and E2E UI) is read from this PR's check-runs, not re-run here. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CbrXDpQYUAJ8MKZpNL)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 813fd9b commit e41642d

8 files changed

Lines changed: 484 additions & 9 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/i18n': patch
4+
---
5+
6+
feat(studio): the formula editor inserts a field by its label, leads a CEL syntax fault with a plain sentence, and keeps a new finding in view
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
The Studio CEL editor (a formula field's "Formula (CEL)", and the other CEL editors that share it) gains three authoring aids:
12+
13+
- **Insert field.** The formula editor has an **Insert field** picker listing the object's other fields by the label the author knows them by (with the machine name beside it), searchable by either. Choosing one inserts its CEL reference (`record.` followed by the field's name, e.g. `record.estimated_hours`) at the caret, replacing any selected text. The field being edited is not offered.
14+
- **A syntax fault in plain words.** When `@objectstack/formula` refuses an expression with its catch-all CEL code (`invalid-cel`: a missing operand, an unclosed bracket or string, a value that does not fit its operator), the editor leads with a plain sentence and then shows the engine's own fault with its source excerpt in monospace, so the `^` mark lines up under the spot it points to. Refusals that carry their own prescription (an unknown field with its "did you mean", an unknown function, a template brace) keep their message. The producer's message is unchanged for every other reader (the Save gate, the client spec check, the server-verdict comparison).
15+
- **The finding stays in view.** A finding that appears while the author types is scrolled into view, so in a scrolling inspector it no longer lands below the visible area behind the **Remove** footer.
16+
17+
`@object-ui/i18n` gains `perm.cel.insertField`, `perm.cel.searchFields`, `perm.cel.noMatchingFields` and `engine.celLint.invalidCel` in the studio namespace (en, zh).
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#27837 — the formula editor's three authoring aids, on the shared
5+
* CEL editor:
6+
*
7+
* 1. a CEL syntax fault (`invalid-cel`, the real engine's verdict) leads with
8+
* a plain sentence, and the engine's fault with its source excerpt follows
9+
* as written, in monospace; a refusal with its own prescription keeps its
10+
* message, and the producer's `message` is unchanged for every caller;
11+
* 2. "Insert field" lists the fields by label and inserts the CEL reference
12+
* at the caret — `record.<name>` in `record` scope, the bare name otherwise;
13+
* 3. a finding that appears while the author types is scrolled into view.
14+
*/
15+
16+
import * as React from 'react';
17+
import { describe, it, expect, afterEach, vi } from 'vitest';
18+
import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/react';
19+
import userEvent from '@testing-library/user-event';
20+
import { CelPredicateField } from './CelPredicateField';
21+
import { lintCelPredicate, __setCelFormulaLoader } from './celAuthoring';
22+
23+
// jsdom draws no layout and has no `scrollIntoView`; the scroll tests stub it
24+
// on the prototype and this puts back whatever was there.
25+
const ORIGINAL_SCROLL = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'scrollIntoView');
26+
function stubScrollIntoView() {
27+
const spy = vi.fn();
28+
Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', { value: spy, configurable: true, writable: true });
29+
return spy;
30+
}
31+
32+
afterEach(() => {
33+
cleanup();
34+
__setCelFormulaLoader(undefined);
35+
if (ORIGINAL_SCROLL) Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', ORIGINAL_SCROLL);
36+
else delete (HTMLElement.prototype as { scrollIntoView?: unknown }).scrollIntoView;
37+
});
38+
39+
const t = (k: string) => k;
40+
41+
const FIELDS = [
42+
{ name: 'estimated_hours', label: 'Estimated Hours' },
43+
{ name: 'hourly_rate', label: 'Hourly Rate' },
44+
];
45+
46+
function Formula({ initial = '', ...rest }: { initial?: string } & Record<string, unknown>) {
47+
const [v, setV] = React.useState(initial);
48+
return (
49+
<>
50+
<CelPredicateField
51+
id="f"
52+
value={v}
53+
onChange={setV}
54+
label="Formula"
55+
objectName="project_task"
56+
fieldNames={FIELDS.map((f) => f.name)}
57+
fieldOptions={FIELDS}
58+
scope="record"
59+
roots={['record']}
60+
role="value"
61+
t={t}
62+
{...rest}
63+
/>
64+
<output data-testid="value">{v}</output>
65+
</>
66+
);
67+
}
68+
69+
describe('a CEL syntax fault reads in plain words, with where it is (objectstack#27837)', () => {
70+
it('the lint keeps the producer message and adds the plain key and the engine detail', async () => {
71+
const issues = await lintCelPredicate('record.estimated_hours *', {
72+
fields: ['estimated_hours'],
73+
scope: 'record',
74+
role: 'value',
75+
});
76+
const err = issues.find((i) => i.severity === 'error');
77+
expect(err?.plain?.key).toBe('engine.celLint.invalidCel');
78+
// The detail is the engine's, with its source excerpt; the message is the
79+
// producer's whole sentence, which still carries that detail.
80+
expect(err?.plain?.detail).toContain('record.estimated_hours *');
81+
expect(err?.message).toContain(err!.plain!.detail);
82+
});
83+
84+
it('a refusal that carries its own prescription keeps its message and gets no plain lead', async () => {
85+
const issues = await lintCelPredicate('record.estimated_hour * 2', {
86+
fields: ['estimated_hours'],
87+
scope: 'record',
88+
role: 'value',
89+
});
90+
const err = issues.find((i) => i.severity === 'error');
91+
expect(err?.message).toContain('estimated_hours');
92+
expect(err?.plain).toBeUndefined();
93+
});
94+
95+
it('only a code the table names is led: an inherited property name is not one', async () => {
96+
__setCelFormulaLoader(async () => ({
97+
validateExpression: () => ({
98+
ok: false,
99+
errors: [{ message: 'm', code: 'constructor', params: { detail: 'd' } }],
100+
warnings: [],
101+
}),
102+
}));
103+
const [err] = await lintCelPredicate('x', { scope: 'record', role: 'value' });
104+
expect(err).toEqual({ severity: 'error', message: 'm' });
105+
});
106+
107+
it('the editor shows the plain sentence, then the engine excerpt in monospace', async () => {
108+
render(<Formula initial="record.estimated_hours *" />);
109+
const plain = await screen.findByTestId('cel-plain-error');
110+
expect(plain.textContent).toContain('engine.celLint.invalidCel');
111+
const excerpt = plain.querySelector('pre');
112+
expect(excerpt?.textContent).toContain('record.estimated_hours *');
113+
expect(excerpt?.textContent).toContain('^');
114+
// The technical lead-in of the producer sentence is not what the author reads first.
115+
expect(plain.textContent?.startsWith('engine.celLint.invalidCel')).toBe(true);
116+
});
117+
});
118+
119+
describe('Insert field: by label, inserting the CEL name (objectstack#27837)', () => {
120+
it('lists the fields by label and inserts record.<name> at the caret', async () => {
121+
const user = userEvent.setup();
122+
render(<Formula initial=" * 2" />);
123+
const ta = screen.getByRole('combobox', { name: 'Formula' }) as HTMLTextAreaElement;
124+
ta.focus();
125+
ta.setSelectionRange(0, 0);
126+
await user.click(screen.getByTestId('cel-insert-field'));
127+
const option = await screen.findByRole('option', { name: /Estimated Hours/ });
128+
expect(option.textContent).toContain('estimated_hours');
129+
await user.click(option);
130+
expect(screen.getByTestId('value').textContent).toBe('record.estimated_hours * 2');
131+
await waitFor(() => expect(document.activeElement).toBe(ta));
132+
expect(ta.selectionStart).toBe('record.estimated_hours'.length);
133+
});
134+
135+
it('replaces the selected text, and the search matches the label', async () => {
136+
const user = userEvent.setup();
137+
render(<Formula initial="record.x * 2" />);
138+
const ta = screen.getByRole('combobox', { name: 'Formula' }) as HTMLTextAreaElement;
139+
ta.focus();
140+
ta.setSelectionRange(0, 'record.x'.length);
141+
await user.click(screen.getByTestId('cel-insert-field'));
142+
await user.type(await screen.findByPlaceholderText('perm.cel.searchFields'), 'rate');
143+
expect(screen.queryByRole('option', { name: /Estimated Hours/ })).toBeNull();
144+
await user.click(screen.getByRole('option', { name: /Hourly Rate/ }));
145+
expect(screen.getByTestId('value').textContent).toBe('record.hourly_rate * 2');
146+
});
147+
148+
it('in flattened scope the bare name goes in', async () => {
149+
const user = userEvent.setup();
150+
render(<Formula scope="flattened" role="predicate" />);
151+
await user.click(screen.getByTestId('cel-insert-field'));
152+
await user.click(await screen.findByRole('option', { name: /Hourly Rate/ }));
153+
expect(screen.getByTestId('value').textContent).toBe('hourly_rate');
154+
});
155+
156+
it('no picker without fieldOptions, and none while read-only', () => {
157+
const { rerender } = render(<Formula fieldOptions={undefined} />);
158+
expect(screen.queryByTestId('cel-insert-field')).toBeNull();
159+
rerender(<Formula disabled />);
160+
expect(screen.queryByTestId('cel-insert-field')).toBeNull();
161+
});
162+
});
163+
164+
describe('a new finding is brought into view while typing (objectstack#27837)', () => {
165+
it('scrolls the findings into view once they appear under a focused editor', async () => {
166+
const spy = stubScrollIntoView();
167+
render(<Formula />);
168+
const ta = screen.getByRole('combobox', { name: 'Formula' });
169+
ta.focus();
170+
fireEvent.change(ta, { target: { value: 'record.estimated_hours *' } });
171+
await screen.findByTestId('cel-plain-error');
172+
await waitFor(() => expect(spy).toHaveBeenCalledWith({ block: 'nearest' }));
173+
});
174+
175+
it('does not scroll for a finding read on mount (the editor not focused)', async () => {
176+
const spy = stubScrollIntoView();
177+
render(<Formula initial="record.estimated_hours *" />);
178+
await screen.findByTestId('cel-plain-error');
179+
expect(spy).not.toHaveBeenCalled();
180+
});
181+
});

0 commit comments

Comments
 (0)