Skip to content

Restore lost streaming reveal-emit + add e2e Phase 15 cadence test - #55

Merged
lukemarsden merged 3 commits into
mainfrom
fix/restore-streaming-reveal-emit
May 11, 2026
Merged

Restore lost streaming reveal-emit + add e2e Phase 15 cadence test#55
lukemarsden merged 3 commits into
mainfrom
fix/restore-streaming-reveal-emit

Conversation

@lukemarsden

Copy link
Copy Markdown

Summary

Cherry-picks edbbb5a5e8 ("Emit EntryUpdated after streaming-reveal drain so WS sync sees fresh content") back onto main, and adds an e2e regression test that asserts streaming patches arrive incrementally so this fix doesn't get silently dropped again.

Why this is needed

User report: streaming content appears in real time in Zed's UI but takes a long time to show up in Helix.

The fix originally landed via Helix PR helixml/helix#2296 on May 6 (Helix bumped ZED_COMMIT to edbbb5a5e8). On May 8 the cancel_current_turn work merged and bumped ZED_COMMIT to fe8f4f4e3f, which forked off a Zed branch that does NOT include edbbb5a5e8 — silently losing the fix. The May 11 upstream merge (#54 / helixml/helix#2403) didn't pick it back up either.

Without the fix:

  • LLM chunk arrives → push_assistant_content_block_with_indent emits EntryUpdated(idx) and stashes the text in streaming_text_buffer.pending.
  • external_websocket_sync reads markdown.source() via content_only(cx) — but the new text is still in pending, so it reads stale content.
  • The 16ms start_streaming_reveal task drains pending into the markdown entity but does NOT emit EntryUpdated.
  • WS sync only learns about freshly drained content the next time push_chunk fires EntryUpdated. Between chunks the text sits invisible.

In a controlled e2e against zed-agent + claude-sonnet-4-5 streaming a ~2.8 KB prose response, this halves the number of message_added events Helix observes (≈58 with the fix, ≈31 without).

Changes

crates/acp_thread/src/acp_thread.rs (1 file, 12 inserts) — cherry-pick of edbbb5a5e8. Adds cx.emit(AcpThreadEvent::EntryUpdated(entries_len - 1)) after the drain in start_streaming_reveal.

crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go + run_e2e.sh — new Phase 15 ("Streaming patches arrive incrementally"). Sends a 400-word plain-prose prompt, records (timestamp, content_length) for every assistant message_added on the phase-15 thread, and asserts:

  • >= 40 message_added events arrive (calibrated ~58 with fix vs ~31 without — comfortably between with margin for LLM/throttle variance).
  • Longest gap between consecutive message_added events <= 20s.
  • >= 30% of the final content is observed by the streaming midpoint.

Bumps the test-server timeout to 360s/agent and the run_e2e.sh default to 300s/agent to accommodate the extra ~30s for Phase 15.

Verification

Verified locally: rebuilt Zed with and without the cherry-pick and ran E2E_AGENTS=zed-agent ./run_docker_e2e.sh against each.

Build Phase 15 result Samples Max gap % by midpoint
With fix ✅ PASSED 59 446 ms 43%
Without fix ❌ FAILED ("only 31 ... need >= 40") 31

Companion Helix PR

helixml/helix# bumps ZED_COMMIT to this branch's head.

Merge order (per helixml/helix CLAUDE.md)

Merge this Zed PR first, then the Helix PR.

🤖 Generated with Claude Code

lukemarsden and others added 3 commits May 11, 2026 14:02
…content

Spec-Ref: helix-specs@b6c43b8a9:001895_i-thought-we-flushed
The existing e2e suite only asserts on the final response after
message_completed, by which point flush_streaming_text has run and
content is fully drained. That left a regression class invisible:
streaming patches from external_websocket_sync that show up in stale
chunks (or only at the Stopped re-send) instead of arriving live.

Phase 15 sends a long prose prompt with no tool calls and records
(timestamp, content_length) for every assistant message_added on the
phase-15 thread between thread_created and message_completed. Three
asserts run at validation:

  1. >= 5 assistant message_added events (catches "almost no streaming
     events arrive — content shows up in a single end-of-turn burst").
  2. Longest gap between consecutive message_added events <= 20s
     (catches multi-minute pauses while content sits in the streaming
     buffer with no EntryUpdated re-emit).
  3. >= 30% of the final content length is observed by the midpoint of
     the streaming duration (catches "all content arrives at the end").

Bumps test-server timeout to 360s/agent and the run_e2e.sh default
to 300s/agent to accommodate the extra ~30–60s for Phase 15. Also
bumps the validateStore expected counts from 4→5 sessions and 7→8
completed interactions per round to account for the new phase.

This phase fails reliably without the companion fix that re-adds
`cx.emit(AcpThreadEvent::EntryUpdated(entries_len - 1))` after the
streaming-reveal drain in acp_thread.rs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Initial loose threshold (>=5 samples) was too permissive: in a
controlled e2e the without-fix path still yields ~31 message_added
events for a ~2.5 KB streaming response (one per LLM chunk), whereas
the with-fix path yields ~58 events (chunk-arrival emissions PLUS
streaming-reveal drain emissions). Both passed minSamples=5.

Calibrated against zed-agent + claude-sonnet-4-5 (verified locally):

  with cherry-pick:    59 samples  ✅ (passes minSamples=40)
  without cherry-pick: 31 samples  ❌ FAIL: only 31 ... need >= 40

Bumping the threshold to 40 sits comfortably between the two with
margin for LLM/throttle variance. If a future legitimate change to
STREAMING_THROTTLE_INTERVAL or chunk batching shifts the baseline,
the calibration comment in the code points at where to recheck.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@lukemarsden
lukemarsden merged commit cd4e279 into main May 11, 2026
23 of 24 checks passed
lukemarsden added a commit to helixml/helix that referenced this pull request May 11, 2026
Was pointing at the pre-merge branch tip (41a62b2e61). Now that
helixml/zed#55 merged, update to the merge commit on main
(cd4e279d80) so CI builds against helixml/zed/main rather than a
deleted branch ref.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant