Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .claude/commands/mxcli-dev/review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# /mxcli-dev:review — PR Review

Run a structured review of the current branch's changes against the CLAUDE.md
checklist, then check the recurring findings table below for patterns that have
burned us before.

## Steps

1. Run `gh pr view` and `gh pr diff` (or `git diff main...HEAD`) to read the change.
2. Work through the CLAUDE.md "PR / Commit Review Checklist" in full.
3. Then check every row in the Recurring Findings table below — flag any match.
4. Report: blockers first, then moderate issues, then minor. Include a concrete fix
option for every blocker (not just "this is wrong").
5. After the review: **add a row** to the Recurring Findings table for any new
pattern not already covered.

---

## Recurring Findings

Patterns caught in real reviews. Each row is a class of mistake worth checking
proactively. Add a row after every review that surfaces something new.

| # | Finding | Category | Canonical fix |
|---|---------|----------|---------------|
| 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` |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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/` |
| 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 |

---

## After Every Review

- [ ] All blockers have a concrete fix option stated.
- [ ] Recurring Findings table updated with any new pattern.
- [ ] If docs-only PR: every function name, path, and PR reference verified against
live code before approving.
77 changes: 77 additions & 0 deletions .claude/skills/fix-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Fix Issue Skill

A fast-path workflow for diagnosing and fixing bugs in mxcli. Each fix appends
to the symptom table below, so the next similar issue costs fewer reads.

## How to Use

1. Match the issue symptom to a row in the table — go straight to that file.
2. Follow the fix pattern for that row.
3. Write a failing test first, then implement.
4. After the fix: **add a new row** to the table if the symptom is not already covered.

---

## Symptom → Layer → File Table

| Symptom | Root cause layer | First file to open | Fix pattern |
|---------|-----------------|-------------------|-------------|
| `DESCRIBE` shows `$var = LIST OPERATION ...;` | Missing parser case | `sdk/mpr/parser_microflow.go` → `parseListOperation()` | Add `case "Microflows$XxxType":` returning the correct struct |
| `DESCRIBE` shows `$var = ACTION ...;` | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatActionStatement()` | Add `case *microflows.XxxAction:` with `fmt.Sprintf` output |
| `DESCRIBE` shows `$var = LIST OPERATION %T;` (with type name) | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatListOperation()` | Add `case *microflows.XxxOperation:` before the `default` |
| Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker |
| `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$Type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` |
| CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics |
| CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` |
| Parser returns `nil` for a known BSON type | Unhandled `default` in a `parseXxx()` switch | `sdk/mpr/parser_microflow.go` or `parser_page.go` | Find the switch by grepping for `default: return nil`; add the missing case |
| MDL check gives "unexpected token" on valid-looking syntax | Grammar missing rule or token | `mdl/grammar/MDLParser.g4` + `MDLLexer.g4` | Add rule/token, run `make grammar` |
| CE7054 "parameters updated" / CE7067 "does not support body entity" after `SEND REST REQUEST` | `addSendRestRequestAction` emitted wrong BSON: all params as query params, BodyVariable set for JSON bodies | `mdl/executor/cmd_microflows_builder_calls.go` → `addSendRestRequestAction` | Look up operation via `fb.restServices`; route path/query params with `buildRestParameterMappings`; suppress BodyVariable for JSON/TEMPLATE/FILE via `shouldSetBodyVariable` |

---

## TDD Protocol

Always follow this order — never implement before the test exists:

```
Step 1: Write a failing unit test (parser test or formatter test)
Step 2: Confirm it fails to compile or fails at runtime
Step 3: Implement the minimum code to make it pass
Step 4: Run: /c/Users/Ylber.Sadiku/go/go/bin/go test ./mdl/executor/... ./sdk/mpr/...
Step 5: Add the symptom row to the table above if not already present
```

Parser tests go in `sdk/mpr/parser_<domain>_test.go`.
Formatter tests go in `mdl/executor/cmd_<domain>_format_<area>_test.go`.

---

## Issue #212 — Reference Fix (seeding example)

**Symptom:** `DESCRIBE MICROFLOW` showed `$var = LIST OPERATION ...;` for
`Microflows$Find`, `Microflows$Filter`, `Microflows$ListRange`.

**Root cause:** `parseListOperation()` in `sdk/mpr/parser_microflow.go` had no
cases for these three BSON types — they fell to `default: return nil`.

**Files changed:**
| File | Change |
|------|--------|
| `sdk/microflows/microflows_actions.go` | Added `FindByAttributeOperation`, `FilterByAttributeOperation`, `RangeOperation` |
| `sdk/mpr/parser_microflow.go` | Added 3 parser cases |
| `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases |
| `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests |
| `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests |

**Key insight:** `Microflows$ListRange` stores offset/limit inside a nested
`CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before
extracting `OffsetExpression`/`LimitExpression`.

---

## After Every Fix — Checklist

- [ ] Failing test written before implementation
- [ ] `go test ./mdl/executor/... ./sdk/mpr/...` passes
- [ ] New symptom row added to the table above (if not already covered)
- [ ] PR title: `fix: <one-line description matching the symptom>`
18 changes: 17 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ Available namespaces: `DomainModels`, `Enumerations`, `Microflows`, `Pages`, `Mo

When reviewing pull requests or validating work before commit, verify these items:

### Bug fixes
- [ ] **Fix-issue skill consulted** — read `.claude/skills/fix-issue.md` before diagnosing; match symptom to table before opening files
- [ ] **Symptom table updated** — new symptom/layer/file mapping added to `.claude/skills/fix-issue.md` if not already covered
- [ ] **Test written first** — failing test exists before implementation (parser test in `sdk/mpr/`, formatter test in `mdl/executor/`)

### Overlap & duplication
- [ ] Check `docs/11-proposals/` for existing proposals covering the same functionality
- [ ] Search the codebase for existing implementations (grep for key function names, command names, types)
Expand Down Expand Up @@ -355,11 +360,22 @@ mxcli new MyApp --version 10.24.0 --output-dir ./projects/my-app

Steps performed: downloads MxBuild → `mx create-project` → `mxcli init` → downloads correct Linux mxcli binary for devcontainer. The result is a ready-to-open project with `.devcontainer/`, AI tooling, and a working `./mxcli` binary.

### Slash Command Namespaces

Commands in `.claude/commands/` are organised by audience:

| Namespace | Folder | Invoked as | Purpose |
|-----------|--------|------------|---------|
| `mendix:` | `.claude/commands/mendix/` | `/mendix:lint` | mxcli **user** commands — synced to Mendix projects via `mxcli init` |
| `mxcli-dev:` | `.claude/commands/mxcli-dev/` | `/mxcli-dev:review` | **Contributor** commands — this repo only, never synced to user projects |

Both namespaces are discoverable by typing `/mxcli` in Claude Code. Add new contributor tooling (review workflows, debugging helpers, etc.) under `mxcli-dev/`. Add commands intended for Mendix project users under `mendix/`.

### mxcli init

`mxcli init` creates a `.claude/` folder with skills, commands, CLAUDE.md, and VS Code MDL extension in a target Mendix project. Source of truth for synced assets:
- Skills: `reference/mendix-repl/templates/.claude/skills/`
- Commands: `.claude/commands/mendix/`
- Commands: `.claude/commands/mendix/` (the `mxcli-dev/` folder is **not** synced)
- VS Code extension: `vscode-mdl/vscode-mdl-*.vsix`

Build-time sync: `make build` syncs everything automatically. Individual targets: `make sync-skills`, `make sync-commands`, `make sync-vsix`.
Expand Down
1 change: 1 addition & 0 deletions mdl/executor/cmd_microflows_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type flowBuilder struct {
reader *mpr.Reader // For looking up page/microflow references
hierarchy *ContainerHierarchy // For resolving container IDs to module names
pendingAnnotations *ast.ActivityAnnotations // Pending annotations to attach to next activity
restServices []*model.ConsumedRestService // Cached REST services for parameter classification
}

// addError records a validation error during flow building.
Expand Down
111 changes: 95 additions & 16 deletions mdl/executor/cmd_microflows_builder_calls.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,16 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode
// Build operation reference: Module.Service.Operation
operationQN := s.Operation.String()

// Look up the operation definition to classify parameters and body kind.
// s.Operation.Module = "MfTest", s.Operation.Name = "RC_TestApi.PostJsonTemplate"
var opDef *model.RestClientOperation
if fb.restServices != nil && s.Operation.Module != "" && strings.Contains(s.Operation.Name, ".") {
dotIdx := strings.Index(s.Operation.Name, ".")
serviceName := s.Operation.Name[:dotIdx]
opName := s.Operation.Name[dotIdx+1:]
opDef = lookupRestOperation(fb.restServices, serviceName, opName)
}

// Build OutputVariable
var outputVar *microflows.RestOutputVar
if s.OutputVariable != "" {
Expand All @@ -778,29 +788,20 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode
}
}

// Build BodyVariable
// Build BodyVariable only for EXPORT_MAPPING body kind.
// For JSON / TEMPLATE / FILE bodies, the body expression lives on the
// operation definition itself and must NOT be set here (CE7067).
var bodyVar *microflows.RestBodyVar
if s.BodyVariable != "" {
if s.BodyVariable != "" && shouldSetBodyVariable(opDef) {
bodyVar = &microflows.RestBodyVar{
BaseElement: model.BaseElement{ID: model.ID(mpr.GenerateID())},
VariableName: s.BodyVariable,
}
}

// Build parameter mappings from WITH clause
var paramMappings []*microflows.RestParameterMapping
var queryParamMappings []*microflows.RestQueryParameterMapping
for _, p := range s.Parameters {
// Determine if path or query param by convention:
// the executor can't distinguish at this level, so we emit both
// and let the BSON field names sort it out. For now, emit as
// query parameter mappings (most common use case).
queryParamMappings = append(queryParamMappings, &microflows.RestQueryParameterMapping{
Parameter: operationQN + "." + p.Name,
Value: p.Expression,
Included: "Yes",
})
}
// Build parameter mappings, routing to ParameterMappings (path) or
// QueryParameterMappings (query) based on the operation definition.
paramMappings, queryParamMappings := buildRestParameterMappings(s.Parameters, opDef, operationQN)

// RestOperationCallAction does not support custom error handling (CE6035).
// ON ERROR clauses in the MDL are silently ignored for this action type.
Expand Down Expand Up @@ -831,6 +832,84 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode
return activity.ID
}

// lookupRestOperation finds a specific operation in a consumed REST service list.
func lookupRestOperation(services []*model.ConsumedRestService, serviceName, opName string) *model.RestClientOperation {
for _, svc := range services {
if svc.Name != serviceName {
continue
}
for _, op := range svc.Operations {
if op.Name == opName {
return op
}
}
}
return nil
}

// shouldSetBodyVariable returns true if a BodyVariable BSON field should be
// emitted for a call to the given operation.
// For JSON, TEMPLATE, and FILE body kinds, the body expression lives on the
// operation definition and must not be overridden by a BodyVariable (CE7067).
// For EXPORT_MAPPING, the caller provides an entity to export via BodyVariable.
// When the operation definition is unknown (nil), we preserve old behaviour and
// set BodyVariable so the caller's intent is not silently dropped.
func shouldSetBodyVariable(op *model.RestClientOperation) bool {
if op == nil {
return true // unknown operation — preserve caller intent
}
switch op.BodyType {
case "JSON", "TEMPLATE", "FILE":
return false
default:
// EXPORT_MAPPING or empty (no body) — only set if EXPORT_MAPPING
return op.BodyType == "EXPORT_MAPPING"
}
}

// buildRestParameterMappings splits parameter bindings from a SEND REST REQUEST
// WITH clause into path parameter mappings and query parameter mappings,
// using the operation definition to determine which is which.
// When op is nil (operation not found), all parameters fall back to query
// parameter mappings (preserves old behaviour).
func buildRestParameterMappings(
params []ast.SendRestParamDef,
op *model.RestClientOperation,
operationQN string,
) ([]*microflows.RestParameterMapping, []*microflows.RestQueryParameterMapping) {
if len(params) == 0 {
return nil, nil
}

// Build lookup sets from the operation definition.
pathParamSet := map[string]bool{}
if op != nil {
for _, p := range op.Parameters {
pathParamSet[p.Name] = true
}
}

var pathMappings []*microflows.RestParameterMapping
var queryMappings []*microflows.RestQueryParameterMapping

for _, p := range params {
if pathParamSet[p.Name] {
pathMappings = append(pathMappings, &microflows.RestParameterMapping{
Parameter: operationQN + "." + p.Name,
Value: p.Expression,
})
} else {
queryMappings = append(queryMappings, &microflows.RestQueryParameterMapping{
Parameter: operationQN + "." + p.Name,
Value: p.Expression,
Included: "Yes",
})
}
}

return pathMappings, queryMappings
}

// addExecuteDatabaseQueryAction creates an EXECUTE DATABASE QUERY statement.
func (fb *flowBuilder) addExecuteDatabaseQueryAction(s *ast.ExecuteDatabaseQueryStmt) model.ID {
// DynamicQuery is a Mendix expression — string literals need single quotes
Expand Down
2 changes: 2 additions & 0 deletions mdl/executor/cmd_microflows_builder_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
measurer: fb.measurer, // Share measurer
reader: fb.reader, // Share reader
hierarchy: fb.hierarchy, // Share hierarchy
restServices: fb.restServices, // Share REST services for parameter classification
}

// Process loop body statements and connect them with flows
Expand Down Expand Up @@ -360,6 +361,7 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
measurer: fb.measurer,
reader: fb.reader,
hierarchy: fb.hierarchy,
restServices: fb.restServices,
}

var lastBodyID model.ID
Expand Down
1 change: 1 addition & 0 deletions mdl/executor/cmd_microflows_builder_flows.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in
measurer: fb.measurer,
reader: fb.reader,
hierarchy: fb.hierarchy,
restServices: fb.restServices,
}

var lastErrID model.ID
Expand Down
Loading