From 3b75e0be4b9ec0aae5f37b7f7d6efb8d7eb9eef8 Mon Sep 17 00:00:00 2001 From: Jon Bailey <297513015+Pitchfork-and-Torch@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:11:32 -0400 Subject: [PATCH] Fail closed when gizmoduck user-label lookup is Err or missing GizmoduckAuthorHydrator kept Failed/omitted reads in the batch, then assemble used get_or_default so SpamHighRecall and LowQuality looked absent. Omit those ids before assemble. Confirmed NotFound stays unlabeled. --- .../hydration/gizmoduck_hydrator.rs | 95 ++++++++++++++++++- visibility-filtering/hydration/mod.rs | 34 ++++++- 2 files changed, 125 insertions(+), 4 deletions(-) diff --git a/visibility-filtering/hydration/gizmoduck_hydrator.rs b/visibility-filtering/hydration/gizmoduck_hydrator.rs index 2f07001f..f3d9cad1 100644 --- a/visibility-filtering/hydration/gizmoduck_hydrator.rs +++ b/visibility-filtering/hydration/gizmoduck_hydrator.rs @@ -1,5 +1,7 @@ use crate::clients::gizmoduck_client::GizmoduckLookup; -use crate::hydration::batch::{AuthorHydrationBatch, HydrationBatch, TweetHydrationBatch}; +use crate::hydration::batch::{ + AuthorHydrationBatch, Hydrated, HydrationBatch, TweetHydrationBatch, +}; use crate::hydration::fallback_cache::FallbackCache; use crate::hydration::metrics::{record_batch_size, timed_results}; use crate::hydration::{keyed_by_author, tweets_per_author}; @@ -77,6 +79,28 @@ impl GizmoduckAuthorHydrator { } } +/// Failed or omitted gizmoduck reads must not look unlabeled. +/// `SpamHighRecallUserLabelRule` (and sibling user-label drops) only check +/// type presence on `AuthorFeatures`. `get_or_default` turns Failed into an +/// empty `UserLabelSet`, so a store error would let a spam / low-quality +/// author rank. Confirmed `NotFound` stays unlabeled. +pub(crate) fn author_lookup_failed(hydrated: Option<&Hydrated>) -> bool { + match hydrated { + Some(Hydrated::Found(_)) | Some(Hydrated::NotFound) => false, + Some(Hydrated::Failed(_)) | None => true, + } +} + +pub(crate) fn retain_candidates_with_usable_author_features( + candidates: Vec, + author_features: &TweetHydrationBatch, +) -> Vec { + candidates + .into_iter() + .filter(|c| !author_lookup_failed(author_features.hydrated(&c.tweet_id))) + .collect() +} + fn author_features(user_result: GizmoduckUserResult) -> AuthorFeatures { user_result .user @@ -257,7 +281,7 @@ mod tests { } #[test] - fn error_and_missing_author_results_fail_open_but_stay_failed() { + fn error_and_missing_author_results_stay_failed_and_are_not_usable() { let (a10, a20) = (candidate(1, 10).author_id, candidate(2, 20).author_id); let user_results: AuthorHydrationBatch = HydrationBatch::from_results( [a10, a20], @@ -273,9 +297,74 @@ mod tests { by_tweet.hydrated(&tweet_id), Some(Hydrated::Failed(_)) )); + assert!(author_lookup_failed(by_tweet.hydrated(&tweet_id))); + // get_or_default still looks unlabeled — that is why Failed must + // be omitted before assemble, not assembled as empty labels. let feature = by_tweet.get_or_default(&tweet_id); assert!(!feature.is_suspended); - assert!(!feature.is_deactivated); + assert!(!feature.user_labels.has_label(LabelValue::SPAM_HIGH_RECALL)); + assert!(!feature.user_labels.has_label(LabelValue::LOW_QUALITY)); } + + let kept = retain_candidates_with_usable_author_features( + vec![candidate(1, 10), candidate(2, 20)], + &by_tweet, + ); + assert!(kept.is_empty()); + } + + #[test] + fn confirmed_not_found_stays_unlabeled_and_usable() { + let by_tweet: TweetHydrationBatch = HydrationBatch::from_results( + [TweetId(1)], + HashMap::from([(TweetId(1), Ok::<_, anyhow::Error>(None))]), + ); + + assert!(matches!( + by_tweet.hydrated(&TweetId(1)), + Some(Hydrated::NotFound) + )); + assert!(!author_lookup_failed(by_tweet.hydrated(&TweetId(1)))); + assert!(!by_tweet + .get_or_default(&TweetId(1)) + .user_labels + .has_label(LabelValue::SPAM_HIGH_RECALL)); + + let kept = retain_candidates_with_usable_author_features(vec![candidate(1, 10)], &by_tweet); + assert_eq!(kept.len(), 1); + assert_eq!(kept[0].tweet_id, TweetId(1)); + } + + #[test] + fn found_spam_and_low_quality_labels_stay_usable() { + let labeled = AuthorFeatures { + user_labels: UserLabelSet::new( + [LabelValue::SPAM_HIGH_RECALL, LabelValue::LOW_QUALITY] + .into_iter() + .collect(), + ), + ..Default::default() + }; + let by_tweet: TweetHydrationBatch = HydrationBatch::from_results( + [TweetId(1)], + HashMap::from([(TweetId(1), Ok::<_, anyhow::Error>(Some(labeled)))]), + ); + + assert!(!author_lookup_failed(by_tweet.hydrated(&TweetId(1)))); + let feature = by_tweet.get(&TweetId(1)).expect("found"); + assert!(feature.user_labels.has_label(LabelValue::SPAM_HIGH_RECALL)); + assert!(feature.user_labels.has_label(LabelValue::LOW_QUALITY)); + + let kept = retain_candidates_with_usable_author_features(vec![candidate(1, 10)], &by_tweet); + assert_eq!(kept.len(), 1); + } + + #[test] + fn missing_batch_slot_is_failed_not_unlabeled() { + let empty: TweetHydrationBatch = HydrationBatch::empty(); + assert!(author_lookup_failed(empty.hydrated(&TweetId(1)))); + let kept = + retain_candidates_with_usable_author_features(vec![candidate(1, 10)], &empty); + assert!(kept.is_empty()); } } diff --git a/visibility-filtering/hydration/mod.rs b/visibility-filtering/hydration/mod.rs index 0ea33a16..4d7d3d95 100644 --- a/visibility-filtering/hydration/mod.rs +++ b/visibility-filtering/hydration/mod.rs @@ -20,7 +20,9 @@ use crate::safety_label_source::SafetyLabelSource; use batch::TweetHydrationBatch; use exclusive_content_hydrator::ExclusiveContentHydrator; use fallback_cache::FallbackCache; -use gizmoduck_hydrator::GizmoduckAuthorHydrator; +use gizmoduck_hydrator::{ + retain_candidates_with_usable_author_features, GizmoduckAuthorHydrator, +}; use safety_label_hydrator::{SafetyLabelHydration, SafetyLabelHydrator}; use socialgraph_hydrator::SocialgraphHydrator; use std::collections::HashMap; @@ -202,6 +204,9 @@ impl HydrationPipeline { (core_datas, candidates, author_features, relationships), ) = tokio::join!(independent_group, author_hop); + let candidates = + retain_candidates_with_usable_author_features(candidates, &author_features); + let SafetyLabelHydration { label_types, label_response, @@ -316,6 +321,33 @@ mod tests { } } + #[test] + fn assemble_omits_failed_gizmoduck_authors_after_retain() { + let resolved = resolve_candidate(&raw(1, Some(10)), &HashMap::new()).unwrap(); + let author_features = TweetHydrationBatch::from_results( + [TweetId(1)], + HashMap::from([( + TweetId(1), + Err::, _>("gizmoduck unavailable"), + )]), + ); + let usable = retain_candidates_with_usable_author_features( + vec![resolved], + &author_features, + ); + assert!(usable.is_empty()); + + let assembled = CandidateFeatures { + tweet_features: HashMap::new(), + author_features, + safety_labels: HashMap::new(), + relationships: TweetHydrationBatch::empty(), + exclusive_content: HashMap::new(), + } + .assemble(&usable); + assert!(assembled.is_empty()); + } + #[test] fn author_candidate_counts_deduplicate_shared_authors() { let candidates: Vec<_> = [(1, 10), (2, 10), (3, 20)]