From afcdab842f6d7ed18766eff102c7e8f2990b622f Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 1/2] feat(genres): add canonical metadata and storage --- crates/libcalibre/src/custom_columns.rs | 14 ++ crates/libcalibre/src/lib.rs | 4 +- crates/libcalibre/src/library.rs | 133 +++++++++++++++++++ crates/libcalibre/src/queries/books.rs | 8 ++ crates/libcalibre/src/queries/genres.rs | 39 ++++++ crates/libcalibre/src/queries/mod.rs | 1 + crates/libcalibre/tests/query_test.rs | 81 ++++++++++- docs/adr/0003-canonical-book-genres.md | 59 ++++++++ src-tauri/src/libs/calibre/command.rs | 14 ++ src-tauri/src/libs/calibre/query.rs | 1 + src-tauri/src/main.rs | 1 + src-tauri/src/metadata/hardcover.rs | 39 +++++- src-tauri/src/metadata/marcxml.rs | 23 +++- src-tauri/src/metadata/model.rs | 59 ++++++++ src-tauri/src/metadata/openlibrary.rs | 2 + src/bindings.ts | 23 ++++ src/components/pages/EditBook.tsx | 40 +++++- src/lib/hooks/use-metadata-book-actions.ts | 5 + src/lib/metadata-providers/aggregate.test.ts | 39 ++++++ src/lib/metadata-providers/aggregate.ts | 16 ++- 20 files changed, 590 insertions(+), 11 deletions(-) create mode 100644 crates/libcalibre/src/queries/genres.rs create mode 100644 docs/adr/0003-canonical-book-genres.md diff --git a/crates/libcalibre/src/custom_columns.rs b/crates/libcalibre/src/custom_columns.rs index 9367ccb6..3fefb36f 100644 --- a/crates/libcalibre/src/custom_columns.rs +++ b/crates/libcalibre/src/custom_columns.rs @@ -275,6 +275,20 @@ pub(crate) fn find_by_label_and_kind( .map_err(CalibreError::from) } +pub(crate) fn find_by_label( + conn: &mut SqliteConnection, + column_label: &str, +) -> Result, CalibreError> { + use crate::schema::custom_columns::dsl::*; + + custom_columns + .filter(label.eq(column_label)) + .first::(conn) + .optional() + .map(|row| row.map(CustomColumnRow::into_column)) + .map_err(CalibreError::from) +} + // ============================================================================= // Column creation // ============================================================================= diff --git a/crates/libcalibre/src/lib.rs b/crates/libcalibre/src/lib.rs index 5aabf3ef..324a30bb 100644 --- a/crates/libcalibre/src/lib.rs +++ b/crates/libcalibre/src/lib.rs @@ -20,8 +20,8 @@ pub use custom_columns::{CustomColumn, CustomColumnKind, CustomColumnSpec, Custo pub use error::CalibreError; pub use library::{ Author as LibraryAuthor, AuthorAdd, AuthorSummary, AuthorUpdate, Book as LibraryBook, BookAdd, - BookFileInfo, BookIdentifier, BookPage, BookQuery, BookSortOrder, BookUpdate, Library, - ResolvedBookAsset, SeriesSummary, TagSummary, + BookFileInfo, BookIdentifier, BookPage, BookQuery, BookSortOrder, BookUpdate, GenreSummary, + Library, ResolvedBookAsset, SeriesSummary, TagSummary, }; pub use stats::{library_stats, LibraryStats}; pub use types::{AuthorId, BookFileId, BookId}; diff --git a/crates/libcalibre/src/library.rs b/crates/libcalibre/src/library.rs index f1bfff3f..48ff7595 100644 --- a/crates/libcalibre/src/library.rs +++ b/crates/libcalibre/src/library.rs @@ -172,6 +172,8 @@ pub struct BookQuery { pub series_id: Option, /// Only books linked to this tag. pub tag_id: Option, + /// Only books linked to this resolved Citadel genre value. + pub genre_id: Option, /// Exclude books marked read (filtered in SQL, so paging and totals stay /// correct). pub hide_read: bool, @@ -221,6 +223,13 @@ pub struct TagSummary { pub book_count: i64, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct GenreSummary { + pub id: i32, + pub name: String, + pub book_count: i64, +} + impl Library { pub fn new(db_path: ValidDbPath) -> Result { let conn = establish_connection(&db_path.database_path)?; @@ -910,6 +919,18 @@ impl Library { } else { None }; + let genre_column = match query.genre_id { + Some(genre_id) => match self.genres_column(false)? { + Some(column) => Some((column.id, genre_id)), + None => { + return Ok(BookPage { + items: Vec::new(), + total: 0, + }) + } + }, + None => None, + }; let filters = book_queries::BookPageFilters { text: query @@ -920,6 +941,7 @@ impl Library { author_id: query.author_id, series_id: query.series_id, tag_id: query.tag_id, + genre_column, hide_read_column, require_file: query.require_file, }; @@ -966,6 +988,18 @@ impl Library { } else { None }; + let genre_column = match query.genre_id { + Some(genre_id) => match self.genres_column(false)? { + Some(column) => Some((column.id, genre_id)), + None => { + return Ok(BookPage { + items: Vec::new(), + total: 0, + }) + } + }, + None => None, + }; let filters = book_queries::BookPageFilters { text: query .text @@ -975,6 +1009,7 @@ impl Library { author_id: query.author_id, series_id: query.series_id, tag_id: query.tag_id, + genre_column, hide_read_column, require_file: true, }; @@ -1043,6 +1078,89 @@ impl Library { crate::queries::tags::list_all(&mut self.conn) } + pub fn list_genres(&mut self) -> Result, CalibreError> { + let Some(column) = self.genres_column(false)? else { + return Ok(Vec::new()); + }; + crate::queries::genres::list_with_book_counts(&mut self.conn, column.id) + } + + pub fn set_book_genres( + &mut self, + book_id: BookId, + genres: Vec, + ) -> Result<(), CalibreError> { + let genres = normalize_genre_names(genres); + if genres.is_empty() && self.genres_column(false)?.is_none() { + return Ok(()); + } + let column = self + .genres_column(true)? + .expect("genre column is created when requested"); + custom_columns::set_value( + &mut self.conn, + &column, + book_id, + Some(CustomValue::TextMultiple(genres)), + )?; + book_queries::touch(&mut self.conn, book_id).map(|_| ()) + } + + pub fn add_book_genres( + &mut self, + book_id: BookId, + genres: Vec, + ) -> Result<(), CalibreError> { + let additions = normalize_genre_names(genres); + if additions.is_empty() { + return Ok(()); + } + let mut merged = match self.genres_column(false)? { + Some(column) => match custom_columns::get_value(&mut self.conn, &column, book_id)? { + Some(CustomValue::TextMultiple(values)) => values, + None => Vec::new(), + Some(_) => unreachable!("genre column compatibility is checked"), + }, + None => Vec::new(), + }; + for addition in additions { + if !merged + .iter() + .any(|existing| existing.eq_ignore_ascii_case(&addition)) + { + merged.push(addition); + } + } + self.set_book_genres(book_id, merged) + } + + fn genres_column(&mut self, create: bool) -> Result, CalibreError> { + const LABEL: &str = "citadel_genres"; + if let Some(column) = custom_columns::find_by_label(&mut self.conn, LABEL)? { + if column.kind != CustomColumnKind::Text || !column.is_multiple { + return Err(CalibreError::InvalidCustomColumn(format!( + "#{LABEL} must be a multiple-value text column" + ))); + } + return Ok(Some(column)); + } + if !create { + return Ok(None); + } + custom_columns::create( + &mut self.conn, + &CustomColumnSpec { + label: LABEL.to_string(), + name: "Genres".to_string(), + kind: CustomColumnKind::Text, + is_multiple: true, + enum_values: Vec::new(), + display: None, + }, + ) + .map(Some) + } + pub fn search_books(&mut self, query: &str) -> Result, CalibreError> { let query = query.trim(); if query.is_empty() { @@ -1080,6 +1198,21 @@ impl Library { } } +fn normalize_genre_names(genres: Vec) -> Vec { + let mut normalized = Vec::::new(); + for genre in genres { + let genre = genre.split_whitespace().collect::>().join(" "); + if !genre.is_empty() + && !normalized + .iter() + .any(|existing| existing.eq_ignore_ascii_case(&genre)) + { + normalized.push(genre); + } + } + normalized +} + // ============================================================================= // Metadata OPF generation (ported from calibre_client.rs) // ============================================================================= diff --git a/crates/libcalibre/src/queries/books.rs b/crates/libcalibre/src/queries/books.rs index b933ae65..3633063a 100644 --- a/crates/libcalibre/src/queries/books.rs +++ b/crates/libcalibre/src/queries/books.rs @@ -104,6 +104,7 @@ pub(crate) struct BookPageFilters<'a> { pub author_id: Option, pub series_id: Option, pub tag_id: Option, + pub genre_column: Option<(i32, i32)>, /// Id of the `read` bool custom column. When set, books marked read are /// excluded. pub hide_read_column: Option, @@ -183,6 +184,13 @@ fn filter_where_sql(filters: &BookPageFilters) -> String { )); } + if let Some((column_id, genre_id)) = filters.genre_column { + clauses.push(format!( + "EXISTS (SELECT 1 FROM books_custom_column_{column_id}_link gc \ + WHERE gc.book = books.id AND gc.value = {genre_id})" + )); + } + if let Some(n) = filters.hide_read_column { clauses.push(format!( "NOT EXISTS (SELECT 1 FROM custom_column_{n} cc \ diff --git a/crates/libcalibre/src/queries/genres.rs b/crates/libcalibre/src/queries/genres.rs new file mode 100644 index 00000000..4e4e8dc3 --- /dev/null +++ b/crates/libcalibre/src/queries/genres.rs @@ -0,0 +1,39 @@ +use diesel::sql_query; +use diesel::sql_types::{BigInt, Integer, Text}; +use diesel::{QueryableByName, RunQueryDsl, SqliteConnection}; + +use crate::{CalibreError, GenreSummary}; + +pub(crate) fn list_with_book_counts( + conn: &mut SqliteConnection, + column_id: i32, +) -> Result, CalibreError> { + #[derive(QueryableByName)] + struct GenreRow { + #[diesel(sql_type = Integer)] + id: i32, + #[diesel(sql_type = Text)] + name: String, + #[diesel(sql_type = BigInt)] + book_count: i64, + } + + let rows: Vec = sql_query(format!( + "SELECT genre.id AS id, genre.value AS name, COUNT(link.book) AS book_count + FROM custom_column_{column_id} genre + LEFT JOIN books_custom_column_{column_id}_link link ON link.value = genre.id + GROUP BY genre.id, genre.value + ORDER BY genre.value COLLATE NOCASE, genre.value, genre.id" + )) + .load(conn) + .map_err(CalibreError::from)?; + + Ok(rows + .into_iter() + .map(|row| GenreSummary { + id: row.id, + name: row.name, + book_count: row.book_count, + }) + .collect()) +} diff --git a/crates/libcalibre/src/queries/mod.rs b/crates/libcalibre/src/queries/mod.rs index 50692487..4db2b053 100644 --- a/crates/libcalibre/src/queries/mod.rs +++ b/crates/libcalibre/src/queries/mod.rs @@ -3,6 +3,7 @@ pub mod book_descriptions; pub mod book_files; pub mod book_identifiers; pub mod books; +pub mod genres; pub mod languages; pub mod series; pub mod tags; diff --git a/crates/libcalibre/tests/query_test.rs b/crates/libcalibre/tests/query_test.rs index 1031471b..8923d759 100644 --- a/crates/libcalibre/tests/query_test.rs +++ b/crates/libcalibre/tests/query_test.rs @@ -7,7 +7,10 @@ use common::setup_with_library; use diesel::prelude::*; use diesel::sql_query; use diesel::sql_types::{Integer, Text}; -use libcalibre::{BookAdd, BookId, BookPage, BookQuery, BookSortOrder, Library}; +use libcalibre::{ + BookAdd, BookId, BookPage, BookQuery, BookSortOrder, CustomColumnKind, CustomColumnSpec, + Library, +}; use std::collections::HashMap; fn book(title: &str, author_names: &[&str]) -> BookAdd { @@ -731,6 +734,82 @@ fn test_list_tags_empty_library() { assert!(lib.list_tags().unwrap().is_empty()); } +#[test] +fn test_genres_are_distinct_persistent_and_queryable() { + let (_temp, mut lib) = setup_with_library(); + let first = lib + .add_book(BookAdd { + tags: Some(vec!["Not a Genre".to_string()]), + ..book("First", &[]) + }) + .unwrap(); + let second = lib.add_book(book("Second", &[])).unwrap(); + + assert!(lib.list_genres().unwrap().is_empty()); + assert!(lib + .custom_columns() + .unwrap() + .iter() + .all(|column| column.label != "citadel_genres")); + + lib.set_book_genres( + first.id, + vec![" Science Fiction ".to_string(), "Mystery".to_string()], + ) + .unwrap(); + lib.set_book_genres(second.id, vec!["science fiction".to_string()]) + .unwrap(); + lib.add_book_genres(first.id, vec!["Fantasy".to_string(), "mystery".to_string()]) + .unwrap(); + + let genres = lib.list_genres().unwrap(); + let summary: Vec<_> = genres + .iter() + .map(|genre| (genre.name.as_str(), genre.book_count)) + .collect(); + assert_eq!( + summary, + [("Fantasy", 1), ("Mystery", 1), ("Science Fiction", 2)] + ); + assert!(!genres.iter().any(|genre| genre.name == "Not a Genre")); + + let science_fiction = genres + .iter() + .find(|genre| genre.name == "Science Fiction") + .unwrap(); + let page = query( + &mut lib, + BookQuery { + genre_id: Some(science_fiction.id), + limit: Some(1), + offset: 1, + ..BookQuery::default() + }, + ); + assert_eq!(titles(&page), ["Second"]); + assert_eq!(page.total, 2); +} + +#[test] +fn test_incompatible_citadel_genres_column_fails_actionably() { + let (_temp, mut lib) = setup_with_library(); + let book_id = lib.add_book(book("Book", &[])).unwrap().id; + lib.create_custom_column(CustomColumnSpec { + label: "citadel_genres".to_string(), + name: "Genres".to_string(), + kind: CustomColumnKind::Bool, + is_multiple: false, + enum_values: Vec::new(), + display: None, + }) + .unwrap(); + + let error = lib + .set_book_genres(book_id, vec!["Fantasy".to_string()]) + .unwrap_err(); + assert!(error.to_string().contains("multiple-value text column")); +} + // ============================================================================= // Series listing // ============================================================================= diff --git a/docs/adr/0003-canonical-book-genres.md b/docs/adr/0003-canonical-book-genres.md new file mode 100644 index 00000000..cabdbdf3 --- /dev/null +++ b/docs/adr/0003-canonical-book-genres.md @@ -0,0 +1,59 @@ +# ADR 0003: Store resolved book genres in a Calibre custom column + +## Status + +Accepted + +## Context + +Citadel needs a stable Genre facet that is distinct from arbitrary Calibre tags and provider subject headings. Calibre has no universal built-in genre field, while Citadel already supports Calibre-compatible custom columns. + +Provider data has different authority levels: + +- Hardcover's documented search result includes the book's top five genres in `document.genres`. Its GraphQL schema also models categorized taggings, but Citadel does not need that broader relationship graph for v1. +- MARC 21 field 655 is explicitly a genre/form term. Other 6XX fields are subject access fields and are not genres. +- Open Library exposes subjects through the APIs Citadel uses. Subjects are classification hints, not authoritative genres. + +Citadel must preserve manual choices when metadata is refreshed and keep the library portable to Calibre. OPDS is read-only and must not create schema merely because a catalog is browsed. + +## Decision + +Resolved genres live in a Calibre multi-value text custom column with label `citadel_genres` and display name `Genres`. + +- A compatible existing `#citadel_genres` text/multiple column is adopted. +- An absent column is created only when a user explicitly accepts genre suggestions. +- An incompatible existing `#citadel_genres` column produces an actionable error. Citadel does not replace or reinterpret it. +- Another genre-like custom column, including `#genres`, is never adopted implicitly. A future explicit mapping may opt into one. +- OPDS and other reads treat an absent column as an empty genre vocabulary and never create it. + +Provider results carry genre candidates separately from subjects. A candidate records its display name, provider field, and optional source vocabulary. Only these inputs are authoritative in v1: + +- Hardcover search result genres. +- MARC 655 genre/form terms, retaining subfield 2 when present as the vocabulary. + +Open Library subjects and MARC topical/subject fields remain subject suggestions and are never silently promoted. + +Citadel uses provider-native labels rather than imposing a new universal taxonomy. It trims and collapses whitespace, rejects empty values, and deduplicates case-insensitively while preserving the first display spelling and deterministic provider order. Unknown labels remain valid. Alias maps, hierarchy, confidence scoring, and cross-provider vocabulary mapping require evidence and are deferred. + +Stored genres are user-authoritative: + +- Choosing a metadata record does not itself change genres. New and existing-book flows require an explicit genre acceptance action. +- On an existing book, lookup results are suggestions. Applying other metadata does not overwrite stored genres; the user explicitly accepts or edits genre suggestions. +- A refresh never silently replaces a non-empty genre list. +- The generic custom-field editor remains an escape hatch and edits the same `#citadel_genres` value. + +The OPDS Genre vocabulary and books-by-genre queries read only resolved values from `#citadel_genres`. Provider-specific logic never enters the OPDS crate. + +## Consequences + +- Genre assignments travel with the Calibre library and are visible/editable in Calibre. +- Citadel does not need app-owned companion metadata or hidden provenance state to protect manual edits. +- Provider provenance exists on lookup candidates, while the accepted stored list intentionally represents the user's resolved choice. +- Libraries without accepted genre data show an empty Genre section. +- Future taxonomy or automatic-merge work can add explicit provenance storage through a separate migration without changing the v1 OPDS contract. + +## Sources + +- [Hardcover search API guide](https://github.com/hardcoverapp/hardcover-docs/blob/main/src/content/docs/api/guides/Searching.mdx) +- [MARC 21 field 655 — Index Term, Genre/Form](https://www.loc.gov/marc/bibliographic/concise/bd655.html) +- [Open Library API documentation](https://openlibrary.org/developers/api) diff --git a/src-tauri/src/libs/calibre/command.rs b/src-tauri/src/libs/calibre/command.rs index 01e74a2e..0a93fb4e 100644 --- a/src-tauri/src/libs/calibre/command.rs +++ b/src-tauri/src/libs/calibre/command.rs @@ -51,6 +51,20 @@ pub fn clb_cmd_set_custom_value( })? } +#[tauri::command] +#[specta::specta] +pub fn clb_cmd_add_book_genres( + state: tauri::State, + book_id: String, + genres: Vec, +) -> Result<(), String> { + state.with_library(|lib| { + let book_id = book_id.parse::().map_err(|error| error.to_string())?; + lib.add_book_genres(libcalibre::BookId::from(book_id), genres) + .map_err(|error| error.to_string()) + })? +} + #[tauri::command] #[specta::specta] pub fn clb_cmd_create_library( diff --git a/src-tauri/src/libs/calibre/query.rs b/src-tauri/src/libs/calibre/query.rs index cfb2f190..09239187 100644 --- a/src-tauri/src/libs/calibre/query.rs +++ b/src-tauri/src/libs/calibre/query.rs @@ -117,6 +117,7 @@ pub fn clb_query_books( author_id, series_id: query.series_id, tag_id: None, + genre_id: None, hide_read: query.hide_read, require_file: false, sort: query.sort.into(), diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index 597085dd..4c1d57b9 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -59,6 +59,7 @@ fn run_tauri_backend() -> std::io::Result<()> { calibre::query::clb_query_list_custom_columns, calibre::query::clb_query_get_custom_values_for_book, calibre::command::clb_cmd_set_custom_value, + calibre::command::clb_cmd_add_book_genres, // Author query and manipulation commands calibre::query::clb_query_list_all_authors, calibre::command::clb_cmd_create_authors, diff --git a/src-tauri/src/metadata/hardcover.rs b/src-tauri/src/metadata/hardcover.rs index 137b0f3e..2113fc2c 100644 --- a/src-tauri/src/metadata/hardcover.rs +++ b/src-tauri/src/metadata/hardcover.rs @@ -1,7 +1,9 @@ use serde::Deserialize; use serde_json::json; -use crate::metadata::model::{pick_preferred_isbn, BookMetadata, MetadataProvider}; +use crate::metadata::model::{ + normalize_genre_candidates, pick_preferred_isbn, BookMetadata, GenreCandidate, MetadataProvider, +}; // --------------------------------------------------------------------------- // Private GraphQL response types (serde deserialization) @@ -67,6 +69,8 @@ struct GqlBookDocument { slug: Option, #[serde(default)] contributions: Vec, + #[serde(default)] + genres: Vec, } #[derive(Deserialize)] @@ -166,6 +170,13 @@ fn document_to_metadata(doc: GqlBookDocument) -> BookMetadata { image_url: doc.image.and_then(|i| i.0), publisher: None, subjects: Vec::new(), + genre_candidates: normalize_genre_candidates(doc.genres.into_iter().map(|name| { + GenreCandidate { + name, + source: "hardcover:genre".to_string(), + vocabulary: Some("hardcover".to_string()), + } + })), language_code: None, slug: doc.slug, } @@ -224,3 +235,29 @@ pub async fn test(api_key: &str) -> Result<(), String> { Err("Invalid API response format".to_string()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn search_document_genres_are_normalized_with_provenance() { + let document: GqlBookDocument = serde_json::from_value(serde_json::json!({ + "id": 42, + "title": "Book", + "genres": [" Science Fiction ", "science fiction", "Fantasy"] + })) + .unwrap(); + let metadata = document_to_metadata(document); + let names: Vec<_> = metadata + .genre_candidates + .iter() + .map(|genre| genre.name.as_str()) + .collect(); + assert_eq!(names, ["Science Fiction", "Fantasy"]); + assert!(metadata + .genre_candidates + .iter() + .all(|genre| genre.source == "hardcover:genre")); + } +} diff --git a/src-tauri/src/metadata/marcxml.rs b/src-tauri/src/metadata/marcxml.rs index 4801291a..12dd56fa 100644 --- a/src-tauri/src/metadata/marcxml.rs +++ b/src-tauri/src/metadata/marcxml.rs @@ -2,7 +2,9 @@ use quick_xml::events::Event; use quick_xml::name::ResolveResult; use quick_xml::reader::NsReader; -use crate::metadata::model::{pick_preferred_isbn, BookMetadata, MetadataProvider}; +use crate::metadata::model::{ + normalize_genre_candidates, pick_preferred_isbn, BookMetadata, GenreCandidate, MetadataProvider, +}; /// The MARC21/slim record namespace, shared by LoC and DNB. Matching on it lets /// us tell the MARC `` apart from the SRU envelope's ``. @@ -229,6 +231,7 @@ fn map_record(rec: &RawRecord, provider: MetadataProvider) -> BookMetadata { image_url: None, publisher: publisher(rec), subjects: subjects(rec), + genre_candidates: genres(rec), language_code: language_code(rec), slug: None, } @@ -337,7 +340,7 @@ fn subjects(rec: &RawRecord) -> Vec { for field in &rec.datafields { if matches!( field.tag.as_str(), - "600" | "610" | "611" | "630" | "648" | "650" | "651" | "655" + "600" | "610" | "611" | "630" | "648" | "650" | "651" ) { if let Some(value) = field.first(b'a') { let cleaned = strip_trailing_punct(value); @@ -350,6 +353,17 @@ fn subjects(rec: &RawRecord) -> Vec { out } +fn genres(rec: &RawRecord) -> Vec { + normalize_genre_candidates(rec.datafields("655").filter_map(|field| { + let name = field.first(b'a').map(strip_trailing_punct)?; + (!name.is_empty()).then(|| GenreCandidate { + name, + source: "marc:655".to_string(), + vocabulary: field.first(b'2').map(strip_trailing_punct), + }) + })) +} + /// MARC language from the 008 fixed field (chars 35-37), falling back to 041$a. fn language_code(rec: &RawRecord) -> Option { let from_008 = rec @@ -396,7 +410,10 @@ mod tests { assert_eq!(b.release_year, Some(1996)); assert_eq!(b.publisher.as_deref(), Some("Bantam Books")); assert_eq!(b.language_code.as_deref(), Some("eng")); - assert!(b.subjects.iter().any(|s| s == "Fantasy fiction")); + assert!(!b.subjects.iter().any(|s| s == "Fantasy fiction")); + assert_eq!(b.genre_candidates.len(), 1); + assert_eq!(b.genre_candidates[0].name, "Fantasy fiction"); + assert_eq!(b.genre_candidates[0].source, "marc:655"); assert_eq!(b.identifier_label, "lccn"); assert!(b.image_url.is_none()); } diff --git a/src-tauri/src/metadata/model.rs b/src-tauri/src/metadata/model.rs index d3da6b09..10c61c3b 100644 --- a/src-tauri/src/metadata/model.rs +++ b/src-tauri/src/metadata/model.rs @@ -75,12 +75,47 @@ pub struct BookMetadata { pub publisher: Option, /// Subject headings, mapped to tag suggestions on the frontend. pub subjects: Vec, + /// Explicit provider genre/form terms. Subject headings are never placed + /// here merely because they look genre-like. + pub genre_candidates: Vec, /// MARC 3-letter language code. Parsed now; applied once CDL-2 lands. pub language_code: Option, /// Hardcover deep-link slug; `None` for other providers. pub slug: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Type)] +pub struct GenreCandidate { + pub name: String, + /// Stable provider field identity, such as `hardcover:genre` or + /// `marc:655`. + pub source: String, + /// Provider vocabulary or thesaurus when one is declared. + pub vocabulary: Option, +} + +pub fn normalize_genre_candidates( + candidates: impl IntoIterator, +) -> Vec { + let mut normalized = Vec::::new(); + for mut candidate in candidates { + candidate.name = candidate + .name + .split_whitespace() + .collect::>() + .join(" "); + if candidate.name.is_empty() + || normalized + .iter() + .any(|existing| existing.name.eq_ignore_ascii_case(&candidate.name)) + { + continue; + } + normalized.push(candidate); + } + normalized +} + #[derive(Debug, Clone, Serialize, Deserialize, Type)] pub struct ProviderStatus { pub provider: MetadataProvider, @@ -207,4 +242,28 @@ mod tests { assert!(!isbn_equivalent("", "9780553103540")); assert!(!isbn_equivalent("xyz", "9780553103540")); } + + #[test] + fn genres_are_trimmed_deduplicated_and_keep_first_provenance() { + let genres = normalize_genre_candidates([ + GenreCandidate { + name: " Science Fiction ".to_string(), + source: "hardcover:genre".to_string(), + vocabulary: Some("hardcover".to_string()), + }, + GenreCandidate { + name: "science fiction".to_string(), + source: "marc:655".to_string(), + vocabulary: Some("lcgft".to_string()), + }, + GenreCandidate { + name: " ".to_string(), + source: "marc:655".to_string(), + vocabulary: None, + }, + ]); + assert_eq!(genres.len(), 1); + assert_eq!(genres[0].name, "Science Fiction"); + assert_eq!(genres[0].source, "hardcover:genre"); + } } diff --git a/src-tauri/src/metadata/openlibrary.rs b/src-tauri/src/metadata/openlibrary.rs index 46e5e47c..5ac36f6d 100644 --- a/src-tauri/src/metadata/openlibrary.rs +++ b/src-tauri/src/metadata/openlibrary.rs @@ -146,6 +146,7 @@ fn map_data_book(book: OlDataBook, queried_isbn: &str) -> BookMetadata { image_url, publisher: book.publishers.into_iter().next().map(|p| p.name), subjects: dedupe_subjects(book.subjects.into_iter().map(|s| s.name)), + genre_candidates: Vec::new(), language_code: None, slug: None, } @@ -176,6 +177,7 @@ fn map_search_doc(doc: OlSearchDoc) -> BookMetadata { image_url, publisher: doc.publisher.into_iter().next(), subjects: dedupe_subjects(doc.subject.into_iter()), + genre_candidates: Vec::new(), language_code: None, slug: None, } diff --git a/src/bindings.ts b/src/bindings.ts index 9edde3da..1964330a 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -208,6 +208,14 @@ async clbCmdSetCustomValue(bookId: string, columnId: number, value: CustomValueD else return { status: "error", error: e as any }; } }, +async clbCmdAddBookGenres(bookId: string, genres: string[]) : Promise> { + try { + return { status: "ok", data: await TAURI_INVOKE("clb_cmd_add_book_genres", { bookId, genres }) }; +} catch (e) { + if(e instanceof Error) throw e; + else return { status: "error", error: e as any }; +} +}, async clbQueryListAllAuthors() : Promise> { try { return { status: "ok", data: await TAURI_INVOKE("clb_query_list_all_authors") }; @@ -389,6 +397,11 @@ provider_id: string; identifier_label: string; title: string; subtitle: string | * Subject headings, mapped to tag suggestions on the frontend. */ subjects: string[]; +/** + * Explicit provider genre/form terms. Subject headings are never placed + * here merely because they look genre-like. + */ +genre_candidates: GenreCandidate[]; /** * MARC 3-letter language code. Parsed now; applied once CDL-2 lands. */ @@ -467,6 +480,16 @@ export type DetectionSource = */ "default-folder" export type GeneratedOpdsCredentials = { username: string; password: string } +export type GenreCandidate = { name: string; +/** + * Stable provider field identity, such as `hardcover:genre` or + * `marc:655`. + */ +source: string; +/** + * Provider vocabulary or thesaurus when one is declared. + */ +vocabulary: string | null } /** * Book identifiers, such as ISBN, DOI, Google Books ID, etc. */ diff --git a/src/components/pages/EditBook.tsx b/src/components/pages/EditBook.tsx index 53c3894b..68bd72c0 100644 --- a/src/components/pages/EditBook.tsx +++ b/src/components/pages/EditBook.tsx @@ -82,11 +82,13 @@ const FormRow = ({ const SubjectSuggestions = ({ subjects, source, + fallbackLabel = "Suggested subjects:", onAdd, onAddAll, }: { subjects: string[]; source: string | null; + fallbackLabel?: string; onAdd: (subject: string) => void; onAddAll: () => void; }) => { @@ -94,7 +96,7 @@ const SubjectSuggestions = ({ return (
- {source ? `From ${source}:` : "Suggested subjects:"} + {source ? `From ${source}:` : fallbackLabel} {subjects.map((subject) => (
+ {suggestableGenres.length > 0 && ( + + void addGenres([genre])} + onAddAll={() => void addGenres(suggestableGenres)} + /> + + )} ( [], ); + const [lastResolvedGenres, setLastResolvedGenres] = useState([]); const [lastSubjectsSource, setLastSubjectsSource] = useState( null, ); @@ -144,6 +145,9 @@ export const useMetadataBookActions = ({ form.setFieldValue("authorList", chosen.authors); } setLastResolvedSubjects(chosen.subjects); + setLastResolvedGenres( + chosen.genre_candidates.map((candidate) => candidate.name), + ); setLastSubjectsSource( chosen.subjects.length > 0 ? getDescriptor(chosen.provider).displayName @@ -299,6 +303,7 @@ export const useMetadataBookActions = ({ (id) => getDescriptor(id).displayName, ), lastResolvedSubjects, + lastResolvedGenres, lastSubjectsSource, isbnIdentifier, fetchFromIsbn, diff --git a/src/lib/metadata-providers/aggregate.test.ts b/src/lib/metadata-providers/aggregate.test.ts index 9ef95909..24a9259b 100644 --- a/src/lib/metadata-providers/aggregate.test.ts +++ b/src/lib/metadata-providers/aggregate.test.ts @@ -18,6 +18,7 @@ const book = ( image_url: null, publisher: null, subjects: [], + genre_candidates: [], language_code: null, slug: null, ...overrides, @@ -86,6 +87,44 @@ describe("mergeResults", () => { expect(results[0]?.book.publisher).toBeNull(); }); + it("combines only provenanced genre candidates across matching records", () => { + const results = mergeResults( + [ + book("loc", { + isbn: "9780553103540", + genre_candidates: [ + { name: "Fantasy", source: "marc:655", vocabulary: "lcgft" }, + ], + }), + book("hardcover", { + isbn: "9780553103540", + genre_candidates: [ + { + name: "fantasy", + source: "hardcover:genre", + vocabulary: "hardcover", + }, + { + name: "Epic Fantasy", + source: "hardcover:genre", + vocabulary: "hardcover", + }, + ], + }), + ], + ORDER, + null, + ); + expect(results[0]?.book.genre_candidates).toEqual([ + { name: "Fantasy", source: "marc:655", vocabulary: "lcgft" }, + { + name: "Epic Fantasy", + source: "hardcover:genre", + vocabulary: "hardcover", + }, + ]); + }); + it("keeps ISBN-less records as distinct rows", () => { const results = mergeResults( [book("loc", { isbn: null }), book("dnb", { isbn: null })], diff --git a/src/lib/metadata-providers/aggregate.ts b/src/lib/metadata-providers/aggregate.ts index 434f1e73..1f972a0a 100644 --- a/src/lib/metadata-providers/aggregate.ts +++ b/src/lib/metadata-providers/aggregate.ts @@ -8,7 +8,7 @@ import type { } from "./types"; export interface AggregatedResult { - /** The winning whole record (never field-spliced, except a borrowed cover). */ + /** The winning record, plus a borrowed cover and merged genre candidates. */ book: BookMetadata; /** Display names of other providers that returned the same ISBN. */ alsoOn: string[]; @@ -19,8 +19,9 @@ export interface AggregatedResult { /** * Merge results from several providers into one list. Records sharing an ISBN * collapse to the single highest-preference provider's WHOLE record — no - * field-level merge — with the sole exception that a cover-less winner borrows - * a peer's cover. Rows matching `pinIsbn` are pinned to the top. + * field-level merge — except that a cover-less winner borrows a peer's cover + * and explicit, provenanced genre candidates are combined. Rows matching + * `pinIsbn` are pinned to the top. */ export const mergeResults = ( books: BookMetadata[], @@ -63,6 +64,15 @@ export const mergeResults = ( const peerCover = sorted.find((member) => member.image_url); if (peerCover) winner.image_url = peerCover.image_url; } + const seenGenres = new Set(); + winner.genre_candidates = sorted + .flatMap((member) => member.genre_candidates) + .filter((candidate) => { + const key = candidate.name.trim().toLocaleLowerCase(); + if (!key || seenGenres.has(key)) return false; + seenGenres.add(key); + return true; + }); const alsoOn = [ ...new Set( From f49ca9f1ab2b9fc5a5366639a1c5251f54716e94 Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sun, 2 Aug 2026 02:15:20 -0400 Subject: [PATCH 2/2] feat(opds): serve canonical genre feeds --- crates/citadel-opds/src/catalog.rs | 97 +++++++++++++++++++++++++----- crates/citadel-server/src/lib.rs | 25 +++++--- src-tauri/src/state.rs | 23 ++++--- 3 files changed, 115 insertions(+), 30 deletions(-) diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index e4d0fc8c..36910d65 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -64,8 +64,7 @@ pub struct CatalogBookQuery { impl CatalogBookQuery { /// Translate the protocol query into libcalibre's bounded query contract. - /// Genre becomes available when CDL-30 adds its canonical library filter. - pub fn into_calibre(self) -> Option { + pub fn into_calibre(self) -> BookQuery { let mut query = BookQuery { limit: Some(self.limit), offset: self.offset, @@ -82,10 +81,10 @@ impl CatalogBookQuery { CatalogFilter::Author(id) => query.author_id = Some(AuthorId(id)), CatalogFilter::Series(id) => query.series_id = Some(id), CatalogFilter::Tag(id) => query.tag_id = Some(id), - CatalogFilter::Genre(_) => return None, + CatalogFilter::Genre(id) => query.genre_id = Some(id), CatalogFilter::Search(text) => query.text = Some(text), } - Some(query) + query } } @@ -1133,13 +1132,7 @@ mod tests { let mut library = self.library.lock().unwrap(); let uuid = library.library_uuid()?; let updated = library.catalog_updated_at()?; - let page = match query.into_calibre() { - Some(query) => library.query_acquirable_books_with(query)?, - None => BookPage { - items: Vec::new(), - total: 0, - }, - }; + let page = library.query_acquirable_books_with(query.into_calibre())?; Ok((uuid, updated, page)) } @@ -1182,6 +1175,19 @@ mod tests { }) } + fn genres(&self) -> Result, CalibreError> { + self.library.lock().unwrap().list_genres().map(|items| { + items + .into_iter() + .map(|item| CatalogFacet { + id: item.id, + title: item.name, + book_count: Some(item.book_count), + }) + .collect() + }) + } + fn book_file( &self, book_id: BookId, @@ -1412,21 +1418,20 @@ mod tests { limit: 50, offset: 100, } - .into_calibre() - .unwrap(); + .into_calibre(); assert!(query.hide_read); assert_eq!(query.sort, BookSortOrder::UpdatedDesc); assert_eq!(query.limit, Some(50)); assert_eq!(query.offset, 100); - assert!(CatalogBookQuery { + let genre = CatalogBookQuery { filter: CatalogFilter::Genre(1), sort: CatalogSort::Title, limit: 50, offset: 0, } - .into_calibre() - .is_none()); + .into_calibre(); + assert_eq!(genre.genre_id, Some(1)); } #[tokio::test] @@ -1789,6 +1794,66 @@ mod tests { server.abort(); } + #[tokio::test] + async fn real_library_genre_navigation_is_distinct_from_tags() { + let (directory, mut library) = test_library(); + let source_path = directory.path().join("shared.epub"); + std::fs::write(&source_path, b"book").unwrap(); + let fantasy = library + .add_book(BookAdd { + title: "Genre Match".to_string(), + author_names: vec!["Author".to_string()], + tags: Some(vec!["Unrelated Tag".to_string()]), + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: HashMap::new(), + language: None, + file_paths: vec![source_path.clone()], + }) + .unwrap(); + library + .add_book(BookAdd { + title: "Other Book".to_string(), + author_names: vec!["Author".to_string()], + tags: Some(vec!["Fantasy".to_string()]), + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: HashMap::new(), + language: None, + file_paths: vec![source_path], + }) + .unwrap(); + library + .set_book_genres(fantasy.id, vec!["Fantasy".to_string()]) + .unwrap(); + + let (base, server) = loopback(Arc::new(LibrarySource { + library: Mutex::new(library), + })) + .await; + let navigation = reqwest::get(format!("{base}/opds/genres")).await.unwrap(); + assert_eq!(navigation.status(), StatusCode::OK); + let navigation = parsed_feed(&navigation.bytes().await.unwrap()); + assert_eq!(navigation.titles, ["Fantasy"]); + assert_eq!(navigation.subsection_hrefs.len(), 1); + + let books = reqwest::get(format!("{base}{}", navigation.subsection_hrefs[0])) + .await + .unwrap(); + assert_eq!(books.status(), StatusCode::OK); + let books = parsed_feed(&books.bytes().await.unwrap()); + assert_eq!(books.titles, ["Genre Match"]); + server.abort(); + } + #[tokio::test] async fn empty_and_unavailable_catalogs_return_valid_non_sensitive_responses() { let (base, server) = loopback(Arc::new(MemorySource { books: Vec::new() })).await; diff --git a/crates/citadel-server/src/lib.rs b/crates/citadel-server/src/lib.rs index eab92e82..16463557 100644 --- a/crates/citadel-server/src/lib.rs +++ b/crates/citadel-server/src/lib.rs @@ -66,13 +66,7 @@ impl CatalogSource for CalibreCatalogSource { let mut library = self.library.lock().expect("server library mutex poisoned"); let library_id = library.library_uuid()?; let updated_at = library.catalog_updated_at()?; - let page = match query.into_calibre() { - Some(query) => library.query_acquirable_books_with(query)?, - None => BookPage { - items: Vec::new(), - total: 0, - }, - }; + let page = library.query_acquirable_books_with(query.into_calibre())?; Ok((library_id, updated_at, page)) } @@ -126,6 +120,23 @@ impl CatalogSource for CalibreCatalogSource { }) } + fn genres(&self) -> Result, CalibreError> { + self.library + .lock() + .expect("server library mutex poisoned") + .list_genres() + .map(|genres| { + genres + .into_iter() + .map(|genre| CatalogFacet { + id: genre.id, + title: genre.name, + book_count: Some(genre.book_count), + }) + .collect() + }) + } + fn book_file(&self, book_id: BookId, format: &str) -> Result { self.library .lock() diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index c5e76ca5..79cf2ebe 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -151,13 +151,7 @@ impl CitadelState { .ok_or(CalibreError::LibraryNotInitialized)?; let library_uuid = library.library_uuid()?; let updated_at = library.catalog_updated_at()?; - let page = match query.into_calibre() { - Some(query) => library.query_acquirable_books_with(query)?, - None => BookPage { - items: Vec::new(), - total: 0, - }, - }; + let page = library.query_acquirable_books_with(query.into_calibre())?; Ok((library_uuid, updated_at, page)) } @@ -233,6 +227,21 @@ impl CatalogSource for CitadelState { }) } + fn genres(&self) -> Result, CalibreError> { + self.with_opds_library(|library| { + library.list_genres().map(|genres| { + genres + .into_iter() + .map(|genre| CatalogFacet { + id: genre.id, + title: genre.name, + book_count: Some(genre.book_count), + }) + .collect() + }) + }) + } + fn book_file(&self, book_id: BookId, format: &str) -> Result { self.resolve_book_asset(book_id, format) }