diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 27561d093..5c06947aa 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -24,7 +24,7 @@ # Details * HAL 9000 has contributed spec clarifications for layer boundary DI exception, ULID scope, ACMS pipeline contracts, and TUI component interfaces (PR #10451): documented architectural invariants including the DI container exception, clarified ULID identifier scope distinguishing domain entities from internal implementation details, added per-stage protocol contracts for all 10 ACMS pipeline stages with storage tier definitions, budget enforcement protocol, and context assembly output format, and defined public interfaces with verifiable checks for 8 TUI components. - +* HAL 9000 has contributed rename of `ActorSelectionOverlay._render` to `_refresh_display` to avoid shadowing Textual Widget internal method (PR #11042). Below are some of the specific details of various contributions. diff --git a/features/steps/tui_first_run_steps.py b/features/steps/tui_first_run_steps.py index b6e9ca9db..2d1a731be 100644 --- a/features/steps/tui_first_run_steps.py +++ b/features/steps/tui_first_run_steps.py @@ -488,3 +488,99 @@ def step_persona_bar_reflects_actor(context: object) -> None: assert "anthropic/claude-sonnet-4-20250514" in bar._text, ( f"Expected actor in persona bar, got: {bar._text}" ) + + +# --------------------------------------------------------------------------- +# TDD regression: issue #11039 - no _render() shadowing Textual Widget._render +# --------------------------------------------------------------------------- + + +@then("the overlay should not override Textual Widget._render") +def step_overlay_no_render_override(context: object) -> None: + """Verify that ActorSelectionOverlay does NOT define its own ``_render``. + + The Shadowing Bug (issue #11039): ActorSelectionOverlay once defined + ``def _render(self) -> None`` which returned ``None``, shadowing + ``textual.widgets.Static._render()`` (which returns a + :class:`textual.strip.Strip`). This caused the Textual layout engine to + crash with ``AttributeError: 'NoneType' object has no attribute + 'get_height'``. + + The fix renamed the method to ``_refresh_display`` so that the inherited + base-class ``_render`` is untouched and returns a proper renderable. + """ + from cleveragents.tui.widgets.actor_selection_overlay import ( + ActorSelectionOverlay, + ) + + cls = ActorSelectionOverlay + # The overlay should NOT define its own _render in its __dict__. + # If it did, _render found on the class would resolve to this method + # rather than the Textual base-class implementation. + has_own_render = "_render" in cls.__dict__ + assert not has_own_render, ( + "ActorSelectionOverlay defines its own _render() which shadows " + "Textual Widget._render. This is the bug from issue #11039." + ) + + +@then("the overlay _refresh_display method should be callable") +def step_overlay_refresh_display_callable(context: object) -> None: + """Verify ``_refresh_display`` exists and is callable after show().""" + assert hasattr(context._overlay, "_refresh_display"), ( + "Overlay must have _refresh_display method" + ) + refresh = context._overlay._refresh_display + assert callable(refresh), "_refresh_display must be callable" + + +@then("the inherited Widget._render should return a proper renderable") +def step_inherited_render_returns_renderable(context: object) -> None: + """Verify the inherited ``Widget._render()`` returns a non-None renderable. + + Bug #11039: ``ActorSelectionOverlay`` defined its own ``_render() -> None`` + which returned ``None``, causing Textual's layout engine to crash. The + fix renamed it to ``_refresh_display()``. This assertion confirms that + when Textual IS available, calling the real inherited ``_render()`` + produces a proper renderable (not ``None``). When Textual is not + available (``_FallbackStatic`` has no ``_render``), the check passes + trivially since the fallback case cannot trigger the shadowing crash. + """ + + from cleveragents.tui.widgets.actor_selection_overlay import ( + ActorSelectionOverlay, + ) + + cls = ActorSelectionOverlay + overlay = context._overlay + + render_from_mro = None + for base in cls.__mro__[1:]: + if "_render" in base.__dict__: + render_from_mro = base.__dict__["_render"] + break + + if render_from_mro is None: + return + + result = render_from_mro(overlay) + assert result is not None, ( + "Bug #11039 regression: inherited Widget._render() returned None. " + "ActorSelectionOverlay may still be shadowing the base-class " + "_render method, which would cause a NoneType crash during " + "Textual layout in textual >=1.0." + ) + + +@then("_refresh_display should produce correct overlay content") +def step_refresh_display_produces_content(context: object) -> None: + """Invoke ``_refresh_display()`` and verify it renders correct content.""" + from typing import cast + + overlay = context._overlay + overlay._refresh_display() + content: str = cast(str, overlay._text) + assert content, "_refresh_display must produce non-empty rendered overlay content" + assert "Welcome to CleverAgents" in content, ( + "_refresh_display content must include the welcome header" + ) diff --git a/features/tui_first_run.feature b/features/tui_first_run.feature index 3d82993da..b41ba2304 100644 --- a/features/tui_first_run.feature +++ b/features/tui_first_run.feature @@ -194,3 +194,23 @@ Feature: TUI first-run experience with actor selection overlay And I call _complete_first_run with actor "anthropic/claude-sonnet-4-20250514" Then the registry should contain a persona named "default" And the persona bar should reflect the new actor + + # ----------------------------------------------------------------------- + # TDD regression: issue #11039 - no _render() shadowing Textual Widget._render + # ----------------------------------------------------------------------- + # The overlay formerly defined its own _render() returning None, which + # shadowed textual.widgets.Static._render and caused layout-engine crashes. + # This scenario ensures the rename to _refresh_display eliminated that bug. + + @tdd_issue @tdd_issue_11039 + Scenario: ActorSelectionOverlay does not define its own _render method + Given a new ActorSelectionOverlay + Then the overlay should not override Textual Widget._render + And the inherited Widget._render should return a proper renderable + + @tdd_issue @tdd_issue_11039 + Scenario: ActorSelectionOverlay has refresh_display callable instead of render + Given a new ActorSelectionOverlay + When I call show on the overlay + Then the overlay _refresh_display method should be callable + And _refresh_display should produce correct overlay content