From eb37f775c2f80f1b7002d5192a700bbb1b7a6a99 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 10:37:57 -0700 Subject: [PATCH 01/34] Accept 3-param tracking callbacks in SDK health check The SDK types allow both `(experiment, result)` and `(experiment, result, userContext)` tracking callbacks, but DevTools flagged anything other than exactly 2 params as an implementation problem. Treat 2 or 3 params as valid, and stay quiet instead of warning when the param list can't be determined at all. Also: - Share the logic between the SDK tab, the panel copy, and the background icon status so they can't drift. - Replace the `/\(([^)]+)\)/` param scrape, which truncated at destructured params and default values containing parens, and mis-parsed single-param arrow functions. - Forward extra args through the patched trackingCallback and onFeatureUsage wrappers so a userContext arg isn't swallowed. --- src/app/components/SdkTab/SdkItemPanel.tsx | 29 +++-- src/app/components/SdkTab/index.tsx | 30 ++--- src/background/index.ts | 9 +- src/content_script/embed_script.ts | 24 ++-- src/utils/sdkCallbacks.test.ts | 122 +++++++++++++++++++++ src/utils/sdkCallbacks.ts | 76 +++++++++++++ 6 files changed, 248 insertions(+), 42 deletions(-) create mode 100644 src/utils/sdkCallbacks.test.ts create mode 100644 src/utils/sdkCallbacks.ts diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index ae8473e..a987cea 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -23,6 +23,7 @@ import { useResponsiveContext } from "@/app/hooks/useResponsive"; import { SdkItem } from "./index"; import useSdkData from "@/app/hooks/useSdkData"; import { SDKHealthCheckResult } from "devtools"; +import { trackingCallbackParamsAreValid } from "@/utils/sdkCallbacks"; import { getActiveTabId } from "@/app/hooks/useTabState"; import { paddedVersionString } from "@growthbook/growthbook"; @@ -531,30 +532,28 @@ function trackingCallbackPanel({ }: SDKHealthCheckResult) { return ( - {trackingCallbackParams?.length === 2 ? ( - <> - The SDK is using a{" "} - trackingCallback. - - ) : !hasTrackingCallback ? ( + {!hasTrackingCallback ? ( <> The SDK is not using a{" "} trackingCallback. You will need to add one to track experiment exposure to your data warehouse. + ) : trackingCallbackParamsAreValid(trackingCallbackParams) ? ( + <> + The SDK is using a{" "} + trackingCallback. + ) : ( <> The SDK is using a{" "} trackingCallback with{" "} - {trackingCallbackParams?.length ? ( - {trackingCallbackParams.length} - ) : ( - <> - an unknown number of - - )}{" "} - param{trackingCallbackParams?.length !== 1 ? "s" : ""} instead of 2. - Please check your implementation. + + {trackingCallbackParams?.length ?? 0} + {" "} + param{trackingCallbackParams?.length === 1 ? "" : "s"} instead of{" "} + (experiment, result) or{" "} + (experiment, result, userContext). Please check your + implementation. )} diff --git a/src/app/components/SdkTab/index.tsx b/src/app/components/SdkTab/index.tsx index 4f11d70..9730efa 100644 --- a/src/app/components/SdkTab/index.tsx +++ b/src/app/components/SdkTab/index.tsx @@ -9,6 +9,7 @@ import SdkItemPanel from "./SdkItemPanel"; import useSdkData from "@/app/hooks/useSdkData"; import { paddedVersionString } from "@growthbook/growthbook"; import packageJson from "@growthbook/growthbook/package.json"; +import { hasTrackingCallbackIssues } from "@/utils/sdkCallbacks"; const latestSdkVersion = packageJson.version; const latestSdkParts = latestSdkVersion.split("."); @@ -62,18 +63,20 @@ export default function SdkTab() { : isRemoteEval ? "Remote Eval" : "Plain Text"; - const trackingCallbackStatus = - trackingCallbackParams?.length === 2 - ? "Found" - : !hasTrackingCallback - ? "None Found" - : "Found (issues)"; - const trackingCallbackStatusColor = - trackingCallbackParams?.length === 2 - ? "green" - : !hasTrackingCallback - ? "red" - : "orange"; + const trackingCallbackIssues = hasTrackingCallbackIssues({ + hasTrackingCallback, + trackingCallbackParams, + }); + const trackingCallbackStatus = !hasTrackingCallback + ? "None Found" + : trackingCallbackIssues + ? "Found (issues)" + : "Found"; + const trackingCallbackStatusColor = !hasTrackingCallback + ? "red" + : trackingCallbackIssues + ? "orange" + : "green"; const canConnectStatus = sdkFound === undefined ? "Loading..." @@ -301,8 +304,7 @@ export function getSdkStatus( (!sdkData.canConnect && !numExternalSdks) || (sdkData.canConnect && !sdkData.hasPayload) || (!sdkData.hasTrackingCallback && !numExternalSdks) || - (sdkData.hasTrackingCallback && - sdkData.trackingCallbackParams?.length !== 2) || + hasTrackingCallbackIssues(sdkData) || (sdkData.hasPayload && !sdkData.payloadDecrypted) || (paddedVersionString(sdkData.version) < paddedVersionString(latestMinorSdkVersion) && diff --git a/src/background/index.ts b/src/background/index.ts index 6466e1d..c975163 100644 --- a/src/background/index.ts +++ b/src/background/index.ts @@ -22,6 +22,7 @@ import { } from "@/background/visualEditorHandlers"; import packageJson from "@growthbook/growthbook/package.json"; import { paddedVersionString } from "@growthbook/growthbook"; +import { hasTrackingCallbackIssues } from "@/utils/sdkCallbacks"; const latestSdkVersion = packageJson.version; const latestSdkParts = latestSdkVersion.split("."); @@ -210,9 +211,8 @@ const UpdateTabIconBasedOnSDK = ( : !data.hasPayload ? "No SDK payload\n" : "SDK connected\n") + - (data.trackingCallbackParams?.length !== 2 - ? "Tracking callback issues\n" - : "") + + (!data.hasTrackingCallback ? "No tracking callback\n" : "") + + (hasTrackingCallbackIssues(data) ? "Tracking callback issues\n" : "") + (!data.payloadDecrypted ? "Decryption issues\n" : "") + (paddedVersionString(data.version) < paddedVersionString(latestMinorSdkVersion) @@ -309,8 +309,7 @@ export function getSdkStatus( (!sdkData.canConnect && !numExternalSdks) || (sdkData.canConnect && !sdkData.hasPayload) || (!sdkData.hasTrackingCallback && !numExternalSdks) || - (sdkData.hasTrackingCallback && - sdkData.trackingCallbackParams?.length !== 2) || + hasTrackingCallbackIssues(sdkData) || (sdkData.hasPayload && !sdkData.payloadDecrypted) || (paddedVersionString(sdkData.version) < paddedVersionString(latestMinorSdkVersion) && diff --git a/src/content_script/embed_script.ts b/src/content_script/embed_script.ts index 282e7e1..8e4180c 100644 --- a/src/content_script/embed_script.ts +++ b/src/content_script/embed_script.ts @@ -10,6 +10,7 @@ import type { } from "@growthbook/growthbook"; import type { ErrorMessage, SDKHealthCheckResult } from "devtools"; import { Attributes } from "@growthbook/growthbook"; +import { parseCallbackParams } from "@/utils/sdkCallbacks"; type LogUnionWithSource = LogUnion & { source?: string; clientKey?: string }; @@ -458,6 +459,8 @@ function subscribeToSdkChanges( const patchedCallBack = ( experiment: Experiment, result: Result, + // Newer SDKs pass a third userContext arg - forward whatever we get + ...rest: unknown[] ) => { if (!hasSdkLogSupport) { gb.logs!.push({ @@ -473,16 +476,16 @@ function subscribeToSdkChanges( { experiment, result }, ]); } - callback(experiment, result); + (callback as (...args: unknown[]) => unknown)( + experiment, + result, + ...rest, + ); }; if ("isNoopCallback" in callback && callback.isNoopCallback) { patchedCallBack.isNoopCallback = true; } else { - patchedCallBack.originalParams = callback - .toString() - .match(/\(([^)]+)\)/)?.[1] - .split(",") - .map((param: string) => param.trim()); + patchedCallBack.originalParams = parseCallbackParams(callback); } _setTrackingCallback?.call(gb, patchedCallBack); pushAppUpdates(); @@ -499,7 +502,12 @@ function subscribeToSdkChanges( // Feature usage callbacks // @ts-expect-error Context is private but we still need to write it here - gb.context.onFeatureUsage = (key: string, result: FeatureResult) => { + gb.context.onFeatureUsage = ( + key: string, + result: FeatureResult, + // Newer SDKs pass a third userContext arg - forward whatever we get + ...rest: unknown[] + ) => { if (!hasSdkLogSupport) { gb.logs!.push({ featureKey: key, @@ -509,7 +517,7 @@ function subscribeToSdkChanges( }); } if (typeof onFeatureUsage === "function") { - onFeatureUsage(key, result); + (onFeatureUsage as (...args: unknown[]) => unknown)(key, result, ...rest); } }; if (!onFeatureUsage || typeof onFeatureUsage !== "function") { diff --git a/src/utils/sdkCallbacks.test.ts b/src/utils/sdkCallbacks.test.ts new file mode 100644 index 0000000..2141ea3 --- /dev/null +++ b/src/utils/sdkCallbacks.test.ts @@ -0,0 +1,122 @@ +import { + hasTrackingCallbackIssues, + parseCallbackParams, + trackingCallbackParamsAreValid, +} from "./sdkCallbacks"; + +describe("parseCallbackParams", () => { + it("parses arrow functions", () => { + expect(parseCallbackParams((experiment, result) => {})).toEqual([ + "experiment", + "result", + ]); + }); + + it("parses arrow functions with a userContext param", () => { + expect( + parseCallbackParams((experiment, result, userContext) => {}), + ).toEqual(["experiment", "result", "userContext"]); + }); + + it("parses function expressions", () => { + expect( + parseCallbackParams(function (experiment: any, result: any) {}), + ).toEqual(["experiment", "result"]); + }); + + it("parses async functions", () => { + expect( + parseCallbackParams(async function (experiment: any, result: any) {}), + ).toEqual(["experiment", "result"]); + }); + + it("parses a single unparenthesized arrow param", () => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const cb = (experiment: any) => ({ key: experiment.key }); + expect(parseCallbackParams(cb)).toEqual(["experiment"]); + }); + + it("does not truncate at a destructured param", () => { + expect( + parseCallbackParams( + (experiment, { variationId }, userContext) => undefined, + ), + ).toEqual(["experiment", "{ variationId }", "userContext"]); + }); + + it("does not truncate at a default value containing parens", () => { + expect( + parseCallbackParams((experiment, result = getDefault(), extra) => {}), + ).toEqual(["experiment", "result = getDefault()", "extra"]); + }); + + it("parses rest params", () => { + expect(parseCallbackParams((...args) => {})).toEqual(["...args"]); + }); + + it("parses zero params", () => { + expect(parseCallbackParams(() => {})).toEqual([]); + }); + + it("returns undefined for native/bound functions", () => { + const bound = ((a: any, b: any) => {}).bind(null); + expect(parseCallbackParams(bound)).toBeUndefined(); + }); +}); + +describe("trackingCallbackParamsAreValid", () => { + it("accepts 2 params", () => { + expect(trackingCallbackParamsAreValid(["experiment", "result"])).toBe(true); + }); + + it("accepts 3 params", () => { + expect( + trackingCallbackParamsAreValid(["experiment", "result", "userContext"]), + ).toBe(true); + }); + + it("accepts rest params", () => { + expect(trackingCallbackParamsAreValid(["...args"])).toBe(true); + }); + + it("accepts unknown params rather than warning", () => { + expect(trackingCallbackParamsAreValid(undefined)).toBe(true); + }); + + it("rejects too few params", () => { + expect(trackingCallbackParamsAreValid(["experiment"])).toBe(false); + expect(trackingCallbackParamsAreValid([])).toBe(false); + }); + + it("rejects too many params", () => { + expect(trackingCallbackParamsAreValid(["a", "b", "c", "d"])).toBe(false); + }); +}); + +describe("hasTrackingCallbackIssues", () => { + it("does not flag a missing tracking callback as an issue", () => { + expect(hasTrackingCallbackIssues({ hasTrackingCallback: false })).toBe( + false, + ); + }); + + it("does not flag a 3-param tracking callback", () => { + expect( + hasTrackingCallbackIssues({ + hasTrackingCallback: true, + trackingCallbackParams: ["experiment", "result", "userContext"], + }), + ).toBe(false); + }); + + it("flags a 1-param tracking callback", () => { + expect( + hasTrackingCallbackIssues({ + hasTrackingCallback: true, + trackingCallbackParams: ["experiment"], + }), + ).toBe(true); + }); +}); + +declare function getDefault(): any; diff --git a/src/utils/sdkCallbacks.ts b/src/utils/sdkCallbacks.ts new file mode 100644 index 0000000..c631551 --- /dev/null +++ b/src/utils/sdkCallbacks.ts @@ -0,0 +1,76 @@ +// Helpers for inspecting the callbacks a page's SDK was initialized with. +// Shared by the injected embed script (which reads them off the SDK context), +// the background worker (icon status), and the SDK tab UI. + +// A trackingCallback may be written as either `(experiment, result)` or +// `(experiment, result, userContext)` - the SDK types allow both. +const VALID_TRACKING_CALLBACK_PARAM_COUNTS = [2, 3]; + +// Pull the parameter names off a function's source. Returns undefined when we +// can't tell (native/bound functions, minified oddities) so callers can avoid +// warning about something they didn't actually detect. +export function parseCallbackParams( + callback: (...args: any[]) => any, +): string[] | undefined { + let src: string; + try { + src = callback.toString(); + } catch (e) { + return undefined; + } + if (src.includes("[native code]")) return undefined; + + // Single-param arrow function without parens, eg `experiment => ...` + const bareArrowParam = src.match(/^\s*(?:async\s+)?([A-Za-z_$][\w$]*)\s*=>/); + if (bareArrowParam) return [bareArrowParam[1]]; + + const open = src.indexOf("("); + if (open === -1) return undefined; + + // Walk to the matching close paren, tracking nesting so that default values + // and destructured params don't cut the list short. + const params: string[] = []; + let current = ""; + let depth = 0; + for (let i = open; i < src.length; i++) { + const char = src[i]; + if (char === "(" || char === "[" || char === "{") { + depth++; + if (depth === 1) continue; + } else if (char === ")" || char === "]" || char === "}") { + depth--; + if (depth === 0) { + if (current.trim()) params.push(current.trim()); + return params; + } + } else if (char === "," && depth === 1) { + if (current.trim()) params.push(current.trim()); + current = ""; + continue; + } + current += char; + } + // Never found the closing paren + return undefined; +} + +export function trackingCallbackParamsAreValid( + params: string[] | undefined, +): boolean { + // Couldn't determine the params - don't claim there's a problem + if (!params) return true; + // Rest params (`...args`) can stand in for any number of arguments + if (params.some((param) => param.startsWith("..."))) return true; + return VALID_TRACKING_CALLBACK_PARAM_COUNTS.includes(params.length); +} + +export function hasTrackingCallbackIssues({ + hasTrackingCallback, + trackingCallbackParams, +}: { + hasTrackingCallback?: boolean; + trackingCallbackParams?: string[]; +}): boolean { + if (!hasTrackingCallback) return false; + return !trackingCallbackParamsAreValid(trackingCallbackParams); +} From bc7113814377fd261ae14432acb71b812a7dedbb Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 10:54:32 -0700 Subject: [PATCH 02/34] Bump @growthbook/growthbook to 1.7.0 DevTools derives its "latest SDK version" from the bundled SDK's package.json, so a stale dependency means stale outdated-version warnings. 1.7.0 also passes a userContext arg to instance-level trackingCallbacks, which the arg forwarding in the previous commit now preserves. --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 8ff2c85..f2175f4 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "@chakra-ui/react": "^2.8.2", "@emotion/react": "^11.13.0", "@emotion/styled": "^11.13.0", - "@growthbook/growthbook": "^1.6.5", + "@growthbook/growthbook": "^1.7.0", "@medv/finder": "^3.2.0", "@phosphor-icons/react": "^2.1.7", "@radix-ui/colors": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 270d224..b8c0215 100644 --- a/yarn.lock +++ b/yarn.lock @@ -641,10 +641,10 @@ resolved "https://registry.yarnpkg.com/@floating-ui/utils/-/utils-0.2.9.tgz#50dea3616bc8191fb8e112283b49eaff03e78429" integrity sha512-MDWhGtE+eHw5JW7lq4qhc5yRLS11ERl1c7Z6Xd0a58DozHES6EnNNwUWbMiG4J9Cgj053Bhk8zvlhFYKVhULwg== -"@growthbook/growthbook@^1.6.5": - version "1.6.5" - resolved "https://registry.yarnpkg.com/@growthbook/growthbook/-/growthbook-1.6.5.tgz#c9e2119187ee3288525a77a7c64353276f5ba91b" - integrity sha512-mUaMsgeUTpRIUOTn33EUXHRK6j7pxBjwqH4WpQyq+pukjd1AIzWlEa6w7i6bInJUcweGgP2beXZmaP6b6UPn7A== +"@growthbook/growthbook@^1.7.0": + version "1.7.0" + resolved "https://registry.yarnpkg.com/@growthbook/growthbook/-/growthbook-1.7.0.tgz#4507d6ca08c9e9c8fa311b708292181d37ac1a17" + integrity sha512-Vmad3fWRh1XaFH4q+AS62ofX6Q3IxSTD7sBGuhEHCaad8VyZerxrVdojucjzlnJDUocfrnZFUSpEkR6R/wCV+A== dependencies: dom-mutator "^0.6.0" From 7eabf5ec878636955d89d2f0fddc11b10184fef2 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 11:22:34 -0700 Subject: [PATCH 03/34] Show trackingCallback param count on the SDK tab The row now reads "Found (3 params)" and the panel names the detected signature, so the callback shape is visible without opening the page's source. --- src/app/components/SdkTab/SdkItemPanel.tsx | 11 ++++++++++- src/app/components/SdkTab/index.tsx | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index a987cea..6748ba5 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -541,7 +541,16 @@ function trackingCallbackPanel({ ) : trackingCallbackParamsAreValid(trackingCallbackParams) ? ( <> The SDK is using a{" "} - trackingCallback. + trackingCallback + {trackingCallbackParams ? ( + <> + {" "} + with {trackingCallbackParams.length} param + {trackingCallbackParams.length === 1 ? "" : "s"}:{" "} + ({trackingCallbackParams.join(", ")}) + + ) : null} + . ) : ( <> diff --git a/src/app/components/SdkTab/index.tsx b/src/app/components/SdkTab/index.tsx index 9730efa..1b786c4 100644 --- a/src/app/components/SdkTab/index.tsx +++ b/src/app/components/SdkTab/index.tsx @@ -71,7 +71,9 @@ export default function SdkTab() { ? "None Found" : trackingCallbackIssues ? "Found (issues)" - : "Found"; + : trackingCallbackParams + ? `Found (${trackingCallbackParams.length} params)` + : "Found"; const trackingCallbackStatusColor = !hasTrackingCallback ? "red" : trackingCallbackIssues From dd4b2e5b05d420f35fcd85aaf985777fd75354b1 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 11:31:07 -0700 Subject: [PATCH 04/34] Add test-page harness for SDK health checks Self-contained page for exercising the SDK tab with no GrowthBook account: apiHost points at the same static server, and a static api/features/local-test payload satisfies the canConnect probe. ?params=1|2|3|4 switches the trackingCallback arity and &ofu=1 supplies an onFeatureUsage callback. The SDK bundle is copied in by `yarn test-page` rather than vendored. --- package.json | 1 + test-page/.gitignore | 2 + test-page/README.md | 29 +++++ test-page/api/features/local-test | 23 ++++ test-page/index.html | 170 ++++++++++++++++++++++++++++++ 5 files changed, 225 insertions(+) create mode 100644 test-page/.gitignore create mode 100644 test-page/README.md create mode 100644 test-page/api/features/local-test create mode 100644 test-page/index.html diff --git a/package.json b/package.json index f2175f4..3f5d86d 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "clean": "rimraf dist", "test": "npx jest", "style": "prettier --write \"src/**/*.{ts,tsx}\"", + "test-page": "cp node_modules/@growthbook/growthbook/dist/bundles/index.min.js test-page/growthbook.js && cd test-page && python3 -m http.server 8899", "build:chrome": "yarn clean && yarn build && cp public/manifest.chrome.json dist/manifest.json && cd dist && rm -f manifest.*.json", "build:firefox": "yarn clean && yarn build && cp public/manifest.firefox.json dist/manifest.json && cd dist && rm -f manifest.*.json", "package:chrome": "yarn clean && yarn build && cp public/manifest.chrome.json dist/manifest.json && cd dist && rm -f manifest.*.json && zip -r build.chrome.zip . && mv build.chrome.zip ../", diff --git a/test-page/.gitignore b/test-page/.gitignore new file mode 100644 index 0000000..82344ec --- /dev/null +++ b/test-page/.gitignore @@ -0,0 +1,2 @@ +# Copied from node_modules by `yarn test-page` +growthbook.js diff --git a/test-page/README.md b/test-page/README.md new file mode 100644 index 0000000..33783d0 --- /dev/null +++ b/test-page/README.md @@ -0,0 +1,29 @@ +# SDK health-check test page + +A self-contained page for exercising the DevTools SDK tab without a real +GrowthBook account. + +```sh +yarn test-page # copies the SDK bundle in, serves on :8899 +``` + +Then load `dist/` as an unpacked extension and open http://localhost:8899. + +`apiHost` points at this same static server, and `api/features/local-test` is +a static payload, so `GET /api/features/local-test` returns 200 — which is +exactly the probe DevTools uses to decide "Connected". No API key needed. + +## Switches + +| URL | Tracking Callback row | +| ----------- | --------------------- | +| `?params=2` | Found (2 params) | +| `?params=3` | Found (3 params) | +| `?params=1` | Found (issues) | +| `?params=4` | Found (issues) | + +Add `&ofu=1` (or click the toggle) to supply an `onFeatureUsage` callback and +flip the "On Feature Usage Callback" row to Yes. + +Note: the SDK dedupes feature usage by value, so re-evaluating a feature to the +same value fires `onFeatureUsage` only once. diff --git a/test-page/api/features/local-test b/test-page/api/features/local-test new file mode 100644 index 0000000..cab389a --- /dev/null +++ b/test-page/api/features/local-test @@ -0,0 +1,23 @@ +{ + "status": 200, + "dateUpdated": "2026-08-26T00:00:00.000Z", + "features": { + "my-feature": { + "defaultValue": "off", + "rules": [ + { + "key": "my-experiment", + "variations": ["off", "on"], + "weights": [0.5, 0.5], + "hashAttribute": "id", + "coverage": 1 + } + ] + }, + "banner-color": { + "defaultValue": "blue", + "rules": [{ "condition": { "country": "US" }, "force": "green" }] + } + }, + "experiments": [] +} diff --git a/test-page/index.html b/test-page/index.html new file mode 100644 index 0000000..6f037ea --- /dev/null +++ b/test-page/index.html @@ -0,0 +1,170 @@ + + + + GB DevTools - trackingCallback param counts + + + +

trackingCallback param counts

+ + + + +

Active signature: -

+

SDK payload: loading…

+

+ + +

+ 
(no calls yet)
+ + + + + From 6035d9a5cc5c6eba7048bccc82cd9a42ab0d3916 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 11:32:54 -0700 Subject: [PATCH 05/34] Revert "Add test-page harness for SDK health checks" This reverts commit dd4b2e5b05d420f35fcd85aaf985777fd75354b1. --- package.json | 1 - test-page/.gitignore | 2 - test-page/README.md | 29 ----- test-page/api/features/local-test | 23 ---- test-page/index.html | 170 ------------------------------ 5 files changed, 225 deletions(-) delete mode 100644 test-page/.gitignore delete mode 100644 test-page/README.md delete mode 100644 test-page/api/features/local-test delete mode 100644 test-page/index.html diff --git a/package.json b/package.json index 3f5d86d..f2175f4 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,6 @@ "clean": "rimraf dist", "test": "npx jest", "style": "prettier --write \"src/**/*.{ts,tsx}\"", - "test-page": "cp node_modules/@growthbook/growthbook/dist/bundles/index.min.js test-page/growthbook.js && cd test-page && python3 -m http.server 8899", "build:chrome": "yarn clean && yarn build && cp public/manifest.chrome.json dist/manifest.json && cd dist && rm -f manifest.*.json", "build:firefox": "yarn clean && yarn build && cp public/manifest.firefox.json dist/manifest.json && cd dist && rm -f manifest.*.json", "package:chrome": "yarn clean && yarn build && cp public/manifest.chrome.json dist/manifest.json && cd dist && rm -f manifest.*.json && zip -r build.chrome.zip . && mv build.chrome.zip ../", diff --git a/test-page/.gitignore b/test-page/.gitignore deleted file mode 100644 index 82344ec..0000000 --- a/test-page/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# Copied from node_modules by `yarn test-page` -growthbook.js diff --git a/test-page/README.md b/test-page/README.md deleted file mode 100644 index 33783d0..0000000 --- a/test-page/README.md +++ /dev/null @@ -1,29 +0,0 @@ -# SDK health-check test page - -A self-contained page for exercising the DevTools SDK tab without a real -GrowthBook account. - -```sh -yarn test-page # copies the SDK bundle in, serves on :8899 -``` - -Then load `dist/` as an unpacked extension and open http://localhost:8899. - -`apiHost` points at this same static server, and `api/features/local-test` is -a static payload, so `GET /api/features/local-test` returns 200 — which is -exactly the probe DevTools uses to decide "Connected". No API key needed. - -## Switches - -| URL | Tracking Callback row | -| ----------- | --------------------- | -| `?params=2` | Found (2 params) | -| `?params=3` | Found (3 params) | -| `?params=1` | Found (issues) | -| `?params=4` | Found (issues) | - -Add `&ofu=1` (or click the toggle) to supply an `onFeatureUsage` callback and -flip the "On Feature Usage Callback" row to Yes. - -Note: the SDK dedupes feature usage by value, so re-evaluating a feature to the -same value fires `onFeatureUsage` only once. diff --git a/test-page/api/features/local-test b/test-page/api/features/local-test deleted file mode 100644 index cab389a..0000000 --- a/test-page/api/features/local-test +++ /dev/null @@ -1,23 +0,0 @@ -{ - "status": 200, - "dateUpdated": "2026-08-26T00:00:00.000Z", - "features": { - "my-feature": { - "defaultValue": "off", - "rules": [ - { - "key": "my-experiment", - "variations": ["off", "on"], - "weights": [0.5, 0.5], - "hashAttribute": "id", - "coverage": 1 - } - ] - }, - "banner-color": { - "defaultValue": "blue", - "rules": [{ "condition": { "country": "US" }, "force": "green" }] - } - }, - "experiments": [] -} diff --git a/test-page/index.html b/test-page/index.html deleted file mode 100644 index 6f037ea..0000000 --- a/test-page/index.html +++ /dev/null @@ -1,170 +0,0 @@ - - - - GB DevTools - trackingCallback param counts - - - -

trackingCallback param counts

- - - - -

Active signature: -

-

SDK payload: loading…

-

- - -

- 
(no calls yet)
- - - - - From f8726273afe200968aac01a45fdd4f6a60d00d9f Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 12:01:36 -0700 Subject: [PATCH 06/34] Drop pointless arg forwarding from the onFeatureUsage wrapper onFeatureUsage only ever lands in the SDK's user context, which core.ts calls as cb(key, result); the 3-arg form is GrowthBookClient's global context, which DevTools never patches. So the rest param was always empty and the cast only silenced a type error for an argument that cannot arrive. trackingCallback genuinely does receive a third userContext arg in 1.7+, so that forwarding stays - but now via a typed optional param rather than a rest spread plus a cast. --- src/content_script/embed_script.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/content_script/embed_script.ts b/src/content_script/embed_script.ts index 8e4180c..8ceea0e 100644 --- a/src/content_script/embed_script.ts +++ b/src/content_script/embed_script.ts @@ -459,8 +459,8 @@ function subscribeToSdkChanges( const patchedCallBack = ( experiment: Experiment, result: Result, - // Newer SDKs pass a third userContext arg - forward whatever we get - ...rest: unknown[] + // SDK 1.7+ passes a third userContext arg - forward it or it's lost + user?: Parameters[2], ) => { if (!hasSdkLogSupport) { gb.logs!.push({ @@ -476,11 +476,7 @@ function subscribeToSdkChanges( { experiment, result }, ]); } - (callback as (...args: unknown[]) => unknown)( - experiment, - result, - ...rest, - ); + callback(experiment, result, user); }; if ("isNoopCallback" in callback && callback.isNoopCallback) { patchedCallBack.isNoopCallback = true; @@ -502,12 +498,7 @@ function subscribeToSdkChanges( // Feature usage callbacks // @ts-expect-error Context is private but we still need to write it here - gb.context.onFeatureUsage = ( - key: string, - result: FeatureResult, - // Newer SDKs pass a third userContext arg - forward whatever we get - ...rest: unknown[] - ) => { + gb.context.onFeatureUsage = (key: string, result: FeatureResult) => { if (!hasSdkLogSupport) { gb.logs!.push({ featureKey: key, @@ -517,7 +508,10 @@ function subscribeToSdkChanges( }); } if (typeof onFeatureUsage === "function") { - (onFeatureUsage as (...args: unknown[]) => unknown)(key, result, ...rest); + // Only ever called with 2 args - onFeatureUsage lives in the SDK's user + // context, and core.ts calls those as cb(key, result). The 3-arg form is + // for GrowthBookClient's global context, which DevTools doesn't patch. + onFeatureUsage(key, result); } }; if (!onFeatureUsage || typeof onFeatureUsage !== "function") { From 1011df0c0ca61ac5a84b872c4e6b036d28fd5ad1 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 14:52:19 -0700 Subject: [PATCH 07/34] Fix defects found in code review - deferred tracking calls kept only {experiment, result}, but fireDeferredTrackingCalls replays them as trackingCallback(experiment, result, call.user), so a replayed 3-param callback got user === undefined - the wrapper discarded the callback's return value, so the SDK could no longer await an async trackingCallback - a zero-arity forwarding wrapper parsed to [] and warned; that shape is common in minified analytics code and works fine - parseCallbackParams desynced on brackets and commas inside string defaults, eg (experiment, result, sep = ",") - the toString guard could throw a TypeError out of the injected script when a page shadowed toString to return a non-string - the row read "Found (1 params)" and counted rest params, disagreeing with the detail panel Also makes the bare-arrow test exercise the branch it names - a type annotation forced parens, so deleting the branch left the suite green. --- src/app/components/SdkTab/index.tsx | 8 +++- src/content_script/embed_script.ts | 15 ++++--- src/utils/sdkCallbacks.test.ts | 69 ++++++++++++++++++++++++----- src/utils/sdkCallbacks.ts | 47 +++++++++++--------- 4 files changed, 99 insertions(+), 40 deletions(-) diff --git a/src/app/components/SdkTab/index.tsx b/src/app/components/SdkTab/index.tsx index 1b786c4..3d50446 100644 --- a/src/app/components/SdkTab/index.tsx +++ b/src/app/components/SdkTab/index.tsx @@ -71,8 +71,12 @@ export default function SdkTab() { ? "None Found" : trackingCallbackIssues ? "Found (issues)" - : trackingCallbackParams - ? `Found (${trackingCallbackParams.length} params)` + : // A rest param stands in for any arity, so a count would mislead + trackingCallbackParams?.length && + !trackingCallbackParams.some((p) => p.startsWith("...")) + ? `Found (${trackingCallbackParams.length} param${ + trackingCallbackParams.length === 1 ? "" : "s" + })` : "Found"; const trackingCallbackStatusColor = !hasTrackingCallback ? "red" diff --git a/src/content_script/embed_script.ts b/src/content_script/embed_script.ts index 8ceea0e..7482de8 100644 --- a/src/content_script/embed_script.ts +++ b/src/content_script/embed_script.ts @@ -7,6 +7,7 @@ import type { Options, Result, TrackingCallback, + TrackingUserContext, } from "@growthbook/growthbook"; import type { ErrorMessage, SDKHealthCheckResult } from "devtools"; import { Attributes } from "@growthbook/growthbook"; @@ -459,8 +460,8 @@ function subscribeToSdkChanges( const patchedCallBack = ( experiment: Experiment, result: Result, - // SDK 1.7+ passes a third userContext arg - forward it or it's lost - user?: Parameters[2], + // SDK 1.7+ passes a third userContext arg + user?: TrackingUserContext, ) => { if (!hasSdkLogSupport) { gb.logs!.push({ @@ -471,12 +472,15 @@ function subscribeToSdkChanges( }); } if ("isNoopCallback" in callback && callback.isNoopCallback) { + // Keep `user` - fireDeferredTrackingCalls replays these as + // trackingCallback(call.experiment, call.result, call.user) gb.setDeferredTrackingCalls?.([ ...gb.getDeferredTrackingCalls(), - { experiment, result }, + { experiment, result, user }, ]); } - callback(experiment, result, user); + // Returned so the SDK can still await an async trackingCallback + return callback(experiment, result, user); }; if ("isNoopCallback" in callback && callback.isNoopCallback) { patchedCallBack.isNoopCallback = true; @@ -508,9 +512,6 @@ function subscribeToSdkChanges( }); } if (typeof onFeatureUsage === "function") { - // Only ever called with 2 args - onFeatureUsage lives in the SDK's user - // context, and core.ts calls those as cb(key, result). The 3-arg form is - // for GrowthBookClient's global context, which DevTools doesn't patch. onFeatureUsage(key, result); } }; diff --git a/src/utils/sdkCallbacks.test.ts b/src/utils/sdkCallbacks.test.ts index 2141ea3..bb46107 100644 --- a/src/utils/sdkCallbacks.test.ts +++ b/src/utils/sdkCallbacks.test.ts @@ -4,6 +4,10 @@ import { trackingCallbackParamsAreValid, } from "./sdkCallbacks"; +function getDefault() { + return undefined; +} + describe("parseCallbackParams", () => { it("parses arrow functions", () => { expect(parseCallbackParams((experiment, result) => {})).toEqual([ @@ -19,23 +23,64 @@ describe("parseCallbackParams", () => { }); it("parses function expressions", () => { - expect( - parseCallbackParams(function (experiment: any, result: any) {}), - ).toEqual(["experiment", "result"]); + expect(parseCallbackParams(function (experiment, result) {})).toEqual([ + "experiment", + "result", + ]); }); it("parses async functions", () => { - expect( - parseCallbackParams(async function (experiment: any, result: any) {}), - ).toEqual(["experiment", "result"]); + expect(parseCallbackParams(async function (experiment, result) {})).toEqual( + ["experiment", "result"], + ); }); it("parses a single unparenthesized arrow param", () => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const cb = (experiment: any) => ({ key: experiment.key }); + // Built at runtime - a type annotation forces parens, which would silently + // take the paren-walking path instead of the bare-arrow branch + const cb = Function("return experiment => ({ key: experiment.key })")(); + expect(cb.toString()).toMatch(/^experiment\s*=>/); expect(parseCallbackParams(cb)).toEqual(["experiment"]); }); + it("does not read a bare arrow's body as its params", () => { + const cb = Function( + 'return e => window.gtag("event", "x", { id: e.key })', + )(); + expect(parseCallbackParams(cb)).toEqual(["e"]); + }); + + it("does not split on a comma inside a string default", () => { + expect( + parseCallbackParams((experiment, result, sep = ",") => [ + experiment, + result, + sep, + ]), + ).toEqual(["experiment", "result", 'sep = ","']); + }); + + it("does not desync on a bracket inside a string default", () => { + expect(parseCallbackParams((a, b = "){[") => [a, b])).toEqual([ + "a", + 'b = "){["', + ]); + }); + + it("returns undefined when toString returns a non-string", () => { + const cb: any = () => {}; + cb.toString = () => 12345; + expect(parseCallbackParams(cb)).toBeUndefined(); + }); + + it("returns undefined when toString throws", () => { + const cb: any = () => {}; + cb.toString = () => { + throw new Error("nope"); + }; + expect(parseCallbackParams(cb)).toBeUndefined(); + }); + it("does not truncate at a destructured param", () => { expect( parseCallbackParams( @@ -85,7 +130,11 @@ describe("trackingCallbackParamsAreValid", () => { it("rejects too few params", () => { expect(trackingCallbackParamsAreValid(["experiment"])).toBe(false); - expect(trackingCallbackParamsAreValid([])).toBe(false); + }); + + it("accepts a zero-arity forwarding wrapper rather than warning", () => { + // `function () { cb.apply(this, arguments) }` parses to [] and works fine + expect(trackingCallbackParamsAreValid([])).toBe(true); }); it("rejects too many params", () => { @@ -118,5 +167,3 @@ describe("hasTrackingCallbackIssues", () => { ).toBe(true); }); }); - -declare function getDefault(): any; diff --git a/src/utils/sdkCallbacks.ts b/src/utils/sdkCallbacks.ts index c631551..81886af 100644 --- a/src/utils/sdkCallbacks.ts +++ b/src/utils/sdkCallbacks.ts @@ -1,20 +1,14 @@ -// Helpers for inspecting the callbacks a page's SDK was initialized with. -// Shared by the injected embed script (which reads them off the SDK context), -// the background worker (icon status), and the SDK tab UI. +import type { SDKHealthCheckResult } from "devtools"; -// A trackingCallback may be written as either `(experiment, result)` or -// `(experiment, result, userContext)` - the SDK types allow both. -const VALID_TRACKING_CALLBACK_PARAM_COUNTS = [2, 3]; - -// Pull the parameter names off a function's source. Returns undefined when we -// can't tell (native/bound functions, minified oddities) so callers can avoid -// warning about something they didn't actually detect. +// Pull the parameter names off a function's source. Returns undefined when we can't tell, eg native or bound functions export function parseCallbackParams( callback: (...args: any[]) => any, ): string[] | undefined { let src: string; try { + // A page can shadow toString, so it may throw or return a non-string src = callback.toString(); + if (typeof src !== "string") return undefined; } catch (e) { return undefined; } @@ -27,13 +21,25 @@ export function parseCallbackParams( const open = src.indexOf("("); if (open === -1) return undefined; - // Walk to the matching close paren, tracking nesting so that default values - // and destructured params don't cut the list short. + // Walk to the matching close paren so defaults and destructured params don't cut the list short const params: string[] = []; let current = ""; let depth = 0; + let quote: string | null = null; for (let i = open; i < src.length; i++) { const char = src[i]; + // Brackets and commas inside a string default aren't structure + if (quote) { + current += char; + if (char === "\\") current += src[++i] ?? ""; + else if (char === quote) quote = null; + continue; + } + if (char === '"' || char === "'" || char === "`") { + quote = char; + current += char; + continue; + } if (char === "(" || char === "[" || char === "{") { depth++; if (depth === 1) continue; @@ -50,27 +56,28 @@ export function parseCallbackParams( } current += char; } - // Never found the closing paren return undefined; } +// A trackingCallback may be written as `(experiment, result)` or `(experiment, result, userContext)` export function trackingCallbackParamsAreValid( params: string[] | undefined, ): boolean { - // Couldn't determine the params - don't claim there's a problem if (!params) return true; - // Rest params (`...args`) can stand in for any number of arguments + // Declaring nothing is indistinguishable from an arity-erasing forwarding + // wrapper (`function () { cb.apply(this, arguments) }`), which works fine + if (params.length === 0) return true; if (params.some((param) => param.startsWith("..."))) return true; - return VALID_TRACKING_CALLBACK_PARAM_COUNTS.includes(params.length); + return params.length === 2 || params.length === 3; } export function hasTrackingCallbackIssues({ hasTrackingCallback, trackingCallbackParams, -}: { - hasTrackingCallback?: boolean; - trackingCallbackParams?: string[]; -}): boolean { +}: Pick< + SDKHealthCheckResult, + "hasTrackingCallback" | "trackingCallbackParams" +>): boolean { if (!hasTrackingCallback) return false; return !trackingCallbackParamsAreValid(trackingCallbackParams); } From 6afcd46949650be0caf718ae46f2ff0dfe248f29 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 15:04:47 -0700 Subject: [PATCH 08/34] Tidy up branch: drop over-defensive guards and verbose comments - parseCallbackParams no longer wraps toString in try/catch with a typeof guard; main never did and callers pass a real function - trim multi-line comments down to one line each - flatten the nested ternary behind the SDK tab row label - straighten out the trackingCallback panel branches so each renders a whole sentence instead of splicing a conditional before the period - drop the any casts from the tests --- src/app/components/SdkTab/SdkItemPanel.tsx | 33 ++++++++++------------ src/app/components/SdkTab/index.tsx | 14 +++++---- src/content_script/embed_script.ts | 4 +-- src/utils/sdkCallbacks.test.ts | 19 ++----------- src/utils/sdkCallbacks.ts | 12 ++------ 5 files changed, 28 insertions(+), 54 deletions(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index 6748ba5..4bdeeb7 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -538,32 +538,29 @@ function trackingCallbackPanel({ trackingCallback. You will need to add one to track experiment exposure to your data warehouse. - ) : trackingCallbackParamsAreValid(trackingCallbackParams) ? ( - <> - The SDK is using a{" "} - trackingCallback - {trackingCallbackParams ? ( - <> - {" "} - with {trackingCallbackParams.length} param - {trackingCallbackParams.length === 1 ? "" : "s"}:{" "} - ({trackingCallbackParams.join(", ")}) - - ) : null} - . - - ) : ( + ) : !trackingCallbackParamsAreValid(trackingCallbackParams) ? ( <> The SDK is using a{" "} trackingCallback with{" "} - - {trackingCallbackParams?.length ?? 0} - {" "} + {trackingCallbackParams?.length}{" "} param{trackingCallbackParams?.length === 1 ? "" : "s"} instead of{" "} (experiment, result) or{" "} (experiment, result, userContext). Please check your implementation. + ) : trackingCallbackParams?.length ? ( + <> + The SDK is using a{" "} + trackingCallback with{" "} + {trackingCallbackParams.length} param + {trackingCallbackParams.length === 1 ? "" : "s"}:{" "} + ({trackingCallbackParams.join(", ")}). + + ) : ( + <> + The SDK is using a{" "} + trackingCallback. + )} ); diff --git a/src/app/components/SdkTab/index.tsx b/src/app/components/SdkTab/index.tsx index 3d50446..1429470 100644 --- a/src/app/components/SdkTab/index.tsx +++ b/src/app/components/SdkTab/index.tsx @@ -67,16 +67,18 @@ export default function SdkTab() { hasTrackingCallback, trackingCallbackParams, }); + // A rest param stands in for any arity, so a count would mislead + const trackingCallbackParamCount = trackingCallbackParams?.some((p) => + p.startsWith("..."), + ) + ? undefined + : trackingCallbackParams?.length; const trackingCallbackStatus = !hasTrackingCallback ? "None Found" : trackingCallbackIssues ? "Found (issues)" - : // A rest param stands in for any arity, so a count would mislead - trackingCallbackParams?.length && - !trackingCallbackParams.some((p) => p.startsWith("...")) - ? `Found (${trackingCallbackParams.length} param${ - trackingCallbackParams.length === 1 ? "" : "s" - })` + : trackingCallbackParamCount + ? `Found (${trackingCallbackParamCount} param${trackingCallbackParamCount === 1 ? "" : "s"})` : "Found"; const trackingCallbackStatusColor = !hasTrackingCallback ? "red" diff --git a/src/content_script/embed_script.ts b/src/content_script/embed_script.ts index 7482de8..6574d9c 100644 --- a/src/content_script/embed_script.ts +++ b/src/content_script/embed_script.ts @@ -472,14 +472,12 @@ function subscribeToSdkChanges( }); } if ("isNoopCallback" in callback && callback.isNoopCallback) { - // Keep `user` - fireDeferredTrackingCalls replays these as - // trackingCallback(call.experiment, call.result, call.user) + // fireDeferredTrackingCalls replays these with call.user gb.setDeferredTrackingCalls?.([ ...gb.getDeferredTrackingCalls(), { experiment, result, user }, ]); } - // Returned so the SDK can still await an async trackingCallback return callback(experiment, result, user); }; if ("isNoopCallback" in callback && callback.isNoopCallback) { diff --git a/src/utils/sdkCallbacks.test.ts b/src/utils/sdkCallbacks.test.ts index bb46107..edcd785 100644 --- a/src/utils/sdkCallbacks.test.ts +++ b/src/utils/sdkCallbacks.test.ts @@ -67,20 +67,6 @@ describe("parseCallbackParams", () => { ]); }); - it("returns undefined when toString returns a non-string", () => { - const cb: any = () => {}; - cb.toString = () => 12345; - expect(parseCallbackParams(cb)).toBeUndefined(); - }); - - it("returns undefined when toString throws", () => { - const cb: any = () => {}; - cb.toString = () => { - throw new Error("nope"); - }; - expect(parseCallbackParams(cb)).toBeUndefined(); - }); - it("does not truncate at a destructured param", () => { expect( parseCallbackParams( @@ -103,9 +89,8 @@ describe("parseCallbackParams", () => { expect(parseCallbackParams(() => {})).toEqual([]); }); - it("returns undefined for native/bound functions", () => { - const bound = ((a: any, b: any) => {}).bind(null); - expect(parseCallbackParams(bound)).toBeUndefined(); + it("returns undefined for native functions", () => { + expect(parseCallbackParams(Math.max)).toBeUndefined(); }); }); diff --git a/src/utils/sdkCallbacks.ts b/src/utils/sdkCallbacks.ts index 81886af..0ae0e8b 100644 --- a/src/utils/sdkCallbacks.ts +++ b/src/utils/sdkCallbacks.ts @@ -4,14 +4,7 @@ import type { SDKHealthCheckResult } from "devtools"; export function parseCallbackParams( callback: (...args: any[]) => any, ): string[] | undefined { - let src: string; - try { - // A page can shadow toString, so it may throw or return a non-string - src = callback.toString(); - if (typeof src !== "string") return undefined; - } catch (e) { - return undefined; - } + const src = callback.toString(); if (src.includes("[native code]")) return undefined; // Single-param arrow function without parens, eg `experiment => ...` @@ -64,8 +57,7 @@ export function trackingCallbackParamsAreValid( params: string[] | undefined, ): boolean { if (!params) return true; - // Declaring nothing is indistinguishable from an arity-erasing forwarding - // wrapper (`function () { cb.apply(this, arguments) }`), which works fine + // Zero params means a forwarding wrapper that reads `arguments` instead if (params.length === 0) return true; if (params.some((param) => param.startsWith("..."))) return true; return params.length === 2 || params.length === 3; From dbc570791b307f31195887f21a52093119c5d954 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 15:40:33 -0700 Subject: [PATCH 09/34] Match the trackingCallback param count to the SDK version 1.7.0 is where the SDK started calling the instance-level callback as (experiment, result, userContext); through 1.6.5 it only ever passed two args. So the expected arity is exact in both directions - two params on 1.7.0+ silently drops the userContext payload, and three on anything older leaves that param permanently undefined. Both now read as issues, with a panel message naming the version and what is lost. Stays lenient when the page's SDK version is unknown. --- src/app/components/SdkTab/SdkItemPanel.tsx | 34 ++++++++++++++++++++-- src/app/components/SdkTab/index.tsx | 1 + src/utils/sdkCallbacks.test.ts | 28 ++++++++++++++++++ src/utils/sdkCallbacks.ts | 27 ++++++++++++++--- 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index 4bdeeb7..770f286 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -23,7 +23,11 @@ import { useResponsiveContext } from "@/app/hooks/useResponsive"; import { SdkItem } from "./index"; import useSdkData from "@/app/hooks/useSdkData"; import { SDKHealthCheckResult } from "devtools"; -import { trackingCallbackParamsAreValid } from "@/utils/sdkCallbacks"; +import { + expectsUserContextParam, + trackingCallbackParamsAreValid, + USER_CONTEXT_SDK_VERSION, +} from "@/utils/sdkCallbacks"; import { getActiveTabId } from "@/app/hooks/useTabState"; import { paddedVersionString } from "@growthbook/growthbook"; @@ -529,7 +533,14 @@ function versionPanel({ function trackingCallbackPanel({ trackingCallbackParams, hasTrackingCallback, + version, }: SDKHealthCheckResult) { + const missingUserContext = + expectsUserContextParam(version) && trackingCallbackParams?.length === 2; + const unusedUserContext = + version && + !expectsUserContextParam(version) && + trackingCallbackParams?.length === 3; return ( {!hasTrackingCallback ? ( @@ -538,7 +549,26 @@ function trackingCallbackPanel({ trackingCallback. You will need to add one to track experiment exposure to your data warehouse. - ) : !trackingCallbackParamsAreValid(trackingCallbackParams) ? ( + ) : missingUserContext ? ( + <> + The SDK is using a{" "} + trackingCallback with{" "} + 2 params, but SDK {version} calls + it as (experiment, result, userContext). Without the + third param you lose the user’s attributes, and the tracked + experiment and feature keys that come with it. Add{" "} + userContext to your implementation. + + ) : unusedUserContext ? ( + <> + The SDK is using a{" "} + trackingCallback with{" "} + 3 params, but SDK {version} only + calls it as (experiment, result), so{" "} + userContext is always undefined. Upgrade to{" "} + {USER_CONTEXT_SDK_VERSION} or later to receive it. + + ) : !trackingCallbackParamsAreValid(trackingCallbackParams, version) ? ( <> The SDK is using a{" "} trackingCallback with{" "} diff --git a/src/app/components/SdkTab/index.tsx b/src/app/components/SdkTab/index.tsx index 1429470..b197122 100644 --- a/src/app/components/SdkTab/index.tsx +++ b/src/app/components/SdkTab/index.tsx @@ -66,6 +66,7 @@ export default function SdkTab() { const trackingCallbackIssues = hasTrackingCallbackIssues({ hasTrackingCallback, trackingCallbackParams, + version, }); // A rest param stands in for any arity, so a count would mislead const trackingCallbackParamCount = trackingCallbackParams?.some((p) => diff --git a/src/utils/sdkCallbacks.test.ts b/src/utils/sdkCallbacks.test.ts index edcd785..412fef0 100644 --- a/src/utils/sdkCallbacks.test.ts +++ b/src/utils/sdkCallbacks.test.ts @@ -125,6 +125,34 @@ describe("trackingCallbackParamsAreValid", () => { it("rejects too many params", () => { expect(trackingCallbackParamsAreValid(["a", "b", "c", "d"])).toBe(false); }); + + it("rejects 2 params once the SDK passes a userContext", () => { + expect( + trackingCallbackParamsAreValid(["experiment", "result"], "1.7.0"), + ).toBe(false); + expect( + trackingCallbackParamsAreValid(["experiment", "result"], "1.8.2"), + ).toBe(false); + }); + + it("accepts 2 params on SDKs that never pass a userContext", () => { + expect( + trackingCallbackParamsAreValid(["experiment", "result"], "1.6.5"), + ).toBe(true); + expect( + trackingCallbackParamsAreValid(["experiment", "result"], "0.36.0"), + ).toBe(true); + }); + + it("rejects 3 params on SDKs that never pass a userContext", () => { + const params = ["experiment", "result", "userContext"]; + expect(trackingCallbackParamsAreValid(params, "1.6.5")).toBe(false); + expect(trackingCallbackParamsAreValid(params, "1.7.0")).toBe(true); + }); + + it("stays lenient when the version is unknown", () => { + expect(trackingCallbackParamsAreValid(["experiment", "result"])).toBe(true); + }); }); describe("hasTrackingCallbackIssues", () => { diff --git a/src/utils/sdkCallbacks.ts b/src/utils/sdkCallbacks.ts index 0ae0e8b..341c931 100644 --- a/src/utils/sdkCallbacks.ts +++ b/src/utils/sdkCallbacks.ts @@ -1,3 +1,4 @@ +import { paddedVersionString } from "@growthbook/growthbook"; import type { SDKHealthCheckResult } from "devtools"; // Pull the parameter names off a function's source. Returns undefined when we can't tell, eg native or bound functions @@ -52,24 +53,42 @@ export function parseCallbackParams( return undefined; } -// A trackingCallback may be written as `(experiment, result)` or `(experiment, result, userContext)` +// Before this the SDK only ever called trackingCallback(experiment, result), so +// a third param was dead weight. From here it passes the userContext too +export const USER_CONTEXT_SDK_VERSION = "1.7.0"; + +export function expectsUserContextParam(version?: string): boolean { + if (!version) return false; + return ( + paddedVersionString(version) >= + paddedVersionString(USER_CONTEXT_SDK_VERSION) + ); +} + +// A trackingCallback takes `(experiment, result)`, or `(experiment, result, userContext)` on 1.7.0+ export function trackingCallbackParamsAreValid( params: string[] | undefined, + version?: string, ): boolean { if (!params) return true; // Zero params means a forwarding wrapper that reads `arguments` instead if (params.length === 0) return true; if (params.some((param) => param.startsWith("..."))) return true; - return params.length === 2 || params.length === 3; + // Without a version to compare against, either shape may be right + if (!version) return params.length === 2 || params.length === 3; + // Omitting userContext drops what it carries, declaring it on an SDK that + // never passes one leaves it permanently undefined + return params.length === (expectsUserContextParam(version) ? 3 : 2); } export function hasTrackingCallbackIssues({ hasTrackingCallback, trackingCallbackParams, + version, }: Pick< SDKHealthCheckResult, - "hasTrackingCallback" | "trackingCallbackParams" + "hasTrackingCallback" | "trackingCallbackParams" | "version" >): boolean { if (!hasTrackingCallback) return false; - return !trackingCallbackParamsAreValid(trackingCallbackParams); + return !trackingCallbackParamsAreValid(trackingCallbackParams, version); } From 5350b41487139796d5090558259e773112f9ac07 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 15:50:19 -0700 Subject: [PATCH 10/34] Soften the userContext panel copy The third param is not what carries the user's attributes - omitting it just means missing newer features like contextual bandits. --- src/app/components/SdkTab/SdkItemPanel.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index 770f286..b4f65a5 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -555,8 +555,7 @@ function trackingCallbackPanel({ trackingCallback with{" "} 2 params, but SDK {version} calls it as (experiment, result, userContext). Without the - third param you lose the user’s attributes, and the tracked - experiment and feature keys that come with it. Add{" "} + third param you will not get some newer features. Add{" "} userContext to your implementation. ) : unusedUserContext ? ( @@ -566,7 +565,8 @@ function trackingCallbackPanel({ 3 params, but SDK {version} only calls it as (experiment, result), so{" "} userContext is always undefined. Upgrade to{" "} - {USER_CONTEXT_SDK_VERSION} or later to receive it. + {USER_CONTEXT_SDK_VERSION} or later to use the newer features that + rely on it. ) : !trackingCallbackParamsAreValid(trackingCallbackParams, version) ? ( <> From 0aff5c5a1b3af6d981d931e91210544b1a4c7341 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 15:53:35 -0700 Subject: [PATCH 11/34] Trim the userContext panel copy --- src/app/components/SdkTab/SdkItemPanel.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index b4f65a5..dc4b24d 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -553,20 +553,16 @@ function trackingCallbackPanel({ <> The SDK is using a{" "} trackingCallback with{" "} - 2 params, but SDK {version} calls - it as (experiment, result, userContext). Without the - third param you will not get some newer features. Add{" "} - userContext to your implementation. + 2 params. Add a third{" "} + userContext param to use newer features in SDK {version}. ) : unusedUserContext ? ( <> The SDK is using a{" "} trackingCallback with{" "} - 3 params, but SDK {version} only - calls it as (experiment, result), so{" "} - userContext is always undefined. Upgrade to{" "} - {USER_CONTEXT_SDK_VERSION} or later to use the newer features that - rely on it. + 3 params, but SDK {version} never + passes userContext. Upgrade to {USER_CONTEXT_SDK_VERSION}{" "} + or later to use it. ) : !trackingCallbackParamsAreValid(trackingCallbackParams, version) ? ( <> From e66f80256fa669bebffa0024ed34b45b3fc5744f Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 15:55:55 -0700 Subject: [PATCH 12/34] Drop the version number from the userContext hint --- src/app/components/SdkTab/SdkItemPanel.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/components/SdkTab/SdkItemPanel.tsx b/src/app/components/SdkTab/SdkItemPanel.tsx index dc4b24d..71b478d 100644 --- a/src/app/components/SdkTab/SdkItemPanel.tsx +++ b/src/app/components/SdkTab/SdkItemPanel.tsx @@ -554,7 +554,7 @@ function trackingCallbackPanel({ The SDK is using a{" "} trackingCallback with{" "} 2 params. Add a third{" "} - userContext param to use newer features in SDK {version}. + userContext param to use newer features. ) : unusedUserContext ? ( <> From 1ba5ab80e04ba98a24e78f28ab87acb659142441 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Wed, 26 Aug 2026 16:52:25 -0700 Subject: [PATCH 13/34] Surface contextual bandits in the Experiments tab Contextual bandit rules keep their variations under contextualVariations so pre-1.7 SDKs skip them, but getFeatureExperiments gated on rule.variations - so they never reached the Experiments tab at all. Read either field, then mark and explain the ones that are bandits. - "Contextual Bandit" badge on the experiment row, the detail header, and matching exposure entries in Event Logs - detail section with the weights actually applied to this user, as percentages to match Rule.tsx, and the attributes the matched context tested - both collapsible, since a bandit can have many of either - calls out the three states that otherwise look like a healthy bandit: ref missing from the payload, no matching context, and not bucketed Weights come from the feature's experimentResult: the SDK applies bandit logic while evaluating the rule, so gb.run() and the raw rule never carry them. --- src/app/components/ContextualBanditDetail.tsx | 178 ++++++++++++++++++ src/app/components/ExperimentDetail.tsx | 34 +++- src/app/components/ExperimentsTab.tsx | 23 ++- src/app/components/LogsList.tsx | 19 +- src/app/utils/logs.ts | 3 + src/utils/contextualBandits.test.ts | 129 +++++++++++++ src/utils/contextualBandits.ts | 93 +++++++++ 7 files changed, 471 insertions(+), 8 deletions(-) create mode 100644 src/app/components/ContextualBanditDetail.tsx create mode 100644 src/utils/contextualBandits.test.ts create mode 100644 src/utils/contextualBandits.ts diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx new file mode 100644 index 0000000..3f14e4a --- /dev/null +++ b/src/app/components/ContextualBanditDetail.tsx @@ -0,0 +1,178 @@ +import React from "react"; +import { Badge, Text } from "@radix-ui/themes"; +import * as Accordion from "@radix-ui/react-accordion"; +import { PiCaretRightFill } from "react-icons/pi"; +import { Experiment } from "@growthbook/growthbook"; +import useTabState from "@/app/hooks/useTabState"; +import useSdkData from "@/app/hooks/useSdkData"; +import useGBSandboxEval from "@/app/hooks/useGBSandboxEval"; +import { + ContextualBanditDefinitions, + getMatchedContextAttributes, + usedFallbackWeights, +} from "@/utils/contextualBandits"; +import ValueField from "@/app/components/ValueField"; + +// Matches how experiment weights are shown elsewhere, eg Rule.tsx +function formatWeight(weight: number) { + return Math.round(weight * 1000) / 10 + "%"; +} + +export default function ContextualBanditDetail({ + experiment, + variationNames, + forcedVariation, +}: { + experiment: Experiment & { contextualBanditRef?: string }; + variationNames: string[]; + forcedVariation?: number; +}) { + const sdkData = useSdkData(); + const [attributes] = useTabState>("attributes", {}); + const { evaluatedFeatures } = useGBSandboxEval(); + + const banditRef = experiment.contextualBanditRef; + const definitions = sdkData?.payload?.contextualBandits as + | ContextualBanditDefinitions + | undefined; + const definition = banditRef ? definitions?.[banditRef] : undefined; + + // The SDK only applies bandit weights while evaluating the feature rule, so + // they land on the feature's experimentResult - never on gb.run() or the raw rule + const banditResult = Object.values(evaluatedFeatures) + .map((f) => f?.result?.experimentResult) + .find( + (r) => r?.key === experiment.key && r?.variationWeights !== undefined, + ); + const cb = banditResult + ? { + leafId: banditResult.leafId ?? -1, + variationWeights: banditResult.variationWeights ?? [], + banditVersion: banditResult.banditVersion, + } + : undefined; + const weights = cb?.variationWeights; + const isFallback = usedFallbackWeights(cb); + const context = getMatchedContextAttributes(definition, cb, attributes || {}); + const contextKeys = Object.keys(context || {}); + const isForced = forcedVariation !== undefined; + + return ( + <> +
Contextual Bandit
+ + {!banditRef ? null : !definition ? ( + + The bandit {banditRef} is not in the SDK payload, so + fallback weights were used. + + ) : !cb ? ( + + This user was not bucketed into the experiment, so no bandit weights + were applied. + + ) : ( + <> + {isFallback ? ( + + No context matched this user, so fallback weights were used. + + ) : null} + {isForced ? ( + + A forced variation is active, so these weights are overridden and + not what the bandit would serve. + + ) : null} + + + {weights?.length ? ( + + + + + Weights for this context + + + {weights.length} variation{weights.length === 1 ? "" : "s"} + + + +
+ {weights.map((weight, i) => ( +
+
+ {i} · {variationNames[i] ?? `Variation ${i}`} +
+
+
+
+
+ {formatWeight(weight)} +
+
+ ))} +
+ + + ) : null} + + {contextKeys.length ? ( + + + + + Context used for this user + + + {contextKeys.length} attribute + {contextKeys.length === 1 ? "" : "s"} + + + +
+ {contextKeys.map((key) => ( +
+
+ {key} +
+ +
+ ))} +
+ + + ) : null} + + + )} + + ); +} + +export function ContextualBanditBadge() { + return ( + + Contextual Bandit + + ); +} diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index 6d69df5..f53e356 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -38,7 +38,11 @@ import useGlobalState from "@/app/hooks/useGlobalState"; import { APP_ORIGIN, CLOUD_APP_ORIGIN } from "@/app/components/Settings"; import useTabState from "@/app/hooks/useTabState"; import { SelectedExperiment } from "@/app/components/ExperimentsTab"; -import { AutoExperimentVariation, FeatureDefinition, isURLTargeted } from "@growthbook/growthbook"; +import { + AutoExperimentVariation, + FeatureDefinition, + isURLTargeted, +} from "@growthbook/growthbook"; import clsx from "clsx"; import DebugLogger, { DebugLogAccordion } from "@/app/components/DebugLogger"; import { TbEyeSearch } from "react-icons/tb"; @@ -47,6 +51,9 @@ import { EvaluationSourceViewer, } from "@/app/components/FeatureDetail"; import { LogUnionWithSource } from "@/app/utils/logs"; +import ContextualBanditDetail, { + ContextualBanditBadge, +} from "@/app/components/ContextualBanditDetail"; export default function ExperimentDetail({ selectedEid, @@ -139,8 +146,8 @@ export default function ExperimentDetail({ const expFeatures = selectedExperiment?.experiment?.features ?? []; for (const fid of expFeatures) { const rule0 = (features[fid]?.rules ?? [])[0] as any; - const holdoutFid = rule0?.parentConditions?.find( - (pc: { id?: string }) => pc.id?.startsWith("$holdout:"), + const holdoutFid = rule0?.parentConditions?.find((pc: { id?: string }) => + pc.id?.startsWith("$holdout:"), )?.id; if (!holdoutFid) continue; const holdoutExpKey = (features[holdoutFid]?.rules?.[0] as any)?.key; @@ -257,6 +264,11 @@ export default function ExperimentDetail({ {selectedExperiment?.experiment ? getExperimentDisplayName(selectedExperiment.experiment) : selectedEid} + {types?.contextualBandit ? ( + + + + ) : null} ) : null} + {types?.contextualBandit && selectedExperiment?.experiment ? ( + undefined) ?? + [] + ).map((m, i) => m?.name ?? `Variation ${i}`)} + forcedVariation={ + selectedEid && selectedEid in forcedVariations + ? forcedVariations[selectedEid] + : undefined + } + /> + ) : null} +
Implementation {(types?.redirect ? 1 : 0) + diff --git a/src/app/components/ExperimentsTab.tsx b/src/app/components/ExperimentsTab.tsx index 7911a50..c16f390 100644 --- a/src/app/components/ExperimentsTab.tsx +++ b/src/app/components/ExperimentsTab.tsx @@ -8,7 +8,7 @@ import useTabState from "../hooks/useTabState"; import useGBSandboxEval, { EvaluatedExperiment, } from "@/app/hooks/useGBSandboxEval"; -import { Link, Switch, Tooltip } from "@radix-ui/themes"; +import { Badge, Link, Switch, Tooltip } from "@radix-ui/themes"; import { PiDesktopFill, PiFlagFill, PiLinkBold, PiXBold } from "react-icons/pi"; import clsx from "clsx"; import { MW, NAV_H } from "@/app"; @@ -24,9 +24,11 @@ import FeatureExperimentStatusIcon from "@/app/components/FeatureExperimentStatu import { useResponsiveContext } from "../hooks/useResponsive"; import { TbEyeSearch } from "react-icons/tb"; import { LogUnionWithSource } from "@/app/utils/logs"; +import { isContextualBandit, ruleVariations } from "@/utils/contextualBandits"; export type ExperimentWithFeatures = (AutoExperiment | Experiment) & { features?: string[]; + contextualBanditRef?: string; featureTypes?: Record; isDraft?: boolean; isInactive?: boolean; @@ -51,7 +53,6 @@ export default function ExperimentsTab() { // de-dupe const allExperiments = useMemo(() => { - const merged: ExperimentWithFeatures[] = [ ...experiments, ...featureExperiments, @@ -315,6 +316,11 @@ export default function ExperimentsTab() { > {types ? (
+ {types.contextualBandit ? ( + + Contextual Bandit + + ) : null} {types.redirect ? ( @@ -402,7 +408,12 @@ export type SelectedExperiment = { eid: string; experiment: ExperimentWithFeatures; meta?: any; - types: { features?: string[]; redirect?: boolean; visual?: boolean }; + types: { + features?: string[]; + redirect?: boolean; + visual?: boolean; + contextualBandit?: boolean; + }; evaluatedExperiment?: EvaluatedExperiment; isForced: boolean; }; @@ -455,6 +466,7 @@ export function getExperimentTypes(experiment: ExperimentWithFeatures) { visual: experiment?.variations?.some( (v) => v?.domMutations?.length || v?.css || v?.js, ), + contextualBandit: isContextualBandit(experiment), }; } @@ -466,13 +478,16 @@ export function getFeatureExperiments( const feature = features[fid]; const details = getFeatureDetails({ fid, features }); for (const rule of feature.rules || []) { - if (rule.variations) { + const variations = ruleVariations(rule); + if (variations) { // @ts-ignore experiments.push({ key: rule.key ?? fid, features: [fid], featureTypes: { [fid]: details.valueType }, ...rule, + // Contextual bandit rules carry these under contextualVariations + variations, }); } } diff --git a/src/app/components/LogsList.tsx b/src/app/components/LogsList.tsx index 7da6f89..3589ce1 100644 --- a/src/app/components/LogsList.tsx +++ b/src/app/components/LogsList.tsx @@ -1,4 +1,12 @@ -import { Box, Checkbox, Flex, Link, Text, Tooltip } from "@radix-ui/themes"; +import { + Badge, + Box, + Checkbox, + Flex, + Link, + Text, + Tooltip, +} from "@radix-ui/themes"; import React, { ReactNode, useMemo, useState } from "react"; import useTabState from "../hooks/useTabState"; import { useSearch } from "../hooks/useSearch"; @@ -250,6 +258,15 @@ export default function LogsList({ )} > {evt.eventInfo} + {evt.isContextualBandit ? ( + + Contextual Bandit + + ) : null}
{!isResponsive && (
diff --git a/src/app/utils/logs.ts b/src/app/utils/logs.ts index 43fdc33..7cd8a88 100644 --- a/src/app/utils/logs.ts +++ b/src/app/utils/logs.ts @@ -4,6 +4,7 @@ export interface FlattenedLogEvent { logType: string; timestamp: string; eventInfo: string; + isContextualBandit?: boolean; details: Record; context: { source?: string; @@ -34,6 +35,8 @@ export function reshapeEventLog(evt: LogUnionWithSource): FlattenedLogEvent { logType: evt.logType, timestamp: evt.timestamp, eventInfo: evt.experiment.name || "", + // Bandit results only carry these when the user was bucketed in + isContextualBandit: evt.result?.variationWeights !== undefined, details: { experiment: evt.experiment, result: evt.result, diff --git a/src/utils/contextualBandits.test.ts b/src/utils/contextualBandits.test.ts new file mode 100644 index 0000000..f169866 --- /dev/null +++ b/src/utils/contextualBandits.test.ts @@ -0,0 +1,129 @@ +import { + conditionAttributeKeys, + getMatchedContextAttributes, + isContextualBandit, + ruleVariations, + usedFallbackWeights, +} from "./contextualBandits"; + +describe("ruleVariations", () => { + it("reads contextualVariations, which bandit rules use instead", () => { + expect(ruleVariations({ contextualVariations: ["a", "b", "c"] })).toEqual([ + "a", + "b", + "c", + ]); + }); + + it("falls back to variations for a normal experiment rule", () => { + expect(ruleVariations({ variations: ["on", "off"] })).toEqual([ + "on", + "off", + ]); + }); + + it("prefers contextualVariations when a rule somehow has both", () => { + expect( + ruleVariations({ variations: ["x"], contextualVariations: ["a", "b"] }), + ).toEqual(["a", "b"]); + }); + + it("returns undefined for a rule with neither", () => { + expect(ruleVariations({ force: true })).toBeUndefined(); + }); +}); + +describe("isContextualBandit", () => { + it("keys off contextualBanditRef", () => { + expect(isContextualBandit({ contextualBanditRef: "cb_x" })).toBe(true); + expect(isContextualBandit({})).toBe(false); + expect(isContextualBandit(undefined)).toBe(false); + }); +}); + +describe("usedFallbackWeights", () => { + it("treats leafId -1 as the fallback", () => { + expect(usedFallbackWeights({ leafId: -1, variationWeights: [1] })).toBe( + true, + ); + expect(usedFallbackWeights({ leafId: 0, variationWeights: [1] })).toBe( + false, + ); + expect(usedFallbackWeights(undefined)).toBe(false); + }); +}); + +describe("conditionAttributeKeys", () => { + it("reads plain attribute names", () => { + expect(conditionAttributeKeys({ country: "US", device: "mobile" })).toEqual( + ["country", "device"], + ); + }); + + it("descends through $and / $or wrappers", () => { + expect( + conditionAttributeKeys({ + $or: [{ country: "US" }, { $and: [{ plan: "pro" }, { seats: 5 }] }], + }), + ).toEqual(["country", "plan", "seats"]); + }); + + it("does not report operators as attributes", () => { + expect(conditionAttributeKeys({ age: { $gt: 18 } })).toEqual(["age"]); + }); +}); + +describe("getMatchedContextAttributes", () => { + const definition = { + banditVersion: 4, + contexts: [ + { + leafId: 0, + condition: { country: "US", device: "mobile" }, + weights: [0.12, 0.63, 0.25], + }, + { leafId: 1, condition: { country: "US" }, weights: [0.3, 0.4, 0.3] }, + ], + }; + const attributes = { id: "u1", country: "US", device: "mobile", plan: "pro" }; + + it("returns only the attributes the matched context tested", () => { + expect( + getMatchedContextAttributes( + definition, + { leafId: 0, variationWeights: [] }, + attributes, + ), + ).toEqual({ country: "US", device: "mobile" }); + }); + + it("narrows to the matched context, not every context", () => { + expect( + getMatchedContextAttributes( + definition, + { leafId: 1, variationWeights: [] }, + attributes, + ), + ).toEqual({ country: "US" }); + }); + + it("returns nothing when the weights came from the fallback", () => { + expect( + getMatchedContextAttributes( + definition, + { leafId: -1, variationWeights: [] }, + attributes, + ), + ).toBeUndefined(); + }); + + it("skips attributes the user does not have set", () => { + expect( + getMatchedContextAttributes( + definition, + { leafId: 0, variationWeights: [] }, + { country: "US" }, + ), + ).toEqual({ country: "US" }); + }); +}); diff --git a/src/utils/contextualBandits.ts b/src/utils/contextualBandits.ts new file mode 100644 index 0000000..faba345 --- /dev/null +++ b/src/utils/contextualBandits.ts @@ -0,0 +1,93 @@ +import type { Experiment, FeatureRule } from "@growthbook/growthbook"; + +// The SDK declares these but doesn't export them from its entry point +export type CBContext = { + leafId: number; + variationWeights: number[]; + banditVersion?: number; +}; +export type ContextualBanditDefinition = { + banditVersion?: number; + contexts: { + leafId: number; + condition: Record; + weights: number[]; + }[]; +}; +export type ContextualBanditDefinitions = Record< + string, + ContextualBanditDefinition +>; + +// A contextual bandit rule keeps its variations under `contextualVariations` so +// that pre-1.7 SDKs skip the rule instead of evaluating it with no weights +export function ruleVariations( + rule: FeatureRule, +): Experiment["variations"] | undefined { + // FeatureRule types these as T[], but an experiment needs at least two + return (rule.contextualVariations ?? rule.variations) as + | Experiment["variations"] + | undefined; +} + +export function isContextualBandit( + experiment: { contextualBanditRef?: string } | undefined, +): boolean { + return !!experiment?.contextualBanditRef; +} + +// The SDK only sets `contextualBandit` when the user was actually bucketed in, +// so its absence on a bandit experiment means the weights aren't live +export function getContextualBandit( + experiment: Experiment | undefined, +): CBContext | undefined { + return experiment?.contextualBandit; +} + +// leafId -1 means no context matched and the SDK fell back to aggregate weights +export const FALLBACK_LEAF_ID = -1; + +export function usedFallbackWeights(cb: CBContext | undefined): boolean { + return cb?.leafId === FALLBACK_LEAF_ID; +} + +// The attributes a bandit's matched context actually tested - this is the +// "contextual" part, and the only piece of the definition worth showing +export function getMatchedContextAttributes( + definition: ContextualBanditDefinition | undefined, + cb: CBContext | undefined, + attributes: Record, +): Record | undefined { + if (!definition || !cb || usedFallbackWeights(cb)) return undefined; + const context = definition.contexts.find((c) => c.leafId === cb.leafId); + if (!context) return undefined; + const used: Record = {}; + for (const key of conditionAttributeKeys(context.condition)) { + if (key in attributes) used[key] = attributes[key]; + } + return used; +} + +// Pull attribute names out of a mongo-style condition, skipping $and/$or/$not +// wrappers so nested conditions still report the attributes they test +export function conditionAttributeKeys( + condition: Record, +): string[] { + const keys = new Set(); + const walk = (node: unknown) => { + if (Array.isArray(node)) { + node.forEach(walk); + return; + } + if (!node || typeof node !== "object") return; + for (const [key, value] of Object.entries(node)) { + if (key.startsWith("$")) { + walk(value); + } else { + keys.add(key); + } + } + }; + walk(condition); + return [...keys]; +} From 51865d81318995b0e0564156317be60e22836dda Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Thu, 27 Aug 2026 10:41:21 -0700 Subject: [PATCH 14/34] Detect contextual bandits by contextualVariations, not the bandit ref A live payload marks a contextual bandit rule with contextualVariations alone: contextualBanditRef and the top-level contextualBandits block only appear once the bandit has trained contexts. Keying detection off the ref meant a real bandit rendered as an ordinary experiment. Detection now keys off contextualVariations, and the detail panel handles a bandit with no trained contexts by showing the rule's fallback weights and saying that is what every user gets. --- src/app/components/ContextualBanditDetail.tsx | 191 +++++++++--------- src/app/components/ExperimentsTab.tsx | 1 + src/utils/contextualBandits.ts | 11 +- src/utils/realPayload.test.ts | 41 ++++ 4 files changed, 148 insertions(+), 96 deletions(-) create mode 100644 src/utils/realPayload.test.ts diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 3f14e4a..4061024 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -51,7 +51,10 @@ export default function ContextualBanditDetail({ banditVersion: banditResult.banditVersion, } : undefined; - const weights = cb?.variationWeights; + // The rule's own weights, used when the payload has no trained contexts + const aggregateWeights = experiment.weights; + const weights = cb?.variationWeights ?? aggregateWeights; + const hasContextWeights = !!cb?.variationWeights; const isFallback = usedFallbackWeights(cb); const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); @@ -61,110 +64,110 @@ export default function ContextualBanditDetail({ <>
Contextual Bandit
- {!banditRef ? null : !definition ? ( + {banditRef && !definition ? ( The bandit {banditRef} is not in the SDK payload, so fallback weights were used. - ) : !cb ? ( + ) : !banditRef ? ( - This user was not bucketed into the experiment, so no bandit weights - were applied. + No trained contexts are in the SDK payload yet, so every user gets the + fallback weights below. - ) : ( - <> - {isFallback ? ( - - No context matched this user, so fallback weights were used. - - ) : null} - {isForced ? ( - - A forced variation is active, so these weights are overridden and - not what the bandit would serve. - - ) : null} + ) : null} - - {weights?.length ? ( - - - - - Weights for this context - - - {weights.length} variation{weights.length === 1 ? "" : "s"} - - - -
- {weights.map((weight, i) => ( + {isFallback ? ( + + No context matched this user, so fallback weights were used. + + ) : null} + {isForced ? ( + + A forced variation is active, so these weights are overridden and not + what the bandit would serve. + + ) : null} + + + {weights?.length ? ( + + + + + {hasContextWeights + ? "Weights for this context" + : "Fallback weights"} + + + {weights.length} variation{weights.length === 1 ? "" : "s"} + + + +
+ {weights.map((weight, i) => ( +
+
+ {i} · {variationNames[i] ?? `Variation ${i}`} +
+
-
- {i} · {variationNames[i] ?? `Variation ${i}`} -
-
-
-
-
- {formatWeight(weight)} -
-
- ))} + className="h-full rounded-full bg-violet-9" + style={{ width: `${weight * 100}%` }} + /> +
+
+ {formatWeight(weight)} +
- - - ) : null} + ))} +
+ + + ) : null} - {contextKeys.length ? ( - - - - - Context used for this user - - - {contextKeys.length} attribute - {contextKeys.length === 1 ? "" : "s"} - - - -
- {contextKeys.map((key) => ( -
-
- {key} -
- -
- ))} + {contextKeys.length ? ( + + + + + Context used for this user + + + {contextKeys.length} attribute + {contextKeys.length === 1 ? "" : "s"} + + + +
+ {contextKeys.map((key) => ( +
+
+ {key} +
+
- - - ) : null} - - - )} + ))} +
+ + + ) : null} + ); } diff --git a/src/app/components/ExperimentsTab.tsx b/src/app/components/ExperimentsTab.tsx index c16f390..23634d0 100644 --- a/src/app/components/ExperimentsTab.tsx +++ b/src/app/components/ExperimentsTab.tsx @@ -29,6 +29,7 @@ import { isContextualBandit, ruleVariations } from "@/utils/contextualBandits"; export type ExperimentWithFeatures = (AutoExperiment | Experiment) & { features?: string[]; contextualBanditRef?: string; + contextualVariations?: unknown[]; featureTypes?: Record; isDraft?: boolean; isInactive?: boolean; diff --git a/src/utils/contextualBandits.ts b/src/utils/contextualBandits.ts index faba345..777f65d 100644 --- a/src/utils/contextualBandits.ts +++ b/src/utils/contextualBandits.ts @@ -30,10 +30,17 @@ export function ruleVariations( | undefined; } +// `contextualVariations` is what marks the rule: the backend always emits it in +// place of `variations`, while `contextualBanditRef` and the payload's +// `contextualBandits` block only appear once the bandit has trained contexts export function isContextualBandit( - experiment: { contextualBanditRef?: string } | undefined, + experiment: + | { contextualVariations?: unknown[]; contextualBanditRef?: string } + | undefined, ): boolean { - return !!experiment?.contextualBanditRef; + return ( + !!experiment?.contextualVariations || !!experiment?.contextualBanditRef + ); } // The SDK only sets `contextualBandit` when the user was actually bucketed in, diff --git a/src/utils/realPayload.test.ts b/src/utils/realPayload.test.ts new file mode 100644 index 0000000..d899e7c --- /dev/null +++ b/src/utils/realPayload.test.ts @@ -0,0 +1,41 @@ +import { isContextualBandit, ruleVariations } from "./contextualBandits"; + +// The teresa-test-cb rule exactly as a live GrowthBook payload serves it: +// contextualVariations is present, but there is no contextualBanditRef and no +// top-level contextualBandits block until the bandit has trained contexts. +const realRule = { + id: "fr_19g6mms6mcr6p", + hashAttribute: "id", + seed: "d5212dbe-3cee-430b-9a43-75d5c7b13474", + hashVersion: 2, + disableStickyBucketing: true, + contextualVariations: [true, true, true, true, true], + weights: [0.2, 0.2, 0.2, 0.2, 0.2], + key: "teresa-test-cb", + meta: [{ key: "0" }, { key: "1" }, { key: "2" }, { key: "3" }, { key: "4" }], + phase: "0", +}; + +describe("a contextual bandit from a live payload", () => { + it("is detected without a contextualBanditRef", () => { + expect(isContextualBandit(realRule)).toBe(true); + }); + + it("reaches the experiment list via contextualVariations", () => { + expect(ruleVariations(realRule)).toHaveLength(5); + }); + + it("is still detected once the ref appears", () => { + expect( + isContextualBandit({ ...realRule, contextualBanditRef: "cb_x" }), + ).toBe(true); + }); + + it("does not flag an ordinary experiment rule", () => { + const ordinaryRule: { + variations: boolean[]; + contextualVariations?: unknown[]; + } = { variations: [true, false] }; + expect(isContextualBandit(ordinaryRule)).toBe(false); + }); +}); From 2774e6109ce23dc73adcd5986f0fca56da9cca25 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Thu, 27 Aug 2026 10:43:21 -0700 Subject: [PATCH 15/34] Move the contextual bandit section up the detail panel It sat just above Implementation, well below the fold, so a bandit read as an ordinary experiment on open. It now follows Current value, and its heading matches the label style of the sections around it. --- src/app/components/ContextualBanditDetail.tsx | 2 +- src/app/components/ExperimentDetail.tsx | 32 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 4061024..388122f 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -62,7 +62,7 @@ export default function ContextualBanditDetail({ return ( <> -
Contextual Bandit
+
Contextual Bandit
{banditRef && !definition ? ( diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index f53e356..cc1da66 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -438,6 +438,22 @@ export default function ExperimentDetail({ customPrismOuterStyle={{ marginTop: 4 }} /> + {types?.contextualBandit && selectedExperiment?.experiment ? ( + undefined) ?? + [] + ).map((m, i) => m?.name ?? `Variation ${i}`)} + forcedVariation={ + selectedEid && selectedEid in forcedVariations + ? forcedVariations[selectedEid] + : undefined + } + /> + ) : null} + {evaluations.length ? ( ) : null} - {types?.contextualBandit && selectedExperiment?.experiment ? ( - undefined) ?? - [] - ).map((m, i) => m?.name ?? `Variation ${i}`)} - forcedVariation={ - selectedEid && selectedEid in forcedVariations - ? forcedVariations[selectedEid] - : undefined - } - /> - ) : null} -
Implementation {(types?.redirect ? 1 : 0) + From 1b72feb36c693fcde65633da3d815c17fd8b4dba Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Thu, 27 Aug 2026 10:51:35 -0700 Subject: [PATCH 16/34] Rework the contextual bandit panel to the agreed design - lead with a meta box: bandit ref, current leaf, bandit version - "Variation Weights" replaces the fallback wording, and the assigned variation is emphasised in the bar list - hide Targeting and Traffic for bandits, whose static rule weights contradict the dynamic ones shown above - render context values directly rather than through ValueField --- src/app/components/ContextualBanditDetail.tsx | 103 ++++++++++------- src/app/components/ExperimentDetail.tsx | 105 +++++++++--------- 2 files changed, 120 insertions(+), 88 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 388122f..023432f 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -8,22 +8,36 @@ import useSdkData from "@/app/hooks/useSdkData"; import useGBSandboxEval from "@/app/hooks/useGBSandboxEval"; import { ContextualBanditDefinitions, + FALLBACK_LEAF_ID, getMatchedContextAttributes, usedFallbackWeights, } from "@/utils/contextualBandits"; -import ValueField from "@/app/components/ValueField"; // Matches how experiment weights are shown elsewhere, eg Rule.tsx function formatWeight(weight: number) { return Math.round(weight * 1000) / 10 + "%"; } +function MetaRow({ label, value }: { label: string; value: React.ReactNode }) { + return ( +
+
+ {label} +
+
{value}
+
+ ); +} + export default function ContextualBanditDetail({ experiment, variationNames, forcedVariation, }: { - experiment: Experiment & { contextualBanditRef?: string }; + experiment: Experiment & { + contextualBanditRef?: string; + contextualVariations?: unknown[]; + }; variationNames: string[]; forcedVariation?: number; }) { @@ -46,47 +60,48 @@ export default function ContextualBanditDetail({ ); const cb = banditResult ? { - leafId: banditResult.leafId ?? -1, + leafId: banditResult.leafId ?? FALLBACK_LEAF_ID, variationWeights: banditResult.variationWeights ?? [], banditVersion: banditResult.banditVersion, } : undefined; - // The rule's own weights, used when the payload has no trained contexts - const aggregateWeights = experiment.weights; - const weights = cb?.variationWeights ?? aggregateWeights; - const hasContextWeights = !!cb?.variationWeights; + + const weights = cb?.variationWeights ?? experiment.weights; const isFallback = usedFallbackWeights(cb); const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; + const selectedVariation = banditResult?.variationId; + + const leafLabel = !cb + ? "No trained contexts in payload" + : isFallback + ? "No matching context" + : cb.leafId; return ( <>
Contextual Bandit
- {banditRef && !definition ? ( - - The bandit {banditRef} is not in the SDK payload, so - fallback weights were used. - - ) : !banditRef ? ( - - No trained contexts are in the SDK payload yet, so every user gets the - fallback weights below. - - ) : null} +
+ {banditRef ? : null} + + {cb?.banditVersion !== undefined ? ( + + ) : null} +
- {isFallback ? ( - - No context matched this user, so fallback weights were used. - - ) : null} {isForced ? ( A forced variation is active, so these weights are overridden and not what the bandit would serve. ) : null} + {banditRef && !definition ? ( + + The bandit {banditRef} is not in the SDK payload. + + ) : null} - {hasContextWeights - ? "Weights for this context" - : "Fallback weights"} + Variation Weights {weights.length} variation{weights.length === 1 ? "" : "s"} @@ -113,16 +126,34 @@ export default function ContextualBanditDetail({ key={i} className="flex items-center gap-2 py-0.5 text-xs" > -
+
{i} · {variationNames[i] ?? `Variation ${i}`}
-
+
{formatWeight(weight)}
@@ -147,20 +178,16 @@ export default function ContextualBanditDetail({
{contextKeys.map((key) => ( -
+
{key}
- +
+ {JSON.stringify(context?.[key])} +
))}
diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index cc1da66..cff2f7f 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -567,61 +567,66 @@ export default function ExperimentDetail({ ))}
-
- Targeting and Traffic -
+ {/* A bandit's weights are dynamic, so the static ones here mislead */} + {!types?.contextualBandit ? ( + <> +
+ Targeting and Traffic +
- {urlPatterns?.length ? ( -
-
URL Targeting
-
    - {urlPatterns.map((pattern, i) => ( -
  • -
    {pattern.pattern}
    - {pattern.type !== "simple" && ( -
    - ({pattern.type} - {pattern.include ? ", exclude" : ""}) -
    - )} -
    - {isURLTargeted(url, [pattern]) ? ( -
    - Current URL targeted + {urlPatterns?.length ? ( +
    +
    URL Targeting
    +
      + {urlPatterns.map((pattern, i) => ( +
    • +
      {pattern.pattern}
      + {pattern.type !== "simple" && ( +
      + ({pattern.type} + {pattern.include ? ", exclude" : ""}) +
      + )} +
      + {isURLTargeted(url, [pattern]) ? ( +
      + Current URL targeted +
      + ) : ( +
      + Current URL excluded +
      + )}
      - ) : ( -
      - Current URL excluded -
      - )} -
    -
    • - ))} -
-
- ) : null} + + ))} + +
+ ) : null} -
-
Experiment
+
+
Experiment
-
- {condition || parentConditions ? ( - - ) : null} +
+ {condition || parentConditions ? ( + + ) : null} - -
-
+ +
+
+ + ) : null} {selectedExperiment ? (
From 79911ef7716e59237db10c97f7911db129f0fa22 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Thu, 27 Aug 2026 10:56:08 -0700 Subject: [PATCH 17/34] Bring the bandit panel in line with the mockup - ref, current leaf and version as a compact sub-line under the heading - leaf shown as a pill beside the weights label when a context matched - Setup block with the mockup's checks: whether trackingCallback receives userContext (bandit rewards can't be attributed without it) and whether a bandit definition is in the payload - weights read "Weights for this context" when a context matched, and "Variation Weights" otherwise --- src/app/components/ContextualBanditDetail.tsx | 87 ++++++++++++++++--- 1 file changed, 74 insertions(+), 13 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 023432f..6e8d439 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -1,7 +1,11 @@ import React from "react"; import { Badge, Text } from "@radix-ui/themes"; import * as Accordion from "@radix-ui/react-accordion"; -import { PiCaretRightFill } from "react-icons/pi"; +import { + PiCaretRightFill, + PiCheckCircleFill, + PiWarningFill, +} from "react-icons/pi"; import { Experiment } from "@growthbook/growthbook"; import useTabState from "@/app/hooks/useTabState"; import useSdkData from "@/app/hooks/useSdkData"; @@ -12,19 +16,36 @@ import { getMatchedContextAttributes, usedFallbackWeights, } from "@/utils/contextualBandits"; +import { + expectsUserContextParam, + trackingCallbackParamsAreValid, +} from "@/utils/sdkCallbacks"; // Matches how experiment weights are shown elsewhere, eg Rule.tsx function formatWeight(weight: number) { return Math.round(weight * 1000) / 10 + "%"; } -function MetaRow({ label, value }: { label: string; value: React.ReactNode }) { +function Check({ + ok, + children, + hint, +}: { + ok: boolean; + children: React.ReactNode; + hint?: string; +}) { return ( -
-
- {label} +
+ {ok ? ( + + ) : ( + + )} +
+
{children}
+ {hint ?
{hint}
: null}
-
{value}
); } @@ -68,11 +89,23 @@ export default function ContextualBanditDetail({ const weights = cb?.variationWeights ?? experiment.weights; const isFallback = usedFallbackWeights(cb); + // Only call them context weights when a real context actually matched + const hasContextWeights = !!cb?.variationWeights?.length && !isFallback; const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; const selectedVariation = banditResult?.variationId; + // Bandit rewards are attributed per context, so the callback must receive + // the userContext arg that SDK 1.7+ passes as its third param + const callbackPassesUserContext = + !!sdkData?.hasTrackingCallback && + expectsUserContextParam(sdkData?.version) && + trackingCallbackParamsAreValid( + sdkData?.trackingCallbackParams, + sdkData?.version, + ); + const leafLabel = !cb ? "No trained contexts in payload" : isFallback @@ -83,12 +116,14 @@ export default function ContextualBanditDetail({ <>
Contextual Bandit
-
- {banditRef ? : null} - - {cb?.banditVersion !== undefined ? ( - - ) : null} +
+ {[ + banditRef ? `contextualBanditRef: ${banditRef}` : null, + `leaf: ${leafLabel}`, + cb?.banditVersion !== undefined ? `v${cb.banditVersion}` : null, + ] + .filter(Boolean) + .join(" · ")}
{isForced ? ( @@ -113,8 +148,15 @@ export default function ContextualBanditDetail({ - Variation Weights + {hasContextWeights + ? "Weights for this context" + : "Variation Weights"} + {hasContextWeights && !isFallback ? ( + + leaf {cb?.leafId} + + ) : null} {weights.length} variation{weights.length === 1 ? "" : "s"} @@ -195,6 +237,25 @@ export default function ContextualBanditDetail({ ) : null} + +
Setup
+ + {callbackPassesUserContext + ? "trackingCallback passes userContext" + : "trackingCallback is missing the userContext param"} + + + {definition + ? "Bandit definition found in payload" + : "No bandit definition in payload"} + ); } From 8d80387ac290fd8e69e768b54ee875305e406e33 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Thu, 27 Aug 2026 12:02:18 -0700 Subject: [PATCH 18/34] Tidy the contextual bandit code - drop the unused getContextualBandit export and un-export the types and condition walker that nothing outside the module names - cut multi-line explanatory comments down to one line each - use clsx for the conditional weight-row classes, as elsewhere in the app - cover the condition walker through getMatchedContextAttributes now that it is private --- src/app/components/ContextualBanditDetail.tsx | 35 +++++++--------- src/app/components/ExperimentDetail.tsx | 2 +- src/app/components/ExperimentsTab.tsx | 1 - src/app/utils/logs.ts | 1 - src/utils/contextualBandits.test.ts | 40 +++++++++---------- src/utils/contextualBandits.ts | 38 +++++------------- src/utils/sdkCallbacks.ts | 7 +--- 7 files changed, 47 insertions(+), 77 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 6e8d439..5524bfc 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -1,6 +1,7 @@ import React from "react"; import { Badge, Text } from "@radix-ui/themes"; import * as Accordion from "@radix-ui/react-accordion"; +import clsx from "clsx"; import { PiCaretRightFill, PiCheckCircleFill, @@ -21,7 +22,7 @@ import { trackingCallbackParamsAreValid, } from "@/utils/sdkCallbacks"; -// Matches how experiment weights are shown elsewhere, eg Rule.tsx +// Matches Rule.tsx function formatWeight(weight: number) { return Math.round(weight * 1000) / 10 + "%"; } @@ -72,8 +73,7 @@ export default function ContextualBanditDetail({ | undefined; const definition = banditRef ? definitions?.[banditRef] : undefined; - // The SDK only applies bandit weights while evaluating the feature rule, so - // they land on the feature's experimentResult - never on gb.run() or the raw rule + // Bandit weights are applied during feature evaluation, so they land here const banditResult = Object.values(evaluatedFeatures) .map((f) => f?.result?.experimentResult) .find( @@ -89,15 +89,13 @@ export default function ContextualBanditDetail({ const weights = cb?.variationWeights ?? experiment.weights; const isFallback = usedFallbackWeights(cb); - // Only call them context weights when a real context actually matched const hasContextWeights = !!cb?.variationWeights?.length && !isFallback; const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; const selectedVariation = banditResult?.variationId; - // Bandit rewards are attributed per context, so the callback must receive - // the userContext arg that SDK 1.7+ passes as its third param + // Rewards are attributed per context, so userContext has to reach the callback const callbackPassesUserContext = !!sdkData?.hasTrackingCallback && expectsUserContextParam(sdkData?.version) && @@ -169,31 +167,28 @@ export default function ContextualBanditDetail({ className="flex items-center gap-2 py-0.5 text-xs" >
{i} · {variationNames[i] ?? `Variation ${i}`}
{formatWeight(weight)} diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index cff2f7f..2f2a9cb 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -567,7 +567,7 @@ export default function ExperimentDetail({ ))}
- {/* A bandit's weights are dynamic, so the static ones here mislead */} + {/* A bandit's weights are dynamic, so the static ones mislead */} {!types?.contextualBandit ? ( <>
diff --git a/src/app/components/ExperimentsTab.tsx b/src/app/components/ExperimentsTab.tsx index 23634d0..bc4a919 100644 --- a/src/app/components/ExperimentsTab.tsx +++ b/src/app/components/ExperimentsTab.tsx @@ -487,7 +487,6 @@ export function getFeatureExperiments( features: [fid], featureTypes: { [fid]: details.valueType }, ...rule, - // Contextual bandit rules carry these under contextualVariations variations, }); } diff --git a/src/app/utils/logs.ts b/src/app/utils/logs.ts index 7cd8a88..8f7a02b 100644 --- a/src/app/utils/logs.ts +++ b/src/app/utils/logs.ts @@ -35,7 +35,6 @@ export function reshapeEventLog(evt: LogUnionWithSource): FlattenedLogEvent { logType: evt.logType, timestamp: evt.timestamp, eventInfo: evt.experiment.name || "", - // Bandit results only carry these when the user was bucketed in isContextualBandit: evt.result?.variationWeights !== undefined, details: { experiment: evt.experiment, diff --git a/src/utils/contextualBandits.test.ts b/src/utils/contextualBandits.test.ts index f169866..88aa0fe 100644 --- a/src/utils/contextualBandits.test.ts +++ b/src/utils/contextualBandits.test.ts @@ -1,5 +1,4 @@ import { - conditionAttributeKeys, getMatchedContextAttributes, isContextualBandit, ruleVariations, @@ -53,26 +52,6 @@ describe("usedFallbackWeights", () => { }); }); -describe("conditionAttributeKeys", () => { - it("reads plain attribute names", () => { - expect(conditionAttributeKeys({ country: "US", device: "mobile" })).toEqual( - ["country", "device"], - ); - }); - - it("descends through $and / $or wrappers", () => { - expect( - conditionAttributeKeys({ - $or: [{ country: "US" }, { $and: [{ plan: "pro" }, { seats: 5 }] }], - }), - ).toEqual(["country", "plan", "seats"]); - }); - - it("does not report operators as attributes", () => { - expect(conditionAttributeKeys({ age: { $gt: 18 } })).toEqual(["age"]); - }); -}); - describe("getMatchedContextAttributes", () => { const definition = { banditVersion: 4, @@ -117,6 +96,25 @@ describe("getMatchedContextAttributes", () => { ).toBeUndefined(); }); + it("descends through $and / $or in a context condition", () => { + const nested = { + contexts: [ + { + leafId: 0, + condition: { $or: [{ country: "US" }, { plan: "pro" }] }, + weights: [0.5, 0.5], + }, + ], + }; + expect( + getMatchedContextAttributes( + nested, + { leafId: 0, variationWeights: [] }, + attributes, + ), + ).toEqual({ country: "US", plan: "pro" }); + }); + it("skips attributes the user does not have set", () => { expect( getMatchedContextAttributes( diff --git a/src/utils/contextualBandits.ts b/src/utils/contextualBandits.ts index 777f65d..90ded63 100644 --- a/src/utils/contextualBandits.ts +++ b/src/utils/contextualBandits.ts @@ -1,12 +1,12 @@ import type { Experiment, FeatureRule } from "@growthbook/growthbook"; -// The SDK declares these but doesn't export them from its entry point -export type CBContext = { +// Declared by the SDK but not exported from its entry point +type CBContext = { leafId: number; variationWeights: number[]; banditVersion?: number; }; -export type ContextualBanditDefinition = { +type ContextualBanditDefinition = { banditVersion?: number; contexts: { leafId: number; @@ -19,20 +19,19 @@ export type ContextualBanditDefinitions = Record< ContextualBanditDefinition >; -// A contextual bandit rule keeps its variations under `contextualVariations` so -// that pre-1.7 SDKs skip the rule instead of evaluating it with no weights +// leafId -1 means no context matched and the SDK used the aggregate weights +export const FALLBACK_LEAF_ID = -1; + +// Bandit rules keep their variations here so pre-1.7 SDKs skip the rule export function ruleVariations( rule: FeatureRule, ): Experiment["variations"] | undefined { - // FeatureRule types these as T[], but an experiment needs at least two return (rule.contextualVariations ?? rule.variations) as | Experiment["variations"] | undefined; } -// `contextualVariations` is what marks the rule: the backend always emits it in -// place of `variations`, while `contextualBanditRef` and the payload's -// `contextualBandits` block only appear once the bandit has trained contexts +// contextualBanditRef only appears once the bandit has trained contexts export function isContextualBandit( experiment: | { contextualVariations?: unknown[]; contextualBanditRef?: string } @@ -43,23 +42,11 @@ export function isContextualBandit( ); } -// The SDK only sets `contextualBandit` when the user was actually bucketed in, -// so its absence on a bandit experiment means the weights aren't live -export function getContextualBandit( - experiment: Experiment | undefined, -): CBContext | undefined { - return experiment?.contextualBandit; -} - -// leafId -1 means no context matched and the SDK fell back to aggregate weights -export const FALLBACK_LEAF_ID = -1; - export function usedFallbackWeights(cb: CBContext | undefined): boolean { return cb?.leafId === FALLBACK_LEAF_ID; } -// The attributes a bandit's matched context actually tested - this is the -// "contextual" part, and the only piece of the definition worth showing +// The attributes the matched context tested - the "contextual" part export function getMatchedContextAttributes( definition: ContextualBanditDefinition | undefined, cb: CBContext | undefined, @@ -75,11 +62,8 @@ export function getMatchedContextAttributes( return used; } -// Pull attribute names out of a mongo-style condition, skipping $and/$or/$not -// wrappers so nested conditions still report the attributes they test -export function conditionAttributeKeys( - condition: Record, -): string[] { +// Attribute names in a condition, descending through $and/$or wrappers +function conditionAttributeKeys(condition: Record): string[] { const keys = new Set(); const walk = (node: unknown) => { if (Array.isArray(node)) { diff --git a/src/utils/sdkCallbacks.ts b/src/utils/sdkCallbacks.ts index 341c931..2c00712 100644 --- a/src/utils/sdkCallbacks.ts +++ b/src/utils/sdkCallbacks.ts @@ -22,7 +22,6 @@ export function parseCallbackParams( let quote: string | null = null; for (let i = open; i < src.length; i++) { const char = src[i]; - // Brackets and commas inside a string default aren't structure if (quote) { current += char; if (char === "\\") current += src[++i] ?? ""; @@ -53,8 +52,7 @@ export function parseCallbackParams( return undefined; } -// Before this the SDK only ever called trackingCallback(experiment, result), so -// a third param was dead weight. From here it passes the userContext too +// From this version the SDK also passes a userContext to trackingCallback export const USER_CONTEXT_SDK_VERSION = "1.7.0"; export function expectsUserContextParam(version?: string): boolean { @@ -74,10 +72,7 @@ export function trackingCallbackParamsAreValid( // Zero params means a forwarding wrapper that reads `arguments` instead if (params.length === 0) return true; if (params.some((param) => param.startsWith("..."))) return true; - // Without a version to compare against, either shape may be right if (!version) return params.length === 2 || params.length === 3; - // Omitting userContext drops what it carries, declaring it on an SDK that - // never passes one leaves it permanently undefined return params.length === (expectsUserContextParam(version) ? 3 : 2); } From 73156b9b5fa2d2c568110d3f3994e740b8cf4f41 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Fri, 28 Aug 2026 13:02:40 -0700 Subject: [PATCH 19/34] Fix the bandit result lookup, which never matched Result.key is the variation key (meta.key, or the variation index), not the experiment key, so matching it against experiment.key found nothing: the panel always reported no trained contexts and fell back to the rule's static weights. Match on the feature result's experiment key instead, and only build the context object when bandit weights are actually present. --- src/app/components/ContextualBanditDetail.tsx | 10 +++--- src/utils/contextualBandits.test.ts | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 5524bfc..4dd05d5 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -74,12 +74,10 @@ export default function ContextualBanditDetail({ const definition = banditRef ? definitions?.[banditRef] : undefined; // Bandit weights are applied during feature evaluation, so they land here - const banditResult = Object.values(evaluatedFeatures) - .map((f) => f?.result?.experimentResult) - .find( - (r) => r?.key === experiment.key && r?.variationWeights !== undefined, - ); - const cb = banditResult + const banditResult = Object.values(evaluatedFeatures).find( + (f) => f?.result?.experiment?.key === experiment.key, + )?.result?.experimentResult; + const cb = banditResult?.variationWeights ? { leafId: banditResult.leafId ?? FALLBACK_LEAF_ID, variationWeights: banditResult.variationWeights ?? [], diff --git a/src/utils/contextualBandits.test.ts b/src/utils/contextualBandits.test.ts index 88aa0fe..3234bbc 100644 --- a/src/utils/contextualBandits.test.ts +++ b/src/utils/contextualBandits.test.ts @@ -125,3 +125,36 @@ describe("getMatchedContextAttributes", () => { ).toEqual({ country: "US" }); }); }); + +// Result.key is the variation key ("0", "1", …), not the experiment key, so a +// lookup keyed on it silently finds nothing and the bandit panel goes inert +describe("finding a bandit result among evaluated features", () => { + const evaluatedFeatures = { + f1: { + result: { + experiment: { key: "my-bandit" }, + experimentResult: { + key: "1", + variationId: 1, + leafId: 3, + variationWeights: [0.02, 0.96, 0.02], + banditVersion: 9, + }, + }, + }, + }; + + it("matches on the experiment key, not the result key", () => { + const found = Object.values(evaluatedFeatures).find( + (f) => f?.result?.experiment?.key === "my-bandit", + )?.result?.experimentResult; + expect(found?.leafId).toBe(3); + expect(found?.variationWeights).toEqual([0.02, 0.96, 0.02]); + }); + + it("does not match the result key against the experiment key", () => { + expect(evaluatedFeatures.f1.result.experimentResult.key).not.toBe( + "my-bandit", + ); + }); +}); From c68460184e9b13cd3d236cc5de8c7bca4f8924fd Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Fri, 28 Aug 2026 13:06:50 -0700 Subject: [PATCH 20/34] Address the rest of the code review - evaluate bandit experiments from the feature result rather than run(), which skips the feature-rule path and so reports a variation the page is not serving - read that result from the parent instead of mounting a third sandbox evaluation in the detail panel - detect bandits in the logs via the runtime experiment's contextualBandit; the ref and contextualVariations are stripped by the time a log is built - reuse ContextualBanditBadge in the experiments list and logs - stop reporting an untrained bandit as a warning, and stop blaming the callback for a userContext an older SDK never passes - say "Not applied for this user" when a definition exists but no bandit weights were used, rather than claiming the payload has none --- src/app/components/ContextualBanditDetail.tsx | 89 ++++++++++--------- src/app/components/ExperimentDetail.tsx | 1 + src/app/components/ExperimentsTab.tsx | 5 +- src/app/components/LogsList.tsx | 11 +-- src/app/hooks/useGBSandboxEval.ts | 15 +++- src/app/utils/logs.ts | 3 +- src/utils/contextualBandits.ts | 7 ++ 7 files changed, 78 insertions(+), 53 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 4dd05d5..4970eee 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -5,12 +5,12 @@ import clsx from "clsx"; import { PiCaretRightFill, PiCheckCircleFill, + PiInfoBold, PiWarningFill, } from "react-icons/pi"; -import { Experiment } from "@growthbook/growthbook"; +import { Experiment, Result } from "@growthbook/growthbook"; import useTabState from "@/app/hooks/useTabState"; import useSdkData from "@/app/hooks/useSdkData"; -import useGBSandboxEval from "@/app/hooks/useGBSandboxEval"; import { ContextualBanditDefinitions, FALLBACK_LEAF_ID, @@ -29,16 +29,20 @@ function formatWeight(weight: number) { function Check({ ok, + info, children, hint, }: { - ok: boolean; + ok?: boolean; + info?: boolean; children: React.ReactNode; hint?: string; }) { return (
- {ok ? ( + {info ? ( + + ) : ok ? ( ) : ( @@ -55,6 +59,7 @@ export default function ContextualBanditDetail({ experiment, variationNames, forcedVariation, + result, }: { experiment: Experiment & { contextualBanditRef?: string; @@ -62,26 +67,21 @@ export default function ContextualBanditDetail({ }; variationNames: string[]; forcedVariation?: number; + result?: Result; }) { const sdkData = useSdkData(); const [attributes] = useTabState>("attributes", {}); - const { evaluatedFeatures } = useGBSandboxEval(); - const banditRef = experiment.contextualBanditRef; const definitions = sdkData?.payload?.contextualBandits as | ContextualBanditDefinitions | undefined; const definition = banditRef ? definitions?.[banditRef] : undefined; - // Bandit weights are applied during feature evaluation, so they land here - const banditResult = Object.values(evaluatedFeatures).find( - (f) => f?.result?.experiment?.key === experiment.key, - )?.result?.experimentResult; - const cb = banditResult?.variationWeights + const cb = result?.variationWeights ? { - leafId: banditResult.leafId ?? FALLBACK_LEAF_ID, - variationWeights: banditResult.variationWeights ?? [], - banditVersion: banditResult.banditVersion, + leafId: result.leafId ?? FALLBACK_LEAF_ID, + variationWeights: result.variationWeights, + banditVersion: result.banditVersion, } : undefined; @@ -91,7 +91,7 @@ export default function ContextualBanditDetail({ const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; - const selectedVariation = banditResult?.variationId; + const selectedVariation = result?.variationId; // Rewards are attributed per context, so userContext has to reach the callback const callbackPassesUserContext = @@ -102,11 +102,13 @@ export default function ContextualBanditDetail({ sdkData?.version, ); - const leafLabel = !cb - ? "No trained contexts in payload" - : isFallback + const leafLabel = cb + ? isFallback ? "No matching context" - : cb.leafId; + : cb.leafId + : definition + ? "Not applied for this user" + : "No trained contexts yet"; return ( <> @@ -148,11 +150,7 @@ export default function ContextualBanditDetail({ ? "Weights for this context" : "Variation Weights"} - {hasContextWeights && !isFallback ? ( - - leaf {cb?.leafId} - - ) : null} + {weights.length} variation{weights.length === 1 ? "" : "s"} @@ -232,23 +230,32 @@ export default function ContextualBanditDetail({
Setup
- - {callbackPassesUserContext - ? "trackingCallback passes userContext" - : "trackingCallback is missing the userContext param"} - - - {definition - ? "Bandit definition found in payload" - : "No bandit definition in payload"} - + {!expectsUserContextParam(sdkData?.version) ? ( + + SDK {sdkData?.version ?? "version unknown"} does not pass userContext + to trackingCallback + + ) : ( + + {callbackPassesUserContext + ? "trackingCallback passes userContext" + : "trackingCallback is missing the userContext param"} + + )} + {definition ? ( + Bandit definition found in payload + ) : banditRef ? ( + Bandit definition missing from payload + ) : ( + No trained contexts yet, so weights are still even + )} ); } diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index 2f2a9cb..2b2a282 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -451,6 +451,7 @@ export default function ExperimentDetail({ ? forcedVariations[selectedEid] : undefined } + result={selectedExperiment.evaluatedExperiment?.result} /> ) : null} diff --git a/src/app/components/ExperimentsTab.tsx b/src/app/components/ExperimentsTab.tsx index bc4a919..8f6af83 100644 --- a/src/app/components/ExperimentsTab.tsx +++ b/src/app/components/ExperimentsTab.tsx @@ -25,6 +25,7 @@ import { useResponsiveContext } from "../hooks/useResponsive"; import { TbEyeSearch } from "react-icons/tb"; import { LogUnionWithSource } from "@/app/utils/logs"; import { isContextualBandit, ruleVariations } from "@/utils/contextualBandits"; +import { ContextualBanditBadge } from "@/app/components/ContextualBanditDetail"; export type ExperimentWithFeatures = (AutoExperiment | Experiment) & { features?: string[]; @@ -318,9 +319,7 @@ export default function ExperimentsTab() { {types ? (
{types.contextualBandit ? ( - - Contextual Bandit - + ) : null} {types.redirect ? ( diff --git a/src/app/components/LogsList.tsx b/src/app/components/LogsList.tsx index 3589ce1..b758444 100644 --- a/src/app/components/LogsList.tsx +++ b/src/app/components/LogsList.tsx @@ -23,6 +23,7 @@ import { import ValueField from "./ValueField"; import clsx from "clsx"; import { LogUnionWithSource } from "@/app/utils/logs"; +import { ContextualBanditBadge } from "@/app/components/ContextualBanditDetail"; export const HEADER_H = 40; @@ -259,13 +260,9 @@ export default function LogsList({ > {evt.eventInfo} {evt.isContextualBandit ? ( - - Contextual Bandit - + + + ) : null}
{!isResponsive && ( diff --git a/src/app/hooks/useGBSandboxEval.ts b/src/app/hooks/useGBSandboxEval.ts index 5d0c6d6..cbc6b12 100644 --- a/src/app/hooks/useGBSandboxEval.ts +++ b/src/app/hooks/useGBSandboxEval.ts @@ -172,9 +172,22 @@ export default function useGBSandboxEval() { }; } + // run() skips the feature-rule path, so it never applies bandit weights. + // Reuse the feature's own result for those to avoid reporting a variation + // the page isn't serving. + const banditResults = new Map>(); + for (const fid in evaluatedFeatures) { + const featureResult = evaluatedFeatures[fid]?.result; + const key = featureResult?.experiment?.key; + if (key && featureResult?.experimentResult?.variationWeights) { + banditResults.set(key, featureResult.experimentResult); + } + } + [...experiments, ...featureExperiments].forEach((experiment) => { + const banditResult = banditResults.get(experiment.key); growthbook.debug = true; - const result = growthbook.run(experiment); + const result = banditResult ?? growthbook.run(experiment); growthbook.debug = false; const debug = [...log]; log = []; diff --git a/src/app/utils/logs.ts b/src/app/utils/logs.ts index 8f7a02b..c8d31f8 100644 --- a/src/app/utils/logs.ts +++ b/src/app/utils/logs.ts @@ -1,4 +1,5 @@ import { LogUnion } from "@growthbook/growthbook"; +import { appliedContextualBandit } from "@/utils/contextualBandits"; export interface FlattenedLogEvent { logType: string; @@ -35,7 +36,7 @@ export function reshapeEventLog(evt: LogUnionWithSource): FlattenedLogEvent { logType: evt.logType, timestamp: evt.timestamp, eventInfo: evt.experiment.name || "", - isContextualBandit: evt.result?.variationWeights !== undefined, + isContextualBandit: appliedContextualBandit(evt.experiment), details: { experiment: evt.experiment, result: evt.result, diff --git a/src/utils/contextualBandits.ts b/src/utils/contextualBandits.ts index 90ded63..53307ee 100644 --- a/src/utils/contextualBandits.ts +++ b/src/utils/contextualBandits.ts @@ -42,6 +42,13 @@ export function isContextualBandit( ); } +// The runtime experiment only keeps this once bandit weights were applied +export function appliedContextualBandit( + experiment: { contextualBandit?: CBContext } | undefined, +): boolean { + return !!experiment?.contextualBandit; +} + export function usedFallbackWeights(cb: CBContext | undefined): boolean { return cb?.leafId === FALLBACK_LEAF_ID; } From ffd3a756697a4eef04957e673326f6c12638d882 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Fri, 28 Aug 2026 13:11:55 -0700 Subject: [PATCH 21/34] Drop the leaf line when no bandit weights were applied For a bandit with no trained contexts the sub-line read "leaf: No trained contexts yet", labelling a leaf that does not exist and repeating what the setup check says. The leaf now only appears once weights were applied, and a definition that exists but was not used is called out in the checks. --- src/app/components/ContextualBanditDetail.tsx | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 4970eee..e50b8da 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -102,27 +102,27 @@ export default function ContextualBanditDetail({ sdkData?.version, ); - const leafLabel = cb - ? isFallback + // Only meaningful once bandit weights were actually applied + const leafLabel = !cb + ? undefined + : isFallback ? "No matching context" - : cb.leafId - : definition - ? "Not applied for this user" - : "No trained contexts yet"; + : `leaf: ${cb.leafId}`; + const subline = [ + banditRef ? `contextualBanditRef: ${banditRef}` : null, + leafLabel, + cb?.banditVersion !== undefined ? `v${cb.banditVersion}` : null, + ] + .filter(Boolean) + .join(" · "); return ( <>
Contextual Bandit
-
- {[ - banditRef ? `contextualBanditRef: ${banditRef}` : null, - `leaf: ${leafLabel}`, - cb?.banditVersion !== undefined ? `v${cb.banditVersion}` : null, - ] - .filter(Boolean) - .join(" · ")} -
+ {subline ? ( +
{subline}
+ ) : null} {isForced ? ( @@ -249,7 +249,9 @@ export default function ContextualBanditDetail({ : "trackingCallback is missing the userContext param"} )} - {definition ? ( + {definition && !cb ? ( + Bandit weights were not applied for this user + ) : definition ? ( Bandit definition found in payload ) : banditRef ? ( Bandit definition missing from payload From 4a8f5299f7ecb1cf7d43372c8cf08478316c665f Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Fri, 28 Aug 2026 14:46:51 -0700 Subject: [PATCH 22/34] Use a dropdown for variation selection past four variations Radio cards stop being scannable once an experiment has more than a handful of variations, which a bandit routinely does. Also say what to do when a forced variation is hiding the bandit weights: forcing sets hashUsed false, and the SDK then drops leafId and variationWeights entirely. --- src/app/components/ContextualBanditDetail.tsx | 8 ++++-- src/app/components/ExperimentDetail.tsx | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index e50b8da..709a4af 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -124,7 +124,7 @@ export default function ContextualBanditDetail({
{subline}
) : null} - {isForced ? ( + {isForced && cb ? ( A forced variation is active, so these weights are overridden and not what the bandit would serve. @@ -250,7 +250,11 @@ export default function ContextualBanditDetail({ )} {definition && !cb ? ( - Bandit weights were not applied for this user + + {isForced + ? "Clear the forced variation to see the live bandit weights" + : "Bandit weights were not applied for this user"} + ) : definition ? ( Bandit definition found in payload ) : banditRef ? ( diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index 2b2a282..c681ee5 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -5,6 +5,7 @@ import { IconButton, Link, RadioCards, + Select, Tooltip, } from "@radix-ui/themes"; import { @@ -685,6 +686,32 @@ function EditableVariationField({ if (!variationsMeta || !experiment) return null; + // Cards stop being scannable past a handful of variations + if (variationsMeta.length > 4) { + return ( +
+ setValue(parseInt(s))} + > + + + {variationsMeta.map((meta, i) => ( + +
+ + + {getVariationSummary({ experiment, i })} + +
+ + ))} + + +
+ ); + } + return (
Date: Mon, 31 Aug 2026 12:53:07 -0700 Subject: [PATCH 23/34] Fix the variation dropdown and surface the current leaf The dropdown used position="popper", unlike every other Select in the app; inside the fixed, scrolling detail panel its menu did not open. Match the existing variant="soft" usage. The leaf was buried in the grey sub-line and hard to find, so it now has its own labelled row. --- src/app/components/ContextualBanditDetail.tsx | 12 ++++++++++-- src/app/components/ExperimentDetail.tsx | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 709a4af..b5e328c 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -107,10 +107,9 @@ export default function ContextualBanditDetail({ ? undefined : isFallback ? "No matching context" - : `leaf: ${cb.leafId}`; + : cb.leafId; const subline = [ banditRef ? `contextualBanditRef: ${banditRef}` : null, - leafLabel, cb?.banditVersion !== undefined ? `v${cb.banditVersion}` : null, ] .filter(Boolean) @@ -124,6 +123,15 @@ export default function ContextualBanditDetail({
{subline}
) : null} + {leafLabel !== undefined ? ( +
+
+ Current leaf +
+
{leafLabel}
+
+ ) : null} + {isForced && cb ? ( A forced variation is active, so these weights are overridden and not diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index c681ee5..a6a0464 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -695,7 +695,7 @@ function EditableVariationField({ onValueChange={(s: string) => setValue(parseInt(s))} > - + {variationsMeta.map((meta, i) => (
From 2067eb193813c061c5ba576ba7d42099cd45a21d Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 13:02:45 -0700 Subject: [PATCH 24/34] Name the missing hash attribute when a bandit rule is skipped Without a value for the rule's hashAttribute the SDK logs "Skip because missing hashAttribute" and returns no result at all, so the panel said only that weights were not applied. It now names the attribute. --- src/app/components/ContextualBanditDetail.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index b5e328c..096c5ff 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -91,6 +91,10 @@ export default function ContextualBanditDetail({ const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; + // Without a value for the hash attribute the SDK skips the rule outright, + // so there is no assignment and no bandit data at all + const hashAttribute = experiment.hashAttribute ?? "id"; + const missingHashValue = !attributes?.[hashAttribute]; const selectedVariation = result?.variationId; // Rewards are attributed per context, so userContext has to reach the callback @@ -259,9 +263,11 @@ export default function ContextualBanditDetail({ )} {definition && !cb ? ( - {isForced - ? "Clear the forced variation to see the live bandit weights" - : "Bandit weights were not applied for this user"} + {missingHashValue + ? `No value for the "${hashAttribute}" attribute, so the rule is skipped` + : isForced + ? "Clear the forced variation to see the live bandit weights" + : "Bandit weights were not applied for this user"} ) : definition ? ( Bandit definition found in payload From 453b86a2e4b86120d4fbdee3ad3ecdbf6a5f319f Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 13:21:34 -0700 Subject: [PATCH 25/34] Make Current value a variation dropdown Selecting by value reads better than by index when the variations are plain strings, as a bandit's usually are. Object-valued variations keep the syntax-highlighted display, where a dropdown of JSON blobs would be unreadable. --- src/app/components/ExperimentDetail.tsx | 39 +++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index a6a0464..ea8cd4f 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -433,11 +433,40 @@ export default function ExperimentDetail({ ) : null}
- + {/* Picking by value beats picking by index when the variations are + readable strings, as a bandit's usually are */} + {selectedEid && + variations && + variations.length > 1 && + variations.every((v) => v === null || typeof v !== "object") ? ( +
+ { + setForcedVariation(selectedEid, parseInt(v)); + setOverrideExperiment(true); + }} + > + + + {variations.map((variation, i) => ( + +
+ + {String(variation)} +
+ + ))} + + +
+ ) : ( + + )} {types?.contextualBandit && selectedExperiment?.experiment ? ( Date: Mon, 31 Aug 2026 13:26:40 -0700 Subject: [PATCH 26/34] Revert Current value to a display, and lift the variation menu above the panel Current value goes back to the read-only value display. The variation dropdown rendered but would not open: the detail panel sets z-index 1000 inline, and the portaled menu landed underneath it. Give the menu content a higher z-index and keep popper positioning, which the nav select already uses. --- src/app/components/ExperimentDetail.tsx | 47 +++++++------------------ 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index ea8cd4f..f1060ea 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -433,40 +433,11 @@ export default function ExperimentDetail({ ) : null}
- {/* Picking by value beats picking by index when the variations are - readable strings, as a bandit's usually are */} - {selectedEid && - variations && - variations.length > 1 && - variations.every((v) => v === null || typeof v !== "object") ? ( -
- { - setForcedVariation(selectedEid, parseInt(v)); - setOverrideExperiment(true); - }} - > - - - {variations.map((variation, i) => ( - -
- - {String(variation)} -
- - ))} - - -
- ) : ( - - )} + {types?.contextualBandit && selectedExperiment?.experiment ? ( setValue(parseInt(s))} > - + {/* The detail panel sits at z-index 1000, so the portaled menu + needs to clear it */} + {variationsMeta.map((meta, i) => (
From 2f0a414ed90b4e521ba7078317aa1d6d5bad3b9a Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 13:40:57 -0700 Subject: [PATCH 27/34] Stop the sandbox evaluation crashing on rules without meta useGBSandboxEval stuffs rule metadata for its debug log, but when a rule had no meta it substituted a single-element array however many variations the rule had. The SDK indexes experiment.meta by variation index, so getExperimentResult threw on any variation past the first, the whole evaluate() call died, and every experiment then rendered as Inactive with a null value - the bandit panel included. Build the stuffed meta per variation, keeping any entries the rule already had, and keep the single tag for rules with no variations since the debug log reads meta[0]. --- src/app/components/ExperimentDetail.tsx | 7 ++-- src/app/hooks/useGBSandboxEval.ts | 11 +++++-- src/utils/contextualBandits.test.ts | 43 +++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index f1060ea..dd1667b 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -52,6 +52,7 @@ import { EvaluationSourceViewer, } from "@/app/components/FeatureDetail"; import { LogUnionWithSource } from "@/app/utils/logs"; +import { useSelectMenuPortal } from "@/app/SelectMenuPortal"; import ContextualBanditDetail, { ContextualBanditBadge, } from "@/app/components/ContextualBanditDetail"; @@ -678,6 +679,7 @@ function EditableVariationField({ evaluatedValue?: number; setValue: (v: any) => void; }) { + const menuPortalTarget = useSelectMenuPortal(); let variationsMeta: { key?: string; name?: string }[] | undefined = experiment?.meta ?? experiment?.variations?.map((variation, i) => ({ @@ -695,11 +697,12 @@ function EditableVariationField({ onValueChange={(s: string) => setValue(parseInt(s))} > - {/* The detail panel sits at z-index 1000, so the portaled menu - needs to clear it */} + {/* Portal into the app's menu container, as SelectField does - + the detail panel's stacking context otherwise hides the menu */} {variationsMeta.map((meta, i) => ( diff --git a/src/app/hooks/useGBSandboxEval.ts b/src/app/hooks/useGBSandboxEval.ts index cbc6b12..b14adcf 100644 --- a/src/app/hooks/useGBSandboxEval.ts +++ b/src/app/hooks/useGBSandboxEval.ts @@ -12,6 +12,7 @@ import { import useTabState from "@/app/hooks/useTabState"; import { DebugLog } from "devtools"; import { getFeatureExperiments } from "@/app/components/ExperimentsTab"; +import { ruleVariations } from "@/utils/contextualBandits"; import useSdkData from "./useSdkData"; import { FeatureDefinitionWithId } from "@/app/components/FeaturesTab"; import { LogUnionWithSource } from "@/app/utils/logs"; @@ -103,8 +104,14 @@ export default function useGBSandboxEval() { ...rule, // Stuff rule index + featureId into meta so it survives into exp.meta. // featureId is needed for experiment-ref rules where exp.key !== fid. - meta: rule.meta - ? rule.meta.map((m) => ({ ...m, ruleI: i, featureId: fid })) + // Must stay as long as the variations: the SDK indexes meta by + // variation and throws on a short array (core.ts getExperimentResult). + meta: ruleVariations(rule)?.length + ? ruleVariations(rule)!.map((_, vi) => ({ + ...(rule.meta?.[vi] ?? {}), + ruleI: i, + featureId: fid, + })) : [{ ruleI: i, featureId: fid }], })); } diff --git a/src/utils/contextualBandits.test.ts b/src/utils/contextualBandits.test.ts index 3234bbc..9bc0019 100644 --- a/src/utils/contextualBandits.test.ts +++ b/src/utils/contextualBandits.test.ts @@ -158,3 +158,46 @@ describe("finding a bandit result among evaluated features", () => { ); }); }); + +// The sandbox stuffs rule metadata for its debug log. The SDK indexes +// experiment.meta by variation index and throws on a short array, which took +// down the whole evaluation and left every experiment showing as inactive. +describe("stuffed rule meta", () => { + const stuff = (rule: any, i = 0, fid = "f1") => ({ + ...rule, + meta: ruleVariations(rule)?.length + ? ruleVariations(rule)!.map((_, vi) => ({ + ...(rule.meta?.[vi] ?? {}), + ruleI: i, + featureId: fid, + })) + : [{ ruleI: i, featureId: fid }], + }); + + it("stays as long as the variations when the rule has no meta", () => { + expect(stuff({ variations: ["off", "on"] }).meta).toHaveLength(2); + }); + + it("stays as long as the variations for a bandit rule", () => { + expect( + stuff({ contextualVariations: ["a", "b", "c", "d"] }).meta, + ).toHaveLength(4); + }); + + it("keeps existing meta entries", () => { + const meta = stuff({ + variations: ["a", "b"], + meta: [ + { key: "0", name: "A" }, + { key: "1", name: "B" }, + ], + }).meta; + expect(meta[1]).toMatchObject({ key: "1", name: "B", ruleI: 0 }); + }); + + it("still tags a rule with no variations, which the debug log reads", () => { + expect(stuff({ force: true }).meta).toEqual([ + { ruleI: 0, featureId: "f1" }, + ]); + }); +}); From 7670f733469798da98aa871b098f833b18bbd923 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 13:55:59 -0700 Subject: [PATCH 28/34] Use the app's SelectField for the variation dropdown Three attempts at making a Radix Select open inside the detail panel failed. The app already has SelectField, which portals its menu through SelectMenuPortalProvider - the component that exists precisely because menus in these panels need an explicit portal target. Use it, keeping the variation icons via formatOptionLabel and sort disabled so the options stay in variation order. Also hide the debug box under Enrollment Status when there is no message; lastDebugLog defaults to an empty string, which rendered an empty console panel. --- src/app/components/ExperimentDetail.tsx | 44 +++++++++---------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index dd1667b..0ca048b 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -5,7 +5,6 @@ import { IconButton, Link, RadioCards, - Select, Tooltip, } from "@radix-ui/themes"; import { @@ -52,7 +51,7 @@ import { EvaluationSourceViewer, } from "@/app/components/FeatureDetail"; import { LogUnionWithSource } from "@/app/utils/logs"; -import { useSelectMenuPortal } from "@/app/SelectMenuPortal"; +import SelectField from "@/app/components/Forms/SelectField"; import ContextualBanditDetail, { ContextualBanditBadge, } from "@/app/components/ContextualBanditDetail"; @@ -328,7 +327,7 @@ export default function ExperimentDetail({ Inactive
)} - {lastDebugLog !== "In experiment" && ( + {lastDebugLog && lastDebugLog !== "In experiment" && (
void; }) { - const menuPortalTarget = useSelectMenuPortal(); let variationsMeta: { key?: string; name?: string }[] | undefined = experiment?.meta ?? experiment?.variations?.map((variation, i) => ({ @@ -692,31 +690,21 @@ function EditableVariationField({ if (variationsMeta.length > 4) { return (
- setValue(parseInt(s))} - > - - {/* Portal into the app's menu container, as SelectField does - - the detail panel's stacking context otherwise hides the menu */} - - {variationsMeta.map((meta, i) => ( - -
- - - {getVariationSummary({ experiment, i })} - -
- - ))} - - + sort={false} + options={variationsMeta.map((meta, i) => ({ + label: getVariationSummary({ experiment, i }), + value: i + "", + }))} + onChange={(v) => setValue(parseInt(v))} + formatOptionLabel={(opt) => ( +
+ + {opt.label} +
+ )} + />
); } From 5813e9030d2d6330a0f86c8339d6e0773f75cade Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 13:58:06 -0700 Subject: [PATCH 29/34] Show the variation name in the closed dropdown, and drop two noisy log lines SelectField labels the selected value with the raw value, so the closed dropdown read "1" rather than the variation name. Derive the name from the index in formatOptionLabel, which covers the trigger and the menu. Also stop echoing "In experiment" and "Force via dev tools" in the debug box: the panel states both itself, right above it. --- src/app/components/ExperimentDetail.tsx | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/app/components/ExperimentDetail.tsx b/src/app/components/ExperimentDetail.tsx index 0ca048b..cc76768 100644 --- a/src/app/components/ExperimentDetail.tsx +++ b/src/app/components/ExperimentDetail.tsx @@ -56,6 +56,9 @@ import ContextualBanditDetail, { ContextualBanditBadge, } from "@/app/components/ContextualBanditDetail"; +// The panel states these itself, so echoing the SDK log adds nothing +const REDUNDANT_DEBUG_LOGS = ["In experiment", "Force via dev tools"]; + export default function ExperimentDetail({ selectedEid, setSelectedEid, @@ -327,7 +330,7 @@ export default function ExperimentDetail({ Inactive
)} - {lastDebugLog && lastDebugLog !== "In experiment" && ( + {lastDebugLog && !REDUNDANT_DEBUG_LOGS.includes(lastDebugLog) && (
setValue(parseInt(v))} - formatOptionLabel={(opt) => ( -
- - {opt.label} -
- )} + formatOptionLabel={(opt) => { + // SelectField labels the selected value with the raw value, so + // derive the name from the index for the trigger and the menu alike + const i = parseInt(opt.value); + return ( +
+ + + {getVariationSummary({ experiment, i })} + +
+ ); + }} />
); From 72ee18775a68c8b0dd2c58c464026978cc2b74a8 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 14:01:19 -0700 Subject: [PATCH 30/34] Match the app's collapsible standard in the bandit panel Both sections used a plain Text heading with the caret, which read as a title rather than a toggle and sat tight against the row above. Use the bold Link trigger and accordion my-4 spacing that SdkItemPanel already uses for its collapsible sections. --- src/app/components/ContextualBanditDetail.tsx | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 096c5ff..a343aaf 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Badge, Text } from "@radix-ui/themes"; +import { Badge, Link, Text } from "@radix-ui/themes"; import * as Accordion from "@radix-ui/react-accordion"; import clsx from "clsx"; import { @@ -149,19 +149,24 @@ export default function ContextualBanditDetail({ ) : null} {weights?.length ? ( - - + + {hasContextWeights ? "Weights for this context" : "Variation Weights"} - + {weights.length} variation{weights.length === 1 ? "" : "s"} @@ -210,11 +215,16 @@ export default function ContextualBanditDetail({ {contextKeys.length ? ( - - + + Context used for this user - + {contextKeys.length} attribute {contextKeys.length === 1 ? "" : "s"} From 180abd869c66fef6224d0d7b65ef1f806da7a102 Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 14:09:01 -0700 Subject: [PATCH 31/34] Space the collapsible content off its trigger Both accordion bodies started flush against the toggle. --- src/app/components/ContextualBanditDetail.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index a343aaf..a86270d 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -173,7 +173,7 @@ export default function ContextualBanditDetail({ -
+
{weights.map((weight, i) => (
-
+
{contextKeys.map((key) => (
Date: Mon, 31 Aug 2026 14:10:55 -0700 Subject: [PATCH 32/34] Put the collapsible spacing above the title, not below it Reverts the content margin from 180abd8: the gap belongs between the previous section and the toggle, so each trigger now carries the top margin. --- src/app/components/ContextualBanditDetail.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index a86270d..5032b9b 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -155,7 +155,7 @@ export default function ContextualBanditDetail({ > {weights?.length ? ( - + -
+
{weights.map((weight, i) => (
- + -
+
{contextKeys.map((key) => (
Date: Mon, 31 Aug 2026 14:46:27 -0700 Subject: [PATCH 33/34] Fix defects from the second review pass - Rule.tsx destructured rule.variations, so a bandit rule rendered in the Features tab as a rollout with no variations. Use ruleVariations, the same fix already applied to getFeatureExperiments. - Don't highlight variation 0 for a user the SDK skipped: it clamps the index to 0 with inExperiment false, so the weights list contradicted the Inactive status shown above it. - The missing-hash-attribute diagnostic sat behind , which the common bandit shape never has, so it never fired. Hoist it, and count fallbackAttribute as satisfying the hash value. - Omit isContextualBandit when false: useSearch matches JSON.stringify(item), so a literal false made every experiment log row match a search for "bandit". - Reserve room for the badge in split view, where the types container is absolutely positioned and the name ran underneath it. --- src/app/components/ContextualBanditDetail.tsx | 25 ++++++++---- src/app/components/ExperimentsTab.tsx | 9 ++++- src/app/components/Rule.tsx | 39 +++++++++++++------ src/app/utils/logs.ts | 6 ++- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 5032b9b..90624ba 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -94,8 +94,15 @@ export default function ContextualBanditDetail({ // Without a value for the hash attribute the SDK skips the rule outright, // so there is no assignment and no bandit data at all const hashAttribute = experiment.hashAttribute ?? "id"; - const missingHashValue = !attributes?.[hashAttribute]; - const selectedVariation = result?.variationId; + const fallbackAttribute = experiment.fallbackAttribute; + const missingHashValue = + !attributes?.[hashAttribute] && + !(fallbackAttribute && attributes?.[fallbackAttribute]); + // The SDK clamps a skipped assignment to 0 with inExperiment false, so + // without this guard the panel bolds variation 0 for a user it did not serve + const selectedVariation = result?.inExperiment + ? result.variationId + : undefined; // Rewards are attributed per context, so userContext has to reach the callback const callbackPassesUserContext = @@ -271,13 +278,15 @@ export default function ContextualBanditDetail({ : "trackingCallback is missing the userContext param"} )} - {definition && !cb ? ( + {missingHashValue ? ( + + {`No value for the "${hashAttribute}" attribute, so the rule is skipped`} + + ) : definition && !cb ? ( - {missingHashValue - ? `No value for the "${hashAttribute}" attribute, so the rule is skipped` - : isForced - ? "Clear the forced variation to see the live bandit weights" - : "Bandit weights were not applied for this user"} + {isForced + ? "Clear the forced variation to see the live bandit weights" + : "Bandit weights were not applied for this user"} ) : definition ? ( Bandit definition found in payload diff --git a/src/app/components/ExperimentsTab.tsx b/src/app/components/ExperimentsTab.tsx index 8f6af83..476c7af 100644 --- a/src/app/components/ExperimentsTab.tsx +++ b/src/app/components/ExperimentsTab.tsx @@ -8,7 +8,7 @@ import useTabState from "../hooks/useTabState"; import useGBSandboxEval, { EvaluatedExperiment, } from "@/app/hooks/useGBSandboxEval"; -import { Badge, Link, Switch, Tooltip } from "@radix-ui/themes"; +import { Link, Switch, Tooltip } from "@radix-ui/themes"; import { PiDesktopFill, PiFlagFill, PiLinkBold, PiXBold } from "react-icons/pi"; import clsx from "clsx"; import { MW, NAV_H } from "@/app"; @@ -292,7 +292,12 @@ export default function ExperimentsTab() { onClick={() => clickExperiment(eid, changeId)} >
diff --git a/src/app/components/Rule.tsx b/src/app/components/Rule.tsx index a184084..011a648 100644 --- a/src/app/components/Rule.tsx +++ b/src/app/components/Rule.tsx @@ -23,10 +23,19 @@ import { import { EvaluatedFeature } from "@/app/hooks/useGBSandboxEval"; import DebugLogger from "@/app/components/DebugLogger"; import useGlobalState from "@/app/hooks/useGlobalState"; -import { formatExperimentKey, holdoutIdFromFid } from "@/app/components/ExperimentsTab"; +import { + formatExperimentKey, + holdoutIdFromFid, +} from "@/app/components/ExperimentsTab"; import { isDark, Theme } from "@/app"; +import { ruleVariations } from "@/utils/contextualBandits"; -type RuleType = "force" | "rollout" | "experiment" | "prerequisite" | "safe-rollout"; +type RuleType = + | "force" + | "rollout" + | "experiment" + | "prerequisite" + | "safe-rollout"; const RULE_MATCHED_LOGS = [ "Force", @@ -102,19 +111,19 @@ export default function Rule({ condition, parentConditions, force, - variations, weights, hashAttribute, coverage, namespace, } = rule; + // Bandit rules keep theirs under contextualVariations + const variations = ruleVariations(rule); const key = rule.key ?? fid; - let ruleType: RuleType = - rule?.key?.startsWith("srk_") - ? "safe-rollout" - : variations - ? "experiment" - : "coverage" in rule + let ruleType: RuleType = rule?.key?.startsWith("srk_") + ? "safe-rollout" + : variations + ? "experiment" + : "coverage" in rule ? "rollout" : rule?.parentConditions?.some((p) => p.gate) ? "prerequisite" @@ -535,7 +544,11 @@ export function ConditionDisplay({
); - const renderOrBlock = (branches: Condition[][], blockKey: string, prefix: string) => ( + const renderOrBlock = ( + branches: Condition[][], + blockKey: string, + prefix: string, + ) => (
{prefix} [ @@ -550,7 +563,11 @@ export function ConditionDisplay({ )}
{group.map((cond, i) => - renderCondRow(cond, `${blockKey}-${gi}-${i}`, i > 0 ? "AND" : null), + renderCondRow( + cond, + `${blockKey}-${gi}-${i}`, + i > 0 ? "AND" : null, + ), )}
diff --git a/src/app/utils/logs.ts b/src/app/utils/logs.ts index c8d31f8..6026bd1 100644 --- a/src/app/utils/logs.ts +++ b/src/app/utils/logs.ts @@ -36,7 +36,11 @@ export function reshapeEventLog(evt: LogUnionWithSource): FlattenedLogEvent { logType: evt.logType, timestamp: evt.timestamp, eventInfo: evt.experiment.name || "", - isContextualBandit: appliedContextualBandit(evt.experiment), + // Omitted when false: useSearch matches JSON.stringify(item), so a + // literal false makes every row match a search for "bandit" + ...(appliedContextualBandit(evt.experiment) + ? { isContextualBandit: true } + : {}), details: { experiment: evt.experiment, result: evt.result, From 66ef81bb1fdd4d2cbe7debf47182f57ee4f671ea Mon Sep 17 00:00:00 2001 From: gazzdingo Date: Mon, 31 Aug 2026 17:15:45 -0700 Subject: [PATCH 34/34] Fix three defects from the review Bandit experiments had an empty Results log: `banditResult ?? run()` short-circuited run(), which is what populates the debug log. Always run, and prefer the bandit result afterwards. Remove the amber forced-variation warning, gated on `isForced && cb`, which can never both be true - forcing sets hashUsed false and the SDK then drops the weights that produce cb. The setup check already covers that case with something actionable. Gate the panel on the page's SDK version. DevTools evaluates with its own bundled SDK, so on a pre-1.7 page it would report a bandit assignment the page never made; say so instead. --- src/app/components/ContextualBanditDetail.tsx | 25 ++++++++++++++----- src/app/hooks/useGBSandboxEval.ts | 6 +++-- src/utils/contextualBandits.test.ts | 17 +++++++++++++ src/utils/contextualBandits.ts | 12 +++++++++ 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/app/components/ContextualBanditDetail.tsx b/src/app/components/ContextualBanditDetail.tsx index 90624ba..68d69ab 100644 --- a/src/app/components/ContextualBanditDetail.tsx +++ b/src/app/components/ContextualBanditDetail.tsx @@ -15,7 +15,9 @@ import { ContextualBanditDefinitions, FALLBACK_LEAF_ID, getMatchedContextAttributes, + sdkSupportsContextualBandits, usedFallbackWeights, + CONTEXTUAL_BANDIT_SDK_VERSION, } from "@/utils/contextualBandits"; import { expectsUserContextParam, @@ -91,6 +93,9 @@ export default function ContextualBanditDetail({ const context = getMatchedContextAttributes(definition, cb, attributes || {}); const contextKeys = Object.keys(context || {}); const isForced = forcedVariation !== undefined; + // DevTools evaluates with its own bundled SDK, so without this the panel + // would report a bandit assignment a pre-1.7 page never made + const sdkSupported = sdkSupportsContextualBandits(sdkData?.version); // Without a value for the hash attribute the SDK skips the rule outright, // so there is no assignment and no bandit data at all const hashAttribute = experiment.hashAttribute ?? "id"; @@ -126,6 +131,20 @@ export default function ContextualBanditDetail({ .filter(Boolean) .join(" · "); + if (!sdkSupported) { + return ( + <> +
Contextual Bandit
+ + SDK {sdkData?.version} does not support contextual bandits, so the + page skips this rule. Anything shown below would be DevTools’ + own evaluation, not what the page served. Upgrade to{" "} + {CONTEXTUAL_BANDIT_SDK_VERSION} or later to run it. + + + ); + } + return ( <>
Contextual Bandit
@@ -143,12 +162,6 @@ export default function ContextualBanditDetail({
) : null} - {isForced && cb ? ( - - A forced variation is active, so these weights are overridden and not - what the bandit would serve. - - ) : null} {banditRef && !definition ? ( The bandit {banditRef} is not in the SDK payload. diff --git a/src/app/hooks/useGBSandboxEval.ts b/src/app/hooks/useGBSandboxEval.ts index b14adcf..41ee014 100644 --- a/src/app/hooks/useGBSandboxEval.ts +++ b/src/app/hooks/useGBSandboxEval.ts @@ -192,12 +192,14 @@ export default function useGBSandboxEval() { } [...experiments, ...featureExperiments].forEach((experiment) => { - const banditResult = banditResults.get(experiment.key); growthbook.debug = true; - const result = banditResult ?? growthbook.run(experiment); + // Always run: run() is what populates the debug log, even when the + // bandit's own result is the one we report + const ranResult = growthbook.run(experiment); growthbook.debug = false; const debug = [...log]; log = []; + const result = banditResults.get(experiment.key) ?? ranResult; evaluatedExperiments.push({ key: experiment.key, diff --git a/src/utils/contextualBandits.test.ts b/src/utils/contextualBandits.test.ts index 9bc0019..7e2727d 100644 --- a/src/utils/contextualBandits.test.ts +++ b/src/utils/contextualBandits.test.ts @@ -2,6 +2,7 @@ import { getMatchedContextAttributes, isContextualBandit, ruleVariations, + sdkSupportsContextualBandits, usedFallbackWeights, } from "./contextualBandits"; @@ -201,3 +202,19 @@ describe("stuffed rule meta", () => { ]); }); }); + +describe("sdkSupportsContextualBandits", () => { + it("is false below 1.7.0, where the SDK skips bandit rules", () => { + expect(sdkSupportsContextualBandits("1.6.5")).toBe(false); + expect(sdkSupportsContextualBandits("0.36.0")).toBe(false); + }); + + it("is true from 1.7.0", () => { + expect(sdkSupportsContextualBandits("1.7.0")).toBe(true); + expect(sdkSupportsContextualBandits("1.8.2")).toBe(true); + }); + + it("assumes support when the version is unknown", () => { + expect(sdkSupportsContextualBandits(undefined)).toBe(true); + }); +}); diff --git a/src/utils/contextualBandits.ts b/src/utils/contextualBandits.ts index 53307ee..0f9f9e4 100644 --- a/src/utils/contextualBandits.ts +++ b/src/utils/contextualBandits.ts @@ -1,3 +1,4 @@ +import { paddedVersionString } from "@growthbook/growthbook"; import type { Experiment, FeatureRule } from "@growthbook/growthbook"; // Declared by the SDK but not exported from its entry point @@ -22,6 +23,17 @@ export type ContextualBanditDefinitions = Record< // leafId -1 means no context matched and the SDK used the aggregate weights export const FALLBACK_LEAF_ID = -1; +// Contextual bandits arrived in 1.7.0; older SDKs skip the rule entirely +export const CONTEXTUAL_BANDIT_SDK_VERSION = "1.7.0"; + +export function sdkSupportsContextualBandits(version?: string): boolean { + if (!version) return true; + return ( + paddedVersionString(version) >= + paddedVersionString(CONTEXTUAL_BANDIT_SDK_VERSION) + ); +} + // Bandit rules keep their variations here so pre-1.7 SDKs skip the rule export function ruleVariations( rule: FeatureRule,