From de3b2b2354e7cb12a2cf975253cb46e84813c2ed Mon Sep 17 00:00:00 2001 From: Ako Date: Tue, 1 Sep 2026 08:24:09 +0000 Subject: [PATCH] =?UTF-8?q?docs(wiki):=20digest=20mdl/visitor=20=E2=80=94?= =?UTF-8?q?=20two=20new=20pages=20and=20the=20first=20re-sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 27 of the 28 mdl/visitor findings (96%). Two pages are new and one is a re-sync — the first any page has had since the initial synthesis on 2026-05-24. - expression-translation-drift (13). Deliberately distinct from platform-semantics-gaps: there the MDL is illegal Mendix, here the MDL is correct, the emitted Mendix expression is well-formed, and it says something else. The worst instance is the quietest — an additive chain rebuilt with its operators reordered, so a microflow computed a different number than its source said with mxcli check, mx check and the build all green, and the corruption in the stored document rather than in DESCRIBE. Three ways meaning is lost (a literal changes type, an operator changes, a function resolves to the wrong overload), plus the ANTLR trap behind the standard fix: GetText() excludes hidden tokens and a source-interval slice includes them, so preserving raw source inherits every comment in that span. - misleading-diagnostics (6). Graded above "the message could be clearer" because a wrong hint costs however long the reader spends acting on it: they blamed their quoting, renamed an attribute that was fine, or concluded a construct was unsupported. A hint's precision matters more than its coverage — the "unescaped apostrophe" hint matched any short lowercase word and so fired on every genuine error at `on`, `in`, `as`, `to`, `by`. - visitor-wiring-gaps — RE-SYNCED. It described one size of gap (a field); the findings show three. A field, a structure (ELSIF arms, which Mendix has no native form for and which must be lowered into nested ifs), and a whole statement that parses, exits 0 and dispatches to nothing — the last presenting as an empty result rather than a missing feature, which is what lets it survive. Added the neighbouring failure where a field is wired to the WRONG thing, which reports success and changes meaning rather than losing it. Every sources: path and every [[wiki-link]] across all 25 pages verified to resolve. Co-Authored-By: Claude Opus 5 --- .claude/skills/maintain-wiki.md | 2 + CLAUDE.md | 2 +- docs-wiki/SYNC_LOG.md | 4 +- .../expression-translation-drift.md | 81 +++++++++++++++++++ .../bug-patterns/misleading-diagnostics.md | 66 +++++++++++++++ docs-wiki/bug-patterns/visitor-wiring-gaps.md | 61 +++++++++++--- 6 files changed, 204 insertions(+), 12 deletions(-) create mode 100644 docs-wiki/bug-patterns/expression-translation-drift.md create mode 100644 docs-wiki/bug-patterns/misleading-diagnostics.md diff --git a/.claude/skills/maintain-wiki.md b/.claude/skills/maintain-wiki.md index 7d3d98f8f..02f758ce8 100644 --- a/.claude/skills/maintain-wiki.md +++ b/.claude/skills/maintain-wiki.md @@ -163,6 +163,8 @@ isn't better served by an existing page or a different doc artifact. | `bug-patterns/capability-gap-as-parse-error.md` | bug-pattern | an unspelled capability presenting as `no viable alternative`, and the workarounds that follow | | `bug-patterns/keyword-collisions.md` | bug-pattern | MDL's keyword set occupying positions where user data lives | | `bug-patterns/scripts-that-cannot-rerun.md` | bug-pattern | statement-level idempotence, and why it is not write-level idempotence | +| `bug-patterns/expression-translation-drift.md` | bug-pattern | MDL expression to Mendix expression, where the translation changes the meaning | +| `bug-patterns/misleading-diagnostics.md` | bug-pattern | hints that fire on the wrong thing and send the reader somewhere the problem is not | ## Adding a new page diff --git a/CLAUDE.md b/CLAUDE.md index 4af0738f2..737e678fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -564,7 +564,7 @@ mxcli uses a layered documentation system — each artifact type has a single ca **ADRs are immutable once accepted.** Supersede with a new ADR rather than editing in place. Conventions and template in [`docs/13-decisions/README.md`](docs/13-decisions/README.md). -**Bug findings are read in the opposite order from how they are written.** A fix *appends* one record to `.claude/skills/fix-issue/findings/.jsonl`; a diagnosis *starts* at `docs-wiki/bug-patterns/`, which digests those records into failure classes, and drills into the findings only for the specific instance. The findings are append-only evidence — grep them, or query them with DuckDB (`select … from 'findings/*.jsonl'`), never read them whole. The digest covers `mdl/executor` (83% of its findings), `cmd/mxcli` (92%), `mdl/backend` (99%) and `mdl/grammar` (92%) across 23 pattern pages; `make digest-status` reports the gap per area, so a pattern miss means "not yet digested", not "not seen before". +**Bug findings are read in the opposite order from how they are written.** A fix *appends* one record to `.claude/skills/fix-issue/findings/.jsonl`; a diagnosis *starts* at `docs-wiki/bug-patterns/`, which digests those records into failure classes, and drills into the findings only for the specific instance. The findings are append-only evidence — grep them, or query them with DuckDB (`select … from 'findings/*.jsonl'`), never read them whole. The digest covers `mdl/executor` (83% of its findings), `cmd/mxcli` (92%), `mdl/backend` (99%), `mdl/grammar` (92%) and `mdl/visitor` (96%) across 25 pattern pages; `make digest-status` reports the gap per area, so a pattern miss means "not yet digested", not "not seen before". **The wiki is synthesized, not stated.** It frames and connects across the other artifacts — it never restates content that has a canonical home. Rules and seed page list in [`.claude/skills/maintain-wiki.md`](.claude/skills/maintain-wiki.md). diff --git a/docs-wiki/SYNC_LOG.md b/docs-wiki/SYNC_LOG.md index 5eea4be60..74275c034 100644 --- a/docs-wiki/SYNC_LOG.md +++ b/docs-wiki/SYNC_LOG.md @@ -52,4 +52,6 @@ not capture, because sources are upstream of the commit. | 2026-08-31 | bug-patterns/capability-gap-as-parse-error.md | .claude/skills/fix-issue/findings/mdl-grammar.jsonl (20 records), mdl/grammar/MDLParser.g4, docs/13-decisions/0003-mdl-is-sql-shaped.md | New page (added to seed table). First pass over mdl/grammar. A parse error is indistinguishable from a mistake the user made, so the reports in this class are workarounds rather than feature requests; the gap is bidirectional (DESCRIBE must do something too, and rebuilding a DIFFERENT document is worse than a parse error); a narrow/wide statement pair is a whitelist | | 2026-08-31 | bug-patterns/keyword-collisions.md | .claude/skills/fix-issue/findings/mdl-grammar.jsonl (8 records), mdl/grammar/MDLLexer.g4, mdl/executor/identifier_quoting.go | New page (added to seed table). The distinguishing question is how a collision fails, not whether one happens: a parse error is recoverable, a different valid parse is not. Records that a grammar alternative and its visitor case are one change, and the control-binary sweep for proving a relaxation | | 2026-08-31 | bug-patterns/scripts-that-cannot-rerun.md | .claude/skills/fix-issue/findings/mdl-grammar.jsonl (5 records), mdl/grammar/domains/MDLDomainModel.g4, docs/13-decisions/0003-mdl-is-sql-shaped.md | New page (added to seed table). Statement-level idempotence, kept explicitly apart from ADR-0008 write-level idempotence, which is what people usually mean when they say mxcli is idempotent. The silent variant (a duplicated index) is worse than the error | - +| 2026-08-31 | bug-patterns/expression-translation-drift.md | .claude/skills/fix-issue/findings/mdl-visitor.jsonl (13 records), mdl/visitor/visitor_microflow_expression.go, mdl/visitor/visitor_helpers.go | New page (added to seed table). First pass over mdl/visitor. Distinct from platform-semantics-gaps: there the MDL is illegal Mendix, here the MDL is correct and the TRANSLATION says something else — the worst case a microflow computing a different number with every check green. Records the ANTLR hidden-token trap and why the same expression text means different things in different slots | +| 2026-08-31 | bug-patterns/misleading-diagnostics.md | .claude/skills/fix-issue/findings/mdl-visitor.jsonl (6 records), mdl/visitor/visitor.go | New page (added to seed table). Graded above "the message could be clearer": a wrong hint costs however long the reader spends acting on it, and in the reported cases they blamed their quoting, renamed an attribute that was fine, or concluded a construct was unsupported. A hint's precision matters more than its coverage | +| 2026-08-31 | bug-patterns/visitor-wiring-gaps.md | .claude/skills/fix-issue/findings/mdl-visitor.jsonl, mdl/visitor/visitor_enumeration.go, mdl/visitor/visitor_helpers.go | **Re-sync** (first since the 2026-05-24 initial synthesis). Broadened from one size of gap to three — a field, a structure (ELSIF arms lowered into nested ifs), and a whole statement that parses and dispatches to nothing — and added the neighbouring failure where a field is wired to the WRONG thing, which reports success and changes meaning. sources: updated from the findings directory to the specific shard; wiki-links normalised to bare slugs | diff --git a/docs-wiki/bug-patterns/expression-translation-drift.md b/docs-wiki/bug-patterns/expression-translation-drift.md new file mode 100644 index 000000000..d0572cb46 --- /dev/null +++ b/docs-wiki/bug-patterns/expression-translation-drift.md @@ -0,0 +1,81 @@ +--- +title: The Expression Translation Loses Meaning +category: bug-pattern +last-synced: ced830e0 +sources: + - .claude/skills/fix-issue/findings/mdl-visitor.jsonl + - mdl/visitor/visitor_microflow_expression.go + - mdl/visitor/visitor_helpers.go +--- + +> **Do not duplicate**: the Mendix expression rules themselves are +> [[platform-semantics-gaps]] and the skills; each translation's fix is in the +> findings. This page is about the step in between. + +## What this is + +An MDL expression is not a Mendix expression. The visitor parses one and emits +the other, and thirteen `mdl/visitor` findings are that translation changing +what the expression *means*. This is a different class from writing something +Mendix forbids: here the MDL is correct, the Mendix expression is well-formed, +and it says something else. + +The worst of them is the quietest. An additive chain came back from the model +with its operators reordered, so **a microflow computed a different number than +its source said** — with `mxcli check`, `mx check` and the build all green, and +the corruption in the stored document rather than in the description. + +## How it fits + +**Three ways meaning is lost.** + +*A literal changes type.* A decimal literal losing its fraction; an enumeration +value stored as a quoted string instead of a qualified name; a bare identifier +that needed a `$currentObject/` prefix and did not get one. Each produces an +expression Mendix parses and evaluates differently, or rejects with a generic +`CE0117` that names nothing. + +*An operator changes.* `/` is division in most languages and member navigation in +Mendix, so the same characters mean two things depending on what is either side +of them. An additive chain rebuilt without walking its children in order swaps +operands. + +*A function resolves to the wrong overload.* `contains` and `find` exist as both +string functions and list operations, and the visitor has to decide from the +argument shapes which one was meant — a string literal in the second position +means the string function, two bare variables are genuinely ambiguous at parse +time and get decided later from the declared type. + +**Preserving raw source is the standard fix, and it carries hidden tokens with +it.** Where the rebuilt expression is lossy, the visitor keeps the original text. +The trap is that ANTLR's `GetText()` excludes hidden tokens while a source-interval +slice *includes* them — so any code reaching for original text inherits every +comment in that span, and a `--` comment between two operands ends up inside the +Mendix expression. Replacing a comment with whitespace rather than with nothing +matters for the same reason: `1 --c\n+ 2` must not become `1+ 2`. + +**Quoting is the other systematic leak.** The guidance to quote identifiers is +about the MDL *parser*; a quote that survives into the stored expression produces +`Mod."Entity".Attr`, which Mendix rejects. The tell is a **half-stripped** name in +the error message — one half went through the unquoting reader and the other did +not, which localises the bug to a `GetText()` call that should have been the +structured accessor. + +**The same expression text means different things in different slots.** A widget +`Visible:` is a client expression and needs bare identifiers prefixed; a +datasource `where` is real XPath and must not be. An enum in the first stringifies +to a qualified name and in the second to a quoted string. A fix applied to +"expressions" rather than to a specific slot breaks the other one. + +**Nothing downstream can catch this.** The output is a syntactically valid Mendix +expression, so `mx check` is satisfied, the build succeeds, and the only +observable difference is a value at runtime. Verification has to be the round +trip plus reading the stored bytes — `strings` on the `.mxunit` is what showed the +swapped operands. + +## See also + +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the individual + literals, operators and overloads +- [[platform-semantics-gaps]] — expressions Mendix forbids outright +- [[keyword-collisions]] — where the quoting leak starts diff --git a/docs-wiki/bug-patterns/misleading-diagnostics.md b/docs-wiki/bug-patterns/misleading-diagnostics.md new file mode 100644 index 000000000..b03954989 --- /dev/null +++ b/docs-wiki/bug-patterns/misleading-diagnostics.md @@ -0,0 +1,66 @@ +--- +title: A Wrong Hint Is Worse Than No Hint +category: bug-pattern +last-synced: ced830e0 +sources: + - .claude/skills/fix-issue/findings/mdl-visitor.jsonl + - mdl/visitor/visitor.go +--- + +> **Do not duplicate**: the individual hints and their trigger conditions live in +> the findings and in `mdl/visitor/`. This page is about why the class is graded +> higher than "the message could be clearer". + +## What this is + +ANTLR's raw errors name tokens, not intentions: `no viable alternative at input +'add'` for a missing keyword, `mismatched input '=' expecting ')'` for a +caption written with the wrong separator. mxcli adds hints on top. Six +`mdl/visitor` findings are those hints firing on the wrong thing and **sending +the reader somewhere the problem is not**. + +A missing hint costs the reader a minute of confusion. A wrong one costs however +long they spend acting on it — and in the reported cases they blamed their +quoting, renamed an attribute that was fine, or concluded a construct was +unsupported. + +## How it fits + +**A pattern broad enough to be useful is broad enough to be wrong.** The +"unescaped apostrophe" hint matched any short lowercase word, so every genuine +parse error on `on`, `in`, `as`, `to` or `by` was diagnosed as a quoting mistake. +Narrowing it to the actual contraction suffixes — `s`, `t`, `d`, `m`, `re`, `ve`, +`ll` — keeps the real cases and drops the false ones. A hint's precision matters +more than its coverage, because its whole value is that the reader trusts it. + +**Key the hint off the source line, not the parser message.** The token error for +a misplaced `index` clause lands on the index *name*, so the message never +mentions `index` at all and no message-matching hint can fire. Reading the +offending line is what makes the diagnosis possible — and it also lets the hint +discriminate against the valid form of the same construct. + +**A hint that recommends a rename is a strong claim.** One offered three +alternative names for an attribute that parsed perfectly well elsewhere, because +it conflated two kinds of reserved word: an MDL *parser* keyword, which quoting +escapes and the model keeps, and a name the *platform* reserves, which quoting +does not help with at all. Conflating them is expensive in both directions — +advising a quote where the platform reserves the name produces something that +parses and then fails the build. + +**Some errors are the platform's, and the hint should say so.** Mendix XPath +cannot compute values, so `[Seq = $Game/MoveSeq + 1]` is a limitation rather than +a syntax mistake; a bare `mismatched input '+'` reads as the latter. Naming the +constraint turns a dead end into a redesign. + +**The reachable-form question.** Several of these are not really about wording: +the reader's next question is "then how *do* I write it?", and a hint that names +the working spelling answers it. Where there is no working spelling, saying so is +still better than a token error — that is the boundary with +[[capability-gap-as-parse-error]]. + +## See also + +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the hints, + their triggers and their controls +- [[capability-gap-as-parse-error]] — when the honest hint is "you cannot" +- [[keyword-collisions]] — the two kinds of reserved word, kept apart diff --git a/docs-wiki/bug-patterns/visitor-wiring-gaps.md b/docs-wiki/bug-patterns/visitor-wiring-gaps.md index 35baee242..3b65149ac 100644 --- a/docs-wiki/bug-patterns/visitor-wiring-gaps.md +++ b/docs-wiki/bug-patterns/visitor-wiring-gaps.md @@ -1,28 +1,69 @@ --- title: Visitor Wiring Gaps category: bug-pattern -last-synced: 4e185f73 +last-synced: ced830e0 sources: - - .claude/skills/fix-issue/findings/ + - .claude/skills/fix-issue/findings/mdl-visitor.jsonl - mdl/visitor/visitor_enumeration.go + - mdl/visitor/visitor_helpers.go --- -> **Do not duplicate**: the specific fix recipe (issue #393) lives in the `.claude/skills/fix-issue/findings/` symptom row, and the canonical wiring blocks live in `mdl/visitor/`. This page describes the pattern only. +> **Do not duplicate**: the per-instance fix recipes and the exact blocks to copy +> live in the findings; the canonical wiring lives in `mdl/visitor/`. This page +> describes the pattern only. ## What this is -A family of "parsed-but-not-stored" bugs: a property the user set in a `CREATE` statement silently vanishes after a roundtrip. The grammar accepts it, the AST struct has a field for it, the model and writer both carry it, and `DESCRIBE` knows how to print it — yet it never appears, because the value was never copied out of the parse tree into the AST. +A family of "parsed-but-not-stored" bugs. The grammar accepts the input, the AST +struct has a field for it, the model and writer both carry it, and `DESCRIBE` +knows how to print it — and it never appears, because the value was never copied +out of the parse tree into the AST. + +The visitor is the one **hand-written** bridge in the grammar → visitor → AST → +executor → backend pipeline, and it is per-statement boilerplate with no compiler +check that every field was copied. That is the whole mechanism. ## How it fits -mxcli's pipeline runs grammar → visitor → AST → executor → backend, and the visitor is the one hand-written bridge in that chain. Each `ExitCreateXxxStatement` must explicitly read fields off the ANTLR parse-tree context and assign them onto the AST struct. When a new `CREATE` variant is added, it is easy to wire every other layer and forget one assignment in the visitor — so the field round-trips through nothing. +**The gap comes in three sizes, and they present very differently.** + +*A field.* The original instance: an enum-level doc comment vanishing after a +round trip, a `CREATE OR REPLACE` flag silently running as a plain `CREATE`. Every +layer below the visitor is correct, and the loss is one document type wide. The +canonical fix is to diff the broken visitor against a known-good sibling — the +enumeration and constant visitors share the same two standard blocks verbatim. + +*A structure.* An `ELSIF` chain whose middle arms were dropped on write. Mendix +has no native `elsif`, so each arm has to be lowered into a nested `if` in the +previous arm's `else` branch; a visitor that reads only the first and last arms +produces a valid microflow that is missing behaviour. The read path was never +wrong — a round trip showed `if … else …` because the arms were absent from the +model, which is the diagnostic that separates a write gap from a read gap. + +*A whole statement.* A `DROP` or a `SHOW` that parses, exits 0, prints nothing +and does nothing, because the statement reached the AST and no dispatch case. The +symptom reads as an empty result — "this page has no content", "there are no +connections" — rather than as a missing feature, which is what makes it survive. +Anything that can end in an empty output needs to distinguish *nothing found* +from *nothing ran*. -The tell-tale: every layer below the visitor is correct (the field exists in the AST, the writer serializes it, `DESCRIBE` prints it for other types) but a doc-comment or `OR MODIFY`/`OR REPLACE` flag disappears for one specific document type. The class recurs because visitor methods are per-statement boilerplate with no compiler check that all fields were copied. +**The neighbouring failure is a field wired to the wrong thing**, which is worse +than one not wired at all: a delete-behaviour keyword storing a different +behaviour, a dollar-quoted SQL body overwritten by a parameter's default. Those +report success and change the model's meaning, where an unwired field merely +loses it. -The canonical fix is to diff the broken visitor against a known-good sibling. The enumeration and constant visitors in [`mdl/visitor/`](../../mdl/visitor/visitor_enumeration.go) share the same two standard blocks verbatim — `stmt.Documentation = findDocCommentText(ctx)` and the `OR MODIFY`/`OR REPLACE` flag detection. The exact blocks to copy are in the symptom table. +**Guard the class, not the instance.** These recur because the boilerplate is +per-statement. The remedies that have held are structural: route every qualified +name through one accessor rather than `GetText()`, and assert reachability — a +test that every exported validator is called from the one entry point catches a +whole statement that was never wired, which no per-feature test can. ## See also -- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the per-instance fix recipes for this pattern -- [[architecture/mdl-execution]] — the grammar → visitor → AST → executor pipeline this gap sits in -- [[rationale/mdl-as-sql]] — why MDL statements roundtrip through `DESCRIBE` +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the per-instance + recipes and the blocks to copy +- [[mdl-execution]] — the pipeline this gap sits in +- [[duplicate-resolver-drift]] — the sibling shape one layer down +- [[expression-translation-drift]] — where the visitor changes meaning rather than + dropping it