Sign up
View stargazersSign up

perf(server): stop tracing per-event live coalescer work#29758

merged1
main
GitHub fork: teedole/stop-tracing-live-coalescer
opened4d agobyOonlydole
1 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

Problem

The local server trace (server.trace.ndjson, 10 rotated files of 10 MB plus the live one, with no sampling) keeps only about two hours of history during active use. Issue #28196 reports the same rotation. Per-event spans that finish in under a millisecond take up a large share of it.

On one install, ThreadLiveEventCoalescer.cancelWindow, flushPending and offerAll wrote about 9,100, 9,300 and 6,200 spans in a 112-minute window. That was 9.2 MB, or 8% of a 113 MB trace, with each span under 1 ms.

Change

cancelWindow, flushPending and offerAll now use Effect.fnUntraced, with a one-line comment giving the reason. They run for every live thread event or batch. makeThreadLiveEventCoalescer keeps its span, which is written once per subscription when it starts. flushPending still runs uninterruptibly.

This follows #29640, which stopped tracing project enrichment cache reads for the same reason.

Scope and approval

This is a focused performance fix for trace volume and part of #28196. It changes only span wrappers in one file. Nothing in the repo reads these span names.

Verification

  • Before the change, I confirmed on current main that all three functions were wrapped in named Effect.fn spans.
  • I searched for readers of the span names in tests, docs and span-name assertions, and found none.
  • vp test run apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.test.ts: 12 of 12 pass.
  • vp run check:changed passes: format, lint, knip, typecheck and tests for the changed files.

The change adds no tests, because it doesn't alter behavior. That matches #29640.

Made with Claude Opus 5.5 through the Claude Code harness in T3 Code.

Checks

24 successful · 15 skipped
All checks passed
5f9fcff

Added 1 commit

  • perf(server): stop tracing per-event live coalescer work5f9fcff
Cchatgpt-codex-connector[bot]4d ago
Synced from GitHub

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

ReviewStatusCommitReview trigger
đź“ť Code Reviewâś… Completed 2026-11-03T00:28:18.256487Z5f9fcffPR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with đź‘€ while any review is running, comments if it has suggestions, and reacts with đź‘Ť once all reviews finish with no findings.

CCopilotBotreviewed4d ago
Synced from GitHub

🟢 Approval recommended

No active review issues are reported for the change to reduce routine server trace volume.

No open issues 🎉

What changed in this PR

Reduces routine server trace volume from live thread event processing, addressing part of #28196 and following the approach in #29640.

Changes:

  • Switches cancelWindow, flushPending, and offerAll to Effect.fnUntraced.
  • Documents why these high-frequency operations omit spans.
  • Retains the subscription creation span and uninterruptible flushing behavior.
FileDescription
apps/​server/​src/​orchestration-v2/​ThreadLiveEventCoalescer.tsRemoves three per-event tracing wrappers without changing coalescing logic.

Review effort: Balanced

MmacroscopeappBot4d ago
Synced from GitHub

Approvability

Verdict: Approved at 5f9fcff

Macroscope's review found this PR approvable — Removes per-event tracing from three live-coalescer helpers to reduce trace noise and overhead. Event delivery, buffering safeguards, and product defaults remain unchanged; the impact is limited to fine-grained diagnostics.

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

CcoderabbitaiBot4d 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 5b9731cedc06f310c394f48e66d50a614397c0aa and 5f9fcff9f171cddd8056dc25654b9cd3b154b4fc.

External image Files selected for processing: 1
  • apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts

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

Walkthrough

Three operations in ThreadLiveEventCoalescer now use Effect.fnUntraced. A comment notes that frequently invoked operations should not emit sub-millisecond trace spans. The implementations remain unchanged.

Changes: Coalescer tracing

Layer / File(s)Summary
Switch coalescer operations to untraced functions
apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts		cancelWindow, flushPending, and offerAll now use Effect.fnUntraced. A comment on cancelWindow notes the tracing frequency concern. Their implementations remain unchanged.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: External image Minimal · up to 5f9fc

No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: removing per-event tracing from the server live coalescer.
Description checkPassedThe description includes Problem, Change, Scope and approval, and Verification sections. It explains the trace-volume issue, the implementation, the focused scope, related issues, and specific test re…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes Problem, Change, Scope and approval, and Verification sections. It explains the trace-volume issue, the implementation, the focused scope, related issues, and specific test results. It does not provide a direct issue link or approval comment, but the focused-fix rationale makes the description mostly complete.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Mmaria4d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 5f9fcff9f171cddd8056dc25654b9cd3b154b4fc on a linux boat vm. reproduced on github/main (61c3794c32) first in a separate worktree, stopped that dev server, then ran the exact pr head. installed each checkout with vp i. used repository playwright with /usr/bin/google-chrome, created a local git project through the app, and selected codex gpt-6.1-sol. no mocks, replay, fixtures, or seeded data.

the same prompt ran git status --short and streamed 60 numbered sentences to COALESCER_COMPLETE. main's isolated trace contained 31 cancelWindow, 59 flushPending, and 47 offerAll spans, totaling 51,723 bytes. the pr emitted zero of all three, including the neighboring checks below. the subscription construction span remained (main: 3; pr: 7). these are observed trace counts, not a latency or long-term retention estimate.

all 60 sentences in the provider's final response were present in the client. reload restored the completed history. reload during another running command restored its running state and stop control. clicking stop produced codex's interrupted completion and the client's "stopped by user" state. a follow-up turn completed with AFTER_STOP_OK.

blacksmith passed the 12 existing coalescer tests and vp run check:changed HEAD^ (format, lint, knip, graphql, affected typechecks, 13 tests). fetched the parent into the shallow worker before the latter check. applicable github ci checks are green. no source changes needed.

short command transcript

main: real codex response completed

pr: same prompt completed with all 60 lines

pr: real codex streaming through the web client

pr: stopped turn followed by a successful new turn

Not verified

native ios/android and electron clients, macos/windows, relay/tunnel connections, other providers, multi-hour trace rotation, and saturation/error/overflow paths. this linux pass exercised the local web client and real codex app-server path.