diff --git a/docs/reports/test_full_live_workflow_progress_20260608_pm.md b/docs/reports/test_full_live_workflow_progress_20260608_pm.md new file mode 100644 index 00000000..50ce2cd6 --- /dev/null +++ b/docs/reports/test_full_live_workflow_progress_20260608_pm.md @@ -0,0 +1,94 @@ +# Progress Report: test_full_live_workflow IM_ASSERT Investigation (2026-06-08 PM) + +**Supersedes:** `docs/reports/test_full_live_workflow_imgui_assert_20260608.md` (initial root cause) +**Date:** 2026-06-08 PM +**Status:** 3 PRs landed (PR1 audit, PR2 wrap+health, PR3 pre-flight check). PR4 (real fix) deferred. Investigation continues. +**Related:** `conductor/todos/TODO_test_full_live_workflow_v2.md` + +--- + +## TL;DR + +PR2 (wrap + health endpoint) and PR3 (pre-flight health check) are landed. The pre-flight check now fails the test fast in **76s with a clear, actionable message** instead of **200s with a confusing 120s timeout**. + +**The IM_ASSERT itself is still happening**, but it's no longer silently poisoning the test. The user can now see: +- The exact RuntimeError: `IM_ASSERT( (0) && "Missing End()" ) --- imgui.cpp:11662` +- The full traceback pointing to `src/gui_2.py:619` in `app.run` +- A note that the new test cannot proceed with dirty state + +**The actual IM_ASSERT trigger (which `begin()` is missing its `end()`) is still unidentified.** The static `check_imgui_scopes.py` audit found 3 false positives. The real bug needs targeted investigation. + +--- + +## What Was Landed (3 PRs) + +### PR1: Audit Findings (no commit, documented) +- Ran `scripts/check_imgui_scopes.py` against `src/gui_2.py` +- Found 3 "extra end*" calls at lines 2920, 3843, 5455 +- All 3 are **false positives**: + - Line 2920 (`render_persona_editor_window`): the `if not is_embedded:` pattern around the matching `begin()` makes the audit script confused + - Line 3843 (`render_discussion_entry`): try/except early return path makes the audit see 2 end_groups for 1 begin_group + - Line 5455 (`render_tier_stream_panel`): same try/except pattern +- The static audit cannot find the real bug because it doesn't understand control flow + +### PR2: Wrap `immapp.run` + `/api/gui_health` endpoint +**Commit:** `1c565da7` +- `src/gui_2.py:618` — `immapp.run` is now wrapped in `try/except RuntimeError`. On IM_ASSERT: + - Logs the error to stderr at ERROR level (NOT silent) + - Records `_gui_degraded_reason` and `_last_imgui_assert` on the controller + - Returns from `run()` so the hook server keeps serving +- `src/app_controller.py` — new state attributes +- `src/api_hooks.py` — new `/api/gui_health` endpoint +- `src/api_hook_client.py` — new `get_gui_health()` method +- 4 new unit tests + 1 live test, all passing + +### PR3: Pre-flight health check in test_full_live_workflow +**Commit:** `51ecace4` +- `tests/test_live_workflow.py:43-57` — pre-flight check at start of test +- If `client.get_gui_health()['healthy']` is False, fails fast with a clear message + +**Verification (latest run, 2026-06-08 PM):** +| Test | Before | After | +|------|--------|-------| +| `test_full_live_workflow` isolation | 11.5s PASS | 13.45s PASS | +| Tier-3 batch (50 files) | 200s FAIL (timeout) | 164.9s FAIL (fast-fail in 76s) | +| Failure message | `RuntimeError: cannot schedule new futures after shutdown` (after 120s) | `Failed: GUI is degraded before test starts. degraded_reason='immapp.run raised RuntimeError: IM_ASSERT(...)'. This is likely caused by a prior test in the same live_gui session crashing the GUI.` | + +--- + +## What Is Still Needed (PR1 follow-up + PR4) + +### PR1 follow-up: Find the actual IM_ASSERT trigger +The `IM_ASSERT` in `MainDockSpace: Missing End()` says SOMETHING opened a `begin()` without a matching `end()`. After 4 sims have run their panel renders, the cumulative ImGui scope stack has an unbalanced entry. The offending render function is unknown. + +**Candidates to investigate:** +- `render_tier_stream_panel` (called by the 4 tier windows, line 5409) +- `render_execution_panel` (the execution sim opens this) +- `render_mma_modal` (MMA approval dialogs) +- `render_files_and_media` (file context panel) +- `render_persona_editor_window` (opened by ai_settings sim) +- `render_topic_picker` (MMA topic selection modal) +- `render_mma_dashboard` (the dashboard that shows the tier streams) +- The mma_state_update / mma_stream rendering code + +**The trigger is cumulative state corruption** — the bug doesn't fire on a single render. After 4 sims each opening different panels, the ImGui scope stack has a phantom `begin()` from one of them that never got its `end()`. + +### PR4: Apply the fix +Once PR1 follow-up identifies the offending function, fix it using the `imscope` context manager from `src/imgui_scopes.py` (per the `conductor/workflow.md` defer-not-catch pattern). + +### Future Track Foundation +A dedicated track for the broader test infrastructure improvements: +- Per-test auto-respawn in `live_gui` fixture (avoiding dirty state in the first place) +- Per-file fixture scope (more isolation) +- The `IM_ASSERT` itself is a render function bug that should be tracked separately + +--- + +## Next Steps + +1. **Investigate the actual IM_ASSERT trigger** via targeted log analysis +2. **Apply TDD fix** once identified +3. **Verify** test_full_live_workflow passes in tier-3 batch +4. **Write future track foundation** document + +The investigation continues. The next step is to add targeted logging to find which render function is leaving an unbalanced scope. diff --git a/src/app_controller.py b/src/app_controller.py index c7ee130f..bd0e693a 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -1231,10 +1231,30 @@ class AppController: "__reduce__", "__reduce_ex__", "__getnewargs__", ): raise AttributeError(name) - # Only return a default for the UI flag pattern (set in init_state). - # Other lazy attributes (e.g. persona_manager set in - # _load_active_project) should raise so hasattr() returns False. - if name.startswith("ui_") or name == "rag_engine": + # Only return a default for the explicit UI flag list (those defined + # in __init__ as defaults). The previous implementation returned None for + # ANY ui_ attribute, which broke hasattr() and the App.__setattr__ + # routing logic. For any other missing attribute, raise AttributeError + # so hasattr() returns False correctly. + _UI_FLAG_DEFAULTS = { + "ui_active_tool_preset", "ui_active_bias_profile", + "ui_ai_input", "ui_disc_new_name_input", "ui_disc_new_role_input", + "ui_epic_input", "ui_new_track_name", "ui_new_track_desc", + "ui_new_track_type", "ui_project_conductor_dir", + "ui_conductor_setup_summary", "ui_last_script_text", + "ui_last_script_output", "ui_new_ticket_id", "ui_new_ticket_desc", + "ui_new_ticket_target", "ui_new_ticket_deps", "ui_output_dir", + "ui_files_base_dir", "ui_shots_base_dir", "ui_project_git_dir", + "ui_project_system_prompt", "ui_project_execution_mode", + "ui_gemini_cli_path", "ui_word_wrap", "ui_auto_add_history", + "ui_separate_message_panel", "ui_separate_response_panel", + "ui_separate_tool_calls_panel", "ui_global_system_prompt", + "ui_base_system_prompt", "ui_use_default_base_prompt", + "ui_project_context_marker", "ui_agent_tools", "ui_manual_approve", + "ui_disc_truncate_pairs", "ui_auto_scroll_comms", + "ui_auto_scroll_tool_calls", "ui_focus_agent", "ui_active_persona", + } + if name in _UI_FLAG_DEFAULTS or name == "rag_engine": return None raise AttributeError(name) diff --git a/src/gui_2.py b/src/gui_2.py index fd602927..a4e9f5b3 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -688,7 +688,9 @@ class App: def __getattr__(self, name: str) -> Any: if name == 'controller': raise AttributeError(name) - return getattr(self.controller, name) + if hasattr(self, 'controller') and hasattr(self.controller, name): + return getattr(self.controller, name) + raise AttributeError(name) def __setattr__(self, name: str, value: Any) -> None: if name != 'controller' and hasattr(self, 'controller') and hasattr(self.controller, name): diff --git a/tests/test_app_controller_getattr_ui_bug.py b/tests/test_app_controller_getattr_ui_bug.py new file mode 100644 index 00000000..eaccc785 --- /dev/null +++ b/tests/test_app_controller_getattr_ui_bug.py @@ -0,0 +1,46 @@ +"""Regression tests for AppController.__getattr__ ui_* default bug. + +The bug: `AppController.__getattr__` returns `None` for any attribute +starting with `ui_` (e.g. `ui_separate_task_dag`, `ui_approve_modal_preview`). +This makes `hasattr(ctrl, 'ui_X')` return True for ANY ui_X attribute, +even if the attribute was never set on the instance. + +Consequence: `App.__setattr__` checks `hasattr(self.controller, name)` +to decide where to route assignments. With the controller's broken +__getattr__, all `ui_*` assignments get routed to the controller. +But the controller's `__getattr__` then returns None on read, +silently dropping the assigned value. + +This breaks the `if not hasattr(app, 'foo'): app.foo = False` +pattern in render_approve_script_modal (gui_2.py:4894): the hasattr +returns True (via the controller), so the assignment never runs, +and the next line tries to call checkbox with None. + +The fix: AppController.__getattr__ should only return None for +attributes that have been set (in self.__dict__). For unset +attributes, raise AttributeError so hasattr() returns False. +""" +import pytest +import sys +import os +from typing import Any + +ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +sys.path.insert(0, ROOT) + + +def test_hasattr_returns_false_for_unset_ui_attribute(mock_app: Any) -> None: + """hasattr(ctrl, 'ui_test_attr_xyz') must return False when the + attribute is not set on the instance. Otherwise the App.__setattr__ + routing logic misbehaves. + """ + app = mock_app + ctrl = app.controller + # Use a unique attribute name guaranteed not to be in init_state() + attr_name = "ui_test_attr_xyz_unique_12345" + # Force the attribute lookup + assert hasattr(ctrl, attr_name) is False, ( + f"hasattr(ctrl, {attr_name!r}) returned True for an unset attribute. " + "AppController.__getattr__ is returning None for any ui_* attribute, " + "which breaks hasattr() and the App.__setattr__ routing logic." + ) diff --git a/tests/test_app_getattr_hasattr_bug.py b/tests/test_app_getattr_hasattr_bug.py new file mode 100644 index 00000000..5a0ddc80 --- /dev/null +++ b/tests/test_app_getattr_hasattr_bug.py @@ -0,0 +1,40 @@ +"""Regression tests for the App.__getattr__ passthrough bug. + +The bug: `App.__getattr__` delegates unknown attributes to the +controller via `getattr(self.controller, name)`. If the controller +doesn't have the attribute either, `getattr` returns None (the default). + +This breaks `hasattr(app, 'unknown_attr')`: + - hasattr() checks the instance dict first + - If not found, it calls __getattr__ + - __getattr__ returns None instead of raising AttributeError + - hasattr() sees a non-exception result, returns True + - Caller's `if not hasattr(...)` is False, never initializes the attr + +The fix: __getattr__ should raise AttributeError for attributes that +the controller also doesn't have, so hasattr() works correctly. +""" +import pytest +import sys +import os +from typing import Any + +ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +sys.path.insert(0, ROOT) + + +def test_hasattr_returns_false_for_unset_attribute(mock_app: Any) -> None: + """hasattr(app, 'definitely_unique_test_attr_xyz123') must return False + when neither App nor controller has the attribute. This is the exact + pattern that fails in render_approve_script_modal (gui_2.py:4894): + `if not hasattr(app, 'foo'): app.foo = False`. + """ + app = mock_app + # Use a unique attribute name guaranteed not to exist + attr_name = "definitely_unique_test_attr_xyz123" + # Assert hasattr returns False + assert hasattr(app, attr_name) is False, ( + f"hasattr returned True for {attr_name!r} which is not set on App " + "or controller. This breaks the `if not hasattr: initialize` " + "pattern in render_approve_script_modal (gui_2.py:4894)." + )