allow passing a decision point via in assign call by danoswaltCL · Pull Request #3414 · CarnegieLearningWeb/UpGrade

danoswaltCL · 2026-11-06T21:14:06Z

API /assign endpoint updated to allow optional decisionPoint to limit the response to just that assignment
Java client updated to rewire the getExperimentCondition method to this new usage automatically
Full-context fetching of valid experiments remains intact, as this is needed for experiment-pooling and keeping the server instance cache warm; this is now pretty optimized and doesn't add overhead, so we don't trim the response until after selecting the experiment from the pool.

Also:

Most of the 'extra' files changed are from reconciling multiple "Decision-Point" types (it got confusing to know which implementation to use or borrow).
Refactors slightly in order to take advantage of shared utility functions
Adds ability to pass --singleDecisionPoint flag to quickburst tester

Testing showed this to be potentially 30%+ quicker /assign calls, across the board

update: this commit [cf42232](cf42232) adds a strategy for caching in client: there is a boolean flag called fullContextCached.

So, if a "full-context" /assign is called, then a "single-decision-point" assign is called, fullContextCached will be true so we will look for that decision-point in the cache and not call out to the server if found.

Whereas if "single-decision-point" /assign is called first, fullContextCached will be false, so that if /assign is then called, it will know to reach out to server.

And if "single-decision-point" /assign is called first, and later is called again w/o a full-context /assign having occurred, the logic will be to look for it in the cache first and if not there, go to the server.

Note the current behavior if the assign call comes back as [], it always refetches if it is called again (it doesn't know that [] was fetched from the server, so this didn't change that. That seems unintentional but also probably not worth changing.
cf42232

…se to just that one assignment. consolidates DecisionPoint types

Copilot

🟡 Changes recommended

The uncached scoped lookup is case-sensitive while the warm-cache lookup is not, causing previously valid decision-point lookups to fail depending on cache state.

1 open issue

Preserve case-insensitive lookup on scoped cache misses · New

What changed in this PR

Adds optional decision-point scoping to /assign and integrates it into the Java client, reducing per-request assignment work while retaining full-context fetching and experiment-pool selection.

Changes:

Limits assignment evaluation to the relevant pool and returns only the requested decision point.
Consolidates decision-point validation and adds scoped-assignment regression tests.
Updates Java single-condition requests and adds a --singleDecisionPoint load-testing mode.

File	Description
`packages/backend/test/unit/services/ExperimentAssignmentService.test.ts`	Tests scoped assignment, pooling, and mark consistency.
`packages/backend/test/unit/controllers/validators/ExperimentAssignmentValidator.test.ts`	Tests optional decision-point validation and target defaults.
`packages/backend/src/api/services/ThompsonSamplingRewardService.ts`	Reuses the shared decision-point reference type.
`packages/backend/src/api/services/ExperimentAssignmentService.ts`	Scopes evaluation to relevant pools and filters returned assignments.
`packages/backend/src/api/controllers/validators/RewardValidator.ts`	Uses shared nested decision-point validation.
`packages/backend/src/api/controllers/validators/MarkExperimentValidator.v6.ts`	Inherits shared decision-point validation.
`packages/backend/src/api/controllers/validators/ExperimentAssignmentValidator.ts`	Accepts an optional decision point.
`packages/backend/src/api/controllers/validators/DecisionPointValidator.ts`	Defines shared validation and target normalization.
`packages/backend/src/api/controllers/validators/BatchAssignValidator.ts`	Reuses shared decision-point validation.
`packages/backend/src/api/controllers/ExperimentClientController.v6.ts`	Documents and forwards the scoped-assignment parameter.
`packages/backend/CLAUDE.md`	Documents dormant preview-user behavior.
`clientlibs/java/src/main/resources/org/upgradeplatform/client/quicktest-burst-ui.html`	Adds an assignment-mode selector.
`clientlibs/java/src/main/java/org/upgradeplatform/requestbeans/ExperimentRequest.java`	Adds an optional decision-point request payload.
`clientlibs/java/src/main/java/org/upgradeplatform/client/QuickTestBurst.java`	Adds scoped-request load testing.
`clientlibs/java/src/main/java/org/upgradeplatform/client/ExperimentClient.java`	Shares request handling and scopes uncached single-condition lookups.
`clientlibs/java/CLAUDE.md`	Documents updated lookup and caching behavior.

_{Review effort: Balanced}

danoswaltCL · 2026-11-09T20:20:41Z

updated to include logic for caching rules for how assign will know whether to look in local cache or skip it, see the note in description.

bcb37 · 2026-11-09T21:44:58Z

Heads-up: this PR and #3408 (client lib parity) both change how the Java client's assignment cache works. A trial merge of the two conflicts in ExperimentClient.java (5 hunks) and clientlibs/java/CLAUDE.md. Beyond the text conflicts, there are a few places where the combined code would compile but misbehave, so whichever lands second needs a deliberate resolution:

Cache clearing vs. fullContextCached (the important one). Client lib parity: audit and Phases A–D #3408 clears the assignment and flag caches once the server confirms init, setGroupMembership, setWorkingGroup or setAltUserIds (invalidateUserDependentCaches() sets allExperiments = null). Here, getAllExperimentConditions() decides whether to fetch from fullContextCached rather than from the list being null. So after a group change the flag would still be true with a null list, and getAllExperimentConditions() would pass null to onSuccess instead of refetching. The clear needs to reset fullContextCached as well.
Two different locks. Client lib parity: audit and Phases A–D #3408 guards allExperiments with assignmentLock: the cache write, the clear, within-subjects rotation, and the lookup that copies an assignment out (the last two were a Copilot fix, so a lookup never sees conditions rotated without their factors). This PR uses synchronized methods (locking on this) plus a volatile flag, and reads the list unlocked. Those don't exclude each other. The list and fullContextCached should change together under assignmentLock.
Matching. isAtDecisionPoint() keeps the old case-insensitive match (and the getTarget().toString() NPE on a null target). Client lib parity: audit and Phases A–D #3408 made decision-point matching exact and null-safe, since the server's /mark and /reward lookups are case-sensitive. Because isAtDecisionPoint() also decides cache hits here, it should use the same exact match.
Null-safety / duplicate helper. requestAssignments()'s failed() calls callbacks.onError(...) without a null check (Client lib parity: audit and Phases A–D #3408 made those null-safe throughout), and the new readResponseToCallback(..., GenericType<T>, UnaryOperator<T>) overlaps with Client lib parity: audit and Phases A–D #3408's (..., GenericType<T>, Consumer<T>) variant. Worth keeping one, null-safe.
Duplicate DecisionPoint beans. This PR adds ExperimentRequest.DecisionPoint; Client lib parity: audit and Phases A–D #3408 added a top-level requestbeans.DecisionPoint (site, target) for rewards. One could be reused.

Things that combine fine: the "no experiment" placeholder (an empty single-decision-point response still gives an Assignment with a null condition), within-subjects rotation on the merged list, and client-level marks filling in ids from whatever's cached. The backend validator refactor here doesn't change the /mark or /reward contracts.

Also, #3408 adds clientlibs/CLIENT_API_COVERAGE.md, which tracks which method and test implement each v6 client endpoint in JS, Java and Python. Once both are in, /assign's new decisionPoint needs a row there: Java ✅ (getExperimentCondition), JS and Python ❌ (or ➖ with a reason if that's intentional).

Since this PR's Java change is the smaller of the two, it's probably easiest if #3408 lands first and this one rebases onto it, using the points above as the checklist. Happy to help with the resolution.

danoswaltCL · 2026-11-11T15:25:13Z

@bcb37 fyi i'm working through this now, i'll need to do a full re-testing, i'll let you know if i run into anything. i'm only planning on doing the java client library for this PR to keep the review focused; once it is merged then i'll update the other libraries based on how that gets finalized.

…esponse Brings in client lib parity (#3408). Conflict resolution in the Java client's assignment cache: - fullContextCached is guarded by assignmentLock together with allExperiments (was a volatile flag plus synchronized methods) - invalidateUserDependentCaches() also resets fullContextCached, so a group or identity change refetches instead of returning null - isAtDecisionPoint() uses #3408's exact, null-safe match, for cache hits, lookups and mark metadata alike - getAllExperimentConditions() goes through requestAssignments(), keeping #3408's null-safe callbacks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…load requestAssignments() now uses the null-safe Consumer variant from #3408, so this branch's UnaryOperator variant has no callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Drops ExperimentRequest's inner DecisionPoint in favor of the top-level bean #3408 added for rewards. A null target is now sent as "" (as the reward request does) instead of being omitted; the server treats both as the targetless decision point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Covers the decisionPoint request body (including a null target sent as ""), caching a fetched decision point, re-asking after an empty response, answering from a full-context cache, getAllExperimentConditions ignoring a partial cache, and a cache clear resetting the full-context flag (that last one fails if invalidateUserDependentCaches() doesn't reset fullContextCached). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- CLIENT_API_COVERAGE.md: new row for /assign's decisionPoint. Java sends it from getExperimentCondition; JS and Python don't yet. - Java README: getExperimentCondition asks /assign for one decision point when the cache can't answer, and caches the result; the Caching section notes getAllExperimentConditions ignores a partial cache. - Java CLAUDE.md: fullContextCached is guarded by assignmentLock with the list and cleared with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Scoped and overlapping cold lookups can cache conflicting Thompson Sampling conditions, causing served assignments to diverge from enrollment and reward attribution or subsequent cached results.

2 open issues

1 resolved since last review

Preserve case-insensitive lookup on scoped cache misses

_{Review effort: Balanced}

+			}
+			List<ExperimentsResponse> merged = allExperiments != null ? new ArrayList<>(allExperiments)
+					: new ArrayList<>();
+			merged.addAll(experiments);


+			requestAssignments(new ExperimentRequest(this.context, site, target), this::cacheDecisionPoint,
+					toAssignment);


allow passing a decision point via in assign call to scope the respon…

a67f5c4

…se to just that one assignment. consolidates DecisionPoint types

danoswaltCL requested review from bcb37 and a balanced review from Copilot November 6, 2026 21:14

Copilot started reviewing on behalf of danoswaltCL November 6, 2026 21:14

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Comment thread clientlibs/java/src/main/java/org/upgradeplatform/client/ExperimentClient.java Resolved

caching strategy for individual and or context caches

cf42232

danoswaltCL and others added 5 commits November 11, 2026 10:46

Remove unused readResponseToCallback(GenericType, UnaryOperator) over…

1e112ba

…load requestAssignments() now uses the null-safe Consumer variant from #3408, so this branch's UnaryOperator variant has no callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

danoswaltCL requested a balanced review from Copilot November 11, 2026 22:17

Copilot started reviewing on behalf of danoswaltCL November 11, 2026 22:17

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Navigation Menu

Repository navigation

allow passing a decision point via in assign call - #3414

allow passing a decision point via in assign call#3414
danoswaltCL wants to merge 7 commits into
devfrom
wip/assign-single-dp-response

danoswaltCL commented Nov 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

danoswaltCL commented Nov 9, 2026

Uh oh!

bcb37 commented Nov 9, 2026

Uh oh!

danoswaltCL commented Nov 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		requestAssignments(new ExperimentRequest(this.context, site, target), this::cacheDecisionPoint,
		toAssignment);

Conversation

danoswaltCL commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

danoswaltCL commented Nov 9, 2026

Uh oh!

bcb37 commented Nov 9, 2026

Uh oh!

danoswaltCL commented Nov 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danoswaltCL commented Nov 6, 2026 •

edited

Loading