Skip to content

feat: experiment metadata on flags, exposure gated on inExperiment - #420

Merged
Zaimwa9 merged 2 commits into
mainfrom
feat/experiment-metadata
Sep 22, 2026
Merged

Zaimwa9 merged 2 commits into
mainfrom
feat/experiment-metadata

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Core now sends metadata.experiment { id, name, in_experiment } on /identities flags while the feature's experiment is running (Flagsmith/flagsmith#8532).

  • IFlagsmithFeature.experiment?: { id, name, inExperiment }, lifted from metadata.experiment. undefined on older servers.
  • getExperimentFlag and useExperiment fire the exposure only when inExperiment is true, instead of guessing from variant. Flag data is still returned otherwise.
  • Exposure events carry metadata.experiment_id for attribution.
  • useFlags propagates experiment through the React bindings.

Release after Core (Flagsmith/flagsmith#8532) and Edge (Flagsmith/edge-api#719) are live: against older servers no flag carries experiment, so no exposure is recorded.

How did you test this code?

Jest: new test/experiment.test.ts plus extended React event, variant and type tests, 131 tests passing.
tsc clean.
Manual QA against staging once Core and Edge are deployed.

@Zaimwa9
Zaimwa9 force-pushed the feat/experiment-metadata branch 2 times, most recently from e8b9dae to b82e510 Compare September 16, 2026 08:09
@Zaimwa9
Zaimwa9 force-pushed the feat/experiment-metadata branch from b82e510 to 95253d5 Compare September 16, 2026 08:12
@Zaimwa9
Zaimwa9 marked this pull request as ready for review September 18, 2026 13:20
@Zaimwa9
Zaimwa9 requested a review from a team as a code owner September 18, 2026 13:20
@Zaimwa9
Zaimwa9 requested review from talissoncosta and removed request for a team September 18, 2026 13:20
@Zaimwa9

Zaimwa9 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Comment thread react.tsx Outdated
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

TL;DR: Replacing a running experiment with a new one that resolves to the same assignment drops the new experiment's exposure, so its results are undercounted. The two completed Test checks passed; Jest is not installed locally, so I could not rerun the suite here.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 4/5

🟠 Majors

  • react.tsx:95 — experiment replacements with the same assignment are not exposed.
📝 Walkthrough
  • Flag mapping - lifts the identity response's experiment metadata into public flag objects.
  • Exposure tracking - requires enrolled experiment metadata and attaches the experiment ID for attribution.
  • React bindings - pass experiment metadata through useFlags and use it to gate useExperiment.
🧪 How to verify
  1. Add a React regression that refreshes an enrolled flag from experiment ID 42 to 43 without changing identity, value, or variant; expect two exposures with the two IDs.
  2. Repeat that scenario through getExperimentFlag before flushing the event buffer.
  3. Confirm flags without metadata and flags with in_experiment: false still emit no exposure.
  4. Run npm ci && npm test -- --runInBand && npm run typecheck.
    Automate: Cover the ID-only experiment replacement in both the React hook and core event dedupe paths.

Product take: Accurate exposure attribution is core to experiment decisions. This is a solid capability addition, but missing the first exposure after an experiment replacement can skew results.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

A tiny key omission is enough to lose an experiment’s evidence · reviewed at 95253d5

@Zaimwa9

Zaimwa9 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Comment thread react.tsx
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

TL;DR: The experiment metadata, enrolment gate, and attribution ID are wired through, but the React bindings retain an old experiment name after a name-only server refresh. The completed Test check passed.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 3/5

🟠 Majors

  • react.tsx:84 — React experiment metadata does not refresh after an experiment rename.

⚖️ Acknowledged

  • Include the experiment ID in exposure identity — thread resolved by @Zaimwa9
📝 Walkthrough
  • Core flag mapping - lifts experiment metadata from identity responses, gates exposure recording on enrolment, and adds the experiment ID to exposure metadata.
  • React bindings - propagates experiment metadata through useFlags and changes the exposure effect keys for assignment changes.
  • Event processing - includes the experiment ID in the within-window exposure deduplication key.
🧪 How to verify
  1. In a React hook test, refresh an identified flag with the same value, variant, experiment ID, and enrolment but a different experiment name; assert both useFlags and useExperiment expose the new name.
  2. Confirm that this name-only refresh does not enqueue a second exposure event.
  3. Run npm test -- --runInBand test/react-events.test.tsx test/react-variant.test.tsx test/experiment.test.ts.
  4. Run npm run typecheck.
    Automate: Keep the name-only metadata refresh case as a React regression test.

Product take: Experiment labels can remain stale in React UIs after a rename, even though the SDK has fetched the renamed metadata. This is a solid experimentation capability, but the displayed metadata needs to stay coherent with the refreshed assignment.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

The experiment knows its new name; the hook should too. · reviewed at 348822d

@talissoncosta talissoncosta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@Zaimwa9
Zaimwa9 merged commit 530a7cc into main Sep 22, 2026
1 check passed
@Zaimwa9
Zaimwa9 deleted the feat/experiment-metadata branch September 22, 2026 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants