From d59f5882426c451fcba303a796560f823bb25f1d Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:31:20 -0400 Subject: [PATCH 01/12] feat(search): the server picks the Messages list's order and says so With no `sort`, `GET /v1/messages` now ranks a search with a positive free-text word best match first, and lists one with none newest first. It used to list the oldest first. A `sort` the request names is applied as before, and `relevance` for a search with no free-text word is still `validation-failed`. Each page carries `search` beside its four keys: `sort`, the order the page is in, and `terms`, the free-text terms the search ranks by, read from `Expr::positive_text_terms`. The answer's schema is `ListMessagesResponse`. The document rules name that route and schema as the one page with a fifth key, and now fail any other page with a key beyond the four. Refs #1538 Co-Authored-By: Claude Fable 5.1 --- .../server/src/db/conversation_messages.rs | 45 +++++++- crates/server/server/src/messages_api.rs | 104 ++++++++++++++++-- .../server/server/src/messages_api/tests.rs | 104 +++++++++++++++++- .../server/src/openapi/document_rules.rs | 70 +++++++++++- crates/server/server/src/search/emit.rs | 10 +- crates/server/server/src/search/mod.rs | 10 ++ docs/src/assets/openapi.json | 87 ++++++++++++++- web/src/lib/freeTextTerms.test.ts | 54 --------- web/src/lib/freeTextTerms.ts | 68 ------------ 9 files changed, 396 insertions(+), 156 deletions(-) delete mode 100644 web/src/lib/freeTextTerms.test.ts delete mode 100644 web/src/lib/freeTextTerms.ts diff --git a/crates/server/server/src/db/conversation_messages.rs b/crates/server/server/src/db/conversation_messages.rs index e040a58c4..a519a7624 100644 --- a/crates/server/server/src/db/conversation_messages.rs +++ b/crates/server/server/src/db/conversation_messages.rs @@ -195,12 +195,45 @@ pub const MESSAGE_LIST_SORT_KEYS: [(&str, MessageListSort); 2] = [ ("relevance", MessageListSort::Relevance), ]; -/// Oldest first, as [`DEFAULT_MESSAGE_SORT`] reads a conversation when `sort` -/// is absent. -pub const DEFAULT_MESSAGE_LIST_SORT: [SortKey; 1] = [SortKey { - key: MessageListSort::Date, - direction: Direction::Asc, -}]; +/// The order the Messages list applies when the request names no `sort`: +/// best match first when the query has a free-text word to rank by +/// (`ranked`), and newest first otherwise. A search with words is looking for +/// the messages that hold them, and one with only field words is browsing, +/// where the latest messages come first (#1538). +#[must_use] +pub fn default_message_list_sort(ranked: bool) -> [SortKey; 1] { + if ranked { + [SortKey { + key: MessageListSort::Relevance, + direction: Direction::Asc, + }] + } else { + [SortKey { + key: MessageListSort::Date, + direction: Direction::Desc, + }] + } +} + +/// `order` spelled as `sort=` takes it: `relevance`, `date`, `-date`, or +/// keys joined by commas, such as `relevance,-date`. +#[must_use] +pub fn message_list_sort_text(order: &[SortKey]) -> String { + order + .iter() + .map(|k| { + let name = MESSAGE_LIST_SORT_KEYS + .iter() + .find(|(_, key)| *key == k.key) + .map_or("", |(name, _)| *name); + match k.direction { + Direction::Asc => name.to_string(), + Direction::Desc => format!("-{name}"), + } + }) + .collect::>() + .join(",") +} /// The join a relevance order ranks by: every message the rank query /// matches, with its `bm25()`, keyed by message id. Its one `?` is the rank diff --git a/crates/server/server/src/messages_api.rs b/crates/server/server/src/messages_api.rs index 2ceda5299..ce024f842 100644 --- a/crates/server/server/src/messages_api.rs +++ b/crates/server/server/src/messages_api.rs @@ -9,15 +9,75 @@ use crate::extract::{Json, Path, Query}; use axum::extract::State; +use serde::Serialize; use crate::db::conversation_messages::{ - DEFAULT_MESSAGE_LIST_SORT, DEFAULT_MESSAGE_SORT, MESSAGE_LIST_SORT_KEYS, Message, - count_matching_messages, load_message_list_page, load_messages, + DEFAULT_MESSAGE_SORT, MESSAGE_LIST_SORT_KEYS, Message, count_matching_messages, + default_message_list_sort, load_message_list_page, load_messages, message_list_sort_text, }; use crate::db::sql::SqlParam; -use crate::paging::{ListRequest, Page, PageQuery}; +use crate::paging::{ListRequest, PageQuery}; +use crate::search::parse::TextTerm; use crate::server::{ApiError, AppState, FullAccess}; +/// One page of the Messages list and how the server read its search: the +/// four keys of every page, and `search`, the one key a page carries beside +/// them (`docs/architecture/http-api.md`, "Lists"). +#[derive(Debug, Serialize, utoipa::ToSchema)] +pub(crate) struct ListMessagesResponse { + /// The rows on this page. + pub(crate) items: Vec, + /// Rows matching the query across every page. + pub(crate) total: u64, + /// Page size used. + pub(crate) limit: usize, + /// Page offset used. + pub(crate) offset: usize, + /// How the server read `q` and `sort`: the order it applied and the + /// free-text terms it ranks by. It describes the query, not the rows, so + /// every page of one query carries the same value. + pub(crate) search: MessageSearch, +} + +/// A Messages search as the server read it. +#[derive(Debug, Serialize, utoipa::ToSchema)] +pub(crate) struct MessageSearch { + /// The order the page is in, spelled as `sort` takes it: the `sort` the + /// request named, or with none, `relevance` when `terms` is not empty and + /// `-date` (newest first) when it is. + pub(crate) sort: String, + /// The free-text terms the search ranks by, in the order they were typed: + /// every word and quoted phrase of `q` that is not a field word and not + /// behind `-` or `not`, alone or in a negated group. Empty when `q` has + /// none, and then `relevance` is refused. + pub(crate) terms: Vec, +} + +/// One free-text term of a search: a word, or a quoted phrase. +#[derive(Debug, Serialize, utoipa::ToSchema)] +pub(crate) struct FreeTextTerm { + /// The word, or the phrase without its quotes, as typed. + pub(crate) text: String, + /// True when the word ended in `*` and matches any word it begins; the + /// `*` is not in `text`. Always false for a phrase. + pub(crate) prefix: bool, +} + +impl From<&TextTerm> for FreeTextTerm { + fn from(term: &TextTerm) -> Self { + match term { + TextTerm::Term { text, prefix } => Self { + text: text.clone(), + prefix: *prefix, + }, + TextTerm::Phrase(text) => Self { + text: text.clone(), + prefix: false, + }, + } + } +} + /// Compile a query against the Messages list of the search language. /// /// # Errors @@ -39,9 +99,14 @@ pub(crate) fn message_filter( })?) } -/// Messages matching `q`, oldest first unless `sort` says otherwise: the same -/// rows an Export Run with a `query` scope would hand over, behind a logged-in -/// session with the list defaults and the list's offset ceiling. +/// Messages matching `q`: the same rows an Export Run with a `query` scope +/// would hand over, behind a logged-in session with the list defaults and the +/// list's offset ceiling. +/// +/// With no `sort`, the best match comes first when `q` has a free-text word +/// to rank by, and the newest message first when it has none. The page's +/// `search` says which order it applied and which terms it ranks by, so a +/// client never parses `q` itself. /// /// `sort=relevance` puts the best match first, ranked by the full-text /// index's `bm25()` on the query's free-text words: the words not behind `-` @@ -58,10 +123,10 @@ pub(crate) fn message_filter( ("q" = Option, Query, description = "Search query in the Messages list's words; empty matches every message"), ("limit" = Option, Query, description = "Page size, default 40, max 500"), ("offset" = Option, Query, description = "Page offset, max 50000"), - ("sort" = Option, Query, description = "`date`, `-date` or `relevance` (best match first; needs a free-text word in `q`). Default `date`, oldest first.") + ("sort" = Option, Query, description = "`date`, `-date` or `relevance` (best match first; needs a free-text word in `q`). Default `relevance` when `q` has a free-text word, and `-date`, newest first, when it has none.") ), responses( - (status = 200, body = crate::paging::Page), + (status = 200, body = ListMessagesResponse), crate::problem::openapi::SearchQueryInvalid ) )] @@ -69,31 +134,46 @@ pub(crate) async fn list_messages( State(state): State, FullAccess(auth): FullAccess, Query(query): Query, -) -> Result>, ApiError> { +) -> Result, ApiError> { let mut conn = state.db.acquire().await?; + // No default here: which one applies depends on the query, which is + // compiled only after the sort has been checked. let list = ListRequest::read( &mut conn, auth.account_id, query, &MESSAGE_LIST_SORT_KEYS, - &DEFAULT_MESSAGE_LIST_SORT, + &[], ) .await?; let filter = message_filter(auth.account_id, &list.q, list.clock)?; + let order = if list.order.is_empty() { + default_message_list_sort(filter.rank_query().is_some()).to_vec() + } else { + list.order + }; let items = load_message_list_page( &mut conn, &filter, - &list.order, + &order, list.page.limit, list.page.offset, ) .await?; let total = count_matching_messages(&mut conn, &filter).await?; - Ok(Json(Page { + Ok(Json(ListMessagesResponse { items, total, limit: list.page.limit, offset: list.page.offset, + search: MessageSearch { + sort: message_list_sort_text(&order), + terms: filter + .ranked_terms() + .iter() + .map(FreeTextTerm::from) + .collect(), + }, })) } diff --git a/crates/server/server/src/messages_api/tests.rs b/crates/server/server/src/messages_api/tests.rs index e2d0091a7..07ff7b7f8 100644 --- a/crates/server/server/src/messages_api/tests.rs +++ b/crates/server/server/src/messages_api/tests.rs @@ -101,20 +101,22 @@ async fn the_messages_route_is_a_page_across_every_conversation() { assert_eq!(page["limit"], serde_json::json!(40)); assert_eq!(page["offset"], serde_json::json!(0)); assert_eq!(page["items"].as_array().unwrap().len(), 3); - // `docs/architecture/http-api.md`: a list is {items, total, limit, offset} and nothing else. + // `docs/architecture/http-api.md`, "Lists": a list is {items, total, + // limit, offset}, and the Messages list adds `search`, its one exception. let keys: Vec<&str> = page .as_object() .unwrap() .keys() .map(String::as_str) .collect(); - assert_eq!(keys, ["items", "limit", "offset", "total"]); + assert_eq!(keys, ["items", "limit", "offset", "search", "total"]); } #[tokio::test] async fn a_page_across_two_conversations_names_each_conversations_own_participants() { let (fixture, alice, direct, group) = seeded().await; - let page: serde_json::Value = get_json(&fixture.state, "/v1/messages", &alice.token).await; + let page: serde_json::Value = + get_json(&fixture.state, "/v1/messages?sort=date", &alice.token).await; let handles: Vec<(i64, &str)> = page["items"] .as_array() @@ -1810,6 +1812,102 @@ async fn relevance_ranks_by_the_positive_words_only() { assert_eq!(texts(&page)[0], "the dentist moved it", "{page}"); } +/// With no `sort`, a search with a free-text word puts the best match first +/// and says so, and a search with only field words lists the newest message +/// first and says so (#1538). The client reads the order back rather than +/// parsing `q` to guess it. +#[tokio::test] +async fn with_no_sort_the_server_picks_the_order_and_reports_it() { + let (fixture, alice) = seeded_for_relevance().await; + let page: serde_json::Value = + get_json(&fixture.state, "/v1/messages?q=dentist", &alice.token).await; + assert_eq!( + texts(&page), + [ + "dentist dentist dentist", + "the dentist moved it", + "after work I will call the office of the dentist about next week", + ], + "{page}" + ); + assert_eq!( + page["search"], + serde_json::json!({"sort": "relevance", "terms": [{"text": "dentist", "prefix": false}]}) + ); + + let page: serde_json::Value = + get_json(&fixture.state, "/v1/messages?q=date%3A2024", &alice.token).await; + assert_eq!( + texts(&page), + [ + "nothing to see", + "the dentist moved it", + "after work I will call the office of the dentist about next week", + "dentist dentist dentist", + ], + "{page}" + ); + assert_eq!( + page["search"], + serde_json::json!({"sort": "-date", "terms": []}) + ); +} + +/// A `sort` the request names is applied as it is, and reported in the +/// spelling `sort` takes, keys and signs included. +#[tokio::test] +async fn a_named_sort_is_applied_and_reported() { + let (fixture, alice) = seeded_for_relevance().await; + for (sort, reported) in [ + ("date", "date"), + ("-DATE", "-date"), + ("relevance,date", "relevance,date"), + ] { + let page: serde_json::Value = get_json( + &fixture.state, + &format!("/v1/messages?q=dentist&sort={sort}"), + &alice.token, + ) + .await; + assert_eq!( + page["search"]["sort"], + serde_json::json!(reported), + "{sort}: {page}" + ); + } + let page: serde_json::Value = get_json( + &fixture.state, + "/v1/messages?q=dentist&sort=date", + &alice.token, + ) + .await; + assert_eq!(texts(&page)[0], "dentist dentist dentist", "{page}"); +} + +/// The terms a search ranks by are the free-text words and phrases a match +/// must or may have, in the order typed: a word behind `-` or `not`, a word +/// in a negated group, and a field word's value are none of them. A prefix +/// keeps its `*` as a flag, and a phrase comes back without its quotes. +#[tokio::test] +async fn the_page_names_the_terms_the_search_ranks_by() { + let (fixture, alice) = seeded_for_relevance().await; + // dentist -office not moved -("next week" or call) body:work date:2024 + // "the dentist" or den* + let q = "dentist%20-office%20not%20moved%20-(%22next%20week%22%20or%20call)%20body%3Awork%20date%3A2024%20%22the%20dentist%22%20or%20den*"; + let page: serde_json::Value = + get_json(&fixture.state, &format!("/v1/messages?q={q}"), &alice.token).await; + assert_eq!( + page["search"]["terms"], + serde_json::json!([ + {"text": "dentist", "prefix": false}, + {"text": "the dentist", "prefix": false}, + {"text": "den", "prefix": true}, + ]), + "{page}" + ); + assert_eq!(page["search"]["sort"], serde_json::json!("relevance")); +} + /// Relevance has one direction, best match first: `-relevance` would list the /// unranked messages first and then the worst match, which nobody asks for. #[tokio::test] diff --git a/crates/server/server/src/openapi/document_rules.rs b/crates/server/server/src/openapi/document_rules.rs index 5cb11cb74..9c0bc3ced 100644 --- a/crates/server/server/src/openapi/document_rules.rs +++ b/crates/server/server/src/openapi/document_rules.rs @@ -46,6 +46,31 @@ const MULTIPART_PART: &str = "/v1/assets/{sha256}/uploads/{upload_id}/parts/{par /// The four keys of every page. const PAGE_KEYS: [&str; 4] = ["items", "total", "limit", "offset"]; +/// The one page with a key beside the four, and the one route that answers +/// it: `GET /v1/messages` says how it read its search in `search` +/// (`docs/architecture/http-api.md`, "Lists"). Named here, route and schema +/// both, so a second page with a fifth key fails the rules rather than +/// passing under a name that does not start with `Page_`. +struct PageException { + method: &'static str, + path: &'static str, + schema: &'static str, + key: &'static str, +} + +const PAGE_EXCEPTION: PageException = PageException { + method: "get", + path: "/v1/messages", + schema: "ListMessagesResponse", + key: "search", +}; + +impl PageException { + fn is(&self, op: &Operation) -> bool { + op.method == self.method && op.path == self.path + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn every_operation_keeps_the_rules_the_document_can_show() { let doc: Value = serde_json::from_str(&dump_openapi_json()).unwrap(); @@ -185,18 +210,50 @@ fn read_rules(doc: &Value, op: &Operation, spec: &Value) -> Vec { .map(|field| format!("{field} is optional in a success answer")), ); - for page in page_schemas(doc, spec) { + // The exception is checked by its route as well as its schema: the + // route must answer it, and no other route may. + let answered = schema_named(&spec["responses"]["200"]["content"]["application/json"]["schema"]); + if PAGE_EXCEPTION.is(op) && answered != Some(PAGE_EXCEPTION.schema) { + broken.push(format!( + "the page with a fifth key is {}, and this route answers {answered:?}", + PAGE_EXCEPTION.schema + )); + } + if !PAGE_EXCEPTION.is(op) && answered == Some(PAGE_EXCEPTION.schema) { + broken.push(format!( + "answers {}, the page only {} {} may answer", + PAGE_EXCEPTION.schema, + PAGE_EXCEPTION.method.to_uppercase(), + PAGE_EXCEPTION.path + )); + } + + for page in page_schemas(doc, op, spec) { let required: BTreeSet<&str> = page["required"] .as_array() .into_iter() .flatten() .filter_map(Value::as_str) .collect(); - for key in PAGE_KEYS { + let properties: BTreeSet<&str> = page["properties"] + .as_object() + .into_iter() + .flat_map(|p| p.keys().map(String::as_str)) + .collect(); + let mut keys: BTreeSet<&str> = PAGE_KEYS.into_iter().collect(); + if PAGE_EXCEPTION.is(op) { + keys.insert(PAGE_EXCEPTION.key); + } + for key in &keys { if !required.contains(key) { broken.push(format!("the page it answers has no required {key}")); } } + for key in properties.difference(&keys) { + broken.push(format!( + "the page it answers has {key} beside its four keys (\"Lists\")" + )); + } // A POST that reads the rows its body names answers the whole body // as one page, and takes no paging ("Lists"). if op.method == "get" { @@ -314,7 +371,7 @@ async fn called_rules(doc: &Value, world: &World<'_>, op: &Operation, spec: &Val } } - if op.method == "get" && !page_schemas(doc, spec).is_empty() { + if op.method == "get" && !page_schemas(doc, op, spec).is_empty() { let mut out_of_range = vec!["limit=0", "limit=501"]; // A browse list says its offset ceiling in the parameter's own // description, and must keep to it. @@ -887,15 +944,16 @@ fn is_kebab(segment: &str) -> bool { /// The page schemas a `200` answers: the page it names, or each page of a /// choice between pages, as an account's history answers the account in -/// full and the owner without content. Empty when it answers no page. -fn page_schemas<'d>(doc: &'d Value, spec: &Value) -> Vec<&'d Value> { +/// full and the owner without content, or [`PAGE_EXCEPTION`]'s page on its +/// own route. Empty when it answers no page. +fn page_schemas<'d>(doc: &'d Value, op: &Operation, spec: &Value) -> Vec<&'d Value> { let schemas = &doc["components"]["schemas"]; let Some(name) = schema_named(&spec["responses"]["200"]["content"]["application/json"]["schema"]) else { return Vec::new(); }; - if name.starts_with("Page_") { + if name.starts_with("Page_") || (PAGE_EXCEPTION.is(op) && name == PAGE_EXCEPTION.schema) { return vec![&schemas[name]]; } let choices: Vec<&str> = schemas[name]["oneOf"] diff --git a/crates/server/server/src/search/emit.rs b/crates/server/server/src/search/emit.rs index fbb344afe..bad541c64 100644 --- a/crates/server/server/src/search/emit.rs +++ b/crates/server/server/src/search/emit.rs @@ -56,10 +56,13 @@ pub(crate) fn compile( account_id: i64, zone: chrono_tz::Tz, ) -> Result { - let rank_query = match (list, expr) { - (ListKind::Messages, Some(expr)) => fts::rank_query(&expr.positive_text_terms()), - _ => None, + let ranked_terms: Vec = match (list, expr) { + (ListKind::Messages, Some(expr)) => { + expr.positive_text_terms().into_iter().cloned().collect() + } + _ => Vec::new(), }; + let rank_query = fts::rank_query(&ranked_terms.iter().collect::>()); let (where_sql, params) = compile_where(list, expr, account_id, zone, true)?; // Only a free-text word that is not negated can find a message by an // earlier version alone: a negated one only leaves messages out, and @@ -77,6 +80,7 @@ pub(crate) fn compile( where_sql, params, rank_query, + ranked_terms, final_text, earlier_version_match, }) diff --git a/crates/server/server/src/search/mod.rs b/crates/server/server/src/search/mod.rs index fd2232488..35e485550 100644 --- a/crates/server/server/src/search/mod.rs +++ b/crates/server/server/src/search/mod.rs @@ -88,6 +88,7 @@ pub struct Filter { where_sql: String, params: Vec, rank_query: Option, + ranked_terms: Vec, final_text: Option<(String, Vec)>, earlier_version_match: Option<(String, Vec)>, } @@ -111,6 +112,14 @@ impl Filter { self.rank_query.as_deref() } + /// The free-text terms [`Self::rank_query`] is built from, in the order + /// they were typed: every one not behind `-` or `not`, alone or in a + /// negated group (`Expr::positive_text_terms`). Empty exactly when + /// `rank_query` is `None`. + pub(crate) fn ranked_terms(&self) -> &[parse::TextTerm] { + &self.ranked_terms + } + /// A `SELECT` of the ids of the earlier versions that hold one of the /// query's free-text words not behind `-` or `not`, with the one value /// it binds: the versions a hit found only by an earlier version was @@ -182,6 +191,7 @@ pub fn compile_messages_of_conversations(req: CompileRequest<'_>) -> Result { - it("reads plain words and leaves field words out", () => { - expect(freeTextTerms("from:Alice photo")).toEqual([{ text: "photo", prefix: false }]); - expect(freeTextTerms('from:"Alice Smith" date:2024')).toEqual([]); - }); - - it("reads a quoted phrase as one term, with a doubled quote as one quote", () => { - expect(freeTextTerms('"see you ""there"""')).toEqual([ - { text: 'see you "there"', prefix: false }, - ]); - }); - - it("reads a trailing star as a prefix", () => { - expect(freeTextTerms("avoc*")).toEqual([{ text: "avoc", prefix: true }]); - }); - - it("leaves out a word behind a minus or not, and every word in a negated group", () => { - expect(freeTextTerms("dentist -office")).toEqual([{ text: "dentist", prefix: false }]); - expect(freeTextTerms("not dentist")).toEqual([]); - expect(freeTextTerms("-(toast or guacamole) avocado")).toEqual([ - { text: "avocado", prefix: false }, - ]); - expect(freeTextTerms("not (toast guacamole) avocado")).toEqual([ - { text: "avocado", prefix: false }, - ]); - }); - - it("reads the words on both sides of or and and, and not the operators", () => { - expect(freeTextTerms("moved OR dentist and (toast)")).toEqual([ - { text: "moved", prefix: false }, - { text: "dentist", prefix: false }, - { text: "toast", prefix: false }, - ]); - }); - - it("reads a word with a colon that is not a field word as text", () => { - expect(freeTextTerms("12:30 https://example.com")).toEqual([ - { text: "12:30", prefix: false }, - { text: "https://example.com", prefix: false }, - ]); - }); -}); - -describe("hasFreeText", () => { - it("is true only when a positive free-text word is left", () => { - expect(hasFreeText("from:Alice photo")).toBe(true); - expect(hasFreeText("from:Alice date:2024")).toBe(false); - expect(hasFreeText("-photo")).toBe(false); - expect(hasFreeText("")).toBe(false); - }); -}); diff --git a/web/src/lib/freeTextTerms.ts b/web/src/lib/freeTextTerms.ts deleted file mode 100644 index 21012a5c8..000000000 --- a/web/src/lib/freeTextTerms.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { searchTokens } from "./searchQuery"; - -/** One free-text term of a query: a word or a quoted phrase, and whether it ended in `*`. */ -export type FreeTextTerm = { text: string; prefix: boolean }; - -/** `word:` at the start of a token, as the server's lexer reads a field word. */ -const FIELD_HEAD = /^[A-Za-z][A-Za-z-]*:(?!\/)/; - -/** - * The free-text terms a match must or may have, in the order they were - * typed: every word and quoted phrase that is not a `word:value`, not an - * operator (`or`, `and`, `not`), and not behind `-` or `not`, alone or in a - * negated group. - * - * The server ranks a Messages search by these same terms - * (`Expr::positive_text_terms` in `crates/server/server/src/search/parse.rs`), - * so a query this finds none in is one the server will not sort by - * relevance. The Messages list draws them in bold. - */ -export function freeTextTerms(query: string): FreeTextTerm[] { - const terms: FreeTextTerm[] = []; - /** Whether each open group is negated, innermost last. */ - const groups: boolean[] = []; - let pendingNot = false; - for (const token of searchTokens(query)) { - const raw = query.slice(token.start, token.end); - const inNegated = groups.at(-1) ?? false; - if (token.kind === "close") { - groups.pop(); - continue; - } - const minus = raw.startsWith("-") && raw.length > 1; - const negated = inNegated || pendingNot || minus; - if (token.kind === "open") { - groups.push(negated); - pendingNot = false; - continue; - } - const lower = raw.toLowerCase(); - if (lower === "or" || lower === "and") continue; - if (lower === "not") { - pendingNot = true; - continue; - } - pendingNot = false; - const body = minus ? raw.slice(1) : raw; - if (negated || FIELD_HEAD.test(body)) continue; - const term = readTerm(body); - if (term) terms.push(term); - } - return terms; -} - -/** True when `query` has a free-text term to rank by. */ -export function hasFreeText(query: string): boolean { - return freeTextTerms(query).length > 0; -} - -/** One text token as a term: a quoted phrase unquoted, or a word with its `*` read off. */ -function readTerm(body: string): FreeTextTerm | null { - if (body.startsWith('"')) { - const inner = body.endsWith('"') && body.length > 1 ? body.slice(1, -1) : body.slice(1); - const text = inner.replace(/""/g, '"').trim(); - return text ? { text, prefix: false } : null; - } - if (body.endsWith("*") && body.length > 1) return { text: body.slice(0, -1), prefix: true }; - return body ? { text: body, prefix: false } : null; -} From e3cd184d9ef1379e21e278880c3cf305af704e83 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:31:20 -0400 Subject: [PATCH 02/12] feat(web): the Messages list reads its order and terms from the server The Messages list sends no `sort` unless the person picked a Date order. The sort menu shows the order the page says it applied, offers Relevance only when the server returned terms, and the rows bold those terms. `freeTextTerms.ts`, the web's own copy of the server's reading of free text, is deleted, and so is `effectiveMessageSort`. Picking Relevance leaves `sort` out of the address. The menu offers it only when the server already ranks by default, and a kept `relevance` would follow the person to a search with no free-text word, which the server refuses. `useRoutePagedList` answers the latest page as `lastPage`, so a screen reads what a route says beside its rows from the one cache entry. Refs #1538 Co-Authored-By: Claude Fable 5.1 --- web/src/components/MessageSearchRow.tsx | 3 +- web/src/components/ResultsColumn.tsx | 4 +- web/src/lib/messageMatch.ts | 2 +- web/src/lib/messageSearchSort.test.ts | 39 ++++---- web/src/lib/messageSearchSort.ts | 40 ++++---- web/src/lib/queryKeys.ts | 3 +- web/src/lib/resultsView.test.ts | 7 +- web/src/lib/resultsView.ts | 10 +- web/src/lib/routeQuery.ts | 35 ++++--- web/src/lib/serverApi.ts | 13 ++- web/src/lib/serverApi.types.ts | 63 ++++++++++++- web/src/lib/types.ts | 9 ++ web/src/screens/MessageSearchList.test.tsx | 68 +++++++++++--- web/src/screens/MessageSearchList.tsx | 94 +++++++++++-------- .../message/useConversationMessages.test.tsx | 16 +++- 15 files changed, 280 insertions(+), 126 deletions(-) diff --git a/web/src/components/MessageSearchRow.tsx b/web/src/components/MessageSearchRow.tsx index 2b7cdeb40..fb3ab6643 100644 --- a/web/src/components/MessageSearchRow.tsx +++ b/web/src/components/MessageSearchRow.tsx @@ -1,12 +1,11 @@ import type { ReactNode } from "react"; import { deletedInSourceText, UNSENT_TEXT } from "../lib/deletionMarkText"; import { formatDay } from "../lib/formatDate"; -import type { FreeTextTerm } from "../lib/freeTextTerms"; import { type MatchRange, snippet } from "../lib/messageMatch"; import { messageConversationName, messageRowText, messageSenderName } from "../lib/messageRowText"; import { useTimeZone } from "../lib/timeZone"; import { listRowDivider } from "../lib/tw"; -import type { Message } from "../lib/types"; +import type { FreeTextTerm, Message } from "../lib/types"; import { focusRing } from "../lib/uiStyles"; import PlainButton from "./PlainButton"; diff --git a/web/src/components/ResultsColumn.tsx b/web/src/components/ResultsColumn.tsx index 2ca3112f6..3169d0ea6 100644 --- a/web/src/components/ResultsColumn.tsx +++ b/web/src/components/ResultsColumn.tsx @@ -1,7 +1,7 @@ import { type Key, ToggleButton, ToggleButtonGroup } from "react-aria-components"; import { useNavigate, useSearchParams } from "react-router-dom"; import { matchedVersionIndexes } from "../lib/earlierVersionMatch"; -import { messageSortParam } from "../lib/messageSearchSort"; +import { pickedSortParam } from "../lib/messageSearchSort"; import { AT_PARAM, MATCHED_PARAM, @@ -129,7 +129,7 @@ export default function ResultsColumn({ setParam(MESSAGE_SORT_PARAM, messageSortParam(next))} + onSortPick={(next) => setParam(MESSAGE_SORT_PARAM, pickedSortParam(next) ?? "")} selectedId={openedAt(searchParams)} onSelect={(message) => // `q` stays what the person typed, and the tag rides beside it, diff --git a/web/src/lib/messageMatch.ts b/web/src/lib/messageMatch.ts index ee60f9989..e4c21775e 100644 --- a/web/src/lib/messageMatch.ts +++ b/web/src/lib/messageMatch.ts @@ -1,4 +1,4 @@ -import type { FreeTextTerm } from "./freeTextTerms"; +import type { FreeTextTerm } from "./types"; /** A matched span of a string: start inclusive, end exclusive, in UTF-16 units. */ export type MatchRange = [number, number]; diff --git a/web/src/lib/messageSearchSort.test.ts b/web/src/lib/messageSearchSort.test.ts index 2c6f76719..df4385959 100644 --- a/web/src/lib/messageSearchSort.test.ts +++ b/web/src/lib/messageSearchSort.test.ts @@ -1,23 +1,5 @@ import { describe, expect, it } from "vitest"; -import { effectiveMessageSort, messageSortParam } from "./messageSearchSort"; - -describe("effectiveMessageSort", () => { - it("is Relevance by default when the query has a free-text word, and Date newest first otherwise", () => { - expect(effectiveMessageSort(null, true)).toEqual({ sort: "relevance", order: "desc" }); - expect(effectiveMessageSort(null, false)).toEqual({ sort: "date", order: "desc" }); - }); - - it("keeps the person's pick, except Relevance with nothing to rank by", () => { - expect(effectiveMessageSort({ sort: "date", order: "asc" }, true)).toEqual({ - sort: "date", - order: "asc", - }); - expect(effectiveMessageSort({ sort: "relevance", order: "desc" }, false)).toEqual({ - sort: "date", - order: "desc", - }); - }); -}); +import { messageSortFromParam, messageSortParam, pickedSortParam } from "./messageSearchSort"; describe("messageSortParam", () => { it("spells the sort as the route takes it", () => { @@ -27,3 +9,22 @@ describe("messageSortParam", () => { expect(messageSortParam({ sort: "date", order: "desc" })).toBe("-date"); }); }); + +describe("messageSortFromParam", () => { + it("reads the order the server reports, by its first key", () => { + expect(messageSortFromParam("relevance")).toEqual({ sort: "relevance", order: "desc" }); + expect(messageSortFromParam("date")).toEqual({ sort: "date", order: "asc" }); + expect(messageSortFromParam("-date")).toEqual({ sort: "date", order: "desc" }); + expect(messageSortFromParam("relevance,-date")).toEqual({ sort: "relevance", order: "desc" }); + expect(messageSortFromParam("colour")).toBeNull(); + expect(messageSortFromParam(null)).toBeNull(); + }); +}); + +describe("pickedSortParam", () => { + it("keeps a Date order, and nothing for Relevance, which is the server's own order", () => { + expect(pickedSortParam({ sort: "date", order: "asc" })).toBe("date"); + expect(pickedSortParam({ sort: "date", order: "desc" })).toBe("-date"); + expect(pickedSortParam({ sort: "relevance", order: "desc" })).toBeNull(); + }); +}); diff --git a/web/src/lib/messageSearchSort.ts b/web/src/lib/messageSearchSort.ts index dd635df19..8311da9ac 100644 --- a/web/src/lib/messageSearchSort.ts +++ b/web/src/lib/messageSearchSort.ts @@ -7,19 +7,6 @@ export type MessageSearchSortKey = "relevance" | "date"; /** One choice in the Messages list's sort menu. Relevance has no order: best match first. */ export type MessageSearchSort = { sort: MessageSearchSortKey; order: SortOrder }; -/** - * The order the Messages list shows: the one the person picked, unless it is - * Relevance and the query has no free-text word to rank by. With no pick, - * Relevance when there is a word to rank by, and otherwise Date, newest first. - */ -export function effectiveMessageSort( - picked: MessageSearchSort | null, - rankable: boolean, -): MessageSearchSort { - if (picked && (picked.sort === "date" || rankable)) return picked; - return rankable ? { sort: "relevance", order: "desc" } : { sort: "date", order: "desc" }; -} - /** A `sort` value `GET /v1/messages` takes. */ export type MessageSortParam = NonNullable; @@ -38,10 +25,29 @@ export function messageSortParam(s: MessageSearchSort): MessageSortParam { return SORT_PARAMS[s.sort === "relevance" ? "relevance" : s.order]; } -/** The choice a `sort` value stands for, or null for a value it is not. */ +/** + * The choice a `sort` value stands for, or null for a value it is not. A + * value of several keys, such as `relevance,-date`, stands for its first, + * which decides the order. + */ export function messageSortFromParam(param: string | null): MessageSearchSort | null { - if (param === SORT_PARAMS.relevance) return { sort: "relevance", order: "desc" }; - if (param === SORT_PARAMS.asc) return { sort: "date", order: "asc" }; - if (param === SORT_PARAMS.desc) return { sort: "date", order: "desc" }; + const first = param?.split(",")[0]?.trim() ?? null; + if (first === SORT_PARAMS.relevance) return { sort: "relevance", order: "desc" }; + if (first === SORT_PARAMS.asc) return { sort: "date", order: "asc" }; + if (first === SORT_PARAMS.desc) return { sort: "date", order: "desc" }; return null; } + +/** + * What the person's pick in the sort menu keeps: a Date order as its `sort` + * value, or nothing for Relevance. + * + * Relevance is never kept as a pick. The menu offers it only for a search the + * server already ranks when no `sort` is sent, so picking it means "the + * server's order". A kept `relevance` would follow the person to their next + * search, and the server refuses it for a search with no free-text word. + */ +export function pickedSortParam(s: MessageSearchSort): "date" | "-date" | null { + if (s.sort === "relevance") return null; + return s.order === "asc" ? "date" : "-date"; +} diff --git a/web/src/lib/queryKeys.ts b/web/src/lib/queryKeys.ts index 52ec00fee..7ae776650 100644 --- a/web/src/lib/queryKeys.ts +++ b/web/src/lib/queryKeys.ts @@ -82,7 +82,8 @@ export const keys = { }, /** * The Messages list: the messages a search matches across every - * conversation, `GET /v1/messages`, one entry per query and sort. + * conversation, `GET /v1/messages`, one entry per query and sort. The + * sort is `""` when the person picked none and the server picks it. */ messages: { all: ["messages"] as const, diff --git a/web/src/lib/resultsView.test.ts b/web/src/lib/resultsView.test.ts index f425440e6..81bc4eb5d 100644 --- a/web/src/lib/resultsView.test.ts +++ b/web/src/lib/resultsView.test.ts @@ -18,11 +18,8 @@ describe("resultsView", () => { }); describe("pickedMessageSort", () => { - it("reads the route's spelling, and nothing else", () => { - expect(pickedMessageSort(params("sort=relevance"))).toEqual({ - sort: "relevance", - order: "desc", - }); + it("reads a Date order in the route's spelling, and nothing else", () => { + expect(pickedMessageSort(params("sort=relevance"))).toBeNull(); expect(pickedMessageSort(params("sort=date"))).toEqual({ sort: "date", order: "asc" }); expect(pickedMessageSort(params("sort=-date"))).toEqual({ sort: "date", order: "desc" }); expect(pickedMessageSort(params("sort=colour"))).toBeNull(); diff --git a/web/src/lib/resultsView.ts b/web/src/lib/resultsView.ts index 8fbb12bb7..ad52cd74a 100644 --- a/web/src/lib/resultsView.ts +++ b/web/src/lib/resultsView.ts @@ -47,9 +47,15 @@ export function resultsView(params: URLSearchParams): ResultsView { return params.get(VIEW_PARAM) === "messages" ? "messages" : "conversations"; } -/** The sort the person picked in the Messages list, or null for the default. */ +/** + * The sort the person picked in the Messages list, or null for the server's + * own order. Only a Date order is a pick (`pickedSortParam`), so `relevance` + * in the address reads as no pick, and the server ranks the search when it + * has a free-text word to rank by. + */ export function pickedMessageSort(params: URLSearchParams): MessageSearchSort | null { - return messageSortFromParam(params.get(MESSAGE_SORT_PARAM)); + const picked = messageSortFromParam(params.get(MESSAGE_SORT_PARAM)); + return picked?.sort === "date" ? picked : null; } /** The Message Tag `params` lists by on `/messages/:id`, or null for none. */ diff --git a/web/src/lib/routeQuery.ts b/web/src/lib/routeQuery.ts index a76414a59..34a734eb3 100644 --- a/web/src/lib/routeQuery.ts +++ b/web/src/lib/routeQuery.ts @@ -161,17 +161,26 @@ export type OffsetPage = { */ const NO_PAGES: readonly unknown[] = []; -/** How a screen loads one page. */ -export type PagedFetchPage = (args: { +/** + * How a screen loads one page. A route that answers more than the rows and + * the total, as the Messages list answers its `search`, names the rest as `E`. + */ +export type PagedFetchPage = (args: { limit: number; offset: number; signal: AbortSignal; -}) => Promise>; +}) => Promise & E>; /** What a long list needs to render itself while it fills. */ -export type PagedListResult = { +export type PagedListResult = { items: T[]; total: number; + /** + * The latest page the server answered, or null before the first. A screen + * reads from it what the route says beside the rows, such as the order the + * Messages list applied. + */ + lastPage: (OffsetPage & E) | null; /** No page has arrived yet: the list has nothing to show. */ loading: boolean; /** The first page is being fetched again behind rows already on screen. */ @@ -222,9 +231,9 @@ function distinctRows(pages: readonly OffsetP * and another removed between two fetches leaves the total the same, and a * row skipped that way stays missing until the list is next fetched. */ -export function useRoutePagedList( +export function useRoutePagedList( key: RouteQueryKey, - fetchPage: PagedFetchPage, + fetchPage: PagedFetchPage, opts?: { firstPageSize?: number; fillPageSize?: number; @@ -236,7 +245,7 @@ export function useRoutePagedList( /** False holds the list back, as `enabled` does on `useQuery`. */ enabled?: boolean; }, -): PagedListResult { +): PagedListResult { const account = useAccountScope(); const firstPageSize = opts?.firstPageSize ?? PAGE_SIZE_FIRST; const fillPageSize = opts?.fillPageSize ?? PAGE_SIZE_FILL; @@ -249,13 +258,8 @@ export function useRoutePagedList( const keyText = JSON.stringify(queryKey); const largePagesFor = useRef(null); - const query = useInfiniteQuery< - OffsetPage, - Error, - InfiniteData>, - unknown[], - number - >({ + type P = OffsetPage & E; + const query = useInfiniteQuery, unknown[], number>({ queryKey, enabled: opts?.enabled ?? true, initialPageParam: 0, @@ -277,7 +281,7 @@ export function useRoutePagedList( }, }); - const pages = query.data?.pages ?? (NO_PAGES as OffsetPage[]); + const pages = query.data?.pages ?? (NO_PAGES as P[]); // A new array every render defeats every memo downstream (the tag menu and // its effect included), so this is the one place that must not recompute // unless the query actually produced new pages. @@ -294,6 +298,7 @@ export function useRoutePagedList( return { items, total: pages[pages.length - 1]?.total ?? 0, + lastPage: pages[pages.length - 1] ?? null, loading: query.isPending, // A refetch of what is already on screen, as opposed to a first load or a // page being appended. diff --git a/web/src/lib/serverApi.ts b/web/src/lib/serverApi.ts index c9268c0c5..59d87acf5 100644 --- a/web/src/lib/serverApi.ts +++ b/web/src/lib/serverApi.ts @@ -541,8 +541,10 @@ export type MessagesListParams = { offset?: number; limit?: number; /** - * `date` (oldest first, the default), `-date`, or `relevance`: best match - * first, which needs a free-text word in `q` (`hasFreeText`). + * `date` (oldest first), `-date`, or `relevance`: best match first, which + * needs a free-text word in `q`. Left out, the server picks: `relevance` + * when `q` has a free-text word and `-date` when it has none, and the + * page's `search` says which it applied. */ sort?: "date" | "-date" | "relevance"; }; @@ -555,8 +557,11 @@ export type MessagesListParams = { export function listMessages( params: MessagesListParams, opts?: RequestOptions, -): Promise { - return apiClient.get(withQuery("/v1/messages", query(params)), opts); +): Promise { + return apiClient.get( + withQuery("/v1/messages", query(params)), + opts, + ); } /** Every source a conversation's messages came from. */ diff --git a/web/src/lib/serverApi.types.ts b/web/src/lib/serverApi.types.ts index 1b70b933a..2f7b1488e 100644 --- a/web/src/lib/serverApi.types.ts +++ b/web/src/lib/serverApi.types.ts @@ -1192,8 +1192,13 @@ export interface paths { cookie?: never; }; /** - * Messages matching `q`, oldest first unless `sort` says otherwise: the same rows an Export Run with a `query` scope would hand over, behind a logged-in session with the list defaults and the list's offset ceiling. - * @description `sort=relevance` puts the best match first, ranked by the full-text + * Messages matching `q`: the same rows an Export Run with a `query` scope would hand over, behind a logged-in session with the list defaults and the list's offset ceiling. + * @description With no `sort`, the best match comes first when `q` has a free-text word + * to rank by, and the newest message first when it has none. The page's + * `search` says which order it applied and which terms it ranks by, so a + * client never parses `q` itself. + * + * `sort=relevance` puts the best match first, ranked by the full-text * index's `bm25()` on the query's free-text words: the words not behind `-` * or `not`. A query with no such word has nothing to rank by, and * `relevance` is then `validation-failed`. Ties, and a message found only by @@ -2792,6 +2797,16 @@ export interface components { /** @description The spelling, without the colon. */ word: string; }; + /** @description One free-text term of a search: a word, or a quoted phrase. */ + FreeTextTerm: { + /** + * @description True when the word ended in `*` and matches any word it begins; the + * `*` is not in `text`. Always false for a phrase. + */ + prefix: boolean; + /** @description The word, or the phrase without its quotes, as typed. */ + text: string; + }; /** * @description Which contacts `POST /v1/contacts/address-book` writes: the Contacts * list's search and its checked rows. @@ -3208,6 +3223,30 @@ export interface components { /** @description The most lines this page could hold. */ limit: number; }; + /** + * @description One page of the Messages list and how the server read its search: the + * four keys of every page, and `search`, the one key a page carries beside + * them (`docs/architecture/http-api.md`, "Lists"). + */ + ListMessagesResponse: { + /** @description The rows on this page. */ + items: components["schemas"]["Message"][]; + /** @description Page size used. */ + limit: number; + /** @description Page offset used. */ + offset: number; + /** + * @description How the server read `q` and `sort`: the order it applied and the + * free-text terms it ranks by. It describes the query, not the rows, so + * every page of one query carries the same value. + */ + search: components["schemas"]["MessageSearch"]; + /** + * Format: int64 + * @description Rows matching the query across every page. + */ + total: number; + }; /** @description Body for `POST /v1/contacts/unmatched-identities`. */ ListUnmatchedIdentitiesRequest: { /** @description Raw identifiers — phone numbers, emails — as they appear in an export. */ @@ -3412,6 +3451,22 @@ export interface components { /** @description Participants of the conversation. */ participants: components["schemas"]["Participant"][]; }; + /** @description A Messages search as the server read it. */ + MessageSearch: { + /** + * @description The order the page is in, spelled as `sort` takes it: the `sort` the + * request named, or with none, `relevance` when `terms` is not empty and + * `-date` (newest first) when it is. + */ + sort: string; + /** + * @description The free-text terms the search ranks by, in the order they were typed: + * every word and quoted phrase of `q` that is not a field word and not + * behind `-` or `not`, alone or in a negated group. Empty when `q` has + * none, and then `relevance` is refused. + */ + terms: components["schemas"]["FreeTextTerm"][]; + }; /** @description One Contact Group or Message Tag: its id and name. */ NamedSet: { /** @@ -11911,7 +11966,7 @@ export interface operations { limit?: number; /** @description Page offset, max 50000 */ offset?: number; - /** @description `date`, `-date` or `relevance` (best match first; needs a free-text word in `q`). Default `date`, oldest first. */ + /** @description `date`, `-date` or `relevance` (best match first; needs a free-text word in `q`). Default `relevance` when `q` has a free-text word, and `-date`, newest first, when it has none. */ sort?: string; }; header?: never; @@ -11926,7 +11981,7 @@ export interface operations { [name: string]: unknown; }; content: { - "application/json": components["schemas"]["Page_Message"]; + "application/json": components["schemas"]["ListMessagesResponse"]; }; }; /** @description [`authentication-required`](https://messagecrate.app/docs/developer/reference/errors/authentication-required): The request carried no usable credential: the `Authorization: Bearer ` header is missing, malformed, unknown or expired. */ diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index c278d1df0..d76e5808b 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -31,6 +31,15 @@ export type MessageTapback = Schema["Tapback"]; * same row shape the Export routes return, since one loader serves both. */ export type Message = Schema["Message"]; +/** + * A Messages search as the server read it: the order `GET /v1/messages` + * applied and the free-text terms it ranks by, which the Messages list bolds. + */ +export type MessageSearch = Schema["MessageSearch"]; + +/** One free-text term the server ranks a Messages search by: a word or a phrase. */ +export type FreeTextTerm = Schema["FreeTextTerm"]; + export type AttachmentMediaMode = "copy" | "convert" | "compress" | "skip"; export interface ExtractConfig { diff --git a/web/src/screens/MessageSearchList.test.tsx b/web/src/screens/MessageSearchList.test.tsx index 7ee1c8504..168ff954e 100644 --- a/web/src/screens/MessageSearchList.test.tsx +++ b/web/src/screens/MessageSearchList.test.tsx @@ -4,6 +4,8 @@ import { cleanup, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { MessageSearchSort } from "../lib/messageSearchSort"; import { listMessages } from "../lib/serverApi"; +import type { MessageSearch } from "../lib/types"; +import { message } from "../test/apiShapes"; import { mockedAuth, Providers } from "../test/providers"; import { setupUser } from "../test/user"; import MessageSearchList from "./MessageSearchList"; @@ -17,6 +19,21 @@ vi.mock("../lib/serverApi", async (importOriginal) => ({ const listMessagesMock = vi.mocked(listMessages); +// jsdom lays nothing out, so the real list would measure no room for a row. +// Every row is drawn here, which is all these tests need of it. +vi.mock("../components/VirtualList", async () => { + const { createElement } = await import("react"); + return { + default: ({ + count, + renderItem, + }: { + count: number; + renderItem: (index: number) => import("react").ReactNode; + }) => createElement("div", null, ...Array.from({ length: count }, (_, i) => renderItem(i))), + }; +}); + // jsdom has no ResizeObserver; VirtualList observes its scroll container on mount. class StubResizeObserver { observe() {} @@ -27,9 +44,14 @@ class StubResizeObserver { beforeEach(() => { vi.stubGlobal("ResizeObserver", StubResizeObserver); listMessagesMock.mockReset(); - listMessagesMock.mockResolvedValue({ items: [], total: 12408, limit: 40, offset: 0 }); + answer({ sort: "relevance", terms: [{ text: "photo", prefix: false }] }); }); +/** The server answers every page with `search`, and 12,408 messages in all. */ +function answer(search: MessageSearch, items = [] as ReturnType[]) { + listMessagesMock.mockResolvedValue({ items, total: 12408, limit: 40, offset: 0, search }); +} + afterEach(() => { vi.unstubAllGlobals(); cleanup(); @@ -65,32 +87,52 @@ describe("MessageSearchList", () => { expect(listMessagesMock).not.toHaveBeenCalled(); }); - it("shows the total, and sorts by relevance by default for a free-text word", async () => { + it("sends no sort, and shows the order the server applied", async () => { renderList("from:Alice photo"); expect(await screen.findByText("12,408 messages")).toBeVisible(); expect(listMessagesMock).toHaveBeenCalledWith( - { q: "from:Alice photo", sort: "relevance", limit: 40, offset: 0 }, + { q: "from:Alice photo", limit: 40, offset: 0 }, expect.anything(), ); expect(screen.getByRole("button", { name: "Sort messages by Relevance" })).toBeVisible(); expect(await sortChoices()).toEqual(["Relevance", "Date"]); }); - it("offers only Date, newest first, when the search has no free-text word", async () => { + it("offers only Date when the server returns no terms to rank by", async () => { + answer({ sort: "-date", terms: [] }); renderList("from:Alice date:2024"); - await waitFor(() => - expect(listMessagesMock).toHaveBeenCalledWith( - expect.objectContaining({ q: "from:Alice date:2024", sort: "-date" }), - expect.anything(), - ), + expect( + await screen.findByRole("button", { name: "Sort messages by Date, Newest first" }), + ).toBeVisible(); + expect(listMessagesMock).toHaveBeenCalledWith( + { q: "from:Alice date:2024", limit: 40, offset: 0 }, + expect.anything(), ); + expect(await sortChoices()).toEqual(["Date", "Oldest first", "Newest first"]); + }); + + it("takes the order and the terms from the server, not from the words typed", async () => { + // A free-text word the server reports no term for: the menu follows the + // server, and offers no Relevance. + answer({ sort: "-date", terms: [] }); + renderList("photo"); expect( - screen.getByRole("button", { name: "Sort messages by Date, Newest first" }), + await screen.findByRole("button", { name: "Sort messages by Date, Newest first" }), ).toBeVisible(); expect(await sortChoices()).toEqual(["Date", "Oldest first", "Newest first"]); }); - it("keeps a picked date order, and hands a new pick to the caller", async () => { + it("bolds the terms the server returned", async () => { + answer({ sort: "relevance", terms: [{ text: "dent", prefix: true }] }, [ + message({ id: 7, text: "Here is the photo from the dentist" }), + ]); + renderList("whatever was typed"); + const row = await screen.findByRole("button", { name: /photo from the dentist/ }); + expect([...row.querySelectorAll("strong")].map((b) => b.textContent)).toEqual(["dentist"]); + }); + + it("sends a picked date order, and hands a new pick to the caller", async () => { + answer({ sort: "date", terms: [{ text: "photo", prefix: false }] }); const { onSortPick } = renderList("photo", { sort: "date", order: "asc" }); await waitFor(() => expect(listMessagesMock).toHaveBeenCalledWith( @@ -99,7 +141,9 @@ describe("MessageSearchList", () => { ), ); const user = setupUser(); - await user.click(screen.getByRole("button", { name: "Sort messages by Date, Oldest first" })); + await user.click( + await screen.findByRole("button", { name: "Sort messages by Date, Oldest first" }), + ); await user.click(screen.getByRole("menuitemradio", { name: "Relevance" })); expect(onSortPick).toHaveBeenCalledWith({ sort: "relevance", order: "asc" }); }); diff --git a/web/src/screens/MessageSearchList.tsx b/web/src/screens/MessageSearchList.tsx index bbbc300c2..33c89a540 100644 --- a/web/src/screens/MessageSearchList.tsx +++ b/web/src/screens/MessageSearchList.tsx @@ -1,22 +1,22 @@ -import { type ReactNode, useCallback, useMemo } from "react"; +import { useCallback } from "react"; import ListRangeHeader from "../components/ListRangeHeader"; import MessageSearchRow from "../components/MessageSearchRow"; import SortMenu, { type SortField } from "../components/SortMenu"; import VirtualList from "../components/VirtualList"; import { apiErrorMessage } from "../lib/apiErrorMessage"; -import { freeTextTerms } from "../lib/freeTextTerms"; import { MAX_LIST_OFFSET } from "../lib/listPaging"; import { messageCount } from "../lib/messageRowText"; import { - effectiveMessageSort, type MessageSearchSort, type MessageSearchSortKey, + type MessageSortParam, + messageSortFromParam, messageSortParam, } from "../lib/messageSearchSort"; import { keys } from "../lib/queryKeys"; import { type PagedFetchPage, useRoutePagedList } from "../lib/routeQuery"; import { listMessages } from "../lib/serverApi"; -import type { Message } from "../lib/types"; +import type { FreeTextTerm, Message, MessageSearch } from "../lib/types"; import { useDebouncedQuery } from "../lib/useDebouncedQuery"; /** Rows read at a time as the person scrolls. */ @@ -25,16 +25,24 @@ const PAGE_SIZE = 40; const RELEVANCE: SortField = { id: "relevance", label: "Relevance" }; const DATE: SortField = { id: "date", label: "Date" }; +/** No terms to bold: one shared value, so a row's memo sees the same array. */ +const NO_TERMS: readonly FreeTextTerm[] = []; + +/** What a page of `GET /v1/messages` says beside its rows: the server's reading of the search. */ +type MessagesPageExtra = { search: MessageSearch }; + /** * The Messages list: one row per message the search matches, from every * conversation (#313). It reads 40 rows at a time as the person scrolls and * shows the total, up to the route's offset ceiling of 50,000. An empty * search lists nothing and asks for one. * - * The sort menu offers Relevance only when the query has a free-text word - * to rank by, and it is then the default; otherwise the default is Date, - * newest first. A pick is the caller's to keep, so it survives opening a - * result. + * The server picks the order unless the person picked one, and every page + * says which order it applied and which free-text terms it ranks by (#1538). + * The sort menu shows that order, offers Relevance only when terms came + * back, and the rows bold those terms, so nothing here parses the search to + * guess what the server will do. A pick is the caller's to keep, so it + * survives opening a result. */ export default function MessageSearchList({ query, @@ -50,24 +58,7 @@ export default function MessageSearchList({ onSelect: (message: Message) => void; }) { const debouncedQ = useDebouncedQuery(query); - const q = debouncedQ.trim(); - const terms = useMemo(() => freeTextTerms(q), [q]); - const rankable = terms.length > 0; - const sort = effectiveMessageSort(sortPick, rankable); - - const sortMenu = ( - - ); if (!q) { return ( @@ -80,13 +71,14 @@ export default function MessageSearchList({ ); } + const sortParam = sortPick ? messageSortParam(sortPick) : null; return ( @@ -95,34 +87,54 @@ export default function MessageSearchList({ function MessageResults({ q, - sort, - terms, - sortMenu, + sortParam, + sortPick, + onSortPick, selectedId, onSelect, }: { q: string; - sort: MessageSearchSort; - terms: ReturnType; - sortMenu: ReactNode; + /** The `sort` to send, or null to let the server pick. */ + sortParam: MessageSortParam | null; + sortPick: MessageSearchSort | null; + onSortPick: (next: MessageSearchSort) => void; selectedId: number | null; onSelect: (message: Message) => void; }) { - const sortParam = messageSortParam(sort); - const fetchPage = useCallback>( + const fetchPage = useCallback>( async ({ limit, offset, signal }) => { - const res = await listMessages({ q, sort: sortParam, limit, offset }, { signal }); - return { items: res.items, total: res.total }; + const res = await listMessages( + sortParam ? { q, sort: sortParam, limit, offset } : { q, limit, offset }, + { signal }, + ); + return { items: res.items, total: res.total, search: res.search }; }, [q, sortParam], ); - const { items, total, loading, refreshing, filling, error, hasMore, loadMore } = - useRoutePagedList(keys.messages.list(q, sortParam), fetchPage, { + const { items, total, lastPage, loading, refreshing, filling, error, hasMore, loadMore } = + useRoutePagedList(keys.messages.list(q, sortParam ?? ""), fetchPage, { firstPageSize: PAGE_SIZE, fillPageSize: PAGE_SIZE, maxOffset: MAX_LIST_OFFSET, }); + const search = lastPage?.search ?? null; + const terms = search?.terms ?? NO_TERMS; + // Until the first page says which order it is in, the menu shows the + // person's pick, or nothing when the server is picking. + const sort = search ? messageSortFromParam(search.sort) : sortPick; + const sortMenu = sort ? ( + 0 ? [RELEVANCE, DATE] : [DATE]} + unordered={["relevance"]} + sort={sort.sort} + order={sort.order} + onChange={onSortPick} + itemNoun="messages" + ascLabel="Oldest first" + descLabel="Newest first" + /> + ) : null; if (error && items.length === 0) { return (
diff --git a/web/src/screens/message/useConversationMessages.test.tsx b/web/src/screens/message/useConversationMessages.test.tsx index 945d7df07..94aab4620 100644 --- a/web/src/screens/message/useConversationMessages.test.tsx +++ b/web/src/screens/message/useConversationMessages.test.tsx @@ -53,6 +53,9 @@ function deferred() { return { promise, resolve, reject }; } +/** What `GET /v1/messages` says of a Find search: newest first, as Find asks. */ +const FIND_SEARCH = { sort: "-date", terms: [] }; + type MessagePage = { items: Message[]; total: number; limit: number; offset: number }; function page(items: Message[]): MessagePage { return { items, total: items.length, limit: 50, offset: 0 }; @@ -215,6 +218,7 @@ describe("useConversationMessages", () => { })) as unknown as typeof listConversationMessages); // The newest match is found only by its earlier version; the older one by its text. searchMessages.mockResolvedValue({ + search: FIND_SEARCH, items: [ { ...message(60), @@ -259,6 +263,7 @@ describe("useConversationMessages", () => { }); // `pizz` finds message 60 only by its first version; `pizza tonight` by its second. searchMessages.mockImplementation((async ({ q }: { q?: string }) => ({ + search: FIND_SEARCH, items: [ { ...message(60), @@ -296,6 +301,7 @@ describe("useConversationMessages", () => { offset: 0, })) as unknown as typeof listConversationMessages); searchMessages.mockResolvedValue({ + search: FIND_SEARCH, items: [ { ...message(60), @@ -353,7 +359,13 @@ describe("useConversationMessages", () => { limit: 50, offset: params.around === undefined ? 98 : 0, })) as unknown as typeof listConversationMessages); - searchMessages.mockResolvedValue({ items: [message(3)], total: 40, limit: 1, offset: 0 }); + searchMessages.mockResolvedValue({ + search: FIND_SEARCH, + items: [message(3)], + total: 40, + limit: 1, + offset: 0, + }); const { result } = renderHook(() => useConversationMessages(7), { wrapper: Providers }); await waitFor(() => expect(result.current.loading).toBe(false)); @@ -381,6 +393,7 @@ describe("useConversationMessages", () => { offset: 0, })) as unknown as typeof listConversationMessages); searchMessages.mockResolvedValue({ + search: FIND_SEARCH, items: [message(60), message(20)], total: 2, limit: 50, @@ -430,6 +443,7 @@ describe("useConversationMessages", () => { page([message(params.around ?? matches)])) as unknown as typeof listConversationMessages); // The server pages the matches 50 at a time, newest first, and counts all of them. searchMessages.mockImplementation(async ({ offset = 0, limit = 50 }) => ({ + search: FIND_SEARCH, items: Array.from({ length: Math.max(0, Math.min(limit, matches - offset)) }, (_, i) => message(matches - offset - i), ), From 1bc194132f6a9ae2dae7721d97a119471eee57d6 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:31:20 -0400 Subject: [PATCH 03/12] docs: record the Messages list's default order and its `search` key `http-api.md`, "Lists", gains the one written exception to the four-key page, with its reason and the rejected alternative. `search.md` records that the server picks the Messages list's order when no `sort` is sent. CHANGELOG entry under 0.11.0. Refs #1538 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 11 +++++++++++ docs/architecture/http-api.md | 20 ++++++++++++++++++++ docs/architecture/search.md | 11 +++++++++++ 3 files changed, 42 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b30258ba..2ea1ceafa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,17 @@ released versions carry their date on the heading. ### Design +- 2026-10-05: **The server picks the order of the Messages list.** With no + `sort`, `GET /v1/messages` now puts the best match first when the search + has a plain word to rank by, and the newest message first when it has + none. It used to list the oldest first. Each page says which order it + applied and which words it ranks by, in a new `search` key beside `items`, + and the web app reads both from there: the sort menu offers Relevance only + when words came back, and the list draws those words in bold. Nothing + changes on screen, except that a search the web app and the server once + read differently now shows the server's reading. Picking Relevance now + leaves `sort` out of the address, so the next search starts in the + server's order. - 2026-10-05: **A message's reply count is counted when it is read.** It is the number of replies Message Crate shows that quote the message, so a reply hidden as a duplicate no longer counts, and a reply that quotes a diff --git a/docs/architecture/http-api.md b/docs/architecture/http-api.md index c22509765..f8bc11f4e 100644 --- a/docs/architecture/http-api.md +++ b/docs/architecture/http-api.md @@ -264,6 +264,26 @@ below). The list key is always `items`. Why: the web app has one paged type and one hook, and a second shape is a second convention. +The Messages list, `GET /v1/messages`, is the one page with a key beside the +four: `search`, which says how the server read the request. `search.sort` is +the order the page is in, spelled as the `sort` parameter takes it, and +`search.terms` is the free-text terms the query ranks by, each a `text` and +whether it is a `prefix`. The key describes the query, not the rows, so every +page of one query carries the same value. +Why: with no `sort`, the server picks the order from the query, best match +first when it has a free-text term and newest first when it has none +(`docs/architecture/search.md`), and the web app shows that order in its sort +menu and draws the terms in bold. Only the module that parses the search +language knows which words those are +(`docs/adr/0004-one-search-language-compiled-in-one-module.md`). The web app +once read them from `q` with a parser of its own, a second copy of the +grammar that nothing held to the first (#1538). +Rejected: a route of its own that reads a query and answers its terms. A +query is not a resource, and every new search would wait for that answer +before it could ask for its first page in the right order. +`openapi/document_rules.rs` names the route and its schema, +`ListMessagesResponse`, and fails any other page with a key beyond the four. + A `POST` that reads the rows its body names — contact summaries, unmatched identities — answers the whole of that body as one page and takes no `offset` or `limit`: `total` is the row count, `limit` is the cap the body is held to, diff --git a/docs/architecture/search.md b/docs/architecture/search.md index 20d9cdf4c..bc5a56839 100644 --- a/docs/architecture/search.md +++ b/docs/architecture/search.md @@ -71,6 +71,17 @@ Each rule holds on every list, and each has its reason. Why: a word that only excludes says nothing about how well a message matches, and a silent fallback to date would show an order the person did not pick. +- **With no `sort`, the server picks the Messages list's order.** A query + with a positive free-text word is ranked best match first, and a query + with none, field words only or nothing, lists the newest message first. + Every page says which order it applied and which terms it ranks by, in + `search` (`docs/architecture/http-api.md`, "Lists"), from + `Expr::positive_text_terms`. A `sort` the request names is applied as it + is. Why: a search for words is looking for the messages that hold them, + and a search by fields alone is browsing, where the latest come first. The + choice depends on which words rank, which only this module knows, so the + server makes it and the client reads it back rather than parsing `q` + itself (#1538). - **Compile is pure.** Compiling reads no database and no clock. Anything that needs a lookup (the last Import Run, a Contact Group by name) is a subquery, and today's date and the account's time zone are inputs. Why: the same From 3c00441785f9e371b42f237f3ab8ebd411496bb0 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:35:13 -0400 Subject: [PATCH 04/12] docs(changelog): one unbroken Design list after the merge Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fd3c52f6..196b83dbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,7 +56,6 @@ released versions carry their date on the heading. read differently now shows the server's reading. Picking Relevance now leaves `sort` out of the address, so the next search starts in the server's order. - - 2026-10-05: **A message's reply count is counted when it is read.** It is the number of replies Message Crate shows that quote the message, so a reply hidden as a duplicate no longer counts, and a reply that quotes a From 7b345b7786457eef4d431a659209483d59691176 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:47:17 -0400 Subject: [PATCH 05/12] docs(changelog): say free-text word, the term search.md uses Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 196b83dbb..7c9e688ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ released versions carry their date on the heading. be read in full: …` before. - 2026-10-05: **The server picks the order of the Messages list.** With no `sort`, `GET /v1/messages` now puts the best match first when the search - has a plain word to rank by, and the newest message first when it has + has a free-text word to rank by, and the newest message first when it has none. It used to list the oldest first. Each page says which order it applied and which words it ranks by, in a new `search` key beside `items`, and the web app reads both from there: the sort menu offers Relevance only From a5e26e3e41db056d1fe2bebe7244d2916bb55ace Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:47:17 -0400 Subject: [PATCH 06/12] test(openapi): the page rules know a page by its four keys, not its name A hand-written page under a name without the Page_ prefix was not seen as a page, so the no-extra-keys rule never ran on it. http-api.md gives the check its own paragraph, apart from the rejected alternative. Co-Authored-By: Claude Opus 5.5 --- .../server/src/openapi/document_rules.rs | 34 +++++++++++++------ docs/architecture/http-api.md | 5 ++- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/server/server/src/openapi/document_rules.rs b/crates/server/server/src/openapi/document_rules.rs index 9c0bc3ced..b53470d09 100644 --- a/crates/server/server/src/openapi/document_rules.rs +++ b/crates/server/server/src/openapi/document_rules.rs @@ -49,8 +49,9 @@ const PAGE_KEYS: [&str; 4] = ["items", "total", "limit", "offset"]; /// The one page with a key beside the four, and the one route that answers /// it: `GET /v1/messages` says how it read its search in `search` /// (`docs/architecture/http-api.md`, "Lists"). Named here, route and schema -/// both, so a second page with a fifth key fails the rules rather than -/// passing under a name that does not start with `Page_`. +/// both. Any other schema with the four keys is a page too, whatever its +/// name ([`page_schemas`]), so a second page with a fifth key fails the +/// rules. struct PageException { method: &'static str, path: &'static str, @@ -228,7 +229,7 @@ fn read_rules(doc: &Value, op: &Operation, spec: &Value) -> Vec { )); } - for page in page_schemas(doc, op, spec) { + for page in page_schemas(doc, spec) { let required: BTreeSet<&str> = page["required"] .as_array() .into_iter() @@ -371,7 +372,7 @@ async fn called_rules(doc: &Value, world: &World<'_>, op: &Operation, spec: &Val } } - if op.method == "get" && !page_schemas(doc, op, spec).is_empty() { + if op.method == "get" && !page_schemas(doc, spec).is_empty() { let mut out_of_range = vec!["limit=0", "limit=501"]; // A browse list says its offset ceiling in the parameter's own // description, and must keep to it. @@ -944,31 +945,42 @@ fn is_kebab(segment: &str) -> bool { /// The page schemas a `200` answers: the page it names, or each page of a /// choice between pages, as an account's history answers the account in -/// full and the owner without content, or [`PAGE_EXCEPTION`]'s page on its -/// own route. Empty when it answers no page. -fn page_schemas<'d>(doc: &'d Value, op: &Operation, spec: &Value) -> Vec<&'d Value> { +/// full and the owner without content. A schema is a page by its shape, +/// whatever its name: it has all four [`PAGE_KEYS`], so a page under a name +/// that does not start with `Page_` is still held to the page rules. Empty +/// when it answers no page. +fn page_schemas<'d>(doc: &'d Value, spec: &Value) -> Vec<&'d Value> { let schemas = &doc["components"]["schemas"]; let Some(name) = schema_named(&spec["responses"]["200"]["content"]["application/json"]["schema"]) else { return Vec::new(); }; - if name.starts_with("Page_") || (PAGE_EXCEPTION.is(op) && name == PAGE_EXCEPTION.schema) { + if is_page(&schemas[name]) { return vec![&schemas[name]]; } - let choices: Vec<&str> = schemas[name]["oneOf"] + let choices: Vec<&Value> = schemas[name]["oneOf"] .as_array() .into_iter() .flatten() .filter_map(schema_named) + .map(|c| &schemas[c]) .collect(); - if !choices.is_empty() && choices.iter().all(|c| c.starts_with("Page_")) { - choices.into_iter().map(|c| &schemas[c]).collect() + if !choices.is_empty() && choices.iter().all(|c| is_page(c)) { + choices } else { Vec::new() } } +/// Whether `schema` has the shape of a page: all four [`PAGE_KEYS`] among +/// its properties. +fn is_page(schema: &Value) -> bool { + PAGE_KEYS + .iter() + .all(|key| schema["properties"].get(*key).is_some()) +} + /// What the operation says about its `offset`. fn offset_description(spec: &Value) -> &str { spec["parameters"] diff --git a/docs/architecture/http-api.md b/docs/architecture/http-api.md index f8bc11f4e..2a7162302 100644 --- a/docs/architecture/http-api.md +++ b/docs/architecture/http-api.md @@ -267,7 +267,7 @@ second convention. The Messages list, `GET /v1/messages`, is the one page with a key beside the four: `search`, which says how the server read the request. `search.sort` is the order the page is in, spelled as the `sort` parameter takes it, and -`search.terms` is the free-text terms the query ranks by, each a `text` and +`search.terms` holds the free-text terms the query ranks by, each a `text` and whether it is a `prefix`. The key describes the query, not the rows, so every page of one query carries the same value. Why: with no `sort`, the server picks the order from the query, best match @@ -281,8 +281,11 @@ grammar that nothing held to the first (#1538). Rejected: a route of its own that reads a query and answers its terms. A query is not a resource, and every new search would wait for that answer before it could ask for its first page in the right order. + `openapi/document_rules.rs` names the route and its schema, `ListMessagesResponse`, and fails any other page with a key beyond the four. +It knows a page by its four keys, not by its name, so a page under a new +name is held to the same rule. A `POST` that reads the rows its body names — contact summaries, unmatched identities — answers the whole of that body as one page and takes no `offset` From 108f39deaca2d2a9c451f46706c4b7b6e85ba915 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:47:17 -0400 Subject: [PATCH 07/12] refactor(web): pickedSortParam reads the Date values from SORT_PARAMS Co-Authored-By: Claude Opus 5.5 --- web/src/lib/messageSearchSort.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/web/src/lib/messageSearchSort.ts b/web/src/lib/messageSearchSort.ts index 8311da9ac..7321646e1 100644 --- a/web/src/lib/messageSearchSort.ts +++ b/web/src/lib/messageSearchSort.ts @@ -14,11 +14,11 @@ export type MessageSortParam = NonNullable; * The `sort` value for each choice, read both ways: Relevance has one order, * and Date one value per order. */ -const SORT_PARAMS: Readonly> = { +const SORT_PARAMS = { relevance: "relevance", asc: "date", desc: "-date", -}; +} as const satisfies Readonly>; /** The `sort` parameter `GET /v1/messages` takes for `s`. */ export function messageSortParam(s: MessageSearchSort): MessageSortParam { @@ -47,7 +47,6 @@ export function messageSortFromParam(param: string | null): MessageSearchSort | * server's order". A kept `relevance` would follow the person to their next * search, and the server refuses it for a search with no free-text word. */ -export function pickedSortParam(s: MessageSearchSort): "date" | "-date" | null { - if (s.sort === "relevance") return null; - return s.order === "asc" ? "date" : "-date"; +export function pickedSortParam(s: MessageSearchSort): (typeof SORT_PARAMS)[SortOrder] | null { + return s.sort === "relevance" ? null : SORT_PARAMS[s.order]; } From 7970ba08b9ba8b19357f196a0ce9159082892c38 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:47:17 -0400 Subject: [PATCH 08/12] docs(server): say why ListMessagesResponse writes out the page keys Co-Authored-By: Claude Opus 5.5 --- crates/server/server/src/messages_api.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/server/server/src/messages_api.rs b/crates/server/server/src/messages_api.rs index ce024f842..2d2f70e5a 100644 --- a/crates/server/server/src/messages_api.rs +++ b/crates/server/server/src/messages_api.rs @@ -23,6 +23,10 @@ use crate::server::{ApiError, AppState, FullAccess}; /// One page of the Messages list and how the server read its search: the /// four keys of every page, and `search`, the one key a page carries beside /// them (`docs/architecture/http-api.md`, "Lists"). +// The four keys are written out rather than taken from `Page` with +// `#[serde(flatten)]`: utoipa describes a flattened field as an `allOf` of +// two schemas, which gives the page no `properties` of its own for the page +// rules (`openapi/document_rules.rs`) and the generated web types to read. #[derive(Debug, Serialize, utoipa::ToSchema)] pub(crate) struct ListMessagesResponse { /// The rows on this page. From 5fb347f28e0d384bfef72013966dbfb88ec99aa9 Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:47:17 -0400 Subject: [PATCH 09/12] refactor(search): a Filter builds its rank query from its ranked terms The Filter kept both, and nothing checked that they agreed. message_list_sort_text now fails loudly on a key missing from MESSAGE_LIST_SORT_KEYS rather than reporting an empty sort. Co-Authored-By: Claude Opus 5.5 --- crates/server/server/src/db/conversation_messages.rs | 5 +++-- crates/server/server/src/search/emit.rs | 1 - crates/server/server/src/search/mod.rs | 10 ++++------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/server/server/src/db/conversation_messages.rs b/crates/server/server/src/db/conversation_messages.rs index a519a7624..03d9aa8d2 100644 --- a/crates/server/server/src/db/conversation_messages.rs +++ b/crates/server/server/src/db/conversation_messages.rs @@ -225,7 +225,8 @@ pub fn message_list_sort_text(order: &[SortKey]) -> String { let name = MESSAGE_LIST_SORT_KEYS .iter() .find(|(_, key)| *key == k.key) - .map_or("", |(name, _)| *name); + .map(|(name, _)| *name) + .expect("MESSAGE_LIST_SORT_KEYS names every MessageListSort key"); match k.direction { Direction::Asc => name.to_string(), Direction::Desc => format!("-{name}"), @@ -393,7 +394,7 @@ pub(crate) fn message_list_page_sql( )); }; from_sql.push_str(RANK_JOIN_SQL); - params.push(SqlParam::Text(rank_query.to_string())); + params.push(SqlParam::Text(rank_query)); } params.extend_from_slice(filter.params()); diff --git a/crates/server/server/src/search/emit.rs b/crates/server/server/src/search/emit.rs index bad541c64..198fb6e68 100644 --- a/crates/server/server/src/search/emit.rs +++ b/crates/server/server/src/search/emit.rs @@ -79,7 +79,6 @@ pub(crate) fn compile( Ok(Filter { where_sql, params, - rank_query, ranked_terms, final_text, earlier_version_match, diff --git a/crates/server/server/src/search/mod.rs b/crates/server/server/src/search/mod.rs index 35e485550..b78475050 100644 --- a/crates/server/server/src/search/mod.rs +++ b/crates/server/server/src/search/mod.rs @@ -87,7 +87,6 @@ pub fn today_in(zone: chrono_tz::Tz) -> NaiveDate { pub struct Filter { where_sql: String, params: Vec, - rank_query: Option, ranked_terms: Vec, final_text: Option<(String, Vec)>, earlier_version_match: Option<(String, Vec)>, @@ -108,14 +107,14 @@ impl Filter { /// The full-text query a Messages search ranks by: its free-text words /// not under a negation, joined by `OR`, for `bm25()`. `None` on the /// other lists, and for a query with no such word. - pub fn rank_query(&self) -> Option<&str> { - self.rank_query.as_deref() + pub fn rank_query(&self) -> Option { + fts::rank_query(&self.ranked_terms.iter().collect::>()) } /// The free-text terms [`Self::rank_query`] is built from, in the order /// they were typed: every one not behind `-` or `not`, alone or in a - /// negated group (`Expr::positive_text_terms`). Empty exactly when - /// `rank_query` is `None`. + /// negated group (`Expr::positive_text_terms`). Empty when there is + /// nothing to rank by, and so `rank_query` is `None`. pub(crate) fn ranked_terms(&self) -> &[parse::TextTerm] { &self.ranked_terms } @@ -190,7 +189,6 @@ pub fn compile_messages_of_conversations(req: CompileRequest<'_>) -> Result Date: Mon, 5 Oct 2026 23:52:21 -0400 Subject: [PATCH 10/12] refactor(search): compile reads its rank query from the Filter it builds The query was built twice from the same terms, once in compile and once in Filter::rank_query. The default order asks whether there are terms rather than building the query to throw it away. Co-Authored-By: Claude Opus 5.5 --- crates/server/server/src/messages_api.rs | 2 +- crates/server/server/src/search/emit.rs | 29 +++++++++++------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/crates/server/server/src/messages_api.rs b/crates/server/server/src/messages_api.rs index 2d2f70e5a..27262b4da 100644 --- a/crates/server/server/src/messages_api.rs +++ b/crates/server/server/src/messages_api.rs @@ -152,7 +152,7 @@ pub(crate) async fn list_messages( .await?; let filter = message_filter(auth.account_id, &list.q, list.clock)?; let order = if list.order.is_empty() { - default_message_list_sort(filter.rank_query().is_some()).to_vec() + default_message_list_sort(!filter.ranked_terms().is_empty()).to_vec() } else { list.order }; diff --git a/crates/server/server/src/search/emit.rs b/crates/server/server/src/search/emit.rs index 198fb6e68..54ed83f36 100644 --- a/crates/server/server/src/search/emit.rs +++ b/crates/server/server/src/search/emit.rs @@ -62,27 +62,24 @@ pub(crate) fn compile( } _ => Vec::new(), }; - let rank_query = fts::rank_query(&ranked_terms.iter().collect::>()); let (where_sql, params) = compile_where(list, expr, account_id, zone, true)?; + let mut filter = Filter { + where_sql, + params, + ranked_terms, + final_text: None, + earlier_version_match: None, + }; // Only a free-text word that is not negated can find a message by an // earlier version alone: a negated one only leaves messages out, and // reading earlier versions too leaves out more, never adds. - let final_text = match rank_query { - Some(_) => Some(compile_where(list, expr, account_id, zone, false)?), - None => None, - }; - let earlier_version_match = rank_query.as_deref().map(|query| { + if let Some(query) = filter.rank_query() { + filter.final_text = Some(compile_where(list, expr, account_id, zone, false)?); let mut out = Sql::default(); - fts::version_ids_matching(&mut out, query); - (out.text, out.params) - }); - Ok(Filter { - where_sql, - params, - ranked_terms, - final_text, - earlier_version_match, - }) + fts::version_ids_matching(&mut out, &query); + filter.earlier_version_match = Some((out.text, out.params)); + } + Ok(filter) } /// The WHERE fragment and its values for `expr` on `list`, its free-text From 72d3d8bfd8511b2599dcf690a1af7e34bc024c8e Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:52:21 -0400 Subject: [PATCH 11/12] refactor(server): MessageListSort names its own keys An exhaustive match gives each key its spelling, so a new key without a name fails the build rather than a request. Co-Authored-By: Claude Opus 5.5 --- .../server/src/db/conversation_messages.rs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/server/server/src/db/conversation_messages.rs b/crates/server/server/src/db/conversation_messages.rs index 03d9aa8d2..438b9895e 100644 --- a/crates/server/server/src/db/conversation_messages.rs +++ b/crates/server/server/src/db/conversation_messages.rs @@ -189,10 +189,24 @@ pub enum MessageListSort { Relevance, } +impl MessageListSort { + /// The key as `sort=` spells it. + #[must_use] + pub const fn name(self) -> &'static str { + match self { + Self::Date => "date", + Self::Relevance => "relevance", + } + } +} + /// The Messages list's keys, as `sort=` spells them. pub const MESSAGE_LIST_SORT_KEYS: [(&str, MessageListSort); 2] = [ - ("date", MessageListSort::Date), - ("relevance", MessageListSort::Relevance), + (MessageListSort::Date.name(), MessageListSort::Date), + ( + MessageListSort::Relevance.name(), + MessageListSort::Relevance, + ), ]; /// The order the Messages list applies when the request names no `sort`: @@ -221,16 +235,9 @@ pub fn default_message_list_sort(ranked: bool) -> [SortKey; 1] pub fn message_list_sort_text(order: &[SortKey]) -> String { order .iter() - .map(|k| { - let name = MESSAGE_LIST_SORT_KEYS - .iter() - .find(|(_, key)| *key == k.key) - .map(|(name, _)| *name) - .expect("MESSAGE_LIST_SORT_KEYS names every MessageListSort key"); - match k.direction { - Direction::Asc => name.to_string(), - Direction::Desc => format!("-{name}"), - } + .map(|k| match k.direction { + Direction::Asc => k.key.name().to_string(), + Direction::Desc => format!("-{}", k.key.name()), }) .collect::>() .join(",") From ccddf875d05ce21d7bfbdf22304057389066c77e Mon Sep 17 00:00:00 2001 From: Matt Beisser <225018+mbeisser1@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:52:21 -0400 Subject: [PATCH 12/12] test(openapi): a Page_ schema stays a page when it loses a key Knowing a page only by its four keys meant a Page that lost one was no longer a page, and every page rule stopped running without failing. Co-Authored-By: Claude Opus 5.5 --- .../server/src/openapi/document_rules.rs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/server/server/src/openapi/document_rules.rs b/crates/server/server/src/openapi/document_rules.rs index b53470d09..668c5dc98 100644 --- a/crates/server/server/src/openapi/document_rules.rs +++ b/crates/server/server/src/openapi/document_rules.rs @@ -945,10 +945,9 @@ fn is_kebab(segment: &str) -> bool { /// The page schemas a `200` answers: the page it names, or each page of a /// choice between pages, as an account's history answers the account in -/// full and the owner without content. A schema is a page by its shape, -/// whatever its name: it has all four [`PAGE_KEYS`], so a page under a name -/// that does not start with `Page_` is still held to the page rules. Empty -/// when it answers no page. +/// full and the owner without content. A schema is a page by its name or by +/// its shape ([`is_page`]), so a page under a name that does not start with +/// `Page_` is still held to the page rules. Empty when it answers no page. fn page_schemas<'d>(doc: &'d Value, spec: &Value) -> Vec<&'d Value> { let schemas = &doc["components"]["schemas"]; let Some(name) = @@ -956,29 +955,30 @@ fn page_schemas<'d>(doc: &'d Value, spec: &Value) -> Vec<&'d Value> { else { return Vec::new(); }; - if is_page(&schemas[name]) { + if is_page(name, &schemas[name]) { return vec![&schemas[name]]; } - let choices: Vec<&Value> = schemas[name]["oneOf"] + let choices: Vec<&str> = schemas[name]["oneOf"] .as_array() .into_iter() .flatten() .filter_map(schema_named) - .map(|c| &schemas[c]) .collect(); - if !choices.is_empty() && choices.iter().all(|c| is_page(c)) { - choices + if !choices.is_empty() && choices.iter().all(|c| is_page(c, &schemas[*c])) { + choices.into_iter().map(|c| &schemas[c]).collect() } else { Vec::new() } } -/// Whether `schema` has the shape of a page: all four [`PAGE_KEYS`] among -/// its properties. -fn is_page(schema: &Value) -> bool { - PAGE_KEYS - .iter() - .all(|key| schema["properties"].get(*key).is_some()) +/// Whether the schema `name` is a page: `Page`'s own schemas by their +/// `Page_` name, so one that lost a key still fails the page rules, and any +/// other schema with all four [`PAGE_KEYS`] among its properties. +fn is_page(name: &str, schema: &Value) -> bool { + name.starts_with("Page_") + || PAGE_KEYS + .iter() + .all(|key| schema["properties"].get(*key).is_some()) } /// What the operation says about its `offset`.