mirror of https://github.com/garrytan/gstack.git
fix(design/variants): guard --count against NaN/zero/negative input
`design variants --count <bad value>` 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) <noreply@anthropic.com>
This commit is contained in:
parent
c7ae63201a
commit
68f646d1e5
|
|
@ -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<void> {
|
|||
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...`);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue