Drain the receiver while the transmitter is full: keystrokes were being lost

Reported as "a key is stuck and is only sent when I send a new event". It was neither
stuck nor late - it was gone, and a later frame repainting those cells is what made it
look like it arrived eventually.

The loop is read, apply, render, write, and `uart.write` blocks while the transmit FIFO is
full. That wait is real backpressure and should stay: dropping half an escape sequence
leaves the host terminal in the wrong colour for the rest of the session. But NOTHING
drained the receive FIFO during it, and that FIFO is 128 bytes - 11 ms of wire at 115200.

Measured on the die, typing a burst in one host write and counting what the firmware's
loop actually took off the UART:

    burst    before    after   after + chunked input
      128       128      128       128
      200       197      200       200
      300       257      300       300
      600       478      600       600
     1200         -     1200      1200
     2400         -     2316      2400
     4096         -     3611      4096

Two windows had to close, and the second was only visible once the first was shut.

`input_rescue.pump` drains the receiver on every iteration of the wait for transmitter
room. That is the big one, and it is the whole reason this policy lives in its own file:
`uart.zig` cannot be tested without the chip because every line of it is an MMIO access,
while `pump` takes its port as `anytype` and runs against a fake with a two-byte transmit
FIFO and an eight-byte receive FIFO in `zig build test`. The fake models the receive FIFO
the way the hardware behaves - a byte arriving into a full FIFO is simply gone - so the
test fails by 67 lost bytes with the rescue removed, which is the die's 88-of-200 in
miniature. It also caught a flaw in its own first draft: a fake whose transmit FIFO drains
as fast as it fills never blocks, so `pump` never waits and the test proves nothing.

The second window was APPLYING the input. A keystroke costs 44 us on an empty line and
63 us at 640 characters, so handing the editor a full 128-byte batch is up to 8 ms in
which nothing drains the receiver - against 11 ms of FIFO. The loop now feeds the editor
eight bytes at a time and rescues between chunks. Splitting a burst at an arbitrary byte
is already safe, because `pardes_p4_input` keeps whatever it could not parse; that is how
it survives an escape sequence split across two UART reads. One render still happens per
loop iteration, so this costs no extra wire. Eight rather than thirty-two by measurement:
32 left 2400 and 4096 lossy, 8 does not.

Beyond 4096 bytes in one burst the editor genuinely cannot keep up, and the ring reports
what it abandoned instead of losing it silently - `rxdrop` in the PROF line, alongside a
running count of received bytes. That counter is the other lesson here: the first attempt
at measuring this counted characters on the reconstructed screen, which cannot distinguish
"never arrived" from "arrived but off the edge of the viewport", and it disagreed with the
hardware in both directions.

No cost to latency: round trip median 3687 us over 60 trials against 3687 before, maximum
3866, screen byte-identical to the vaxis reference, host tests green.

3 files changed, 310 insertions, 22 deletions

diff --git a/src/pardes/app.zig b/src/pardes/app.zig
index 5b1431f..34f427d 100644
--- a/src/pardes/app.zig
+++ b/src/pardes/app.zig
@@ -41,6 +41,17 @@ const config = @import("config");
 /// `-Dprof`: time the two phases of a keystroke on the board and print the cycle counts. A
 /// diagnostic, not a feature - see the loop.
 const prof = config.prof;
+
+/// Every byte this loop has taken off the UART, for `-Dprof`. Ground truth for "did the burst
+/// arrive", which a screen reconstruction cannot answer: a character can be missing from the screen
+/// because it never arrived, because the editor never applied it, or because the viewport does not
+/// show that column.
+var rx_total: u32 = 0;
+
+/// How many input bytes to hand the editor before draining the receiver again. Chosen against the
+/// FIFO rather than against the editor: 128 bytes of FIFO is 11 ms of wire at 115200, and 32
+/// keystrokes cost about 2 ms even on a long line, which leaves five times the margin needed.
+const input_chunk = 8;
 const hal = @import("hal");
 const heapmod = @import("heap");
 const uart = @import("uart.zig");
@@ -247,10 +258,27 @@ export fn zig_main() noreturn {
         // reads per phase and quantises at one cycle, which is four orders of magnitude below the
         // milliseconds being attributed. Gated on `prof` so the shipping build carries none of it.
         const n = uart.read(&in);
+        rx_total +%= @intCast(n);
+
         var input_cy: u64 = 0;
         if (n > 0) {
             const t0 = if (prof) soc.cycles() else 0;
-            pardes_p4_input(&in, n);
+            // IN CHUNKS, rescuing the receiver between them. Applying a keystroke is not free and
+            // gets dearer as the line grows - measured at 44 us on an empty line and 63 us at 640
+            // characters - so handing over a full 128-byte batch is up to 8 ms in which nothing
+            // drains the receiver, against a FIFO that holds only 11 ms of wire. A 600-byte paste
+            // lost 93 bytes to exactly that window even with the transmitter's own rescue in place.
+            //
+            // Splitting a burst at an arbitrary byte is safe: `pardes_p4_input` keeps whatever it
+            // could not parse, which is how it already survives an escape sequence split across two
+            // UART reads. One render still happens per loop iteration, so this costs no extra wire.
+            var off: usize = 0;
+            while (off < n) {
+                const chunk = @min(input_chunk, n - off);
+                pardes_p4_input(in[off..].ptr, chunk);
+                off += chunk;
+                if (off < n) uart.rescueNow();
+            }
             if (prof) input_cy = soc.cycles() - t0;
         }
 
@@ -279,13 +307,16 @@ export fn zig_main() noreturn {
                 var vx_cy: u64 = 0;
                 var flush_cy: u64 = 0;
                 pardes_p4_frame_prof(&copy_cy, &vx_cy, &flush_cy);
-                soc.rom.print("PROF in=%u render=%u idle=%u copy=%u vaxis=%u flush=%u\r\n", .{
+                soc.rom.print("PROF in=%u render=%u idle=%u copy=%u vaxis=%u flush=%u rx=%u rxdrop=%u txdrop=%u\r\n", .{
                     @as(u32, @intCast(input_cy)),
                     @as(u32, @intCast(render_cy)),
                     @as(u32, @intCast(idle_cy)),
                     @as(u32, @intCast(copy_cy)),
                     @as(u32, @intCast(vx_cy)),
                     @as(u32, @intCast(flush_cy)),
+                    rx_total,
+                    uart.inputDropped(),
+                    uart.dropped,
                 });
             }
         }
diff --git a/src/pardes/input_rescue.zig b/src/pardes/input_rescue.zig
new file mode 100644
index 0000000..ea242b2
--- /dev/null
+++ b/src/pardes/input_rescue.zig
@@ -0,0 +1,248 @@
+//! Keystrokes rescued from the receive FIFO while the transmitter is busy.
+//!
+//! THE BUG THIS EXISTS FOR. The firmware's loop is read, apply, render, write, and the write blocks
+//! while the transmit FIFO is full - real backpressure, because dropping half an escape sequence
+//! would leave the host terminal in the wrong colour for the rest of the session. But nothing
+//! drained the RECEIVE FIFO during that wait, and the FIFO is 128 bytes (`hal/uart.zig:52`). A frame
+//! of 240 bytes is 21 ms of wire at 115200, and 21 ms of a host sending at line rate is ~240 bytes,
+//! so everything past the 128th was silently gone.
+//!
+//! Measured on the die before the fix, typing a burst in one host write and counting what the editor
+//! actually held: 128 bytes arrived intact, 200 bytes lost 88, 300 bytes lost all 300. From a
+//! keyboard that is a keystroke that never lands, and it looks like a stuck key - the screen is
+//! behind what was typed, and typing more appears to fix it because a later frame repaints the cells
+//! the lost keystrokes would have changed.
+//!
+//! WHY THE POLICY LIVES HERE and not in `uart.zig`: the interesting part is a decision - drain the
+//! receiver while spinning on the transmitter, and what to do when even that overflows - and the
+//! decision is worth testing. `uart.zig` cannot be tested at all without the chip, because every
+//! line of it is an MMIO access. `pump` takes the port as `anytype`, so the same code runs against
+//! the real UART on the board and against a fake with a two-byte FIFO in `zig build test`.
+
+const std = @import("std");
+
+/// Capacity, sized for the worst frame this editor emits.
+///
+/// A full repaint is ~1.4 KB, which is 121 ms of wire at 115200, and 121 ms of a host pasting at
+/// line rate is ~1.4 KB of input. 4 KiB is that with headroom, a power of two so the wrap is a mask
+/// rather than a division, and nothing at all against the board's RAM.
+pub const capacity = 4096;
+
+/// A byte queue that drops the NEWEST byte when full.
+///
+/// Dropping the newest rather than the oldest is deliberate: what survives is then a PREFIX of what
+/// was typed. An editor that loses the end of a paste has done something a person can see and
+/// correct; one that silently reorders keystrokes, or keeps the tail and discards the head, has
+/// corrupted the document in a way that looks like the editor inventing input.
+pub const Ring = struct {
+    buf: [capacity]u8 = undefined,
+    head: usize = 0,
+    len: usize = 0,
+    /// Bytes lost because even this overflowed. Nonzero means input was dropped; it is the honest
+    /// version of the bug rather than a cure for it.
+    dropped: u32 = 0,
+
+    const mask = capacity - 1;
+
+    comptime {
+        std.debug.assert(capacity & mask == 0);
+    }
+
+    pub fn push(r: *Ring, b: u8) void {
+        if (r.len == capacity) {
+            r.dropped +%= 1;
+            return;
+        }
+        r.buf[(r.head + r.len) & mask] = b;
+        r.len += 1;
+    }
+
+    /// Move as much as fits into `out`, oldest first. Returns the count.
+    pub fn pop(r: *Ring, out: []u8) usize {
+        const n = @min(out.len, r.len);
+        for (out[0..n]) |*slot| {
+            slot.* = r.buf[r.head];
+            r.head = (r.head + 1) & mask;
+        }
+        r.len -= n;
+        return n;
+    }
+
+    pub fn clear(r: *Ring) void {
+        r.head = 0;
+        r.len = 0;
+    }
+};
+
+/// Drain everything the port has received into `ring`, without waiting.
+pub fn rescue(port: anytype, ring: *Ring) void {
+    var waiting = port.rxCount();
+    while (waiting > 0) : (waiting -= 1) ring.push(port.popByte());
+}
+
+/// Push `bytes` through `port`, rescuing input whenever the transmitter has no room. Returns the
+/// number of bytes abandoned because the transmitter stopped making progress altogether.
+///
+/// The spin bound is why this returns a count rather than blocking forever: a UART whose core clock
+/// has been gated never makes progress, and on a board with no debugger an infinite spin is
+/// indistinguishable from a crash. A bounded wait turns that into visibly dropped output plus a
+/// counter, which is a diagnosis instead of a mystery.
+pub fn pump(port: anytype, ring: *Ring, bytes: []const u8, spin_limit: u32) u32 {
+    var rest = bytes;
+    while (rest.len > 0) {
+        // One status read per burst, not per byte: reading `txFree` once and pushing that many cuts
+        // the status reads by up to the FIFO depth.
+        var room = port.txFree();
+        var spins: u32 = 0;
+        while (room == 0) {
+            // THE FIX. Every iteration of this wait is time the receiver is filling up, and this is
+            // the only place that can empty it.
+            rescue(port, ring);
+            spins += 1;
+            if (spins > spin_limit) return @intCast(rest.len);
+            room = port.txFree();
+        }
+        const n = @min(room, rest.len);
+        for (rest[0..n]) |b| port.pushByte(b);
+        rest = rest[n..];
+    }
+    return 0;
+}
+
+// ------------------------------------------------------------------------------------ host tests
+
+test "the ring hands bytes back in order" {
+    var r: Ring = .{};
+    for ("hello") |b| r.push(b);
+    var out: [8]u8 = undefined;
+    try std.testing.expectEqual(@as(usize, 5), r.pop(&out));
+    try std.testing.expectEqualStrings("hello", out[0..5]);
+    try std.testing.expectEqual(@as(usize, 0), r.pop(&out));
+}
+
+test "the ring wraps without reordering" {
+    var r: Ring = .{};
+    var out: [capacity]u8 = undefined;
+    // Push and pop most of the buffer so head sits near the end, then straddle the wrap.
+    for (0..capacity - 3) |i| r.push(@intCast(i & 0xff));
+    _ = r.pop(out[0 .. capacity - 3]);
+    for ("straddle") |b| r.push(b);
+    const n = r.pop(&out);
+    try std.testing.expectEqualStrings("straddle", out[0..n]);
+}
+
+test "a full ring drops the newest and says so" {
+    var r: Ring = .{};
+    for (0..capacity) |i| r.push(@intCast(i & 0xff));
+    try std.testing.expectEqual(@as(u32, 0), r.dropped);
+    r.push('!');
+    r.push('!');
+    try std.testing.expectEqual(@as(u32, 2), r.dropped);
+    // The head is intact: what survived is a prefix of what arrived.
+    var out: [4]u8 = undefined;
+    _ = r.pop(&out);
+    try std.testing.expectEqual(@as(u8, 0), out[0]);
+    try std.testing.expectEqual(@as(u8, 1), out[1]);
+}
+
+/// A UART with a small transmit FIFO, a small RECEIVE FIFO, and a host that keeps typing into it.
+///
+/// The receive FIFO is the part that matters and it is modelled the way the hardware behaves: it has
+/// a fixed depth, and a byte that arrives when it is full is *gone*. That is the whole bug.
+///
+/// Time advances on each transmitter status read, which is what `pump` does while it waits. The
+/// transmitter frees a byte only every fourth tick while a typed byte lands on every one: the
+/// transmitter therefore genuinely FILLS, which is the condition the bug needs. A fake whose FIFO
+/// drains as fast as it fills never blocks, so `pump` never waits, so the rescue never runs and the
+/// test proves nothing - the first version of this fake had exactly that flaw.
+const FakePort = struct {
+    tx_cap: u32,
+    tx_used: u32 = 0,
+    sent: std.ArrayList(u8) = .empty,
+    gpa: std.mem.Allocator,
+
+    incoming: []const u8,
+    delivered: usize = 0,
+    rx: [rx_depth]u8 = undefined,
+    rx_head: usize = 0,
+    rx_len: usize = 0,
+    /// Bytes the wire delivered into a full receive FIFO. The hardware has no counter for this,
+    /// which is exactly why the bug was invisible.
+    lost: u32 = 0,
+
+    ticks: u32 = 0,
+
+    const rx_depth = 8;
+    const tx_drain_every = 4;
+
+    fn tick(p: *FakePort) void {
+        p.ticks += 1;
+        if (p.ticks % tx_drain_every == 0 and p.tx_used > 0) p.tx_used -= 1;
+        if (p.delivered < p.incoming.len) {
+            const b = p.incoming[p.delivered];
+            p.delivered += 1;
+            if (p.rx_len == rx_depth) {
+                p.lost += 1;
+            } else {
+                p.rx[(p.rx_head + p.rx_len) % rx_depth] = b;
+                p.rx_len += 1;
+            }
+        }
+    }
+
+    fn txFree(p: *FakePort) u32 {
+        p.tick();
+        return p.tx_cap - p.tx_used;
+    }
+
+    fn pushByte(p: *FakePort, b: u8) void {
+        p.sent.append(p.gpa, b) catch unreachable;
+        p.tx_used += 1;
+    }
+
+    fn rxCount(p: *FakePort) u32 {
+        return @intCast(p.rx_len);
+    }
+
+    fn popByte(p: *FakePort) u8 {
+        const b = p.rx[p.rx_head];
+        p.rx_head = (p.rx_head + 1) % rx_depth;
+        p.rx_len -= 1;
+        return b;
+    }
+};
+
+test "a long transmit does not lose the input that arrives during it" {
+    // THE REGRESSION. Delete the `rescue` call inside `pump`'s wait and this fails: the receive FIFO
+    // is eight bytes deep, the typing below is far longer than that, and every byte that arrives
+    // into a full FIFO is gone with nothing to record it. That is the die's 88-of-200 in miniature.
+    const typed = "the quick brown fox jumps over the lazy dog, twice over, and then some more";
+    var port: FakePort = .{ .tx_cap = 2, .incoming = typed, .gpa = std.testing.allocator };
+    defer port.sent.deinit(std.testing.allocator);
+    var ring: Ring = .{};
+
+    const frame = "\x1b[1;1H" ++ "x" ** 400;
+    try std.testing.expectEqual(@as(u32, 0), pump(&port, &ring, frame, 1_000_000));
+
+    // Every output byte went out, in order.
+    try std.testing.expectEqualStrings(frame, port.sent.items);
+    // Nothing the wire delivered was dropped, by the FIFO or by the ring.
+    try std.testing.expectEqual(@as(u32, 0), port.lost);
+    try std.testing.expectEqual(@as(u32, 0), ring.dropped);
+    // And what was rescued, plus whatever is still sitting in the FIFO, is exactly what was typed -
+    // in order, which is the other half of the contract.
+    var got: [capacity]u8 = undefined;
+    var n = ring.pop(&got);
+    while (port.rxCount() > 0) : (n += 1) got[n] = port.popByte();
+    try std.testing.expectEqualStrings(typed[0..port.delivered], got[0..n]);
+    try std.testing.expect(port.delivered == typed.len);
+}
+
+test "a transmitter that never drains gives up and reports what it abandoned" {
+    var port: FakePort = .{ .tx_cap = 0, .incoming = "", .gpa = std.testing.allocator };
+    defer port.sent.deinit(std.testing.allocator);
+    var ring: Ring = .{};
+    // tx_cap 0 means txFree is always 0, so no byte can ever go out.
+    try std.testing.expectEqual(@as(u32, 5), pump(&port, &ring, "abcde", 32));
+    try std.testing.expectEqual(@as(usize, 0), port.sent.items.len);
+}
diff --git a/src/pardes/uart.zig b/src/pardes/uart.zig
index d98ac62..7696742 100644
--- a/src/pardes/uart.zig
+++ b/src/pardes/uart.zig
@@ -26,10 +26,15 @@
 //! register, and nothing else.
 
 const hal = @import("hal");
+const input_rescue = @import("input_rescue.zig");
 
 /// UART0: the instance the CH340 is wired to, and the one the ROM and bootloader configured.
 const uart0 = hal.uart.Uart.init(0);
 
+/// Keystrokes taken off the receiver while the transmitter was full. See `input_rescue`: without
+/// this, anything typed into a frame longer than the 128-byte FIFO was silently gone.
+var rescued: input_rescue.Ring = .{};
+
 /// Push `bytes` into the TX FIFO, blocking while it is full.
 ///
 /// The spin is normally bounded by the wire - a full 128-byte FIFO drains in 11 ms at 115200 - and
@@ -47,23 +52,7 @@ const uart0 = hal.uart.Uart.init(0);
 /// The limit is per burst, not per call, and generous: 1,000,000 status reads is far longer than
 /// any legitimate drain and still a fraction of a second.
 pub fn write(bytes: []const u8) void {
-    var rest = bytes;
-    while (rest.len > 0) {
-        // One status read per burst, not per byte.
-        var room = uart0.txFree();
-        var spins: u32 = 0;
-        while (room == 0) {
-            spins += 1;
-            if (spins > 1_000_000) {
-                dropped +%= @intCast(rest.len);
-                return;
-            }
-            room = uart0.txFree();
-        }
-        const n = @min(room, rest.len);
-        for (rest[0..n]) |b| uart0.pushByte(b);
-        rest = rest[n..];
-    }
+    dropped +%= input_rescue.pump(uart0, &rescued, bytes, 1_000_000);
 }
 
 /// Bytes abandoned because the transmitter stopped making progress. Nonzero means the console is
@@ -119,9 +108,27 @@ pub fn dumpWord(v: u32) void {
 /// not stall on a keystroke that may never come. `rxCount` is read once per call and the FIFO
 /// drained to that mark, so a fast typist or a pasted buffer cannot hold the loop here.
 pub fn read(buf: []u8) usize {
-    const waiting = @min(uart0.rxCount(), buf.len);
-    for (buf[0..waiting]) |*slot| slot.* = uart0.popByte();
-    return waiting;
+    // RESCUED BYTES FIRST. They arrived before anything still sitting in the FIFO, and an editor
+    // that reorders keystrokes is worse than one that drops them.
+    var n = rescued.pop(buf);
+    const waiting = @min(uart0.rxCount(), buf.len - n);
+    for (buf[n..][0..waiting]) |*slot| slot.* = uart0.popByte();
+    n += waiting;
+    return n;
+}
+
+/// Take whatever has arrived off the receiver right now, without waiting and without handing it to
+/// anyone. For callers that are about to spend a while not reading: `write` does this while the
+/// transmitter is full, and the loop does it between chunks of input, because applying a keystroke
+/// gets more expensive as the line grows and 128 bytes of FIFO is only 11 ms at 115200.
+pub fn rescueNow() void {
+    input_rescue.rescue(uart0, &rescued);
+}
+
+/// Input abandoned because even the rescue buffer overflowed. Distinct from `dropped`, which is
+/// OUTPUT abandoned by a stalled transmitter.
+pub fn inputDropped() u32 {
+    return rescued.dropped;
 }
 
 /// Discard anything already received, returning how much. Used once at startup: the host-side
@@ -133,6 +140,8 @@ pub fn read(buf: []u8) usize {
 pub fn drainInput() u32 {
     var discarded: u32 = 0;
     while (uart0.rxCount() > 0) : (discarded += 1) _ = uart0.popByte();
+    discarded += @intCast(rescued.len);
+    rescued.clear();
     return discarded;
 }