Side A fixes real correctness bugs (out-of-band delegate attribution instead of embedding agent tag in DSL body, erroring on insufficient sibling pool instead of silently falling back to all items) and consistently renames the route across server code and all test suites. Side B mainly improves href encoding (a smaller correctness nit) and rewrites a browser test for stronger assertions, which is valuable but narrower in scope and impact than A's fixes to core RPC/vote semantics.
constitution · epochs · watch · epoch 3
comparison
c_11d6a0ec9839 (tommy-mor) vs c_4a5c84c0a37b (tommy-mor)
download prompt · raw event · cmp_3c314531536cd5
council reasoning
A lands real behavioral fixes: WEB_BROWSER_AGENT moved to out-of-band delegate_opt (correct attribution), sibling pools with <2 items return None instead of falling back to the global item set, and dead “swap sides” UI is removed. B’s display_path href fix is worthwhile and the 45-pair ranking assertion is strong test value, but it is a narrower production change plus test hardening versus A’s multiple correctness fixes.
Side A fixes multiple production behaviors: it correctly passes delegate attribution out-of-band instead of embedding it in DSL text, prevents an incorrect fallback to all items when a sibling pool has fewer than two candidates by returning no next pair, and simplifies the public interface by renaming `/vote/compare` to `/vote` while removing the unnecessary swap button. Side B improves URL generation to use display paths and substantially strengthens browser test coverage with a full 45-pair ranking verification, but much of its patch is test code rather than changes to runtime behavior.
sides
A — c_11d6a0ec9839 (tommy-mor)
message
[36e92cf2] Fix /vote/compare page: attribution, fallback, swap button, and route rename. - Pass WEB_BROWSER_AGENT as delegate_opt out-of-band (not in DSL body text) - Error instead of falling back to all items when sibling pool < 2 - Remove pointless "swap sides" button - Rename route /vote/compare → /vote Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
diff preview
diff --git a/server/src/api/rpc.rs b/server/src/api/rpc.rs
index 0b7070a2b7289e5c40778c8d062bd4150285a8df..8e3d8e34922ddaaa89ea699edd99402bd40fdb1b 100644
--- a/server/src/api/rpc.rs
+++ b/server/src/api/rpc.rs
@@ -281,6 +281,7 @@ pub async fn rpc_post_with_bearer(
bearer_token: &str,
room: String,
thread_tag: String,
+ delegate_opt: Option<String>,
text: String,
) -> Result<RpcResult, RpcErr> {
use axum::http::{header, HeaderMap, HeaderValue};
@@ -288,7 +289,7 @@ pub async fn rpc_post_with_bearer(
let hv = HeaderValue::from_str(&format!("Bearer {bearer_token}"))
.map_err(|_| ("invalid session token".into(), None))?;
headers.insert(header::AUTHORIZATION, hv);
- rpc_post(state, &headers, room, thread_tag, None, text, false).await
+ rpc_post(state, &headers, room, thread_tag, delegate_opt, text, false).await
}
async fn rpc_check(
diff --git a/server/src/api/ui_html.rs b/server/src/api/ui_html.rs
index a13a94767d8905adc09e31effff838e166781b6c..5faa642451d69555cb391974beb0ad22c9355c8c 100644
--- a/server/src/api/ui_html.rs
+++ b/server/src/api/ui_html.rs
@@ -118,6 +118,7 @@ async fn dispatch_ui_action(
&session.bearer,
room.clone(),
thread_tag.clone(),
+ None,
text,
)
.await
@@ -246,8 +247,7 @@ async fn dispatch_ui_action(
}
let text = format!(
- "@{}\n{{\n{}\n}}\n{} {}:{} {}\n",
- crate::api::auth::WEB_BROWSER_AGENT,
+ "{{\n{}\n}}\n{} {}:{} {}\n",
exp,
left_id.as_str(),
rl,
@@ -260,6 +260,7 @@ async fn dispatch_ui_action(
&session.bearer,
room.clone(),
thread_tag.clone(),
+ Some(crate::api::auth::WEB_BROWSER_AGENT.to_string()),
text,
)
.await
diff --git a/server/src/html/forum/nav.rs b/server/src/html/forum/nav.rs
index d33dc50f0b89a5ed7730c3bca204a67f6309388f..ae67f550a4bdf0bbeff58384c29a715015b64bd5 100644
--- a/server/src/html/forum/nav.rs
+++ b/server/src/html/forum/nav.rs
@@ -85,7 +85,7 @@ impl ThreadNav {
format!("{}/{}/{}", self.thread_path_prefix, tag, idx)
}
- /// Empty for public; `/r/:seg` for room — prefix for routes like `/vote/compare`.
+ /// Empty for public; `/r/:seg` for room — prefix for routes like `/vote`.
pub(crate) fn room_path_prefix_for_vote_compare(&self) -> String {
match &self.scope {
ScopeId::Public => String::new(),
diff --git a/server/src/html/garden/vote.rs b/server/src/html/garden/vote.rs
index bdb4008c29ba49d7dc97b1cd2c32d9cdfce1ca04..1d7fc8aa7436bfa9c1186dfd940a8d751ab7e088 100644
--- a/server/src/html/garden/vote.rs
+++ b/server/src/html/garden/vote.rs
@@ -208,7 +208,7 @@ pub(crate) async fn vote_compare_post_success_js(
state: &AppState,
nav: &ThreadNav,
_room_wire: &str,
- thread_tag: &str,
+ _thread_tag: &str,
left: &ItemId,
right: &ItemId,
_post_id: &str,
@@ -218,8 +218,7 @@ pub(crate) async fn vote_compare_post_success_js(
let content = content_for_garden_view(&reduced, &nav.scope());
let edge_history = vote_edge_history_markup(content, left, right);
let next_pair = suggest_next_vote_pair(content, left, right);
- let nav_markup =
- vote_compare_nav_markup(nav, next_pair.as_ref(), left, right, Some(thread_tag));
+ let nav_markup = vote_compare_nav_markup(nav, next_pair.as_ref());
drop(reduced);
JsBuilder::new()
.morph_inner_selector("#vote-edge-history-region", edge_history)
@@ -236,7 +235,7 @@ pub(super) fn vote_compare_href(
let left_q = urlencoding::encode(left.as_str());
let right_q = urlencoding::encode(right.as_str());
let base = format!(
- "{}/vote/compare?left={}&right={}",
+ "{}/vote?left={}&right={}",
nav.room_path_prefix_for_vote_compare(),
left_q,
right_q
@@ -251,12 +250,8 @@ pub(super) fn vote_compare_href(
fn vote_compare_nav_markup(
nav: &ThreadNav,
next_pair: Option<&(ItemId, ItemId)>,
- left: &ItemId,
- right: &ItemId,
- thread_override: Option<&str>,
) -> maud::Markup {
let next_pair_href = next_pair.map(|(nl, nr)| vote_compare_href(nav, nl, nr, None));
- let swap_pair_href = vote_compare_href(nav, right, left, thread_override);
html! {
div class="vote-compare-nav" {
@if let Some(href) = &next_pair_href {
@@ -264,7 +259,6 @@ fn vote_compare_nav_markup(
} @else {
span class="vote-compare-next is-disabled" { "no next pair" }
}
- a class="vote-compare-next" href=(swap_pair_href) { "swap sides" }
}
}
}
@@ -274,7 +268,7 @@ pub(super) fn suggest_next_vote_pair(
current_left: &ItemId,
current_right: &ItemId,
) -> Option<(ItemId, ItemId)> {
- let mut pool: Vec<ItemId> = if current_left.parent().as_ref().map(|p| p.as_str())
+ let pool: Vec<ItemId> = if current_left.parent().as_ref().map(|p| p.as_str())
== current_right.parent().as_ref().map(|p| p.as_str())
{
current_left
@@ -291,7 +285,7 @@ pub(super) fn suggest_next_vote_pair(
Vec::new()
};
if pool.len() < 2 {
- pool = content.items.iter().cloned().collect();
+ return None;
}
suggest_next_pair_in_pool(
&content.ranking_group,
@@ -334,7 +328,7 @@ pub struct VoteCompareQuery {
pub thread: Option<String>,
}
-/// Public pairwise vote UI — `/vote/compare?left=&right=&thread=`.
+/// Public pairwise vote UI — `/vote?left=&right=&thread=`.
pub async fn vote_compare_page(
State(state): State<AppState>,
Query(q): Query<VoteCompareQuery>,
@@ -426,7 +420,7 @@ async fn vote_compare_inner(
let next_path = uri
.path_and_query()
.map(|pq| pq.as_str().to_string())
- .unwrap_or_else(|| "/vote/compare".into());
+ .unwrap_or_else(|| "/vote".into());
let rpc_json = template_json_compact(&json!({
"action": "vote_compare_post",
@@ -462,7 +456,7 @@ async fn vote_compare_inner(
Some(&item_bodies_for_cards),
))
}
- (vote_compare_nav_markup(&nav, next_pair.as_ref(), &left, &right, q.thread.as_deref()))
+ (vote_compare_nav_markup(&nav, next_pair.as_ref()))
div id="vote-edge-history-region" {
(edge_history)
}
diff --git a/server/src/html/ui_action.rs b/server/src/html/ui_action.rs
index c7b643c8dde37dd1e3001f5ff716fe4f79d24c5e..dd5294202569454acf9f41b458718700bbf96f39 100644
--- a/server/src/html/ui_action.rs
+++ b/server/src/html/ui_action.rs
@@ -38,7 +38,7 @@ pub enum HtmlUiAction {
#[serde(default)]
form_id: Option<String>,
},
- /// Post a pairwise vote from `/vote/compare` (browser compose).
+ /// Post a pairwise vote from `/vote` (browser compose).
VoteComparePost {
room: String,
thread_tag: String,
diff --git a/server/src/lib.rs b/server/src/lib.rs
index ad8e31099c807fb5844acb16cd5086a2f19327a7..e102e73ec167fa35dcc22d4ec8c6f40202982429 100644
--- a/server/src/lib.rs
+++ b/server/src/lib.rs
@@ -82,9 +82,9 @@ pub fn create_app(state: AppState) -> Router {
.route("/u/:username", get(crate::html::user_profile_page))
.route("/try", get(crate::html::editor_page))
.route("/try/check", post(crate::html::editor_check))
- .route("/vote/compare", get(crate::html::vote_compare_page))
+ .route("/vote", get(crate::html::vote_compare_page))
.route(
- "/r/:room_key/vote/compare",
+ "/r/:room_key/vote",
get(crate::html::room_vote_compare_page),
)
.route("/~", get(crate::html::garden_index))
diff --git a/server/tests/integration_html.rs b/server/tests/integration_html.rs
index a25a456c7cafd74a69e65e0c2e8ed2bfb2e27498..b3e31376737b2d5aabb739f408984a1f5ae0c974 100644
--- a/server/tests/integration_html.rs
+++ b/server/tests/integration_html.rs
@@ -52,12 +52,12 @@ async fn test_view_counts_increment_and_display() {
.normalized_storage()
.to_storage_string();
let vote_q_right_first = format!(
- "/vote/compare?right={}&left={}",
+ "/vote?right={}&left={}",
urlencoding::encode(&right),
urlencoding::encode(&left)
);
let vote_q_left_first = format!(
- "/vote/compare?left={}&right={}",
+ "/vote?left={}&right={}",
urlencoding::encode(&left),
urlencoding::encode(&right)
);
@@ -148,7 +148,7 @@ https://github.com/ghvotehi/a/issues/10 {\n\
.normalized_storage()
.to_storage_string();
let q = format!(
- "/vote/compare?left={}&right={}",
+ "/vote?left={}&right={}",
urlencoding::encode(&left),
urlencoding::encode(&right)
);
diff --git a/server/tests/integration_ui.rs b/server/tests/integration_ui.rs
index 091afd922d28f48ab36aa9b9084aa825036d1f74..23db7b5672418d5bb0ab7529e05ef1f89e59062a 100644
--- a/server/tests/integration_ui.rs
+++ b/server/tests/integration_ui.rs
@@ -110,7 +110,7 @@ async fn test_choose_username_carries_redirect_next() {
.expect("pending session must exist");
pending.provider = Some("google".to_string());
pending.provider_id = Some("google-user-redirect".to_string());
- pending.redirect_next = Some("/vote/compare?left=%7E%2Fa&right=%7E%2Fb".to_string());
+ pending.redirect_next = Some("/vote?left=%7E%2Fa&right=%7E%2Fb".to_string());
}
let choose = client
@@ -122,7 +122,7 @@ async fn test_choose_username_carries_redirect_next() {
assert_eq!(choose.status(), reqwest::StatusCode::OK);
let body = choose.text().await.unwrap();
- assert!(body.contains("window.location = \"/vote/compare?left=%7E%2Fa&right=%7E%2Fb\""));
+ assert!(body.contains("window.location = \"/vote?left=%7E%2Fa&right=%7E%2Fb\""));
}
#[tokio::test]
diff --git a/test/browser_vote_compare.clj b/test/browser_vote_compare.clj
index 7cd1e467fdd8a4480df1be2afd78adb45b9cdc6d..a98e144756421e1fa5064db6a9fc0dd8b08471c7 100644
--- a/test/browser_vote_compare.clj
+++ b/test/browser_vote_compare.clj
@@ -66,7 +66,7 @@
:headers {"Authorization" (str "Bearer " alice-token)})
post-json (json/parse-string (:body post-resp) false)
_ (is (true? (get-in post-json ["results" 0 "ok"])) "seed items + edge vote via rpc")
- cmp-url (str base-url "/vote/compare?left=" (enc left-url) "&right=" (enc third-url))]
+ cmp-url (str base-url "/vote?left=" (enc left-url) "&right=" (enc third-url))]
(core/with-playwright [pw]
(core/with-browser [browser (core/launch-chromium pw {:headless true :channel "chrome"})]
(core/with-context [ctx (core/new-context browser)]
B — c_4a5c84c0a37b (tommy-mor)
message
[0728c06a] Vote pool: use display_path in hrefs; test all 45 pairs + assert ranking. - vote_compare_href and vote_pool_href now encode ~/… and -/… as their short display forms (not the full https://slug.social/… storage URL), matching what users see in the item display and DSL. - Rewrite browser_vote_pool test to vote all C(10,2)=45 pairs in the pool, always preferring the alphabetically-earlier letter, then query GetGardenRank and assert the 10 items form one component ranked a→j. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
diff preview
diff --git a/server/src/html/garden/vote.rs b/server/src/html/garden/vote.rs
index 2682cfcbd2f834b56459a02831aa225cffe67c58..d0ec78cd675eae284d056fb3b8eaf5cc853d6263 100644
--- a/server/src/html/garden/vote.rs
+++ b/server/src/html/garden/vote.rs
@@ -234,8 +234,10 @@ pub(super) fn vote_compare_href(
thread_override: Option<&str>,
pool: Option<&ItemId>,
) -> String {
- let left_q = urlencoding::encode(left.as_str());
- let right_q = urlencoding::encode(right.as_str());
+ let left_dp = left.display_path();
+ let right_dp = right.display_path();
+ let left_q = urlencoding::encode(&left_dp);
+ let right_q = urlencoding::encode(&right_dp);
let mut base = format!(
"{}/vote?left={}&right={}",
nav.room_path_prefix_for_vote_compare(),
@@ -246,16 +248,20 @@ pub(super) fn vote_compare_href(
base = format!("{}&thread={}", base, urlencoding::encode(t));
}
if let Some(p) = pool {
- base = format!("{}&pool={}", base, urlencoding::encode(p.as_str()));
+ let pool_dp = p.display_path();
+ base = format!("{}&pool={}", base, urlencoding::encode(&pool_dp));
}
base
}
pub(super) fn vote_pool_href(nav: &ThreadNav, pool_item_str: &str) -> String {
+ let display = ItemId::parse(pool_item_str)
+ .map(|i| i.display_path())
+ .unwrap_or_else(|| pool_item_str.to_string());
format!(
"{}/vote?pool={}",
nav.room_path_prefix_for_vote_compare(),
- urlencoding::encode(pool_item_str)
+ urlencoding::encode(&display)
)
}
diff --git a/test/browser_vote_pool.clj b/test/browser_vote_pool.clj
index d51f05db47dc3cb013e56e65f3a1edfbbcbd96ab..23d0bd80b02bd8b1b48853454bed02793296550e 100644
--- a/test/browser_vote_pool.clj
+++ b/test/browser_vote_pool.clj
@@ -1,6 +1,8 @@
(ns test.browser-vote-pool
- "Pool-scoped voting: seed ~/pool/a-j, enter via /vote?pool=~/pool, follow
- the vote → next-pair → vote sequence until no next pair or 15 iterations."
+ "Pool-scoped voting: seed ~/pool/a-j (10 letters), follow the
+ vote → next-pair sequence for all C(10,2)=45 pairs voting the
+ alphabetically-earlier item each time, then assert the garden
+ ranking is a…j in order."
(:require [babashka.fs :as fs]
[cheshire.core :as json]
[clojure.string :as str]
@@ -11,26 +13,38 @@
[test.common :as common]
[test.oauth :as oauth]))
+(def letters ["a" "b" "c" "d" "e" "f" "g" "h" "i" "j"])
+(def total-pairs (/ (* (count letters) (dec (count letters))) 2)) ; C(10,2) = 45
+
(defn- wait-for-text [pg selector expected timeout-ms]
(let [deadline (+ (System/currentTimeMillis) timeout-ms)]
(loop []
- (let [text (locator/text-content (page/locator pg selector))]
+ (let [text (try (locator/text-content (page/locator pg selector)) (catch Exception _ nil))]
(if (and (string? text) (str/includes? text expected))
true
(if (< (System/currentTimeMillis) deadline)
- (do (Thread/sleep 200) (recur))
+ (do (Thread/sleep 150) (recur))
false))))))
(defn- element-text [pg selector]
- (try (locator/text-content (page/locator pg selector)) (catch Exception _ nil)))
+ (try (locator/text-content (page/locator pg selector)) (catch Exception _ "")))
(defn- enc [^String s]
(java.net.URLEncoder/encode s "UTF-8"))
-(def letters ["a" "b" "c" "d" "e" "f" "g" "h" "i" "j"])
+;; Extract the terminal path segment, e.g. "~/pool/c" → "c".
+(defn- leaf [path] (last (str/split path #"/")))
+
+;; Set the hidden ratio inputs so the alphabetically-earlier item wins.
+(defn- set-ratio! [pg left-text right-text]
+ (let [[rl rr] (if (neg? (compare (leaf left-text) (leaf right-text)))
+ [100 0] ; left is earlier → prefer left
+ [0 100])] ; right is earlier → prefer right
+ (page/evaluate pg (str "document.getElementById('vote-ratio-left').value='" rl "'"))
+ (page/evaluate pg (str "document.getElementById('vote-ratio-right').value='" rr "'"))))
(defn vote-pool-flow! []
- (println "\n━━━ browser vote pool (/vote?pool= seeds + follow next-pair sequence) ━━━\n")
+ (println (str "\n━━━ browser vote pool (all " total-pairs " pairs → sorted ranking) ━━━\n"))
(common/letlocals
(bind build (common/run-cargo-build-release! ["slugsocial-server"]))
@@ -54,7 +68,6 @@
(let [alice-token (oauth/fetch-bearer-token! base-url :username "alice")
thread-tag "browser-vote-pool"
- ;; seed ~/pool/a through ~/pool/j as items with bodies
item-lines (str/join "\n"
(map (fn [l] (str "~/pool/" l " {" l "}")) letters))
raw (str "# " thread-tag "\n\n~/pool {root}\n" item-lines "\n")
@@ -77,51 +90,57 @@
(page/navigate pg (str base-url "/login"))
(is (wait-for-text pg "body" "@alice" 15000) "alice session after login")
- ;; Enter via pool URL — page picks first pair automatically.
(page/navigate pg pool-url)
(is (wait-for-text pg "body.view-vote-compare" "compare" 15000)
"pool entry: vote compare page loads")
- ;; Verify the initial pair is within the pool.
- (let [pair-text (element-text pg ".vote-compare-pair")]
- (is (and (string? pair-text) (str/includes? pair-text "~/pool/"))
- (str "initial pair is within ~/pool: " pair-text)))
-
- ;; Follow vote → next-pair sequence up to 15 iterations.
- (let [votes-cast
- (loop [i 0]
- (if (>= i 15)
- i
- (let [explanation (str "pool vote " i " reason")]
- (locator/fill (page/locator pg "#vote-explain") explanation)
- (locator/click (page/locator pg "#vote-compare-form button[type=submit]"))
- ;; Wait for edge history morph confirming the vote landed.
- (if-not (wait-for-text pg "ul.vote-edge-history" explanation 20000)
- (do (println " vote" i "history morph timed out — stopping")
- i)
- (let [has-next (wait-for-text pg "[data-testid=\"vote-next-pair\"]"
- "next pair" 8000)]
- (if-not has-next
- ;; "no next pair" — pool exhausted.
- (do (println " no next pair after vote" i " — pool exhausted")
- (inc i))
- (do
- ;; Verify the pair on this page is within the pool before advancing.
- (let [pt (element-text pg ".vote-compare-pair")]
- (is (and (string? pt) (str/includes? pt "~/pool/"))
- (str "pair at vote " i " is within ~/pool: " pt)))
- (locator/click (page/locator pg "[data-testid=\"vote-next-pair\"]"))
- ;; Wait for next pair to load.
- (wait-for-text pg "body.view-vote-compare" "compare" 10000)
- (recur (inc i)))))))))]
-
- (is (>= votes-cast 1) (str "cast at least 1 vote, got: " votes-cast))
- (println (str " pool voting sequence complete: " votes-cast " vote(s) cast")))
-
- ;; After the sequence, the current page is still a pool-scoped vote page.
- (let [url (page/url pg)]
- (is (str/includes? (or url "") "/vote")
- (str "still on /vote after sequence: " url))))))))
+ ;; Vote all 45 pairs, always preferring the alphabetically-earlier item.
+ (loop [votes-cast 0]
+ (when (< votes-cast total-pairs)
+ (let [left-text (element-text pg ".vote-compare-left code")
+ right-text (element-text pg ".vote-compare-right code")]
+ (is (str/includes? left-text "~/pool/")
+ (str "vote " votes-cast ": left is in pool: " left-text))
+ (is (str/includes? right-text "~/pool/")
+ (str "vote " votes-cast ": right is in pool: " right-text))
+ (set-ratio! pg left-text right-text)
+ (let [winner (if (neg? (compare (leaf left-text) (leaf right-text)))
+ (leaf left-text) (leaf right-text))]
+ (locator/fill (page/locator pg "#vote-explain")
+ (str "prefer " winner)))
+ (locator/click (page/locator pg "#vote-compare-form button[type=submit]"))
+ (is (wait-for-text pg "ul.vote-edge-history" "prefer " 20000)
+ (str "vote " votes-cast " appears in edge history"))
+ (when (< (inc votes-cast) total-pairs)
+ (is (wait-for-text pg "[data-testid=\"vote-next-pair\"]" "next pair" 8000)
+ (str "next pair available after vote " votes-cast))
+ (locator/click (page/locator pg "[data-testid=\"vote-next-pair\"]"))
+ (is (wait-for-text pg "body.view-vote-compare" "compare" 10000)
+ (str "vote page loaded for pair " (inc votes-cast))))
+ (recur (inc votes-cast)))))
+
+ (println (str " cast all " total-pairs " votes"))
+
+ ;; Query the ranking via RPC and assert alphabetical order.
+ (let [rank-resp (oauth/http-post-json
+ (str base-url "/api/v0/rpc")
+ [{"GetGardenRank" {"room" "public"
+ "parent_path" "~/pool"}}]
+ :headers {"Authorization" (str "Bearer " alice-token)})
+ rank-json (json/parse-string (:body rank-resp) true)
+ result (get-in rank-json [:results 0 :result :GardenRank])
+ components (:components result)
+ unranked (:unranked_items result)
+ ranked (mapv :item (mapcat :ranking components))
+ ranked-leaves (mapv #(last (str/split % #"[/~]+")) ranked)]
+ (is (= 1 (count components))
+ (str "all 10 items form one connected component (got " (count components) ")"))
+ (is (empty? unranked)
+ (str "no unranked items (got " (count unranked) ")"))
+ (is (= 10 (count ranked))
+ (str "10 items ranked (got " (count ranked) ")"))
+ (is (= letters ranked-leaves)
+ (str "ranking is alphabetical a→j (got " ranked-leaves ")"))))))))
(finally
(when-some [s @!server] (common/kill-server s))
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.