Skip to content

Fail closed when primary VF lookup is Err or missing - #15

Closed
Pitchfork-and-Torch wants to merge 19 commits into
mainfrom
cursor/vf-primary-miss-fail-closed-1c99
Closed

Pitchfork-and-Torch wants to merge 19 commits into
mainfrom
cursor/vf-primary-miss-fail-closed-1c99

Conversation

@Pitchfork-and-Torch

@Pitchfork-and-Torch Pitchfork-and-Torch commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Bug

VFCandidateHydrator turns a primary VF Err or a missing result key into visibility_reason = None. VFFilter keeps None.

Some(Err) used to return hydrator Err. Hydrator::update_all skips that write, so the candidate keeps the default None and still serves. A missing map key was written as None directly.

Ok(None) is a successful Allow. That path is unchanged.

This is not xai-org#119 (retweet primary looked up by wrapper id). This is not xai-org#117 (QuoteHydrator TES / socialgraph). This is not label or TES-flag miss work.

  • Entry: VFCandidateHydrator::resolve_visibility (Following uses the same function)
  • Sink: VFFilter::should_drop (None => false)
  • Break: VF RPC error and omitted id both become Allow
  • Viewer effect: a post that VF did not evaluate still serves on For You and Latest Following
  • Twin: XaiVfClient::results_to_map already writes UnspecifiedReason for a missing response id. The hydrator was throwing that away on Err and on Strato keys that never come back.

Fix

Stamp FilteredReason::UnspecifiedReason on primary Err and missing key so update_all writes it and VFFilter drops (Some(_) => true). Same sentinel the Xai VF client already uses.

Ancillary Err / missing key now set drop_ancillary_posts so a quote, reply, or retweet whose child was not evaluated is also dropped. Successful ancillary Allow (Ok(None)) is unchanged. Interstitial on a child is still not treated as Drop (leftover).

Rebased onto latest xai-org/x-algorithm main. Kept upstream's combined result map and in_network_ids dedup. VFFollowingCandidateHydrator now calls the same resolve_visibility so Latest Following is also fail-closed.

Tests

  • Primary Err stamps UnspecifiedReason (no longer hydrator Err)
  • Primary missing key stamps UnspecifiedReason
  • Primary Ok(None) stays None (Allow)
  • Ancillary Err / missing key drop; ancillary Allow does not
  • VFFilter drops UnspecifiedReason and still keeps None

cargo test cannot run. Public dump has no Home Mixer manifest.

Leftover

Quote / retweet / ancestor Action::Interstitial still does not set drop_ancillary_posts. Wrapper can survive a soft verdict on the child. Separate from lookup miss.

Upstream: xai-org#121

Open in Web Open in Cursor 

CI agent and others added 18 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
VF Err and a missing result key were collapsed to visibility_reason None.
Hydrator Err is skipped by update_all, so VFFilter treated both as Allow.
Stamp UnspecifiedReason so the filter drops, matching XaiVfClient missing ids.
Successful Ok(None) Allow is unchanged.

Rebased onto xai-org/x-algorithm main. Keep upstream's combined result map
and in_network_ids dedup. Following hydrator uses the same resolve path.

Co-authored-by: Jon Bailey <Pitchfork-and-Torch@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/vf-primary-miss-fail-closed-1c99 branch from 1e33e66 to 3644090 Compare September 7, 2026 04:14
@Pitchfork-and-Torch
Pitchfork-and-Torch marked this pull request as ready for review September 7, 2026 04:31
@cursor

cursor Bot commented Sep 7, 2026

Copy link
Copy Markdown

Superseded. Do not merge this PR.

This branch is the same head as upstream xai-org#121 (cursor/vf-primary-miss-fail-closed-1c99). Against Pitchfork main that shows as 297-file conflict noise.

Clean fork twin, based on latest Pitchfork main, with only the VF fail-closed change: #26

Upstream xai-org#121 is untouched and should stay on that branch. Do not force-push or rewrite cursor/vf-primary-miss-fail-closed-1c99.

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