finding(core): ExpressionEvaluator.registerFunction upper-cases the name, so a lower-case call in an expression renders its own ${...} source

Out-of-scope finding from #5343, measured while replacing the fabricated getExpressionEvaluator() example in content/docs/guide/expressions.md with the real surface. Filed unassigned, not claiming. Observation-class: nothing is broken for the built-in formula names, but the public method's contract is silently case-folding.

Measured, not inferred (`origin/main` = `bdf8cf76e`, run against the built `packages/core/dist/`)

const fmt = (v) => new Intl.NumberFormat('en-US', { style: 'currency', currency: 'USD' }).format(v);

evaluateExpression('${formatCurrency(price)}', { formatCurrency: fmt, price: 1234.5 })
  → '$1,234.50'                        // context-supplied function: exact name, works

const ev = new ExpressionEvaluator({ price: 1234.5 });
ev.registerFunction('formatCurrency', fmt);
ev.evaluate('${formatCurrency(price)}')  → '${formatCurrency(price)}'   // the SOURCE, verbatim
ev.evaluate('${FORMATCURRENCY(price)}')  → '$1,234.50'

Mechanism

ExpressionEvaluator.registerFunction(name, fn) delegates to FormulaFunctions.register, which stores under name.toUpperCase() (packages/core/src/evaluator/FormulaFunctions.ts:38). That is right for the built-ins — they are spreadsheet-style names (SUM, IF) — but the public method is spelled like "register a JS function" and the fold is silent.
The lookup that reaches an expression is formulas.toObject(), which hands over the stored (upper-cased) keys (FormulaFunctions.ts:66), merged under the context in evaluateExpression (ExpressionEvaluator.ts:159).
The failure is then absorbed: evaluate() catches, console.warns and returns defaultValue ?? expression (ExpressionEvaluator.ts:~448), so the reader sees the raw ${formatCurrency(price)} text rendered on screen rather than an error. The soft-fail itself looks deliberate; it is what makes the case-fold hard to notice.

Two candidate directions (not pre-judged)

Keep the fold, document it on registerFunction (and say the call site must use the upper-case name) — cheapest, and consistent with the formula vocabulary.
Make registerFunction case-preserving for the name it is given while keeping the built-ins' case-insensitive lookup — a behaviour change on a public method; needs a decision, not a drive-by.

The doc side is already settled independently: #5343 rewrote that guide section to teach context-supplied functions (exact name, verified above), so no documentation currently depends on registerFunction.

Searched for duplicates

registerFunction, formula uppercase, expression evaluator case over open issues: zero hits.

Generated by Claude Code

Triage (objectui finding round): promoted to pm:queue · domain:ui · type Task, scoped to direction 1 only — document the case-fold on registerFunction (JSDoc + the method's contract statement: call sites use the upper-case name), which makes declared = enforced without touching behavior. Direction 2 (case-preserving registration) is a behavior change on a public method: if the dev concludes it is genuinely needed, stop and report — it needs a Feature card through the decision inbox, not a drive-by. S, sonnet; Clause-②: no (docs on the method, no acceptance change).

Generated by Claude Code

Claim: PM loop round 3 (batch raised to 5 at the maintainer's instruction — 「任务很多，并发保持5」)
Session: session_012u2pRjcqAYtoEjgr3wwhnK
Branch: claude/issue-5363-registerfunction-casefold-doc
Worktree: objectui-issue-5363
Domain: domain:ui
File surface: packages/core/src/evaluator/ExpressionEvaluator.ts (+ tests if a pin is warranted, + a changeset) (stop on breach; explain in the report)
Container & model: S, mode:subagent, model: opus — triage suggested sonnet, and for the writing that is right. I am going one tier up for one reason: the card reads like a bug to fix, triage ruled it a doc-only card, and the judgment being bought here is the dev declining to "fix" the behaviour. That refusal is the deliverable, and it is not a mechanical property.
Clause-②: no — documenting an existing case-fold on the method changes no acceptance behaviour and widens no public surface. ⚠️ Flips immediately if the dev takes direction 2 (case-preserving registration), which is a behaviour change on a public method; that is a stop-and-report, not a drive-by, and with Fable exhausted for this seat it would wait rather than drop tier.
Serial constraints cleared: none — packages/core/** has no in-flight claim in this lane and no PR this session touched it. Round 3's other four hold apps/console/** (#5521), packages/app-shell/src/views/studio-design/** (#5477), packages/components/src/renderers/layout/grid.tsx (#4787) and packages/plugin-detail/** (#4998) — all disjoint from this one.

Located on origin/main this round: packages/core/src/evaluator/ExpressionEvaluator.ts is the sole non-test match for registerFunction.

Generated by Claude Code

docs(core): state the registerFunction case-fold on the method itself (…

os-dev-report

(marker above is literal text, not an HTML comment — the angle-bracket form is eaten by the sanitizer)

{
  "issue": 5363,
  "status": "done",
  "branch": "claude/issue-5363-registerfunction-casefold-doc",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5578",
  "premise_still_valid": true,
  "summary": "AGREED WITH TRIAGE — direction 1, no fork. Documented the case-fold on ExpressionEvaluator.registerFunction (JSDoc + contract statement: expressions must call the UPPER-CASE name) and did NOT change behaviour. I re-derived the fork instead of taking it on trust and direction 2 is worse on three of the four axes: the fold is the REGISTRY's contract, not this method's accident (FormulaFunctions.register folds, and every built-in SUM/IF/UPPER is registered through that same call), so a case-preserving registerFunction would put two dialects in one registry — precisely the hazard commandment #0.1 exists to stop — and it is BREAKING rather than merely unobserved, because ActionRunner.scriptAwait.test.ts already registers 'doWrite' and calls DOWRITE(). Real-business-need evidence is absent: registerFunction has zero non-test call sites in the repo, zero doc consumers (#5343 already moved the guide to context-supplied functions), so nothing is pulling for the change. Re-verification also sharpened the card: the REGISTRY API is case-insensitive (has/get answer to the original spelling) and only EXPRESSIONS are case-sensitive, because the scope is built from toObject() — that is why the fold stays invisible until an expression runs, and it is what the new JSDoc leads with. Changeset is a real patch, not the empty-frontmatter exemption, because the JSDoc is emitted into the published dist/evaluator/ExpressionEvaluator.d.ts (measured) and is therefore what consumers see on hover.",
  "tests": "All at 29d23acc0 (head of branch); exit codes captured before any pipe, each gate quoted by its own verdict line. pnpm exec vitest run packages/core/ => 'Test Files 93 passed (93)' / 'Tests 1945 passed (1945)'. The touched file alone => 'Test Files 1 passed (1)' / 'Tests 82 passed (82)' (79 pre-existing + 3 new). pnpm --filter @object-ui/core lint => LINT_EXIT=0, '513 problems (0 errors, 513 warnings)' all pre-existing no-explicit-any. pnpm --filter @object-ui/core type-check => TYPECHECK_EXIT=0 (tsc --noEmit && tsc -p tsconfig.test.json). check-control-bytes EXIT=0 ('scanned 4629 tracked text file(s)'); check-changeset-presence EXIT=0 ('2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'); check-changeset-no-major EXIT=0; check-changeset-fixed EXIT=0. ABLATION (can the pin fail?): FormulaFunctions.register mutated to set(name, fn) — direction 2 simulated — gives 'Tests 3 failed | 79 passed (82)': ALL THREE new cases go red while every pre-existing case stays green, so nothing previously covered this fold and the pin is a real tripwire rather than decorative. Rebuild statement: the suite resolves its subject by RELATIVE SOURCE PATH ('../ExpressionEvaluator'), so no dist is in the loop and no rebuild sits between mutation and result. Mutation confirmed on disk by anchored grep -c on BOTH the removed and the injected text (1->0 and 0->1) plus git diff --stat — never an editor exit code; the script carried trap '<restore>' EXIT INT TERM, and the tree was verified byte-identical to HEAD afterwards (git status --porcelain empty). DECLARED NARROWING: repo-wide pnpm lint / pnpm type-check left to CI. Evidence — eslint's own population for packages/core is 185 files (count read from --format json, not guessed) and both touched files are in it; root eslint.config.js declares no projectService/parserOptions.project, so linting is NOT type-aware and this diff cannot move a verdict in an untouched file. For type-check the invariance is stronger: the ExpressionEvaluator.ts diff contains ZERO non-comment lines (single hunk at line 349; lines 1-200 byte-identical to origin/main), so the emitted type signature is unchanged and no consumer package can be affected.",
  "open_questions": [],
  "out_of_scope_findings": [
    "NOT filed, reported instead (PM may overrule): FormulaFunctions.register carries the same undocumented fold on its own public method. It is outside the dispatched file surface, and unlike the facade its name.toUpperCase() is visible in its one-line body, so the declared-vs-enforced gap this card closes does not really exist there. Filing it would spend triage on a marginal doc edit.",
    "NOT filed, mine not the repo's: my first vitest invocation used `pnpm --filter ... test -- <path>`, and the repo's own guard rejected it loudly, naming objectui#3288/#3378 and the correct form (`pnpm exec vitest run <path>` from the repo root). The guard worked exactly as designed — recorded only because it changed how every later run in this task was invoked."
  ]
}

Notes for the PM

Scope: no breach. Exactly three files — packages/core/src/evaluator/ExpressionEvaluator.ts, packages/core/src/evaluator/__tests__/ExpressionEvaluator.test.ts, .changeset/registerfunction-casefold-doc-5363.md — independently confirmed by the presence gate's own count ("3 file(s) changed, 2 of them under the src/ of a package the release covers").
Clause-② stays "no". I did not take direction 2, so the flag the claim comment warned about does not trip. No acceptance behaviour changed and no public surface widened.
skip-changeset label: not applicable and not created. This PR carries a real changeset, and per the dispatch that label does not exist in objectui — I did not add or mint one.
If the decision inbox ever opens direction 2, my recommendation would not be case-preserving registration but the stricter opposite: reject a non-upper-case name at registration time, so the mistake is loud at the authoring moment instead of silent at render time (the "declared = enforced" maximum, and the best of the four axes for AI-authored metadata). That is equally a public-method behaviour change and equally breaks the two existing 'doWrite' / 'doFailingWrite' call sites, so it belongs on a Feature card, not here. Recorded so the option is not lost, not as a request to act.

Generated by Claude Code

PM · review of PR #5578 — you held the line, and then made the line enforceable

The card reads like a bug. Triage ruled it a docs card and forbade the behavioural fix. I dispatched a tier above triage's suggestion specifically to buy the refusal. You refused — and then did something better than refusing.

The docs card ships with a guard that direction 2 cannot be taken silently

ExpressionEvaluator.test.ts gains three cases pinning the half that was uncovered … so making registration case-preserving fails a test instead of silently invalidating the new JSDoc.

That is the part I did not ask for and should have. A JSDoc stating a contract is only as good as the thing stopping the code from drifting out from under it; documenting the fold and leaving it unpinned would have created a new declared-but-unenforced surface while closing an old one. Now direction 2 remains available as a Feature card through the inbox — and arrives loudly, as a red test, rather than as prose quietly going false.

You also named the vacuous version you avoided: "Registering 'DOUBLE' and then calling DOUBLE(...) would prove none of that." Correct — that test is green whether or not the fold exists.

You found the mechanism, not just the symptom

The card says a lower-case call renders its own ${...} source. You explained why nobody notices, and it is two independent things:

The registry API stays case-insensitive — getFormulas().has('formatCurrency') is true and .get() returns the function — so the fold is invisible from the API. Only expressions see the stored key, because the evaluation scope is built from FormulaFunctions.toObject(), a plain object whose identifiers match case-sensitively.
A wrong-case call site does not raise. evaluate() catches, warns, and returns defaultValue ?? expression; evaluateExpression() is the throwing sibling.

That pair is the actual answer to "how did this survive", and it is now on the method where the next author will hit it. Documenting the escape hatch — pass the function as context data to keep exact spelling — turns the JSDoc from a warning into a usable one.

The changeset judgment is right and non-obvious

patch, not an empty changeset, because the JSDoc is emitted into the published dist/evaluator/ExpressionEvaluator.d.ts — it is what consumers see on hover. A docs change on a published type is a consumer-visible change. I would have accepted an empty changeset here without arguing; yours is the better reading, and it is reasoned rather than defaulted.

Scope

Path surface (get_files): 3 files — packages/core/src/evaluator/ExpressionEvaluator.ts, its test, and the changeset. No governed surface. Behaviour untouched, as ruled: registerFunction still delegates to FormulaFunctions.register unchanged.

CI has not converged. No verdict and no enqueue until the gate jobs read completed: success by name.

Generated by Claude Code

PM · addendum — you re-derived the fork instead of obeying it

I dispatched this a tier above triage's suggestion to buy one thing: a dev willing to not take the tempting behavioural fix. What came back is better than compliance — you rebuilt the argument from scratch and found direction 2 worse on three of the four axes, so agreeing with triage is now a conclusion rather than an instruction followed:

It is the registry's contract, not this method's accident. FormulaFunctions.register folds, and every built-in (SUM, IF, UPPER) is registered through that same call. A case-preserving registerFunction would put two dialects in one registry — exactly the hazard #0.1 exists to stop.
It is breaking, not merely unobserved. ActionRunner.scriptAwait.test.ts already registers 'doWrite' and calls DOWRITE(). Direction 2 breaks a live call site today.
Nothing is pulling for it. registerFunction has zero non-test call sites in the repo and zero doc consumers — The getting-started guides teach ~14 symbols the packages do not export — one decision per symbol, applied everywhere (the #5160 class, on content/docs/guide/**) #5343 already moved the guide to context-supplied functions.

That last measurement is the one triage could not have supplied, and it is what turns "not now" into "not this, and here is why."

The ablation proves the pin is a tripwire rather than decoration: mutating FormulaFunctions.register to set(name, fn) — direction 2, simulated — turns all three new cases red while every pre-existing case stays green. So nothing previously covered this fold, which is precisely why the JSDoc needed a guard rather than trust.

The type-check narrowing argument is the strongest form of it I have seen this session: "the ExpressionEvaluator.ts diff contains ZERO non-comment lines … so the emitted type signature is unchanged and no consumer package can be affected." That is not a claim about scope, it is a proof of invariance.

The two things you declined to file, and why I agree

FormulaFunctions.register carries the same undocumented fold — but its name.toUpperCase() is visible in its one-line body, so the declared-vs-enforced gap this card closes does not really exist there. "Filing it would spend triage on a marginal doc edit." Correct. Deciding not to file is a judgment too, and a lane that files everything drowns the seat that grades it.
The repo guard rejecting pnpm --filter … test -- <path> — "mine not the repo's". Right call: the guard worked as designed. Recording it because it changed every later invocation, without inflating it into a finding, is the honest middle.

Review conclusion unchanged. CI still converging; no enqueue until the gate jobs read completed: success by name.

Generated by Claude Code

ACCEPT — PR #5578

Gates read by name on head 29d23acc0, all 22 check runs accounted for:

Test (shard 1/4) (2/4) (3/4) (4/4), Type Check, Lint, Build & E2E, Build Docs, Bundle Analysis, Doc Snippet Type Check, Doc Component Type Check, Internal Docs Link Check, Changeset Declaration, Changeset Bump Policy, Changeset Fixed Group Check, Control Byte Scan, Skill Guide Path Check, Live E2E (informational), label — every one completed: success. The three non-success runs are matrix no-ops, all skipped: Test (coverage), the unexpanded Test (coverage shard ${{ matrix.shard }}/4) placeholder, and dependabot. mergeable_state: clean.

The fork was the whole decision, and it holds

Triage's direction 1 (state the fold, do not change it) rests on one checkable claim: that making registerFunction case-preserving would be breaking, not merely unobserved. Verified on origin/main rather than relayed —

packages/core/src/actions/__tests__/ActionRunner.scriptAwait.test.ts:36:    runner.getEvaluator().registerFunction('doWrite', () => write);
packages/core/src/actions/__tests__/ActionRunner.scriptAwait.test.ts:38:    const action: ActionDef = { type: 'script', target: 'DOWRITE()' };

A live test registers one spelling and calls the folded one. Direction 2 turns that red today. That correctly makes it a Feature card through the decision inbox, not a drive-by, and the dev was right to decline to write it.

"Changes no behavior" — verified, not asserted

The ExpressionEvaluator.ts hunk is a single JSDoc block at line 349. The method body is byte-identical:

registerFunction(name: string, fn: (...args: any[]) => any): void {
  this.formulas.register(name, fn);

Zero non-comment lines changed, as the report claims.

The one new claim the report's probe table did not measure

A docs PR's entire product is its claims, so I checked the block's one assertion that no probe row covered — that a context-supplied function keeps its exact spelling. It is correct, and for the stated reason:

const mergedContext = { ...formulaObj, ...contextObj };   // :160

Context is spread last, so context entries win over formulas and are never folded. The @example's last line returns '$1,234.50' as documented.

Tests

Three cases, all exercising the throwing method, all of which direction 2 turns red — so the pin is a real tripwire on the behaviour this card declined to change, and none of them is the vacuous "register DOUBLE, call DOUBLE" shape the dispatch warned against. The case-insensitive-registry case (has('double') true while getNames() contains DOUBLE and not double) is the one that pins why the fold stays invisible, which is the half nothing covered.

One imprecision, filed rather than reworked: #5580

Within the new block, evaluateExpression names two different entities four lines apart — the prose's {@link evaluateExpression} resolves to the sibling method (:142, bare expression, throws), while the @example's last line is the module-level export (:368, context bag, fail-soft via evaluate). Both statements are individually true, and bound the way TSDoc resolves them each is the right one.

Not blocking, and the reason is measured rather than charitable: the method's second parameter is typed { sanitize?: boolean }, so a reader who binds the example to the method gets a compile error at the call site, not a silent wrong answer. That is a bounded, self-correcting misread — proportionate to a one-line follow-up, not to a rework cycle on a green, behaviour-neutral PR.

Filed as #5580 with the suggested wording.

Changeset

Agreed with the dev's reading over .changeset/README.md's "no changesets for documentation updates": that rule is about documentation files. This is source in a released package, and the JSDoc is emitted into the published .d.ts, which the dev measured. A real patch is right; the empty-frontmatter exemption would have understated what npm ships.

Proceeding to ready-for-review and the merge queue.

Generated by Claude Code

docs(core): state the registerFunction case-fold on the method itself (…

Navigation Menu

Repository navigation

finding(core): `ExpressionEvaluator.registerFunction` upper-cases the name, so a lower-case call in an expression renders its own `${...}` source #5363

Measured, not inferred (`origin/main` = `bdf8cf76e`, run against the built `packages/core/dist/`)

Mechanism

Two candidate directions (not pre-judged)

Searched for duplicates

Notes for the PM

PM · review of PR #5578 — you held the line, and then made the line enforceable

The docs card ships with a guard that direction 2 cannot be taken silently

You found the mechanism, not just the symptom

The changeset judgment is right and non-obvious

Scope

PM · addendum — you re-derived the fork instead of obeying it

The two things you declined to file, and why I agree

ACCEPT — PR #5578

The fork was the whole decision, and it holds

"Changes no behavior" — verified, not asserted

The one new claim the report's probe table did not measure

Tests

One imprecision, filed rather than reworked: #5580

Changeset

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

finding(core): ExpressionEvaluator.registerFunction upper-cases the name, so a lower-case call in an expression renders its own ${...} source #5363

Description

Measured, not inferred (origin/main = bdf8cf76e, run against the built packages/core/dist/)

Mechanism

Two candidate directions (not pre-judged)

Searched for duplicates

Activity

os-zhuang commented on Aug 21, 2026

os-sales commented on Aug 21, 2026

os-sales commented on Aug 21, 2026

Notes for the PM

os-sales commented on Aug 21, 2026

PM · review of PR #5578 — you held the line, and then made the line enforceable

The docs card ships with a guard that direction 2 cannot be taken silently

You found the mechanism, not just the symptom

The changeset judgment is right and non-obvious

Scope

os-sales commented on Aug 21, 2026

PM · addendum — you re-derived the fork instead of obeying it

The two things you declined to file, and why I agree

os-sales commented on Aug 21, 2026

ACCEPT — PR #5578

The fork was the whole decision, and it holds

"Changes no behavior" — verified, not asserted

The one new claim the report's probe table did not measure

Tests

One imprecision, filed rather than reworked: #5580

Changeset

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

finding(core): `ExpressionEvaluator.registerFunction` upper-cases the name, so a lower-case call in an expression renders its own `${...}` source #5363

Measured, not inferred (`origin/main` = `bdf8cf76e`, run against the built `packages/core/dist/`)