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