Skip to content

Commit fe7c3ad

Browse files
steipetesteipete
committed
fix(openai): drop orphan phase-tagged response ids
1 parent b60fd8b commit fe7c3ad

5 files changed

Lines changed: 55 additions & 28 deletions

File tree

‎src/agents/openai-responses-replay.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
export function resolveReplayableResponsesMessageId(params: {
22
replayResponsesItemIds: boolean;
33
textSignatureId?: string;
4-
textSignaturePhase?: "commentary" | "final_answer";
54
fallbackId: string;
65
fallbackOrdinal: number;
76
previousReplayItemWasReasoning: boolean;
@@ -14,8 +13,5 @@ export function resolveReplayableResponsesMessageId(params: {
1413
? params.fallbackId
1514
: `${params.fallbackId}_${params.fallbackOrdinal}`;
1615
}
17-
if (params.textSignaturePhase) {
18-
return params.textSignatureId;
19-
}
2016
return params.previousReplayItemWasReasoning ? params.textSignatureId : undefined;
2117
}

‎src/agents/openai-responses.reasoning-replay.test.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,11 @@ describe("openai-responses reasoning replay", () => {
244244
resolveReplayableResponsesMessageId({
245245
replayResponsesItemIds: true,
246246
textSignatureId: "msg_commentary",
247-
textSignaturePhase: "commentary",
248247
fallbackId: "msg_0",
248+
fallbackOrdinal: 0,
249249
previousReplayItemWasReasoning: false,
250250
}),
251-
).toBe("msg_commentary");
251+
).toBeUndefined();
252252

253253
expect(
254254
resolveReplayableResponsesMessageId({
@@ -270,7 +270,7 @@ describe("openai-responses reasoning replay", () => {
270270
});
271271

272272
it.each(["commentary", "final_answer"] as const)(
273-
"replays assistant message phase metadata for %s",
273+
"replays assistant message id and phase metadata for %s when paired with reasoning",
274274
async (phase) => {
275275
const assistantWithText = buildAssistantMessage({
276276
stopReason: "stop",
@@ -301,6 +301,34 @@ describe("openai-responses reasoning replay", () => {
301301
},
302302
);
303303

304+
it.each(["commentary", "final_answer"] as const)(
305+
"omits phase-tagged assistant message id for %s when reasoning is absent",
306+
async (phase) => {
307+
const assistantWithText = buildAssistantMessage({
308+
stopReason: "stop",
309+
content: [
310+
{
311+
type: "text",
312+
text: "hello",
313+
textSignature: JSON.stringify({ v: 1, id: `msg_${phase}`, phase }),
314+
},
315+
],
316+
});
317+
318+
const { input } = await runAbortedOpenAIResponsesStream({
319+
messages: [
320+
{ role: "user", content: "Hi", timestamp: Date.now() },
321+
assistantWithText,
322+
{ role: "user", content: "Ok", timestamp: Date.now() },
323+
],
324+
});
325+
326+
const [replayedMessage] = extractInputMessages(input);
327+
expect(replayedMessage).toMatchObject({ phase });
328+
expect(replayedMessage).not.toHaveProperty("id");
329+
},
330+
);
331+
304332
it("replays a synthetic id while preserving phase for id-less text signatures", async () => {
305333
// After a reasoning drop the sanitizer keeps the phase but removes the msg_*
306334
// id. The conversion must then emit a unique synthetic id per text block AND

‎src/agents/openai-transport-stream.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ import {
5757
applyOpenAIResponsesPayloadPolicy,
5858
resolveOpenAIResponsesPayloadPolicy,
5959
} from "./openai-responses-payload-policy.js";
60-
import { resolveOpenAIStrictToolSetting } from "./openai-strict-tool-setting.js";
6160
import { resolveReplayableResponsesMessageId } from "./openai-responses-replay.js";
61+
import { resolveOpenAIStrictToolSetting } from "./openai-strict-tool-setting.js";
6262
import {
6363
findOpenAIStrictToolSchemaDiagnostics,
6464
normalizeOpenAIStrictToolParameters,
@@ -1155,7 +1155,6 @@ function convertResponsesMessages(
11551155
let msgId = resolveReplayableResponsesMessageId({
11561156
replayResponsesItemIds: shouldReplayResponsesItemIds,
11571157
textSignatureId: textSignature?.id,
1158-
textSignaturePhase: textSignature?.phase,
11591158
fallbackId: `msg_${msgIndex}`,
11601159
fallbackOrdinal: textFallbackOrdinal,
11611160
previousReplayItemWasReasoning,

‎src/llm/providers/openai-responses-shared.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe("convertResponsesTools", () => {
127127
describe("convertResponsesMessages", () => {
128128
const allowedToolCallProviders = new Set(["openai", "openai-codex", "opencode"]);
129129

130-
it("preserves phase-tagged assistant replay ids without reasoning", () => {
130+
it("omits phase-tagged assistant replay ids without reasoning", () => {
131131
const input = convertResponsesMessages(
132132
nativeOpenAIModel,
133133
{
@@ -177,9 +177,19 @@ describe("convertResponsesMessages", () => {
177177
item.phase === "commentary",
178178
),
179179
).toMatchObject({
180-
id: "msg_commentary",
181180
phase: "commentary",
182181
});
182+
expect(
183+
input.find(
184+
(item) =>
185+
item &&
186+
typeof item === "object" &&
187+
"role" in item &&
188+
item.role === "assistant" &&
189+
"phase" in item &&
190+
item.phase === "commentary",
191+
),
192+
).not.toHaveProperty("id");
183193
});
184194

185195
it("omits raw signed assistant ids when the paired reasoning item is absent", () => {

‎src/llm/providers/openai-responses-shared.ts‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,14 @@ function parseTextSignature(
8080

8181
function resolveReplayableResponsesMessageId(params: {
8282
textSignatureId?: string;
83-
textSignaturePhase?: TextSignatureV1["phase"];
8483
fallbackId: string;
84+
fallbackOrdinal: number;
8585
previousReplayItemWasReasoning: boolean;
8686
}): string | undefined {
8787
if (!params.textSignatureId) {
88-
return params.fallbackId;
89-
}
90-
if (params.textSignaturePhase) {
91-
return params.textSignatureId;
88+
return params.fallbackOrdinal === 0
89+
? params.fallbackId
90+
: `${params.fallbackId}_${params.fallbackOrdinal}`;
9291
}
9392
return params.previousReplayItemWasReasoning ? params.textSignatureId : undefined;
9493
}
@@ -255,19 +254,14 @@ export function convertResponsesMessages<TApi extends Api>(
255254
} else if (block.type === "text") {
256255
const textBlock = block;
257256
const parsedSignature = parseTextSignature(textBlock.textSignature);
258-
let msgId = parsedSignature?.id;
259-
if (!msgId) {
260-
// Reasoning-dropped/model-switch replay strips textSignature, which can
261-
// leave several text blocks in one assistant turn without ids. msgIndex
262-
// is per-message, so disambiguate fallbacks to avoid duplicate item ids
263-
// (issue #88019).
264-
msgId =
265-
textFallbackOrdinal === 0
266-
? `msg_${msgIndex}`
267-
: `msg_${msgIndex}_${textFallbackOrdinal}`;
257+
let msgId = resolveReplayableResponsesMessageId({
258+
textSignatureId: parsedSignature?.id,
259+
fallbackId: `msg_${msgIndex}`,
260+
fallbackOrdinal: textFallbackOrdinal,
261+
previousReplayItemWasReasoning,
262+
});
263+
if (!parsedSignature?.id) {
268264
textFallbackOrdinal += 1;
269-
} else if (!parsedSignature?.phase && !previousReplayItemWasReasoning) {
270-
msgId = undefined;
271265
}
272266
if (msgId && msgId.length > 64) {
273267
msgId = `msg_${shortHash(msgId)}`;

0 commit comments

Comments
 (0)