Skip to content

Aggregates broke when SET became optional (upstream CI regression) - #112

Merged
ako merged 1 commit into
mainfrom
claude/mxbuild-diagnostics-spike-emta6h
Aug 7, 2026
Merged

Aggregates broke when SET became optional (upstream CI regression)#112
ako merged 1 commit into
mainfrom
claude/mxbuild-diagnostics-spike-emta6h

Conversation

@ako

@ako ako commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Fixes the TestMxCheck_DoctypeScripts/02-microflow-examples.mdl failure on the upstream sync PR — six errors on both engines, from a regression in e0744b9e ("make SET optional"):

[error] [CE0109] "Undefined variable 'ProductList.Price'." at Aggregate list activity 'Take sum ProductList.Price'
[error] [CE0015] "Aggregate function must specify a valid attribute." at Aggregate list activity 'Take sum ProductList'

What broke

Making SET optional turned setStatement into $X = <expr>, which overlaps every VARIABLE EQUALS <function-call> statement in the grammar. It sat at alternative 5 of ~50, and ANTLR's ALL(*) picks the lowest-numbered alternative that matches — so $Sum = sum($List.Price) stopped reaching aggregateListStatement and fell into the generic SET conversion, which joined the list and the attribute into one name and dropped the per-item expression.

Two fixes, because there were two defects

  1. setStatement moved last in microflowStatement, so it only claims what no dedicated rule parses. Restores the previous routing for aggregates, list operations and RANGE alike.
  2. The SET conversion now agrees with the canonical one (buildSetAggregate), and buildListAggregateAsFunction appends the per-item expression argument it never appended. This half was wrong before the bare form ever reached it — set $X = sum($List.Price) produced the same broken aggregate on main today.

Verification

Mendix 11.12.1 locally (CI used 11.12.2).

Check Result
TestMxCheck_DoctypeScripts — all scripts, both engines, real mx check ok, 629s
go test ./mdl/... ./cmd/... pass
Control sweep, all mdl-examples/**/*.mdl, new vs main binary no differences
New bug-test script → blank project → mx check 0 errors (control binary: 8 errors, the reported CE0109s)
Both fixes reverted, new tests re-run fail with the reported symptom

Note on why this shipped

The original SET? change was swept with a control binary over every example — but with mxcli check, which parses and validates and never serializes. This defect lives between the AST and the BSON, where only exec + mx check can see it. Recorded in the symptom table: for a grammar change, the control sweep has to run the integration gate, not check.

Files

  • mdl/grammar/domains/MDLMicroflow.g4setStatement last, with a comment saying why
  • mdl/visitor/visitor_microflow_statements.gobuildSetAggregate replaces extractVariableAndAttribute
  • mdl/visitor/visitor_microflow_expression.gobuildListAggregateAsFunction appends the expression argument
  • mdl/visitor/visitor_microflow_aggregate_test.go — new
  • mdl-examples/bug-tests/aggregate-after-optional-set.mdl — new
  • .claude/skills/fix-issue.md — symptom row

🤖 Generated with Claude Code

https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4


Generated by Claude Code

Making SET optional put `$X = <expr>` in front of every
`VARIABLE EQUALS <function-call>` statement in the grammar. ANTLR picks the
lowest-numbered alternative that matches, so `$Sum = sum($List.Price)` stopped
reaching aggregateListStatement and fell through to the generic SET conversion,
which joined the list and the attribute into one name. mxbuild rejected the
result on both engines:

  [CE0109] "Undefined variable 'ProductList.Price'."
  [CE0015] "Aggregate function must specify a valid attribute."

Move setStatement last so it only claims what no dedicated rule parses, and make
the SET conversion agree with the canonical one — it also dropped the per-item
expression of `sum($List, $currentObject/Price * 0.21)`, which
buildListAggregateAsFunction never appended in the first place. That half was
wrong before the bare form ever reached it, so `set $X = sum($List.Price)` was
broken too.

Proved by reverting both halves: the new tests fail with the reported symptom,
and the same script executed against a blank 11.12.1 project reports 8 errors on
a control binary and 0 with the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
@ako
ako merged commit cbe06d5 into main Aug 7, 2026
3 checks passed
ako pushed a commit that referenced this pull request Aug 8, 2026
Brings the branch up to date with main (PR #112) so the PR merges cleanly and
CI runs against the current base. No conflicts; the fix-issue.md symptom table
merged via the union driver with both sides' rows intact and no duplicates.
Full suite green on the merged tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants