Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Here's a visual recap of what changed: Open the full interactive recap |
There was a problem hiding this comment.
Builder reviewed your changes and found 5 potential issues 🟡
Review Details
This PR adds Builder-managed gpt-image-1 / gpt-image-2 image support, introduces dedicated brand-kit preset editor routes for skeleton workflows, and threads those presets through generation, reference selection, and run refresh behavior. The overall direction is sound: the new preset surface, mask-processing helpers, and managed-provider integration are coherent, and the added tests improve confidence around the provider path.
Risk assessment: Standard (medium). The change set is UI-heavy, but it also alters core generation behavior, provider request construction, and run lifecycle handling.
Key findings:
- 🟡
gpt-image-2skeleton runs currently replace the normal reference set with only the plate + mask, so explicit/source/subject/style references silently stop affecting output on the inpaint path. - 🟡 The new preset editor does not round-trip the full stored
skeletonSpec, so editing an existing advanced skeleton preset can silently deletecontentRegionand non-logo foreground layers. - 🟡 Fill skeleton presets can still select
gpt-image-2, but generation routes them into mask inpainting anyway, which changes semantics and can fail on opaque plates. - 🟡 The stale-run watchdog is now shorter than the managed-image wait budget, so slow-but-healthy runs can be marked failed prematurely.
Good patterns: the builder/OpenAI model plumbing is centralized cleanly, the mask extraction helpers are straightforward, and the dedicated preset route is a good UX improvement.
🧪 Browser testing: Will run after this review (PR touches UI code)
| if (useSkeletonInpaint) { | ||
| references = await inpaintReferencesForSkeleton(skeletonAssets); |
There was a problem hiding this comment.
🟡 gpt-image-2 skeleton inpaint drops non-skeleton references
This branch replaces the normal selectReferences() result with only edit_target and mask, so referenceAssetIds, sourceAssetId / subjectAssetId, and automatic style anchors are silently ignored on the inpaint path. Users can supply references that affect every other generation mode but have no effect here.
Additional Info
Reported by 2/4 review agents; manually verified that only the non-inpaint branch loads explicit/source/subject/style references.
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🟡
Review Details
This incremental update materially improves the original PR: the generation path now gates gpt-image-2 inpaint to cutout skeletons, the preset editor preserves existing contentRegion / non-logo foreground data on save, save-time mask dimension validation was added, and the server-side stale image timeout was extended to match the longer managed-generation budget. Overall the architecture remains sound: preset configuration, generation dispatch, and image-processing responsibilities are still separated cleanly.
Risk assessment: Standard (medium), because these changes still touch generation behavior, preset persistence, and live run lifecycle.
New findings from this incremental pass:
- 🟡 The client generation tray still treats pending runs as stale after 6 minutes, while the server now waits 10 minutes, so slow-but-healthy runs can still surface inconsistent client/server status.
- 🟡 The new skeleton validation helper only runs when both background and mask are present, so background-only skeleton presets can still save invalid cross-library plate references and fail later at generation time.
- 🟡 The preset editor still enables “Place logo in skeleton” for
gpt-image-2cutout presets even though the inpaint path skips local foreground compositing, so that checkbox is silently ignored.
I also verified and resolved four previously posted comments that are now fixed; the remaining unresolved reference-handling comment was not reposted.
🧪 Browser testing: Skipped — browser automation tooling was unavailable to executor sessions in the prior UI verification attempt, so this incremental pass could not run live browser checks.
| // Only reconcile a still-pending slot once it is old enough that the run cannot | ||
| // plausibly still be generating. Keep this above the server generation budget | ||
| // (IMAGE_GENERATION_REQUEST_TIMEOUT_MS, default 300s) and in step with | ||
| // STALE_IMAGE_RUN_MS in refresh-generation-run, so a slow-but-healthy run (e.g. | ||
| // gpt-image-2) is not flagged "interrupted" and flipped to an error slot before | ||
| // its finished image arrives. | ||
| const STALE_PENDING_RUN_MS = 6 * 60 * 1000; |
There was a problem hiding this comment.
🟡 Client stale-run threshold is no longer aligned with server timeout
The tray still treats pending runs as stale after 6 minutes, but refresh-generation-run now waits 10 minutes before failing them. That 6-10 minute gap can still make slow-but-healthy gpt-image-* runs look inconsistent between the live client state and the server reconciler.
Additional Info
Reported by 1/4 review agents and manually verified: GenerationResults.tsx keeps STALE_PENDING_RUN_MS at 6 min while refresh-generation-run.ts now uses 10 min.
| const maskAssetId = | ||
| skeleton.mask?.type === "asset" && typeof skeleton.mask.assetId === "string" | ||
| ? skeleton.mask.assetId | ||
| : ""; | ||
| if (!backgroundAssetId || !maskAssetId) return; |
There was a problem hiding this comment.
🟡 Background-only skeleton presets bypass the new save-time validation
This helper returns early unless both a background and mask are present, so the common background-only skeleton case never verifies that the plate belongs to the same library. Invalid plate references can still be saved and only fail later when generation loads skeleton assets.
Additional Info
Reported by 3/4 review agents and manually verified against create/update-generation-preset callers plus the early return in _preset-skeleton-validation.ts.
| <Checkbox | ||
| id="preset-skeleton-logo" | ||
| checked={form.skeletonLogo} | ||
| disabled={readOnly || !library?.canonicalLogoAssetId} |
There was a problem hiding this comment.
🟡 Skeleton logo toggle is silently ignored for gpt-image-2 cutout presets
The editor still lets users enable “Place logo in skeleton” for gpt-image-2 cutout presets, but the inpaint path skips local foreground compositing entirely. That means the saved checkbox has no effect during generation and the logo never appears.
Additional Info
Reported by 1/4 review agents and manually verified against the checkbox here plus generate-image.ts, where gpt-image-2 inpaint uses only plate+mask references and skips applyPresetSkeleton().

Adds Builder-managed
gpt-image-1andgpt-image-2support, plus dedicated brand kit preset editing pages for skeleton workflows. Presets can now use uploaded brand plates, transparent gpt-image-1 cutouts, or gpt-image-2 source + mask inpainting for plate-aware generation.Changes
gpt-image-1andgpt-image-2to Builder image model support and aspect-ratio handling.gpt-image-1with transparent PNG output.gpt-image-2skeleton generations through managed edit/inpaint mode with plate source + mask references.