fix(tui): reduce large transcript stalls#3259
Conversation
Tail appended transcript JSONL instead of rebuilding rendered history on every poll, collapse compacted history for live chat rendering, and replace synchronous session rewrites so tailers detect historical changes. Fixes #3258
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bcbf1515d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| size: stat.size, | ||
| mtimeMs: stat.mtimeMs, | ||
| offset: stat.size, | ||
| pending: "", |
There was a problem hiding this comment.
When the session file grows between the earlier statSync and the full readFileSync, the rebuild parses the newly appended bytes from data, but the tail state records size/offset from the older stat.size. On the next poll stat.size > state.size, so #appendLocal reads from the stale offset and appends entries that were already rendered, duplicating transcript rows for agents that append while the viewer is opening or doing a full rebuild. Re-stat after the read or base the recorded offset on data.byteLength.
Useful? React with 👍 / 👎.
| if (result.newSize < fromByte) { | ||
| // Host transcript rotated/truncated — restart from 0. | ||
| this.#remoteBytes = 0; | ||
| this.#remoteEntries = []; | ||
| this.#fetchRemote(); |
There was a problem hiding this comment.
In the collab-guest path, a host-side transcript truncation/rotation only resets the byte cursor before fetching from byte 0 again. Because this change now appends parsed remote entries directly into the existing ChatTranscriptBuilder, the refetched transcript is rendered after the stale pre-rotation rows instead of replacing them; the old #remoteEntries = [] plus rebuild behavior no longer exists. Clear the builder/model state before the restart so rotated host transcripts do not show duplicated/stale history.
Useful? React with 👍 / 👎.
If the session file grew between the earlier statSync and readFileSync inside #loadLocalFull, the rebuild rendered the appended bytes but the tail state recorded the pre-race stat.size. The next poll's #appendLocal would then read from that stale offset and re-render the bytes already in the rebuild, duplicating rows for agents that append during a viewer open or full rebuild. Anchor size/offset to data.byteLength and re-stat for mtime/identity so the post-read clock matches what's on disk. Fixes #3258
|
Good catch — fixed in
|
The append-based collab-guest path only reset the byte cursor on host transcript rotation/truncation, so the refetched bytes stacked on top of the stale pre-rotation rows still in the ChatTranscriptBuilder. Drop the builder and model state before refetching from byte 0 so rotated host transcripts replace stale history instead of duplicating it. Fixes #3258
|
Good catch — fixed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e65b8e4cc7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| size: data.byteLength, | ||
| mtimeMs: post.mtimeMs, | ||
| offset: data.byteLength, | ||
| pending: "", |
There was a problem hiding this comment.
When a full rebuild reads the session while another process is midway through appending a large JSONL entry, parseSessionEntries(data.toString(...)) ignores the incomplete trailing line, but the new tail state advances offset to data.byteLength and clears pending. The next poll then reads only the remainder of that same line, so the completed entry is malformed and the message can disappear from the transcript until a later rewrite/rotation forces another full read. This affects local agent/advisor viewers opened during an in-progress append; keep the bytes after the last newline as pending and only advance the cursor past complete lines.
Useful? React with 👍 / 👎.
When a full rebuild caught the session file mid-append, #loadLocalFull cleared the pending buffer but advanced offset past the headless trailing bytes. The next poll then read only the completion bytes, parsed a headless fragment, and silently dropped the completed entry. Carry the bytes after the last newline as pending so the completion concatenates with them on the next #appendLocal pass. Fixes #3258
|
Good catch — fixed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bc0e56c20
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
|
|
||
| if (options?.transcript) { | ||
| if (options?.transcript && !options.collapseCompactedHistory) { |
There was a problem hiding this comment.
With collapseCompactedHistory: true, transcript mode now falls through into the LLM compaction branch. For OpenAI remote compactions (preserveData.openaiRemoteCompaction.replacementHistory), that branch skips the firstKeptEntryId-to-compaction messages behind if (!remoteReplacementHistory), so initial render/rebuild of a remotely compacted session shows only the summary and messages after compaction, hiding the recent kept turns that were visible before this change. The collapsed display path should still emit those kept SessionEntry messages even when the provider payload is used for LLM replay.
Useful? React with 👍 / 👎.
| if (state.path !== sessionFile || state.dev !== stat.dev || state.ino !== stat.ino || stat.size < state.size) | ||
| return false; | ||
| for (const sentinel of state.sentinels) { | ||
| const current = readFileRangeSync(sessionFile, sentinel.offset, sentinel.bytes.byteLength); |
There was a problem hiding this comment.
On a local viewer, after statSync succeeds, #canAppendLocal opens the file again for each sentinel without any catch. If the session file is deleted or rotated in that small window, fs.openSync throws out of #refresh/the poll timer instead of clearing or rebuilding the transcript, which can take down the TUI while an agent/session is being removed. Treat sentinel read failures as non-appendable or missing rather than letting them escape.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| it("replaces the file identity so transcript tailers detect rewrites", async () => { | ||
| const { FileSessionStorage } = await import("@oh-my-pi/pi-coding-agent/session/session-storage"); |
There was a problem hiding this comment.
This new test repeats await import(...) for a statically known module. The root AGENTS.md Code Quality rule says “NEVER use inline imports” and to use top-level imports instead; there is no runtime-selected specifier here, so this should be a normal FileSessionStorage import to keep the changed test within the package rules.
Useful? React with 👍 / 👎.
1 participant
Repro
Focused regression tests reproduce the large-session hot paths: local transcript append renders
TAILMARKERonly after a whole-filefs.readFileSync, live display transcript rebuilds ignorecollapseCompactedHistory, and a remote header-only first fetch must leave the viewer onNo messages yet.. Command:bun test packages/coding-agent/test/agent-hub-advisor-scroll.test.ts packages/coding-agent/test/compaction.test.ts packages/coding-agent/test/modes/utils/render-initial-messages.test.tsfailed before the fix with those assertions.Cause
AgentTranscriptViewer.#refreshkeyed local files only by size/mtime and calledChatTranscriptBuilder.rebuild()after reading and parsing the whole session file on every change.UiHelpers.renderInitialMessages()andInteractiveMode.rebuildChatFromMessages()always requested the full inline transcript fromAgentSession.buildTranscriptSessionContext(), so compacted history stayed on the live render surface.FileSessionStorage.writeTextSync()rewrote in place, making same-path historical rewrites hard for a tailing viewer to distinguish from appends.Fix
ChatTranscriptBuilder.append()and switched local/remote transcript refreshes to append parsed JSONL tails when file identity and sentinels are stable.collapseCompactedHistoryfor transcript contexts and wired live chat rebuilds to use the compacted display tail while full transcript contexts remain available by default.Verification
Passed:
bun test packages/coding-agent/test/agent-hub-advisor-scroll.test.ts packages/coding-agent/test/compaction.test.ts packages/tui/test/resize-viewport-defer.test.ts packages/coding-agent/test/modes/utils/render-initial-messages.test.ts packages/coding-agent/test/session-storage.test.ts && bun check. Fixes #3258