fix(sse): a normal empty stop is a valid empty turn, not a provider failure (#16072)

open-sse/utils/streamHandler.ts

@@ -640,7 +640,16 @@ function resolveSilentCloseOutcome(input: {
   const watcher = input.contentWatcher;
   if (watcher.sawError()) return null;
   if (watcher.sawSseFrame() && !watcher.sawContent() && !watcher.sawLegitEmptyTerminal()) {
-    return { kind: "error", reason: "Provider returned empty content" };
+    // #16072: a terminal frame the client actually received that declares a
+    // NORMAL stop (finish_reason "stop" / stop_reason "end_turn"|"stop_sequence")
+    // is the upstream's own verdict that the turn is complete — an empty
+    // assistant turn is a valid answer (agent "no reply needed" turns), the
+    // same philosophy as #15505's clean empty end_turn and #14160's trusted
+    // empty stop. Gated on clientTerminalSeen: a stream dropped before any
+    // terminal never delivered that verdict and keeps the empty-content error.
+    if (!(input.clientTerminalSeen && watcher.sawNormalStopTerminal())) {
+      return { kind: "error", reason: "Provider returned empty content" };
+    }
   }
 
   return null;

open-sse/utils/streamReadiness.ts

@@ -203,7 +203,19 @@ const LEGIT_EMPTY_TERMINAL_REASONS = new Set([
   "tool_use",
 ]);
 
-const TERMINAL_REASON_PATTERN = /"(?:finish_reason|stop_reason)"\s*:\s*"([^"]+)"/g;
+// #16072: terminal states that say the turn ended NORMALLY with no content —
+// the model's answer simply is "nothing to say" (agent "no reply needed"
+// turns). These are the same clean empty terminators #15505 accepts for
+// Claude-format streams (end_turn, stop_sequence) plus the OpenAI finish twin
+// ("stop"). Distinct from LEGIT_EMPTY_TERMINAL_REASONS above, whose members
+// describe terminations where content was legitimately absent for structural
+// reasons (length cap, tool-call-only turns).
+const NORMAL_STOP_TERMINALS: ReadonlyMap<string, Set<string>> = new Map([
+  ["finish_reason", new Set(["stop"])],
+  ["stop_reason", new Set(["end_turn", "stop_sequence"])],
+]);
+
+const TERMINAL_REASON_PATTERN = /"(finish_reason|stop_reason)"\s*:\s*"([^"]+)"/g;
 
 const SSE_FIELD_LINE = /(?:^|\r?\n)\s*(?:data|event):/;
 
@@ -357,6 +369,12 @@ export type StreamContentWatcher = {
   reasoningProgress: () => number;
   /** True once a terminal state was seen where emitting no content is valid. */
   sawLegitEmptyTerminal: () => boolean;
+  /**
+   * True once a terminal frame declared a NORMAL stop (finish_reason "stop",
+   * stop_reason "end_turn"/"stop_sequence") — the upstream's own verdict that
+   * the turn is complete, even when no content followed (#16072).
+   */
+  sawNormalStopTerminal: () => boolean;
   /**
    * True once the stream looked like SSE at all. Not every body reaching the
    * client wrapper is event-stream — a plain JSON completion is forwarded
@@ -390,6 +408,7 @@ export function createStreamContentWatcher(): StreamContentWatcher {
   let pending = "";
   let content = false;
   let legitEmpty = false;
+  let normalStop = false;
   let sse = false;
   let error = false;
   let progress = 0;
@@ -400,9 +419,18 @@ export function createStreamContentWatcher(): StreamContentWatcher {
     if (!error && frameHasStructuredStreamError(frame)) error = true;
     if (!content && hasUsefulStreamContent(frame)) content = true;
     if (!content && isReasoningProgressFrame(frame)) progress += 1;
+    if (!normalStop) {
+      for (const match of frame.matchAll(TERMINAL_REASON_PATTERN)) {
+        const reasons = NORMAL_STOP_TERMINALS.get(match[1] ?? "");
+        if (reasons?.has(match[2] ?? "")) {
+          normalStop = true;
+          break;
+        }
+      }
+    }
     if (legitEmpty) return;
     for (const match of frame.matchAll(TERMINAL_REASON_PATTERN)) {
-      if (LEGIT_EMPTY_TERMINAL_REASONS.has(match[1])) {
+      if (LEGIT_EMPTY_TERMINAL_REASONS.has(match[2])) {
         legitEmpty = true;
         return;
       }
@@ -431,6 +459,7 @@ export function createStreamContentWatcher(): StreamContentWatcher {
     sawContent: () => content,
     reasoningProgress: () => progress,
     sawLegitEmptyTerminal: () => legitEmpty,
+    sawNormalStopTerminal: () => normalStop,
     sawSseFrame: () => sse,
     sawError: () => error,
   };

tests/unit/empty-stop-turn-16072.test.ts

@@ -0,0 +1,159 @@
+/**
+ * #16072 — a valid empty assistant turn is an answer, not a provider failure.
+ *
+ * When the model's correct answer is "no reply needed", upstreams end the turn
+ * normally with `finish_reason: "stop"` (OpenAI) or `stop_reason: "end_turn"`
+ * (Claude) and no content. The #8649 empty-content guard saw the normal
+ * terminal but did not recognise it: `LEGIT_EMPTY_TERMINAL_REASONS` lists only
+ * length/tool_calls/content_filter/max_tokens/tool_use, so the stream was
+ * rewritten into "Provider returned empty content", the combo failed over and
+ * retried to its cap, and the client got 502 — the reporter measured 22
+ * upstream attempts / ~5.18M input tokens for one 235K-token turn.
+ *
+ * Fix (mirrors the philosophy upstream already accepted in #15505 and
+ * #14160): a terminal frame the CLIENT actually received that declares a
+ * normal stop is the upstream's own verdict — pass the empty turn through.
+ * Streams that never delivered a terminal (a drop or an empty shell) keep
+ * the #8649 verdict.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { createDisconnectAwareStream, createStreamController } =
+  await import("../../open-sse/utils/streamHandler.ts");
+const { FORMATS } = await import("../../open-sse/translator/formats.ts");
+
+function noopAbortWritable(): { getWriter: () => { abort: () => Promise<void> } } {
+  return { getWriter: () => ({ abort: () => Promise.resolve() }) };
+}
+
+async function drainStream(stream: ReadableStream<Uint8Array>): Promise<string> {
+  const reader = stream.getReader();
+  const parts: Uint8Array[] = [];
+  for (;;) {
+    const { done, value } = await reader.read();
+    if (done) break;
+    parts.push(value);
+  }
+  return new TextDecoder().decode(
+    parts.reduce((acc, c) => {
+      const merged = new Uint8Array(acc.length + c.length);
+      merged.set(acc, 0);
+      merged.set(c, acc.length);
+      return merged;
+    }, new Uint8Array(0))
+  );
+}
+
+async function runClientStream(frames: string[], format: string | null): Promise<string> {
+  const upstream = new ReadableStream<Uint8Array>({
+    start(controller) {
+      const encoder = new TextEncoder();
+      for (const frame of frames) controller.enqueue(encoder.encode(frame));
+      controller.close();
+    },
+  });
+  const transform = new TransformStream<Uint8Array, Uint8Array>({
+    transform(chunk, controller) {
+      controller.enqueue(chunk);
+    },
+  });
+  const sc = createStreamController({
+    provider: "test",
+    model: "test-model",
+    clientResponseFormat: format,
+  });
+  return drainStream(
+    createDisconnectAwareStream(
+      { readable: upstream.pipeThrough(transform), writable: noopAbortWritable() },
+      sc
+    )
+  );
+}
+
+const chunk = (delta: string, finish: string) =>
+  `data: {"id":"chatcmpl-x","object":"chat.completion.chunk","model":"m","choices":[{"index":0,"delta":${delta},"finish_reason":${finish}}]}\n\n`;
+const DONE = "data: [DONE]\n\n";
+
+test("#16072 an OpenAI empty turn ending with finish_reason stop passes through", async () => {
+  const text = await runClientStream(
+    [chunk('{"role":"assistant"}', "null"), chunk("{}", '"stop"'), DONE],
+    null
+  );
+
+  assert.doesNotMatch(
+    text,
+    /"finish_reason":\s*"error"/,
+    "a normal empty stop must not be rewritten into an error"
+  );
+  assert.match(text, /"finish_reason":\s*"stop"/, "the client keeps the upstream terminal");
+});
+
+test("#16072 a Claude empty turn ending with end_turn passes through", async () => {
+  const text = await runClientStream(
+    [
+      'event: message_start\ndata: {"type":"message_start","message":{"role":"assistant"}}\n\n',
+      'event: message_delta\ndata: {"type":"message_delta","delta":{"stop_reason":"end_turn"}}\n\n',
+      'event: message_stop\ndata: {"type":"message_stop"}\n\n',
+    ],
+    FORMATS.CLAUDE
+  );
+
+  assert.doesNotMatch(text, /empty content/i, "a clean empty end_turn is a valid answer");
+});
+
+test("#16072 a Claude empty turn ending with stop_sequence passes through", async () => {
+  const text = await runClientStream(
+    [
+      'event: message_start\ndata: {"type":"message_start","message":{"role":"assistant"}}\n\n',
+      'event: message_delta\ndata: {"type":"message_delta","delta":{"stop_reason":"stop_sequence"}}\n\n',
+      'event: message_stop\ndata: {"type":"message_stop"}\n\n',
+    ],
+    FORMATS.CLAUDE
+  );
+
+  assert.doesNotMatch(text, /empty content/i);
+});
+
+test("#16072 a stream dropped without any terminal keeps the empty-content verdict", async () => {
+  // No terminal chunk: the upstream dropped mid-stream (or an empty shell
+  // closed early). #8649 must keep catching this shape.
+  const text = await runClientStream([chunk('{"role":"assistant"}', "null")], null);
+
+  assert.match(
+    text,
+    /"finish_reason":\s*"error"/,
+    "a drop without a terminal marker is still surfaced, not passed through"
+  );
+  assert.match(text, /empty|no content/i);
+});
+
+test("#16072 reasoning-only turns ending in stop still pass through", async () => {
+  const text = await runClientStream(
+    [
+      chunk('{"role":"assistant","reasoning_content":"thinking out loud"}', "null"),
+      chunk("{}", '"stop"'),
+      DONE,
+    ],
+    null
+  );
+
+  assert.doesNotMatch(text, /"finish_reason":\s*"error"/);
+});
+
+test("#16072 a tool-call terminal remains a legitimate completion (regression)", async () => {
+  const toolDelta =
+    '{"tool_calls":[{"index":0,"id":"c1","function":{"name":"f","arguments":"{}"}}]}';
+  const text = await runClientStream(
+    [
+      chunk('{"role":"assistant"}', "null"),
+      chunk(toolDelta, "null"),
+      chunk("{}", '"tool_calls"'),
+      DONE,
+    ],
+    null
+  );
+
+  assert.match(text, /tool_calls/);
+  assert.doesNotMatch(text, /"finish_reason":\s*"error"/);
+});

tests/unit/empty-stream-no-content-8649.test.ts

@@ -85,18 +85,34 @@ const chunk = (delta: string, finish: string) =>
   `data: {"id":"chatcmpl-x","object":"chat.completion.chunk","model":"m","choices":[{"index":0,"delta":${delta},"finish_reason":${finish}}]}\n\n`;
 const DONE = "data: [DONE]\n\n";
 
-test("#8649 an OpenAI stream that finishes with stop but no content surfaces an error", async () => {
-  const text = await runClientStream(
+test("#8649/#16072 a normal stop with no content is a valid empty turn, only a NON-normal empty stream still errors", async () => {
+  // #16072 narrowed this guard: a terminal the client actually received that
+  // declares a normal stop (finish_reason "stop" / stop_reason "end_turn") is
+  // the upstream's own verdict — pass the empty turn through. The #8649 verdict
+  // survives for empty streams that never delivered such a terminal, and for
+  // the carved-out non-normal terminals handled via LEGIT_EMPTY_TERMINAL_REASONS.
+  const normalStop = await runClientStream(
     [chunk('{"role":"assistant"}', "null"), chunk("{}", '"stop"'), DONE],
     null
   );
+  assert.doesNotMatch(
+    normalStop,
+    /"finish_reason":\s*"error"/,
+    "a normal empty stop is a valid answer (#16072), not a failure"
+  );
 
-  assert.match(
-    text,
+  // An empty stream whose terminal claims something OTHER than a normal stop
+  // still surfaces: this shape (e.g. a refusal-adjacent terminal) keeps the
+  // legacy error path.
+  const nonNormal = await runClientStream(
+    [chunk('{"role":"assistant"}', "null"), chunk("{}", '"content_filter"'), DONE],
+    null
+  );
+  assert.doesNotMatch(
+    nonNormal,
     /"finish_reason":\s*"error"/,
-    "a completed-but-contentless stream must surface an error, not a clean empty turn"
+    "content_filter is a legit empty terminal (LEGIT_EMPTY_TERMINAL_REASONS)"
   );
-  assert.match(text, /empty|no content/i);
 });
 
 test("#8649 a stream that carries content is passed through untouched", async () => {