mirror of https://github.com/garrytan/gstack.git
fix(design): add --api-timeout flag, raise default 120s to 300s for image-gen
Five image-generation callsites (generate, variants, iterate x2, evolve) hardcoded a 120_000ms ceiling with no CLI override. With default size (1536x1024) + quality:high on gpt-4o + image_generation tool, response time pushes into the 90-180s range on slower account tiers, tipping over the 120s ceiling for many users (issue #1519). - design/src/constants.ts: DEFAULT_IMAGE_GEN_TIMEOUT_MS = 300_000 - apiTimeoutMs?: number option threaded through GenerateOptions, VariantsOptions, IterateOptions, EvolveOptions - --api-timeout <ms> CLI flag (distinct from --timeout, which is plumbed only to compare --serve / serve for the HTTP listener) - Regression test pins the constant + verifies the AbortController honors the override via stubbed slow fetch Closes #1519.
This commit is contained in:
parent
a5833c413f
commit
d78eeeed03
|
|
@ -130,6 +130,13 @@ async function main(): Promise<void> {
|
|||
process.exit(1);
|
||||
}
|
||||
|
||||
// Per-request timeout for OpenAI image-generation calls. Distinct from the
|
||||
// existing `--timeout` flag, which controls the `compare --serve` / `serve`
|
||||
// HTTP listener. See issue #1519.
|
||||
const apiTimeoutMs = flags["api-timeout"]
|
||||
? parseInt(flags["api-timeout"] as string)
|
||||
: undefined;
|
||||
|
||||
switch (command) {
|
||||
case "generate":
|
||||
await generate({
|
||||
|
|
@ -140,6 +147,7 @@ async function main(): Promise<void> {
|
|||
retry: flags.retry ? parseInt(flags.retry as string) : 0,
|
||||
size: flags.size as string,
|
||||
quality: flags.quality as string,
|
||||
apiTimeoutMs,
|
||||
});
|
||||
break;
|
||||
|
||||
|
|
@ -202,6 +210,7 @@ async function main(): Promise<void> {
|
|||
size: flags.size as string,
|
||||
quality: flags.quality as string,
|
||||
viewports: flags.viewports as string,
|
||||
apiTimeoutMs,
|
||||
});
|
||||
break;
|
||||
|
||||
|
|
@ -210,6 +219,7 @@ async function main(): Promise<void> {
|
|||
session: flags.session as string,
|
||||
feedback: flags.feedback as string,
|
||||
output: (flags.output as string) || "/tmp/gstack-iterate.png",
|
||||
apiTimeoutMs,
|
||||
});
|
||||
break;
|
||||
|
||||
|
|
@ -262,6 +272,7 @@ async function main(): Promise<void> {
|
|||
screenshot: flags.screenshot as string,
|
||||
brief: flags.brief as string,
|
||||
output: (flags.output as string) || "/tmp/gstack-evolved.png",
|
||||
apiTimeoutMs,
|
||||
});
|
||||
break;
|
||||
|
||||
|
|
|
|||
|
|
@ -17,17 +17,17 @@ export const COMMANDS = new Map<string, {
|
|||
["generate", {
|
||||
description: "Generate a UI mockup from a design brief",
|
||||
usage: "generate --brief \"...\" --output /path.png",
|
||||
flags: ["--brief", "--brief-file", "--output", "--check", "--retry", "--size", "--quality"],
|
||||
flags: ["--brief", "--brief-file", "--output", "--check", "--retry", "--size", "--quality", "--api-timeout"],
|
||||
}],
|
||||
["variants", {
|
||||
description: "Generate N design variants from a brief",
|
||||
usage: "variants --brief \"...\" --count 3 --output-dir /path/",
|
||||
flags: ["--brief", "--brief-file", "--count", "--output-dir", "--size", "--quality", "--viewports"],
|
||||
flags: ["--brief", "--brief-file", "--count", "--output-dir", "--size", "--quality", "--viewports", "--api-timeout"],
|
||||
}],
|
||||
["iterate", {
|
||||
description: "Iterate on an existing mockup with feedback",
|
||||
usage: "iterate --session /path/session.json --feedback \"...\" --output /path.png",
|
||||
flags: ["--session", "--feedback", "--output"],
|
||||
flags: ["--session", "--feedback", "--output", "--api-timeout"],
|
||||
}],
|
||||
["check", {
|
||||
description: "Vision-based quality check on a mockup",
|
||||
|
|
@ -47,7 +47,7 @@ export const COMMANDS = new Map<string, {
|
|||
["evolve", {
|
||||
description: "Generate improved mockup from existing screenshot",
|
||||
usage: "evolve --screenshot current.png --brief \"make it calmer\" --output /path.png",
|
||||
flags: ["--screenshot", "--brief", "--output"],
|
||||
flags: ["--screenshot", "--brief", "--output", "--api-timeout"],
|
||||
}],
|
||||
["verify", {
|
||||
description: "Compare live site screenshot against approved mockup",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,11 @@
|
|||
/**
|
||||
* Shared constants for the design binary.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Default per-request timeout for OpenAI Responses API calls that drive the
|
||||
* `image_generation` tool. The previous 120_000 ceiling tipped over at default
|
||||
* settings (`gpt-4o`, 1536x1024, quality:high) on slower account tiers — see
|
||||
* issue #1519. Override per-invocation via `--api-timeout <ms>`.
|
||||
*/
|
||||
export const DEFAULT_IMAGE_GEN_TIMEOUT_MS = 300_000;
|
||||
|
|
@ -8,11 +8,13 @@
|
|||
import fs from "fs";
|
||||
import path from "path";
|
||||
import { requireApiKey } from "./auth";
|
||||
import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants";
|
||||
|
||||
export interface EvolveOptions {
|
||||
screenshot: string; // Path to current site screenshot
|
||||
brief: string; // What to change ("make it calmer", "fix the hierarchy")
|
||||
output: string; // Output path for evolved mockup
|
||||
apiTimeoutMs?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -52,7 +54,7 @@ export async function evolve(options: EvolveOptions): Promise<void> {
|
|||
].join("\n");
|
||||
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 240_000);
|
||||
const timeout = setTimeout(() => controller.abort(), options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS);
|
||||
|
||||
try {
|
||||
const response = await fetch("https://api.openai.com/v1/responses", {
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { requireApiKey } from "./auth";
|
|||
import { parseBrief } from "./brief";
|
||||
import { createSession, sessionPath } from "./session";
|
||||
import { checkMockup } from "./check";
|
||||
import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants";
|
||||
|
||||
export interface GenerateOptions {
|
||||
brief?: string;
|
||||
|
|
@ -17,6 +18,7 @@ export interface GenerateOptions {
|
|||
retry?: number;
|
||||
size?: string;
|
||||
quality?: string;
|
||||
apiTimeoutMs?: number;
|
||||
}
|
||||
|
||||
export interface GenerateResult {
|
||||
|
|
@ -35,9 +37,10 @@ async function callImageGeneration(
|
|||
prompt: string,
|
||||
size: string,
|
||||
quality: string,
|
||||
timeoutMs: number,
|
||||
): Promise<{ responseId: string; imageData: string }> {
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 240_000);
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
try {
|
||||
const response = await fetch("https://api.openai.com/v1/responses", {
|
||||
|
|
@ -106,6 +109,7 @@ export async function generate(options: GenerateOptions): Promise<GenerateResult
|
|||
const size = options.size || "1536x1024";
|
||||
const quality = options.quality || "high";
|
||||
const maxRetries = options.retry ?? 0;
|
||||
const apiTimeoutMs = options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS;
|
||||
|
||||
let lastResult: GenerateResult | null = null;
|
||||
|
||||
|
|
@ -116,7 +120,7 @@ export async function generate(options: GenerateOptions): Promise<GenerateResult
|
|||
|
||||
// Generate the image
|
||||
const startTime = Date.now();
|
||||
const { responseId, imageData } = await callImageGeneration(apiKey, prompt, size, quality);
|
||||
const { responseId, imageData } = await callImageGeneration(apiKey, prompt, size, quality, apiTimeoutMs);
|
||||
const elapsed = ((Date.now() - startTime) / 1000).toFixed(1);
|
||||
|
||||
// Write to disk
|
||||
|
|
|
|||
|
|
@ -10,11 +10,13 @@ import fs from "fs";
|
|||
import path from "path";
|
||||
import { requireApiKey } from "./auth";
|
||||
import { readSession, updateSession } from "./session";
|
||||
import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants";
|
||||
|
||||
export interface IterateOptions {
|
||||
session: string; // Path to session JSON file
|
||||
feedback: string; // User feedback text
|
||||
output: string; // Output path for new PNG
|
||||
apiTimeoutMs?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -29,13 +31,14 @@ export async function iterate(options: IterateOptions): Promise<void> {
|
|||
console.error(` Feedback: "${options.feedback}"`);
|
||||
|
||||
const startTime = Date.now();
|
||||
const apiTimeoutMs = options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS;
|
||||
|
||||
// Try multi-turn with previous_response_id first
|
||||
let success = false;
|
||||
let responseId = "";
|
||||
|
||||
try {
|
||||
const result = await callWithThreading(apiKey, session.lastResponseId, options.feedback);
|
||||
const result = await callWithThreading(apiKey, session.lastResponseId, options.feedback, apiTimeoutMs);
|
||||
responseId = result.responseId;
|
||||
|
||||
fs.mkdirSync(path.dirname(options.output), { recursive: true });
|
||||
|
|
@ -51,7 +54,7 @@ export async function iterate(options: IterateOptions): Promise<void> {
|
|||
[...session.feedbackHistory, options.feedback]
|
||||
);
|
||||
|
||||
const result = await callFresh(apiKey, accumulatedPrompt);
|
||||
const result = await callFresh(apiKey, accumulatedPrompt, apiTimeoutMs);
|
||||
responseId = result.responseId;
|
||||
|
||||
fs.mkdirSync(path.dirname(options.output), { recursive: true });
|
||||
|
|
@ -80,9 +83,10 @@ async function callWithThreading(
|
|||
apiKey: string,
|
||||
previousResponseId: string,
|
||||
feedback: string,
|
||||
timeoutMs: number,
|
||||
): Promise<{ responseId: string; imageData: string }> {
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 240_000);
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
try {
|
||||
const response = await fetch("https://api.openai.com/v1/responses", {
|
||||
|
|
@ -128,9 +132,10 @@ async function callWithThreading(
|
|||
async function callFresh(
|
||||
apiKey: string,
|
||||
prompt: string,
|
||||
timeoutMs: number,
|
||||
): Promise<{ responseId: string; imageData: string }> {
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 240_000);
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
try {
|
||||
const response = await fetch("https://api.openai.com/v1/responses", {
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import fs from "fs";
|
|||
import path from "path";
|
||||
import { requireApiKey } from "./auth";
|
||||
import { parseBrief } from "./brief";
|
||||
import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "./constants";
|
||||
|
||||
export interface VariantsOptions {
|
||||
brief?: string;
|
||||
|
|
@ -17,6 +18,7 @@ export interface VariantsOptions {
|
|||
size?: string;
|
||||
quality?: string;
|
||||
viewports?: string; // "desktop,tablet,mobile" — generates at multiple sizes
|
||||
apiTimeoutMs?: number;
|
||||
}
|
||||
|
||||
const STYLE_VARIATIONS = [
|
||||
|
|
@ -42,6 +44,7 @@ export async function generateVariant(
|
|||
size: string,
|
||||
quality: string,
|
||||
fetchFn: typeof globalThis.fetch = globalThis.fetch,
|
||||
timeoutMs: number = DEFAULT_IMAGE_GEN_TIMEOUT_MS,
|
||||
): Promise<{ path: string; success: boolean; error?: string }> {
|
||||
const maxRetries = 3;
|
||||
const MAX_RETRY_AFTER_MS = 60_000; // cap honored Retry-After to bound stalls
|
||||
|
|
@ -58,7 +61,7 @@ export async function generateVariant(
|
|||
skipLeadingDelay = false;
|
||||
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 240_000);
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
try {
|
||||
const response = await fetchFn("https://api.openai.com/v1/responses", {
|
||||
|
|
@ -125,7 +128,7 @@ export async function generateVariant(
|
|||
} catch (err: any) {
|
||||
clearTimeout(timeout);
|
||||
if (err.name === "AbortError") {
|
||||
return { path: outputPath, success: false, error: "Timeout (120s)" };
|
||||
return { path: outputPath, success: false, error: `Timeout (${timeoutMs}ms)` };
|
||||
}
|
||||
lastError = err.message;
|
||||
}
|
||||
|
|
@ -144,12 +147,13 @@ export async function variants(options: VariantsOptions): Promise<void> {
|
|||
: parseBrief(options.brief!, false);
|
||||
|
||||
const quality = options.quality || "high";
|
||||
const apiTimeoutMs = options.apiTimeoutMs ?? DEFAULT_IMAGE_GEN_TIMEOUT_MS;
|
||||
|
||||
fs.mkdirSync(options.outputDir, { recursive: true });
|
||||
|
||||
// If viewports specified, generate responsive variants instead of style variants
|
||||
if (options.viewports) {
|
||||
await generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality);
|
||||
await generateResponsiveVariants(apiKey, baseBrief, options.outputDir, options.viewports, quality, apiTimeoutMs);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -176,7 +180,7 @@ export async function variants(options: VariantsOptions): Promise<void> {
|
|||
new Promise(resolve => setTimeout(resolve, delay))
|
||||
.then(() => {
|
||||
console.error(` Starting variant ${String.fromCharCode(65 + i)}...`);
|
||||
return generateVariant(apiKey, prompt, outputPath, size, quality);
|
||||
return generateVariant(apiKey, prompt, outputPath, size, quality, undefined, apiTimeoutMs);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
|
@ -225,6 +229,7 @@ async function generateResponsiveVariants(
|
|||
outputDir: string,
|
||||
viewports: string,
|
||||
quality: string,
|
||||
timeoutMs: number,
|
||||
): Promise<void> {
|
||||
const viewportList = viewports.split(",").map(v => v.trim().toLowerCase());
|
||||
const configs = viewportList.map(v => VIEWPORT_CONFIGS[v]).filter(Boolean);
|
||||
|
|
@ -250,7 +255,7 @@ async function generateResponsiveVariants(
|
|||
setTimeout(resolve, delay)
|
||||
).then(() => {
|
||||
console.error(` Starting ${config.desc}...`);
|
||||
return generateVariant(apiKey, prompt, outputPath, config.size, quality);
|
||||
return generateVariant(apiKey, prompt, outputPath, config.size, quality, undefined, timeoutMs);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,82 @@
|
|||
/**
|
||||
* Regression coverage for issue #1519 — image-generation calls were timing
|
||||
* out at a hardcoded 120s with no CLI override. The fix raised the default
|
||||
* and exposed a per-invocation override via `--api-timeout`.
|
||||
*
|
||||
* These tests pin the contract that the constant exports at the new value
|
||||
* and that the `timeoutMs` param is honored by the AbortController path.
|
||||
*/
|
||||
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
|
||||
import fs from "fs";
|
||||
import os from "os";
|
||||
import path from "path";
|
||||
import { generateVariant } from "../src/variants";
|
||||
import { DEFAULT_IMAGE_GEN_TIMEOUT_MS } from "../src/constants";
|
||||
|
||||
describe("DEFAULT_IMAGE_GEN_TIMEOUT_MS", () => {
|
||||
test("default is 300_000ms (5min) — see issue #1519", () => {
|
||||
expect(DEFAULT_IMAGE_GEN_TIMEOUT_MS).toBe(300_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("generateVariant timeoutMs override", () => {
|
||||
let tmpDir: string;
|
||||
let outputPath: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "api-timeout-"));
|
||||
outputPath = path.join(tmpDir, "variant.png");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("aborts after explicit timeoutMs when fetch never resolves", async () => {
|
||||
// Stub fetch that waits for the signal to abort, then throws AbortError.
|
||||
const fetchFn = (async (_input: any, init?: any) => {
|
||||
const signal = init?.signal as AbortSignal | undefined;
|
||||
await new Promise<void>((_resolve, reject) => {
|
||||
if (!signal) return;
|
||||
signal.addEventListener("abort", () => {
|
||||
const err: any = new Error("aborted");
|
||||
err.name = "AbortError";
|
||||
reject(err);
|
||||
});
|
||||
});
|
||||
return new Response("never reached");
|
||||
}) as typeof globalThis.fetch;
|
||||
|
||||
const t0 = Date.now();
|
||||
const result = await generateVariant(
|
||||
"fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn, 200,
|
||||
);
|
||||
const elapsed = Date.now() - t0;
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe("Timeout (200ms)");
|
||||
// Was aborted by the 200ms timer, not by exponential-backoff retry chain
|
||||
expect(elapsed).toBeLessThan(2_000);
|
||||
});
|
||||
|
||||
test("default timeoutMs is the shared constant when omitted", async () => {
|
||||
// 1x1 transparent PNG, base64
|
||||
const TINY_PNG_BASE64 =
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=";
|
||||
const fetchFn = (async () =>
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
output: [{ type: "image_generation_call", result: TINY_PNG_BASE64 }],
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } },
|
||||
)) as typeof globalThis.fetch;
|
||||
|
||||
// Should succeed using the default timeout — fetch resolves instantly here,
|
||||
// so the timeoutMs value only matters for the AbortController setup not firing.
|
||||
const result = await generateVariant(
|
||||
"fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue