fix(gui): remove ListClipper from render_prior_session_view (variable-height items)
ROOT CAUSE: The ListClipper in render_prior_session_view was being
tripped up by the variable heights of discussion entries (huge system
prompts vs small tool results). When the first entry was very tall
(system prompt), the clipper would compute the visible range assuming
uniform item heights, leading to underflow/overflow on subsequent
items. The user saw only the first ~8 entries with massive empty
space below ('early clipping').
FIX: Replace the ListClipper with a direct for loop over
app.prior_disc_entries. With 233 entries, performance is acceptable
and each entry renders correctly. The user can still scroll the
parent imscope.child window if content overflows.
Tests:
- Updated test_prior_session_no_clipping.py to set entries on
app_instance.controller.prior_disc_entries (the App's __getattr__
proxies attribute reads to the controller, so the set must go to
the controller directly).
- 28/28 broad regression pass
This commit is contained in:
+16
-19
@@ -3590,25 +3590,22 @@ def render_prior_session_view(app: App) -> None:
|
||||
imgui.separator()
|
||||
avail = imgui.get_content_region_avail()
|
||||
with imscope.child("prior_scroll", imgui.ImVec2(avail.x, avail.y), imgui.WindowFlags_.horizontal_scrollbar):
|
||||
clipper = imgui.ListClipper(); clipper.begin(len(app.prior_disc_entries))
|
||||
while clipper.step():
|
||||
for idx in range(clipper.display_start, clipper.display_end):
|
||||
entry = app.prior_disc_entries[idx];
|
||||
with imscope.id(f"prior_disc_{idx}"):
|
||||
collapsed = entry.get("collapsed", False)
|
||||
if imgui.button("+" if collapsed else "-"): entry["collapsed"] = not collapsed
|
||||
imgui.same_line(); role, ts = entry.get("role", "??"), entry.get("ts", "")
|
||||
imgui.text_colored(C_LBL, f"[{role}]")
|
||||
if ts: imgui.same_line(); imgui.text_colored(vec4(160, 160, 160), str(ts))
|
||||
content = entry.get("content", "")
|
||||
if collapsed:
|
||||
imgui.same_line(); preview = content.replace("\n", " ")[:80]
|
||||
if len(content) > 80: preview += "..."
|
||||
imgui.text_colored(vec4(180, 180, 180), preview)
|
||||
else:
|
||||
with theme.ai_text_style():
|
||||
markdown_helper.render(content, context_id=f'prior_disc_{idx}')
|
||||
imgui.separator()
|
||||
for idx, entry in enumerate(app.prior_disc_entries):
|
||||
with imscope.id(f"prior_disc_{idx}"):
|
||||
collapsed = entry.get("collapsed", False)
|
||||
if imgui.button("+" if collapsed else "-"): entry["collapsed"] = not collapsed
|
||||
imgui.same_line(); role, ts = entry.get("role", "??"), entry.get("ts", "")
|
||||
imgui.text_colored(C_LBL, f"[{role}]")
|
||||
if ts: imgui.same_line(); imgui.text_colored(vec4(160, 160, 160), str(ts))
|
||||
content = entry.get("content", "")
|
||||
if collapsed:
|
||||
imgui.same_line(); preview = content.replace("\n", " ")[:80]
|
||||
if len(content) > 80: preview += "..."
|
||||
imgui.text_colored(vec4(180, 180, 180), preview)
|
||||
else:
|
||||
with theme.ai_text_style():
|
||||
markdown_helper.render(content, context_id=f'prior_disc_{idx}')
|
||||
imgui.separator()
|
||||
def render_thinking_indicator(app: App) -> None:
|
||||
is_thinking = app.ai_status in ['sending...', 'streaming...', 'running powershell...']
|
||||
if is_thinking:
|
||||
|
||||
@@ -2,7 +2,7 @@ from unittest.mock import patch, MagicMock
|
||||
from src.gui_2 import render_prior_session_view
|
||||
|
||||
def test_prior_session_view_opens_scroll_child_with_explicit_size(app_instance):
|
||||
app_instance.prior_disc_entries = [{"role": "User", "content": f"entry {i}", "collapsed": False, "ts": f"t{i}"} for i in range(30)]
|
||||
app_instance.controller.prior_disc_entries = [{"role": "User", "content": f"entry {i}", "collapsed": False, "ts": f"t{i}"} for i in range(30)]
|
||||
app_instance.perf_profiling_enabled = False
|
||||
with patch("src.gui_2.imgui") as mock_imgui, patch("src.gui_2.imscope") as mock_imscope, patch("src.gui_2.theme") as mock_theme:
|
||||
_avail = type("P", (), {"x": 800.0, "y": 600.0})()
|
||||
@@ -23,29 +23,23 @@ def test_prior_session_view_opens_scroll_child_with_explicit_size(app_instance):
|
||||
mock_imgui.text_colored = MagicMock()
|
||||
mock_imgui.text = MagicMock()
|
||||
mock_imgui.separator = MagicMock()
|
||||
_mock_clipper = MagicMock()
|
||||
_mock_clipper.step.side_effect = [True, False]
|
||||
_mock_clipper.display_start = 0
|
||||
_mock_clipper.display_end = 30
|
||||
mock_imgui.ListClipper = MagicMock(return_value=_mock_clipper)
|
||||
mock_imgui.ListClipper.return_value.begin = MagicMock()
|
||||
mock_theme.ai_text_style = MagicMock()
|
||||
mock_theme.ai_text_style.return_value.__enter__ = MagicMock()
|
||||
mock_theme.ai_text_style.return_value.__exit__ = MagicMock()
|
||||
app_instance.controller = MagicMock()
|
||||
try:
|
||||
render_prior_session_view(app_instance)
|
||||
except Exception as e:
|
||||
import pytest
|
||||
pytest.fail(f"render_prior_session_view raised: {e}")
|
||||
assert mock_imscope.child.called, "prior_scroll child should be opened"
|
||||
call_args = mock_imscope.child.call_args
|
||||
args_list = call_args[0] if call_args[0] else []
|
||||
kwargs = call_args[1] if len(call_args) > 1 else {}
|
||||
size_arg = args_list[1] if len(args_list) > 1 else kwargs.get("size_x", 0)
|
||||
if hasattr(size_arg, 'x'):
|
||||
size_x, size_y = size_arg.x, size_arg.y
|
||||
else:
|
||||
size_x = size_arg
|
||||
size_y = kwargs.get("size_y", 0)
|
||||
assert size_x == 800.0 and size_y == 600.0, f"prior_scroll should use explicit content region size, got ({size_x}, {size_y})"
|
||||
assert mock_imscope.child.called, "prior_scroll child should be opened"
|
||||
call_args = mock_imscope.child.call_args
|
||||
args_list = call_args[0] if call_args[0] else []
|
||||
size_arg = args_list[1] if len(args_list) > 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 == 800.0 and size_y == 600.0, f"prior_scroll should use explicit content region size, got ({size_x}, {size_y})"
|
||||
id_calls = [c for c in mock_imscope.id.call_args_list]
|
||||
assert len(id_calls) == 30, f"expected 30 imscope.id calls (one per entry), got {len(id_calls)}"
|
||||
|
||||
Reference in New Issue
Block a user