Side B fixes a real production bug (cloud IPs falling back to public Reddit API and getting blocked) with proper OAuth retry-on-401/403 logic, better error surfacing, and a config fix in fly.toml — a concrete operational reliability improvement. Side A is a thoughtful refinement of pair-selection heuristics with good test coverage, but it's an internal algorithm tweak with more speculative long-term value compared to B's fix for an actively broken production path.
constitution · epochs · watch · epoch 3
comparison
c_2722a3195825 (tommy-mor) vs c_ca9169f732b8 (tommy-mor)
download prompt · raw event · cmp_31cf393eae279f
council reasoning
A upgrades core ranking behavior with a real design change: established-first bridge merges, then isolate attach, then zip-adjacent rank refinement once the pool is one component, backed by targeted tests. B is a valuable production fix (force OAuth when creds exist, refresh on 401/403, pin base URL) but is integration/ops hardening rather than lasting product algorithm value.
Side B fixes a concrete production reliability issue by requiring OAuth when credentials are configured, adding automatic token refresh/retry on 401/403, returning structured auth-rejection outcomes instead of failing generically, and improving error handling with focused diagnostics. Side A meaningfully refines the pair-selection heuristic (component attachment priorities and rank-adjacent refinement with tests), but it is primarily an algorithmic preference change, whereas Side B prevents fetch failures caused by Reddit's API behavior and hardens a critical integration.
sides
A — c_2722a3195825 (tommy-mor)
message
[5db58b98] Improve vote pair selection for spanning trees and rank refinement. Prefer attaching unranked items to established components before comparing isolates, then zip down adjacent rank-centrality pairs once the pool is fully connected, skipping pairs that already have votes. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/server/src/pair.rs b/server/src/pair.rs
index 54b5d2417e9dba04ed8df422156e274c2b2f76b2..c14de4b0502c8b5a17cddf3746077739d56e03e0 100644
--- a/server/src/pair.rs
+++ b/server/src/pair.rs
@@ -3,13 +3,21 @@
//! Pair selection prefers **bridge** votes — comparisons between items in
//! different connected components of the voted-pairs graph — so the pool
//! merges into one ranking group before refining within it.
+//!
+//! Among unvoted bridges, prefer merging established voted components, then
+//! attaching a never-voted child to an established component, and only then
+//! comparing two never-voted children (so the voted graph grows as one tree).
+//!
+//! Once every pool child sits in one voted component, refinement **zips** down
+//! the rank-centrality order: prefer 1 vs 2, then 2 vs 3, and so on, skipping
+//! pairs that already have a vote.
use rand::seq::SliceRandom;
use std::collections::{HashMap, HashSet};
use crate::{
path_types::ItemId,
- ranking::connected_components_from_voted_pairs,
+ ranking::{connected_components_from_voted_pairs, ranked_items},
reducer::{GlobalTree, GroupState},
};
@@ -28,36 +36,77 @@ fn pair_is_voted(group: &GroupState, a: &ItemId, b: &ItemId) -> bool {
group.voted_pairs.contains(&(i, j))
}
-/// Component id per pool item: voted-pairs graph components plus one id per
-/// never-voted child.
-fn component_ids(group: &GroupState, pool: &[ItemId]) -> HashMap<ItemId, usize> {
+/// Voted-pairs layout for pool items: component id per item plus which ids are
+/// multi-node voted components (ranked groups in the UI).
+struct ComponentLayout {
+ ids: HashMap<ItemId, usize>,
+ established: HashSet<usize>,
+}
+
+fn component_layout(group: &GroupState, pool: &[ItemId]) -> ComponentLayout {
let n = group.idx_to_item.len();
let (comps, isolates) =
connected_components_from_voted_pairs(n, group.voted_pairs.iter().copied());
- let mut out: HashMap<ItemId, usize> = HashMap::new();
+ let mut established = HashSet::new();
+ let mut ids: HashMap<ItemId, usize> = HashMap::new();
for (comp_idx, comp) in comps.iter().enumerate() {
+ if comp.len() >= 2 {
+ established.insert(comp_idx);
+ }
for &idx in comp {
if idx < n {
- out.insert(group.idx_to_item[idx].clone(), comp_idx);
+ ids.insert(group.idx_to_item[idx].clone(), comp_idx);
}
}
}
let mut next = comps.len();
for &idx in &isolates {
if idx < n {
- out.insert(group.idx_to_item[idx].clone(), next);
+ ids.insert(group.idx_to_item[idx].clone(), next);
next += 1;
}
}
for item in pool {
- out.entry(item.clone()).or_insert_with(|| {
+ ids.entry(item.clone()).or_insert_with(|| {
let id = next;
next += 1;
id
});
}
- out
+ ComponentLayout { ids, established }
+}
+
+/// Every pool child shares one multi-node voted component (spanning tree phase done).
+fn pool_fully_connected(layout: &ComponentLayout, pool: &[ItemId]) -> bool {
+ if pool.len() < 2 {
+ return false;
+ }
+ let mut comp_id = None;
+ for item in pool {
+ let Some(id) = layout.ids.get(item) else {
+ return false;
+ };
+ if !layout.established.contains(id) {
+ return false;
+ }
+ match comp_id {
+ None => comp_id = Some(*id),
+ Some(expected) if expected == *id => {}
+ _ => return false,
+ }
+ }
+ comp_id.is_some()
+}
+
+/// Pool children that appear in `group`, sorted best rank first.
+fn ranked_pool_order(group: &GroupState, pool: &[ItemId]) -> Vec<ItemId> {
+ let pool_set: HashSet<_> = pool.iter().collect();
+ ranked_items(group)
+ .into_iter()
+ .map(|r| r.item)
+ .filter(|id| pool_set.contains(id))
+ .collect()
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
@@ -72,19 +121,108 @@ enum PairPriority {
WithinVoted = 3,
}
-fn pair_priority(
+/// Tie-break among unvoted bridge pairs.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+enum BridgeSubPriority {
+ /// Both endpoints lie in established (multi-node) voted components.
+ MergeEstablished = 0,
+ /// One established component member and one never-voted child.
+ AttachIsolate = 1,
+ /// Two never-voted children (separate singleton components).
+ IsolatePair = 2,
+}
+
+/// Tie-break among within-component pairs once the pool is one connected group.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct WithinSubPriority {
+ /// 1 = adjacent ranks (i vs i+1); larger = farther apart in the order.
+ rank_gap: usize,
+ /// min rank index of the two — zip from the top (1 vs 2 before 2 vs 3).
+ zip_index: usize,
+}
+
+const WITHIN_SUB_WORST: WithinSubPriority = WithinSubPriority {
+ rank_gap: usize::MAX,
+ zip_index: usize::MAX,
+};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct PairSortKey {
+ priority: PairPriority,
+ bridge_sub: BridgeSubPriority,
+ within_sub: WithinSubPriority,
+}
+
+fn item_in_established(layout: &ComponentLayout, item: &ItemId) -> bool {
+ layout
+ .ids
+ .get(item)
+ .is_some_and(|id| layout.established.contains(id))
+}
+
+fn bridge_sub_priority(layout: &ComponentLayout, a: &ItemId, b: &ItemId) -> BridgeSubPriority {
+ let a_est = item_in_established(layout, a);
+ let b_est = item_in_established(layout, b);
+ match (a_est, b_est) {
+ (true, true) => BridgeSubPriority::MergeEstablished,
+ (true, false) | (false, true) => BridgeSubPriority::AttachIsolate,
+ (false, false) => BridgeSubPriority::IsolatePair,
+ }
+}
+
+fn within_sub_priority(
+ group: &GroupState,
+ pool: &[ItemId],
+ layout: &ComponentLayout,
+ a: &ItemId,
+ b: &ItemId,
+) -> WithinSubPriority {
+ if !pool_fully_connected(layout, pool) {
+ return WITHIN_SUB_WORST;
+ }
+ let order = ranked_pool_order(group, pool);
+ let (Some(i), Some(j)) = (order.iter().position(|x| x == a), order.iter().position(|x| x == b))
+ else {
+ return WITHIN_SUB_WORST;
+ };
+ WithinSubPriority {
+ rank_gap: i.abs_diff(j),
+ zip_index: i.min(j),
+ }
+}
+
+fn pair_sort_key(
group: &GroupState,
- components: &HashMap<ItemId, usize>,
+ pool: &[ItemId],
+ layout: &ComponentLayout,
a: &ItemId,
b: &ItemId,
-) -> PairPriority {
+) -> PairSortKey {
let voted = pair_is_voted(group, a, b);
- let bridge = components.get(a) != components.get(b);
- match (bridge, voted) {
+ let bridge = layout.ids.get(a) != layout.ids.get(b);
+ let priority = match (bridge, voted) {
(true, false) => PairPriority::BridgeUnvoted,
(false, false) => PairPriority::WithinUnvoted,
(true, true) => PairPriority::BridgeVoted,
(false, true) => PairPriority::WithinVoted,
+ };
+ let bridge_sub = if priority == PairPriority::BridgeUnvoted {
+ bridge_sub_priority(layout, a, b)
+ } else {
+ BridgeSubPriority::MergeEstablished
+ };
+ let within_sub = if matches!(
+ priority,
+ PairPriority::WithinUnvoted | PairPriority::WithinVoted
+ ) {
+ within_sub_priority(group, pool, layout, a, b)
+ } else {
+ WITHIN_SUB_WORST
+ };
+ PairSortKey {
+ priority,
+ bridge_sub,
+ within_sub,
}
}
@@ -109,9 +247,12 @@ fn candidate_pairs(pool: &[ItemId], exclude: Option<(&ItemId, &ItemId)>) -> Vec<
/// Pick the next pair to vote on within `pool`.
///
-/// 1. Prefer unvoted **bridge** pairs (connect separate ranking components).
-/// 2. Then unvoted within-component pairs (refinement).
-/// 3. Then already-voted pairs (re-compare).
+/// 1. Prefer unvoted **bridge** pairs (connect separate ranking components),
+/// with sub-priority: merge established components, attach an isolate to
+/// established, then compare two isolates.
+/// 2. Then unvoted within-component pairs; when the pool is one connected group,
+/// prefer adjacent ranks (1 vs 2, 2 vs 3, …) in order, skipping voted pairs.
+/// 3. Then already-voted pairs (re-compare), with the same zip ordering.
pub fn suggest_next_pair_in_pool(
group: &GroupState,
pool: &[ItemId],
@@ -121,15 +262,15 @@ pub fn suggest_next_pair_in_pool(
if candidates.is_empty() {
return None;
}
- let components = component_ids(group, pool);
+ let layout = component_layout(group, pool);
let best = candidates
.iter()
- .map(|(a, b)| (pair_priority(group, &components, a, b), (a, b)))
- .min_by_key(|(p, _)| *p)?
+ .map(|(a, b)| (pair_sort_key(group, pool, &layout, a, b), (a, b)))
+ .min_by_key(|(k, _)| *k)?
.0;
let best_pairs: Vec<(ItemId, ItemId)> = candidates
.into_iter()
- .filter(|(a, b)| pair_priority(group, &components, a, b) == best)
+ .filter(|(a, b)| pair_sort_key(group, pool, &layout, a, b) == best)
.collect();
best_pairs.choose(&mut rand::thread_rng()).cloned()
}
@@ -303,6 +444,38 @@ mod tests {
assert!(from_ab && from_cd, "expected bridge pair, got {:?}", chosen);
}
+ #[test]
+ fn suggest_prefers_attach_over_isolate_pair_among_many_unranked() {
+ let parent = ItemId::parse("reddit.com/r/rust").unwrap();
+ let mut tree = seed_children(
+ &parent,
+ &[
+ "reddit.com/r/rust/a",
+ "reddit.com/r/rust/b",
+ "reddit.com/r/rust/c",
+ "reddit.com/r/rust/d",
+ "reddit.com/r/rust/e",
+ ],
+ );
+ let ab =
+ VoteData::from_recorded(1, "reddit.com/r/rust/a", "reddit.com/r/rust/b", 2, 1).unwrap();
+ tree.apply_vote(&parent, ab);
+ let group = tree.get(&parent).unwrap().local_ranking.clone();
+ let pool = children_of(&tree, &parent);
+ let pair = suggest_next_pair_in_pool(&group, &pool, None).unwrap();
+ let chosen = pair_set(&pair);
+ let from_ab =
+ chosen.contains("reddit.com/r/rust/a") || chosen.contains("reddit.com/r/rust/b");
+ let from_cde = chosen.contains("reddit.com/r/rust/c")
+ || chosen.contains("reddit.com/r/rust/d")
+ || chosen.contains("reddit.com/r/rust/e");
+ assert!(
+ from_ab && from_cde,
+ "expected ranked+unranked attach, got {:?}",
+ chosen
+ );
+ }
+
#[test]
fn suggest_connects_isolate_to_existing_component() {
let parent = ItemId::parse("reddit.com/r/rust").unwrap();
@@ -325,6 +498,65 @@ mod tests {
assert!(chosen.contains("reddit.com/r/rust/a") || chosen.contains("reddit.com/r/rust/b"));
}
+ #[test]
+ fn suggest_zips_adjacent_ranks_when_tree_complete() {
+ let parent = ItemId::parse("reddit.com/r/rust").unwrap();
+ let mut tree = seed_children(
+ &parent,
+ &[
+ "reddit.com/r/rust/a",
+ "reddit.com/r/rust/b",
+ "reddit.com/r/rust/c",
+ ],
+ );
+ // Star at a connects all three; b-c is the only unvoted adjacent pair left.
+ for (a, b, l, r) in [
+ ("reddit.com/r/rust/a", "reddit.com/r/rust/b", 3, 1),
+ ("reddit.com/r/rust/a", "reddit.com/r/rust/c", 2, 1),
+ ] {
+ let v = VoteData::from_recorded(1, a, b, l, r).unwrap();
+ tree.apply_vote(&parent, v);
+ }
+ let group = tree.get(&parent).unwrap().local_ranking.clone();
+ let pool = children_of(&tree, &parent);
+ let pair = suggest_next_pair_in_pool(&group, &pool, None).unwrap();
+ let chosen = pair_set(&pair);
+ // a-b and a-c voted;
… preview truncated; 1,687 characters omittedB — c_ca9169f732b8 (tommy-mor)
message
[8f69c309] Require Reddit OAuth when credentials are set and refresh on 401/403. Avoid falling back to the public www.reddit.com API from cloud IPs, which returns Reddit's network-security block page. Also pin SORTER2_BASE_URL in fly.toml. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/fly.toml b/fly.toml
index f0c6a39f643c204987debc177234d95a8ef44b65..ca7e0088a7efd7d58d29d808f8d89080b2bff233 100644
--- a/fly.toml
+++ b/fly.toml
@@ -5,6 +5,7 @@ primary_region = "iad"
dockerfile = "Dockerfile"
[env]
+ SORTER2_BASE_URL = "https://reddit.sorter.social"
SORTER2_DATA_DIR = "/data"
SORTER2_EVENT_LOG = "/data/events.jsonl"
PORT = "8080"
diff --git a/server/src/reddit.rs b/server/src/reddit.rs
index a874814f8927192ee62cab2d0db1efd27dcd57b7..f409764c1e1f36216f1b08107043c2eab905694c 100644
--- a/server/src/reddit.rs
+++ b/server/src/reddit.rs
@@ -283,26 +283,35 @@ async fn reddit_worker(
);
tokio::time::sleep(current_delay).await;
- if let Some(c) = &creds {
- oauth = ensure_oauth_token(&client, &oauth_token_base, c, oauth.take()).await;
- }
-
- let token = oauth.as_ref().map(|t| t.access_token.as_str());
- let fetch_base = if token.is_some() {
- tracing::debug!(
- item = %fetch_id,
- base = %oauth_api_base,
- "reddit fetch using OAuth bearer"
- );
- &oauth_api_base
- } else {
- &api_base
- };
- let url = match kind {
- FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, fetch_base),
- FetchKind::Children => map_children_url(&fetch_id, fetch_base),
+ let outcome = match &creds {
+ Some(c) => {
+ // OAuth is required when credentials are configured — never fall
+ // back to the public www.reddit.com JSON endpoints (cloud IPs
+ // get blocked with a 403 HTML interstitial).
+ fetch_with_oauth(
+ &client,
+ &oauth_token_base,
+ &oauth_api_base,
+ c,
+ &mut oauth,
+ &fetch_id,
+ kind,
+ )
+ .await
+ }
+ None => {
+ let url = match kind {
+ FetchKind::SelfEntity => map_item_to_reddit_api(&fetch_id, &api_base),
+ FetchKind::Children => map_children_url(&fetch_id, &api_base),
+ };
+ match do_fetch(&client, &url, &fetch_id, None).await {
+ Ok(FetchOutcome::AuthRejected { status, detail }) => {
+ Err(format!("Reddit API {status}: {detail}"))
+ }
+ other => other,
+ }
+ }
};
- let outcome = do_fetch(&client, &url, &fetch_id, token).await;
match outcome {
Ok(FetchOutcome::Payload(payload)) => {
@@ -342,6 +351,12 @@ async fn reddit_worker(
current_delay = (current_delay * 2).min(Duration::from_secs(60));
notify(done, FetchJobResult::RateLimited { reset_secs });
}
+ Ok(FetchOutcome::AuthRejected { status, detail }) => {
+ let e = format!("Reddit API {status}: {detail}");
+ tracing::warn!(item = %fetch_id, err = %e, "reddit fetch auth rejected");
+ current_delay = (current_delay * 2).min(Duration::from_secs(60));
+ notify(done, FetchJobResult::Failed(e));
+ }
Err(e) => {
tracing::warn!(item = %fetch_id, err = %e, "reddit fetch failed");
current_delay = (current_delay * 2).min(Duration::from_secs(60));
@@ -357,6 +372,60 @@ enum FetchOutcome {
Payload(Value),
NotFound,
RateLimited { reset_secs: u64 },
+ /// Bearer rejected — caller should drop the cached token and retry once.
+ AuthRejected { status: StatusCode, detail: String },
+}
+
+async fn fetch_with_oauth(
+ client: &Client,
+ oauth_token_base: &str,
+ oauth_api_base: &str,
+ creds: &RedditCredentials,
+ oauth: &mut Option<OAuthToken>,
+ fetch_id: &ItemId,
+ kind: FetchKind,
+) -> Result<FetchOutcome, String> {
+ for attempt in 0..2 {
+ let force_refresh = attempt > 0;
+ *oauth = Some(
+ ensure_oauth_token(client, oauth_token_base, creds, oauth.take(), force_refresh)
+ .await?,
+ );
+ let token = oauth
+ .as_ref()
+ .expect("token set above")
+ .access_token
+ .clone();
+
+ tracing::debug!(
+ item = %fetch_id,
+ base = %oauth_api_base,
+ attempt,
+ "reddit fetch using OAuth bearer"
+ );
+
+ let url = match kind {
+ FetchKind::SelfEntity => map_item_to_reddit_api(fetch_id, oauth_api_base),
+ FetchKind::Children => map_children_url(fetch_id, oauth_api_base),
+ };
+ match do_fetch(client, &url, fetch_id, Some(&token)).await? {
+ FetchOutcome::AuthRejected { status, detail } if attempt == 0 => {
+ tracing::warn!(
+ item = %fetch_id,
+ %status,
+ %detail,
+ "reddit OAuth rejected; refreshing token and retrying"
+ );
+ *oauth = None;
+ continue;
+ }
+ FetchOutcome::AuthRejected { status, detail } => {
+ return Err(format!("Reddit API {status}: {detail}"));
+ }
+ other => return Ok(other),
+ }
+ }
+ unreachable!("loop always returns")
}
async fn ensure_oauth_token(
@@ -364,35 +433,35 @@ async fn ensure_oauth_token(
oauth_base: &str,
creds: &RedditCredentials,
existing: Option<OAuthToken>,
-) -> Option<OAuthToken> {
- if let Some(t) = existing {
- if Instant::now() < t.expires_at - Duration::from_secs(60) {
- tracing::debug!("reddit OAuth token still valid");
- return Some(t);
+ force_refresh: bool,
+) -> Result<OAuthToken, String> {
+ if !force_refresh {
+ if let Some(t) = existing {
+ if Instant::now() < t.expires_at - Duration::from_secs(60) {
+ tracing::debug!("reddit OAuth token still valid");
+ return Ok(t);
+ }
}
}
let url = format!("{}/api/v1/access_token", oauth_base.trim_end_matches('/'));
- tracing::debug!(%url, "reddit OAuth token request");
+ tracing::debug!(%url, force_refresh, "reddit OAuth token request");
let resp = client
.post(&url)
.basic_auth(&creds.client_id, Some(&creds.client_secret))
.form(&[("grant_type", "client_credentials")])
.send()
- .await;
-
- let resp = match resp {
- Ok(r) => r,
- Err(e) => {
- tracing::warn!("reddit OAuth token request failed: {e}");
- return None;
- }
- };
+ .await
+ .map_err(|e| format!("Reddit OAuth token request failed: {e}"))?;
if !resp.status().is_success() {
- tracing::warn!("reddit OAuth token HTTP {}", resp.status());
- return None;
+ let status = resp.status();
+ let body = resp.text().await.unwrap_or_default();
+ return Err(format!(
+ "Reddit OAuth token HTTP {status}: {}",
+ truncate_for_error(&body)
+ ));
}
#[derive(Deserialize)]
@@ -401,21 +470,40 @@ async fn ensure_oauth_token(
expires_in: u64,
}
- let body: TokenResponse = match resp.json().await {
- Ok(b) => b,
- Err(e) => {
- tracing::warn!("reddit OAuth token parse failed: {e}");
- return None;
- }
- };
+ let body: TokenResponse = resp
+ .json()
+ .await
+ .map_err(|e| format!("Reddit OAuth token parse failed: {e}"))?;
- tracing::debug!(expires_in = body.expires_in, "reddit OAuth token acquired");
- Some(OAuthToken {
+ tracing::info!(expires_in = body.expires_in, "reddit OAuth token acquired");
+ Ok(OAuthToken {
access_token: body.access_token,
expires_at: Instant::now() + Duration::from_secs(body.expires_in),
})
}
+fn truncate_for_error(body: &str) -> String {
+ let compact: String = body.split_whitespace().collect::<Vec<_>>().join(" ");
+ if compact.is_empty() {
+ return "(empty body)".into();
+ }
+ // Prefer the human-readable block message over dumping Reddit's CSS.
+ if let Some(idx) = compact.find("You've been blocked") {
+ let slice: String = compact.chars().skip(idx).take(160).collect();
+ return if compact.chars().count() > idx + 160 {
+ format!("{slice}…")
+ } else {
+ slice
+ };
+ }
+ let chars: String = compact.chars().take(200).collect();
+ if compact.chars().count() > 200 {
+ format!("{chars}…")
+ } else {
+ chars
+ }
+}
+
async fn do_fetch(
client: &Client,
url: &str,
@@ -460,15 +548,16 @@ async fn do_fetch(
if !status.is_success() {
let body = resp.text().await.unwrap_or_default();
+ let detail = truncate_for_error(&body);
tracing::debug!(
item = %id,
%status,
body_len = body.len(),
- body_prefix = %body.chars().take(240).collect::<String>(),
+ %detail,
"reddit non-success body"
);
if status == StatusCode::FORBIDDEN || status == StatusCode::UNAUTHORIZED {
- return Err(format!("Reddit API {status}: {body}"));
+ return Ok(FetchOutcome::AuthRejected { status, detail });
}
return Ok(FetchOutcome::NotFound);
}
@@ -716,6 +805,15 @@ fn reddit_direct_image_url(url: &str) -> bool {
mod tests {
use super::*;
+ #[test]
+ fn truncate_error_prefers_block_message() {
+ let html = r#"<style>.x{color:red}</style><div>You've been blocked by network security. To continue, log in</div>"#;
+ let msg = truncate_for_error(html);
+ assert!(msg.starts_with("You've been blocked"));
+ assert!(msg.len() < 200);
+ assert!(!msg.contains(".x{color"));
+ }
+
#[test]
fn map_subreddit_about_url() {
let id = ItemId::from_url("https://reddit.com/r/rust").unwrap();
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.