fix(check): report MDL042 for @caption on a while loop (#1187) - #1188
Conversation
A while loop builds the same Microflows$LoopedActivity as a for-each loop, which has no Caption property, so @caption on it was dropped by exec. MDL042 was raised only in the LoopStmt case; it now lives in checkCaptionOnLoop and is called for WhileStmt too, pointing to @annotation instead.
AI Code ReviewCritical IssuesNone found. Moderate IssuesNone found. Minor IssuesNone found. What Looks Good
RecommendationApprove the PR. The change is focused, well-tested, and aligns with project conventions. No further action is needed. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
ako
left a comment
There was a problem hiding this comment.
Reviewed against the checklist by re-running the claims. No blockers — the fix is correct, minimal, and complete for its class. Two moderate items and two minor ones below.
Verified independently
- The premise holds.
generated/metamodel.MicroflowsLoopedActivity— the arbiter per CLAUDE.md — declares noCaption. A caption on a loop genuinely has nowhere to go. - The scope is complete: there is no third case. I checked every caption-bearing type in the semantic model against the metamodel.
ActionActivity,ExclusiveSplit,InheritanceSplitandAnnotationall have a metamodelCaption;LoopedActivityis the only one that does not.loopandwhileare the only statements that build it, so the two cases here are the whole class — worth stating explicitly, because "we fixed the second instance" and "the class is closed" are different claims and only the second one retires the bug. - The control works. Deleting the
checkCaptionOnLoop(stmt.Annotations, "a while loop")line takesTestValidateMicroflow_CaptionOnWhileWarnsto FAIL; restored, it passes. - MDL042 is right on both backends — the MCP payload for a
LoopedActivity(mdl/backend/mcp/microflow.go:477) sends no caption either, so the unconditional warning is not wrong on the PED path. - No stale claims elsewhere — every other
@captionexample across the skills, docs-site andsyntaxtopics sits on anActionActivity, which genuinely carries one.control-flow.mdwas the only doc that needed the update. make test: exit 0, zero failures. Finding recorded, CHANGELOG entry present.
Moderate 1 — dead code that contradicts the fix
mdl/executor/cmd_microflows_builder_annotations.go:118 still runs:
case *microflows.LoopedActivity:
// LOOP / WHILE activities can carry a caption just like
// splits and action activities.
if ann.Caption != "" { activity.Caption = ann.Caption }That comment says the opposite of what MDL042 now tells the user, two files away. Nothing reads it back: the gen writer (microflow_write.go:380) sets no caption on a LoopedActivity, and the read path cannot populate one. cmd_microflows_show_helpers.go:800 likewise emits @caption for a loop, unreachable from any stored document.
Why it is worth fixing in this PR rather than later: the next contributor reads the builder, concludes the new warning is wrong, and deletes MDL042 — reopening #1187 from the other side. The assignment is the more convincing artefact, because it looks like working code while the warning looks like an opinion.
Fix: delete the case *microflows.LoopedActivity: branch (and the describe branch at show_helpers.go:800), or replace the comment with one stating that the caption is dropped at the storage boundary and MDL042 is the diagnostic. sdk/microflows.LoopedActivity.Caption can stay or go — the metamodel does not declare it, so it cannot survive a write either way.
Moderate 2 — no mdl-examples/bug-tests/ script
Checklist item for every bug fix, so the regression can be confirmed in Studio Pro. The issue already contains the exact reproduction.
Fix: add mdl-examples/bug-tests/microflow-1187-while-caption.mdl with the reporter's CountMonths snippet. Note the naming convention in that directory — a topic prefix plus the qualified number, since three numbering namespaces collide there.
Minor
-
The
CONFLICTINGstatus is an artefact, the same one as #1165. Both sides appended to.claude/skills/fix-issue/findings/mdl-executor.jsonl. Measured with a plain 3-way merge of each candidate file: that file exits 1,CHANGELOG.mdexits 0..gitattributes:70setsmerge=unionfor exactly this case and GitHub's check does not apply that driver, so a localgit merge origin/mainand push should clear it with no content decision to make. -
The recorded finding's own insight is not implemented. It says to key the check "on what the builder writes (here: every
LoopedActivity) rather than on the MDL keyword that led there" — the code still keys on two AST cases calling a shared helper. Harmless today, since I confirmed no third statement builds aLoopedActivity; but the finding and the code now say different things, and the finding is the artefact the next person greps.
…adicts Review follow-up on mendixlabs#1188. MDL042 now tells the author a loop's @caption is dropped because the activity has no Caption property -- while two files away the builder still ran case *microflows.LoopedActivity: // LOOP / WHILE activities can carry a caption just like splits activity.Caption = ann.Caption and the describer still emitted @caption for one. Nothing read either back: generated/metamodel -- the arbiter -- declares no Caption on Microflows$LoopedActivity, microflow_write.go sets none on the gen object, and the reader therefore cannot populate one. The next contributor reads the builder, concludes the new warning is wrong and deletes MDL042, reopening mendixlabs#1187 from the other side. Measured before deleting anything, with the UNMODIFIED branch build against a Mendix 11.6.6 project: `@caption` on a loop and on a while are both absent from `describe microflow` after `exec`, while `@annotation` on either round-trips. So the value died at the gen boundary and the assignment only ever populated an in-memory field. That is also why the code survived: three tests asserted it. They tested the semantic object, never storage, so they passed throughout and would have failed on the correct fix. Inverted rather than deleted, each naming the measurement: TestLoopCaptionPreserved -> TestLoopCaptionNotStorable TestWhileLoopCaptionPreserved -> TestWhileLoopCaptionNotStorable TestEmitObjectAnnotations_LoopCaption -> ..._LoopCaptionNotEmitted The describe test's real value was mdlQuote escaping, which TestMdlQuote_* already covers directly, so retargeting loses nothing. Reinstating the assignment fails the two builder tests -- checked. Also adds the bug-test script the checklist asks for, covering both captions (MDL042 x2) and both @annotation forms (silent), and keeps sdk/microflows LoopedActivity.Caption with a comment saying it cannot be stored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AI Code ReviewCritical IssuesNone found. Moderate IssuesNone found. Minor Issues
What Looks Good
RecommendationApprove the PR. It resolves the reported bug (MDL042 missing for Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
Closes #1187
What
@captionabove awhileloop passedmxcli checkwith no warning,execcreated the microflow, anddescribeshowed the loop without the caption. The same caption on aloophas been reported as MDL042 for some time.Both statements build a
Microflows$LoopedActivity. Awhilejust carries aWhileLoopConditionwhere a for-each loop has an iterator.LoopedActivityhas no Caption property, so the caption is dropped either way. MDL042 was raised only in the*ast.LoopStmtcase ofvalidate_microflow.go, so awhilefell through.The check now lives in one helper,
checkCaptionOnLoop, which both cases call. Its message names the statement ("a loop" / "a while loop") and points to@annotation, which survivesexecand comes back fromdescribe. There are no syntax changes and no builder changes, and nothing new is written to the model.Reproduced on v0.24.0 against a blank Mendix 11.12.1 project:
@captiononloop@captiononwhileexec@annotationonwhileChanges
mdl/executor/validate_microflow.go: the sharedcheckCaptionOnLoop, called forLoopStmtandWhileStmt.mdl/executor/validate_microflow_loop_caption_test.go: two new tests,TestValidateMicroflow_CaptionOnWhileWarnsandTestValidateMicroflow_AnnotationOnWhileNoWarn.write-microflows/reference/control-flow.md: the MDL042 note now coverswhileas well asloop.CHANGELOG.md([Unreleased]/ Fixed), plus one line in.claude/skills/fix-issue/findings/mdl-executor.jsonl.Tests
WhileStmtcall and confirmed thatTestValidateMicroflow_CaptionOnWhileWarnsfails, then put it back and confirmed it passes.looptests pass unchanged.push-test.ymllocally on macOS: build,make test, lint-go, check-mdl, check-findings, check-wiki-pages, tunnel deps, the skill and docs-site MDL blocks, integration tests againstmx11.12.1, and govulncheck. All green.