[Detail Bug] Chat: Failed mid-stream responses leave orphaned user messages in conversation history

[detail-app]

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_8102879f-11df-4a3f-b382-78141d8fccb8

Introduced in 6f249e0 by @WilliamAGH on Sep 1, 2025

Summary

• Context: ChatController.java:119 and GuidedLearningController.java:241 add user messages to ChatMemoryService eagerly (before streaming starts). The assistant message is added in doOnComplete() only after successful streaming.
• Bug: When the stream fails (even after emitting partial chunks), doOnComplete is NOT called (Reactor guarantee), leaving the user message orphaned. The controller's onErrorResume handler sends an error event but does not roll back the eagerly-added user message.
• Actual vs. expected: Actual: after a mid-stream failure, the next user message results in duplicated user entries (e.g. [User, User, Assistant]) for what the user perceives as one exchange. Expected: if streaming fails, the user message added for that attempt should be rolled back (or the turn should otherwise remain consistent) so subsequent exchanges don’t include duplicate user messages.
• Impact: The next user message triggers another addUser call, resulting in [User, User, Assistant] in history for what the user perceives as a single exchange.

Code with Bug

// History snapshot taken BEFORE addUser
List<Message> history = new ArrayList<>(chatMemory.getHistory(sessionId));

// User message added EAGERLY - before streaming
chatMemory.addUser(sessionId, userQuery);  // <-- BUG 🔴 added before stream, not rolled back on error

return Flux.defer(() -> {
    // ... build prompt from history snapshot ...
    return openAIStreamingService.streamResponse(...)
            .flatMapMany(streamingResult -> {
                // ... concatenate events ...
            })
            .doOnComplete(() -> {
                // Assistant added ONLY on success
                // CRITICAL: doOnComplete is NOT called if stream errors!
                chatMemory.addAssistant(sessionId, fullResponse.toString());
            });
})
.onErrorResume(error -> {
    // Sends error event to client
    // DOES NOT roll back the user message added above
    return sseSupport.streamErrorEvent(errorDetail, diagnostics, retryable);
});

Explanation

• The controllers add the user message to chat memory before the streaming Flux begins.
• The assistant message is only appended in doOnComplete(), which Reactor does not invoke when a stream terminates with onError.
• Therefore, any mid-stream error leaves chat memory in a partially-updated state: the user message remains but the assistant message never gets added.
• Subsequent user input adds another user message, producing duplicated user turns in history.

Codebase Inconsistency

• OpenAIStreamingService intentionally skips provider fallback once any chunk has been emitted, causing mid-stream failures to propagate to the controller:

.onErrorResume(streamingFailure -> {
    if (!attemptContext.hasNextProvider()
            || emittedTextChunk.get()  // <-- BUG 🔴 mid-stream failures bypass fallback; error reaches controller
            || !providerRoutingService.isStreamingFallbackEligible(streamingFailure)) {
        return Flux.error(streamingFailure);
    }
    // ... fallback logic ...
});

• The frontend also does not auto-retry once any chunk has streamed:

if (hasStreamedAnyChunk) {
    return false;  // NO automatic retry
}

Failing Test

src/test/java/com/williamcallahan/javachat/web/ChatControllerReactiveBugTest.java and src/test/java/com/williamcallahan/javachat/service/ChatMemoryServiceDuplicateBugTest.java contain 8 tests total; 4 currently fail because the orphaned user message remains after a partial-stream error.

Test Output:

ChatControllerReactiveBugTest > FAILED TEST: After partial stream failure, history should be empty FAILED
ChatControllerReactiveBugTest > FAILED TEST: After failure + retry, history should be [User, Assistant] FAILED
ChatMemoryServiceDuplicateBugTest > FAILED TEST: After failed stream + retry, history should be [User, Assistant] FAILED
ChatMemoryServiceDuplicateBugTest > FAILED TEST: Multiple retries should still produce [User, Assistant] FAILED

8 tests completed, 4 failed

Recommended Fix

Add a rollback method to ChatMemoryService and invoke it in the controllers’ onErrorResume so any eagerly-added user message is removed when streaming fails.

// In ChatMemoryService.java:
public void removeLastUserMessage(String sessionId) {
    Objects.requireNonNull(sessionId, REQUIRE_SESSION_ID);
    SessionConversation sessionConversation = sessionConversations.get(sessionId);
    if (sessionConversation != null) {
        sessionConversation.removeLastUserMessage();
    }
}

// In SessionConversation:
synchronized void removeLastUserMessage() {
    if (!historyMessages.isEmpty() && historyMessages.getLast() instanceof UserMessage) {
        historyMessages.removeLast();
        turnHistory.removeLast();
    }
}

// In ChatController.java:
.onErrorResume(error -> {
    chatMemory.removeLastUserMessage(sessionId);  // ROLLBACK
    // ... existing error handling ...
});

History

This bug was introduced in commit 6f249e0. The initial commit established the pattern of eagerly adding the user message to ChatMemoryService before streaming starts and adding the assistant message only in doOnComplete(). Subsequent commits (183a3fe, a5765a4) added onErrorResume for graceful error reporting to clients but did not add rollback for the eagerly-added user message.