From 68f646d1e51b938c8f8ef8381a2fd26f0e665992 Mon Sep 17 00:00:00 2001 From: Jayesh Betala Date: Wed, 17 Jun 2026 12:32:19 +0530 Subject: [PATCH] fix(design/variants): guard --count against NaN/zero/negative input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `design variants --count ` silently produced zero variants because the generation loop only clamped the upper bound. `--count` arrives via `parseInt`, which yields NaN for non-numeric input ("abc") and passes zero/negative values straight through. `Math.min(NaN, 7)` is NaN, so the `for (i = 0; i < NaN; i++)` loop never ran and the JSON result emitted `"count": null` — a confusing success-with-nothing for any caller parsing it. Add an exported `normalizeVariantCount` helper that clamps to the supported [1, 7] range and falls back to the default of 3 for non-finite input, then use it where the bare upper-bound cap was. Pure and unit-tested so the behavior is pinned without an API key. Fixes #2032 Co-Authored-By: Claude Opus 4.8 (1M context) --- design/src/variants.ts | 19 ++++++++++++- design/test/variants-retry-after.test.ts | 34 +++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/design/src/variants.ts b/design/src/variants.ts index 257079dea..fc63423a0 100644 --- a/design/src/variants.ts +++ b/design/src/variants.ts @@ -19,6 +19,23 @@ export interface VariantsOptions { viewports?: string; // "desktop,tablet,mobile" — generates at multiple sizes } +/** + * Normalize a requested variant count to the supported range. + * + * `--count` reaches us via `parseInt`, which yields `NaN` for non-numeric + * input and happily passes zero/negative values through. The generation loop + * only capped the upper bound, so any of those silently produced zero variants + * and emitted `"count": null`/`0`/negative in the JSON result. Clamp to the + * supported `[1, 7]` range; fall back to the default of 3 when the value is not + * a finite number (an unparseable flag, not an explicit out-of-range request). + * + * Exported for direct unit testing. + */ +export function normalizeVariantCount(count: number): number { + if (!Number.isFinite(count)) return 3; + return Math.max(1, Math.min(Math.trunc(count), 7)); +} + const STYLE_VARIATIONS = [ "", // First variant uses the brief as-is "Use a bolder, more dramatic visual style with stronger contrast and larger typography.", @@ -153,7 +170,7 @@ export async function variants(options: VariantsOptions): Promise { return; } - const count = Math.min(options.count, 7); // Cap at 7 style variations + const count = normalizeVariantCount(options.count); // Clamp to [1, 7] style variations const size = options.size || "1536x1024"; console.error(`Generating ${count} variants...`); diff --git a/design/test/variants-retry-after.test.ts b/design/test/variants-retry-after.test.ts index 2060791d5..1c2b3d5da 100644 --- a/design/test/variants-retry-after.test.ts +++ b/design/test/variants-retry-after.test.ts @@ -2,7 +2,7 @@ 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 { generateVariant, normalizeVariantCount } from "../src/variants"; // 1x1 transparent PNG, base64 — valid bytes that fs.writeFileSync can write. const TINY_PNG_BASE64 = @@ -131,3 +131,35 @@ describe("generateVariant Retry-After handling", () => { expect(gap).toBeLessThan(500); }); }); + +describe("normalizeVariantCount", () => { + test("non-numeric (NaN) falls back to the default of 3", () => { + // Regression: `--count abc` -> parseInt -> NaN -> Math.min(NaN, 7) -> NaN, + // so the loop ran zero times and the JSON result emitted `"count": null`. + expect(normalizeVariantCount(NaN)).toBe(3); + expect(normalizeVariantCount(Number.parseInt("abc", 10))).toBe(3); + }); + + test("zero and negative counts clamp up to the minimum of 1", () => { + // Regression: `--count 0` / `--count -2` produced zero variants. + expect(normalizeVariantCount(0)).toBe(1); + expect(normalizeVariantCount(-2)).toBe(1); + }); + + test("counts above the cap clamp down to 7", () => { + expect(normalizeVariantCount(10)).toBe(7); + expect(normalizeVariantCount(7)).toBe(7); + }); + + test("in-range counts pass through unchanged", () => { + expect(normalizeVariantCount(1)).toBe(1); + expect(normalizeVariantCount(3)).toBe(3); + expect(normalizeVariantCount(6)).toBe(6); + }); + + test("fractional counts truncate toward zero before clamping", () => { + expect(normalizeVariantCount(2.9)).toBe(2); + expect(normalizeVariantCount(0.5)).toBe(1); + expect(normalizeVariantCount(Infinity)).toBe(3); + }); +});