Address high-priority review gaps before merging the dev snapshot
The release branch still had unresolved review comments around payload-rule atomicity, brand blocking, compact fallback controls, and response compatibility edge-cases. This patch pulls those behaviors back in line with the intended contracts and adds focused regression coverage for the fragile bridges. Constraint: The fixes must stay narrow to the PR branch so review/merge can continue without reopening the larger dev snapshot conflict set Rejected: Leave outdated-looking threads unresolved until after merge | reviewers still see concrete unfixed regressions in the current release branch Rejected: Re-run another broad merge from dev/main | too risky while the PR branch is already green and mergeable Confidence: medium Scope-risk: moderate Directive: If these release-branch fixes need to persist for future release cuts, backport them onto dev instead of re-fighting the same review threads next time Tested: Focused vitest for settings events, brand matcher, compact responses, codex compatibility, anthropic stream bridge, openai chat response bridge; typecheck; repo drift check; git diff --check Not-tested: Full npm test; live PR thread resolution state after push
This commit is contained in:
@@ -155,6 +155,64 @@ describe('openai chat response bridge', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('builds synthetic chunks for multi-choice finals even when some choices omit toolCalls', () => {
|
||||
const chunks = buildNormalizedFinalToOpenAiChatChunks({
|
||||
id: 'chatcmpl-multi-1',
|
||||
model: 'gpt-5',
|
||||
created: 123,
|
||||
content: '',
|
||||
reasoningContent: '',
|
||||
finishReason: 'stop',
|
||||
toolCalls: [],
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
toolCalls: [{
|
||||
id: 'call_multi_1',
|
||||
name: 'lookup_weather',
|
||||
arguments: '{"city":"Paris"}',
|
||||
}],
|
||||
finishReason: 'tool_calls',
|
||||
},
|
||||
{
|
||||
index: 1,
|
||||
role: 'assistant',
|
||||
content: 'done',
|
||||
finishReason: 'stop',
|
||||
},
|
||||
],
|
||||
} as any);
|
||||
|
||||
expect(chunks[0]).toMatchObject({
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
tool_calls: [{
|
||||
id: 'call_multi_1',
|
||||
type: 'function',
|
||||
}],
|
||||
},
|
||||
},
|
||||
{
|
||||
index: 1,
|
||||
delta: {
|
||||
role: 'assistant',
|
||||
content: 'done',
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(chunks[1]).toMatchObject({
|
||||
choices: [
|
||||
{ index: 0, finish_reason: 'tool_calls' },
|
||||
{ index: 1, finish_reason: 'stop' },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the outbound facade pointed at the response bridge object', () => {
|
||||
expect(openAiChatOutbound).toBe(openAiChatResponseBridge);
|
||||
});
|
||||
|
||||
@@ -58,10 +58,24 @@ export function buildNormalizedFinalToOpenAiChatPayload(
|
||||
usage?: unknown,
|
||||
) {
|
||||
const chatNormalized = normalized as OpenAiChatNormalizedFinalResponse;
|
||||
const normalizedUsage = isRecord(usage)
|
||||
&& typeof usage.promptTokens === 'number'
|
||||
&& typeof usage.completionTokens === 'number'
|
||||
&& typeof usage.totalTokens === 'number'
|
||||
? {
|
||||
promptTokens: usage.promptTokens,
|
||||
completionTokens: usage.completionTokens,
|
||||
totalTokens: usage.totalTokens,
|
||||
}
|
||||
: {
|
||||
promptTokens: 0,
|
||||
completionTokens: 0,
|
||||
totalTokens: 0,
|
||||
};
|
||||
const payload = serializeFinalResponse(
|
||||
'openai',
|
||||
normalized,
|
||||
usage as { promptTokens: number; completionTokens: number; totalTokens: number },
|
||||
normalizedUsage,
|
||||
) as Record<string, unknown>;
|
||||
|
||||
const choices = Array.isArray(chatNormalized.choices) && chatNormalized.choices.length > 0
|
||||
@@ -136,13 +150,14 @@ export function buildNormalizedFinalToOpenAiChatChunks(normalized: NormalizedFin
|
||||
created: normalized.created,
|
||||
model: normalized.model,
|
||||
choices: choices.map((choice) => {
|
||||
const toolCalls = Array.isArray(choice.toolCalls) ? choice.toolCalls : [];
|
||||
const delta: Record<string, unknown> = {
|
||||
role: choice.role || 'assistant',
|
||||
content: choice.content || '',
|
||||
};
|
||||
if (choice.reasoningContent) delta.reasoning_content = choice.reasoningContent;
|
||||
if (choice.toolCalls.length > 0) {
|
||||
delta.tool_calls = choice.toolCalls.map((toolCall, toolIndex) => ({
|
||||
if (toolCalls.length > 0) {
|
||||
delta.tool_calls = toolCalls.map((toolCall, toolIndex) => ({
|
||||
index: toolIndex,
|
||||
id: toolCall.id,
|
||||
type: 'function',
|
||||
@@ -167,11 +182,14 @@ export function buildNormalizedFinalToOpenAiChatChunks(normalized: NormalizedFin
|
||||
object: 'chat.completion.chunk',
|
||||
created: normalized.created,
|
||||
model: normalized.model,
|
||||
choices: choices.map((choice) => ({
|
||||
index: choice.index,
|
||||
delta: {},
|
||||
finish_reason: choice.finishReason || (choice.toolCalls.length > 0 ? 'tool_calls' : 'stop'),
|
||||
})).sort((left, right) => left.index - right.index),
|
||||
choices: choices.map((choice) => {
|
||||
const toolCalls = Array.isArray(choice.toolCalls) ? choice.toolCalls : [];
|
||||
return {
|
||||
index: choice.index,
|
||||
delta: {},
|
||||
finish_reason: choice.finishReason || (toolCalls.length > 0 ? 'tool_calls' : 'stop'),
|
||||
};
|
||||
}).sort((left, right) => left.index - right.index),
|
||||
},
|
||||
]
|
||||
: buildSyntheticOpenAiChunks(normalized);
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { normalizeCodexResponsesBodyForProxy } from './codexCompatibility.js';
|
||||
|
||||
const CODEX_DEFAULT_INSTRUCTIONS = 'You are a helpful coding assistant.';
|
||||
import {
|
||||
CODEX_DEFAULT_INSTRUCTIONS,
|
||||
normalizeCodexResponsesBodyForProxy,
|
||||
} from './codexCompatibility.js';
|
||||
|
||||
describe('normalizeCodexResponsesBodyForProxy', () => {
|
||||
it('extracts codex system input into top-level instructions before proxying upstream', () => {
|
||||
@@ -22,6 +23,7 @@ describe('normalizeCodexResponsesBodyForProxy', () => {
|
||||
max_output_tokens: 512,
|
||||
max_completion_tokens: 256,
|
||||
max_tokens: 128,
|
||||
stream_options: { include_usage: true },
|
||||
temperature: 0.3,
|
||||
store: true,
|
||||
}, 'codex');
|
||||
@@ -40,6 +42,62 @@ describe('normalizeCodexResponsesBodyForProxy', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps non-text system content in input while lifting only the text blocks into instructions', () => {
|
||||
const imageBlock = {
|
||||
type: 'input_image',
|
||||
image_url: 'https://example.com/reference.png',
|
||||
};
|
||||
|
||||
const body = normalizeCodexResponsesBodyForProxy({
|
||||
input: [
|
||||
{
|
||||
type: 'message',
|
||||
role: 'system',
|
||||
content: [
|
||||
{ type: 'input_text', text: 'keep the image context' },
|
||||
imageBlock,
|
||||
],
|
||||
},
|
||||
{
|
||||
type: 'message',
|
||||
role: 'user',
|
||||
content: [{ type: 'input_text', text: 'hello' }],
|
||||
},
|
||||
],
|
||||
}, 'codex');
|
||||
|
||||
expect(body.instructions).toBe('keep the image context');
|
||||
expect(body.input).toEqual([
|
||||
{
|
||||
type: 'message',
|
||||
role: 'system',
|
||||
content: [imageBlock],
|
||||
},
|
||||
{
|
||||
type: 'message',
|
||||
role: 'user',
|
||||
content: [{ type: 'input_text', text: 'hello' }],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('joins multiple text blocks with separators instead of squashing them together', () => {
|
||||
const body = normalizeCodexResponsesBodyForProxy({
|
||||
input: [
|
||||
{
|
||||
type: 'message',
|
||||
role: 'system',
|
||||
content: [
|
||||
{ type: 'input_text', text: 'first line' },
|
||||
{ type: 'input_text', text: 'second line' },
|
||||
],
|
||||
},
|
||||
],
|
||||
}, 'codex');
|
||||
|
||||
expect(body.instructions).toBe('first line\nsecond line');
|
||||
});
|
||||
|
||||
it('supplies a non-empty default instructions string for codex when missing', () => {
|
||||
const body = normalizeCodexResponsesBodyForProxy({
|
||||
input: 'hello',
|
||||
|
||||
@@ -6,17 +6,9 @@ function asTrimmedString(value: unknown): string {
|
||||
return typeof value === 'string' ? value.trim() : '';
|
||||
}
|
||||
|
||||
const CODEX_DEFAULT_INSTRUCTIONS = 'You are a helpful coding assistant.';
|
||||
export const CODEX_DEFAULT_INSTRUCTIONS = 'You are a helpful coding assistant.';
|
||||
|
||||
function extractTextFromContent(content: unknown): string {
|
||||
if (typeof content === 'string') return content;
|
||||
if (Array.isArray(content)) {
|
||||
return content
|
||||
.map((item) => extractTextFromContent(item))
|
||||
.filter((item) => item.length > 0)
|
||||
.join('');
|
||||
}
|
||||
if (!isRecord(content)) return '';
|
||||
function extractTextValue(content: Record<string, unknown>): string {
|
||||
if (typeof content.text === 'string') return content.text;
|
||||
if (typeof content.content === 'string') return content.content;
|
||||
if (typeof content.output_text === 'string') return content.output_text;
|
||||
@@ -24,6 +16,75 @@ function extractTextFromContent(content: unknown): string {
|
||||
return '';
|
||||
}
|
||||
|
||||
function isTextOnlyContentRecord(content: Record<string, unknown>): boolean {
|
||||
const blockType = asTrimmedString(content.type).toLowerCase();
|
||||
if (blockType === 'input_text' || blockType === 'output_text' || blockType === 'text') {
|
||||
return true;
|
||||
}
|
||||
const keys = Object.keys(content);
|
||||
return keys.length > 0 && keys.every((key) => (
|
||||
key === 'type'
|
||||
|| key === 'text'
|
||||
|| key === 'content'
|
||||
|| key === 'output_text'
|
||||
|| key === 'input_text'
|
||||
));
|
||||
}
|
||||
|
||||
function splitExtractedSystemContent(content: unknown): {
|
||||
extractedTexts: string[];
|
||||
remainingContent?: unknown;
|
||||
} {
|
||||
if (typeof content === 'string') {
|
||||
const trimmed = content.trim();
|
||||
return trimmed ? { extractedTexts: [trimmed] } : { extractedTexts: [] };
|
||||
}
|
||||
|
||||
if (Array.isArray(content)) {
|
||||
const extractedTexts: string[] = [];
|
||||
const remainingItems: unknown[] = [];
|
||||
|
||||
for (const item of content) {
|
||||
if (typeof item === 'string') {
|
||||
const trimmed = item.trim();
|
||||
if (trimmed) extractedTexts.push(trimmed);
|
||||
else remainingItems.push(item);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!isRecord(item)) {
|
||||
remainingItems.push(item);
|
||||
continue;
|
||||
}
|
||||
|
||||
const textValue = extractTextValue(item).trim();
|
||||
if (textValue && isTextOnlyContentRecord(item)) {
|
||||
extractedTexts.push(textValue);
|
||||
continue;
|
||||
}
|
||||
|
||||
remainingItems.push(item);
|
||||
}
|
||||
|
||||
return {
|
||||
extractedTexts,
|
||||
...(remainingItems.length > 0 ? { remainingContent: remainingItems } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
if (!isRecord(content)) return { extractedTexts: [] };
|
||||
|
||||
const textValue = extractTextValue(content).trim();
|
||||
if (textValue && isTextOnlyContentRecord(content)) {
|
||||
return { extractedTexts: [textValue] };
|
||||
}
|
||||
|
||||
return {
|
||||
extractedTexts: [],
|
||||
remainingContent: content,
|
||||
};
|
||||
}
|
||||
|
||||
function extractSystemMessagesToInstructions(
|
||||
body: Record<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
@@ -46,10 +107,19 @@ function extractSystemMessagesToInstructions(
|
||||
continue;
|
||||
}
|
||||
|
||||
const text = extractTextFromContent(item.content);
|
||||
if (text.trim()) {
|
||||
extractedSystemTexts.push(text);
|
||||
const { extractedTexts, remainingContent } = splitExtractedSystemContent(item.content);
|
||||
if (extractedTexts.length > 0) {
|
||||
extractedSystemTexts.push(extractedTexts.join('\n'));
|
||||
if (remainingContent !== undefined) {
|
||||
remainingInput.push({
|
||||
...item,
|
||||
content: remainingContent,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
remainingInput.push(item);
|
||||
}
|
||||
|
||||
if (extractedSystemTexts.length <= 0) return body;
|
||||
@@ -99,6 +169,7 @@ function stripCodexUnsupportedResponsesFields(
|
||||
delete next.max_output_tokens;
|
||||
delete next.max_completion_tokens;
|
||||
delete next.max_tokens;
|
||||
delete next.stream_options;
|
||||
return next;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user