Uh oh!
There was an error while loading. Please reload this page.
feat(react): evaluate the spec-declared expression-bindable text keys - #6981
Merged
Conversation
objectui#4795 Direction 1, maintainer ruling 2026-08-25. The evaluation memo now evaluates the top-level text keys `@objectstack/spec` declares as expression-bindable, consuming `expressionBindableTextKeysFor` rather than keeping a twin list. `statistic.value` / `card.title` / `button.label` and their siblings are now evaluated at the one place that produces evaluated schema, so the existing top-level read-back sites see the evaluated value with no per-component patch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
…e gap The published rules and three guides carried the now-retired claim that `title`/`label`/`value`/`description` are never template-evaluated, and prescribed host pre-resolution or a `text` node's `content` as the only routes. objectui#4795 makes that false for the spec-declared rows, so protocol.md gains a "Bindable Text Keys" rule and the guides point at it; the still-true half (an undeclared type reads these keys raw) is kept and stated as the boundary. `skill-guide-provider-envelope.test.tsx`'s node-key reading is flipped to the new behaviour with the undeclared-key case added beside it, so the file still states both fates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Contributor
ℹ️ Console Performance Budget — not measuredThis run did not produce a console bundle to measure, so there is no pass/fail verdict for the performance budget. This is not a budget violation. Nothing was measured — the numbers a real violation would carry are simply absent.
See the workflow run for details. No package size report: it is only generated from a complete package build, so a partial one is never shown. |
This was referenced Aug 31, 2026
…kfile CI installs with `--frozen-lockfile`, which compares the recorded SPECIFIER, not the resolved version — so bumping packages/react's range without regenerating pnpm-lock.yaml failed the install before any job did work, taking all 13 checks red with the build skipped. The bump itself is required, and measured rather than assumed: with `^17.0.0` restored and react rebuilt, `check:spec-floors` reports @object-ui/react [floor-too-low] packages/react/dist/SchemaRenderer.js references `expressionBindableTextKeysFor` from @objectstack/spec/ui, which @objectstack/spec@17.0.0 does not export so `^17.0.0` would be a false public claim (objectui#5793's defect class). The same gate names the lowest honest floor as ^17.1.0 — not ^17.2.0, which this branch declared on a misread CHANGELOG heading; the declaration shipped in 17.1.0. Floor corrected down to the truth rather than left over-claiming. Lockfile regenerated with `pnpm install --lockfile-only`; the only change is that one specifier line, and the resolved version stays 17.2.0, so nothing about what installs changes. `pnpm install --frozen-lockfile` now exits 0 (reproduced failing at 3012daf first). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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 was referenced Aug 31, 2026
os-zhuang
approved these changes
Aug 31, 2026
os-zhuang
marked this pull request as ready for review
August 31, 2026 07:31
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refs #4795 — the evaluation half (ruling item 1) ships here in full. The
build-time rejection (ruling item 2) does not, for two independent reasons
stated below. This card must stay open: it still carries Direction 1's
restart record, the two parked sub-questions, and now item 2.
Authored in session
session_013hfmP9hoMd3dJwTh85J4yB.⛔ GOVERNED SURFACE — do not enqueue this PR
This diff touches four files under
skills/, which isGOVERNED_SURFACES[2](
id: 'skills-catalog') inscripts/check-governed-queue-guard.mjs. Askeddirectly about this diff, the guard answers:
So this stays draft however green CI goes; a human (⚠️
os-zhuang/hotlong)merges it.
AGENTS.mdline 378 says the opposite — that repo-rootskills/**is not governed and may take the ordinary self-enqueue path. That text is
stale (the guard landed under objectui#6596, 2026-08-27); the contradiction is
filed as #6975. The guard, not the doc, is authoritative here.
Premise re-verification (the unlock scan's mandatory re-pricing)
@objectstack/specis installed and resolved in this worktree at 17.2.0, and itdoes declare the closed set from objectstack#9599 — verified by reading the
installed package, not the card:
Hot controls, so the reading is a measurement rather than a hopeful grep:
COMPONENT_NODE_VISIBILITY_KEYSandACTION_PARAM_BUILTIN_KEYS(both knowndeclared) → found;
EXPRESSION_BINDABLE_NUMERIC_KEYS(a name that must notexist) → 0 hits.
this PR said the declaration shipped in 17.2.0 and set
packages/react's floorto
^17.2.0. That was a misread of the spec CHANGELOG (the objectstack#9599entry sits under the
## 17.1.0heading, not the## 17.2.0one above it).check:spec-floorscomputes the true floor from the built artifact and names it:^17.1.0. Corrected down to the truth rather than left over-claiming — seethe CI-install section.
four-key list in view; what actually shipped is that list plus a per-component
carriage map —
statisticcarrieslabel/value/description,cardcarries
title/description,buttoncarrieslabel, and every other typegets the frozen empty set. That difference is what most of the rest of this
PR body is about.
The card's title is false, and so was half of what the guides taught
Re-derived here rather than restated: with the
propertiesenvelope,value/title/labelwere already both evaluated and read back before this PR (PR#5122's evaluation leg plus the existing hoist). The two shapes that were
genuinely broken, and are what this PR fixes, are the top-level ones.
What ships
One leg in the evaluation memo (
packages/react/src/SchemaRenderer.tsx),beside the existing
contentleg:Converged, not patched per component — this is the ruling's caution, and it is
a measurement here, not a promise.
data-display/statistic.tsx,layout/card.tsxandform/button.tsxare not in this diff. They alreadyread these keys off the node; nothing was writing an evaluated value there. The
end-to-end test drives the real registered renderers and passes with those three
files untouched.
Two deliberate choices, both pinned:
the name of the function. A row added upstream starts working here with no
edit.
ui:prefix stripping. The specstates the answer for an unlisted type is the empty set. Normalizing would
look harmless and would in the same motion grant rows to
element:buttonandpage:card, whose renderers read config out of the bag viareadProps()andnever touch these keys on the node — re-manufacturing the
evaluated-but-not-read-back half this card exists to close. The authored
corpus also spells these types bare (
statistic42,card135,button158nodes;
ui:*zero).The CI install, and why the floor moves at all
The first push took all 13 checks red with the build skipped — the signature
of
pnpm installdying before any job works. CI installs withpnpm install --frozen-lockfile, which compares the recorded specifier, notthe resolved version. The manifest bump without a lockfile regen is exactly that
mismatch. Reproduced at
3012dafffbefore fixing:The bump is required on the merits, and that is measured, not assumed. With
^17.0.0restored andpackages/reactrebuilt,check:spec-floorsreports:So reverting the manifest was not available:
^17.0.0would be a false publicclaim, objectui#5793's exact defect class ("the range is the claim, and the claim
is what is wrong"). Floor set to the gate-named
^17.1.0, lockfile regeneratedwith
pnpm install --lockfile-only. The lockfile diff is one line — thespecifier — and the resolved version stays
17.2.0, so nothing about whatinstalls changes.
pnpm install --frozen-lockfilenow exits 0.Why the build-time rejection is NOT in this PR
Reason 1 — it would decide parked sub-question ③. Its only natural home is
objectui check(packages/cli/src/commands/check.ts). Whether to build thepublish-time half there is explicitly awaiting the maintainer. The file-contention
reason the earlier seat gave has lifted (no open PR touches
packages/cli), sothis is the scope boundary, not a blocked file.
Reason 2 — the corpus measurement says the rejection's scope is undecided.
Clause-② required measuring before shipping the narrowing. Census over 736 JSON
documents and
jsondoc fences (2747 typed nodes), counting top-level keyswhose value contains
${:visible/hidden/disabled…)contentstatistic.value,statistic.label)badge.text,badge.variant,alert.message,chart.data,action.condition)The two readings differ by 32 occurrences, and the difference is not a
detail:
type: "text"withvalue: "${…}"— taught acrosscontent/docs/guide/expressions.mdand siblings.basic/text.tsxrendersschema.content || schema.value, sotext.valueIS a real top-levelread-back site; it simply has no spec row, so it renders the literal today.
The literal reading accepts a form that visibly breaks; the carriage-map
reading rejects the platform's own documented examples.
page.title— wheretype: "page"is a pagedocument, not a component node, and
titleis the page's own title. Acarriage-map gate needs a "what is a component node" rule the ruling never
states.
stat-card.value×4, on a type that is not registered at all.The ruling's wording ("any key outside that closed list") selects the literal
reading; the ruling's stated goal ("no more blank renders or literal
${data.n}reaching users") is only met by the carriage-map reading, which the wording
predates. That is a contract decision, and choosing it silently inside a
diagnostic is exactly what the dispatch forbids.
Docs
The published rules and three guides asserted the retired claim that these four
keys are never template-evaluated, and prescribed host pre-resolution as the
only route.
page-builder.mdwent further: its "Also wrong" example was{ "type": "statistic", "value": "${…}", "label": "${…}" }— which this PRmakes correct. Corrected surgically, with the still-true half (an undeclared
type reads these keys raw) kept and stated as the boundary.
Published-skills line readings, per the reporting rule:
skills/objectui/rules/protocol.mdskills/objectui/guides/page-builder.mdskills/objectui/guides/schema-expressions.mdskills/objectui/guides/data-integration.mdVerification — all on the final commit
ec519f94dpnpm install --frozen-lockfile3012daffffirst)vitest run packages/react/vitest run packages/components/type-check(react, components)Done/ exit 0 — react's runstsc --noEmit && tsc -p tsconfig.test.json, so the new test files are typecheckedeslint . --no-inline-config(repo-wide, 4031 files)check:spec-floors@object-ui/reactclears at^17.1.0; 0floor-too-low; 3 residual findings all[no-artifact]on plugins outside this diffcheck:governed-queue-guardOK — 132 cases pass; asked about this diff, it names the fourskills/files and refuses the queuecheck:skills-pathsOK (94/95 stated paths resolve; 1 baselined)check:doc-fencesOK — every TypeScript block in 224 documentscheck:doc-typesOK — every documented component type is registeredcheck:doc-snippets271 of 271 blocks judged, 0 failed(after the build it names)check:control-bytes,check:phantom-deps,check:self-import,check:spec-symbolscheck-changeset-presence✅ 4 source files of 2 released packages changed, 1 changesetAblation (the feature). Mutation: the lookup call replaced by
[] as string[], so the loop body survives and only the key source dies. Provenon disk before running — removed-text
grep -c= 0, injected-text = 1, andgit hash-objectdiffered from the HEAD blob. Both test files resolve@object-ui/reactthrough the root vitest alias topackages/react/src, soboth legs are source-resolved and no rebuild leg is involved. Result:
8 failed | 10 passed (the 10 are the negative/inert pins, which correctly
still hold under ablation). Restore leg proven byte-identically:
git hash-objectback to7f9427de8f203e55d960ac5f86359ed46a7ce000, matchingHEAD, andgit diff HEADempty.Ablation (the floor). Same discipline on the manifest: reverted to
^17.0.0, confirmed on disk (git hash-objectdiffered from the HEAD blob),rebuilt, and read the gate —
[floor-too-low], quoted above. Restored via trap.Before/after on the pin: 4 failed | 8 passed → 12 passed.
File overlap to flag for review
packages/components/src/__tests__/skill-guide-provider-envelope.test.tsxisalso touched by live PR #6963, in a disjoint region (its
COLUMNSfixture at~L57–70; this PR's edits are the docblock at ~L20 and the assertion at ~L161).
Its
a node-level title is read but never evaluatedcase pinned exactly thedefect this ruling retires, so it is flipped, with the undeclared-key case added
beside it so the file still states both fates.
Out of scope, filed separately
text→value,although
basic/text.tsxreads it at the node's top level and the guides teachit. Directly gates the scope of ruling item 2.
buttonrow citesaction/action-button.tsx,but the map is keyed on the bare name, so
action:button(5 corpus nodes)resolves to the empty set.
⛔ Draft, and staying draft: governed surface. Not marked ready, not enqueued, no
auto-merge. Direction 2 untouched and still rejected; Direction 3's shipped
render-time half (PR #5129) unaffected.