Skip to content

Stop RankingScorer from ranking posts with no Phoenix heads - #29

Closed
Pitchfork-and-Torch wants to merge 19 commits into
mainfrom
cursor/phoenix-ranker-fail-closed-372c
Closed

Pitchfork-and-Torch wants to merge 19 commits into
mainfrom
cursor/phoenix-ranker-fail-closed-372c

Conversation

@Pitchfork-and-Torch

Copy link
Copy Markdown
Owner

Bug

Phoenix lookup miss, missing scoring_sequence, or overflow past PHOENIX_CLIENT_MAX_CANDIDATES leaves every Phoenix head None. RankingScorer::apply treated that as 0, then offset_score wrote Some(NEGATIVE_SCORES_OFFSET). TopKScoreSelector only drops score == None. Those posts still ranked.

Some(0.0) is a real Phoenix prediction and still ranks.

This is not #3 (ranker xDS). This is not #31 (author-size IPS). This is not a taste weight tweak.

Five-line proof

  • Entry: PhoenixScorer / predictions.candidate_scores leave all heads None on miss
  • Sink: PhoenixCandidatePipeline scorers [PhoenixScorer, RankingScorer, VMRanker] then TopKScoreSelector (score.unwrap_or(NEG_INFINITY))
  • Break: apply(None) = 0 plus offset wrote a finite rank for an unscored post
  • Viewer effect: For You could serve a tweet Phoenix never scored, including overflow and no-sequence requests
  • Twin: scorer update_all already skips Err; prediction failure already returned Err

Change

  • RankingScorer and PhoenixScoresRankingScorer: no heads -> score / weighted_score stay None
  • PhoenixScorer: missing scoring_sequence returns Err so update_all does not stamp empty scores or last_scored_at_ms

Tests

  • Default heads: score unset
  • Some(0.0) favorite: still ranks
  • Mixed slate: only the unscored candidate is unset
  • Missing sequence: Err, not Ok(default)

cargo test cannot run. Public dump has no home-mixer crate manifest.

Fork PR: none

Open in Web Open in Cursor 

CI agent and others added 19 commits August 14, 2026 20:55
in_network_ids is passed to the VF client without deduplication, while
oon_ids is deduped four lines below. retweeted_tweet_id is pushed for
every candidate that has one, so the same ID repeats once per retweet of
a given post — most often when that post is going viral.

Neither VfClient implementation dedupes its input: StratoVfClient builds
one call per element, and XaiVfClient chunks by XAI_VF_MAX_BATCH_SIZE, so
duplicates consume batch slots and can force an extra round trip.

Not a correctness issue — results collapse into a HashMap keyed by tweet
ID — but redundant work on the For You serving path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deduplicate in_network_ids before VF lookup
Missing Phoenix scores used to become 0 and then NEGATIVE_SCORES_OFFSET,
so lookup misses and a missing scoring_sequence still made TopK. Leave
score unset so TopK drops them. Some(0.0) still ranks.

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

Copy link
Copy Markdown
Owner Author

Wrong base. Ship vehicle is upstream xai-org#142. Do not merge.

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.

4 participants