Repository navigation
fix(app-shell): the condition builder's is-empty / is-not-empty compile a null-or-empty check on a non-boolean field, and the Rules tab passes field types (objectui#11894) - #11935
Merged
Conversation
…ompile a null-or-empty check on a non-boolean field (objectui#11894) The two value-less operators are typed by the field catalog: a boolean field keeps "is true" / "is false" (bare subject / `!`), any other declared field gets "is not empty" / "is empty", compiled by the spec's `expandEmptyOperator` arm (null only; null or ''; null or an empty list). An undeclared subject keeps the untyped pair and its emission. A new row starts on `equals`. `parse` reads every new emission back into the same row; a stored `!field` on a non-boolean field no longer round-trips and opens in the raw editor verbatim. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
…g helper Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
… field's type (objectui#11894) ObjectValidationsPanel's field list carried names, labels and hidden flags only, so at the validation-rule mount every field read as undeclared and the benchmark rule still saved as `!record.due_date`. FieldOpt now carries the field's `type` and `multiple`, copied from the draft the way useObjectFields and ResourceEditPage already read them. Pin through the panel; changeset re-read against the final diff (the multi-value sentence is qualified). Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Resolves ObjectValidationsPanel's FieldOpt and fields memo as a union with objectui#11931: `type` (now read by the New menu's presets and by the condition builder), `system` and `multiple`. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
… condition builder (objectui#11894) Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
…g objectui#11931 The Rules tab passes `type` since objectui#11931; this change adds `multiple` there. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Contributor
❌ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. Which half objected:
📦 Bundle Size Report
Size Limits
|
objectui#11931 was reverted on main (3c888c6), so ObjectValidationsPanel's FieldOpt and fields memo carry this change's own `type` and `multiple` again; `system` and the presets are gone with the revert. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
…ain after the objectui#11931 revert The builder pin's metadata-client stub answers `withPreviewDrafts`, which useObjectFields calls since objectui#11895. The panel pin and the changeset no longer name the reverted presets: this change is again what makes the Rules tab pass `type`, as well as `multiple`. Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11894
Clause-②: no
What this changes
Studio's condition builder typed none of its value-less operators. is true / set compiled to the bare subject and is false / empty to
!plus the subject, whatever the field was. CEL's!, and a bare&&operand, take a bool. So on a date, text or lookup field the predicate could not be evaluated, and a server-evaluated condition refused the write it guarded. The benchmark rule (record.status == 'done' && !record.due_date) refused every write to a done ticket withno such overload: !null, or!stringwhen the ticket had a due date.ConditionBuilder(packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx):booleanwhen the type is in the spec'sBOOLEAN_VALUE_TYPES(by reference);expandEmptyOperator({ type, multiple })assigns it. No local copy of the table.S/!S).null_only:S == null/S != null;text:S == null || S == ''and its complement;multi_value:S == null || size(S) == 0and its complement;formulafield: undeclared on purpose. Its value takes itsreturnType, which no catalog row carries. This follows the spec's own boolean comparand door, which defers a formula without a readablereturnType. It also keeps "is true" on a boolean formula compiling as it did.equalsinstead oftruthy.S == ''is well typed for every value.parseand stored conditions:parsereads every new emission back into the row it was built as. It splits at top-level joins and respects parentheses and string literals. A lone compound check reads as one row, not two comparisons.!fieldon a typed non-boolean field no longer round-trips, so it opens in the raw editor exactly as written. This is the component's existing round-trip-or-raw rule, and the same non-retroactive shapeConditionBuilder.referenceValue.test.tsxpins. ⛔ No silent rewrite on load or on a later edit.objectNamelands after the first render, the stored value is re-read under the declared types, unless the author has already edited it. A builder-made check then reopens as rows, and a stored!fieldopens in the raw editor, the same way at a mount that fetches its fields as at one that is handed them.== nulland nothas(): the server evaluates a validation rule or a hook condition over a record whose declared fields are all present. A field with no value holdsnull, sohas()is true for an empty field. objectql's rule-validator contract says the same: test emptiness with!= null, and keephas()for an undeclared key. Measured with the formula engine:has(record.due_date)on{ due_date: null }istrue.Four new designer rows (en, zh) in
metadata-admin/i18n.ts:engine.inspector.condition.op.isTrue/isFalse/isNotEmpty/isEmpty. The two existing rows stay for undeclared subjects. No language-pack key, no export, no published type member:ConditionBuilder.d.tsis not reachable fromdist/index.d.ts(declaration-graph walk over the published 17.7.0 tarball;SchemaForm.d.tsreached, as the control).The in-place producer change: the Studio Rules tab
The benchmark's own mount handed the builder no types.
ObjectValidationsPanel'sFieldOptcarried{ name, label, hidden }only, so every field there read as undeclared and the rule still saved as!record.due_date.FieldOptnow carriestypeandmultiple. Itsfieldsmemo copiese.def.typewhen it is a string, andmultiple: e.def.multiple === true, the shapeuseObjectFieldsandResourceEditPagealready read.The claim was amended for exactly this: card comment 6055312520 adds
FieldOptand thefieldsmemo ofstudio-design/ObjectValidationsPanel.tsx, plus one pin beside it, to the File surface. No other region of that file differs frommain.The other mounts already passed the type: hook condition, action visible / disabled, page-block visibility and flow conditions through
useObjectFields, and the schema-form condition widget throughResourceEditPage's catalog. None of them passesmultiple, so the changeset says where the multi-value arm reaches a multi-capable type.Evidence (head
ec8411f3)Pins evaluate the CEL the builder emits with
@objectstack/formula'sExpressionEngine.evaluate(expr, { record, previous }), the call objectql'scheckPredicatemakes:ConditionBuilder.typedEmptyOps-11894.test.tsx(26 tests):''and for null;tags, and on a lookup withmultiple: trueagainst a single lookup;!S/S;equals; the fetched-catalog re-read;no such overload) in the same engine, both ways.ObjectValidationsPanel.typedEmptyOps-11894.test.tsx(5 tests), mounting the Rules tab:record.status == 'done' && record.due_date == null, which evaluates true with no due date and false with one;!record.due_dateopens in the raw editor verbatim;multiplebuilds "is empty" asrecord.status == 'done' && (record.watchers == null || size(record.watchers) == 0), which evaluates true for[]and for null and false for a member;!record.urgent/record.urgent.Ablations, each through objectstack
scripts/ablation-replace.mjsin wrap mode. Each mutation was proven on disk (anchor count and blob), and each restore was proven: the blob equals HEAD andgit diff HEADis empty. No build leg in either: both pins import the subject by relative path, not through a package export.compileRow's falsy arm emits!Sagain (atd235d45): the builder pin went 10 failed / 16 passed, the date pin among the reds. Expected direction: red.The
typecopy is dropped from the Rules tab memo (atec8411f3): the panel pin went 5 failed / 5, the benchmark pin among the reds. Themultiplepin and the boolean control are red too: without a type every field there reads as undeclared, and the operators are worded by type.The
multiplecopy is dropped from the same memo (at4cc894f): 1 failed / 4 passed. The red one is themultiplepin (receivedrecord.status == 'done' && record.watchers == null). That line is unchanged since.Tests at
00a030d, before the later merges ofmain, under the shared verify lock: 374 files / 3416 tests passed. The set is every app-shell test whose import closure reachesConditionBuilder.tsx, plus the app-shell tests that read the designer i18n table. The closure is derived from static relative imports,import(), side-effect imports andvi.mockspecifiers, so it is blind to a test that reaches the builder through a package specifier or reads sources from disk.Tests at
ec8411f3, after mergingmain1c51e973: 17 files / 177 tests passed. They are everyObjectValidationsPanel*.test.tsx, everyConditionBuilder*.test.tsx(objectui#11895 changed nine of them), anduseObjectFields.draftOverlay-11895. The full package suite is CI's.Bundle Analysisonec8411f3: success. Console Performance Budget: eager closure 3308.0 KB gzip (336 chunks) against a budget of 3312.0 KB, PASS; main entry chunk 71.3 KB against 350 KB.pnpm --filter @object-ui/app-shell type-check: exit 0 atec8411f3, after a turbo build of its dependency closure. It runstsc -p tsconfig.test.jsontoo, which types both new test files.pnpm exec eslint: 0 errors atec8411f3. Across the panel and both pins there is 1 warning, in the panel: the pre-existingset-state-in-effectthatmain's ownObjectValidationsPanel.tsxblob carries.ConditionBuilder.tsxcarries the 7 warnings of its baseline (measured atd235d45; the file is unchanged since).check:changeset-claims,check:pending-changeset-literals,check:new-line-citations,check:control-bytes,check:test-path-roots,check:i18n-keys,check:i18n-drift,check:i18n-designer-parity,check:vi-mock-specifiers,check-changeset-presence,check-changeset-no-major: all exit 0 atec8411f3.check:spec-floors: NOT MEASURED locally (it needs a workspace build). The fact it judges was read directly:@objectstack/spec@17.6.0, app-shell's declared floor, exportsBOOLEAN_VALUE_TYPES,expandEmptyOperatorandEmptyOperatorArmfrom./data.The full app-shell package suite and a browser pass are NOT MEASURED locally; CI runs the farm. The eager budget is the Bundle Analysis reading above.
Acceptance notes
Implemented by Claude Code, session
https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8(the dispatching seat's session).Generated by Claude Code