Skip to content

Update parsing of ScrollContainer and TabControl#219

Open
NikolaSimsic wants to merge 1 commit intomendixlabs:mainfrom
NikolaSimsic:main
Open

Update parsing of ScrollContainer and TabControl#219
NikolaSimsic wants to merge 1 commit intomendixlabs:mainfrom
NikolaSimsic:main

Conversation

@NikolaSimsic
Copy link
Copy Markdown

The problem:

When mxcli runs DESCRIBE PAGE, it walks the widget tree in the .mpr BSON and outputs every widget with its ID. The Python extractor (extract_mendix_artifacts.py) then parses that output to build the artifact map. But two container widget types were not traversing their children:

ScrollContainer — Mendix stores its children in CenterRegion.Widgets, not in Widgets directly (like most other containers). So mxcli saw the ScrollContainer itself but never recursed into it — every widget nested inside a ScrollContainer was invisible.

TabControl — Mendix stores children in TabPages[].Widgets (an array of tab pages, each with its own Widgets array). Same problem — mxcli saw the TabControl but not the widgets on each tab.

The impact on test impact analysis:

Without this fix, the extractor was missing 115 widget IDs (509 → 624 after the fix). Those missing IDs meant the linkage map couldn't connect certain test tags to their Mendix project artifacts. Any test that asserted on a widget inside a ScrollContainer or TabControl had a broken linkage — the system wouldn't know which page (and therefore which appdev code) that test exercises.

In short: mxcli couldn't see inside these two container types → extractor missed widgets → linkage map had gaps → test impact analysis would have blind spots for those areas.

@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None

Moderate Issues

None

Minor Issues

  • Test coverage: The PR fixes a bug in DESCRIBE PAGE output but doesn't add test coverage for the fixed behavior. While the PR description mentions manual verification (widget count increase from 509→624), automated test coverage would prevent regression. The doctest files don't show existing tests for DESCRIBE PAGE output validation, but adding a test case in mdl-examples/doctype-tests/ (e.g., a page with ScrollContainer/TabControl containing widgets) would strengthen the fix.

What Looks Good

  • Correctly scoped fix: Changes are isolated to the two affected widget types in the DESCRIBE PAGE executor logic (parsing and output).
  • Follows existing patterns: Mirrors the handling of DivContainer/GroupBox widgets in the same files.
  • Clear documentation: Inline comments explain the non-standard child storage locations (CenterRegion.Widgets, TabPages[].Widgets).
  • No syntax changes: Since this is a behavioral bug fix (not new MDL syntax), full-stack consistency checks (grammar/AST/etc.) don't apply.
  • Maintains output structure: Preserves the widget hierarchy in DESCRIBE output (nested blocks for children) while exposing previously hidden widget IDs.

Recommendation

Approve the PR. The fix correctly resolves the described issue with minimal, focused changes. While adding a test case would be ideal, the bug fix itself is correct and addresses a clear gap in the existing functionality. The changes are consistent with the codebase patterns and don't introduce any risks.


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

@ako
Copy link
Copy Markdown
Collaborator

ako commented Apr 17, 2026

Review

The Go code changes are legitimate bug fixes, but the motivation and framing are for an external tool, not for mxcli itself.

What's correct

  • ScrollContainer fix (cmd_pages_describe_parse.go): Mendix stores children in CenterRegion.Widgets, not Widgets. Without this, DESCRIBE PAGE missed all widgets nested inside scroll containers. Correct BSON traversal with a fallback to Widgets for older formats.
  • TabControl fix: Mendix stores children in TabPages[].Widgets. Same issue — children were invisible. Correct fix: iterate tab pages, recurse into each tab's widgets.
  • Output side (cmd_pages_describe_output.go): adds TABCONTAINER and SCROLLCONTAINER as MDL widget types with proper child recursion. Follows the existing pattern used by CONTAINER/GROUPBOX.

Concerns

  1. The PR is motivated by an external Python script, not mxcli users. The description frames this as fixing "the Python extractor" and "test impact analysis" with an external linkage map. Those are valid use cases, but mxcli doesn't own or ship extract_mendix_artifacts.py. The PR should be framed as: "DESCRIBE PAGE was missing widgets inside ScrollContainer and TabControl" — that's the mxcli bug. The external tooling context belongs in the contributor's internal docs, not in the PR description.

  2. No tests. No unit test for the new parser paths, no MDL example in mdl-examples/. Per the checklist, bug fixes should include a test script in mdl-examples/bug-tests/. A simple page with a ScrollContainer containing a TextBox would verify the fix doesn't regress.

  3. TabControl drops TabPage metadata. The parser recurses into TabPages[].Widgets but doesn't preserve the tab page name, caption, or index. So DESCRIBE PAGE will show TABCONTAINER { TEXTBOX ... } without any indication of which tab each widget belongs to. The output handler similarly iterates w.Children without tab-page grouping. This is a loss of structure — consider preserving TABPAGE as an intermediate node.

  4. Two near-identical code blocks. The ScrollContainer and TabControl output cases in cmd_pages_describe_output.go are copy-pasted (lines differ only in the type check and header name). Same pattern as the existing DivContainer/GroupBox block. Not worth abstracting for 3 cases, but worth noting.

  5. SCROLLCONTAINER and TABCONTAINER aren't in the MDL grammar. These are new widget type keywords in DESCRIBE output. If someone tries to re-execute the DESCRIBE output (create a page with SCROLLCONTAINER), the parser won't accept it. For DESCRIBE-only output this is acceptable, but worth documenting as read-only.

  6. CI: build-and-test hasn't run (only review check shows). Should verify build passes before merge.

Verdict

The parser fix is correct and valuable — mxcli genuinely couldn't see inside these containers. Approve after (2) a test is added and (3) tab-page structure is preserved or explicitly noted as a follow-up. The PR description should be reframed around the mxcli bug, not the external Python script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants