You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Run a structured review of the current branch's changes against the CLAUDE.md
4
+
checklist, then check the recurring findings table below for patterns that have
5
+
burned us before.
6
+
7
+
## Steps
8
+
9
+
1. Run `gh pr view` and `gh pr diff` (or `git diff main...HEAD`) to read the change.
10
+
2. Work through the CLAUDE.md "PR / Commit Review Checklist" in full.
11
+
3. Then check every row in the Recurring Findings table below — flag any match.
12
+
4. Report: blockers first, then moderate issues, then minor. Include a concrete fix
13
+
option for every blocker (not just "this is wrong").
14
+
5. After the review: **add a row** to the Recurring Findings table for any new
15
+
pattern not already covered.
16
+
17
+
---
18
+
19
+
## Recurring Findings
20
+
21
+
Patterns caught in real reviews. Each row is a class of mistake worth checking
22
+
proactively. Add a row after every review that surfaces something new.
23
+
24
+
| # | Finding | Category | Canonical fix |
25
+
|---|---------|----------|---------------|
26
+
| 1 | Formatter emits a keyword not present in `MDLParser.g4` → DESCRIBE output won't re-parse (e.g. `RANGE(...)`) | DESCRIBE roundtrip | Grep grammar before assuming a keyword is valid; if construct can't be expressed yet, emit `-- TypeName(field=value) — not yet expressible in MDL`|
27
+
| 2 | Output uses `$currentObject/Attr` prefix — non-idiomatic; Studio Pro uses bare attribute names | Idiomatic output | Verify against a real Studio Pro BSON sample before choosing a prefix convention |
28
+
| 3 | Malformed BSON field (missing key, wrong type) produces silent garbage output (e.g. `RANGE($x, , )`) | Error handling | Default missing numeric fields to `"0"`; or emit `-- malformed <TypeName>` rather than broken MDL |
29
+
| 4 | No DESCRIBE roundtrip test — grammar gap went undetected until human review | Test coverage | Add roundtrip test: format struct → MDL string → parse → confirm no error |
30
+
| 5 | Hardcoded personal path in committed file (e.g. `/c/Users/Ylber.Sadiku/...`) | Docs quality | Use bare commands (`go test ./...`) without absolute paths in any committed doc or skill |
31
+
| 6 | Docs-only PR cites an unmerged PR as a "model example" — cited PR had blockers | Docs quality | Only cite merged, verified PRs; or annotate with known gaps if citing in-flight work |
32
+
| 7 | Skill/doc table references a function that doesn't exist (e.g. `formatActionStatement()` vs `formatAction()`) | Docs quality | Grep function names before writing: `grep -r "func formatA" mdl/executor/`|
33
+
| 8 | "Always X" rule is too absolute for trivial edge cases (e.g. "always write failing test first" for one-char typos) | Docs quality | Soften to "prefer X" or add an exception clause; include the reasoning so readers can judge edge cases |
34
+
| 9 | Doc comment promises a fallback/feature that doesn't exist in the code (e.g., "raw-map fallback in the client" when no such fallback was implemented) | Docs quality | Grep for function/type names referenced in doc comments to confirm they exist before committing |
35
+
36
+
---
37
+
38
+
## After Every Review
39
+
40
+
-[ ] All blockers have a concrete fix option stated.
41
+
-[ ] Recurring Findings table updated with any new pattern.
42
+
-[ ] If docs-only PR: every function name, path, and PR reference verified against
@@ -186,15 +186,15 @@ Sequence contains no matching element
186
186
187
187
### Finding the Offending mxunit File
188
188
189
-
Write a Go tool (or use the pattern below) to compare property keys of mxcli-written files against Studio Pro-native files of the same `$Type`:
189
+
Write a Go tool (or use the pattern below) to compare property keys of mxcli-written files against Studio Pro-native files of the same `$type`:
190
190
191
191
```go
192
-
// Walk mprcontents/, group files by $Type, compare key sets
192
+
// Walk mprcontents/, group files by $type, compare key sets
193
193
// Files with EXTRA keys (vs native files of same type) = the crash cause
194
194
// Files with MISSING keys = also crash cause for mx diff
195
195
```
196
196
197
-
The principle: for each `$Type`, ALL instances must have the **exact same set of non-$ property names**. Any deviation → crash.
197
+
The principle: for each `$type`, ALL instances must have the **exact same set of non-$ property names**. Any deviation → crash.
198
198
199
199
### Version-Specific Properties
200
200
@@ -229,7 +229,7 @@ Some properties only exist in certain Mendix versions. Before adding a property
229
229
230
230
**Symptom**: Studio Pro crashes when opening a project with `System.InvalidOperationException: Sequence contains no matching element` at `Mendix.Modeler.Storage.Mpr.MprProperty..ctor`.
231
231
232
-
**Root cause**: A BSON document contains a property (field name) that does not exist in the Mendix type definition for its `$Type`. Studio Pro's `MprProperty` constructor uses `First()` to look up each BSON field in the type cache, and crashes on unrecognized fields.
232
+
**Root cause**: A BSON document contains a property (field name) that does not exist in the Mendix type definition for its `$type`. Studio Pro's `MprProperty` constructor uses `First()` to look up each BSON field in the type cache, and crashes on unrecognized fields.
@@ -272,7 +272,7 @@ for t, props in crash_props.items():
272
272
273
273
3.**Extra properties = the crash cause**. The fix is to remove those fields from the writer function.
274
274
275
-
**Example**: `DomainModels$CrossAssociation` had `ParentConnection` and `ChildConnection` copied from `DomainModels$Association`, but these fields don't exist on `CrossAssociation`. Removing them fixed the crash.
275
+
**Example**: `DomainModels$CrossAssociation` had `ParentConnection` and `ChildConnection` copied from `DomainModels$association`, but these fields don't exist on `CrossAssociation`. Removing them fixed the crash.
276
276
277
277
**Key principle**: When copying serialization code between similar types (e.g., Association → CrossAssociation), always verify which fields belong to each type by checking a baseline project's BSON.
278
278
@@ -282,7 +282,7 @@ for t, props in crash_props.items():
282
282
283
283
**Root cause**: Two variants:
284
284
285
-
1.**Association stored as Attribute**: In `ChangeActionItem` BSON, an association name was written to the `Attribute` field instead of the `Association` field. Check the executor code that builds `MemberChange` — it must query the domain model to distinguish associations from attributes.
285
+
1.**Association stored as Attribute**: In `ChangeActionItem` BSON, an association name was written to the `attribute` field instead of the `association` field. Check the executor code that builds `MemberChange` — it must query the domain model to distinguish associations from attributes.
286
286
287
287
2.**Entity treated as Enumeration**: In `CreateVariableAction` BSON, an entity qualified name was used as `DataTypes$EnumerationType` instead of `DataTypes$ObjectType`. Check `buildDataType()` in the visitor — bare qualified names default to `TypeEnumeration` and need catalog-based disambiguation.
0 commit comments