Client lib parity audit + Phase A correctness fixes by bcb37 · Pull Request #3407 · CarnegieLearningWeb/UpGrade

bcb37 · 2026-10-28T18:52:22Z

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. It also implements Phase A: the correctness fixes that need no API or product decisions. No backend changes.

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, 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 clients are pooled per UpgradeClient. Adds close() / aclose() and with / async with support.

Docs

Root CLAUDE.md now lists the Python lib in place of a non-existent C# lib.
java/CLAUDE.md now documents the tests and the timeouts, which are configurable via properties.

Behavior change to review

The JS default client effectively made zero retries before. It now makes up to 3, including for non-idempotent POST /mark and /log. If a request reached the server but the response was lost, that can write duplicates. The audit flags the same issue for Java. We could restrict JS retries to requests that are safe to repeat if preferred.

Testing

JS: jest: 95/95 pass, ESLint/Prettier clean. The new DefaultHttpClient.spec.ts fails against the old code.
Python: pytest: 223/223 pass; ruff check and mypy clean.
Java: ExperimentClientTest: 6/6 pass, 15 consecutive runs. These were run via TestNG directly because the local Maven mirror couldn't resolve the surefire TestNG provider; mvn test hasn't been run yet.

Not in this PR (needs decisions; see audit §5)

Phase B (feature parity): Java sendReward, JS awaitResult on sendReward, feature-flag session groups for Java/Python, removing dead endpoints (JS logCaliper, Java addGroupMetrics/addSingleMetrics, which hit routes the backend no longer serves).
Phase C (aligning behavior): site/target case sensitivity, null vs. empty assignment, init() cache clearing, retry/timeout policy, argument order.
Phase D: guardrails / contract tests.

🤖 Generated with Claude Code

Adds clientlibs/CLIENT_LIB_PARITY_AUDIT.md, an audit of the JS, Java and Python client libs against each other and the backend /v6 API, with a phased reconciliation plan. Implements Phase A: fixes that need no API or product decisions. Java: - markDecisionPoint sends the cached assignment's experimentId and condition id (all overloads, plus Assignment.markDecisionPoint), so the server marks the experiment the user was actually assigned instead of re-selecting one - rotate assignedFactor alongside assignedCondition on within-subjects marks - populate assignment/feature-flag caches before invoking onSuccess, so calls chained from inside the callback see a warm cache - null-safe callbacks and properties map; null-safe target lookup - first TestNG suite (src/test/java) against an in-process HttpServer JS: - DefaultHttpClient: retries never ran (axios rejects non-2xx before the retry branch); now retries network errors and 5xx with backoff, fails fast on 4xx, and sets a 5s default timeout - ISendRewardResponse gains mode/success - markDecisionPoint no longer throws when the assignment cache is null - fix 3 pre-existing ApiService mark tests missing the context field Python: - _sync methods call ApiService's sync implementations instead of asyncio.run(), so they work inside a running event loop - reuse pooled httpx clients per UpgradeClient; add close()/aclose() and sync/async context manager support Docs: root CLAUDE.md lists the python lib (not a nonexistent csharp lib); java/CLAUDE.md documents tests and configurable timeouts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Automatic retries can duplicate non-idempotent reward submissions and distort assignment counts, while Python cross-loop reuse abandons unclosed connection pools.

2 open issues

What changed in this PR

This PR documents differences among UpGrade’s JavaScript, Java, and Python clients and implements Phase A correctness fixes against the existing /v6 API, without backend changes.

Changes:

Java adds cached experiment metadata to marks, aligns factor rotation, and improves callback and null handling.
JavaScript updates retry handling, adds a default timeout, and handles missing assignment caches.
Python uses native synchronous requests and pooled HTTP clients with lifecycle methods; tests and documentation support the changes.

File	Description
`clientlibs/python/tests/test_client.py`	Tests synchronous calls, connection reuse, and cleanup.
`clientlibs/python/src/upgrade_client_lib/client.py`	Uses sync API methods and exposes lifecycle management.
`clientlibs/python/src/upgrade_client_lib/api_service.py`	Pools HTTP clients and adds cleanup methods.
`clientlibs/js/src/types/Interfaces.ts`	Adds reward response mode and success fields.
`clientlibs/js/src/DefaultHttpClient/DefaultHttpClient.ts`	Revises retries and adds a five-second timeout.
`clientlibs/js/src/DefaultHttpClient/DefaultHttpClient.spec.ts`	Tests timeout configuration and retry behavior.
`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 payloads 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 experiment and condition IDs in marks.
`clientlibs/java/src/main/java/org/upgradeplatform/client/ExperimentClient.java`	Updates mark metadata, rotation, caching, 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 phased reconciliation work.
`CLAUDE.md`	Corrects the client-library directory listing.

_{Review effort: Balanced}

+      const isRetryable =
+        (statusCode === undefined || statusCode >= 500) && !this.skipRetryOnStatusCodes.includes(statusCode);


+        if self._async_client is None or self._async_client_loop is not loop:
+            self._async_client = httpx.AsyncClient(timeout=self._timeout)
+            self._async_client_loop = loop


bcb37 · 2026-10-28T19:20:21Z

Superseded by #3408. The head branch was renamed from wip/clientlib-parity-phase-a to wip/clientlib-parity so it can carry later phases too, and GitHub closes a PR when its head branch is renamed.

Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:52

Copilot started reviewing on behalf of bcb37 October 28, 2026 18:52

View session

bcb37 requested review from danoswaltCL and zackcl October 28, 2026 18:53

Copilot AI reviewed Oct 28, 2026

View reviewed changes

bcb37 closed this Oct 28, 2026

bcb37 deleted the wip/clientlib-parity-phase-a branch October 28, 2026 19:18

Navigation Menu

Repository navigation

Client lib parity audit + Phase A correctness fixes - #3407

Client lib parity audit + Phase A correctness fixes#3407
bcb37 wants to merge 1 commit into
devfrom
wip/clientlib-parity-phase-a

bcb37 commented Oct 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bcb37 commented Oct 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const isRetryable =
		(statusCode === undefined \|\| statusCode >= 500) && !this.skipRetryOnStatusCodes.includes(statusCode);

Conversation

bcb37 commented Oct 28, 2026

Summary

Java

JS

Python

Docs

Behavior change to review

Testing

Not in this PR (needs decisions; see audit §5)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

bcb37 commented Oct 28, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants