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 | |
| 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.
| -rw-r--r-- | src/lua_event_bridge.zig | 15 | ||||
| -rw-r--r-- | src/tui_app.zig | 216 | ||||
| -rw-r--r-- | src/tui_component.zig | 52 | ||||
| -rw-r--r-- | src/tui_components.zig | 64 | ||||
| -rw-r--r-- | src/tui_engine.zig | 176 |
5 files changed, 464 insertions, 59 deletions
diff --git a/src/lua_event_bridge.zig b/src/lua_event_bridge.zig index f6d67e0..c4acb1d 100644 --- a/src/lua_event_bridge.zig +++ b/src/lua_event_bridge.zig @@ -797,9 +797,11 @@ test "Lua-defined component renders lines through the bridged vtable" { try testing.expectEqual(@as(usize, 2), lines.len); try testing.expectEqualStrings("hello", lines[0]); try testing.expectEqualStrings("world", lines[1]); - // firstLineChanged is cache-derived: first render dirties from 0. - try testing.expectEqual(@as(?usize, 0), chosen.?.firstLineChanged()); - // A second identical render reports no change. + // firstLineChanged is cache-derived and is the LIVE signal: after a render + // the cache is clean, so it reports null (the first-render diff index 0 is + // retained internally as bookkeeping only). + try testing.expectEqual(@as(?usize, null), chosen.?.firstLineChanged()); + // A second identical render still reports no change. _ = try chosen.?.render(80, testing.allocator); try testing.expectEqual(@as(?usize, null), chosen.?.firstLineChanged()); } @@ -1258,10 +1260,11 @@ test "bridged firstLineChanged is cache-derived: append stays near the tail" { try bridge.attachBus(&bus); const chosen = bus.fire("tool", null, .{ .tool = .{} }).?; - // First render: 2 lines, dirty from 0. + // First render: 2 lines. After the render the cache is clean, so the live + // signal is null (the first-render diff index 0 is internal bookkeeping). _ = try chosen.render(80, testing.allocator); - try testing.expectEqual(@as(?usize, 0), chosen.firstLineChanged()); - // No change: null. + try testing.expectEqual(@as(?usize, null), chosen.firstLineChanged()); + // No change: still null. _ = try chosen.render(80, testing.allocator); try testing.expectEqual(@as(?usize, null), chosen.firstLineChanged()); diff --git a/src/tui_app.zig b/src/tui_app.zig index dbcf8f9..81894fa 100644 --- a/src/tui_app.zig +++ b/src/tui_app.zig @@ -458,17 +458,20 @@ pub const App = struct { /// transcript layout changes (a layout change forces a full redraw inside /// the engine, which is correct here). fn rebuildEngineList(self: *App) !void { - // Clear and re-add. `removeComponent` is O(n) per call, so clear by - // re-initializing the slot list via repeated pops is awkward; instead - // remove the pinned components, then append the new entry, then re-add - // the pinned ones. To keep it simple and correct we drain & rebuild. - while (self.engine.componentCount() > 0) { - const first = self.engine.slots.items[0].comp; - _ = self.engine.removeComponent(first); - } - for (self.transcript.items) |e| try self.engine.addComponent(e.comp()); - try self.engine.addComponent(self.input_box.comp()); - try self.engine.addComponent(self.footer.comp()); + // Build the desired component list (transcript entries top-to-bottom, + // then the pinned input box and footer) and hand it to the engine to + // reconcile IN PLACE. `syncComponents` preserves the baseline of every + // unchanged slot, so the dominant case — appending a new transcript + // entry — stays on the differential path instead of forcing a full + // (and, for scrolled content, scrollback-clearing) redraw. See + // `Engine.syncComponents`. + var comps: std.ArrayList(Component) = .empty; + defer comps.deinit(self.alloc); + try comps.ensureTotalCapacity(self.alloc, self.transcript.items.len + 2); + for (self.transcript.items) |e| comps.appendAssumeCapacity(e.comp()); + comps.appendAssumeCapacity(self.input_box.comp()); + comps.appendAssumeCapacity(self.footer.comp()); + try self.engine.syncComponents(comps.items); } /// Spawn a new assistant-text entry for the given block index and return @@ -649,6 +652,69 @@ pub const App = struct { return box; } + /// Seed the transcript from a resumed `Conversation` (the `--resume` path). + /// Walks every stored message top-to-bottom and spawns the matching + /// transcript entry with its content set wholesale — the same components + /// the live event stream would have produced, but materialized in one shot + /// from history rather than streamed. + /// + /// Tool results correlate back to their `ToolUse` component by + /// `tool_use_id` via the router's id map (populated by `spawnTool` + + /// `putToolId`), exactly like the live `tool_dispatch_complete` path. A + /// user-role message that carries ONLY `ToolResult` blocks is the + /// tool-output carrier, not a user bubble, so it feeds the matching tool + /// boxes instead of rendering as user text. `System` blocks are not part of + /// the visible transcript and are skipped. + /// + /// Call once at startup, BEFORE the welcome banner / first paint, so the + /// engine's first frame already contains the full restored history. + pub fn seedFromConversation(self: *App, conv: *const panto.Conversation) !void { + for (conv.messages.items) |msg| { + for (msg.content.items) |block| { + switch (block) { + .Text => |tb| { + if (tb.items.len == 0) continue; + switch (msg.role) { + .assistant => { + const box = try self.spawnAssistant(0); + try box.setText(tb.items); + }, + .user => try self.spawnUser(tb.items), + // System text never renders in the transcript. + .system => {}, + } + }, + .Thinking => |th| { + if (th.text.items.len == 0) continue; + const box = try self.spawnThinking(0); + try box.setText(th.text.items); + }, + .ToolUse => |tu| { + const box = try self.spawnTool(0); + try box.setName(tu.name); + try box.setInput(tu.input.items); + try self.router.putToolId(tu.id, box); + }, + .ToolResult => |tr| { + const box = self.router.getToolById(tr.tool_use_id) orelse continue; + var text: std.ArrayList(u8) = .empty; + defer text.deinit(self.alloc); + try tr.appendTextInto(self.alloc, &text); + try box.setOutput(text.items); + box.setResultOk(!tr.is_error); + }, + .CompactionSummary => |cs| { + _ = try self.spawnCompaction(cs.text.items); + }, + .System => {}, + } + } + } + // The id map was a replay scratchpad; clear it so the first live turn + // starts clean (mirrors `beginTurn`). + self.router.reset(); + } + /// Toggle the global tool-use collapse state (ctrl+o) and apply it to every /// tool-use component in the transcript. No "active component": we iterate /// the whole list and flip each one. Requests a render. @@ -1068,6 +1134,10 @@ pub fn runLoop(app: *App, term: *Terminal, opts: RunOptions) !void { if (opts.version.len != 0) try welcome.setVersion(opts.version); } + // On `--resume`, materialize the restored conversation into transcript + // components below the banner, so the first paint shows the full history. + try app.seedFromConversation(&opts.agent.conversation); + app.input_box.setFocused(true); try app.rebuildEngineList(); try app.renderNow(); @@ -1665,6 +1735,118 @@ test "routeEvent: an unmatched tool_use_id is ignored, matched siblings still ro try testing.expectEqualStrings("real", known.output.?.items); } +test "seedFromConversation materializes resumed history into transcript components" { + const alloc = testing.allocator; + const h = try Harness.make(alloc); + defer h.teardown(alloc); + + // Build a resumed conversation: system (skipped), user, assistant text + + // a tool call, the tool result (in a user-role carrier), and a final + // assistant reply. + var conv = panto.Conversation.init(alloc); + defer conv.deinit(); + try conv.addSystemMessage("you are a helpful assistant"); // must NOT render + try conv.addUserMessage(&.{.{ .Text = try panto.textualBlockFromSlice(alloc, "hello there") }}); + try conv.addAssistantMessage(&.{ + .{ .Text = try panto.textualBlockFromSlice(alloc, "on it") }, + .{ .ToolUse = .{ + .id = try alloc.dupe(u8, "call-1"), + .name = try alloc.dupe(u8, "bash"), + .input = try panto.textualBlockFromSlice(alloc, "{\"cmd\":\"ls\"}"), + } }, + }, null); + { + var parts: std.ArrayList(panto.ResultPartStored) = .empty; + try parts.append(alloc, .{ .text = try panto.textualBlockFromSlice(alloc, "file-a\nfile-b") }); + try conv.addUserMessage(&.{.{ .ToolResult = .{ + .tool_use_id = try alloc.dupe(u8, "call-1"), + .parts = parts, + } }}); + } + try conv.addAssistantMessage(&.{ + .{ .Text = try panto.textualBlockFromSlice(alloc, "done") }, + }, null); + + try h.app.seedFromConversation(&conv); + + // Entries: user, assistant("on it"), tool, assistant("done"). The system + // message and the tool-result carrier produce no standalone entries. + try testing.expectEqual(@as(usize, 4), h.app.transcript.items.len); + try testing.expect(h.app.transcript.items[0].kind == .user); + try testing.expectEqualStrings("hello there", h.app.transcript.items[0].kind.user.buffer.items); + try testing.expect(h.app.transcript.items[1].kind == .assistant); + try testing.expectEqualStrings("on it", h.app.transcript.items[1].kind.assistant.buffer.items); + try testing.expect(h.app.transcript.items[2].kind == .tool); + const tool = h.app.transcript.items[2].kind.tool; + try testing.expectEqualStrings("bash", tool.name.?.items); + try testing.expect(std.mem.indexOf(u8, tool.input.items, "ls") != null); + // The tool result correlated by id and set the output + success state. + try testing.expect(tool.output != null); + try testing.expect(std.mem.indexOf(u8, tool.output.?.items, "file-b") != null); + try testing.expectEqual(@as(?bool, true), tool.result_ok); + try testing.expect(h.app.transcript.items[3].kind == .assistant); + try testing.expectEqualStrings("done", h.app.transcript.items[3].kind.assistant.buffer.items); + + // The id map was a replay scratchpad and is cleared afterward. + try testing.expect(h.app.router.getToolById("call-1") == null); + + // It renders through the real engine without error. + try h.app.rebuildEngineList(); + try h.app.renderNow(); + const out = h.buf.written(); + try testing.expect(std.mem.indexOf(u8, out, "hello there") != null); + try testing.expect(std.mem.indexOf(u8, out, "done") != null); +} + +test "streaming a new block with expanded tools stays differential (no scrollback flash)" { + // Regression for the expanded-tool-call flash: with tall (scrolled) + // content, appending a new transcript entry must NOT trigger a + // scrollback-clearing full redraw. Before the syncComponents fix this + // emitted a full_clear on the first frame after each new block. + const alloc = testing.allocator; + const h = try Harness.make(alloc); + defer h.teardown(alloc); + // Short engine so content scrolls => viewport_top > 0 when expanded. + h.engine.resize(80, 12); + h.app.input_box.setFocused(true); + try h.app.rebuildEngineList(); + + // A tool call with a big multi-line output, expanded via ctrl+o. + try h.app.routeEvent(.{ .block_start = .{ .block_type = .ToolUse, .index = 0 } }); + const tool = h.app.router.get(0).?.tool; + try tool.setName("bash"); + var big: std.ArrayList(u8) = .empty; + defer big.deinit(alloc); + var i: usize = 0; + while (i < 40) : (i += 1) { + var lb: [32]u8 = undefined; + try big.appendSlice(alloc, std.fmt.bufPrint(&lb, "output line {d}\n", .{i}) catch unreachable); + } + try tool.setOutput(big.items); + tool.setResultOk(true); + h.app.toggleToolCollapse(); + try testing.expect(!tool.collapsed); + try h.app.renderNow(); + try testing.expect(h.engine.viewport_top > 0); + + // Open a new assistant text block and stream into it. No frame across the + // new-block boundary or the deltas may clear the scrollback. + var clears: usize = 0; + h.buf.clearRetainingCapacity(); + try h.app.routeEvent(.{ .block_start = .{ .block_type = .Text, .index = 1 } }); + try h.app.renderNow(); + if (std.mem.indexOf(u8, h.buf.written(), terminal_mod.seq.full_clear) != null) clears += 1; + + const chunks = [_][]const u8{ "Lorem ", "ipsum ", "dolor ", "sit ", "amet " }; + for (chunks) |c| { + h.buf.clearRetainingCapacity(); + try h.app.routeEvent(delta(1, c)); + try h.app.renderNow(); + if (std.mem.indexOf(u8, h.buf.written(), terminal_mod.seq.full_clear) != null) clears += 1; + } + try testing.expectEqual(@as(usize, 0), clears); +} + test "toggleToolCollapse flips every tool component globally" { const alloc = testing.allocator; const h = try Harness.make(alloc); @@ -2141,12 +2323,18 @@ test "event wiring: mid-stream swap at tool_details takes over and keeps driving // Args were driven into the default box too. try testing.expect(std.mem.indexOf(u8, box.input.items, "path") != null); - // The override still renders (not the default), even after the result - // drove the default box. + // The override still owns the slot (not the default), even after the + // result drove the default box. Delivering the result dirtied the DEFAULT + // box, but the override is what renders that slot and it did NOT change, + // so the next differential frame must NOT repaint the default's content: + // the swapped-in marker stays on screen and "the-output" never appears. + // (Under the incremental model an unchanged slot is not re-emitted; we + // assert the slot is still the override and that the default content is + // absent from the frame.) + try testing.expect(h.app.transcript.items[0].override.?.ptr == marker.comp().ptr); h.buf.clearRetainingCapacity(); try h.app.renderNow(); out = h.buf.written(); - try testing.expect(std.mem.indexOf(u8, out, "SWAPPED-AT-DETAILS") != null); try testing.expect(std.mem.indexOf(u8, out, "the-output") == null); } diff --git a/src/tui_component.zig b/src/tui_component.zig index a2b0ea4..b213e3f 100644 --- a/src/tui_component.zig +++ b/src/tui_component.zig @@ -119,7 +119,16 @@ pub const Focusable = struct { /// 2. records the lowest differing index as the "changed-from" value, /// 3. replaces the cache with a copy of the new lines, /// 4. marks the cache clean. -/// - After a clean render with no diff, `firstLineChanged()` returns null. +/// - Once the cache is CLEAN, `firstLineChanged()` returns null regardless +/// of the diff `store` recorded. `firstLineChanged()` is the LIVE +/// pre-render dirty signal (the engine reads it once per slot BEFORE +/// `render` to decide whether to re-render and where to cut); a clean +/// component is already up to date on screen and has nothing pending, so +/// it must report null. The recorded diff index is retained separately in +/// `changed_from` as render bookkeeping and is NOT a live signal — exposing +/// it while clean would peg the differential cut to a stale line on every +/// later frame (a first-paint store records `changed_from == 0`, which +/// would otherwise force the cut to line 0 forever). /// /// Because `firstLineChanged` is computed purely from cache state, it cannot /// drift from a hand-managed field. Components embed this struct and route @@ -132,8 +141,11 @@ pub const RenderCache = struct { /// Owned copies of the last rendered lines, or null when never rendered or /// invalidated. lines: ?[][]u8 = null, - /// Lowest line index that changed at the last `store`, or 0 when dirty - /// with no prior cache. Null only when clean with no change. + /// Lowest line index that changed at the last `store` (or the pending + /// dirty line while dirty; 0 when dirty with no prior cache). This is + /// render bookkeeping, NOT the live signal: when the cache is clean, + /// `firstLineChanged()` returns null even though this still holds the last + /// diff index. Null only when a clean store found no change. changed_from: ?usize = null, dirty: bool = true, @@ -202,8 +214,17 @@ pub const RenderCache = struct { /// Derived dirty signal. Returns the lowest changed line index, or null /// when the cache is clean and nothing changed at the last store. pub fn firstLineChanged(self: *const RenderCache) ?usize { + // The engine reads this as the PRE-render dirty signal (once per slot, + // before `render`). A clean cache has no pending change to repaint, so + // it must report null — even though the last `store` recorded a + // `changed_from` diff index. Returning that retained index while clean + // makes an unchanged component spuriously cut the differential repaint + // at its stale diff line on EVERY later frame (notably a first-paint + // store leaves `changed_from == 0`, which would peg the cut to line 0 + // forever). The retained `changed_from` is internal render bookkeeping, + // not a live signal; only the dirty state drives a repaint. if (self.dirty) return self.changed_from orelse 0; - return self.changed_from; + return null; } /// Record a successful render. Diffs `new_lines` against the cache, @@ -264,12 +285,15 @@ test "RenderCache: store cleans, no-change render returns null" { const a = [_][]const u8{ "one", "two" }; try c.store(&a); - // First store after empty cache => changed from 0. - try std.testing.expectEqual(@as(?usize, 0), c.firstLineChanged()); + // store cleans the cache, so the live signal is null (clean => null). + // The recorded diff (first store after empty => 0) lives in changed_from. + try std.testing.expectEqual(@as(?usize, null), c.firstLineChanged()); + try std.testing.expectEqual(@as(?usize, 0), c.changed_from); - // Re-store identical => no change. + // Re-store identical => no change, still clean. try c.store(&a); try std.testing.expectEqual(@as(?usize, null), c.firstLineChanged()); + try std.testing.expectEqual(@as(?usize, null), c.changed_from); } test "RenderCache: store reports lowest differing line" { @@ -280,7 +304,10 @@ test "RenderCache: store reports lowest differing line" { try c.store(&a); const b = [_][]const u8{ "one", "TWO", "three" }; try c.store(&b); - try std.testing.expectEqual(@as(?usize, 1), c.firstLineChanged()); + // The diff store computed is recorded in changed_from; the live signal is + // null because the cache is now clean (already rendered on screen). + try std.testing.expectEqual(@as(?usize, 1), c.changed_from); + try std.testing.expectEqual(@as(?usize, null), c.firstLineChanged()); } test "RenderCache: line-count change reports the boundary" { @@ -291,7 +318,8 @@ test "RenderCache: line-count change reports the boundary" { try c.store(&a); const b = [_][]const u8{ "one", "two", "three" }; try c.store(&b); - try std.testing.expectEqual(@as(?usize, 2), c.firstLineChanged()); + try std.testing.expectEqual(@as(?usize, 2), c.changed_from); + try std.testing.expectEqual(@as(?usize, null), c.firstLineChanged()); } test "RenderCache: markDirtyAppend keeps baseline and reports a tail hint, diff recovers exact change" { @@ -307,10 +335,12 @@ test "RenderCache: markDirtyAppend keeps baseline and reports a tail hint, diff try std.testing.expect(c.lines != null); // A render that only adds a tail line: diff recovers the exact boundary (4), - // NOT 0 — the streaming-tail property. + // NOT 0 — the streaming-tail property. The cache is clean after store, so + // the recorded diff lives in changed_from and the live signal is null. const b = [_][]const u8{ "l0", "l1", "l2", "l3", "l4" }; try c.store(&b); - try std.testing.expectEqual(@as(?usize, 4), c.firstLineChanged()); + try std.testing.expectEqual(@as(?usize, 4), c.changed_from); + try std.testing.expectEqual(@as(?usize, null), c.firstLineChanged()); } test "RenderCache: markDirtyAppend with no baseline reports 0" { diff --git a/src/tui_components.zig b/src/tui_components.zig index a9648ee..1ce5b56 100644 --- a/src/tui_components.zig +++ b/src/tui_components.zig @@ -1580,7 +1580,7 @@ pub const CompactionSummary = struct { /// component holds no global state. pub const ToolUse = struct { /// Number of trailing output lines shown when collapsed. - pub const collapsed_tail_lines: usize = 5; + pub const collapsed_tail_lines: usize = 8; alloc: std.mem.Allocator, cache: RenderCache, @@ -1854,8 +1854,9 @@ test "AssistantText: renders wrapped text within width" { // renderBlock adds 2 margin lines: 3 content + 2 = 5. try testing.expectEqual(@as(usize, 5), lines.len); for (lines) |l| try testing.expect(vw(l) <= 7); - // First render after empty cache => changed from 0. - try testing.expectEqual(@as(?usize, 0), at.comp().firstLineChanged()); + // After a render the cache is clean, so the live signal is null. (The + // first-render diff index 0 is retained internally in changed_from.) + try testing.expectEqual(@as(?usize, null), at.comp().firstLineChanged()); } test "AssistantText: streaming keeps firstLineChanged near the tail, not 0" { @@ -2530,24 +2531,33 @@ test "ToolUse: stage 2 shows name + verbatim json + placeholder" { for (lines) |l| try testing.expect(vw(l) <= 60); } -test "ToolUse: collapsed shows only the last 5 output lines (default)" { +test "ToolUse: collapsed shows only the last collapsed_tail_lines output lines (default)" { var t = ToolUse.init(testing.allocator); defer t.deinit(); try t.setName("read"); try t.setInput("{}"); - // 8 short output lines -> collapsed shows the marker + last 5. - try t.setOutput("l1\nl2\nl3\nl4\nl5\nl6\nl7\nl8"); + // 12 short output lines -> collapsed shows the marker + the last + // collapsed_tail_lines (8) of them, eliding l1..l4. + try t.setOutput("l1\nl2\nl3\nl4\nl5\nl6\nl7\nl8\nl9\nl10\nl11\nl12"); const collapsed = try t.comp().render(40, testing.allocator); - // margin+pad+header+sep+marker+l4..l8+pad+margin = 1+1+1+1+1+5+1+1 = 12 - // Last output line (l8) is at lines[len-3] (before bottom pad + margin). - try testing.expect(std.mem.indexOf(u8, collapsed[collapsed.len - 3], "l8") != null); - try testing.expect(std.mem.indexOf(u8, collapsed[collapsed.len - 7], "l4") != null); - // The earliest output lines are elided when collapsed. - var has_l1 = false; + // margin+pad+header+sep+marker+(tail lines)+pad+margin + // = 1+1+1+1+1+collapsed_tail_lines+1+1 = 7 + collapsed_tail_lines + try testing.expectEqual(@as(usize, 7 + ToolUse.collapsed_tail_lines), collapsed.len); + // Last output line (l12) is at lines[len-3] (before bottom pad + margin); + // the first SHOWN tail line is at lines[len-3-(tail-1)]. + // Tokens are rendered as " l<n>" followed by right-padding, so match on a + // trailing space to avoid "l1" spuriously matching inside "l10".."l12". + try testing.expect(std.mem.indexOf(u8, collapsed[collapsed.len - 3], "l12 ") != null); + try testing.expect(std.mem.indexOf(u8, collapsed[collapsed.len - 3 - (ToolUse.collapsed_tail_lines - 1)], "l5 ") != null); + // The earliest output lines (l1..l4) are elided when collapsed. + var has_elided = false; for (collapsed) |l| { - if (std.mem.indexOf(u8, l, "l1") != null) has_l1 = true; + if (std.mem.indexOf(u8, l, "l1 ") != null or + std.mem.indexOf(u8, l, "l2 ") != null or + std.mem.indexOf(u8, l, "l3 ") != null or + std.mem.indexOf(u8, l, "l4 ") != null) has_elided = true; } - try testing.expect(!has_l1); + try testing.expect(!has_elided); // Expanding shows everything. t.setCollapsed(false); @@ -2555,7 +2565,7 @@ test "ToolUse: collapsed shows only the last 5 output lines (default)" { try testing.expect(expanded.len > collapsed.len); var has_l1_exp = false; for (expanded) |l| { - if (std.mem.indexOf(u8, l, "l1") != null) has_l1_exp = true; + if (std.mem.indexOf(u8, l, "l1 ") != null) has_l1_exp = true; } try testing.expect(has_l1_exp); } @@ -2620,33 +2630,31 @@ test "ToolUse: collapse/expand is a length change with a cache-derived firstLine defer t.deinit(); try t.setName("read"); try t.setInput("{}"); - try t.setOutput("l1\nl2\nl3\nl4\nl5\nl6\nl7\nl8"); + // 12 output lines, so collapsing (tail = collapsed_tail_lines = 8) truncates. + try t.setOutput("l1\nl2\nl3\nl4\nl5\nl6\nl7\nl8\nl9\nl10\nl11\nl12"); // Default collapsed: - // margin+pad+header+sep+truncmark+5lines+pad+margin = 1+1+1+1+1+5+1+1 = 12 + // margin+pad+header+sep+truncmark+8lines+pad+margin = 1+1+1+1+1+8+1+1 = 15 const collapsed = try t.comp().render(40, testing.allocator); - try testing.expectEqual(@as(usize, 12), collapsed.len); - // A stable re-render is clean (cache-derived, no drift). + try testing.expectEqual(@as(usize, 15), collapsed.len); + // A stable re-render is clean: the live signal is null. _ = try t.comp().render(40, testing.allocator); try testing.expectEqual(@as(?usize, null), t.comp().firstLineChanged()); - // Expand: margin+pad+header+sep+8lines+pad+margin = 1+1+1+1+8+1+1 = 14 + // Expand: margin+pad+header+sep+12lines+pad+margin = 1+1+1+1+12+1+1 = 18 t.setCollapsed(false); // While dirty (full drop), the signal is the cache-derived 0. try testing.expectEqual(@as(?usize, 0), t.comp().firstLineChanged()); const expanded = try t.comp().render(40, testing.allocator); - try testing.expectEqual(@as(usize, 14), expanded.len); - // After the render the baseline was dropped on the toggle, so the diff - // reports from 0 — cache-derived, not a hand-managed value. - try testing.expectEqual(@as(?usize, 0), t.comp().firstLineChanged()); - // Stable re-render is clean again. - _ = try t.comp().render(40, testing.allocator); + try testing.expectEqual(@as(usize, 18), expanded.len); + // After the render the cache is clean, so the live signal is null again + // (the toggle's drop-from-0 diff is internal bookkeeping, not a live cut). try testing.expectEqual(@as(?usize, null), t.comp().firstLineChanged()); - // Collapse again: shrink back to 12 rows. + // Collapse again: shrink back to 15 rows. t.setCollapsed(true); const recollapsed = try t.comp().render(40, testing.allocator); - try testing.expectEqual(@as(usize, 12), recollapsed.len); + try testing.expectEqual(@as(usize, 15), recollapsed.len); } test "ToolUse: args are rendered VERBATIM (no pretty-print) and within width" { 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(); |
