From 5e76e4aa90e70245404c10085e809880635572a0 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 23 Jun 2026 11:20:45 -0600 Subject: Fix tool override swaps to stay differential Treat 1:1 component swaps and pure insertions as differential TUI updates instead of structural changes that force full redraws. Keep full redraws for net removals and edits above the viewport, and add tests covering the streaming-tool flicker regression and scrollback behavior. Also remove the unfinished Go ToolSource batch-registration bridge. --- src/tui_engine.zig | 71 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 7 deletions(-) (limited to 'src/tui_engine.zig') diff --git a/src/tui_engine.zig b/src/tui_engine.zig index f63e269..4924347 100644 --- a/src/tui_engine.zig +++ b/src/tui_engine.zig @@ -398,10 +398,14 @@ pub const Engine = struct { old[old.len - 1 - suf].comp.ptr == comps[comps.len - 1 - suf].ptr) suf += 1; // OLD slots in [pre, old.len - suf) are replaced/removed; NEW comps in - // [pre, comps.len - suf) are inserted. A pure insertion replaces zero - // old slots; anything else is a structural change. + // [pre, comps.len - suf) are inserted. const old_changed = old.len - suf - pre; // old slots dropped - const structural_change = old_changed != 0; + const new_inserted = comps.len - suf - pre; // new slots added + // Only a NET REMOVAL forces a full redraw. A pure insertion or a 1:1 + // in-place replacement (override swap) stays differential: the + // replaced/inserted slot first-renders from an empty baseline, so the + // cut lands at THAT slot's offset (clear_to_end clears any orphaned tail). + const structural_change = new_inserted < old_changed; // Detach the preserved suffix slots (with their baselines) before we // mutate the middle, then re-attach them after the inserted slots. @@ -1499,7 +1503,15 @@ test "height change forces a full redraw" { try testing.expect(std.mem.indexOf(u8, out, terminal.seq.full_clear) != null); } -test "change above viewport_top forces a full redraw" { +test "change above viewport_top forces a full redraw (keeps scrollback honest)" { + // A change to a line that has scrolled into native scrollback (above + // viewport_top) cannot be patched in place — the cursor can't reach it. + // The ONLY way to make scrollback reflect the change is a clearing full + // redraw (\x1b[3J wipes history) + reprint from the top. We accept that + // cost to keep scrollback honest; the differential path can't address + // those lines. (Tool INPUT streaming does not hit this: the tool is small + // and in-view while args stream — it has no output yet — so the changing + // header stays at/below viewport_top.) var buf = std.Io.Writer.Allocating.init(testing.allocator); defer buf.deinit(); // Short viewport so most content scrolls into native scrollback. @@ -1595,7 +1607,13 @@ test "syncComponents: removal in the middle forces a full redraw (structural cha try testing.expect(std.mem.indexOf(u8, out, "b") == null); } -test "syncComponents: replacing a slot (override swap) resets only that baseline" { +test "syncComponents: replacing a slot (override swap) stays differential, no reprint-from-top" { + // A 1:1 in-place swap (e.g. a tool-renderer override installed on a + // `tool_delta`) must NOT force a full reprint. The replaced slot + // first-renders from an empty baseline, so the differential cut lands at + // THAT slot's offset and only the slot (and anything below it) repaints. + // Forcing a full redraw here reprinted the whole transcript from line 0 on + // every delta — the streaming-tool flicker. var buf = std.Io.Writer.Allocating.init(testing.allocator); defer buf.deinit(); var eng = makeEngine(&buf, 80, 100); @@ -1607,15 +1625,54 @@ test "syncComponents: replacing a slot (override swap) resets only that baseline try eng.syncComponents(&.{ a.comp(), b.comp() }); try eng.render(); - // Swap b -> b2 at the same position: structural change (pointer differs). + // Swap b -> b2 at the same position (pointer differs, count unchanged). buf.clearRetainingCapacity(); try eng.syncComponents(&.{ a.comp(), b2.comp() }); - try testing.expect(eng.force_full); + try testing.expect(!eng.force_full); // NOT a full redraw try eng.render(); const out = buf.written(); try testing.expect(std.mem.indexOf(u8, out, "b2") != null); } +test "syncComponents: override swap with scrolled content does not reprint from the top (tool-stream flicker)" { + // The real streaming-tool flicker. A tool-renderer override is re-installed + // on every `tool_delta`; the Lua bridge mints a fresh component per + // `set_component`, so the App swaps the slot every delta. With earlier + // content scrolled into native scrollback (viewport_top > 0), forcing a + // full redraw reprinted the ENTIRE conversation from line 0 each delta — + // the visible flash. The swap must stay differential and never re-emit the + // scrolled-away prefix. + var buf = std.Io.Writer.Allocating.init(testing.allocator); + defer buf.deinit(); + // Short viewport so the header scrolls off: total 9 lines, height 8 => + // viewport_top 1. The swapped tool slot stays in view (offset 6 >= 1). + var eng = makeEngine(&buf, 80, 8); + defer eng.deinit(); + + // [tall header h0..h5][tool][input][footer]; header h0 scrolls off the top. + var header = FakeComponent{ .scripts = &.{&.{ "h0", "h1", "h2", "h3", "h4", "h5" }}, .first_changed = &.{0} }; + var tool = FakeComponent{ .scripts = &.{&.{"tool_v0"}}, .first_changed = &.{0} }; + var tool2 = FakeComponent{ .scripts = &.{&.{"tool_v1"}}, .first_changed = &.{0} }; + var input = FakeComponent{ .scripts = &.{&.{"input"}}, .first_changed = &.{0} }; + var footer = FakeComponent{ .scripts = &.{&.{"f0"}}, .first_changed = &.{0} }; + + try eng.syncComponents(&.{ header.comp(), tool.comp(), input.comp(), footer.comp() }); + try eng.render(); + try testing.expect(eng.viewport_top > 0); + + // Swap the tool component (the per-delta override re-install). + buf.clearRetainingCapacity(); + try eng.syncComponents(&.{ header.comp(), tool2.comp(), input.comp(), footer.comp() }); + try eng.render(); + + const out = buf.written(); + // No screen clear and — the flicker signature — no reprint from line 0: + // the scrolled-away header top is never re-emitted. + try testing.expect(std.mem.indexOf(u8, out, terminal.seq.full_clear) == null); + try testing.expect(std.mem.indexOf(u8, out, "h0") == null); + try testing.expect(std.mem.indexOf(u8, out, "tool_v1") != null); // the swap shows +} + test "width overflow is a hard error" { var buf = std.Io.Writer.Allocating.init(testing.allocator); defer buf.deinit(); -- cgit v1.3