[AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules

[esrakartalOpt]

Summary

Implements the exclude_targeted_deliveries field on Holdout objects. When enabled, users bucketed into a holdout are still served Targeted Delivery rules normally, while A/B Test and Multi-Armed Bandit rules continue to respect the holdout.

Changes

• Added exclude_targeted_deliveries boolean to holdout and local holdout JSON schemas in constants
• Updated decision service to defer holdout decisions for Targeted Delivery rules when the flag is true
• Added unit tests covering all combinations of the flag with different rule types and backward compatibility

Jira Ticket

FSSDK-12735

[esrakartalOpt]

Looks good to me 👍 But, due to I am the AI owner, I cannot approve or merge the PR.

[pvcraven]

Claude gives me the stuff below. Might be worth seeing if legit?

1. Dead code path: global_holdout_decision threading through the experiment-rule chain never fires in production.
   In get_decision_for_flag (lib/optimizely/decision_service.rb:224-232), get_variation_for_feature_experiment is only invoked inside unless exclude_td_holdout_decision, and it's called without
   passing exclude_td_holdout_decision — so the global_holdout_decision parameter added to get_variation_for_feature_experiment (line 339) and consumed at get_variation_from_experiment_rule Step 2
   (line 475) can never be non-nil via any real call path from decide/decide_all/decide_for_keys. The actual ETD behavior for experiments is fully implemented by the outer skip (unless
   exclude_td_holdout_decision) — the inner plumbing is unreachable.
   The new spec tests (e.g. "blocks all experiment rules when global_holdout_decision is set", spec/decision_service_holdout_spec.rb:1097) call get_variation_from_experiment_rule directly with a
   synthetic holdout_decision argument, exercising code that decide() never triggers. This gives false confidence — it verifies a branch, not the integration behavior.
   Similarly, get_variation_for_feature_rollout's new global_holdout_decision param is threaded all the way to get_variation_from_delivery_rule, where it's renamed _global_holdout_decision and
   explicitly ignored ("No blocking here"). That's ~4 signatures and a new VariationResult/DecisionResult field added for logic that has no effect at runtime.
   Recommend either removing this unused plumbing (keep the simpler outer skip, which is what actually implements the feature), or, if it's meant to be a safety net for other/future callers, add
   an integration test that exercises it via get_decision_for_flag/decide rather than only via direct private-method calls.

2. Multiple ETD-flagged global holdouts silently drop earlier matches.
   In the holdouts.each loop (decision_service.rb:203-220), if more than one global holdout has excludeTargetedDeliveries == true and both match the user, exclude_td_holdout_decision is
   overwritten by the last match — the first holdout's decision (and its impression) is lost. Likely a rare config, but worth a comment or explicit "first match wins" guard if that's intended.

3. excludeTargetedDeliveries added to localHoldouts schema but never read for local holdouts.
   lib/optimizely/helpers/constants.rb adds the field to both the global holdouts and localHoldouts schemas (lines ~349 and ~372). But get_variation_from_experiment_rule's local-holdout check
   (Step 3) and get_variation_from_delivery_rule's local-holdout check (Step 3) block unconditionally, never inspecting excludeTargetedDeliveries on the local holdout. This is confirmed
   intentional by the new spec ("local holdout MUST still apply — excludeTargetedDeliveries is ignored for local holdouts"), but that makes the schema addition to localHoldouts misleading —
   datafile authors/other tooling could reasonably assume it has an effect there. Consider dropping the field from the local schema, or adding a code comment explaining it's global-only by design.