From 0f3ec369bfdecde0b1a3fa79564856424a2adcef Mon Sep 17 00:00:00 2001 From: Thomas Hallock Date: Tue, 11 Nov 2025 18:59:44 -0600 Subject: [PATCH] fix: consolidate worksheet validation constants and increase MAX_PAGES to 100 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Problem**: Worksheet seeds were not persisting because configs with >20 pages failed Zod schema validation (max was 20), even though the code elsewhere allowed up to 2000 total problems. When validation failed, the config fell back to defaults, losing the saved seed. **Changes**: 1. Created `constants/validation.ts` with centralized limits: - MAX_PAGES: 100 (was 20 in schemas) - MAX_TOTAL_PROBLEMS: 2000 - MAX_PROBLEMS_PER_PAGE: 100 - MAX_COLS: 10 - DIGIT_RANGE: { MIN: 1, MAX: 5 } - FONT_SIZE: { MIN: 8, MAX: 32 } - Helper function validateWorksheetLimits() 2. Updated all Zod schema versions (V1-V4) in `config-schemas.ts` to use constants 3. Updated runtime validation in `validation.ts` to use constants 4. Enhanced settings API (`route.ts`) to: - Validate worksheet limits before saving - Validate against Zod schema - Return clear error messages with actionable guidance 5. Enhanced auto-save hook (`useWorksheetAutoSave.ts`) to: - Return saveError state - Surface validation errors to user - Clear errors on successful save 6. Removed TEN-FRAMES DEBUG logging from displayRules.ts **Impact**: Worksheet configs with 21-100 pages will now pass validation and persist correctly, including the seed. Users will see clear error messages if their config exceeds limits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/app/api/worksheets/settings/route.ts | 32 ++++++++ .../app/create/worksheets/config-schemas.ts | 61 +++++++++++----- .../create/worksheets/constants/validation.ts | 73 +++++++++++++++++++ .../src/app/create/worksheets/displayRules.ts | 13 +--- .../worksheets/hooks/useWorksheetAutoSave.ts | 15 +++- .../src/app/create/worksheets/validation.ts | 39 +++++++--- 6 files changed, 190 insertions(+), 43 deletions(-) create mode 100644 apps/web/src/app/create/worksheets/constants/validation.ts diff --git a/apps/web/src/app/api/worksheets/settings/route.ts b/apps/web/src/app/api/worksheets/settings/route.ts index 47167326..655b64ce 100644 --- a/apps/web/src/app/api/worksheets/settings/route.ts +++ b/apps/web/src/app/api/worksheets/settings/route.ts @@ -6,7 +6,12 @@ import { parseAdditionConfig, serializeAdditionConfig, defaultAdditionConfig, + additionConfigSchema, } from '@/app/create/worksheets/config-schemas' +import { + WORKSHEET_LIMITS, + validateWorksheetLimits, +} from '@/app/create/worksheets/constants/validation' /** * GET /api/worksheets/settings?type=addition @@ -105,6 +110,33 @@ export async function POST(req: NextRequest) { ) } + // Validate worksheet limits before saving + const validation = validateWorksheetLimits(config.problemsPerPage, config.pages) + if (!validation.valid) { + return NextResponse.json( + { + success: false, + error: validation.error, + }, + { status: 400 } + ) + } + + // Validate against schema (this will check all field types and ranges) + const schemaValidation = additionConfigSchema.safeParse({ ...config, version: 4 }) + if (!schemaValidation.success) { + const errorMessages = schemaValidation.error.issues + .map((err) => `${err.path.join('.')}: ${err.message}`) + .join(', ') + return NextResponse.json( + { + success: false, + error: `Invalid configuration: ${errorMessages}`, + }, + { status: 400 } + ) + } + // Serialize config (adds version automatically) const configJson = serializeAdditionConfig(config) diff --git a/apps/web/src/app/create/worksheets/config-schemas.ts b/apps/web/src/app/create/worksheets/config-schemas.ts index ac074711..8fc85bea 100644 --- a/apps/web/src/app/create/worksheets/config-schemas.ts +++ b/apps/web/src/app/create/worksheets/config-schemas.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import { getProfileFromConfig } from './difficultyProfiles' +import { WORKSHEET_LIMITS } from './constants/validation' /** * Versioned worksheet config schemas with type-safe validation and migration @@ -29,9 +30,9 @@ const ADDITION_CURRENT_VERSION = 4 */ export const additionConfigV1Schema = z.object({ version: z.literal(1), - problemsPerPage: z.number().int().min(1).max(100), - cols: z.number().int().min(1).max(10), - pages: z.number().int().min(1).max(20), + problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE), + cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS), + pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES), orientation: z.enum(['portrait', 'landscape']), name: z.string(), pAnyStart: z.number().min(0).max(1), @@ -44,7 +45,11 @@ export const additionConfigV1Schema = z.object({ showCellBorder: z.boolean(), showTenFrames: z.boolean(), showTenFramesForAll: z.boolean(), - fontSize: z.number().int().min(8).max(32), + fontSize: z + .number() + .int() + .min(WORKSHEET_LIMITS.FONT_SIZE.MIN) + .max(WORKSHEET_LIMITS.FONT_SIZE.MAX), }) export type AdditionConfigV1 = z.infer @@ -55,9 +60,9 @@ export type AdditionConfigV1 = z.infer */ export const additionConfigV2Schema = z.object({ version: z.literal(2), - problemsPerPage: z.number().int().min(1).max(100), - cols: z.number().int().min(1).max(10), - pages: z.number().int().min(1).max(20), + problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE), + cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS), + pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES), orientation: z.enum(['portrait', 'landscape']), name: z.string(), pAnyStart: z.number().min(0).max(1), @@ -128,7 +133,11 @@ export const additionConfigV2Schema = z.object({ difficultyProfile: z.string().optional(), // V2: Keep fontSize and showTenFramesForAll for now (may refactor later) - fontSize: z.number().int().min(8).max(32), + fontSize: z + .number() + .int() + .min(WORKSHEET_LIMITS.FONT_SIZE.MIN) + .max(WORKSHEET_LIMITS.FONT_SIZE.MAX), showTenFramesForAll: z.boolean(), }) @@ -144,12 +153,16 @@ const additionConfigV3BaseSchema = z.object({ version: z.literal(3), // Core worksheet settings - problemsPerPage: z.number().int().min(1).max(100), - cols: z.number().int().min(1).max(10), - pages: z.number().int().min(1).max(20), + problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE), + cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS), + pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES), orientation: z.enum(['portrait', 'landscape']), name: z.string(), - fontSize: z.number().int().min(8).max(32), + fontSize: z + .number() + .int() + .min(WORKSHEET_LIMITS.FONT_SIZE.MIN) + .max(WORKSHEET_LIMITS.FONT_SIZE.MAX), // Regrouping probabilities (shared between modes) pAnyStart: z.number().min(0).max(1), @@ -265,18 +278,30 @@ const additionConfigV4BaseSchema = z.object({ version: z.literal(4), // Core worksheet settings - problemsPerPage: z.number().int().min(1).max(100), - cols: z.number().int().min(1).max(10), - pages: z.number().int().min(1).max(20), + problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE), + cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS), + pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES), orientation: z.enum(['portrait', 'landscape']), name: z.string(), - fontSize: z.number().int().min(8).max(32), + fontSize: z + .number() + .int() + .min(WORKSHEET_LIMITS.FONT_SIZE.MIN) + .max(WORKSHEET_LIMITS.FONT_SIZE.MAX), // V4: Digit range for problem generation digitRange: z .object({ - min: z.number().int().min(1).max(5), - max: z.number().int().min(1).max(5), + min: z + .number() + .int() + .min(WORKSHEET_LIMITS.DIGIT_RANGE.MIN) + .max(WORKSHEET_LIMITS.DIGIT_RANGE.MAX), + max: z + .number() + .int() + .min(WORKSHEET_LIMITS.DIGIT_RANGE.MIN) + .max(WORKSHEET_LIMITS.DIGIT_RANGE.MAX), }) .refine((data) => data.min <= data.max, { message: 'min must be less than or equal to max', diff --git a/apps/web/src/app/create/worksheets/constants/validation.ts b/apps/web/src/app/create/worksheets/constants/validation.ts new file mode 100644 index 00000000..6968b8e0 --- /dev/null +++ b/apps/web/src/app/create/worksheets/constants/validation.ts @@ -0,0 +1,73 @@ +/** + * Validation constants for worksheet generation + * + * These constants define the limits for worksheet configuration. + * Keep these in sync across: + * - Zod schemas (config-schemas.ts) + * - Runtime validation (validation.ts) + * - UI components (forms, sliders, etc.) + */ + +export const WORKSHEET_LIMITS = { + /** Maximum total problems across all pages */ + MAX_TOTAL_PROBLEMS: 2000, + + /** Maximum problems per page */ + MAX_PROBLEMS_PER_PAGE: 100, + + /** Maximum number of pages */ + MAX_PAGES: 100, + + /** Maximum columns per page */ + MAX_COLS: 10, + + /** Minimum/maximum digit range for problems */ + DIGIT_RANGE: { + MIN: 1, + MAX: 5, + }, + + /** Font size limits */ + FONT_SIZE: { + MIN: 8, + MAX: 32, + }, +} as const + +/** + * Validate that worksheet config doesn't exceed limits + * + * IMPORTANT: problemsPerPage * pages must not exceed MAX_TOTAL_PROBLEMS + */ +export function validateWorksheetLimits( + problemsPerPage: number, + pages: number +): { + valid: boolean + error?: string +} { + const total = problemsPerPage * pages + + if (total > WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS) { + return { + valid: false, + error: `Total problems (${total}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS}`, + } + } + + if (problemsPerPage > WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE) { + return { + valid: false, + error: `Problems per page (${problemsPerPage}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE}`, + } + } + + if (pages > WORKSHEET_LIMITS.MAX_PAGES) { + return { + valid: false, + error: `Pages (${pages}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_PAGES}`, + } + } + + return { valid: true } +} diff --git a/apps/web/src/app/create/worksheets/displayRules.ts b/apps/web/src/app/create/worksheets/displayRules.ts index 47d4da32..fcc7cc0b 100644 --- a/apps/web/src/app/create/worksheets/displayRules.ts +++ b/apps/web/src/app/create/worksheets/displayRules.ts @@ -68,7 +68,7 @@ export function resolveDisplayForProblem( rules: DisplayRules, problem: AnyProblemMeta ): ResolvedDisplayOptions { - const resolved = { + return { showCarryBoxes: evaluateRule(rules.carryBoxes, problem), showAnswerBoxes: evaluateRule(rules.answerBoxes, problem), showPlaceValueColors: evaluateRule(rules.placeValueColors, problem), @@ -78,15 +78,4 @@ export function resolveDisplayForProblem( showBorrowNotation: evaluateRule(rules.borrowNotation, problem), showBorrowingHints: evaluateRule(rules.borrowingHints, problem), } - - // DEBUG: Ten-frames evaluation - console.log('[TEN-FRAMES DEBUG]', { - rule: rules.tenFrames, - requiresRegrouping: - 'requiresRegrouping' in problem ? problem.requiresRegrouping : problem.requiresBorrowing, - regroupCount: 'regroupCount' in problem ? problem.regroupCount : problem.borrowCount, - resolved: resolved.showTenFrames, - }) - - return resolved } diff --git a/apps/web/src/app/create/worksheets/hooks/useWorksheetAutoSave.ts b/apps/web/src/app/create/worksheets/hooks/useWorksheetAutoSave.ts index 9c5dad68..a3c6e7b5 100644 --- a/apps/web/src/app/create/worksheets/hooks/useWorksheetAutoSave.ts +++ b/apps/web/src/app/create/worksheets/hooks/useWorksheetAutoSave.ts @@ -7,6 +7,7 @@ import { extractConfigFields } from '../utils/extractConfigFields' interface UseWorksheetAutoSaveReturn { isSaving: boolean lastSaved: Date | null + saveError: string | null } /** @@ -26,6 +27,7 @@ export function useWorksheetAutoSave( ): UseWorksheetAutoSaveReturn { const [isSaving, setIsSaving] = useState(false) const [lastSaved, setLastSaved] = useState(null) + const [saveError, setSaveError] = useState(null) // Store the previous formState for auto-save to detect real changes const prevAutoSaveFormStateRef = useRef(formState) @@ -45,6 +47,7 @@ export function useWorksheetAutoSave( const timer = setTimeout(async () => { console.log('[useWorksheetAutoSave] Attempting to save settings...') setIsSaving(true) + setSaveError(null) // Clear previous errors try { // Extract persisted config fields (includes seed/prngAlgorithm, excludes date and derived state) const config = extractConfigFields(formState) @@ -64,15 +67,24 @@ export function useWorksheetAutoSave( if (data.success) { console.log('[useWorksheetAutoSave] ✓ Settings saved successfully') setLastSaved(new Date()) + setSaveError(null) + } else if (data.error) { + // Validation error from server + console.error('[useWorksheetAutoSave] Validation error:', data.error) + setSaveError(data.error) } else { console.log('[useWorksheetAutoSave] Save skipped') } } else { + const errorText = await response.text() console.error('[useWorksheetAutoSave] Save failed with status:', response.status) + setSaveError(`Failed to save settings (${response.status}): ${errorText}`) } } catch (error) { - // Silently fail - settings persistence is not critical + // Surface error to user + const errorMessage = error instanceof Error ? error.message : 'Unknown error' console.error('[useWorksheetAutoSave] Settings save error:', error) + setSaveError(`Failed to save settings: ${errorMessage}`) } finally { setIsSaving(false) } @@ -84,5 +96,6 @@ export function useWorksheetAutoSave( return { isSaving, lastSaved, + saveError, } } diff --git a/apps/web/src/app/create/worksheets/validation.ts b/apps/web/src/app/create/worksheets/validation.ts index dad34edc..1d5972ea 100644 --- a/apps/web/src/app/create/worksheets/validation.ts +++ b/apps/web/src/app/create/worksheets/validation.ts @@ -7,6 +7,7 @@ import type { } from '@/app/create/worksheets/types' import type { DisplayRules } from './displayRules' import { getSkillById } from './skills' +import { WORKSHEET_LIMITS } from './constants/validation' /** * Get current date formatted as "Month Day, Year" @@ -28,14 +29,14 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati // Validate total (must be positive, reasonable limit) const total = formState.total ?? 20 - if (total < 1 || total > 2000) { - errors.push('Total problems must be between 1 and 2000') + if (total < 1 || total > WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS) { + errors.push(`Total problems must be between 1 and ${WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS}`) } // Validate cols and auto-calculate rows const cols = formState.cols ?? 4 - if (cols < 1 || cols > 10) { - errors.push('Columns must be between 1 and 10') + if (cols < 1 || cols > WORKSHEET_LIMITS.MAX_COLS) { + errors.push(`Columns must be between 1 and ${WORKSHEET_LIMITS.MAX_COLS}`) } // Auto-calculate rows to fit all problems @@ -60,18 +61,32 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati // Validate fontSize const fontSize = formState.fontSize ?? 16 - if (fontSize < 8 || fontSize > 32) { - errors.push('Font size must be between 8 and 32') + if (fontSize < WORKSHEET_LIMITS.FONT_SIZE.MIN || fontSize > WORKSHEET_LIMITS.FONT_SIZE.MAX) { + errors.push( + `Font size must be between ${WORKSHEET_LIMITS.FONT_SIZE.MIN} and ${WORKSHEET_LIMITS.FONT_SIZE.MAX}` + ) } // V4: Validate digitRange (min and max must be 1-5, min <= max) // Note: Same range applies to both addition and subtraction const digitRange = formState.digitRange ?? { min: 2, max: 2 } - if (!digitRange.min || digitRange.min < 1 || digitRange.min > 5) { - errors.push('Digit range min must be between 1 and 5') + if ( + !digitRange.min || + digitRange.min < WORKSHEET_LIMITS.DIGIT_RANGE.MIN || + digitRange.min > WORKSHEET_LIMITS.DIGIT_RANGE.MAX + ) { + errors.push( + `Digit range min must be between ${WORKSHEET_LIMITS.DIGIT_RANGE.MIN} and ${WORKSHEET_LIMITS.DIGIT_RANGE.MAX}` + ) } - if (!digitRange.max || digitRange.max < 1 || digitRange.max > 5) { - errors.push('Digit range max must be between 1 and 5') + if ( + !digitRange.max || + digitRange.max < WORKSHEET_LIMITS.DIGIT_RANGE.MIN || + digitRange.max > WORKSHEET_LIMITS.DIGIT_RANGE.MAX + ) { + errors.push( + `Digit range max must be between ${WORKSHEET_LIMITS.DIGIT_RANGE.MIN} and ${WORKSHEET_LIMITS.DIGIT_RANGE.MAX}` + ) } if (digitRange.min > digitRange.max) { errors.push('Digit range min cannot be greater than max') @@ -101,7 +116,7 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati const pages = formState.pages ?? 1 // Determine mode (default to 'smart' if not specified) - const mode = formState.mode ?? 'smart' + const mode: 'smart' | 'manual' | 'mastery' = formState.mode ?? 'smart' // Shared fields for both modes const sharedFields = { @@ -232,7 +247,7 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati if (addSkill?.recommendedScaffolding && subSkill?.recommendedScaffolding) { // Merge user's displayRules with skill's recommended scaffolding // User's displayRules take precedence for problemNumbers and cellBorders (layout options) - const userDisplayRules = formState.displayRules || {} + const userDisplayRules: Partial = formState.displayRules || {} config = { ...baseConfig,