Skip to content

Commit 38fc529

Browse files
fix(lint)!: judge a roll-up's summaryOperations.filter keys against its child object (#31337)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XF9CLMiiKFscnGZpZTirGFK)_ Refs #31224 Clause-②: no (narrowing) Metadata-protocol: narrowed Skills-docs: none — `skills/**` and `content/docs/**` teach `summaryOperations.filter` keys that are fields of the child object, which this rule accepts. The reasoning ships in the `os explain object-field-ref-unknown` entry. ## 维护者速读 - **改了什么**:`@objectstack/lint` 的 `object-field-ref-unknown` 规则新增一个位置:汇总字段 `summaryOperations.filter` 里的每个键(含 `$and` / `$or` / `$not` 分组),都按 `summaryOperations.object` 指向的子对象来判定,子对象没有的键报 error,并给出最接近的字段名;重算查询不能执行的写法也报 error:点号路径(改写为一层嵌套 `{ account: { status } }`)、读时计算的字段(如 formula)、关系下的第二层条件。另加 `os explain` 段落、测试和一个 lint minor changeset。 - **为什么改**:#31224 第 D 项。引擎把这个 filter 当作子对象聚合查询的 `where`(与父外键匹配取 AND)。在真实 SqlDriver(better-sqlite3)上实测:键写错时,每次插入子记录都报 `SummaryRecomputeError`(记录其实已写入),汇总值一直停在旧值。之前没有任何门检查这些键:单个字段的 Zod 解析看不到另一个对象的字段表。 - **风险与回滚**:收窄:`os lint/validate/build` 与对象保存/发布门会拒绝此前接受的汇总字段。普查为零:showcase 4 个带 filter 的汇总(`status`、`billable`、`amount`,都是子对象真实字段),hotcrm 0 个。回滚即 revert。 - **先看**:`packages/lint/src/validate-object-field-refs.ts` 里 `summaryOperations.filter` 那一段,以及模块头新增的一条位置说明。 - **两分钟验证**:`pnpm --filter @objectstack/lint exec vitest run src/validate-object-field-refs.test.ts`,预期全部通过,其中新 describe 的 10 条为本卡新增。 - **需要你做的**:无,席位做契约复审后落地。 ## What changes 1. `packages/lint/src/validate-object-field-refs.ts`: a new field-level position in `validateObjectFieldRefs`. For a field whose `summaryOperations` names an `object` and carries a `filter`, the position reads the authored keys itself (`judgeSummaryFilter`): it descends `$and` / `$or` / `$not` groups and operator objects; a dotted relation key is refused with the nested-form cure, a dotted computed or scalar head (`classifyDottedFilterHead`) or a formula key (`isVirtualSearchField`) with the denormalise cure; the nested one-level relation form is accepted, and beneath it an empty condition, a dotted key, an undeclared key or a second level is refused. Each key is resolved against the CHILD object, never the object that owns the summary. The rule's shared `judge` does the resolution, so the verdict, the "Did you mean", severity `error` and the three family skips are the same as at every other position: a child the stack does not define, a child with no readable field map, and a registry-injected system column. Path: `objects[i].fields.FIELD.summaryOperations.filter.KEY` (group arms keep their index, e.g. `…filter.$and[1].$or[1].stauts`). Hint: "Fix the key — `summaryOperations.filter` runs on the child object `summaryOperations.object` names, so each key is a field of that child, not of the object that owns this summary." The module header gains the position with its reader and the measurement. 2. `packages/lint/src/rule-explanations.ts`: an `os explain object-field-ref-unknown` paragraph for the position, saying which object, what is walked, and what a miss costs. 3. `.changeset/31224-summary-filter-key-unknown.md`: `'@objectstack/lint': minor`, BREAKING banner, ADR-0087 `not-required (no-migration-prescription)`. **Why lint, not parse.** The verdict needs the child object's field map, which one field's Zod parse cannot see. The rule runs where the object graph is: `validateObjectFieldRefs` is in the reference-integrity suite with `runtimeTypes: ['flow', 'object']`, so the error reaches `os lint` / `os validate` / `os build`, the live object save and the per-item and package publish. A draft stores with the finding in `_diagnostics`. **Measured consequence** (one-off probe, not kept): a real `SqlDriver` (better-sqlite3) plus a real `ObjectQL`, with parent `pr_report.total = sum(pr_line.amount)`. - With `filter: { status: 'ok' }`, a child insert gives `total = 5`. - With `filter: { stauts: 'ok' }`, the same insert answers `SummaryRecomputeError` ("the triggering records WERE written") and `total` stays `0`. No `packages/spec`, metadata-protocol source, runtime or `ui/` change. ## Tests - `packages/lint/src/validate-object-field-refs.test.ts`, new describe "summaryOperations.filter (addresses the CHILD object, #31224)": - **Refused**: a misspelt key (error, path, "Did you mean", hint); a key only the parent has; misses inside `$and` / `$or` / `$not` (paths pinned). - **Accepted**: child fields with operators, and the nested one-level relation form `{ account: { status, industry: { $in } } }`. - **Refused, the shapes the recompute query refuses**: - the dotted spelling `account.status`, whose hint is the nested spelling; - a dotted path into a scalar field, with the denormalise cure; - a dotted key with an unknown head, which gets the unknown verdict; - a `formula` key, at the top level and one level down; - beneath a relation: an unknown related key, a dotted key, a second relation holding a condition, and an empty condition. - **Not judged**: a non-`summary` field. - **Not judged**: a child the stack does not define; a roll-up with no filter. - The file's two coverage lists, which require every position to have fired, gain `.summaryOperations.filter.`. - `packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts`: an `object-field-ref-unknown` row in `CASES` (a self-referential roll-up with `filter: { stauts: 'open' }`). The draft saves and reads `valid: false`; publish refuses with the same words; the package publish refuses and rolls back. - **Reverse check** (one-off, not kept): with the new block guarded off, 5 red (the three refusals and both coverage lists); restored, green. - **Census**: showcase has 4 filtered roll-ups (`expense-report.object.ts`: `status`, `billable`, `amount`, all fields of `showcase_expense_line`), 0 unknown keys; hotcrm `main` has 0 filtered roll-ups. Control: 9 roll-ups in examples carry the live key `summaryOperations.field`. ## Acceptance notes - Item D of #31224. Item C landed as #31287. Item B1 follows in its own PR. Item A is not built (seat ruling). So `Refs`, not `Fixes`. - The declaration follows the seat ruling: `no (narrowing)` (no new export, since the position joins an existing rule) with `narrowed`. ## Verification Final head `8fc74827`: the branch was restarted from `origin/main` `11c1326d` (#31287 merged). `8fc74827` follows the contract review (6716423527): the position reads the authored keys itself instead of `walkFilterFieldKeys`, which flattens a dotted key and a nested condition into one path. The recompute's `where` serves only the nested one-level form, so a dotted key, a computed field and a second relation level are refused, and the three texts no longer teach the dotted spelling. Measured on a real `SqlDriver` + `ObjectQL` (one-off probe, not kept): `{ report: { name: 'r' } }` sums (total 5); `{ 'report.name': 'r' }` and a `formula` key each answer `SummaryRecomputeError` on the child insert, and the total stays 0. - **Gates on `8fc74827`**: 67 derived by `dispatch-gates --commands`, all exit 0. They ran over the full build of `834abfdb`, with `@objectstack/lint` rebuilt for `8fc74827` and the teardown fixture commit fetched. `--ran` reconciles 67 accounted, 0 NOT-MEASURED, 0 unrun. - **Changeset level**: `check-changeset-no-major` with a `pull_request` event carrying this body exits 0. - **Build and dogfood**: full `pnpm build` (119 tasks), then `pnpm --filter @objectstack/dogfood test` under the verify lock (rerun on `8fc74827` after the lint rebuild): 435 files passed, 3 skipped; 3610 tests passed, 41 skipped; VERDICT command-exit 0. - **Suites**: `@objectstack/lint` 197 files / 8363 tests passed. `@objectstack/metadata-protocol` 324 files passed, 4 skipped (22538 tests), including the new `CASES` row (51/51 in that file). CI runs the full set. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV <!-- signed: domain:spec#1 · os-project-manager · session_01FEkQ5mw7HFYn9o4LEGMQdK --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent da7fce5 commit 38fc529

5 files changed

Lines changed: 382 additions & 1 deletion

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/lint': minor
3+
---
4+
5+
fix(lint)!: a roll-up summary whose `summaryOperations.filter` names a key its child object lacks is refused at build and publish (`object-field-ref-unknown`), because every child write then fails its recompute
6+
7+
Clause-②: no (narrowing) — `os validate`, `os build`, `os lint` and the object save and publish doors refuse a summary field they accepted before. No export is added: the position joins the existing `object-field-ref-unknown` rule.
8+
Metadata-protocol: narrowed
9+
Skills-docs: none — `skills/**` and `content/docs/**` teach `summaryOperations.filter` keys that are fields of the child object, which this rule accepts. The reasoning ships in the `os explain object-field-ref-unknown` entry.
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed or renamed: summaryOperations.filter keeps its shape and meaning, and lint now judges its keys against the child object the way lookupFilters[].field is judged against the referenced one. A refused key never filtered what the author meant (every child write answered a recompute failure and the summary stayed stale), so there is no rewrite that keeps the author's intent: the author names a real child field, and the verdict names the nearest one. -->
12+
13+
**BREAKING** accept-set narrowing in `os validate` / `os build` / `os lint` and at the object save and publish doors (`PUT /api/v1/meta/object/NAME` live save, the per-item and package publish; Studio, REST and MCP authors), through the `object-field-ref-unknown` rule.
14+
15+
- **Before.** A summary field with `summaryOperations: { object: 'invoice_line', function: 'sum', field: 'amount', filter: { stauts: 'paid' } }` parsed, built and published. The engine runs the filter as the `where` of an aggregate on the child, ANDed with the parent-FK match. Measured on a real `SqlDriver` (better-sqlite3): every insert of a child row then answered `SummaryRecomputeError` ("the triggering records WERE written"), and the summary kept a stale value.
16+
- **Now.** Each key the child lacks is refused `error` at `objects[i].fields.FIELD.summaryOperations.filter.KEY`, naming the key, the child object and the nearest field ("Did you mean"). Keys inside `$and` / `$or` / `$not` groups are judged too. Three more shapes the recompute query refuses are refused the same way, each measured on the same driver as a `SummaryRecomputeError` on every child insert:
17+
- a dotted key such as `account.status`: the cure is the nested one-level form the query serves, `{ account: { status: VALUE } }`;
18+
- a child field computed on read, such as a `formula`: denormalise it onto the child as a stored field;
19+
- beneath a relation, a key the related object lacks, a dotted key, a second relation holding a condition of its own, or an empty condition. The nested form reaches one level.
20+
- **Accepted.** Child fields with any operator, and the nested one-level relation form `{ relation: { field: VALUE } }` over fields of the related object.
21+
- **Not judged.** A child object the stack does not define, a child with no readable field map, and a registry-injected system column (resolved per object), the same skips every position of this rule takes.
22+
23+
**What an author does.** Nothing for a filter whose keys are fields of the child object. For a refused key: name the child field you meant (the verdict suggests the nearest one), or drop the key.

‎packages/lint/src/rule-explanations.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4249,6 +4249,15 @@ const OBJECT_FIELD_REF_UNKNOWN_EXPLANATION: RuleExplanation = {
42494249
'(the picker scopes its candidates by that key). The referenced object is judged only on ' +
42504250
'`lookup`, `master_detail` and `user` fields, the types that render the picker, and a bare ' +
42514251
'`dependsOn` name that misses on the owning object is reported once, there.',
4252+
'A summary field\'s `summaryOperations.filter` is judged against the CHILD object ' +
4253+
'`summaryOperations.object` names: the engine ANDs it with the parent-FK match and runs it as the ' +
4254+
'query on that child, so every key (inside `$and` / `$or` / `$not` groups) is a field of the child. ' +
4255+
'That query refuses what it cannot serve, and each refusal makes every child write answer a roll-up ' +
4256+
'recompute failure (the row is still written) while the summary keeps a stale value. So the rule ' +
4257+
'refuses a key the child lacks; a dotted key such as `account.status` (write the nested one-level ' +
4258+
'form `{ account: { status: VALUE } }` instead); a field computed on read, such as a `formula` ' +
4259+
'(denormalise it onto the child as a stored field); and, beneath a relation, a key the related ' +
4260+
'object lacks, a dotted key or a second relation holding a condition of its own.',
42524261
'`lookupColumns`, `dependsOn` and `indexes[].fields` are read verbatim (a picker column key, a ' +
42534262
'record key, a physical column), so a dotted name there is one name no field carries; the ' +
42544263
'other positions resolve a relationship path. Not judged: an object this stack does not ' +

‎packages/lint/src/validate-object-field-refs.test.ts‎

Lines changed: 126 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,130 @@ describe('validateObjectFieldRefs — lookupFilters[].field (addresses the REFER
532532
});
533533
});
534534

535+
describe('validateObjectFieldRefs — summaryOperations.filter (addresses the CHILD object, #31224)', () => {
536+
// `crm_account` is the parent; `crm_invoice` (with an FK to it) the child.
537+
const rollup = (filter: unknown, over: Record<string, unknown> = {}) => ({
538+
objects: [
539+
account({
540+
fields: {
541+
...account().fields,
542+
billed: { type: 'summary', summaryOperations: { object: 'crm_invoice', field: 'total', function: 'sum', filter, ...over } },
543+
},
544+
}),
545+
invoice({ account: lookup() }),
546+
],
547+
});
548+
549+
it('REFUSES a misspelt key against the child object, at its path, as an error', () => {
550+
const findings = validateObjectFieldRefs(rollup({ regoin: 'emea' }));
551+
expect(findings).toHaveLength(1);
552+
expect(findings[0]).toMatchObject({
553+
severity: 'error',
554+
rule: OBJECT_FIELD_REF_UNKNOWN,
555+
path: 'objects[0].fields.billed.summaryOperations.filter.regoin',
556+
});
557+
expect(findings[0]!.message).toContain('"regoin" is not a field on object "crm_invoice"');
558+
expect(findings[0]!.message).toContain('Did you mean "region"');
559+
expect(findings[0]!.hint).toContain('runs on the child object `summaryOperations.object` names');
560+
});
561+
562+
it('REFUSES a key only the PARENT (the object owning the summary) has', () => {
563+
const findings = validateObjectFieldRefs(rollup({ industry: 'saas' }));
564+
expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.billed.summaryOperations.filter.industry']);
565+
});
566+
567+
it('walks $and / $or / $not groups and reads past operator objects', () => {
568+
const findings = validateObjectFieldRefs(rollup({
569+
$and: [{ total: { $gte: 100 } }, { $or: [{ region: 'emea' }, { stauts: 'paid' }] }],
570+
$not: { isssued_on: { $lt: '2026-01-01' } },
571+
}));
572+
expect(findings.map((f) => f.path)).toEqual([
573+
'objects[0].fields.billed.summaryOperations.filter.$and[1].$or[1].stauts',
574+
'objects[0].fields.billed.summaryOperations.filter.$not.isssued_on',
575+
]);
576+
});
577+
578+
it('passes child fields with operators, and the nested one-level relation form the engine serves', () => {
579+
expect(validateObjectFieldRefs(rollup({
580+
total: { $gt: 0 },
581+
issued_on: { $gte: '2026-01-01' },
582+
region: { $in: ['emea', 'apac'] },
583+
account: { status: 'active', industry: { $in: ['saas'] } },
584+
}))).toEqual([]);
585+
});
586+
587+
it('REFUSES the dotted spelling of a relation path, with the nested spelling as the cure', () => {
588+
const findings = validateObjectFieldRefs(rollup({ 'account.status': 'active' }));
589+
expect(findings).toHaveLength(1);
590+
expect(findings[0]).toMatchObject({
591+
severity: 'error',
592+
rule: OBJECT_FIELD_REF_UNKNOWN,
593+
path: 'objects[0].fields.billed.summaryOperations.filter.account.status',
594+
});
595+
expect(findings[0]!.message).toContain('dotted path through the relation "account"');
596+
expect(findings[0]!.message).toContain('every recompute fails');
597+
expect(findings[0]!.hint).toBe('Nest the condition beneath the relation field: { account: { status: VALUE } }.');
598+
});
599+
600+
it('REFUSES a dotted path into a scalar field, and judges a dotted key whose head is unknown as unknown', () => {
601+
const findings = validateObjectFieldRefs(rollup({ 'region.code': 'x', 'regoin.code': 'y' }));
602+
expect(findings.map((f) => f.path)).toEqual([
603+
'objects[0].fields.billed.summaryOperations.filter.region.code',
604+
'objects[0].fields.billed.summaryOperations.filter.regoin.code',
605+
]);
606+
expect(findings[0]!.message).toContain('a dotted path into "region", a text field');
607+
expect(findings[0]!.hint).toContain('Denormalise the value onto the child object');
608+
expect(findings[1]!.message).toContain('"regoin" is not a field on object "crm_invoice"');
609+
});
610+
611+
it('REFUSES a child field computed on read (formula), at the top level and one level down', () => {
612+
const stack = rollup({ is_big: true, account: { score_band: 'a' } }) as any;
613+
stack.objects[1].fields.is_big = { type: 'formula', expression: 'total > 100' };
614+
stack.objects[0].fields.score_band = { type: 'formula', expression: "'a'" };
615+
const findings = validateObjectFieldRefs(stack);
616+
expect(findings.map((f) => f.path)).toEqual([
617+
'objects[0].fields.billed.summaryOperations.filter.is_big',
618+
'objects[0].fields.billed.summaryOperations.filter.account.score_band',
619+
]);
620+
expect(findings[0]!.message).toContain('a formula field of "crm_invoice", computed on read');
621+
for (const f of findings) expect(f.hint).toContain('Denormalise the value onto the child object');
622+
});
623+
624+
it('REFUSES what the nested form cannot serve: an unknown related key, a dotted one, a second relation level, an empty condition', () => {
625+
const stack = rollup({
626+
$and: [
627+
{ account: { statsu: 'active' } },
628+
{ account: { 'parent.name': 'x' } },
629+
{ account: { parent: { name: 'x' } } },
630+
{ account: {} },
631+
],
632+
}) as any;
633+
stack.objects[0].fields.parent = { type: 'lookup', reference: 'crm_account' };
634+
const findings = validateObjectFieldRefs(stack);
635+
expect(findings.map((f) => f.path)).toEqual([
636+
'objects[0].fields.billed.summaryOperations.filter.$and[0].account.statsu',
637+
'objects[0].fields.billed.summaryOperations.filter.$and[1].account.parent.name',
638+
'objects[0].fields.billed.summaryOperations.filter.$and[2].account.parent',
639+
'objects[0].fields.billed.summaryOperations.filter.$and[3].account',
640+
]);
641+
expect(findings[0]!.message).toContain('"statsu" is not a field on object "crm_account"');
642+
expect(findings[1]!.message).toContain('reaches one level only');
643+
expect(findings[2]!.message).toContain('is itself a relation of "crm_account"');
644+
expect(findings[3]!.message).toContain('names no field of "crm_account"');
645+
});
646+
647+
it('judges only a `summary` field', () => {
648+
const stack = rollup({ regoin: 'emea' }) as any;
649+
stack.objects[0].fields.billed.type = 'number';
650+
expect(validateObjectFieldRefs(stack)).toEqual([]);
651+
});
652+
653+
it('does not judge a filter whose child the stack does not define, or a roll-up with no filter', () => {
654+
expect(validateObjectFieldRefs(rollup({ anything: 1 }, { object: 'elsewhere_line' }))).toEqual([]);
655+
expect(validateObjectFieldRefs(rollup(undefined))).toEqual([]);
656+
});
657+
});
658+
535659
describe('validateObjectFieldRefs — dependsOn (the gate on the OWNER, the filter on the REFERENCE)', () => {
536660
it('REFUSES a misspelt name in the string arm against the owning object — once, not twice', () => {
537661
const findings = validateObjectFieldRefs(twoObjects({
@@ -755,6 +879,7 @@ describe('[#22161] one-line verdicts — object-field-ref-unknown', () => {
755879
'.lookupColumns[',
756880
'.lookupFilters[',
757881
'.dependsOn[',
882+
'.summaryOperations.filter.',
758883
]) {
759884
expect(converted.some((f) => f.path.includes(position)), position).toBe(true);
760885
}
@@ -773,7 +898,7 @@ describe('[#22161] one-line fix — object-field-ref-unknown', () => {
773898
const converted = fired.filter((f) => f.rule === OBJECT_FIELD_REF_UNKNOWN);
774899
// Coverage control: every position fired (the verdict block above names
775900
// them), so the bound reads each prescription.
776-
for (const position of ['.highlightFields[', '.publicSharing.redactFields[', '.indexes[', '.relatedListColumns[', '.lookupColumns[', '.lookupFilters[', '.dependsOn[']) {
901+
for (const position of ['.highlightFields[', '.publicSharing.redactFields[', '.indexes[', '.relatedListColumns[', '.lookupColumns[', '.lookupFilters[', '.dependsOn[', '.summaryOperations.filter.']) {
777902
expect(converted.some((f) => f.path.includes(position)), position).toBe(true);
778903
}
779904
for (const f of converted) {

0 commit comments

Comments
 (0)