Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add reward metric and query on mooclet create #2207

Merged
merged 5 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
ASSIGNMENT_ALGORITHM,
MoocletTSConfigurablePolicyParametersDTO,
MoocletPolicyParametersDTO,
SUPPORTED_MOOCLET_ALGORITHMS,
} from 'upgrade_types';
import { Type } from 'class-transformer';

Expand Down Expand Up @@ -464,6 +465,9 @@ abstract class BaseExperimentWithoutPayload {
public type: EXPERIMENT_TYPE;
}

const isMoocletAssigmentAlgorithm = (experiment: ExperimentDTO) =>
experiment.assignmentAlgorithm && SUPPORTED_MOOCLET_ALGORITHMS.includes(experiment.assignmentAlgorithm);

export class ExperimentDTO extends BaseExperimentWithoutPayload {
@IsOptional()
@IsArray()
Expand All @@ -472,14 +476,7 @@ export class ExperimentDTO extends BaseExperimentWithoutPayload {
public conditionPayloads?: ConditionPayloadValidator[];

// This should be validated when assignmentAlgorithm is not RANDOM or STRATIFIED_RANDOM_SAMPLING
@ValidateIf(
(experiment) =>
experiment.assignmentAlgorithm &&
!(
experiment.assignmentAlgorithm === ASSIGNMENT_ALGORITHM.RANDOM ||
experiment.assignmentAlgorithm === ASSIGNMENT_ALGORITHM.STRATIFIED_RANDOM_SAMPLING
)
)
@ValidateIf(isMoocletAssigmentAlgorithm)
@IsDefined()
@ValidateNested()
@Type(() => MoocletPolicyParametersDTO, {
Expand All @@ -493,6 +490,10 @@ export class ExperimentDTO extends BaseExperimentWithoutPayload {
keepDiscriminatorProperty: true,
})
public moocletPolicyParameters?: MoocletPolicyParametersDTO;

@ValidateIf(isMoocletAssigmentAlgorithm)
@IsDefined()
public rewardMetricKey?: string;
}

export class OldExperimentDTO extends BaseExperimentWithoutPayload {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -833,16 +833,15 @@ export class ExperimentController {
@Params({ validate: true }) { id }: ExperimentIdValidator,
@Req() request: AppRequest
): Promise<ExperimentDTO> {
const experiment = await this.experimentService.getSingleExperiment(id, request.logger);
let experiment = await this.experimentService.getSingleExperiment(id, request.logger);

if (SUPPORTED_MOOCLET_ALGORITHMS.includes(experiment.assignmentAlgorithm)) {
if (!env.mooclets?.enabled) {
throw new BadRequestError(
'MoocletPolicyParameters are present in the experiment but Mooclet is not enabled in the environment'
);
} else {
const policyParametersResponse = await this.moocletExperimentService.getPolicyParametersByExperimentId(id);
experiment.moocletPolicyParameters = policyParametersResponse.parameters;
experiment = await this.moocletExperimentService.attachRewardKeyAndPolicyParamsToExperimentDTO(experiment);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,10 @@ export class MoocletExperimentRef {

@Column({ nullable: true })
variableId?: number;

@Column({ nullable: true })
rewardMetricKey?: string;

@Column({ nullable: true })
outcomeVariableName?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import { plainToClass } from 'class-transformer';
import { StratificationFactorRepository } from '../repositories/StratificationFactorRepository';
import { ExperimentDetailsForCSVData } from '../repositories/AnalyticsRepository';
import { compare } from 'compare-versions';
import { MetricService } from './MetricService';

const errorRemovePart = 'instance of ExperimentDTO has failed the validation:\n - ';
const stratificationErrorMessage =
Expand Down Expand Up @@ -126,7 +127,8 @@ export class ExperimentService {
protected scheduledJobService: ScheduledJobService,
protected errorService: ErrorService,
protected cacheService: CacheService,
protected queryService: QueryService
protected queryService: QueryService,
protected metricService: MetricService
) {}

public async find(logger?: UpgradeLogger): Promise<ExperimentDTO[]> {
Expand Down
39 changes: 13 additions & 26 deletions backend/packages/Upgrade/src/api/services/MoocletDataService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
MoocletPolicyParametersResponseDetails,
MoocletVariableRequestBody,
MoocletVariableResponseDetails,
MoocletValueRequestBody,
MoocletValueResponseDetails,
} from '../../types/Mooclet';
import { UpgradeLogger } from '../../../src/lib/logger/UpgradeLogger';

Expand Down Expand Up @@ -154,35 +156,20 @@ export class MoocletDataService {
return response;
}

/* not yet implemented */
public async postNewReward(requestBody: MoocletValueRequestBody): Promise<MoocletValueResponseDetails> {
const endpoint = '/value';

// public async postNewValue(requestBody: any): Promise<any> {
// const endpoint = '/value';

// const requestParams: MoocletProxyRequestParams = {
// method: 'POST',
// url: this.apiUrl + endpoint,
// apiToken: this.apiToken,
// body: requestBody,
// };

// const response = await this.fetchExternalMoocletsData(requestParams);

// return response;
// }

// public async deleteValue(valueId: number): Promise<any> {
// const endpoint = `/value/${valueId}`;
// const requestParams: MoocletProxyRequestParams = {
// method: 'DELETE',
// url: this.apiUrl + endpoint,
// apiToken: this.apiToken,
// };
const requestParams: MoocletProxyRequestParams = {
method: 'POST',
url: this.apiUrl + endpoint,
apiToken: this.apiToken,
body: requestBody,
};

// const response = await this.fetchExternalMoocletsData(requestParams);
const response = await this.fetchExternalMoocletsData(requestParams);

// return response;
// }
return response;
}

public async postNewVariable(requestBody: MoocletVariableRequestBody): Promise<MoocletVariableResponseDetails> {
const endpoint = '/variable';
Expand Down
116 changes: 93 additions & 23 deletions backend/packages/Upgrade/src/api/services/MoocletExperimentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import { PreviewUserService } from './PreviewUserService';
import { QueryService } from './QueryService';
import { ScheduledJobService } from './ScheduledJobService';
import { SegmentService } from './SegmentService';
import { UnprocessableEntityException } from '@nestjs/common';
import { MoocletExperimentRef } from '../models/MoocletExperimentRef';
import { MoocletVersionConditionMap } from '../models/MoocletVersionConditionMap';
import { v4 as uuid } from 'uuid';
Expand All @@ -47,8 +46,17 @@ import { ConditionValidator, ExperimentDTO } from '../DTO/ExperimentDTO';
import { UserDTO } from '../DTO/UserDTO';
import { Experiment } from '../models/Experiment';
import { UpgradeLogger } from '../../lib/logger/UpgradeLogger';
import { ASSIGNMENT_ALGORITHM, MoocletPolicyParametersDTO, SUPPORTED_MOOCLET_ALGORITHMS } from 'upgrade_types';
import {
ASSIGNMENT_ALGORITHM,
IMetricMetaData,
MoocletPolicyParametersDTO,
MoocletTSConfigurablePolicyParametersDTO,
REPEATED_MEASURE,
SUPPORTED_MOOCLET_ALGORITHMS,
} from 'upgrade_types';
import { ExperimentCondition } from '../models/ExperimentCondition';
import { MetricService } from './MetricService';
import { Metric } from '../models/Metric';

export interface SyncCreateParams {
experimentDTO: ExperimentDTO;
Expand Down Expand Up @@ -96,7 +104,8 @@ export class MoocletExperimentService extends ExperimentService {
scheduledJobService: ScheduledJobService,
errorService: ErrorService,
cacheService: CacheService,
queryService: QueryService
queryService: QueryService,
metricService: MetricService
) {
super(
experimentRepository,
Expand Down Expand Up @@ -124,7 +133,8 @@ export class MoocletExperimentService extends ExperimentService {
scheduledJobService,
errorService,
cacheService,
queryService
queryService,
metricService
);
}

Expand All @@ -136,11 +146,38 @@ export class MoocletExperimentService extends ExperimentService {
return this.dataSource.transaction((manager) => this.handleDeleteMoocletTransaction(manager, params));
}

/**
* handleCreateMoocletTransaction
*
* 1. Create and save an experiment-specific reward metric for the experiment
* 2. Attach a percent-success reward metric query to the experimentDTO before saving
* 3. Save the upgrade experiment
* 4. Create and save the Mooclet experiment resources (outputs MoocletExperimentRef)
* 5. Assign the rewardMetricKey to the MoocletExperimentRef
* 6. Save the MoocletExperimentRef and VersionConditionMaps
*
* On any error, rollback the Mooclet resources and abort the transaction
*/

private async handleCreateMoocletTransaction(
manager: EntityManager,
params: SyncCreateParams
): Promise<ExperimentDTO> {
const moocletPolicyParameters = params.experimentDTO.moocletPolicyParameters;
const { moocletPolicyParameters, queries, rewardMetricKey, context } = params.experimentDTO;

// create reward metric
try {
await this.createAndSaveRewardMetric(rewardMetricKey, context[0]);
} catch (error) {
logger.error({
message: 'Failed to create reward metric',
error,
rewardMetricKey,
});
throw error;
}

this.attachRewardMetricQuery(rewardMetricKey, queries);

// create Upgrade Experiment. If this fails, then mooclet resources will not be created, and the UpGrade experiment transaction will abort
const experimentResponse = await this.createExperiment(manager, params);
Expand All @@ -151,6 +188,8 @@ export class MoocletExperimentService extends ExperimentService {
moocletPolicyParameters
);

moocletExperimentRefResponse.rewardMetricKey = rewardMetricKey;

logger.info({
message: 'Mooclet experiment created successfully:',
moocletExperimentRef: JSON.stringify(moocletExperimentRefResponse),
Expand Down Expand Up @@ -215,6 +254,11 @@ export class MoocletExperimentService extends ExperimentService {
deleteResponse = await super.delete(params.experimentId, currentUser, {
existingEntityManager: manager,
});

if (moocletExperimentRef.rewardMetricKey) {
await manager.getRepository(Metric).delete(moocletExperimentRef.rewardMetricKey);
}

// delete the mooclet resources. If this fails, the transaction will abort and the upgrade experiment will not be deleted,
// but the Mooclet resources may not be deleted either
await this.orchestrateDeleteMoocletResources(moocletExperimentRef);
Expand All @@ -232,7 +276,7 @@ export class MoocletExperimentService extends ExperimentService {
experimentId: params.experimentId,
user: currentUser,
});
throw new UnprocessableEntityException('Failed to delete experiment with Mooclet');
throw error;
}
}

Expand Down Expand Up @@ -299,16 +343,16 @@ export class MoocletExperimentService extends ExperimentService {
});

const moocletVariableResponse = await this.createVariableIfNeeded(
moocletPolicyParametersResponse,
upgradeExperiment.assignmentAlgorithm,
moocletResponse
moocletPolicyParameters,
upgradeExperiment.assignmentAlgorithm
);
logger.debug({
message: `[Mooclet Creation] 5. Variable created (if needed):`,
moocletVariableResponse: JSON.stringify(moocletVariableResponse),
});

moocletExperimentRef.variableId = moocletVariableResponse?.id;
moocletExperimentRef.outcomeVariableName = moocletVariableResponse?.name;
} catch (err) {
await this.orchestrateDeleteMoocletResources(moocletExperimentRef);
throw err;
Expand All @@ -319,6 +363,33 @@ export class MoocletExperimentService extends ExperimentService {
return moocletExperimentRef;
}

private attachRewardMetricQuery(rewardMetricKey: string, queries: any[]) {
queries.push({
id: uuid(),
name: 'Success Rate',
query: {
operationType: 'percentage',
compareFn: '=',
compareValue: 'SUCCESS',
},
metric: {
key: rewardMetricKey,
},
repeatedMeasure: REPEATED_MEASURE.mostRecent,
});
}

public async createAndSaveRewardMetric(rewardMetricKey: string, context: string): Promise<Metric[]> {
const metric = {
id: uuid(),
metric: rewardMetricKey,
datatype: IMetricMetaData.CATEGORICAL,
allowedValues: ['SUCCESS', 'FAILURE'],
};

return this.metricService.saveAllMetrics([metric], [context], logger);
}

private createMoocletVersionConditionMaps(
moocletVersionsResponse: MoocletVersionResponseDetails[],
upgradeExperiment: ExperimentDTO
Expand Down Expand Up @@ -409,12 +480,16 @@ export class MoocletExperimentService extends ExperimentService {
}
}

public async getPolicyParametersByExperimentId(
experimentId: string
): Promise<MoocletPolicyParametersResponseDetails> {
public async attachRewardKeyAndPolicyParamsToExperimentDTO(experiment: ExperimentDTO): Promise<ExperimentDTO> {
try {
const moocletExperimentRef = await this.getMoocletExperimentRefByUpgradeExperimentId(experimentId);
return await this.moocletDataService.getPolicyParameters(moocletExperimentRef.policyParametersId);
const moocletExperimentRef = await this.getMoocletExperimentRefByUpgradeExperimentId(experiment.id);
const policyParamters = await this.moocletDataService.getPolicyParameters(
moocletExperimentRef.policyParametersId
);
experiment.rewardMetricKey = moocletExperimentRef.rewardMetricKey;
experiment.moocletPolicyParameters = policyParamters.parameters;

return experiment;
} catch (err) {
throw new Error(`Failed to get Mooclet policy parameters: ${err}`);
}
Expand Down Expand Up @@ -484,16 +559,15 @@ export class MoocletExperimentService extends ExperimentService {
}

private async createVariableIfNeeded(
moocletPolicyParametersResponse: MoocletPolicyParametersResponseDetails,
assignmentAlgorithm: string,
moocletResponse: MoocletResponseDetails
moocletPolicyParameters: MoocletPolicyParametersDTO,
assignmentAlgorithm: string
): Promise<MoocletVariableResponseDetails> {
if (!moocletPolicyParametersResponse || assignmentAlgorithm !== ASSIGNMENT_ALGORITHM.MOOCLET_TS_CONFIGURABLE) {
if (!moocletPolicyParameters || assignmentAlgorithm !== ASSIGNMENT_ALGORITHM.MOOCLET_TS_CONFIGURABLE) {
return null;
}

const variableRequest: MoocletVariableRequestBody = {
name: this.createOutcomeVariableName(moocletResponse.name),
name: (moocletPolicyParameters as MoocletTSConfigurablePolicyParametersDTO)?.outcome_variable_name,
};

try {
Expand All @@ -504,10 +578,6 @@ export class MoocletExperimentService extends ExperimentService {
}
}

private createOutcomeVariableName(moocletName: string): string {
return 'TS_CONFIG_' + moocletName;
}

public async getMoocletExperimentRefByUpgradeExperimentId(
upgradeExperimentId: string
): Promise<MoocletExperimentRef | undefined> {
Expand Down
Loading
Loading