fix(client): never leave predicted echo on a moved or repainted row
Guesses survived a Full frame and were drawn again at their old position over the new content, and a daemon frame that scrolled carried drawn guesses up a row where nothing repaired them. The client now erases drawn guesses before writing each daemon frame and redraws them after, forgets guesses on a Full frame, and draws none while an Enter awaits its echo.
- commit
- dced086a00176fef80b8317802dcba066fa74fbe
- author
- brice · · 2026-10-08 05:58 UTC
- parent
- 861e555
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +101 -4
M internal/usecase/client/attachment_echo.go +16 -0
| @@ -53,6 +53,22 @@ func (e *attachmentEcho) input(seq uint64, data []byte) error { | |||
| 53 | 53 | return e.draw() | |
| 54 | 54 | } | |
| 55 | 55 | ||
| 56 | + | // beforeOutput erases drawn guesses before the foreground writes a daemon | |
| 57 | + | // frame, so the frame never scrolls or overwrites around a guess. | |
| 58 | + | func (e *attachmentEcho) beforeOutput(output protocol.Output) error { | |
| 59 | + | if e == nil || len(output.Data) == 0 { | |
| 60 | + | return nil | |
| 61 | + | } | |
| 62 | + | data := e.predictor.undraw() | |
| 63 | + | if len(data) == 0 { | |
| 64 | + | return nil | |
| 65 | + | } | |
| 66 | + | if _, err := e.out.writePrediction(data); err != nil { | |
| 67 | + | return fmt.Errorf("erasing predicted echo: %w", err) | |
| 68 | + | } | |
| 69 | + | return nil | |
| 70 | + | } | |
| 71 | + | ||
| 56 | 72 | // output mirrors one Output after the foreground wrote it. | |
| 57 | 73 | func (e *attachmentEcho) output(output protocol.Output) error { | |
| 58 | 74 | if e == nil { | |
| @@ -53,6 +53,22 @@ func (e *attachmentEcho) input(seq uint64, data []byte) error { | |||||
| 53 | return e.draw() | 53 | return e.draw() | ||
| 54 | } | 54 | } | ||
| 55 | 55 | ||||
| 56 | + | // beforeOutput erases drawn guesses before the foreground writes a daemon | |||
| 57 | + | // frame, so the frame never scrolls or overwrites around a guess. | |||
| 58 | + | func (e *attachmentEcho) beforeOutput(output protocol.Output) error { | |||
| 59 | + | if e == nil || len(output.Data) == 0 { | |||
| 60 | + | return nil | |||
| 61 | + | } | |||
| 62 | + | data := e.predictor.undraw() | |||
| 63 | + | if len(data) == 0 { | |||
| 64 | + | return nil | |||
| 65 | + | } | |||
| 66 | + | if _, err := e.out.writePrediction(data); err != nil { | |||
| 67 | + | return fmt.Errorf("erasing predicted echo: %w", err) | |||
| 68 | + | } | |||
| 69 | + | return nil | |||
| 70 | + | } | |||
| 71 | + | ||||
| 56 | // output mirrors one Output after the foreground wrote it. | 72 | // output mirrors one Output after the foreground wrote it. | ||
| 57 | func (e *attachmentEcho) output(output protocol.Output) error { | 73 | func (e *attachmentEcho) output(output protocol.Output) error { | ||
| 58 | if e == nil { | 74 | if e == nil { | ||
M internal/usecase/client/attachment_session.go +3 -0
| @@ -542,6 +542,9 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 542 | 542 | } | |
| 543 | 543 | } | |
| 544 | 544 | w.noteCommitted(fg, state.context) | |
| 545 | + | if err := echo.beforeOutput(typed); err != nil { | |
| 546 | + | return w.settle(ctx, fg, stream, token, err) | |
| 547 | + | } | |
| 545 | 548 | if err := fg.Output(state.uiContext(ports.UIContext{Generation: attachmentActionableGeneration(fg, token)}, ports.UIStatusAttached), typed.Data); err != nil { | |
| 546 | 549 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: fmt.Errorf("publishing output: %w", err)} | |
| 547 | 550 | } | |
| @@ -542,6 +542,9 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 542 | } | 542 | } | ||
| 543 | } | 543 | } | ||
| 544 | w.noteCommitted(fg, state.context) | 544 | w.noteCommitted(fg, state.context) | ||
| 545 | + | if err := echo.beforeOutput(typed); err != nil { | |||
| 546 | + | return w.settle(ctx, fg, stream, token, err) | |||
| 547 | + | } | |||
| 545 | if err := fg.Output(state.uiContext(ports.UIContext{Generation: attachmentActionableGeneration(fg, token)}, ports.UIStatusAttached), typed.Data); err != nil { | 548 | if err := fg.Output(state.uiContext(ports.UIContext{Generation: attachmentActionableGeneration(fg, token)}, ports.UIStatusAttached), typed.Data); err != nil { | ||
| 546 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: fmt.Errorf("publishing output: %w", err)} | 549 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: fmt.Errorf("publishing output: %w", err)} | ||
| 547 | } | 550 | } | ||
M internal/usecase/client/predictive_echo.go +30 -4
| @@ -158,9 +158,9 @@ func (p *echoPredictor) applyOutput(output protocol.Output, now time.Time) { | |||
| 158 | 158 | p.forget() | |
| 159 | 159 | } | |
| 160 | 160 | if output.Full { | |
| 161 | - | // A full frame repainted every drawn guess. | |
| 162 | - | p.drawn = p.drawn[:0] | |
| 163 | - | p.cursorMove = false | |
| 161 | + | // A full frame repaints everything: guesses placed against the old | |
| 162 | + | // frame would land on unrelated content. | |
| 163 | + | p.forget() | |
| 164 | 164 | } | |
| 165 | 165 | if data := p.apc.strip(output.Data); len(data) > 0 { | |
| 166 | 166 | p.screen.Write(data) | |
| @@ -608,6 +608,30 @@ func (p *echoPredictor) needsTick() bool { | |||
| 608 | 608 | return p.mode == domain.EchoPredictAdaptive && p.active() && !(p.glitch > 0 && p.flagging) | |
| 609 | 609 | } | |
| 610 | 610 | ||
| 611 | + | // undraw returns the bytes that repaint every drawn guess from the mirror, so | |
| 612 | + | // a daemon frame lands on the screen the daemon believes it has. A frame that | |
| 613 | + | // scrolls would otherwise move a guess where nothing ever repairs it. The | |
| 614 | + | // guesses stay pending and render draws them again after the frame. | |
| 615 | + | func (p *echoPredictor) undraw() []byte { | |
| 616 | + | if len(p.drawn) == 0 { | |
| 617 | + | return nil | |
| 618 | + | } | |
| 619 | + | var out bytes.Buffer | |
| 620 | + | if p.drawnRow < p.screen.Rows() { | |
| 621 | + | for _, col := range p.drawn { | |
| 622 | + | if col < p.screen.Columns() { | |
| 623 | + | p.restoreCell(&out, p.drawnRow, col) | |
| 624 | + | } | |
| 625 | + | } | |
| 626 | + | } | |
| 627 | + | p.drawn = p.drawn[:0] | |
| 628 | + | if out.Len() > 0 { | |
| 629 | + | out.WriteString("\x1b[0m") | |
| 630 | + | writeEchoCUP(&out, p.screen.CursorRow(), p.screen.CursorCol()) | |
| 631 | + | } | |
| 632 | + | return out.Bytes() | |
| 633 | + | } | |
| 634 | + | ||
| 611 | 635 | // render returns the terminal bytes that bring the screen from the last | |
| 612 | 636 | // render to the current guesses: drawn cells that no longer show a guess are | |
| 613 | 637 | // repainted from the mirror, shown guesses are drawn again (a daemon frame | |
| @@ -617,7 +641,9 @@ func (p *echoPredictor) render() []byte { | |||
| 617 | 641 | var out bytes.Buffer | |
| 618 | 642 | show := p.displaying() && p.enabled() | |
| 619 | 643 | var want []int | |
| 620 | - | if show && p.cells != nil && p.row < p.screen.Rows() { | |
| 644 | + | // Until the daemon answers an unknown key (Enter), the guesses' row may | |
| 645 | + | // have moved, so none is drawn. | |
| 646 | + | if show && p.cells != nil && p.row < p.screen.Rows() && p.echo >= p.holdUntil { | |
| 621 | 647 | for col := range p.cells { | |
| 622 | 648 | cell := &p.cells[col] | |
| 623 | 649 | if !cell.active || cell.unknown || cell.tentative(p.confEpoch) { | |
| @@ -158,9 +158,9 @@ func (p *echoPredictor) applyOutput(output protocol.Output, now time.Time) { | |||||
| 158 | p.forget() | 158 | p.forget() | ||
| 159 | } | 159 | } | ||
| 160 | if output.Full { | 160 | if output.Full { | ||
| 161 | - | // A full frame repainted every drawn guess. | 161 | + | // A full frame repaints everything: guesses placed against the old |
| 162 | - | p.drawn = p.drawn[:0] | 162 | + | // frame would land on unrelated content. |
| 163 | - | p.cursorMove = false | 163 | + | p.forget() |
| 164 | } | 164 | } | ||
| 165 | if data := p.apc.strip(output.Data); len(data) > 0 { | 165 | if data := p.apc.strip(output.Data); len(data) > 0 { | ||
| 166 | p.screen.Write(data) | 166 | p.screen.Write(data) | ||
| @@ -608,6 +608,30 @@ func (p *echoPredictor) needsTick() bool { | |||||
| 608 | return p.mode == domain.EchoPredictAdaptive && p.active() && !(p.glitch > 0 && p.flagging) | 608 | return p.mode == domain.EchoPredictAdaptive && p.active() && !(p.glitch > 0 && p.flagging) | ||
| 609 | } | 609 | } | ||
| 610 | 610 | ||||
| 611 | + | // undraw returns the bytes that repaint every drawn guess from the mirror, so | |||
| 612 | + | // a daemon frame lands on the screen the daemon believes it has. A frame that | |||
| 613 | + | // scrolls would otherwise move a guess where nothing ever repairs it. The | |||
| 614 | + | // guesses stay pending and render draws them again after the frame. | |||
| 615 | + | func (p *echoPredictor) undraw() []byte { | |||
| 616 | + | if len(p.drawn) == 0 { | |||
| 617 | + | return nil | |||
| 618 | + | } | |||
| 619 | + | var out bytes.Buffer | |||
| 620 | + | if p.drawnRow < p.screen.Rows() { | |||
| 621 | + | for _, col := range p.drawn { | |||
| 622 | + | if col < p.screen.Columns() { | |||
| 623 | + | p.restoreCell(&out, p.drawnRow, col) | |||
| 624 | + | } | |||
| 625 | + | } | |||
| 626 | + | } | |||
| 627 | + | p.drawn = p.drawn[:0] | |||
| 628 | + | if out.Len() > 0 { | |||
| 629 | + | out.WriteString("\x1b[0m") | |||
| 630 | + | writeEchoCUP(&out, p.screen.CursorRow(), p.screen.CursorCol()) | |||
| 631 | + | } | |||
| 632 | + | return out.Bytes() | |||
| 633 | + | } | |||
| 634 | + | ||||
| 611 | // render returns the terminal bytes that bring the screen from the last | 635 | // render returns the terminal bytes that bring the screen from the last | ||
| 612 | // render to the current guesses: drawn cells that no longer show a guess are | 636 | // render to the current guesses: drawn cells that no longer show a guess are | ||
| 613 | // repainted from the mirror, shown guesses are drawn again (a daemon frame | 637 | // repainted from the mirror, shown guesses are drawn again (a daemon frame | ||
| @@ -617,7 +641,9 @@ func (p *echoPredictor) render() []byte { | |||||
| 617 | var out bytes.Buffer | 641 | var out bytes.Buffer | ||
| 618 | show := p.displaying() && p.enabled() | 642 | show := p.displaying() && p.enabled() | ||
| 619 | var want []int | 643 | var want []int | ||
| 620 | - | if show && p.cells != nil && p.row < p.screen.Rows() { | 644 | + | // Until the daemon answers an unknown key (Enter), the guesses' row may |
| 645 | + | // have moved, so none is drawn. | |||
| 646 | + | if show && p.cells != nil && p.row < p.screen.Rows() && p.echo >= p.holdUntil { | |||
| 621 | for col := range p.cells { | 647 | for col := range p.cells { | ||
| 622 | cell := &p.cells[col] | 648 | cell := &p.cells[col] | ||
| 623 | if !cell.active || cell.unknown || cell.tentative(p.confEpoch) { | 649 | if !cell.active || cell.unknown || cell.tentative(p.confEpoch) { | ||
M internal/usecase/client/predictive_echo_test.go +52 -0
| @@ -90,6 +90,58 @@ func TestEchoPredictorDisplay(t *testing.T) { | |||
| 90 | 90 | } | |
| 91 | 91 | } | |
| 92 | 92 | ||
| 93 | + | // TestEchoPredictorNeverDrawsStaleGuesses covers daemon frames that arrive | |
| 94 | + | // before the echo acknowledgement and move or replace the guessed row. | |
| 95 | + | func TestEchoPredictorNeverDrawsStaleGuesses(t *testing.T) { | |
| 96 | + | t.Parallel() | |
| 97 | + | tests := []struct { | |
| 98 | + | name string | |
| 99 | + | input string | |
| 100 | + | frame protocol.Output | |
| 101 | + | }{ | |
| 102 | + | { | |
| 103 | + | name: "enter then a scrolled prompt before the ack", | |
| 104 | + | input: "b\r", | |
| 105 | + | frame: protocol.Output{Epoch: 1, New: 2, Size: domain.Size{Cols: 20, Rows: 3}, Data: []byte("b\r\nfoo\r\nbar\r\n$ ")}, | |
| 106 | + | }, | |
| 107 | + | { | |
| 108 | + | name: "full frame before the ack", | |
| 109 | + | input: "b", | |
| 110 | + | frame: protocol.Output{Epoch: 1, New: 2, Full: true, Size: domain.Size{Cols: 20, Rows: 3}, Data: []byte("\x1b[H\x1b[2Jother")}, | |
| 111 | + | }, | |
| 112 | + | } | |
| 113 | + | for _, tt := range tests { | |
| 114 | + | t.Run(tt.name, func(t *testing.T) { | |
| 115 | + | t.Parallel() | |
| 116 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |
| 117 | + | h.warm(100 * time.Millisecond) | |
| 118 | + | h.confirm() | |
| 119 | + | h.typed(tt.input) | |
| 120 | + | h.p.undraw() | |
| 121 | + | h.p.applyOutput(tt.frame, h.now) | |
| 122 | + | if got := h.p.render(); bytes.Contains(got, []byte("b")) { | |
| 123 | + | t.Fatalf("render = %q redraws a guess over the new frame", got) | |
| 124 | + | } | |
| 125 | + | }) | |
| 126 | + | } | |
| 127 | + | } | |
| 128 | + | ||
| 129 | + | func TestEchoPredictorUndrawRestoresMirror(t *testing.T) { | |
| 130 | + | t.Parallel() | |
| 131 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |
| 132 | + | h.warm(100 * time.Millisecond) | |
| 133 | + | h.confirm() | |
| 134 | + | if got := h.typed("b"); !bytes.Contains(got, []byte("b")) { | |
| 135 | + | t.Fatalf("render = %q, want the guess drawn", got) | |
| 136 | + | } | |
| 137 | + | if got, want := string(h.p.undraw()), "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"; got != want { | |
| 138 | + | t.Fatalf("undraw = %q, want %q", got, want) | |
| 139 | + | } | |
| 140 | + | if got := h.p.undraw(); len(got) != 0 { | |
| 141 | + | t.Fatalf("second undraw = %q, want nothing", got) | |
| 142 | + | } | |
| 143 | + | } | |
| 144 | + | ||
| 93 | 145 | func TestEchoPredictorConfirmationRepaintsFromMirror(t *testing.T) { | |
| 94 | 146 | t.Parallel() | |
| 95 | 147 | h := newEchoHarness(t, domain.EchoPredictAdaptive) | |
| @@ -90,6 +90,58 @@ func TestEchoPredictorDisplay(t *testing.T) { | |||||
| 90 | } | 90 | } | ||
| 91 | } | 91 | } | ||
| 92 | 92 | ||||
| 93 | + | // TestEchoPredictorNeverDrawsStaleGuesses covers daemon frames that arrive | |||
| 94 | + | // before the echo acknowledgement and move or replace the guessed row. | |||
| 95 | + | func TestEchoPredictorNeverDrawsStaleGuesses(t *testing.T) { | |||
| 96 | + | t.Parallel() | |||
| 97 | + | tests := []struct { | |||
| 98 | + | name string | |||
| 99 | + | input string | |||
| 100 | + | frame protocol.Output | |||
| 101 | + | }{ | |||
| 102 | + | { | |||
| 103 | + | name: "enter then a scrolled prompt before the ack", | |||
| 104 | + | input: "b\r", | |||
| 105 | + | frame: protocol.Output{Epoch: 1, New: 2, Size: domain.Size{Cols: 20, Rows: 3}, Data: []byte("b\r\nfoo\r\nbar\r\n$ ")}, | |||
| 106 | + | }, | |||
| 107 | + | { | |||
| 108 | + | name: "full frame before the ack", | |||
| 109 | + | input: "b", | |||
| 110 | + | frame: protocol.Output{Epoch: 1, New: 2, Full: true, Size: domain.Size{Cols: 20, Rows: 3}, Data: []byte("\x1b[H\x1b[2Jother")}, | |||
| 111 | + | }, | |||
| 112 | + | } | |||
| 113 | + | for _, tt := range tests { | |||
| 114 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 115 | + | t.Parallel() | |||
| 116 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |||
| 117 | + | h.warm(100 * time.Millisecond) | |||
| 118 | + | h.confirm() | |||
| 119 | + | h.typed(tt.input) | |||
| 120 | + | h.p.undraw() | |||
| 121 | + | h.p.applyOutput(tt.frame, h.now) | |||
| 122 | + | if got := h.p.render(); bytes.Contains(got, []byte("b")) { | |||
| 123 | + | t.Fatalf("render = %q redraws a guess over the new frame", got) | |||
| 124 | + | } | |||
| 125 | + | }) | |||
| 126 | + | } | |||
| 127 | + | } | |||
| 128 | + | ||||
| 129 | + | func TestEchoPredictorUndrawRestoresMirror(t *testing.T) { | |||
| 130 | + | t.Parallel() | |||
| 131 | + | h := newEchoHarness(t, domain.EchoPredictAlways) | |||
| 132 | + | h.warm(100 * time.Millisecond) | |||
| 133 | + | h.confirm() | |||
| 134 | + | if got := h.typed("b"); !bytes.Contains(got, []byte("b")) { | |||
| 135 | + | t.Fatalf("render = %q, want the guess drawn", got) | |||
| 136 | + | } | |||
| 137 | + | if got, want := string(h.p.undraw()), "\x1b[1;4H\x1b[0m \x1b[0m\x1b[1;4H"; got != want { | |||
| 138 | + | t.Fatalf("undraw = %q, want %q", got, want) | |||
| 139 | + | } | |||
| 140 | + | if got := h.p.undraw(); len(got) != 0 { | |||
| 141 | + | t.Fatalf("second undraw = %q, want nothing", got) | |||
| 142 | + | } | |||
| 143 | + | } | |||
| 144 | + | ||||
| 93 | func TestEchoPredictorConfirmationRepaintsFromMirror(t *testing.T) { | 145 | func TestEchoPredictorConfirmationRepaintsFromMirror(t *testing.T) { | ||
| 94 | t.Parallel() | 146 | t.Parallel() | ||
| 95 | h := newEchoHarness(t, domain.EchoPredictAdaptive) | 147 | h := newEchoHarness(t, domain.EchoPredictAdaptive) | ||