Skip to content

Commit 9738c35

Browse files
claude[bot]claude
andauthored
fix(formula): unknown-function refusal names the function and points at the callable set (#14204)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5WBDtaUnoz5XuJ6jk8pQ5 * chore: changeset for the unknown-function prescription Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5WBDtaUnoz5XuJ6jk8pQ5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b992b1d commit 9738c35

3 files changed

Lines changed: 295 additions & 4 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/formula": patch
3+
---
4+
5+
fix(formula): an unknown-function refusal names the function and points at the callable set (#13821)
6+
7+
`validateExpression` refused an unknown CEL function correctly and then handed
8+
the author a prescription that could not succeed: "`predicate`s are bare CEL
9+
(e.g. `record.rating >= 4`)" — advice to write bare CEL, on a source that
10+
already is bare CEL and parses fine. An unknown-function fault is graded `type`
11+
by the engine's own `check()`, so it fell through `bracesHint` (null, no brace)
12+
and out to that generic dialect trailer.
13+
14+
This is the second leg of the repair #7073 / PR #7209 made for the `bounds`
15+
class, for the same reason `boundsHint`'s doc-comment gives: an author who obeys
16+
the last sentence they were given — an LLM author above all — rewrites the
17+
dialect, learns nothing, and comes back with the same unresolvable name. The
18+
last sentence pointed at the one thing that was already correct.
19+
20+
The `type` class now gets its own prescription, which **names the function that
21+
did not resolve** and points at the callable set `introspectScope` publishes:
22+
23+
```
24+
invalid CEL predicate: found no matching overload for 'dyn.nosuchmethod(string)'
25+
26+
> 1 | record.x.nosuchmethod('a')
27+
^ — `nosuchmethod` is not a callable name here — a NAME fault, not a
28+
dialect mistake, so re-spelling the expression will not fix it. The callable
29+
names this platform advertises for authoring are the `functions` list
30+
`introspectScope` returns (`CEL_STDLIB_FUNCTIONS`) — pick one of those, or
31+
precompute the value in a stored field and reference that field instead.
32+
```
33+
34+
The front half is unchanged: it is cel-js's own vocabulary and matches the
35+
runtime fault exactly. Only the trailer after the dash is new. `CEL_STDLIB_FUNCTIONS`
36+
itself is untouched, and the message names no member count — what the catalog
37+
contains is being adjudicated separately, and a sentence asserting a size would
38+
be falsified by that ruling.
39+
40+
**The did-you-mean suggestion is thresholded, and the threshold is the point.**
41+
Against this catalog the shared `nearestName` budget answers
42+
`nearestName('can', CEL_STDLIB_FUNCTIONS)` with `'min'` — two edits on a
43+
three-character name, a jump from a permission verb to a numeric function. That
44+
suggestion is worse than silence: an author who takes it writes
45+
`min(object, verb)`. This class therefore narrows locally to at most one edit per
46+
three characters of the longer name, keeping the case that makes suggesting
47+
worthwhile (`isBlnk` → `isBlank`) and refusing the measured hazard (`can` → no
48+
suggestion). Both cases are pinned. The shared `nearestName` budget is unchanged,
49+
so field-name suggestions are unaffected.
50+
51+
Message-only. The refusal fires on exactly the same inputs it did before — no
52+
rule id, severity, match set or gate behaviour changed. Faults in the `type`
53+
class that name no unresolvable call keep the existing trailer: an operator or
54+
ternary mismatch (`1 + 'a'`), and a real function handed arguments no overload
55+
accepts (`upper(1, 2)`, which produces the same cel-js message shape) — calling
56+
`upper` "not a callable name" would replace a useless sentence with a false one.

‎packages/formula/src/validate.test.ts‎

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { describe, it, expect } from 'vitest';
2-
import { validateExpression, introspectScope, expectedDialect, inferExpressionType } from './validate';
2+
import {
3+
validateExpression,
4+
introspectScope,
5+
expectedDialect,
6+
inferExpressionType,
7+
nearestName,
8+
CEL_STDLIB_FUNCTIONS,
9+
} from './validate';
310
import { firstUndeclaredReference } from './cel-engine';
411

512
describe('validateExpression (ADR-0032)', () => {
@@ -199,6 +206,122 @@ describe('validateExpression (ADR-0032)', () => {
199206
});
200207
});
201208

209+
// #13821 — the second leg of #7073. An unknown-function refusal is graded
210+
// `type` by the engine's own `check()`, so before this it fell through to
211+
// `bracesHint` (null, no brace) and out to the dialect trailer: "predicates
212+
// are bare CEL" handed to an author whose source already IS bare CEL and
213+
// parses fine. The guard itself was and stays correct — only the sentence the
214+
// author is told to act on was wrong.
215+
//
216+
// These assert the SPECIFIC prescription, not merely that an error fires; an
217+
// error already fired before the repair. And the `bounds` control below is
218+
// load-bearing: it proves a new class was routed rather than the shared tail
219+
// replaced for everyone.
220+
describe('unknown function vs dialect: the prescription follows the fault class (#13821)', () => {
221+
const dialectTrailer = (role: 'predicate' | 'value') =>
222+
` — ${role}s are bare CEL (e.g. \`record.rating >= 4\`).`;
223+
224+
it.each([
225+
{ name: 'a receiver call', source: "record.x.nosuchmethod('a')", fn: 'nosuchmethod' },
226+
{ name: 'a bare call', source: 'totallyBogusFn(1,2)', fn: 'totallyBogusFn' },
227+
{ name: 'a call nested in a conjunction', source: "record.a == 1 && zzzznope(record.b)", fn: 'zzzznope' },
228+
{ name: 'a receiver-only cel-js name used bare', source: "split(record.name, ',') == ['a']", fn: 'split' },
229+
])('names the unresolvable function and points at the callable set — $name', ({ source, fn }) => {
230+
const r = validateExpression('predicate', source);
231+
expect(r.ok).toBe(false);
232+
expect(r.errors).toHaveLength(1);
233+
const { message } = r.errors[0];
234+
// The front half — cel-js's own vocabulary, matching the runtime fault — is kept.
235+
expect(message).toMatch(/^invalid CEL predicate: found no matching overload for /);
236+
// The prescription NAMES the function that did not resolve…
237+
expect(message).toContain(`\`${fn}\` is not a callable name here`);
238+
expect(message).toMatch(/NAME fault, not a dialect mistake/);
239+
// …and points at the callable set, via the introspection API that publishes it.
240+
expect(message).toContain('`introspectScope`');
241+
expect(message).toContain('`CEL_STDLIB_FUNCTIONS`');
242+
// ⛔ The defect itself: the dialect trailer must NOT reach this class.
243+
expect(message).not.toContain(dialectTrailer('predicate'));
244+
expect(message).not.toMatch(/bare CEL/);
245+
});
246+
247+
it('applies to the `value` role too — one producer, all ~10 slots', () => {
248+
const r = validateExpression('value', 'nosuchfn(1)');
249+
expect(r.ok).toBe(false);
250+
expect(r.errors[0].message).toMatch(/^invalid CEL value:/);
251+
expect(r.errors[0].message).toContain('`nosuchfn` is not a callable name here');
252+
expect(r.errors[0].message).not.toContain(dialectTrailer('value'));
253+
});
254+
255+
// ⛔ The message may never state how many functions the catalog holds: what
256+
// the catalog contains is being adjudicated separately, and a sentence
257+
// asserting a count would be falsified by that ruling without failing here.
258+
it('refers to the callable set without asserting its size', () => {
259+
const message = validateExpression('predicate', 'totallyBogusFn(1,2)').errors[0].message;
260+
expect(message).not.toMatch(/\b\d+\s+(?:functions|names|entries)\b/);
261+
});
262+
263+
// did-you-mean is measured dangerous, so both directions are pinned. The
264+
// threshold is the whole safety argument; either case flipping is a
265+
// regression, not a tuning.
266+
describe('did-you-mean is thresholded (both measured cases pinned)', () => {
267+
it('suggests on a real typo: `isBlnk` → `isBlank`', () => {
268+
const message = validateExpression('predicate', 'isBlnk(record.name)').errors[0].message;
269+
expect(message).toContain('`isBlnk` is not a callable name here');
270+
expect(message).toContain('Did you mean `isBlank`?');
271+
});
272+
273+
it('stays SILENT on a distant match: `can` must never be answered with `min`', () => {
274+
// `nearestName('can', CEL_STDLIB_FUNCTIONS)` is `'min'` — two edits on a
275+
// three-character name, a jump from a permission verb to a numeric
276+
// function. Worse than silence: an author who takes it writes
277+
// `min(object, verb)`.
278+
const message = validateExpression('predicate', 'current_user.can(object, verb)').errors[0].message;
279+
expect(message).toContain('`can` is not a callable name here');
280+
expect(message).not.toMatch(/Did you mean/);
281+
expect(message).not.toMatch(/`min`/);
282+
});
283+
284+
it('leaves the shared `nearestName` budget alone — this class narrows locally', () => {
285+
// The hazard is this catalog's, not the heuristic's: field-name
286+
// suggestions keep the shared budget. If this ever stops answering
287+
// `'min'`, the local threshold is no longer the thing protecting the
288+
// message and the pin above has quietly become vacuous.
289+
expect(nearestName('can', CEL_STDLIB_FUNCTIONS)).toBe('min');
290+
expect(nearestName('isBlnk', CEL_STDLIB_FUNCTIONS)).toBe('isBlank');
291+
});
292+
});
293+
294+
// The flipped pins. The refusal surface may never shrink, and this arm may
295+
// never speak for faults it cannot name.
296+
it('keeps the dialect trailer on a real function given arguments no overload accepts', () => {
297+
// Same cel-js message SHAPE, different fault: `upper` exists. Calling it
298+
// "not a callable name" would replace a useless sentence with a false one.
299+
const r = validateExpression('predicate', 'upper(1, 2)');
300+
expect(r.ok).toBe(false);
301+
expect(r.errors[0].message).toMatch(/found no matching overload for 'upper\(int, int\)'/);
302+
expect(r.errors[0].message).toContain(dialectTrailer('predicate'));
303+
expect(r.errors[0].message).not.toMatch(/NAME fault/);
304+
});
305+
306+
it.each([
307+
{ name: 'an operator type mismatch', source: "1 + 'a'" },
308+
{ name: 'a ternary branch mismatch', source: "record.rating >= 4 ? 1 : 'x'" },
309+
])('keeps the dialect trailer on a `type` fault that names no call — $name', ({ source }) => {
310+
const r = validateExpression('predicate', source);
311+
expect(r.ok).toBe(false);
312+
expect(r.errors[0].message).toContain(dialectTrailer('predicate'));
313+
expect(r.errors[0].message).not.toMatch(/NAME fault/);
314+
});
315+
316+
it('leaves the `bounds` prescription untouched — a new class was routed, not the shared tail replaced', () => {
317+
const overBudget = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
318+
const message = validateExpression('predicate', overBudget).errors[0].message;
319+
expect(message).toMatch(/SIZE fault, not a dialect mistake/);
320+
expect(message).toMatch(/Shrink it/);
321+
expect(message).not.toMatch(/NAME fault/);
322+
});
323+
});
324+
202325
describe('templates', () => {
203326
it('accepts a valid {{ path }} template', () => {
204327
const r = validateExpression('template', 'Hot lead: {{ record.full_name }}');

‎packages/formula/src/validate.ts‎

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,105 @@ function boundsHint(source: string): string | null {
254254
);
255255
}
256256

257+
/**
258+
* cel-js's unknown-call vocabulary, both of its spellings — a bare call
259+
* (`` `found no matching overload for 'totallyBogusFn(int, int)'` ``) and a
260+
* receiver call (`` `…for 'dyn.nosuchmethod(string)'` ``). Both are emitted from
261+
* one template family in `cel-js/lib/operators.js`, and the name we want is the
262+
* segment immediately before the argument list, after any receiver-type prefix.
263+
*
264+
* Anchored on the closing `)'` so the greedy receiver prefix cannot run past the
265+
* call into the source excerpt cel-js appends on the following lines.
266+
*/
267+
const NO_OVERLOAD_RE = /found no matching overload for '(?:.*[.])?([A-Za-z_$][\w$]*)\(.*?\)'/;
268+
269+
/**
270+
* The nearest advertised callable to `name`, or `undefined` when nothing is
271+
* close enough that a suggestion beats silence.
272+
*
273+
* Deliberately STRICTER than {@link nearestName}'s shared budget, and that
274+
* difference is the entire reason this function exists rather than a call to
275+
* the shared one. `nearest` spends `max(2, floor(name.length / 3))` edits —
276+
* right for a field name checked against the handful of fields on one object,
277+
* measurably wrong against this catalog: `nearestName('can', CEL_STDLIB_FUNCTIONS)`
278+
* answers `'min'`. Two edits on a three-character name, jumping from a
279+
* permission verb to a numeric function — a confident suggestion across an
280+
* unrelated namespace, which is worse than silence. An author who takes it (an
281+
* LLM author above all, following the last sentence it was handed) writes
282+
* `min(object, verb)` and is further from working than before it asked.
283+
*
284+
* The budget here is proportional rather than floored: at most one edit per
285+
* three characters of the LONGER name, so at least two thirds of a suggestion
286+
* must already be typed. That keeps the case that makes suggesting worthwhile
287+
* (`isBlnk` → `isBlank`, one edit in seven) and refuses the measured hazard
288+
* (`can` → `min`, two edits in three). Both are pinned in `validate.test.ts`;
289+
* a change to this budget that loses either is a regression, not a tuning.
290+
*
291+
* The distance metric stays the module's one {@link levenshtein} — only the
292+
* acceptance budget is class-specific, which is what the hazard is about.
293+
*/
294+
function nearestCallable(name: string): string | undefined {
295+
let best: string | undefined;
296+
let bestDistance = Infinity;
297+
for (const candidate of CEL_STDLIB_FUNCTIONS) {
298+
const distance = levenshtein(name, candidate);
299+
if (distance > Math.floor(Math.max(name.length, candidate.length) / 3)) continue;
300+
if (distance >= bestDistance) continue;
301+
bestDistance = distance;
302+
best = candidate;
303+
}
304+
return best;
305+
}
306+
307+
/**
308+
* The prescription for the **unknown-name** arm of a `type` refusal — an
309+
* expression that is perfectly good CEL and merely calls something by a name
310+
* that resolves to nothing in this position.
311+
*
312+
* The second leg of the repair {@link boundsHint} made for the `bounds` class
313+
* (#7073). Until this hint, every unknown-function refusal ended with the same
314+
* dialect trailer ("`predicate`s are bare CEL (e.g. `record.rating >= 4`)"),
315+
* and that sentence is actively wrong here for exactly the reason it was wrong
316+
* for `bounds`: the source already IS bare CEL and parses fine. An author who
317+
* obeys the last sentence they were given rewrites the dialect, learns nothing,
318+
* and comes back with the same unresolvable name. The front half (cel-js's own
319+
* `found no matching overload for '…'`) was right all along and is kept
320+
* verbatim; only the prescription lied.
321+
*
322+
* ### Why the name must be checked against the advertised catalog first
323+
*
324+
* cel-js emits ONE message shape for two different faults: a name that resolves
325+
* to nothing (`upperr(record.name)`) and a real function handed arguments no
326+
* overload accepts (`upper(1, 2)` → `found no matching overload for
327+
* 'upper(int, int)'`). Telling the second author that `upper` "is not a
328+
* callable name" would be a fresh false statement in place of a merely useless
329+
* one, so an advertised name falls through to the existing trailer untouched.
330+
*
331+
* ### Why the wording is "not a callable name HERE"
332+
*
333+
* {@link CEL_STDLIB_FUNCTIONS} is a curated bare-callable subset, not an oracle
334+
* for existence — 33 further names cel-js registers are callable only on a
335+
* receiver (`record.name.split(',')` works; bare `split(…)` faults here and
336+
* lands in this arm). So the message may say the name cannot be called in this
337+
* position, and may point at what IS advertised, but must not claim the name
338+
* does not exist. For the same reason it names no size: what the catalog
339+
* contains is being adjudicated separately, and a message asserting a count
340+
* would be falsified by that ruling.
341+
*/
342+
function unknownFunctionHint(celMessage: string): string | null {
343+
const name = NO_OVERLOAD_RE.exec(celMessage)?.[1];
344+
if (!name || CEL_STDLIB_FUNCTIONS.includes(name)) return null;
345+
const suggestion = nearestCallable(name);
346+
return (
347+
`\`${name}\` is not a callable name here — a NAME fault, not a dialect mistake, so ` +
348+
`re-spelling the expression will not fix it.` +
349+
(suggestion ? ` Did you mean \`${suggestion}\`?` : '') +
350+
` The callable names this platform advertises for authoring are the \`functions\` list ` +
351+
`\`introspectScope\` returns (\`CEL_STDLIB_FUNCTIONS\`) — pick one of those, or precompute ` +
352+
`the value in a stored field and reference that field instead.`
353+
);
354+
}
355+
257356
function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined, errors: ExprValidationError[]): void {
258357
if (!schema?.fields || schema.fields.length === 0) return;
259358
const known = new Set(schema.fields);
@@ -400,9 +499,22 @@ export function validateExpression(
400499
if (!compiled.ok) {
401500
// #7073 — a bounds refusal gets the SIZE prescription, never the dialect
402501
// trailer: the source is already bare CEL, so "write bare CEL" is advice
403-
// that cannot succeed. Checked first because the class is certain (it comes
404-
// from the engine's own verdict) while the braces hint is a heuristic.
405-
const hint = (compiled.error.kind === 'bounds' ? boundsHint(source) : null) ?? bracesHint(source);
502+
// that cannot succeed. #13821 routes the `type` class the same way for the
503+
// same reason, one class per arm. Both are checked before the braces hint
504+
// because the class is certain (it comes from the engine's own verdict)
505+
// while the braces hint is a heuristic.
506+
//
507+
// A `type` fault that names no unresolvable call — an operator or ternary
508+
// mismatch (`1 + 'a'`, `no such overload: int + string`), or a real function
509+
// handed wrong arguments — returns null from `unknownFunctionHint` and keeps
510+
// the existing trailer: this arm has a name to hand back or it says nothing.
511+
const classHint =
512+
compiled.error.kind === 'bounds'
513+
? boundsHint(source)
514+
: compiled.error.kind === 'type'
515+
? unknownFunctionHint(compiled.error.message)
516+
: null;
517+
const hint = classHint ?? bracesHint(source);
406518
errors.push({
407519
source,
408520
message:

0 commit comments

Comments
 (0)