Skip to content

fix(empty-completion): keep a discarded attempt's reasoning out of the retry's item #7331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vadymhimself wants to merge 1 commit into from
Closed

fix(empty-completion): keep a discarded attempt's reasoning out of the retry's item #7331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/server/responses/empty-completion-guard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ export async function* guardEmptyCompletionEventStream(
heldBytes = 0;
return released;
};
// Held events from a discarded attempt are released with the retry's output, so a boundary
// keeps the retry's reasoning from being appended to the discarded attempt's reasoning item.
const markRetryBoundary = () => {
if (held.length > 0 && held.at(-1)?.type !== "assistant_boundary") held.push({ type: "assistant_boundary" });
};

while (true) {
let terminalSeen = false;
Expand Down Expand Up @@ -277,6 +282,7 @@ export async function* guardEmptyCompletionEventStream(
if (retries < maxRetries) {
// Suppress the terminal: the client must never see a completed event
// for a turn that produced nothing. Retry the identical turn.
markRetryBoundary();
retries += 1;
try {
source = await options.continuation();
Expand All @@ -298,6 +304,7 @@ export async function* guardEmptyCompletionEventStream(
// malformed function call). Nothing actionable reached the client, so this is the same
// replay-safe shape as an empty completion: retry the identical turn once.
usage = mergeUsage(usage, event.usage);
markRetryBoundary();
retries += 1;
try {
source = await options.continuation();
Expand Down Expand Up @@ -349,6 +356,7 @@ export async function* guardEmptyCompletionEventStream(
// actionable reached the client. Retry once, then surface a stated error
// instead of letting the bridge reduce the turn to adapter_eof.
if (!sawContent && !passthrough && retries < maxRetries) {
markRetryBoundary();
retries += 1;
try {
source = await options.continuation();
Expand Down
23 changes: 22 additions & 1 deletion tests/responses/empty-completion-guard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isContentEvent,
observeEmptyCompletion,
} from "../../src/server/responses/empty-completion-guard";
import { bridgeToResponsesSSE } from "../../src/bridge/sse";
import type { AdapterEvent } from "../../src/types";

function collect(source: AsyncIterable<AdapterEvent>): Promise<AdapterEvent[]> {
Expand Down Expand Up @@ -115,12 +116,31 @@ describe("empty-completion guard retry", () => {
expect(withoutHeartbeats(events)).toEqual([
{ type: "thinking_delta", thinking: "first attempt" },
{ type: "reasoning_raw_delta", text: "raw" },
{ type: "assistant_boundary" },
{ type: "thinking_delta", thinking: "second attempt" },
{ type: "text_delta", text: "finally an answer" },
{ type: "done", usage: { inputTokens: 30, outputTokens: 5, totalTokens: 35 } },
]);
});

test("the discarded attempt's reasoning stays out of the retry's reasoning item", async () => {
const guarded = guardEmptyCompletionEventStream({
firstEvents: eventsOf({ type: "thinking_delta", thinking: "discarded" }, { type: "done" }),
continuation: () => eventsOf(
{ type: "thinking_delta", thinking: "retry" },
{ type: "text_delta", text: "answer" },
{ type: "done" },
),
});
const sse = await new Response(bridgeToResponsesSSE(guarded, "fixture-model")).text();
const reasoning = sse.split("\n")
.filter(line => line.startsWith("data: {"))
.map(line => JSON.parse(line.slice(6)) as { type: string; item?: { type: string; summary?: { text: string }[] } })
.filter(event => event.type === "response.output_item.done" && event.item?.type === "reasoning")
.map(event => event.item!.summary!.map(part => part.text).join(""));
expect(reasoning).toEqual(["discarded", "retry"]);
});

test("usage is merged across attempts", async () => {
const events = await collect(guardEmptyCompletionEventStream({
firstEvents: eventsOf(
Expand Down Expand Up @@ -321,7 +341,7 @@ describe("empty-completion guard retry", () => {

expect(continuations).toBe(1);
expect(events.map(event => event.type)).toEqual([
"heartbeat", "heartbeat", "thinking_delta", "text_delta", "done",
"heartbeat", "heartbeat", "thinking_delta", "assistant_boundary", "text_delta", "done",
]);
});

Expand All @@ -341,6 +361,7 @@ describe("empty-completion guard retry", () => {
expect(continuations).toBe(1);
expect(withoutHeartbeats(events)).toEqual([
{ type: "thinking_delta", thinking: "..." },
{ type: "assistant_boundary" },
{ type: "text_delta", text: "recovered" },
{ type: "done" },
]);
Expand Down
Loading