diff options
| -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(); |