Derive commission form generation fields from a zod-free server spec leaf - #6823
Merged
Conversation
…eaf (#6816) server/lib/creativeCommissionValidation.js pulls zod, so commissionForm.js could only mirror its per-key type/bounds/default spec by hand. That hand copy had already drifted three ways: durationMode's default (client 'auto' vs server 'manual'), the video backend enum (client missing fal/reactor), and the image backend enum (client missing agy). Move GENERATION_KEY_DEFS, ABILITY_GENERATION_SPEC, and the quality/aspect ratio/backend enums into a new dependency-free leaf, server/lib/creativeCommissionSpec.js (the creativeBriefLimits.js precedent). creativeCommissionValidation.js now imports and re-exports them, still building the same Zod schema from GENERATION_KEY_DEFS. Also resolves the durationMode compatibility split explicitly: the descriptor now carries both `default` ('auto', the fresh-commission seed from #4494) and `legacyAbsent` ('manual', what a record with no key at all has always meant). abilityAdapters.js's coerceGenerationValue and its three raw duration reads now resolve an absent value through this split (resolvedDefault / GENERATION_KEY_DEFS.durationMode.legacyAbsent) instead of a bare `.default`, so stored-record behavior is unchanged.
…ec leaf (#6816) commissionForm.js hand-mirrored server/lib/creativeCommissionValidation.js's generation-key bounds/defaults/options because that module pulled zod. Now that the spec lives in the zod-free creativeCommissionSpec.js, the form imports GENERATION_KEY_DEFS / ABILITY_GENERATION_SPEC directly and builds GENERATION_FIELDS_BY_ABILITY / GENERATION_DEFAULTS_BY_ABILITY from it, keeping only display labels and the render-backend-pin wiring as client-side presentation metadata. IMAGE_BACKEND_OPTIONS / VIDEO_BACKEND_OPTIONS now map over the server's mode enums via the shared MODE_LABELS (client/src/lib/imageGenModes.js), so the form immediately offers agy (image) and fal/reactor (video) instead of needing a second hand-edit whenever generationModes.js gains a backend. generationToForm now projects a REAL record's absent key through the spec's legacyAbsent reading (falls back to ABILITY_GENERATION_SPEC's defaults, which prefer legacyAbsent over default) rather than the fresh-commission default, so an existing record with no durationMode still projects as 'manual' — matching abilityAdapters.js. A brand-new/blank commission (toForm({}) / blankForm()) is unaffected: it seeds every key from GENERATION_DEFAULTS_BY_ABILITY's plain default ('auto' for durationMode), replacing the old ad hoc video/music-video-only Object.hasOwn check with a general mechanism that works for any key a future spec entry marks legacyAbsent. Also switches the RENDER_BACKEND_AUTO import from imageGenBackends.js (which pulls lucide-react) to its node-safe half, imageGenModes.js — required for the server-workspace parity test landing in the next commit to import this module at all.
… spec (#6816) No test previously compared commissionForm.js's hand-copied generation fields/defaults/backend options against the server's actual spec — that gap is how the durationMode default, the video backend enum (fal/reactor), and the image backend enum (agy) drifted silently for months. creativeCommissionSpec.parity.test.js imports commissionForm.js from the server workspace (the reviewerConfig.test.js pattern) and asserts every ability field's bounds/options/defaults match GENERATION_KEY_DEFS, every non-auto backend mode has a real client label, and an absent durationMode resolves identically on the ability adapter and the client form. Raises the server suite's static-import budget (importScoping.test.js) by the standard ~1.5k allowance: the new creativeCommissionSpec.js leaf adds +1 to every closure already reaching creativeCommissionValidation.js (itself widely reached through validation.js's flat re-export) without pulling in anything those closures didn't already have — the same shape as #6590/#6617.
- commissionForm.js: reuse the shared modeLabel() helper (client/src/lib/
imageGenModes.js) instead of reaching into MODE_LABELS directly, and drop
a redundant `|| 'video'` fallback toForm() no longer needs now that
abilityOr() already covers it.
- abilityAdapters.js: isAutoDuration() now delegates to the same
coerceGenerationValue() every other key's absent-value resolution already
goes through, instead of a bespoke `??` on the raw value — also makes it
correctly resolve an INVALID (not just absent) durationMode.
- creativeCommissionSpec.parity.test.js: drop assertions that could only
ever pass or fail together (blankForm() vs toForm({}); video vs
music-video for a per-key, ability-agnostic resolution), and add coverage
for the plain-select option labels (quality/durationMode) alongside the
existing backend-mode label check.
- commissionForm.test.js: add the agy/fal/reactor round-trip case the
acceptance criteria named explicitly.
…tion as legacy, not fresh
toForm()'s blank-vs-real branch keyed only on `c.generation == null`, so a
persisted commission whose generation object happened to be absent or null
would have been mis-seeded with the fresh-commission default (durationMode
'auto') instead of the legacy reading ('manual') abilityAdapters.js already
applies to such a record. Every real API response always sends a populated
generation object today, so this wasn't reachable through the app, but the
helper's own contract should not depend on that.
Add `&& !c.id` to the blank check — ids are minted server-side and never
present on an unsaved commission, so this distinguishes "not yet persisted"
from "persisted but sparse" without relying on `generation` alone. Verified
against the existing CommissionConfigForm harness that pre-seeds a NEW
commission with just a brief (no id, no generation) — that still correctly
seeds the fresh default.
6 tasks
… defaults (#6816) mergeGenerationForAbility was a bare alias of generationToForm, so once that projection became legacy-aware a form switched from Image to Video seeded durationMode as 'manual' — on a brand-new commission too — where #4494's product choice is 'Creative Director chooses'. A key the switch introduces was never on the record, so the legacy reading does not describe it: the switch now falls back to the same fresh defaults a blank form gets, while a stored record with the key missing still projects as 'manual'. The two projections share one filler and differ only in which defaults they fall back to.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GENERATION_KEY_DEFS,ABILITY_GENERATION_SPEC, and the quality/aspect-ratio/image-mode/video-mode enums) out ofserver/lib/creativeCommissionValidation.js(which pullszod) into a new zod-free leaf,server/lib/creativeCommissionSpec.js— the same pattern ascreativeBriefLimits.js.client/src/components/creative-commission/commissionForm.jsto derive its generation fields, bounds, defaults, and backend-mode options directly from that leaf instead of hand-copying them, keeping only display labels and the render-backend-pin wiring as client-side presentation metadata.generationModes.jsreaches the form automatically via the sharedmodeLabellookup.durationModecompatibility split explicitly: the descriptor now carries bothdefault('auto', the fresh-commission seed from feat: let creative director choose video duration #4494) andlegacyAbsent('manual', what a stored record with no key has always meant). The server's ability-adapter sanitizer and the client's record projection both resolve an absent value through this same split, so a legacy record still projects/renders as a fixed length while a brand-new commission still seeds "Creative Director chooses."server/lib/creativeCommissionSpec.parity.test.js, importing the client module from the server workspace, asserting every ability field's bounds/options/defaults match the spec, every non-auto backend mode has a real label, and the durationMode split resolves identically on both sides.Test plan
server/lib/creativeCommissionSpec.parity.test.js,server/lib/index.test.js,server/lib/importScoping.test.js,server/lib/validation.test.js,server/services/creativeCommissions/{abilityAdapters,store,directive,scheduler}.test.js,server/routes/creativeCommissions.test.js— all green (536 tests).client/src/components/creative-commission/{commissionForm,CommissionConfigForm}.test.jsx,client/src/pages/{CreativeCommissionDetail,CreativeCommissions}.test.jsx,client/src/lib/index.test.js— all green (63 tests), including new coverage for the durationMode legacy split and the agy/fal/reactor round-trip.vite buildsucceeds; the commission-form bundle chunk carries nozod/child_process/node:fsreferences.Closes #6816