[bcb37]

Summary

The JS, Java, and Python client libs have drifted apart. This PR adds clientlibs/CLIENT_LIB_PARITY_AUDIT.md, an audit of all three against each other and the backend /v6 API with a phased reconciliation plan, and implements Phase A (correctness fixes) and Phase B (feature parity). No backend changes; everything uses the existing /v6 API.

Phase A: correctness fixes

Java

• markDecisionPoint now sends experimentId and condition id from the cached assignment, across all overloads and Assignment.markDecisionPoint(). Previously Java sent only conditionCode, so the server re-ran experiment selection and could mark a different experiment when several share a decision point.
• Within-subjects marks now rotate assignedFactor alongside assignedCondition.
• The assignment and feature-flag caches are now filled before onSuccess runs. Previously a call chained from inside the callback (e.g. a mark) could hit a cold cache.
• Null-safe callbacks, null-safe properties map, null-safe target lookup.
• First unit test suite (src/test/java, TestNG). It runs ExperimentClient against an in-process HttpServer.

JS

• DefaultHttpClient never retried: axios rejects non-2xx responses before the retry branch runs, and that branch would have retried 4xx rather than 5xx anyway. It now retries network errors and 5xx once, after a 300 ms wait, for idempotent requests only (see Retry policy), fails fast on 4xx, and sets a 5s default timeout. Because the retry code never ran before, turning on the old default of 3 retries would have meant up to ~22s of retrying that callers can't turn off, so the default is 1 (DEFAULT_MAX_RETRIES / DEFAULT_RETRY_BACKOFF_MS). Making retries configurable is left for a follow-up.
• ISendRewardResponse gains mode / success to match the backend.
• markDecisionPoint no longer throws when the assignment cache is null (e.g. after a failed /assign).
• Fixed 3 ApiService mark tests that already fail on dev: their expected bodies were missing context.

Python

• The _sync methods use ApiService's real sync implementations instead of asyncio.run(), so they work inside a running event loop (Jupyter, async frameworks).
• HTTP connections are pooled (see below). Adds close() / aclose() and with / async with support.

Phase B: feature parity

Rewards

• Java (previously had none): sendRewardForExperiment(value, experimentId, [awaitResult], cb) and sendRewardForDecisionPoint(value, context, site, target, [awaitResult], cb). Named methods mean a caller can't pass neither identifier. context is explicit, matching JS and Python. A 409 (with awaitResult) is reported to onError and not retried.
• JS: sendReward accepts awaitResult, sent as ?awaitResult=true. Python already had it.

Feature-flag session groups (groupsForSession + includeStoredUserGroups), previously JS-only

• Java: setFeatureFlagUserGroupsForSession(groups, includeStored) / clearFeatureFlagUserGroupsForSession(). Setter-only, so the constructor overloads don't double.
• Python: a feature_flag_user_groups_for_session= constructor kwarg plus set_feature_flag_user_groups_for_session(). Both accept a FeatureFlagUserGroupsForSession model or a dict, and both fields are required.
• In all three libs, changing the session groups clears the cached flags.

Dead endpoint methods removed

These were breaking removals, but every call already failed with a 404.

• JS logCaliper(): POST /v6/log/caliper was removed in remove all client API code for versions < 5 #1985.
• Java addGroupMetrics() / addSingleMetrics(): POST /v6/metric was never a client route. MATHia doesn't use them. The Metric bean stays because the /log response uses it.

Retry policy

Retries are limited to requests that are safe to repeat, in both JS and Java:

• Retried: /init, /groupmembership, /workinggroup, /useraliases (upserts), /assign (read-only), and /featureflag (records exposures if-not-exists).
• Never retried: /mark, /log and /reward. None of them are deduplicated server-side, so resending after a lost response would record the write twice, and a duplicated reward biases adaptive assignment.

JS marks safe requests with an optional retryable flag on the request config, which custom lite-build HTTP clients can also honor. This changes Java's existing behavior: it previously retried /mark and /log once. Python never retries.

Python connection pooling

The sync client is pooled per UpgradeClient. The async client is pooled only inside async with client:, because an AsyncClient must be closed on the loop that opened it. Outside that block each async request uses a short-lived client, same as dev, so successive asyncio.run() calls don't strand connection pools.

Phase C: aligning behavior

• Exact decision point matching everywhere. The server's /mark and /reward lookups are case-sensitive. Java matched case-insensitively, so it returned conditions whose marks the server couldn't attribute to the experiment. Python now treats a None target as "".
• Caches cleared when the user may have changed. In all three libs, init(), setGroupMembership, setWorkingGroup and setAltUserIds clear the assignment and feature-flag caches once the server confirms the call. Previously only Python's init() cleared anything. Clearing is best-effort: a lookup that was already in flight can still cache a response from before the change. That leaves the cache exactly as stale as dev always did, so it's still a strict improvement, and each README's new "Caching" section tells callers to await these calls before reading, or pass ignoreCache. (A generation-counter guard against that race was built, then removed after review as over-engineering.) Java reads and writes both caches under a lock, since they're written on Jersey threads and dev had no synchronization there. One side effect: within-subjects rotation restarts after one of these calls.
• Python Assignment.mark_decision_point() now rotates within-subjects assignments, like the client-level mark and like JS.
• Java's "no experiment here" placeholder keeps its null getAssignedCondition(), so callers that detect "no experiment" with a null check keep working. Making it an empty Condition to match JS was tried and reverted as a breaking change. Assignment gains null-safe getCondition()/getPayload(), an assignment with an empty condition list no longer throws, and the docs now say the condition is null for the placeholder (and that the Assignment itself is never null). The experiment type stays null as Java's "no experiment" signal; JS reports SIMPLE, and Python returns None.
• Java auth token is optional, like JS and Python. The Authorization header is only sent when there's a token; servers with client auth enabled (toCheckAuth) answer a request without one with a 401.
• READMEs recommend applying the payload and marking through the assignment. The payload defaults to the condition code, so this keeps working when custom payloads are added. They also warn that a client-level mark must pass the condition code: for Thompson Sampling (and group-assigned stratified random sampling) the server enrolls the user in the condition whose code matches what's sent, so a payload value silently skips enrollment.
• Documented rather than changed: markDecisionPoint argument order (changing it would silently swap string arguments for existing callers), and the timeout/retry differences, now described in each README (the comparison is in clientlibs/TIMEOUT_RETRY_POLICY.md). This also closes the JS quick start's code block, which was never closed.

Phase D: guardrails

• Java request-serialization tests for every endpoint. ExperimentClientTest checks the HTTP method and request body of /init (all three overloads), /groupmembership, /workinggroup, /useraliases, /assign, /mark, /log, /featureflag and /reward against what the backend's validators expect, plus the standard client headers. /init is checked for its exact field set, since the backend rejects unknown fields there. (The /groupmembership and /workinggroup beans also send null createdAt/updatedAt/versionNumber/requestedUserId, which the server strips.)
• Client API coverage checklist. clientlibs/CLIENT_API_COVERAGE.md records which method and test implement each v6 client endpoint in each library, with a workflow: mark libraries ❌ when an endpoint changes, flip them back to ✅ when implemented, and review before a client-library release. It deliberately doesn't restate the contract (the controller and its validators are the source of truth; swagger may lag), apart from two behaviors the libraries depend on: /init rejects unknown body fields, and /mark must get the condition code. The root CLAUDE.md points to it.

Testing

• JS: jest: 110/110 pass, ESLint/Prettier clean.
• Python: pytest: 240/240 pass; ruff check and mypy clean.
• Java: mvn test passes at the branch head (ExperimentClientTest, 32 tests). Also 32/32 from a mvn clean build run directly via TestNG, 5 runs in a row.
• ⚠️ The first commit (2bf0337) has a Java compile error (a stray brace); it's fixed in b19d284. Pre-commit hooks don't build the client libs, so review the Java at the branch head.

Not in this PR

• Configurable HTTP behavior (retries, timeouts) for the JS default client: a follow-up.
• Automated contract tests against the backend: not pursued; the coverage checklist covers the gap for now.
• Small cleanups: clientlibs/js/docs (TypeDoc output) is stale and needs regenerating; the Java README still documents several methods that no longer exist; Python's __version__ says 6.5.0; Python feature flags come back as FeatureFlag objects rather than strings; Java's group requests send harmless null fields.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Retries can duplicate non-idempotent writes and bias future assignments, while Python event-loop changes leave previous HTTP connection pools unclosed.

2 open issues

• Retries duplicate non-idempotent writes · New
• AsyncClient connection pools leak across event loops · New

What changed in this PR

This PR audits parity across the JavaScript, Java, and Python client libraries and implements Phase A correctness fixes without backend changes.

Changes:

• Java marks include cached assignment IDs, rotate factors with conditions, and populate caches before callbacks.
• JavaScript gains working retries, a default timeout, reward response fields, and null-safe assignment lookup.
• Python uses native synchronous requests and pooled HTTP clients with lifecycle methods; tests and documentation cover the updates.

File	Description
clientlibs/​python/​tests/​test_client.py	Adds sync, connection reuse, and lifecycle tests.
clientlibs/​python/​src/​upgrade_client_lib/​client.py	Uses native sync calls and exposes cleanup methods.
clientlibs/​python/​src/​upgrade_client_lib/​api_service.py	Pools HTTP clients and tracks async loop ownership.
clientlibs/​js/​src/​types/​Interfaces.ts	Adds reward response mode and success fields.
clientlibs/​js/​src/​DefaultHttpClient/​DefaultHttpClient.ts	Implements retries, backoff, and a default timeout.
clientlibs/​js/​src/​DefaultHttpClient/​DefaultHttpClient.spec.ts	Tests retry behavior and timeout configuration.
clientlibs/​js/​src/​DataService/​DataService.ts	Guards assignment lookup against a null cache.
clientlibs/​js/​src/​DataService/​DataService.spec.ts	Tests lookup before assignments are loaded.
clientlibs/​js/​src/​ApiService/​ApiService.spec.ts	Corrects expected mark bodies to include context.
clientlibs/​java/​src/​test/​java/​org/​upgradeplatform/​client/​ExperimentClientTest.java	Adds HTTP-backed client regression tests.
clientlibs/​java/​src/​main/​java/​org/​upgradeplatform/​utils/​APIService.java	Allows a null properties map.
clientlibs/​java/​src/​main/​java/​org/​upgradeplatform/​responsebeans/​Assignment.java	Includes assignment IDs in mark requests.
clientlibs/​java/​src/​main/​java/​org/​upgradeplatform/​client/​ExperimentClient.java	Updates mark metadata, rotation, cache timing, and null handling.
clientlibs/​java/​CLAUDE.md	Documents tests and configurable timeouts.
clientlibs/​java/​.gitignore	Ignores TestNG output.
clientlibs/​CLIENT_LIB_PARITY_AUDIT.md	Records client differences and a phased reconciliation plan.
CLAUDE.md	Corrects the client-library listing to name Python.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Both Python and Java clients allow in-flight feature-flag responses to repopulate invalidated caches after session groups change, causing subsequent lookups to keep returning flags evaluated for the old groups.

2 open issues

• Prevent stale requests from restoring invalidated feature-flag cache · New
• Guard feature-flag cache writes against session-group changes · New

2 resolved since last review

• Retries duplicate non-idempotent writes
• AsyncClient connection pools leak across event loops

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Unresolved thread-safety issues can leak HTTP client pools during lazy initialization and cause feature-flag lookups to raise AttributeError when session groups are cleared concurrently.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Protect lazy HTTP client initialization from concurrent access

clientlibs/​python/​src/​upgrade_client_lib/​api_service.py:120

Two threads making their first _sync call can both pass this check and construct separate httpx.Client instances. One assignment can overwrite the other pool, leaving it unavailable to close() or aclose() for cleanup. Protect lazy initialization with a per-instance lock and coordinate close() with the same lock.

Use a stable feature-flag options snapshot during request construction

clientlibs/​python/​src/​upgrade_client_lib/​api_service.py:333

A synchronous feature-flag lookup can fail if another thread clears the session groups between this check and model_dump(): the second attribute read sees None and raises AttributeError before sending the request. The cache-generation lock does not protect request-body construction. Capture the options in a local variable and use that same snapshot for both the check and serialization.

2 resolved since last review

• Guard feature-flag cache writes against session-group changes
• Prevent stale requests from restoring invalidated feature-flag cache

_{Review effort: Balanced}

[danoswaltCL]

@copilot can you turn this PR summary into a code-review requirement / QA checklist, organized by client library, that I can check off one by one online here in the github UI? refer to the phased planning docs within the files being added. This should be concatenated to the PR description (don't overwrite the current summary).

[Copilot]

@copilot can you edit this checklist based on the changes in the most recent commit?

@copilot can you turn this PR summary into a code-review requirement / QA checklist, organized by client library, that I can check off one by one online here in the github UI? refer to the phased plan...

I can’t directly edit the PR description with the GitHub operations available in this session. Here is an append-ready checklist for the description; it follows the phased plan in the parity audit, §5 and the timeout/retry policy.

---

Code-review / QA checklist

Java client

• Verify all markDecisionPoint overloads, including Assignment.markDecisionPoint(), send the cached experiment ID and condition ID.
• Verify within-subjects marks rotate both the assigned condition and factor.
• Verify assignment and feature-flag caches are populated before success callbacks; null callbacks, properties, and target lookups are safe.
• Verify reward methods cover both experiment-ID and decision-point lookup forms, require explicit identifiers/context, default awaitResult to false, and report awaited 409 responses through onError.
• Verify session feature-flag groups can be set/cleared, invalidate the flag cache, and prevent an in-flight response for old groups from repopulating it.
• Verify decision-point matching is exact; successful init, group-membership, and working-group changes invalidate the relevant caches, while alias changes do not.
• Verify a missing auth token omits the Authorization header, and retries are limited to safe requests (never /mark, /log, or /reward).
• Verify removed metric request methods are absent while response types still used by /log remain.
• Run mvn test at the PR head; confirm request serialization coverage for the client endpoints.

JavaScript client

• Verify the default HTTP client retries only network failures and 5xx for requests marked retryable, fails fast on 4xx, and applies the documented default timeout.
• Verify /mark, /log, and /reward are never retried; custom/lite HTTP clients can honor the optional retryable flag.
• Verify reward responses include mode/success and sendReward forwards awaitResult.
• Verify markDecisionPoint handles a missing assignment cache and sends the expected request context.
• Verify changing/clearing feature-flag session groups invalidates the cache and an old in-flight response is not cached.
• Verify successful init, group-membership, and working-group changes invalidate assignment and flag caches, with a generation guard against stale assignment responses; alias changes do not invalidate them.
• Verify the removed logCaliper method is absent.
• Run Jest, ESLint, and Prettier; confirm the documented test suite passes.

Python client

• Verify sync methods use the real synchronous API implementations and work when called from a running event loop.
• Verify sync pooling and async context-managed pooling close their clients; requests outside async with do not reuse a pool across event loops.
• Verify session feature-flag groups accept the model/dict forms, require both fields, invalidate the cache, and use generation/locking so stale in-flight responses are not cached.
• Verify successful init, group-membership, and working-group changes invalidate assignment and flag caches; alias changes do not. Confirm stale /assign responses cannot refill the cache.
• Verify decision-point matching is exact and None target is treated as an empty string.
• Run pytest, Ruff, and mypy; confirm the documented test suite passes.

Cross-client / scope checks

• Verify the retry allowlist is consistent with the audit: /init, /groupmembership, /workinggroup, /useraliases, /assign, and /featureflag may retry; /mark, /log, and /reward may not.
• Confirm the timeout/retry alignment decision and the no-experiment-at-decision-point behavior remain explicitly deferred as described in the planning docs.
• Confirm Phase D guardrails (shared endpoint contract checklist/tests and Java/Python CI coverage) are identified as follow-up work, not represented as completed in this PR.
• Confirm the PR remains limited to client libraries and existing /v6 APIs; no backend behavior or endpoint contract changed.

[bcb37]

@copilot can you edit the checklist you created (or append an updated one) based on the changes from the most recent commit?

[Copilot]

@copilot can you edit the checklist you created (or append an updated one) based on the changes from the most recent commit?

I can’t directly edit the PR description with the available GitHub operations, but here is a corrected, append-ready checklist. Use this version in place of the earlier checklist; it reflects the latest commit and the decisions in the phased audit and timeout/retry policy.

---

Code-review / QA checklist

Java client

• Verify markDecisionPoint overloads, including Assignment.markDecisionPoint(), send the cached experiment and condition IDs; within-subjects marks rotate the assigned condition and factor.
• Verify assignment and feature-flag caches are populated before success callbacks, and null callbacks, properties, and target lookups are safe.
• Verify reward methods support experiment-ID and decision-point forms, require the relevant identifiers/context, default awaitResult to false, and report awaited 409 responses through onError.
• Verify session feature-flag groups can be set/cleared, invalidate the flag cache, and prevent an in-flight response for old groups from repopulating it.
• Verify decision-point matching is exact. Successful init, group-membership, working-group, and alt-user-ID calls clear assignment and flag caches; failed calls leave caches intact.
• Verify no-experiment results use an empty Condition (including the site/target lookup), getCondition()/getPayload() are safe, and the experiment type remains null as documented.
• Verify a missing auth token omits the Authorization header; safe requests alone are retried, never /mark, /log, or /reward.
• Verify removed metric request methods are absent while response types used by /log remain.
• Run mvn test at the PR head and verify request serialization coverage for the client endpoints.

JavaScript client

• Verify the default HTTP client retries only network failures and 5xx for requests marked retryable, fails fast on 4xx, and uses the documented default timeout; /mark, /log, and /reward are never retried.
• Verify reward responses include mode/success and sendReward forwards awaitResult; markDecisionPoint safely handles a missing assignment cache.
• Verify changing/clearing feature-flag session groups invalidates the cache and an old in-flight response is not cached.
• Verify successful init, group-membership, working-group, and alt-user-ID calls clear assignment and flag caches; failures do not. Stale in-flight assignment responses must not refill a cleared cache.
• Verify the removed logCaliper method is absent.
• Verify the README documents the chosen timeout/retry policy: keep the current per-library differences rather than standardize them.
• Run Jest, ESLint, and Prettier.

Python client

• Verify sync methods use synchronous API implementations and work when called from a running event loop; sync pooling and async context-managed pooling close clients correctly without reusing pools across event loops.
• Verify session feature-flag groups accept model/dict forms, require both fields, invalidate the cache, and prevent stale in-flight responses from being cached.
• Verify successful init, group-membership, working-group, and alt-user-ID calls clear assignment and flag caches; failures do not, and stale assignment responses cannot refill a cleared cache.
• Verify decision-point matching is exact, None target is treated as an empty string, and Assignment.mark_decision_point() rotates within-subjects assignments in both async and sync paths.
• Verify the README documents the chosen timeout/retry policy and run pytest, Ruff, and mypy.

Cross-client / follow-up scope

• Verify the retry allowlist is consistent: /init, /groupmembership, /workinggroup, /useraliases, /assign, and /featureflag may retry; /mark, /log, and /reward may not.
• Verify no-experiment behavior remains intentionally library-specific: JS placeholder, Java empty-condition placeholder with null type, Python None.
• Verify Phase D guardrails (shared endpoint contract checklist/tests and Java/Python CI coverage) remain identified as follow-up work, not completed functionality.
• Confirm the PR remains limited to client libraries and existing /v6 APIs; no backend behavior or endpoint contract changed.

The latest commit adds the Java placeholder behavior, Python assignment-level within-subjects rotation, and cache invalidation after setAltUserIds in all three libraries; those items are included above. The timeout/retry decision is now documented as keeping the current differences. The latest workflow is still in progress, so I can’t confirm a full test run at this head yet; the previously listed PR checks passed before this commit.

[danoswaltCL]

I wouldn't make this update, we have always recommended using payload by default, as using payloads let's the implementing code be more experiment agnostic... i.e. a decision-point set up to only look for the condition itself will need updated later the moment someone wants to use payloads in an experiment, whereas if they just used the payload in the first place, they always get the correct thing, regardless of whether they use custom payloads or not.

[bcb37]

I'd forgotten that. But there's still an issue with the README because it then includes client.markDecisionPoint(decisionPoint.site, decisionPoint.target, condition, MARKED_DECISION_POINT_STATUS.CONDITION_APPLIED);

Sending the payload value as condition to the mark method. In a TS experiment, if there are custom payloads, this will not enroll the student, even if they got an assignment. I can change it so that the mark method is called on the assignment, rather than the client.

[danoswaltCL]

can this change be removed too? i don't see a purpose, seems related ultimately to the same unlikely cache-clearing scenarios we backed out other changes for.

[danoswaltCL]

same as comment for the getDecisionPointAssignment, what does have to do with making the libraries consistent? is there a bug this fixes if worrying about cache-clearing isn't worth it?

[bcb37]

nope, it's just more uncleared cruft. It can go.

[bcb37]

(removing it from python too)

[danoswaltCL]

if we're touching the retry stuff anyway (i see this fixes a real bug or two), we may as well set the retries and backoff as constants also, and... I think i've changed my mind on the 3 retries question, I think 1 retry is sufficient as a default, because if we're not allowing the user to configure this (they probably should be able to, but we kicked the can on that), it may be very annoying that it retries a bunch of times with no way to turn it off...

[bcb37]

I thought we determined that changes to the retry stuff was out of scope for this. Maybe another ticket?

[danoswaltCL]

i only mentioned it because there are still a number of retry changes in here, so i didn't know if you were planning on backing those out as well or what. i bring it up because this appears to fix an issue that prevented any retries from happening due to the error throwing before the retry logic, so if that is now fixed, unexpected multiple retries could start firing, which according to the docs it generated, could take up to 22 seconds to complete the way it is configured by default for 5xx type errors.

could be better handled all together as an immediate 'http-client-config-parity' follow-up branch?

[bcb37]

I'll change it to one retry here, rather than revert to a misleading state where it wasn't actually using the configured settings. We can then revisit later, along with the other timeout/retry issues.

[danoswaltCL]

ok, yeah, a bit of history is coming back to me now; one of the original maddening things Jenn and I had to figure out the hard way when investigating another ol' assign-prog mystery. it turned out the java client was set to retry 3 times in this manual way, and it was retrying stuff like 404s that were considered non-errors in the elastic-beanstalk monitors, so it was not obvious why we had some nonsensical things going on with wild traffic and timeout issues..... anyway, what was fixed in the java client only partially made its way to the javascript one i guess, setting it to retry once on 5xx errors seems like a sane default.

[danoswaltCL]

tests for the js client and java seem like they are good, mainly i tested rewards and feature-flags and some basic cache-clearing. the python changes looks reasonable to me from what i can tell, python is messed up on my machine right now so i didn't run that but i am less concerned with that one anyway as it's so new i trust you've got that handled.

so just a few more suggestions in there

[Copilot]

🟡 Changes recommended

Critical issues remain: cache invalidation can misattribute JavaScript assignment marks to the wrong experiment, and unsynchronized Java assignment mutations can pair a condition with another treatment’s factors.

2 open issues

• Lock paired condition and factor cache updates · New
• Preserve assignment IDs when marking after cache invalidation · New

_{Review effort: Balanced}