diff options
| author | t <t@tjp.lol> | 2026-06-10 14:57:31 -0600 |
|---|---|---|
| committer | t <t@tjp.lol> | 2026-06-10 15:28:16 -0600 |
| commit | cee08918509689adbdbd26b6384d5ef3a47e91a2 (patch) | |
| tree | ce2c6bd2269a086fa34edc682696bdf12d9e5b23 /src/tui_engine.zig | |
| parent | ea4bff482374fa8bce38636311fa582e3c87ad47 (diff) | |
fix: eliminate scrollback-clearing flash when streaming with expanded tool output
The root cause was a two-part bug:
1. Engine slot management (tui_engine.zig): The app rebuilds its full
component list on every new transcript entry. The old approach drained
every slot then re-added them all, resetting every component's render
baseline. When earlier content had scrolled into native scrollback
(viewport_top > 0), the engine's viewport-escape rule escalated any
cut above the viewport top to a scrollback-clearing full redraw — the
visible flash on every streamed block while a tool call was expanded.
Fix: replace drain+rebuild with syncComponents(), which reconciles the
live slot list in place by finding the longest common prefix and suffix
(matched by component pointer). Prefix and suffix slots keep their
baselines verbatim; only the changed region in the middle gets new
empty slots. A pure insertion (the dominant case: a new transcript
entry appended just before the pinned input+footer suffix) stays on
the differential path with no clear. A structural change (slot
replaced or removed) sets force_full for correctness.
2. RenderCache dirty signal (tui_component.zig): firstLineChanged()
was returning the retained changed_from diff index even when the cache
was clean. A first-paint store records changed_from == 0, so a clean
but previously-rendered component was pegging the differential cut to
line 0 on every subsequent frame, making unchanged slots above the
insertion point drag the cut all the way up. Fix: when the cache is
clean, firstLineChanged() returns null unconditionally. The recorded
changed_from is now internal render bookkeeping only, not a live
signal.
Additional changes:
- ToolUse.collapsed_tail_lines raised from 5 to 8 lines.
- ToolUse collapsed-view test updated to use 12 output lines and
collapsed_tail_lines symbolically so the numbers self-document.
- tui_app.zig: override-slot test comment and assertion clarified to
reflect that the override owns the slot pointer (not just renders it),
and the SWAPPED-AT-DETAILS render check replaced with a pointer-equality
check.
- lua_event_bridge.zig: minor related fixes.
- New tests: syncComponents middle-insertion no-clear, syncComponents
structural removal forces full, syncComponents override swap; streaming
expanded-tools no-clear regression test in tui_app.
Diffstat (limited to 'src/tui_engine.zig')
| -rw-r--r-- | src/tui_engine.zig | 176 |
1 files changed, 176 insertions, 0 deletions
diff --git a/src/tui_engine.zig b/src/tui_engine.zig index 59bce06..6c5b372 100644 --- a/src/tui_engine.zig +++ b/src/tui_engine.zig @@ -350,6 +350,89 @@ pub const Engine = struct { return self.slots.items.len; } + /// Reconcile the live slot list against `comps` IN PLACE, preserving the + /// render baseline of every slot whose component pointer is unchanged. + /// + /// This is the structure-aware replacement for "drain every slot + re-add + /// all" (which reset EVERY baseline and forced a full redraw on every + /// transcript mutation). The app rebuilds its component list on each new + /// entry, but the overwhelmingly common shape is an APPEND at the end + /// (a new transcript entry) with all earlier slots unchanged. Resetting + /// the unchanged slots' baselines turned that append into a full redraw, + /// and when earlier content had scrolled into native scrollback + /// (`viewport_top > 0`) the viewport-escape rule escalated it to a + /// SCROLLBACK-CLEARING redraw — the visible "flash" on every streamed + /// block while tool output was expanded. + /// + /// Reconciliation anchors on the longest common PREFIX and the longest + /// common SUFFIX (both matched by `comp.ptr`); everything between them is + /// the changed region. + /// - Prefix and suffix slots keep their baseline verbatim (no re-render, + /// no forced full): unchanged components above/below the change point + /// stay clean and reprint from cache only if they sit below the cut. + /// - PURE INSERTION (prefix + suffix account for every OLD slot; the new + /// list only ADDS items in the middle): the inserted slots get EMPTY + /// baselines, so they first-render at their own offset and the cut + /// lands at the insertion point. This covers the dominant case — a new + /// transcript entry appended just before the pinned input box + footer + /// suffix. The frame stays on the differential path with NO clear, and + /// `force_full` is deliberately NOT set; that is the whole point. + /// - STRUCTURAL CHANGE (a slot in the changed region is REPLACED or + /// REMOVED — an override swap or a transcript removal): `force_full` is + /// set. Correctness for scrolled content is still guaranteed by the + /// viewport-escape rule in `render` (a cut above `viewport_top` + /// escalates to a clearing redraw); only the rare mid-list mutation + /// pays that cost, not every append. + pub fn syncComponents(self: *Engine, comps: []const Component) !void { + const old = self.slots.items; + + // Longest common prefix by component pointer. + var pre: usize = 0; + const pre_max = @min(old.len, comps.len); + while (pre < pre_max and old[pre].comp.ptr == comps[pre].ptr) pre += 1; + + // Longest common suffix, not overlapping the prefix on either side. + var suf: usize = 0; + const suf_max = @min(old.len - pre, comps.len - pre); + while (suf < suf_max and + 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. + const old_changed = old.len - suf - pre; // old slots dropped + const structural_change = old_changed != 0; + + // Detach the preserved suffix slots (with their baselines) before we + // mutate the middle, then re-attach them after the inserted slots. + var suffix_slots: std.ArrayList(Slot) = .empty; + defer suffix_slots.deinit(self.alloc); + try suffix_slots.ensureTotalCapacity(self.alloc, suf); + { + var k: usize = 0; + while (k < suf) : (k += 1) suffix_slots.appendAssumeCapacity(old[old.len - suf + k]); + } + + // Pop everything from the divergence point down to the prefix. The + // top `suf` entries are the preserved suffix (copied out above — pop + // WITHOUT freeing their lines, they will be re-attached). The next + // `old_changed` entries are genuinely replaced/removed — free them. + var popped: usize = 0; + while (self.slots.items.len > pre) : (popped += 1) { + var removed = self.slots.pop().?; + if (popped >= suf) self.freeSlotLines(&removed); + } + + // Insert the new middle components with empty baselines, then restore + // the preserved suffix slots. + for (comps[pre .. comps.len - suf]) |comp| { + try self.slots.append(self.alloc, .{ .comp = comp }); + } + for (suffix_slots.items) |slot| try self.slots.append(self.alloc, slot); + + if (structural_change) self.force_full = true; + } + // -- resize ------------------------------------------------------------ /// Apply a new terminal size. A width change (wrapping changes) or height @@ -1399,6 +1482,99 @@ test "change above viewport_top forces a full redraw" { try testing.expect(std.mem.indexOf(u8, out, terminal.seq.full_clear) != null); } +test "syncComponents: middle insertion before a pinned suffix does NOT clear scrollback (the expanded-tools flash fix)" { + // The regression: the app rebuilds its component list on every new + // transcript entry. The desired list is [..transcript.., input, footer], + // so a new entry is INSERTED just before the pinned input+footer suffix. + // The old drain+rebuild reset every baseline and forced a full redraw; + // with scrolled content (viewport_top > 0) that escalated to a + // scrollback-CLEARING redraw — the visible flash. syncComponents must keep + // the unchanged prefix/suffix baselines and stay on the differential path. + var buf = std.Io.Writer.Allocating.init(testing.allocator); + defer buf.deinit(); + // Viewport tall enough that the insertion point (transcript tail, just + // above the 2-line pinned suffix) stays visible, but short enough that the + // tall tool body has scrolled some lines into native scrollback + // (viewport_top > 0). This mirrors the real app: the new entry lands at + // the bottom, below viewport_top, so it MUST stay on the differential path. + var eng = makeEngine(&buf, 80, 6); + defer eng.deinit(); + + // A tall "tool" body, plus pinned input + footer (the suffix). Each + // unchanged component reports firstLineChanged 0 on first paint, then null + // (clean) on later frames — the real RenderCache contract. Preserved slots + // must therefore contribute NO cut, so the insert stays at the tail. + var tool = FakeComponent{ .scripts = &.{ &.{ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7" }, &.{ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7" } }, .first_changed = &.{ 0, null } }; + var input = FakeComponent{ .scripts = &.{ &.{"input"}, &.{"input"} }, .first_changed = &.{ 0, null } }; + var footer = FakeComponent{ .scripts = &.{ &.{"f0"}, &.{"f0"} }, .first_changed = &.{ 0, null } }; + try eng.syncComponents(&.{ tool.comp(), input.comp(), footer.comp() }); + try eng.render(); // first paint + try testing.expect(eng.viewport_top > 0); + + // Advance the preserved components to their clean (null) signal. + tool.advance(); + input.advance(); + footer.advance(); + + // Insert a new entry between the tool body and the pinned input/footer. + var entry = FakeComponent{ .scripts = &.{&.{ "new0", "new1" }}, .first_changed = &.{0} }; + buf.clearRetainingCapacity(); + try eng.syncComponents(&.{ tool.comp(), entry.comp(), input.comp(), footer.comp() }); + try eng.render(); + + const out = buf.written(); + // The inserted content is drawn, but WITHOUT a scrollback wipe. + try testing.expect(std.mem.indexOf(u8, out, "new0") != null); + try testing.expect(std.mem.indexOf(u8, out, terminal.seq.full_clear) == null); + // The unchanged tool body was NOT re-rendered (its baseline survived): + // only the first paint rendered it. + try testing.expectEqual(@as(usize, 1), tool.render_calls); +} + +test "syncComponents: removal in the middle forces a full redraw (structural change)" { + var buf = std.Io.Writer.Allocating.init(testing.allocator); + defer buf.deinit(); + var eng = makeEngine(&buf, 80, 100); // tall: nothing scrolls + defer eng.deinit(); + + var a = FakeComponent{ .scripts = &.{&.{"a"}}, .first_changed = &.{0} }; + var b = FakeComponent{ .scripts = &.{&.{"b"}}, .first_changed = &.{0} }; + var c = FakeComponent{ .scripts = &.{&.{"c"}}, .first_changed = &.{0} }; + try eng.syncComponents(&.{ a.comp(), b.comp(), c.comp() }); + try eng.render(); + try testing.expectEqual(@as(usize, 3), eng.componentCount()); + + // Remove the middle component => structural change => force_full. + buf.clearRetainingCapacity(); + try eng.syncComponents(&.{ a.comp(), c.comp() }); + try testing.expect(eng.force_full); + try eng.render(); + try testing.expectEqual(@as(usize, 2), eng.componentCount()); + const out = buf.written(); + try testing.expect(std.mem.indexOf(u8, out, "b") == null); +} + +test "syncComponents: replacing a slot (override swap) resets only that baseline" { + var buf = std.Io.Writer.Allocating.init(testing.allocator); + defer buf.deinit(); + var eng = makeEngine(&buf, 80, 100); + defer eng.deinit(); + + var a = FakeComponent{ .scripts = &.{&.{"a"}}, .first_changed = &.{0} }; + var b = FakeComponent{ .scripts = &.{&.{"b"}}, .first_changed = &.{0} }; + var b2 = FakeComponent{ .scripts = &.{&.{"b2"}}, .first_changed = &.{0} }; + try eng.syncComponents(&.{ a.comp(), b.comp() }); + try eng.render(); + + // Swap b -> b2 at the same position: structural change (pointer differs). + buf.clearRetainingCapacity(); + try eng.syncComponents(&.{ a.comp(), b2.comp() }); + try testing.expect(eng.force_full); + try eng.render(); + const out = buf.written(); + try testing.expect(std.mem.indexOf(u8, out, "b2") != null); +} + test "width overflow is a hard error" { var buf = std.Io.Writer.Allocating.init(testing.allocator); defer buf.deinit(); |
