refactor: code quality — deterministic output, doc comments, naming

[retran]

Why

PRs #235–#238 performed the structural refactoring. This final PR addresses code quality issues found during 10 iterations of self-review across the entire branch (129 files). No behavioral changes — only correctness fixes, documentation, and cleanup.

What changed (incremental from PR #238)

62 files changed, +345/-346 (net -1 lines)

Correctness:

• Sort map keys before iteration in asyncapi.go, widget_builder.go, datagrid_builder.go, cmd_alter_page.go for deterministic serialization output
• Fix UUID v4 version/variant bits in GenerateDeterministicID
• Add float64→int64 precision guard (±2^53) in json_utils.go
• Add len(s) < 19 panic guard in datetime normalization
• Fix TestHash_EmptyInput to expect correct SHA-256 of empty input

Documentation:

• Add doc comments on all exported backend interfaces and methods (ModuleBackend, FolderBackend, WorkflowMutator methods, WidgetObjectBuilder, PageMutator)
• Document string-ID convention for SDK writer layer methods
• Document WidgetObjectBuilder deferred error handling design
• Replace storage-specific terminology ("BSON", "raw BSON") with storage-agnostic language in interface docs

Naming consistency:

• Rename reader field/references to backend across executor (flowBuilder.reader, countByModuleFromReader, stale comments)
• Rename xmlDataServices → xmlDataService
• Update stale file-level comments in reader_types.go
• Rename withContainer test helper param from parentID to parentContainerID

Cleanup:

• Simplify convert.go — remove redundant deep-copy conversions where types are already identical (passthrough)
• Remove trailing whitespace, blank lines, fix alignment in mock struct fields
• Apply go fmt formatting across 17 files
• Keep sdk/mpr/writer_core.go backward-compatible fallback for invalid/empty UUIDs (tests use placeholder IDs)

Review-driven improvements

• IDToBsonBinaryErr: Error-returning variant of IDToBsonBinary for user-supplied/untrusted IDs. IDToBsonBinary now delegates to IDToBsonBinaryErr internally.
• GenerateIDErr: Error-returning variant of GenerateID for callers that can handle OS entropy failure gracefully. GenerateID now delegates to GenerateIDErr internally.
• NewIDBsonBinaryErr: Composes GenerateIDErr + IDToBsonBinaryErr for fully error-returning ID generation.
• .gitignore: Added .playwright-cli/ to prevent test automation logs from being committed.

Stack

PR 5/5 (final) in the shared-types refactoring stack.
Merge order: #232 → #235 → #236 → #237 → #238 → this

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR completes the shared-types refactoring by moving executor/catalog code off sdk/mpr-specific types and raw BSON manipulation, improving determinism and testability while tightening correctness around IDs and JSON handling.

Changes:

• Refactors many executor/catalog paths to use mdl/types and backend abstractions (incl. page mutation) instead of direct sdk/mpr/BSON operations.
• Introduces mdl/bsonutil to perform UUID↔BSON Binary conversions without pulling in CGO-heavy dependencies.
• Adds a large set of MockBackend-based tests for SHOW/DESCRIBE commands and connection failure paths.

Reviewed changes

Copilot reviewed 112 out of 155 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
mdl/executor/cmd_widgets.go	Switch widget updates from raw BSON edits to backend PageMutator flow
mdl/executor/cmd_structure.go	Rename “reader” terminology to “backend” and update helper naming
mdl/executor/cmd_settings_mock_test.go	Add MockBackend coverage for settings output
mdl/executor/cmd_security_write.go	Move security write DTOs to mdl/types and normalize error wrapping
mdl/executor/cmd_security_mock_test.go	Add MockBackend coverage for security show/describe flows
mdl/executor/cmd_rest_clients_mock_test.go	Add MockBackend coverage for REST clients show/describe
mdl/executor/cmd_rename.go	Switch rename hit types from sdk/mpr to mdl/types
mdl/executor/cmd_published_rest_mock_test.go	Add MockBackend coverage for published REST services
mdl/executor/cmd_pages_mock_test.go	Add MockBackend coverage for pages/snippets/layouts show
mdl/executor/cmd_pages_create_v3.go	Page/snippet creation uses backend references; wires widget backend
mdl/executor/cmd_pages_builder_v3_pluggable.go	Remove BSON-based pluggable widget builder implementation
mdl/executor/cmd_pages_builder_v3_layout.go	Replace ID generation from mpr.GenerateID to types.GenerateID
mdl/executor/cmd_pages_builder_input_filters.go	Remove BSON-based filter widget construction helpers
mdl/executor/cmd_pages_builder_input_cloning.go	Remove BSON deep-clone helpers (moved/centralized elsewhere)
mdl/executor/cmd_pages_builder_input.go	Shift snippet/microflow resolution to backend APIs
mdl/executor/cmd_pages_builder.go	Make pageBuilder backend-driven; introduce widgetBackend + types-based folders
mdl/executor/cmd_odata_mock_test.go	Add MockBackend coverage for OData client/service show/describe
mdl/executor/cmd_odata.go	Switch ID generation and EDMX parsing to mdl/types
mdl/executor/cmd_notconnected_mock_test.go	Add negative-path tests for “not connected” handling across commands
mdl/executor/cmd_navigation_mock_test.go	Add MockBackend coverage for navigation show/describe
mdl/executor/cmd_navigation.go	Move navigation DTOs/specs to mdl/types
mdl/executor/cmd_modules_mock_test.go	Add MockBackend coverage for module listing and counting
mdl/executor/cmd_misc_mock_test.go	Add MockBackend coverage for version display
mdl/executor/cmd_microflows_mock_test.go	Add MockBackend coverage for microflows show/describe
mdl/executor/cmd_microflows_helpers.go	Update enum lookup helpers to use backend instead of reader
mdl/executor/cmd_microflows_create.go	Switch ID generation + flowBuilder wiring to backend
mdl/executor/cmd_microflows_builder_workflow.go	Use types.GenerateID for workflow/microflow object creation
mdl/executor/cmd_microflows_builder_graph.go	Use types.GenerateID for microflow graph objects
mdl/executor/cmd_microflows_builder_flows.go	Use types.GenerateID and pass backend through builders
mdl/executor/cmd_microflows_builder_control.go	Use types.GenerateID and pass backend through loop/while builders
mdl/executor/cmd_microflows_builder_annotations.go	Use types.GenerateID for annotations and flows
mdl/executor/cmd_microflows_builder.go	Rename reader field to backend for reference resolution
mdl/executor/cmd_mermaid_mock_test.go	Add MockBackend coverage for mermaid domain model description
mdl/executor/cmd_lint.go	Update lint context to use executor Reader accessor
mdl/executor/cmd_jsonstructures_mock_test.go	Add MockBackend coverage for JSON structures show
mdl/executor/cmd_jsonstructures.go	Move JSON structure model/types and JSON utilities to mdl/types
mdl/executor/cmd_javascript_actions_mock_test.go	Add MockBackend coverage for JavaScript actions show/describe
mdl/executor/cmd_javaactions_mock_test.go	Add MockBackend coverage for Java actions show/describe
mdl/executor/cmd_javaactions.go	Switch Java action ID generation to types.GenerateID
mdl/executor/cmd_import_mappings_mock_test.go	Add MockBackend coverage for import mappings show
mdl/executor/cmd_import_mappings.go	Switch JSON element types + attribute typing resolution to backend + mdl/types
mdl/executor/cmd_imagecollections_mock_test.go	Add MockBackend coverage for image collections show/describe
mdl/executor/cmd_imagecollections.go	Move image collection types to mdl/types
mdl/executor/cmd_fragments_mock_test.go	Add Mock-context tests for fragment listing
mdl/executor/cmd_folders.go	Switch folder info type from sdk/mpr to mdl/types
mdl/executor/cmd_features.go	Update documentation wording from reader to connection state
mdl/executor/cmd_export_mappings_mock_test.go	Add MockBackend coverage for export mappings show
mdl/executor/cmd_export_mappings.go	Switch JSON element types + attribute typing resolution to backend + mdl/types
mdl/executor/cmd_enumerations_mock_test.go	Simplify and expand enumeration MockBackend tests (incl. describe)
mdl/executor/cmd_entities_mock_test.go	Add MockBackend coverage for entities show/filter
mdl/executor/cmd_entities.go	Switch ID generation + error wrapping to types/mdlerrors
mdl/executor/cmd_diff_local.go	Delegate microflow parsing from raw maps to backend
mdl/executor/cmd_dbconnection_mock_test.go	Add MockBackend coverage for DB connections show/describe
mdl/executor/cmd_datatransformer_mock_test.go	Add MockBackend coverage for data transformers list/describe
mdl/executor/cmd_contract.go	Move EDMX/AsyncAPI parsing and DTOs to mdl/types
mdl/executor/cmd_constants_mock_test.go	Add MockBackend coverage for constants show/describe
mdl/executor/cmd_catalog.go	Update docs and local executor wiring from reader→backend
mdl/executor/cmd_businessevents_mock_test.go	Add MockBackend coverage for business event services show/describe
mdl/executor/cmd_businessevents.go	Switch ID generation to types.GenerateID
mdl/executor/cmd_associations_mock_test.go	Add MockBackend coverage for associations show/filter
mdl/executor/cmd_agenteditor_mock_test.go	Add MockBackend coverage for agent editor show/describe commands
mdl/executor/bugfix_test.go	Rename reader reference to backend in regression test
mdl/executor/bugfix_regression_test.go	Update test comment to backend terminology
mdl/catalog/builder_references.go	Switch navigation menu item DTO type to mdl/types
mdl/catalog/builder_navigation.go	Switch navigation menu item DTO type to mdl/types
mdl/catalog/builder_contract.go	Switch EDMX/AsyncAPI parsing to mdl/types
mdl/catalog/builder.go	Switch catalog reader shared DTO types to mdl/types
mdl/bsonutil/bsonutil.go	Add CGO-free BSON Binary ↔ UUID conversion utilities
mdl/backend/workflow.go	Remove unrelated backend interface definitions from workflow backend file
mdl/backend/security.go	Move member access DTO types to mdl/types
mdl/backend/page.go	Clarify snippet API expectations in comments
mdl/backend/navigation.go	Move navigation DTO/spec types to mdl/types
mdl/backend/mpr/datagrid_builder_test.go	Update tests to mpr backend pkg; use bsonutil + bytes.Equal
mdl/backend/mpr/convert.go	Add conversion glue between sdk/mpr and mdl/types
mdl/backend/mock/mock_workflow.go	Switch image collection DTOs to mdl/types
mdl/backend/mock/mock_security.go	Switch entity access revocation DTO to mdl/types
mdl/backend/mock/mock_navigation.go	Switch navigation DTO/spec types to mdl/types
mdl/backend/mock/mock_mutation.go	Add mock implementations for mutation/serialization/widget builder backends
mdl/backend/mock/mock_module.go	Switch folder DTOs to mdl/types
mdl/backend/mock/mock_microflow.go	Add ParseMicroflowFromRaw hook to MockBackend
mdl/backend/mock/mock_mapping.go	Switch JSON structure DTOs to mdl/types
mdl/backend/mock/mock_java.go	Switch Java/JS action DTOs to mdl/types
mdl/backend/mock/mock_infrastructure.go	Switch core DTOs to mdl/types and expand AgentEditor mocks
mdl/backend/mock/mock_connection.go	Switch version DTOs to mdl/types
mdl/backend/mock/backend.go	Update MockBackend fields and interfaces to mdl/types; add new backend hooks
mdl/backend/microflow.go	Add ParseMicroflowFromRaw to MicroflowBackend interface
mdl/backend/mapping.go	Switch JSON structure DTOs to mdl/types and document DeleteJsonStructure contract
mdl/backend/java.go	Switch Java/JS action DTOs to mdl/types
mdl/backend/infrastructure.go	Switch raw-unit and metadata DTOs to mdl/types; add Settings/Image/ScheduledEvent backends
mdl/backend/doc.go	Update backend package docs for mdl/types + conversion layer
mdl/backend/connection.go	Switch version/folder DTOs to mdl/types and improve interface docs
mdl/backend/backend.go	Expand FullBackend to include mutation/serialization/widget builder backends
examples/create_datagrid2_page/main.go	Wire executor to mpr backend via backend factory
cmd/mxcli/project_tree.go	Switch menu item DTO type to mdl/types
cmd/mxcli/main.go	Wire CLI executor to mpr backend via backend factory

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 87 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 84 out of 87 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

mdl/executor/hierarchy.go:68

• NewContainerHierarchy ignores errors from src.ListUnits() and src.ListFolders() (using _). If either call fails, the hierarchy will be silently incomplete and can mis-resolve module/folder paths, producing incorrect qualified names/counts downstream. Propagate these errors (or at least surface them) so callers can fail fast instead of operating on partial data.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

AI Code Review

What Looks Good

This PR is a code quality improvement refactor with no behavioral changes, as stated. It successfully addresses multiple areas:

Correctness fixes:

• Deterministic serialization via map key sorting in multiple files (asyncapi.go, widget_builder.go, etc.)
• Proper UUID v4 version/variant bit fixing in GenerateDeterministicID
• Float64→int64 precision guard (±2^53) in json_utils.go
• Datetime normalization panic guard (len(s) < 19)
• Corrected SHA-256 expectation for empty input in TestHash_EmptyInput

Documentation improvements:

• Added doc comments to all exported backend interfaces and methods
• Documented string-ID convention for SDK writer layer methods
• Clarified WidgetObjectBuilder deferred error handling design
• Replaced storage-specific terminology with storage-agnostic language

Naming consistency:

• Consistently renamed reader → backend across executor code
• Fixed xmlDataServices → xmlDataService
• Updated stale file-level comments
• Renamed test helper parameter parentID → parentContainerID

Cleanup:

• Simplified convert.go by removing redundant deep-copy conversions
• Removed trailing whitespace, fixed alignment
• Applied go fmt formatting across 17 files
• Maintained backward-compatible UUID fallback for tests
• Added error-returning variants of ID generation functions (IDToBsonBinaryErr, GenerateIDErr, NewIDBsonBinaryErr)
• Added .playwright-cli/ to .gitignore

The PR follows the project's contribution guidelines and is appropriately scoped as the final cleanup in the shared-types refactoring stack. All changes are non-behavioral and improve code maintainability, correctness, and readability.

Recommendation

Approve - This PR meets all quality standards and addresses the checklist items appropriately. The correctness fixes resolve actual issues, the documentation improves maintainability, and the naming consistency/cleanup enhances code quality without introducing behavioral changes or violating any review criteria.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source