From c3ce0b8f4dc3d8bcdbf5de4b3951ff7a609e7c59 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 11:59:36 +0000 Subject: [PATCH 1/2] fix(formula): unknown-function refusal names the function and points at the callable set An unknown-function refusal is graded `type` by the engine's own check(), so it fell through bracesHint to the generic dialect trailer -- "predicates are bare CEL" handed to an author whose source already is bare CEL and parses fine. Second leg of the repair #7073 made for the bounds class. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q5WBDtaUnoz5XuJ6jk8pQ5 --- packages/formula/src/validate.test.ts | 125 +++++++++++++++++++++++++- packages/formula/src/validate.ts | 118 +++++++++++++++++++++++- 2 files changed, 239 insertions(+), 4 deletions(-) diff --git a/packages/formula/src/validate.test.ts b/packages/formula/src/validate.test.ts index ffdeb147fb..d1eb5eb225 100644 --- a/packages/formula/src/validate.test.ts +++ b/packages/formula/src/validate.test.ts @@ -1,5 +1,12 @@ import { describe, it, expect } from 'vitest'; -import { validateExpression, introspectScope, expectedDialect, inferExpressionType } from './validate'; +import { + validateExpression, + introspectScope, + expectedDialect, + inferExpressionType, + nearestName, + CEL_STDLIB_FUNCTIONS, +} from './validate'; import { firstUndeclaredReference } from './cel-engine'; describe('validateExpression (ADR-0032)', () => { @@ -199,6 +206,122 @@ describe('validateExpression (ADR-0032)', () => { }); }); + // #13821 — the second leg of #7073. An unknown-function refusal is graded + // `type` by the engine's own `check()`, so before this it fell through to + // `bracesHint` (null, no brace) and out to the dialect trailer: "predicates + // are bare CEL" handed to an author whose source already IS bare CEL and + // parses fine. The guard itself was and stays correct — only the sentence the + // author is told to act on was wrong. + // + // These assert the SPECIFIC prescription, not merely that an error fires; an + // error already fired before the repair. And the `bounds` control below is + // load-bearing: it proves a new class was routed rather than the shared tail + // replaced for everyone. + describe('unknown function vs dialect: the prescription follows the fault class (#13821)', () => { + const dialectTrailer = (role: 'predicate' | 'value') => + ` — ${role}s are bare CEL (e.g. \`record.rating >= 4\`).`; + + it.each([ + { name: 'a receiver call', source: "record.x.nosuchmethod('a')", fn: 'nosuchmethod' }, + { name: 'a bare call', source: 'totallyBogusFn(1,2)', fn: 'totallyBogusFn' }, + { name: 'a call nested in a conjunction', source: "record.a == 1 && zzzznope(record.b)", fn: 'zzzznope' }, + { name: 'a receiver-only cel-js name used bare', source: "split(record.name, ',') == ['a']", fn: 'split' }, + ])('names the unresolvable function and points at the callable set — $name', ({ source, fn }) => { + const r = validateExpression('predicate', source); + expect(r.ok).toBe(false); + expect(r.errors).toHaveLength(1); + const { message } = r.errors[0]; + // The front half — cel-js's own vocabulary, matching the runtime fault — is kept. + expect(message).toMatch(/^invalid CEL predicate: found no matching overload for /); + // The prescription NAMES the function that did not resolve… + expect(message).toContain(`\`${fn}\` is not a callable name here`); + expect(message).toMatch(/NAME fault, not a dialect mistake/); + // …and points at the callable set, via the introspection API that publishes it. + expect(message).toContain('`introspectScope`'); + expect(message).toContain('`CEL_STDLIB_FUNCTIONS`'); + // ⛔ The defect itself: the dialect trailer must NOT reach this class. + expect(message).not.toContain(dialectTrailer('predicate')); + expect(message).not.toMatch(/bare CEL/); + }); + + it('applies to the `value` role too — one producer, all ~10 slots', () => { + const r = validateExpression('value', 'nosuchfn(1)'); + expect(r.ok).toBe(false); + expect(r.errors[0].message).toMatch(/^invalid CEL value:/); + expect(r.errors[0].message).toContain('`nosuchfn` is not a callable name here'); + expect(r.errors[0].message).not.toContain(dialectTrailer('value')); + }); + + // ⛔ The message may never state how many functions the catalog holds: what + // the catalog contains is being adjudicated separately, and a sentence + // asserting a count would be falsified by that ruling without failing here. + it('refers to the callable set without asserting its size', () => { + const message = validateExpression('predicate', 'totallyBogusFn(1,2)').errors[0].message; + expect(message).not.toMatch(/\b\d+\s+(?:functions|names|entries)\b/); + }); + + // did-you-mean is measured dangerous, so both directions are pinned. The + // threshold is the whole safety argument; either case flipping is a + // regression, not a tuning. + describe('did-you-mean is thresholded (both measured cases pinned)', () => { + it('suggests on a real typo: `isBlnk` → `isBlank`', () => { + const message = validateExpression('predicate', 'isBlnk(record.name)').errors[0].message; + expect(message).toContain('`isBlnk` is not a callable name here'); + expect(message).toContain('Did you mean `isBlank`?'); + }); + + it('stays SILENT on a distant match: `can` must never be answered with `min`', () => { + // `nearestName('can', CEL_STDLIB_FUNCTIONS)` is `'min'` — two edits on a + // three-character name, a jump from a permission verb to a numeric + // function. Worse than silence: an author who takes it writes + // `min(object, verb)`. + const message = validateExpression('predicate', 'current_user.can(object, verb)').errors[0].message; + expect(message).toContain('`can` is not a callable name here'); + expect(message).not.toMatch(/Did you mean/); + expect(message).not.toMatch(/`min`/); + }); + + it('leaves the shared `nearestName` budget alone — this class narrows locally', () => { + // The hazard is this catalog's, not the heuristic's: field-name + // suggestions keep the shared budget. If this ever stops answering + // `'min'`, the local threshold is no longer the thing protecting the + // message and the pin above has quietly become vacuous. + expect(nearestName('can', CEL_STDLIB_FUNCTIONS)).toBe('min'); + expect(nearestName('isBlnk', CEL_STDLIB_FUNCTIONS)).toBe('isBlank'); + }); + }); + + // The flipped pins. The refusal surface may never shrink, and this arm may + // never speak for faults it cannot name. + it('keeps the dialect trailer on a real function given arguments no overload accepts', () => { + // Same cel-js message SHAPE, different fault: `upper` exists. Calling it + // "not a callable name" would replace a useless sentence with a false one. + const r = validateExpression('predicate', 'upper(1, 2)'); + expect(r.ok).toBe(false); + expect(r.errors[0].message).toMatch(/found no matching overload for 'upper\(int, int\)'/); + expect(r.errors[0].message).toContain(dialectTrailer('predicate')); + expect(r.errors[0].message).not.toMatch(/NAME fault/); + }); + + it.each([ + { name: 'an operator type mismatch', source: "1 + 'a'" }, + { name: 'a ternary branch mismatch', source: "record.rating >= 4 ? 1 : 'x'" }, + ])('keeps the dialect trailer on a `type` fault that names no call — $name', ({ source }) => { + const r = validateExpression('predicate', source); + expect(r.ok).toBe(false); + expect(r.errors[0].message).toContain(dialectTrailer('predicate')); + expect(r.errors[0].message).not.toMatch(/NAME fault/); + }); + + it('leaves the `bounds` prescription untouched — a new class was routed, not the shared tail replaced', () => { + const overBudget = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && '); + const message = validateExpression('predicate', overBudget).errors[0].message; + expect(message).toMatch(/SIZE fault, not a dialect mistake/); + expect(message).toMatch(/Shrink it/); + expect(message).not.toMatch(/NAME fault/); + }); + }); + describe('templates', () => { it('accepts a valid {{ path }} template', () => { const r = validateExpression('template', 'Hot lead: {{ record.full_name }}'); diff --git a/packages/formula/src/validate.ts b/packages/formula/src/validate.ts index a4c4d8611b..c23c151f6f 100644 --- a/packages/formula/src/validate.ts +++ b/packages/formula/src/validate.ts @@ -254,6 +254,105 @@ function boundsHint(source: string): string | null { ); } +/** + * cel-js's unknown-call vocabulary, both of its spellings — a bare call + * (`` `found no matching overload for 'totallyBogusFn(int, int)'` ``) and a + * receiver call (`` `…for 'dyn.nosuchmethod(string)'` ``). Both are emitted from + * one template family in `cel-js/lib/operators.js`, and the name we want is the + * segment immediately before the argument list, after any receiver-type prefix. + * + * Anchored on the closing `)'` so the greedy receiver prefix cannot run past the + * call into the source excerpt cel-js appends on the following lines. + */ +const NO_OVERLOAD_RE = /found no matching overload for '(?:.*[.])?([A-Za-z_$][\w$]*)\(.*?\)'/; + +/** + * The nearest advertised callable to `name`, or `undefined` when nothing is + * close enough that a suggestion beats silence. + * + * Deliberately STRICTER than {@link nearestName}'s shared budget, and that + * difference is the entire reason this function exists rather than a call to + * the shared one. `nearest` spends `max(2, floor(name.length / 3))` edits — + * right for a field name checked against the handful of fields on one object, + * measurably wrong against this catalog: `nearestName('can', CEL_STDLIB_FUNCTIONS)` + * answers `'min'`. Two edits on a three-character name, jumping from a + * permission verb to a numeric function — a confident suggestion across an + * unrelated namespace, which is worse than silence. An author who takes it (an + * LLM author above all, following the last sentence it was handed) writes + * `min(object, verb)` and is further from working than before it asked. + * + * The budget here is proportional rather than floored: at most one edit per + * three characters of the LONGER name, so at least two thirds of a suggestion + * must already be typed. That keeps the case that makes suggesting worthwhile + * (`isBlnk` → `isBlank`, one edit in seven) and refuses the measured hazard + * (`can` → `min`, two edits in three). Both are pinned in `validate.test.ts`; + * a change to this budget that loses either is a regression, not a tuning. + * + * The distance metric stays the module's one {@link levenshtein} — only the + * acceptance budget is class-specific, which is what the hazard is about. + */ +function nearestCallable(name: string): string | undefined { + let best: string | undefined; + let bestDistance = Infinity; + for (const candidate of CEL_STDLIB_FUNCTIONS) { + const distance = levenshtein(name, candidate); + if (distance > Math.floor(Math.max(name.length, candidate.length) / 3)) continue; + if (distance >= bestDistance) continue; + bestDistance = distance; + best = candidate; + } + return best; +} + +/** + * The prescription for the **unknown-name** arm of a `type` refusal — an + * expression that is perfectly good CEL and merely calls something by a name + * that resolves to nothing in this position. + * + * The second leg of the repair {@link boundsHint} made for the `bounds` class + * (#7073). Until this hint, every unknown-function refusal ended with the same + * dialect trailer ("`predicate`s are bare CEL (e.g. `record.rating >= 4`)"), + * and that sentence is actively wrong here for exactly the reason it was wrong + * for `bounds`: the source already IS bare CEL and parses fine. An author who + * obeys the last sentence they were given rewrites the dialect, learns nothing, + * and comes back with the same unresolvable name. The front half (cel-js's own + * `found no matching overload for '…'`) was right all along and is kept + * verbatim; only the prescription lied. + * + * ### Why the name must be checked against the advertised catalog first + * + * cel-js emits ONE message shape for two different faults: a name that resolves + * to nothing (`upperr(record.name)`) and a real function handed arguments no + * overload accepts (`upper(1, 2)` → `found no matching overload for + * 'upper(int, int)'`). Telling the second author that `upper` "is not a + * callable name" would be a fresh false statement in place of a merely useless + * one, so an advertised name falls through to the existing trailer untouched. + * + * ### Why the wording is "not a callable name HERE" + * + * {@link CEL_STDLIB_FUNCTIONS} is a curated bare-callable subset, not an oracle + * for existence — 33 further names cel-js registers are callable only on a + * receiver (`record.name.split(',')` works; bare `split(…)` faults here and + * lands in this arm). So the message may say the name cannot be called in this + * position, and may point at what IS advertised, but must not claim the name + * does not exist. For the same reason it names no size: what the catalog + * contains is being adjudicated separately, and a message asserting a count + * would be falsified by that ruling. + */ +function unknownFunctionHint(celMessage: string): string | null { + const name = NO_OVERLOAD_RE.exec(celMessage)?.[1]; + if (!name || CEL_STDLIB_FUNCTIONS.includes(name)) return null; + const suggestion = nearestCallable(name); + return ( + `\`${name}\` is not a callable name here — a NAME fault, not a dialect mistake, so ` + + `re-spelling the expression will not fix it.` + + (suggestion ? ` Did you mean \`${suggestion}\`?` : '') + + ` The callable names this platform advertises for authoring are the \`functions\` list ` + + `\`introspectScope\` returns (\`CEL_STDLIB_FUNCTIONS\`) — pick one of those, or precompute ` + + `the value in a stored field and reference that field instead.` + ); +} + function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined, errors: ExprValidationError[]): void { if (!schema?.fields || schema.fields.length === 0) return; const known = new Set(schema.fields); @@ -400,9 +499,22 @@ export function validateExpression( if (!compiled.ok) { // #7073 — a bounds refusal gets the SIZE prescription, never the dialect // trailer: the source is already bare CEL, so "write bare CEL" is advice - // that cannot succeed. Checked first because the class is certain (it comes - // from the engine's own verdict) while the braces hint is a heuristic. - const hint = (compiled.error.kind === 'bounds' ? boundsHint(source) : null) ?? bracesHint(source); + // that cannot succeed. #13821 routes the `type` class the same way for the + // same reason, one class per arm. Both are checked before the braces hint + // because the class is certain (it comes from the engine's own verdict) + // while the braces hint is a heuristic. + // + // A `type` fault that names no unresolvable call — an operator or ternary + // mismatch (`1 + 'a'`, `no such overload: int + string`), or a real function + // handed wrong arguments — returns null from `unknownFunctionHint` and keeps + // the existing trailer: this arm has a name to hand back or it says nothing. + const classHint = + compiled.error.kind === 'bounds' + ? boundsHint(source) + : compiled.error.kind === 'type' + ? unknownFunctionHint(compiled.error.message) + : null; + const hint = classHint ?? bracesHint(source); errors.push({ source, message: From dabda374fae7901355f53c55ea9c8c205bfe976c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 12:05:33 +0000 Subject: [PATCH 2/2] chore: changeset for the unknown-function prescription Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q5WBDtaUnoz5XuJ6jk8pQ5 --- .../formula-unknown-function-prescription.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .changeset/formula-unknown-function-prescription.md diff --git a/.changeset/formula-unknown-function-prescription.md b/.changeset/formula-unknown-function-prescription.md new file mode 100644 index 0000000000..c6cceff402 --- /dev/null +++ b/.changeset/formula-unknown-function-prescription.md @@ -0,0 +1,56 @@ +--- +"@objectstack/formula": patch +--- + +fix(formula): an unknown-function refusal names the function and points at the callable set (#13821) + +`validateExpression` refused an unknown CEL function correctly and then handed +the author a prescription that could not succeed: "`predicate`s are bare CEL +(e.g. `record.rating >= 4`)" — advice to write bare CEL, on a source that +already is bare CEL and parses fine. An unknown-function fault is graded `type` +by the engine's own `check()`, so it fell through `bracesHint` (null, no brace) +and out to that generic dialect trailer. + +This is the second leg of the repair #7073 / PR #7209 made for the `bounds` +class, for the same reason `boundsHint`'s doc-comment gives: an author who obeys +the last sentence they were given — an LLM author above all — rewrites the +dialect, learns nothing, and comes back with the same unresolvable name. The +last sentence pointed at the one thing that was already correct. + +The `type` class now gets its own prescription, which **names the function that +did not resolve** and points at the callable set `introspectScope` publishes: + +``` +invalid CEL predicate: found no matching overload for 'dyn.nosuchmethod(string)' + +> 1 | record.x.nosuchmethod('a') + ^ — `nosuchmethod` is not a callable name here — a NAME fault, not a +dialect mistake, so re-spelling the expression will not fix it. The callable +names this platform advertises for authoring are the `functions` list +`introspectScope` returns (`CEL_STDLIB_FUNCTIONS`) — pick one of those, or +precompute the value in a stored field and reference that field instead. +``` + +The front half is unchanged: it is cel-js's own vocabulary and matches the +runtime fault exactly. Only the trailer after the dash is new. `CEL_STDLIB_FUNCTIONS` +itself is untouched, and the message names no member count — what the catalog +contains is being adjudicated separately, and a sentence asserting a size would +be falsified by that ruling. + +**The did-you-mean suggestion is thresholded, and the threshold is the point.** +Against this catalog the shared `nearestName` budget answers +`nearestName('can', CEL_STDLIB_FUNCTIONS)` with `'min'` — two edits on a +three-character name, a jump from a permission verb to a numeric function. That +suggestion is worse than silence: an author who takes it writes +`min(object, verb)`. This class therefore narrows locally to at most one edit per +three characters of the longer name, keeping the case that makes suggesting +worthwhile (`isBlnk` → `isBlank`) and refusing the measured hazard (`can` → no +suggestion). Both cases are pinned. The shared `nearestName` budget is unchanged, +so field-name suggestions are unaffected. + +Message-only. The refusal fires on exactly the same inputs it did before — no +rule id, severity, match set or gate behaviour changed. Faults in the `type` +class that name no unresolvable call keep the existing trailer: an operator or +ternary mismatch (`1 + 'a'`), and a real function handed arguments no overload +accepts (`upper(1, 2)`, which produces the same cel-js message shape) — calling +`upper` "not a callable name" would replace a useless sentence with a false one.