Skip to content

Stop DedupConversationFilter from keeping retweets of mid-thread replies - #189

Closed
Pitchfork-and-Torch wants to merge 8 commits into
xai-org:mainfrom
Pitchfork-and-Torch:cursor/convo-collapse-rt-reply-4ce8
Closed

Stop DedupConversationFilter from keeping retweets of mid-thread replies#189
Pitchfork-and-Torch wants to merge 8 commits into
xai-org:mainfrom
Pitchfork-and-Torch:cursor/convo-collapse-rt-reply-4ce8

Conversation

@Pitchfork-and-Torch

Copy link
Copy Markdown

DedupConversationFilter keys replies by min(ancestors) and retweets with empty ancestors by get_original_tweet_id(). That only collapses a retweet of the root. A retweet of a mid-thread reply keys to the reply id, so the RT and the rest of the thread both survive post-selection.

Proof

  • Entry: DedupConversationFilter::get_conversation_id (phoenix post-selection)
  • Sink: For You TopK after VF — one kept post per conversation_id
  • Break: RT of reply 11 with ancestors [2, 1] keys to 11; the reply keys to 1
  • Viewer: a viral reply-RT and another post from the same thread both take For You slots
  • Twin: existing tests already collapse RT-of-root (retweeted_tweet_id == min(ancestors)); Thunder fills ancestors on replies, not on retweets

This is not #97/#102 (gap expansion / unhydrated grandparents). Not #99 (self-reply chains). Not #101/#111 (muted/blocked ancestors). Not #179 (seen/served parent). Not #180 (Phoenix VF parents).

Fix: before scoring, map each slate tweet/ancestor onto the smallest conversation root already present. Retweets without ancestors use that root when the original is on the slate (as a reply or as someone’s ancestor). Unrelated originals stay on their own tweet id. Replies still key by min(ancestors), so shallow [2] vs [2, 1] stays two conversations.

Tests: RT of the reply itself, RT of a reply listed as a sibling ancestor, unrelated RT unchanged, existing root-RT and multi-conversation cases unchanged.

cargo test cannot run here. Public dump has no Home Mixer manifest. Standalone rustc model of old vs new keying passed 4/4.

Pitchfork-and-Torch and others added 8 commits August 14, 2026 07:43
* 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>
Retweets without ancestors keyed to the original tweet id, so a retweet of
a reply stayed in a different bucket than the rest of that conversation.
Map originals onto the min-ancestor root already present on the slate.

Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
@Pitchfork-and-Torch

Copy link
Copy Markdown
Author

This draft is contaminated: 19 files, behind main, and it mixes the DedupConversationFilter conversation-root mapping with unrelated fork work (author_size_ips, scarecrow, abuse-enforcement, README, FEED_FAIRNESS, and more).

Superseded by the clean one-file PR on current main: #193

Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants