Skip to content

Commit 6458840

Browse files
committed
Add cross-parameter stage validation.
1 parent 867f9e6 commit 6458840

File tree

10 files changed

+164
-35
lines changed

10 files changed

+164
-35
lines changed

functions/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"deploy": "firebase deploy --only functions",
1212
"logs": "firebase functions:log",
1313
"test": "npm run test:unit && npm run test:firestore",
14-
"test:firestore": "firebase -c ../firebase-test.json emulators:exec --only firestore,functions --project=demo-deliberate-lab \"npx jest --runInBand $npm_package_config_firestore_tests\"",
14+
"test:firestore": "GOOGLE_CLOUD_PROJECT=demo-deliberate-lab firebase -c ../firebase-test.json emulators:exec --only firestore,functions --project=demo-deliberate-lab \"npx jest --runInBand $npm_package_config_firestore_tests\"",
1515
"test:unit": "npx jest --testPathIgnorePatterns=$npm_package_config_firestore_tests",
1616
"typecheck": "tsc --noEmit",
1717
"migrate:require-full-time": "npx tsx src/migrations/migrate-require-full-time.ts",

functions/src/dl_api/experiments.dl_api.integration.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
ProlificConfig,
1616
Visibility,
1717
createExperimentConfig,
18+
createPrivateChatStage,
1819
} from '@deliberation-lab/utils';
1920
import {
2021
TestContext,
@@ -895,4 +896,25 @@ describe('API Experiment Creation Integration Tests', () => {
895896
}
896897
});
897898
});
899+
900+
// ==========================================================================
901+
// Stage Validation Tests
902+
// ==========================================================================
903+
904+
describe('Stage validation', () => {
905+
it('should reject experiment creation with invalid stage config (minTurns > maxTurns)', async () => {
906+
const invalidStage = createPrivateChatStage({
907+
name: 'Invalid chat',
908+
minNumberOfTurns: 10,
909+
maxNumberOfTurns: 3,
910+
});
911+
912+
const response = await apiRequest('POST', '/v1/experiments', {
913+
name: 'Invalid experiment',
914+
stages: JSON.parse(JSON.stringify([invalidStage])),
915+
});
916+
917+
expect(response.ok).toBe(false);
918+
});
919+
});
898920
});

functions/src/experiment.utils.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {getExperimentDownload} from './data';
2222
import {generateVariablesForScope} from './variables.utils';
2323
import {AuthGuard} from './utils/auth-guard';
2424
import {createCohortFromDefinition} from './cohort.utils';
25+
import {validateStages} from './utils/validation';
2526

2627
/**
2728
* Create cohorts from cohort definitions and update the definitions with generated IDs.
@@ -80,6 +81,12 @@ export async function writeExperimentFromTemplate(
8081
): Promise<string> {
8182
const {collectionName = 'experiments'} = options;
8283

84+
// Validate stage configs (schema + cross-field business rules)
85+
const stageValidation = validateStages(template.stageConfigs);
86+
if (!stageValidation.valid) {
87+
throw new Error(`Invalid stage configuration: ${stageValidation.error}`);
88+
}
89+
8390
// Set up experiment config with stageIds
8491
const experimentConfig = createExperimentConfig(
8592
template.stageConfigs,
@@ -298,7 +305,11 @@ export interface UpdateExperimentOptions {
298305
*/
299306
export interface UpdateExperimentResult {
300307
success: boolean;
301-
error?: 'not-found' | 'not-owner' | 'cohort-definitions-locked';
308+
error?:
309+
| 'not-found'
310+
| 'not-owner'
311+
| 'cohort-definitions-locked'
312+
| 'invalid-stages';
302313
}
303314

304315
/**
@@ -322,6 +333,12 @@ export async function updateExperimentFromTemplate(
322333
): Promise<UpdateExperimentResult> {
323334
const {collectionName = 'experiments'} = options;
324335

336+
// Validate stage configs (schema + cross-field business rules)
337+
const stageValidation = validateStages(template.stageConfigs);
338+
if (!stageValidation.valid) {
339+
return {success: false, error: 'invalid-stages'};
340+
}
341+
325342
// Set up experiment config with stageIds
326343
const experimentConfig = createExperimentConfig(
327344
template.stageConfigs,

functions/src/utils/validation.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/** Pretty printing and analysis utils for typebox validation */
22

33
import {
4+
BaseStageConfig,
45
CONFIG_DATA,
56
CohortParticipantConfig,
67
CohortParticipantConfigSchema,
@@ -111,12 +112,17 @@ export const checkUnionErrorOnPath = (
111112
: Value.Errors(validator, value);
112113
};
113114

115+
/** Schema-only map extracted from CONFIG_DATA for union error resolution. */
116+
const CONFIG_DATA_SCHEMAS: Record<Index, TSchema> = Object.fromEntries(
117+
Object.entries(CONFIG_DATA).map(([key, entry]) => [key, entry.schema]),
118+
);
119+
114120
// Variants
115121
export const checkConfigDataUnionOnPath = (
116122
data: unknown,
117123
path: string,
118124
references?: TSchema[],
119-
) => checkUnionErrorOnPath(data, path, CONFIG_DATA, references);
125+
) => checkUnionErrorOnPath(data, path, CONFIG_DATA_SCHEMAS, references);
120126

121127
// ************************************************************************* //
122128
// STAGE VALIDATION //
@@ -187,6 +193,19 @@ export function validateStages(stages: unknown[]): ValidationResult {
187193
errorMessages.push(` - ${error.path}: ${error.message}`);
188194
}
189195
}
196+
} else {
197+
// Schema passed — run cross-field business rule validation
198+
const stageObj = stage as BaseStageConfig;
199+
const entry = CONFIG_DATA[stageObj.kind as string];
200+
if (entry?.validate) {
201+
const result = entry.validate(stageObj);
202+
if (!result.valid) {
203+
const stageName = stageObj?.name || 'unnamed';
204+
errorMessages.push(
205+
`Stage ${i} (name: "${stageName}", kind: "${stageObj.kind}"): ${result.error}`,
206+
);
207+
}
208+
}
190209
}
191210
}
192211

utils/src/export-schemas.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ function collectSchemasWithId(
5454
}
5555

5656
// Verify all stage configs have $id for proper naming in $defs
57-
for (const [key, schema] of Object.entries(CONFIG_DATA)) {
58-
if (!(schema as Record<string, unknown>).$id) {
57+
for (const [key, entry] of Object.entries(CONFIG_DATA)) {
58+
if (!(entry.schema as Record<string, unknown>).$id) {
5959
throw new Error(
6060
`Stage config "${key}" is missing $id. Add $id to its TypeBox definition.`,
6161
);

utils/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export * from './stages/stage.handler';
7373
export * from './stages/stage.manager';
7474
export * from './stages/stage.prompts';
7575
export * from './stages/stage.validation';
76+
export * from './stages/stage.schemas';
7677

7778
export * from './stages/chat_stage';
7879
export * from './stages/chat_stage.manager';

utils/src/stages/chat_stage.validation.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import {Type, type Static} from '@sinclair/typebox';
22
import {UnifiedTimestampSchema} from '../shared.validation';
3-
import {StageKind} from './stage';
3+
import {BaseStageConfig, StageKind} from './stage';
44
import {ChatDiscussionType} from './chat_stage';
5-
import {BaseStageConfigSchema} from './stage.schemas';
5+
import {
6+
BaseStageConfigSchema,
7+
type StageValidationResult,
8+
} from './stage.schemas';
69
import {UserType} from '../participant';
10+
import {ChatStageConfig} from './chat_stage';
711

812
/** Shorthand for strict TypeBox object validation */
913
const strict = {additionalProperties: false} as const;
@@ -70,6 +74,23 @@ export const ChatStageConfigData = Type.Composite(
7074
{$id: 'ChatStageConfig', ...strict},
7175
);
7276

77+
/** Validate cross-field business rules for chat time configs. */
78+
export function validateChatStageConfig(
79+
stage: BaseStageConfig,
80+
): StageValidationResult {
81+
const {timeMinimumInMinutes: min, timeLimitInMinutes: max} =
82+
stage as ChatStageConfig;
83+
84+
if (min != null && max != null && min > max) {
85+
return {
86+
valid: false,
87+
error: `timeMinimumInMinutes (${min}) cannot exceed timeLimitInMinutes (${max})`,
88+
};
89+
}
90+
91+
return {valid: true};
92+
}
93+
7394
// ************************************************************************* //
7495
// updateChatMessage endpoint //
7596
// ************************************************************************* //

utils/src/stages/private_chat_stage.validation.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import {Type, type Static} from '@sinclair/typebox';
22
import {UnifiedTimestampSchema} from '../shared.validation';
3-
import {StageKind} from './stage';
4-
import {BaseStageConfigSchema} from './stage.schemas';
3+
import {BaseStageConfig, StageKind} from './stage';
4+
import {
5+
BaseStageConfigSchema,
6+
type StageValidationResult,
7+
} from './stage.schemas';
8+
import {validateChatStageConfig} from './chat_stage.validation';
9+
import {PrivateChatStageConfig} from './private_chat_stage';
510

611
/** Shorthand for strict TypeBox object validation */
712
const strict = {additionalProperties: false} as const;
@@ -37,3 +42,24 @@ export const PrivateChatStageConfigData = Type.Composite(
3742
],
3843
{$id: 'PrivateChatStageConfig', ...strict},
3944
);
45+
46+
/** Validate cross-field business rules for private chat stage configs. */
47+
export function validatePrivateChatStageConfig(
48+
stage: BaseStageConfig,
49+
): StageValidationResult {
50+
// Check time constraints (shared with group chat)
51+
const timeResult = validateChatStageConfig(stage);
52+
if (!timeResult.valid) return timeResult;
53+
54+
const {minNumberOfTurns: minTurns, maxNumberOfTurns: maxTurns} =
55+
stage as PrivateChatStageConfig;
56+
57+
if (minTurns != null && maxTurns != null && minTurns > maxTurns) {
58+
return {
59+
valid: false,
60+
error: `minNumberOfTurns (${minTurns}) cannot exceed maxNumberOfTurns (${maxTurns})`,
61+
};
62+
}
63+
64+
return {valid: true};
65+
}

utils/src/stages/stage.schemas.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ import {StageKind} from './stage';
66
* These are defined in a separate file to ensure they're bundled before Type.Ref() calls.
77
*/
88

9+
/** Result type for cross-field stage config validators. */
10+
export type StageValidationResult =
11+
| {valid: true}
12+
| {valid: false; error: string};
13+
914
/** StageTextConfig input validation. */
1015
export const StageTextConfigSchema = Type.Object(
1116
{
Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
1-
import {Type} from '@sinclair/typebox';
2-
import {StageKind} from './stage';
1+
import {Type, type TSchema} from '@sinclair/typebox';
2+
import {BaseStageConfig, StageKind} from './stage';
3+
import {type StageValidationResult} from './stage.schemas';
34
import {
45
AssetAllocationStageConfigData,
56
MultiAssetAllocationStageConfigData,
67
} from './asset_allocation_stage.validation';
7-
import {ChatStageConfigData} from './chat_stage.validation';
8+
import {
9+
ChatStageConfigData,
10+
validateChatStageConfig,
11+
} from './chat_stage.validation';
812
import {ChipStageConfigData} from './chip_stage.validation';
913
import {ComprehensionStageConfigData} from './comprehension_stage.validation';
1014
import {FlipCardStageConfigData} from './flipcard_stage.validation';
1115
import {RankingStageConfigData} from './ranking_stage.validation';
1216
import {InfoStageConfigData} from './info_stage.validation';
1317
import {PayoutStageConfigData} from './payout_stage.validation';
14-
import {PrivateChatStageConfigData} from './private_chat_stage.validation';
18+
import {
19+
PrivateChatStageConfigData,
20+
validatePrivateChatStageConfig,
21+
} from './private_chat_stage.validation';
1522
import {ProfileStageConfigData} from './profile_stage.validation';
1623
import {RevealStageConfigData} from './reveal_stage.validation';
1724
import {RoleStageConfigData} from './role_stage.validation';
@@ -35,28 +42,39 @@ export const StageKindData = Type.Enum(StageKind, {$id: 'StageKind'});
3542
// writeExperiment, updateStageConfig endpoints //
3643
// ************************************************************************* //
3744

38-
/** Map of stage kinds to their validators */
39-
export const CONFIG_DATA = {
40-
assetAllocation: AssetAllocationStageConfigData,
41-
multiAssetAllocation: MultiAssetAllocationStageConfigData,
42-
chat: ChatStageConfigData,
43-
chip: ChipStageConfigData,
44-
comprehension: ComprehensionStageConfigData,
45-
flipcard: FlipCardStageConfigData,
46-
info: InfoStageConfigData,
47-
payout: PayoutStageConfigData,
48-
privateChat: PrivateChatStageConfigData,
49-
profile: ProfileStageConfigData,
50-
ranking: RankingStageConfigData,
51-
reveal: RevealStageConfigData,
52-
role: RoleStageConfigData,
53-
salesperson: SalespersonStageConfigData,
54-
stockinfo: StockInfoStageConfigData,
55-
surveyPerParticipant: SurveyPerParticipantStageConfigData,
56-
survey: SurveyStageConfigData,
57-
tos: TOSStageConfigData,
58-
transfer: TransferStageConfigData,
45+
/** Stage config entry with schema and optional cross-field validator. */
46+
export interface StageConfigEntry {
47+
schema: TSchema;
48+
validate?: (stage: BaseStageConfig) => StageValidationResult;
49+
}
50+
51+
/** Map of stage kinds to their schema and optional validator */
52+
export const CONFIG_DATA: Record<string, StageConfigEntry> = {
53+
assetAllocation: {schema: AssetAllocationStageConfigData},
54+
multiAssetAllocation: {schema: MultiAssetAllocationStageConfigData},
55+
chat: {schema: ChatStageConfigData, validate: validateChatStageConfig},
56+
chip: {schema: ChipStageConfigData},
57+
comprehension: {schema: ComprehensionStageConfigData},
58+
flipcard: {schema: FlipCardStageConfigData},
59+
info: {schema: InfoStageConfigData},
60+
payout: {schema: PayoutStageConfigData},
61+
privateChat: {
62+
schema: PrivateChatStageConfigData,
63+
validate: validatePrivateChatStageConfig,
64+
},
65+
profile: {schema: ProfileStageConfigData},
66+
ranking: {schema: RankingStageConfigData},
67+
reveal: {schema: RevealStageConfigData},
68+
role: {schema: RoleStageConfigData},
69+
salesperson: {schema: SalespersonStageConfigData},
70+
stockinfo: {schema: StockInfoStageConfigData},
71+
surveyPerParticipant: {schema: SurveyPerParticipantStageConfigData},
72+
survey: {schema: SurveyStageConfigData},
73+
tos: {schema: TOSStageConfigData},
74+
transfer: {schema: TransferStageConfigData},
5975
};
6076

6177
/** StageConfig input validation (union of all stage types) */
62-
export const StageConfigData = Type.Union(Object.values(CONFIG_DATA));
78+
export const StageConfigData = Type.Union(
79+
Object.values(CONFIG_DATA).map((entry) => entry.schema),
80+
);

0 commit comments

Comments
 (0)