Skip to content

Commit a02bc95

Browse files
os-zhuangclaude
andcommitted
feat(cli): add phantom-aggregation flow lint (#1870)
A node-config key naming a capability the automation engine lacks (aggregations/aggregate/groupBy/rollup/having) is silently ignored at runtime — the node runs and computes nothing (templates publication_rollup). Flag it and point the author to the data-layer equivalent: Field.summary for a cross-object rollup, Field.formula for a per-record computed value. New rule flow-phantom-aggregation; advisory warning. 3 tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0d9be46 commit a02bc95

3 files changed

Lines changed: 70 additions & 9 deletions

File tree

.changeset/lint-date-equality-filter.md

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@
22
"@objectstack/cli": minor
33
---
44

5-
feat(cli): lint date-EQUALITY against time values in flow query filters (#1874)
5+
feat(cli): two new flow authoring anti-pattern lints — date-equality filters (#1874) and phantom aggregation (#1870)
66

7-
The flow anti-pattern lint already flagged a record-change trigger CONDITION
8-
using date equality (`end_date == daysFromNow(60)`). It now also scans
9-
get_record/query node FILTERS for the same footgun: a field bound directly, or
10-
via `$eq` / `$in`, to a time-function value (`daysFromNow`/`today`/`now`/…).
11-
A `Field.date` is stored with a time component, so an exact match against a
12-
re-computed timestamp silently returns nothing — the failure the templates
13-
discrete-tier alerts hit. Range operators (`$gte`/`$lt` day windows) are the
14-
correct shape and are never flagged. Advisory warning; never fails the build.
7+
Extends the build-time flow anti-pattern lint (advisory warnings, never fail the build):
8+
9+
- **flow-date-equality-filter (#1874)**: a get_record/query filter that binds a
10+
field directly, or via `$eq`/`$in`, to a time-function value
11+
(`daysFromNow`/`today`/`now`/…). A `Field.date` stores a time component, so an
12+
exact match against a re-computed timestamp silently returns nothing. Range
13+
operators (`$gte`/`$lt` day windows) are the correct shape and are exempt.
14+
- **flow-phantom-aggregation (#1870)**: a node config key naming a capability the
15+
automation engine does not have (`aggregations`/`aggregate`/`groupBy`/`rollup`/
16+
`having`). There is no aggregate node, so the key is silently ignored and the
17+
node computes nothing. Points the author to `Field.summary` / `Field.formula`.

packages/cli/src/utils/lint-flow-patterns.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
lintFlowPatterns,
66
FLOW_TIME_RELATIVE_ANTIPATTERN,
77
FLOW_DATE_EQUALITY_FILTER,
8+
FLOW_PHANTOM_AGGREGATION,
89
FLOW_DOUBLE_BRACE_INTERP,
910
FLOW_BARE_DOLLAR_REF,
1011
} from './lint-flow-patterns.js';
@@ -110,6 +111,36 @@ describe('lintFlowPatterns — date-equality in query filter (#1874)', () => {
110111
});
111112
});
112113

114+
describe('lintFlowPatterns — phantom aggregation capability (#1870)', () => {
115+
const scriptNode = (config: Record<string, unknown>) => ({
116+
flows: [{
117+
name: 'rollup',
118+
nodes: [
119+
{ id: 'start', type: 'start', config: {} },
120+
{ id: 'sum', type: 'script', config },
121+
],
122+
edges: [],
123+
}],
124+
});
125+
126+
it('flags `aggregations` on a script node (publication_rollup bug)', () => {
127+
const fnds = lintFlowPatterns(scriptNode({ aggregations: { total: { sum: 'amount' } } }));
128+
expect(fnds).toHaveLength(1);
129+
expect(fnds[0].rule).toBe(FLOW_PHANTOM_AGGREGATION);
130+
expect(fnds[0].hint).toMatch(/Field\.summary/);
131+
});
132+
133+
it('flags groupBy / rollup / aggregate / having too', () => {
134+
for (const key of ['groupBy', 'rollup', 'aggregate', 'having']) {
135+
expect(lintFlowPatterns(scriptNode({ [key]: {} })).map((f) => f.rule)).toContain(FLOW_PHANTOM_AGGREGATION);
136+
}
137+
});
138+
139+
it('does NOT flag an ordinary script/function node', () => {
140+
expect(lintFlowPatterns(scriptNode({ function: 'helpdesk.triage', inputs: { x: 1 } }))).toHaveLength(0);
141+
});
142+
});
143+
113144
/** A flow with a create_record node carrying `config`. */
114145
const nodeFlow = (config: Record<string, unknown>) => ({
115146
flows: [{

packages/cli/src/utils/lint-flow-patterns.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,18 @@ const DATE_EQ = new RegExp(
4747

4848
export const FLOW_TIME_RELATIVE_ANTIPATTERN = 'flow-time-relative-antipattern';
4949
export const FLOW_DATE_EQUALITY_FILTER = 'flow-date-equality-filter';
50+
export const FLOW_PHANTOM_AGGREGATION = 'flow-phantom-aggregation';
5051
export const FLOW_DOUBLE_BRACE_INTERP = 'flow-double-brace-interpolation';
5152
export const FLOW_BARE_DOLLAR_REF = 'flow-bare-dollar-reference';
5253

54+
/**
55+
* Node-config keys that name a capability the automation engine does NOT have.
56+
* There is no aggregate node, so a `script`/`loop`/… node carrying these keys is
57+
* silently ignored — the node runs and computes nothing (templates #1870,
58+
* `publication_rollup`). Aggregation belongs in the data layer, not a flow.
59+
*/
60+
const PHANTOM_AGG_KEYS = new Set(['aggregations', 'aggregate', 'groupBy', 'rollup', 'having']);
61+
5362
/** If `v` is a CEL expression whose source calls a time function, return that source. */
5463
function celTimeSource(v: unknown): string | null {
5564
if (v && typeof v === 'object' && (v as AnyRec).dialect === 'cel') {
@@ -180,6 +189,24 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] {
180189
const cfg = (node.config ?? {}) as AnyRec;
181190
if (cfg.filter) scanFilterForDateEquality(cfg.filter, `${nodeWhere} filter`, findings);
182191

192+
// (a3) #1870 — a node-config key naming a non-existent capability (there is
193+
// no aggregate node) is silently ignored at runtime, so the node
194+
// computes nothing. Point the author at the data-layer equivalent.
195+
for (const key of Object.keys(cfg)) {
196+
if (PHANTOM_AGG_KEYS.has(key)) {
197+
findings.push({
198+
where: nodeWhere,
199+
message:
200+
`node config has \`${key}\` — the automation engine has no aggregate node, so \`${key}\` is ` +
201+
`silently ignored and this node computes nothing at runtime.`,
202+
hint:
203+
`Aggregation belongs in the data layer: use \`Field.summary\` for a cross-object rollup ` +
204+
`(sum/count of children), or \`Field.formula\` for a per-record computed value. (#1870)`,
205+
rule: FLOW_PHANTOM_AGGREGATION,
206+
});
207+
}
208+
}
209+
183210
const strings: string[] = [];
184211
collectTemplateStrings(node.config, undefined, strings);
185212
for (const str of strings) {

0 commit comments

Comments
 (0)