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_component.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_component.zig')
| -rw-r--r-- | src/tui_component.zig | 52 |
1 files changed, 41 insertions, 11 deletions
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" { |
