removed mooclet infrastructure, implemented native thompson-sampling … - #3304
Open
danoswaltCL wants to merge 10 commits into
Open
removed mooclet infrastructure, implemented native thompson-sampling …#3304danoswaltCL wants to merge 10 commits into
danoswaltCL wants to merge 10 commits into
Conversation
…algorithm, added weight estimation mechanism Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the migration away from the external Mooclet integration by implementing native Thompson Sampling end-to-end (backend, frontend, shared types, and client SDKs), and adds an “estimated weight” mechanism in reward summaries.
Changes:
- Replaced Mooclet adaptive experiment infrastructure with native Thompson Sampling services, entities, migrations, and endpoints.
- Updated frontend to use
thompsonSamplingConfig, added reward summary UI column (estimatedWeight) and updated labels/tooltips. - Updated shared
upgrade_typesand client libraries (JS/Python) to reflect the new reward behavior and removed Mooclet artifacts/config flags.
Reviewed changes
Copilot reviewed 93 out of 93 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| postman/ClientAPI.postman_collection.json | Update reward endpoint description |
| packages/types/src/Mooclet/MoocletTSConfigurablePolicyParametersDTO.ts | Remove Mooclet TS DTO |
| packages/types/src/Mooclet/MoocletPolicyParametersDTO.ts | Remove Mooclet policy base DTO |
| packages/types/src/Mooclet/index.ts | Remove Mooclet exports/constants |
| packages/types/src/index.ts | Re-export moved experiment interfaces |
| packages/types/src/Experiment/interfaces.ts | Add priors/reward types + estimatedWeight |
| packages/types/src/Experiment/enums.ts | Rename algorithm enum; remove Mooclet errors |
| packages/types/CLAUDE.md | Remove Mooclet folder note |
| packages/frontend/projects/upgrade/src/environments/environment.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.staging.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.qa.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.prod.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.local.example.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.demo.prod.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment.bsnl.ts | Remove moocletToggle |
| packages/frontend/projects/upgrade/src/environments/environment-types.ts | Remove moocletToggle; rename rewards endpoint |
| packages/frontend/projects/upgrade/src/assets/i18n/en.json | Rename TS labels; add estimated weight strings |
| packages/frontend/.../ts-configurable-reward-count-table.component.ts | Add estimatedWeight column + tooltip module |
| packages/frontend/.../ts-configurable-reward-count-table.component.scss | Add estimated-weight-column styling |
| packages/frontend/.../ts-configurable-reward-count-table.component.html | Render estimated weight column |
| packages/frontend/.../enrollment-condition-expandable-row.component.ts | Swap helper service to Thompson Sampling |
| packages/frontend/.../enrollment-condition-expandable-row.component.html | Rename mooclet checks to TS checks |
| packages/frontend/.../experiment-details-page-content.component.ts | Show reward feedback for TS algorithm |
| packages/frontend/.../experiment-conditions-table.component.ts | Rename input; switch TS column set |
| packages/frontend/.../experiment-conditions-table.component.html | Rename bindings; TS-specific N/A rendering |
| packages/frontend/.../experiment-conditions-section-card.component.ts | Swap helper; use thompsonSamplingConfig priors |
| packages/frontend/.../experiment-conditions-section-card.component.html | Bind to thompsonSamplingConfig priors |
| packages/frontend/.../upsert-experiment-modal.component.ts | Replace mooclet params with thompsonSamplingConfig |
| packages/frontend/.../upsert-experiment-modal.component.html | Render TS form for TS algorithm |
| packages/frontend/.../ts-configurable-policy-parameters-form.component.ts | Convert form to TS config fields |
| packages/frontend/.../ts-configurable-policy-parameters-form.component.html | Update control names for renamed fields |
| packages/frontend/.../edit-condition-prior-modal.component.ts | Use ThompsonSamplingHelperService validators |
| packages/frontend/.../thompson-sampling-helper.service.ts | New TS helper + validators + overview formatting |
| packages/frontend/.../experiments.selectors.ts | Use TS overview formatter; rename disabled field |
| packages/frontend/.../experiments.model.ts | Add ThompsonSamplingConfigDTO + overview labels rename |
| packages/frontend/.../experiments.effects.ts | Fetch rewards summary via new data service method |
| packages/frontend/.../experiments.effects.spec.ts | Remove old rewards effect tests |
| packages/frontend/.../mooclet-helper.service.ts | Remove Mooclet helper service |
| packages/frontend/.../mooclet-helper.service.spec.ts | Remove Mooclet helper service tests |
| packages/frontend/.../experiments.service.ts | Update prior update path to thompsonSamplingConfig.priors |
| packages/frontend/.../experiments.data.service.ts | Add fetchRewardsDataForExperiment; remove mooclet fetch |
| packages/frontend/.../api-endpoints.constants.ts | Rename rewards endpoint constant |
| packages/backend/test/unit/services/ThompsonSamplingService.test.ts | Add TS selection + weight estimation tests |
| packages/backend/test/unit/services/MoocletDataService.test.ts | Remove Mooclet data service tests |
| packages/backend/test/unit/services/ExperimentService.test.ts | Remove Mooclet deps from unit wiring/comments |
| packages/backend/test/unit/services/ExperimentAssignmentService.test.ts | Remove Mooclet mock; add TS placeholders |
| packages/backend/test/unit/controllers/mocks/MoocletRewardsServiceMock.ts | Remove Mooclet rewards mock |
| packages/backend/test/unit/controllers/mocks/MoocletExperimentServiceMock.ts | Remove Mooclet experiment mock |
| packages/backend/test/unit/controllers/ExperimentController.test.ts | Remove Mooclet cases; add TS crud placeholder |
| packages/backend/src/types/Mooclet.ts | Remove Mooclet type definitions |
| packages/backend/src/env.ts | Remove mooclets env block |
| packages/backend/src/database/migrations/1781395200000-bootstrapThompsonSamplingConfigs.ts | Bootstrap missing TS configs/posteriors |
| packages/backend/src/database/migrations/1781308800000-cleanupMoocletEntities.ts | Drop Mooclet tables; migrate enum value |
| packages/backend/src/database/migrations/1781222400000-thompsonSamplingEntities.ts | Create TS tables + enum addition |
| packages/backend/src/api/services/ThompsonSamplingService.ts | New TS selection + weight estimation engine |
| packages/backend/src/api/services/ThompsonSamplingRewardService.ts | New native reward recording + posterior increment |
| packages/backend/src/api/services/ThompsonSamplingExperimentCrudService.ts | New TS config CRUD + rewards summary |
| packages/backend/src/api/services/MoocletRewardsService.ts | Remove Mooclet rewards service |
| packages/backend/src/api/services/MoocletDataService.ts | Remove Mooclet proxy/data service |
| packages/backend/src/api/services/ImportExportService.ts | Remove Mooclet import/export paths |
| packages/backend/src/api/services/ExperimentService.ts | Remove Mooclet validation/transaction notes |
| packages/backend/src/api/services/ExperimentAssignmentService.ts | Replace Mooclet assignment with TS assignment |
| packages/backend/src/api/repositories/ThompsonSamplingRewardRepository.ts | New TS reward repository |
| packages/backend/src/api/repositories/ThompsonSamplingExperimentConfigRepository.ts | New TS config repository queries |
| packages/backend/src/api/repositories/MoocletExperimentRefRepository.ts | Remove Mooclet ref repository |
| packages/backend/src/api/repositories/ConditionPosteriorStateRepository.ts | New posterior state repository |
| packages/backend/src/api/models/ThompsonSamplingReward.ts | New reward entity |
| packages/backend/src/api/models/ThompsonSamplingExperimentConfig.ts | New TS config entity |
| packages/backend/src/api/models/MoocletVersionConditionMap.ts | Remove Mooclet mapping entity |
| packages/backend/src/api/models/MoocletExperimentRef.ts | Remove Mooclet ref entity |
| packages/backend/src/api/models/ConditionPosteriorState.ts | New posterior state entity |
| packages/backend/src/api/middlewares/ErrorHandlerMiddleware.ts | Remove Mooclet error cases |
| packages/backend/src/api/errors/MoocletError.ts | Remove Mooclet error type |
| packages/backend/src/api/DTO/ExperimentDTO.ts | Replace moocletPolicyParameters with thompsonSamplingConfig |
| packages/backend/src/api/controllers/ExperimentController.ts | Create/update TS config; add rewards summary endpoint |
| packages/backend/src/api/controllers/ExperimentClientController.v6.ts | Rewire /v6/reward to native TS reward service |
| packages/backend/rest-client-vscode/MoocletAPI.http | Remove Mooclet REST client doc |
| packages/backend/CLAUDE.md | Remove Mooclet transaction notes |
| packages/backend/.env.example | Remove MOOCLETS_* env vars |
| packages/backend/.env.docker.local.example | Remove MOOCLETS_* env vars |
| clientlibs/python/tests/test_client.py | Update reward response expectations |
| clientlibs/python/tests/test_api_service.py | Update reward response expectations |
| clientlibs/python/src/upgrade_client_lib/types/responses.py | Remove RewardDetails from response model |
| clientlibs/python/src/upgrade_client_lib/types/init.py | Stop exporting RewardDetails |
| clientlibs/python/src/upgrade_client_lib/client.py | Update reward docstring wording |
| clientlibs/python/BUILD_PLAN.md | Update reward response contract |
| clientlibs/js/src/UpGradeClient/UpgradeClient.ts | Update reward docstring wording |
| clientlibs/js/src/types/Interfaces.ts | Remove reward details from response interface |
| CLAUDE.md | Document native TS migration plan/progress |
| .claude/skills/setup-perftrace/SKILL.md | Update /v6/reward instrumentation reference |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… instead of waiting, update the wording of some things
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Several backend paths have correctness risks (TS config update semantics, repo query filtering, and reward batching concurrency) that can lead to mis-recorded rewards or broken Thompson assignment after updates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
packages/backend/src/api/services/ThompsonSamplingExperimentCrudService.ts:77
- invalidateConfigCache() runs before priors are updated (and returns early when priors are absent). If priors are updated, the cache can be repopulated with stale ConditionPosteriorState rows between invalidation and the later writes, causing TTL-bounded staleness on the reward path.
packages/backend/src/api/repositories/ThompsonSamplingExperimentConfigRepository.ts:36 - findConfigsForActivelyEnrollingExperiments() should also filter by experiment.assignmentAlgorithm = THOMPSON_SAMPLING; otherwise it can return configs for non-TS experiments if those rows exist.
public async findConfigsForActivelyEnrollingExperiments(): Promise<ThompsonSamplingExperimentConfig[]> {
return this.createQueryBuilder('config')
.leftJoinAndSelect('config.conditionPosteriorStates', 'conditionPosteriorStates')
.leftJoinAndSelect('config.experiment', 'experiment')
.where('experiment.state = :state', { state: EXPERIMENT_STATE.ENROLLING })
.getMany();
- Files reviewed: 95/95 changed files
- Comments generated: 5
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…tions later, avoids larger refactors since we will not likely add many algorithms, and we just dont want to guess and make overly abstract for no reason
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Thompson Sampling reward batching/flush logic is vulnerable to concurrent double-flushes that can corrupt posterior counts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
packages/backend/src/api/services/ThompsonSamplingRewardService.ts:174
- flushIfBatchReady() reads all posterior states and then flushes pending counts using the previously-read values, but there’s no locking/transactional guard. If multiple rewards are processed concurrently for the same config, two calls can observe the same pending counts and both flush them, double-incrementing totalCount/successCount/failureCount.
- Files reviewed: 100/100 changed files
- Comments generated: 2
- Review effort level: Lite
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical transactionality, reward durability, concurrency, migration, and compatibility issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
packages/backend/src/api/services/ThompsonSamplingExperimentCrudService.ts:110
- Imported and duplicated priors are lost here.
ExperimentService.addExperimentInDB()regenerates every condition ID (ExperimentService.ts:1342-1349), while thepriorsrecord remains keyed by the source IDs, so every lookup by the new ID misses and silently falls back to Beta(1,1). Carry the old-to-new condition mapping into adaptive config creation and remap the prior keys.
packages/backend/src/api/services/ThompsonSamplingRewardService.ts:105 - The raw reward is committed before the posterior state is verified or updated. If the state lookup or any later increment fails, the audit table contains a reward that never affects assignment, and there is no reconciliation caller for
findByExperimentAndCondition; because the HTTP request already succeeded, the client cannot retry. Persist the event and posterior mutation atomically, or use a transactional outbox with retry processing.
- Files reviewed: 100/100 changed files
- Comments generated: 10
- Review effort level: Balanced
Comment on lines
+1050
to
+1052
| const createdExperiment = await this.experimentService.create(experiment, currentUser, request.logger); | ||
|
|
||
| return this.experimentService.create(experiment, currentUser, request.logger); | ||
| await this.adaptiveExperimentConfigDispatcher.createConfigIfApplicable(experiment, createdExperiment); |
| request: RewardValidator, | ||
| logger: UpgradeLogger | ||
| ): IThompsonSamplingRewardResponse { | ||
| this.processReward(user, request, logger).catch((error) => { |
Comment on lines
+150
to
+154
| await this.posteriorStateRepository.increment({ id: state.id }, 'pendingTotalCount', 1); | ||
| if (success) { | ||
| await this.posteriorStateRepository.increment({ id: state.id }, 'pendingSuccessCount', 1); | ||
| } else { | ||
| await this.posteriorStateRepository.increment({ id: state.id }, 'pendingFailureCount', 1); |
Comment on lines
+119
to
+123
| await queryRunner.query(` | ||
| INSERT INTO "thompson_sampling_experiment_config" ("experimentId", "versionNumber") | ||
| SELECT e.id, 1 | ||
| FROM "experiment" e | ||
| WHERE e."assignmentAlgorithm" = 'thompson_sampling' |
Comment on lines
+183
to
+185
| { | ||
| value: ASSIGNMENT_ALGORITHM.THOMPSON_SAMPLING, | ||
| description: 'experiments.upsert-experiment-modal.assignment-algorithm-thompson-sampling-description.text', |
Comment on lines
+44
to
+46
| const result = await this.experimentService.create(experiment, currentUser, logger); | ||
| await this.adaptiveExperimentConfigDispatcher.createConfigIfApplicable(experiment, result); | ||
| createdExperiments.push(await this.adaptiveExperimentConfigDispatcher.attachConfigToExperiment(result)); |
Comment on lines
+59
to
+67
| public async syncConfigIfApplicable(experiment: ExperimentDTO, updatedExperiment: ExperimentDTO): Promise<void> { | ||
| if (experiment.assignmentAlgorithm !== ASSIGNMENT_ALGORITHM.THOMPSON_SAMPLING) { | ||
| return; | ||
| } | ||
| await this.syncConditions(updatedExperiment.id, updatedExperiment.conditions); | ||
| if (experiment.thompsonSamplingConfig) { | ||
| await this.updateConfig(updatedExperiment.id, experiment.thompsonSamplingConfig); | ||
| } | ||
| } |
| `ALTER TYPE "public"."experiment_assignmentalgorithm_enum" RENAME TO "experiment_assignmentalgorithm_enum_old"` | ||
| ); | ||
| await queryRunner.query( | ||
| `CREATE TYPE "public"."experiment_assignmentalgorithm_enum" AS ENUM('random', 'stratified random sampling', 'uniform_random', 'thompson_sampling')` |
| `ALTER TYPE "public"."experiment_assignmentalgorithm_enum" RENAME TO "experiment_assignmentalgorithm_enum_old"` | ||
| ); | ||
| await queryRunner.query( | ||
| `CREATE TYPE "public"."experiment_assignmentalgorithm_enum" AS ENUM('random', 'stratified random sampling', 'uniform_random', 'ts_configurable')` |
Comment on lines
496
to
+497
| } else { | ||
| throw new Error(`Unsupported mooclet algorithm selected: ${algorithm}`); | ||
| this.thompsonSamplingConfigFormValue = undefined; |
…ig sync, concurrent reward race, TS+within-subjects validation, orphaned experiment on partial create failure, weight-map collision, partial-update field nulling, and stuck TS form validity Verified each finding against current HEAD before fixing (several of Copilot's 24 comments were already stale, posted against earlier commits) — see CLAUDE.md for the full rundown of what was fixed vs. already resolved vs. out of scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
danoswaltCL
requested review from
bcb37 and
zackcl
and
a balanced review from Copilot
September 4, 2026 20:18
danoswaltCL
marked this pull request as ready for review
September 4, 2026 20:18
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two critical transaction-consistency defects and two moderate reward-durability/prior-remapping defects remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
packages/backend/src/api/services/ThompsonSamplingRewardService.ts:54
- This acknowledges the reward before any durable write has occurred. If the process restarts, crashes, or is scaled down after the response but before this untracked promise completes, the caller receives success while the reward is permanently lost. Put the work on a durable queue before acknowledging, or await at least a durable reward-event insert and process posterior updates asynchronously from that record.
- Files reviewed: 101/101 changed files
- Comments generated: 3
- Review effort level: Balanced
Comment on lines
+1250
to
+1252
| const updatedExperiment = await this.experimentService.update({ ...experiment, id }, currentUser, request.logger); | ||
|
|
||
| if (updatedMoocletExperiment) { | ||
| return updatedMoocletExperiment; | ||
| } | ||
| } else { | ||
| // if mooclet is not enabled, but experiment has mooclet params, throw error | ||
| if ('moocletPolicyParameters' in experiment) { | ||
| throw new BadRequestError( | ||
| 'Failed to update Experiment: moocletPolicyParameters was provided but mooclets are not enabled on backend.' | ||
| ); | ||
| } | ||
| } | ||
| await this.adaptiveExperimentConfigDispatcher.syncConfigIfApplicable(experiment, updatedExperiment); |
Comment on lines
+101
to
+105
| await this.tsRewardRepository.save({ | ||
| experimentId: config.experimentId, | ||
| conditionId, | ||
| userId: user.id, | ||
| success, |
Comment on lines
+47
to
+50
| await this.createConfig( | ||
| createdExperiment.id, | ||
| createdExperiment.conditions, | ||
| experiment.thompsonSamplingConfig ?? {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…algorithm, added weight estimation mechanism