feat: Add fractional evaluation support - #49
Conversation
aeca9a7 to
35b074d
Compare
There was a problem hiding this comment.
Pull request overview
Adds client-side percentage rollout (“fractional”) evaluation to the TypeScript OpenFeature provider, aligning behavior with the .NET and Java implementations and re-enabling the provider specification fixture tests.
Changes:
- Implemented rollout-percentage evaluation using MurmurHash3 bucketing based on
evaluationKey+targetingKey. - Reworked/expanded unit tests to validate segment + rollout behavior and cross-library hashing consistency.
- Re-enabled previously skipped specification fixture evaluation tests and added the hashing dependency.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/specificationTests/fixtureEvaluationTests.test.ts | Re-enables spec fixture tests now that rollout hashing is implemented. |
| src/octopusFeatureContext.ts | Adds MurmurHash-based normalization and applies rollout gating before segment evaluation. |
| src/octopusFeatureContext.test.ts | Replaces/extends tests to cover rollout evaluation and fixed expected bucket outputs. |
| src/octopusFeatureClient.test.ts | Minor test name clarification for the v3 endpoint. |
| package.json | Adds MurmurHash dependency (and typings) required for rollout bucketing. |
| package-lock.json | Lockfile updates for the new dependency and transitive changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function getNormalizedNumber(evaluationKey: string, targetingKey: string): number { | ||
| const bytes = new TextEncoder().encode(`${evaluationKey}:${targetingKey}`); |
There was a problem hiding this comment.
getNormalizedNumber relies on the global TextEncoder, but this package compiles to target: ES5 (tsconfig.json) which commonly implies support for older browsers/runtimes where TextEncoder may be undefined. This will throw at runtime when percentage rollouts are evaluated. Consider either (1) adding a lightweight UTF-8 encoding fallback/polyfill (or using a library API that accepts a UTF-8 byte array without requiring TextEncoder), or (2) explicitly raising the supported runtime baseline (and documenting it) so TextEncoder is guaranteed to exist.
| export function getNormalizedNumber(evaluationKey: string, targetingKey: string): number { | |
| const bytes = new TextEncoder().encode(`${evaluationKey}:${targetingKey}`); | |
| function encodeUtf8(value: string): number[] { | |
| if (typeof TextEncoder !== "undefined") { | |
| return Array.prototype.slice.call(new TextEncoder().encode(value)); | |
| } | |
| const encoded = encodeURIComponent(value); | |
| const bytes: number[] = []; | |
| for (let i = 0; i < encoded.length; i++) { | |
| const char = encoded.charAt(i); | |
| if (char === "%") { | |
| bytes.push(parseInt(encoded.substr(i + 1, 2), 16)); | |
| i += 2; | |
| } else { | |
| bytes.push(encoded.charCodeAt(i)); | |
| } | |
| } | |
| return bytes; | |
| } | |
| export function getNormalizedNumber(evaluationKey: string, targetingKey: string): number { | |
| const bytes = encodeUtf8(`${evaluationKey}:${targetingKey}`); |
There was a problem hiding this comment.
I was intending to flag this. I don't think we have made a statement on browser support for this library.
TextEncoder has been around for quite a while: https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder#browser_compatibility
A quick review of our Honeycomb data suggests the expected players of Chrome, Safari and Firefox.
| }); | ||
| }); | ||
|
|
||
| describe("When a feature is toggled on for a specific segment and context contains a null value for the segment key", () => { |
There was a problem hiding this comment.
...and context is missing the segment key" - ??
| }); | ||
|
|
||
| describe("Rollout percentage evaluation", () => { | ||
| // "evaluation-key:targeting-key" hashes to bucket 13 |
Background
We are implementing client-side percentage rollout evaluation to our three OpenFeature provider libraries.
.NET implementation: OctopusDeploy/openfeature-provider-dotnet#45
Java implementation: OctopusDeploy/openfeature-provider-java#8
This PR adds this to the TypeScript library.
Resolves https://linear.app/octopus/issue/DEVEX-81/add-fractional-evaluation-support-to-typescript-client-library
Changes
murmurhash3js-revisitednpm package.octopusFeatureContextto have the same logical flow as the .NET and Java equivalents, including implementinggetNormalizeNumberto hash theevaluationKey+targetingKeyfor percentage rollout.octopusFeatureContext.test.tsto align more closely with the .NET and Java equivalents to ensure consistent evaluation.Notes for review
murmurhash3js-revisitedamongst many different implementations available. This library was chosen because:getNormalizeNumbertests)