Skip to content

Commit e1c2f51

Browse files
committed
fix(lint): an over-budget visibility predicate is a SIZE fault, not "not valid CEL" (#7217)
`visibility-predicate-syntax` reported a predicate that overruns a `DEFAULT_LIMITS` parse bound as "not valid CEL" and handed it the dialect prescription — a false headline plus advice that cannot succeed on a source already spelled in bare CEL. An author who follows it rewrites operators that were never wrong and returns with the same over-budget predicate. The verdict is untouched: same inputs refused, same severity, one finding per broken predicate. `celRefusal` now asks `parseCelToAstWithReason` — which `parseCelToAst` is literally implemented as, same env and same limits — so no source changes colour; only the explanation does. A bounds refusal is reported under a new error-level id `visibility-predicate-over-budget`, naming the bound and its platform value, for the reason #6778 / PR #6831 split `rls-predicate-over-budget` off `rls-predicate-unparseable`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KJATVrh6V2ysutYUJigh3B
1 parent 0caf122 commit e1c2f51

4 files changed

Lines changed: 381 additions & 59 deletions

File tree

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

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ export {
166166
VISIBILITY_ROOT_MISLAYERED,
167167
VISIBILITY_BARE_IDENTIFIER,
168168
VISIBILITY_PREDICATE_SYNTAX,
169+
VISIBILITY_PREDICATE_OVER_BUDGET,
169170
} from './validate-visibility-predicates.js';
170171
export type {
171172
VisibilityFinding,

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

Lines changed: 152 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
VISIBILITY_ROOT_MISLAYERED,
77
VISIBILITY_BARE_IDENTIFIER,
88
VISIBILITY_PREDICATE_SYNTAX,
9+
VISIBILITY_PREDICATE_OVER_BUDGET,
910
} from './validate-visibility-predicates.js';
1011
import { AUTHORING_RULES } from './authoring-rules.js';
1112

@@ -713,19 +714,15 @@ describe('visibility-predicate-syntax (#6253)', () => {
713714
expect(validateVisibilityPredicates(formStack('type(record.x) == string'))).toEqual([]);
714715
});
715716

716-
it('a `DEFAULT_LIMITS` overrun is reported too, in the front end\'s own words', () => {
717-
// `parseCelToAst` also returns null for a source over the platform bounds.
718-
// That is a bounds fault, not a syntax one, and the message says so rather
719-
// than pretending to have found a typo — the same way ADR-0032 already
720-
// reports it under the "invalid CEL predicate" heading.
717+
it('a `DEFAULT_LIMITS` overrun is NOT this rule any more — it is `over-budget` (#7217)', () => {
718+
// Was: "reported too, in the front end's own words", asserted under
719+
// `visibility-predicate-syntax`. #7217 keeps the verdict and moves the
720+
// wording and the id; this case is now the exclusivity pin for the split,
721+
// and its full coverage lives in the `over-budget` block below.
721722
const overrun = `record.a${' + record.b'.repeat(400)}`;
722-
const findings = syntaxFindings(formStack(overrun));
723-
expect(findings).toHaveLength(1);
724-
expect(findings[0].message).toContain('Exceeded maxAstNodes');
725-
// The echoed predicate is elided, so one runaway expression cannot flood
726-
// the console with a 4KB finding.
727-
expect(findings[0].message).not.toContain(overrun);
728-
expect(findings[0].message).toContain('...');
723+
expect(syntaxFindings(formStack(overrun))).toEqual([]);
724+
expect(validateVisibilityPredicates(formStack(overrun)).map((f) => f.rule))
725+
.toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
729726
});
730727

731728
it.each([
@@ -806,3 +803,146 @@ describe('visibility-predicate-syntax (#6253)', () => {
806803
expect([...entry!.commands].sort()).toEqual(['build', 'lint', 'validate']);
807804
});
808805
});
806+
807+
// ─────────────────────────────────────────────────────────────────────
808+
// `visibility-predicate-over-budget` — #7217.
809+
//
810+
// The defect this closes is an INSTRUCTION that makes an obedient author
811+
// worse. An over-budget `visibleWhen` is flawless bare CEL; the gate refused it
812+
// (correctly) as "not valid CEL" (false) and prescribed the dialect ("write
813+
// `==` not `===`, `&&` not `and` …"), which is advice that cannot succeed. An
814+
// LLM author follows the last sentence it was handed, rewrites operators that
815+
// were never wrong, and returns with the same 80-clause predicate.
816+
//
817+
// The verdict is untouched: the same sources are refused before and after, at
818+
// the same severity, one finding each. Only the class, the id and the words
819+
// changed. Both directions are pinned deliberately — a fix that turned EVERY
820+
// refusal into a size refusal would be green on the first block below, which is
821+
// why the syntax block re-asserts the dialect wording it must not lose.
822+
// ─────────────────────────────────────────────────────────────────────
823+
824+
/** Only the over-budget findings. */
825+
function overBudgetFindings(stack: Record<string, unknown>, opts?: { layer: 'runtime' | 'metadata' }) {
826+
return validateVisibilityPredicates(stack, opts).filter((f) => f.rule === VISIBILITY_PREDICATE_OVER_BUDGET);
827+
}
828+
829+
/** The escalation's own shape — 80-term conjunction, `maxAstNodes` (#6833 / #7073). */
830+
const OVER_AST_NODES = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
831+
/** 60-level parenthesis nest — `maxDepth`. Recursion that leaves no AST node. */
832+
const OVER_DEPTH = `${'('.repeat(60)}record.a${')'.repeat(60)} == 1`;
833+
/** 200-element list literal — `maxListElements`. */
834+
const OVER_LIST = `record.id in [${Array.from({ length: 200 }, (_, i) => `'u${i}'`).join(',')}]`;
835+
836+
describe('visibility-predicate-over-budget (#7217)', () => {
837+
describe('the acceptance pair', () => {
838+
it('an over-budget but valid predicate names the SIZE fault and the bound, never the dialect', () => {
839+
const findings = validateVisibilityPredicates(formStack(OVER_AST_NODES));
840+
841+
// The whole reported set: one finding, the new id, still gating.
842+
expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
843+
expect(findings[0].severity).toBe('error');
844+
expect(findings[0].path).toBe('views[0].sections[0].fields[0]');
845+
expect(findings[0].where).toBe('view "task_form"');
846+
847+
// ⛔ The headline was FALSE: this IS valid CEL.
848+
expect(findings[0].message).not.toContain('is not valid CEL');
849+
expect(findings[0].message).toContain('syntactically valid CEL');
850+
// The front end's own summary is quoted, and the bound is NAMED with the
851+
// platform's value for it — which is what "shrink it to fit" needs.
852+
expect(findings[0].message).toContain('Exceeded maxAstNodes (256)');
853+
expect(findings[0].message).toContain('`maxAstNodes` budget (platform limit 256)');
854+
// The consequence is unchanged: it still falls OPEN on screen.
855+
expect(findings[0].message).toContain('#5149');
856+
857+
// ⛔ The defect itself: the dialect prescription must not reach this class.
858+
expect(findings[0].hint).not.toMatch(/bare CEL/);
859+
expect(findings[0].hint).not.toContain('`===`');
860+
expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake');
861+
expect(findings[0].hint).toContain("`record.f in ['a', 'b', …]`");
862+
});
863+
864+
it('the SAME predicate under budget is clean — paired so it cannot pass vacuously', () => {
865+
// Delete the rule and the first expectation goes red; loosen the verdict
866+
// (report everything) and the second does.
867+
expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1);
868+
const underBudget = Array.from({ length: 8 }, (_, i) => `record.f${i} == ${i}`).join(' && ');
869+
expect(validateVisibilityPredicates(formStack(underBudget))).toEqual([]);
870+
});
871+
});
872+
873+
it.each([
874+
['maxAstNodes (80-term conjunction)', OVER_AST_NODES, 'maxAstNodes', 256],
875+
['maxDepth (60-level nest)', OVER_DEPTH, 'maxDepth', 32],
876+
['maxListElements (200-element list)', OVER_LIST, 'maxListElements', 64],
877+
])('%s — the bound that was actually exceeded is the one named', (_name, source, limit, value) => {
878+
// Not one hard-coded bound: the name and its value are read off the front
879+
// end's structured overrun, so a source that overruns a DIFFERENT axis is
880+
// sent to shorten that axis and not `maxAstNodes` by default.
881+
const findings = overBudgetFindings(formStack(source as string));
882+
expect(findings).toHaveLength(1);
883+
expect(findings[0].message).toContain(`\`${limit}\` budget (platform limit ${value})`);
884+
expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake');
885+
});
886+
887+
it('a genuine dialect fault keeps the #6253 id, message and hint verbatim', () => {
888+
// The flipped pin. The obvious way to get #7217 wrong is to turn EVERY
889+
// refusal into a size refusal — green on every case above, and a total loss
890+
// of the wording #6253 shipped.
891+
const findings = validateVisibilityPredicates(formStack('country === "USA"'));
892+
expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]);
893+
expect(findings[0].message).toContain('is not valid CEL');
894+
expect(findings[0].hint).toContain('`===` is not a CEL operator');
895+
expect(findings[0].hint).not.toMatch(/SIZE fault/);
896+
});
897+
898+
it('a dialect fault with no single-token equivalent keeps the FALLBACK dialect hint', () => {
899+
// The arm the card names: the fallback hint fires when no NON_CEL_SPELLINGS
900+
// row matches — which is exactly the arm an over-budget source used to land
901+
// in. It must still fire for a source that genuinely is not CEL.
902+
const findings = validateVisibilityPredicates(formStack('record.stage @@ "won"'));
903+
expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]);
904+
expect(findings[0].hint).toContain('Visibility predicates are bare CEL');
905+
expect(findings[0].hint).not.toMatch(/SIZE fault/);
906+
});
907+
908+
it('still exactly ONE finding — the bare-identifier rule stays out of the way', () => {
909+
// An over-budget source yields no AST, so there are no identifiers to judge.
910+
// The exclusivity property #6253/#6128 established survives the split.
911+
const rootless = Array.from({ length: 80 }, (_, i) => `f${i} == ${i}`).join(' && ');
912+
expect(validateVisibilityPredicates(formStack(rootless)).map((f) => f.rule))
913+
.toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]);
914+
});
915+
916+
it('elides the echoed predicate — one runaway expression cannot flood the console', () => {
917+
const findings = overBudgetFindings(formStack(OVER_AST_NODES));
918+
expect(findings[0].message).not.toContain(OVER_AST_NODES);
919+
expect(findings[0].message).toContain('...');
920+
});
921+
922+
it('a blank predicate is still not a fault of any class', () => {
923+
// `parseCelToAstWithReason` answers `empty` rather than `parse`; paired with
924+
// a live case so the assertion can actually fail.
925+
expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1);
926+
expect(validateVisibilityPredicates(formStack(' '))).toEqual([]);
927+
expect(validateVisibilityPredicates(formStack(undefined))).toEqual([]);
928+
});
929+
930+
it('speaks the LAYER\'s binding root in its remedy', () => {
931+
// A `*.form.ts` author binds `data`, so a `record.`-flavoured example would
932+
// be a second wrong prescription in a rule whose whole point is that the
933+
// prescription must be followable.
934+
const metadata = overBudgetFindings(formStack(OVER_AST_NODES), { layer: 'metadata' });
935+
expect(metadata).toHaveLength(1);
936+
expect(metadata[0].hint).toContain("`data.f in ['a', 'b', …]`");
937+
expect(metadata[0].hint).not.toContain('`record.f in');
938+
});
939+
940+
it('reaches page components too — the id is not view-only', () => {
941+
const stack = {
942+
pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: OVER_AST_NODES }] }] }],
943+
};
944+
const findings = overBudgetFindings(stack);
945+
expect(findings.map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']);
946+
expect(findings[0].where).toBe('page "p"');
947+
});
948+
});

0 commit comments

Comments
 (0)