[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; within-subjects rotation and the lookups that copy an assignment out of the cache hold the same lock, so a lookup never sees conditions rotated without their factors. 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.
• Marking an Assignment uses the ids it was created with, in all three libs. JS's Assignment used to look up the experimentId and condition id in the cache at mark time. With this PR's cache clearing, marking an Assignment held from before a group or alias change could send no experimentId, or another experiment's ids if the cache had been refilled with a different experiment at the same decision point. It now sends its own, as Java's (since Phase A) and Python's already do. Client-level marks, which have no Assignment, still fill in ids from the cache in all three.
• 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: 113/113 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.
• Merging with dev: a trial merge of current dev into this branch is clean (no conflicts), and all three libraries build and pass their tests on the merged code, including dev's new Java QuickTestBurst* files.
• ⚠️ 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.

[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}

[danoswaltCL]

@copilot explain this potential issue via a sequence of events that would make this come up? we should not bending over backwards to guard against the entire universe of pointless/thoughtless usage patterns from the library user, that is a recipe for bloating our code and causing bugs due to undue complexity.

[Copilot]

One concrete sequence is: the client fetches assignment A for a decision point and returns an Assignment; the app applies A; a successful init() or group update then clears the assignment cache; the app later calls markDecisionPoint() on that already-held Assignment. Assignment retains site/target/condition code but not the experiment/condition IDs, and the mark path looks those IDs up in the now-empty cache, so the server can re-select instead of attributing the mark to A. If another assignment has refilled the cache by then, it could attach that experiment’s IDs instead.

This doesn’t require an in-flight response race or failing to await the update—it’s an old Assignment object used after user state changed. I agree a generation guard would be disproportionate; the narrower question is whether we want to support marking an assignment object after such a change. If not, callers can fetch and mark before changing user/group state, then fetch a fresh assignment afterward. I haven’t changed code based on this question.

[bcb37]

Fixed in 5c586c1. A JS Assignment now keeps the experimentId and condition id it was created with, and passes them to ApiService.markDecisionPoint(), which uses them instead of looking them up in the cache. So marking an Assignment held from before init() or a group/alias change records the experiment the user was actually shown. This brings JS in line with Java and Python, whose Assignment marks already send their own snapshot's ids. On dev the JS cache was never cleared, so the lookup rarely mattered; this PR's cache clearing is what made it matter. Client-level marks (site, target and condition code, no Assignment) still fill in the ids from the cache, as in the other two libraries. The new test in UpgradeClient.cache.spec.ts covers your sequence (A shown, groups change, B refetched at the same decision point, A marked); it fails against the previous code.

[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 edit the PR description directly with the available GitHub operations. Here’s an updated append-ready checklist for the description; the Java smoke-test item reflects the latest fd234c6 commit. It follows the phased parity audit (§5) and the timeout/retry policy.

Code-review / QA checklist

Java client

• Verify markDecisionPoint overloads and Assignment.markDecisionPoint() send the cached experiment ID and condition ID; verify within-subjects marks rotate both condition and factor.
• Verify assignment and feature-flag caches are populated before success callbacks; callback/property/target null cases are safe.
• Verify successful init, group-membership, working-group, and alias updates invalidate assignment and flag caches, and failed updates do not.
• Verify exact site/target matching; no-experiment results retain the documented null condition/type behavior, while getCondition()/getPayload() are safe.
• Verify reward methods support experiment-ID and decision-point lookup, require the relevant identifiers/context, default awaitResult to false, and route awaited 409s to onError.
• Verify feature-flag session groups can be set/cleared, are sent in the request, and invalidate cached flags.
• Verify auth is omitted without a token and retries are limited to safe requests—not /mark, /log, or /reward.
• Verify removed metric request methods are absent while /log response beans remain.
• Run mvn test and review request-serialization coverage. For a live-server smoke test, verify the updated QuickTest exercises both feature-flag session-group modes and both reward lookup forms, queued and awaited.

JavaScript client

• Verify default HTTP retries handle network failures/5xx once after the configured default backoff, fail fast on 4xx, and use the default timeout; confirm only retry-safe requests are marked retryable.
• Verify sendReward supports awaitResult and its response type includes mode and success.
• Verify markDecisionPoint is safe after a failed /assign, sends context and the condition code, and uses cached assignment metadata when available.
• Verify successful user/group updates and changes to session feature-flag groups clear the relevant caches; failed updates leave caches intact.
• Verify feature-flag session-group options/setter require both fields together and session-group changes invalidate cached flags.
• Verify exact decision-point matching and removal of the dead logCaliper endpoint method.
• Run the JS Jest suite and the existing ESLint/Prettier checks.

Python client

• Verify sync methods use the sync ApiService implementations and work when called from an already-running event loop.
• Verify sync connection pooling and close()/context-manager support; async pooling is scoped to async with, with aclose() support.
• Verify reward await_result behavior and feature-flag session groups via constructor and setter (model and dict forms), including cache invalidation.
• Verify successful user/group updates clear relevant caches, exact site/target matching treats None target as "", and Assignment.mark_decision_point() rotates within-subjects assignments.
• Verify Python does not retry requests, as documented.
• Run pytest, Ruff, and mypy.

Cross-library / release checks

• Review CLIENT_API_COVERAGE.md against the backend v6 controller/validators; confirm each endpoint is ✅ or intentionally ➖.
• Review each README’s caching and mark/payload guidance, plus the timeout/retry policy; confirm docs match implemented behavior.
• Before release, resolve or explicitly defer every ❌ in the coverage checklist.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because the Python, JavaScript, and Java examples mishandle valid assignments without payloads while still marking conditions as applied.

1 open issue

• Preserve assignment IDs when marking after cache invalidation

1 resolved since last review

• Lock paired condition and factor cache updates

_{Review effort: Balanced}

[bcb37]

Re: the latest Copilot review ("the Python, JavaScript, and Java examples mishandle valid assignments without payloads while still marking conditions as applied"): fixed in 4a58f1d.

• Valid assignments without a payload. The default payload (the condition code) is only created when an experiment is updated, not when it's created, so an experiment created through the API without payloads and never edited has none, and /assign returns payload: null. All three READMEs still apply the payload first, as we recommend, but now fall back to the condition code when there isn't one (getPayload()?.value ?? getCondition() in JS, and the equivalent in Java and Python).
• Marking. The JS and Java examples now mark NO_CONDITION_ASSIGNED when there's no condition at all (the "no experiment here" placeholder), instead of CONDITION_APPLIED. Python returns None rather than a placeholder and only marks inside if assignment:, so it only needed the fallback.

Whether the backend should always create default payloads (at experiment creation, or as a fallback in the /assign response) is a separate question for a backend PR; the README fallback is still worth having either way, since the client libraries can talk to servers without such a change.

[Copilot]

🟢 Approval recommended

No active review issues are reported for the client-library API parity changes.

1 open issue

• Preserve assignment IDs when marking after cache invalidation

_{Review effort: Balanced}