From 3730bc7df5279c073b23f82f2eb86f8913abe993 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Fri, 18 Sep 2026 18:56:15 +0200 Subject: [PATCH] docs(terminal): correct what selectSession does with the viewport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSDoc on `_syncStickyScrollBaseline` said `selectSession` deliberately ends at the bottom, so the baseline the replay samples is already true there. It does not. `selectSession` calls `scrollToBottom()` after the write and then ends at `scrollToLastNonEmptyLine()` (app.js:6512), which targets `lastNonEmptyLine - rows + 2` and therefore parks ABOVE `baseY` whenever the replayed frame keeps trailing blank rows — which a full capture does on purpose, since no transform that can delete a line may run over one. Its baseline really is a stale true. What covers it is the sticky snap itself: since de864e7d that snap fires only when the flush found the viewport already at the bottom (`preserveViewportY === null`), which a parked selectSession viewport is not. That commit landed on master after this branch was cut, so the guard arrives with the merge rather than being present here. `_onSessionClearTerminal` is unchanged in the comment and was correct: it resets and rewrites with no scroll afterwards, so it does end at the bottom. Comment only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/public/terminal-ui.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index b56f5510..9b56d0aa 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -3142,11 +3142,22 @@ Object.assign(CodemanApp.prototype, { * always true. A caller that then restores the reader's position would have * that restore undone by the next flush. * - * Every caller that scrolls the viewport somewhere other than the bottom - * after a load must call this, so the baseline describes the position the - * caller chose. `selectSession` and `_onSessionClearTerminal` deliberately - * end at the bottom, so for them the sampled true is already the truth and - * they do not call it. + * `_onSessionNeedsRefresh` and `_maybeRefetchFullHistory` restore a position + * and both call this, so their baseline describes the position they chose. + * + * The other two load paths do not call it, for different reasons. + * `_onSessionClearTerminal` resets and rewrites with no scroll afterwards, + * so the sampled true is already the truth there. `selectSession` does NOT + * end at the bottom, whatever its `scrollToBottom()` after the write + * suggests: it ends at `scrollToLastNonEmptyLine()`, which targets + * `lastNonEmptyLine - rows + 2` and therefore parks ABOVE `baseY` whenever + * the replayed frame keeps trailing blank rows, which a full capture does on + * purpose. Its baseline is a stale true. What decides whether that matters + * is the sticky snap in `flushPendingWrites`, and since de864e7d that snap + * fires only when the flush found the viewport already at the bottom + * (`preserveViewportY === null`), which a parked selectSession viewport is + * not. Do not read the absent call here as a claim that selectSession lands + * at the bottom. */ _syncStickyScrollBaseline() { this._wasAtBottomBeforeWrite = this.isTerminalAtBottom();