Sign up
View stargazers13Sign up

fix(server): avoid reparsing streamed replies from the beginning#24432

open3
main
fix/issue-19992
opened22h agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 22h ago

Problem

Long assistant replies inside an unfinished code fence or paragraph repeatedly parse the accumulated text while waiting for a Markdown boundary. Asking an agent for a 1,500-line fenced reply reproduced repeated full scans in the real app; the deterministic regression test performs 81,800 newline searches for 400 appended lines on main.

Change

Keep scan progress per active assistant message and resume newline, fence, and partial-line checks from the previous offsets. Reset that progress when a provider replaces text, and discard it when streaming ends. Delivery boundaries and the 400 ms delivery gate stay the same.

Scope and approval

Fixes https://github.com/pingdotgg/t3code/issues/19992 GitCafe: https://git.cafe/pingdotgg/t3code/issues/19992

Triaged bug via the automated issue flow. Focused server performance fix with unchanged delivered output, contracts, settings, and clients.

Verification

  • Red regression: 81,800 newline searches on main; 803 at the fix for 400 appended lines. The closing-fence output assertion passes.

  • vp test run apps/server/src/orchestration-v2/assistantStreaming.test.ts apps/server/src/orchestration-v2/RunExecutionService.test.ts: 47 tests passed.

  • Targeted lint and formatting on both changed files pass; server typecheck passes.

  • Two independent reviews found no blockers. Differential streams exercised appends, truncations, replacements, partial Markdown, throttling, and final events without output mismatches.

  • Before runtime transcript and red failure. Live baseline measured 345 splitter calls over 44.9 seconds and 14.7M input characters for 13.8K new characters. Separate profiles measured modest splitter CPU cost, not a severe bottleneck.

  • Proof matrix: empty render matrix; server-only change, no changed visual surface.

  • After runtime transcript, measurements, and sanitized snapshots and measurement drivers: 1,788 sampled live updates parsed 749 newly completed lines with 2,537 newline searches. Final delivery and persistence exactly matched all 1,500 rows (112,910 characters).

  • Proof audit independently validated all published snapshots, exact final text, and the base/head regression counts: SUFFICIENT.

Runtime evidence was captured at 0f25149cef against a46f80ee28. The subsequent main merge changes client files only; the scanner and tests are identical. No affected visual surface is unverified. The provider omitted a trailing newline after the closing fence: natural newline-terminated closure was not observed, and the supplemental inspector probe is explicitly labelled. Initial scan and gaps between capture windows were not measured. Server exit during inspector cleanup occurred after evidence capture; its cause is unconfirmed.

The native prefix comparison remains O(n) per processed update so provider replacements can be detected. This removes repeated Markdown parsing; it does not make total work strictly linear across every update.

Implemented with Claude Opus 5.5 through the Claude harness in T3 Code; reviewed with GPT-6.1-Sol and Claude Opus 5.5. Issue lead: GPT-6.1-Sol through the Codex harness in T3 Code.

Checks

All checks passed

Added 3 commits

  • fix(server): streamed replies scan each line once while waiting for a boundary0f25149
  • test(server): reproduce #19992 repeated scanning of streamed assistant text2aee68c
  • chore: merge main into fix/issue-199928de253e
MmacroscopeappBot22h ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This server-side performance fix changes the existing assistant streaming path from stateless full parsing to stateful incremental scanning, including replacement detection and partial-line handling. The scope is focused and tested, but the new parser state and lifecycle behavior are sufficiently nontrivial to warrant human review.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot22h ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB0 B (0.0%)2.0 KiB✅
CodexLive turn WebSocket decoded20.9 KiB20.9 KiB0 B (0.0%)29.3 KiB✅
CodexLive turn messages220 (0.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.2 KiB1.2 KiB0 B (0.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.2 KiB21.2 KiB0 B (0.0%)29.3 KiB✅
ClaudeLive turn messages220 (0.0%)8✅

Baseline: d53873b · PR result: 8de253e · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot22h ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between d53873b6a4799e87afd63c8d288ef0089f5a3748 and 8de253eebffb79842ddaf2457b98142ce77931d4.

External image Files selected for processing: 2
  • apps/server/src/orchestration-v2/assistantStreaming.test.ts
  • apps/server/src/orchestration-v2/assistantStreaming.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.

Walkthrough

The assistant-text boundary scanner now retains per-text progress and resumes from the last partial line when streamed text grows. It resets scan state when text is replaced. The streaming filter stores scan and delivery state per message. Tests cover replacement replies and long open code fences.

Changes: Assistant streaming scan

Layer / File(s)Summary
Incremental boundary scanning
apps/server/src/orchestration-v2/assistantStreaming.ts		The scanner resumes from saved positions, applies the existing fence, blank-line, and heading rules, and recognizes qualifying partial list items. splitBufferedAssistantText uses the shared scanner.
Streaming filter state and validation
apps/server/src/orchestration-v2/assistantStreaming.ts, apps/server/src/orchestration-v2/assistantStreaming.test.ts		The filter stores scan progress with delivery state, clears state for non-streaming messages, and retains the existing 400 ms throttle. Tests cover replacement replies and long open code fences.

Priority: External image Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · up to 8de25

The reviewed streaming changes have no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main server performance fix: avoiding repeated reparsing of streamed replies.
Description checkPassedThe description includes complete Problem, Change, Scope and approval, and Verification sections. It provides reproduction details, linked issue context, test results, runtime evidence, limitations, a…
Linked Issues checkPassed[#19992] Requirement: per-update server work stays roughly constant as a streamed reply grows. The cited defect: makeAssistantStreamingFilter calls splitBufferedAssistantText on the full accumulated t…
Out of Scope Changes checkPassedThe whole-PR diff (merge base d53873b6 to head 8de253ee) changes two files: apps/server/src/orchestration-v2/assistantStreaming.ts and assistantStreaming.test.ts. All source hunks serve the incrementa…
Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It provides reproduction details, linked issue context, test results, runtime evidence, limitations, and agent attribution.

Full details: Linked Issues check

Explanation

[#19992] Requirement: per-update server work stays roughly constant as a streamed reply grows. The cited defect: makeAssistantStreamingFilter calls splitBufferedAssistantText on the full accumulated text on each update after the 400 ms window. At the reviewed head, advanceScan (assistantStreaming.ts) keeps per-message scan state in the streams map. It stores text, lineStart, newlineSearchFrom, openFence, boundary, titleAwaitingContent, and partial-line offsets. When the new text starts with the previous text, scanning resumes from those offsets, so each complete line is processed once. The partial line is checked incrementally with sticky regexes (INDENT_PATTERN, LIST_MARKER_PATTERN, TRAILING_SPACE_PATTERN). A non-prefix replacement resets the scan. A non-streaming message deletes its state. The partial-line list check matches LIST_ITEM_START_PATTERN on the trimmed line: indent [ \t]*, a marker followed by [ \t], and content after the marker. Fence, blank-line, heading, and section-title rules are unchanged. The 400 ms gate and turn-mode suppression are unchanged. The added test 'scans each streamed line once while a long code fence is still open' asserts fewer than four newline indexOf searches per line over 400 appended lines. The replacement test checks that a replaced reply is rescanned. Residual cost: startsWith compares the previous text on each update. This is a native linear comparison, not a Markdown rescan. The PR discloses it. The reported Markdown rescanning is removed. I did not execute the tests. The verdict relies on the PR's reported test results and on code reading.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff (merge base d53873b6 to head 8de253ee) changes two files: apps/server/src/orchestration-v2/assistantStreaming.ts and assistantStreaming.test.ts. All source hunks serve the incremental scanner (advanceScan, startScan, trimLineEnd, stickyMatchEnd, partialLineStartsListItem, readyBoundary), the per-message state in makeAssistantStreamingFilter, or the splitBufferedAssistantText refactor onto the same scanner. The test hunks add a provider update fixture and tests for replacement rescans, newline-search count, and closing-fence output. These support #19992. No contracts, settings, client, or unrelated server files change. RunExecutionService.test.ts appears only in the PR's test run and is not in the changed-file diff.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.