B is a broader type-safety refactor that removes stringly-typed item paths in favor of CanonicalItemUrl/newtype Deref impls, reducing risk of mismatched string keys across resolve_item, connectivity stats, and RPC pairing logic—real structural improvement with test coverage. A is a smaller, well-tested fix improving color-gradient correctness for rank rows, which is valuable but narrower in scope and impact than B's cross-cutting type refactor.
constitution · epochs · watch · epoch 3
comparison
c_66eb04076a98 (tommy-mor) vs c_c42f908efc44 (tommy-mor)
download prompt · raw event · cmp_b7128d14e6646c
council reasoning
A replaces ordinal/list-position gradient coloring with min–max normalization of actual vote mass (score_gradient_t + per-group min/max), a precise behavioral fix that keeps similar scores visually close and is backed by targeted tests; B is a worthwhile but largely mechanical type-propagation refactor (Deref on URL newtypes, CanonicalItemUrl through resolve_item/pools/maps) that removes String churn without changing user-visible behavior or fixing a defect.
Side B performs a broad but coherent type-safety refactor by making `resolve_item` return `CanonicalItemUrl` instead of `String`, propagating canonical URL types through validation, ranking, RPC, and pair selection, and eliminating repeated parsing/wrapping logic while adding `Deref<str>` support for URL newtypes. Side A improves UI behavior by changing row coloring from list position to score-based min–max normalization with solid tests, but its impact is limited to presentation whereas Side B strengthens core API correctness and maintainability across many code paths.
sides
A — c_66eb04076a98 (tommy-mor)
message
[0366806e] Color rank rows by vote mass within each group, not list position. Min–max normalization keeps similar scores visually close while still using the full gradient as groups grow and absolute mass dilutes. Co-authored-by: Cursor <cursoragent@cursor.com>
diff preview
diff --git a/server/src/html/mod.rs b/server/src/html/mod.rs
index a58cbbee3490a08a625cb06df06848c59a615d65..4eff2e19ed4d303ff8e80c1eabd8a15b4990e643 100644
--- a/server/src/html/mod.rs
+++ b/server/src/html/mod.rs
@@ -264,12 +264,19 @@ pub fn scope_theme_style(parent: &ItemId) -> String {
)
}
-fn rank_row_style(parent: &ItemId, ordinal: usize, total: usize) -> String {
- let t = if total <= 1 {
- 0.0
- } else {
- ordinal as f64 / (total - 1) as f64
- };
+/// Map vote mass to gradient position using the group's score range, not raw mass or
+/// list position. Vote mass sums to 1 across the component, so absolute values dilute
+/// as N grows; min–max within the visible list preserves similar scores → similar colors.
+fn score_gradient_t(score: f64, min_score: f64, max_score: f64) -> f64 {
+ let spread = max_score - min_score;
+ if spread < 1e-9 {
+ return 0.5;
+ }
+ ((max_score - score) / spread).clamp(0.0, 1.0)
+}
+
+fn rank_row_style(parent: &ItemId, score: f64, min_score: f64, max_score: f64) -> String {
+ let t = score_gradient_t(score, min_score, max_score);
let base_hue = scope_base_hue(parent);
let hue = (base_hue + 118.0 * t) % 360.0;
let lightness = 0.74 - 0.34 * t;
@@ -295,14 +302,15 @@ fn rank_list(
highlighted: &HashSet<ItemId>,
tree: &GlobalTree,
) -> Markup {
- let group_len = items.len();
+ let min_score = items.iter().map(|r| r.score).fold(f64::INFINITY, f64::min);
+ let max_score = items.iter().map(|r| r.score).fold(f64::NEG_INFINITY, f64::max);
html! {
@if !items.is_empty() {
h3 class="rank-heading muted small" { (label) }
ol class="rank-list" {
@for (i, r) in items.iter().enumerate() {
@let href = item_href(&r.item);
- @let style = rank_row_style(parent, i, group_len);
+ @let style = rank_row_style(parent, r.score, min_score, max_score);
@let class = rank_row_class(&r.item, highlighted);
li class=(class)
data-rank-item=(r.item.as_str())
@@ -517,19 +525,37 @@ pub async fn browse(State(state): State<AppState>, uri: Uri) -> impl IntoRespons
#[cfg(test)]
mod tests {
- use super::{rank_row_style, SORTER_UI_JS};
+ use super::{rank_row_style, score_gradient_t, SORTER_UI_JS};
use crate::path_types::ItemId;
#[test]
- fn rank_row_style_gradients_per_group_not_globally() {
+ fn score_gradient_t_uses_group_range_not_absolute_mass() {
+ assert!((score_gradient_t(0.12, 0.08, 0.12) - 0.0).abs() < 1e-9);
+ assert!((score_gradient_t(0.08, 0.08, 0.12) - 1.0).abs() < 1e-9);
+ // Raw 12% mass would map near the dark end globally; within this group it's the top.
+ assert!(score_gradient_t(0.12, 0.08, 0.12) < score_gradient_t(0.12, 0.0, 1.0));
+ }
+
+ #[test]
+ fn score_gradient_t_similar_scores_similar_t() {
+ let a = score_gradient_t(0.41, 0.20, 0.60);
+ let b = score_gradient_t(0.40, 0.20, 0.60);
+ assert!((a - b).abs() < 0.05);
+ assert!((a - score_gradient_t(0.60, 0.20, 0.60)).abs() > 0.3);
+ }
+
+ #[test]
+ fn score_gradient_t_tied_scores_neutral() {
+ assert!((score_gradient_t(0.25, 0.25, 0.25) - 0.5).abs() < 1e-9);
+ }
+
+ #[test]
+ fn rank_row_style_same_inputs_same_color() {
let parent = ItemId::opaque("test-scope");
- let first_in_four = rank_row_style(&parent, 0, 4);
- let last_in_four = rank_row_style(&parent, 3, 4);
- let first_in_two = rank_row_style(&parent, 0, 2);
- let last_in_two = rank_row_style(&parent, 1, 2);
- assert_eq!(first_in_four, first_in_two);
- assert_eq!(last_in_four, last_in_two);
- assert_ne!(first_in_four, last_in_four);
+ assert_eq!(
+ rank_row_style(&parent, 0.33, 0.20, 0.60),
+ rank_row_style(&parent, 0.33, 0.20, 0.60),
+ );
}
#[test]
B — c_c42f908efc44 (tommy-mor)
message
[674964ef] refactor: Deref for href newtypes, CanonicalItemUrl through resolve_item - Implement Deref<Target=str> for GardenItemUrl, ForumThreadUrl, TildeOntologyPath - resolve_item returns CanonicalItemUrl; validate uses HashSet<CanonicalItemUrl> - compute_scope_rank_changes keys are CanonicalItemUrl; pair RPC uses Vec pool - pick_random_distinct_canonical; connectivity stats on &[CanonicalItemUrl] - Global rank unranked uses stored ids before GardenItemUrl mapping Made-with: Cursor
diff preview
diff --git a/server/src/api/helpers.rs b/server/src/api/helpers.rs
index 03b3e77911ccd662bec8635345dafe2593cf242e..1b291db83df7364a026f2e147e0a29a70a399371 100644
--- a/server/src/api/helpers.rs
+++ b/server/src/api/helpers.rs
@@ -30,13 +30,13 @@ pub fn now_ms() -> i64 {
t.as_millis() as i64
}
-/// Resolve an item path as a first-class canonical path.
-pub fn resolve_item(item: &str) -> Result<String, String> {
+/// Resolve DSL/user input to a stored canonical item id.
+pub fn resolve_item(item: &str) -> Result<CanonicalItemUrl, String> {
let canonical = canonicalize_item(item);
if canonical.is_empty() {
return Err(format!("empty item path: `{}`", item));
}
- Ok(canonical)
+ Ok(CanonicalItemUrl(canonical))
}
pub fn parse_parent_specs(parent: Option<&String>) -> Vec<String> {
@@ -94,7 +94,7 @@ pub fn paginate_rankings(
(out_components, out_unranked)
}
-pub fn pick_random_distinct(items: &[String]) -> Option<(String, String)> {
+pub fn pick_random_distinct_canonical(items: &[CanonicalItemUrl]) -> Option<(CanonicalItemUrl, CanonicalItemUrl)> {
use rand::seq::SliceRandom;
if items.len() < 2 {
return None;
@@ -123,15 +123,12 @@ pub fn is_pair_voted(group: &crate::reducer::GroupState, a: &str, b: &str) -> bo
group.voted_pairs.contains(&(i, j))
}
-pub fn compute_connectivity_stats(group: &crate::reducer::GroupState, pool: &[String]) -> ConnectivityStats {
+pub fn compute_connectivity_stats(group: &crate::reducer::GroupState, pool: &[CanonicalItemUrl]) -> ConnectivityStats {
let n = pool.len();
let global_idxs: Vec<Option<usize>> = pool
.iter()
- .map(|it| {
- let key = CanonicalItemUrl(it.clone());
- group.item_to_idx.get(&key).copied()
- })
+ .map(|it| group.item_to_idx.get(it).copied())
.collect();
let present: Vec<usize> = global_idxs.iter().filter_map(|x| *x).collect();
diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs
index cf22cb0129366c3aed031bc86f3197a4321cb806..a10ce662105cff8fad949c6b83f7035ce79bed18 100644
--- a/server/src/api/mod.rs
+++ b/server/src/api/mod.rs
@@ -24,7 +24,7 @@ pub use auth::{
pub use helpers::{
api_error, compute_connectivity_stats, is_pair_voted, now_ms, paginate_rankings,
- parse_parent_specs, pick_random_distinct, resolve_item, sha256_hex, vote_touches_path,
+ parse_parent_specs, pick_random_distinct_canonical, resolve_item, sha256_hex, vote_touches_path,
};
pub use rpc::handle_rpc_batch;
diff --git a/server/src/api/rpc.rs b/server/src/api/rpc.rs
index 5f7d50188f1381267402f2e57e671234ef5db2fd..de0955d0887d32740e1fd18365205c5bdb53c247 100644
--- a/server/src/api/rpc.rs
+++ b/server/src/api/rpc.rs
@@ -29,7 +29,7 @@ use crate::{
use super::auth::verify_bearer_principal;
use super::helpers::{
compute_connectivity_stats, is_pair_voted, now_ms, paginate_rankings, parse_parent_specs,
- pick_random_distinct, resolve_item, vote_touches_path,
+ pick_random_distinct_canonical, resolve_item, vote_touches_path,
};
use super::validate::{normalize_room_and_thread, validate_ingest_document};
@@ -146,21 +146,21 @@ fn authorize_room_read(reduced: &ReducerState, headers: &HeaderMap, room: &str)
}
fn compute_scope_rank_changes(
- parent: &str,
+ parent: &CanonicalItemUrl,
before: &crate::scope_rank::ChildrenRankings,
after: &crate::scope_rank::ChildrenRankings,
room_wire: &str,
) -> Option<ScopeRankChanges> {
- fn build_positions(rankings: &crate::scope_rank::ChildrenRankings) -> HashMap<String, Option<RankPosition>> {
+ fn build_positions(rankings: &crate::scope_rank::ChildrenRankings) -> HashMap<CanonicalItemUrl, Option<RankPosition>> {
let mut map = HashMap::new();
for comp in &rankings.component_rankings {
let total = comp.ranked.len();
for (i, item) in comp.ranked.iter().enumerate() {
- map.insert(item.item.as_str().to_string(), Some(RankPosition { rank: i + 1, of: total }));
+ map.insert(item.item.clone(), Some(RankPosition { rank: i + 1, of: total }));
}
}
for item in &rankings.unranked_items {
- map.insert(item.as_str().to_string(), None);
+ map.insert(item.clone(), None);
}
map
}
@@ -168,7 +168,7 @@ fn compute_scope_rank_changes(
let before_pos = build_positions(before);
let after_pos = build_positions(after);
- let all_items: std::collections::BTreeSet<String> = before_pos.keys().cloned()
+ let all_items: std::collections::BTreeSet<CanonicalItemUrl> = before_pos.keys().cloned()
.chain(after_pos.keys().cloned())
.collect();
@@ -184,7 +184,7 @@ fn compute_scope_rank_changes(
};
if changed {
changes.push(RankChange {
- item: GardenItemUrl::from_storage_str(&item, room_wire),
+ item: GardenItemUrl::from_stored(&item, room_wire),
before: b,
after: a,
});
@@ -203,11 +203,7 @@ fn compute_scope_rank_changes(
});
Some(ScopeRankChanges {
- parent: if parent.is_empty() {
- "/".to_string()
- } else {
- GardenItemUrl::from_storage_str(parent, room_wire).into_inner()
- },
+ parent: GardenItemUrl::from_stored(parent, room_wire).into_inner(),
changes,
})
}
@@ -473,8 +469,8 @@ async fn rpc_post(
for s in &v.doc.statements {
if let dsl::Stmt::Vote { item1, item2, .. } = s {
if let (Ok(a), Ok(b)) = (resolve_item(item1), resolve_item(item2)) {
- if let Some(p) = CanonicalItemUrl::parse(&a).and_then(|c| c.parent()) { parents.insert(p); }
- if let Some(p) = CanonicalItemUrl::parse(&b).and_then(|c| c.parent()) { parents.insert(p); }
+ if let Some(p) = a.parent() { parents.insert(p); }
+ if let Some(p) = b.parent() { parents.insert(p); }
}
}
}
@@ -525,7 +521,7 @@ async fn rpc_post(
.filter_map(|p| {
let before = pre_rankings.get(p)?;
let after = crate::scope_rank::build_children_rankings(content, p);
- compute_scope_rank_changes(p.as_str(), before, &after, &room_key)
+ compute_scope_rank_changes(p, before, &after, &room_key)
})
.collect();
if v.is_empty() { None } else { Some(v) }
@@ -638,8 +634,8 @@ async fn rpc_check(
for s in &v.doc.statements {
if let dsl::Stmt::Vote { item1, item2, .. } = s {
if let (Ok(a), Ok(b)) = (resolve_item(item1), resolve_item(item2)) {
- if let Some(p) = CanonicalItemUrl::parse(&a).and_then(|c| c.parent()) { parents.insert(p); }
- if let Some(p) = CanonicalItemUrl::parse(&b).and_then(|c| c.parent()) { parents.insert(p); }
+ if let Some(p) = a.parent() { parents.insert(p); }
+ if let Some(p) = b.parent() { parents.insert(p); }
}
}
}
@@ -961,7 +957,7 @@ fn rpc_search(reduced: &ReducerState, q: &str, limit: usize, principal: Option<&
async fn rpc_get_pair(state: &AppState, room: String, parent_path: String) -> Result<RpcResult, RpcErr> {
let scope = scope_from_room_wire(&room);
let reduced_arc = state.reduced.clone();
- let pool: Vec<String> = {
+ let pool: Vec<CanonicalItemUrl> = {
let reduced = reduced_arc.read().await;
let content = content_for_room(&reduced, &room);
let tmp = if parent_path.trim().is_empty() {
@@ -970,12 +966,11 @@ async fn rpc_get_pair(state: &AppState, room: String, parent_path: String) -> Re
Some(parent_path.clone())
};
let specs = parse_parent_specs(tmp.as_ref());
- let raw_pool: Vec<CanonicalItemUrl> = if specs.is_empty() {
+ if specs.is_empty() {
content.ranking_group.idx_to_item.clone()
} else {
crate::scope_rank::resolve_scope(content, &specs)
- };
- raw_pool.into_iter().map(|it| it.0).collect()
+ }
};
if pool.len() < 2 {
return Err((
@@ -983,31 +978,30 @@ async fn rpc_get_pair(state: &AppState, room: String, parent_path: String) -> Re
Some("add items via ingest".into()),
));
}
- let selected: Option<(String, String)> = {
+ let selected: Option<(CanonicalItemUrl, CanonicalItemUrl)> = {
let mut reduced = reduced_arc.write().await;
let content = reduced.content.entry(scope.clone()).or_default();
let group = &mut content.ranking_group;
if group.idx_to_item.is_empty() {
- pick_random_distinct(&pool)
+ pick_random_distinct_canonical(&pool)
} else {
let mut rng = rand::thread_rng();
- let idxs: Vec<usize> = pool.iter()
- .filter_map(|it| {
- let key = CanonicalItemUrl(it.clone());
- group.item_to_idx.get(&key).copied()
- })
+ let idxs: Vec<usize> = pool
+ .iter()
+ .filter_map(|it| group.item_to_idx.get(it).copied())
.collect();
let ranked = ranked_items_subset(group, &idxs, 10000, 1e-8);
- let ranked_set: HashSet<String> = ranked.iter().map(|r| r.item.as_str().to_string()).collect();
- let unsorted: Vec<String> = pool.iter()
+ let ranked_set: HashSet<CanonicalItemUrl> = ranked.iter().map(|r| r.item.clone()).collect();
+ let unsorted: Vec<CanonicalItemUrl> = pool
+ .iter()
.filter(|it| !ranked_set.contains(*it))
.cloned()
.collect();
- let mut pick: Option<(String, String)> = None;
+ let mut pick: Option<(CanonicalItemUrl, CanonicalItemUrl)> = None;
if !unsorted.is_empty() {
if let Some(left) = unsorted.choose(&mut rng).cloned() {
- let mut candidates: Vec<String> = if !ranked.is_empty() {
- ranked.iter().map(|r| r.item.as_str().to_string()).collect()
+ let mut candidates: Vec<CanonicalItemUrl> = if !ranked.is_empty() {
+ ranked.iter().map(|r| r.item.clone()).collect()
} else {
pool.clone()
};
@@ -1021,21 +1015,21 @@ async fn rpc_get_pair(state: &AppState, room: String, parent_path: String) -> Re
let a = ranked[i].item.as_str();
let b = ranked[i + 1].item.as_str();
if a != b && !is_pair_voted(group, a, b) {
- pick = Some((a.to_string(), b.to_string()));
+ pick = Some((ranked[i].item.clone(), ranked[i + 1].item.clone()));
break;
}
}
if pick.is_none() {
for _ in 0..64 {
let (Some(a), Some(b)) = (pool.choose(&mut rng).cloned(), pool.choose(&mut rng).cloned()) else { break; };
- if a != b && !is_pair_voted(group, &a, &b) {
+ if a != b && !is_pair_voted(group, a.as_str(), b.as_str()) {
pick = Some((a, b));
break;
}
}
}
}
- pick.or_else(|| pick_random_distinct(&pool))
+ pick.or_else(|| pick_random_distinct_canonical(&pool))
}
};
let Some((left, right)) = selected else {
@@ -1043,8 +1037,8 @@ async fn rpc_get_pair(state: &AppState, room: String, parent_path: String) -> Re
};
let reduced = reduced_arc.read().await;
let content = content_for_ro
… preview truncated; 8,683 characters omittedHardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.