Commit B is a substantive refactor that removes an entire theme-switching subsystem (CSS files, cookie handling, form endpoint) and replaces query-param item addressing with a cleaner path-based `/~/` browse route, backed by new URL parsing logic and tests. Commit A is a smaller, more localized fix (replacing a fallback default with an explicit panic/expect and correcting doc comments/indexing semantics), which is useful but far more limited in scope and lasting architectural impact.
constitution · epochs · watch · epoch 3
comparison
c_6a02ffb06a41 (tommy-mor) vs c_14dd365ffe22 (tommy-mor)
download prompt · raw event · cmp_98306d06303d1d
council reasoning
B introduces a lasting routing design (`ItemId::browse_href` / `/~/…`), removes the theme cookie/CSS machinery, and wires real browse handlers with tests—foundational product structure. A is a correct but narrow consistency fix (0-based `thread_post_index`, `expect` instead of silent `unwrap_or(0)`, always-on post links) with supporting asserts, so it matters less overall.
Side B makes a substantial architectural change by replacing query-parameter navigation with canonical `/~/...` browse routes, centralizing URL generation/parsing in `ItemId` (`browse_href`, `from_browse_uri`), updating routing, and adding integration tests for the new behavior. Side A mainly tightens an invariant by replacing a silent fallback with `expect`, aligns documentation/tests around 0-based thread indices, and always renders the post link, which is useful but much narrower in lasting impact.
sides
A — c_6a02ffb06a41 (tommy-mor)
message
[3bc88847] removed optional
diff preview
diff --git a/server/src/api/rpc.rs b/server/src/api/rpc.rs
index ba93ca87182d49f57dffc8220f604ffa150f9a6c..5049a26b096bd8435d6eb9e75ccb751b6f489061 100644
--- a/server/src/api/rpc.rs
+++ b/server/src/api/rpc.rs
@@ -1337,8 +1337,7 @@ pub async fn handle_rpc_batch(
.ingests_by_scope_thread
.get(&(scope.clone(), e.thread.clone()))
.and_then(|q| q.iter().rev().position(|id| id == &e.post_id))
- .map(|i| i + 1)
- .unwrap_or(0);
+ .expect("rank history post_id must be in ingests_by_scope_thread for (scope, thread)");
RankHistoryRow {
ts: e.ts,
scope_rank: e.scope_rank,
diff --git a/server/src/html/garden.rs b/server/src/html/garden.rs
index 319feb15a6b68d2b5df98b4289fedbc9bdd048d3..23245d6fdfc9019b199ab8417150faf5f3297067 100644
--- a/server/src/html/garden.rs
+++ b/server/src/html/garden.rs
@@ -450,6 +450,7 @@ struct RankHistoryEntryView {
scope_total: usize,
scope_rank_delta: i32,
thread: String,
+ /// 0-based index as [`crate::html::forum::ingest::thread_post_index_in_scope`] / `/t/tag/N`.
thread_post_index: usize,
caused_by: Vec<crate::reducer::VoteData>,
}
@@ -573,11 +574,11 @@ fn build_rank_history(
})
.unwrap_or_default();
- let thread_post_index = reduced.ingests_by_scope_thread
+ let thread_post_index = reduced
+ .ingests_by_scope_thread
.get(&(scope.clone(), e.thread.clone()))
.and_then(|q| q.iter().rev().position(|id| id == &e.post_id))
- .map(|i| i + 1)
- .unwrap_or(0);
+ .expect("rank history post_id must be in ingests_by_scope_thread for (scope, thread)");
RankHistoryEntryView {
ts: e.ts,
@@ -704,11 +705,9 @@ async fn render_scope_view(
span class="muted" { (ago) (label) }
" · "
a href=(thread_href(&e.thread)) { "#" (e.thread) }
- @if e.thread_post_index > 0 {
- " "
- a href=(format!("{}/{}", thread_href(&e.thread), e.thread_post_index)) {
- span class="muted" { "post #" (e.thread_post_index) }
- }
+ " "
+ a href=(format!("{}/{}", thread_href(&e.thread), e.thread_post_index)) {
+ span class="muted" { "post #" (e.thread_post_index) }
}
}
@if e.caused_by.is_empty() {
diff --git a/server/tests/integration.rs b/server/tests/integration.rs
index d4c5bfe9c6f1c71dc61878bd8c5e729b1b7c69ef..9766adb43ad315a64f5df17b79f66718ce149509 100644
--- a/server/tests/integration.rs
+++ b/server/tests/integration.rs
@@ -1322,6 +1322,11 @@ async fn test_rank_history() {
assert_eq!(entry["scope_rank_delta"], 0, "delta is 0 on first appearance");
let caused_by = entry["caused_by"].as_array().unwrap();
assert_eq!(caused_by.len(), 2, "both votes in the ingest touched rust");
+ assert_eq!(
+ entry["thread_post_index"],
+ 0,
+ "rank history links use same 0-based index as /t/hist-test/0"
+ );
ingest(
"00000000-0000-0000-0000-000000000002:rig:test/model",
@@ -1349,6 +1354,16 @@ async fn test_rank_history() {
assert_eq!(caused_by2.len(), 1);
assert!(caused_by2[0]["a"].as_str().unwrap().ends_with("python") ||
caused_by2[0]["b"].as_str().unwrap().ends_with("python"));
+ assert_eq!(
+ hist2[0]["thread_post_index"],
+ 0,
+ "first hist-test post is chronological index 0"
+ );
+ assert_eq!(
+ hist2[1]["thread_post_index"],
+ 1,
+ "second ingest is chronological index 1"
+ );
let hist_rust2 = rpc_batch(
&client,
diff --git a/types/src/lib.rs b/types/src/lib.rs
index edbb923e4d7d41ad82dfc254c3bd697562383527..49abba8ea9f786ef68e3157d2a5d309e15e09ba0 100644
--- a/types/src/lib.rs
+++ b/types/src/lib.rs
@@ -253,7 +253,7 @@ pub struct FeedPost {
/// Primary thread tag (without #), if the ingest declared one.
#[serde(skip_serializing_if = "Option::is_none")]
pub thread: Option<String>,
- /// 1-indexed chronological position of this post within the thread.
+ /// 1-based display ordinal for this post within the thread (feed only; URLs use 0-based paths).
#[serde(skip_serializing_if = "Option::is_none")]
pub thread_post_index: Option<usize>,
/// Full raw body of the ingest document.
@@ -628,7 +628,7 @@ pub struct RankHistoryRow {
pub score: f64,
/// Thread tag of the ingest that triggered this rank change.
pub thread: String,
- /// 1-indexed chronological position of this post within the thread.
+ /// 0-indexed chronological position of this post within the thread (same as `/t/tag/N` routes).
pub thread_post_index: usize,
/// Votes from this ingest that directly touched this item. Empty when change was transitive.
pub caused_by: Vec<VoteRow>,
B — c_14dd365ffe22 (tommy-mor)
message
[8d5328d3] -theme
diff preview
diff --git a/server/src/api/ui_html.rs b/server/src/api/ui_html.rs
index 82b2aa51d21ada1d0d849d3ddfc3a81e4241d861..e74e42e0f0ffc4224f5ef5421b486a5f35434999 100644
--- a/server/src/api/ui_html.rs
+++ b/server/src/api/ui_html.rs
@@ -73,11 +73,7 @@ pub async fn post_ui_html(
ensure_partial_tree(&mut tree, &item);
}
let _ = state.ensure_node(&item).await;
- let dest = if item.is_root() {
- "/".to_string()
- } else {
- format!("/?item={}", item.as_str())
- };
+ let dest = item.browse_href();
JsBuilder::new()
.raw(&format!(
"window.location.href={};",
diff --git a/server/src/html/mod.rs b/server/src/html/mod.rs
index c973cb718ac74b95570dabea76e24459417790b9..a8353c6de0cd6d268219e552b7ca1ba4e3000585 100644
--- a/server/src/html/mod.rs
+++ b/server/src/html/mod.rs
@@ -1,13 +1,10 @@
use axum::{
body::Body,
extract::{Path, State},
- http::{header, HeaderValue, StatusCode, Uri},
+ http::{header, StatusCode, Uri},
response::{IntoResponse, Response},
- Form,
};
-use axum_extra::extract::cookie::CookieJar;
use maud::{html, Markup, DOCTYPE};
-use serde::Deserialize;
use crate::{
form_template::template_json_compact,
@@ -15,96 +12,25 @@ use crate::{
path_types::ItemId,
ranking::{top_bottom, RankedItem},
reducer::{GroupState, NodeState},
- state::{parse_item_param, AppState},
+ state::AppState,
ui_action::UI_RPC_FIELD,
};
-const THEME_DEFAULT_CSS: &str = include_str!("../../static/theme_default.css");
-const THEME_RETRO_CSS: &str = include_str!("../../static/theme_retro.css");
+const SORTER_CSS: &str = include_str!("../../static/sorter.css");
const SORTER_UI_JS: &str = include_str!("../../static/sorter_ui.js");
-pub const SORTER_THEME_COOKIE: &str = "sorter-theme";
-
-pub fn normalize_theme(raw: &str) -> &'static str {
- match raw {
- "retro" => "retro",
- _ => "default",
- }
-}
-
-pub fn theme_from_jar(jar: &CookieJar) -> &'static str {
- jar.get(SORTER_THEME_COOKIE)
- .map(|c| normalize_theme(c.value()))
- .unwrap_or("default")
-}
-
-pub fn theme_next_from_uri(uri: &Uri) -> String {
- uri.path_and_query()
- .map(|pq| pq.as_str().to_string())
- .filter(|s| !s.is_empty())
- .unwrap_or_else(|| "/".to_string())
-}
-
-pub fn theme_cookie_header_value(theme: &str) -> HeaderValue {
- let t = normalize_theme(theme);
- let s = format!("{SORTER_THEME_COOKIE}={t}; Path=/; SameSite=Lax; Max-Age=31536000");
- HeaderValue::from_str(&s).expect("theme cookie must be ASCII")
-}
-
-fn sanitize_theme_next(next: Option<&str>) -> String {
- let s = next.unwrap_or("/").trim();
- if s.starts_with('/') && !s.starts_with("//") && s.len() < 8192 {
- s.to_string()
- } else {
- "/".to_string()
- }
-}
-
-#[derive(Debug, Deserialize)]
-pub struct ThemeForm {
- theme: String,
- next: Option<String>,
-}
-
-pub async fn post_theme(Form(form): Form<ThemeForm>) -> impl IntoResponse {
- let theme = normalize_theme(&form.theme);
- let next = sanitize_theme_next(form.next.as_deref());
- let loc =
- HeaderValue::try_from(next.as_str()).unwrap_or_else(|_| HeaderValue::from_static("/"));
- Response::builder()
- .status(StatusCode::SEE_OTHER)
- .header(header::LOCATION, loc)
- .header(header::SET_COOKIE, theme_cookie_header_value(theme))
- .body(Body::empty())
- .expect("theme redirect response")
-}
-
pub async fn serve_static(Path(filename): Path<String>) -> impl IntoResponse {
- if filename == "sorter_ui.js" {
- return Response::builder()
- .status(StatusCode::OK)
- .header(header::CONTENT_TYPE, "text/javascript; charset=utf-8")
- .header(header::CACHE_CONTROL, "public, max-age=3600")
- .body(SORTER_UI_JS.to_string())
- .unwrap()
- .into_response();
- }
-
- let theme = filename
- .strip_prefix("theme_")
- .and_then(|s| s.strip_suffix(".css"));
-
- let css = match theme {
- Some("default") => THEME_DEFAULT_CSS,
- Some("retro") => THEME_RETRO_CSS,
+ let (content_type, body) = match filename.as_str() {
+ "sorter.css" => ("text/css; charset=utf-8", SORTER_CSS),
+ "sorter_ui.js" => ("text/javascript; charset=utf-8", SORTER_UI_JS),
_ => return (StatusCode::NOT_FOUND, "static file not found").into_response(),
};
Response::builder()
.status(StatusCode::OK)
- .header(header::CONTENT_TYPE, "text/css; charset=utf-8")
+ .header(header::CONTENT_TYPE, content_type)
.header(header::CACHE_CONTROL, "public, max-age=3600")
- .body(css.to_string())
+ .body(body.to_string())
.unwrap()
.into_response()
}
@@ -162,8 +88,7 @@ fn asset_version() -> &'static str {
V.get_or_init(|| {
use std::hash::{Hash, Hasher};
let mut h = std::collections::hash_map::DefaultHasher::new();
- THEME_DEFAULT_CSS.hash(&mut h);
- THEME_RETRO_CSS.hash(&mut h);
+ SORTER_CSS.hash(&mut h);
SORTER_UI_JS.hash(&mut h);
format!("{:x}", h.finish())
})
@@ -176,9 +101,9 @@ pub fn now_ms() -> i64 {
t.as_millis() as i64
}
-fn layout(title: &str, body: Markup, views: u64, theme: &str, theme_next: &str) -> Markup {
+fn layout(title: &str, body: Markup, views: u64) -> Markup {
let ver = asset_version();
- let css_href = format!("/static/theme_{theme}.css?v={ver}");
+ let css_href = format!("/static/sorter.css?v={ver}");
let js_src = format!("/static/sorter_ui.js?v={ver}");
html! {
(DOCTYPE)
@@ -187,7 +112,7 @@ fn layout(title: &str, body: Markup, views: u64, theme: &str, theme_next: &str)
meta charset="utf-8";
meta name="viewport" content="width=device-width, initial-scale=1";
title { (title) }
- link rel="stylesheet" href=(css_href) id="theme-stylesheet";
+ link rel="stylesheet" href=(css_href);
script src="https://unpkg.com/idiomorph@0.3.0/dist/idiomorph.min.js" {}
}
body class="home" {
@@ -196,21 +121,6 @@ fn layout(title: &str, body: Markup, views: u64, theme: &str, theme_next: &str)
}
div id="errors" {}
(body)
- div id="controls" {
- a href="https://github.com/sortersocial/sorter2" id="src-link" { "src" }
- form id="sorter-theme-form" method="post" action="/theme" data-navigate="full" {
- input type="hidden" name="next" value=(theme_next);
- select id="theme-select" name="theme" onchange="this.form.submit()" aria-label="Theme" {
- @for (val, label) in [("default", "default"), ("retro", "retro")] {
- @if theme == val {
- option value=(val) selected { (label) }
- } @else {
- option value=(val) { (label) }
- }
- }
- }
- }
- }
script src=(js_src) {}
}
}
@@ -218,11 +128,7 @@ fn layout(title: &str, body: Markup, views: u64, theme: &str, theme_next: &str)
}
fn item_href(id: &ItemId) -> String {
- if id.is_root() {
- "/".to_string()
- } else {
- format!("/?item={}", id.as_str())
- }
+ id.browse_href()
}
fn segment_label(seg: &str) -> &str {
@@ -361,39 +267,10 @@ pub fn vote_panel(parent: &ItemId) -> Markup {
}
-fn query_param(uri: &Uri, key: &str) -> Option<String> {
- let q = uri.query()?;
- q.split('&').find_map(|pair| {
- let mut it = pair.splitn(2, '=');
- if it.next()? == key {
- Some(it.next().unwrap_or("").to_string())
- } else {
- None
- }
- })
-}
-
-pub async fn home(
- State(state): State<AppState>,
- jar: CookieJar,
- uri: Uri,
-) -> impl IntoResponse {
+async fn item_page(state: AppState, uri: Uri, item: ItemId) -> Markup {
let path = uri.path().to_string();
state.views.increment(path.clone());
let views = state.views.get_views(&path);
- let theme = theme_from_jar(&jar);
- let theme_next = theme_next_from_uri(&uri);
-
- let item_raw = query_param(&uri, "item")
- .or_else(|| query_param(&uri, "sub").map(|sub| {
- if sub.is_empty() {
- String::new()
- } else {
- format!("reddit.com/r/{sub}")
- }
- }))
- .unwrap_or_default();
- let item = parse_item_param(&item_raw);
let tree = state.tree.read().await;
let empty_node = NodeState::default();
@@ -408,5 +285,14 @@ pub async fn home(
(vote_panel(&item))
(ranking_panel(&item, group))
};
- layout("sorter2", body, views, theme, &theme_next)
+ layout("sorter2", body, views)
+}
+
+pub async fn home(State(state): State<AppState>, uri: Uri) -> impl IntoResponse {
+ item_page(state, uri, ItemId::root()).await
+}
+
+pub async fn browse(State(state): State<AppState>, uri: Uri) -> impl IntoResponse {
+ let item = ItemId::from_browse_uri(uri.path()).unwrap_or(ItemId::root());
+ item_page(state, uri, item).await
}
diff --git a/server/src/lib.rs b/server/src/lib.rs
index 14e7cfbc38feb5d07aff859b64bce6e2ec45cf91..cd56743192919cf4dcea539b3d8873a21fb7e72b 100644
--- a/server/src/lib.rs
+++ b/server/src/lib.rs
@@ -30,9 +30,9 @@ pub fn create_app(state: AppState) -> Router {
Router::new()
.route("/healthz", get(|| async { "ok" }))
.route("/static/:filename", get(crate::html::serve_static))
+ .route("/~/*item_path", get(crate::html::browse))
.route("/", get(crate::html::home))
.route("/ui", post(crate::api::ui_html::post_ui_html))
- .route("/theme", post(crate::html::post_theme))
.with_state(state)
.layer(TraceLayer::new_for_http())
}
diff --git a/server/src/path_types.rs b/server/src/path_types.rs
index b5c41444f7bcd4d8d289ed3af464bc3f14d0df99..12dce9888f5cd4e1a0974d12d6468368d0f775b9 100644
--- a/server/src/path_types.rs
+++ b/server/src/path_types.rs
@@ -90,6 +90,50 @@ impl ItemId {
paths
}
+ /// Full URL for the browser location bar after `/~/`.
+ pub fn to_browse_url(&self) -> String {
+ if self.is_root() {
+ return String::new();
+ }
+ if self.as_str().contains("://") {
+ return self.as_str().to_string();
+ }
+ if self
+ .segments()
+ .first()
+ .is_some_and(|s| s.contains('.'))
+ {
+ format!("https://{}", self.as_str())
+ } else {
+ self.as_str().to_string()
+ }
+ }
+
+ /// App route, e.g. `/~/https://reddit.com/r/rust`.
+ pub fn browse_href(&self) -> String {
+ if self.is_root() {
+ "/".to_string()
+ } else {
+ format!("/~/{}", self.to_browse_url())
+ }
+ }
+
+ /// Parse the tail after `/~/` in a request path.
+ pub fn from_browse_tail(tail: &str) -> ItemId {
+ let raw = normalize_browse_tail(tail);
+ if raw.is_empty() {
+ return ItemId::root();
+ }
+ ItemId::from_url(&raw)
+ .or_else(|| ItemId::parse(&raw))
+ .unwrap_or_else(|| ItemId::opaque(raw))
+ }
+
+ pub fn from_browse_uri(path: &str) -> Option<ItemId> {
+ path.strip_prefix("/~/")
+ .map(ItemId::from_browse_tail)
+ }
+
fn canonicalize(raw: &str) -> Option<String> {
let s = raw.trim();
if s.is_empty() {
@@ -155
… preview truncated; 6,669 characters omittedHardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.