Fail closed when gizmoduck viewer-age lookup is Err - #160
Closed
Pitchfork-and-Torch wants to merge 8 commits into
Closed
Fail closed when gizmoduck viewer-age lookup is Err#160Pitchfork-and-Torch wants to merge 8 commits into
Pitchfork-and-Torch wants to merge 8 commits into
Conversation
* Add meritocratic author-size IPS to RankingScorer. * Make For You rank by merit, not reach. Add size-aware OON relief for small creators, origin-author diversity so viral originals cannot flood via many retweeters, and mute/block symmetry for quotes and reposts. Complements author-size IPS; docs in FEED_FAIRNESS. Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
…Hydrator (#6) VFCandidateHydrator asks visibility filtering twice per request: once at TimelineHome for in-network candidates (plus repost sources) and once at TimelineHomeRecommendations for out-of-network candidates (plus ancestors and quoted posts). It then merged both answers into one HashMap keyed by tweet id, with the recommendations map applied last. A tweet id can be in both sets. The common case is a followed author's own thread: the root post is an in-network candidate, and the reply in the same thread lists the root as an ancestor. The same happens whenever another selected candidate quotes or replies to an in-network post. In every such case the recommendations verdict overwrote the in-network verdict, so the in-network post was judged under the rules that are meant to apply only to recommendations from accounts the viewer does not follow (SpamHighRecall, NsfwHighRecall, DoNotAmplify, NsfwText, FosnrAbuseInsults, the NSFW author/tweet flags, DMCA and geo-restricted media, and the OON-only user labels in visibility-filtering/rules/registry.rs). VFFilter then removed the post from the viewer's For You feed even though the viewer follows the author and README.md states that "the same post is allowed to a follower". The same collision runs the other way for an out-of-network candidate that is also the source of a followed account's repost: the merge order decides which verdict wins, and neither order is right for both cases. Keep the two result maps separate and route every lookup to the map matching how the id was requested: a candidate's own verdict comes from the map for its in_network flag; ancestors and quoted posts read the recommendations map; repost sources read the in-network map. No VF rule changes and no extra RPCs. Tests cover both collision directions, the ancillary routing, tombstoned ancestors, interstitials, error propagation, and an end-to-end hydrate() run with a client that answers Allow at TimelineHome and Drop at TimelineHomeRecommendations. The end-to-end test fails on the previous code with the root post carrying the recommendations-only drop reason. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
#7) The allowlist is an exemption check that runs before any rule can fire. Until now a Manhattan GET failure during that check was swallowed inside ManhattanAllowlist::get_entity and returned as None, which the fetch helpers read as is_allowlisted: false. Enforcement then continued into the rule pipeline for an account or post that may have been exempt, while the neighbouring Gizmoduck and credibility fetches on the same path abort with `?` and are retried. Make the allowlist lookup behave like those fetches: - ManhattanAllowlist::get / get_entity return anyhow::Result<Option<_>>. Ok(None) means the store confirmed the key is absent. A GET error or an undecodable stored entry is returned as Err instead of None. - fetch_user_allowlist / fetch_entity_allowlist return Result and propagate the error. Only a confirmed absence maps to "not allowlisted". - run_enforcement_inner uses `?` on the allowlist lookups, so a store error aborts the attempt and the score lands in the existing retry queue (backoff, then dropped without enforcing) rather than proceeding to rules. - Admin handlers: GET /allowlist/{id} and GET /allowlist/{type}/{id} return 500 on a read error instead of 404; bulk upsert reports a failed pre-read as a per-row error; the DELETE audit snapshot stays best-effort. Adds unit tests for the lookup-to-facts conversion, including one that asserts a store error is not turned into is_allowlisted=false. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
Rule 7429 writes an NSFW_CARD_IMAGE verdict (7-day TTL) for a card URL when its image scores near-perfect NSFW, and has a cleanup branch meant to delete that verdict when a later score for the same card comes back clean. The cleanup branch could never run: - The rule condition required IsNearPerfectNsfw (precision >= 0.999), but the cleanup guard required !IsHighPrecisionNsfw (precision < 0.95). Both cannot hold, so the branch was dead code. - The age check computed creation - now, which is never positive. - The threshold 60 * 60 * 1000 was compared against seconds, i.e. about 41 days, longer than the verdict's own 7-day TTL. Widen the condition to also admit clean card-image scores, compute the age as now - creation in seconds, and use a one-hour threshold. Require a present precision score on the cleanup path so a media update with no NSFW score cannot clear a verdict. The write path is unchanged. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
…eport (#9) Daily post-label rows store carried (posts that received the label in the observation window) and removed (how many of those were later taken off or expired). The monthly aggregate keeps both. The public Under the Hood report summed only carried, so a post that was labeled and then cleared still counted toward posts and percentageOfPosts. The label copy is present tense ("Post hidden from recommendations to non-followers"). Subtract removed, floored at zero, so the report shows posts that still carry the label. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
#10) Drop rules only look at label type presence. Hydration copied every proto key into that set and threw away expires_at_msec, so a TTL-bound label such as SpamHighRecall kept suppressing out-of-network posts after the intended window. Filter expired proto rows before building the type set. A missing expiry stays permanent. Adds unit tests for the expiry fence and a drop-rule case that an expired SpamHighRecall allows. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
#11) GetTweetLabelInfoFromURL listed tweets that share a URL, then asked GetTweetRtfLabels whether each already had the interstitial. A failed read was replaced with an empty list, so every tweet looked unlabeled. Bot 7413 (NSFW_Card_Image_URL_to_Tweet_Verdict) uses those two lists directly: PUT applies NSFW_CARD_IMAGE to notLabeled, and DELETE only removes it from labeled. After a URL-verdict delete, a Strato miss left the tweet interstitial in place. A miss on PUT could also apply the label on an unknown state. Treat a failed lookup as already labeled so DELETE can still clear and PUT will not apply. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
Gizmoduck viewer RPC error/timeout used to set ViewerAge::Unknown, which skipped both underage and no-stated-age NSFW gates. Treat a logged-in miss as NotStated so SensitiveViewerNoStatedAgeDropRule applies. Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
Author
|
Dirty head: +1422 across fairness/README/ads/scarecrow unrelated to viewer-age. Prefer clean #159 (focused VF viewer LookupFailed). Closing this twin. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
ViewerHydratormaps gizmoduck viewer RPC error/timeout toViewerAge::Unknown.viewer_has_no_stated_ageonly matchedNotStated.SensitiveViewerUnderageDropRuleonly matchesKnown(age < 18). Both NSFW/gore gates therefore Allowed.Okwithuser_existsand no birthday is alreadyNotStated. Logged-out is already dropped bySensitiveViewerLoggedOutDropRule. Those paths are unchanged.This is not #133 (age 0 mapped to
Known(0)). #133 left RPC error as Unknown fail-open. This is not #144 / #149 (author gizmoduck labels/flags).Five-line proof
ViewerHydrator::hydrate(error/timeout used to setViewerAge::Unknown)SensitiveViewerNoStatedAgeDropRule(viewer_has_no_stated_age)Change
On gizmoduck error/timeout for a logged-in viewer, set
ViewerAge::NotStated.viewer_has_no_stated_agealso treats logged-inUnknownas no stated age so a leftover Unknown (user_exists=false) uses the same jurisdiction-scoped gate. Confirmed Known ages and logged-out are unchanged.Tests
Standalone decision-table harness (same match arms): 19 assertions passed.
cargo testcannot run. Public dump has no visibility-filtering manifest.Fork PR: none