From 3b2f264d218088062697e4fef22661723cd1fd28 Mon Sep 17 00:00:00 2001 From: Benjamin Blanchard Date: Fri, 26 Jun 2026 15:41:02 -0400 Subject: [PATCH 1/5] use the aggregated count for all repeated enrollments to determine round-robin cycle --- .../services/ExperimentAssignmentService.ts | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/backend/src/api/services/ExperimentAssignmentService.ts b/packages/backend/src/api/services/ExperimentAssignmentService.ts index a32bb3b48..6761685c2 100644 --- a/packages/backend/src/api/services/ExperimentAssignmentService.ts +++ b/packages/backend/src/api/services/ExperimentAssignmentService.ts @@ -1,9 +1,6 @@ import { GroupEnrollmentRepository } from '../repositories/GroupEnrollmentRepository'; import { IndividualEnrollmentRepository } from '../repositories/IndividualEnrollmentRepository'; -import { - RepeatedEnrollmentDataCount, - RepeatedEnrollmentRepository, -} from '../repositories/RepeatedEnrollmentRepository'; +import { RepeatedEnrollmentRepository } from '../repositories/RepeatedEnrollmentRepository'; import { IndividualEnrollment } from '../models/IndividualEnrollment'; import { ErrorWithType } from '../errors/ErrorWithType'; import { InjectRepository } from '../../typeorm-typedi-extensions'; @@ -426,21 +423,24 @@ export class ExperimentAssignmentService { ); }) ); - let repeatedEnrollmentCounts: RepeatedEnrollmentDataCount[] = []; + let repeatedEnrollmentCount = 0; if (filteredExperiments.some((experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS)) { const filteredWithinSubjectExperiments = filteredExperiments.filter( (experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ); - const allWithinSubjectDecisionPoints = filteredWithinSubjectExperiments - .map((experiment) => this.getActiveDecisionPoints(experiment)) - .flat(); + const allWithinSubjectDecisionPoints = filteredWithinSubjectExperiments.flatMap((experiment) => + this.getActiveDecisionPoints(experiment) + ); - repeatedEnrollmentCounts = await this.repeatedEnrollmentRepository.getRepeatedEnrollmentCount( + const repeatedEnrollmentCounts = await this.repeatedEnrollmentRepository.getRepeatedEnrollmentCount( userId, allWithinSubjectDecisionPoints.map((dp) => dp.id), logger ); + if (repeatedEnrollmentCounts?.length > 0) { + repeatedEnrollmentCount = repeatedEnrollmentCounts.reduce((acc, curr) => acc + curr.count, 0); + } } return filteredExperiments.reduce((accumulator, experiment, index) => { const assignment = experimentAssignment[index]; @@ -454,7 +454,7 @@ export class ExperimentAssignmentService { conditionPayloads, type, factors, - repeatedEnrollmentCounts || [], + repeatedEnrollmentCount, logger ); return [...accumulator, ...decisionPoints]; @@ -551,7 +551,7 @@ export class ExperimentAssignmentService { conditionPayloads, type, factors, - [], + 0, logger ); } @@ -894,7 +894,7 @@ export class ExperimentAssignmentService { conditionPayloads: ConditionPayloadDTO[], type: EXPERIMENT_TYPE, factors: FactorDTO[], - repeatedEnrollmentCounts: { userId: string; decisionPointId: string; count: number }[], + repeatedEnrollmentCount: number, logger: UpgradeLogger ): IExperimentAssignmentv5[] { return experiment.partitions @@ -928,11 +928,14 @@ export class ExperimentAssignmentService { }; if (experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS) { - const count = - repeatedEnrollmentCounts.find( - (repeatedEnrollment) => repeatedEnrollment.decisionPointId === decisionPoint.id - )?.count || 0; - return withInSubjectType(experiment, conditionPayloads, decisionPoint, factors, userId, count); + return withInSubjectType( + experiment, + conditionPayloads, + decisionPoint, + factors, + userId, + repeatedEnrollmentCount + ); } else { const experimentId = experiment.id; return { From 945a4e067acf93d81060c6b5ccbba060bcd9d8d6 Mon Sep 17 00:00:00 2001 From: Benjamin Blanchard Date: Fri, 26 Jun 2026 16:29:30 -0400 Subject: [PATCH 2/5] get results for each experiment, rather than all --- .../RepeatedEnrollmentRepository.ts | 11 +++--- .../services/ExperimentAssignmentService.ts | 36 ++++++++----------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts b/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts index afde9a0e1..44471ee00 100644 --- a/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts +++ b/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts @@ -6,23 +6,24 @@ import repositoryError from './utils/repositoryError'; export interface RepeatedEnrollmentDataCount { userId: string; - decisionPointId: string; + experimentId: string; count: number; } @EntityRepository(RepeatedEnrollment) export class RepeatedEnrollmentRepository extends Repository { public async getRepeatedEnrollmentCount( userId: string, - decisionPointsIds: string[], + experimentIds: string[], logger: UpgradeLogger ): Promise { const result = await this.createQueryBuilder('repeatedEnrollment') - .select(['ie.userId as "userId"', 'ie.partitionId as "decisionPointId"']) + .select(['ie.userId as "userId"', 'ie.experimentId as "experimentId"']) .addSelect('COUNT(*) as count') .leftJoin('repeatedEnrollment.individualEnrollment', 'ie') + .leftJoin('ie.experiment', 'experiment') .where('ie.userId = :userId', { userId }) - .andWhere('ie.partitionId IN (:...decisionPointsIds)', { decisionPointsIds }) - .groupBy('ie.userId , ie.partitionId , ie.id') + .andWhere('ie.experimentId IN (:...experimentIds)', { experimentIds }) + .groupBy('ie.userId , ie.experimentId , ie.id') .getRawMany() .catch((errorMsg: any) => { const errorMsgString = repositoryError( diff --git a/packages/backend/src/api/services/ExperimentAssignmentService.ts b/packages/backend/src/api/services/ExperimentAssignmentService.ts index 6761685c2..cc2405211 100644 --- a/packages/backend/src/api/services/ExperimentAssignmentService.ts +++ b/packages/backend/src/api/services/ExperimentAssignmentService.ts @@ -1,6 +1,9 @@ import { GroupEnrollmentRepository } from '../repositories/GroupEnrollmentRepository'; import { IndividualEnrollmentRepository } from '../repositories/IndividualEnrollmentRepository'; -import { RepeatedEnrollmentRepository } from '../repositories/RepeatedEnrollmentRepository'; +import { + RepeatedEnrollmentDataCount, + RepeatedEnrollmentRepository, +} from '../repositories/RepeatedEnrollmentRepository'; import { IndividualEnrollment } from '../models/IndividualEnrollment'; import { ErrorWithType } from '../errors/ErrorWithType'; import { InjectRepository } from '../../typeorm-typedi-extensions'; @@ -423,24 +426,17 @@ export class ExperimentAssignmentService { ); }) ); - let repeatedEnrollmentCount = 0; + let repeatedEnrollmentCounts = []; if (filteredExperiments.some((experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS)) { const filteredWithinSubjectExperiments = filteredExperiments.filter( (experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ); - const allWithinSubjectDecisionPoints = filteredWithinSubjectExperiments.flatMap((experiment) => - this.getActiveDecisionPoints(experiment) - ); - - const repeatedEnrollmentCounts = await this.repeatedEnrollmentRepository.getRepeatedEnrollmentCount( + repeatedEnrollmentCounts = await this.repeatedEnrollmentRepository.getRepeatedEnrollmentCount( userId, - allWithinSubjectDecisionPoints.map((dp) => dp.id), + filteredWithinSubjectExperiments.map((experiment) => experiment.id), logger ); - if (repeatedEnrollmentCounts?.length > 0) { - repeatedEnrollmentCount = repeatedEnrollmentCounts.reduce((acc, curr) => acc + curr.count, 0); - } } return filteredExperiments.reduce((accumulator, experiment, index) => { const assignment = experimentAssignment[index]; @@ -454,7 +450,7 @@ export class ExperimentAssignmentService { conditionPayloads, type, factors, - repeatedEnrollmentCount, + repeatedEnrollmentCounts, logger ); return [...accumulator, ...decisionPoints]; @@ -551,7 +547,7 @@ export class ExperimentAssignmentService { conditionPayloads, type, factors, - 0, + [], logger ); } @@ -894,7 +890,7 @@ export class ExperimentAssignmentService { conditionPayloads: ConditionPayloadDTO[], type: EXPERIMENT_TYPE, factors: FactorDTO[], - repeatedEnrollmentCount: number, + repeatedEnrollmentCounts: RepeatedEnrollmentDataCount[], logger: UpgradeLogger ): IExperimentAssignmentv5[] { return experiment.partitions @@ -928,14 +924,10 @@ export class ExperimentAssignmentService { }; if (experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS) { - return withInSubjectType( - experiment, - conditionPayloads, - decisionPoint, - factors, - userId, - repeatedEnrollmentCount - ); + const count = + repeatedEnrollmentCounts?.find((repeatedEnrollment) => repeatedEnrollment.experimentId === experiment.id) + ?.count || 0; + return withInSubjectType(experiment, conditionPayloads, decisionPoint, factors, userId, count); } else { const experimentId = experiment.id; return { From 4d1c98c97bbbecfc5e790ab5aeedf93bdcc59696 Mon Sep 17 00:00:00 2001 From: Ben Blanchard Date: Fri, 26 Jun 2026 16:38:15 -0400 Subject: [PATCH 3/5] explicit type Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../backend/src/api/services/ExperimentAssignmentService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/api/services/ExperimentAssignmentService.ts b/packages/backend/src/api/services/ExperimentAssignmentService.ts index cc2405211..8edd249bd 100644 --- a/packages/backend/src/api/services/ExperimentAssignmentService.ts +++ b/packages/backend/src/api/services/ExperimentAssignmentService.ts @@ -426,7 +426,7 @@ export class ExperimentAssignmentService { ); }) ); - let repeatedEnrollmentCounts = []; +let repeatedEnrollmentCounts: RepeatedEnrollmentDataCount[] = []; if (filteredExperiments.some((experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS)) { const filteredWithinSubjectExperiments = filteredExperiments.filter( (experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS From 91739ddcc7ac3c4df99dde78e1864a48afc9df18 Mon Sep 17 00:00:00 2001 From: Benjamin Blanchard Date: Fri, 26 Jun 2026 16:50:49 -0400 Subject: [PATCH 4/5] fix query --- .../src/api/repositories/RepeatedEnrollmentRepository.ts | 3 +-- .../backend/src/api/services/ExperimentAssignmentService.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts b/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts index 44471ee00..e5b7d19c8 100644 --- a/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts +++ b/packages/backend/src/api/repositories/RepeatedEnrollmentRepository.ts @@ -20,10 +20,9 @@ export class RepeatedEnrollmentRepository extends Repository .select(['ie.userId as "userId"', 'ie.experimentId as "experimentId"']) .addSelect('COUNT(*) as count') .leftJoin('repeatedEnrollment.individualEnrollment', 'ie') - .leftJoin('ie.experiment', 'experiment') .where('ie.userId = :userId', { userId }) .andWhere('ie.experimentId IN (:...experimentIds)', { experimentIds }) - .groupBy('ie.userId , ie.experimentId , ie.id') + .groupBy('ie.userId, ie.experimentId') .getRawMany() .catch((errorMsg: any) => { const errorMsgString = repositoryError( diff --git a/packages/backend/src/api/services/ExperimentAssignmentService.ts b/packages/backend/src/api/services/ExperimentAssignmentService.ts index 8edd249bd..b15f2a570 100644 --- a/packages/backend/src/api/services/ExperimentAssignmentService.ts +++ b/packages/backend/src/api/services/ExperimentAssignmentService.ts @@ -426,7 +426,7 @@ export class ExperimentAssignmentService { ); }) ); -let repeatedEnrollmentCounts: RepeatedEnrollmentDataCount[] = []; + let repeatedEnrollmentCounts: RepeatedEnrollmentDataCount[] = []; if (filteredExperiments.some((experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS)) { const filteredWithinSubjectExperiments = filteredExperiments.filter( (experiment) => experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS From 10564dfd1521733917ef6446ec69fb4443883317 Mon Sep 17 00:00:00 2001 From: Benjamin Blanchard Date: Mon, 29 Jun 2026 09:39:26 -0400 Subject: [PATCH 5/5] add unit test for single rotation counter across DPs --- .../ExperimentAssignmentService.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/backend/test/unit/services/ExperimentAssignmentService.test.ts b/packages/backend/test/unit/services/ExperimentAssignmentService.test.ts index 28588d8d2..7f6781b6e 100644 --- a/packages/backend/test/unit/services/ExperimentAssignmentService.test.ts +++ b/packages/backend/test/unit/services/ExperimentAssignmentService.test.ts @@ -409,6 +409,47 @@ describe('Experiment Assignment Service Test', () => { expect(result[0].assignedCondition).toMatchObject(cond); }); + it('should use a single shared rotation counter across all decision points in a within-subject experiment', async () => { + const context = 'context'; + const userDoc = { id: 'user123', group: { schoolId: ['school1'] }, workingGroup: {} }; + const exp = structuredClone(simpleWithinSubjectOrderedRoundRobinExperiment); + + // Add a second decision point to the experiment + const secondPartition = { + ...exp.partitions[0], + id: 'dp-2-id', + twoCharacterId: 'W2', + site: 'CurriculumSequence', + target: 'W2', + }; + exp.partitions = [...exp.partitions, secondPartition]; + + // Simulate the user has already been through the rotation once (count = 1) + // For ORDERED_ROUND_ROBIN with 2 conditions and count=1, the second condition should be first + const repeatedEnrollmentCount = 1; + testedModule.repeatedEnrollmentRepository = { + getRepeatedEnrollmentCount: sandbox + .stub() + .resolves([{ userId: userDoc.id, experimentId: exp.id, count: repeatedEnrollmentCount }]), + }; + + testedModule.experimentService.getCachedValidExperiments = sandbox.stub().resolves([exp]); + testedModule.experimentUserService = { getOriginalUserDoc: sandbox.stub().resolves(userDoc) }; + + const result = await testedModule.getAllExperimentConditions(userDoc, context, loggerMock); + + // Both decision points should be returned + expect(result.length).toEqual(2); + + // Both DPs must have the same assigned condition order — they share one rotation counter + expect(result[0].assignedCondition[0].conditionCode).toEqual(result[1].assignedCondition[0].conditionCode); + expect(result[0].assignedCondition[1].conditionCode).toEqual(result[1].assignedCondition[1].conditionCode); + + // With count=1, the rotation should have advanced past position 0 — condition at index 1 should now be first + expect(result[0].assignedCondition[0].conditionCode).toEqual(exp.conditions[1].conditionCode); + expect(result[0].assignedCondition[1].conditionCode).toEqual(exp.conditions[0].conditionCode); + }); + it('should return the assigned condition for a simple group experiment', async () => { const context = 'context'; const userDoc = { id: 'user123', group: { 'add-group1': ['school1'] }, workingGroup: { 'add-group1': 'school1' } };