Side A adds a genuine, well-tested feature (pretty `/~/` browse URLs backed by a canonical ItemId encoding) while cleanly removing an unused theme system, keeping the codebase coherent and reducing surface area. Side B's refactor swaps a bounded Deque for an unbounded List and defers capping to read time, which removes the write-time trim guarantee and risks unbounded storage growth for recent_votes, a more questionable trade-off despite its own test coverage.
constitution · epochs · watch · epoch 3
comparison
c_14dd365ffe22 (tommy-mor) vs c_a896b2dc05d5 (tommy-mor)
download prompt · raw event · cmp_ed7f18a3800d4c
council reasoning
A introduces a durable browse URL scheme (`/~/*` routes, `ItemId::browse_href`/`from_browse_uri` with normalization and tests) and deletes the entire theme subsystem (cookies, `/theme` handler, dual CSS, controls UI), replacing brittle `/?item=` redirects. B only swaps `Deque`/`VecDeque` recent-votes for append-only `List`/`Vec` with read-time capping and drops write-time trim—useful internal simplification, but narrower and leaves on-disk growth unbounded.
Side A introduces a lasting routing and URL design change by adding canonical browse paths (`/~/...`), centralizing link generation with `ItemId::browse_href()`, parsing browse URLs, updating redirects, and adding integration/tests for the new navigation behavior. Although it also removes the theme feature and simplifies static asset handling, the new browse URL infrastructure affects routing and link consistency across the project, whereas Side B is primarily an internal storage refactor from deque to append-only list with query-time capping and corresponding cleanup/tests.
sides
A — 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 omittedB — c_a896b2dc05d5 (tommy-mor)
message
[1531154d] dequeue -> vec
diff preview
diff --git a/server/src/projection_apply.rs b/server/src/projection_apply.rs
index 9c8990a8af927f35d3344c8d0872a516aba56b86..ad404bacb8bcdd5ae0e682cff97f974fd44528ea 100644
--- a/server/src/projection_apply.rs
+++ b/server/src/projection_apply.rs
@@ -6,8 +6,6 @@
//! batch as the (non-idempotent) edge merges guarantees exactly-once application
//! across replay.
-use std::collections::BTreeSet;
-
use crate::{
event_log::EventLogError,
events::{Event, EventRecord},
@@ -44,7 +42,6 @@ pub fn apply_records(
let db = projection_store.db();
let mut batch = db.batch();
- let mut vote_parents: BTreeSet<ItemId> = BTreeSet::new();
let mut last_seq = 0u64;
for record in records {
@@ -70,7 +67,6 @@ pub fn apply_records(
*ts,
)
.map_err(|e| EventLogError::Apply(e.to_string()))?;
- vote_parents.insert(parent);
}
Event::NodeEnsured { id } => {
let parsed = parse_event_id(id)?;
@@ -85,11 +81,5 @@ pub fn apply_records(
.commit_with(durable::Durability::DisableWal)
.map_err(|e| EventLogError::Apply(e.to_string()))?;
- for parent in vote_parents {
- projection_store
- .trim_recent_votes(&parent)
- .map_err(|e| EventLogError::Apply(e.to_string()))?;
- }
-
Ok(())
}
diff --git a/server/src/projection_store.rs b/server/src/projection_store.rs
index 8576d671f351004426207894ac35594ddb0f70cf..9a8953d010029d3639dc3987687554bab8b7663e 100644
--- a/server/src/projection_store.rs
+++ b/server/src/projection_store.rs
@@ -18,7 +18,7 @@ use crate::{
const PROJECTION_CURSOR_KEY: &str = "cursor";
const PROJECTION_SCHEMA_KEY: &str = "schema_version";
-const PROJECTION_SCHEMA_VERSION: u64 = 3;
+const PROJECTION_SCHEMA_VERSION: u64 = 4;
#[derive(Debug, thiserror::Error)]
pub enum ProjectionStoreError {
@@ -142,16 +142,6 @@ impl ProjectionStore {
Ok(tree)
}
- /// Cap a node's recent-vote window after applying votes (best-effort, blind).
- pub(crate) fn trim_recent_votes(&self, parent: &ItemId) -> Result<(), ProjectionStoreError> {
- node(parent).recent_votes().truncate_back(
- &self.db,
- crate::storage_schema::RECENT_VOTES_CAP,
- Durability::DisableWal,
- )?;
- Ok(())
- }
-
/// Cache Reddit display content outside the event log (must be evicted per policy).
pub fn put_ephemeral_content(
&self,
diff --git a/server/src/reducer.rs b/server/src/reducer.rs
index 0c75c85150bb9e5f578bbadf58b3e43f8a80be4b..759918b8c0eb8f8bf1ed0911d8877adaa55c8ea6 100644
--- a/server/src/reducer.rs
+++ b/server/src/reducer.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet, VecDeque};
+use std::collections::{HashMap, HashSet};
use serde::{Deserialize, Serialize};
@@ -52,7 +52,7 @@ pub struct GroupState {
pub idx_to_item: Vec<ItemId>,
pub edges: HashMap<(usize, usize), f64>,
pub voted_pairs: HashSet<(usize, usize)>,
- pub recent_votes: VecDeque<VoteData>,
+ pub recent_votes: Vec<VoteData>,
}
impl GroupState {
@@ -62,7 +62,7 @@ impl GroupState {
idx_to_item: Vec::new(),
edges: HashMap::new(),
voted_pairs: HashSet::new(),
- recent_votes: VecDeque::with_capacity(200),
+ recent_votes: Vec::new(),
}
}
@@ -111,10 +111,7 @@ impl GroupState {
self.add_edge_weight(b_idx, a_idx, w_a);
self.add_edge_weight(a_idx, b_idx, w_b);
- self.recent_votes.push_front(vote);
- while self.recent_votes.len() > 200 {
- self.recent_votes.pop_back();
- }
+ self.recent_votes.push(vote);
}
}
diff --git a/server/src/storage_dto.rs b/server/src/storage_dto.rs
index 9dfb13c53efe4389277625a6ab3bfc18f566a453..3fd6db5cb909ac4896bd8a3ecace796de5f08781 100644
--- a/server/src/storage_dto.rs
+++ b/server/src/storage_dto.rs
@@ -39,7 +39,7 @@ pub struct StoredEntityDataV1 {
pub link_url: Option<String>,
}
-/// One vote stored in a node's `recent_votes` deque.
+/// One vote stored in a node's `recent_votes` list.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct StoredVoteV1 {
pub version: u32,
diff --git a/server/src/storage_schema.rs b/server/src/storage_schema.rs
index bd26e665e084b95b10fdfff091c31e8dc84d07b8..5d2bb1d56927fb61c7c6d2d8602bd6882327f862 100644
--- a/server/src/storage_schema.rs
+++ b/server/src/storage_schema.rs
@@ -2,13 +2,13 @@
//! durable collections instead of one blob per node.
//!
//! A vote updates a handful of keys: a few edge-weight merges, a voted-pair flag,
-//! a recent-vote deque push, and child-link set entries. The in-memory
+//! a recent-vote list append, and child-link set entries. The in-memory
//! [`crate::reducer::GroupState`] is reconstructed from these keys on read for
//! rank-centrality.
use std::collections::{BTreeSet, HashMap, HashSet};
-use durable::{Batch, Db, Deque, Durable, Leaf, Map, Sum};
+use durable::{Batch, Db, Durable, Leaf, List, Map, Sum};
use crate::{
path_types::ItemId,
@@ -38,8 +38,8 @@ pub struct NodeSchema {
pub edges: Map<EdgeKey, Sum<f64>>,
/// Voted pairs `(min, max) -> true`.
pub voted_pairs: Map<PairKey, Leaf<bool>>,
- /// Recent votes, newest at the front (capped on write).
- pub recent_votes: Deque<Leaf<StoredVoteV1>>,
+ /// Recent votes, append-only oldest-first (cap applied on read).
+ pub recent_votes: List<Leaf<StoredVoteV1>>,
/// When ephemeral Reddit display content was last fetched (ms); absent after eviction.
pub fetched_at: Leaf<i64>,
}
@@ -55,7 +55,7 @@ pub struct Store {
pub view_meta: Map<String, Leaf<u64>>,
}
-/// Cap on the per-node recent-vote window (matches the in-memory reducer).
+/// Max recent votes returned when loading a node (query-time cap only).
pub const RECENT_VOTES_CAP: u64 = 200;
fn id_key(id: &ItemId) -> String {
@@ -148,11 +148,14 @@ fn build_group_state(
}
}
- // Deque is front=newest; in-memory VecDeque is also front=newest.
- let mut recent_votes = std::collections::VecDeque::new();
- for stored in np.recent_votes().iter(db)? {
- recent_votes.push_back(decode_vote(stored).map_err(durable::Error::Deserialize)?);
- }
+ // List is index order (oldest first); keep the newest RECENT_VOTES_CAP entries.
+ let stored = np.recent_votes().iter(db)?;
+ let cap = RECENT_VOTES_CAP as usize;
+ let start = stored.len().saturating_sub(cap);
+ let recent_votes = stored[start..]
+ .iter()
+ .map(|s| decode_vote(s.clone()).map_err(durable::Error::Deserialize))
+ .collect::<Result<Vec<_>, _>>()?;
Ok(GroupState {
item_to_idx,
@@ -248,7 +251,7 @@ pub fn vote_writes(
};
batch.write(pnode.voted_pairs().key(&(lo, hi)).set(&true));
- // Recent votes (newest at front).
+ // Recent votes (append-only; cap on read).
let stored = encode_vote(&VoteData {
ts,
a: a_id,
@@ -260,7 +263,7 @@ pub fn vote_writes(
delegate: None,
thread_tag: "default".to_string(),
});
- batch.push_front(&pnode.recent_votes(), &stored)?;
+ batch.push(&pnode.recent_votes(), &stored)?;
Ok(())
}
@@ -314,6 +317,35 @@ mod tests {
assert!(load_node_state(&db, &parent).unwrap().is_none());
}
+ #[test]
+ fn load_caps_recent_votes_at_query_time() {
+ let dir = tempfile::tempdir().unwrap();
+ let db = Db::open(dir.path()).unwrap();
+ let parent = ItemId::root();
+
+ let mut batch = db.batch();
+ for i in 0..RECENT_VOTES_CAP + 10 {
+ vote_writes(&mut batch, &parent, "alpha", "beta", 1, 0, i as i64).unwrap();
+ }
+ batch.commit().unwrap();
+
+ assert_eq!(
+ node(&parent).recent_votes().len(&db).unwrap(),
+ RECENT_VOTES_CAP + 10
+ );
+
+ let node_state = load_node_state(&db, &parent).unwrap().unwrap();
+ assert_eq!(node_state.local_ranking.recent_votes.len(), RECENT_VOTES_CAP as usize);
+ assert_eq!(
+ node_state.local_ranking.recent_votes.first().map(|v| v.ts),
+ Some(10)
+ );
+ assert_eq!(
+ node_state.local_ranking.recent_votes.last().map(|v| v.ts),
+ Some(RECENT_VOTES_CAP as i64 + 9)
+ );
+ }
+
#[test]
fn missing_node_is_none() {
let dir = tempfile::tempdir().unwrap();
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.