Skip to content

🤖 Add integration test for OpenAI reasoning error #72

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
ammario wants to merge 9 commits into from
Closed

🤖 Add integration test for OpenAI reasoning error #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
96552da
Strip reasoning parts from message history before sending to OpenAI
ammario Oct 7, 2025
5ad3be7
Make reasoning stripping OpenAI-specific
ammario Oct 7, 2025
610e3fb
Clear provider metadata for OpenAI instead of stripping reasoning
ammario Oct 7, 2025
d88bd56
Merge remote-tracking branch 'origin/main' into tokens
ammario Oct 7, 2025
08d7398
Fix linting errors and reasoning metadata clearing
ammario Oct 7, 2025
042118f
🤖 Clear providerMetadata from tool messages too
ammario Oct 7, 2025
40db50c
Merge main into tokens
ammario Oct 7, 2025
6390d0c
🤖 Add integration test to reproduce OpenAI reasoning error
ammario Oct 7, 2025
7944e6d
🤖 Fix test logic to properly detect stream errors
ammario Oct 7, 2025
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
29 changes: 14 additions & 15 deletions src/services/aiService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
validateAnthropicCompliance,
addInterruptedSentinel,
filterEmptyAssistantMessages,
stripReasoningForOpenAI,
clearProviderMetadataForOpenAI,
} from "@/utils/messages/modelMessageTransform";
import { applyCacheControl } from "@/utils/ai/cacheStrategy";
import type { HistoryService } from "./historyService";
Expand Down Expand Up @@ -283,31 +283,31 @@ export class AIService extends EventEmitter {
const [providerName] = modelString.split(":");

// Filter out assistant messages with only reasoning (no text/tools)
let filteredMessages = filterEmptyAssistantMessages(messages);
const filteredMessages = filterEmptyAssistantMessages(messages);
log.debug(`Filtered ${messages.length - filteredMessages.length} empty assistant messages`);
log.debug_obj(`${workspaceId}/1a_filtered_messages.json`, filteredMessages);

// OpenAI-specific: Strip reasoning parts from history
// OpenAI manages reasoning via previousResponseId; sending Anthropic-style reasoning
// parts creates orphaned reasoning items that cause API errors
if (providerName === "openai") {
filteredMessages = stripReasoningForOpenAI(filteredMessages);
log.debug("Stripped reasoning parts for OpenAI");
log.debug_obj(`${workspaceId}/1b_openai_stripped.json`, filteredMessages);
}

// Add [INTERRUPTED] sentinel to partial messages (for model context)
const messagesWithSentinel = addInterruptedSentinel(filteredMessages);

// Convert CmuxMessage to ModelMessage format using Vercel AI SDK utility
// Type assertion needed because CmuxMessage has custom tool parts for interrupted tools
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
const modelMessages = convertToModelMessages(messagesWithSentinel as any);
let modelMessages = convertToModelMessages(messagesWithSentinel as any);

log.debug_obj(`${workspaceId}/2_model_messages.json`, modelMessages);

// OpenAI-specific: Clear provider metadata to prevent reasoning/tool errors
// OpenAI manages reasoning via previousResponseId; sending stale provider metadata
// from history causes "reasoning without following item" and tool call errors
if (providerName === "openai") {
modelMessages = clearProviderMetadataForOpenAI(modelMessages);
log.debug("Cleared provider metadata for OpenAI");
log.debug_obj(`${workspaceId}/2a_openai_cleaned.json`, modelMessages);
}

// Apply ModelMessage transforms based on provider requirements
const transformedMessages = transformModelMessages(modelMessages, providerName);
const transformedMessages = transformModelMessages(modelMessages);

// Apply cache control for Anthropic models AFTER transformation
const finalMessages = applyCacheControl(transformedMessages, modelString);
Expand Down Expand Up @@ -387,8 +387,7 @@ export class AIService extends EventEmitter {
timestamp: Date.now(),
},
providerOptions,
maxOutputTokens,
toolPolicy
maxOutputTokens
);

if (!streamResult.success) {
Expand Down
118 changes: 56 additions & 62 deletions src/utils/messages/modelMessageTransform.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe("modelMessageTransform", () => {
assistantMsg,
];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toEqual(messages);
});

Expand All @@ -38,7 +38,7 @@ describe("modelMessageTransform", () => {
};
const messages: ModelMessage[] = [assistantMsg1, assistantMsg2];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toEqual(messages);
});

Expand All @@ -52,7 +52,7 @@ describe("modelMessageTransform", () => {
};
const messages: ModelMessage[] = [assistantMsg];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);

// Should only keep text, strip interrupted tool calls
expect(result).toHaveLength(1);
Expand All @@ -71,7 +71,7 @@ describe("modelMessageTransform", () => {
};
const messages: ModelMessage[] = [assistantMsg];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);

// Should filter out the entire message since it only has orphaned tool calls
expect(result).toHaveLength(0);
Expand Down Expand Up @@ -108,7 +108,7 @@ describe("modelMessageTransform", () => {
};
const messages: ModelMessage[] = [assistantMsg, toolMsg];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);

// Should have: text message, tool calls (only call1 & call2), tool results
expect(result).toHaveLength(3);
Expand Down Expand Up @@ -198,7 +198,7 @@ describe("modelMessageTransform", () => {
};
const messages: ModelMessage[] = [assistantMsg, toolMsg];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);

// Should split into multiple messages with tool results properly placed
expect(result.length).toBeGreaterThan(2);
Expand Down Expand Up @@ -323,7 +323,7 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toHaveLength(1);
expect(result[0].role).toBe("user");
expect((result[0].content as Array<{ type: string; text: string }>)[0].text).toBe("Hello");
Expand All @@ -341,7 +341,7 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toHaveLength(1);
expect(result[0].role).toBe("user");
expect((result[0].content as Array<{ type: string; text: string }>)[0].text).toBe(
Expand All @@ -365,7 +365,7 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toHaveLength(1);
expect(result[0].role).toBe("user");
expect((result[0].content as Array<{ type: string; text: string }>)[0].text).toBe(
Expand All @@ -389,7 +389,7 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "anthropic");
const result = transformModelMessages(messages);
expect(result).toHaveLength(3);
expect(result[0].role).toBe("user");
expect((result[0].content as Array<{ type: string; text: string }>)[0].text).toBe("Hello");
Expand Down Expand Up @@ -517,8 +517,8 @@ describe("modelMessageTransform", () => {
});
});

describe("reasoning part stripping for OpenAI", () => {
it("should strip reasoning parts for OpenAI provider", () => {
describe("reasoning part handling", () => {
it("should preserve reasoning parts for both OpenAI and Anthropic", () => {
const messages: ModelMessage[] = [
{
role: "user",
Expand All @@ -533,46 +533,28 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "openai");
// Both providers should preserve reasoning parts
// OpenAI-specific metadata clearing is done in aiService.ts, not in transformModelMessages
const resultOpenAI = transformModelMessages(messages);
const resultAnthropic = transformModelMessages(messages);

// Should have 2 messages, assistant message should only have text
expect(result).toHaveLength(2);
expect(result[1].role).toBe("assistant");
expect((result[1] as AssistantModelMessage).content).toEqual([
{ type: "text", text: "Here's the solution" },
]);
});

it("should preserve reasoning parts for Anthropic provider", () => {
const messages: ModelMessage[] = [
{
role: "user",
content: [{ type: "text", text: "Solve this problem" }],
},
{
role: "assistant",
content: [
{ type: "reasoning", text: "Let me think about this..." },
{ type: "text", text: "Here's the solution" },
],
},
];
// Both should have 2 messages with reasoning and text preserved
expect(resultOpenAI).toHaveLength(2);
expect(resultAnthropic).toHaveLength(2);

const result = transformModelMessages(messages, "anthropic");

// Should have 2 messages, assistant message should have both reasoning and text
expect(result).toHaveLength(2);
expect(result[1].role).toBe("assistant");
const content = (result[1] as AssistantModelMessage).content;
expect(Array.isArray(content)).toBe(true);
if (Array.isArray(content)) {
expect(content).toHaveLength(2);
expect(content[0]).toEqual({ type: "reasoning", text: "Let me think about this..." });
expect(content[1]).toEqual({ type: "text", text: "Here's the solution" });
for (const result of [resultOpenAI, resultAnthropic]) {
expect(result[1].role).toBe("assistant");
const content = (result[1] as AssistantModelMessage).content;
expect(Array.isArray(content)).toBe(true);
if (Array.isArray(content)) {
expect(content).toHaveLength(2);
expect(content[0]).toEqual({ type: "reasoning", text: "Let me think about this..." });
expect(content[1]).toEqual({ type: "text", text: "Here's the solution" });
}
}
});

it("should filter out reasoning-only messages for OpenAI", () => {
it("should filter out reasoning-only messages for all providers", () => {
const messages: ModelMessage[] = [
{
role: "user",
Expand All @@ -584,14 +566,18 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "openai");
// Both providers should filter reasoning-only messages
const resultOpenAI = transformModelMessages(messages);
const resultAnthropic = transformModelMessages(messages);

// Should only have user message, reasoning-only assistant message should be filtered out
expect(result).toHaveLength(1);
expect(result[0].role).toBe("user");
// Should only have user message for both providers
expect(resultOpenAI).toHaveLength(1);
expect(resultOpenAI[0].role).toBe("user");
expect(resultAnthropic).toHaveLength(1);
expect(resultAnthropic[0].role).toBe("user");
});

it("should preserve tool calls when stripping reasoning for OpenAI", () => {
it("should preserve reasoning and tool calls in messages", () => {
const messages: ModelMessage[] = [
{
role: "user",
Expand All @@ -618,9 +604,9 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "openai");
const result = transformModelMessages(messages);

// Should have user, text, tool-call, tool-result (no reasoning)
// Should split into text message and tool-call/tool-result messages
expect(result.length).toBeGreaterThan(2);

// Find the assistant message with text
Expand All @@ -633,8 +619,8 @@ describe("modelMessageTransform", () => {
if (textMessage) {
const content = (textMessage as AssistantModelMessage).content;
if (Array.isArray(content)) {
// Should not have reasoning parts
expect(content.some((c) => c.type === "reasoning")).toBe(false);
// Should have reasoning parts preserved
expect(content.some((c) => c.type === "reasoning")).toBe(true);
// Should have text
expect(content.some((c) => c.type === "text")).toBe(true);
}
Expand All @@ -649,7 +635,7 @@ describe("modelMessageTransform", () => {
expect(toolCallMessage).toBeDefined();
});

it("should handle multiple reasoning parts for OpenAI", () => {
it("should coalesce multiple consecutive reasoning parts", () => {
const messages: ModelMessage[] = [
{
role: "user",
Expand All @@ -665,14 +651,22 @@ describe("modelMessageTransform", () => {
},
];

const result = transformModelMessages(messages, "openai");
const result = transformModelMessages(messages);

// Should have 2 messages, assistant should only have text
// Should have 2 messages, assistant should have coalesced reasoning and text
expect(result).toHaveLength(2);
expect(result[1].role).toBe("assistant");
expect((result[1] as AssistantModelMessage).content).toEqual([
{ type: "text", text: "Final answer" },
]);
const content = (result[1] as AssistantModelMessage).content;
expect(Array.isArray(content)).toBe(true);
if (Array.isArray(content)) {
// Should coalesce the two reasoning parts into one
expect(content).toHaveLength(2);
expect(content[0]).toEqual({
type: "reasoning",
text: "First, I'll consider...Then, I'll analyze...",
});
expect(content[1]).toEqual({ type: "text", text: "Final answer" });
}
});
});
});
Loading
Loading