From 480f33b4cd1cad6eccc746cd5edc49054bc271d4 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 20:56:11 +0000 Subject: [PATCH 1/3] docs: add discussion dialog AI visibility design --- ...-discussion-dialog-ai-visibility-design.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-08-discussion-dialog-ai-visibility-design.md diff --git a/docs/superpowers/specs/2026-04-08-discussion-dialog-ai-visibility-design.md b/docs/superpowers/specs/2026-04-08-discussion-dialog-ai-visibility-design.md new file mode 100644 index 00000000..0066d34c --- /dev/null +++ b/docs/superpowers/specs/2026-04-08-discussion-dialog-ai-visibility-design.md @@ -0,0 +1,59 @@ +# Discussion Dialog AI Visibility Design + +## Goal +When no AI explanation has been generated yet, the discussion dialog should not show the AI summary field or model-information field. The dialog should show only the `Explain with AI` button. After the user requests an explanation, the AI summary textbox should appear. If generation succeeds, the textbox shows the generated explanation and the regenerate button appears. If generation fails, the textbox should still appear and show the error message. + +## Recommended approach +Use the existing controls and change only their visibility and state. + +Why this approach: +- minimal layout risk in an existing wx dialog +- preserves current button handlers and async flow +- avoids rebuilding controls dynamically, which is more fragile for accessibility and focus behavior + +## UI behavior + +### Initial state +- Show the forecast discussion section as today. +- Hide the plain-language summary label. +- Hide the plain-language summary textbox. +- Hide the model-information label and textbox. +- Show `Explain with AI`. +- Hide `Regenerate Explanation`. + +### During generation +- Reveal the plain-language summary label and textbox. +- Put `Generating plain language summary...` into the AI textbox. +- Keep model-information controls hidden until success. +- Keep `Regenerate Explanation` hidden. +- Disable `Explain with AI` while the request is running. + +### On success +- Keep the AI summary controls visible. +- Fill the AI textbox with the generated explanation. +- Reveal model-information controls. +- Reveal `Regenerate Explanation`. +- Hide `Explain with AI`, since regenerate becomes the follow-up action. + +### On failure +- Keep the AI summary controls visible. +- Fill the AI textbox with the error message. +- Keep model-information controls hidden. +- Reveal `Regenerate Explanation` so the user can retry immediately. +- Hide `Explain with AI` after the first attempt, for consistency with the success state. + +## Implementation notes +- Add helper methods for AI section visibility/state so the dialog does not scatter `Show/Hide/Layout` calls across multiple handlers. +- Keep existing async explanation flow intact. +- Preserve current screen-reader-friendly naming on the text controls. +- Re-layout the dialog after visibility changes. + +## Testing +- Add/update dialog tests to cover: + - initial state hides AI summary and model info + - starting explanation reveals summary area with loading text + - success reveals summary, model info, and regenerate button while hiding explain + - failure reveals summary with error text, keeps model info hidden, and shows regenerate + +## Scope +This change is limited to the discussion dialog presentation and its tests. It does not change explanation generation, model selection, or prompt behavior. From 80216166d4e9a67c69bc46cf74bebad733153472 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 20:58:50 +0000 Subject: [PATCH 2/3] docs: add discussion dialog AI visibility plan --- ...6-04-08-discussion-dialog-ai-visibility.md | 316 ++++++++++++++++++ 1 file changed, 316 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-08-discussion-dialog-ai-visibility.md diff --git a/docs/superpowers/plans/2026-04-08-discussion-dialog-ai-visibility.md b/docs/superpowers/plans/2026-04-08-discussion-dialog-ai-visibility.md new file mode 100644 index 00000000..0db9c91b --- /dev/null +++ b/docs/superpowers/plans/2026-04-08-discussion-dialog-ai-visibility.md @@ -0,0 +1,316 @@ +# Discussion Dialog AI Visibility Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Hide the AI summary and model-info fields in the discussion dialog until the user requests an explanation, then reveal them appropriately for loading, success, and error states. + +**Architecture:** Keep the existing wx controls and event flow, but centralize visibility/state transitions in small helper methods on `DiscussionDialog`. Drive the change with tests first so the dialog's initial, loading, success, and error states are locked down before touching implementation. + +**Tech Stack:** Python, wxPython, pytest, unittest.mock + +--- + +## File map + +- Modify: `src/accessiweather/ui/dialogs/discussion_dialog.py` + - Add helper methods to show/hide the AI section and synchronize button visibility. + - Update initial, loading, success, and error states. +- Modify: `tests/test_discussion_dialog.py` + - Add focused tests for the dialog state transitions. + +### Task 1: Lock down the desired dialog states with tests + +**Files:** +- Modify: `tests/test_discussion_dialog.py` +- Test: `tests/test_discussion_dialog.py` + +- [ ] **Step 1: Write the failing tests** + +Add focused state tests like this near the discussion-dialog tests: + +```python +class _VisibleControl: + def __init__(self): + self.visible = True + self.enabled = True + self.value = "" + + def Show(self): + self.visible = True + + def Hide(self): + self.visible = False + + def IsShown(self): + return self.visible + + def Enable(self): + self.enabled = True + + def Disable(self): + self.enabled = False + + def SetValue(self, value): + self.value = value + + +class _FakeSizer: + def __init__(self): + self.layout_calls = 0 + + def Layout(self): + self.layout_calls += 1 + + +def _build_dialog_state(): + dialog = SimpleNamespace() + dialog.explanation_header = _VisibleControl() + dialog.explanation_display = _VisibleControl() + dialog.model_info_label = _VisibleControl() + dialog.model_info = _VisibleControl() + dialog.explain_button = _VisibleControl() + dialog.regenerate_button = _VisibleControl() + dialog._sizer = _FakeSizer() + dialog.GetSizer = lambda: dialog._sizer + dialog._set_status = MagicMock() + return dialog + + +def test_setup_initial_state_hides_ai_fields(): + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog.discussion_display = _VisibleControl() + dialog.explain_button.Disable() + + discussion_dialog.DiscussionDialog._setup_initial_state(dialog) + + assert dialog.explanation_header.IsShown() is False + assert dialog.explanation_display.IsShown() is False + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is False + + +def test_on_explain_reveals_summary_area_with_loading_text(): + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._current_discussion = "Forecast text" + dialog._is_explaining = False + dialog.app = SimpleNamespace(run_async=MagicMock()) + dialog._do_explain = MagicMock(return_value="task") + + discussion_dialog.DiscussionDialog._on_explain(dialog, None) + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert dialog.explanation_display.value == "Generating plain language summary..." + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is False + + +def test_on_explain_complete_shows_summary_model_info_and_regenerate(): + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._is_explaining = True + + discussion_dialog.DiscussionDialog._on_explain_complete( + dialog, + "Plain explanation", + "openrouter/auto", + 123, + 0.0, + False, + ) + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert dialog.explanation_display.value == "Plain explanation" + assert dialog.model_info_label.IsShown() is True + assert dialog.model_info.IsShown() is True + assert dialog.regenerate_button.IsShown() is True + assert dialog.explain_button.IsShown() is False + + +def test_on_explain_error_shows_error_text_and_regenerate_only(): + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._is_explaining = True + + discussion_dialog.DiscussionDialog._on_explain_error(dialog, "boom") + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert "Failed to generate explanation: boom" in dialog.explanation_display.value + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is True + assert dialog.explain_button.IsShown() is False +``` + +- [ ] **Step 2: Run the new tests to verify they fail** + +Run: + +```bash +pytest tests/test_discussion_dialog.py -k "setup_initial_state_hides_ai_fields or on_explain_reveals_summary_area_with_loading_text or on_explain_complete_shows_summary_model_info_and_regenerate or on_explain_error_shows_error_text_and_regenerate_only" -v +``` + +Expected: FAIL because `DiscussionDialog` does not yet hide/show the AI controls that way. + +- [ ] **Step 3: Commit the failing-test checkpoint only if you want an explicit red-state commit** + +```bash +git add tests/test_discussion_dialog.py +git commit -m "test: cover discussion dialog AI visibility states" +``` + +### Task 2: Implement the dialog state helpers and wire them into the existing flow + +**Files:** +- Modify: `src/accessiweather/ui/dialogs/discussion_dialog.py` +- Test: `tests/test_discussion_dialog.py` + +- [ ] **Step 1: Write the minimal implementation helpers** + +Update widget creation so the explanation header is stored on `self`, then add helpers like this: + +```python +self.explanation_header = wx.StaticText(panel, label="Plain Language Summary:") +main_sizer.Add(self.explanation_header, 0, wx.LEFT | wx.RIGHT, 10) +``` + +```python +def _layout_dialog(self) -> None: + sizer = self.GetSizer() + if sizer: + sizer.Layout() + + +def _show_ai_summary_section(self) -> None: + self.explanation_header.Show() + self.explanation_display.Show() + + +def _hide_ai_summary_section(self) -> None: + self.explanation_header.Hide() + self.explanation_display.Hide() + + +def _show_model_info(self) -> None: + self.model_info_label.Show() + self.model_info.Show() + + +def _hide_model_info(self) -> None: + self.model_info_label.Hide() + self.model_info.Hide() + + +def _set_post_explain_buttons(self, has_attempted_explanation: bool) -> None: + if has_attempted_explanation: + self.explain_button.Hide() + self.regenerate_button.Show() + else: + self.explain_button.Show() + self.regenerate_button.Hide() + self._layout_dialog() +``` + +- [ ] **Step 2: Update the initial state to hide AI controls before first generation** + +Change `_setup_initial_state()` to: + +```python +def _setup_initial_state(self) -> None: + self.discussion_display.SetValue("Loading...") + self.explanation_display.SetValue("") + self.model_info.SetValue("") + self._hide_ai_summary_section() + self._hide_model_info() + self._set_post_explain_buttons(has_attempted_explanation=False) + self.explain_button.Disable() +``` + +- [ ] **Step 3: Update the loading, success, and error handlers** + +Adjust `_on_explain()`, `_on_explain_complete()`, and `_on_explain_error()` like this: + +```python +def _on_explain(self, event) -> None: + if not self._current_discussion or self._is_explaining: + return + + self._is_explaining = True + self._show_ai_summary_section() + self._hide_model_info() + self.regenerate_button.Hide() + self.explain_button.Disable() + self.explanation_display.SetValue("Generating plain language summary...") + self._layout_dialog() + self._set_status("Generating AI explanation...") + self.app.run_async(self._do_explain()) +``` + +```python +def _on_explain_complete(...): + self._is_explaining = False + self._show_ai_summary_section() + self.explanation_display.SetValue(explanation) + ... + self._show_model_info() + self._set_post_explain_buttons(has_attempted_explanation=True) + self.regenerate_button.Enable() + self._set_status(f"Explanation generated using {model_used}.") +``` + +```python +def _on_explain_error(self, error: str) -> None: + self._is_explaining = False + self._show_ai_summary_section() + self._hide_model_info() + self.explanation_display.SetValue( + f"Failed to generate explanation: {error}\n\n" + "Please check your OpenRouter API key in Settings." + ) + self._set_post_explain_buttons(has_attempted_explanation=True) + self.regenerate_button.Enable() + self._set_status("Explanation failed.") +``` + +- [ ] **Step 4: Run the focused tests to verify they pass** + +Run: + +```bash +pytest tests/test_discussion_dialog.py -k "setup_initial_state_hides_ai_fields or on_explain_reveals_summary_area_with_loading_text or on_explain_complete_shows_summary_model_info_and_regenerate or on_explain_error_shows_error_text_and_regenerate_only" -v +``` + +Expected: PASS + +- [ ] **Step 5: Run the full discussion-dialog test file** + +Run: + +```bash +pytest tests/test_discussion_dialog.py -v +``` + +Expected: PASS + +- [ ] **Step 6: Commit the implementation** + +```bash +git add src/accessiweather/ui/dialogs/discussion_dialog.py tests/test_discussion_dialog.py +git commit -m "feat: streamline discussion dialog AI visibility" +``` + +## Self-review + +- Spec coverage: initial hidden AI state, loading reveal, success reveal, failure reveal, and regenerate behavior are all covered by Tasks 1 and 2. +- Placeholder scan: no TODO/TBD placeholders remain. +- Type consistency: helper and method names match the dialog file's current structure and use existing wx control names. From a4704fe6bafcd485fda5b478ea47b04fb9dab2f6 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 21:29:20 +0000 Subject: [PATCH 3/3] feat(ui): hide discussion AI summary until requested --- .../ui/dialogs/discussion_dialog.py | 69 +++++++-- tests/test_discussion_dialog.py | 133 ++++++++++++++++++ 2 files changed, 190 insertions(+), 12 deletions(-) diff --git a/src/accessiweather/ui/dialogs/discussion_dialog.py b/src/accessiweather/ui/dialogs/discussion_dialog.py index b2d2bcb3..667cd059 100644 --- a/src/accessiweather/ui/dialogs/discussion_dialog.py +++ b/src/accessiweather/ui/dialogs/discussion_dialog.py @@ -76,8 +76,8 @@ def _create_widgets(self) -> None: main_sizer.Add(self.discussion_display, 1, wx.ALL | wx.EXPAND, 10) # AI Explanation section - explanation_header = wx.StaticText(panel, label="Plain Language Summary:") - main_sizer.Add(explanation_header, 0, wx.LEFT | wx.RIGHT, 10) + self.explanation_header = wx.StaticText(panel, label="Plain Language Summary:") + main_sizer.Add(self.explanation_header, 0, wx.LEFT | wx.RIGHT, 10) self.explanation_display = wx.TextCtrl( panel, @@ -135,12 +135,54 @@ def _bind_events(self) -> None: def _setup_initial_state(self) -> None: """Set up initial state.""" self.discussion_display.SetValue("Loading...") - self.explanation_display.SetValue( - "Click 'Explain with AI' to generate a plain language summary.\n\n" - "Note: Requires an OpenRouter API key configured in Settings." - ) + self.explanation_display.SetValue("") + self.model_info.SetValue("") + DiscussionDialog._hide_ai_summary_section(self) + DiscussionDialog._hide_model_info(self) + DiscussionDialog._set_post_explain_buttons(self, has_attempted_explanation=False) self.explain_button.Disable() + def _layout_dialog(self) -> None: + """Refresh dialog layout after visibility changes.""" + sizer = self.GetSizer() + if sizer: + sizer.Layout() + + def _show_ai_summary_section(self) -> None: + """Show the AI summary header and textbox.""" + self.explanation_header.Show() + self.explanation_display.Show() + DiscussionDialog._layout_dialog(self) + + def _hide_ai_summary_section(self) -> None: + """Hide the AI summary header and textbox.""" + self.explanation_header.Hide() + self.explanation_display.Hide() + DiscussionDialog._layout_dialog(self) + + def _show_model_info(self) -> None: + """Show model information controls.""" + self.model_info_label.Show() + self.model_info.Show() + DiscussionDialog._layout_dialog(self) + + def _hide_model_info(self) -> None: + """Hide model information controls and clear stale text.""" + self.model_info.SetValue("") + self.model_info_label.Hide() + self.model_info.Hide() + DiscussionDialog._layout_dialog(self) + + def _set_post_explain_buttons(self, has_attempted_explanation: bool) -> None: + """Toggle explain/regenerate buttons based on explanation history.""" + if has_attempted_explanation: + self.explain_button.Hide() + self.regenerate_button.Show() + else: + self.explain_button.Show() + self.regenerate_button.Hide() + DiscussionDialog._layout_dialog(self) + def _set_status(self, message: str) -> None: """Update the status label.""" self.status_label.SetLabel(message) @@ -262,6 +304,9 @@ def _on_explain(self, event) -> None: return self._is_explaining = True + DiscussionDialog._show_ai_summary_section(self) + DiscussionDialog._hide_model_info(self) + DiscussionDialog._set_post_explain_buttons(self, has_attempted_explanation=False) self.explain_button.Disable() self.explanation_display.SetValue("Generating plain language summary...") self._set_status("Generating AI explanation...") @@ -330,17 +375,15 @@ def _on_explain_complete( ) -> None: """Handle explanation completion.""" self._is_explaining = False - self.explain_button.Enable() + DiscussionDialog._show_ai_summary_section(self) + DiscussionDialog._set_post_explain_buttons(self, has_attempted_explanation=True) self.explanation_display.SetValue(explanation) cost_text = "No cost" if estimated_cost == 0 else f"~${estimated_cost:.6f}" info = f"Model: {model_used}\nTokens: {token_count}\nCost: {cost_text}" if cached: info += "\nCached: Yes" self.model_info.SetValue(info) - self.model_info_label.Show() - self.model_info.Show() - self.regenerate_button.Show() - self.GetSizer().Layout() + DiscussionDialog._show_model_info(self) self._set_status(f"Explanation generated using {model_used}.") def _on_regenerate(self, event) -> None: @@ -353,7 +396,9 @@ def _on_regenerate(self, event) -> None: def _on_explain_error(self, error: str) -> None: """Handle explanation error.""" self._is_explaining = False - self.explain_button.Enable() + DiscussionDialog._show_ai_summary_section(self) + DiscussionDialog._hide_model_info(self) + DiscussionDialog._set_post_explain_buttons(self, has_attempted_explanation=True) self.explanation_display.SetValue( f"Failed to generate explanation: {error}\n\n" "Please check your OpenRouter API key in Settings." diff --git a/tests/test_discussion_dialog.py b/tests/test_discussion_dialog.py index ef0fd34a..44b3b564 100644 --- a/tests/test_discussion_dialog.py +++ b/tests/test_discussion_dialog.py @@ -358,6 +358,139 @@ def test_discussion_starts_with_error_message(self, mock_app): assert is_error is True +# ============================================================================= +# Dialog Visibility State Tests +# ============================================================================= + + +class _VisibleControl: + def __init__(self): + self.visible = True + self.enabled = True + self.value = "" + + def Show(self): + self.visible = True + + def Hide(self): + self.visible = False + + def IsShown(self): + return self.visible + + def Enable(self): + self.enabled = True + + def Disable(self): + self.enabled = False + + def SetValue(self, value): + self.value = value + + +class _FakeSizer: + def __init__(self): + self.layout_calls = 0 + + def Layout(self): + self.layout_calls += 1 + + +def _build_dialog_state(): + dialog = SimpleNamespace() + dialog.explanation_header = _VisibleControl() + dialog.explanation_display = _VisibleControl() + dialog.model_info_label = _VisibleControl() + dialog.model_info = _VisibleControl() + dialog.explain_button = _VisibleControl() + dialog.regenerate_button = _VisibleControl() + dialog.discussion_display = _VisibleControl() + dialog._sizer = _FakeSizer() + dialog.GetSizer = lambda: dialog._sizer + dialog._set_status = MagicMock() + return dialog + + +class TestDiscussionDialogVisibilityStates: + def test_setup_initial_state_hides_ai_fields(self): + """Test initial dialog state hides AI content until requested.""" + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog.explain_button.Disable() + + discussion_dialog.DiscussionDialog._setup_initial_state(dialog) + + assert dialog.explanation_header.IsShown() is False + assert dialog.explanation_display.IsShown() is False + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is False + assert dialog.explain_button.IsShown() is True + + def test_on_explain_reveals_summary_area_with_loading_text(self): + """Test explanation start reveals summary area and loading state.""" + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._current_discussion = "Forecast text" + dialog._is_explaining = False + dialog.app = SimpleNamespace(run_async=MagicMock()) + dialog._do_explain = MagicMock(return_value="task") + + discussion_dialog.DiscussionDialog._on_explain(dialog, None) + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert dialog.explanation_display.value == "Generating plain language summary..." + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is False + assert dialog.explain_button.IsShown() is True + assert dialog.explain_button.enabled is False + + def test_on_explain_complete_shows_summary_model_info_and_regenerate(self): + """Test successful explanation shows summary, model info, and regenerate.""" + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._is_explaining = True + + discussion_dialog.DiscussionDialog._on_explain_complete( + dialog, + "Plain explanation", + "openrouter/auto", + 123, + 0.0, + False, + ) + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert dialog.explanation_display.value == "Plain explanation" + assert dialog.model_info_label.IsShown() is True + assert dialog.model_info.IsShown() is True + assert dialog.regenerate_button.IsShown() is True + assert dialog.explain_button.IsShown() is False + + def test_on_explain_error_shows_error_text_and_regenerate_only(self): + """Test failed explanation keeps summary visible and offers retry.""" + from accessiweather.ui.dialogs import discussion_dialog + + dialog = _build_dialog_state() + dialog._is_explaining = True + + discussion_dialog.DiscussionDialog._on_explain_error(dialog, "boom") + + assert dialog.explanation_header.IsShown() is True + assert dialog.explanation_display.IsShown() is True + assert "Failed to generate explanation: boom" in dialog.explanation_display.value + assert dialog.model_info_label.IsShown() is False + assert dialog.model_info.IsShown() is False + assert dialog.regenerate_button.IsShown() is True + assert dialog.explain_button.IsShown() is False + + # ============================================================================= # Model Configuration Tests # =============================================================================