Skip to content

UI TestEngine: expose disabled/blocked state on entries#5961

Merged
Grantim merged 3 commits into
masterfrom
ui-testengine-disabled-blocked
Apr 23, 2026
Merged

UI TestEngine: expose disabled/blocked state on entries#5961
Grantim merged 3 commits into
masterfrom
ui-testengine-disabled-blocked

Conversation

@Grantim
Copy link
Copy Markdown
Contributor

@Grantim Grantim commented Apr 22, 2026

Summary

  • Adds a human-readable status string to UI::TestEngine::Control::TypedEntry, populated each frame.
  • pressButton / writeValue<T> now return a typed error instead of silently succeeding when the target widget isn't interactive.
  • MCP ui.listEntries and the Python binding mrviewerpy.UiEntry surface the same status field.

Why

When an MCP or Python test client drove pressButton at a widget drawn under ImGui::BeginDisabled / UI::button(active=false) / a requirement-gated ribbon item, the TestEngine accepted the press but the runtime path dropped it. Agents had no way to detect the failure except by tailing logs. Likewise for clicks occluded by a modal popup. This surfaces both conditions via one consistent field and turns silent no-ops into checkable errors.

The status string

One string per entry, exactly one of:

Value Meaning
"available" Entry accepts input
"disabled" Disabled, no specific reason known
"disabled: <reason>" Disabled with caller-supplied reason (e.g. a ribbon requirements string like "Select exactly one Object")
"disabled: blocked by modal '<name>'" A modal popup named <name> is currently on top of this entry
"disabled: blocked by modal popup" Modal open, its window name isn't determinable

Agents can branch on status == "available" or status.startswith("disabled"), or pattern-match the whole string for reporting.

Mechanics

  • UI::TestEngine::EntryAttributes is a single-field struct { std::string_view disabledReason; } — non-empty marks the widget disabled with that reason. The test engine also auto-detects ImGui::BeginDisabled via GImGui->CurrentItemFlags & ImGuiItemFlags_Disabled and fills a generic fallback reason when the caller didn't set one.
  • ButtonEntry / ValueEntry store just a std::string disabledReason — disabled iff non-empty.
  • TypedEntry::status is built in listEntries from (entry's reason, top ImGui popup's window name) via a small composeStatus helper.
  • pressButton / writeValue<T> errors use the same format: "<fn> <path>: <status>".

Call-site updates

  • buttonEx: forwards customParams.enabled as disabledReason = "inactive" when disabled.
  • RibbonButtonDrawer: forwards the existing requirements string directly (empty = enabled).
  • All other existing call sites continue to work unchanged — createButton(name) etc. still compiles because the new param is defaulted.

Python binding

mrviewerpy.UiEntry gains a single .status: str property mirroring the MCP shape, per the sync directive at the top of MRUITestEngineControl.h.

Test plan

Verified end-to-end against a Debug build of MeshInspector via the MCP client:

  • ui.listEntries on RibbonSceneButtons with no scene: each entry reports status: "disabled: At least one objects should be in scene" / "disabled: Select at least one Object" / "disabled: Select exactly one Object" per its ribbon requirement.
  • ui.listEntries on the Toolbar with no scene: mixed "available" (Create Simple Objects, Move object, Select objects, Viewer settings) and "disabled: <specific requirement>" (Fit data, Clone, Transform object, etc.).
  • ui.pressButton on a disabled entry returns pressButton <path>: disabled: <reason> instead of succeeding silently.
  • Opening a modal popup (Toolbar Customize) flips every root-level entry to "disabled: blocked by modal 'Toolbar Customize'"; entries inside the popup's own group stay "available".
  • ui.pressButton on a blocked root entry returns ...: disabled: blocked by modal '<name>'.

🤖 Generated with Claude Code

Previously `pressButton` / `writeValue*` against a widget drawn under
`ImGui::BeginDisabled` / `UI::button(active=false)` / a requirement-
gated ribbon item silently no-op'd at runtime — MCP and Python test
clients had no way to detect these failures. Likewise, a click against
an entry occluded by an open modal popup succeeded at the TestEngine
layer but never reached the intended widget.

- ButtonEntry/ValueEntry gain a `disabled` bool, auto-OR'd from
  `GImGui->CurrentItemFlags & ImGuiItemFlags_Disabled` with an
  explicit caller flag. `buttonEx` forwards `!customParams.enabled`;
  ribbon drawer forwards `!requirements.empty()`.
- TypedEntry gains `disabled` + `blocked`; `blocked` is a heuristic
  (any ImGui popup open => root-level entries flagged).
- `pressButton` / `writeValue<T>` return a typed error on disabled/
  blocked instead of succeeding silently.
- MCP `ui.listEntries` output schema adds both fields (new
  `Mcp::Schema::Bool`); description updated to explain them.
- Python binding `mrviewerpy.UiEntry` mirrors the fields (per the
  sync comment at the top of `MRUITestEngineControl.h`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread source/MRViewer/MRUITestEngineControl.h Outdated
… string

Addresses PR review feedback (@Fedr): "May be better return requirements
string with human readable explanation why the element is inaccessible
instead of flags disabled/blocked?"

The previous iteration exposed four fields on `TypedEntry` (disabled,
disabledReason, blocked, blockingModal). Agents now get one unified
`status` string instead:
  * "available"                                  — entry accepts input
  * "disabled"                                   — disabled, no reason known
  * "disabled: <reason>"                         — caller-supplied reason
                                                   (e.g. unmet ribbon
                                                   requirement)
  * "disabled: blocked by modal '<name>'"        — modal popup on top
  * "disabled: blocked by modal popup"           — modal open, name unknown

`pressButton` / `writeValue` errors use the same format: `"pressButton
<path>: <status>"`.

Internal simplifications:
  * `EntryAttributes` is just `{std::string_view disabledReason;}` —
    non-empty marks the widget disabled with that reason. The bool is
    redundant with emptiness.
  * `ButtonEntry`/`ValueEntry` similarly drop the bool; disabled state
    is the emptiness of `disabledReason`.
  * ImGui's native `BeginDisabled` is still auto-detected and fills a
    generic fallback reason when the caller didn't supply one, so
    widgets wrapping in ImGui's mechanism don't need to set anything.
  * `topBlockingModalName` now returns `std::string_view` into ImGui's
    popup-stack buffer (valid on the GUI thread).
  * `Schema::Bool` added in the first commit is reverted — no longer
    needed.

Call sites become read-at-a-glance:
  * `buttonEx`: `{ .disabledReason = customParams.enabled ? "" : "inactive" }`
  * Ribbon:    `{ .disabledReason = requirements }` (empty = enabled)

Live-verified via the MeshInspector MCP: root-level entries report
`"available"` / `"disabled: <ribbon requirement>"` / `"disabled: blocked
by modal 'Toolbar Customize'"` as expected, and `pressButton` errors
carry the same `status` phrasing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ImGui::SetNextItemAllowOverlap();
bool pressed = ImGui::ButtonEx( ( "##wholeChildBtn" + item.item->name() ).c_str(), itemSize, ImGuiButtonFlags_AllowOverlap );
pressed = UI::TestEngine::createButton( item.item->name() ) || pressed; // Must not short-circuit.
// `requirements` empty = no unmet requirements = button enabled; non-empty = disabled with this reason.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment seems to be excess.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in fe2141b. The call site createButton(name, { .disabledReason = requirements }) reads itself.

Comment thread source/MRViewer/MRRibbonButtonDrawer.cpp
Comment thread source/MRViewer/MRUITestEngineControl.h Outdated
std::string name;
EntryType type;

// Human-readable interaction status. Always one of:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Always" is a strong statement. Refer to the code that creates these statuses to be able to update the comment on the code changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shortened in fe2141b. Comment now points to composeStatus() in MRUITestEngineControl.cpp as the source of truth, so it can't drift when the set of status strings changes.

Comment thread source/MRViewer/MRUIStyle.cpp Outdated
bool simulateClick = customParams.enableTestEngine && TestEngine::createButton( customParams.testEngineName.empty() ? label : customParams.testEngineName );
bool simulateClick = customParams.enableTestEngine && TestEngine::createButton(
customParams.testEngineName.empty() ? label : customParams.testEngineName,
{ .disabledReason = customParams.enabled ? std::string_view{} : std::string_view{ "inactive" } } );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably would be more readable to have a standalone variable and set the field explicitly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in fe2141bEntryAttributes attrs local with if (!customParams.enabled) attrs.disabledReason = "inactive";.

Comment thread source/MRViewer/MRUITestEngine.h Outdated
{

// Optional attributes reported to the test engine each frame alongside a widget registration.
// Passed to `createButton` / `createValue` / `createValueTentative`. All fields default to no-op.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is excess. Please note that this kind of information tends to become obsolete very easily on any code change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in fe2141b.

Comment thread source/MRViewer/MRUITestEngine.h Outdated
Comment on lines +25 to +30
// Non-empty marks the widget as disabled, with this human-readable reason ("Select an object
// first.", unmet requirements, etc.). Empty means "not explicitly disabled by caller" — the
// test engine may still infer disabled state from ImGui's CurrentItemFlags (BeginDisabled)
// and fill in a generic reason, so widgets wrapped in ImGui's native BeginDisabled don't need
// to set this. Surfaced on the listed entry and used by pressButton/writeValue to return a
// meaningful error instead of silently succeeding.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very verbose, reduce it to 5-10 words.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed to one line in fe2141b.

Comment thread source/MRViewer/MRUITestEngine.h Outdated
// Call this every frame when drawing a button you want to track (regardless of whether it returns true of false).
// If this returns true, simulate a button click.
[[nodiscard]] MRVIEWER_API bool createButton( std::string_view name );
// If this returns true, simulate a button click. See `EntryAttributes` for optional disabled-state reporting.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition is excess.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in fe2141b.

// and fill in a generic reason, so widgets wrapped in ImGui's native BeginDisabled don't need
// to set this. Surfaced on the listed entry and used by pressButton/writeValue to return a
// meaningful error instead of silently succeeding.
std::string_view disabledReason;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why std::string_view and not std::string? How long does a user have to store the reason string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept string_viewcreateButton copies the reason into the entry before returning, so the view only needs to live for the duration of the call. Added that as a one-line comment on the field in fe2141b. Happy to switch to std::string if you'd prefer the allocation over the lifetime rule.

Style-only follow-up on @oitel's review.

- MRUITestEngine.h: trim verbose EntryAttributes preamble + field doc;
  revert the extra sentence on `createButton`; add a one-line lifetime
  note on `EntryAttributes::disabledReason`.
- MRUITestEngineControl.h: shorten `TypedEntry::status` comment; point
  to `composeStatus()` in the .cpp as the source of truth so the
  description can't drift when the value set changes.
- MRUIStyle.cpp: extract `EntryAttributes attrs` local in `buttonEx`
  for readability; set `disabledReason` explicitly behind an `if`.
- MRRibbonButtonDrawer.cpp: drop the "`requirements` empty = enabled"
  comment — the call site reads itself.

No functional change. Live-verified all three `status` variants
(available / disabled:<reason> / disabled: blocked by modal '<name>')
via MCP.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Grantim Grantim merged commit 54f089c into master Apr 23, 2026
35 checks passed
Fedr pushed a commit that referenced this pull request Apr 23, 2026
…#5968)

* UI TestEngine: precise modal-blocked detection (fix #5961 regression)

The rootLevelBlocked() heuristic introduced in #5961 flagged every
root-level entry as blocked whenever any popup was open, regardless of
whether the entry itself lived *inside* the modal. This broke every
test_ui scenario that pressed a button inside a modal (e.g. `Don't
Save` in the `New scene` confirmation dialog) — pressButton threw
`disabled: blocked by modal '...'`, Python propagated, MeshInspector
hit std::terminate and exited with code 6. Caught by the ubuntu22-arm64
CI run on MeshInspectorCode#7242:

    RuntimeError: pressButton Don't Save:
        disabled: blocked by modal 'New scene##new scene'
    terminate called without an active exception

Root cause: the TestEngine tree doesn't mirror ImGui's window stack,
so a global "any popup open" check can't distinguish
"button behind the modal" from "button inside the modal."

Fix: do the check at widget registration time (where ImGui's
GetCurrentWindow() is valid) and fold the result into the existing
disabledReason string on the internal entry. A widget whose current
window is not in the topmost blocking modal's ancestor chain gets
disabledReason = "blocked by modal '<name>'" for this frame; widgets
inside the modal (or with no modal open) get empty reason.

- MRUITestEngine.cpp: effectiveDisabledReason now returns std::string
  and includes a modal-fallback branch via ImGui::GetTopMostPopupModal()
  + ParentWindow-chain walk.
- MRUITestEngineControl.cpp: rootLevelBlocked() and
  topBlockingModalName() removed; composeStatus collapses to a single
  string_view input; listEntries / pressButton / writeValue drop the
  blockingModal branch and the path.size()==1 special case.

No header changes. No API surface change — status strings still have
the same shape: "available" | "disabled: <reason>" |
"disabled: blocked by modal '<name>'".

Also switches from IsPopupOpen(AnyPopupId|AnyPopupLevel) to
GetTopMostPopupModal() so non-modal popups (tooltips, BeginPopup) no
longer count as blockers.

Verified live via MCP against the build: behind-modal root entries
report the modal-blocked status and pressButton errors cleanly, while
entries inside the modal (Toolbar Customize's `Reset to default`,
`##About`, etc.) show "available" and press successfully.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* UI TestEngine: extract imGuiBlockingModalName helper

Addresses @oitel's review on #5968: the inline
`GetTopMostPopupModal() + ParentWindow walk` was a dense block inside
`effectiveDisabledReason`. Extracted into a peer of
`imGuiContextSaysDisabled()` — returns the blocking modal's name (or
empty) so the caller can treat it as a single value. The
reason-composer now reads as a three-line if-chain.

No behavior change — live-verified that `status` and typed errors
produce identical output before/after the extraction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Grantim added a commit that referenced this pull request Apr 24, 2026
…#5976)

Refactor `Control::pressButton` / `writeValue<T>` from `Expected<void>` to `Expected<std::string>` — empty on success, non-empty `"disabled[: <reason>]"` on silent no-op. Hard errors (path/type/range) still go through `unexpected`. Python binding logs the status and does not throw (restores pre-#5961 silent contract that `test_all_scens` relies on); MCP handler translates non-empty status into a thrown `runtime_error` so its error wire-format stays identical.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Grantim Grantim deleted the ui-testengine-disabled-blocked branch May 5, 2026 14:00
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.

3 participants