fix(formula): give an over-budget expression a size prescription, not the dialect trailer (#7073)

.changeset/formula-bounds-prescription.md

@@ -0,0 +1,37 @@
+---
+'@objectstack/formula': patch
+---
+
+`validateExpression`: give an over-budget expression a SIZE prescription instead of the dialect trailer (#7073)
+
+ADR-0032's shared validator appended one trailer to every `celEngine.compile`
+refusal, byte for byte — `— predicates are bare CEL (e.g. \`record.rating >= 4\`).`
+That sentence is right for a dialect mistake and actively wrong for a **bounds**
+refusal: an 80-clause conjunction is already bare CEL, perfectly good syntax, and
+merely over the platform's parse budget. The author was told to change the one
+thing that was never wrong; an AI author, which obeys the last sentence it was
+handed, rewrites the dialect and regresses. Reported by #6833's measurement.
+
+The refusal is unchanged — same inputs refused, same `Exceeded maxAstNodes (256)`
+front half from cel-js. Only the prescription is now class-aware: a `bounds`
+verdict (read off the engine's own `error.kind`, with the exceeded bound named by
+`parseCelToAstWithReason`) produces
+
+> invalid CEL predicate: Exceeded maxAstNodes (256) … — this is valid CEL that
+> exceeds the `maxAstNodes` budget (limit 256) — a SIZE fault, not a dialect
+> mistake, so re-spelling the expression will not fix it. Shrink it (fewer
+> clauses, shallower nesting, fewer list elements), or precompute the heavy part
+> into a stored field and reference that field instead. …
+
+while a genuine dialect/syntax fault keeps the old trailer verbatim. Fixed once at
+the producer, so all ~10 expression slots benefit — build, metadata registration,
+lint's `validateStackExpressions`, and the `validate_expression` tool. The
+remedies are deliberately slot-generic: the slots' combination semantics differ,
+so PR #6831's RLS-specific "splitting the top-level `&&` widens the grant" is not
+portable and splitting is offered only with a caveat.
+
+Also documents, text-only, the completeness gap in `cel-pushdown-limits.ts`'s
+"nothing else needs to move at GA": a third lint gate (`validateStackExpressions`)
+covers the same `sharingRules[].condition` and is mode-agnostic, so during the
+rc grace window lint is stricter than the runtime — benign, tightening-direction,
+and self-healing at GA. No behaviour change there.

packages/formula/src/cel-pushdown-limits.ts

@@ -59,6 +59,32 @@
  * lint suites pin "the lint verdict IS the consumer's verdict" in both
  * directions — so authoring-time reporting flips with the runtime, by
  * construction, and cannot drift from it.
+ *
+ * ### The third gate is NOT downstream of this switch — and that is fine (#7073)
+ *
+ * "Nothing else needs to move" is the right conclusion but the two-gate list
+ * above is not the whole set. A **third** lint gate reaches the same
+ * `sharingRules[].condition` field: `validateStackExpressions`, which goes
+ * through ADR-0032's shared `validateExpression` → `celEngine.compile`. That
+ * path applies {@link DEFAULT_LIMITS} **unconditionally** and never reads this
+ * switch (measured on #6833: `celPushdownLimitsMode` appears nowhere in
+ * `validate.ts` or `cel-engine.ts`'s compile path), so it is mode-agnostic by
+ * construction rather than by oversight.
+ *
+ * The consequence, stated plainly so the next reader does not "discover" it as
+ * a bug: **during the grace window lint is STRICTER than the runtime.** An
+ * over-budget `condition` is a gating lint ERROR today, while the pushdown path
+ * still compiles it under `rc-grace`. That divergence runs in the tightening
+ * direction — the author is told at authoring time about a source that will be
+ * refused at GA — and it **self-heals at GA**, when the runtime catches up to
+ * the position lint already holds. #6833's measurement graded it benign on
+ * exactly those grounds. Loosening lint to chase the grace window would be a
+ * regression, not a fix: it would restore the silent acceptance #6132 closed.
+ *
+ * So the GA checklist is unchanged. What #7073 corrected on that third gate is
+ * the message's PRESCRIPTION, not its verdict: an over-budget expression used
+ * to be told "predicates are bare CEL", the dialect trailer, which sends the
+ * author to rewrite a dialect that was never wrong.
  */
 
 /** How the pushdown path answers a source that overruns a `DEFAULT_LIMITS` bound. */

packages/formula/src/validate.test.ts

@@ -50,6 +50,93 @@ describe('validateExpression (ADR-0032)', () => {
     });
   });
 
+  // #7073 — the trailer used to be undifferentiated: EVERY `celEngine.compile`
+  // refusal, bounds included, ended with the dialect prescription
+  // "`predicate`s are bare CEL (e.g. `record.rating >= 4`)". For a
+  // syntactically perfect but over-budget expression that sentence is advice
+  // that cannot succeed — the source IS bare CEL — and an author who follows
+  // the last sentence they were handed (an LLM author above all) rewrites the
+  // dialect and regresses.
+  //
+  // Both directions are pinned, deliberately. A test asserting only "the
+  // bounds message changed" would stay green on a fix that ALSO stripped the
+  // dialect trailer from genuine dialect faults, i.e. that shrank the refusal
+  // surface while appearing to widen it.
+  describe('bounds vs dialect: the prescription follows the fault class (#7073)', () => {
+    /** 80-term conjunction — the escalation's `maxAstNodes` shape (#6833's fixture). */
+    const OVER_AST_NODES = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
+    /** 60-level parenthesis nest — `maxDepth`. Counts recursion that leaves no AST node. */
+    const OVER_DEPTH = `${'('.repeat(60)}record.a${')'.repeat(60)} == 1`;
+    /** 200-element list literal — `maxListElements`. */
+    const OVER_LIST = `record.id in [${Array.from({ length: 200 }, (_, i) => `'u${i}'`).join(',')}]`;
+
+    /** The byte-for-byte dialect trailer, per role. Must survive on dialect faults. */
+    const dialectTrailer = (role: 'predicate' | 'value') =>
+      ` — ${role}s are bare CEL (e.g. \`record.rating >= 4\`).`;
+
+    describe.each([
+      { name: 'maxAstNodes (80-term conjunction)', source: OVER_AST_NODES, limit: 'maxAstNodes' },
+      { name: 'maxDepth (60-level nest)', source: OVER_DEPTH, limit: 'maxDepth' },
+      { name: 'maxListElements (200-element list)', source: OVER_LIST, limit: 'maxListElements' },
+    ])('an over-budget but valid CEL $name', ({ source, limit }) => {
+      it('is refused, names the exceeded bound and its value, and never says "bare CEL"', () => {
+        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 reason — was always right; keep it.
+        expect(message).toMatch(/^invalid CEL predicate:/);
+        expect(message).toMatch(/Exceeded/);
+        // The bound is NAMED with the platform's value for it.
+        expect(message).toContain(`\`${limit}\` budget (limit `);
+        // …and the prescription is a size prescription, not a dialect one.
+        expect(message).toMatch(/SIZE fault, not a dialect mistake/);
+        expect(message).toMatch(/Shrink it/);
+        // ⛔ 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', source);
+        expect(r.ok).toBe(false);
+        expect(r.errors[0].message).toMatch(/^invalid CEL value:/);
+        expect(r.errors[0].message).toMatch(/SIZE fault, not a dialect mistake/);
+        expect(r.errors[0].message).not.toContain(dialectTrailer('value'));
+      });
+    });
+
+    // The flipped pin. The refusal surface may never shrink: a genuine dialect
+    // or syntax fault keeps the ADR-0032 §1d trailer, byte for byte.
+    it.each([
+      { name: 'an unterminated comparison', source: 'record.stage ==' },
+      { name: 'a stray token', source: 'record.stage @@ "won"' },
+      { name: 'a SQL-dialect predicate', source: "stage = 'won' AND rating >= 4" },
+    ])('keeps the dialect trailer verbatim on $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(/SIZE fault/);
+    });
+
+    it('keeps the #1491 braces hint on a brace fault (it outranks no bounds fault)', () => {
+      const r = validateExpression('predicate', '{record.rating} >= 4');
+      expect(r.ok).toBe(false);
+      expect(r.errors[0].message).toMatch(/template brace was used inside a CEL expression/);
+      expect(r.errors[0].message).not.toMatch(/SIZE fault/);
+    });
+
+    it('offers no split remedy without a caveat — combination semantics differ per slot', () => {
+      // PR #6831's RLS sentence ("splitting the top-level `&&` widens the
+      // grant") is TRUE for a security predicate and wrong-to-meaningless for a
+      // formula value. This shared producer therefore ships the caveat, not the
+      // slot-specific claim.
+      const message = validateExpression('predicate', OVER_AST_NODES).errors[0].message;
+      expect(message).toMatch(/changes how they combine at this authoring site/);
+      expect(message).not.toMatch(/widen|grant|permission/i);
+    });
+  });
+
   describe('templates', () => {
     it('accepts a valid {{ path }} template', () => {
       const r = validateExpression('template', 'Hot lead: {{ record.full_name }}');

packages/formula/src/validate.ts

@@ -17,7 +17,14 @@
  * This validator detects that specific mistake and returns the exact fix.
  */
 
-import { celEngine, firstUndeclaredReference, firstTypeMismatch, inferCelType, type FieldCelType } from './cel-engine';
+import {
+  celEngine,
+  firstUndeclaredReference,
+  firstTypeMismatch,
+  inferCelType,
+  parseCelToAstWithReason,
+  type FieldCelType,
+} from './cel-engine';
 import { templateEngine } from './template-engine';
 
 export type FieldRole = 'predicate' | 'value' | 'template';
@@ -195,6 +202,58 @@ function bracesHint(source: string): string | null {
   );
 }
 
+/**
+ * The prescription for a **bounds** refusal — an expression that is perfectly
+ * good CEL and merely too big for the platform's parse budget (#7073).
+ *
+ * Until #7073 every `celEngine.compile` refusal got the same dialect trailer
+ * ("`predicate`s are bare CEL (e.g. `record.rating >= 4`)"), byte for byte,
+ * including this class. That sentence is actively wrong here: the source is
+ * already bare CEL, so 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 80-clause conjunction. The front half of the message
+ * (cel-js's own `Exceeded maxAstNodes (256)`) was right all along; only the
+ * prescription lied.
+ *
+ * The class comes from `celEngine.compile`'s own `kind: 'bounds'`; WHICH bound
+ * and its value come from {@link parseCelToAstWithReason}, the same
+ * reason-carrying entrance `@objectstack/lint`'s RLS gate reads (#6778 /
+ * PR #6831 — the consumer-side instance of this same defect family). Called
+ * WITHOUT `admitOverLimit`, so it takes neither the unbounded parse nor the
+ * overrun measurement: this path needs only the bound's NAME, and a refusal
+ * must not pay to re-parse a source it has just judged too large.
+ *
+ * ### Why the remedies are generic
+ *
+ * `validateExpression` is ADR-0032's shared validator: one message serves all
+ * ~10 expression slots (flow/automation conditions, `Field.formula`, validation
+ * rules, `visibleWhen`, `sharingRules[].condition`, the `validate_expression`
+ * tool …). Their COMBINATION semantics differ, so PR #6831's RLS-specific
+ * sentence ("splitting the top-level `&&` widens the grant") is not portable —
+ * it is true for a security predicate and false, or merely meaningless, for a
+ * formula value. Shrinking and denormalising are safe everywhere; splitting is
+ * offered only with the caveat that the site decides what splitting means.
+ */
+function boundsHint(source: string): string | null {
+  const parsed = parseCelToAstWithReason(source);
+  // `celEngine.compile` said `bounds`, and both verdicts are graded by the same
+  // `classifyCelFault`, so this holds — but a narrowing that ever stopped
+  // holding must degrade to the old trailer, never to a wrong bound name.
+  if (parsed.ok || parsed.kind !== 'bounds') return null;
+  const { limit, limitValue } = parsed.overrun;
+  const bound =
+    limit && limitValue != null
+      ? `the \`${limit}\` budget (limit ${limitValue})`
+      : "one of the platform's parse budgets";
+  return (
+    `this is valid CEL that exceeds ${bound} — a SIZE fault, not a dialect mistake, so ` +
+    `re-spelling the expression will not fix it. Shrink it (fewer clauses, shallower nesting, ` +
+    `fewer list elements), or precompute the heavy part into a stored field and reference that ` +
+    `field instead. Splitting it into several expressions changes how they combine at this ` +
+    `authoring site, so check that site's semantics before doing that.`
+  );
+}
+
 function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined, errors: ExprValidationError[]): void {
   if (!schema?.fields || schema.fields.length === 0) return;
   const known = new Set(schema.fields);
@@ -339,7 +398,11 @@ export function validateExpression(
   }
   const compiled = celEngine.compile(source);
   if (!compiled.ok) {
-    const hint = bracesHint(source);
+    // #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);
     errors.push({
       source,
       message: