From 49b4ef50f8e5eea12877c5a6423823da18db0563 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 16:24:11 +0000 Subject: [PATCH] fix(tui): fixed audited scrollback tail rows Separated audited offerable transcript rows from durable snapshot rows so lower finalized content below a live block can be repaired instead of duplicated when the live block grows. Added transcript and virtual-terminal regressions for the lower finalized tail case. Fixes #4326 --- packages/coding-agent/CHANGELOG.md | 4 + .../modes/components/transcript-container.ts | 17 ++++ .../transcript-streaming-commit-repro.test.ts | 25 +++++ packages/tui/CHANGELOG.md | 4 + packages/tui/src/tui.ts | 97 +++++++++++++++---- .../test/streaming-scrollback-defer.test.ts | 40 ++++++++ 6 files changed, 167 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bb87143c3..e1878e241 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed transcript native-scrollback boundaries so finalized content below a live block is offered while still audited, preventing stale lower-row duplication when the live block grows ([#4326](https://github.com/can1357/oh-my-pi/issues/4326)). + ## [16.3.2] - 2026-07-02 ### Breaking Changes diff --git a/packages/coding-agent/src/modes/components/transcript-container.ts b/packages/coding-agent/src/modes/components/transcript-container.ts index a4abc2379..1f5f74737 100644 --- a/packages/coding-agent/src/modes/components/transcript-container.ts +++ b/packages/coding-agent/src/modes/components/transcript-container.ts @@ -443,6 +443,10 @@ export class TranscriptContainer // drift after commit; the engine commits them audit-exempt. Provisional // (commit-unstable) blocks never extend it. #nativeScrollbackSnapshotSafeEnd: number | undefined; + // Local line index through which lower finalized siblings are safe to OFFER to + // native scrollback while still audited. Unlike snapshotSafeEnd, rows below a + // live block are not durable: growth above them must repair stale history. + #nativeScrollbackOfferSafeEnd: number | undefined; // Persistent assembled transcript rows. Rows before the stable floor are // byte-identical to the previous render; rows at/after it were re-pushed. #lines: string[] = []; @@ -491,6 +495,10 @@ export class TranscriptContainer return this.#nativeScrollbackSnapshotSafeEnd; } + getNativeScrollbackOfferSafeEnd(): number | undefined { + return this.#nativeScrollbackOfferSafeEnd; + } + /** * Whether `component` sits below a still-mutating block — i.e. inside the * live region, where its rows cannot have been committed to native @@ -583,6 +591,7 @@ export class TranscriptContainer this.#nativeScrollbackLiveRegionStart = undefined; this.#nativeScrollbackCommitSafeEnd = undefined; this.#nativeScrollbackSnapshotSafeEnd = undefined; + this.#nativeScrollbackOfferSafeEnd = undefined; const count = this.children.length; @@ -629,6 +638,10 @@ export class TranscriptContainer // liveStartIndex; empty leading blocks (or a separator) must not claim it // early. let liveRecorded = false; + // Prefix boundary for finalized siblings rendered below the first live + // block. These rows may be offered to native scrollback, but they cannot + // extend snapshotSafeEnd because a live block above can still move them. + let offerSafeEnd: number | undefined; // Frame row cursor: rows emitted (reused or pushed) so far. let row = 0; let stableRows = 0; @@ -770,6 +783,9 @@ export class TranscriptContainer // rows around as it grows, so the run closes there. if (!(finalized && safeLength >= contribution.length)) commitSafeOpen = false; } + if (i > liveStartIndex && finalized) { + offerSafeEnd = blockStart + contribution.length; + } segments[i] = { component: child, @@ -788,6 +804,7 @@ export class TranscriptContainer // Trailing shrink: blocks removed from the tail leave stale rows behind // when every surviving segment was reused. if (lines.length !== row) lines.length = row; + this.#nativeScrollbackOfferSafeEnd = offerSafeEnd; this.#segments = segments; this.#stableRowsFloor = Math.min(stableFloorBefore, stableRows, row); return lines; diff --git a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts index 25532f3f2..155d6c557 100644 --- a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts +++ b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts @@ -2,6 +2,16 @@ import { describe, expect, it } from "bun:test"; import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; import type { Component } from "@oh-my-pi/pi-tui"; +class FinalizedBlock implements Component { + #lines: string[]; + constructor(lines: string[]) { + this.#lines = [...lines]; + } + render(width: number): string[] { + return this.#lines.map(line => line.slice(0, width)); + } +} + class MutableLiveBlock implements Component { #lines: string[]; constructor(lines: string[]) { @@ -34,4 +44,19 @@ describe("transcript streaming commit (assistant text)", () => { // off head must be committable to native scrollback (tmux pane history). expect(chat.getNativeScrollbackCommitSafeEnd()).toBe(3); }); + + it("offers lower finalized siblings without making them durable", () => { + const chat = new TranscriptContainer(); + const top = new FinalizedBlock(["top-0"]); + const live = new MutableLiveBlock(["live-0"]); + const tail = new FinalizedBlock(["tail-0", "tail-1"]); + chat.addChild(top); + chat.addChild(live); + chat.addChild(tail); + + expect(chat.render(80)).toEqual(["top-0", "", "live-0", "", "tail-0", "tail-1"]); + expect(chat.getNativeScrollbackLiveRegionStart()).toBe(2); + expect(chat.getNativeScrollbackSnapshotSafeEnd()).toBe(3); + expect(chat.getNativeScrollbackOfferSafeEnd()).toBe(6); + }); }); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index aab3a8bfd..5a524c234 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed native scrollback repair for audited transcript rows that were offered below a still-live block, preventing lower finalized rows from duplicating when the live block grows ([#4326](https://github.com/can1357/oh-my-pi/issues/4326)). + ## [16.3.0] - 2026-07-02 ### Fixed diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index d59015ab2..1a1af5ab5 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -212,6 +212,11 @@ export interface OverlayFocusOwner { * dropped row or an audit re-anchor spray. Provisional live blocks (collapsing * tool/edit previews whose head is a throwaway tail window) omit it. Defaults to * `commitSafeEnd ?? liveRegionStart` when absent. + * `getNativeScrollbackOfferSafeEnd` optionally reports the deepest prefix row + * that may physically enter native scrollback while still remaining audited. + * This is for finalized lower siblings under a live block: the rows may scroll + * off, but a later live-block insertion above them must trigger repair instead + * of becoming durable audit-exempt history. * * When several root children report a seam in the same frame, the topmost * one (and its commit-safe / snapshot-safe extension) defines the boundary: @@ -222,6 +227,7 @@ export interface NativeScrollbackLiveRegion { getNativeScrollbackLiveRegionStart(): number | undefined; getNativeScrollbackCommitSafeEnd?(): number | undefined; getNativeScrollbackSnapshotSafeEnd?(): number | undefined; + getNativeScrollbackOfferSafeEnd?(): number | undefined; } export interface NativeScrollbackCommittedRows { @@ -251,6 +257,10 @@ function getNativeScrollbackSnapshotSafeEnd(component: Component): number | unde return (component as Component & Partial).getNativeScrollbackSnapshotSafeEnd?.(); } +function getNativeScrollbackOfferSafeEnd(component: Component): number | undefined { + return (component as Component & Partial).getNativeScrollbackOfferSafeEnd?.(); +} + /** * Opt-in stability report for components that mutate their returned render * array in place across frames (instead of returning a fresh array per @@ -628,6 +638,7 @@ interface FrameSegment { liveLocalStart?: number; commitLocalEnd?: number; snapshotLocalEnd?: number; + offerLocalEnd?: number; } /** Depth-first identity search through `Container`-shaped children. */ @@ -1009,23 +1020,27 @@ export class TUI extends Container { // #auditCommittedPrefix). Holds references to component-cached strings, so // the audit is a pointer walk in the common case. #committedPrefix: string[] = []; - // The committed prefix [0, committedRows) splits into three audit zones by - // two monotone marks auditRows ≤ durableRows ≤ committedRows: + // The committed prefix [0, committedRows) splits into four zones by three + // monotone marks auditRows ≤ durableRows ≤ offerRows ≤ committedRows: // [0, auditRows) BYTE-STABLE — audited (re-anchor on any shift). // [auditRows, durableRows) DURABLE snapshot — exempt: rows may drift in // place (a streaming table widening) without re-anchoring, so their // expected drift never sprays duplicate snapshots. - // [durableRows, committedRows) FORCED-overflow — audited: rows committed + // [durableRows, offerRows) OFFERED — audited: rows were allowed into + // native scrollback while a live block above could still shift them. + // A mismatch here requires a destructive replay, not a duplicate tail. + // [offerRows, committedRows) FORCED-overflow — audited: rows committed // only because they scrolled above the window under a commit-unstable // barrier; auditing them re-anchors (duplication, never loss) when the // barrier later shifts/finalizes/removes, instead of stranding a stale // prefix that silently drops the rows beneath it. - // Both marks re-base on a wholesale re-slice (full paint / shrink / geometry) - // and otherwise advance per the persistence rules in #updateCommittedAuditRows. + // Marks re-base on a wholesale re-slice (full paint / shrink / geometry) and + // otherwise advance per the persistence rules in #updateCommittedAuditRows. // #auditCommittedPrefix audits [0, committedRows) skipping the exempt window // [auditRows, durableRows). #committedPrefixAuditRows = 0; #committedPrefixDurableRows = 0; + #committedPrefixOfferRows = 0; // Frame row currently mapped to screen row 0. Monotonic between full // paints: a shrink never re-exposes scrolled-off rows (they cannot be // un-scrolled without rewriting history); live rows repaint at fixed @@ -1036,6 +1051,7 @@ export class TUI extends Container { #nativeScrollbackLiveRegionStart: number | undefined; #nativeScrollbackCommitSafeEnd: number | undefined; #nativeScrollbackSnapshotSafeEnd: number | undefined; + #nativeScrollbackOfferSafeEnd: number | undefined; #fullRedrawCount = 0; // Caps how many inline images render as live graphics; older ones fall back // to text via a purge + full redraw. Cap is configured by the host app. @@ -1155,6 +1171,7 @@ export class TUI extends Container { this.#nativeScrollbackLiveRegionStart = undefined; this.#nativeScrollbackCommitSafeEnd = undefined; this.#nativeScrollbackSnapshotSafeEnd = undefined; + this.#nativeScrollbackOfferSafeEnd = undefined; const children = this.children; const previousSegments = this.#frameSegments; const segments: FrameSegment[] = new Array(children.length); @@ -1177,12 +1194,14 @@ export class TUI extends Container { let liveLocalStart: number | undefined; let commitLocalEnd: number | undefined; let snapshotLocalEnd: number | undefined; + let offerLocalEnd: number | undefined; let reported: number | undefined; if (reuse) { childLines = previous.lines; liveLocalStart = previous.liveLocalStart; commitLocalEnd = previous.commitLocalEnd; snapshotLocalEnd = previous.snapshotLocalEnd; + offerLocalEnd = previous.offerLocalEnd; } else { // Feed the engine's committed-row claim (from the previous frame's // emit) before rendering so the child can skip re-deriving blocks @@ -1211,6 +1230,13 @@ export class TUI extends Container { ? Math.max(snapshotFloor, Math.min(childLines.length, Math.trunc(snapshotSafeEnd))) : childLines.length; } + const offerSafeEnd = getNativeScrollbackOfferSafeEnd(child); + if (offerSafeEnd !== undefined) { + const offerFloor = snapshotLocalEnd ?? commitLocalEnd ?? liveLocalStart; + offerLocalEnd = Number.isFinite(offerSafeEnd) + ? Math.max(offerFloor, Math.min(childLines.length, Math.trunc(offerSafeEnd))) + : childLines.length; + } } // Consume the stability report unconditionally for implementers: // reading re-bases the component's baseline to the state this @@ -1234,6 +1260,9 @@ export class TUI extends Container { if (snapshotLocalEnd !== undefined) { this.#nativeScrollbackSnapshotSafeEnd = offset + snapshotLocalEnd; } + if (offerLocalEnd !== undefined) { + this.#nativeScrollbackOfferSafeEnd = offset + offerLocalEnd; + } } if (chainStable) { if (previous !== undefined && previous.component === child && previous.start === offset) { @@ -1264,6 +1293,7 @@ export class TUI extends Container { liveLocalStart, commitLocalEnd, snapshotLocalEnd, + offerLocalEnd, }; offset += childLines.length; } @@ -2664,6 +2694,7 @@ export class TUI extends Container { const liveRegionStart = this.#nativeScrollbackLiveRegionStart; const commitSafeEnd = this.#nativeScrollbackCommitSafeEnd; const snapshotSafeEnd = this.#nativeScrollbackSnapshotSafeEnd; + const offerSafeEnd = this.#nativeScrollbackOfferSafeEnd; // Commit boundaries (also used by the window/commit math in section 3), // hoisted above the audit gate because the resync needs byteStableBoundary @@ -2671,22 +2702,27 @@ export class TUI extends Container { // The commit floor is windowTop in every non-frozen path (see chunkTo), so // whatever scrolls above the window is committed — never committed nowhere // AND painted nowhere (the loss bug). The boundaries no longer gate the - // commit; they define the audit-exempt span. byteStableBoundary: rows below + // commit; they define audit and repair spans. byteStableBoundary: rows below // it are byte-stable (never re-layout), audited. durableBoundary: rows in // [byteStableBoundary, durableBoundary) are durable — permanent on scroll-off // but may drift in place (a streaming table re-aligning), committed - // audit-EXEMPT. Rows at/beyond durableBoundary committed only because they - // scrolled above the window (a commit-unstable barrier over a long tail) are - // forced-overflow rows: audited, so a later shift/finalize/removal re-anchors - // (duplication, never loss) instead of stranding a stale prefix. Built on the - // finalized prefix (live-region start); the whole frame when the root reports - // no seam (shell semantics: whatever scrolls is final). + // audit-EXEMPT. offerBoundary: rows in [durableBoundary, offerBoundary) + // were explicitly allowed to enter native scrollback while remaining + // audited; if they later shift, the stale physical history is repaired by a + // destructive replay instead of duplicate recommit. Rows at/beyond + // offerBoundary committed only because they scrolled above the window (a + // commit-unstable barrier over a long tail) are forced-overflow rows: + // audited, so a later shift/finalize/removal re-anchors (duplication, never + // loss) instead of stranding a stale prefix. Built on the finalized prefix + // (live-region start); the whole frame when the root reports no seam (shell + // semantics: whatever scrolls is final). const frameLength = rawFrame.length; const byteStableBoundary = Math.max(0, Math.min(frameLength, commitSafeEnd ?? liveRegionStart ?? frameLength)); const durableBoundary = Math.max( byteStableBoundary, Math.min(frameLength, snapshotSafeEnd ?? byteStableBoundary), ); + const offerBoundary = Math.max(durableBoundary, Math.min(frameLength, offerSafeEnd ?? durableBoundary)); // 2. Transition state captured before any emitter runs. const prevWindowTop = this.#windowTopRow; @@ -2731,6 +2767,7 @@ export class TUI extends Container { this.#committedPrefixDurableRows < this.#committedRows ? this.#committedRows : this.#committedPrefixAuditRows; const hardAuditEnd = Math.min(this.#committedRows, durableBoundary); const needHardAudit = this.#committedPrefixDurableRows < hardAuditEnd; + let repairOfferedScrollback = false; const auditRan = this.#hasEverRendered && !geometryChanged && @@ -2738,14 +2775,21 @@ export class TUI extends Container { (this.#renderStablePrefixRows < auditUpper || needHardAudit); if (auditRan) { const committedRowsBeforeAudit = this.#committedRows; + const offeredRowsBeforeAudit = this.#committedPrefixOfferRows; + const durableRowsBeforeAudit = this.#committedPrefixDurableRows; this.#auditCommittedPrefix(rawFrame, durableBoundary); committedRowsResynced = this.#committedRows !== committedRowsBeforeAudit; + repairOfferedScrollback = + offeredRowsBeforeAudit > durableRowsBeforeAudit && + committedRowsResynced && + this.#committedRows < offeredRowsBeforeAudit; } // Committed-prefix state this frame's commit math extends from (post-audit). // Drives the audit-rows / durable-rows caps recomputed after the emit. const preCommitRows = this.#committedRows; const preCommitAuditRows = this.#committedPrefixAuditRows; const preCommitDurableRows = this.#committedPrefixDurableRows; + const preCommitOfferRows = this.#committedPrefixOfferRows; // 3. Window and commit math (lengths only; content prepared below). let hasVisibleOverlay = false; @@ -2763,7 +2807,7 @@ export class TUI extends Container { // place, because an ED3 rewrap is unsafe (pane scrollback / alt-screen // feedback loop), so committed history keeps its old wrap. const firstPaint = !this.#hasEverRendered; - const replaceRequested = this.#clearScrollbackOnNextRender; + const replaceRequested = this.#clearScrollbackOnNextRender || repairOfferedScrollback; const geometryRebuild = geometryChanged && !resizeRepaintsInPlace(); const fullPaint = firstPaint || replaceRequested || geometryRebuild; let windowTop: number; @@ -2875,8 +2919,10 @@ export class TUI extends Container { preCommitRows, preCommitAuditRows, preCommitDurableRows, + preCommitOfferRows, byteStableBoundary, durableBoundary, + offerBoundary, false, ); this.#clearScrollbackOnNextRender = false; @@ -2904,8 +2950,10 @@ export class TUI extends Container { preCommitRows, preCommitAuditRows, preCommitDurableRows, + preCommitOfferRows, byteStableBoundary, durableBoundary, + offerBoundary, auditRan, ); } @@ -2932,6 +2980,7 @@ export class TUI extends Container { this.#committedRows = resyncTo; this.#committedPrefixAuditRows = Math.min(this.#committedPrefixAuditRows, resyncTo); this.#committedPrefixDurableRows = Math.min(this.#committedPrefixDurableRows, resyncTo); + this.#committedPrefixOfferRows = Math.min(this.#committedPrefixOfferRows, resyncTo); prefix.length = resyncTo; if ($flag("PI_DEBUG_REDRAW")) { const msg = `[${new Date().toISOString()}] commit resync: committed prefix diverged at row ${resyncTo}; recommitting\n`; @@ -2944,21 +2993,24 @@ export class TUI extends Container { * #committedPrefixAuditRows field doc for the three audit zones). * * auditRows tracks the byte-stable boundary; durableRows the durable snapshot - * boundary. A wholesale re-slice (full paint / shrink / geometry) re-bases - * each mark from the current frame (min(committed, boundary)). An incremental - * extend keeps a mark once a row past it has committed (mark < committed): a - * later RISE in a boundary (a table finalizing) must neither pull - * already-committed stale snapshots back under the byte-stable cap nor - * retroactively exempt forced-overflow rows already audited. durableRows is - * floored at auditRows so the exempt window can never invert. + * boundary; offerRows the deepest explicit audited-offer boundary. A wholesale + * re-slice (full paint / shrink / geometry) re-bases each mark from the + * current frame. An incremental extend keeps a mark once a row past it has + * committed (mark < committed): a later RISE in a boundary (a table finalizing) + * must neither pull already-committed stale snapshots back under the + * byte-stable cap nor retroactively exempt forced-overflow rows already + * audited. durableRows is floored at auditRows; offerRows is floored at + * durableRows. */ #updateCommittedAuditRows( resliced: boolean, preCommittedRows: number, preAuditRows: number, preDurableRows: number, + preOfferRows: number, byteStableBoundary: number, durableBoundary: number, + offerBoundary: number, hardAudited: boolean, ): void { const committed = this.#committedRows; @@ -2975,8 +3027,13 @@ export class TUI extends Container { resliced || preDurableRows >= preCommittedRows || hardAudited ? Math.min(committed, durableBoundary) : Math.min(preDurableRows, committed); + const offerRows = + resliced || preOfferRows >= preCommittedRows + ? Math.min(committed, offerBoundary) + : Math.min(preOfferRows, committed); this.#committedPrefixAuditRows = auditRows; this.#committedPrefixDurableRows = Math.max(auditRows, durableRows); + this.#committedPrefixOfferRows = Math.max(this.#committedPrefixDurableRows, offerRows); } /** diff --git a/packages/tui/test/streaming-scrollback-defer.test.ts b/packages/tui/test/streaming-scrollback-defer.test.ts index 1fdd86018..3b020d871 100644 --- a/packages/tui/test/streaming-scrollback-defer.test.ts +++ b/packages/tui/test/streaming-scrollback-defer.test.ts @@ -645,6 +645,7 @@ class SeamComponent implements Component, NativeScrollbackLiveRegion { liveStart: number | undefined; commitSafe: number | undefined; snapSafe: number | undefined; + offerSafe: number | undefined; invalidate(): void {} @@ -663,6 +664,9 @@ class SeamComponent implements Component, NativeScrollbackLiveRegion { getNativeScrollbackSnapshotSafeEnd(): number | undefined { return this.snapSafe; } + getNativeScrollbackOfferSafeEnd(): number | undefined { + return this.offerSafe; + } } /** Indices in `buffer` where `needle` begins as a contiguous run. */ @@ -954,6 +958,42 @@ describe("scrollback commit gap — commit-unstable barriers", () => { } }); + it("repairs offered finalized tail rows when a live block above grows", async () => { + if (process.platform === "win32") return; + const term = new VirtualTerminal(20, 5); + overrideProbe(term, undefined); + const tui = new TUI(term); + const root = new SeamComponent(); + + try { + tui.addChild(root); + tui.start(); + await settle(term); + const writes = capture(term); + + const f1 = ["live-0", ...rows("tail-", 10)]; + root.lines = f1; + root.liveStart = 0; + root.snapSafe = 1; + root.offerSafe = f1.length; + tui.requestRender(); + await settle(term); + + const f2 = ["live-0", "live-1", ...rows("tail-", 10)]; + root.lines = f2; + root.offerSafe = f2.length; + tui.requestRender(); + await settle(term); + + const buffer = term.getScrollBuffer().map(line => line.trimEnd()); + expect(buffer).toEqual(f2); + for (const row of rows("tail-", 10)) expect(buffer.filter(line => line === row)).toHaveLength(1); + expect(eraseScrollbackCount(writes)).toBeGreaterThan(0); + } finally { + tui.stop(); + } + }); + it("does not lose a single-row finalize edit above an unchanged tail (reviewer repro)", async () => { if (process.platform === "win32") return; const term = new VirtualTerminal(20, 4);