Repository navigation
perf(native-chat): save new reply text without copying the growing reply - #32315
brennanb2025 wants to merge 23 commits into
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
There was a problem hiding this comment.
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:
788929a2-62ef-4a44-b5ed-55e3db38d109
📒 Files selected for processing: 37
src/main/claude/claude-streamed-text-checkpoints.tssrc/main/claude/claude-streamed-thinking.tssrc/main/claude/claude-structured-journal-translation.tssrc/main/codex/codex-structured-item-streams.tssrc/main/codex/codex-structured-journal-sink.tssrc/main/codex/codex-structured-journal-translation-streams.test.tssrc/main/native-chat/agent-session-journal/journal-cursor.tssrc/main/native-chat/agent-session-journal/journal-item-appender.tssrc/main/native-chat/agent-session-journal/journal-open.tssrc/main/native-chat/agent-session-journal/journal-provider-activity.tssrc/main/native-chat/agent-session-journal/journal-reducer.tssrc/main/native-chat/agent-session-journal/journal-row-schema.tssrc/main/native-chat/agent-session-journal/journal-row-writer.tssrc/main/native-chat/agent-session-journal/journal-step-writer.tssrc/main/native-chat/agent-session-journal/journal-store-collaborators.tssrc/main/native-chat/agent-session-journal/journal-store-contracts.tssrc/main/native-chat/agent-session-journal/journal-store.tssrc/main/native-chat/agent-session-journal/journal-streamed-text-revision.tssrc/main/native-chat/agent-session-journal/journal-text-fold.tssrc/main/native-chat/agent-session-journal/journal-text-index.tssrc/main/native-chat/agent-session-journal/journal-text-persistence.test.tssrc/main/native-chat/agent-session-journal/journal-text-pruner.tssrc/main/native-chat/agent-session-journal/journal-text-rows.tssrc/main/native-chat/agent-session-journal/journal-text-writer.tssrc/main/native-chat/agent-session-timeline/provider-timeline-text-streams.tssrc/main/native-chat/agent-session-timeline/provider-timeline-transition-layout.tssrc/main/native-chat/agent-session-wire/agent-session-delta-coalescer.test.tssrc/main/native-chat/agent-session-wire/agent-session-delta-coalescer.tssrc/main/native-chat/agent-session-wire/agent-session-history-latest-turn.tssrc/main/native-chat/agent-session-wire/agent-session-history-page.tssrc/main/native-chat/agent-session-wire/agent-session-journal-batch.tssrc/main/native-chat/agent-session-wire/agent-session-streamed-text-bounds.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-sink.tssrc/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.tssrc/main/native-chat/agent-session-wire/structured-agent-session-sink-admission.tssrc/main/native-chat/agent-session-wire/structured-agent-session-text-append.tssrc/shared/agent-session-journal-types.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments: 1
src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts: 1
266-272: 🚀 Performance & Scalability |Trivial | ⚡ Quick win Compute the item byte estimate once per append.
appendItemcallsestimateStructuredAgentSessionItemBytes(identity, body, options)three times. Each call does a fullJSON.stringifyofbody. This path carries whole bodies, for example Codex completed agent messages throughappendCodexLifecycleItem→tryAppendItem. A whole body can be up to 16 MiB, so each append does three full serializations instead of one. All three values are the same, so compute the estimate once.♻️ Proposed refactor
const appendItem: NonNullable<StructuredAgentSessionEventSink['tryAppendItem']> = ( identity, body, options - ) => - queue.submit( + ) => { + const bytes = estimateStructuredAgentSessionItemBytes(identity, body, options) + return queue.submit( { - bytes: estimateStructuredAgentSessionItemBytes(identity, body, options), - ...text.hold(estimateStructuredAgentSessionItemBytes(identity, body, options)), + bytes, + ...text.hold(bytes), run: (bound) => bound.journal.appendItem(identity, body, { ...structuredAgentSessionJournalAppendOptions(bound.fence, options), - reservedBytes: estimateStructuredAgentSessionItemBytes(identity, body, options) + reservedBytes: bytes }) }, options ) + }

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:
e859de00-a9a4-400b-849b-ee5ef4fe2492
Files selected for processing: 74-
src/main/acp/acp-structured-antigravity.test.tssrc/main/acp/acp-structured-opencode-recordings.test.tssrc/main/acp/acp-timeline-recordings.test.tssrc/main/claude/claude-journal-translator-contract.tssrc/main/claude/claude-message-journaling.tssrc/main/claude/claude-open-turn.tssrc/main/claude/claude-result-journaling.tssrc/main/claude/claude-streamed-block-identity.tssrc/main/claude/claude-streamed-terminal.tssrc/main/claude/claude-streamed-text-checkpoints.test.tssrc/main/claude/claude-streamed-text-checkpoints.tssrc/main/claude/claude-streamed-text-contracts.tssrc/main/claude/claude-streamed-thinking.tssrc/main/claude/claude-structured-journal-translation.test.tssrc/main/claude/claude-structured-journal-translation.tssrc/main/claude/claude-structured-provider-fallback.tssrc/main/claude/claude-structured-session-journal-control.tssrc/main/claude/claude-terminal-failure.tssrc/main/codex/codex-stream-finishes.tssrc/main/codex/codex-structured-item-stream-contracts.tssrc/main/codex/codex-structured-item-streams.tssrc/main/codex/codex-structured-journal-items.tssrc/main/codex/codex-structured-journal-settlement.tssrc/main/codex/codex-structured-journal-sink.tssrc/main/codex/codex-thread-item-identity.tssrc/main/codex/codex-unfinished-item-body.tssrc/main/native-chat/agent-session-journal/journal-cursor.tssrc/main/native-chat/agent-session-journal/journal-item-appender.tssrc/main/native-chat/agent-session-journal/journal-item-fold.tssrc/main/native-chat/agent-session-journal/journal-item-readers.tssrc/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.tssrc/main/native-chat/agent-session-journal/journal-open.tssrc/main/native-chat/agent-session-journal/journal-provider-activity.tssrc/main/native-chat/agent-session-journal/journal-reducer.tssrc/main/native-chat/agent-session-journal/journal-row-schema.tssrc/main/native-chat/agent-session-journal/journal-row-table.tssrc/main/native-chat/agent-session-journal/journal-row-writer.tssrc/main/native-chat/agent-session-journal/journal-step-writer.tssrc/main/native-chat/agent-session-journal/journal-store-collaborators.tssrc/main/native-chat/agent-session-journal/journal-store-contracts.tssrc/main/native-chat/agent-session-journal/journal-store.tssrc/main/native-chat/agent-session-journal/journal-streamed-text-revision.tssrc/main/native-chat/agent-session-journal/journal-text-contract.test.tssrc/main/native-chat/agent-session-journal/journal-text-fold.tssrc/main/native-chat/agent-session-journal/journal-text-index.tssrc/main/native-chat/agent-session-journal/journal-text-persistence.test.tssrc/main/native-chat/agent-session-journal/journal-text-pruner.tssrc/main/native-chat/agent-session-journal/journal-text-query.tssrc/main/native-chat/agent-session-journal/journal-text-rows.tssrc/main/native-chat/agent-session-journal/journal-text-test-support.tssrc/main/native-chat/agent-session-journal/journal-text-writer.tssrc/main/native-chat/agent-session-timeline/provider-timeline-assembler-refusal.test.tssrc/main/native-chat/agent-session-timeline/provider-timeline-plan.tssrc/main/native-chat/agent-session-timeline/provider-timeline-text-streams.tssrc/main/native-chat/agent-session-timeline/provider-timeline-text-write.tssrc/main/native-chat/agent-session-timeline/provider-timeline-transition-layout.tssrc/main/native-chat/agent-session-wire/agent-session-delta-coalescer-contracts.tssrc/main/native-chat/agent-session-wire/agent-session-delta-coalescer.tssrc/main/native-chat/agent-session-wire/agent-session-history-forward-read-budget.test.tssrc/main/native-chat/agent-session-wire/agent-session-journal-batch.tssrc/main/native-chat/agent-session-wire/agent-session-streamed-text-bounds.tssrc/main/native-chat/agent-session-wire/agent-session-text-encoding.tssrc/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-sink-estimate.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-event-sink.tssrc/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.tssrc/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.tssrc/main/native-chat/agent-session-wire/structured-agent-session-stale-state-settlement.tssrc/main/native-chat/agent-session-wire/structured-agent-session-text-append.tssrc/main/native-chat/agent-session-wire/structured-agent-session-transition.tssrc/main/native-chat/agent-session-wire/structured-agent-session-unfinished-work.tssrc/main/pi/rpc-capture-replay.test.ts
Files with no reviewable changes: 2-
- src/main/claude/claude-streamed-text-checkpoints.test.ts
- src/main/claude/claude-result-journaling.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
-
There was a problem hiding this comment.
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:
e9edd329-6c83-4214-ae23-bb3774114337
Files selected for processing: 12-
src/main/claude/claude-streamed-terminal.tssrc/main/claude/claude-streamed-text-checkpoints.test.tssrc/main/claude/claude-streamed-text-checkpoints.tssrc/main/claude/claude-streamed-text-contracts.tssrc/main/claude/claude-streamed-thinking.tssrc/main/codex/codex-stream-finishes.tssrc/main/codex/codex-structured-item-stream-contracts.tssrc/main/codex/codex-structured-item-streams.tssrc/main/codex/codex-structured-journal-items.tssrc/main/native-chat/agent-session-journal/journal-text-persistence.test.tssrc/main/native-chat/agent-session-wire/agent-session-delta-coalescer.tssrc/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; 6 remain after this review.
-
|
Long chat replies previously sat unchanged for seconds while the agent continued producing text: the host waited for increasingly large additions, and each save repeated the reply so far. This PR saves committed text suffixes on the shared window, then authoritative whole replies on completion, interruption and correction. Existing client frames and the landed subscriber seam are preserved. The full loop completed two broad review rounds, followed by narrow verification of repairs, CI failures and main conflict integrations. Provider, storage, architecture/reference, performance and readiness source reviews closed the seven original roots and both later findings. The proven simplifications were applied. The repairs stay inside R and add no further mechanism, saved field, wire operation or second guard. Effective counts against pinned main Full CI passed on d49bfc4, including static analysis/typecheck, all five unit shards, cross-version wire checks, both packages and WSL. The final integration’s targeted evidence is 167 passing cases across 12 named files, no reported skips; earlier repair runs overlap and are not added into a distinct-test total. A final review-thread audit found one internal partial-capability fallback that could save a placeholder instead of reasoning. The existing whole-route text selection was consolidated to preserve the actual string; its exact regression failed before the repair and 40 tests in three named files passed afterward, with the normal hook. No shipped caller has the failing capability combination; the live sink supplies both preparation/transition APIs and import supplies neither. All five review threads are resolved: three fixed and two refuted against their concrete input and binding fail-safe policy. The earlier popup/Windows-hook timeouts did not recur; their exact causes remain unknown. One admitted M4Air QA round used unpackaged production build outputs with owned Electron (the runtime reported isDev=true), rather than an installed release package, and ran real Claude and Codex on 2910345:
The invalid hidden-root metrics capture and stages that completed before the intended fault receive no recovery/performance credit. Only the still-unpassed Codex fault case was restaged. All owned apps, agents and drivers were stopped by recorded identity; the lease was released. The QA head carries across Matched same-machine finite live runs used the same 643-byte prompt and per-agent settings on main Small live replies wrote more logical payload on the branch: Claude 186,429 → 223,215 bytes and Codex 208,267 → 347,564; retained payload fell to 22,828/25,958 bytes. Separately, the actual shipped-endpoint 30/300-piece corpus at 164.1 KB wrote 25.31 MB on main versus 0.732 MB on the branch, including final/correction/lifecycle records, with matching final text, reopened body/revision equality, successful history reads and no replay damage. This corpus does not separately assert every history/replay body, ID or ownership field; the persisted consumer contracts cover those separately. The corpus counter counts StatementSync.run calls, not every SQL operation. The separately reviewed operation-bound contracts and this corpus do not establish universal byte savings or CPU/physical-WAL improvement. OpenCode/Pi before measurements remain missing. Deferred concerns and limits: Claude’s next reply described the interrupted request as unanswered despite its exact visible saved partial. The selected provider transcript/ancestry and actual resume identity were traced; exact resumed model input was not captured, and the resume/input construction is unchanged. The external Session History panel also retained its earlier summary while the central transcript and canonical history showed the next exchange; its attribution is unknown. Provider-context completeness and R causality remain unknown, rather than a claimed context pass or proven R defect. A synthetic test that explicitly repeats a final provider frame after forced refusal produced duplicate bodies; the supported single-frame/retained-source terminal retry passes with exactly one whole. No actual duplicate provider emission was captured. Neither observation authorizes normal content loss or a speculative new guard. The first-hand whole-mechanism comparison is complete: the common pattern is committed pieces plus authoritative wholes, and successful database application owns publication. Intended differences preserve whole-item wire/phone compatibility, independently routed hosts, bounded durable retirement and safe direct old-reader refusal. The existing older-client subscriber limitation is temporary, tracked in STA-11842. Failed retirement validation retains its pieces and stops that drain; a later whole commit may retry it. Persistent malformed support can hold other retirement in that drain. This is the ruled fail-safe policy, not a timer spin or permission to discard content, and user actions are not gated by cleanup. Intended abnormal difference: history with a gap can miss interior events deleted outside Orca. The genuine pinned older production decoder safely refused current-writer database copies before and after retirement; this was not a full old-app run. Phone/second-client, old-peer, CLI folder/SSH routing and other unchanged consumers are covered through persisted integration/contracts; mocked SSH routing is not real SSH transport evidence. Dropped runtime cells: standalone SSH, phone/released second-client, old-host, older-binary refusal copies, folder topology and unchanged settings. No new screen appearance required a separate screenshot-only run. For Brennan’s review: the key judgment points are producer admission and final ownership and bounded suffix retirement. Actual full-renderer screenshots follow; main is shown beside the branch only for the matched gap flow where main ran.
|
















Problem and change
Long chat replies can sit unchanged for seconds while the agent is still producing text. The previous growth gate waits for increasingly large additions before saving and publishing an update, and each saved update repeats the reply so far.
This change saves newly arrived assistant and reasoning text on the shared 60 ms window, with authoritative whole replies on completion, interruption and correction. Existing whole-item client messages, mixed-version peers, SSH and folder workspaces keep their contracts. The database keeps its existing tables and version; an early private-format marker lets an older binary directly opening a newer-format chat refuse it safely.
Superseded pieces retire against a retained authoritative whole, and surviving events replay in order. Intended abnormal difference: history with a gap can miss interior events deleted outside Orca. Observed malformed rows, broken unfinished text chains and legacy-prefix gaps still fail validation. Cleanup preserves healthy content, placement, activity, lifecycle and controls; cleanup failure must not block a user action.
Current verification
The replacement is committed and pushed at
d49bfc4220f95e8293f213145c6eeb7468d562ec, including the landed subscriber integration and pinned main conflict integrations. The normal pre-commit hook passed. Effective PR counts against fetched main6ed2e6d300e3afb440f6b1064f97732029cbcfbf: 3,763 production additions / 964 deletions; 2,331 test/support additions / 233 deletions, across 102 paths. Inherited main changes are excluded from those counts.Final independent source/integration and bounded QA-evidence reviews are complete. The single real-app QA round completed lifecycle/recovery with Claude and Codex and collected the matched after-gap evidence. Full CI passed on the current head, including static analysis/typecheck, all five unit shards, wire compatibility, both package jobs and WSL jobs. Independent provider/storage/architecture, whole-mechanism reference and narrow integration reviews closed all seven original roots and both later findings; the final test-only timeout-diagnostic review also found no issue. Leading whitespace is preserved, selected Codex completion/release uses exact producer keys, and ordinary Claude frames keep their ordinary admission path. No additional guard, mechanism, saved field or wire operation was introduced by these repairs.
The actual app head is
291034538222179e35ac6295078ffc97600a4bea; later commits are test diagnostics, a pinned main conflict merge, and the internal fallback repair described below. The test-only commit and main merge carry the app evidence under the staleness rule. The final fallback repair was independently verified to leave every shipped caller’s lifecycle/recovery behavior unchanged; qa_head carries the app evidence over this tested internal contract correction; current-head review and CI verify the resulting source. The earlier browser-popup and Windows hook timeouts did not recur in the full green run. Their exact causes remain unproven, rather than being described as fixed app bugs.The final review-thread check also found an internal sink combination that exposes text preparation without a transition writer. Its whole completion could replace reasoning with a placeholder. The fix derives the actual whole text from the same existing route selection. No shipped caller has that capability combination; live sinks supply both APIs and import sinks neither. The exact regression failed before repair and 40 tests across three named files passed after it; the normal commit hook passed. No additional mechanism, saved field or wire operation was added.
Targeted verification includes the initial 235 tests across 23 named files, subsequent affected-path runs, 154 passing tests across seven files for the preceding repair, and 167 passing cases across 12 named files with no reported skips for the final conflict integration; these runs overlap and are not a distinct-test total. Genuine pinned older production decoder execution safely refused current-writer database copies before and after retirement. Persisted history/consumer and CLI routing integration contracts also passed. The older-decoder check is not a full older-app run, and mocked SSH routing is not a real SSH transport run. Tests and a successful hook establish neither the architecture nor real-app readiness. A constructed repeated-final input after forced refusal produced duplicate bodies; source tracing found the supported reader delivers each received frame once and retries the retained source on the normal terminal path, which passes with one whole body. No actual duplicate provider emission was captured. This remains a deferred synthetic-input concern, not an idempotency guarantee or an accepted normal-flow regression.
One admitted M4Air lease used unpackaged production outputs with owned Electron (runtime isDev=true), rather than an installed release package, and exercised actual previous-build-created chats through graceful upgrade, two ordinary real replies per agent, graceful restart and next reply; Stop → Interrupted → next reply; and separate active host faults with 645 Claude / 287 Codex durable suffixes, exact saved-text/order recovery and new real replies with no automatic revival observed before the next send. Recovery advances the partial revisions 647/289 → 648/290, adds recovered metadata and one crash-status item, and interrupts the old turn while preserving the 24,068/8,707-byte post-kill bodies. Whole item envelopes are not byte-identical. Expected lifecycle/context-usage metadata changes are recorded separately from text equality. Claude’s next reply described the interrupted request as unanswered despite the visible saved partial; exact resumed model input was not captured, source resume/input construction is unchanged, and attribution remains unknown. The external Session History panel retained its earlier summary while the central transcript/canonical history showed the next exchange; attribution is also unknown. These are deferred observations, not a claimed context-preservation pass or a proven R defect. Other phone/second-client, older-peer, old-reader refusal, SSH and folder routes use contract/integration tests under the current QA table. Screenshot runs are added only for changed screen appearance.
Measurements
The accepted before capture used Windows, real Claude/Codex replies, and main
dcf2795b1d022779c0bd0b9d3a9361824427b36e. Across four replies, assistant-row JSON inserted was 166,474 / 169,206 / 219,458 / 243,515 bytes for approximately 14.8–21.0 KB of final text. Maximum positive DOM growth gaps were 5.706 / 5.539 / 13.609 / 14.804 seconds while provider text continued arriving. Host CPU was 1,688 / 1,344 / 2,531 / 2,516 ms. These are logical saved bytes and DOM observations, not physical SQLite writes or compositor paint measurements.A same-machine finite comparison on the M4Air used pinned main
0399ffa2c6e78379bd9a264c5ab53f6b77fd90dcand the actual app head291034538222179e35ac6295078ffc97600a4bea, the exact 643-byte baseline prompt, and matched per-agent settings. The maximum positive DOM-growth gap fell from 6,069 to 230 ms for Claude and 12,179 to 710 ms for Codex. The lead-recorded host aggregates associate source events with the main gaps; raw DOM events were independently recomputed, while the six host/ingress/publication raw traces were not included in the selected transfer and those aggregates/live write totals were not independently recomputed; the R replies had no observed 5–15 second growth gap. These are finite samples with different generated answers, not a fixed-output causal experiment. The first invalid H captures observed a hidden transcript and are excluded; only the corrected visible-root captures are credited.The small live R replies wrote more logical journal payload, while retaining less: Claude main/R inserted 186,429/223,215 bytes and retained 186,429/22,828; Codex inserted 208,267/347,564 and retained 208,267/25,958. This is not a universal write-byte reduction claim. Separately, the shipped-endpoint corpus benchmark at 164.1 KB of text wrote about 25.31 MB on main versus 0.732 MB on R, including final/correction/lifecycle records, with matching final text, reopened body/revision equality, successful history reads and no replay damage. The corpus does not separately assert every replay/history body, ID or ownership field; persisted consumer contracts cover these separately. Its counter measures StatementSync.run calls, not all SQL operations. The separately reviewed 30/300-piece contracts check the operation bound.
OpenCode and Pi before-numbers are missing. DOM/RAF is not compositor paint; logical journal bytes are not physical SQLite/WAL writes. CPU endpoints include setup/navigation and do not support a matched reply CPU improvement claim.
Builds on merged #30418. No upstream skill source or registry material is copied.
Reference comparison and scope
The final first-hand whole-mechanism comparison is complete. The common pattern is committed text pieces plus authoritative final/correction wholes, with successful database application owning publication. Intended differences preserve Orca’s existing whole-item wire format for mixed-version desktop/phone peers, separate execution hosts, bounded durable retirement and safe direct old-reader refusal. The frozen landed subscriber seam is unchanged. The pre-existing older-client subscriber limitation is temporary and tracked in STA-11842; this PR does not expand it. The abnormal externally deleted history difference is stated above. No new screen or changed screen appearance is added.
Dropped runtime cells under the current QA table: standalone SSH, phone/released second-client, old-host, older-binary refusal copies, folder topology and unchanged settings; their contracts remain tested. This is one real lifecycle/recovery QA round, with only unpassed fault cases restaged, rather than repeated full rounds.