fix(ui): drain active thinking entry on MessageComplete (#861 RC3)
Closes the order-of-events race in #861 root cause 3: when the engine bursts events, the dispatch loop can pull `MessageComplete` off the channel ahead of `ThinkingComplete`. Today's `MessageComplete` reads `app.last_reasoning.take()` to attach the reasoning block to the assistant message in `api_messages`. If `ThinkingComplete` has not fired yet, `last_reasoning` is `None` and the thinking content is dropped — DeepSeek V4 then returns HTTP 400 on the next turn because it requires `reasoning_content` replay for assistant messages that carry tool calls. Adds a defensive head-of-handler drain in `MessageComplete`: when `streaming_thinking_active_entry.is_some()`, finalize the active thinking entry and stash the reasoning buffer into `last_reasoning` before the existing body runs. The drain is a no-op in the normal case where `ThinkingComplete` arrived first (the entry has already been cleared), so this branch is order-independent. Adds `message_complete_drain_preserves_thinking_when_thinking_complete_lost` which exercises the head-of-handler invariant directly: with a thinking entry still active and `last_reasoning` empty, the drain must move the buffer into `last_reasoning` before downstream reads. Refs #861 (RC3). RC1 and RC2 are already addressed by the existing `finalize_current_streaming_thinking` plumbing in `apply_engine_error_to_app` and `start_streaming_thinking_block`; RC4 (streaming-time truncation affordance) is left out of this PR to keep the scope on the data-loss path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -698,6 +698,24 @@ async fn run_event_loop(
|
||||
}
|
||||
}
|
||||
EngineEvent::MessageComplete { .. } => {
|
||||
// #861 RC3: defensive drain of a still-active thinking
|
||||
// entry. Normally `ThinkingComplete` arrives first and
|
||||
// populates `last_reasoning` before we get here, but
|
||||
// when the engine bursts events the channel can
|
||||
// deliver `MessageComplete` first, in which case
|
||||
// `last_reasoning.take()` below would be `None` and
|
||||
// the thinking block would be dropped from
|
||||
// `api_messages` — causing a DeepSeek HTTP 400 on the
|
||||
// next turn (V4 thinking-mode requires
|
||||
// `reasoning_content` replay). Inline-finalize the
|
||||
// thinking entry here so this branch is order-
|
||||
// independent.
|
||||
if app.streaming_thinking_active_entry.is_some() {
|
||||
if finalize_current_streaming_thinking(app) {
|
||||
transcript_batch_updated = true;
|
||||
}
|
||||
stash_reasoning_buffer_into_last_reasoning(app);
|
||||
}
|
||||
if let Some(index) = app.streaming_message_index.take() {
|
||||
let remaining = app.streaming_state.finalize_block_text(0);
|
||||
if !remaining.is_empty() {
|
||||
|
||||
@@ -3860,6 +3860,57 @@ fn engine_error_finalizes_active_thinking_block() {
|
||||
assert!(app.streaming_thinking_active_entry.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn message_complete_drain_preserves_thinking_when_thinking_complete_lost() {
|
||||
// #861 RC3: when the engine bursts events, `MessageComplete` can be
|
||||
// dispatched ahead of `ThinkingComplete`. Without the defensive drain,
|
||||
// `app.last_reasoning` would be `None` at `last_reasoning.take()` time
|
||||
// and the thinking block would be dropped from `api_messages`,
|
||||
// causing a DeepSeek HTTP 400 on the next turn (V4 thinking-mode
|
||||
// requires `reasoning_content` replay).
|
||||
//
|
||||
// This test exercises the head-of-handler drain in isolation: with a
|
||||
// thinking entry still active and `last_reasoning` empty, the drain
|
||||
// must transfer `reasoning_buffer` into `last_reasoning` before the
|
||||
// remainder of `MessageComplete` reads it.
|
||||
let mut app = create_test_app();
|
||||
|
||||
let _ = ensure_streaming_thinking_active_entry(&mut app);
|
||||
app.thinking_started_at = Some(Instant::now());
|
||||
app.streaming_state.start_thinking(0, None);
|
||||
app.streaming_state.push_content(0, "deep reasoning text");
|
||||
let _ = app.streaming_state.commit_text(0);
|
||||
app.reasoning_buffer.push_str("deep reasoning text");
|
||||
|
||||
assert!(
|
||||
app.last_reasoning.is_none(),
|
||||
"precondition: ThinkingComplete has NOT fired"
|
||||
);
|
||||
assert!(
|
||||
app.streaming_thinking_active_entry.is_some(),
|
||||
"precondition: thinking entry is still active"
|
||||
);
|
||||
|
||||
// Mirror the head of `EngineEvent::MessageComplete` — the new defensive
|
||||
// drain installed by the #861 RC3 fix.
|
||||
if app.streaming_thinking_active_entry.is_some() {
|
||||
let _ = finalize_current_streaming_thinking(&mut app);
|
||||
stash_reasoning_buffer_into_last_reasoning(&mut app);
|
||||
}
|
||||
|
||||
assert!(
|
||||
app.last_reasoning
|
||||
.as_deref()
|
||||
.is_some_and(|s| s.contains("deep reasoning text")),
|
||||
"defensive drain must move reasoning into last_reasoning so the\
|
||||
downstream `last_reasoning.take()` produces a Thinking block"
|
||||
);
|
||||
assert!(
|
||||
app.streaming_thinking_active_entry.is_none(),
|
||||
"thinking entry must be cleared after the drain"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn second_thinking_block_appends_new_entry_in_same_active_cell() {
|
||||
// Real V4 turns can emit Thinking → Tool → Thinking → Tool before any
|
||||
|
||||
Reference in New Issue
Block a user