mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 16:39:42 +02:00
fix(predictive-echo): anchor hold after unpredicted wire edits (review findings)
Independent post-build review found three gaps, all one family: input that changes the composer without a prediction leaves the DISPLAYED cursor stale for one RTT, and anchoring a new run on it painted ghosts one cell off (blank-neutral, so they lived out the full TTL: "tehh" on backspace-then-retype, exactly on the links the feature targets). Fix: the addon now HOLDS new predictions after any such edit (backspace with nothing outstanding = deleting echoed text, clearPredictions, and now also IME/plain-paste 'text' commits, which the hook clears like 'clear') until the next PARSED write releases the hold. The inline predictChar reconcile deliberately does not count: only the emitter pass or the public reconcile() is the display-caught-up contract. Worst case is exactly one unpredicted keystroke, whose own echo releases the hold. Also patched the one bypass path the PR had missed: _handleCjkInput now clears predictions like insertTerminalText and the other bypass sends. Package suite 230, vm gating 85, E2E 10/10 all green after the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -83,6 +83,14 @@ rules and why each exists:
|
|||||||
- **No drop on baseY change**: codex streams push lines to history while the
|
- **No drop on baseY change**: codex streams push lines to history while the
|
||||||
composer stays viewport-pinned; predictions are row-relative to the pinned
|
composer stays viewport-pinned; predictions are row-relative to the pinned
|
||||||
composer and remain valid (measured above).
|
composer and remain valid (measured above).
|
||||||
|
- **Anchor hold** (added by the independent post-build review): after any wire
|
||||||
|
input whose cursor effect the display has not shown yet (backspace with
|
||||||
|
nothing outstanding = deleting echoed text, every 'clear'-classified input,
|
||||||
|
an IME/plain-paste 'text' commit, and the bypass send paths), new
|
||||||
|
predictions are suppressed until the next PARSED write. Anchoring on the
|
||||||
|
stale cursor painted ghosts one cell off ("tehh" on backspace-then-retype
|
||||||
|
within RTT), blank-neutral and therefore TTL-lived. Worst case is exactly
|
||||||
|
one unpredicted keystroke: its own echo is a write, which releases the hold.
|
||||||
- **predictBackspace()** pops the newest outstanding record (informational
|
- **predictBackspace()** pops the newest outstanding record (informational
|
||||||
return; the consumer forwards `\x7f` unconditionally). Deleting already-echoed
|
return; the consumer forwards `\x7f` unconditionally). Deleting already-echoed
|
||||||
text renders at RTT in v1.
|
text renders at RTT in v1.
|
||||||
|
|||||||
@@ -5,6 +5,7 @@
|
|||||||
### Minor Changes
|
### Minor Changes
|
||||||
|
|
||||||
- **New addon: `PredictiveEchoAddon`, mosh-style write-through prediction.** The second echo mode for per-keystroke TUIs (OpenAI Codex's composer, live pickers) that buffer-until-Enter starves. Every keystroke is sent by the consumer immediately and unchanged; the addon paints the predicted glyph at the predicted cell and reconciles against the PARSED terminal buffer: confirmation requires the cell match plus a cursor advance past the record, foreign non-blank content on two consecutive passes cascades a drop, blank cells are neutral, a TTL bounds everything, and scroll/resize/sustained cursor moves clear the run. Visual-only by construction; it cannot gate, delay or rewrite input.
|
- **New addon: `PredictiveEchoAddon`, mosh-style write-through prediction.** The second echo mode for per-keystroke TUIs (OpenAI Codex's composer, live pickers) that buffer-until-Enter starves. Every keystroke is sent by the consumer immediately and unchanged; the addon paints the predicted glyph at the predicted cell and reconciles against the PARSED terminal buffer: confirmation requires the cell match plus a cursor advance past the record, foreign non-blank content on two consecutive passes cascades a drop, blank cells are neutral, a TTL bounds everything, and scroll/resize/sustained cursor moves clear the run. Visual-only by construction; it cannot gate, delay or rewrite input.
|
||||||
|
- Anchor-hold rule: after an unpredicted wire edit (backspace into echoed text, cleared input, an IME text commit) new predictions hold until the next parsed write, so a stale displayed cursor can never mis-anchor a run (worst case: exactly one unpredicted keystroke).
|
||||||
- New exports: `PredictiveEchoAddon`, `PredictiveEchoOptions`, `PredictionState`, plus the long-intended `charCellWidth` / `stringCellWidth` helpers.
|
- New exports: `PredictiveEchoAddon`, `PredictiveEchoOptions`, `PredictionState`, plus the long-intended `charCellWidth` / `stringCellWidth` helpers.
|
||||||
- `XtermTerminal` type gains OPTIONAL members (`buffer.active.cursorX/cursorY`, `getLine().getCell?`, `onWriteParsed?`, `onResize?`). Additive only: existing consumers and mocks are unaffected.
|
- `XtermTerminal` type gains OPTIONAL members (`buffer.active.cursorX/cursorY`, `getLine().getCell?`, `onWriteParsed?`, `onResize?`). Additive only: existing consumers and mocks are unaffected.
|
||||||
- IIFE build exposes `window.PredictiveEchoAddon` and a self-activating `window.PredictiveEchoOverlay`, alongside the unchanged `ZerolagInputAddon` / `LocalEchoOverlay` globals.
|
- IIFE build exposes `window.PredictiveEchoAddon` and a self-activating `window.PredictiveEchoOverlay`, alongside the unchanged `ZerolagInputAddon` / `LocalEchoOverlay` globals.
|
||||||
|
|||||||
@@ -333,7 +333,12 @@ identical in-place repaint, never false-confirms). A cell showing foreign
|
|||||||
non-blank content on two consecutive passes drops that prediction and all
|
non-blank content on two consecutive passes drops that prediction and all
|
||||||
later ones (one pass tolerates half-parsed frames). Blank cells are neutral:
|
later ones (one pass tolerates half-parsed frames). Blank cells are neutral:
|
||||||
they are what "not yet echoed" looks like. Whatever remains is dropped by TTL.
|
they are what "not yet echoed" looks like. Whatever remains is dropped by TTL.
|
||||||
Scrolling up, resizing, or a sustained cursor move clears the run.
|
Scrolling up, resizing, or a sustained cursor move clears the run. After a
|
||||||
|
backspace into already-echoed text, a cleared input, or a multi-char commit,
|
||||||
|
the addon **holds** new predictions until the next parsed write: the displayed
|
||||||
|
cursor is stale for one round trip, and anchoring on it would paint ghosts one
|
||||||
|
cell off (worst case: exactly one unpredicted keystroke, whose own echo
|
||||||
|
releases the hold).
|
||||||
|
|
||||||
### API
|
### API
|
||||||
|
|
||||||
|
|||||||
@@ -88,6 +88,14 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
private _confirmedTotal = 0;
|
private _confirmedTotal = 0;
|
||||||
private _droppedTotal = 0;
|
private _droppedTotal = 0;
|
||||||
private _ttlTimer: ReturnType<typeof setTimeout> | null = null;
|
private _ttlTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
|
/** Anchor hold: set after an unpredicted wire edit (backspace into echoed
|
||||||
|
* text, any cleared input, an IME text commit). While held, new
|
||||||
|
* predictions are suppressed: the displayed cursor is stale until the
|
||||||
|
* next parsed write, and anchoring on it paints ghosts one cell off
|
||||||
|
* (found by review: backspace-then-retype within RTT). Cleared by the
|
||||||
|
* onWriteParsed pass and by public reconcile(), never by the inline
|
||||||
|
* predictChar pass (which runs before the display could catch up). */
|
||||||
|
private _anchorHold = false;
|
||||||
private _reconcileScheduled = false;
|
private _reconcileScheduled = false;
|
||||||
private _disposables: Array<{ dispose(): void }> = [];
|
private _disposables: Array<{ dispose(): void }> = [];
|
||||||
private _predictWhen: ((terminal: XtermTerminal) => boolean) | null;
|
private _predictWhen: ((terminal: XtermTerminal) => boolean) | null;
|
||||||
@@ -141,6 +149,7 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
this._reconcileScheduled = true;
|
this._reconcileScheduled = true;
|
||||||
queueMicrotask(() => {
|
queueMicrotask(() => {
|
||||||
this._reconcileScheduled = false;
|
this._reconcileScheduled = false;
|
||||||
|
this._anchorHold = false; // a parse pass ran: the display caught up
|
||||||
this._safeReconcile();
|
this._safeReconcile();
|
||||||
});
|
});
|
||||||
})
|
})
|
||||||
@@ -183,6 +192,7 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
predictChar(ch: string): boolean {
|
predictChar(ch: string): boolean {
|
||||||
try {
|
try {
|
||||||
this._reconcile();
|
this._reconcile();
|
||||||
|
if (this._anchorHold) return false; // display has not caught up with a wire edit
|
||||||
|
|
||||||
const t = this._terminal;
|
const t = this._terminal;
|
||||||
if (!t || !this._container) return false;
|
if (!t || !this._container) return false;
|
||||||
@@ -247,7 +257,12 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
predictBackspace(): boolean {
|
predictBackspace(): boolean {
|
||||||
try {
|
try {
|
||||||
const rec = this._outstanding.pop();
|
const rec = this._outstanding.pop();
|
||||||
if (!rec) return false;
|
if (!rec) {
|
||||||
|
// \x7f goes to the wire and will delete ECHOED text: the cursor is
|
||||||
|
// about to move in a way we cannot see yet
|
||||||
|
this._anchorHold = true;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
removePredictionSpan(this._spans, rec.seq);
|
removePredictionSpan(this._spans, rec.seq);
|
||||||
if (this._outstanding.length === 0) this._resetRun();
|
if (this._outstanding.length === 0) this._resetRun();
|
||||||
return true;
|
return true;
|
||||||
@@ -256,9 +271,12 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Drop every outstanding prediction and its spans. */
|
/** Drop every outstanding prediction and its spans. Also arms the anchor
|
||||||
|
* hold: consumers clear on inputs (Enter, Esc, arrows, pastes) whose
|
||||||
|
* cursor effect is unknown until the next parsed write. */
|
||||||
clearPredictions(): void {
|
clearPredictions(): void {
|
||||||
try {
|
try {
|
||||||
|
this._anchorHold = true;
|
||||||
this._droppedTotal += this._outstanding.length;
|
this._droppedTotal += this._outstanding.length;
|
||||||
this._outstanding = [];
|
this._outstanding = [];
|
||||||
clearAllSpans(this._spans);
|
clearAllSpans(this._spans);
|
||||||
@@ -268,8 +286,10 @@ export class PredictiveEchoAddon implements XtermAddon {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Manual reconcile pass, for consumers without onWriteParsed. */
|
/** Manual reconcile pass, for consumers without onWriteParsed. By contract
|
||||||
|
* it is called after writes parsed, so it also releases the anchor hold. */
|
||||||
reconcile(): void {
|
reconcile(): void {
|
||||||
|
this._anchorHold = false;
|
||||||
this._safeReconcile();
|
this._safeReconcile();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -62,7 +62,7 @@ async function replay(name: string) {
|
|||||||
let painted = false;
|
let painted = false;
|
||||||
if (kind === 'char') painted = addon.predictChar(line.data);
|
if (kind === 'char') painted = addon.predictChar(line.data);
|
||||||
else if (kind === 'backspace') addon.predictBackspace();
|
else if (kind === 'backspace') addon.predictBackspace();
|
||||||
else if (kind === 'clear') addon.clearPredictions();
|
else addon.clearPredictions(); // 'clear' AND 'text', like the terminal-ui hook
|
||||||
// Span/record parity and grid bounds hold at every step
|
// Span/record parity and grid bounds hold at every step
|
||||||
expect(rt.spanCount()).toBe(addon.state.outstanding);
|
expect(rt.spanCount()).toBe(addon.state.outstanding);
|
||||||
assertSpansInGrid(rt);
|
assertSpansInGrid(rt);
|
||||||
|
|||||||
@@ -335,6 +335,7 @@ describe('PredictiveEchoAddon', () => {
|
|||||||
|
|
||||||
it('predictBackspace pops newest, returns false when empty, never touches confirmed', async () => {
|
it('predictBackspace pops newest, returns false when empty, never touches confirmed', async () => {
|
||||||
expect(addon.predictBackspace()).toBe(false);
|
expect(addon.predictBackspace()).toBe(false);
|
||||||
|
addon.reconcile(); // the empty pop armed the anchor hold; release it
|
||||||
addon.predictChar('a');
|
addon.predictChar('a');
|
||||||
addon.predictChar('b');
|
addon.predictChar('b');
|
||||||
expect(addon.predictBackspace()).toBe(true);
|
expect(addon.predictBackspace()).toBe(true);
|
||||||
@@ -478,6 +479,7 @@ describe('PredictiveEchoAddon', () => {
|
|||||||
rows.style.color = 'rgb(255, 0, 0)'; // skin change
|
rows.style.color = 'rgb(255, 0, 0)'; // skin change
|
||||||
a.refreshFont();
|
a.refreshFont();
|
||||||
a.clearPredictions();
|
a.clearPredictions();
|
||||||
|
a.reconcile(); // release the anchor hold armed by the clear
|
||||||
a.predictChar('v');
|
a.predictChar('v');
|
||||||
const span2 = themed.terminal.element.querySelector('.xterm-screen span') as HTMLSpanElement;
|
const span2 = themed.terminal.element.querySelector('.xterm-screen span') as HTMLSpanElement;
|
||||||
expect(span2.style.color).toBe('rgb(255, 0, 0)');
|
expect(span2.style.color).toBe('rgb(255, 0, 0)');
|
||||||
@@ -485,6 +487,37 @@ describe('PredictiveEchoAddon', () => {
|
|||||||
themed.cleanup();
|
themed.cleanup();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('anchor hold: backspace into echoed text suppresses prediction until a write parses', async () => {
|
||||||
|
// \x7f went to the wire with nothing outstanding: the cursor will move
|
||||||
|
// in a way the display has not shown, so anchoring now paints one cell
|
||||||
|
// off (review finding: "tehh" ghosts on backspace-then-retype at RTT)
|
||||||
|
expect(addon.predictBackspace()).toBe(false);
|
||||||
|
expect(addon.predictChar('x')).toBe(false);
|
||||||
|
expect(spansOf(mock)).toHaveLength(0);
|
||||||
|
mock.fireWriteParsed(); // the display caught up
|
||||||
|
await flushMicrotasks();
|
||||||
|
expect(addon.predictChar('x')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('anchor hold: clearPredictions suppresses until a write parses (or manual reconcile)', async () => {
|
||||||
|
addon.predictChar('a');
|
||||||
|
addon.clearPredictions(); // consumer saw Enter/Esc/arrow/paste
|
||||||
|
expect(addon.predictChar('b')).toBe(false);
|
||||||
|
mock.fireWriteParsed();
|
||||||
|
await flushMicrotasks();
|
||||||
|
expect(addon.predictChar('b')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('anchor hold: the inline predictChar reconcile does NOT release it', () => {
|
||||||
|
addon.clearPredictions();
|
||||||
|
// Several keystrokes in a row before any echo: all suppressed, because
|
||||||
|
// predictChar's inline pass must not count as the display catching up
|
||||||
|
expect(addon.predictChar('a')).toBe(false);
|
||||||
|
expect(addon.predictChar('b')).toBe(false);
|
||||||
|
addon.reconcile(); // public/manual pass IS the caught-up contract
|
||||||
|
expect(addon.predictChar('c')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('state getter reports outstanding/confirmedTotal/droppedTotal/anchor', async () => {
|
it('state getter reports outstanding/confirmedTotal/droppedTotal/anchor', async () => {
|
||||||
expect(addon.state).toEqual({ outstanding: 0, confirmedTotal: 0, droppedTotal: 0, anchor: null });
|
expect(addon.state).toEqual({ outstanding: 0, confirmedTotal: 0, droppedTotal: 0, anchor: null });
|
||||||
addon.predictChar('a');
|
addon.predictChar('a');
|
||||||
|
|||||||
@@ -2563,8 +2563,10 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
const kind = window.CodemanTerminalInput.classifyPredictInput(data);
|
const kind = window.CodemanTerminalInput.classifyPredictInput(data);
|
||||||
if (kind === 'char') this._predictiveEcho.predictChar(data);
|
if (kind === 'char') this._predictiveEcho.predictChar(data);
|
||||||
else if (kind === 'backspace') this._predictiveEcho.predictBackspace();
|
else if (kind === 'backspace') this._predictiveEcho.predictBackspace();
|
||||||
else if (kind === 'clear') this._predictiveEcho.clearPredictions();
|
// 'clear' AND 'text' (plain paste, IME word commits) both change the
|
||||||
// kind === 'text' (plain multi-char paste): wire only, no visual
|
// composer in ways the display has not shown yet: clear the run and let
|
||||||
|
// the addon's anchor hold suppress prediction until the echo catches up
|
||||||
|
else this._predictiveEcho.clearPredictions();
|
||||||
} catch {
|
} catch {
|
||||||
/* predictions must never block the wire */
|
/* predictions must never block the wire */
|
||||||
}
|
}
|
||||||
@@ -2577,6 +2579,8 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
_crashDiag.log(`CJK send DROP no-session len=${text.length}`);
|
_crashDiag.log(`CJK send DROP no-session len=${text.length}`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// Bypasses onData (like insertTerminalText): predictions cannot see this
|
||||||
|
if (this._localEchoPolicy === 'predict') this._predictiveEcho?.clearPredictions();
|
||||||
_crashDiag.log(`CJK send→${this.activeSessionId.slice(0, 8)} len=${text.length}`);
|
_crashDiag.log(`CJK send→${this.activeSessionId.slice(0, 8)} len=${text.length}`);
|
||||||
this._sendInputAsync(this.activeSessionId, text);
|
this._sendInputAsync(this.activeSessionId, text);
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -411,11 +411,13 @@ describe('_predictHookOnData (wire neutrality)', () => {
|
|||||||
expect(app._predictiveEcho!.clearPredictions).toHaveBeenCalled();
|
expect(app._predictiveEcho!.clearPredictions).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("kind 'text' (plain paste) takes no visual action", () => {
|
it("kind 'text' (plain paste, IME commit) clears the run like 'clear'", () => {
|
||||||
|
// Review finding: an IME word-commit changes the composer without a
|
||||||
|
// prediction; new predictions after it would mis-anchor until cascade.
|
||||||
const app = makePredictApp();
|
const app = makePredictApp();
|
||||||
app._predictHookOnData('pasted text');
|
app._predictHookOnData('pasted text');
|
||||||
expect(app._predictiveEcho!.predictChar).not.toHaveBeenCalled();
|
expect(app._predictiveEcho!.predictChar).not.toHaveBeenCalled();
|
||||||
expect(app._predictiveEcho!.clearPredictions).not.toHaveBeenCalled();
|
expect(app._predictiveEcho!.clearPredictions).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('never touches _pendingInput and never sends (visual-only pin)', () => {
|
it('never touches _pendingInput and never sends (visual-only pin)', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user