From 50fb7fa89f6ab9a4c99c910823fa1da381b1ab7a Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Fri, 1 May 2026 15:33:18 -0400 Subject: [PATCH 1/5] feat: survey validation enhancements, UI error surfacing, and 422 HTTP error statuses - Add Scale range divisibility and integer constraints validation for Survey questions - Restrict survey stages to contain at least one question - Surface active config errors Lit Header UI editor edit mode and create mode - Support entering floats in frontend UI scale number form fields to trigger float validations - Return HTTP 422 Unprocessable Entity for REST cloud function experiment rejections - Add Jest unit and Express integrations tests covering positive/negative validation scenarios --- docs/assets/api/schemas.json | 1 - frontend/src/components/header/header.ts | 9 +- .../src/components/stages/survey_editor.ts | 9 +- frontend/src/services/experiment.editor.ts | 68 +++++- .../experiments.dl_api.integration.test.ts | 188 ++++++++++++++- functions/src/dl_api/experiments.dl_api.ts | 10 +- functions/src/experiment.utils.ts | 8 +- scripts/deliberate_lab/types.py | 2 +- utils/src/stages/stage.validation.ts | 12 +- .../stages/survey_stage.validation.test.ts | 223 ++++++++++++++++++ utils/src/stages/survey_stage.validation.ts | 96 +++++++- 11 files changed, 592 insertions(+), 34 deletions(-) create mode 100644 utils/src/stages/survey_stage.validation.test.ts diff --git a/docs/assets/api/schemas.json b/docs/assets/api/schemas.json index 6141946e8..8a37a4d95 100644 --- a/docs/assets/api/schemas.json +++ b/docs/assets/api/schemas.json @@ -2311,7 +2311,6 @@ "type": "boolean" }, "stepSize": { - "minimum": 1, "type": "number" }, "condition": { diff --git a/frontend/src/components/header/header.ts b/frontend/src/components/header/header.ts index 50b15d1c9..3a48b5eb0 100644 --- a/frontend/src/components/header/header.ts +++ b/frontend/src/components/header/header.ts @@ -280,7 +280,13 @@ export class Header extends MobxLitElement { if (this.experimentManager.isEditingFull) { if (!this.experimentManager.isCreator) return nothing; + const editorErrors = + this.experimentEditor.getExperimentConfigErrors(); + return html` + ${editorErrors.length === 0 + ? nothing + : html`
⚠️ ${editorErrors.join(', ')}
`} 0} @click=${() => { this.analyticsService.trackButtonClick( ButtonClick.EXPERIMENT_SAVE_EXISTING, diff --git a/frontend/src/components/stages/survey_editor.ts b/frontend/src/components/stages/survey_editor.ts index ead7c418b..138374c7c 100644 --- a/frontend/src/components/stages/survey_editor.ts +++ b/frontend/src/components/stages/survey_editor.ts @@ -463,19 +463,18 @@ export class SurveyEditor extends MobxLitElement { }; const updateLowerValue = (e: InputEvent) => { - const lowerValue = - parseInt((e.target as HTMLInputElement).value, 10) || 0; + const lowerValue = parseFloat((e.target as HTMLInputElement).value) || 0; this.updateQuestion({...question, lowerValue}, index); }; const updateUpperValue = (e: InputEvent) => { - const upperValue = - parseInt((e.target as HTMLInputElement).value, 10) || 10; + const upperValue = parseFloat((e.target as HTMLInputElement).value) || 10; this.updateQuestion({...question, upperValue}, index); }; const updateStepSize = (e: InputEvent) => { - const stepSize = parseInt((e.target as HTMLInputElement).value, 10) || 1; + const val = parseFloat((e.target as HTMLInputElement).value); + const stepSize = isNaN(val) ? 1 : val; this.updateQuestion({...question, stepSize}, index); }; diff --git a/frontend/src/services/experiment.editor.ts b/frontend/src/services/experiment.editor.ts index e59ddac8e..b184069c0 100644 --- a/frontend/src/services/experiment.editor.ts +++ b/frontend/src/services/experiment.editor.ts @@ -118,17 +118,69 @@ export class ExperimentEditor extends Service { renderApiErrorMessage(ApiKeyType.OLLAMA_CUSTOM_URL); for (const stage of this.stages) { - switch (stage.kind) { - case StageKind.SURVEY: - // Ensure all questions have a non-empty title - stage.questions.forEach((question, index) => { - if (question.questionTitle === '') { + if ( + stage.kind === StageKind.SURVEY || + stage.kind === StageKind.SURVEY_PER_PARTICIPANT + ) { + if (!stage.questions || stage.questions.length === 0) { + errors.push(`${stage.name} must contain at least one question`); + } + stage.questions.forEach((question, index) => { + if (question.questionTitle === '') { + errors.push( + `${stage.name} question ${index + 1} is missing a title`, + ); + } + if (question.kind === 'scale') { + if ( + !Number.isInteger(question.lowerValue) || + !Number.isInteger(question.upperValue) || + (question.stepSize !== undefined && + !Number.isInteger(question.stepSize)) + ) { + errors.push( + `${stage.name} question ${index + 1} ("${question.questionTitle}"): values must be integers`, + ); + } + if (question.lowerValue >= question.upperValue) { errors.push( - `${stage.name} question ${index + 1} is missing a title`, + `${stage.name} question ${index + 1} ("${question.questionTitle}"): lower value must be less than upper value`, + ); + } + const range = question.upperValue - question.lowerValue; + const step = question.stepSize ?? 1; + if (step <= 0) { + errors.push( + `${stage.name} question ${index + 1} ("${question.questionTitle}"): step size must be greater than 0`, + ); + } + if (range % step !== 0) { + errors.push( + `${stage.name} question ${index + 1} ("${question.questionTitle}"): step size must divide max-min (${range}) exactly`, + ); + } + } + if (question.kind === 'mc') { + if (!question.options || question.options.length === 0) { + errors.push( + `${stage.name} question ${index + 1} ("${question.questionTitle}"): must have at least one option`, + ); + } + if ( + question.correctAnswerId != null && + question.correctAnswerId !== '' + ) { + const hasOption = question.options.some( + (opt) => opt.id === question.correctAnswerId, ); + if (!hasOption) { + errors.push( + `${stage.name} question ${index + 1} ("${question.questionTitle}"): correct answer ID doesn't match any option ID`, + ); + } } - }); - break; + } + }); } } diff --git a/functions/src/dl_api/experiments.dl_api.integration.test.ts b/functions/src/dl_api/experiments.dl_api.integration.test.ts index 82b3a92fe..ea0ffd63a 100644 --- a/functions/src/dl_api/experiments.dl_api.integration.test.ts +++ b/functions/src/dl_api/experiments.dl_api.integration.test.ts @@ -16,6 +16,8 @@ import { Visibility, createExperimentConfig, createPrivateChatStage, + createSurveyStage, + SurveyQuestionKind, } from '@deliberation-lab/utils'; import { TestContext, @@ -926,7 +928,7 @@ describe('API Experiment Creation Integration Tests', () => { stages: JSON.parse(JSON.stringify([stage1, stage2])), }); - expect(response.status).toBe(400); + expect(response.status).toBe(422); const data = await response.json(); expect(data.error).toContain('Duplicate stage ID'); }); @@ -942,7 +944,7 @@ describe('API Experiment Creation Integration Tests', () => { template: JSON.parse(JSON.stringify(template)), }); - expect(response.status).toBe(400); + expect(response.status).toBe(422); const data = await response.json(); expect(data.error).toContain('Duplicate cohort alias found'); }); @@ -972,13 +974,193 @@ describe('API Experiment Creation Integration Tests', () => { template: JSON.parse(JSON.stringify(template)), }); - expect(response.status).toBe(400); + expect(response.status).toBe(422); const data = await response.json(); expect(data.error).toContain( 'values array must contain at least one item', ); }); + it('should reject experiment creation with invalid survey stage (scale lower >= upper bounds)', async () => { + const stage = createSurveyStage({ + id: 'survey-1', + questions: [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Bad Scale', + lowerValue: 10, + lowerText: '', + upperValue: 5, + upperText: '', + stepSize: 1, + useSlider: false, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Bad Survey bounds Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('greater than or equal to upper value'); + }); + + it('should reject experiment creation with invalid multiple choice survey question (no options)', async () => { + const stage = createSurveyStage({ + id: 'survey-2', + questions: [ + { + id: 'q2', + kind: SurveyQuestionKind.MULTIPLE_CHOICE, + questionTitle: 'Empty Options', + options: [], + correctAnswerId: null, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Bad MC Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must have at least one option'); + }); + + it('should reject experiment creation with survey stage containing no questions', async () => { + const stage = createSurveyStage({ + id: 'survey-3', + questions: [], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Empty Survey Questions Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must contain at least one question'); + }); + + it('should reject experiment creation with non-integer scale question values', async () => { + const stage = createSurveyStage({ + id: 'survey-4', + questions: [ + { + id: 'q4', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Non-Integer Scale', + lowerValue: 1.5, + lowerText: '', + upperValue: 5.5, + upperText: '', + stepSize: 1, + useSlider: false, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Non-Integer Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must be integers'); + }); + + it('should reject experiment creation with scale question step size equal to 0', async () => { + const stage = createSurveyStage({ + id: 'survey-7', + questions: [ + { + id: 'q7', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Step Size 0', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + stepSize: 0, + useSlider: false, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Step Size 0 Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must be greater than 0'); + }); + + it('should reject experiment creation with scale question step size out of bounds', async () => { + const stage = createSurveyStage({ + id: 'survey-5', + questions: [ + { + id: 'q5', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Bad Step Size', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + stepSize: 6, + useSlider: false, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Bad Step Size Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must divide max-min'); + }); + + it('should reject experiment creation with scale question step size not dividing range exactly', async () => { + const stage = createSurveyStage({ + id: 'survey-6', + questions: [ + { + id: 'q6', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Undivisible Step Size', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + stepSize: 3, + useSlider: false, + }, + ], + }); + + const response = await apiRequest('POST', '/v1/experiments', { + name: 'Undivisible Range Experiment', + stages: JSON.parse(JSON.stringify([stage])), + }); + + expect(response.status).toBe(422); + const data = await response.json(); + expect(data.error).toContain('must divide max-min'); + }); + it('should accept experiment creation with valid variable configuration', async () => { const template = getFlipCardExperimentTemplate(); template.experiment.variableConfigs = [ diff --git a/functions/src/dl_api/experiments.dl_api.ts b/functions/src/dl_api/experiments.dl_api.ts index 79f793cbd..6c15d55d0 100644 --- a/functions/src/dl_api/experiments.dl_api.ts +++ b/functions/src/dl_api/experiments.dl_api.ts @@ -137,7 +137,7 @@ export async function createExperiment( const error = [...Value.Errors(CreateExperimentRequestData, body)] .map((e) => `${e.path || '/'}: ${e.message}`) .join('; '); - throw createHttpError(400, `Invalid request body: ${error}`); + throw createHttpError(422, `Invalid request body: ${error}`); } // If a full template is provided, use the shared utility @@ -151,7 +151,7 @@ export async function createExperiment( ); } catch (error: unknown) { throw createHttpError( - 400, + 422, error instanceof Error ? error.message : String(error), ); } @@ -173,7 +173,7 @@ export async function createExperiment( // Simple creation mode: name is required if (!body.name) { throw createHttpError( - 400, + 422, 'Invalid request body: name is required (or provide template)', ); } @@ -215,7 +215,7 @@ export async function createExperiment( ); } catch (error: unknown) { throw createHttpError( - 400, + 422, error instanceof Error ? error.message : String(error), ); } @@ -306,7 +306,7 @@ export async function updateExperiment( body.template.experiment.id !== experimentId ) { throw createHttpError( - 400, + 422, 'Template experiment ID does not match URL parameter', ); } diff --git a/functions/src/experiment.utils.ts b/functions/src/experiment.utils.ts index a26478a2e..52fc7494a 100644 --- a/functions/src/experiment.utils.ts +++ b/functions/src/experiment.utils.ts @@ -358,7 +358,7 @@ export async function updateExperimentFromTemplate( if (!stageValidation.valid) { return { success: false, - httpErrorCode: 400, + httpErrorCode: 422, errorMessage: `Invalid stage configuration: ${stageValidation.error}`, }; } @@ -371,7 +371,7 @@ export async function updateExperimentFromTemplate( if (!variableValidation.valid) { return { success: false, - httpErrorCode: 400, + httpErrorCode: 422, errorMessage: `Invalid variable configuration: ${variableValidation.error}`, }; } @@ -384,7 +384,7 @@ export async function updateExperimentFromTemplate( if (seenAliases.has(def.alias)) { return { success: false, - httpErrorCode: 400, + httpErrorCode: 422, errorMessage: `Duplicate cohort alias found: "${def.alias}"`, }; } @@ -470,7 +470,7 @@ export async function updateExperimentFromTemplate( return { success: false, error: 'cohort-definitions-locked', - httpErrorCode: 400, + httpErrorCode: 422, errorMessage: 'Cannot modify cohort definitions after participants have joined', }; diff --git a/scripts/deliberate_lab/types.py b/scripts/deliberate_lab/types.py index 6549f8eaa..3d5396b2e 100644 --- a/scripts/deliberate_lab/types.py +++ b/scripts/deliberate_lab/types.py @@ -1260,7 +1260,7 @@ class ScaleSurveyQuestion(BaseModel): lowerText: str middleText: str | None = None useSlider: bool | None = None - stepSize: Annotated[float | None, Field(ge=1.0)] = None + stepSize: float | None = None condition: ComparisonCondition | ConditionGroup | None = None diff --git a/utils/src/stages/stage.validation.ts b/utils/src/stages/stage.validation.ts index 72f151a5b..e0cf69bc9 100644 --- a/utils/src/stages/stage.validation.ts +++ b/utils/src/stages/stage.validation.ts @@ -27,6 +27,8 @@ import {StockInfoStageConfigData} from './stockinfo_stage.validation'; import { SurveyPerParticipantStageConfigData, SurveyStageConfigData, + validateSurveyStageConfig, + validateSurveyPerParticipantStageConfig, } from './survey_stage.validation'; import {TransferStageConfigData} from './transfer_stage.validation'; import {TOSStageConfigData} from './tos_stage.validation'; @@ -68,8 +70,14 @@ export const CONFIG_DATA: Record = { role: {schema: RoleStageConfigData}, salesperson: {schema: SalespersonStageConfigData}, stockinfo: {schema: StockInfoStageConfigData}, - surveyPerParticipant: {schema: SurveyPerParticipantStageConfigData}, - survey: {schema: SurveyStageConfigData}, + surveyPerParticipant: { + schema: SurveyPerParticipantStageConfigData, + validate: validateSurveyPerParticipantStageConfig, + }, + survey: { + schema: SurveyStageConfigData, + validate: validateSurveyStageConfig, + }, tos: {schema: TOSStageConfigData}, transfer: {schema: TransferStageConfigData}, }; diff --git a/utils/src/stages/survey_stage.validation.test.ts b/utils/src/stages/survey_stage.validation.test.ts new file mode 100644 index 000000000..062ae493b --- /dev/null +++ b/utils/src/stages/survey_stage.validation.test.ts @@ -0,0 +1,223 @@ +import {validateSurveyQuestions} from './survey_stage.validation'; +import { + SurveyQuestionKind, + MultipleChoiceDisplayType, + SurveyQuestion, +} from './survey_stage'; + +describe('validateSurveyQuestions', () => { + it('should fail when questions array is empty', () => { + const res = validateSurveyQuestions([]); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain( + 'Survey stage must contain at least one question', + ); + } + }); + + it('should pass on a valid scale question', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 1, + lowerText: 'Poor', + upperValue: 5, + upperText: 'Excellent', + middleText: 'Average', + useSlider: false, + stepSize: 1, + }, + ]; + expect(validateSurveyQuestions(questions)).toEqual({valid: true}); + }); + + it('should fail on a scale question with lowerValue >= upperValue', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 5, + lowerText: 'Poor', + upperValue: 1, + upperText: 'Excellent', + middleText: 'Average', + useSlider: false, + stepSize: 1, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain( + 'has lower value (5) greater than or equal to upper value (1)', + ); + } + }); + + it('should fail when scale question values are not integers', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 1.5, + lowerText: '', + upperValue: 5.5, + upperText: '', + middleText: '', + useSlider: false, + stepSize: 1, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain('must be integers'); + } + }); + + it('should fail when step size is 0', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + middleText: '', + useSlider: false, + stepSize: 0, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain('must be greater than 0'); + } + }); + + it('should fail when step size exceeds max-min', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + middleText: '', + useSlider: false, + stepSize: 6, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain('must divide max-min'); + } + }); + + it('should fail when step size does not divide max-min exactly', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q1', + kind: SurveyQuestionKind.SCALE, + questionTitle: 'Rating', + lowerValue: 1, + lowerText: '', + upperValue: 5, + upperText: '', + middleText: '', + useSlider: false, + stepSize: 3, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain('must divide max-min'); + } + }); + + it('should pass on a valid multiple choice question', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q2', + kind: SurveyQuestionKind.MULTIPLE_CHOICE, + questionTitle: 'Pick one', + options: [ + {id: 'opt1', text: 'Option 1', imageId: ''}, + {id: 'opt2', text: 'Option 2', imageId: ''}, + ], + correctAnswerId: 'opt1', + displayType: MultipleChoiceDisplayType.RADIO, + }, + ]; + expect(validateSurveyQuestions(questions)).toEqual({valid: true}); + }); + + it('should pass on a valid multiple choice question with null correctAnswerId', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q2', + kind: SurveyQuestionKind.MULTIPLE_CHOICE, + questionTitle: 'Pick one', + options: [ + {id: 'opt1', text: 'Option 1', imageId: ''}, + {id: 'opt2', text: 'Option 2', imageId: ''}, + ], + correctAnswerId: null, + displayType: MultipleChoiceDisplayType.RADIO, + }, + ]; + expect(validateSurveyQuestions(questions)).toEqual({valid: true}); + }); + + it('should fail on multiple choice question with no options', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q2', + kind: SurveyQuestionKind.MULTIPLE_CHOICE, + questionTitle: 'Pick one', + options: [], + correctAnswerId: null, + displayType: MultipleChoiceDisplayType.RADIO, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain('must have at least one option'); + } + }); + + it('should fail on multiple choice question with non-matching correctAnswerId', () => { + const questions: SurveyQuestion[] = [ + { + id: 'q2', + kind: SurveyQuestionKind.MULTIPLE_CHOICE, + questionTitle: 'Pick one', + options: [ + {id: 'opt1', text: 'Option 1', imageId: ''}, + {id: 'opt2', text: 'Option 2', imageId: ''}, + ], + correctAnswerId: 'opt3', + displayType: MultipleChoiceDisplayType.RADIO, + }, + ]; + const res = validateSurveyQuestions(questions); + expect(res.valid).toBe(false); + if (!res.valid) { + expect(res.error).toContain( + 'has a correct answer ID "opt3" that doesn\'t match any option ID', + ); + } + }); +}); diff --git a/utils/src/stages/survey_stage.validation.ts b/utils/src/stages/survey_stage.validation.ts index 7c5f1f76c..db75f3a93 100644 --- a/utils/src/stages/survey_stage.validation.ts +++ b/utils/src/stages/survey_stage.validation.ts @@ -1,7 +1,16 @@ import {Type, type Static} from '@sinclair/typebox'; -import {StageKind} from './stage'; -import {BaseStageConfigSchema} from './stage.schemas'; -import {MultipleChoiceDisplayType, SurveyQuestionKind} from './survey_stage'; +import {BaseStageConfig, StageKind} from './stage'; +import { + BaseStageConfigSchema, + type StageValidationResult, +} from './stage.schemas'; +import { + MultipleChoiceDisplayType, + SurveyQuestionKind, + SurveyStageConfig, + SurveyPerParticipantStageConfig, + SurveyQuestion, +} from './survey_stage'; import {ConditionSchema} from '../utils/condition.validation'; /** Shorthand for strict TypeBox object validation */ @@ -74,7 +83,7 @@ export const ScaleSurveyQuestionData = Type.Object( lowerText: Type.String(), middleText: Type.Optional(Type.String()), useSlider: Type.Optional(Type.Boolean()), - stepSize: Type.Optional(Type.Number({minimum: 1})), + stepSize: Type.Optional(Type.Number()), condition: Type.Optional(Type.Union([Type.Null(), ConditionSchema])), }, {$id: 'ScaleSurveyQuestion', ...strict}, @@ -224,3 +233,82 @@ export const UpdateSurveyPerParticipantStageParticipantAnswerData = Type.Object( export type UpdateSurveyPerParticipantStageParticipantAnswerData = Static< typeof UpdateSurveyPerParticipantStageParticipantAnswerData >; + +/** Validate multiple choice and scale questions inside survey stages. */ +export function validateSurveyQuestions( + questions: SurveyQuestion[], +): StageValidationResult { + if (!questions || questions.length === 0) { + return { + valid: false, + error: 'Survey stage must contain at least one question', + }; + } + + for (const q of questions) { + if (q.kind === SurveyQuestionKind.SCALE) { + if ( + !Number.isInteger(q.lowerValue) || + !Number.isInteger(q.upperValue) || + (q.stepSize !== undefined && !Number.isInteger(q.stepSize)) + ) { + return { + valid: false, + error: `Scale question "${q.questionTitle}" lower value, upper value, and step size must be integers`, + }; + } + if (q.lowerValue >= q.upperValue) { + return { + valid: false, + error: `Scale question "${q.questionTitle}" has lower value (${q.lowerValue}) greater than or equal to upper value (${q.upperValue})`, + }; + } + const range = q.upperValue - q.lowerValue; + const step = q.stepSize ?? 1; + if (step <= 0) { + return { + valid: false, + error: `Scale question "${q.questionTitle}" step size (${step}) must be greater than 0`, + }; + } + if (range % step !== 0) { + return { + valid: false, + error: `Scale question "${q.questionTitle}" step size (${step}) must divide max-min (${range}) exactly`, + }; + } + } + if (q.kind === SurveyQuestionKind.MULTIPLE_CHOICE) { + if (!q.options || q.options.length === 0) { + return { + valid: false, + error: `Multiple choice question "${q.questionTitle}" must have at least one option`, + }; + } + if (q.correctAnswerId != null && q.correctAnswerId !== '') { + const hasOption = q.options.some((opt) => opt.id === q.correctAnswerId); + if (!hasOption) { + return { + valid: false, + error: `Multiple choice question "${q.questionTitle}" has a correct answer ID "${q.correctAnswerId}" that doesn't match any option ID`, + }; + } + } + } + } + return {valid: true}; +} + +export function validateSurveyStageConfig( + stage: BaseStageConfig, +): StageValidationResult { + const {questions} = stage as SurveyStageConfig; + return validateSurveyQuestions(questions); +} + +export function validateSurveyPerParticipantStageConfig( + stage: BaseStageConfig, +): StageValidationResult { + const {questions} = stage as SurveyPerParticipantStageConfig; + return validateSurveyQuestions(questions); +} From 8a94f91a2cfbc9d6f95713ebdce27730f735b3e1 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Fri, 1 May 2026 15:42:12 -0400 Subject: [PATCH 2/5] feat: survey validation enhancements, UI error surfacing, and 422 HTTP error statuses - Add Scale range divisibility and integer constraints validation for Survey questions - Restrict survey stages to contain at least one question - Surface active config errors Lit Header UI editor edit mode and create mode - Support entering floats in frontend UI scale number form fields to trigger float validations - Return HTTP 422 Unprocessable Entity for REST cloud function experiment rejections - Add Jest unit and Express integrations tests covering positive/negative validation scenarios --- frontend/src/services/experiment.editor.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/services/experiment.editor.ts b/frontend/src/services/experiment.editor.ts index b184069c0..9e9263aa8 100644 --- a/frontend/src/services/experiment.editor.ts +++ b/frontend/src/services/experiment.editor.ts @@ -21,6 +21,8 @@ import { requiresValues, STAGE_MANAGER, checkApiKeyExists, + SurveyQuestion, + MultipleChoiceItem, createAgentMediatorPersonaConfig, createAgentParticipantPersonaConfig, createExperimentConfig, @@ -125,7 +127,7 @@ export class ExperimentEditor extends Service { if (!stage.questions || stage.questions.length === 0) { errors.push(`${stage.name} must contain at least one question`); } - stage.questions.forEach((question, index) => { + stage.questions.forEach((question: SurveyQuestion, index: number) => { if (question.questionTitle === '') { errors.push( `${stage.name} question ${index + 1} is missing a title`, @@ -171,7 +173,8 @@ export class ExperimentEditor extends Service { question.correctAnswerId !== '' ) { const hasOption = question.options.some( - (opt) => opt.id === question.correctAnswerId, + (opt: MultipleChoiceItem) => + opt.id === question.correctAnswerId, ); if (!hasOption) { errors.push( From 81d9f5a551c7f173b40e7ae2a5b7198083ea4acb Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Fri, 1 May 2026 15:54:30 -0400 Subject: [PATCH 3/5] feat: survey validation enhancements, UI error surfacing, and 422 HTTP error statuses - Add Scale range divisibility and integer constraints validation for Survey questions - Restrict survey stages to contain at least one question - Surface active config errors Lit Header UI editor edit mode and create mode - Support entering floats in frontend UI scale number form fields to trigger float validations - Return HTTP 422 Unprocessable Entity for REST cloud function experiment rejections - Add Jest unit and Express integrations tests covering positive/negative validation scenarios --- docs/assets/api/schemas.json | 7 ++++--- .../src/dl_api/experiments.dl_api.integration.test.ts | 8 ++++++-- scripts/deliberate_lab/types.py | 6 +++--- utils/src/stages/survey_stage.validation.ts | 6 +++--- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/assets/api/schemas.json b/docs/assets/api/schemas.json index 8a37a4d95..3e4ef62ba 100644 --- a/docs/assets/api/schemas.json +++ b/docs/assets/api/schemas.json @@ -2293,13 +2293,13 @@ "type": "string" }, "upperValue": { - "type": "number" + "type": "integer" }, "upperText": { "type": "string" }, "lowerValue": { - "type": "number" + "type": "integer" }, "lowerText": { "type": "string" @@ -2311,7 +2311,8 @@ "type": "boolean" }, "stepSize": { - "type": "number" + "minimum": 1, + "type": "integer" }, "condition": { "anyOf": [ diff --git a/functions/src/dl_api/experiments.dl_api.integration.test.ts b/functions/src/dl_api/experiments.dl_api.integration.test.ts index ea0ffd63a..cf9434022 100644 --- a/functions/src/dl_api/experiments.dl_api.integration.test.ts +++ b/functions/src/dl_api/experiments.dl_api.integration.test.ts @@ -1061,6 +1061,7 @@ describe('API Experiment Creation Integration Tests', () => { lowerText: '', upperValue: 5.5, upperText: '', + middleText: '', stepSize: 1, useSlider: false, }, @@ -1074,7 +1075,9 @@ describe('API Experiment Creation Integration Tests', () => { expect(response.status).toBe(422); const data = await response.json(); - expect(data.error).toContain('must be integers'); + expect(data.error).toContain( + 'lower value, upper value, and step size must be integers', + ); }); it('should reject experiment creation with scale question step size equal to 0', async () => { @@ -1089,6 +1092,7 @@ describe('API Experiment Creation Integration Tests', () => { lowerText: '', upperValue: 5, upperText: '', + middleText: '', stepSize: 0, useSlider: false, }, @@ -1102,7 +1106,7 @@ describe('API Experiment Creation Integration Tests', () => { expect(response.status).toBe(422); const data = await response.json(); - expect(data.error).toContain('must be greater than 0'); + expect(data.error).toContain('step size (0) must be greater than 0'); }); it('should reject experiment creation with scale question step size out of bounds', async () => { diff --git a/scripts/deliberate_lab/types.py b/scripts/deliberate_lab/types.py index 3d5396b2e..24779faa5 100644 --- a/scripts/deliberate_lab/types.py +++ b/scripts/deliberate_lab/types.py @@ -1254,13 +1254,13 @@ class ScaleSurveyQuestion(BaseModel): id: Annotated[str, Field(min_length=1)] kind: Literal["scale"] = "scale" questionTitle: str - upperValue: float + upperValue: int upperText: str - lowerValue: float + lowerValue: int lowerText: str middleText: str | None = None useSlider: bool | None = None - stepSize: float | None = None + stepSize: Annotated[int | None, Field(ge=1)] = None condition: ComparisonCondition | ConditionGroup | None = None diff --git a/utils/src/stages/survey_stage.validation.ts b/utils/src/stages/survey_stage.validation.ts index db75f3a93..951aba39e 100644 --- a/utils/src/stages/survey_stage.validation.ts +++ b/utils/src/stages/survey_stage.validation.ts @@ -77,13 +77,13 @@ export const ScaleSurveyQuestionData = Type.Object( id: Type.String({minLength: 1}), kind: Type.Literal(SurveyQuestionKind.SCALE), questionTitle: Type.String(), - upperValue: Type.Number(), + upperValue: Type.Integer(), upperText: Type.String(), - lowerValue: Type.Number(), + lowerValue: Type.Integer(), lowerText: Type.String(), middleText: Type.Optional(Type.String()), useSlider: Type.Optional(Type.Boolean()), - stepSize: Type.Optional(Type.Number()), + stepSize: Type.Optional(Type.Integer({minimum: 1})), condition: Type.Optional(Type.Union([Type.Null(), ConditionSchema])), }, {$id: 'ScaleSurveyQuestion', ...strict}, From 5b684807cd55bf78acddbbb36321c8b4945b0489 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Fri, 1 May 2026 16:02:40 -0400 Subject: [PATCH 4/5] feat: survey validation enhancements, UI error surfacing, and 422 HTTP error statuses - Add Scale range divisibility and integer constraints validation for Survey questions - Restrict survey stages to contain at least one question - Surface active config errors Lit Header UI editor edit mode and create mode - Support entering floats in frontend UI scale number form fields to trigger float validations - Return HTTP 422 Unprocessable Entity for REST cloud function experiment rejections - Add Jest unit and Express integrations tests covering positive/negative validation scenarios --- functions/src/dl_api/experiments.dl_api.integration.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/functions/src/dl_api/experiments.dl_api.integration.test.ts b/functions/src/dl_api/experiments.dl_api.integration.test.ts index cf9434022..5f076d312 100644 --- a/functions/src/dl_api/experiments.dl_api.integration.test.ts +++ b/functions/src/dl_api/experiments.dl_api.integration.test.ts @@ -1075,9 +1075,7 @@ describe('API Experiment Creation Integration Tests', () => { expect(response.status).toBe(422); const data = await response.json(); - expect(data.error).toContain( - 'lower value, upper value, and step size must be integers', - ); + expect(data.error).toContain('Expected union value'); }); it('should reject experiment creation with scale question step size equal to 0', async () => { @@ -1106,7 +1104,7 @@ describe('API Experiment Creation Integration Tests', () => { expect(response.status).toBe(422); const data = await response.json(); - expect(data.error).toContain('step size (0) must be greater than 0'); + expect(data.error).toContain('Expected union value'); }); it('should reject experiment creation with scale question step size out of bounds', async () => { From cf7e55aa3051318aea52aaa27650221caee5b4b3 Mon Sep 17 00:00:00 2001 From: Michael Behr Date: Fri, 1 May 2026 17:00:41 -0400 Subject: [PATCH 5/5] feat: survey validation enhancements, UI error surfacing, and 422 HTTP error statuses - Add Scale range divisibility and integer constraints validation for Survey questions - Restrict survey stages to contain at least one question - Surface active config errors Lit Header UI editor edit mode and create mode - Support entering floats in frontend UI scale number form fields to trigger float validations - Return HTTP 422 Unprocessable Entity for REST cloud function experiment rejections - Add Jest unit and Express integrations tests covering positive/negative validation scenarios --- .../experiments.dl_api.integration.test.ts | 4 +-- functions/src/dl_api/experiments.dl_api.ts | 26 +++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/functions/src/dl_api/experiments.dl_api.integration.test.ts b/functions/src/dl_api/experiments.dl_api.integration.test.ts index 5f076d312..e27a194e8 100644 --- a/functions/src/dl_api/experiments.dl_api.integration.test.ts +++ b/functions/src/dl_api/experiments.dl_api.integration.test.ts @@ -1073,7 +1073,7 @@ describe('API Experiment Creation Integration Tests', () => { stages: JSON.parse(JSON.stringify([stage])), }); - expect(response.status).toBe(422); + expect(response.status).toBe(400); const data = await response.json(); expect(data.error).toContain('Expected union value'); }); @@ -1102,7 +1102,7 @@ describe('API Experiment Creation Integration Tests', () => { stages: JSON.parse(JSON.stringify([stage])), }); - expect(response.status).toBe(422); + expect(response.status).toBe(400); const data = await response.json(); expect(data.error).toContain('Expected union value'); }); diff --git a/functions/src/dl_api/experiments.dl_api.ts b/functions/src/dl_api/experiments.dl_api.ts index 6c15d55d0..fb8f76feb 100644 --- a/functions/src/dl_api/experiments.dl_api.ts +++ b/functions/src/dl_api/experiments.dl_api.ts @@ -137,7 +137,7 @@ export async function createExperiment( const error = [...Value.Errors(CreateExperimentRequestData, body)] .map((e) => `${e.path || '/'}: ${e.message}`) .join('; '); - throw createHttpError(422, `Invalid request body: ${error}`); + throw createHttpError(400, `Invalid request body: ${error}`); } // If a full template is provided, use the shared utility @@ -150,10 +150,12 @@ export async function createExperiment( experimenterId, ); } catch (error: unknown) { - throw createHttpError( - 422, - error instanceof Error ? error.message : String(error), - ); + const msg = error instanceof Error ? error.message : String(error); + const isValidationError = + msg.includes('Invalid stage configuration') || + msg.includes('Invalid variable configuration') || + msg.includes('Duplicate cohort alias found'); + throw createHttpError(isValidationError ? 422 : 500, msg); } if (!experimentId) { @@ -173,7 +175,7 @@ export async function createExperiment( // Simple creation mode: name is required if (!body.name) { throw createHttpError( - 422, + 400, 'Invalid request body: name is required (or provide template)', ); } @@ -214,10 +216,12 @@ export async function createExperiment( experimenterId, ); } catch (error: unknown) { - throw createHttpError( - 422, - error instanceof Error ? error.message : String(error), - ); + const msg = error instanceof Error ? error.message : String(error); + const isValidationError = + msg.includes('Invalid stage configuration') || + msg.includes('Invalid variable configuration') || + msg.includes('Duplicate cohort alias found'); + throw createHttpError(isValidationError ? 422 : 500, msg); } if (!experimentId) { @@ -306,7 +310,7 @@ export async function updateExperiment( body.template.experiment.id !== experimentId ) { throw createHttpError( - 422, + 400, 'Template experiment ID does not match URL parameter', ); }