diff --git a/visibility-filtering/hydration/exclusive_content_hydrator.rs b/visibility-filtering/hydration/exclusive_content_hydrator.rs index 19ad806b..f1ea36a5 100644 --- a/visibility-filtering/hydration/exclusive_content_hydrator.rs +++ b/visibility-filtering/hydration/exclusive_content_hydrator.rs @@ -1,6 +1,8 @@ use crate::clients::socialgraph_client::SocialgraphClient; -use crate::hydration::metrics::{batch_outcome, record_batch_size, timed_rpc, HydratorOutcome}; -use crate::models::{ExclusiveContentFeatures, TweetId, Viewer}; +use crate::hydration::metrics::{ + batch_outcome, record_batch_size, timed_rpc, HydratorOutcome, +}; +use crate::models::{ExclusiveContentFeatures, ExclusiveHydration, TweetId, Viewer}; use crate::rules::SafetyLevel; use std::collections::{HashMap, HashSet}; use std::sync::Arc; @@ -21,12 +23,12 @@ impl ExclusiveContentHydrator { tweet_ids: &[TweetId], viewer: Viewer, safety_level: SafetyLevel, - ) -> HashMap> { + ) -> HashMap { let raw_ids: Vec = tweet_ids.iter().map(|t| t.0).collect(); let candidate_count = raw_ids.len(); record_batch_size(CLIENT, candidate_count); - let exclusive_controls = timed_rpc( + let (outcome, exclusive_controls) = timed_rpc( CLIENT, "get_exclusive_controls", safety_level, @@ -37,21 +39,28 @@ impl ExclusiveContentHydrator { ) .await; - let root_author_ids: Vec = exclusive_controls - .values() - .filter_map(|r| { - r.as_ref() - .ok() - .and_then(|opt| opt.as_ref()) - .map(|ctrl| ctrl.conversation_author_id) + let authors: HashMap, String>> = exclusive_controls + .into_iter() + .map(|(id, result)| { + ( + id, + result + .map(|opt| opt.map(|ctrl| ctrl.conversation_author_id)) + .map_err(|_| "tes exclusive_controls failed".to_string()), + ) }) + .collect(); + + let root_author_ids: Vec = authors + .values() + .filter_map(|r| r.as_ref().ok().copied().flatten()) .collect::>() .into_iter() .collect(); let super_follows = match viewer.user_id() { Some(vid) if !root_author_ids.is_empty() => { - timed_rpc( + let (_, follows) = timed_rpc( CLIENT, "batch_check_super_follows", safety_level, @@ -61,30 +70,111 @@ impl ExclusiveContentHydrator { self.sg_client .batch_check_super_follows(vid, &root_author_ids), ) - .await + .await; + follows } _ => HashMap::new(), }; - tweet_ids - .iter() - .map(|tweet_id| { - let features = exclusive_controls - .get(&tweet_id.0) - .and_then(|r| r.as_ref().ok()) - .and_then(|opt| opt.as_ref()) - .map(|ctrl| { - let author_id = ctrl.conversation_author_id; - ExclusiveContentFeatures { - conversation_author_id: author_id, - viewer_super_follows_author: super_follows - .get(&author_id) - .copied() - .unwrap_or(false), - } - }); - (*tweet_id, features) + resolve_exclusive( + tweet_ids, + outcome == HydratorOutcome::Timeout, + &authors, + &super_follows, + ) + } +} + +pub(crate) fn resolve_exclusive( + tweet_ids: &[TweetId], + timed_out: bool, + exclusive_authors: &HashMap, String>>, + super_follows: &HashMap, +) -> HashMap { + tweet_ids + .iter() + .map(|tweet_id| { + if timed_out { + return (*tweet_id, ExclusiveHydration::Failed); + } + let hydration = match exclusive_authors.get(&tweet_id.0) { + Some(Err(_)) => ExclusiveHydration::Failed, + Some(Ok(None)) | None => ExclusiveHydration::Public, + Some(Ok(Some(author_id))) => { + ExclusiveHydration::Exclusive(ExclusiveContentFeatures { + conversation_author_id: *author_id, + viewer_super_follows_author: super_follows + .get(author_id) + .copied() + .unwrap_or(false), + }) + } + }; + (*tweet_id, hydration) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn id(n: u64) -> TweetId { + TweetId(n) + } + + #[test] + fn tes_ok_none_is_public() { + let authors = HashMap::from([(1u64, Ok(None))]); + let out = resolve_exclusive(&[id(1)], false, &authors, &HashMap::new()); + assert_eq!(out[&id(1)], ExclusiveHydration::Public); + } + + #[test] + fn tes_ok_some_is_exclusive() { + let authors = HashMap::from([(1u64, Ok(Some(42u64)))]); + let follows = HashMap::from([(42u64, false)]); + let out = resolve_exclusive(&[id(1)], false, &authors, &follows); + assert_eq!( + out[&id(1)], + ExclusiveHydration::Exclusive(ExclusiveContentFeatures { + conversation_author_id: 42, + viewer_super_follows_author: false, }) - .collect() + ); + } + + #[test] + fn tes_err_is_failed() { + let authors = HashMap::from([(1u64, Err("tes unavailable".to_string()))]); + let out = resolve_exclusive(&[id(1)], false, &authors, &HashMap::new()); + assert_eq!(out[&id(1)], ExclusiveHydration::Failed); + } + + #[test] + fn tes_timeout_fails_the_whole_batch() { + let authors = HashMap::new(); + let out = resolve_exclusive(&[id(1), id(2)], true, &authors, &HashMap::new()); + assert_eq!(out[&id(1)], ExclusiveHydration::Failed); + assert_eq!(out[&id(2)], ExclusiveHydration::Failed); + } + + #[test] + fn successful_empty_map_stays_public() { + let out = resolve_exclusive(&[id(1)], false, &HashMap::new(), &HashMap::new()); + assert_eq!(out[&id(1)], ExclusiveHydration::Public); + } + + #[test] + fn subscriber_flag_comes_from_super_follows_map() { + let authors = HashMap::from([(1u64, Ok(Some(42u64)))]); + let follows = HashMap::from([(42u64, true)]); + let out = resolve_exclusive(&[id(1)], false, &authors, &follows); + match &out[&id(1)] { + ExclusiveHydration::Exclusive(features) => { + assert!(features.viewer_super_follows_author); + } + other => panic!("expected exclusive, got {other:?}"), + } } } diff --git a/visibility-filtering/hydration/metrics.rs b/visibility-filtering/hydration/metrics.rs index 6e4a075d..4ee69597 100644 --- a/visibility-filtering/hydration/metrics.rs +++ b/visibility-filtering/hydration/metrics.rs @@ -236,7 +236,7 @@ pub(crate) async fn timed_rpc( timeout: Duration, classify: impl FnOnce(&T) -> HydratorOutcome, fut: impl Future, -) -> T { +) -> (HydratorOutcome, T) { let start = Instant::now(); let (outcome, body) = match tokio::time::timeout(timeout, fut).await { Ok(body) => { @@ -253,7 +253,7 @@ pub(crate) async fn timed_rpc( candidate_count, start.elapsed().as_secs_f64() * 1000.0, ); - body + (outcome, body) } pub(crate) async fn timed_results( diff --git a/visibility-filtering/hydration/mod.rs b/visibility-filtering/hydration/mod.rs index e986da8b..19045154 100644 --- a/visibility-filtering/hydration/mod.rs +++ b/visibility-filtering/hydration/mod.rs @@ -11,7 +11,7 @@ pub mod viewer_hydrator; use crate::clients::gizmoduck_client::GizmoduckLookup; use crate::clients::socialgraph_client::SocialgraphClient; use crate::models::{ - assemble, resolve_candidates, AuthorFeatures, AuthorId, ExclusiveContentFeatures, + assemble, resolve_candidates, AuthorFeatures, AuthorId, ExclusiveHydration, HydratedTweetCandidate, RawCandidate, SafetyLabelMap, TweetCandidateInput, TweetFeatures, TweetId, Viewer, ViewerAuthorRelationship, ViewerFeatures, }; @@ -62,7 +62,7 @@ struct CandidateFeatures { author_features: TweetHydrationBatch, safety_labels: HashMap, relationships: TweetHydrationBatch, - exclusive_content: HashMap>, + exclusive_content: HashMap, } impl CandidateFeatures { @@ -70,6 +70,11 @@ impl CandidateFeatures { candidates .iter() .map(|c| { + let exclusive = self + .exclusive_content + .get(&c.tweet_id) + .cloned() + .unwrap_or_default(); assemble( c, self.tweet_features @@ -82,10 +87,8 @@ impl CandidateFeatures { .cloned() .unwrap_or_default(), self.relationships.get_or_default(&c.tweet_id), - self.exclusive_content - .get(&c.tweet_id) - .cloned() - .unwrap_or_default(), + exclusive.features(), + exclusive.failed(), ) }) .collect() @@ -314,6 +317,7 @@ mod tests { assert_eq!(c.tweet_features.core.source_tweet_id, Some(2)); assert!(c.author_features.is_suspended); assert!(c.relationship.viewer_follows_author); + assert!(!c.exclusive_hydration_failed); } fn raw(tweet_id: u64, request_author_id: Option) -> RawCandidate { diff --git a/visibility-filtering/models/exclusive_content.rs b/visibility-filtering/models/exclusive_content.rs index edf3b70d..c61c6944 100644 --- a/visibility-filtering/models/exclusive_content.rs +++ b/visibility-filtering/models/exclusive_content.rs @@ -1,5 +1,31 @@ -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ExclusiveContentFeatures { pub conversation_author_id: u64, pub viewer_super_follows_author: bool, } + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ExclusiveHydration { + Public, + Exclusive(ExclusiveContentFeatures), + Failed, +} + +impl Default for ExclusiveHydration { + fn default() -> Self { + ExclusiveHydration::Public + } +} + +impl ExclusiveHydration { + pub fn features(&self) -> Option { + match self { + ExclusiveHydration::Exclusive(features) => Some(features.clone()), + ExclusiveHydration::Public | ExclusiveHydration::Failed => None, + } + } + + pub fn failed(&self) -> bool { + matches!(self, ExclusiveHydration::Failed) + } +} diff --git a/visibility-filtering/models/mod.rs b/visibility-filtering/models/mod.rs index 3856762e..aecc2c7f 100644 --- a/visibility-filtering/models/mod.rs +++ b/visibility-filtering/models/mod.rs @@ -6,7 +6,7 @@ pub mod tweet; pub mod viewer; pub use author::{AuthorFeatures, AuthorLabel, AuthorLabelSet}; -pub use exclusive_content::ExclusiveContentFeatures; +pub use exclusive_content::{ExclusiveContentFeatures, ExclusiveHydration}; pub use relationship::ViewerAuthorRelationship; pub use safety_labels::{SafetyLabelMap, SafetyLabelType}; pub use tweet::{CoreFeature, MediaFeature, NsfwFeature, TweetFeatures}; @@ -74,6 +74,7 @@ pub struct HydratedTweetCandidate { pub safety_labels: SafetyLabelMap, pub relationship: ViewerAuthorRelationship, pub exclusive_content: Option, + pub exclusive_hydration_failed: bool, } impl HydratedTweetCandidate { @@ -146,6 +147,7 @@ pub fn assemble( safety_labels: SafetyLabelMap, relationship: ViewerAuthorRelationship, exclusive_content: Option, + exclusive_hydration_failed: bool, ) -> HydratedTweetCandidate { HydratedTweetCandidate { tweet_id: candidate.tweet_id.0, @@ -155,6 +157,7 @@ pub fn assemble( safety_labels, relationship, exclusive_content, + exclusive_hydration_failed, } } diff --git a/visibility-filtering/rules/context.rs b/visibility-filtering/rules/context.rs index 3805ad8f..5d65f976 100644 --- a/visibility-filtering/rules/context.rs +++ b/visibility-filtering/rules/context.rs @@ -191,6 +191,11 @@ impl TweetPredicates<'_> { pub fn is_exclusive(&self) -> bool { self.ctx.candidate.exclusive_content.is_some() } + + #[inline] + pub fn exclusive_hydration_failed(&self) -> bool { + self.ctx.candidate.exclusive_hydration_failed + } } #[derive(Clone, Copy)] diff --git a/visibility-filtering/rules/golden_corpus.rs b/visibility-filtering/rules/golden_corpus.rs index c1ccc397..15b4d991 100644 --- a/visibility-filtering/rules/golden_corpus.rs +++ b/visibility-filtering/rules/golden_corpus.rs @@ -163,6 +163,12 @@ fn exclusive_candidate(viewer_super_follows_author: bool) -> HydratedTweetCandid c } +fn exclusive_hydration_failed_candidate() -> HydratedTweetCandidate { + let mut c = candidate().build(); + c.exclusive_hydration_failed = true; + c +} + fn viewer_in_country(code: &str) -> ViewerFeatures { ViewerFeatures { country_code: Some(code.to_string()), @@ -692,6 +698,22 @@ fn exclusive_content_cases() -> Vec { expected_action: Drop(FilteredReason::ExclusiveTweet), expected_decided_by: Some("DropExclusiveTweetContentRule"), }, + Case { + name: "exclusive_hydration_failure_drops", + level: TimelineHome, + viewer: viewer(VIEWER_ID), + candidate: exclusive_hydration_failed_candidate(), + expected_action: Drop(FilteredReason::ExclusiveTweet), + expected_decided_by: Some("ExclusiveHydrationFailureDropRule"), + }, + Case { + name: "exclusive_hydration_failure_drops_recommendations", + level: TimelineHomeRecommendations, + viewer: viewer(VIEWER_ID), + candidate: exclusive_hydration_failed_candidate(), + expected_action: Drop(FilteredReason::ExclusiveTweet), + expected_decided_by: Some("ExclusiveHydrationFailureDropRule"), + }, ] } diff --git a/visibility-filtering/rules/registry.rs b/visibility-filtering/rules/registry.rs index d0a89bb8..36b92ab7 100644 --- a/visibility-filtering/rules/registry.rs +++ b/visibility-filtering/rules/registry.rs @@ -220,6 +220,7 @@ rust_vf: "SensitiveViewerLoggedOutDropRule", "SensitiveViewerUnderageDropRule", "SensitiveViewerNoStatedAgeDropRule", + "ExclusiveHydrationFailureDropRule", "DropExclusiveTweetContentRule", "NsfwHighPrecisionInterstitialRule", "GoreAndViolenceInterstitialRule", diff --git a/visibility-filtering/rules/tweet_rules.rs b/visibility-filtering/rules/tweet_rules.rs index 9b533e15..87d5e254 100644 --- a/visibility-filtering/rules/tweet_rules.rs +++ b/visibility-filtering/rules/tweet_rules.rs @@ -154,6 +154,13 @@ pub(super) const OON_TWEET_LABEL_DROPS: &[RuleSpec] = &[ }, ]; +fn drop_exclusive_hydration_failure(context: &RuleContext<'_>) -> VfAction { + if context.tweet().exclusive_hydration_failed() { + return VfAction::Drop(FilteredReason::ExclusiveTweet); + } + VfAction::Allow +} + fn drop_exclusive_tweet_content(context: &RuleContext<'_>) -> VfAction { if !context.tweet().is_exclusive() { return VfAction::Allow; @@ -178,10 +185,16 @@ fn drop_exclusive_tweet_content(context: &RuleContext<'_>) -> VfAction { VfAction::Drop(FilteredReason::ExclusiveTweet) } -pub(super) const EXCLUSIVE_TWEET_DROP: &[RuleSpec] = &[RuleSpec::Custom { - name: "DropExclusiveTweetContentRule", - evaluate: drop_exclusive_tweet_content, -}]; +pub(super) const EXCLUSIVE_TWEET_DROP: &[RuleSpec] = &[ + RuleSpec::Custom { + name: "ExclusiveHydrationFailureDropRule", + evaluate: drop_exclusive_hydration_failure, + }, + RuleSpec::Custom { + name: "DropExclusiveTweetContentRule", + evaluate: drop_exclusive_tweet_content, + }, +]; pub(super) const NSFW_AUTHOR_INTERSTITIAL: &[RuleSpec] = &[RuleSpec::Tweet { name: "NsfwAuthorInterstitialRule", @@ -526,7 +539,22 @@ mod tests { #[test] fn exclusive_content_axis() { - let spec = &EXCLUSIVE_TWEET_DROP[0]; + let fail_closed = &EXCLUSIVE_TWEET_DROP[0]; + let spec = &EXCLUSIVE_TWEET_DROP[1]; + assert_eq!(fail_closed.name(), "ExclusiveHydrationFailureDropRule"); + assert_eq!(spec.name(), "DropExclusiveTweetContentRule"); + + let mut failed = candidate().build(); + failed.exclusive_hydration_failed = true; + assert_drops( + fail_closed, + &viewer(VIEWER_ID), + &failed, + &FilteredReason::ExclusiveTweet, + ); + assert_allows(fail_closed, &viewer(VIEWER_ID), &candidate().build()); + assert_allows(spec, &viewer(VIEWER_ID), &failed); + assert_allows(spec, &viewer(VIEWER_ID), &candidate().build()); let exclusive = exclusive_candidate(1, 100, 100);