Skip to content

perf(native-chat): stop long replies from freezing while they stream - #35462

Open
brennanb2025 wants to merge 33 commits into
mainfrom
brennanb2025/nc-perf-text-cadence-2
Open

brennanb2025 wants to merge 33 commits into
mainfrom
brennanb2025/nc-perf-text-cadence-2

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 26 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3605 $\color{#cf222e}{\Huge{\mathbf{−}}}$​277 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3328
Prod 26 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​887 $\color{#cf222e}{\Huge{\mathbf{−}}}$​306 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​581

ELI5

While an agent writes a long reply in Native Chat, the reply could sit still for several seconds and then jump ahead. Saves waited for the reply to grow by a fixed share of its length, so the longer the reply got, the longer each wait. Now the reply text is saved at most every 400 ms, so it keeps moving.

What Changed

Before: assistant and reasoning text saved a new copy only after the text grew by about 12.5%. A 20,000-character reply waited for another 2,500 characters between saves, which looked like a freeze. Replays of four recorded replies had maximum gaps of 6.1–15.0 s.

After: assistant and reasoning text use a 400 ms elapsed-time gate. The first save still lands in the existing 60 ms window. The same replays drop to maximum gaps of 0.5–1.3 s with identical final text. Windows low-spec runs measured 0.56–0.62 s for Claude against 7.7–8.7 s on main.

  • Claude, Codex and the shared provider timeline (claude-streamed-text-checkpoints.ts, codex-structured-item-stream*.ts, provider-timeline-text-streams.ts) keep their existing full-text owners and switch only the gate. Command and file output keep their growth gate.
  • Codex plans also schedule a delivery for the last accepted save plus 400 ms, so a provider pause doesn't hide the tail until the next frame. A scheduled save checks it still belongs to the current session (codex-acquisition-scheduled-owner.ts) and routes a closed or failed save to that session's existing failure path.
  • Usage reports no longer force a save of continuing text.
  • Completion and explicit flushes save whatever tail is buffered.
  • Same-role, text-only reasoning revisions use NORMAL durability. Creation, receipts and lifecycle batches keep FULL.
  • Assistant and reasoning text are event-driven, so during a provider pause their buffered tail can stay hidden until the next delta or lifecycle barrier. Plans are the exception (scheduled above). Event-loop stalls mean 400 ms is a target, not a guarantee.

Changed in the takeover:

Why

A time gate bounds how long text can sit unsaved, whatever the reply length. The growth gate's waits grow with the reply. This PR keeps the existing owners and save path instead of adding a second text store or a saved timer. Plans get a scheduled delivery because they often pause mid-document, and a paused plan looked unfinished.

The cost is more saved revisions while a reply streams. The PR was held until #36053 could reclaim them; the storage numbers are under Testing.

Linked Issue

No dedicated issue. Storage context: #34916 (journal growth), which #36053 addresses and this PR must not undo. This replaces the approach in #32315.

Visual Proof

Adopted Plan whole-window recordings from the earlier QA: reviewed head · pinned main. The difference is timing (largest DOM update gap 567 ms on the head vs 5,714 ms on main during the Plan stream), not layout.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Storage with #36053, the gate this PR was held for. I streamed the same long turn through the real translator, journal sink and SQLite journal on main (b6467f0) and on this head, then ran #36053's compaction to completion followed by incremental vacuum. Codex streamed reasoning (4k chars), a plan (8k) and a reply (24k). Claude streamed thinking (4k) and a reply (24k). Deltas were 2–8 characters every 10–40 ms, with a 1–3 s pause about every 300 deltas: 227 s and 174 s of simulated streaming.

Main before cleanup Head before cleanup Main after cleanup Head after cleanup
Codex DB file 659,456 B 4,612,096 B 212,992 B 212,992 B
Codex saved records (value bytes) 241 (402,680) 885 (4,014,516) 23 (37,620) 23 (37,624)
Claude DB file 561,152 B 4,116,480 B 208,896 B 208,896 B
Claude saved records (value bytes) 217 (312,022) 947 (3,568,881) 24 (30,193) 24 (30,194)
  • After cleanup the file is the same size on both, with the same records. The 1–4 extra value bytes are longer revision and sequence numbers (283 vs 46, for example), not content.
  • Reopening from disk after cleanup restored every item at full length on both.
  • Between cleanup passes the head's file is about 7× main's for these turns. The sweeper runs a minute after launch and then every six hours, so a heavy session can hold that extra until the next pass.

After the merge.

  • The PR's 26 vitest files: 277 tests pass.
  • The wider directories that load the changed code (src/main/{codex,claude,native-chat,jsonl-rpc,pi,acp,runtime/orchestration}): 1,169 files and 10,858 tests pass.
  • Changed-file oxlint and oxfmt are clean.

Carried from before the takeover. None of this was rerun on the merged head.

  • Earlier QA: the Windows low-spec timing above, and Stop/restart preserving text, first position and time.
  • QA 3583 on Codex CLI 0.165.0: a real CLI Plan adopted into native chat, paused, completed, closed mid-stream and reopened with the same text, identity and order.
  • That QA reproduced three main behaviours unchanged by this PR: a completed Plan folds behind "Worked for", the CLI seed's completed Plan is missing from imported history, and the Stop warning tracked in STA-12558.

Risk

The riskiest part is saved chat history. A wrong gate or ordering could save a reply out of order or drop its tail.

Back out: a plain revert. Records already saved stay readable, and cleanup reclaims the extra revisions.

Level: Medium

AI Disclosure

Review

Takeover delta (merge resolution, removed tests, extracted owner check), reviewed against main b6467f0 with #36053 and #37216:

  • Codex: no findings. It also ran the focused suites, 274 tests across 25 files.
  • A second Claude reviewer was stopped before it reported, so this is one reviewer, not the usual two.

Before the takeover, the cadence, ordering and plan-scheduling design went through several independent review rounds. The earlier body records them; they aren't repeated here.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

  • Cross-platform: timers and SQLite only, no platform branches. Timing was measured on Windows and macOS.
  • Remote and mobile: the host saves and folds as before, so remote hosts and phones need no change.
  • Folder workspaces: not affected; this is chat persistence only.
  • Out of scope, found during the takeover: when 257 or more Codex items stream at once, the oldest stream's state is evicted. If that item streams again, its saved text restarts from the new deltas. Main does the same.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • I stated the risk: what could break, for whom, and how to back it out
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[High impact] The PR changes saved text cadence and durability across native-chat provider streams.

Fix the extra argument in the history test before merging.

Findings

  1. P1 Extra argument breaks type checking ▶

Summary

Streamed assistant and reasoning text now use a 400 ms elapsed-time gate instead of percentage growth.

  • Assistant and reasoning streams save full text snapshots on an elapsed-time cadence.
  • Codex saves pending text before stream limits can evict it.
  • Text-only assistant and reasoning revisions can use NORMAL journal durability.
  • Claude activity updates now come from its context facts component.

One new helper call blocks type checking. brennanb2025 acknowledged full-copy storage growth and deferred its fix to #36024 and #36053, with no interim shipping approved.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Provider text arrives] --> B[Keep full text]
  B --> C[60 ms window]
  C --> D{400 ms since accepted save?}
  D -->|Yes| E[Try save and publish]
  D -->|No| F[Keep tail until more text or a barrier]
  G[Stop, completion, or explicit flush] --> E
  E --> H[Append journal snapshot]
Loading

Reviews (10) · Last reviewed commit: "Merge main to include the CI test fix" · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 589054f8-f14d-4226-8caa-3f5624bb21ee
Commits

Reviewing files that changed from the base of the PR and between 79d6b68 and 82f1859.

Files selected for processing: 16
  • src/main/claude/claude-checkpoint-test-support.ts
  • src/main/claude/claude-context-facts.ts
  • src/main/claude/claude-streamed-text-checkpoints.test.ts
  • src/main/claude/claude-streamed-text-checkpoints.ts
  • src/main/claude/claude-streamed-thinking.ts
  • src/main/claude/claude-structured-journal-translation.ts
  • src/main/claude/claude-structured-reasoning.test.ts
  • src/main/codex/codex-structured-item-stream-bounds.ts
  • src/main/codex/codex-structured-item-streams.ts
  • src/main/codex/codex-structured-journal-translation-streams.test.ts
  • src/main/codex/codex-text-cadence-pressure.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-refusal.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-streams.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-text-streams.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts
  • src/main/native-chat/agent-session-wire/agent-session-text-cadence.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The changes add time-based admission for streamed text checkpoints across Claude, Codex, and provider timeline streams. Persistence callbacks report whether appends and publications were accepted. Codex stream state trimming now preserves buffered content when persistence is refused, and pending patches are capped. Journal writes track eligible assistant and reasoning text rows and can blank prior text while retaining the latest revision. Tests cover timed writes, refusal and retry behavior, journal reopening, and downstream projections.

Merge Risk: Minimal · up to 82f18

Streamed text now saves on a fixed cadence. The final text is still preserved on Stop or completion, and the earlier concerns about leftover reasoning routes and skipped finalizers no longer apply. Normal CI and live-agent checks should still finish before merge.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description is detailed and covers the change, rationale, testing, risk, visual proof, compatibility, and rollback plan. However, the required Linked Issue section does not provide a dedicated iss… Add the actual issue link in the Linked Issue section using the required Fixes #<issue-number> format, or document the applicable maintainer exception. Mark the validation checklist items complete or state clearly that CI will run them.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary change: preventing long native-chat replies from freezing during streaming.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 24 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description is detailed and covers the change, rationale, testing, risk, visual proof, compatibility, and rollback plan. However, the required Linked Issue section does not provide a dedicated issue in the required Fixes # form; it only references storage-related issues. The final checklist also leaves the full lint, typecheck, test, and build validation unchecked.

Resolution

Add the actual issue link in the Linked Issue section using the required Fixes #&lt;issue-number&gt; format, or document the applicable maintainer exception. Mark the validation checklist items complete or state clearly that CI will run them.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Resolve merge conflicts
  • Resolve merge conflict in branch brennanb2025/nc-perf-text-cadence-2
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5be73157-8d85-44d4-a632-6f2418c8f9dd
Commits

Reviewing files that changed from the base of the PR and between 552691f and d823e1d.

Files selected for processing: 19
  • src/main/claude/claude-streamed-text-checkpoints.test.ts
  • src/main/claude/claude-streamed-text-checkpoints.ts
  • src/main/claude/claude-streamed-thinking.ts
  • src/main/claude/claude-structured-journal-translation.ts
  • src/main/codex/codex-structured-item-streams.ts
  • src/main/codex/codex-structured-journal-sink.ts
  • src/main/codex/codex-structured-journal-translation-settlement.test.ts
  • src/main/codex/codex-structured-journal-translation-streams.test.ts
  • src/main/native-chat/agent-session-journal/journal-row-writer.spec.ts
  • src/main/native-chat/agent-session-journal/journal-row-writer.ts
  • src/main/native-chat/agent-session-journal/journal-streamed-text-revision.test.ts
  • src/main/native-chat/agent-session-journal/journal-streamed-text-revision.ts
  • src/main/native-chat/agent-session-journal/journal-text-predecessors.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-streams.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-text-streams.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts
  • src/main/native-chat/agent-session-wire/agent-session-text-prefix.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-item-publication.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Gate predecessor blanking on structural equality. · journal-text-predecessors.ts:65-96

src/main/native-chat/agent-session-journal/journal-text-predecessors.ts:65-96
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Gate predecessor blanking on structural equality.

JournalRowWriter stages every inserted row. The structural comparison in isJournalStreamedTextRevision() only selects the fsync mode.

appendItem() can create a higher-revision row with text-only blocks and changed metadata such as ownerFence, executionOwner, turnScope, linkage, or body fields. JournalTextPredecessors.stage() checks only role and revision before updating the predecessor with blank text. This can erase the previous revision’s text for a non-text-equivalent revision.

Add a structural-equality gate to stage(). Reuse the normalized comparison used by isJournalStreamedTextRevision().


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d817adce-feba-4653-90e2-859dcb4631d3
Commits

Reviewing files that changed from the base of the PR and between d823e1d and b6271c4.

Files selected for processing: 15
  • src/main/claude/claude-journal-turn-scope.test.ts
  • src/main/claude/claude-open-turn.ts
  • src/main/claude/claude-streamed-text-checkpoints.test.ts
  • src/main/claude/claude-streamed-text-checkpoints.ts
  • src/main/claude/claude-streamed-thinking.ts
  • src/main/claude/claude-structured-journal-translation.ts
  • src/main/codex/codex-structured-item-stream-bounds.ts
  • src/main/codex/codex-structured-item-streams.ts
  • src/main/codex/codex-structured-journal-translation-streams.test.ts
  • src/main/native-chat/agent-session-journal/journal-row-writer.spec.ts
  • src/main/native-chat/agent-session-journal/journal-text-predecessors.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-item-publication.test.ts
  • src/main/runtime/orchestration/structured-worker-journal-page.test.ts
Files skipped from review as they are similar to previous changes: 2
  • src/main/native-chat/agent-session-journal/journal-row-writer.spec.ts
  • src/main/native-chat/agent-session-journal/journal-text-predecessors.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5ea1b33f-ac2e-4e8f-99c5-ba1dc773d9ee
Commits

Reviewing files that changed from the base of the PR and between b6271c4 and 97b19c2.

Files selected for processing: 44
  • src/main/acp/acp-history-journal-overlay.ts
  • src/main/claude/claude-checkpoint-import.test.ts
  • src/main/claude/claude-checkpoint-input.ts
  • src/main/claude/claude-checkpoint-intent.ts
  • src/main/claude/claude-checkpoint-test-support.ts
  • src/main/claude/claude-context-facts.ts
  • src/main/claude/claude-context-usage-test-support.ts
  • src/main/claude/claude-final-output-coverage.ts
  • src/main/claude/claude-journal-translator-contract.ts
  • src/main/claude/claude-journal-turn-scope.test.ts
  • src/main/claude/claude-message-journaling.ts
  • src/main/claude/claude-provisional-row-corrections.ts
  • src/main/claude/claude-stream-json-connection-close.test.ts
  • src/main/claude/claude-stream-json-connection.ts
  • src/main/claude/claude-stream-json-output.ts
  • src/main/claude/claude-streamed-text-checkpoints.test.ts
  • src/main/claude/claude-streamed-text-checkpoints.ts
  • src/main/claude/claude-streamed-thinking.ts
  • src/main/claude/claude-structured-event-delivery.ts
  • src/main/claude/claude-structured-journal-translation-subagents.test.ts
  • src/main/claude/claude-structured-journal-translation.test.ts
  • src/main/claude/claude-structured-journal-translation.ts
  • src/main/claude/claude-structured-resume-point.ts
  • src/main/claude/claude-structured-session-close.test.ts
  • src/main/claude/claude-structured-session-close.ts
  • src/main/claude/claude-structured-session-exit-lifecycle.ts
  • src/main/claude/claude-structured-session-journal-control.ts
  • src/main/claude/claude-transcript-catch-up-test-support.ts
  • src/main/claude/claude-transcript-catch-up.ts
  • src/main/claude/claude-transcript-import-plan.ts
  • src/main/claude/claude-transcript-item-reader.ts
  • src/main/claude/structured/claude-structured-session-acquisition.ts
  • src/main/claude/structured/claude-structured-session-open.ts
  • src/main/native-chat/agent-session-journal/journal-step-writer.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-streams.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-estimate.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-history-catch-up.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-output-drain.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-sink-operation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-transition.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7cf60e25-fe00-42c1-9d64-336f81640c71
Commits

Reviewing files that changed from the base of the PR and between 97b19c2 and 79d6b68.

Files selected for processing: 10
  • src/main/claude/claude-checkpoint-input.ts
  • src/main/claude/claude-final-output-coverage.ts
  • src/main/claude/claude-streamed-text-checkpoints.ts
  • src/main/claude/claude-streamed-thinking.ts
  • src/main/codex/codex-structured-journal-streams-scaling.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-refusal.test.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler-streams.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts
  • src/main/native-chat/agent-session-wire/agent-session-delta-work-test-support.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Comments Outside Diff

These findings could not be posted inline.

  • P1 Returning blocks lose their prefixes src/main/claude/claude-streamed-text-checkpoints.ts:125 ▶

    After 256 unfinished Claude streams, the coalescer evicts the oldest snapshot. If another delta arrives for that block, append() starts a new snapshot from only that delta, even though latestText retains the prefix. The next save overwrites the journal item with only the suffix. Seed returning streams from their retained text, or prevent unfinished streams from being evicted.

  • P1 Refused Claude saves disappear src/main/claude/claude-streamed-text-checkpoints.ts:107 ▶

    When the journal queue refuses a save, persist still advances checkpointLengths and returns no result to the coalescer. The coalescer marks the text clean, and later flush() skips it. finish() also drops the block even when its terminal save is refused. Reasoning or an interrupted answer without a later full body can disappear from history. Retain refused work until the sink accepts it, and retry when the queue drains.

  • P1 Paused replies stay frozen src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts:120 ▶

    When text arrives within 400 ms of the previous save and the provider pauses, the scheduled flush skips or consumes the update without scheduling its deadline. Claude, Codex, and timeline text can then stay hidden until another delta or lifecycle event arrives. Keep the update pending and schedule a wake for the remaining delay.

Comment on lines +210 to +212
expect(storedBytes).toBeGreaterThan(
Buffer.byteLength(JSON.stringify(final)) + (4096 * 100 * 101) / 2
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Long replies bloat saved conversations

Removing predecessor retirement keeps every full-text checkpoint on disk while replies now save roughly every 400 ms. This test confirms that a roughly 400 KiB answer retains over 20 MiB of growing prefixes. Longer replies increase that multiplier, and replayJournal reads and parses every saved prefix when the conversation opens.

Keep failed writes safe without dropping the retained-text bound, or add a replacement cleanup path before enabling the faster cadence.

Knowledge Base Used: Native agent conversations

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@brennanb2025

brennanb2025 commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

NOT READY — repairs independently accepted and CI green at 17a6bda. #36053 and the integrated retirement, reader/cursor and byte proof remain required. No interim shipping.

Usage reports could force repeated saves of a growing reply. Codex could also lose a context-usage update when the journal queue was full. Continuing text now keeps its save cadence, and a refused Codex update stays queued for retry. Independent review accepts the runtime repair and its test-only follow-up at 17a6bda7d3d6fc664270afaa1557568bc0f3188b. Fresh CI passed on this exact head (PR Checks run 41233518337): all 12 test shards, static analysis/typecheck, relay integration, cross-version wire compatibility, persistence, packaging and all three WSL distros. All 33 configured checks passed; 18 configured skips remain unexercised. #36053 and the integrated storage proof still hold this PR. No interim shipping.

Usage reports leave continuing named assistant and reasoning text on its ordinary save cadence; explicitly suppressed frames on the shared timeline follow the same rule. An anonymous row that becomes visible still places its meaningful unpublished predecessors first; real tools, plans, visible fallback rows and lifecycle barriers retain their ordering rules. Primary Codex usage returns the selected revision/publication admission unchanged. A refused reading remains in the existing retry path; admission transfers ownership to the native queue and does not mean COMMIT. Missing publishing support and invalid readings retain their accepted no-op behavior; no fallback capability, eager target read, extra owner or timer was added. The execution-late revision preserves the current row’s lifecycle and other fields, and creates no context-only row when no target exists.

The new native proof exercises pending assistant text and clean reasoning against actual saturated ordinary and lifecycle queues, an actual 25 ms attempt while held, then legitimate low-water retry. It observes context 17,611/258,400 before another provider frame or disposal, with exactly one committed context input and one context publication. Textless final, dispatch echo and fresh reopen retain exact text and the original item envelope. Old production returns fail both cases by consuming usage without the context commit. Three selected-refusal cases reach the real adapter/retry through closed, failed and replaced owners and preserve the original reason, release read/timer resources and protect the successor. Their native SQL/close primitive coverage is carried separately; these are focused tests, not new app runs.

Earlier QA carries for unchanged long replies, Stop/restart/reopen, copied-reader and adopted-Plan flows at QA head e3b680a; its limits remain below. The earlier green CI at 4f694ad is historical. The Plan scheduling choice is unchanged. #36053, integrated FULL Plan timed revisions/refusals, nested/shared/repeated retirement, readers/cursors, body/whole-record bytes and landing proof remain required. The storage-maintenance and growth review remain open. No interim shipping.

Long native-chat replies could arrive in coarse bursts while an agent was still producing text. This change uses the common pattern of whole-text updates on an ordinary 400 ms active-stream cadence, keeping meaningful text, first position and ordering intact. Review repairs preserve pressure retry/drain and closed-owner termination, prevent empty increments from consuming a delivery window, and remove foreground rewriting of older saved revisions. A failed write cannot use that removed rewrite path to blank previously committed text; only committed successor content may become visible. An exact refresh of the intact old committed snapshot is permitted separately.

The plan producer now schedules a retained changed tail for 400 ms after its last successfully appended-and-published full value. Its one wake selects only due plans and leaves an assistant's existing delivery window alone. Established barriers save or retire the same pending value, so a later timer cannot recreate a completed row or move its original position. Pressure retains the full value for a positive retry; terminal failure reaches the original current session, with text retained until actual settlement. Plans remain FULL-durability complete status snapshots; no saved field, wire operation or row kind is added. Assistant/reasoning ordinary delivery still depends on arrivals or real barriers. Neither path promises a universal 400 ms visible gap, and event-loop stalls, provider pauses, forced saves and refusal are distinct limits. Retained Claude consumed-attempt and legacy pressure qualifications remain disclosed exclusions.

Independent source/proof review accepted implementation 9ceb2914 at test-only follow-up 2696fb8d, with no new production finding. Follow-up e2d613fe repaired fixture factory typing (+2/−2); then 16212e8e repaired a test Set-iteration probe (+12/−4). The same independent reviewer accepted both with no new finding, verifying scoped measurement equivalence for the probe; QA head e3b680a has independent inverse/source acceptance and passed exact-head CI (PR Checks run 41055217256). Those historical checks included static analysis/typecheck, all 12 test shards, relay integration, wire compatibility, packaging and WSL distro checks. Configured skips remain unexercised checks. The earlier 8255904e green run is also historical. Main 2cc33b8d898 is now merged at 7d3c487039b: the three affected named journal/worker files pass 13 cases after test-only adaptation to canonical outcomes, preserving main’s fixture split and this PR’s cadence and reopen contracts. Independent merge verification confirmed both-parent production preservation. CI run 41182029417 at that merge failed in shards 1, 9 and 10 on fixture assumptions; its five failing instances remain recorded. Follow-up 4f694ad changed four test files only: correct native outcome fault injection and persisted rollback/reopen checks, awaited asynchronous closes, valid row timestamps, canonical outcome assertions and committed-input byte labels. All 131 existing focused cases passed, and the continued reviewer accepted the repair without lost assertions or runtime changes. Fresh exact-head CI passed at 4f694ad (PR Checks run 41183170211): all 12 shards, static/typecheck, relay, wire, persistence, packaging and three WSL distros; 33 configured checks pass and 18 skips remain unexercised. The merge alone does not stale earlier app QA; no new app run is claimed. That earlier Plan proof comprised 128 distinct tracked cases across 11 files, plus four separate recorded-TEXT replays through the production translator, deferred sink and native journal; repeated lifecycle/calibration executions are not additional cases. Replays remap captured reply text/intervals into one plan; they are not actual Codex plan arrivals, live plan-size incidence or new QA. Logical body/whole-row JSON bytes are distinct from physical DB/WAL allocation. The reference comparison covers the current whole mechanism; the attempted reference sync failed, so the reviewer used pinned read-only caches and corrected the source note without claiming latest-remote sync.

CI run 41231657928 on 070946caf51c924e86c5eba0a87328277d9cd132 failed static/typecheck job 124627054356: acp-history-catch-up.test.ts:297 loses the narrowed message-body type inside a nested callback. Follow-up 17a6bda7d3d6fc664270afaa1557568bc0f3188b captures that same guarded body locally and uses it synchronously, preserving every predicate, block guard, expected role/text tuple, native/public/reopen assertion and input. Independent review accepts the exact +9/−7 test-only patch; it adds equivalent test code and leaves production/support bytes unchanged. Fresh CI passed on this exact head (PR Checks run 41233518337): all 12 test shards, static analysis/typecheck, relay integration, cross-version wire compatibility, persistence, packaging and all three WSL distros. All 33 configured checks passed; 18 configured skips remain unexercised. No production failure or other job disposition is inferred from the earlier diagnostic; earlier green runs are historical.

Test consolidation retains 10 of the 12 introduced metadata cases and transfers two obligations into existing cases; the ten new admission cases are separate. The final native/terminal file passed 5 cases; the context file passed 14 before an equivalent Number.NaN spelling change; the five-file targeted contracts passed 11 with 50 skipped; the Pi file passed 12 before an equivalent last-match expression change. Actual recorded Pi usage at a dirty text predecessor does no full materialization, and final canonical/public/history/worker/reopened text remains exact. Fixed N/10N workloads retain both text roles, usage shapes, whitespace, real-boundary calibration and exact finals. ACP history retains usage plus named and anonymous finalization; its role assertion caused the historical CI typing failure, now repaired by the accepted equivalent local alias; fresh exact-head CI now passes. No runtime failure was observed in that assertion. Both ancillary-read cases now execute eight retries and later mailbox/dispatch assertions with the unchanged history scan bound. Historical 53 focused passes, seven selected existing cases and ten private replay cases are separate carried populations, not new test inventory. Old-force, dropped-text and empty-order negative controls remain discriminating.

Verification and limits. Real Windows-low runs used Claude claude-opus-5[1m]/medium and Codex gpt-6.1-sol/low. Both completed long replies; Stop/reopen preserved accepted text and its original position/time; restart and real follow-ups preserved state. Release 1.4.244 read a stopped-data COPY of the earlier pre-#36024 QA build with 17/17 exact text/identity/loadability checks. This is carried earlier evidence, not a new release-reader measurement of keyed outcomes. Existing already-blanked fixtures are test evidence, not an additional release-GUI profile. Native transaction-fault, failed-content publication, pressure-loop and real-trigger N/10N contracts remain supported by their recorded narrow tests.

Positive DOM update gaps, ms Branch p95 / max, reply 1; reply 2 Pinned main p95 / max, reply 1; reply 2
Claude original replies 519 / 616; 509 / 563 6,435 / 8,737; 5,881 / 7,717
Codex original replies Unavailable: observer held a replaced container 3,107 / 3,672; 2,600 / 3,122
Codex separate targeted retries 667 / 1,237; 606 / 697 No paired retry; do not substitute these into the original four-reply bracket

These are recorded real replies, not constructor replay. Generated text, durations and conversation context differ; main also imported six earlier provider items. DOM mutation timing is not compositor paint or a controlled startup/throughput/CPU comparison. Historical measurements on a different machine/model are supplementary only.

Removing foreground retirement increases retained payload until background retirement. For the original four replies on pre-#36024 storage, branch retained/cumulative direct INSERT JSON was 9,233,713 bytes for 79,323 final-text bytes, versus main 790,188 for 71,049. Stopped DB allocation was 10,203,136 versus 1,208,320 bytes; main was sampled later and its whole DB includes the imported history. Logical payload, cumulative inserted bytes and physical DB/WAL allocation are different measurements. Compaction cannot undo cumulative inserted bytes; no universal in-flight WAL bound is claimed. #36024 landed, replacing ordinary raw input-row persistence with keyed saved-state outcomes. These old samples are not current outcome-record costs. Each cadence save that executes, including a successful Plan wake, synchronously validates the full transient input, encodes complete changed items/scalars, performs per-emitted-key indexed lookup/INSERT, updates coverage and commits in the same transaction. It does not add a second whole-chat save after a raw-row INSERT. Journal writes do not coalesce; separate publications can. Suppressed/stale/refused append work does no outcome SQL, but can pay translation/estimation; publication refusal can follow an admitted append. Development builds additionally sample a full restore/equality check at most once per chat per minute. Historical save/COMMIT/body counters omit these operations, so no numeric total per-save cost is claimed for this merged head. #36053 retirement plus integrated body/outcome-record/metadata and DB/WAL measurement remain required.

Codex still displayed “Codex could not confirm Stop. Check the chat before continuing.” The targeted case and matched main control both reached Interrupted / Working 0 with preserved text, acknowledged pause, accepted interrupt, root exit 0 and no recorded errors, but process-tree verification remained unavailable and workload exit unproven. Reproduced on main; tracked in STA-12558. Under the existing scoped disposition this is not a #35462 regression, and it is not a clean Stop pass. Neither descendant exit nor surviving descendants were proved; this PR includes no lifecycle fix or warning waiver.

Adopted-Plan QA passed with the limits below. Fresh native chats still have no Plan-mode control. On M4Air ticket 3583, real Codex CLI 0.165.0 (gpt-6.1-sol, medium) created a conversation through its supported Plan UI, stayed in Plan mode and exited before Session History → Resume in New Chat adopted it. Both native turns on pinned main and all four on the reviewed head retained Plan mode and emitted genuine item/plan/delta events. The head completed its plan, reopened with the same full text and logical identity/order/time, answered a follow-up, then preserved its last accepted 2,135-character partial plan through a live mid-stream graceful close/reopen and answered another follow-up. There was no later mutation of that interrupted plan. The completed provider and journal bodies match exactly: 8,383 characters on the head and 9,217 on main. Rendered-text comparisons separately normalize Markdown; they are not byte-for-byte formatting or DOM-node identity checks.

During the observed Plan stream, the head's largest positive DOM update gap was 567 ms; pinned main's was 5,714 ms. Seven sampled head provider pauses contained a publication before the next outgoing frame; main's two sampled pauses contained none, with the next saves about 0.93–1.02 seconds after the preceding frame. Five head publications at the preceding accepted save plus 401–404 ms strongly support the scheduled wake. Outgoing trace times do not identify callbacks or exclude every delayed generic callback; two other gaps fit the generic 60 ms window. These are different generated plans and small natural-pause samples, not a universal 400 ms promise. The head's early provider trace and final approximately 2.6 seconds before abort were not retained; accepted-text preservation does not prove every upstream byte arrived before close. The late reopen observer cannot exclude an earlier transient flash. Injected refusal/failure cases remain focused-test evidence.

Pinned main independently reproduced three existing observations: a completed Plan folds behind “Worked for” until expanded; the CLI seed's completed Plan is omitted from imported native history; and an unhandled read-state notification is saved as a raw row, not seen rendered in these runs. This is a pass for the changed native Plan stream/lifecycle, not complete seed-history import. The rigs used separate stable throwaway homes without profile copies. Config hash values matched; secret scanning was bounded. All 42 recorded numeric process identities were absent in the final check, the lead terminal closed, assigned ports had no listeners, and ticket 3583 was released.

Earlier lifecycle QA at 1df6e25 carries only for unchanged long-reply, Stop/restart and copied-reader flows. The separate QA 3525 fresh/default-mode attempts on head and main used Codex 0.162.1, had no native Plan control and emitted no Plan events; their ordinary AgentMessage checks are not Plan evidence. Their invalid or late timing observers supply no new timing proof. That earlier lead copied a throwaway rig whose absolute account-home path let the head reuse the baseline Codex home, limiting that run's carry evidence. QA 3583 used independent fresh head/main rigs and supersedes the earlier Plan-unexercised disposition. The settled Plan scheduling choice remains unchanged.

The independent review checked scheduler isolation, original failure ownership, no-head pressure progress, metadata/final/late-key behavior, native FULL/reopen contracts, operation-count scaling, references and maintainability. Cumulative source at e3b680a is 17 production files +616/−249, with 47 exact moved additions and 569 conservatively counted additions including one nonruntime comment; tests +3296/−267 and support +137/0. Those totals were independently recalculated at that QA head. At the historical premetadata head after the main merge, production/support counts were unchanged and tests are +3353/−266 across 19 files after the test-only repair; those are historical whole-PR totals against merged main. Independent integration review accepts production preservation and all test/evidence repairs. Forecast overrun and substantial fixture cost are disclosed; size and green tests are not architecture evidence. The 13-slot borrowed-state checkpoint interface remains a disclosed reading cost; no better complete cut or required ownership rewrite was established. Its plan-only reason is now documented. The later consolidation described above removes repeated proof while preserving the distinct failure boundaries. No new full-content owner, accepted clock, second guard or generic scheduling framework was added.

Current totals against merged main 2cc33b8d898b961c197b7863ac7dbe921821803f are 23 production files +723/−307, 27 test files +4648/−275 and two support files +137/0. Conservative production additions are 651 after 47 exact moved lines, 16 exact moved type lines and nine connected formatting lines; before this repair they were 635. The original 220–320 estimate and 640 review signal remain disclosed: the measured crossing comes solely from the reviewed result propagation and is addressed by that design settlement. Pruning alone is +121/−223, a 102-line net reduction, less than the forecast 115–175. New admission proof and focused test placement are charged separately; deletion of tests does not offset production growth. The five native/terminal cases use existing constructors in a focused test file; the original lifecycle test is unchanged, with no new support/helper module or production injection seam. The later +9/−7 history assertion repair adds two net test lines, no cases, and does not change the pruning total or production count.

Metadata work remains proportional to metadata arrivals. A resumed/newest target still uses the existing journal scan; context revisions still serialize complete changed metadata and can require FULL commits. In the previously accepted long-reply synthetic-usage replay, one Codex workload still made 4,251 native commits, including 3,983 FULL commits, versus 271/3 in its control. That unchanged-workload evidence is carried, not rerun. Independent metadata removes the extra text joins and saves; it does not establish a CPU, latency, physical-I/O or storage bound. Logical text, body, transient-row and saved-record byte populations remain separate. The integrated current-format storage measurements and retirement proof remain required.

Authorized main merges 71a1b345 and ab11dbe2 were reviewed narrowly; the latter integrates main a88581d8, including #36147. No routine second main sync was performed. Stop warning remains the matched-main issue tracked in STA-12558, not a clean Stop/OS-exit proof. Deferred P3 items include the generic protected-empty-key helper outside the actual caller domain, incomplete historical report-helper stderr/version evidence, and exact 60 ms attribution (a technical default, not a located Brennan quotation). Storage dependencies and integrated cost/reader proof remain mandatory.

For Brennan's review

Flow Branch / reader screenshots Main / comparison
Adopted Codex Plan, settled disclosure Head e3b680a, adopted Codex Plan after completion Main a88581d8, adopted Codex Plan after completion
Prior-release seed Claude seed Seed shared with both builds; Codex seed URL mapping absent
Claude long reply 1 streaming · completed streaming · completed
Claude long reply 2 streaming · completed streaming · completed
Codex long reply 1 streaming · completed streaming · completed
Codex long reply 2 streaming · completed streaming · completed
Codex separate DOM retry 1 streaming · completed No paired main retry; original main replies are separate
Codex separate DOM retry 2 streaming · completed No paired main retry; original main replies are separate
Claude Stop / history reopen before Stop · after Stop · reopened Original E recovery case; later matched Codex control below
Codex Stop / history reopen before Stop · after Stop · reopened Original E recovery case; later matched Codex control below
Claude app restart / follow-up before restart · reopened · real follow-up No main run for this flow
Codex app restart / follow-up before restart · reopened · real follow-up No main run for this flow
Release COPY reader — Claude completed reply · Stop item · restart · follow-up Release 1.4.244 reads stopped E COPY; representative views, not 17 separate linked images
Release COPY reader — Codex completed reply · Stop item · restart · follow-up Release 1.4.244 reads stopped E COPY; representative views, not 17 separate linked images
Targeted Codex Stop attribution 71a1 streaming · 71a1 warning main f7bb streaming · main f7bb warning

Screenshot receipt limits: the branch/release captures above were downloaded from their historical uploads with exact original blob/SHA256 matches, uploaded through gh image, and downloaded again through authenticated GitHub requests with identical SHA256. The four targeted Stop links retain their original authenticated HTTP-200 download hashes. The eight main comparisons were recovered from the final remote receipt manifest, and the original PNGs were copied with their recorded hashes, uploaded through gh image and downloaded again with identical SHA256. The first former main URL returned authenticated HTTP 404 from this Mac; that negative receipt is retained. No outcome is inferred for the other seven former URLs. A Codex seed link remains unavailable in this bundle. The COPY reader's 17 checks have eight exact screenshot-path matches in this local upload snapshot; the linked representative views do not imply 17 individually uploaded captures. The main comparison links above are now mapped; the remaining seed/per-item screenshot limitations stay explicit. No passed flow needs another run.

Human judgment points; producer links retain the QA head and storage points at the merged head:

brennanb2025 and others added 8 commits October 31, 2026 09:30
Resolve import conflicts from main's module moves. #37216 stopped the
journal sink refusing writes under backpressure, so drop the tests that
forced a sink refusal and keep one that proves a time-gated tail is saved
before an incoming stream evicts it. Move the scheduled plan owner check
out of codex-structured-session-acquire.ts to stay under its line cap.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants