Fix scrollback correctness: rebuild_pending flag + unified erase path

Replace the semantically incorrect surgical trim logic with a unified
scrollback-validity mechanism:

- Rename `resize_pending` and `scroll_cleared` flags to a single
  `rebuild_pending` bool. Both `terminal.resize()` and a new CSI 3J
  scanner in `fnWriteInput` set this flag; only the redraw pass resets
  it and owns all associated state (scrollback_in_buffer, first row hash).

- Scan raw vtWrite input for CSI 3J (`\x1B[3J`). libghostty clears its
  scrollback on this sequence but provides no callback. The flag is
  necessary because CSI 3J followed by enough new output to restore the
  same scrollback depth is invisible to both count-based and hash-based
  detection.

- Replace four scattered erase paths (resize_pending block, rotation
  hash mismatch, surgical trim fallback, scrollback_cleared check) with
  a single `scrollback_stale` local that collects all invalidation
  signals and erases once. The rotation hash check is skipped when the
  flag is already set (avoiding a redundant viewport scroll).

- Remove the surgical trim (delete top N lines when libghostty scrollback
  shrinks). It assumed rows shrink only by eviction with the rest intact,
  which is wrong for CSI 3J and for reflow on resize. A full erase is
  correct in all cases and no more expensive in practice.

- Add ERT test `ghostel-test-scrollback-csi3j-then-refill` covering the
  previously undetectable scenario: CSI 3J followed by enough new output
  to restore the original scrollback depth must not leave stale rows.

Closes #160. Supersedes #166.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: 2 people

src/module.zig

@@ -165,6 +165,7 @@ fn fnWriteInput(raw_env: ?*c.emacs_env, _: isize, args: [*c]c.emacs_value, _: ?*
     // pair split across two writes — chunk A ending with \r, chunk B
     // starting with \n — is not mis-normalized into \r\r\n.  The final
     // value is persisted back for the next call. An empty input
+
     // round-trips the flag unchanged.
     var seg_start: usize = 0;
     var prev_was_cr: bool = term.last_input_was_cr;
@@ -183,6 +184,23 @@ fn fnWriteInput(raw_env: ?*c.emacs_env, _: isize, args: [*c]c.emacs_value, _: ?*
     }
     term.last_input_was_cr = prev_was_cr;
 
+    // Detect CSI 3 J (erase scrollback). libghostty processes it and clears
+    // its scrollback, but provides no notification. Set rebuild_pending so the
+    // next redraw erases the Emacs buffer and re-fetches from libghostty.
+    // The flag is necessary because CSI 3 J followed by enough new output to
+    // restore the same scrollback depth is undetectable by count or hash alone.
+    //
+    // This is a stateless substring scan over a single write, so it misses:
+    // (1) sequences split across two writes (ESC in one, "[3J" in the next);
+    // (2) non-canonical forms — parameter padding like `\x1B[03J`, or the
+    //     8-bit C1 form `\x9B3J`.
+    // libghostty's stateful VT parser still clears its scrollback in those
+    // cases, so the `libghostty_sb < scrollback_in_buffer` shrink check in
+    // redraw() is the fallback that catches them.
+    if (std.mem.indexOf(u8, raw, "\x1B[3J") != null) {
+        term.rebuild_pending = true;
+    }
+
     // Scan for OSC sequences that libghostty-vt discards (7, 51, 52, 133).
     // One pass, dispatched by code in document order.
     dispatchPostWriteOscs(env, term, raw);
@@ -624,7 +642,7 @@ fn fnSetSize(raw_env: ?*c.emacs_env, _: isize, args: [*c]c.emacs_value, _: ?*any
         return env.nil();
     };
     // Reflow invalidates the materialized scrollback — terminal.resize()
-    // sets resize_pending so the next redraw() erases and rebuilds under
+    // sets rebuild_pending so the next redraw() erases and rebuilds under
     // inhibit-redisplay, avoiding a visible blank frame.
 
     return env.nil();

src/render.zig

@@ -819,7 +819,6 @@ fn insertScrollbackRange(
     return inserted;
 }
 
-
 /// Convert a terminal column to an Emacs character offset by iterating
 /// the row's cells.  Returns `true` and positions point on success;
 /// `false` if the cell data is unavailable (caller should fall back to
@@ -891,8 +890,8 @@ fn positionCursorByCell(env: emacs.Env, term: *Terminal, cx: u16, cy: u16) bool
 /// On each call we:
 ///   1. Force libghostty's viewport to the bottom (active screen).
 ///   2. Poll `getTotalRows()` against `term.scrollback_in_buffer` to
-///      detect rows that scrolled off the top (append to buffer) or
-///      rows evicted by libghostty's scrollback cap (trim from buffer).
+///      detect rows that scrolled off the top of the viewport and promoted them
+///      to the scrollback part of the buffer
 ///   3. Render the viewport into the tail of the buffer, anchored at
 ///      the line that follows the last scrollback row.
 ///
@@ -936,17 +935,6 @@ pub fn redraw(env: emacs.Env, term: *Terminal, force_full_arg: bool) void {
         return;
     }
 
-    // ---- Deferred resize buffer reset ----------------------------------------
-    // terminal.resize() sets resize_pending instead of immediately erasing the
-    // buffer, so the old frame stays visible until this redraw replaces it
-    // (the caller binds inhibit-redisplay around us).  Erase now and force a
-    // full rebuild of scrollback + viewport.
-    if (term.resize_pending) {
-        env.eraseBuffer();
-        term.resize_pending = false;
-        force_full = true;
-    }
-
     // Resolve default colors once — used for both the scrollback append
     // path and the viewport render path.  These always succeed (the
     // render state always has resolved default colors), so batching is safe.
@@ -964,45 +952,62 @@ pub fn redraw(env: emacs.Env, term: *Terminal, force_full_arg: bool) void {
         _ = gt.c.ghostty_render_state_get_multi(term.render_state, color_keys.len, &color_keys, @ptrCast(&color_values), null);
     }
 
-    // ---- Scrollback rotation detection ------------------------------------
-    // When libghostty's scrollback is at its byte cap, sustained writes
-    // evict the oldest rows and push new ones, so the row at scrollback
-    // index 0 changes underneath us. The normal delta-sync below tracks
-    // `total_rows` deltas, but those don't capture content rotation —
-    // if the count is unchanged (or even shrinking) the trim path would
-    // remove our top rows under the *assumption* they match the rows
-    // libghostty just evicted, which isn't true after rotation.
+    // ---- Scrollback validity ------------------------------------------------
+    // Two signals can invalidate buffered scrollback and are collected here;
+    // a third (rotation) is checked below.
     //
-    // Detect rotation by hashing the first scrollback row whenever
-    // writes have happened since the last redraw and we have scrollback.
-    // A change means the top row is no longer the row we materialized
-    // → wipe the buffer and let the delta-sync below re-fetch everything
-    // fresh from libghostty.
+    //   rebuild_pending: set by terminal.resize() and by the CSI 3 J scanner
+    //                    in vtWrite. Defers the Emacs-buffer erase into the
+    //                    redraw pass where inhibit-redisplay prevents a
+    //                    visible blank frame.
     //
-    // When the hash matches (no rotation), stash it in `cached_row0_hash`
-    // and reuse at end-of-redraw. Promotion / insert-at-tail don't shift
-    // row 0, so the start-of-redraw hash is still valid. Trim and
-    // rotation-erase invalidate the cache.
+    //   rotation:        checked below via a row-0 hash; only computed when
+    //                    rebuild_pending hasn't already fired.
+    var scrollback_stale = term.rebuild_pending;
+    term.rebuild_pending = false;
+
+    // ---- Rotation detection ------------------------------------------------
+    // When libghostty's scrollback cap is saturated, sustained writes evict
+    // the oldest rows. total_rows doesn't change, so the delta-sync below
+    // would see nothing to do — detect the churn by hashing the first
+    // scrollback row and comparing to the value sampled at the last redraw.
+    //
+    // Skip when scrollback is already known to be stale: the viewport scroll
+    // that sampling requires is wasted work if we are about to erase anyway.
+    // On a hash match, stash the value for reuse at end-of-redraw (promotion
+    // and insert-at-tail don't shift row 0, so it stays valid).
     var cached_row0_hash: ?u64 = null;
-    if (term.wrote_since_redraw and term.scrollback_in_buffer > 0 and term.first_scrollback_row_hash != 0) {
+    if (!scrollback_stale and
+        term.wrote_since_redraw and
+        term.scrollback_in_buffer > 0 and
+        term.first_scrollback_row_hash != 0)
+    {
         const new_hash = computeFirstScrollbackRowHash(term);
         // computeFirstScrollbackRowHash scrolled libghostty's viewport to
-        // sample row 0 and the defer restored the offset, but the render
-        // state may now be stale — refresh it before continuing.
+        // sample row 0; the render state is now stale — refresh it.
         if (gt.c.ghostty_render_state_update(term.render_state, term.terminal) != gt.SUCCESS) return;
         if (new_hash != term.first_scrollback_row_hash) {
-            // Rotation detected — erase the buffer entirely and force a
-            // full viewport render. The delta-sync below will then see
-            // libghostty_sb - 0 = libghostty_sb and refetch everything.
-            env.eraseBuffer();
-            term.scrollback_in_buffer = 0;
-            term.first_scrollback_row_hash = 0;
-            force_full = true;
+            scrollback_stale = true;
         } else {
             cached_row0_hash = new_hash;
         }
     }
 
+    // Compare counts: scrollback shrinking means the buffered rows are no
+    // longer valid (CSI 3 J or resize would have set rebuild_pending, but
+    // this catches any other unexpected reduction).
+    const total_rows = term.getTotalRows();
+    const libghostty_sb: usize = if (total_rows > term.rows) total_rows - term.rows else 0;
+    if (libghostty_sb < term.scrollback_in_buffer) scrollback_stale = true;
+
+    // If scrollback is stale for any reason, erase it completely.
+    if (scrollback_stale) {
+        env.eraseBuffer();
+        term.scrollback_in_buffer = 0;
+        term.first_scrollback_row_hash = 0;
+        force_full = true;
+    }
+
     // ---- Scrollback sync ---------------------------------------------------
     // libghostty stores scrollback + active screen in a single row space.
     // The rows "above" the viewport are scrollback; our invariant is that
@@ -1012,8 +1017,6 @@ pub fn redraw(env: emacs.Env, term: *Terminal, force_full_arg: bool) void {
     // by walking from point-min and reusing point() after any insert/trim
     // touches it. forwardLine is O(scrollback) so doing it twice would
     // double the per-redraw cost in long-running sessions.
-    const total_rows = term.getTotalRows();
-    const libghostty_sb: usize = if (total_rows > term.rows) total_rows - term.rows else 0;
 
     // Walk to the current viewport start (line scrollback_in_buffer + 1).
     env.gotoCharN(1);
@@ -1102,31 +1105,6 @@ pub fn redraw(env: emacs.Env, term: *Terminal, force_full_arg: bool) void {
             term.scrollViewport(gt.SCROLL_BOTTOM, 0);
             if (gt.c.ghostty_render_state_update(term.render_state, term.terminal) != gt.SUCCESS) return;
         }
-    } else if (libghostty_sb < term.scrollback_in_buffer) {
-        // libghostty's scrollback cap evicted the oldest rows — trim the
-        // same number of lines from the top of the buffer. Trim shifts
-        // row 0 so the start-of-redraw hash is stale.
-        cached_row0_hash = null;
-        const delta = term.scrollback_in_buffer - libghostty_sb;
-        env.gotoCharN(1);
-        if (env.forwardLine(@as(i64, @intCast(delta))) == 0) {
-            env.deleteRegion(env.makeInteger(1), env.point());
-            term.scrollback_in_buffer -= delta;
-            // After the delete, the new viewport start has shifted down.
-            // forwardLine is already at the right line (point-min + delta
-            // lines was the next surviving row, now line 1+scrollback_in_buffer).
-            // Recompute by walking the remaining scrollback rows.
-        } else {
-            // Ran off the end — buffer is out of sync; rebuild from scratch.
-            env.eraseBuffer();
-            term.scrollback_in_buffer = 0;
-            force_full = true;
-        }
-        env.gotoCharN(1);
-        if (term.scrollback_in_buffer > 0) {
-            _ = env.forwardLine(@as(i64, @intCast(term.scrollback_in_buffer)));
-        }
-        viewport_start_int = env.extractInteger(env.point());
     }
 
     // Check dirty state — cells are only redrawn when dirty, but cursor

src/terminal.zig

@@ -42,12 +42,10 @@ scrollback_in_buffer: usize = 0,
 /// activity" from "writes happened but total_rows plateaued".
 wrote_since_redraw: bool = false,
 
-/// Set by `resize`, cleared at the start of `redraw`. When true, the
-/// next redraw will erase the buffer and force a full rebuild.  This
-/// defers the buffer erasure from the synchronous resize call (which
-/// would leave the buffer visibly empty until the timer-driven redraw)
-/// into the redraw pass where `inhibit-redisplay` prevents flicker.
-resize_pending: bool = false,
+/// When true, the next redraw erases the Emacs buffer,
+/// resets scrollback state, and forces a full rebuild.  The erase is deferred
+/// so `inhibit-redisplay` can prevent a visible blank frame.
+rebuild_pending: bool = false,
 
 /// True iff the last byte of the previous `fnWriteInput` input was
 /// `\r`. Carries the bare-LF detection state across write-input calls
@@ -224,20 +222,16 @@ pub fn vtWrite(self: *Self, data: []const u8) void {
 
 /// Resize the terminal.
 ///
-/// Resets `scrollback_in_buffer` because libghostty reflows wrapped rows
-/// on resize and the row count above the viewport no longer matches what
-/// we have in the Emacs buffer.  Sets `resize_pending` so the next
-/// `redraw()` erases the buffer under `inhibit-redisplay` and rebuilds
-/// scrollback from scratch — avoiding a visible blank frame.
+/// Also sets `rebuild_pending` so the next `redraw()` erases the Emacs buffer
+/// under `inhibit-redisplay` and rebuilds scrollback from scratch — avoiding
+/// a visible blank frame between the resize and the first repaint.
 pub fn resize(self: *Self, cols: u16, rows: u16) !void {
     if (gt.c.ghostty_terminal_resize(self.terminal, cols, rows, 1, 1) != gt.SUCCESS) {
         return error.ResizeFailed;
     }
     self.cols = cols;
     self.rows = rows;
-    self.scrollback_in_buffer = 0;
-    self.first_scrollback_row_hash = 0;
-    self.resize_pending = true;
+    self.rebuild_pending = true;
     self.last_input_was_cr = false;
 }
 

test/ghostel-test.el

@@ -501,6 +501,58 @@ scrolling libghostty's viewport."
               (should-not (string-match-p "line [0-9]" content)))))
       (kill-buffer buf))))
 
+(ert-deftest ghostel-test-scrollback-csi3j-then-refill ()
+  "CSI 3 J must not leave stale pre-clear rows in the buffer.
+
+Scenario (5-row terminal, 10 before-* rows, CSI 3J, 5 after-* rows,
+single redraw):
+  - After the first redraw: before-00..before-05 are in scrollback (6
+    rows scrolled off), before-06..before-09 fill the viewport.
+  - CSI 3J clears libghostty's scrollback; the viewport is unchanged.
+  - Five new after-* rows scroll before-06..before-09 and after-00 into
+    libghostty's freshly-cleared scrollback (5 rows); after-01..after-04
+    are left in the viewport.
+  - At the next redraw, libghostty_sb (5) < scrollback_in_buffer (6),
+    so count-based detection triggers a full rebuild.  The rebuild_pending
+    flag (set by the CSI 3J scanner in vtWrite) also fires, ensuring the
+    erase happens even in scenarios where counts happen to match."
+  (let ((buf (generate-new-buffer " *ghostel-test-csi3j-refill*")))
+    (unwind-protect
+        (with-current-buffer buf
+          (let* ((term (ghostel--new 5 80 1000))
+                 (inhibit-read-only t))
+            ;; Phase 1: fill scrollback with 10 "before" rows and redraw.
+            (dotimes (i 10)
+              (ghostel--write-input term (format "before-%02d\r\n" i)))
+            (ghostel--redraw term t)
+            ;; Confirm before-00..before-05 are now in the buffer's scrollback
+            ;; and before-06..before-09 are in the viewport.
+            (let ((content (buffer-substring-no-properties (point-min) (point-max))))
+              (should (string-match-p "before-00" content))
+              (should (string-match-p "before-05" content))
+              (should (string-match-p "before-09" content)))
+            ;; Phase 2: CSI 3 J (erase scrollback only) then immediately
+            ;; write 5 "after" rows — no redraw in between.  before-06..before-09
+            ;; scroll off into libghostty's freshly-cleared scrollback as the
+            ;; after-* rows push through the viewport.
+            (ghostel--write-input term "\e[3J")
+            (dotimes (i 5)
+              (ghostel--write-input term (format "after-%02d\r\n" i)))
+            ;; Phase 3: single redraw — must rebuild from libghostty.
+            (ghostel--redraw term t)
+            (let ((content (buffer-substring-no-properties (point-min) (point-max))))
+              ;; Rows that were in scrollback when CSI 3J fired are gone.
+              (should-not (string-match-p "before-00" content))
+              (should-not (string-match-p "before-05" content))
+              ;; Rows that were in the viewport during CSI 3J are now in
+              ;; libghostty's new scrollback and must be present.
+              (should (string-match-p "before-06" content))
+              (should (string-match-p "before-09" content))
+              ;; after-00 scrolled into scrollback; after-01..after-04 in viewport.
+              (should (string-match-p "after-00" content))
+              (should (string-match-p "after-04" content)))))
+      (kill-buffer buf))))
+
 ;; -----------------------------------------------------------------------
 ;; Test: SGR styling (bold, color, etc.)
 ;; -----------------------------------------------------------------------