[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 with backoff for idempotent requests only (see Retry policy), fails fast on 4xx, and sets a 5s default timeout.
• 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 groups clears the cached flags. A flag response that was requested under the previous groups is never cached over that change; the caller still receives it. JS had this race already.

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. The assignment cache gets the same generation guard as flags, so an in-flight /assign can't refill a cleared cache. 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 now has an empty Condition instead of null, so getAssignedCondition().getConditionCode() no longer throws an NPE. Assignment gains getCondition()/getPayload(). Its experiment type stays null, as Java's explicit "no experiment" signal (JS reports SIMPLE; Python returns None and is unchanged).
• 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.
• 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 fixes the JS README quick start, which marked with the payload value instead of getCondition().

Testing

• JS: jest: 111/111 pass, ESLint/Prettier clean.
• Python: pytest: 242/242 pass; ruff check and mypy clean.
• Java: mvn test passes at the branch head (ExperimentClientTest, 26 tests). Also 26/26 from a mvn clean build run directly via TestNG, 5–10 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 (see audit §5)

• Phase D: guardrails / contract tests (and CI builds/tests for the Java/Python libs, which would have caught the compile error above).
• clientlibs/js/docs (TypeDoc output) is stale and needs regenerating, and the Java README still documents several methods that no longer exist.

🤖 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]

can we leave this new 'generation' stuff out of these updates, for this and the other libs? it's really a new feature, the scope of this update should be making the libs reasonably consistent.

IMO it's not worth it anyway, it's for an edge case that shouldn't come up unless the consumer app is shooting themselves in the foot by not waiting; it may hide this more than help, so i don't think it's worth the effort.

[bcb37]

Right, I wasn't sure where we landed on this. It doesn't change the method contract, and it was done to address a (theoretical) vulnerability introduced by the new behavior of emptying the cache when user/group information is changed. But, yeah, nobody should be changing group information after getting assignments without waiting for the assignments data to return.
The "new feature" is arguably really clearing the assignment cache on successful calls to any of the four endpoints that change user data (init, workingGroup, groupMembership, alias). It's perhaps a stretch to claim that those changes were made for consistency, since only one of them previously did that, and only for init.
@danoswaltCL should we hold off on the cache-clearing change for this version?

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