Skip to content

[AI-FSSDK] [FSSDK-12813] Normalize decision event campaign_id, variation_id, and entity_id#1158

Open
jaeopt wants to merge 2 commits into
masterfrom
ai/jaeopt/FSSDK-12813-holdout-event
Open

[AI-FSSDK] [FSSDK-12813] Normalize decision event campaign_id, variation_id, and entity_id#1158
jaeopt wants to merge 2 commits into
masterfrom
ai/jaeopt/FSSDK-12813-holdout-event

Conversation

@jaeopt

@jaeopt jaeopt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Normalize three decision-event ID fields uniformly across every decision type (experiment, feature test, rollout, holdout) in the event-builder layer. decisions[].campaign_id and impression events[].entity_id accept any non-empty string (IDs may be opaque, e.g. "default-12345", "layer_abc") and fall back to experiment_id only when empty, null, or missing; decisions[].variation_id retains the stricter numeric-string-only contract and falls back to null for any invalid input. The normalization path is silent and never drops or defers event dispatch.

Changes

  • Added isNonEmptyString (used for campaign_id / entity_id) and isNumericString (used for variation_id) validators, plus normalizeCampaignId and normalizeVariationId helpers in the event builder, applied uniformly in makeDecisionSnapshot with no per-type branching.
  • normalizeCampaignId returns the provided id unchanged when it is a non-empty string of any character content; otherwise substitutes experimentId when that itself is a non-empty string; otherwise returns null.
  • normalizeVariationId returns the provided id unchanged only when it is a non-empty decimal-digit string; otherwise returns null.
  • Impression entity_id reuses the normalized campaign_id value so the two fields are byte-identical on the wire.
  • Audited existing event-builder tests per FR-011 — happy-path fixtures use valid IDs; invalid-input expectations updated to normalized values (non-numeric campaign_id / entity_id now passes through unchanged; non-numeric variation_id still normalizes to null).

Jira Ticket

FSSDK-12813

@coveralls

coveralls commented Jun 25, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 77.71% (+0.02%) from 77.691% — ai/jaeopt/FSSDK-12813-holdout-event into master

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the event-builder layer to normalize decision-event identifier fields consistently across all impression decision types (experiment, feature-test, rollout, holdout), ensuring wire payloads use numeric-string IDs when valid, and deterministic fallbacks when invalid.

Changes:

  • Added numeric-string validation plus normalization helpers for decisions[].campaign_id/impression events[].entity_id (fallback to experiment_id or null) and decisions[].variation_id (fallback to null).
  • Updated impression snapshot construction so events[].entity_id is byte-identical to decisions[].campaign_id.
  • Updated/expanded tests to reflect the new normalization contract and adjusted fixtures to use numeric-string IDs for happy-path expectations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/optimizely/index.tests.js Updates expected decision snapshot output so invalid/empty variation_id is asserted as null (normalized) rather than an empty string.
lib/event_processor/event_builder/log_event.ts Implements normalization helpers and applies them in makeDecisionSnapshot, including ensuring entity_id matches normalized campaign_id.
lib/event_processor/event_builder/log_event.spec.ts Updates existing fixtures to numeric-string IDs and adds comprehensive normalization contract tests across decision types and edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants