Conversation
…D reuse BSON int32→int64: - All property values in writer serialization changed from int32 to int64 to match Studio Pro's expected BSON types - Array version markers (int32(2), int32(3)) preserved as int32 - Fixes Studio Pro crash: "Sequence contains no matching element" in MprProperty..ctor when type cache expects int64 but finds int32 CREATE OR REPLACE PAGE: - Reuses existing page UUID and calls UpdatePage instead of DeletePage+ CreatePage, producing git "Modified" diff instead of "Deleted+Added" - Prevents RevStatusCache crash when parsing deleted mxunit base files BSON roundtrip test baseline: - Add golden mxunit files for roundtrip testing (pages, microflows, enums) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diag --check-units: - New command to detect orphan units (DB record, no file) and stale mxunit files (file exists, no DB record) - --fix flag auto-removes stale files - bson dump --format bson output format Grammar fixes for MDL baseline scripts: - Add DISPLAY, STRUCTURE to enumValueName rule - Support String(unlimited) syntax in dataType - Add ATTRIBUTE to attributeName rule - Support parenthesized CREATE ASSOCIATION with optional colons - Add SEND, REQUEST, TABLETWIDTH, PHONEWIDTH keywords Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PLUGGABLEWIDGET syntax with explicit properties, auto datasource,
auto child slots, TextTemplate attribute binding, and keyword properties.
Core changes:
- PLUGGABLEWIDGET 'widget.id' name (key: value) syntax
- CUSTOMWIDGET/TABCONTAINER/TABPAGE grammar tokens
- Auto datasource ordering (step 4.1, before child slots)
- Auto child slot matching by container name
- Object list auto-populate with Required filter
- TextTemplate {AttrName} parameter binding
- Widget lifecycle CLI (widget init/docs)
- DESCRIBE PLUGGABLEWIDGET output for generic widgets
- 20 new widget templates (mendix-11.6)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ako
left a comment
There was a problem hiding this comment.
Review
51K additions — this is a very large PR. It stacks on #67 (which stacks on #66), so all issues from both prior reviews carry forward. The new commit adds ~39K lines, of which ~28K are widget template JSON files.
Scope
This should be at least 3 separate PRs:
- Widget template JSON files (20 new templates) — bulk addition, easy to review separately
- Pluggable widget engine v2 (widget_engine.go, augment.go, loader.go, visitor/executor changes)
- Widget CLI commands (
cmd_widget.go)
Bundling makes this effectively unreviewable at 51K lines.
Code quality issues
1. Silent nil return in deepCloneMap (augment.go)
deepCloneMap serializes to JSON then deserializes back. On error, it returns nil without signaling the caller. This causes cascading nils — cloned properties silently become nil, and downstream code gets mysterious failures instead of a clear error.
2. Unsafe type assertions throughout
Multiple places assume BSON structure shapes without checking. For example, createAttributeObject silently returns nil AttributeRef when the attribute path has fewer than 2 dots, instead of returning an error for invalid paths.
3. Entity context mutation (widget_engine.go)
entityContext is saved/restored around Build() but mutated in-place during DataSource mapping. If an error occurs partway through, the context is left corrupted. Should use an immutable context stack or pass context as a parameter.
4. Placeholder ID generation (augment.go)
Uses atomic.AddInt64 for a counter but ResetPlaceholderCounter() exists for testing — this is a race condition if tests run in parallel. The deterministic format also means IDs may collide across concurrent augmentations.
5. Mode selection swallows ambiguity (widget_engine.go)
When multiple modes have no condition, the first fallback is silently chosen with no warning. This hides accidental duplicate mode definitions that could produce wrong widget output.
6. Debug logging still present from PR #66
The log.Printf("SERIALIZE CHECK: timelineCustom Widgets: ...") debug code in writer_widgets_custom.go is still here from the base commits.
Missing error handling
buildAttributeObjects: No validation that the resolved attribute path actually exists in the domain model before creating a reference- Operations in
widget_engine.go(opAssociation, etc.): Check if source is empty but don't validate the resolved context (e.g., EntityName being set) containsPlaceholderIDgives generic error messages — doesn't indicate which field has the unreplaced placeholder
Design concerns
JSON round-trip for deep clone
Using JSON marshal/unmarshal for deep cloning BSON structures is fragile and has performance implications. Custom BSON types or binary data won't survive the round-trip. A proper BSON deep-clone would be more robust.
Tight coupling to BSON
PluggableWidgetEngine receives and returns bson.D directly with no abstraction. This makes the engine untestable without real BSON structures and means any serialization format changes require rewriting the entire module.
Testing
sdk/mpr/roundtrip_test.gois a good addition (+212 lines) — this addresses the gap flagged in PR #66 review- However: no tests for
deepCloneMaperror paths, no tests for partial operation failures, no tests for mode fallback ambiguity, no tests for placeholder ID uniqueness under concurrency
Design docs committed
docs/plans/2026-03-30-widget-docs-and-customwidget.md and docs/plans/2026-03-30-widget-docs-generation-design.md are committed. Are these intended to live permanently, or should they be removed post-implementation? They describe planned work rather than documenting what was built.
Template files
20 new widget templates added under sdk/widgets/templates/mendix-11.6/. These are extracted from Studio Pro which is the right approach per the repo's guidelines. However:
- Were all templates verified to include both
typeANDobjectfields as required bysdk/widgets/templates/README.md? timeline.jsonshows+0 -0(empty file?) — needs verification
Summary
The core widget engine design is reasonable, but the PR needs:
Stacked on #67. PLUGGABLEWIDGET syntax, auto datasource/child slots, 20 widget templates, widget CLI.