From 96b9b00c97834a8d5190edb2b38fcbae1dba896a Mon Sep 17 00:00:00 2001 From: Ed_ Date: Wed, 3 Jun 2026 14:53:05 -0400 Subject: [PATCH] fix(gui): use imscope.child for comms_scroll (was inside conditional, leaving child open) ROOT CAUSE: render_comms_history_panel had imgui.end_child() nested INSIDE an 'if app._scroll_comms_to_bottom:' block at line 3758. When _scroll_comms_to_bottom was False (the common case), end_child was NOT called, leaving the comms_scroll child window open. This caused the imGui state to corrupt: tab_item.end_tab_item, tab_bar.end_tab_bar, and the outer window.end all saw that the child was still open (WithinEndChildID was set), triggering 'Must call EndChild() and not End()!' assertion. FIX: Convert the entire comms_scroll block to imscope.child (which uses Python's with statement for exception-safe end_child). The scroll-to-bottom logic is now correctly nested INSIDE the with block, and there's no manual end_child to forget. Tests: - Updated test_comms_scroll_no_clipping.py to check imscope.child instead of begin_child - 28/28 broad regression pass --- src/gui_2.py | 142 ++++++++++++------------- tests/test_comms_scroll_no_clipping.py | 28 ++--- 2 files changed, 85 insertions(+), 85 deletions(-) diff --git a/src/gui_2.py b/src/gui_2.py index 6d898b4e..b2038e5c 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -3676,88 +3676,86 @@ def render_comms_history_panel(app: App) -> None: imgui.separator() avail = imgui.get_content_region_avail() - imgui.begin_child("comms_scroll", imgui.ImVec2(avail.x, avail.y), False, imgui.WindowFlags_.horizontal_scrollbar) - log_to_render = app._comms_log_cache - - for i, entry in enumerate(log_to_render): - imgui.push_id(f"comms_entry_{i}") + with imscope.child("comms_scroll", imgui.ImVec2(avail.x, avail.y), imgui.WindowFlags_.horizontal_scrollbar): + log_to_render = app._comms_log_cache - i_display = i + 1 - ts = entry.get("ts", "00:00:00") - direction = entry.get("direction", "??") - kind = entry.get("kind", entry.get("type", "??")) - provider = entry.get("provider", "?") - model = entry.get("model", "?") - tier = entry.get("source_tier", "main") - payload = entry.get("payload", {}) - if not payload and kind not in ("request", "response", "tool_call", "tool_result"): - payload = entry # legacy + for i, entry in enumerate(log_to_render): + imgui.push_id(f"comms_entry_{i}") + + i_display = i + 1 + ts = entry.get("ts", "00:00:00") + direction = entry.get("direction", "??") + kind = entry.get("kind", entry.get("type", "??")) + provider = entry.get("provider", "?") + model = entry.get("model", "?") + tier = entry.get("source_tier", "main") + payload = entry.get("payload", {}) + if not payload and kind not in ("request", "response", "tool_call", "tool_result"): + payload = entry # legacy - # Row 1: #Idx TS DIR KIND Provider/Model [Tier] - imgui.text_colored(C_LBL, f"#{i_display}"); imgui.same_line() - imgui.text_colored(vec4(160, 160, 160), ts) + # Row 1: #Idx TS DIR KIND Provider/Model [Tier] + imgui.text_colored(C_LBL, f"#{i_display}"); imgui.same_line() + imgui.text_colored(vec4(160, 160, 160), ts) - latency = entry.get("latency") or entry.get("metadata", {}).get("latency") - if latency: + latency = entry.get("latency") or entry.get("metadata", {}).get("latency") + if latency: + imgui.same_line() + imgui.text_colored(C_SUB, f" ({latency:.2f}s)") + + ticket_id = entry.get("mma_ticket_id") + if ticket_id: + imgui.same_line() + imgui.text_colored(vec4(255, 120, 120), f"[{ticket_id}]") imgui.same_line() - imgui.text_colored(C_SUB, f" ({latency:.2f}s)") - - ticket_id = entry.get("mma_ticket_id") - if ticket_id: - imgui.same_line() - imgui.text_colored(vec4(255, 120, 120), f"[{ticket_id}]") - imgui.same_line() - d_col = DIR_COLORS.get(direction, C_VAL) - imgui.text_colored(d_col, direction); imgui.same_line() - k_col = KIND_COLORS.get(kind, C_VAL) - imgui.text_colored(k_col, kind); imgui.same_line() - imgui.text_colored(C_LBL, f"{provider}/{model}"); imgui.same_line() - imgui.text_colored(C_SUB, f"[{tier}]") + d_col = DIR_COLORS.get(direction, C_VAL) + imgui.text_colored(d_col, direction); imgui.same_line() + k_col = KIND_COLORS.get(kind, C_VAL) + imgui.text_colored(k_col, kind); imgui.same_line() + imgui.text_colored(C_LBL, f"{provider}/{model}"); imgui.same_line() + imgui.text_colored(C_SUB, f"[{tier}]") - # Optimized content rendering using _render_heavy_text logic - idx_str = str(i) - if kind == "request": - usage = payload.get("usage", {}) - if usage: - inp = usage.get("input_tokens", 0) - imgui.text_colored(C_LBL, f" tokens in:{inp}") - render_heavy_text(app, "message", payload.get("message", ""), idx_str) - if payload.get("system"): - render_heavy_text(app, "system", payload.get("system", ""), idx_str) - elif kind == "response": - r = payload.get("round", 0) - sr = payload.get("stop_reason", "STOP") - usage = payload.get("usage", {}) - usage_str = "" - if usage: - inp = usage.get("input_tokens", 0) - out = usage.get("output_tokens", 0) - cache = usage.get("cache_read_input_tokens", 0) - usage_str = f" in:{inp} out:{out}" - if cache: usage_str += f" cache:{cache}" - imgui.text_colored(C_LBL, f"round: {r} stop_reason: {sr}{usage_str}") + # Optimized content rendering using _render_heavy_text logic + idx_str = str(i) + if kind == "request": + usage = payload.get("usage", {}) + if usage: + inp = usage.get("input_tokens", 0) + imgui.text_colored(C_LBL, f" tokens in:{inp}") + render_heavy_text(app, "message", payload.get("message", ""), idx_str) + if payload.get("system"): + render_heavy_text(app, "system", payload.get("system", ""), idx_str) + elif kind == "response": + r = payload.get("round", 0) + sr = payload.get("stop_reason", "STOP") + usage = payload.get("usage", {}) + usage_str = "" + if usage: + inp = usage.get("input_tokens", 0) + out = usage.get("output_tokens", 0) + cache = usage.get("cache_read_input_tokens", 0) + usage_str = f" in:{inp} out:{out}" + if cache: usage_str += f" cache:{cache}" + imgui.text_colored(C_LBL, f"round: {r} stop_reason: {sr}{usage_str}") - text_content = payload.get("text", "") - segments, parsed_response = thinking_parser.parse_thinking_trace(text_content) - if segments: render_thinking_trace(app, payload, [{"content": s.content, "marker": s.marker} for s in segments], i, is_standalone=not bool(parsed_response.strip())) - if parsed_response: render_heavy_text(app, "text", parsed_response, idx_str) + text_content = payload.get("text", "") + segments, parsed_response = thinking_parser.parse_thinking_trace(text_content) + if segments: render_thinking_trace(app, payload, [{"content": s.content, "marker": s.marker} for s in segments], i, is_standalone=not bool(parsed_response.strip())) + if parsed_response: render_heavy_text(app, "text", parsed_response, idx_str) - tcs = payload.get("tool_calls", []) - if tcs: render_heavy_text(app, "tool_calls", json.dumps(tcs, indent=1), idx_str) + tcs = payload.get("tool_calls", []) + if tcs: render_heavy_text(app, "tool_calls", json.dumps(tcs, indent=1), idx_str) - elif kind == "tool_call": render_heavy_text(app, payload.get("name", "call"), payload.get("script") or json.dumps(payload.get("args", {}), indent=1), idx_str) - elif kind == "tool_result": render_heavy_text(app, payload.get("name", "result"), payload.get("output", ""), idx_str) - else: render_heavy_text(app, "data", str(payload), idx_str) + elif kind == "tool_call": render_heavy_text(app, payload.get("name", "call"), payload.get("script") or json.dumps(payload.get("args", {}), indent=1), idx_str) + elif kind == "tool_result": render_heavy_text(app, payload.get("name", "result"), payload.get("output", ""), idx_str) + else: render_heavy_text(app, "data", str(payload), idx_str) + imgui.separator() + imgui.pop_id() - imgui.separator() - imgui.pop_id() + if app._scroll_comms_to_bottom: + imgui.set_scroll_here_y(1.0) + app._scroll_comms_to_bottom = False - if app._scroll_comms_to_bottom: - imgui.set_scroll_here_y(1.0) - app._scroll_comms_to_bottom = False - - imgui.end_child() - if app.perf_profiling_enabled: app.perf_monitor.end_component("_render_comms_history_panel") + if app.perf_profiling_enabled: app.perf_monitor.end_component("_render_comms_history_panel") def render_takes_panel(app: App) -> None: imgui.text("Takes & Synthesis") diff --git a/tests/test_comms_scroll_no_clipping.py b/tests/test_comms_scroll_no_clipping.py index c4ee4145..c218591e 100644 --- a/tests/test_comms_scroll_no_clipping.py +++ b/tests/test_comms_scroll_no_clipping.py @@ -18,8 +18,10 @@ def test_comms_history_renders_all_entries_not_just_early_subset(app_instance): mock_imgui.push_style_color = MagicMock() mock_imgui.pop_style_color = MagicMock() mock_imgui.set_scroll_here_y = MagicMock() - mock_imgui.get_content_region_avail = MagicMock(return_value=MagicMock(x=800.0, y=600.0)) - mock_imgui.ImVec2 = lambda *a: ("ImVec2", a) + mock_imgui.get_content_region_avail = MagicMock(return_value=type("P", (), {"x": 800.0, "y": 600.0})()) + def _imvec2(x, y=0): + m = MagicMock(); m.x = float(x); m.y = float(y); return m + mock_imgui.ImVec2 = _imvec2 mock_imgui.ImVec4 = lambda *a: ("ImVec4", a) mock_imscope.child = MagicMock() mock_imscope.child.return_value.__enter__ = MagicMock() @@ -35,14 +37,14 @@ def test_comms_history_renders_all_entries_not_just_early_subset(app_instance): except Exception as e: import pytest pytest.fail(f"render_comms_history_panel raised: {e}") - comms_calls = [call for call in mock_imgui.begin_child.call_args_list if call[0][0] == "comms_scroll"] - assert len(comms_calls) == 1, "comms_scroll child should be opened once" - comms_call = next((c for c in mock_imgui.begin_child.call_args_list if c[0][0] == "comms_scroll"), None) - assert comms_call is not None, "comms_scroll child should be among begin_child calls" - args, _ = comms_call - size_arg = args[1] - if isinstance(size_arg, tuple) and len(size_arg) == 2 and isinstance(size_arg[0], str): - actual = size_arg[1] - else: - actual = size_arg - assert actual != (0, 0), f"comms_scroll child should use explicit content region size, got {actual}" + comms_calls = [call for call in mock_imscope.child.call_args_list if call[0][0] == "comms_scroll"] + assert len(comms_calls) == 1, "comms_scroll child should be opened once" + comms_call = comms_calls[0] + args, _ = comms_call + size_arg = args[1] if len(args) > 1 else 0 + if hasattr(size_arg, 'x'): + size_x, size_y = size_arg.x, size_arg.y + else: + size_x = size_arg + size_y = 0 + assert size_x > 0 and size_y > 0, f"comms_scroll child should use explicit content region size, got ({size_x}, {size_y})"