fix(lint): an over-budget visibility predicate is a SIZE fault, not "not valid CEL" (#7217)

.changeset/visibility-predicate-over-budget.md

@@ -0,0 +1,46 @@
+---
+'@objectstack/lint': patch
+---
+
+`visibility-predicate-syntax`: an over-budget predicate is a SIZE fault, not "not valid CEL" (#7217)
+
+view / page `visibleWhen` 的门禁把两类拒绝合成了一类。`parseCelToAst` 对"不是 CEL"
+和"是 CEL、但超过平台解析预算"返回同一个 `null`（生产侧刻意如此），于是一条
+80 项合取的谓词——完全合法的 bare CEL，只是超过 `maxAstNodes` 256——被报成
+`visibility predicate is not valid CEL`，并附上方言处方（"写 `==` 不是 `===`、
+`&&` 不是 `and` …"）。标题是假的，处方在这条源码上根本不可能成功：作者（尤其是
+照着最后一句话执行的 LLM 作者）会去改一堆本来就没错的运算符，然后带着同一条超预算
+谓词回来。#7073 / PR #7209 在 ADR-0032 的共享生产者上修的是同一个缺陷，而本门禁
+按其自身 docblock 刻意不走 `validateExpression`，所以生产侧的修复到不了这里。
+
+**判定不变**：拒绝的输入集合、严重级别、每条坏谓词一个 finding，全部与修复前一致。
+红绿边界仍然是规范前端接受什么——`celRefusal` 改问 `parseCelToAstWithReason`，而
+`parseCelToAst` 本身就是"它把 reason 丢掉"（同一个 env、同一套 limits），所以没有
+任何一条源码换了颜色。变的只有解释。
+
+新增第三个 error 级 id **`visibility-predicate-over-budget`**（与
+`visibility-predicate-syntax` / `visibility-bare-identifier` 并列导出），理由与
+#6778 / PR #6831 在 RLS 侧把 `rls-predicate-over-budget` 从
+`rls-predicate-unparseable` 拆出来时相同：后果相同，**修法不同**，而 `--json`
+消费者与抑制清单都按 id 取值。超预算谓词现在报：
+
+> visibility predicate is syntactically valid CEL but overruns the `maxAstNodes`
+> budget (platform limit 256) (Exceeded maxAstNodes (256)) (predicate: …) …
+>
+> hint: There is no syntax or dialect error to correct here — this is a SIZE
+> fault, not a dialect mistake, so re-spelling the predicate will not fix it.
+> Make it smaller, or move the work off the predicate: (1) collapse a long
+> `record.f == 'a' || record.f == 'b' || …` chain into a single
+> `record.f in ['a', 'b', …]` …; (2) precompute the heavy part into a
+> formula/rollup field on the object and test that one field instead. …
+
+越界的那条界（`maxAstNodes` / `maxDepth` / `maxListElements` / …）与平台取值来自
+前端自己的结构化 `overrun`，不是硬编码，也不是二次解析它的散文（#6223）；提示里的
+绑定根随 layer 走（runtime 用 `record`，`*.form.ts` 元数据表单用 `data`），否则处方
+本身又会是一句照做不了的话。真正的方言/语法错误保持 #6253 的 id、message 与 hint
+逐字不变——两个方向都有 pin。
+
+**兼容性**：这是新增 id，不是改名。抑制 `visibility-predicate-syntax` 的配置从此不再
+抑制超预算这一类——与 #6778 接受的代价相同，而且本来就是抑制错了对象（抑制的是
+"语法"，命中的是"太大"）。仓库内除 `packages/lint` 自身与 changelog 外，没有任何
+配置、文档或示例引用这些 id。

packages/lint/src/index.ts

@@ -166,6 +166,7 @@ export {
   VISIBILITY_ROOT_MISLAYERED,
   VISIBILITY_BARE_IDENTIFIER,
   VISIBILITY_PREDICATE_SYNTAX,
+  VISIBILITY_PREDICATE_OVER_BUDGET,
 } from './validate-visibility-predicates.js';
 export type {
   VisibilityFinding,

packages/lint/src/validate-visibility-predicates.test.ts

@@ -6,6 +6,7 @@ import {
   VISIBILITY_ROOT_MISLAYERED,
   VISIBILITY_BARE_IDENTIFIER,
   VISIBILITY_PREDICATE_SYNTAX,
+  VISIBILITY_PREDICATE_OVER_BUDGET,
 } from './validate-visibility-predicates.js';
 import { AUTHORING_RULES } from './authoring-rules.js';
 
@@ -713,19 +714,15 @@ describe('visibility-predicate-syntax (#6253)', () => {
       expect(validateVisibilityPredicates(formStack('type(record.x) == string'))).toEqual([]);
     });
 
-    it('a `DEFAULT_LIMITS` overrun is reported too, in the front end\'s own words', () => {
-      // `parseCelToAst` also returns null for a source over the platform bounds.
-      // That is a bounds fault, not a syntax one, and the message says so rather
-      // than pretending to have found a typo — the same way ADR-0032 already
-      // reports it under the "invalid CEL predicate" heading.
+    it('a `DEFAULT_LIMITS` overrun is NOT this rule any more — it is `over-budget` (#7217)', () => {
+      // Was: "reported too, in the front end's own words", asserted under
+      // `visibility-predicate-syntax`. #7217 keeps the verdict and moves the
+      // wording and the id; this case is now the exclusivity pin for the split,
+      // and its full coverage lives in the `over-budget` block below.
       const overrun = `record.a${' + record.b'.repeat(400)}`;
-      const findings = syntaxFindings(formStack(overrun));
-      expect(findings).toHaveLength(1);
-      expect(findings[0].message).toContain('Exceeded maxAstNodes');
-      // The echoed predicate is elided, so one runaway expression cannot flood
-      // the console with a 4KB finding.
-      expect(findings[0].message).not.toContain(overrun);
-      expect(findings[0].message).toContain('...');
+      expect(syntaxFindings(formStack(overrun))).toEqual([]);
+      expect(validateVisibilityPredicates(formStack(overrun)).map((f) => f.rule))
+        .toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
     });
 
     it.each([
@@ -806,3 +803,146 @@ describe('visibility-predicate-syntax (#6253)', () => {
     expect([...entry!.commands].sort()).toEqual(['build', 'lint', 'validate']);
   });
 });
+
+// ─────────────────────────────────────────────────────────────────────
+// `visibility-predicate-over-budget` — #7217.
+//
+// The defect this closes is an INSTRUCTION that makes an obedient author
+// worse. An over-budget `visibleWhen` is flawless bare CEL; the gate refused it
+// (correctly) as "not valid CEL" (false) and prescribed the dialect ("write
+// `==` not `===`, `&&` not `and` …"), which is advice that cannot succeed. An
+// LLM author follows the last sentence it was handed, rewrites operators that
+// were never wrong, and returns with the same 80-clause predicate.
+//
+// The verdict is untouched: the same sources are refused before and after, at
+// the same severity, one finding each. Only the class, the id and the words
+// changed. Both directions are pinned deliberately — a fix that turned EVERY
+// refusal into a size refusal would be green on the first block below, which is
+// why the syntax block re-asserts the dialect wording it must not lose.
+// ─────────────────────────────────────────────────────────────────────
+
+/** Only the over-budget findings. */
+function overBudgetFindings(stack: Record<string, unknown>, opts?: { layer: 'runtime' | 'metadata' }) {
+  return validateVisibilityPredicates(stack, opts).filter((f) => f.rule === VISIBILITY_PREDICATE_OVER_BUDGET);
+}
+
+/** The escalation's own shape — 80-term conjunction, `maxAstNodes` (#6833 / #7073). */
+const OVER_AST_NODES = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
+/** 60-level parenthesis nest — `maxDepth`. 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(',')}]`;
+
+describe('visibility-predicate-over-budget (#7217)', () => {
+  describe('the acceptance pair', () => {
+    it('an over-budget but valid predicate names the SIZE fault and the bound, never the dialect', () => {
+      const findings = validateVisibilityPredicates(formStack(OVER_AST_NODES));
+
+      // The whole reported set: one finding, the new id, still gating.
+      expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
+      expect(findings[0].severity).toBe('error');
+      expect(findings[0].path).toBe('views[0].sections[0].fields[0]');
+      expect(findings[0].where).toBe('view "task_form"');
+
+      // ⛔ The headline was FALSE: this IS valid CEL.
+      expect(findings[0].message).not.toContain('is not valid CEL');
+      expect(findings[0].message).toContain('syntactically valid CEL');
+      // The front end's own summary is quoted, and the bound is NAMED with the
+      // platform's value for it — which is what "shrink it to fit" needs.
+      expect(findings[0].message).toContain('Exceeded maxAstNodes (256)');
+      expect(findings[0].message).toContain('`maxAstNodes` budget (platform limit 256)');
+      // The consequence is unchanged: it still falls OPEN on screen.
+      expect(findings[0].message).toContain('#5149');
+
+      // ⛔ The defect itself: the dialect prescription must not reach this class.
+      expect(findings[0].hint).not.toMatch(/bare CEL/);
+      expect(findings[0].hint).not.toContain('`===`');
+      expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake');
+      expect(findings[0].hint).toContain("`record.f in ['a', 'b', …]`");
+    });
+
+    it('the SAME predicate under budget is clean — paired so it cannot pass vacuously', () => {
+      // Delete the rule and the first expectation goes red; loosen the verdict
+      // (report everything) and the second does.
+      expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1);
+      const underBudget = Array.from({ length: 8 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
+      expect(validateVisibilityPredicates(formStack(underBudget))).toEqual([]);
+    });
+  });
+
+  it.each([
+    ['maxAstNodes (80-term conjunction)', OVER_AST_NODES, 'maxAstNodes', 256],
+    ['maxDepth (60-level nest)', OVER_DEPTH, 'maxDepth', 32],
+    ['maxListElements (200-element list)', OVER_LIST, 'maxListElements', 64],
+  ])('%s — the bound that was actually exceeded is the one named', (_name, source, limit, value) => {
+    // Not one hard-coded bound: the name and its value are read off the front
+    // end's structured overrun, so a source that overruns a DIFFERENT axis is
+    // sent to shorten that axis and not `maxAstNodes` by default.
+    const findings = overBudgetFindings(formStack(source as string));
+    expect(findings).toHaveLength(1);
+    expect(findings[0].message).toContain(`\`${limit}\` budget (platform limit ${value})`);
+    expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake');
+  });
+
+  it('a genuine dialect fault keeps the #6253 id, message and hint verbatim', () => {
+    // The flipped pin. The obvious way to get #7217 wrong is to turn EVERY
+    // refusal into a size refusal — green on every case above, and a total loss
+    // of the wording #6253 shipped.
+    const findings = validateVisibilityPredicates(formStack('country === "USA"'));
+    expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]);
+    expect(findings[0].message).toContain('is not valid CEL');
+    expect(findings[0].hint).toContain('`===` is not a CEL operator');
+    expect(findings[0].hint).not.toMatch(/SIZE fault/);
+  });
+
+  it('a dialect fault with no single-token equivalent keeps the FALLBACK dialect hint', () => {
+    // The arm the card names: the fallback hint fires when no NON_CEL_SPELLINGS
+    // row matches — which is exactly the arm an over-budget source used to land
+    // in. It must still fire for a source that genuinely is not CEL.
+    const findings = validateVisibilityPredicates(formStack('record.stage @@ "won"'));
+    expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]);
+    expect(findings[0].hint).toContain('Visibility predicates are bare CEL');
+    expect(findings[0].hint).not.toMatch(/SIZE fault/);
+  });
+
+  it('still exactly ONE finding — the bare-identifier rule stays out of the way', () => {
+    // An over-budget source yields no AST, so there are no identifiers to judge.
+    // The exclusivity property #6253/#6128 established survives the split.
+    const rootless = Array.from({ length: 80 }, (_, i) => `f${i} == ${i}`).join(' && ');
+    expect(validateVisibilityPredicates(formStack(rootless)).map((f) => f.rule))
+      .toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
+  });
+
+  it('elides the echoed predicate — one runaway expression cannot flood the console', () => {
+    const findings = overBudgetFindings(formStack(OVER_AST_NODES));
+    expect(findings[0].message).not.toContain(OVER_AST_NODES);
+    expect(findings[0].message).toContain('...');
+  });
+
+  it('a blank predicate is still not a fault of any class', () => {
+    // `parseCelToAstWithReason` answers `empty` rather than `parse`; paired with
+    // a live case so the assertion can actually fail.
+    expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1);
+    expect(validateVisibilityPredicates(formStack('   '))).toEqual([]);
+    expect(validateVisibilityPredicates(formStack(undefined))).toEqual([]);
+  });
+
+  it('speaks the LAYER\'s binding root in its remedy', () => {
+    // A `*.form.ts` author binds `data`, so a `record.`-flavoured example would
+    // be a second wrong prescription in a rule whose whole point is that the
+    // prescription must be followable.
+    const metadata = overBudgetFindings(formStack(OVER_AST_NODES), { layer: 'metadata' });
+    expect(metadata).toHaveLength(1);
+    expect(metadata[0].hint).toContain("`data.f in ['a', 'b', …]`");
+    expect(metadata[0].hint).not.toContain('`record.f in');
+  });
+
+  it('reaches page components too — the id is not view-only', () => {
+    const stack = {
+      pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: OVER_AST_NODES }] }] }],
+    };
+    const findings = overBudgetFindings(stack);
+    expect(findings.map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']);
+    expect(findings[0].where).toBe('page "p"');
+  });
+});