Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ describe('minimaxProvider', () => {
});
});

it('includes image generation models as image-only entries', () => {
expect(minimaxProvider.models).toEqual(
expect.arrayContaining([
expect.objectContaining({ id: 'image-01', imageOnly: true }),
expect.objectContaining({ id: 'image-01-live', imageOnly: true }),
]),
);
});

it('creates an install plan with per-model metadata for known IDs', () => {
const plan = buildInstallPlan(minimaxProvider, {
baseUrl: 'https://api.minimaxi.com/v1',
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/providers/presets/minimax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export const minimaxProvider: ProviderConfig = {
{ id: 'MiniMax-M2.7-highspeed', contextWindowSize: 204800 },
{ id: 'MiniMax-M2.5', contextWindowSize: 196608 },
{ id: 'MiniMax-M2.5-highspeed', contextWindowSize: 196608 },
{ id: 'image-01', imageOnly: true },
{ id: 'image-01-live', imageOnly: true },
],
modelsEditable: true,
modelNamePrefix: 'MiniMax',
Expand Down
126 changes: 126 additions & 0 deletions packages/core/src/services/image-generation-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,132 @@ describe('generateImage', () => {
});
});

it('uses the MiniMax image generation schema for regional base URLs', async () => {
const fetchFn = vi
.fn<typeof fetch>()
.mockResolvedValueOnce(
new Response(
JSON.stringify({
base_resp: { request_id: 'request-3', status_code: 0 },
data: { image_urls: ['https://cdn.example.com/generated/mm.png'] },
metadata: { success_count: 1, failed_count: 0 },
}),
{ status: 200, headers: { 'content-type': 'application/json' } },
),
)
.mockResolvedValueOnce(
new Response(PNG_BYTES, {
status: 200,
headers: { 'content-type': 'image/png' },
}),
);

const result = await generateImage({
baseUrl: 'https://api.minimax.io/v1',
apiKey: 'secret',
model: 'image-01',
prompt: 'A product card',
size: '1024*1024',
signal: new AbortController().signal,
fetchFn,
});

expect(result).toEqual({
bytes: Buffer.from(PNG_BYTES),
mimeType: 'image/png',
requestId: 'request-3',
});
expect(fetchFn.mock.calls[0]?.[0]).toBe(
'https://api.minimax.io/v1/image_generation',
);
expect(JSON.parse(String(fetchFn.mock.calls[0]?.[1]?.body))).toEqual({
model: 'image-01',
prompt: 'A product card',
n: 1,
prompt_optimizer: true,
response_format: 'url',
width: 1024,
height: 1024,
});
});

it('accepts a full MiniMax image generation endpoint', async () => {
const fetchFn = vi
.fn<typeof fetch>()
.mockResolvedValueOnce(
new Response(
JSON.stringify({
data: { image_urls: ['https://cdn.example.com/generated/mm.png'] },
}),
{ status: 200 },
),
)
.mockResolvedValueOnce(new Response(PNG_BYTES, { status: 200 }));

await generateImage({
baseUrl: 'https://api.minimaxi.com/v1/image_generation',
apiKey: 'secret',
model: 'image-01-live',
prompt: 'poster',
signal: new AbortController().signal,
fetchFn,
});

expect(fetchFn.mock.calls[0]?.[0]).toBe(
'https://api.minimaxi.com/v1/image_generation',
);
});

it('decodes MiniMax base64 image responses without downloading', async () => {
const base64Png = Buffer.from(PNG_BYTES).toString('base64');
const fetchFn = vi.fn<typeof fetch>().mockResolvedValueOnce(
new Response(
JSON.stringify({
data: { image_urls: [`data:image/png;base64,${base64Png}`] },
}),
{ status: 200 },
),
);

const result = await generateImage({
baseUrl: 'https://api.minimaxi.com/v1',
apiKey: 'secret',
model: 'image-01',
prompt: 'poster',
signal: new AbortController().signal,
fetchFn,
});

expect(result.bytes).toEqual(Buffer.from(PNG_BYTES));
expect(fetchFn).toHaveBeenCalledTimes(1);
});

it('surfaces MiniMax application errors returned with HTTP 200', async () => {
const fetchFn = vi.fn<typeof fetch>().mockResolvedValueOnce(
new Response(
JSON.stringify({
base_resp: {
status_code: 1008,
status_msg: 'insufficient balance',
},
}),
{ status: 200 },
),
);

await expect(
generateImage({
baseUrl: 'https://api.minimax.io/v1',
apiKey: 'secret',
model: 'image-01',
prompt: 'poster',
signal: new AbortController().signal,
fetchFn,
}),
).rejects.toThrow('Image generation failed (1008: insufficient balance).');
expect(fetchFn).toHaveBeenCalledTimes(1);
});

it('pins the validated result hostname for the download connection', async () => {
const lookup = vi.fn();
networkPolicyMocks.resolveNetworkTarget.mockResolvedValueOnce({
Expand Down
178 changes: 175 additions & 3 deletions packages/core/src/services/image-generation-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
const PNG_SIGNATURE = Buffer.from([
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a,
]);
const MINIMAX_IMAGE_GENERATION_PATH = '/v1/image_generation';
const MINIMAX_IMAGE_GENERATION_SUFFIX = '/image_generation';

class ResponseSizeLimitError extends Error {}

Expand Down Expand Up @@ -77,6 +79,9 @@ export async function generateImage(
'Image generation baseUrl must be a valid HTTPS URL without credentials, query, or fragment.',
);
}
if (isMiniMaxImageGenerationBaseUrl(baseUrl)) {
return generateMiniMaxImage({ ...request, baseUrl, fetchFn });
}
const generationUrl = baseUrl.endsWith(
'/services/aigc/multimodal-generation/generation',
)
Expand Down Expand Up @@ -146,6 +151,86 @@ export async function generateImage(
};
}

async function generateMiniMaxImage(
request: ImageGenerationRequest & { baseUrl: string; fetchFn: typeof fetch },
): Promise<GeneratedImage> {
const generationUrl = request.baseUrl.endsWith(MINIMAX_IMAGE_GENERATION_PATH)
? request.baseUrl
: `${request.baseUrl}${MINIMAX_IMAGE_GENERATION_SUFFIX}`;
const body: Record<string, unknown> = {
model: request.model,
prompt: request.prompt,
n: 1,
prompt_optimizer: true,
response_format: 'url',
};
const dimensions = parseImageSize(request.size);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The size-absent branch (request body omits width/height) has no test assertion: the schema test pins the body only with size, and 'accepts a full MiniMax image generation endpoint' omits size but asserts only the request URL. — Failure scenario: a mutation that always appends dimensions (e.g. Number(undefined) serialized as null, or a hardcoded default) passes the suite green and sends MiniMax a malformed or wrong-dimension request for users who run image generation without a configured size. Current behavior was probe-confirmed correct (body carries no width/height keys when size is absent) — this only pins it.

In the full-endpoint test, also assert the body:

expect(JSON.parse(String(fetchFn.mock.calls[0]?.[1]?.body))).toEqual({
  model: 'image-01',
  prompt: expect.any(String),
  n: 1,
  prompt_optimizer: true,
  response_format: 'url',
});

— qwen3.8-max via Qwen Code /review (v0.21.10)

if (dimensions) {
body['width'] = dimensions.width;
body['height'] = dimensions.height;
}

let response: Response;
try {
response = await request.fetchFn(generationUrl, {
method: 'POST',
headers: {
Authorization: `Bearer ${request.apiKey}`,
'Content-Type': 'application/json',
},
body: JSON.stringify(body),
redirect: 'error',
signal: combineWithTimeout(request.signal, GENERATION_TIMEOUT_MS),
});
} catch (error) {
throw new Error(
`Image generation request failed: ${getErrorMessage(error)}`,
{ cause: error },
);
}

if (!response.ok) {
let payload: unknown = {};
try {
payload = await readJsonResponse(response, MAX_API_RESPONSE_BYTES);
} catch {
// non-JSON error body — formatImageGenerationError handles missing fields
}
throw new Error(formatImageGenerationError(response.status, payload));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] MiniMax error responses nest diagnostic information under base_resp: { status_code, status_msg }, but formatImageGenerationError reads only top-level code and message fields. MiniMax-specific error details are silently discarded.

Additionally, MiniMax can return HTTP 200 with application-level errors (base_resp.status_code !== 0 and empty image_urls), which produces the misleading "Image generation response did not contain an image URL." error instead of the real cause (authentication failure, rate limit, content moderation).

Consider extracting base_resp error fields before calling formatImageGenerationError, and checking base_resp.status_code after parsing the success response.

— qwen3.7-max via Qwen Code /review

}
const payload = await readJsonResponse(response, MAX_API_RESPONSE_BYTES);
const baseResponse = isRecord(payload) ? payload['base_resp'] : undefined;
const statusCode = readStringOrNumber(baseResponse, 'status_code');
if (statusCode && statusCode !== '0') {
throw new Error(formatImageGenerationError(response.status, payload));
}
const image = findMiniMaxGeneratedImage(payload);
Comment on lines +199 to +207

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This !response.ok error branch in generateMiniMaxImage (~10 lines: the JSON-body read with fallback plus formatImageGenerationError) has no test coverage, while the equivalent DashScope branch is covered by two tests (HTTP 429 throttling and non-JSON error body). — Failure scenario: MiniMax returns 429/401 and this path runs in production unexercised; if a later refactor drops the try/catch around readJsonResponse, a malformed error body throws an unhandled ResponseSizeLimitError instead of the user-facing message and no test catches the regression. Suggested fix: add a test that sends a non-200 response (e.g. 429 with a JSON error body) to a MiniMax base URL and asserts the rejection message matches /rate limit/i and fetchFn is called exactly once (no download attempted).

— qwen3.8-max-preview via Qwen Code /review

if (!image) {
throw new Error('Image generation response did not contain an image URL.');
}
Comment on lines +208 to +210

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This if (!image) guard on the MiniMax path has no test coverage: none of the four new MiniMax tests sends a 200 response with missing data, a non-array or empty image_urls, or non-string candidates. A mutation probe confirmed the gap — deleting this guard leaves all 21 tests green, while a payload like {"base_resp":{"status_code":0},"data":{"image_urls":[]}} then crashes with TypeError: Cannot read properties of undefined (reading 'kind') instead of the clean error below. — Failure scenario: a future edit deletes or reorders this guard → a MiniMax 200 response with empty image_urls crashes with an unhandled TypeError instead of throwing 'Image generation response did not contain an image URL.', and no test fails. Suggested fix: add a test asserting rejects.toThrow('Image generation response did not contain an image URL.') for a payload with data: { image_urls: [] } (optionally with a [null, '', 'https://…'] candidate list to pin the skip logic).

— qwen3.8-max via Qwen Code /review (v0.21.10)


const requestId =
readString(payload, 'request_id', 'requestId') ??
readString(
isRecord(payload) ? payload['base_resp'] : undefined,
'request_id',
);
if (image.kind === 'base64') {
return {
bytes: decodePngBase64Image(image.value),
mimeType: 'image/png',
...(requestId ? { requestId } : {}),
};
}

const bytes = await downloadPng(image.value, request.fetchFn, request.signal);
return {
bytes,
mimeType: 'image/png',
...(requestId ? { requestId } : {}),
};
}

async function readJsonResponse(
response: Response,
maxBytes: number,
Expand Down Expand Up @@ -201,8 +286,12 @@ async function readBoundedBody(
}

function formatImageGenerationError(status: number, payload: unknown): string {
const code = readString(payload, 'code');
const message = readString(payload, 'message');
const baseResponse = isRecord(payload) ? payload['base_resp'] : undefined;
const code =
readStringOrNumber(payload, 'code') ??
readStringOrNumber(baseResponse, 'status_code');
Comment on lines +290 to +292

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] MiniMax application errors delivered with HTTP 200 — the exact shape this PR adds the statusCode && statusCode !== '0' gate for — can never reach the access-denied or content-moderation branches below, because those match HTTP status (401/403) or regex-test the code string only, while MiniMax codes are numeric base_resp.status_code strings like '1004'. The rate-limit branch already tests `${code} ${message}`, so only the access and moderation branches are asymmetric. — Failure scenario: an invalid/expired MINIMAX_API_KEY returned as { base_resp: { status_code: 1004, status_msg: 'API key not valid' } } yields Image generation failed (1004: API key not valid). instead of the access-denied message with its remediation hint — verified with a live probe against this code, including a flip check; even a status_msg literally containing 'access denied' misses the branch. status_msg still surfaces in the suffix, so this is message quality, not lost information.

Mirror the rate-limit branch and test code+message (and/or map known MiniMax auth/moderation codes):

  if (
    status === 401 ||
    status === 403 ||
    /access|permission/i.test(`${code} ${message}`)
  ) {

Note this alone only flips payloads whose status_msg carries access/permission wording — a complete fix would also map known MiniMax auth codes (e.g. 1004).

— qwen3.8-max via Qwen Code /review (v0.21.10)

const message =
readString(payload, 'message') ?? readString(baseResponse, 'status_msg');
const suffix = [code, message].filter(Boolean).join(': ');

if (status === 429 || /throttl|rate.?limit/i.test(`${code} ${message}`)) {
Expand All @@ -218,7 +307,9 @@ function formatImageGenerationError(status: number, payload: unknown): string {
if (/DataInspectionFailed/i.test(code ?? '')) {
return `The image generation endpoint blocked the prompt during content moderation${message ? `: ${message}` : '.'}`;
}
return `Image generation failed with HTTP ${status}${suffix ? ` (${suffix})` : ''}.`;
const statusText =
status >= 200 && status < 300 ? '' : ` with HTTP ${status}`;
return `Image generation failed${statusText}${suffix ? ` (${suffix})` : ''}.`;
}

function findGeneratedImageUrl(payload: unknown): string | undefined {
Expand All @@ -241,6 +332,70 @@ function findGeneratedImageUrl(payload: unknown): string | undefined {
return undefined;
}

function isMiniMaxImageGenerationBaseUrl(baseUrl: string): boolean {
let parsed: URL;
try {
parsed = new URL(baseUrl);
} catch {
return false;
}
const normalizedPath = parsed.pathname.replace(/\/+$/, '');
return (
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com') &&
Comment on lines +344 to +345

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This hostname allowlist exact-matches only the two official hosts, while the chat provider (MINIMAX_HOST_SUFFIXES in core/openaiContentGenerator/provider/minimax.ts) deliberately also matches subdomains of minimax.io / minimaxi.com for proxies (with a documented rationale), and the same two hosts are already declared in two more places (MINIMAX_KNOWN_HOSTS in that provider module and telemetry/gen-ai-provider.ts). — Failure scenario: a user configures an imageOnly MiniMax model against a subdomain proxy or corporate gateway (e.g. https://gateway.minimax.io/v1) — a configuration resolveImageGenerationModel supports — chat works via suffix matching, but image generation silently POSTs the DashScope schema to …/services/aigc/multimodal-generation/generation and fails with a 404/schema error that does not mention the real cause (verified with a live probe against this code). When MiniMax ships a new regional host, all three copies must also be updated in tandem or image generation silently breaks.

Suggested change
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com') &&
(parsed.hostname === 'api.minimax.io' ||
parsed.hostname === 'api.minimaxi.com' ||
parsed.hostname.endsWith('.minimax.io') ||
parsed.hostname.endsWith('.minimaxi.com')) &&

Longer-term, hoist one shared known-hosts/suffixes constant, or carry the wire schema from the resolved provider config instead of sniffing hostnames.

— qwen3.8-max via Qwen Code /review (v0.21.10)

(normalizedPath === '/v1' ||
normalizedPath === MINIMAX_IMAGE_GENERATION_PATH)
Comment on lines +344 to +347

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This hostname allowlist is implicitly coupled to the preset's baseUrl array in packages/core/src/providers/presets/minimax.ts, with no shared constant, comment, or test keeping them in sync. — Failure scenario: a maintainer adds a new MiniMax regional endpoint (e.g. https://api.minimax.eu/v1) to the preset's baseUrl array; with no signal that this routing function also needs updating, image-01 requests on that endpoint silently fall through to the DashScope request schema and fail with a confusing 404 / "did not contain an image URL" error that gives no hint the root cause is a missing hostname here. Suggested fix: export a shared MINIMAX_IMAGE_HOSTNAMES set used by both the preset and this function, or add a test asserting every hostname in the preset's baseUrl is recognized here.

— qwen3.8-max-preview via Qwen Code /review

);
}

function parseImageSize(
value: string | undefined,
): { width: number; height: number } | undefined {
const match = value?.trim().match(/^(\d+)\s*[*xX]\s*(\d+)$/);
if (!match) return undefined;
const width = Number(match[1]);
const height = Number(match[2]);
if (!Number.isSafeInteger(width) || !Number.isSafeInteger(height)) {
return undefined;
}
return { width, height };
}

function findMiniMaxGeneratedImage(
payload: unknown,
): { kind: 'url' | 'base64'; value: string } | undefined {
if (!isRecord(payload)) return undefined;
const data = payload['data'];
if (!isRecord(data)) return undefined;
const imageUrls = data['image_urls'];
if (!Array.isArray(imageUrls)) return undefined;

for (const candidate of imageUrls) {
if (typeof candidate !== 'string' || !candidate.trim()) continue;
const value = candidate.trim();
if (/^https:\/\//i.test(value)) {
return { kind: 'url', value };
}
return { kind: 'base64', value };
}
return undefined;
Comment on lines +376 to +381

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This for loop never iterates past the first non-empty string: once a non-empty candidate is reached, the unconditional return { kind: 'base64', value } exits, so the for/continue structure implies multi-candidate scanning that the control flow defeats. — Failure scenario: if MiniMax ever returns a non-image string before a valid entry (e.g. image_urls: ["content_filter_notice", "https://cdn.example.com/img.png"]), the first string is classified as base64, decodePngBase64Image throws "did not contain a valid PNG image", and the valid HTTPS URL at index 1 is never examined (n: 1 is pinned today, so this is unlikely in practice, but the structure misleads anyone extending to n > 1). Suggested fix: continue past unrecognized entries instead of returning unconditionally, falling through to return undefined only when no usable entry is found.

— qwen3.8-max-preview via Qwen Code /review

}

function decodePngBase64Image(value: string): Buffer {
const match = value.match(/^data:image\/png;base64,(.+)$/i);
const base64 = (match?.[1] ?? value).trim();
Comment on lines +384 to +386

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The PNG-signature validation in decodePngBase64Image has no negative test — removing the check leaves every existing test green (the only base64 test feeds valid PNG bytes). The DashScope download path has two equivalent negative tests ("rejects a download that is not a PNG image" and "rejects a download with only a partial PNG signature"); this base64 path has none. — Failure scenario: if the signature check is accidentally removed or inverted, non-PNG base64 data (a JPEG or truncated payload from a misbehaving endpoint) is returned to the caller as mimeType: 'image/png' with no error, producing a corrupt image downstream. Suggested fix: add a test that puts base64-encoded non-PNG bytes (e.g. Buffer.from('not a png').toString('base64')) in data.image_urls and asserts the rejection message matches /valid PNG/i.

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +385 to +386

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The ?? value fallback — raw base64 without the data:image/png;base64, prefix — is untested: the diff's test-efficacy probe (harness validated) deleted it and every affected test stayed green (mutant survived). findMiniMaxGeneratedImage routes any non-https: candidate to this branch, so bare base64 is a live shape. — Failure scenario: if a later change drops or breaks this fallback (e.g. a "simplification" to prefixed data URIs only), bare-base64 responses would throw a TypeError on (match?.[1]).trim() in production while the suite stays green.

Add a case to image-generation-service.test.ts where image_urls contains raw base64 (no prefix) and assert the decoded bytes:

// image_urls: [Buffer.from(PNG_BYTES).toString('base64')]
// expect the decoded result.bytes to equal PNG_BYTES and fetchFn not to be called

— qwen3.8-max via Qwen Code /review (v0.21.10)

const bytes = Buffer.from(base64, 'base64');
if (
bytes.length < PNG_SIGNATURE.length ||
!bytes.subarray(0, PNG_SIGNATURE.length).equals(PNG_SIGNATURE)
) {
throw new Error(
'Image generation response did not contain a valid PNG image.',
);
}
return bytes;
}

async function downloadPng(
imageUrl: string,
fetchFn: typeof fetch,
Expand Down Expand Up @@ -381,6 +536,23 @@ function readString(value: unknown, ...keys: string[]): string | undefined {
return undefined;
}

function readStringOrNumber(
value: unknown,
...keys: string[]
): string | undefined {
if (!isRecord(value)) return undefined;
for (const key of keys) {
const candidate = value[key];
if (typeof candidate === 'number' && Number.isFinite(candidate)) {
return String(candidate);
}
if (typeof candidate === 'string' && candidate.trim()) {
return candidate.trim();
}
}
return undefined;
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}
Loading