diff --git a/frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx b/frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx index 503dbf827be2..36b6ae4a92d9 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx @@ -381,6 +381,12 @@ export function FeatureFlagTestingTab({ featureFlag }: { featureFlag: FeatureFla + {/* Explains a non-match the coarse reason code can't convey, such as a + behavioral or realtime cohort whose membership isn't fully resolved here. */} + {result.reason_description && ( + {result.reason_description} + )} + {/* Distinct ID used for rollout/variant bucketing. Only shown when the backend echoes an explicit value — it returns null when a different ID was actually bucketed against, and falling back to the requested ID diff --git a/frontend/src/scenes/persons/RelatedFeatureFlags.tsx b/frontend/src/scenes/persons/RelatedFeatureFlags.tsx index d0e9dd0d0377..2d40fa52c82c 100644 --- a/frontend/src/scenes/persons/RelatedFeatureFlags.tsx +++ b/frontend/src/scenes/persons/RelatedFeatureFlags.tsx @@ -113,11 +113,18 @@ const columns: LemonTableColumns = [ width: 150, render: function Render(_, featureFlag: RelatedFeatureFlag) { const matchesSet = featureFlag.evaluation.reason === FeatureFlagMatchReason.ConditionMatch + const description = featureFlag.evaluation.description return ( -
+
{featureFlag.active ? <>{featureFlagMatchMapping[featureFlag.evaluation.reason]} : '--'} {matchesSet && Set {(featureFlag.evaluation.condition_index ?? 0) + 1}} + + {featureFlag.active && !matchesSet && description && ( + + + + )}
) }, diff --git a/frontend/src/scenes/persons/relatedFeatureFlagsLogic.ts b/frontend/src/scenes/persons/relatedFeatureFlagsLogic.ts index 882097150cad..aa7e772186bd 100644 --- a/frontend/src/scenes/persons/relatedFeatureFlagsLogic.ts +++ b/frontend/src/scenes/persons/relatedFeatureFlagsLogic.ts @@ -22,6 +22,7 @@ export interface RelatedFeatureFlag extends FeatureFlagType { export interface FeatureFlagEvaluationType { reason: FeatureFlagMatchReason condition_index?: number + description?: string | null } export interface RelatedFeatureFlagResponse { diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index cafcf51d8c18..2bbffe7a4e72 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -2558,6 +2558,15 @@ class EvaluationReasonSerializer(serializers.Serializer): allow_null=True, help_text="The index of the condition that matched, if applicable", ) + description = serializers.CharField( + required=False, + allow_null=True, + help_text=( + "Human-readable explanation of the evaluation result. Carries the extra signal when the " + "reason code is coarse, for example a non-match decided by a behavioral or realtime cohort " + "whose membership is not fully evaluated here, which can disagree with the cohort's member list." + ), + ) class FlagEvaluationResultSerializer(serializers.Serializer): @@ -2706,6 +2715,15 @@ class FeatureFlagTestEvaluationResponseSerializer(serializers.Serializer): flag_key = serializers.CharField(help_text="Feature flag key") result = serializers.JSONField(help_text="The evaluated value of the feature flag (boolean or variant key string)") reason = serializers.CharField(help_text="The reason for the evaluation result") + reason_description = serializers.CharField( + required=False, + allow_null=True, + help_text=( + "Human-readable explanation of the evaluation result. Set when the reason code is coarse, " + "for example a non-match decided by a behavioral or realtime cohort whose membership is not " + "fully evaluated here, which can disagree with the cohort's member list." + ), + ) condition_index = serializers.IntegerField( allow_null=True, help_text="The index of the condition that matched, if applicable" ) @@ -4153,6 +4171,7 @@ def evaluation_reasons(self, request: request.Request, **kwargs): "evaluation": { "reason": reason_data.get("code", "unknown"), "condition_index": reason_data.get("condition_index"), + "description": reason_data.get("description"), }, } @@ -4479,6 +4498,7 @@ def test_evaluation(self, request: request.Request, **kwargs): # Initialize defaults condition_index = None + reason_description = None payload = None detailed_conditions: list[dict] = [] result: bool | str = False @@ -4496,6 +4516,7 @@ def test_evaluation(self, request: request.Request, **kwargs): # Extract values from the correct nested structures reason = reason_data.get("code", "unknown") if reason_data else "unknown" + reason_description = reason_data.get("description") if reason_data else None condition_index = reason_data.get("condition_index") if reason_data else None payload = metadata.get("payload") if metadata else None # Extract conditions from flag result (only valid path per Rust FlagDetails contract). @@ -4552,6 +4573,7 @@ def test_evaluation(self, request: request.Request, **kwargs): "flag_key": feature_flag.key, "result": result, "reason": reason, + "reason_description": reason_description, "condition_index": condition_index, "payload": payload, "person_properties": _filter_person_properties_for_flag( diff --git a/products/feature_flags/frontend/generated/api.schemas.ts b/products/feature_flags/frontend/generated/api.schemas.ts index 6e4c0c3d6c30..2bb7a03c2430 100644 --- a/products/feature_flags/frontend/generated/api.schemas.ts +++ b/products/feature_flags/frontend/generated/api.schemas.ts @@ -1248,6 +1248,11 @@ export interface FeatureFlagTestEvaluationResponseApi { result: unknown /** The reason for the evaluation result */ reason: string + /** + * Human-readable explanation of the evaluation result. Set when the reason code is coarse, for example a non-match decided by a behavioral or realtime cohort whose membership is not fully evaluated here, which can disagree with the cohort's member list. + * @nullable + */ + reason_description?: string | null /** * The index of the condition that matched, if applicable * @nullable diff --git a/products/product_analytics/backend/tests/api/__snapshots__/test_insight.ambr b/products/product_analytics/backend/tests/api/__snapshots__/test_insight.ambr index 75d277e22f23..56e4d75f5945 100644 --- a/products/product_analytics/backend/tests/api/__snapshots__/test_insight.ambr +++ b/products/product_analytics/backend/tests/api/__snapshots__/test_insight.ambr @@ -34,7 +34,7 @@ e.`$window_id` AS `$window_id`, if(equals(e.event, 'user signed up'), 1, 0) AS step_0, if(equals(e.event, 'user did things'), 1, 0) AS step_1, - ([replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', '')] AS value) AS prop_basic, + ([e__person.properties___fish] AS value) AS prop_basic, prop_basic AS prop FROM events AS e LEFT OUTER JOIN @@ -44,6 +44,16 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'fish'), ''), 'null'), '^"|"$', '') AS properties___fish + FROM person + WHERE and(equals(person.team_id, 99999), in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))))) SETTINGS optimize_aggregation_in_order=1) AS e__person ON equals(if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id), e__person.id) WHERE and(equals(e.team_id, 99999), and(and(greaterOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), lessOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))), in(e.event, tuple('user did things', 'user signed up')), ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0)), or(equals(step_0, 1), equals(step_1, 1)))) GROUP BY aggregation_target HAVING ifNull(greaterOrEquals(step_reached, 0), 0)) @@ -111,7 +121,17 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) - WHERE and(equals(e.team_id, 99999), and(and(greaterOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), lessOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))), in(e.event, tuple('user did things', 'user signed up')), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))), or(equals(step_0, 1), equals(step_1, 1)))) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'fish'), ''), 'null'), '^"|"$', '') AS properties___fish + FROM person + WHERE and(equals(person.team_id, 99999), in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))))) SETTINGS optimize_aggregation_in_order=1) AS e__person ON equals(if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id), e__person.id) + WHERE and(equals(e.team_id, 99999), and(and(greaterOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), lessOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))), in(e.event, tuple('user did things', 'user signed up')), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(e__person.properties___fish, '%fish%'), 0))), or(equals(step_0, 1), equals(step_1, 1)))) GROUP BY aggregation_target HAVING ifNull(greaterOrEquals(step_reached, 0), 0)) GROUP BY breakdown @@ -168,8 +188,8 @@ e.uuid AS uuid, e.`$session_id` AS `$session_id`, e.`$window_id` AS `$window_id`, - if(and(equals(e.event, 'user signed up'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))), 1, 0) AS step_0, - if(and(equals(e.event, 'user did things'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))), 1, 0) AS step_1 + if(and(equals(e.event, 'user signed up'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(e__person.properties___fish, '%fish%'), 0))), 1, 0) AS step_0, + if(and(equals(e.event, 'user did things'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(e__person.properties___fish, '%fish%'), 0))), 1, 0) AS step_1 FROM events AS e LEFT OUTER JOIN (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, @@ -178,6 +198,16 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'fish'), ''), 'null'), '^"|"$', '') AS properties___fish + FROM person + WHERE and(equals(person.team_id, 99999), in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))))) SETTINGS optimize_aggregation_in_order=1) AS e__person ON equals(if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id), e__person.id) WHERE and(equals(e.team_id, 99999), and(and(greaterOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), lessOrEquals(e.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))), in(e.event, tuple('user did things', 'user signed up'))), or(equals(step_0, 1), equals(step_1, 1)))) GROUP BY aggregation_target HAVING ifNull(greaterOrEquals(step_reached, 0), 0)) @@ -294,7 +324,24 @@ (SELECT count() AS total, toStartOfDay(toTimeZone(e.timestamp, 'UTC')) AS day_start FROM events AS e - WHERE and(equals(e.team_id, 99999), greaterOrEquals(e.timestamp, toStartOfInterval(assumeNotNull(toDateTime('2012-01-08 00:00:00', 'UTC')), toIntervalDay(1))), lessOrEquals(e.timestamp, assumeNotNull(toDateTime('2012-01-15 23:59:59', 'UTC'))), equals(e.event, '$pageview'), and(ifNull(greater(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))) + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'fish'), ''), 'null'), '^"|"$', '') AS properties___fish + FROM person + WHERE and(equals(person.team_id, 99999), in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))))) SETTINGS optimize_aggregation_in_order=1) AS e__person ON equals(if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id), e__person.id) + WHERE and(equals(e.team_id, 99999), greaterOrEquals(e.timestamp, toStartOfInterval(assumeNotNull(toDateTime('2012-01-08 00:00:00', 'UTC')), toIntervalDay(1))), lessOrEquals(e.timestamp, assumeNotNull(toDateTime('2012-01-15 23:59:59', 'UTC'))), equals(e.event, '$pageview'), and(ifNull(greater(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(e__person.properties___fish, '%fish%'), 0))) GROUP BY day_start) GROUP BY day_start ORDER BY day_start ASC) @@ -326,7 +373,24 @@ (SELECT count() AS total, toStartOfDay(toTimeZone(e.timestamp, 'UTC')) AS day_start FROM events AS e - WHERE and(equals(e.team_id, 99999), greaterOrEquals(e.timestamp, toStartOfInterval(assumeNotNull(toDateTime('2012-01-08 00:00:00', 'UTC')), toIntervalDay(1))), lessOrEquals(e.timestamp, assumeNotNull(toDateTime('2012-01-15 23:59:59', 'UTC'))), equals(e.event, '$pageview'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))) + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'fish'), ''), 'null'), '^"|"$', '') AS properties___fish + FROM person + WHERE and(equals(person.team_id, 99999), in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))))) SETTINGS optimize_aggregation_in_order=1) AS e__person ON equals(if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id), e__person.id) + WHERE and(equals(e.team_id, 99999), greaterOrEquals(e.timestamp, toStartOfInterval(assumeNotNull(toDateTime('2012-01-08 00:00:00', 'UTC')), toIntervalDay(1))), lessOrEquals(e.timestamp, assumeNotNull(toDateTime('2012-01-15 23:59:59', 'UTC'))), equals(e.event, '$pageview'), and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(e.properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), ifNull(like(e__person.properties___fish, '%fish%'), 0))) GROUP BY day_start) GROUP BY day_start ORDER BY day_start ASC) diff --git a/rust/feature-flags/src/api/types.rs b/rust/feature-flags/src/api/types.rs index d2eb1157e3c8..e8ad2a5424ae 100644 --- a/rust/feature-flags/src/api/types.rs +++ b/rust/feature-flags/src/api/types.rs @@ -575,6 +575,9 @@ impl FromFeatureAndMatch for FlagDetails { FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated => { Some("No matching condition set (group conditions were not evaluated because no group type was provided)".to_string()) } + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated => { + Some("This condition targets a behavioral or realtime cohort. Its membership is not fully evaluated here, so this result can differ from the cohort's member list.".to_string()) + } FeatureFlagMatchReason::OutOfRolloutBound => Some("Out of rollout bound".to_string()), FeatureFlagMatchReason::NoGroupType => Some("No group type".to_string()), FeatureFlagMatchReason::SuperConditionValue => { diff --git a/rust/feature-flags/src/cohorts/cohort_models.rs b/rust/feature-flags/src/cohorts/cohort_models.rs index 064a141dfbce..67751359fa95 100644 --- a/rust/feature-flags/src/cohorts/cohort_models.rs +++ b/rust/feature-flags/src/cohorts/cohort_models.rs @@ -174,7 +174,7 @@ impl Cohort { /// field existed, or a cohort with no filters) defaults to `false` — the safe choice, /// since it routes the cohort through legacy dynamic filter evaluation instead of the /// realtime `cohort_membership` table. - fn has_behavioral_condition(&self) -> bool { + pub(crate) fn has_behavioral_condition(&self) -> bool { self.condition_type .as_ref() .and_then(|value| value.as_object()) diff --git a/rust/feature-flags/src/flags/flag_match_reason.rs b/rust/feature-flags/src/flags/flag_match_reason.rs index 04b0f5a48124..2d11c8cc9c94 100644 --- a/rust/feature-flags/src/flags/flag_match_reason.rs +++ b/rust/feature-flags/src/flags/flag_match_reason.rs @@ -19,6 +19,10 @@ pub enum FeatureFlagMatchReason { /// enriched description carries the extra signal about skipped groups. #[strum(serialize = "no_condition_match_groups_not_evaluated")] NoConditionMatchGroupsNotEvaluated, + /// Person conditions didn't match, but the deciding cohort has a behavioral or lifecycle + /// leaf this evaluator can't fully resolve, so the non-match can disagree with the cohort. + #[strum(serialize = "no_condition_match_cohort_not_evaluated")] + NoConditionMatchCohortNotEvaluated, #[strum(serialize = "holdout_condition_value")] HoldoutConditionValue, #[strum(serialize = "flag_disabled")] @@ -42,6 +46,7 @@ impl FeatureFlagMatchReason { FeatureFlagMatchReason::OutOfRolloutBound => 3, FeatureFlagMatchReason::NoConditionMatch => 2, FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated => 2, + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated => 2, FeatureFlagMatchReason::NoGroupType => 1, FeatureFlagMatchReason::FlagDisabled => 0, FeatureFlagMatchReason::MissingDependency => -1, @@ -71,6 +76,7 @@ impl std::fmt::Display for FeatureFlagMatchReason { FeatureFlagMatchReason::ConditionMatch => "condition_match", FeatureFlagMatchReason::NoConditionMatch => "no_condition_match", FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated => "no_condition_match", + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated => "no_condition_match", FeatureFlagMatchReason::OutOfRolloutBound => "out_of_rollout_bound", FeatureFlagMatchReason::NoGroupType => "no_group_type", FeatureFlagMatchReason::HoldoutConditionValue => "holdout_condition_value", @@ -93,6 +99,7 @@ mod tests { FeatureFlagMatchReason::NoGroupType, // 1 FeatureFlagMatchReason::NoConditionMatch, // 2 FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated, // 2 (same tier) + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated, // 2 (same tier) FeatureFlagMatchReason::OutOfRolloutBound, // 3 FeatureFlagMatchReason::ConditionMatch, // 4 FeatureFlagMatchReason::HoldoutConditionValue, // 5 @@ -131,6 +138,10 @@ mod tests { FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated.to_string(), "no_condition_match" ); + assert_eq!( + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated.to_string(), + "no_condition_match" + ); assert_eq!( FeatureFlagMatchReason::FlagDisabled.to_string(), "flag_disabled" diff --git a/rust/feature-flags/src/flags/flag_matching.rs b/rust/feature-flags/src/flags/flag_matching.rs index 932fdf3bffef..f398acbd5950 100644 --- a/rust/feature-flags/src/flags/flag_matching.rs +++ b/rust/feature-flags/src/flags/flag_matching.rs @@ -1340,6 +1340,7 @@ impl FeatureFlagMatcher { let mut highest_match = FeatureFlagMatchReason::NoGroupType; let mut highest_index = None; let mut had_skipped_group_conditions = false; + let mut had_unevaluable_cohort_conditions = false; // Lazily compute properties per aggregation type. Person and group properties are // cached separately so conditions with different aggregation modes can share them. @@ -1521,6 +1522,10 @@ impl FeatureFlagMatcher { request_hash_key_override, )?; + if reason == FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated { + had_unevaluable_cohort_conditions = true; + } + // OutOfRolloutBound means the condition's property filters (if any) already // matched and only the rollout check failed, so re-evaluating later groups // can't change the outcome. @@ -1595,9 +1600,14 @@ impl FeatureFlagMatcher { // the reason to carry a richer description. The API code still serializes as // "no_condition_match" for backward compatibility, but the description tells the // caller about the skipped group conditions. - if highest_match == FeatureFlagMatchReason::NoConditionMatch && had_skipped_group_conditions - { - highest_match = FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated; + // A cohort that can't be fully evaluated takes precedence over a skipped group + // condition, because it reaches real flag traffic. + if highest_match == FeatureFlagMatchReason::NoConditionMatch { + if had_unevaluable_cohort_conditions { + highest_match = FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated; + } else if had_skipped_group_conditions { + highest_match = FeatureFlagMatchReason::NoConditionMatchGroupsNotEvaluated; + } } // Return with the highest_match reason and index even if no conditions matched @@ -1732,8 +1742,22 @@ impl FeatureFlagMatcher { let cohort_props = property_context .person_properties .unwrap_or(&*EMPTY_PROPERTY_MAP); - if !self.evaluate_cohort_filters(&cohort_filters, cohort_props, cohorts)? { - return Ok((false, FeatureFlagMatchReason::NoConditionMatch)); + if !self.evaluate_cohort_filters(&cohort_filters, cohort_props, cohorts.clone())? { + // A non-match isn't trustworthy when a targeted cohort has a behavioral or + // lifecycle leaf the dynamic path can't resolve, so flag it with a distinct reason. + let cohort_membership_unresolved = cohort_filters.iter().any(|filter| { + filter.get_cohort_id().is_some_and(|cohort_id| { + cohorts + .iter() + .any(|c| c.id == cohort_id && c.has_behavioral_condition()) + }) + }); + let reason = if cohort_membership_unresolved { + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated + } else { + FeatureFlagMatchReason::NoConditionMatch + }; + return Ok((false, reason)); } } } diff --git a/rust/feature-flags/src/flags/test_flag_matching.rs b/rust/feature-flags/src/flags/test_flag_matching.rs index a40d8e9dd95c..f211af374b5e 100644 --- a/rust/feature-flags/src/flags/test_flag_matching.rs +++ b/rust/feature-flags/src/flags/test_flag_matching.rs @@ -3970,6 +3970,120 @@ mod tests { ); } + #[tokio::test] + async fn test_behavioral_cohort_non_match_surfaces_cohort_not_evaluated_reason() { + // A behavioral-cohort non-match must report NoConditionMatchCohortNotEvaluated, not a bare + // NoConditionMatch: the evaluator can't resolve behavioral membership, so the negative is unreliable. + let context = TestContext::new(None).await; + let cohort_cache = Arc::new(CohortCacheManager::new( + context.non_persons_reader.clone(), + None, + None, + )); + let team = context.insert_new_team(None).await.unwrap(); + + let cohort = context + .insert_cohort_with_type_and_condition_type( + team.id, + Some("Behavioral Cohort".to_string()), + plan_cohort_filters("enterprise"), + false, + Some(CohortType::Realtime), + Some(Utc::now()), + None, + Some(behavioral_condition_type()), + None, + ) + .await + .unwrap(); + + let distinct_id = "behavioral_non_member".to_string(); + // Person does NOT satisfy the cohort filters, so dynamic evaluation is a non-match. + context + .insert_person(team.id, distinct_id.clone(), Some(json!({"plan": "free"}))) + .await + .unwrap(); + + let flag = flag_targeting_cohort(team.id, cohort.id); + + let mut matcher = FeatureFlagMatcher::new( + distinct_id.clone(), + None, + team.id, + context.create_postgres_router(), + cohort_cache, + empty_group_type_cache(), + None, + ) + .with_realtime_cohort_evaluation(false); + + matcher + .prepare_flag_evaluation_state(&[&flag]) + .await + .unwrap(); + + let result = matcher.get_match(&flag, None, None, None, &None).unwrap(); + + assert!(!result.matches); + assert_eq!( + result.reason, + FeatureFlagMatchReason::NoConditionMatchCohortNotEvaluated + ); + // Serializes as the backward-compatible code; the enriched signal rides the description. + assert_eq!(result.reason.to_string(), "no_condition_match"); + } + + #[tokio::test] + async fn test_person_property_cohort_non_match_keeps_plain_no_condition_match_reason() { + // A cohort without a behavioral condition is fully evaluable from person properties, + // so a genuine non-match must stay NoConditionMatch and not be over-labeled. + let context = TestContext::new(None).await; + let cohort_cache = Arc::new(CohortCacheManager::new( + context.non_persons_reader.clone(), + None, + None, + )); + let team = context.insert_new_team(None).await.unwrap(); + + let cohort = context + .insert_cohort( + team.id, + Some("Person Property Cohort".to_string()), + plan_cohort_filters("enterprise"), + false, + ) + .await + .unwrap(); + + let distinct_id = "person_property_non_member".to_string(); + context + .insert_person(team.id, distinct_id.clone(), Some(json!({"plan": "free"}))) + .await + .unwrap(); + + let flag = flag_targeting_cohort(team.id, cohort.id); + + let mut matcher = FeatureFlagMatcher::new( + distinct_id.clone(), + None, + team.id, + context.create_postgres_router(), + cohort_cache, + empty_group_type_cache(), + None, + ); + + matcher + .prepare_flag_evaluation_state(&[&flag]) + .await + .unwrap(); + + let result = matcher.get_match(&flag, None, None, None, &None).unwrap(); + + assert!(!result.matches); + assert_eq!(result.reason, FeatureFlagMatchReason::NoConditionMatch); + } + #[tokio::test] async fn test_disambiguated_policy_person_stamp_only_falls_back_to_dynamic() { let context = TestContext::new(None).await; diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index d8a98567d3e3..3ffff6ac4b1f 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -38072,6 +38072,11 @@ export namespace Schemas { result: unknown; /** The reason for the evaluation result */ reason: string; + /** + * Human-readable explanation of the evaluation result. Set when the reason code is coarse, for example a non-match decided by a behavioral or realtime cohort whose membership is not fully evaluated here, which can disagree with the cohort's member list. + * @nullable + */ + reason_description?: string | null; /** * The index of the condition that matched, if applicable * @nullable