Skip to content

[FSSDK-13266] Update dependency constraints for json-schema and murmurhash - #298

Merged
Tamara-Barum merged 10 commits into
masterfrom
tbarum/FSSDK-13266-update-dependency-constraints
Oct 15, 2026
Merged

Tamara-Barum merged 10 commits into
masterfrom
tbarum/FSSDK-13266-update-dependency-constraints

Conversation

@Tamara-Barum

@Tamara-Barum Tamara-Barum commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

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_VERSION bump will follow in a separate release-prep PR.

  • composer.json: justinrainbow/json-schema now allows ^6.0; lastguest/murmurhash now allows ^2.1. Lock file regenerated.
  • Bucketer.php: murmurhash v2 replaced the global 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.
  • Tests: Added empty-string and UTF-8 bucketing cases.
  • CI: New "Latest Deps" job (PHP 8.1 and 8.2) installs json-schema v6 and murmurhash v2, checks those versions actually resolved, and runs the full test suite.

No json-schema code changes were needed; check() works on every allowed version.

Verification

  • All tests pass on the locked versions and on json-schema 6.13.1 / murmurhash 2.1.1.
  • Bucket values match the Python SDK across 10k keys with both murmurhash v1 and v2.

🤖 Generated with Claude Code

…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>
@datadog-optimizely-experimentation

This comment has been minimized.

@coveralls

coveralls commented Oct 12, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 97.303% (-0.03%) from 97.336% — tbarum/FSSDK-13266-update-dependency-constraints into master

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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Optimizely/Utils/Validator.php Outdated
Comment thread composer.json Outdated
Tamara-Barum and others added 3 commits October 12, 2026 13:35
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 alexjoeyyong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/Optimizely/Utils/Validator.php Outdated
Comment thread src/Optimizely/Bucketer.php Outdated
Comment thread composer.json Outdated
Comment thread CHANGELOG.md Outdated
Comment thread .github/workflows/php.yml Outdated

@Mat001 Mat001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/php.yml
Comment thread src/Optimizely/Bucketer.php Outdated
Comment thread src/Optimizely/Utils/Validator.php Outdated

@Mat001 Mat001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The dependency verification command will fail, and the promised changelog entry is missing.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread .github/workflows/php.yml Outdated
Comment thread composer.json
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>

@alexjoeyyong alexjoeyyong left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 a string type hint; Bucketer always 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.lock content-hash from composer.json and it matches. No package versions changed in the lock.
  • In json-schema 5.2.12 and 6.x, check() is return $this->validate(...); the v6 deprecation is docblock-only.

Ponytail findings (what to cut)

  1. Validator.php:L40-46 - delete the method_exists if/else, keep the single check() call (inline).
  2. php.yml:L83 - delete the redundant composer install (inline).
  3. php.yml:L88-89 - delete the two composer show lines (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 in f24ac21 (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 Deps is 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 in decide_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.

Comment thread src/Optimizely/Utils/Validator.php Outdated
Comment thread .github/workflows/php.yml Outdated
Comment thread .github/workflows/php.yml Outdated
- 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>
@alexjoeyyong

Copy link
Copy Markdown

Test results for this PR (run on 8.1 and 8.4)

TLDR

  • Users get the same variations with the new libraries. I compared the current master against this PR across 100,000 simulated decisions and the results were identical, down to the byte.
  • The new hashing library gives the same numbers as the old one. 400,012 different test inputs, 0 differences.
  • The datafile validator gives the same answers on the old and new library (valid file passes, 8 kinds of broken file fail, on both).
  • The checks can actually fail. I deliberately broke the bucketing, and both my comparison and the PR's own unit test caught it.
  • Bonus: on PHP 8.4, master's current json-schema library prints 32 deprecation warnings. The version this PR allows prints none.
  • Nothing here blocks the PR. My earlier requests (trim the Validator change, fix the description) are about keeping the code simple, not about correctness.
  • Not covered: a real project's datafile, 32-bit PHP, and PHP 8.2. Details under "What this does not prove".

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.

# Test What I did Result
1 Same numbers? Fed the old hashing library and the new one the same 400,012 inputs: random bytes, empty text, Japanese text, a 4 KB string, and realistic user + experiment id strings. 0 differences
2 Same decisions? Ran 10,000 simulated users (half with normal ids, half with Japanese-text ids) through all 10 flags in the repo's test datafile. Did this on master and on this PR, then compared the two files. Identical (same checksum 7bee33c3...). 50,454 of 100,000 decisions were "on", across 20 different outcomes, so this wasn't a trivially all-"off" result.
3 Can the test fail? Temporarily changed the hashing seed in the PR copy from 1 to 2, then reran. Caught it. 37% of decisions changed and testGenerateBucketValue failed. I put the file back and checked it matched the PR exactly.
4 Same datafile validation? Ran the validator on a good datafile plus 8 broken ones (missing fields, wrong types, empty, null, not JSON), on the old and new json-schema library. Same answers. Good file passes, all 8 broken ones fail, on both.
5 PHP 8.4 warnings Ran with every warning switched on. No warnings from json-schema 6, murmurhash 2 or our code. 7 come from old Guzzle and Monolog versions that are already in the lock file and also appear on master. master's json-schema 5.2.12 adds 32 more on 8.4, and those go away with 6.13.1.
6 Full unit tests Ran the whole suite on master and on the PR, each on PHP 8.1 and 8.4. 581 tests, 0 failures, all four runs. The PR has 2 extra checks (the new empty-string and Japanese-text cases). The 73 warnings are old PHPUnit notices that master has too.

How I did it

  1. Made two copies of the code in a temporary folder, outside the repo: master, and this PR.
  2. Installed the libraries in Docker. master uses the lock file as is (json-schema 5.2.12, murmurhash 1.3.0). The PR copy used the same upgrade command as the "Latest Deps" CI job, which resolved to json-schema 6.13.1 and murmurhash 2.1.1. I checked both versions before running anything. (I relaxed Composer's PHP-version check, because its Docker image runs a newer PHP than 8.1. The versions it picked match what CI picked.)
  3. Ran small scripts (below) inside Docker containers, php:8.1-cli and php:8.4-cli. The decision script sends no events, so nothing was reported anywhere.
  4. Nothing in the repo was changed, and the temporary seed edit in test 3 was reverted.
Setup commands
git 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-coverage
parity.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

  • I used the test datafile that ships with the repo, not a real project's datafile. It has 10 flags. A real one has more rules and audiences. The scripts take any datafile, so someone can rerun test 2 with a real one from a test project.
  • Multi-armed bandit and similar rule types were not exercised, because the test datafile doesn't have them.
  • Only 64-bit PHP was tested. Bucketer has a comment about 32-bit PHP returning negative hashes, and I did not try that.
  • PHP 8.1 and 8.4 only. CI covers 8.2.
  • The red fullstack-sdk-compatibility-suite (php) check is not covered here. It fails 8 scenarios in the holdouts feature, which this PR doesn't touch. I did not rerun it, and I did not check whether master fails the same way.

Side note, not caused by this PR

Validator::validateJsonSchema passes four arguments to $logger->log(...), but LoggerInterface::log() accepts two. The "what was wrong" line therefore always logs as the literal text [%s] %s. It behaves the same before and after this PR, so I'd handle it as a separate ticket.

Run with Claude Code on Alex Yong's machine.

@Mat001 Mat001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 resolved step and expanded to a PHP 8.1/8.2 matrix with fail-fast: false. Both Latest Deps legs 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.0 requires php: ^7 (no PHP 8) and this SDK needs >=8.1, so ^2.1 is the right floor.
  • Validator.php: reverting to plain check() is safe — v6.4.1's check() 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.

Comment thread .github/workflows/php.yml Outdated
Tamara-Barum and others added 2 commits October 14, 2026 15:49
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 alexjoeyyong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 install and composer show lines 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.

@alexjoeyyong

Copy link
Copy Markdown

Re-ran the tests on the final commit (edda32b)

TLDR

  • Everything passes on the exact commit that would merge. The results match my earlier run on 8cf9279.
  • Users get the same variations on master and on this PR: 100,000 simulated decisions, byte-for-byte identical.
  • The new hashing library gives the same numbers as the old one on 400,012 inputs (0 differences).
  • Full unit suite: 581 tests, 0 failures, on master and the PR, on PHP 8.1 and 8.4.
  • The checks can fail: breaking the hashing on purpose was caught by both my comparison and the PR's own unit test.

I also turned the whole procedure into one script, so anyone can repeat it with one command (see below).

Results

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 298

It 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 minutes

It 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.)
  • git and python3. You do not need PHP or Composer installed; Docker provides them.
  • A local clone of php-sdk with origin pointing 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.php commands by hand and diff the 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.
    Only Failures or Errors matter.

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 to work-<PR>/fixture.json after 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-suite check; 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     # T6

Files

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 fetched

If something goes wrong

  • Docker is not running: open Docker Desktop and rerun.
  • fetch failed: check that git -C <repo> remote get-url origin is 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.php now refuses to emit an empty result.
  • grep -q at the end of a pipe, with set -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 $FAIL
parity.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 Mat001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We said we will add changelog in a separate PR so my comments were addressed.

@Tamara-Barum
Tamara-Barum merged commit f1a319b into master Oct 15, 2026
10 of 11 checks passed
@Tamara-Barum
Tamara-Barum deleted the tbarum/FSSDK-13266-update-dependency-constraints branch October 15, 2026 20:10
Tamara-Barum added a commit that referenced this pull request Oct 15, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tamara-Barum added a commit that referenced this pull request Oct 15, 2026
* [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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants