Side B is a self-contained, verifiable improvement: it fixes a real correctness issue (duplicate '#entity-section' ids breaking Idiomorph morphs when multiple entity sections coexist, e.g. on the vote-compare page) by switching to per-item data-attribute selectors, and it removes ~35 lines of duplicated markup in vote.rs by reusing the shared entity_section renderer. Side A deletes the old engine.rs and rewires registry.rs to call new `graph`/`parse` modules and an external test file, but those new modules and tests aren't shown in the diff, so the actual replacement logic (and whether it preserves correctness/tests) can't be verified from the patch itself.
constitution · epochs · watch · epoch 3
c_f10e7b043e68 (tommy-mor) vs c_c6beb77e8e71 (tommy-mor)
download prompt · raw event · cmp_8a62d0970a4dea
council reasoning
A replaces ad-hoc per-host normals in engine/registry with a graph-based canonicalization API (parse + DFA graph, thin public registry, docs), which is core ItemId identity infrastructure. B is a solid but narrower UI fix: data-entity-section selectors instead of a single #entity-section id, reuse of entity_section on the vote page, and related CSS cleanup—valuable local composition, not foundational domain logic.
Side B makes a concrete functional improvement by replacing the global `#entity-section` target with per-item `data-entity-section` selectors, allowing SSE/Idiomorph updates to address the correct entity block, and it reuses the shared `entity_section` rendering in the voting UI instead of duplicating markup. Side A is primarily a large architectural refactor of the URL canonicalization layer (replacing `engine` with `graph`/`parse` and updating the API), but the shown patch mostly removes code and redirects to new modules whose implementation is not included, making its lasting functional benefit less directly demonstrated by the diff.
sides
A — c_f10e7b043e68 (tommy-mor)
message
[7bb7145d] url stuff
diff preview
diff --git a/AGENTS.md b/AGENTS.md
index e60b9ba6012593361ef10e8fdd9439cd9932e09b..babb889d6fbfb1fa7176c9e6b7544ae17b61dd2e 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -58,4 +58,4 @@ Use **tmux** for `cargo run --package sorter2-server` (dev server). Rebuild afte
- First `cargo test` / `cargo build --release` is slow; Clojure smoke test always does a release build.
- `legacy/` and `ideas/` are not part of the workspace build.
-- **ItemId** for web URLs is a canonical full URL (`https://reddit.com/r/rust`). Rules live in [`server/src/url_rules/`](server/src/url_rules/) (composable Rust, not a config DSL). After changing canonicalization rules, rebuild the projection: `cargo run --package sorter2-server -- replay-index`.
+- **ItemId** for web URLs is a canonical full URL (`https://reddit.com/r/rust`). Rules live in [`server/src/url_rules/graph.rs`](server/src/url_rules/graph.rs): a semantic graph (DFA on host + path, query params in `Context`) with a generic internet fallback for unknown sites. After changing rules, rebuild the projection: `cargo run --package sorter2-server -- replay-index`.
diff --git a/server/src/url_rules/engine.rs b/server/src/url_rules/engine.rs
deleted file mode 100644
index e29b6b48c08deb7bffe031b1e542b1e25a7bef15..0000000000000000000000000000000000000000
--- a/server/src/url_rules/engine.rs
+++ /dev/null
@@ -1,187 +0,0 @@
-//! Composable URL normalization primitives.
-
-use std::collections::HashMap;
-
-use url::Url;
-
-/// Mutable URL view used by rule combinators before serializing to a canonical string.
-#[derive(Debug, Clone)]
-pub struct ParsedUrl {
- pub scheme: String,
- pub host: String,
- pub path_segments: Vec<String>,
- pub query: HashMap<String, String>,
- pub fragment: Option<String>,
-}
-
-impl ParsedUrl {
- pub fn parse(raw: &str) -> Option<Self> {
- let trimmed = raw.trim();
- if trimmed.is_empty() {
- return None;
- }
-
- let with_scheme = if trimmed.contains("://") {
- trimmed.to_string()
- } else if trimmed.starts_with("r/") || trimmed.starts_with("/r/") {
- let rest = trimmed.trim_start_matches('/').trim_start_matches("r/");
- format!("https://reddit.com/r/{rest}")
- } else if trimmed.contains('.') && !trimmed.starts_with('/') {
- format!("https://{trimmed}")
- } else {
- trimmed.to_string()
- };
-
- let url = Url::parse(&with_scheme).ok()?;
- let host = url.host_str()?.to_string();
- let path_segments: Vec<String> = url
- .path_segments()
- .map(|segs| segs.filter(|s| !s.is_empty()).map(str::to_string).collect())
- .unwrap_or_default();
-
- let mut query = HashMap::new();
- for (k, v) in url.query_pairs() {
- query.insert(k.into_owned(), v.into_owned());
- }
-
- Some(Self {
- scheme: url.scheme().to_string(),
- path_segments,
- query,
- fragment: url.fragment().map(str::to_string),
- host,
- })
- }
-
- pub fn with_path_segments(&self, segments: &[String]) -> Self {
- let mut u = self.clone();
- u.path_segments = segments.to_vec();
- u
- }
-
- pub fn to_url(&self) -> Option<Url> {
- let mut url = if self.path_segments.is_empty() {
- Url::parse(&format!("{}://{}", self.scheme, self.host)).ok()?
- } else {
- let path = format!("/{}", self.path_segments.join("/"));
- Url::parse(&format!("{}://{}{}", self.scheme, self.host, path)).ok()?
- };
- if !self.query.is_empty() {
- let mut pairs: Vec<_> = self.query.iter().collect();
- pairs.sort_by(|a, b| a.0.cmp(b.0));
- url.query_pairs_mut().clear();
- for (k, v) in pairs {
- url.query_pairs_mut().append_pair(k, v);
- }
- }
- if let Some(ref frag) = self.fragment {
- url.set_fragment(Some(frag));
- }
- Some(url)
- }
-
- pub fn canonical_string(&self) -> Option<String> {
- let url = self.to_url()?;
- let mut s = url.to_string();
- if self.path_segments.is_empty() {
- s = s.trim_end_matches('/').to_string();
- }
- Some(s)
- }
-}
-
-pub fn force_https(u: &mut ParsedUrl) {
- if u.scheme == "http" {
- u.scheme = "https".to_string();
- }
-}
-
-pub fn drop_fragment(u: &mut ParsedUrl) {
- u.fragment = None;
-}
-
-pub fn strip_www(u: &mut ParsedUrl) {
- if u.host.starts_with("www.") {
- u.host = u.host[4..].to_string();
- }
-}
-
-pub fn lowercase_host(u: &mut ParsedUrl) {
- u.host = u.host.to_ascii_lowercase();
-}
-
-pub fn lowercase_path(u: &mut ParsedUrl) {
- for seg in &mut u.path_segments {
- *seg = seg.to_ascii_lowercase();
- }
-}
-
-pub fn clear_query(u: &mut ParsedUrl) {
- u.query.clear();
-}
-
-pub fn keep_only_query(u: &mut ParsedUrl, keys: &[&str]) {
- u.query
- .retain(|k, _| keys.iter().any(|want| want == &k.as_str()));
-}
-
-pub fn strip_tracking_params(u: &mut ParsedUrl) {
- u.query.retain(|k, _| {
- let lower = k.to_ascii_lowercase();
- !(lower.starts_with("utm_")
- || matches!(
- lower.as_str(),
- "fbclid" | "gclid" | "ref" | "ref_src" | "ref_source" | "mc_cid" | "mc_eid"
- ))
- });
-}
-
-pub fn truncate_after_segment(u: &mut ParsedUrl, name: &str, keep: usize) {
- if let Some(i) = u.path_segments.iter().position(|s| s == name) {
- let end = (i + 1 + keep).min(u.path_segments.len());
- u.path_segments.truncate(end);
- }
-}
-
-pub fn drop_listing_suffix(u: &mut ParsedUrl, suffixes: &[&str]) {
- if u.path_segments.len() >= 3 && u.path_segments.first().map(String::as_str) == Some("r") {
- if let Some(last) = u.path_segments.last() {
- if suffixes.iter().any(|s| *s == last.as_str()) {
- u.path_segments.pop();
- }
- }
- }
-}
-
-pub fn normalize_reddit_host(u: &mut ParsedUrl) {
- if matches!(
- u.host.as_str(),
- "old.reddit.com" | "new.reddit.com" | "www.reddit.com"
- ) {
- u.host = "reddit.com".to_string();
- }
-}
-
-pub fn rewrite_youtu_be(u: &mut ParsedUrl) {
- if u.host == "youtu.be" && u.path_segments.len() == 1 {
- let id = u.path_segments[0].clone();
- u.host = "youtube.com".to_string();
- u.path_segments = vec!["watch".to_string()];
- u.query.insert("v".to_string(), id);
- }
-}
-
-pub fn rewrite_youtube_shorts(u: &mut ParsedUrl) {
- if u.host == "youtube.com" && u.path_segments.first().map(String::as_str) == Some("shorts") {
- if let Some(id) = u.path_segments.get(1).cloned() {
- u.path_segments = vec!["watch".to_string()];
- u.query.insert("v".to_string(), id);
- }
- }
-}
-
-pub fn normalize_youtube_host(u: &mut ParsedUrl) {
- if matches!(u.host.as_str(), "m.youtube.com" | "www.youtube.com") {
- u.host = "youtube.com".to_string();
- }
-}
diff --git a/server/src/url_rules/mod.rs b/server/src/url_rules/mod.rs
index 03d53bd3e82d704a01ba3fd8dd02b7d31422c0de..9e1445346ce77a49dd6a7e7713bf9c57aef353cc 100644
--- a/server/src/url_rules/mod.rs
+++ b/server/src/url_rules/mod.rs
@@ -1,8 +1,12 @@
-//! URL canonicalization and hierarchy rules for [`crate::path_types::ItemId`].
+//! URL canonicalization and hierarchy via a semantic graph (DFA + generic fallback).
-mod engine;
+mod graph;
+mod parse;
mod registry;
+#[cfg(test)]
+mod registry_tests;
+
pub use registry::{
canonicalize_raw, looks_like_url, navigable_breadcrumbs, parent_url, resolve_id, CanonicalResult,
};
diff --git a/server/src/url_rules/registry.rs b/server/src/url_rules/registry.rs
index 14514e9af8385fb2b9b2f35eb9ee14d453d4b97c..8e6c012ea1fc74b864307bdacdf5a0f5db5259fc 100644
--- a/server/src/url_rules/registry.rs
+++ b/server/src/url_rules/registry.rs
@@ -1,12 +1,7 @@
-//! Per-domain canonicalization and hierarchy rules.
+//! Public API: canonical identity and hierarchy via the URL graph.
-use std::collections::HashSet;
-
-use super::engine::{
- clear_query, drop_fragment, drop_listing_suffix, force_https, keep_only_query, lowercase_host,
- lowercase_path, normalize_reddit_host, normalize_youtube_host, rewrite_youtu_be,
- rewrite_youtube_shorts, strip_tracking_params, strip_www, truncate_after_segment, ParsedUrl,
-};
+use super::graph::graph;
+use super::parse::UrlParts;
/// Result of canonicalizing a raw URL string.
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -16,71 +11,16 @@ pub struct CanonicalResult {
pub alias_of: Option<String>,
}
-fn apply_global(u: &mut ParsedUrl) {
- force_https(u);
- drop_fragment(u);
- strip_www(u);
- lowercase_host(u);
- strip_tracking_params(u);
-}
-
-fn normalize_reddit(u: &mut ParsedUrl) {
- normalize_reddit_host(u);
- lowercase_path(u);
- truncate_after_segment(u, "comments", 1);
- drop_listing_suffix(u, &["hot", "top", "new", "rising", "controversial"]);
- clear_query(u);
-}
-
-fn normalize_youtube(u: &mut ParsedUrl) {
- rewrite_youtu_be(u);
- normalize_youtube_host(u);
- rewrite_youtube_shorts(u);
- keep_only_query(u, &["v", "list"]);
-}
-
-fn normalize_default(_u: &mut ParsedUrl) {
- // Global rules only.
-}
-
-fn domain_key(host: &str) -> &'static str {
- if host == "reddit.com" || host.ends_with(".reddit.com") {
- "reddit.com"
- } else if host == "youtube.com" || host == "youtu.be" {
- "youtube.com"
- } else {
- "default"
- }
-}
-
-fn normalize_for_host(u: &mut ParsedUrl) {
- apply_global(u);
- match domain_key(&u.host) {
- "reddit.com" => normalize_reddit(u),
- "youtube.com" => normalize_youtube(u),
- _ => normalize_default(u),
- }
-}
-
-/// Structural path segments that must not become standalone tree nodes when more path follows.
-fn structural_trailing(host: &str) -> &'static [&'static str] {
- match domain_key(host) {
- "reddit.com" => &["comments"],
- _ => &[],
- }
-}
-
/// Canonicalize a raw URL. Returns `None` if the input is not URL-like.
pub fn canonicalize_raw(raw: &str) -> Option<CanonicalResult> {
let trimmed = raw.trim();
if trimmed.is_empty() {
return None;
}
- let mut u = ParsedUrl::parse(trimmed)?;
- let input_snapshot = u.canonical_string()?;
- normalize_for_host(&mut u);
- let canonical = u.canonical_string()?;
- let alias_of = if input_snapshot != canonical {
+ let parts = UrlParts::parse(trimmed)?;
+ let g = graph();
+ let canonical = g.resolve_canonical(&parts)?;
+ let alias_of = if trimmed != canonical {
Some(trimmed.to_string())
} else {
None
@@ -98,35 +38,14 @@ pub fn resolve_id(raw: &str) -> Option<String> {
/// Navigable ancestor URLs from domain root up to and including `canonical` (full URLs).
pub fn navigable_breadcrumbs(canonical: &str) -> Vec<String> {
- let Some(u) = ParsedUrl::parse(canonical) else {
- return vec![canonical.to_string()];
+ let parts = match UrlParts::parse(canonical) {
+ Some(p) => p,
+ None => return vec![canonical.to_string()],
};
- let structural: HashSet<&str> = structural_trailing(&u.host).iter().copied().collect();
- let n = u.path_segments.len();
- let mut out = Vec::new();
-
- // Domain root (no path segments).
- if let Some(base) = u.with_path_segments(&[]).canonical_string() {
- out.push(base);
- }
-
- for i in 0..n {
- let segs: Vec<String> = u.path_segments[..=i].to_vec();
- let is_last = i == n - 1;
- let seg = u.path_segments[i].as_str();
- if structural.contains(seg) && !is_last {
- continue;
- }
- if let Some(url) = u.with_path_segments(&segs).canonical_string() {
- if out.last() != Some(&url) {
-
… preview truncated; 3,144 characters omittedB — c_c6beb77e8e71 (tommy-mor)
message
[8dab9b80] nice
diff preview
diff --git a/server/src/fetch/html.rs b/server/src/fetch/html.rs
index 3e0309ff1c2ac1b17923922d2340ba6710e0f9d1..dadf050515f0473943dad97df5d318032c8cb385 100644
--- a/server/src/fetch/html.rs
+++ b/server/src/fetch/html.rs
@@ -10,13 +10,18 @@ use crate::{
ui_action::UI_RPC_FIELD,
};
-fn entity_panel(node: &NodeState) -> Markup {
+/// CSS selector for Idiomorph / SSE updates of one entity block.
+pub fn entity_section_selector(item: &ItemId) -> String {
+ format!(r#"[data-entity-section="{}"]"#, item.as_str())
+}
+
+pub fn entity_panel(node: &NodeState) -> Markup {
if let Some(markup) = crate::render::reddit::entity_markup(node) {
return markup;
}
html! {
@if let Some(data) = &node.data {
- div id="entity-panel" class="entity-card" {
+ div class="entity-card" {
h2 { (data.title) }
@if let Some(author) = &data.author {
p class="muted small" { "by " (author) }
@@ -76,11 +81,11 @@ pub fn fetch_entity_panel(item: &ItemId, has_data: bool, fetching: bool) -> Mark
}
}
-/// Entity card + fetch control (target `#entity-section` for Idiomorph / SSE).
+/// Entity card + fetch control (morph target [`entity_section_selector`]).
pub fn entity_section(item: &ItemId, node: &NodeState, fetching: bool) -> Markup {
let has_data = node.data.is_some();
html! {
- section id="entity-section" class="demo-panel" {
+ section class="entity-section demo-panel" data-entity-section=(item.as_str()) {
(entity_panel(node))
(fetch_entity_panel(item, has_data, fetching))
}
diff --git a/server/src/fetch/mod.rs b/server/src/fetch/mod.rs
index 35f968c21c69ca557bab7951413e3cfbbccfebfd..f5759a34a1afe4069805441cefde6474a6403550 100644
--- a/server/src/fetch/mod.rs
+++ b/server/src/fetch/mod.rs
@@ -77,8 +77,9 @@ pub fn fetch_entity_stream(
let tree = state.tree.read().await;
let empty = NodeState::default();
let node = tree.get(&id).unwrap_or(&empty);
+ let sel = html::entity_section_selector(&id);
JsBuilder::new()
- .morph_selector("#entity-section", html::entity_section(&id, node, true))
+ .morph_selector(&sel, html::entity_section(&id, node, true))
.build()
};
yield Ok(js_event(fetching_js));
@@ -100,12 +101,13 @@ pub fn fetch_entity_stream(
FetchJobResult::Imported(_)
| FetchJobResult::NotFound
| FetchJobResult::SkippedCached
- | FetchJobResult::SkippedDuplicate => {
+ | FetchJobResult::SkippedDuplicate => {
let tree = state.tree.read().await;
let empty = NodeState::default();
let node = tree.get(&id).unwrap_or(&empty);
+ let sel = html::entity_section_selector(&id);
let mut b = JsBuilder::new()
- .morph_selector("#entity-section", html::entity_section(&id, node, false));
+ .morph_selector(&sel, html::entity_section(&id, node, false));
if kind == FetchKind::Children {
b = b.morph_selector("#ranking-panel", ranking_panel(&id, node, &tree));
}
@@ -115,8 +117,9 @@ pub fn fetch_entity_stream(
let tree = state.tree.read().await;
let empty = NodeState::default();
let node = tree.get(&id).unwrap_or(&empty);
+ let sel = html::entity_section_selector(&id);
let js = JsBuilder::new()
- .morph_selector("#entity-section", html::entity_section(&id, node, false))
+ .morph_selector(&sel, html::entity_section(&id, node, false))
.raw(&error_js(&format!("Reddit rate limit — retry in {reset_secs}s.")))
.build();
yield Ok(js_event(js));
@@ -125,8 +128,9 @@ pub fn fetch_entity_stream(
let tree = state.tree.read().await;
let empty = NodeState::default();
let node = tree.get(&id).unwrap_or(&empty);
+ let sel = html::entity_section_selector(&id);
let js = JsBuilder::new()
- .morph_selector("#entity-section", html::entity_section(&id, node, false))
+ .morph_selector(&sel, html::entity_section(&id, node, false))
.raw(&error_js(&format!("Fetch failed: {msg}")))
.build();
yield Ok(js_event(js));
diff --git a/server/src/html/vote.rs b/server/src/html/vote.rs
index 89ed621ad861214e147add2062224fc4137ff8ad..48752f4d78d4762d1badc44e8df008bf5a45bab4 100644
--- a/server/src/html/vote.rs
+++ b/server/src/html/vote.rs
@@ -8,6 +8,7 @@ use maud::{html, Markup};
use serde::Deserialize;
use crate::{
+ fetch::html::entity_section,
form_template::template_json_compact,
html::JsBuilder,
pair::{children_of, resolve_pair, suggest_next_pair_in_pool},
@@ -169,37 +170,13 @@ pub(crate) fn vote_recorded_morph(
}
fn vote_compare_item_card(tree: &GlobalTree, item: &ItemId, side_class: &str) -> Markup {
- let href = item_href(item);
- let title = child_title(tree, item);
+ let node = tree.get(item).cloned().unwrap_or_else(|| NodeState {
+ id: item.clone(),
+ ..Default::default()
+ });
html! {
div class=(format!("vote-compare-side {side_class}")) {
- a class=(format!("vote-compare-item {side_class}")) href=(href) {
- @if let Some(row) = crate::render::reddit::child_row_markup(tree, item, &href) {
- (row)
- } @else {
- strong { (title) }
- }
- }
- @if let Some(node) = tree.get(item) {
- @if crate::render::reddit::is_reddit_post(item) {
- @if let Some(data) = &node.data {
- @if let Some(src) = data.image_url.as_ref().or(data.thumb_url.as_ref()) {
- figure class="vote-compare-figure" {
- img class="vote-compare-image" src=(src) alt="" loading="lazy";
- }
- }
- @if let Some(author) = &data.author {
- p class="muted small" { "by " (author) }
- }
- }
- } @else if let Some(data) = &node.data {
- @if let Some(body) = &data.body_html {
- div class="vote-compare-item-body" {
- (maud::PreEscaped(body))
- }
- }
- }
- }
+ (entity_section(item, &node, false))
}
}
}
@@ -270,9 +247,6 @@ pub async fn vote_page(
(vote_compare_item_card(&tree, &right, "vote-compare-right"))
}
(vote_back_nav(&parent))
- div id="vote-edge-history-region" {
- (edge_history)
- }
form id="vote-compare-form" method="POST" action="/ui" {
input type="hidden" name=(UI_RPC_FIELD) value=(rpc_json);
input type="hidden" name="ratio_left" id="vote-ratio-left" value="50";
@@ -285,6 +259,9 @@ pub async fn vote_page(
}
(vote_compare_actions(&parent, next_pair.as_ref()))
}
+ div id="vote-edge-history-region" {
+ (edge_history)
+ }
}
};
diff --git a/server/src/render/reddit.rs b/server/src/render/reddit.rs
index 4a18de93cf57d8395ded3caa373a708f39b3f43f..c4f98fc760c32ce1b41a91bd41e97908bd198937 100644
--- a/server/src/render/reddit.rs
+++ b/server/src/render/reddit.rs
@@ -11,7 +11,7 @@ pub fn is_reddit_post(id: &ItemId) -> bool {
id.as_str().starts_with("reddit.com/") && id.as_str().contains("/comments/")
}
-/// Post detail card (`#entity-panel`).
+/// Post detail card (inside [`crate::fetch::html::entity_panel`]).
pub fn entity_markup(node: &NodeState) -> Option<Markup> {
if !is_reddit_post(&node.id) {
return None;
@@ -41,7 +41,7 @@ pub fn child_row_markup(tree: &GlobalTree, id: &ItemId, href: &str) -> Option<Ma
fn post_entity_card(data: &EntityData) -> Markup {
let image = data.image_url.as_ref().or(data.thumb_url.as_ref());
html! {
- div id="entity-panel" class="entity-card reddit-post" {
+ div class="entity-card reddit-post" {
h2 { (data.title) }
@if let Some(author) = &data.author {
p class="muted small" { "by " (author) }
diff --git a/server/static/sorter.css b/server/static/sorter.css
index b95718ce463b200a5667d3014dac2119836a0bef..9379c5f41dedbd41fd8951099a118de1da2a62f4 100644
--- a/server/static/sorter.css
+++ b/server/static/sorter.css
@@ -254,38 +254,11 @@ h1 {
}
.vote-compare-side {
- background: var(--panel);
- border: 1px solid var(--border);
- border-radius: 8px;
- padding: 1rem;
min-height: 120px;
}
-.vote-compare-item {
- color: var(--accent);
- text-decoration: none;
- display: block;
-}
-
-.vote-compare-item:hover {
- text-decoration: underline;
-}
-
-.vote-compare-figure {
- margin: 0.75rem 0 0;
-}
-
-.vote-compare-image {
- display: block;
- max-width: 100%;
- height: auto;
- border-radius: 6px;
- border: 1px solid var(--border);
-}
-
-.vote-compare-item-body {
- margin-top: 0.75rem;
- font-size: 0.9rem;
+.vote-compare-side .entity-section {
+ margin: 0;
}
.vote-compare-nav {
Hardlinks — judgments / attempts / prompt
judgments
attempts
Prompt text is loaded only by the download route.