Skip to content

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility#453

Open
jaeopt wants to merge 3 commits into
masterfrom
ai/jaeopt/FSSDK-12760-local-datafile
Open

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility#453
jaeopt wants to merge 3 commits into
masterfrom
ai/jaeopt/FSSDK-12760-local-datafile

Conversation

@jaeopt

@jaeopt jaeopt commented Jun 12, 2026

Copy link
Copy Markdown

Summary

Partitions holdouts into two top-level datafile sections so Gen 3 local holdouts ship safely alongside Gen 1/Gen 2 SDKs that would otherwise misapply them as global:

  • holdouts — global only; any includedRules field on these entries is stripped during parsing so section membership is the sole signal for scope.
  • localHoldouts — new top-level section; rule-scoped holdouts that carry their real trafficAllocation. Entries without includedRules are logged via the SDK logger and excluded from evaluation (never promoted to global).

Older SDK versions in production ignore the unknown localHoldouts key entirely, so a single datafile artifact can serve every SDK generation.

Changes

  • pkg/config/datafileprojectconfig/entities/entities.go — add LocalHoldouts []Holdout to the Datafile struct; tighten the Holdout.IncludedRules doc to reflect section-based scope.
  • pkg/config/datafileprojectconfig/mappers/holdout.go — change MapHoldouts to take both sections plus an OptimizelyLogProducer; strip IncludedRules on global-section entries; reject (log + skip) local-section entries with nil IncludedRules; nil-logger safe.
  • pkg/config/datafileprojectconfig/config.go — wire datafile.LocalHoldouts and the logger into MapHoldouts; tighten docstrings on GetGlobalHoldouts / GetHoldoutsForRule and the internal field comments.
  • pkg/entities/experiment.go — tighten the Holdout.IncludedRules and IsGlobal() docstrings to note that DatafileProjectConfig strips IncludedRules on global-section entries at parse time.
  • pkg/config/datafileprojectconfig/mappers/holdout_test.go — refactor existing tests for the new signature and add 12 new tests covering the partitioned behavior, invalid local entries (logged and excluded), includedRules stripping on global entries, mixed sections, backward compat, and a nil-logger guard.
  • pkg/config/datafileprojectconfig/config_test.go — add 3 end-to-end JSON datafile tests (partitioning, backward compat without localHoldouts, stripping includedRules on global entries).

Behavior

Generation Behavior with new datafile
Gen 1 / Gen 2 (released) Unknown localHoldouts key is ignored; only holdouts (global) is parsed and applied. No misapplication, no errors, no log noise.
Gen 3 (this PR) holdouts → global (every entry, IncludedRules stripped); localHoldouts → local (rule-scoped via IncludedRules).

Test Plan

  • go build ./... — passes
  • go vet ./... — passes
  • Full test suite (go test ./...) — all packages pass, no regressions
  • 18 holdout mapper tests pass (6 refactored + 12 new)
  • 3 new end-to-end datafile JSON parsing tests pass

Reference Implementations

This change ports the FSSDK-12760 backward-compat partition that already shipped in:

The Go implementation follows Python's semantics on the corner case of an empty includedRules slice in the localHoldouts section (valid local holdout that targets no rules, not an error), matching the prior IsGlobal() contract added in FSSDK-12369.

Jira

FSSDK-12760

jaeopt and others added 3 commits June 12, 2026 13:18
- Log a warning when the same holdout ID appears in both datafile
  sections (the later entry overwrites the earlier one in the ID map)
- Downgrade missing-includedRules log from Error to Warning — this is
  a data quality issue, not a runtime failure
- Replace brittle TestMapHoldoutsErrorMessageWording (asserted on
  specific words) with TestMapHoldoutsDuplicateIDsAcrossSectionsLogsWarning

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants