Repository navigation
[FSSDK-13266] Update dependency constraints for json-schema and murmurhash - #298
Conversation
…rhash Allow justinrainbow/json-schema v6.x and lastguest/murmurhash v2.x to support newer versions of these dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
murmurhash v2 removed the global murmurhash3_int() function in favor of a static class method. Add a function_exists() check to support both APIs. json-schema v6 deprecated check() in favor of validate(). Switch to the new method for forward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The validator change breaks still-supported JSON Schema versions, and CI does not exercise the newly allowed dependency majors.
2 open findings
What changed in this PR
Updates dependency compatibility for newer JSON Schema and MurmurHash releases.
Changes:
- Widens dependency constraints to permit v6 and v2.
- Adds compatibility calls for updated APIs.
- Documents release 4.0.4.
| File | Description |
|---|---|
composer.json |
Widens dependency constraints. |
src/Optimizely/Utils/Validator.php |
Uses the newer validation API. |
src/Optimizely/Bucketer.php |
Supports MurmurHash v1 and v2 APIs. |
CHANGELOG.md |
Documents the release changes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
validate() does not exist in json-schema v1.6 and v2.x. Use method_exists() to call validate() when available and fall back to check() for older versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Tamara-Barum <88795935+Tamara-Barum@users.noreply.github.com>
Co-authored-by: Tamara-Barum <88795935+Tamara-Barum@users.noreply.github.com>
alexjoeyyong
left a comment
There was a problem hiding this comment.
Thanks for this. Widening both constraints is the right call, and the new "latest deps" job is worth having since composer.lock pins the old versions. I read the upstream sources for both packages: the murmur algorithm is identical between 1.3.0 and 2.1.1 (the only difference is abs() on the seed, and HASH_SEED is 1), so bucketing should be unchanged. Requesting changes for a few small things, mostly the stale lockfile and the changelog/version mismatch. Details inline. I did not run CI; the PR checks will.
Mat001
left a comment
There was a problem hiding this comment.
Reviewed against the actual library sources across every version these widened constraints allow (json-schema 1.6 → 6.4.1, murmurhash 1.3.0 → 2.1.1), not just the PR description. Both compatibility shims are correct, the arg types line up, and the risky behavior is guarded by exact-value tests — I found no merge-blocking code defect.
The one thing worth fixing before merge: the new latest_dependency_tests job is the sole safeguard proving v6/v2 don't break bucketing or schema validation, but nothing asserts those majors actually resolved — so it can go green while silently testing the old versions (false positive). Details inline.
Mat001
left a comment
There was a problem hiding this comment.
Overall clean implementation. I requested three fixes.
- Fix Validator.php comment to accurately describe json-schema version history (validate() added in v5, check() deprecated in v6) - De-duplicate bitmask operation in Bucketer.php - Change murmurhash constraint from ^2.0 to ^2.1 (2.0.0 requires PHP 7) - Regenerate composer.lock to match updated constraints - Remove premature CHANGELOG 4.0.4 entry (belongs in release-prep PR) - Add CI job to test with latest dependency versions (PHP 8.1 + 8.2) with version assertion to verify v6/v2 actually resolved - Add edge-case bucketing test keys (empty string, UTF-8) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
composer show does not accept multiple package names as positional arguments — the second name was parsed as a version constraint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Second pass, this time including a ponytail (over-engineering) review. Overall the code is correct and I'd approve once the items below are handled. Thanks for the work here, Tamara.
What I verified
- murmur 1.3.0 and 2.1.1 are the same algorithm (only differences:
abs()on the seed, which is 1 here, and astringtype hint;Bucketeralways passes a string). - I checked all 7 golden bucket values in
testGenerateBucketValue, including the two new ones (''-> 3175,'日本語テスト1886780721'-> 8308), against an independent MurmurHash3 reference implementation. All match. - The Latest Deps job resolved json-schema 6.13.1 and murmurhash 2.1.1 on 8.1 and 8.2, the version gate passed, and the suite passed with the same hard-coded values. So v1 and v2 produce identical buckets on those keys.
- I recomputed the
composer.lockcontent-hash fromcomposer.jsonand it matches. No package versions changed in the lock. - In json-schema 5.2.12 and 6.x,
check()isreturn $this->validate(...); the v6 deprecation is docblock-only.
Ponytail findings (what to cut)
Validator.php:L40-46- delete themethod_existsif/else, keep the singlecheck()call (inline).php.yml:L83- delete the redundantcomposer install(inline).php.yml:L88-89- delete the twocomposer showlines (inline).
Net about -10 lines. The bucketer change and the new test asserts are lean and stay.
Requested changes
- Ponytail 1 (the Validator revert). This is the one I'd hold the approval on. I know Mat001's earlier comment asked for the comment to be tightened while I asked for the branch to be dropped, so the direction was mixed. With the logic checked, dropping it is right, and it also removes the "Fix: use validate()" claim from the description.
- Update the PR description. It says
^2.0(code is^2.1), claims a 4.0.4 CHANGELOG entry that was intentionally removed inf24ac21(Copilot's open comment is about this), and lists two "Fix:" items when nothing was broken. Suggest: "Allow json-schema 6.x and murmurhash 2.x", and note the changelog and version bump go in the release-prep PR. Also mention the lock diff includes unrelated Composer 2.9 metadata and repo URL rewrites.
Not blocking, but needs a maintainer
Latest Depsis not in the required status checks (the list is the compat suite, Integration Tests, Linting, Unit Tests 8.1/8.2 and the Jira check), yet it is the only job that exercises v6/v2. Please add it in branch protection once it has run on master.optimizely/fullstack-sdk-compatibility-suite (php)is red and required: 8 of 975 scenarios fail, all indecide_holdouts_exclude_td.feature(excluding targeted delivery from holdouts), which this PR doesn't touch. I haven't confirmed it also fails on master, but it looks unrelated. Someone will need to decide how to handle it before merge.- Optional: add
'8.4'to the Latest Deps matrix. #296 was an 8.4 deprecation fix and v6/v2 are the kind of upgrade that can reintroduce one.
I did not run CI; the PR checks will.
- Revert Validator.php to use check() directly — it works in all allowed versions (1.6 through 6.x) and the v6 deprecation is docblock-only - Remove redundant composer install step from latest-deps CI job - Remove redundant composer show lines — the php -r gate already echoes and asserts the resolved versions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test results for this PR (run on 8.1 and 8.4)TLDR
What I tested, in plain words"Bucketing" means deciding which variation a user sees. If the new library ever hashed a user differently, existing users would silently switch variations. That is the main risk of this PR, so most of the tests aim at it.
How I did it
Setup commandsgit archive master | tar -x -C wt-base
git archive refs/pr/298 | tar -x -C wt-pr
composer() { docker run --rm -v "$PWD/$1":/app -w /app composer:2 "${@:2}" --no-interaction --no-progress --ignore-platform-req=php; }
composer wt-base install
composer wt-pr update justinrainbow/json-schema lastguest/murmurhash --with-dependencies
php81() { docker run --rm -v "$PWD":/w -w /w php:8.1-cli php "$@"; }
# 1. hash parity
php81 parity.php wt-base/vendor/lastguest/murmurhash wt-pr/vendor/lastguest/murmurhash 200000
# 2. same decisions (fixture = the DATAFILE constant from phpunit_bootstrap.php)
php81 decide_dump.php wt-base fixture.json 5000 > base.json
php81 decide_dump.php wt-pr fixture.json 5000 > pr.json
diff -q base.json pr.json && echo IDENTICAL
# 4. validator
php81 validator_matrix.php wt-base fixture.json > v_base.txt
php81 validator_matrix.php wt-pr fixture.json > v_pr.txt
diff v_base.txt v_pr.txt && echo SAME
# 6. unit tests
docker run --rm -v "$PWD/wt-pr":/app -w /app php:8.1-cli ./vendor/bin/phpunit --no-coverageparity.php<?php
// Hash parity: murmurhash v1 (global fn) vs v2 (static method), same process.
// usage: php parity.php <dir-with-murmurhash3.php> <dir-with-src/lastguest/Murmur.php> [iterations=200000]
[$_, $v1, $v2, $n] = $argv + [null, null, null, 200000];
require "$v1/murmurhash3.php";
require "$v2/src/lastguest/Murmur.php";
mt_srand(298); // reproducible
$fixed = ['', 'a', 'ab', 'abc', 'abcd', 'abcde', '日本語テスト1886780721', "\0", "\xff\xfe", str_repeat('x', 4096),
'ppid11886780721', 'a very very very very very very very very very very very very very very very long ppd string1886780721'];
$bad = 0;
$check = function (string $k) use (&$bad) {
$a = murmurhash3_int($k, 1) & 0xFFFFFFFF;
$b = \lastguest\Murmur::hash3_int($k, 1) & 0xFFFFFFFF;
if ($a !== $b) { $bad++; fprintf(STDERR, "MISMATCH %s v1=%d v2=%d\n", bin2hex($k), $a, $b); }
};
foreach ($fixed as $k) $check($k);
for ($i = 0; $i < $n; $i++) {
$len = mt_rand(0, 40); $k = '';
for ($j = 0; $j < $len; $j++) $k .= chr(mt_rand(0, 255)); // arbitrary bytes covers every tail length + high bytes
$check($k);
$check("user_$i" . mt_rand(1, 9999999999)); // realistic bucketingId.experimentId shape
}
printf("checked %d keys, %d mismatches\n", count($fixed) + 2 * $n, $bad);
exit($bad ? 1 : 0);decide_dump.php<?php
// Dump decisions for N synthetic users across every flag. Never dispatches events.
// usage: php decide_dump.php <sdk-checkout-dir> <datafile.json> [users=2000] [attributes-json='{}'] > out.json
[$_, $repo, $file, $n, $attrs] = $argv + [null, null, null, 2000, '{}'];
require "$repo/vendor/autoload.php";
class NullDispatcher implements \Optimizely\Event\Dispatcher\EventDispatcherInterface {
public function dispatchEvent(\Optimizely\Event\LogEvent $event) { return true; }
}
$client = new \Optimizely\Optimizely(file_get_contents($file), new NullDispatcher());
if (!$client->isValid()) { fwrite(STDERR, "datafile rejected by SDK\n"); exit(2); }
$opts = [\Optimizely\Decide\OptimizelyDecideOption::DISABLE_DECISION_EVENT];
$a = json_decode($attrs, true) ?: [];
$out = [];
for ($i = 0; $i < $n; $i++) {
foreach (["user_$i", "ユーザー_$i"] as $uid) { // ascii + multibyte ids
$ctx = $client->createUserContext($uid, $a);
foreach ($ctx->decideAll($opts) as $flag => $d) {
$out[$uid][$flag] = [$d->getVariationKey(), $d->getEnabled(), $d->getRuleKey()];
}
}
}
ksort($out);
if (!$out || !count(reset($out))) { fwrite(STDERR, "no decisions produced; refusing to emit an empty result\n"); exit(3); }
echo json_encode($out, JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT), "\n";
fprintf(STDERR, "%d users x %d flags\n", count($out), count(reset($out) ?: []));validator_matrix.php<?php
// validateJsonSchema over a datafile + mutations; prints one stable line per case so two runs can be diffed.
// usage: php validator_matrix.php <sdk-checkout-dir> <datafile.json>
[$_, $repo, $file] = $argv + [null, null, null];
require "$repo/vendor/autoload.php";
class Collect implements \Optimizely\Logger\LoggerInterface {
public $lines = [];
public function log($l, $m) { $this->lines[] = $m; }
}
$raw = file_get_contents($file);
// decode to objects (not arrays) so empty {} stays {} when re-encoded
$mut = function (callable $f) use ($raw) { $d = json_decode($raw); $f($d); return json_encode($d); };
$cases = [
'valid' => $raw,
'missing accountId' => $mut(function ($d) { unset($d->accountId); }),
'missing version' => $mut(function ($d) { unset($d->version); }),
'experiments=string' => $mut(function ($d) { $d->experiments = 'nope'; }),
'exp key=int' => $mut(function ($d) { $d->experiments[0]->key = 123; }),
'empty object' => '{}',
'null' => 'null',
'not json' => '{oops',
'empty string' => '',
];
foreach ($cases as $name => $json) {
$log = new Collect();
$ok = \Optimizely\Utils\Validator::validateJsonSchema($json, $log);
// count of debug lines + whether the violation text actually made it into the log
printf("%-20s valid=%-5s logLines=%d violationTextLogged=%s\n", $name, var_export($ok, true), count($log->lines),
preg_match('/\[(?!%s)[^\]]+\] \S/', implode('', $log->lines)) ? 'yes' : 'no');
}What this does not prove
Side note, not caused by this PR
Run with Claude Code on Alex Yong's machine. |
Mat001
left a comment
There was a problem hiding this comment.
Requesting changes on two items before merge (dropped 4.0.4 CHANGELOG/version entry + a CI version-gate floor that understates the real constraint). Everything else from the prior round is correctly addressed, and the one red check (FSC) is an unrelated PHP feature gap — details below.
Verified correctly addressed (thanks for the quick turnaround)
- CI: added the
Verify latest majors resolvedstep and expanded to a PHP 8.1/8.2 matrix withfail-fast: false. BothLatest Depslegs pass — confirming v6 + murmur 2.1 actually resolve and the suite passes under the new majors. - Bucketing parity: added empty-string (
3175) and multibyte UTF-8 (8308) assertions; they pass under both v1.3.0 (Unit Tests) and v2.x (Latest Deps), so v1 ≡ v2 parity is now empirically pinned, not assumed. - murmur constraint
^2.1: correct —2.0.0requiresphp: ^7(no PHP 8) and this SDK needs>=8.1, so^2.1is the right floor. - Validator.php: reverting to plain
check()is safe — v6.4.1'scheck()is a functional alias (return $this->validate(...)) with no runtime deprecation notice.
Changes requested
1. Re-add the 4.0.4 CHANGELOG entry and align the PR description. The revert dropped the ## 4.0.4 CHANGELOG entry, but the description still lists "Add CHANGELOG entry for version 4.0.4" and "Use validate() instead of deprecated check()" — neither is in the code anymore. Failure scenario: 4.0.4 ships with no changelog line and reviewers/release trust a description that contradicts the diff. Please re-add the entry (dated 2026-10-13) and update the two stale bullets. Suggested:
## 4.0.4
October 13, 2026
* Update: Allow `justinrainbow/json-schema` v6.x
* Update: Allow `lastguest/murmurhash` v2.x
* Fix: Add compatibility for murmurhash v2 API (global function replaced with static class method)2. CI version-gate floor doesn't match the constraint. See the inline comment on the Verify latest majors resolved assertion.
On the failing fullstack-sdk-compatibility-suite (php) check — safe to bypass at merge
This failure is not caused by this PR. All 8 failures are in features/decide/decide_holdouts_exclude_td.feature (holdouts + excludeTargetedDeliveries) — a feature the PHP SDK has not implemented (zero holdout references anywhere in src/). The shared compatibility suite ran against SDK_BRANCH: master and is simply ahead of the PHP SDK's feature set, so it fails on master regardless of this PR. This PR touches only dependency constraints + the murmurhash hashing method, not decision/holdout logic. OK to merge with a bypass on this check; the holdout gap should be tracked as its own ticket.
Re-add the user-facing CHANGELOG entry and align the CI version check with the ^2.1 murmurhash constraint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The release entry belongs in the separate release-prep PR alongside the SDK_VERSION bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alexjoeyyong
left a comment
There was a problem hiding this comment.
Re-reviewed at edda32b. Everything I asked for is done:
- Validator.php is back to the original single
check()call (zero diff vs master). - Redundant
composer installandcomposer showlines removed from the Latest Deps job; it passes on 8.1 and 8.2 without them. - PR description corrected (
^2.1, no stray "Fix" claims, changelog/version bump deferred to the release-prep PR). CHANGELOG has no diff vs master. - CI gate tightened to murmurhash >= 2.1.0 per Mat001.
- composer.lock content-hash matches composer.json.
The red fullstack-sdk-compatibility-suite (php) check is the same 8 holdout (exclude-TD) scenarios as before. Master's src has no holdout code at all, so this is a pre-existing PHP SDK feature gap, not this PR. A maintainer will need to override it.
For a maintainer, before or after merge: add Latest Deps to required status checks, since it is the only job exercising json-schema 6 / murmurhash 2. Optional: add PHP 8.4 to its matrix.
Not blocking: the description says bucket values match the Python SDK across 10k keys; I couldn't verify that part myself. My own checks (v1 vs v2 on 400k keys, 100k-decision master-vs-PR diff) are in the earlier comment.
Re-ran the tests on the final commit (
|
| Test | What it checks | Result |
|---|---|---|
| Setup | The PR really got the new libraries | master: json-schema 5.2.12, murmurhash 1.3.0. PR: 6.13.1 and 2.1.1. |
| 1. Same numbers? | Old vs new hashing library, 400,012 inputs | 0 differences |
| 2. Same decisions? | master vs PR, 10,000 simulated users (half with Japanese-text ids), all 10 flags in the repo's test datafile | Identical (checksum 7bee33c3...). 50,454 of 100,000 decisions "on", 20 different outcomes. |
| 3. Can it fail? | Changed the hashing seed on purpose, then restored it | Caught by the comparison and by testGenerateBucketValue. File verified restored. |
| 4. Same validation? | Datafile validator, 1 good datafile + 8 broken ones, old vs new json-schema, PHP 8.1 and 8.4 | Same answers everywhere |
| 5. PHP 8.4 warnings | Every warning switched on | None from json-schema 6, murmurhash 2 or SDK code. (master's json-schema adds 32 warnings that go away with this PR.) |
| 6. Unit tests | Whole suite, master and PR, PHP 8.1 and 8.4 | 581 tests, 0 failures in all four runs. The 73 warnings are old PHPUnit notices that master has too. |
What changed since my last test comment
Only CI and cleanup: the Validator change was reverted to match master, the redundant CI steps were removed, and the CI version gate was tightened. Nothing that affects bucketing, which is why the numbers are unchanged.
How to repeat it
You need Docker running and nothing else (no PHP or Composer install). It takes about 4 minutes and doesn't touch your checkout.
./run_all.sh /path/to/php-sdk 298It prints PASS/FAIL per test and ends with ALL PASSED or SOMETHING FAILED. The guide explains how to read a failure.
GUIDE.md
How to re-check a dependency-bump PR (PHP SDK)
Use this when a PR changes which versions of libraries the SDK allows, and you want proof that
users won't be affected. It was written for PR 298 (json-schema 6.x, murmurhash 2.x) but works for
any PR number.
TLDR
cd /tmp/pr298-runbook
./run_all.sh /path/to/php-sdk <PR-number> # about 4 minutesIt ends with ALL PASSED or SOMETHING FAILED and a list of PASS/FAIL lines. A full log from the
last run is in run_edda32b.log.
What you need
- Docker Desktop, running. (
open -a Docker, then wait about 10 seconds.) gitandpython3. You do not need PHP or Composer installed; Docker provides them.- A local clone of
php-sdkwithoriginpointing at GitHub.
What the script does, in plain words
It never touches your working copy. It downloads two copies into a temp folder:
master (what ships today) and the PR. It installs libraries for each, the same way CI does,
and then checks that the PR didn't change how users are treated.
| Step | Question it answers | What "pass" means |
|---|---|---|
| Setup | Did the PR copy really get the new library versions? | master shows the old versions, the PR shows json-schema 6.x and murmurhash 2.x. If not, the script stops, because every later result would be meaningless. |
| T1 | Does the new hashing library give the same numbers as the old one? | 0 differences over 400,012 inputs. |
| T2 | Do users get the same variations on master and on the PR? | The two result files are identical (same checksum). 10,000 simulated users, 100,000 decisions. |
| T3 | Would these checks actually notice a problem? | The script breaks the hashing on purpose. Both the comparison and the PR's own unit test must complain. Then the file is put back and verified. |
| T4 | Does datafile validation behave the same on the old and new library? | Same answers for a good datafile and 8 broken ones, on PHP 8.1 and 8.4. |
| T5 | Does PHP 8.4 print new warnings? | None from json-schema, murmurhash or SDK code. |
| T6 | Does the whole unit test suite still pass? | No failures on master or the PR, on PHP 8.1 and 8.4. |
How to read the result
- ALL PASSED: nothing about user bucketing or validation changed. Safe from this angle.
- T1 fails: stop. The new hashing library disagrees with the old one, and users would silently
switch variations. The mismatching input is printed (as hex). - T2 fails but T1 passes: the problem is somewhere other than the hash. Run the two
decide_dump.phpcommands by hand anddiffthe files; the first different user/flag is your repro. - T3 fails: the checks are not trustworthy. Don't rely on T2's "identical" until this is fixed.
- T4 fails: validation behaves differently between library versions. Look at which case differs.
- T6 shows
Warnings: 73: that is normal. They are old PHPUnit notices that master has too.
OnlyFailuresorErrorsmatter.
Know the limits
- It uses the datafile that ships in the repo's tests (10 flags), not a real customer project.
To use a real one, copy a datafile towork-<PR>/fixture.jsonafter step Setup and rerun T2 by hand
(commands below). Datafile URL:https://cdn.optimizely.com/datafiles/<SDK_KEY>.json.
Never paste SDK keys into a PR. - 64-bit PHP only. PHP 8.1 and 8.4 only (CI covers 8.2).
- It does not run the
fullstack-sdk-compatibility-suitecheck; that one runs in GitHub. - The decision script sends no events, so it's safe to run against any datafile.
Running one step by hand
After the script has run once, everything is in work-<PR>/:
cd /tmp/pr298-runbook/work-298
php81() { docker run --rm -v "$PWD":/w -w /w php:8.1-cli php "$@"; }
php81 parity.php wt-base/vendor/lastguest/murmurhash wt-pr/vendor/lastguest/murmurhash 200000 # T1
php81 decide_dump.php wt-base fixture.json 5000 > base.json # T2
php81 decide_dump.php wt-pr fixture.json 5000 > pr.json
diff -q base.json pr.json && echo IDENTICAL
php81 validator_matrix.php wt-pr fixture.json # T4
docker run --rm -v "$PWD/wt-pr":/app -w /app php:8.1-cli ./vendor/bin/phpunit --no-coverage # T6Files
| File | Purpose |
|---|---|
run_all.sh |
Runs everything above and prints PASS/FAIL. |
parity.php |
T1: compares old and new hashing on 400k inputs. |
decide_dump.php |
T2/T3: records every decision for N simulated users, with no events sent. |
validator_matrix.php |
T4: runs the datafile validator on a good file and 8 broken ones. |
run_edda32b.log |
Output of the run on PR 298 at commit edda32b. |
Cleanup
rm -rf /tmp/pr298-runbook/work-* # temp copies
git -C /path/to/php-sdk update-ref -d refs/pr/298 # the PR ref the script fetchedIf something goes wrong
Docker is not running: open Docker Desktop and rerun.fetch failed: check thatgit -C <repo> remote get-url originis the GitHub repo and you're logged in.- Slow first run: Docker downloads the PHP and Composer images once (a few minutes). Later runs are faster.
- Composer reports security advisories for old Guzzle/Monolog: expected, they are existing lock-file
versions and not part of this PR.
Bugs found in these scripts while building them (so you don't repeat them)
- A "pass" on empty output. The first draft compared two empty result files and printed IDENTICAL.
decide_dump.phpnow refuses to emit an empty result. grep -qat the end of a pipe, withset -o pipefail, reports a failure even when the match
succeeded. The script captures output first instead.
run_all.sh
#!/usr/bin/env bash
# Re-verify a dependency-bump PR: same bucketing, same validation, still green.
# usage: ./run_all.sh <path-to-sdk-checkout> <pr-number> [users=5000]
# needs: docker running, git, python3. Touches nothing in the checkout except fetching refs/pr/<n>.
set -uo pipefail
SDK=${1:?path to php-sdk checkout}; PR=${2:?PR number}; USERS=${3:-5000}
HERE=$(cd "$(dirname "$0")" && pwd); W=$HERE/work-$PR
FAIL=0; say() { printf '\n== %s\n' "$*"; }; ok() { printf 'PASS %s\n' "$*"; }; bad() { printf 'FAIL %s\n' "$*"; FAIL=1; }
php() { docker run --rm -v "$W":/w -w /w php:"$1"-cli php "${@:2}"; }
php84() { docker run --rm -v "$W":/w -w /w php:8.4-cli php -d error_reporting=-1 -d display_errors=stderr "$@"; }
comp() { docker run --rm -v "$W/$1":/app -w /app composer:2 "${@:2}" --no-interaction --no-progress --ignore-platform-req=php >"$W/$1.composer.log" 2>&1; }
docker info >/dev/null 2>&1 || { echo "Docker is not running"; exit 2; }
rm -rf "$W"; mkdir -p "$W/wt-base" "$W/wt-pr"; cp "$HERE"/{parity,decide_dump,validator_matrix}.php "$W"/
git -C "$SDK" fetch -q origin "pull/$PR/head:refs/pr/$PR" || { echo "fetch failed"; exit 2; }
SHA=$(git -C "$SDK" rev-parse --short refs/pr/$PR)
git -C "$SDK" archive master | tar -x -C "$W/wt-base"
git -C "$SDK" archive "refs/pr/$PR" | tar -x -C "$W/wt-pr"
say "Testing PR $PR at commit $SHA (master = $(git -C "$SDK" rev-parse --short master))"
say "Setup: install libraries"
comp wt-base install
comp wt-pr update justinrainbow/json-schema lastguest/murmurhash --with-dependencies
ver() { grep -A1 "\"name\": \"$2\"" "$W/$1/composer.lock" | grep version | sed -E 's/.*"([0-9][^"]*)".*/\1/'; }
JB=$(ver wt-base justinrainbow/json-schema); MB=$(ver wt-base lastguest/murmurhash)
JP=$(ver wt-pr justinrainbow/json-schema); MP=$(ver wt-pr lastguest/murmurhash)
echo "master: json-schema $JB, murmurhash $MB"; echo "PR: json-schema $JP, murmurhash $MP"
[[ $JP == 6.* && $MP == 2.* ]] && ok "PR copy is really on the new versions" || { bad "PR copy did not get the new versions; every result below is meaningless"; exit 1; }
docker run --rm -v "$W":/w -w /w/wt-base php:8.1-cli php -r 'require "vendor/autoload.php"; require "phpunit_bootstrap.php"; file_put_contents("../fixture.json", DATAFILE);'
[[ -s $W/fixture.json ]] || { bad "could not build fixture datafile"; exit 1; }
say "T1 hash parity (old vs new murmurhash)"
out=$(php 8.1 parity.php wt-base/vendor/lastguest/murmurhash wt-pr/vendor/lastguest/murmurhash 200000 2>&1); echo "$out"
[[ $out == *" 0 mismatches"* ]] && ok "T1" || bad "T1"
say "T2 same decisions (master vs PR, $USERS users x 2 id styles)"
php 8.1 decide_dump.php wt-base fixture.json "$USERS" >"$W/base.json" 2>"$W/base.err"; php 8.1 decide_dump.php wt-pr fixture.json "$USERS" >"$W/pr.json" 2>"$W/pr.err"
cat "$W/pr.err"; shasum "$W/base.json" "$W/pr.json" | cut -c1-12,41-
cmp -s "$W/base.json" "$W/pr.json" && [[ -s $W/base.json ]] && ok "T2 identical" || bad "T2 differs or empty"
python3 - "$W/base.json" <<'EOF'
import json,sys,collections
d=json.load(open(sys.argv[1])); c=collections.Counter((f,v[0],v[1]) for u in d.values() for f,v in u.items())
print(f" {sum(len(u) for u in d.values())} decisions, {len(c)} distinct outcomes, {sum(1 for u in d.values() for v in u.values() if v[1])} enabled")
EOF
say "T3 negative control (break the seed, must be caught)"
B=$W/wt-pr/src/Optimizely/Bucketer.php; cp "$B" "$B.orig"; sed -i.bak 's/\$HASH_SEED = 1;/$HASH_SEED = 2;/' "$B"
php 8.1 decide_dump.php wt-pr fixture.json "$USERS" >"$W/broken.json" 2>/dev/null
cmp -s "$W/base.json" "$W/broken.json" && bad "T3a harness did NOT notice a broken seed" || ok "T3a comparison caught it"
t3b=$(docker run --rm -v "$W/wt-pr":/app -w /app php:8.1-cli ./vendor/bin/phpunit --no-coverage --filter testGenerateBucketValue 2>&1 || true)
[[ $t3b == *"FAILURES"* ]] && ok "T3b unit test caught it" || bad "T3b unit test did NOT catch it"
mv "$B.orig" "$B"; rm -f "$B.bak"
diff <(git -C "$SDK" show "refs/pr/$PR:src/Optimizely/Bucketer.php") "$B" >/dev/null && ok "Bucketer.php restored exactly" || bad "Bucketer.php NOT restored"
say "T4 validator, old vs new json-schema, PHP 8.1 and 8.4"
for v in 8.1 8.4; do
if [[ $v == 8.1 ]]; then run=(php 8.1); else run=(php84); fi
"${run[@]}" validator_matrix.php wt-base fixture.json >"$W/v$v-base.txt" 2>"$W/v$v-base.err"
"${run[@]}" validator_matrix.php wt-pr fixture.json >"$W/v$v-pr.txt" 2>"$W/v$v-pr.err"
diff -q "$W/v$v-base.txt" "$W/v$v-pr.txt" >/dev/null && ok "T4 same answers on $v" || bad "T4 differs on $v"
done
cat "$W/v8.1-pr.txt"
grep -c . "$W/v8.1-pr.txt" >/dev/null; [[ $(grep -c 'valid=true' "$W/v8.1-pr.txt") == 1 ]] && ok "exactly one valid case passes" || bad "unexpected valid/invalid mix"
say "T5 PHP 8.4 warnings"
php84 decide_dump.php wt-pr fixture.json 100 >/dev/null 2>"$W/t5.err"
n_ours=$(cat "$W/t5.err" "$W/v8.4-pr.err" | grep -c -E 'Deprecated|Warning' || true)
echo " warnings with PR deps: $(grep -c Deprecated "$W/v8.4-pr.err") | with master deps: $(grep -c Deprecated "$W/v8.4-base.err")"
grep -h -E 'json-schema|murmur|src/Optimizely' "$W/t5.err" "$W/v8.4-pr.err" >/dev/null && bad "T5 warning from json-schema/murmur/SDK code" || ok "T5 no warnings from json-schema 6, murmurhash 2 or SDK code"
say "T6 full unit suite (master and PR, PHP 8.1 and 8.4)"
for d in wt-base wt-pr; do for v in 8.1 8.4; do
r=$(docker run --rm -v "$W/$d":/app -w /app php:$v-cli sh -c 'mkdir -p build/logs; ./vendor/bin/phpunit --no-coverage 2>&1 | tail -3')
line=$(echo "$r" | grep -E '^(OK|Tests:|WARNINGS|FAILURES)' | tr '\n' ' ')
echo " $d php$v: $line"; [[ $r == *"Failures:"* || $r == *"Errors:"* ]] && bad "T6 $d php$v" || ok "T6 $d php$v"
done; done
say "SUMMARY for PR $PR @ $SHA"
[[ $FAIL == 0 ]] && echo "ALL PASSED" || echo "SOMETHING FAILED, see FAIL lines above"
exit $FAILparity.php
<?php
// Hash parity: murmurhash v1 (global fn) vs v2 (static method), same process.
// usage: php parity.php <dir-with-murmurhash3.php> <dir-with-src/lastguest/Murmur.php> [iterations=200000]
[$_, $v1, $v2, $n] = $argv + [null, null, null, 200000];
require "$v1/murmurhash3.php";
require "$v2/src/lastguest/Murmur.php";
mt_srand(298); // reproducible
$fixed = ['', 'a', 'ab', 'abc', 'abcd', 'abcde', '日本語テスト1886780721', "\0", "\xff\xfe", str_repeat('x', 4096),
'ppid11886780721', 'a very very very very very very very very very very very very very very very long ppd string1886780721'];
$bad = 0;
$check = function (string $k) use (&$bad) {
$a = murmurhash3_int($k, 1) & 0xFFFFFFFF;
$b = \lastguest\Murmur::hash3_int($k, 1) & 0xFFFFFFFF;
if ($a !== $b) { $bad++; fprintf(STDERR, "MISMATCH %s v1=%d v2=%d\n", bin2hex($k), $a, $b); }
};
foreach ($fixed as $k) $check($k);
for ($i = 0; $i < $n; $i++) {
$len = mt_rand(0, 40); $k = '';
for ($j = 0; $j < $len; $j++) $k .= chr(mt_rand(0, 255)); // arbitrary bytes covers every tail length + high bytes
$check($k);
$check("user_$i" . mt_rand(1, 9999999999)); // realistic bucketingId.experimentId shape
}
printf("checked %d keys, %d mismatches\n", count($fixed) + 2 * $n, $bad);
exit($bad ? 1 : 0);decide_dump.php
<?php
// Dump decisions for N synthetic users across every flag. Never dispatches events.
// usage: php decide_dump.php <sdk-checkout-dir> <datafile.json> [users=2000] [attributes-json='{}'] > out.json
[$_, $repo, $file, $n, $attrs] = $argv + [null, null, null, 2000, '{}'];
require "$repo/vendor/autoload.php";
class NullDispatcher implements \Optimizely\Event\Dispatcher\EventDispatcherInterface {
public function dispatchEvent(\Optimizely\Event\LogEvent $event) { return true; }
}
$client = new \Optimizely\Optimizely(file_get_contents($file), new NullDispatcher());
if (!$client->isValid()) { fwrite(STDERR, "datafile rejected by SDK\n"); exit(2); }
$opts = [\Optimizely\Decide\OptimizelyDecideOption::DISABLE_DECISION_EVENT];
$a = json_decode($attrs, true) ?: [];
$out = [];
for ($i = 0; $i < $n; $i++) {
foreach (["user_$i", "ユーザー_$i"] as $uid) { // ascii + multibyte ids
$ctx = $client->createUserContext($uid, $a);
foreach ($ctx->decideAll($opts) as $flag => $d) {
$out[$uid][$flag] = [$d->getVariationKey(), $d->getEnabled(), $d->getRuleKey()];
}
}
}
ksort($out);
if (!$out || !count(reset($out))) { fwrite(STDERR, "no decisions produced; refusing to emit an empty result\n"); exit(3); }
echo json_encode($out, JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT), "\n";
fprintf(STDERR, "%d users x %d flags\n", count($out), count(reset($out) ?: []));validator_matrix.php
<?php
// validateJsonSchema over a datafile + mutations; prints one stable line per case so two runs can be diffed.
// usage: php validator_matrix.php <sdk-checkout-dir> <datafile.json>
[$_, $repo, $file] = $argv + [null, null, null];
require "$repo/vendor/autoload.php";
class Collect implements \Optimizely\Logger\LoggerInterface {
public $lines = [];
public function log($l, $m) { $this->lines[] = $m; }
}
$raw = file_get_contents($file);
// decode to objects (not arrays) so empty {} stays {} when re-encoded
$mut = function (callable $f) use ($raw) { $d = json_decode($raw); $f($d); return json_encode($d); };
$cases = [
'valid' => $raw,
'missing accountId' => $mut(function ($d) { unset($d->accountId); }),
'missing version' => $mut(function ($d) { unset($d->version); }),
'experiments=string' => $mut(function ($d) { $d->experiments = 'nope'; }),
'exp key=int' => $mut(function ($d) { $d->experiments[0]->key = 123; }),
'empty object' => '{}',
'null' => 'null',
'not json' => '{oops',
'empty string' => '',
];
foreach ($cases as $name => $json) {
$log = new Collect();
$ok = \Optimizely\Utils\Validator::validateJsonSchema($json, $log);
// count of debug lines + whether the violation text actually made it into the log
printf("%-20s valid=%-5s logLines=%d violationTextLogged=%s\n", $name, var_export($ok, true), count($log->lines),
preg_match('/\[(?!%s)[^\]]+\] \S/', implode('', $log->lines)) ? 'yes' : 'no');
}Limits (unchanged)
- Used the datafile that ships in the repo's tests (10 flags), not a real project's. Multi-armed bandit rules were not exercised.
- 64-bit PHP only, and PHP 8.1 and 8.4 only (CI covers 8.2).
- The red
fullstack-sdk-compatibility-suite (php)check was not rerun. It fails the same 8 holdout scenarios, and master's source has no holdout code, so it is a pre-existing gap and not this PR.
Run with Claude Code on Alex Yong's machine.
Mat001
left a comment
There was a problem hiding this comment.
We said we will add changelog in a separate PR so my comments were addressed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* [FSSDK-13266] chore: Prepare release 4.0.4 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [FSSDK-13266] List all #298 changes in the 4.0.4 CHANGELOG entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>


Summary
FSSDK-13266
Allows newer versions of two dependencies so customers can upgrade them alongside the SDK. This prepares for the 4.0.4 release; the CHANGELOG entry and
SDK_VERSIONbump will follow in a separate release-prep PR.justinrainbow/json-schemanow allows^6.0;lastguest/murmurhashnow allows^2.1. Lock file regenerated.murmurhash3_int()with\lastguest\Murmur::hash3_int(). The SDK now calls whichever one is installed. Both produce identical hashes, so no users are re-bucketed.No json-schema code changes were needed;
check()works on every allowed version.Verification
🤖 Generated with Claude Code