From 89f7cceaab1886f682337d1bd77b79dfb1c2afba Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Jul 2026 23:47:35 +0000 Subject: [PATCH 1/9] Add book-identifier extraction and Open Library resolve (PRD-0009 slice 1) Draft ADR-0018, the Open Library / Internet Archive read contract spawned by PRD-0009 (Q6a): identifier-gated resolution, a strictly side-effect-free Books API catalog lookup (never the import-on-miss /isbn/{isbn}.json), Read API exact-vs-similar scan availability, and the search-inside grounding-source rules including the scoped ShortBody bypass. Implement the first read-only slice: - sp42-platform: BookIdentifier (ISBN checksum-validated, OCLC, LCCN LC-normalized, OLID canonicalized with author ids rejected) and BookSource; BlockRef carries book_sources alongside cited sources. - sp42-parsoid: cite-template data-mw extraction now also reads isbn/oclc/lccn/ol params (and the cited page), independent of the url= gate, so a URL-less {{cite book}} finally yields something. - sp42-citation extract: a url-less ref with a validated identifier is skipped as BookSource (with its identifiers) instead of blending into NonUrlSource; the page report prints those identifiers. - sp42-citation openlibrary: pure build/parse pairs for the Books API catalog lookup and the Read API scan availability, partitioned so only exact-match scans can ever feed grounding. Wiring resolution into the verify path and report is the next slice. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- crates/sp42-citation/src/citation.rs | 1 + crates/sp42-citation/src/citation/extract.rs | 71 ++- .../sp42-citation/src/citation/openlibrary.rs | 459 ++++++++++++++++++ crates/sp42-citation/src/citation/page.rs | 7 + .../sp42-citation/src/citation_page_report.rs | 41 +- crates/sp42-citation/src/lib.rs | 5 + crates/sp42-mcp/src/page.rs | 1 + crates/sp42-parsoid/src/lib.rs | 196 +++++++- crates/sp42-platform/src/lib.rs | 8 +- crates/sp42-platform/src/wikitext_editor.rs | 238 ++++++++- docs/STATUS.md | 6 + ...-library-internet-archive-read-contract.md | 197 ++++++++ ...n-grounding-and-open-library-enrichment.md | 8 +- 13 files changed, 1195 insertions(+), 43 deletions(-) create mode 100644 crates/sp42-citation/src/citation/openlibrary.rs create mode 100644 docs/domains/references/adr/0018-open-library-internet-archive-read-contract.md diff --git a/crates/sp42-citation/src/citation.rs b/crates/sp42-citation/src/citation.rs index e7f970f8..03a608ec 100644 --- a/crates/sp42-citation/src/citation.rs +++ b/crates/sp42-citation/src/citation.rs @@ -20,6 +20,7 @@ pub mod citoid; pub mod concurrency; pub mod extract; pub mod locate_quote; +pub mod openlibrary; pub mod page; pub mod parsing; pub mod prompts; diff --git a/crates/sp42-citation/src/citation/extract.rs b/crates/sp42-citation/src/citation/extract.rs index 94a360cc..1a00fe5d 100644 --- a/crates/sp42-citation/src/citation/extract.rs +++ b/crates/sp42-citation/src/citation/extract.rs @@ -5,7 +5,7 @@ use crate::citation::page::PageVerificationRequest; use crate::citation::prompts::ClaimContext; use crate::citation::segment::{Sentence, segment_sentences}; use crate::citation::verify::CitationVerificationRequest; -use crate::wikitext_editor::ParsoidBlock; +use crate::wikitext_editor::{BookIdentifier, ParsoidBlock}; /// Maximum preceding in-block sentences carried as context. const MAX_PRECEDING: usize = 3; @@ -33,8 +33,13 @@ pub struct CitationUseSite { #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "snake_case")] pub enum SkippedReason { - /// The ref carries no extractable URL (book/ISBN/offline source). + /// The ref carries no extractable URL **and no book identifier** — there + /// is nothing to fetch and nothing to resolve (offline/unidentified source). NonUrlSource, + /// The ref carries no URL but does carry a validated book identifier + /// (ISBN/OCLC/LCCN/OLID); Open Library catalog resolution (PRD-0009 + /// Layer 1, ADR-0018) is the path that consumes it. + BookSource, } /// A ref that was intentionally not verified. @@ -43,6 +48,11 @@ pub struct SkippedRef { pub ref_id: String, pub reason: SkippedReason, pub block_ordinal: usize, + /// Validated book identifiers carried by the ref's cite templates, in + /// template order — populated with [`SkippedReason::BookSource`] so the + /// report can show *which* book was left unresolved. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub book_identifiers: Vec, } /// A block (or ref) that could not be processed. @@ -61,8 +71,10 @@ pub struct ExtractOutcome { } /// Extract every URL-bearing citation use-site from a page's blocks. -/// Non-URL refs are recorded in `skipped`; blocks that yield no usable claim -/// go to `failures`. Document order is preserved across the page. +/// Non-URL refs are recorded in `skipped` — with the skip reason and the +/// ref's validated book identifiers distinguishing "nothing to resolve" from +/// "a book awaiting catalog resolution" (PRD-0009). Blocks that yield no +/// usable claim go to `failures`. Document order is preserved across the page. #[must_use] pub fn extract_use_sites( blocks: &[ParsoidBlock], @@ -77,10 +89,21 @@ pub fn extract_use_sites( let sentences = segment_sentences(&block.text); for r in &block.refs { if r.sources.is_empty() { + let book_identifiers: Vec = r + .book_sources + .iter() + .flat_map(|book| book.identifiers.iter().cloned()) + .collect(); + let reason = if book_identifiers.is_empty() { + SkippedReason::NonUrlSource + } else { + SkippedReason::BookSource + }; skipped.push(SkippedRef { ref_id: r.ref_id.clone(), - reason: SkippedReason::NonUrlSource, + reason, block_ordinal: block.block_ordinal, + book_identifiers, }); continue; } @@ -205,6 +228,7 @@ mod tests { archive_urls: vec![], }) .collect(), + book_sources: vec![], ref_text: "[1]".into(), named: false, } @@ -218,6 +242,7 @@ mod tests { url: url(primary), archive_urls: archives.iter().map(|u| url(u)).collect(), }], + book_sources: vec![], ref_text: "[1]".into(), named: false, } @@ -284,6 +309,42 @@ mod tests { assert!(out.use_sites.is_empty()); assert_eq!(out.skipped.len(), 1); assert_eq!(out.skipped[0].reason, SkippedReason::NonUrlSource); + assert!(out.skipped[0].book_identifiers.is_empty()); + } + + #[test] + fn book_identifier_ref_is_skipped_with_the_refined_reason() { + // A url-less ref that carries a validated ISBN is distinguishable from + // a plain non-URL source: the report can say which book went unresolved. + let mut r = bref(10, &[]); + r.book_sources = vec![crate::wikitext_editor::BookSource { + identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: Some("42".to_string()), + }]; + let b = block("Cats purr.", vec![r]); + let out = extract_use_sites(&[b], &page()); + assert!(out.use_sites.is_empty()); + assert_eq!(out.skipped.len(), 1); + assert_eq!(out.skipped[0].reason, SkippedReason::BookSource); + assert_eq!( + out.skipped[0].book_identifiers, + vec![BookIdentifier::Isbn("9780140328721".to_string())] + ); + } + + #[test] + fn url_bearing_ref_with_book_identifiers_still_verifies_as_url() { + // url= wins: the ref produces a use-site; the book identifiers ride + // along on the BlockRef without changing the URL verification path. + let mut r = bref(10, &["https://a.test"]); + r.book_sources = vec![crate::wikitext_editor::BookSource { + identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: None, + }]; + let b = block("Cats purr.", vec![r]); + let out = extract_use_sites(&[b], &page()); + assert_eq!(out.use_sites.len(), 1); + assert!(out.skipped.is_empty()); } #[test] diff --git a/crates/sp42-citation/src/citation/openlibrary.rs b/crates/sp42-citation/src/citation/openlibrary.rs new file mode 100644 index 00000000..7eea9477 --- /dev/null +++ b/crates/sp42-citation/src/citation/openlibrary.rs @@ -0,0 +1,459 @@ +//! Open Library resolution for book citations (PRD-0009 Layer 1, ADR-0018). +//! +//! Two strictly read-only lookups, both keyed on a validated +//! [`BookIdentifier`] and both pure `build_*`/`parse_*` pairs over the +//! injected `HttpClient` transport, like the Citoid sidecar +//! ([`crate::citation::citoid`]): +//! +//! - **Catalog resolution** via the Books API +//! (`/api/books?bibkeys={SCHEME}:{value}&jscmd=data&format=json`) — does the +//! edition exist, and what does its record say. A miss is `None`, never a +//! create: the `/isbn/{isbn}.json` endpoint is documented under *Import by +//! ISBN* and can import-on-miss, so this module **must not** address it +//! (ADR-0018 Decision 2). +//! - **Scan availability** via the Read API +//! (`/api/volumes/brief/{scheme}/{value}.json`) — consulted only *after* +//! catalog resolution, and only for readable/borrowable scan discovery. +//! Items are partitioned by their `match` field: only `exact` matches may +//! ever feed grounding; a `similar` match is a different edition and is +//! surfaced as context only (ADR-0018 Decision 3). An empty `items` list +//! means "no usable scan", **not** "no catalog record". + +use std::collections::BTreeMap; +use std::sync::LazyLock; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use url::Url; + +use crate::types::{HttpMethod, HttpRequest}; +use crate::wikitext_editor::BookIdentifier; + +/// The side-effect-free Books API catalog lookup (ADR-0018 Decision 2). +pub const OPEN_LIBRARY_BOOKS_API: &str = "https://openlibrary.org/api/books"; + +/// The Read API scan-availability base (ADR-0018 Decision 3). +pub const OPEN_LIBRARY_READ_API_BASE: &str = "https://openlibrary.org/api/volumes/brief"; + +/// The Books API endpoint parsed once, for the (unreachable) fallback when a +/// built URL somehow fails to parse — keeps the builders panic-free. +static BOOKS_API_BASE: LazyLock = LazyLock::new(|| { + OPEN_LIBRARY_BOOKS_API + .parse() + .expect("static endpoint parses") +}); + +/// The Books API bibkey for an identifier, e.g. `ISBN:9780140328721`. +#[must_use] +pub fn bibkey(identifier: &BookIdentifier) -> String { + match identifier { + BookIdentifier::Isbn(value) => format!("ISBN:{value}"), + BookIdentifier::Oclc(value) => format!("OCLC:{value}"), + BookIdentifier::Lccn(value) => format!("LCCN:{value}"), + BookIdentifier::Olid(value) => format!("OLID:{value}"), + } +} + +/// The Read API path scheme for an identifier (`isbn`/`oclc`/`lccn`/`olid`). +#[must_use] +fn read_api_scheme(identifier: &BookIdentifier) -> &'static str { + match identifier { + BookIdentifier::Isbn(_) => "isbn", + BookIdentifier::Oclc(_) => "oclc", + BookIdentifier::Lccn(_) => "lccn", + BookIdentifier::Olid(_) => "olid", + } +} + +/// The normalized identifier value, without its scheme. +fn identifier_value(identifier: &BookIdentifier) -> &str { + match identifier { + BookIdentifier::Isbn(value) + | BookIdentifier::Oclc(value) + | BookIdentifier::Lccn(value) + | BookIdentifier::Olid(value) => value, + } +} + +/// Build the (read-only GET) Books API catalog lookup for one identifier. +/// +/// This is the **only** catalog-resolution request the resolve lane issues: +/// never `/isbn/{isbn}.json` (import-on-miss; ADR-0018 Decision 2). +#[must_use] +pub fn build_catalog_lookup_request(identifier: &BookIdentifier) -> HttpRequest { + let url = Url::parse_with_params( + OPEN_LIBRARY_BOOKS_API, + &[ + ("bibkeys", bibkey(identifier).as_str()), + ("jscmd", "data"), + ("format", "json"), + ], + ) + .unwrap_or_else(|_| BOOKS_API_BASE.clone()); + HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::new(), + body: Vec::new(), + } +} + +/// An Open Library edition record, from the Books API `jscmd=data` shape. +/// +/// Every field is best-effort: a thin record simply has absences, which is +/// exactly what the enrichment lane (PRD-0009 Layer 3) later proposes to fill. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct OpenLibraryEdition { + /// Edition key, e.g. `/books/OL7826547M`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub key: Option, + /// Human-facing record URL on openlibrary.org. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub record_url: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub title: Option, + /// Author display names, in record order. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub authors: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub publishers: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub publish_date: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub number_of_pages: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub isbn_10: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub isbn_13: Vec, + /// Subject display names (thinness here drives enrichment proposals). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub subjects: Vec, + /// Largest cover image URL present, if any. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cover_url: Option, +} + +/// Parse a Books API `jscmd=data` response for the identifier it was queried +/// with. `None` is a **catalog miss** ("no record found" — never a create) or +/// an unparseable body; per ADR-0018 Decision 2 no import or write follows. +#[must_use] +pub fn parse_catalog_lookup( + identifier: &BookIdentifier, + body: &[u8], +) -> Option { + let parsed: Value = serde_json::from_slice(body).ok()?; + let record = parsed.get(bibkey(identifier))?.as_object()?; + + let string_of = |key: &str| { + record + .get(key) + .and_then(Value::as_str) + .filter(|s| !s.trim().is_empty()) + .map(ToString::to_string) + }; + let names_of = |value: Option<&Value>| -> Vec { + value + .and_then(Value::as_array) + .map(|entries| { + entries + .iter() + .filter_map(|entry| entry.get("name").and_then(Value::as_str)) + .filter(|s| !s.trim().is_empty()) + .map(ToString::to_string) + .collect() + }) + .unwrap_or_default() + }; + let identifier_list = |key: &str| -> Vec { + record + .get("identifiers") + .and_then(|ids| ids.get(key)) + .and_then(Value::as_array) + .map(|values| { + values + .iter() + .filter_map(Value::as_str) + .map(ToString::to_string) + .collect() + }) + .unwrap_or_default() + }; + let cover_url = record.get("cover").and_then(|cover| { + ["large", "medium", "small"] + .iter() + .find_map(|size| cover.get(size).and_then(Value::as_str)) + .map(ToString::to_string) + }); + + Some(OpenLibraryEdition { + key: string_of("key"), + record_url: string_of("url"), + title: string_of("title"), + authors: names_of(record.get("authors")), + publishers: names_of(record.get("publishers")), + publish_date: string_of("publish_date"), + number_of_pages: record.get("number_of_pages").and_then(Value::as_u64), + isbn_10: identifier_list("isbn_10"), + isbn_13: identifier_list("isbn_13"), + subjects: names_of(record.get("subjects")), + cover_url, + }) +} + +/// Build the (read-only GET) Read API scan-availability request. +/// +/// Consulted only **after** catalog resolution (ADR-0018 Decision 3): its +/// answer is "is there a usable online scan", never "does the record exist". +#[must_use] +pub fn build_scan_availability_request(identifier: &BookIdentifier) -> HttpRequest { + let url_string = format!( + "{OPEN_LIBRARY_READ_API_BASE}/{}/{}.json", + read_api_scheme(identifier), + identifier_value(identifier) + ); + // Normalized identifier values are plain alphanumerics; fall back + // defensively to the Books API base (never panics). + let url = url_string + .parse() + .unwrap_or_else(|_| BOOKS_API_BASE.clone()); + HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::new(), + body: Vec::new(), + } +} + +/// One readable/borrowable scan the Read API reported. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ScanItem { + /// Access level as reported (`full access`, `lendable`, …). + pub status: String, + /// The archive.org item URL for the scan. + pub item_url: String, + /// Open Library edition id of the scanned volume, when reported. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ol_edition_id: Option, +} + +/// Scan availability for a resolved edition, partitioned by match quality. +/// +/// Only `exact` items may feed grounding (PRD-0009 / ADR-0018 Decision 3): a +/// `similar` item is a scan of a *different edition* of the same work and is +/// surfaced to the operator as context only — never verified against. Both +/// lists empty means the catalog record has no usable online scan, which is +/// **not** a resolution failure. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ScanAvailability { + pub exact: Vec, + pub similar: Vec, +} + +impl ScanAvailability { + /// The scan eligible to enter grounding: the first **exact** match. + /// `None` when only similar-edition scans (or none) exist — grounding + /// then degrades to `SourceUnavailable` rather than verifying a + /// page-specific citation against a different edition. + #[must_use] + pub fn groundable_scan(&self) -> Option<&ScanItem> { + self.exact.first() + } +} + +/// Parse a Read API brief response into partitioned scan availability. +/// `None` only for an unparseable body; a well-formed response with no items +/// is `Some` with both lists empty ("record may exist, no scan"). +#[must_use] +pub fn parse_scan_availability(body: &[u8]) -> Option { + let parsed: Value = serde_json::from_slice(body).ok()?; + let object = parsed.as_object()?; + let mut availability = ScanAvailability::default(); + let Some(items) = object.get("items") else { + return Some(availability); + }; + for item in items.as_array()? { + let Some(item_url) = item.get("itemURL").and_then(Value::as_str) else { + continue; + }; + let scan = ScanItem { + status: item + .get("status") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + item_url: item_url.to_string(), + ol_edition_id: item + .get("ol-edition-id") + .and_then(Value::as_str) + .map(ToString::to_string), + }; + match item.get("match").and_then(Value::as_str) { + Some("exact") => availability.exact.push(scan), + // An unlabeled match is treated as similar: never ground on it. + _ => availability.similar.push(scan), + } + } + Some(availability) +} + +#[cfg(test)] +mod tests { + use super::{ + BookIdentifier, ScanAvailability, bibkey, build_catalog_lookup_request, + build_scan_availability_request, parse_catalog_lookup, parse_scan_availability, + }; + use crate::types::HttpMethod; + + fn isbn() -> BookIdentifier { + BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn") + } + + #[test] + fn catalog_lookup_addresses_only_the_books_api() { + let request = build_catalog_lookup_request(&isbn()); + assert_eq!(request.method, HttpMethod::Get); + assert_eq!( + request.url.as_str(), + "https://openlibrary.org/api/books?bibkeys=ISBN%3A9780140328721&jscmd=data&format=json" + ); + // The side-effect-free rule (ADR-0018 Decision 2): the import-on-miss + // endpoint must never be addressed by the resolve lane. + assert!(!request.url.path().starts_with("/isbn/")); + } + + #[test] + fn bibkeys_cover_all_schemes() { + assert_eq!(bibkey(&isbn()), "ISBN:9780140328721"); + assert_eq!( + bibkey(&BookIdentifier::oclc("ocm12345678").expect("valid oclc")), + "OCLC:12345678" + ); + assert_eq!( + bibkey(&BookIdentifier::lccn("n 78-890351").expect("valid lccn")), + "LCCN:n78890351" + ); + assert_eq!( + bibkey(&BookIdentifier::olid("7030731M").expect("valid olid")), + "OLID:OL7030731M" + ); + } + + #[test] + fn parse_catalog_lookup_reads_the_data_shape() { + // Trimmed replay of a real Books API jscmd=data response shape. + let body = br#"{ + "ISBN:9780140328721": { + "url": "https://openlibrary.org/books/OL7826547M/Matilda", + "key": "/books/OL7826547M", + "title": "Matilda", + "authors": [{"url": "https://openlibrary.org/authors/OL34184A/Roald_Dahl", "name": "Roald Dahl"}], + "number_of_pages": 240, + "identifiers": { + "isbn_10": ["0140328726"], + "isbn_13": ["9780140328721"], + "openlibrary": ["OL7826547M"] + }, + "publishers": [{"name": "Puffin"}], + "publish_date": "October 1, 1988", + "subjects": [{"name": "School stories", "url": "https://openlibrary.org/subjects/school_stories"}], + "cover": { + "small": "https://covers.openlibrary.org/b/id/8314135-S.jpg", + "medium": "https://covers.openlibrary.org/b/id/8314135-M.jpg", + "large": "https://covers.openlibrary.org/b/id/8314135-L.jpg" + } + } + }"#; + let edition = parse_catalog_lookup(&isbn(), body).expect("record present"); + assert_eq!(edition.key.as_deref(), Some("/books/OL7826547M")); + assert_eq!(edition.title.as_deref(), Some("Matilda")); + assert_eq!(edition.authors, vec!["Roald Dahl"]); + assert_eq!(edition.publishers, vec!["Puffin"]); + assert_eq!(edition.publish_date.as_deref(), Some("October 1, 1988")); + assert_eq!(edition.number_of_pages, Some(240)); + assert_eq!(edition.isbn_10, vec!["0140328726"]); + assert_eq!(edition.isbn_13, vec!["9780140328721"]); + assert_eq!(edition.subjects, vec!["School stories"]); + assert_eq!( + edition.cover_url.as_deref(), + Some("https://covers.openlibrary.org/b/id/8314135-L.jpg") + ); + } + + #[test] + fn parse_catalog_lookup_miss_is_none_never_a_create() { + // A Books API miss is an empty object: "no record found", full stop. + assert!(parse_catalog_lookup(&isbn(), b"{}").is_none()); + assert!(parse_catalog_lookup(&isbn(), b"not json").is_none()); + // A response keyed by a different bibkey is also a miss for ours. + assert!(parse_catalog_lookup(&isbn(), br#"{"ISBN:0000000000": {}}"#).is_none()); + } + + #[test] + fn parse_catalog_lookup_tolerates_a_thin_record() { + let body = br#"{"ISBN:9780140328721": {"title": "Matilda"}}"#; + let edition = parse_catalog_lookup(&isbn(), body).expect("thin record still resolves"); + assert_eq!(edition.title.as_deref(), Some("Matilda")); + assert!(edition.authors.is_empty()); + assert!(edition.subjects.is_empty()); + assert_eq!(edition.number_of_pages, None); + assert_eq!(edition.cover_url, None); + } + + #[test] + fn scan_availability_request_uses_the_brief_read_api() { + let request = build_scan_availability_request(&isbn()); + assert_eq!(request.method, HttpMethod::Get); + assert_eq!( + request.url.as_str(), + "https://openlibrary.org/api/volumes/brief/isbn/9780140328721.json" + ); + let by_olid = + build_scan_availability_request(&BookIdentifier::olid("7030731M").expect("valid olid")); + assert_eq!( + by_olid.url.as_str(), + "https://openlibrary.org/api/volumes/brief/olid/OL7030731M.json" + ); + } + + #[test] + fn scan_items_partition_by_match_and_only_exact_grounds() { + let body = br#"{ + "records": {"/books/OL7826547M": {"recordURL": "https://openlibrary.org/books/OL7826547M"}}, + "items": [ + {"match": "similar", "status": "lendable", "itemURL": "https://archive.org/details/matilda00dahl_1", "ol-edition-id": "OL999M"}, + {"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl", "ol-edition-id": "OL7826547M"} + ] + }"#; + let availability = parse_scan_availability(body).expect("parses"); + assert_eq!(availability.exact.len(), 1); + assert_eq!(availability.similar.len(), 1); + let scan = availability.groundable_scan().expect("exact scan grounds"); + assert_eq!(scan.item_url, "https://archive.org/details/matilda00dahl"); + assert_eq!(scan.status, "full access"); + assert_eq!(scan.ol_edition_id.as_deref(), Some("OL7826547M")); + } + + #[test] + fn similar_only_availability_never_grounds() { + let body = br#"{"items": [{"match": "similar", "status": "lendable", "itemURL": "https://archive.org/details/other-edition"}]}"#; + let availability = parse_scan_availability(body).expect("parses"); + assert!(availability.groundable_scan().is_none()); + assert_eq!(availability.similar.len(), 1); + } + + #[test] + fn empty_items_is_no_scan_not_no_record() { + // A well-formed response with no items parses to an empty availability: + // the catalog record still exists (ADR-0018 Decision 3). + let availability = parse_scan_availability(br#"{"records": {}, "items": []}"#) + .expect("well-formed response parses"); + assert_eq!(availability, ScanAvailability::default()); + // Same for a response omitting items entirely. + assert_eq!( + parse_scan_availability(b"{}").expect("parses"), + ScanAvailability::default() + ); + // Garbage does not parse. + assert!(parse_scan_availability(b"error").is_none()); + } +} diff --git a/crates/sp42-citation/src/citation/page.rs b/crates/sp42-citation/src/citation/page.rs index 167cc6b5..b996795d 100644 --- a/crates/sp42-citation/src/citation/page.rs +++ b/crates/sp42-citation/src/citation/page.rs @@ -423,6 +423,7 @@ mod orchestrator_tests { url: url::Url::parse("https://s.test/x").unwrap(), archive_urls: vec![], }], + book_sources: vec![], ref_text: "[1]".into(), named: false, }, @@ -433,6 +434,7 @@ mod orchestrator_tests { url: url::Url::parse("https://s.test/x").unwrap(), archive_urls: vec![], }], + book_sources: vec![], ref_text: "[2]".into(), named: false, }, @@ -561,6 +563,7 @@ mod orchestrator_tests { archive_urls: vec![], }, ], + book_sources: vec![], ref_text: "[1]".into(), named: false, }], @@ -629,6 +632,7 @@ mod orchestrator_tests { url: primary.clone(), archive_urls: vec![archive.clone()], }], + book_sources: vec![], ref_text: "[1]".into(), named: false, }], @@ -719,6 +723,7 @@ reject it as too short, allowing the verification process to proceed normally." url: primary.clone(), archive_urls: vec![archive.clone()], }], + book_sources: vec![], ref_text: "[1]".into(), named: false, }], @@ -778,6 +783,7 @@ reject it as too short, allowing the verification process to proceed normally." url: primary.clone(), archive_urls: vec![archive.clone()], }], + book_sources: vec![], ref_text: "[1]".into(), named: false, }], @@ -844,6 +850,7 @@ archive is fetched, the queue will be empty and the test will fail, proving the url: url::Url::parse("https://example.com/report.pdf").unwrap(), archive_urls: vec![], }], + book_sources: vec![], ref_text: "[1]".into(), named: false, }], diff --git a/crates/sp42-citation/src/citation_page_report.rs b/crates/sp42-citation/src/citation_page_report.rs index 133af9c1..efc2e188 100644 --- a/crates/sp42-citation/src/citation_page_report.rs +++ b/crates/sp42-citation/src/citation_page_report.rs @@ -104,8 +104,19 @@ fn skipped_section(report: &PageVerificationReport) -> ReportSection { .skipped .iter() .map(|skipped| { + let identifiers = if skipped.book_identifiers.is_empty() { + String::new() + } else { + let joined = skipped + .book_identifiers + .iter() + .map(ToString::to_string) + .collect::>() + .join(","); + format!(" identifiers={joined}") + }; format!( - "ref={} reason={:?} block={}", + "ref={} reason={:?} block={}{identifiers}", skipped.ref_id, skipped.reason, skipped.block_ordinal ) }) @@ -282,19 +293,30 @@ mod tests { title: "Museum".to_string(), // Out of document order on purpose: render must sort by ordinal. findings: vec![unreachable, supported], - skipped: vec![SkippedRef { - ref_id: "cite_book".to_string(), - reason: SkippedReason::NonUrlSource, - block_ordinal: 4, - }], + skipped: vec![ + SkippedRef { + ref_id: "cite_book".to_string(), + reason: SkippedReason::NonUrlSource, + block_ordinal: 4, + book_identifiers: vec![], + }, + SkippedRef { + ref_id: "cite_isbn".to_string(), + reason: SkippedReason::BookSource, + block_ordinal: 5, + book_identifiers: vec![crate::BookIdentifier::Isbn( + "9780140328721".to_string(), + )], + }, + ], extraction_failures: vec![BlockFailure { block_ordinal: 7, reason: "no claim sentence".to_string(), }], stats: PageVerificationStats { - refs_seen: 3, + refs_seen: 4, use_sites_verified: 2, - skipped: 1, + skipped: 2, extraction_failures: 1, supported: 1, partial: 0, @@ -336,6 +358,9 @@ mod tests { assert!(text.contains("Page citation report")); assert!(text.contains("[Skipped] available=true")); assert!(text.contains("ref=cite_book reason=NonUrlSource block=4")); + assert!( + text.contains("ref=cite_isbn reason=BookSource block=5 identifiers=isbn:9780140328721") + ); assert!(text.contains("block=7 reason=no claim sentence")); let markdown = render_page_verification_markdown(&report); diff --git a/crates/sp42-citation/src/lib.rs b/crates/sp42-citation/src/lib.rs index 0c705944..32695659 100644 --- a/crates/sp42-citation/src/lib.rs +++ b/crates/sp42-citation/src/lib.rs @@ -38,6 +38,11 @@ pub use citation::extract::{ BlockFailure, CitationUseSite, ExtractOutcome, SkippedReason, SkippedRef, extract_use_sites, }; pub use citation::locate_quote::{FuzzyLocate, locate_quote, locate_quote_fuzzy}; +pub use citation::openlibrary::{ + OPEN_LIBRARY_BOOKS_API, OPEN_LIBRARY_READ_API_BASE, OpenLibraryEdition, ScanAvailability, + ScanItem, bibkey, build_catalog_lookup_request, build_scan_availability_request, + parse_catalog_lookup, parse_scan_availability, +}; pub use citation::page::{ PageVerificationReport, PageVerificationRequest, PageVerificationStats, verify_page, }; diff --git a/crates/sp42-mcp/src/page.rs b/crates/sp42-mcp/src/page.rs index 7dcdd20e..46fd0e46 100644 --- a/crates/sp42-mcp/src/page.rs +++ b/crates/sp42-mcp/src/page.rs @@ -252,6 +252,7 @@ mod tests { url: url.parse().expect("url parses"), archive_urls: vec![], }], + book_sources: vec![], ref_text: "Example".to_owned(), named: false, }], diff --git a/crates/sp42-parsoid/src/lib.rs b/crates/sp42-parsoid/src/lib.rs index 34251a51..6b5463f3 100644 --- a/crates/sp42-parsoid/src/lib.rs +++ b/crates/sp42-parsoid/src/lib.rs @@ -15,8 +15,8 @@ use parsoid::prelude::*; use parsoid::{Client, ImmutableWikicode}; use sp42_platform::{ - BlockKind, BlockRef, CitedSource, ParsoidBlock, WikiConfig, WikitextEditorError, - WikitextPageRef, + BlockKind, BlockRef, BookIdentifier, BookSource, CitedSource, ParsoidBlock, WikiConfig, + WikitextEditorError, WikitextPageRef, }; use std::collections::HashMap; @@ -99,11 +99,12 @@ pub fn blocks_from_revision( ) -> Result, WikitextEditorError> { let code = Wikicode::new(revision.html()); - // Map Reference.id() -> cited sources (primary URL + archive fallbacks), - // read structurally from cite templates and bare ExtLinks inside each reference's contents. - let mut ref_sources: HashMap> = HashMap::new(); + // Map Reference.id() -> cited sources (primary URL + archive fallbacks) and + // book identifiers (PRD-0009), read structurally from cite templates and + // bare ExtLinks inside each reference's contents. + let mut ref_sources: HashMap = HashMap::new(); for reference in code.filter_references() { - ref_sources.insert(reference.id(), urls_in_reference(&reference)); + ref_sources.insert(reference.id(), sources_in_reference(&reference)); } let mut blocks = Vec::new(); @@ -115,7 +116,7 @@ pub fn blocks_from_revision( /// Recursive walker — emit blocks, don't recurse into a block once emitted. fn walk( node: &impl WikinodeIterator, - ref_sources: &HashMap>, + ref_sources: &HashMap, blocks: &mut Vec, ordinal: &mut usize, ) { @@ -145,7 +146,7 @@ fn build_block( node: &Wikinode, kind: BlockKind, ordinal: usize, - ref_sources: &HashMap>, + ref_sources: &HashMap, ) -> ParsoidBlock { let mut text = String::new(); let mut refs = Vec::new(); @@ -173,7 +174,7 @@ fn collect_block( node: &impl WikinodeIterator, text: &mut String, refs: &mut Vec, - ref_sources: &HashMap>, + ref_sources: &HashMap, ) { for child in node.children() { if let Some(ref_link) = child.as_reference_link() { @@ -184,7 +185,8 @@ fn collect_block( refs.push(BlockRef { offset: text.len(), ref_id: ref_link.id(), - sources, + sources: sources.cited, + book_sources: sources.books, ref_text: child.text_contents(), named: ref_link.name().ok().flatten().is_some(), }); @@ -201,14 +203,26 @@ fn collect_block( } } +/// Everything one reference cites, split by kind (PRD-0009 Layer 1). +#[derive(Debug, Default, Clone)] +struct RefSources { + /// Fetchable sources: primary URL + archive fallbacks. + cited: Vec, + /// Book identifiers from cite templates, independent of any URL. + books: Vec, +} + /// Cited source extraction from a reference's contents. /// Returns one `CitedSource` per cite-template (primary url + archive fallbacks), /// plus `CitedSource` entries for *literal* bare `ExtLink`s not already present in /// templates. Preserves document order: template-derived sources first, then -/// bare-ExtLink sources. -fn urls_in_reference(reference: &Reference) -> Vec { +/// bare-ExtLink sources. Alongside, each cite template carrying a validated book +/// identifier (`isbn`/`oclc`/`lccn`/`ol`) yields one `BookSource`, whether or not +/// it also carries a URL (ADR-0018 Decision 1). +fn sources_in_reference(reference: &Reference) -> RefSources { let contents = reference.contents(); let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); // Cite-template params via data-mw. The transclusion `data-mw` is carried on @@ -226,7 +240,7 @@ fn urls_in_reference(reference: &Reference) -> Vec { if let Some(element) = span.as_node().as_element() { let attributes = element.attributes.borrow(); if let Some(data_mw) = attributes.get("data-mw") { - push_template_sources(data_mw, &mut sources, &mut seen_urls); + push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); } if let Some(about) = attributes.get("about") { transclusion_abouts.insert(about.to_string()); @@ -253,7 +267,10 @@ fn urls_in_reference(reference: &Reference) -> Vec { } } - sources + RefSources { + cited: sources, + books, + } } /// True if `node` (or any ancestor) belongs to a transclusion group, i.e. carries @@ -298,9 +315,12 @@ const URL_PARAM_ALIASES: &[&str] = &[ /// builds one `CitedSource` with that url as primary and /// `archive-url`/`archiveurl` as fallbacks. /// Appends to sources and updates `seen_urls` with all URLs (primary + archives). +/// Independently, each part carrying a validated book identifier appends one +/// `BookSource` to `books` — a template with `isbn=` but no `url=` still counts. fn push_template_sources( data_mw: &str, sources: &mut Vec, + books: &mut Vec, seen_urls: &mut std::collections::HashSet, ) { let Ok(value) = serde_json::from_str::(data_mw) else { @@ -314,6 +334,13 @@ fn push_template_sources( continue; }; + // Book identifiers are extracted before (and regardless of) the URL + // gate below: a URL-less {{cite book}} is exactly the case PRD-0009 + // exists for, and a template with both url= and isbn= yields both. + if let Some(book) = template_book_source(params) { + books.push(book); + } + // Extract the primary url. CS1/CS2 cite templates carry the fetchable // source in `url=` or, for chapter/section-level refs, in a URL-valued // alias (`chapter-url=`, `conference-url=`, …). Check `url=` first, then @@ -354,6 +381,44 @@ fn push_template_sources( } } +/// Read a template part's book identifiers (`isbn`/`oclc`/`lccn`/`ol`, with +/// their uppercase aliases) and cited page (`page`/`p`/`pages`/`pp`), each +/// validated/normalized by the `BookIdentifier` constructors. `None` when no +/// parameter yields a valid identifier — an invalid ISBN is "no identifier", +/// never a guess (ADR-0018 Decision 1). +fn template_book_source(params: &serde_json::Value) -> Option { + type Constructor = fn(&str) -> Option; + + let param = |keys: &[&str]| { + keys.iter().find_map(|key| { + params + .pointer(&format!("/{key}/wt")) + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + }) + }; + + let schemes: [(&[&str], Constructor); 4] = [ + (&["isbn", "ISBN"], BookIdentifier::isbn), + (&["oclc", "OCLC"], BookIdentifier::oclc), + (&["lccn", "LCCN"], BookIdentifier::lccn), + (&["ol", "OL"], BookIdentifier::olid), + ]; + let identifiers: Vec = schemes + .iter() + .filter_map(|(keys, constructor)| param(keys).and_then(constructor)) + .collect(); + if identifiers.is_empty() { + return None; + } + + Some(BookSource { + identifiers, + cited_page: param(&["page", "p", "pages", "pp"]).map(ToString::to_string), + }) +} + #[cfg(test)] mod tests { use super::*; @@ -600,8 +665,9 @@ mod tests { let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite web","href":"./Template:Cite_web"},"params":{"url":{"wt":"https://example.org/article"},"archive-url":{"wt":"https://web.archive.org/web/20240101/example.org/article"},"title":{"wt":"Example Article"}},"i":0}}]}"#; let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); - super::push_template_sources(data_mw, &mut sources, &mut seen_urls); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); // Should extract exactly one CitedSource with primary URL and one archive URL. assert_eq!( @@ -636,8 +702,9 @@ mod tests { let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite web","href":"./Template:Cite_web"},"params":{"url":{"wt":"https://example.org"},"archive-url":{"wt":"https://web.archive.org/web/20240101/example.org"},"archiveurl":{"wt":"https://archive.is/example.org"},"title":{"wt":"Example"}},"i":0}}]}"#; let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); - super::push_template_sources(data_mw, &mut sources, &mut seen_urls); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); assert_eq!(sources.len(), 1); assert_eq!(sources[0].archive_urls.len(), 2); @@ -659,8 +726,9 @@ mod tests { let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite","href":"./Template:Cite"},"params":{"archive-url":{"wt":"https://archive.org/web/example"},"title":{"wt":"No URL"}},"i":0}}]}"#; let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); - super::push_template_sources(data_mw, &mut sources, &mut seen_urls); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); // Should extract nothing. assert!( @@ -671,6 +739,7 @@ mod tests { seen_urls.is_empty(), "archive URL without primary should not be recorded" ); + assert!(books.is_empty(), "no identifier params, no book source"); } #[test] @@ -679,8 +748,9 @@ mod tests { // `url=`) must still yield a citable source, not be dropped as non-URL. let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite book"},"params":{"chapter-url":{"wt":"https://example.org/chapter"},"title":{"wt":"A Book"}},"i":0}}]}"#; let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); - super::push_template_sources(data_mw, &mut sources, &mut seen_urls); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); assert_eq!(sources.len(), 1, "chapter-url should produce one source"); assert_eq!(sources[0].url.as_str(), "https://example.org/chapter"); } @@ -690,9 +760,97 @@ mod tests { // When both `url=` and an alias are present, `url=` wins as primary. let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite book"},"params":{"url":{"wt":"https://example.org/main"},"chapter-url":{"wt":"https://example.org/chapter"}},"i":0}}]}"#; let mut sources = Vec::new(); + let mut books = Vec::new(); let mut seen_urls = std::collections::HashSet::new(); - super::push_template_sources(data_mw, &mut sources, &mut seen_urls); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); assert_eq!(sources.len(), 1); assert_eq!(sources[0].url.as_str(), "https://example.org/main"); } + + #[test] + fn url_less_cite_book_yields_a_book_source_not_a_cited_source() { + // The PRD-0009 case: {{cite book |isbn=… |page=…}} with no url= must + // yield a validated book identifier instead of nothing. + let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite book","href":"./Template:Cite_book"},"params":{"isbn":{"wt":"978-0-14-032872-1"},"title":{"wt":"Matilda"},"page":{"wt":"42"}},"i":0}}]}"#; + let mut sources = Vec::new(); + let mut books = Vec::new(); + let mut seen_urls = std::collections::HashSet::new(); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); + + assert!(sources.is_empty(), "no url param, no cited source"); + assert_eq!(books.len(), 1, "isbn param should yield one book source"); + assert_eq!( + books[0].identifiers, + vec![BookIdentifier::Isbn("9780140328721".to_string())] + ); + assert_eq!(books[0].cited_page.as_deref(), Some("42")); + } + + #[test] + fn cite_book_with_url_and_isbn_yields_both_kinds() { + let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite book"},"params":{"url":{"wt":"https://example.org/book"},"isbn":{"wt":"9780140328721"},"oclc":{"wt":"ocm12345678"}},"i":0}}]}"#; + let mut sources = Vec::new(); + let mut books = Vec::new(); + let mut seen_urls = std::collections::HashSet::new(); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); + + assert_eq!(sources.len(), 1); + assert_eq!(sources[0].url.as_str(), "https://example.org/book"); + assert_eq!(books.len(), 1); + assert_eq!( + books[0].identifiers, + vec![ + BookIdentifier::Isbn("9780140328721".to_string()), + BookIdentifier::Oclc("12345678".to_string()), + ] + ); + assert_eq!(books[0].cited_page, None); + } + + #[test] + fn invalid_identifier_values_yield_no_book_source() { + // A garbled ISBN (bad checksum) and an author OLID are "no identifier", + // never sent upstream (ADR-0018 Decision 1). + let data_mw = r#"{"parts":[{"template":{"target":{"wt":"cite book"},"params":{"isbn":{"wt":"978-0-14-032872-2"},"ol":{"wt":"23919A"},"title":{"wt":"Garbled"}},"i":0}}]}"#; + let mut sources = Vec::new(); + let mut books = Vec::new(); + let mut seen_urls = std::collections::HashSet::new(); + super::push_template_sources(data_mw, &mut sources, &mut books, &mut seen_urls); + assert!(sources.is_empty()); + assert!( + books.is_empty(), + "invalid values must not become identifiers" + ); + } + + #[test] + fn blocks_from_revision_carries_book_sources_onto_the_ref() { + // End-to-end through the DOM pass: a reference whose contents hold a + // url-less cite book lands on the BlockRef as a book source with empty + // cited sources (the extract layer then refines the skip reason). + let html = "\ +
\n\ +

Cats purr.[1]

\n\ +
\n\ +
  1. ↑ Dahl, Roald. Matilda. p. 42.
\n\ +
\n\ +
"; + let revision = ImmutableWikicode::new(html); + let blocks = blocks_from_revision(&revision).expect("blocks"); + let block = blocks + .iter() + .find(|b| !b.refs.is_empty()) + .expect("a block with the ref"); + let r = &block.refs[0]; + assert!( + r.sources.is_empty(), + "url-less cite book has no cited source" + ); + assert_eq!(r.book_sources.len(), 1); + assert_eq!( + r.book_sources[0].identifiers, + vec![BookIdentifier::Isbn("9780140328721".to_string())] + ); + assert_eq!(r.book_sources[0].cited_page.as_deref(), Some("42")); + } } diff --git a/crates/sp42-platform/src/lib.rs b/crates/sp42-platform/src/lib.rs index d8858087..079f0a80 100644 --- a/crates/sp42-platform/src/lib.rs +++ b/crates/sp42-platform/src/lib.rs @@ -174,8 +174,8 @@ pub use wiki_storage::{ render_wiki_storage_index_page, resolve_wiki_storage_document, save_wiki_storage_document, }; pub use wikitext_editor::{ - BlockKind, BlockRef, CitedSource, ParsoidBlock, ScriptedEditorInvocation, - ScriptedWikitextEditor, ScriptedWikitextNode, WikitextEditOutcome, WikitextEditRefusal, - WikitextEditor, WikitextEditorError, WikitextNodeDescriptor, WikitextNodeKind, - WikitextNodeLocator, WikitextPageRef, normalize_anchor_text, + BlockKind, BlockRef, BookIdentifier, BookSource, CitedSource, ParsoidBlock, + ScriptedEditorInvocation, ScriptedWikitextEditor, ScriptedWikitextNode, WikitextEditOutcome, + WikitextEditRefusal, WikitextEditor, WikitextEditorError, WikitextNodeDescriptor, + WikitextNodeKind, WikitextNodeLocator, WikitextPageRef, normalize_anchor_text, }; diff --git a/crates/sp42-platform/src/wikitext_editor.rs b/crates/sp42-platform/src/wikitext_editor.rs index 1e68d01f..653bbed9 100644 --- a/crates/sp42-platform/src/wikitext_editor.rs +++ b/crates/sp42-platform/src/wikitext_editor.rs @@ -93,6 +93,154 @@ pub struct CitedSource { pub archive_urls: Vec, } +/// A positive book identifier read from a cite template's parameters +/// (PRD-0009 Layer 1, ADR-0018 Decision 1). Values are normalized and +/// shape-validated at construction — the constructors return `None` for a +/// value that fails validation, so catalog resolution is gated on genuinely +/// positive identifiers and never guesses. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "snake_case", tag = "scheme", content = "value")] +pub enum BookIdentifier { + /// ISBN-10 or ISBN-13: hyphen/space-free, checksum-validated. + Isbn(String), + /// OCLC control number: digits, `ocm`/`ocn`/`on` prefixes stripped. + Oclc(String), + /// Library of Congress Control Number, LC-normalized (lowercase, + /// space-free, `/`-suffix dropped, hyphenated serial zero-padded to six). + Lccn(String), + /// Open Library **edition or work** id in canonical `OL…M`/`OL…W` form. + /// An author id (`OL…A`) is not a book identifier and is rejected. + Olid(String), +} + +impl BookIdentifier { + /// Normalize and checksum-validate an ISBN-10 or ISBN-13. + #[must_use] + pub fn isbn(raw: &str) -> Option { + let compact: String = raw + .chars() + .filter(|c| !c.is_whitespace() && *c != '-') + .map(|c| c.to_ascii_uppercase()) + .collect(); + let bytes = compact.as_bytes(); + let valid = match bytes.len() { + 10 => { + // Mod-11: positions weighted 10..1; last digit may be X (=10). + bytes[..9].iter().all(u8::is_ascii_digit) + && (bytes[9].is_ascii_digit() || bytes[9] == b'X') + && bytes + .iter() + .enumerate() + .map(|(i, b)| { + let value = if *b == b'X' { 10 } else { u32::from(b - b'0') }; + (10 - u32::try_from(i).unwrap_or(0)) * value + }) + .sum::() + % 11 + == 0 + } + 13 => { + // EAN-13: alternating weights 1 and 3. + bytes.iter().all(u8::is_ascii_digit) + && bytes + .iter() + .enumerate() + .map(|(i, b)| u32::from(b - b'0') * if i % 2 == 0 { 1 } else { 3 }) + .sum::() + % 10 + == 0 + } + _ => false, + }; + valid.then_some(Self::Isbn(compact)) + } + + /// Normalize an OCLC control number: trim, strip the `ocm`/`ocn`/`on` + /// record prefixes, require all-digits. + #[must_use] + pub fn oclc(raw: &str) -> Option { + let trimmed = raw.trim(); + let lower = trimmed.to_ascii_lowercase(); + let digits = ["ocm", "ocn", "on"] + .iter() + .find_map(|prefix| lower.strip_prefix(prefix)) + .unwrap_or(&lower) + .trim(); + (!digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())) + .then(|| Self::Oclc(digits.to_string())) + } + + /// Normalize an LCCN per the Library of Congress normalization rules: + /// lowercase, remove spaces, drop everything from the first `/`, and + /// zero-pad a hyphenated serial to six digits. + #[must_use] + pub fn lccn(raw: &str) -> Option { + let mut value: String = raw + .to_ascii_lowercase() + .chars() + .filter(|c| !c.is_whitespace()) + .collect(); + if let Some(slash) = value.find('/') { + value.truncate(slash); + } + if let Some((prefix, serial)) = value.split_once('-') { + if serial.is_empty() || serial.len() > 6 || !serial.bytes().all(|b| b.is_ascii_digit()) + { + return None; + } + value = format!("{prefix}{serial:0>6}"); + } + (!value.is_empty() && value.bytes().all(|b| b.is_ascii_alphanumeric())) + .then_some(Self::Lccn(value)) + } + + /// Canonicalize an Open Library id from a cite template's `ol=` param + /// (which conventionally omits the `OL` prefix): accept `(OL)?\d+[MW]` + /// case-insensitively, emit `OL…M`/`OL…W`. An author id (`…A`) yields + /// `None` — author records are not book identifiers (PRD-0009). + #[must_use] + pub fn olid(raw: &str) -> Option { + let trimmed = raw.trim(); + if !trimmed.is_ascii() { + return None; + } + let rest = if trimmed.len() >= 2 && trimmed[..2].eq_ignore_ascii_case("OL") { + &trimmed[2..] + } else { + trimmed + }; + let (digits, kind) = rest.split_at(rest.len().saturating_sub(1)); + let kind = kind.to_ascii_uppercase(); + ((kind == "M" || kind == "W") + && !digits.is_empty() + && digits.bytes().all(|b| b.is_ascii_digit())) + .then(|| Self::Olid(format!("OL{digits}{kind}"))) + } +} + +impl std::fmt::Display for BookIdentifier { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Isbn(v) => write!(f, "isbn:{v}"), + Self::Oclc(v) => write!(f, "oclc:{v}"), + Self::Lccn(v) => write!(f, "lccn:{v}"), + Self::Olid(v) => write!(f, "olid:{v}"), + } + } +} + +/// The book identifiers carried by one cite template (PRD-0009 Layer 1): +/// the positive identifiers that gate catalog resolution, plus the cited +/// page for the future search-inside pass (ADR-0018 Decision 4). +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct BookSource { + /// Validated identifiers, in template order (`isbn`, `oclc`, `lccn`, `ol`). + pub identifiers: Vec, + /// Verbatim `page=`/`pages=` value, when present. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cited_page: Option, +} + /// One inline `` within a [`ParsoidBlock`], in document order. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BlockRef { @@ -107,6 +255,11 @@ pub struct BlockRef { /// A bundled ref with multiple cite templates ⇒ multiple cited sources. /// Empty ⇒ a non-URL ref (book/ISBN) that the core records as skipped. pub sources: Vec, + /// Book identifiers from this ref's cite templates (PRD-0009 Layer 1), + /// one entry per template that carries a validated identifier. Extracted + /// independently of `sources`: a template can yield both (url + isbn), + /// either, or neither. + pub book_sources: Vec, /// Rendered text of the marker (e.g. `"[3]"`), for provenance. pub ref_text: String, /// `true` when this is a reuse of a ``. @@ -466,9 +619,9 @@ mod tests { use futures::executor::block_on; use super::{ - ScriptedWikitextEditor, ScriptedWikitextNode, WikitextEditOutcome, WikitextEditRefusal, - WikitextEditor, WikitextEditorError, WikitextNodeKind, WikitextNodeLocator, - WikitextPageRef, normalize_anchor_text, + BookIdentifier, ScriptedWikitextEditor, ScriptedWikitextNode, WikitextEditOutcome, + WikitextEditRefusal, WikitextEditor, WikitextEditorError, WikitextNodeKind, + WikitextNodeLocator, WikitextPageRef, normalize_anchor_text, }; use crate::test_fixtures::fixture_wiki_config; @@ -666,4 +819,83 @@ mod tests { serde_json::from_str(&json).expect("locator should deserialize"); assert_eq!(parsed, locator); } + + #[test] + fn isbn_normalizes_and_validates_both_lengths() { + // Hyphens/spaces stripped, checksum verified (ISBN-13, EAN mod 10). + assert_eq!( + BookIdentifier::isbn("978-0-14-032872-1"), + Some(BookIdentifier::Isbn("9780140328721".to_string())) + ); + // ISBN-10 with an X check digit (mod 11), lowercase x accepted. + assert_eq!( + BookIdentifier::isbn("0-8044-2957-x"), + Some(BookIdentifier::Isbn("080442957X".to_string())) + ); + // Bad checksum, wrong length, and garbage all yield no identifier. + assert_eq!(BookIdentifier::isbn("978-0-14-032872-2"), None); + assert_eq!(BookIdentifier::isbn("12345"), None); + assert_eq!(BookIdentifier::isbn("not-an-isbn"), None); + } + + #[test] + fn oclc_strips_record_prefixes_and_requires_digits() { + assert_eq!( + BookIdentifier::oclc(" ocm12345678 "), + Some(BookIdentifier::Oclc("12345678".to_string())) + ); + assert_eq!( + BookIdentifier::oclc("902731744"), + Some(BookIdentifier::Oclc("902731744".to_string())) + ); + assert_eq!(BookIdentifier::oclc("ocm"), None); + assert_eq!(BookIdentifier::oclc("12a45"), None); + } + + #[test] + fn lccn_applies_library_of_congress_normalization() { + // Spaces removed, lowercased, /-suffix dropped, serial zero-padded. + assert_eq!( + BookIdentifier::lccn("n 78-890351"), + Some(BookIdentifier::Lccn("n78890351".to_string())) + ); + assert_eq!( + BookIdentifier::lccn("85-2 "), + Some(BookIdentifier::Lccn("85000002".to_string())) + ); + assert_eq!( + BookIdentifier::lccn("2001-000002/AC/r932"), + Some(BookIdentifier::Lccn("2001000002".to_string())) + ); + assert_eq!(BookIdentifier::lccn(""), None); + assert_eq!(BookIdentifier::lccn("85-abc"), None); + } + + #[test] + fn olid_canonicalizes_editions_and_works_but_rejects_authors() { + // The cite `ol=` param conventionally omits the OL prefix. + assert_eq!( + BookIdentifier::olid("7030731M"), + Some(BookIdentifier::Olid("OL7030731M".to_string())) + ); + assert_eq!( + BookIdentifier::olid("ol45804W"), + Some(BookIdentifier::Olid("OL45804W".to_string())) + ); + // Author records are not book identifiers (PRD-0009). + assert_eq!(BookIdentifier::olid("OL23919A"), None); + assert_eq!(BookIdentifier::olid("M"), None); + assert_eq!(BookIdentifier::olid("12345"), None); + } + + #[test] + fn book_identifier_serializes_with_scheme_tag_and_displays_compactly() { + let id = BookIdentifier::Isbn("9780140328721".to_string()); + let json = serde_json::to_string(&id).expect("identifier should serialize"); + assert_eq!(json, r#"{"scheme":"isbn","value":"9780140328721"}"#); + assert_eq!(id.to_string(), "isbn:9780140328721"); + let parsed: BookIdentifier = + serde_json::from_str(&json).expect("identifier should deserialize"); + assert_eq!(parsed, id); + } } diff --git a/docs/STATUS.md b/docs/STATUS.md index 73dbeb5f..526de0d9 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -76,6 +76,12 @@ PWA packaging and offline installability are now effectively complete for local (text/markdown/json), defaulting to the latest revision. `frwiki`, `enwiki`, and `testwiki` are registered. The browser Citations tab that renders the same report is in review (PR #81). +- book-citation grounding (PRD-0009, ADR-0018) has its first read-only slice: + validated book identifiers (ISBN/OCLC/LCCN/OLID) are extracted from cite-template + `data-mw`, the page report distinguishes "book identifier present" from plain + non-URL refs, and a side-effect-free Open Library resolve module exists (Books + API catalog lookup + Read API exact-vs-similar scan availability); wiring + resolution into the verify path is the next slice ## Current Verification diff --git a/docs/domains/references/adr/0018-open-library-internet-archive-read-contract.md b/docs/domains/references/adr/0018-open-library-internet-archive-read-contract.md new file mode 100644 index 00000000..a99e3523 --- /dev/null +++ b/docs/domains/references/adr/0018-open-library-internet-archive-read-contract.md @@ -0,0 +1,197 @@ +# ADR-0018: Open Library / Internet Archive read contract + +**Status:** Proposed +**Date:** 2026-07-07 +**Author:** Luis Villa (drafted by Claude Code) + +Spawned by PRD-0009 (book-citation grounding and Open Library enrichment), +resolved Q6(a): the **read-contract** ADR. It fixes how SP42 turns a book +citation's identifier into an Open Library catalog record, how it decides +whether an Internet Archive scan is usable for grounding, and how a +search-inside snippet enters the existing verification pipeline. The **write** +side (enriching an Open Library record) is the separate apply-contract ADR +PRD-0009 also names; nothing here authorizes a write. + +## Context + +The article-level verifier (ADR-0011) records a book citation as +`skipped { NonUrlSource }` — a `{{cite book}}` with an ISBN but no `url=` +yields nothing fetchable, so it gets no verdict and no affordance. PRD-0009 +turns that dead end into resolution (Layer 1) and full-text grounding +(Layer 2), both read-only. Three properties of the external surfaces force a +recorded contract rather than ad-hoc client code: + +1. **One documented Open Library "read" path has a write side-effect.** The + `/isbn/{isbn}.json` endpoint is documented under *Import by ISBN* and can + attempt an import or stage metadata when the ISBN is not in the catalog. A + resolver that "just GETs the obvious URL" silently violates the read-only, + no-auth promise of Layers 1–2 and the PRD's enrich-existing-only boundary. +2. **The Read API conflates two kinds of emptiness and two kinds of match.** + An empty `items` list does not mean "no catalog record" — it means "no + usable online scan." And a returned scan may be only a `similar` match + (another edition of the same work), which must never ground a + page-specific citation against the wrong edition. +3. **IA search-inside snippets are shorter than the generic body floor.** The + fetched-page usability gate short-circuits bodies below a generic length + floor (`ShortBody`) before verification. A verbatim OCR snippet is a valid + grounding body at well below that floor, so routing it through the generic + gate would misclassify real book evidence as unusable. + +## Decision + +### 1. Resolution is gated on a positive, validated identifier + +- The only inputs to catalog resolution are identifiers read **from the cite + template's own parameters** via Parsoid `data-mw` — `isbn`, `oclc`, `lccn`, + and `ol` (the Open Library id param) — the same structured-extraction path + ADR-0011 uses for `url=`/`archive-url=`. Citoid is not in this path (it is a + URL→metadata sidecar; a URL-less book ref never reaches it). +- Extracted values are **normalized and shape-validated at construction**: + ISBNs are stripped of hyphens/spaces and must pass the ISBN-10 (mod 11) or + ISBN-13 (mod 10) checksum; LCCNs get the Library of Congress normalization + (lowercase, space-free, `/`-suffix dropped, serial zero-padded); OCLC + numbers are digits with the `ocm`/`ocn`/`on` prefixes stripped; `ol` values + are canonicalized to `OL…M` / `OL…W` form. An **author** Open Library id + (`OL…A`) is *not* a book identifier and is rejected — consistent with + PRD-0009's author-context exclusion. +- A value that fails validation yields **no identifier** — a garbled ISBN is + treated as "no identifier to resolve," not sent upstream to see what + happens. **No title/author guessing, ever** (PRD-0009 Alternatives): a ref + with no valid identifier stays skipped, with the skip reason distinguishing + "no identifier" from "carries a book identifier." + +### 2. Catalog resolution: the Books API only, side-effect-free + +- The catalog lookup is + `GET https://openlibrary.org/api/books?bibkeys={SCHEME}:{value}&jscmd=data&format=json` + with the bibkey scheme matching the trusted identifier (`ISBN:`, `OCLC:`, + `LCCN:`, `OLID:`). An explicit OLID may equivalently be read directly as + `GET /books/OL…M.json`; both are pure reads. +- The resolver **must not** call `/isbn/{isbn}.json` (import-on-miss; Context + §1). A lookup miss is **"no record found," never a create** — the empty + `{}` Books API response maps to a `None` resolution, and no import, + staging, or retry-with-write endpoint follows. Tests assert the built + requests never address the import path. +- The parsed record keeps the fields the later layers need — edition key and + record URL, title, authors, publishers, publish date, page count, + ISBN-10/13, subjects, cover — parsed defensively (any missing field is + simply absent; a malformed body is a failed parse, not a panic). + +### 3. Scan availability: the Read API, exact matches only, after resolution + +- Scan availability is a **separate, subsequent** question from catalog + existence: + `GET https://openlibrary.org/api/volumes/brief/{scheme}/{value}.json` + (schemes `isbn`, `oclc`, `lccn`, `olid`). It is consulted only for + readable/borrowable scan discovery, never as the catalog lookup. +- Returned items are partitioned by their `match` field. Only **`exact`** + matches are eligible to feed grounding (Layer 2). A `similar` match (a scan + of a *different edition* of the same work) may be surfaced to the operator + as context but never becomes the cited book's source body; a similar-only + response degrades grounding to `SourceUnavailable` with a refined + "similar edition only" reason. +- An **empty `items` list is not a resolution failure**: the catalog record + (from Decision 2) still exists and remains enrichable; only grounding + degrades to `SourceUnavailable`. The two APIs' outcomes are therefore kept + as separate values, never collapsed into one boolean. + +### 4. Grounding source: the search-inside snippet, with a bounded gate bypass + +- A book is **groundable** when the resolved edition carries an `ocaid` and + the archive.org item is a text item whose full-text index allows + search-inside to run (PRD-0009 resolved Q4). The scan's OCR is never + downloaded whole; SP42 queries the item's search-inside endpoint (the + BookReader full-text search, reached via the item's metadata-designated + server) and receives verbatim OCR snippets with page numbers — which + typically works even for lending-restricted scans, so grounding needs no + borrow. +- The returned snippet is a **book-snippet source body**: it feeds the + existing per-use-site verdict contract unchanged (ADR-0006/0007). The + anti-fabrication gate is untouched — `supported`/`partial` still require a + passage verbatim-located in the fetched bytes (here, the snippet), and the + snippet is content-hashed and stored under the ADR-0009 snapshot/replay + discipline. +- The book-snippet body **bypasses only the generic `ShortBody` page-body + floor** (Context §3), via a provenance-checked wrapper: the bypass applies + exclusively to bodies produced by the search-inside path, never to + arbitrary short fetched web pages, which continue to short-circuit as + before. +- Search order for a page-specific citation: the cited `|page=` first, then a + whole-book fallback; the finding records the **scanned** page the passage + was found on (PRD-0009 resolved Q5). +- Two empty outcomes stay distinct (ADR-0007 discipline): **no usable body** + (no `ocaid` / not a text item / no index) → `SourceUnavailable` + (`unreachable`/`unusable` split per ADR-0011 Decision 7); an **indexed scan + whose search returns zero snippets** after both passes → `not_supported`. + +### 5. House transport and test discipline apply unchanged + +- Every request in this contract is a pure `build_*` → `HttpRequest` / + `parse_*(bytes)` pair executed over the injected `HttpClient` trait, like + Citoid (`citation/citoid.rs`). Server-side execution rides the guarded + `sp42-fetch` source face (ADR-0015): openlibrary.org and archive.org hosts + are reachable via attacker-influenced citation content, so they are treated + as untrusted fetch targets (SSRF floor, caps, retry), not first-party APIs. +- All tests replay recorded fixtures; no live network (ADR-0009). Open + Library and IA being third parties, any endpoint drift surfaces as a + fixture update, not a hidden behavior change. +- These endpoints are best-effort context like Citoid: any failure degrades + (no resolution / no grounding) and never blocks the URL-citation flow. + +## Consequences + +- Book refs stop being a uniform `NonUrlSource` dead end: the extractor can + distinguish "no identifier" from "identifier present," and a resolved + record gives the report something honest to print even before grounding + ships. +- The side-effect-free rule is enforceable by test (assert the only endpoints + addressed are `/api/books`, `/books/…json`, `/works/…json`, and + `/api/volumes/brief/…`), so the import-on-miss hazard cannot regress + silently. +- Keeping catalog resolution and scan availability as separate values commits + the report/UI to representing "record exists, no scan" — mildly more + plumbing than one boolean, deliberately. +- The `ShortBody` bypass is scoped to a provenance-checked body type, so the + generic usability gate keeps its meaning for web pages. +- Two more external hosts join the fetch surface; both sit behind the guarded + edge and the existing fixture-replay pattern. + +## Alternatives considered + +- **Resolve via `/isbn/{isbn}.json` (the "obvious" URL).** Rejected: its + import-on-miss behavior is a write side-effect in a read lane (Context §1). +- **Use the Read API as the catalog lookup.** Rejected: it answers + availability, not existence; an empty `items` would be misread as "no + record" and would kill enrichment for every unscanned book. +- **Ground against `similar` matches when no exact scan exists.** Rejected: + verifying a page-specific citation against a different edition is the + classic wrong-edition failure; honesty (`SourceUnavailable` + context for + the operator) beats a plausible-but-wrong verdict. +- **Lower the generic `ShortBody` floor instead of a scoped bypass.** + Rejected: it would weaken the usability gate for every web fetch to + accommodate one provenance-known source type. +- **Title/author fuzzy resolution.** Rejected for the MVP (PRD-0009 + Alternatives); revisit only with a confidence threshold if identifier + coverage proves too thin. + +## Out of scope / non-goals + +- **Any Open Library write** — enrichment proposals and the apply mechanism + are PRD-0009 Layer 3 and its separate apply-contract ADR. +- **archive.org item-metadata editing** (IA-S3/JSON-Patch) — excluded by + PRD-0009's scope boundary. +- **Wikidata as enrichment context** — PRD-0009 Layer 1 allows pulling a + linked book-level Wikidata item as context; that read rides the Wikidata + entity machinery (ADR-0016) and adds no contract here. +- **Queue-scale book discovery** — this contract serves the revision under + review (ADR-0011 footprint), not cross-article scanning. + +## References + +- PRD-0009 (book-citation grounding and Open Library enrichment; resolved + Q1–Q6), ADR-0011 (article-level verification / skip semantics), ADR-0007 + (grounding gate), ADR-0009 (snapshot/replay), ADR-0015 (guarded fetch + edge), ADR-0016 (Wikidata entity read). +- External: Open Library Books API (`/api/books`), Read API + (`/api/volumes/brief`), *Import by ISBN* (`/isbn/{isbn}.json` — avoided), + Internet Archive BookReader search-inside. diff --git a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md index a101f9c2..7073b164 100644 --- a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md +++ b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md @@ -6,10 +6,10 @@ **State:** Draft **Discussion:** design conversation 2026-07-01 (Internet Archive editable book metadata → SP42 integration); no tracking issue yet. -**Spawned ADRs:** none yet. If this PRD is accepted, expect at least one thin ADR -for the Internet Archive / Open Library read contract (resolve + full-text -grounding) and, if the enrichment lane ships, a second reusing ADR-0010's -propose/confirm apply discipline against a non-wiki target. +**Spawned ADRs:** ADR-0018 (Open Library / Internet Archive read contract: +resolve + full-text grounding). If the enrichment lane ships, a second ADR +reusing ADR-0010's propose/confirm apply discipline against a non-wiki target +is still expected. ## Scope boundary From 44c172be390aed77e8a9ea39b5e0943a0a6fed87 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:11:36 +0000 Subject: [PATCH 2/9] Wire Open Library resolution into verify-page (PRD-0009 slice 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page orchestrator now resolves every skipped book ref through the side-effect-free Open Library lane (ADR-0018) and the report shows what the catalog said, so a book citation prints a resolved record instead of a bare identifier. - sp42-citation openlibrary: resolve_book_source executor — tries a ref's identifiers in template order against the Books API, then checks Read API scan availability on the first hit. A clean miss is NotFound (never a create), a transport failure is LookupFailed (existence unknown), and an availability failure degrades to scan-unknown without losing the catalog hit. A recording-client test asserts the whole lane addresses only /api/books and /api/volumes/brief, never the import-on-miss /isbn/{isbn}.json path. - verify_page: a bounded (<=3, REST politeness) resolution pass over the skipped refs' book sources; failures stay inside the books lane and never fail the page. Report gains book_resolutions plus books_resolved/books_not_found/book_lookups_failed stats, all serde-default so older reports still deserialize. - Renderer: a Books section (resolved record with title/authors/record URL and an exact/similar_only/none/unknown scan label; distinct not_found vs lookup_failed lines) and a books lead tally. - SkippedRef now carries its BookSource structures (identifiers + cited page) instead of a flattened identifier list, so resolution keeps per-template identifier order and the cited page for Layer 2. Search-inside grounding (Layer 2) is the next slice. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- Cargo.lock | 1 + crates/sp42-citation/Cargo.toml | 1 + crates/sp42-citation/src/citation/extract.rs | 45 ++- .../sp42-citation/src/citation/openlibrary.rs | 304 +++++++++++++++++- crates/sp42-citation/src/citation/page.rs | 222 ++++++++++++- .../sp42-citation/src/citation_page_report.rs | 189 ++++++++++- crates/sp42-citation/src/lib.rs | 7 +- docs/STATUS.md | 10 +- 8 files changed, 739 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a61517bf..dae12059 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3271,6 +3271,7 @@ dependencies = [ name = "sp42-citation" version = "0.1.0" dependencies = [ + "async-trait", "futures", "percent-encoding", "proptest", diff --git a/crates/sp42-citation/Cargo.toml b/crates/sp42-citation/Cargo.toml index 2823601d..4527acac 100644 --- a/crates/sp42-citation/Cargo.toml +++ b/crates/sp42-citation/Cargo.toml @@ -23,4 +23,5 @@ sp42-reporting = { path = "../sp42-reporting" } sp42-types = { path = "../sp42-types" } [dev-dependencies] +async-trait.workspace = true proptest = "1.9.0" diff --git a/crates/sp42-citation/src/citation/extract.rs b/crates/sp42-citation/src/citation/extract.rs index 1a00fe5d..80fa6087 100644 --- a/crates/sp42-citation/src/citation/extract.rs +++ b/crates/sp42-citation/src/citation/extract.rs @@ -5,7 +5,7 @@ use crate::citation::page::PageVerificationRequest; use crate::citation::prompts::ClaimContext; use crate::citation::segment::{Sentence, segment_sentences}; use crate::citation::verify::CitationVerificationRequest; -use crate::wikitext_editor::{BookIdentifier, ParsoidBlock}; +use crate::wikitext_editor::{BookIdentifier, BookSource, ParsoidBlock}; /// Maximum preceding in-block sentences carried as context. const MAX_PRECEDING: usize = 3; @@ -48,11 +48,24 @@ pub struct SkippedRef { pub ref_id: String, pub reason: SkippedReason, pub block_ordinal: usize, - /// Validated book identifiers carried by the ref's cite templates, in - /// template order — populated with [`SkippedReason::BookSource`] so the - /// report can show *which* book was left unresolved. + /// The ref's book sources (validated identifiers + cited page, one per + /// cite template) — populated with [`SkippedReason::BookSource`]. The page + /// orchestrator resolves these against Open Library (PRD-0009 Layer 1) + /// and the report shows *which* book the ref names. #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub book_identifiers: Vec, + pub book_sources: Vec, +} + +impl SkippedRef { + /// All validated identifiers across the ref's book sources, in template + /// order — the flattened view the skipped-section renderer prints. + #[must_use] + pub fn book_identifiers(&self) -> Vec<&BookIdentifier> { + self.book_sources + .iter() + .flat_map(|book| book.identifiers.iter()) + .collect() + } } /// A block (or ref) that could not be processed. @@ -89,12 +102,10 @@ pub fn extract_use_sites( let sentences = segment_sentences(&block.text); for r in &block.refs { if r.sources.is_empty() { - let book_identifiers: Vec = r - .book_sources - .iter() - .flat_map(|book| book.identifiers.iter().cloned()) - .collect(); - let reason = if book_identifiers.is_empty() { + // Book sources carry at least one validated identifier by + // construction, so their presence is the "book identifier + // present" signal. + let reason = if r.book_sources.is_empty() { SkippedReason::NonUrlSource } else { SkippedReason::BookSource @@ -103,7 +114,7 @@ pub fn extract_use_sites( ref_id: r.ref_id.clone(), reason, block_ordinal: block.block_ordinal, - book_identifiers, + book_sources: r.book_sources.clone(), }); continue; } @@ -309,7 +320,7 @@ mod tests { assert!(out.use_sites.is_empty()); assert_eq!(out.skipped.len(), 1); assert_eq!(out.skipped[0].reason, SkippedReason::NonUrlSource); - assert!(out.skipped[0].book_identifiers.is_empty()); + assert!(out.skipped[0].book_sources.is_empty()); } #[test] @@ -327,8 +338,12 @@ mod tests { assert_eq!(out.skipped.len(), 1); assert_eq!(out.skipped[0].reason, SkippedReason::BookSource); assert_eq!( - out.skipped[0].book_identifiers, - vec![BookIdentifier::Isbn("9780140328721".to_string())] + out.skipped[0].book_identifiers(), + vec![&BookIdentifier::Isbn("9780140328721".to_string())] + ); + assert_eq!( + out.skipped[0].book_sources[0].cited_page.as_deref(), + Some("42") ); } diff --git a/crates/sp42-citation/src/citation/openlibrary.rs b/crates/sp42-citation/src/citation/openlibrary.rs index 7eea9477..64a82e7f 100644 --- a/crates/sp42-citation/src/citation/openlibrary.rs +++ b/crates/sp42-citation/src/citation/openlibrary.rs @@ -27,7 +27,8 @@ use serde_json::Value; use url::Url; use crate::types::{HttpMethod, HttpRequest}; -use crate::wikitext_editor::BookIdentifier; +use crate::wikitext_editor::{BookIdentifier, BookSource}; +use sp42_types::HttpClient; /// The side-effect-free Books API catalog lookup (ADR-0018 Decision 2). pub const OPEN_LIBRARY_BOOKS_API: &str = "https://openlibrary.org/api/books"; @@ -296,18 +297,184 @@ pub fn parse_scan_availability(body: &[u8]) -> Option { Some(availability) } +/// Outcome of resolving one book source against the Open Library catalog. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case", tag = "kind")] +pub enum BookResolutionOutcome { + /// A catalog record was found under one of the ref's identifiers. + Resolved { + /// The identifier that keyed the successful catalog lookup. + identifier: BookIdentifier, + edition: Box, + /// Scan availability for the resolved identifier. `None` means the + /// availability lookup itself failed (unknown); an empty + /// [`ScanAvailability`] means "record exists, no usable scan". + #[serde(default, skip_serializing_if = "Option::is_none")] + scan: Option, + }, + /// Every identifier was looked up cleanly and none has a catalog record. + /// Never a create (ADR-0018 Decision 2). + NotFound, + /// A lookup failed in transport before a clean answer; whether a record + /// exists is unknown, which is different from `NotFound`. + LookupFailed { message: String }, +} + +/// One book citation's Layer-1 resolution, with page provenance +/// (PRD-0009): which ref named the book, what identifiers it carried, and +/// what the catalog said. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BookResolution { + /// The skipped ref this resolution belongs to. + pub ref_id: String, + pub block_ordinal: usize, + /// The identifiers the ref carried, in template order (what was tried). + pub identifiers: Vec, + /// Verbatim cited page from the template, for the future search-inside + /// pass (ADR-0018 Decision 4). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cited_page: Option, + pub outcome: BookResolutionOutcome, +} + +/// Resolve one book source: try its identifiers in template order against the +/// Books API; on the first catalog hit, additionally look up scan +/// availability. Strictly read-only and best-effort — the only endpoints +/// addressed are the two builders above, a miss issues no import or retry +/// against a write path, and any transport failure degrades to +/// [`BookResolutionOutcome::LookupFailed`] rather than an error. +pub async fn resolve_book_source(client: &C, book: &BookSource) -> BookResolutionOutcome +where + C: HttpClient + ?Sized, +{ + let mut failure: Option = None; + for identifier in &book.identifiers { + let response = match client + .execute(build_catalog_lookup_request(identifier)) + .await + { + Ok(response) => response, + Err(error) => { + failure.get_or_insert_with(|| error.to_string()); + continue; + } + }; + if !(200..300).contains(&response.status) { + failure.get_or_insert_with(|| format!("catalog lookup returned {}", response.status)); + continue; + } + // An unparseable 2xx body is a failure, not a miss: "NotFound" is + // reserved for a clean catalog answer with no record under our bibkey. + if serde_json::from_slice::(&response.body).is_err() { + failure.get_or_insert_with(|| "catalog lookup body was not JSON".to_string()); + continue; + } + if let Some(edition) = parse_catalog_lookup(identifier, &response.body) { + let scan = fetch_scan_availability(client, identifier).await; + return BookResolutionOutcome::Resolved { + identifier: identifier.clone(), + edition: Box::new(edition), + scan, + }; + } + // A clean miss under this identifier — try the next one. + } + match failure { + Some(message) => BookResolutionOutcome::LookupFailed { message }, + None => BookResolutionOutcome::NotFound, + } +} + +/// Best-effort Read API availability check; any failure yields `None` +/// (availability unknown — never blocks or fails the resolution). +async fn fetch_scan_availability( + client: &C, + identifier: &BookIdentifier, +) -> Option +where + C: HttpClient + ?Sized, +{ + let response = client + .execute(build_scan_availability_request(identifier)) + .await + .ok()?; + if !(200..300).contains(&response.status) { + return None; + } + parse_scan_availability(&response.body) +} + #[cfg(test)] mod tests { use super::{ - BookIdentifier, ScanAvailability, bibkey, build_catalog_lookup_request, - build_scan_availability_request, parse_catalog_lookup, parse_scan_availability, + BookIdentifier, BookResolutionOutcome, BookSource, HttpClient, ScanAvailability, bibkey, + build_catalog_lookup_request, build_scan_availability_request, parse_catalog_lookup, + parse_scan_availability, resolve_book_source, }; - use crate::types::HttpMethod; + use crate::errors::HttpClientError; + use crate::types::{HttpMethod, HttpRequest, HttpResponse}; + use futures::executor::block_on; + use sp42_types::StubHttpClient; + use std::collections::VecDeque; + use std::sync::Mutex; fn isbn() -> BookIdentifier { BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn") } + fn book(identifiers: Vec) -> BookSource { + BookSource { + identifiers, + cited_page: Some("42".to_string()), + } + } + + fn ok(body: &str) -> HttpResponse { + HttpResponse { + status: 200, + headers: std::collections::BTreeMap::new(), + body: body.as_bytes().to_vec(), + } + } + + const CATALOG_HIT: &str = + r#"{"ISBN:9780140328721": {"key": "/books/OL7826547M", "title": "Matilda"}}"#; + const READ_API_EXACT: &str = r#"{"items": [{"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl"}]}"#; + + /// A stub that also records every requested URL, so tests can assert the + /// resolve lane addresses only the two read endpoints (ADR-0018 Decision 2). + struct RecordingHttpClient { + urls: Mutex>, + responses: Mutex>>, + } + + impl RecordingHttpClient { + fn new(responses: I) -> Self + where + I: IntoIterator>, + { + Self { + urls: Mutex::new(Vec::new()), + responses: Mutex::new(responses.into_iter().collect()), + } + } + } + + #[async_trait::async_trait] + impl HttpClient for RecordingHttpClient { + async fn execute(&self, request: HttpRequest) -> Result { + self.urls + .lock() + .expect("urls lock") + .push(request.url.to_string()); + self.responses + .lock() + .expect("responses lock") + .pop_front() + .expect("a queued response for every request") + } + } + #[test] fn catalog_lookup_addresses_only_the_books_api() { let request = build_catalog_lookup_request(&isbn()); @@ -456,4 +623,133 @@ mod tests { // Garbage does not parse. assert!(parse_scan_availability(b"error").is_none()); } + + #[test] + fn resolve_hit_returns_edition_and_scan() { + let client = StubHttpClient::new(vec![Ok(ok(CATALOG_HIT)), Ok(ok(READ_API_EXACT))]); + let outcome = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + let BookResolutionOutcome::Resolved { + identifier, + edition, + scan, + } = outcome + else { + panic!("expected Resolved, got {outcome:?}"); + }; + assert_eq!(identifier, isbn()); + assert_eq!(edition.title.as_deref(), Some("Matilda")); + let scan = scan.expect("availability was looked up"); + assert_eq!(scan.exact.len(), 1); + assert!(scan.groundable_scan().is_some()); + } + + #[test] + fn resolve_tries_identifiers_in_order_until_a_hit() { + // ISBN misses cleanly; OCLC hits. The resolved identifier records + // which key actually found the record. + let oclc = BookIdentifier::oclc("12345678").expect("valid oclc"); + let oclc_hit = r#"{"OCLC:12345678": {"title": "Matilda"}}"#; + let client = StubHttpClient::new(vec![ + Ok(ok("{}")), + Ok(ok(oclc_hit)), + Ok(ok(r#"{"items": []}"#)), + ]); + let outcome = block_on(resolve_book_source( + &client, + &book(vec![isbn(), oclc.clone()]), + )); + let BookResolutionOutcome::Resolved { + identifier, scan, .. + } = outcome + else { + panic!("expected Resolved, got {outcome:?}"); + }; + assert_eq!(identifier, oclc); + // Empty items = record exists, no usable scan (not unknown). + assert_eq!(scan, Some(ScanAvailability::default())); + } + + #[test] + fn resolve_clean_miss_is_not_found() { + let client = StubHttpClient::new(vec![Ok(ok("{}"))]); + let outcome = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + assert_eq!(outcome, BookResolutionOutcome::NotFound); + } + + #[test] + fn resolve_transport_failure_is_lookup_failed_not_not_found() { + // A failed lookup means "unknown", which must not masquerade as a + // clean catalog miss. + let client = StubHttpClient::new(vec![Err(HttpClientError::InvalidResponse { + message: "connection reset".to_string(), + })]); + let outcome = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + let BookResolutionOutcome::LookupFailed { message } = outcome else { + panic!("expected LookupFailed, got {outcome:?}"); + }; + assert!(message.contains("connection reset")); + } + + #[test] + fn resolve_non_2xx_catalog_answer_is_lookup_failed() { + let client = StubHttpClient::new(vec![Ok(HttpResponse { + status: 503, + headers: std::collections::BTreeMap::new(), + body: Vec::new(), + })]); + let outcome = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + let BookResolutionOutcome::LookupFailed { message } = outcome else { + panic!("expected LookupFailed, got {outcome:?}"); + }; + assert!(message.contains("503")); + } + + #[test] + fn resolve_scan_availability_failure_degrades_to_unknown() { + // The catalog hit stands even when the availability check fails; the + // scan is simply unknown (None), not absent (Some(empty)). + let client = StubHttpClient::new(vec![ + Ok(ok(CATALOG_HIT)), + Err(HttpClientError::InvalidResponse { + message: "read api down".to_string(), + }), + ]); + let outcome = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + let BookResolutionOutcome::Resolved { scan, .. } = outcome else { + panic!("expected Resolved, got {outcome:?}"); + }; + assert_eq!(scan, None); + } + + #[test] + fn resolve_addresses_only_the_read_endpoints() { + // The PRD-0009 DoD assertion: a full resolve (hit) and a miss both + // address only /api/books and /api/volumes/brief — never the + // import-on-miss /isbn/{isbn}.json path, and no write follows a miss. + let client = RecordingHttpClient::new(vec![ + Ok(ok("{}")), // isbn: clean miss + Ok(ok(CATALOG_HIT)), // (second book) isbn: hit + Ok(ok(READ_API_EXACT)), + ]); + let miss = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + assert_eq!(miss, BookResolutionOutcome::NotFound); + let hit = block_on(resolve_book_source(&client, &book(vec![isbn()]))); + assert!(matches!(hit, BookResolutionOutcome::Resolved { .. })); + + let urls = client.urls.lock().expect("urls lock"); + assert_eq!(urls.len(), 3, "miss issues exactly one read, no follow-up"); + for url in urls.iter() { + assert!( + url.starts_with("https://openlibrary.org/api/books?") + || url.starts_with("https://openlibrary.org/api/volumes/brief/"), + "unexpected endpoint addressed: {url}" + ); + // The import path sits at the site root (`openlibrary.org/isbn/…`), + // distinct from the Read API's `/api/volumes/brief/isbn/…` segment. + assert!( + !url.starts_with("https://openlibrary.org/isbn/"), + "import path must never be hit: {url}" + ); + } + } } diff --git a/crates/sp42-citation/src/citation/page.rs b/crates/sp42-citation/src/citation/page.rs index b996795d..5f28ffe3 100644 --- a/crates/sp42-citation/src/citation/page.rs +++ b/crates/sp42-citation/src/citation/page.rs @@ -2,12 +2,14 @@ use crate::citation::concurrency::map_with_concurrency; use crate::citation::extract::{BlockFailure, ExtractOutcome, SkippedRef}; +use crate::citation::openlibrary::{BookResolution, BookResolutionOutcome, resolve_book_source}; use crate::citation::verdict::{CitationVerdict, SupportLevel}; use crate::citation::verify::{ CitationFinding, FetchedSource, SourceUnavailableReason, VerificationOutcome, VerifyOptions, fetch_source, verify_citation_use_site, }; use crate::errors::CitationVerificationError; +use crate::wikitext_editor::BookSource; use sp42_types::{Clock, HttpClient, ModelClient, ModelRef}; use std::collections::{HashMap, HashSet}; @@ -56,6 +58,17 @@ pub struct PageVerificationStats { /// Of `source_unavailable`: fetched 2xx but unreadable (PDF/JS/wrong page) — a /// tool limitation, the citation may be fine. pub source_unavailable_unusable: usize, + /// Book citations resolved to an Open Library record (PRD-0009 Layer 1). + #[serde(default)] + pub books_resolved: usize, + /// Book citations whose identifiers were all looked up cleanly with no + /// catalog record found. + #[serde(default)] + pub books_not_found: usize, + /// Book citations whose catalog lookup failed in transport (existence + /// unknown — distinct from `books_not_found`). + #[serde(default)] + pub book_lookups_failed: usize, } /// Read-only result of verifying every URL-bearing citation on a page. @@ -67,6 +80,10 @@ pub struct PageVerificationReport { pub findings: Vec, pub skipped: Vec, pub extraction_failures: Vec, + /// Open Library resolutions for the skipped book refs (PRD-0009 Layer 1), + /// one entry per book source, in skip order. + #[serde(default)] + pub book_resolutions: Vec, pub stats: PageVerificationStats, } @@ -197,6 +214,7 @@ fn tally_stats( findings: &[CitationFinding], skipped: usize, extraction_failures: usize, + book_resolutions: &[BookResolution], ) -> PageVerificationStats { let mut stats = PageVerificationStats { refs_seen, @@ -205,6 +223,13 @@ fn tally_stats( extraction_failures, ..PageVerificationStats::default() }; + for resolution in book_resolutions { + match &resolution.outcome { + BookResolutionOutcome::Resolved { .. } => stats.books_resolved += 1, + BookResolutionOutcome::NotFound => stats.books_not_found += 1, + BookResolutionOutcome::LookupFailed { .. } => stats.book_lookups_failed += 1, + } + } for f in findings { match f.verdict { CitationVerdict::Judged(SupportLevel::Supported) => stats.supported += 1, @@ -227,6 +252,43 @@ fn tally_stats( stats } +/// Resolve the skipped refs' book sources against Open Library (PRD-0009 +/// Layer 1). Read-only and best-effort: a failure becomes a `LookupFailed` +/// entry, never a page error. Concurrency stays low out of REST politeness +/// (ADR-0015 sizes third-party REST fan-out at <= 3). +async fn resolve_book_citations( + fetch_client: &C, + skipped: &[SkippedRef], + page_concurrency: usize, +) -> Vec +where + C: HttpClient + ?Sized, +{ + let inputs: Vec<(String, usize, BookSource)> = skipped + .iter() + .flat_map(|s| { + s.book_sources + .iter() + .map(|book| (s.ref_id.clone(), s.block_ordinal, book.clone())) + }) + .collect(); + map_with_concurrency( + inputs, + page_concurrency.clamp(1, 3), + |(ref_id, block_ordinal, book), _| async move { + let outcome = resolve_book_source(fetch_client, &book).await; + BookResolution { + ref_id, + block_ordinal, + identifiers: book.identifiers, + cited_page: book.cited_page, + outcome, + } + }, + ) + .await +} + /// Verify every use-site in `extract` against its source. Fetches each distinct /// source URL once (shared via the prefetched option), fans the existing /// per-use-site verifier with bounded concurrency, and assembles a read-only @@ -365,12 +427,16 @@ where } } - // 3. Stats. + // 3. Resolve book citations against Open Library (PRD-0009 Layer 1). + let book_resolutions = resolve_book_citations(fetch_client, &skipped, page_concurrency).await; + + // 4. Stats. let stats = tally_stats( refs_seen, &findings, skipped.len(), extraction_failures.len(), + &book_resolutions, ); PageVerificationReport { @@ -380,6 +446,7 @@ where findings, skipped, extraction_failures, + book_resolutions, stats, } } @@ -445,6 +512,137 @@ mod orchestrator_tests { extract_use_sites(&[b], &page()) } + #[test] + fn book_ref_is_resolved_against_open_library() { + use crate::citation::extract::SkippedReason; + use crate::citation::openlibrary::BookResolutionOutcome; + use crate::wikitext_editor::{BookIdentifier, BookSource}; + use futures::executor::block_on; + + // One url-less book ref, no URL use-sites: the only network traffic is + // the catalog lookup and the availability check, in that order. + let b = ParsoidBlock { + text: "Cats purr.".into(), + refs: vec![BlockRef { + offset: 10, + ref_id: "cite_book".into(), + sources: vec![], + book_sources: vec![BookSource { + identifiers: vec![ + BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn"), + ], + cited_page: Some("42".to_string()), + }], + ref_text: "[1]".into(), + named: false, + }], + block_kind: BlockKind::Paragraph, + block_ordinal: 0, + }; + let extract = extract_use_sites(&[b], &page()); + assert_eq!(extract.skipped[0].reason, SkippedReason::BookSource); + + let http = StubHttpClient::new([ + Ok(HttpResponse { + status: 200, + headers: std::collections::BTreeMap::new(), + body: br#"{"ISBN:9780140328721": {"title": "Matilda", "url": "https://openlibrary.org/books/OL7826547M/Matilda"}}"#.to_vec(), + }), + Ok(HttpResponse { + status: 200, + headers: std::collections::BTreeMap::new(), + body: br#"{"items": [{"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl"}]}"#.to_vec(), + }), + ]); + // No use-sites → no model calls. + let model = StubModelClient::new([]); + let report = block_on(verify_page( + &http, + &model, + &FixedClock::new(0), + &[model_ref()], + &page(), + extract, + VerifyOptions::default(), + 1, + )); + + assert!(report.findings.is_empty()); + assert_eq!(report.skipped.len(), 1); + assert_eq!(report.book_resolutions.len(), 1); + let resolution = &report.book_resolutions[0]; + assert_eq!(resolution.ref_id, "cite_book"); + assert_eq!(resolution.cited_page.as_deref(), Some("42")); + let BookResolutionOutcome::Resolved { edition, scan, .. } = &resolution.outcome else { + panic!("expected Resolved, got {:?}", resolution.outcome); + }; + assert_eq!(edition.title.as_deref(), Some("Matilda")); + assert!( + scan.as_ref() + .expect("availability checked") + .groundable_scan() + .is_some() + ); + assert_eq!(report.stats.books_resolved, 1); + assert_eq!(report.stats.books_not_found, 0); + assert_eq!(report.stats.book_lookups_failed, 0); + // The ref still has no verdict: resolution refines the skip, it does + // not invent a verification outcome (grounding is Layer 2). + assert_eq!(report.stats.skipped, 1); + } + + #[test] + fn book_lookup_failure_never_fails_the_page() { + use crate::citation::openlibrary::BookResolutionOutcome; + use crate::wikitext_editor::{BookIdentifier, BookSource}; + use futures::executor::block_on; + use sp42_types::HttpClientError; + + let b = ParsoidBlock { + text: "Cats purr.".into(), + refs: vec![BlockRef { + offset: 10, + ref_id: "cite_book".into(), + sources: vec![], + book_sources: vec![BookSource { + identifiers: vec![ + BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn"), + ], + cited_page: None, + }], + ref_text: "[1]".into(), + named: false, + }], + block_kind: BlockKind::Paragraph, + block_ordinal: 0, + }; + let extract = extract_use_sites(&[b], &page()); + let http = StubHttpClient::new([Err(HttpClientError::Transport { + message: "openlibrary unreachable".to_string(), + })]); + let model = StubModelClient::new([]); + let report = block_on(verify_page( + &http, + &model, + &FixedClock::new(0), + &[model_ref()], + &page(), + extract, + VerifyOptions::default(), + 1, + )); + assert_eq!(report.book_resolutions.len(), 1); + assert!(matches!( + report.book_resolutions[0].outcome, + BookResolutionOutcome::LookupFailed { .. } + )); + assert_eq!(report.stats.book_lookups_failed, 1); + // The failure stays inside the books lane: no extraction failure, no + // finding, and the skip entry is untouched. + assert!(report.extraction_failures.is_empty()); + assert_eq!(report.stats.skipped, 1); + } + #[test] fn dedupes_fetches_and_verifies_each_use_site() { use futures::executor::block_on; @@ -956,10 +1154,32 @@ mod tests { findings: Vec::new(), skipped: Vec::new(), extraction_failures: Vec::new(), + book_resolutions: Vec::new(), stats: PageVerificationStats::default(), }; let json = serde_json::to_string(&report).expect("serialize"); let back: PageVerificationReport = serde_json::from_str(&json).expect("deserialize"); assert_eq!(report, back); } + + #[test] + fn report_without_book_fields_still_deserializes() { + // A report produced before the book-resolution slice (no + // `book_resolutions`, no book stats) must keep deserializing. + let json = r#"{ + "wiki_id": "frwiki", "rev_id": 42, "title": "Exemple", + "findings": [], "skipped": [], "extraction_failures": [], + "stats": { + "refs_seen": 0, "use_sites_verified": 0, "skipped": 0, + "extraction_failures": 0, "supported": 0, "partial": 0, + "not_supported": 0, "source_unavailable": 0, + "source_unavailable_unreachable": 0, + "source_unavailable_unusable": 0 + } + }"#; + let report: PageVerificationReport = + serde_json::from_str(json).expect("older report deserializes"); + assert!(report.book_resolutions.is_empty()); + assert_eq!(report.stats.books_resolved, 0); + } } diff --git a/crates/sp42-citation/src/citation_page_report.rs b/crates/sp42-citation/src/citation_page_report.rs index efc2e188..21ee41fc 100644 --- a/crates/sp42-citation/src/citation_page_report.rs +++ b/crates/sp42-citation/src/citation_page_report.rs @@ -8,8 +8,8 @@ //! the transform is a free function here (no inherent method — orphan rule). use crate::{ - CitationFinding, CitationVerdict, GroundingStatus, PageVerificationReport, - SourceUnavailableReason, SupportLevel, + BookResolution, BookResolutionOutcome, CitationFinding, CitationVerdict, GroundingStatus, + PageVerificationReport, ScanAvailability, SourceUnavailableReason, SupportLevel, }; use crate::report_document::{ @@ -57,12 +57,20 @@ pub fn page_verification_report_to_document(report: &PageVerificationReport) -> stats.skipped, stats.extraction_failures ), ]; + let mut lead_lines = lead_lines; + if !report.book_resolutions.is_empty() { + lead_lines.push(format!( + "books: resolved={} not_found={} lookup_failed={}", + stats.books_resolved, stats.books_not_found, stats.book_lookups_failed + )); + } ReportDocument::new("Page citation report") .with_lead_lines(lead_lines) .with_sections(vec![ findings_section(report), skipped_section(report), + books_section(report), extraction_failures_section(report), ]) } @@ -104,11 +112,11 @@ fn skipped_section(report: &PageVerificationReport) -> ReportSection { .skipped .iter() .map(|skipped| { - let identifiers = if skipped.book_identifiers.is_empty() { + let book_identifiers = skipped.book_identifiers(); + let identifiers = if book_identifiers.is_empty() { String::new() } else { - let joined = skipped - .book_identifiers + let joined = book_identifiers .iter() .map(ToString::to_string) .collect::>() @@ -124,6 +132,82 @@ fn skipped_section(report: &PageVerificationReport) -> ReportSection { } } +/// The Open Library resolutions for the page's book citations (PRD-0009 +/// Layer 1): what each skipped book ref resolved to, or an honest miss. +fn books_section(report: &PageVerificationReport) -> ReportSection { + if report.book_resolutions.is_empty() { + return ReportSection { + name: "Books".to_string(), + available: false, + summary_lines: vec!["none".to_string()], + }; + } + + ReportSection { + name: "Books".to_string(), + available: true, + summary_lines: report.book_resolutions.iter().map(book_line).collect(), + } +} + +fn book_line(resolution: &BookResolution) -> String { + let identifiers = resolution + .identifiers + .iter() + .map(ToString::to_string) + .collect::>() + .join(","); + let base = format!( + "ref={} block={}", + resolution.ref_id, resolution.block_ordinal + ); + match &resolution.outcome { + BookResolutionOutcome::Resolved { + identifier, + edition, + scan, + } => { + let title = edition + .title + .as_deref() + .map(|t| format!(" title=\"{t}\"")) + .unwrap_or_default(); + let authors = if edition.authors.is_empty() { + String::new() + } else { + format!(" authors=\"{}\"", edition.authors.join(", ")) + }; + let url = edition + .record_url + .as_deref() + .map(|u| format!(" url={u}")) + .unwrap_or_default(); + format!( + "{base} resolved={identifier}{title}{authors}{url} scan={scan}", + scan = scan_label(scan.as_ref()) + ) + } + BookResolutionOutcome::NotFound => { + format!("{base} not_found identifiers={identifiers}") + } + BookResolutionOutcome::LookupFailed { message } => { + format!("{base} lookup_failed identifiers={identifiers} error={message}") + } + } +} + +/// One word for the scan-availability outcome: `exact` may feed grounding, +/// `similar_only` must not (different edition), `none` is a record with no +/// usable scan, `unknown` means the availability lookup itself failed. +fn scan_label(scan: Option<&ScanAvailability>) -> &'static str { + match scan { + None => "unknown", + Some(availability) if availability.groundable_scan().is_some() => "exact", + Some(availability) if !availability.similar.is_empty() => "similar_only", + Some(_) => "none", + } +} + fn extraction_failures_section(report: &PageVerificationReport) -> ReportSection { if report.extraction_failures.is_empty() { return ReportSection { @@ -269,8 +353,9 @@ mod tests { } } - #[test] - fn renders_stats_findings_skipped_and_failures() { + /// A report exercising every section: findings out of order, two skipped + /// refs (one a book), a resolved book, and an extraction failure. + fn sample_report() -> PageVerificationReport { let supported = base_finding( 0, "cite_1", @@ -287,7 +372,7 @@ mod tests { unreachable.grounding_status = GroundingStatus::NotApplicable; unreachable.source_unavailable_reason = Some(SourceUnavailableReason::Unreachable); - let report = PageVerificationReport { + PageVerificationReport { wiki_id: "enwiki".to_string(), rev_id: 12345, title: "Museum".to_string(), @@ -298,21 +383,47 @@ mod tests { ref_id: "cite_book".to_string(), reason: SkippedReason::NonUrlSource, block_ordinal: 4, - book_identifiers: vec![], + book_sources: vec![], }, SkippedRef { ref_id: "cite_isbn".to_string(), reason: SkippedReason::BookSource, block_ordinal: 5, - book_identifiers: vec![crate::BookIdentifier::Isbn( - "9780140328721".to_string(), - )], + book_sources: vec![crate::BookSource { + identifiers: vec![crate::BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: Some("42".to_string()), + }], }, ], extraction_failures: vec![BlockFailure { block_ordinal: 7, reason: "no claim sentence".to_string(), }], + book_resolutions: vec![crate::BookResolution { + ref_id: "cite_isbn".to_string(), + block_ordinal: 5, + identifiers: vec![crate::BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: Some("42".to_string()), + outcome: crate::BookResolutionOutcome::Resolved { + identifier: crate::BookIdentifier::Isbn("9780140328721".to_string()), + edition: Box::new(crate::OpenLibraryEdition { + title: Some("Matilda".to_string()), + authors: vec!["Roald Dahl".to_string()], + record_url: Some( + "https://openlibrary.org/books/OL7826547M/Matilda".to_string(), + ), + ..crate::OpenLibraryEdition::default() + }), + scan: Some(crate::ScanAvailability { + exact: vec![crate::ScanItem { + status: "full access".to_string(), + item_url: "https://archive.org/details/matilda00dahl".to_string(), + ol_edition_id: None, + }], + similar: vec![], + }), + }, + }], stats: PageVerificationStats { refs_seen: 4, use_sites_verified: 2, @@ -324,9 +435,16 @@ mod tests { source_unavailable: 1, source_unavailable_unreachable: 1, source_unavailable_unusable: 0, + books_resolved: 1, + books_not_found: 0, + book_lookups_failed: 0, }, - }; + } + } + #[test] + fn renders_stats_findings_skipped_and_failures() { + let report = sample_report(); let document = page_verification_report_to_document(&report); assert_eq!(document.title, "Page citation report"); assert!( @@ -368,6 +486,48 @@ mod tests { assert!(markdown.contains("## Findings")); } + #[test] + fn renders_books_section_with_resolution_and_scan() { + let report = sample_report(); + let text = render_page_verification_text(&report); + assert!(text.contains("books: resolved=1 not_found=0 lookup_failed=0")); + assert!(text.contains("[Books] available=true")); + assert!(text.contains( + "ref=cite_isbn block=5 resolved=isbn:9780140328721 title=\"Matilda\" \ + authors=\"Roald Dahl\" url=https://openlibrary.org/books/OL7826547M/Matilda \ + scan=exact" + )); + } + + #[test] + fn book_line_reports_misses_and_failures_distinctly() { + let mut report = sample_report(); + report.book_resolutions = vec![ + crate::BookResolution { + ref_id: "cite_isbn".to_string(), + block_ordinal: 5, + identifiers: vec![crate::BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: None, + outcome: crate::BookResolutionOutcome::NotFound, + }, + crate::BookResolution { + ref_id: "cite_lccn".to_string(), + block_ordinal: 6, + identifiers: vec![crate::BookIdentifier::Lccn("n78890351".to_string())], + cited_page: None, + outcome: crate::BookResolutionOutcome::LookupFailed { + message: "openlibrary unreachable".to_string(), + }, + }, + ]; + let text = render_page_verification_text(&report); + assert!(text.contains("ref=cite_isbn block=5 not_found identifiers=isbn:9780140328721")); + assert!(text.contains( + "ref=cite_lccn block=6 lookup_failed identifiers=lccn:n78890351 \ + error=openlibrary unreachable" + )); + } + #[test] fn empty_report_marks_sections_unavailable() { let report = PageVerificationReport { @@ -377,6 +537,7 @@ mod tests { findings: Vec::new(), skipped: Vec::new(), extraction_failures: Vec::new(), + book_resolutions: Vec::new(), stats: PageVerificationStats::default(), }; @@ -412,6 +573,7 @@ mod tests { findings: vec![unusable, archived], skipped: Vec::new(), extraction_failures: Vec::new(), + book_resolutions: Vec::new(), stats: PageVerificationStats::default(), }; @@ -433,6 +595,7 @@ mod tests { findings: vec![finding], skipped: Vec::new(), extraction_failures: Vec::new(), + book_resolutions: Vec::new(), stats: PageVerificationStats::default(), }; diff --git a/crates/sp42-citation/src/lib.rs b/crates/sp42-citation/src/lib.rs index 32695659..c8728eeb 100644 --- a/crates/sp42-citation/src/lib.rs +++ b/crates/sp42-citation/src/lib.rs @@ -39,9 +39,10 @@ pub use citation::extract::{ }; pub use citation::locate_quote::{FuzzyLocate, locate_quote, locate_quote_fuzzy}; pub use citation::openlibrary::{ - OPEN_LIBRARY_BOOKS_API, OPEN_LIBRARY_READ_API_BASE, OpenLibraryEdition, ScanAvailability, - ScanItem, bibkey, build_catalog_lookup_request, build_scan_availability_request, - parse_catalog_lookup, parse_scan_availability, + BookResolution, BookResolutionOutcome, OPEN_LIBRARY_BOOKS_API, OPEN_LIBRARY_READ_API_BASE, + OpenLibraryEdition, ScanAvailability, ScanItem, bibkey, build_catalog_lookup_request, + build_scan_availability_request, parse_catalog_lookup, parse_scan_availability, + resolve_book_source, }; pub use citation::page::{ PageVerificationReport, PageVerificationRequest, PageVerificationStats, verify_page, diff --git a/docs/STATUS.md b/docs/STATUS.md index 526de0d9..17d89a17 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -76,12 +76,14 @@ PWA packaging and offline installability are now effectively complete for local (text/markdown/json), defaulting to the latest revision. `frwiki`, `enwiki`, and `testwiki` are registered. The browser Citations tab that renders the same report is in review (PR #81). -- book-citation grounding (PRD-0009, ADR-0018) has its first read-only slice: +- book-citation grounding (PRD-0009, ADR-0018) has its read-only resolve lane: validated book identifiers (ISBN/OCLC/LCCN/OLID) are extracted from cite-template `data-mw`, the page report distinguishes "book identifier present" from plain - non-URL refs, and a side-effect-free Open Library resolve module exists (Books - API catalog lookup + Read API exact-vs-similar scan availability); wiring - resolution into the verify path is the next slice + non-URL refs, and `verify-page` now resolves each book ref through the + side-effect-free Open Library lookups (Books API catalog + Read API + exact-vs-similar scan availability), rendering a Books report section with + resolved records, honest misses, and scan status; search-inside grounding + (Layer 2) is the next slice ## Current Verification From 2cb01bb3e031f3045024d9b4c409278c5b5f65d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:40:05 +0000 Subject: [PATCH 3/9] Ground book citations in IA search-inside snippets (PRD-0009 slice 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 2 of the book lane (ADR-0018 Decision 4): a resolved book citation with an exact-edition scan now gets a real verdict instead of a skip. - sp42-citation search_inside: the Internet Archive grounding source — item-metadata read for the designated search server (texts-only gate), BookReader full-text search build/parse (verbatim OCR snippets with highlight markers stripped, page numbers kept), conservative claim→query term selection, page-anchored deep links, and prepare_book_grounding mapping every degraded outcome onto the ADR-0007 split: no usable body (not a text item / no index) -> SourceUnavailable; an indexed scan searched with zero snippets -> not_supported, never SourceUnavailable. Cited page first, whole-book fallback, per PRD-0009 Q5. - verify: FetchedSource gains a book_snippet provenance flag whose only effect is bypassing the generic ShortBody floor at the usability gate — arbitrary short web pages still short-circuit, an empty snippet stays unusable (covered by a three-way test). CitationFinding gains optional BookScanProvenance (ocaid, scanned vs cited page, refined note) and two no-model book outcomes for the unavailable/searched-empty cases. - extract: a url-less ref with validated identifiers is now a BookUseSite (same sentence attach as URL refs, shared ordinal sequence) instead of an unconditional skip; only identifier-less refs skip at extract time. - verify_page: the book pass resolves each site against Open Library, grounds resolved books through the snippet body and the model panel (resolved record as context-only metadata sidecar), degrades similar- only/no-scan books to an honest SourceUnavailable pointing at the OL record, and re-skips catalog misses with the refined BookSource reason. The scanned page the passage actually located on is recorded so pagination mismatches surface instead of causing false negatives. - Renderer: finding lines show scan/scanned_page/cited_page/note; the Books section and stats are unchanged from slice 2. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- crates/sp42-citation/src/citation.rs | 1 + crates/sp42-citation/src/citation/extract.rs | 131 ++- crates/sp42-citation/src/citation/page.rs | 783 ++++++++++++++---- .../src/citation/search_inside.rs | 652 +++++++++++++++ crates/sp42-citation/src/citation/storage.rs | 1 + crates/sp42-citation/src/citation/verify.rs | 212 ++++- crates/sp42-citation/src/citation_finding.rs | 1 + .../sp42-citation/src/citation_page_report.rs | 34 +- crates/sp42-citation/src/lib.rs | 18 +- crates/sp42-cli/src/main.rs | 2 + crates/sp42-mcp/src/verify.rs | 1 + docs/STATUS.md | 18 +- 12 files changed, 1648 insertions(+), 206 deletions(-) create mode 100644 crates/sp42-citation/src/citation/search_inside.rs diff --git a/crates/sp42-citation/src/citation.rs b/crates/sp42-citation/src/citation.rs index 03a608ec..022c2717 100644 --- a/crates/sp42-citation/src/citation.rs +++ b/crates/sp42-citation/src/citation.rs @@ -24,6 +24,7 @@ pub mod openlibrary; pub mod page; pub mod parsing; pub mod prompts; +pub mod search_inside; pub mod segment; pub mod source_fetch; pub mod storage; diff --git a/crates/sp42-citation/src/citation/extract.rs b/crates/sp42-citation/src/citation/extract.rs index 80fa6087..fc3f4898 100644 --- a/crates/sp42-citation/src/citation/extract.rs +++ b/crates/sp42-citation/src/citation/extract.rs @@ -37,8 +37,10 @@ pub enum SkippedReason { /// is nothing to fetch and nothing to resolve (offline/unidentified source). NonUrlSource, /// The ref carries no URL but does carry a validated book identifier - /// (ISBN/OCLC/LCCN/OLID); Open Library catalog resolution (PRD-0009 - /// Layer 1, ADR-0018) is the path that consumes it. + /// (ISBN/OCLC/LCCN/OLID) that did **not** resolve to an Open Library + /// record (catalog miss or failed lookup) — assigned by the page + /// orchestrator after resolution (PRD-0009 Layer 1, ADR-0018). A resolved + /// book ref becomes a finding instead (Layer 2). BookSource, } @@ -68,6 +70,26 @@ impl SkippedRef { } } +/// One book-citation use-site (PRD-0009): a claim sentence attached to a +/// url-less ref that carries validated book identifiers. The page +/// orchestrator resolves it against Open Library and — when an exact-edition +/// scan exists — grounds the claim in the scan's search-inside snippets +/// (Layer 2), so it is a claim-bearing unit exactly like [`CitationUseSite`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BookUseSite { + /// Document-order index across the page (shared with URL use-sites). + pub use_site_ordinal: u32, + pub block_ordinal: usize, + /// The originating ref's marker id, for provenance. + pub ref_id: String, + /// The claim sentence the ref attaches to. + pub claim: String, + /// Article title + preceding sentences passed alongside the claim. + pub context: ClaimContext, + /// The book named by this use-site (identifiers + cited page). + pub book: BookSource, +} + /// A block (or ref) that could not be processed. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct BlockFailure { @@ -79,21 +101,28 @@ pub struct BlockFailure { #[derive(Debug, Clone, PartialEq, Eq)] pub struct ExtractOutcome { pub use_sites: Vec, + /// Book-citation use-sites (PRD-0009): url-less refs with validated + /// identifiers, carrying the claim they attach to. NOT in `skipped` — + /// whether one ends as a finding or a refined skip is decided by the page + /// orchestrator after Open Library resolution. + pub book_use_sites: Vec, pub skipped: Vec, pub failures: Vec, } -/// Extract every URL-bearing citation use-site from a page's blocks. -/// Non-URL refs are recorded in `skipped` — with the skip reason and the -/// ref's validated book identifiers distinguishing "nothing to resolve" from -/// "a book awaiting catalog resolution" (PRD-0009). Blocks that yield no -/// usable claim go to `failures`. Document order is preserved across the page. +/// Extract every claim-bearing citation use-site from a page's blocks: one +/// [`CitationUseSite`] per URL source and one [`BookUseSite`] per book source +/// on a url-less ref (PRD-0009). A ref with neither an extractable URL nor a +/// validated book identifier is recorded in `skipped`; refs that yield no +/// usable claim go to `failures`. Document order is preserved across the page +/// and ordinals are shared across both use-site kinds. #[must_use] pub fn extract_use_sites( blocks: &[ParsoidBlock], page: &PageVerificationRequest, ) -> ExtractOutcome { let mut use_sites = Vec::new(); + let mut book_use_sites = Vec::new(); let mut skipped = Vec::new(); let mut failures = Vec::new(); let mut ordinal: u32 = 0; @@ -101,20 +130,13 @@ pub fn extract_use_sites( for block in blocks { let sentences = segment_sentences(&block.text); for r in &block.refs { - if r.sources.is_empty() { - // Book sources carry at least one validated identifier by - // construction, so their presence is the "book identifier - // present" signal. - let reason = if r.book_sources.is_empty() { - SkippedReason::NonUrlSource - } else { - SkippedReason::BookSource - }; + if r.sources.is_empty() && r.book_sources.is_empty() { + // Nothing fetchable and nothing to resolve. skipped.push(SkippedRef { ref_id: r.ref_id.clone(), - reason, + reason: SkippedReason::NonUrlSource, block_ordinal: block.block_ordinal, - book_sources: r.book_sources.clone(), + book_sources: Vec::new(), }); continue; } @@ -146,6 +168,24 @@ pub fn extract_use_sites( preceding_sentences: preceding, }; + // A ref with URL sources rides the URL verification path; its + // book identifiers stay on the BlockRef for future enrichment. + // Only a url-less ref becomes a book use-site (PRD-0009). + if r.sources.is_empty() { + for book in &r.book_sources { + book_use_sites.push(BookUseSite { + use_site_ordinal: ordinal, + block_ordinal: block.block_ordinal, + ref_id: r.ref_id.clone(), + claim: claim.clone(), + context: context.clone(), + book: book.clone(), + }); + ordinal += 1; + } + continue; + } + for source in &r.sources { use_sites.push(CitationUseSite { use_site_ordinal: ordinal, @@ -168,6 +208,7 @@ pub fn extract_use_sites( ExtractOutcome { use_sites, + book_use_sites, skipped, failures, } @@ -324,9 +365,9 @@ mod tests { } #[test] - fn book_identifier_ref_is_skipped_with_the_refined_reason() { - // A url-less ref that carries a validated ISBN is distinguishable from - // a plain non-URL source: the report can say which book went unresolved. + fn book_identifier_ref_becomes_a_book_use_site_with_the_claim() { + // A url-less ref carrying a validated ISBN is a claim-bearing unit: + // it gets the same sentence attach as a URL ref, ready for Layer 2. let mut r = bref(10, &[]); r.book_sources = vec![crate::wikitext_editor::BookSource { identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], @@ -335,22 +376,49 @@ mod tests { let b = block("Cats purr.", vec![r]); let out = extract_use_sites(&[b], &page()); assert!(out.use_sites.is_empty()); - assert_eq!(out.skipped.len(), 1); - assert_eq!(out.skipped[0].reason, SkippedReason::BookSource); - assert_eq!( - out.skipped[0].book_identifiers(), - vec![&BookIdentifier::Isbn("9780140328721".to_string())] - ); + assert!(out.skipped.is_empty(), "book refs are use-sites, not skips"); + assert_eq!(out.book_use_sites.len(), 1); + let site = &out.book_use_sites[0]; + assert_eq!(site.claim, "Cats purr."); + assert_eq!(site.context.article_title, "Cats"); assert_eq!( - out.skipped[0].book_sources[0].cited_page.as_deref(), - Some("42") + site.book.identifiers, + vec![BookIdentifier::Isbn("9780140328721".to_string())] ); + assert_eq!(site.book.cited_page.as_deref(), Some("42")); + } + + #[test] + fn book_use_sites_share_the_ordinal_sequence_with_url_use_sites() { + let mut book_ref = bref(10, &[]); + book_ref.book_sources = vec![crate::wikitext_editor::BookSource { + identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: None, + }]; + let url_ref = bref(22, &["https://a.test"]); + let b = block("Cats purr. Cats sleep.", vec![book_ref, url_ref]); + let out = extract_use_sites(&[b], &page()); + assert_eq!(out.book_use_sites[0].use_site_ordinal, 0); + assert_eq!(out.use_sites[0].use_site_ordinal, 1); + } + + #[test] + fn book_ref_with_empty_claim_is_a_failure_like_the_url_path() { + let mut r = bref(0, &[]); + r.book_sources = vec![crate::wikitext_editor::BookSource { + identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], + cited_page: None, + }]; + let b = block(" ", vec![r]); + let out = extract_use_sites(&[b], &page()); + assert!(out.book_use_sites.is_empty()); + assert_eq!(out.failures.len(), 1); } #[test] fn url_bearing_ref_with_book_identifiers_still_verifies_as_url() { - // url= wins: the ref produces a use-site; the book identifiers ride - // along on the BlockRef without changing the URL verification path. + // url= wins: the ref produces a URL use-site; the book identifiers + // ride along on the BlockRef without spawning a book use-site. let mut r = bref(10, &["https://a.test"]); r.book_sources = vec![crate::wikitext_editor::BookSource { identifiers: vec![BookIdentifier::Isbn("9780140328721".to_string())], @@ -359,6 +427,7 @@ mod tests { let b = block("Cats purr.", vec![r]); let out = extract_use_sites(&[b], &page()); assert_eq!(out.use_sites.len(), 1); + assert!(out.book_use_sites.is_empty()); assert!(out.skipped.is_empty()); } diff --git a/crates/sp42-citation/src/citation/page.rs b/crates/sp42-citation/src/citation/page.rs index 5f28ffe3..b6da17aa 100644 --- a/crates/sp42-citation/src/citation/page.rs +++ b/crates/sp42-citation/src/citation/page.rs @@ -1,15 +1,25 @@ //! Page-level verification: request, orchestrator output report, and stats. +use crate::citation::citoid::CitoidMetadata; use crate::citation::concurrency::map_with_concurrency; -use crate::citation::extract::{BlockFailure, ExtractOutcome, SkippedRef}; -use crate::citation::openlibrary::{BookResolution, BookResolutionOutcome, resolve_book_source}; +use crate::citation::extract::{ + BlockFailure, BookUseSite, ExtractOutcome, SkippedReason, SkippedRef, +}; +use crate::citation::openlibrary::{ + BookResolution, BookResolutionOutcome, OpenLibraryEdition, build_catalog_lookup_request, + resolve_book_source, +}; +use crate::citation::search_inside::{ + BookGroundingPreparation, extract_ocaid, prepare_book_grounding, scan_deep_link, +}; use crate::citation::verdict::{CitationVerdict, SupportLevel}; use crate::citation::verify::{ - CitationFinding, FetchedSource, SourceUnavailableReason, VerificationOutcome, VerifyOptions, - fetch_source, verify_citation_use_site, + BookScanProvenance, CitationFinding, CitationVerificationRequest, FetchedSource, + SourceProvenance, SourceUnavailableReason, VerificationOutcome, VerifyOptions, + book_scan_unavailable_outcome, book_searched_not_supported_outcome, fetch_source, sha256_hex, + verify_citation_use_site, }; use crate::errors::CitationVerificationError; -use crate::wikitext_editor::BookSource; use sp42_types::{Clock, HttpClient, ModelClient, ModelRef}; use std::collections::{HashMap, HashSet}; @@ -252,41 +262,401 @@ fn tally_stats( stats } -/// Resolve the skipped refs' book sources against Open Library (PRD-0009 -/// Layer 1). Read-only and best-effort: a failure becomes a `LookupFailed` -/// entry, never a page error. Concurrency stays low out of REST politeness -/// (ADR-0015 sizes third-party REST fan-out at <= 3). -async fn resolve_book_citations( +/// What one book use-site produced besides its [`BookResolution`]: a finding +/// (resolved book, grounded or honestly `SourceUnavailable`), the refined skip +/// (identifier present, no catalog record / failed lookup), or a verify error. +enum BookVerdict { + Finding(Box), + Skip(SkippedRef), + Failure(BlockFailure), +} + +/// The finding-side provenance URL for a resolved book: the human-facing Open +/// Library record when the edition names one, else the catalog lookup itself. +fn book_record_url(edition: &OpenLibraryEdition, resolution: &BookResolution) -> url::Url { + edition + .record_url + .as_deref() + .and_then(|record| record.parse().ok()) + .unwrap_or_else(|| { + let identifier = resolution + .identifiers + .first() + .expect("book sources carry at least one identifier by construction"); + build_catalog_lookup_request(identifier).url + }) +} + +/// A no-model book finding (unavailable / searched-and-nothing-found), with +/// the ref/claim/context provenance stamped like `verify_one_use_site` does. +#[allow(clippy::too_many_arguments)] // https://github.com/schiste/SP42 distinct named provenance inputs, same trade-off as verify_citation_use_site +fn book_no_model_finding( + site: &BookUseSite, + url: url::Url, + content_hash: String, + http_status: u16, + clock: &dyn Clock, + not_supported: bool, + book_scan: Option, +) -> CitationFinding { + let provenance = SourceProvenance { + url, + content_hash, + fetched_at: clock.now_ms(), + http_status: Some(http_status), + }; + let outcome = if not_supported { + book_searched_not_supported_outcome(&provenance, site.use_site_ordinal) + } else { + book_scan_unavailable_outcome(&provenance, site.use_site_ordinal) + }; + let mut finding = outcome.finding; + finding.ref_id.clone_from(&site.ref_id); + finding.claim.clone_from(&site.claim); + finding + .preceding_context + .clone_from(&site.context.preceding_sentences); + finding.book_scan = book_scan; + finding +} + +/// Judge one book claim against its assembled snippet body: the existing +/// verifier runs over the snippets (scoped short-body bypass), the resolved +/// record rides the metadata sidecar as prompt context (context only, never +/// grounded), and the finding gains the book-scan provenance — including the +/// **scanned** page the passage was actually found on, so a pagination +/// mismatch surfaces instead of a false `not_supported` (PRD-0009 Q5). +#[allow(clippy::too_many_arguments)] // https://github.com/schiste/SP42 injected edges + site, same trade-off as verify_citation_use_site +async fn judge_book_snippets( fetch_client: &C, - skipped: &[SkippedRef], - page_concurrency: usize, -) -> Vec + model_client: &M, + clock: &dyn Clock, + panel: &[ModelRef], + page: &PageVerificationRequest, + site: &BookUseSite, + edition: &OpenLibraryEdition, + ocaid: &str, + options: &VerifyOptions, + body: &crate::citation::search_inside::BookSnippetBody, +) -> BookVerdict where C: HttpClient + ?Sized, + M: ModelClient + ?Sized, { - let inputs: Vec<(String, usize, BookSource)> = skipped - .iter() - .flat_map(|s| { - s.book_sources - .iter() - .map(|book| (s.ref_id.clone(), s.block_ordinal, book.clone())) - }) - .collect(); - map_with_concurrency( - inputs, - page_concurrency.clamp(1, 3), - |(ref_id, block_ordinal, book), _| async move { - let outcome = resolve_book_source(fetch_client, &book).await; - BookResolution { - ref_id, - block_ordinal, - identifiers: book.identifiers, - cited_page: book.cited_page, - outcome, - } - }, + let source_url: url::Url = scan_deep_link(ocaid, body.page_of_passage(None), &body.query) + .parse() + .unwrap_or_else(|_| { + format!("https://archive.org/details/{ocaid}") + .parse() + .expect("archive details url parses") + }); + let request = CitationVerificationRequest { + wiki_id: page.wiki_id.clone(), + rev_id: page.rev_id, + title: page.title.clone(), + claim: site.claim.clone(), + source_url, + }; + let mut opts = options.clone(); + opts.include_metadata = false; + // The resolved record is legitimate prompt context (context only, never + // grounded) — reuse the metadata sidecar slot. + opts.metadata_sidecar = Some(CitoidMetadata { + publication: (!edition.publishers.is_empty()).then(|| edition.publishers.join(", ")), + published: edition.publish_date.clone(), + author: (!edition.authors.is_empty()).then(|| edition.authors.join(", ")), + title: edition.title.clone(), + url: request.source_url.to_string(), + }); + opts.prefetched = Some(FetchedSource { + text: body.text.clone(), + status: 200, + content_type: "text/plain".to_string(), + raw_html: None, + book_snippet: true, + }); + match verify_citation_use_site( + fetch_client, + model_client, + clock, + panel, + &request, + Some(&site.context), + site.use_site_ordinal, + opts, ) .await + { + Ok(outcome) => { + let mut finding = outcome.finding; + finding.ref_id.clone_from(&site.ref_id); + finding.claim.clone_from(&site.claim); + finding + .preceding_context + .clone_from(&site.context.preceding_sentences); + let scanned_page = + body.page_of_passage(finding.passage.as_ref().map(|p| p.quote.as_str())); + finding.book_scan = Some(BookScanProvenance { + ocaid: ocaid.to_string(), + scanned_page, + cited_page: site.book.cited_page.clone(), + note: (!body.cited_page_hit && site.book.cited_page.is_some()) + .then(|| "cited page had no match; whole-book search".to_string()), + }); + BookVerdict::Finding(Box::new(finding)) + } + Err(error) => BookVerdict::Failure(BlockFailure { + block_ordinal: site.block_ordinal, + reason: format!("book verify failed for {}: {error}", site.ref_id), + }), + } +} + +/// Ground and judge one resolved book claim against its exact-edition scan: +/// search-inside snippets become the source body for the existing verifier +/// (with the scoped short-body bypass), and every degraded outcome maps onto +/// the ADR-0007 split (`SourceUnavailable` vs `not_supported`). +#[allow(clippy::too_many_arguments)] // https://github.com/schiste/SP42 injected edges + site, same trade-off as verify_citation_use_site +async fn ground_resolved_book( + fetch_client: &C, + model_client: &M, + clock: &dyn Clock, + panel: &[ModelRef], + page: &PageVerificationRequest, + site: &BookUseSite, + edition: &OpenLibraryEdition, + ocaid: &str, + options: &VerifyOptions, +) -> BookVerdict +where + C: HttpClient + ?Sized, + M: ModelClient + ?Sized, +{ + let preparation = prepare_book_grounding( + fetch_client, + ocaid, + &site.claim, + site.book.cited_page.as_deref(), + ) + .await; + let scan_url = |page_number: Option, query: &str| -> url::Url { + scan_deep_link(ocaid, page_number, query) + .parse() + .unwrap_or_else(|_| { + format!("https://archive.org/details/{ocaid}") + .parse() + .expect("archive details url parses") + }) + }; + match preparation { + BookGroundingPreparation::Body(body) => { + judge_book_snippets( + fetch_client, + model_client, + clock, + panel, + page, + site, + edition, + ocaid, + options, + &body, + ) + .await + } + BookGroundingPreparation::NoMatches { + response_hash, + deep_link, + } => { + let url = deep_link + .parse() + .unwrap_or_else(|_| scan_url(None, &site.claim)); + let book_scan = Some(BookScanProvenance { + ocaid: ocaid.to_string(), + scanned_page: None, + cited_page: site.book.cited_page.clone(), + note: Some("searched, no matching passage".to_string()), + }); + BookVerdict::Finding(Box::new(book_no_model_finding( + site, + url, + response_hash, + 200, + clock, + true, + book_scan, + ))) + } + BookGroundingPreparation::NoUsableBody { detail } => { + let book_scan = Some(BookScanProvenance { + ocaid: ocaid.to_string(), + scanned_page: None, + cited_page: site.book.cited_page.clone(), + note: Some(detail.to_string()), + }); + BookVerdict::Finding(Box::new(book_no_model_finding( + site, + scan_url(None, &site.claim), + sha256_hex(b""), + 200, + clock, + false, + book_scan, + ))) + } + BookGroundingPreparation::Unreachable { message } => { + let book_scan = Some(BookScanProvenance { + ocaid: ocaid.to_string(), + scanned_page: None, + cited_page: site.book.cited_page.clone(), + note: Some(message), + }); + BookVerdict::Finding(Box::new(book_no_model_finding( + site, + scan_url(None, &site.claim), + sha256_hex(b""), + 0, + clock, + false, + book_scan, + ))) + } + } +} + +/// Resolve and judge one book use-site (PRD-0009 Layers 1+2): Open Library +/// resolution always yields a [`BookResolution`] for the report; the verdict +/// side is a finding for a resolved book (grounded when an exact-edition scan +/// is searchable, honest `SourceUnavailable` otherwise) or the refined skip +/// when no catalog record was found. +async fn verify_book_use_site( + fetch_client: &C, + model_client: &M, + clock: &dyn Clock, + panel: &[ModelRef], + page: &PageVerificationRequest, + site: BookUseSite, + options: &VerifyOptions, +) -> (BookResolution, BookVerdict) +where + C: HttpClient + ?Sized, + M: ModelClient + ?Sized, +{ + let outcome = resolve_book_source(fetch_client, &site.book).await; + let resolution = BookResolution { + ref_id: site.ref_id.clone(), + block_ordinal: site.block_ordinal, + identifiers: site.book.identifiers.clone(), + cited_page: site.book.cited_page.clone(), + outcome, + }; + + let verdict = match &resolution.outcome { + BookResolutionOutcome::Resolved { edition, scan, .. } => { + let groundable_ocaid = scan + .as_ref() + .and_then(|availability| availability.groundable_scan()) + .and_then(|item| extract_ocaid(&item.item_url)); + if let Some(ocaid) = groundable_ocaid { + ground_resolved_book( + fetch_client, + model_client, + clock, + panel, + page, + &site, + edition, + &ocaid, + options, + ) + .await + } else { + // Resolved, but no exact-edition scan (similar-only, none, or + // availability unknown): grounding degrades to + // `SourceUnavailable` (ADR-0018 Decision 3); the Books section + // carries the scan state. + BookVerdict::Finding(Box::new(book_no_model_finding( + &site, + book_record_url(edition, &resolution), + sha256_hex(b""), + 200, + clock, + false, + None, + ))) + } + } + BookResolutionOutcome::NotFound | BookResolutionOutcome::LookupFailed { .. } => { + BookVerdict::Skip(SkippedRef { + ref_id: site.ref_id.clone(), + reason: SkippedReason::BookSource, + block_ordinal: site.block_ordinal, + book_sources: vec![site.book.clone()], + }) + } + }; + (resolution, verdict) +} + +/// Prefetch each distinct source URL once, into the shared body cache the +/// use-site fan-out reads — but cap the retained bytes so a citation-heavy +/// page can't OOM the server. Fetches run in concurrency-sized chunks and +/// *caching* stops once retained bodies reach `MAX_PREFETCH_CACHE_BYTES`; any +/// URL past the budget is simply not prefetched, so `verify_one_use_site` +/// lazily (re)fetches it on demand (un-deduped). Peak retained ≈ budget + +/// `page_concurrency` * source cap. A proper evict-after-last-use fix is +/// tracked in SP42#59. A transport error inserts a sentinel (empty text, +/// status 0) so no use-site re-fetches; the empty-text path routes to +/// `SourceUnavailable` via the body-usability gate. +async fn prefetch_bodies( + fetch_client: &C, + use_sites: &[crate::citation::extract::CitationUseSite], + page_concurrency: usize, +) -> HashMap +where + C: HttpClient + ?Sized, +{ + let mut distinct: Vec = Vec::new(); + for us in use_sites { + let u = us.request.source_url.to_string(); + if !distinct.contains(&u) { + distinct.push(u); + } + } + let mut bodies: HashMap = HashMap::new(); + let mut retained_bytes: usize = 0; + let mut budget_hit = false; + for chunk in distinct.chunks(page_concurrency) { + if retained_bytes >= MAX_PREFETCH_CACHE_BYTES { + budget_hit = true; + break; + } + let fetched = map_with_concurrency(chunk.to_vec(), page_concurrency, |url, _| async move { + (url.clone(), fetch_source(fetch_client, &url).await) + }) + .await; + for (url, result) in fetched { + let source = result.unwrap_or_else(|_| FetchedSource { + text: String::new(), + status: 0, + content_type: String::new(), + raw_html: None, + book_snippet: false, + }); + retained_bytes += prefetch_retained_bytes(&source); + bodies.insert(url, source); + } + } + if budget_hit { + tracing::warn!( + distinct_sources = distinct.len(), + cached_sources = bodies.len(), + retained_bytes, + "verify_page source-body cache hit its byte budget; remaining sources will be \ + re-fetched per use-site (un-deduped) to bound memory" + ); + } + bodies } /// Verify every use-site in `extract` against its source. Fetches each distinct @@ -322,14 +692,19 @@ where { let ExtractOutcome { use_sites, + book_use_sites, skipped, failures, } = extract; - // refs_seen = every ref we encountered: those that became use-sites (count by distinct ref_id, - // since extract_use_sites emits one use-site per source URL), those skipped (non-URL), - // and those that failed extraction. - let distinct_use_site_refs: HashSet<&str> = - use_sites.iter().map(|u| u.ref_id.as_str()).collect(); + let mut skipped = skipped; + // refs_seen = every ref we encountered: those that became use-sites (count by distinct + // ref_id across URL and book use-sites, since extract emits one use-site per source), + // those skipped (non-URL, no identifier), and those that failed extraction. + let distinct_use_site_refs: HashSet<&str> = use_sites + .iter() + .map(|u| u.ref_id.as_str()) + .chain(book_use_sites.iter().map(|b| b.ref_id.as_str())) + .collect(); let refs_seen = distinct_use_site_refs.len() + skipped.len() + failures.len(); // Pre-bind shared refs OUTSIDE the closures so the spawned futures capture @@ -340,63 +715,11 @@ where let clock: &dyn Clock = clock; let panel: &[ModelRef] = panel; - // 1. Dedupe: distinct source URLs, fetched once each. - let mut distinct: Vec = Vec::new(); - for us in &use_sites { - let u = us.request.source_url.to_string(); - if !distinct.contains(&u) { - distinct.push(u); - } - } + // 1. Prefetch each distinct source URL once, into a shared body cache. // Page-level concurrency: distinct fetches in flight. The per-use-site panel // concurrency stays `options.concurrency` (applied inside verify_one_use_site). let page_concurrency = page_concurrency.max(1); - // Prefetch the distinct bodies once each and share them — but cap the retained bytes so a - // citation-heavy page can't OOM the server. We fetch in concurrency-sized chunks and stop - // *caching* once retained bodies reach `MAX_PREFETCH_CACHE_BYTES`; any URL past the budget - // is simply not prefetched, so `verify_one_use_site` lazily (re)fetches it on demand - // (un-deduped). Peak retained ≈ budget + `page_concurrency` * source cap. A proper - // evict-after-last-use fix is tracked in SP42#59. - let mut bodies: HashMap = HashMap::new(); - let mut retained_bytes: usize = 0; - let mut budget_hit = false; - for chunk in distinct.chunks(page_concurrency) { - if retained_bytes >= MAX_PREFETCH_CACHE_BYTES { - budget_hit = true; - break; - } - let fetched = map_with_concurrency(chunk.to_vec(), page_concurrency, |url, _| async move { - (url.clone(), fetch_source(fetch_client, &url).await) - }) - .await; - for (url, result) in fetched { - let source = match result { - Ok(source) => source, - Err(_) => { - // Transport error: insert a sentinel (empty text, status 0) so no use-site - // re-fetches. The empty-text path routes to SourceUnavailable via the - // body-usability gate (body_classifier.rs). - FetchedSource { - text: String::new(), - status: 0, - content_type: String::new(), - raw_html: None, - } - } - }; - retained_bytes += prefetch_retained_bytes(&source); - bodies.insert(url, source); - } - } - if budget_hit { - tracing::warn!( - distinct_sources = distinct.len(), - cached_sources = bodies.len(), - retained_bytes, - "verify_page source-body cache hit its byte budget; remaining sources will be \ - re-fetched per use-site (un-deduped) to bound memory" - ); - } + let bodies = prefetch_bodies(fetch_client, &use_sites, page_concurrency).await; // 2. Fan verify over use-sites, sharing the prefetched body. // Every distinct URL is now in `bodies` (including sentinel entries for failed fetches), @@ -427,8 +750,35 @@ where } } - // 3. Resolve book citations against Open Library (PRD-0009 Layer 1). - let book_resolutions = resolve_book_citations(fetch_client, &skipped, page_concurrency).await; + // 3. Book citations (PRD-0009 Layers 1+2): resolve each against Open + // Library, ground resolved books in their exact-edition scan's + // search-inside snippets, and refine the skip for unresolved ones. + // Read-only against two third-party hosts; concurrency stays low out of + // REST politeness (ADR-0015 sizes third-party REST fan-out at <= 3). + let page_ref = page; + let book_options = &options; + let book_results = + map_with_concurrency(book_use_sites, page_concurrency.clamp(1, 3), |site, _| { + verify_book_use_site( + fetch_client, + model_client, + clock, + panel, + page_ref, + site, + book_options, + ) + }) + .await; + let mut book_resolutions = Vec::new(); + for (resolution, verdict) in book_results { + book_resolutions.push(resolution); + match verdict { + BookVerdict::Finding(finding) => findings.push(*finding), + BookVerdict::Skip(skip) => skipped.push(skip), + BookVerdict::Failure(failure) => extraction_failures.push(failure), + } + } // 4. Stats. let stats = tally_stats( @@ -512,50 +862,71 @@ mod orchestrator_tests { extract_use_sites(&[b], &page()) } - #[test] - fn book_ref_is_resolved_against_open_library() { - use crate::citation::extract::SkippedReason; - use crate::citation::openlibrary::BookResolutionOutcome; + fn book_block(cited_page: Option<&str>) -> ParsoidBlock { use crate::wikitext_editor::{BookIdentifier, BookSource}; - use futures::executor::block_on; - - // One url-less book ref, no URL use-sites: the only network traffic is - // the catalog lookup and the availability check, in that order. - let b = ParsoidBlock { - text: "Cats purr.".into(), + ParsoidBlock { + text: "Matilda longed for her parents to be good and loving.".into(), refs: vec![BlockRef { - offset: 10, + offset: 54, ref_id: "cite_book".into(), sources: vec![], book_sources: vec![BookSource { identifiers: vec![ BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn"), ], - cited_page: Some("42".to_string()), + cited_page: cited_page.map(ToString::to_string), }], ref_text: "[1]".into(), named: false, }], block_kind: BlockKind::Paragraph, block_ordinal: 0, - }; - let extract = extract_use_sites(&[b], &page()); - assert_eq!(extract.skipped[0].reason, SkippedReason::BookSource); + } + } + + fn response(body: &str) -> HttpResponse { + HttpResponse { + status: 200, + headers: std::collections::BTreeMap::new(), + body: body.as_bytes().to_vec(), + } + } + + #[test] + fn book_ref_is_resolved_grounded_and_judged() { + use crate::citation::openlibrary::BookResolutionOutcome; + use futures::executor::block_on; + + // One url-less book ref, no URL use-sites. The whole chain over the + // stub, in order: catalog lookup → Read API (exact scan) → item + // metadata → search-inside → model panel over the snippet body. + let extract = extract_use_sites(&[book_block(Some("42"))], &page()); + assert!(extract.skipped.is_empty()); + assert_eq!(extract.book_use_sites.len(), 1); let http = StubHttpClient::new([ - Ok(HttpResponse { - status: 200, - headers: std::collections::BTreeMap::new(), - body: br#"{"ISBN:9780140328721": {"title": "Matilda", "url": "https://openlibrary.org/books/OL7826547M/Matilda"}}"#.to_vec(), - }), - Ok(HttpResponse { - status: 200, - headers: std::collections::BTreeMap::new(), - body: br#"{"items": [{"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl"}]}"#.to_vec(), - }), + Ok(response( + r#"{"ISBN:9780140328721": {"title": "Matilda", "url": "https://openlibrary.org/books/OL7826547M/Matilda", "authors": [{"name": "Roald Dahl"}]}}"#, + )), + Ok(response( + r#"{"items": [{"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl"}]}"#, + )), + Ok(response( + r#"{"server": "ia800300.us.archive.org", "dir": "/12/items/matilda00dahl", "metadata": {"mediatype": "texts"}}"#, + )), + Ok(response( + r#"{"indexed": true, "matches": [{"text": "Matilda longed for her {{{parents}}} to be good and loving.", "par": [{"page": 42}]}]}"#, + )), ]); - // No use-sites → no model calls. - let model = StubModelClient::new([]); + // Panel size 1, repair off: exactly one model call over the snippet. + let model = StubModelClient::new([Ok(completion( + r#"{"verdict":"SUPPORTED","quote":"good and loving"}"#, + ))]); + let options = VerifyOptions { + repair_turn: false, + concurrency: 1, + ..VerifyOptions::default() + }; let report = block_on(verify_page( &http, &model, @@ -563,60 +934,142 @@ mod orchestrator_tests { &[model_ref()], &page(), extract, - VerifyOptions::default(), + options, 1, )); - assert!(report.findings.is_empty()); - assert_eq!(report.skipped.len(), 1); + // The book ref produced a real grounded finding, not a skip. + assert!(report.skipped.is_empty()); + assert_eq!(report.findings.len(), 1); + let finding = &report.findings[0]; + assert_eq!(finding.ref_id, "cite_book"); + assert_eq!( + finding.verdict, + crate::CitationVerdict::Judged(crate::SupportLevel::Supported) + ); + assert_eq!(finding.grounding_status, crate::GroundingStatus::Located); + let scan = finding.book_scan.as_ref().expect("book-scan provenance"); + assert_eq!(scan.ocaid, "matilda00dahl"); + assert_eq!(scan.scanned_page, Some(42)); + assert_eq!(scan.cited_page.as_deref(), Some("42")); + assert!( + finding + .provenance + .url + .as_str() + .starts_with("https://archive.org/details/matilda00dahl/page/42"), + "deep link should anchor the scanned page: {}", + finding.provenance.url + ); + + // The resolution record still feeds the Books section. assert_eq!(report.book_resolutions.len(), 1); - let resolution = &report.book_resolutions[0]; - assert_eq!(resolution.ref_id, "cite_book"); - assert_eq!(resolution.cited_page.as_deref(), Some("42")); - let BookResolutionOutcome::Resolved { edition, scan, .. } = &resolution.outcome else { - panic!("expected Resolved, got {:?}", resolution.outcome); + let BookResolutionOutcome::Resolved { edition, .. } = &report.book_resolutions[0].outcome + else { + panic!("expected Resolved"); }; assert_eq!(edition.title.as_deref(), Some("Matilda")); - assert!( - scan.as_ref() - .expect("availability checked") - .groundable_scan() - .is_some() + assert_eq!(report.stats.books_resolved, 1); + assert_eq!(report.stats.supported, 1); + assert_eq!(report.stats.skipped, 0); + assert_eq!(report.stats.refs_seen, 1); + } + + #[test] + fn indexed_scan_with_no_snippets_is_not_supported_never_unavailable() { + use futures::executor::block_on; + + let extract = extract_use_sites(&[book_block(None)], &page()); + let http = StubHttpClient::new([ + Ok(response(r#"{"ISBN:9780140328721": {"title": "Matilda"}}"#)), + Ok(response( + r#"{"items": [{"match": "exact", "status": "full access", "itemURL": "https://archive.org/details/matilda00dahl"}]}"#, + )), + Ok(response( + r#"{"server": "ia800300.us.archive.org", "dir": "/12/items/matilda00dahl", "metadata": {"mediatype": "texts"}}"#, + )), + Ok(response(r#"{"indexed": true, "matches": []}"#)), + ]); + // Zero snippets → deterministic not_supported, no model call. + let model = StubModelClient::new([]); + let report = block_on(verify_page( + &http, + &model, + &FixedClock::new(0), + &[model_ref()], + &page(), + extract, + VerifyOptions::default(), + 1, + )); + assert_eq!(report.findings.len(), 1); + let finding = &report.findings[0]; + assert_eq!( + finding.verdict, + crate::CitationVerdict::Judged(crate::SupportLevel::NotSupported) + ); + assert_eq!(finding.source_unavailable_reason, None); + assert_eq!( + finding + .book_scan + .as_ref() + .expect("book scan provenance") + .note + .as_deref(), + Some("searched, no matching passage") ); + assert_eq!(report.stats.not_supported, 1); + assert_eq!(report.stats.source_unavailable, 0); + } + + #[test] + fn similar_only_scan_degrades_to_source_unavailable() { + use futures::executor::block_on; + + let extract = extract_use_sites(&[book_block(None)], &page()); + let http = StubHttpClient::new([ + Ok(response( + r#"{"ISBN:9780140328721": {"title": "Matilda", "url": "https://openlibrary.org/books/OL7826547M/Matilda"}}"#, + )), + Ok(response( + r#"{"items": [{"match": "similar", "status": "lendable", "itemURL": "https://archive.org/details/other-edition"}]}"#, + )), + ]); + let model = StubModelClient::new([]); + let report = block_on(verify_page( + &http, + &model, + &FixedClock::new(0), + &[model_ref()], + &page(), + extract, + VerifyOptions::default(), + 1, + )); + // Never verified against a different edition: honest SourceUnavailable, + // pointing the operator at the resolved record. + assert_eq!(report.findings.len(), 1); + let finding = &report.findings[0]; + assert_eq!(finding.verdict, crate::CitationVerdict::SourceUnavailable); + assert_eq!( + finding.source_unavailable_reason, + Some(crate::SourceUnavailableReason::Unusable) + ); + assert_eq!( + finding.provenance.url.as_str(), + "https://openlibrary.org/books/OL7826547M/Matilda" + ); + assert_eq!(report.stats.source_unavailable_unusable, 1); assert_eq!(report.stats.books_resolved, 1); - assert_eq!(report.stats.books_not_found, 0); - assert_eq!(report.stats.book_lookups_failed, 0); - // The ref still has no verdict: resolution refines the skip, it does - // not invent a verification outcome (grounding is Layer 2). - assert_eq!(report.stats.skipped, 1); } #[test] fn book_lookup_failure_never_fails_the_page() { use crate::citation::openlibrary::BookResolutionOutcome; - use crate::wikitext_editor::{BookIdentifier, BookSource}; use futures::executor::block_on; use sp42_types::HttpClientError; - let b = ParsoidBlock { - text: "Cats purr.".into(), - refs: vec![BlockRef { - offset: 10, - ref_id: "cite_book".into(), - sources: vec![], - book_sources: vec![BookSource { - identifiers: vec![ - BookIdentifier::isbn("978-0-14-032872-1").expect("valid isbn"), - ], - cited_page: None, - }], - ref_text: "[1]".into(), - named: false, - }], - block_kind: BlockKind::Paragraph, - block_ordinal: 0, - }; - let extract = extract_use_sites(&[b], &page()); + let extract = extract_use_sites(&[book_block(None)], &page()); let http = StubHttpClient::new([Err(HttpClientError::Transport { message: "openlibrary unreachable".to_string(), })]); @@ -638,8 +1091,14 @@ mod orchestrator_tests { )); assert_eq!(report.stats.book_lookups_failed, 1); // The failure stays inside the books lane: no extraction failure, no - // finding, and the skip entry is untouched. + // finding, and the ref lands in skipped with the refined reason. assert!(report.extraction_failures.is_empty()); + assert!(report.findings.is_empty()); + assert_eq!(report.skipped.len(), 1); + assert_eq!( + report.skipped[0].reason, + crate::citation::extract::SkippedReason::BookSource + ); assert_eq!(report.stats.skipped, 1); } @@ -1116,6 +1575,7 @@ mod tests { status: 200, content_type: "text/html".to_string(), raw_html: Some("x".repeat(100)), + book_snippet: false, }; assert_eq!(prefetch_retained_bytes(&html_heavy), 3 + 100); @@ -1125,6 +1585,7 @@ mod tests { status: 200, content_type: String::new(), raw_html: None, + book_snippet: false, }; assert_eq!(prefetch_retained_bytes(&text_only), 5); } diff --git a/crates/sp42-citation/src/citation/search_inside.rs b/crates/sp42-citation/src/citation/search_inside.rs new file mode 100644 index 00000000..7acc3297 --- /dev/null +++ b/crates/sp42-citation/src/citation/search_inside.rs @@ -0,0 +1,652 @@ +//! Internet Archive search-inside grounding source (PRD-0009 Layer 2, +//! ADR-0018 Decision 4). +//! +//! Turns a resolved exact-edition scan into a **book-snippet source body** the +//! existing verifier can judge: read the item's metadata for its designated +//! server and directory, query the `BookReader` full-text search, and assemble +//! the returned **verbatim OCR snippets** (page-numbered) into a body. The +//! scan's OCR is never downloaded whole — snippet search typically works even +//! for lending-restricted scans, so grounding needs no borrow. +//! +//! Pure `build_*`/`parse_*` pairs over the injected `HttpClient`, like the +//! Open Library module ([`crate::citation::openlibrary`]). Outcome semantics +//! (ADR-0007 discipline, PRD-0009 resolved Q4): +//! - **No usable body** (not a text item / no index / metadata unusable) → +//! `SourceUnavailable`. SP42 could not read the book. +//! - **Searched, nothing found** on an indexed scan → `not_supported` (never +//! `SourceUnavailable`): the source was searched and yielded no passage. + +use std::collections::BTreeMap; +use std::sync::LazyLock; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use url::Url; + +use crate::citation::verify::sha256_hex; +use crate::types::{HttpMethod, HttpRequest}; +use sp42_types::HttpClient; + +/// The archive.org item-metadata endpoint (read-only; names the item's +/// designated `server` and `dir` the full-text search runs on). +pub const ARCHIVE_METADATA_BASE: &str = "https://archive.org/metadata"; + +/// The metadata endpoint parsed once, for the (unreachable) fallback when a +/// built URL somehow fails to parse — keeps the builder panic-free. +static METADATA_BASE: LazyLock = LazyLock::new(|| { + ARCHIVE_METADATA_BASE + .parse() + .expect("static endpoint parses") +}); + +/// Most matches carried into an assembled snippet body. Bounds the body (and +/// the prompt it feeds) on common-word queries with hundreds of hits. +const MAX_SNIPPET_MATCHES: usize = 8; + +/// Most claim terms carried into the search query. +const MAX_QUERY_TERMS: usize = 6; + +/// The archive.org item id (`ocaid`) from a Read API `itemURL`, e.g. +/// `https://archive.org/details/matilda00dahl` → `matilda00dahl`. +#[must_use] +pub fn extract_ocaid(item_url: &str) -> Option { + let url = Url::parse(item_url).ok()?; + if url.host_str() != Some("archive.org") && url.host_str() != Some("www.archive.org") { + return None; + } + let mut segments = url.path_segments()?; + if segments.next() != Some("details") { + return None; + } + segments + .next() + .filter(|s| !s.is_empty()) + .map(ToString::to_string) +} + +/// Build the (read-only GET) item-metadata request for a scan. +#[must_use] +pub fn build_item_metadata_request(ocaid: &str) -> HttpRequest { + let url_string = format!("{ARCHIVE_METADATA_BASE}/{ocaid}"); + let url = url_string.parse().unwrap_or_else(|_| METADATA_BASE.clone()); + HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::new(), + body: Vec::new(), + } +} + +/// Where an item's full-text search runs: the metadata-designated server and +/// item directory, plus whether the item is a text item at all. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ItemLocation { + /// Designated server host, e.g. `ia800300.us.archive.org`. + pub server: String, + /// Item directory on that server, e.g. `/12/items/matilda00dahl`. + pub dir: String, + /// `true` iff `metadata.mediatype` is `texts` — the groundable-item gate. + pub is_text_item: bool, +} + +/// Parse an item-metadata response into the search location. `None` for an +/// unparseable body or one missing the server/dir designation. +#[must_use] +pub fn parse_item_metadata(body: &[u8]) -> Option { + let parsed: Value = serde_json::from_slice(body).ok()?; + let server = parsed.get("server").and_then(Value::as_str)?; + let dir = parsed.get("dir").and_then(Value::as_str)?; + let is_text_item = parsed + .pointer("/metadata/mediatype") + .and_then(Value::as_str) + == Some("texts"); + Some(ItemLocation { + server: server.to_string(), + dir: dir.to_string(), + is_text_item, + }) +} + +/// Build the (read-only GET) `BookReader` full-text search request against the +/// item's designated server. `None` when the metadata-supplied server does +/// not form a valid URL (treated as no-usable-body by the caller). +#[must_use] +pub fn build_search_inside_request( + location: &ItemLocation, + ocaid: &str, + query: &str, +) -> Option { + let base = format!("https://{}/fulltext/inside.php", location.server); + let url = Url::parse_with_params( + &base, + &[ + ("item_id", ocaid), + ("doc", ocaid), + ("path", location.dir.as_str()), + ("q", query), + ], + ) + .ok()?; + Some(HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::new(), + body: Vec::new(), + }) +} + +/// One search-inside match: the verbatim OCR snippet (match markers stripped) +/// and the scanned page it was found on. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SearchInsideMatch { + /// Verbatim OCR snippet text, with the `{{{…}}}` highlight markers + /// removed so the text is exactly what the OCR holds (groundable bytes). + pub text: String, + /// The scanned page number, when the match carries one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub page: Option, +} + +/// A parsed search-inside response. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SearchInsideResult { + /// `false` when the item has no full-text index (search cannot run). + pub indexed: bool, + pub matches: Vec, +} + +/// Parse a search-inside response. `None` for an unparseable body. +#[must_use] +pub fn parse_search_inside(body: &[u8]) -> Option { + let parsed: Value = serde_json::from_slice(body).ok()?; + let object = parsed.as_object()?; + // The BookReader search reports an un-indexed item either explicitly + // (`"indexed": false`) or by erroring without a matches array. + let indexed = object + .get("indexed") + .and_then(Value::as_bool) + .unwrap_or_else(|| object.contains_key("matches")); + let matches = object + .get("matches") + .and_then(Value::as_array) + .map(|entries| { + entries + .iter() + .filter_map(|entry| { + let text = entry.get("text").and_then(Value::as_str)?; + let page = entry + .pointer("/par/0/page") + .and_then(Value::as_u64) + .and_then(|p| u32::try_from(p).ok()); + Some(SearchInsideMatch { + text: text.replace("{{{", "").replace("}}}", ""), + page, + }) + }) + .collect() + }) + .unwrap_or_default(); + Some(SearchInsideResult { indexed, matches }) +} + +/// A conservative full-text query from a claim sentence: the distinct longer +/// words (≥ 5 chars, falling back to ≥ 4, then the trimmed claim), in claim +/// order, capped at [`MAX_QUERY_TERMS`]. Deliberately simple — term selection +/// quality only affects recall, never grounding (the snippet bytes are what +/// the gate locates quotes in); smarter selection can come later. +#[must_use] +pub fn search_query(claim: &str) -> String { + let words: Vec<&str> = claim + .split(|c: char| !c.is_alphanumeric()) + .filter(|w| !w.is_empty()) + .collect(); + for floor in [5, 4] { + let mut seen = Vec::new(); + for word in &words { + if word.chars().count() >= floor && !seen.contains(word) { + seen.push(word); + if seen.len() == MAX_QUERY_TERMS { + break; + } + } + } + if !seen.is_empty() { + return seen.join(" "); + } + } + claim.trim().to_string() +} + +/// A page-anchored deep link into the scan, with the search terms highlighted +/// — the operator's jump to the page that supports (or contradicts) the claim. +#[must_use] +pub fn scan_deep_link(ocaid: &str, page: Option, query: &str) -> String { + let base = match page { + Some(page) => format!("https://archive.org/details/{ocaid}/page/{page}"), + None => format!("https://archive.org/details/{ocaid}"), + }; + Url::parse_with_params(&base, &[("q", query)]).map_or(base, |url| url.to_string()) +} + +/// What the grounding preparation produced for one book claim. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BookGroundingPreparation { + /// Snippets found: a body for the verifier plus page provenance. + Body(BookSnippetBody), + /// The scan is indexed and was searched (both passes), zero snippets → + /// the caller reports `not_supported` (PRD-0009 resolved Q4). + NoMatches { + /// SHA-256 of the search response — the `SourceFetched` grounding. + response_hash: String, + /// Deep link into the scan (no page anchor — nothing located). + deep_link: String, + }, + /// Search-inside cannot run: not a text item, no full-text index, or the + /// item metadata was unusable → `SourceUnavailable` (unusable). + NoUsableBody { + /// Which precondition failed, for the report. + detail: &'static str, + }, + /// A transport failure before an answer → `SourceUnavailable` + /// (unreachable). + Unreachable { message: String }, +} + +/// The assembled book-snippet source body plus its provenance. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BookSnippetBody { + /// The verbatim snippets, joined by blank lines — the groundable bytes. + pub text: String, + /// The matches the body was assembled from (page attribution). + pub matches: Vec, + /// `true` when the cited-page pass matched; `false` means the whole-book + /// fallback supplied the snippets (pagination-mismatch signal). + pub cited_page_hit: bool, + /// The search query that produced the snippets (deep-link highlighting). + pub query: String, +} + +impl BookSnippetBody { + /// The scanned page of the match containing `passage`, falling back to + /// the first match's page. Scan pagination often differs from the cited + /// edition, so the report shows where the passage was actually found. + #[must_use] + pub fn page_of_passage(&self, passage: Option<&str>) -> Option { + passage + .and_then(|quote| { + self.matches + .iter() + .find(|entry| entry.text.contains(quote)) + .and_then(|entry| entry.page) + }) + .or_else(|| self.matches.first().and_then(|entry| entry.page)) + } +} + +/// Prepare the grounding body for one book claim against one scan: metadata → +/// search-inside → cited-page-first snippet selection. Strictly read-only and +/// best-effort; every failure maps onto the ADR-0007 outcome split rather +/// than an error. +pub async fn prepare_book_grounding( + client: &C, + ocaid: &str, + claim: &str, + cited_page: Option<&str>, +) -> BookGroundingPreparation +where + C: HttpClient + ?Sized, +{ + // 1. Item metadata: where does this item's full-text search run? + let metadata_response = match client.execute(build_item_metadata_request(ocaid)).await { + Ok(response) => response, + Err(error) => { + return BookGroundingPreparation::Unreachable { + message: error.to_string(), + }; + } + }; + if !(200..300).contains(&metadata_response.status) { + return BookGroundingPreparation::Unreachable { + message: format!("item metadata returned {}", metadata_response.status), + }; + } + let Some(location) = parse_item_metadata(&metadata_response.body) else { + return BookGroundingPreparation::NoUsableBody { + detail: "item metadata unusable", + }; + }; + if !location.is_text_item { + return BookGroundingPreparation::NoUsableBody { + detail: "not a text item", + }; + } + + // 2. Full-text search on the designated server. + let query = search_query(claim); + let Some(request) = build_search_inside_request(&location, ocaid, &query) else { + return BookGroundingPreparation::NoUsableBody { + detail: "search server unusable", + }; + }; + let search_response = match client.execute(request).await { + Ok(response) => response, + Err(error) => { + return BookGroundingPreparation::Unreachable { + message: error.to_string(), + }; + } + }; + if !(200..300).contains(&search_response.status) { + return BookGroundingPreparation::Unreachable { + message: format!("search-inside returned {}", search_response.status), + }; + } + let Some(result) = parse_search_inside(&search_response.body) else { + return BookGroundingPreparation::NoUsableBody { + detail: "search response unusable", + }; + }; + if !result.indexed { + return BookGroundingPreparation::NoUsableBody { + detail: "no full-text index", + }; + } + if result.matches.is_empty() { + return BookGroundingPreparation::NoMatches { + response_hash: sha256_hex(&search_response.body), + deep_link: scan_deep_link(ocaid, None, &query), + }; + } + + // 3. Cited page first, then fall back to the whole book (PRD-0009 + // resolved Q5). The search returns page-numbered matches for the whole + // book, so the page-first pass is a filter, not a second request. + let cited_page_number = cited_page.and_then(leading_page_number); + let on_cited_page: Vec = cited_page_number + .map(|page| { + result + .matches + .iter() + .filter(|entry| entry.page == Some(page)) + .cloned() + .collect() + }) + .unwrap_or_default(); + let cited_page_hit = !on_cited_page.is_empty(); + let mut selected = if cited_page_hit { + on_cited_page + } else { + result.matches + }; + selected.truncate(MAX_SNIPPET_MATCHES); + + let text = selected + .iter() + .map(|entry| entry.text.trim()) + .filter(|snippet| !snippet.is_empty()) + .collect::>() + .join("\n\n"); + if text.is_empty() { + return BookGroundingPreparation::NoUsableBody { + detail: "empty snippets", + }; + } + BookGroundingPreparation::Body(BookSnippetBody { + text, + matches: selected, + cited_page_hit, + query, + }) +} + +/// The leading page number of a cite `page=`/`pages=` value (`"42"`, +/// `"42–45"`, `"42, 44"` all yield 42). `None` when it does not start with a +/// number (e.g. roman numerals — the whole-book pass covers those). +fn leading_page_number(cited_page: &str) -> Option { + let digits: String = cited_page + .trim() + .chars() + .take_while(char::is_ascii_digit) + .collect(); + digits.parse().ok() +} + +#[cfg(test)] +mod tests { + use super::{ + BookGroundingPreparation, ItemLocation, build_item_metadata_request, + build_search_inside_request, extract_ocaid, parse_item_metadata, parse_search_inside, + prepare_book_grounding, scan_deep_link, search_query, + }; + use crate::types::HttpResponse; + use futures::executor::block_on; + use sp42_types::{HttpClientError, StubHttpClient}; + + fn ok(body: &str) -> HttpResponse { + HttpResponse { + status: 200, + headers: std::collections::BTreeMap::new(), + body: body.as_bytes().to_vec(), + } + } + + const ITEM_METADATA: &str = r#"{ + "server": "ia800300.us.archive.org", + "dir": "/12/items/matilda00dahl", + "metadata": {"identifier": "matilda00dahl", "mediatype": "texts"} + }"#; + + const SEARCH_HITS: &str = r#"{ + "indexed": true, + "matches": [ + {"text": "Matilda longed for her parents to be {{{good}}} and {{{loving}}}.", + "par": [{"page": 42, "boxes": []}]}, + {"text": "a {{{good}}} and {{{loving}}} child somewhere else in the book", + "par": [{"page": 7, "boxes": []}]} + ] + }"#; + + #[test] + fn ocaid_extraction_requires_an_archive_details_url() { + assert_eq!( + extract_ocaid("https://archive.org/details/matilda00dahl").as_deref(), + Some("matilda00dahl") + ); + assert_eq!( + extract_ocaid("https://archive.org/details/matilda00dahl/page/8").as_deref(), + Some("matilda00dahl") + ); + assert_eq!(extract_ocaid("https://example.org/details/x"), None); + assert_eq!(extract_ocaid("https://archive.org/download/x"), None); + assert_eq!(extract_ocaid("not a url"), None); + } + + #[test] + fn metadata_request_and_parse_designate_the_search_server() { + let request = build_item_metadata_request("matilda00dahl"); + assert_eq!( + request.url.as_str(), + "https://archive.org/metadata/matilda00dahl" + ); + let location = parse_item_metadata(ITEM_METADATA.as_bytes()).expect("parses"); + assert_eq!(location.server, "ia800300.us.archive.org"); + assert_eq!(location.dir, "/12/items/matilda00dahl"); + assert!(location.is_text_item); + // A movie item is not groundable. + let movie = r#"{"server": "s", "dir": "/d", "metadata": {"mediatype": "movies"}}"#; + assert!( + !parse_item_metadata(movie.as_bytes()) + .expect("parses") + .is_text_item + ); + assert_eq!(parse_item_metadata(b"{}"), None); + } + + #[test] + fn search_request_targets_the_designated_server() { + let location = ItemLocation { + server: "ia800300.us.archive.org".to_string(), + dir: "/12/items/matilda00dahl".to_string(), + is_text_item: true, + }; + let request = build_search_inside_request(&location, "matilda00dahl", "good loving") + .expect("valid server forms a url"); + let url = request.url.as_str(); + assert!(url.starts_with("https://ia800300.us.archive.org/fulltext/inside.php?")); + assert!(url.contains("item_id=matilda00dahl")); + assert!(url.contains("path=%2F12%2Fitems%2Fmatilda00dahl")); + assert!(url.contains("q=good+loving")); + } + + #[test] + fn parse_search_strips_highlight_markers_and_reads_pages() { + let result = parse_search_inside(SEARCH_HITS.as_bytes()).expect("parses"); + assert!(result.indexed); + assert_eq!(result.matches.len(), 2); + assert_eq!( + result.matches[0].text, + "Matilda longed for her parents to be good and loving." + ); + assert_eq!(result.matches[0].page, Some(42)); + // Unindexed and garbage responses are distinguishable. + assert!( + !parse_search_inside(br#"{"indexed": false}"#) + .expect("parses") + .indexed + ); + assert_eq!(parse_search_inside(b""), None); + } + + #[test] + fn query_takes_distinct_longer_words_in_claim_order() { + assert_eq!( + search_query("Matilda longed for her parents to be good and loving."), + "Matilda longed parents loving" + ); + // Short-word claims fall back to the 4-char floor, then the raw claim. + assert_eq!(search_query("The cat sat down"), "down"); + assert_eq!(search_query("a b c"), "a b c"); + } + + #[test] + fn deep_link_anchors_the_page_and_highlights_the_query() { + assert_eq!( + scan_deep_link("matilda00dahl", Some(42), "good loving"), + "https://archive.org/details/matilda00dahl/page/42?q=good+loving" + ); + assert_eq!( + scan_deep_link("matilda00dahl", None, "good"), + "https://archive.org/details/matilda00dahl?q=good" + ); + } + + #[test] + fn grounding_selects_the_cited_page_first() { + let client = StubHttpClient::new(vec![Ok(ok(ITEM_METADATA)), Ok(ok(SEARCH_HITS))]); + let prep = block_on(prepare_book_grounding( + &client, + "matilda00dahl", + "Matilda longed for her parents to be good and loving.", + Some("42"), + )); + let BookGroundingPreparation::Body(body) = prep else { + panic!("expected a body, got {prep:?}"); + }; + assert!(body.cited_page_hit, "page-42 match should be selected"); + assert_eq!(body.matches.len(), 1, "only the cited-page match"); + assert_eq!(body.matches[0].page, Some(42)); + assert!(body.text.contains("good and loving")); + assert_eq!(body.page_of_passage(Some("good and loving")), Some(42)); + } + + #[test] + fn grounding_falls_back_to_the_whole_book_on_a_page_miss() { + // Cited page 99 has no match: pagination differs — fall back to every + // match and record that the cited-page pass missed. + let client = StubHttpClient::new(vec![Ok(ok(ITEM_METADATA)), Ok(ok(SEARCH_HITS))]); + let prep = block_on(prepare_book_grounding( + &client, + "matilda00dahl", + "Matilda longed for her parents to be good and loving.", + Some("99"), + )); + let BookGroundingPreparation::Body(body) = prep else { + panic!("expected a body, got {prep:?}"); + }; + assert!(!body.cited_page_hit); + assert_eq!( + body.matches.len(), + 2, + "whole-book fallback keeps all matches" + ); + } + + #[test] + fn indexed_scan_with_zero_matches_is_no_matches_not_unusable() { + let client = StubHttpClient::new(vec![ + Ok(ok(ITEM_METADATA)), + Ok(ok(r#"{"indexed": true, "matches": []}"#)), + ]); + let prep = block_on(prepare_book_grounding( + &client, + "matilda00dahl", + "claim text here", + None, + )); + assert!( + matches!(prep, BookGroundingPreparation::NoMatches { .. }), + "searched-and-found-nothing must not read as unusable: {prep:?}" + ); + } + + #[test] + fn non_text_item_and_missing_index_are_no_usable_body() { + let movie = + r#"{"server": "s.archive.org", "dir": "/d", "metadata": {"mediatype": "movies"}}"#; + let client = StubHttpClient::new(vec![Ok(ok(movie))]); + let prep = block_on(prepare_book_grounding( + &client, + "x", + "claim text here", + None, + )); + assert_eq!( + prep, + BookGroundingPreparation::NoUsableBody { + detail: "not a text item" + } + ); + + let client = + StubHttpClient::new(vec![Ok(ok(ITEM_METADATA)), Ok(ok(r#"{"indexed": false}"#))]); + let prep = block_on(prepare_book_grounding( + &client, + "x", + "claim text here", + None, + )); + assert_eq!( + prep, + BookGroundingPreparation::NoUsableBody { + detail: "no full-text index" + } + ); + } + + #[test] + fn transport_failure_is_unreachable() { + let client = StubHttpClient::new(vec![Err(HttpClientError::Transport { + message: "archive.org unreachable".to_string(), + })]); + let prep = block_on(prepare_book_grounding( + &client, + "x", + "claim text here", + None, + )); + let BookGroundingPreparation::Unreachable { message } = prep else { + panic!("expected Unreachable, got {prep:?}"); + }; + assert!(message.contains("archive.org unreachable")); + } +} diff --git a/crates/sp42-citation/src/citation/storage.rs b/crates/sp42-citation/src/citation/storage.rs index 259ce812..9fbd1ef9 100644 --- a/crates/sp42-citation/src/citation/storage.rs +++ b/crates/sp42-citation/src/citation/storage.rs @@ -355,6 +355,7 @@ mod tests { claim: String::new(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: crate::citation::verify::SCHEMA_VERSION, } } diff --git a/crates/sp42-citation/src/citation/verify.rs b/crates/sp42-citation/src/citation/verify.rs index 3173bf17..d8b56d96 100644 --- a/crates/sp42-citation/src/citation/verify.rs +++ b/crates/sp42-citation/src/citation/verify.rs @@ -142,6 +142,27 @@ pub enum GroundingStatus { NotApplicable, } +/// Book-scan provenance for a finding verified against an Internet Archive +/// search-inside snippet (PRD-0009 Layer 2): which scan the snippet came +/// from, the scanned page the passage was found on (which can differ from the +/// cited page — reprints, front-matter offsets), and the cited page echoed so +/// the mismatch surfaces to the operator instead of a false `not_supported`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BookScanProvenance { + /// The archive.org item id of the scan. + pub ocaid: String, + /// The scanned page the located passage was found on, when attributable. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub scanned_page: Option, + /// The cite template's `page=` value, echoed for the mismatch signal. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cited_page: Option, + /// Refined grounding note for the operator ("no full-text index", + /// "item metadata unreachable", …) when the scan could not be searched. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub note: Option, +} + /// Why a `SourceUnavailable` verdict was reached. Derived from the fetch /// status; lets a reviewer tell a dead link from a source we fetched but /// could not read (PDF / JS shell / wrong page). @@ -237,6 +258,11 @@ pub struct CitationFinding { /// (ADR-0009 replay). #[serde(default, skip_serializing_if = "Option::is_none")] pub archive_of: Option, + /// Set when this verdict was produced against an Internet Archive + /// search-inside snippet (PRD-0009 Layer 2): which scan and scanned page. + /// `None` for every web-source finding. Back-compatible (ADR-0009 replay). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub book_scan: Option, /// Schema version (Art. 9.1). pub schema_version: u32, } @@ -610,6 +636,7 @@ pub fn assemble_citation_finding( claim: String::new(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: SCHEMA_VERSION, } } @@ -722,6 +749,7 @@ fn no_quote_finding( claim: String::new(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: SCHEMA_VERSION, } } @@ -798,6 +826,54 @@ fn unusable_source_outcome( } } +/// A no-model, no-quote `SourceUnavailable` outcome — the book lane's +/// short-circuit when a scan cannot be used at all (PRD-0009 Layer 2 / +/// ADR-0018 Decision 4: no exact-edition scan, no full-text index, or the +/// item metadata was unreachable). Unreachable-vs-unusable is derived from +/// `provenance.http_status` exactly like the fetched-web path (`0` → +/// unreachable, 2xx → unusable). +#[must_use] +pub fn book_scan_unavailable_outcome( + provenance: &SourceProvenance, + use_site_ordinal: u32, +) -> VerificationOutcome { + VerificationOutcome { + finding: no_quote_finding( + CitationVerdict::SourceUnavailable, + GroundingStatus::NotApplicable, + PanelAgreement::new(0, 0), + provenance, + use_site_ordinal, + ), + votes: Vec::new(), + } +} + +/// The searched-and-found-nothing outcome (PRD-0009 resolved Q4 / ADR-0018 +/// Decision 4): the scan **is** indexed and search-inside ran, but returned +/// zero snippets after both the cited-page and whole-book passes. That is +/// `not_supported`, not `SourceUnavailable` — the source exists and was +/// searched; it yielded no supporting passage. Deliberately a verdict without +/// a model panel: the judgment IS the deterministic search outcome, grounded +/// by the fetched search response (`SourceFetched` provenance), and it never +/// fabricates a passage (`NotApplicable` grounding, no quote). +#[must_use] +pub fn book_searched_not_supported_outcome( + provenance: &SourceProvenance, + use_site_ordinal: u32, +) -> VerificationOutcome { + VerificationOutcome { + finding: no_quote_finding( + CitationVerdict::Judged(SupportLevel::NotSupported), + GroundingStatus::NotApplicable, + PanelAgreement::new(0, 0), + provenance, + use_site_ordinal, + ), + votes: Vec::new(), + } +} + /// A fetched source body plus the HTTP status it came from. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FetchedSource { @@ -808,6 +884,12 @@ pub struct FetchedSource { /// usability gate for structured paywall markers; consumed there and not /// retained downstream (grounding uses `text`). pub raw_html: Option, + /// `true` only for a body assembled from Internet Archive search-inside + /// snippets by the book-grounding path (PRD-0009 Layer 2). Provenance for + /// the usability gate: a snippet body bypasses **only** the generic + /// short-body floor (ADR-0018 Decision 4) — arbitrary short web pages + /// still short-circuit. Never set by [`fetch_source`]. + pub book_snippet: bool, } /// Fetch a source body (read-only GET), extracting text from HTML and recovering past a @@ -847,6 +929,7 @@ where status: response.status, content_type: String::new(), raw_html: None, + book_snippet: false, }); } let content_type = response @@ -866,6 +949,7 @@ where status: response.status, content_type, raw_html, + book_snippet: false, }) } @@ -982,12 +1066,25 @@ where } else { Some(fetched.text.as_str()) }; - let usability = classify_source_usability( + let mut usability = classify_source_usability( request.source_url.as_str(), &fetched.content_type, fetched.raw_html.as_deref(), body, ); + // ADR-0018 Decision 4: a search-inside snippet body bypasses ONLY the + // generic short-body floor — a verbatim OCR snippet is a valid grounding + // body well below the web-page floor. Every other unusability reason + // still applies, and an empty snippet (`body == None`) stays unusable. + if fetched.book_snippet + && body.is_some() + && usability.reason == super::body_classifier::BodyUsabilityReason::ShortBody + { + usability = super::body_classifier::BodyUsability { + usable: true, + reason: super::body_classifier::BodyUsabilityReason::Ok, + }; + } if !usability.usable { return Ok(unusable_source_outcome( usability.reason, @@ -1453,6 +1550,7 @@ mod tests { status: 200, content_type: String::new(), raw_html: None, + book_snippet: false, }); let outcome = block_on(verify_citation_use_site( &http, @@ -1502,6 +1600,7 @@ mod tests { status: 200, content_type: String::new(), raw_html: None, + book_snippet: false, }), metadata_sidecar: metadata, ..VerifyOptions::default() @@ -2390,6 +2489,117 @@ mod tests { assert!(outcome.votes.is_empty()); } + #[test] + fn book_snippet_bypasses_only_the_short_body_floor() { + // ADR-0018 Decision 4: a search-inside snippet body well below the + // 300-char web-page floor still reaches the panel... + let snippet = "Matilda longed for her parents to be good and loving."; + assert!(snippet.chars().count() < 300, "test premise: short body"); + let fetch = StubHttpClient::new([]); // prefetched → any fetch errors the test + let model_client = StubModelClient::new([Ok(sp42_types::ModelCompletion { + text: r#"{"verdict":"SUPPORTED","quote":"good and loving"}"#.to_string(), + served_model: None, + })]); + let options = VerifyOptions { + repair_turn: false, + prefetched: Some(super::FetchedSource { + text: snippet.to_string(), + status: 200, + content_type: "text/plain".to_string(), + raw_html: None, + book_snippet: true, + }), + ..VerifyOptions::default() + }; + let outcome = block_on(verify_citation_use_site( + &fetch, + &model_client, + &FixedClock::new(1000), + &[model()], + &request( + "Matilda wished for loving parents", + "https://archive.org/details/matilda00dahl/page/42", + ), + None, + 0, + options, + )) + .expect("verifies"); + assert_eq!( + outcome.finding.verdict, + CitationVerdict::Judged(SupportLevel::Supported) + ); + assert_eq!(outcome.finding.grounding_status, GroundingStatus::Located); + + // ...while the same short body WITHOUT the snippet provenance still + // short-circuits as ShortBody with no model call. + let fetch = StubHttpClient::new([]); + let model_client = StubModelClient::new([]); + let options = VerifyOptions { + repair_turn: false, + prefetched: Some(super::FetchedSource { + text: snippet.to_string(), + status: 200, + content_type: "text/plain".to_string(), + raw_html: None, + book_snippet: false, + }), + ..VerifyOptions::default() + }; + let outcome = block_on(verify_citation_use_site( + &fetch, + &model_client, + &FixedClock::new(1000), + &[model()], + &request( + "Matilda wished for loving parents", + "https://example.org/short-page", + ), + None, + 0, + options, + )) + .expect("verifies"); + assert_eq!(outcome.finding.verdict, CitationVerdict::SourceUnavailable); + assert_eq!( + outcome.finding.unusable_reason, + Some(BodyUsabilityReason::ShortBody) + ); + assert!(outcome.votes.is_empty()); + + // The bypass is scoped to the floor: an EMPTY snippet stays unusable + // even with the book provenance flag. + let fetch = StubHttpClient::new([]); + let model_client = StubModelClient::new([]); + let options = VerifyOptions { + repair_turn: false, + prefetched: Some(super::FetchedSource { + text: String::new(), + status: 200, + content_type: "text/plain".to_string(), + raw_html: None, + book_snippet: true, + }), + ..VerifyOptions::default() + }; + let outcome = block_on(verify_citation_use_site( + &fetch, + &model_client, + &FixedClock::new(1000), + &[model()], + &request( + "Matilda wished for loving parents", + "https://archive.org/details/matilda00dahl", + ), + None, + 0, + options, + )) + .expect("verifies"); + assert_eq!(outcome.finding.verdict, CitationVerdict::SourceUnavailable); + assert!(outcome.votes.is_empty()); + } + #[test] fn law360_paywall_stub_short_circuits_no_partial() { // The #42 case: a large nav-chrome/paywall body with the claim's entity in a diff --git a/crates/sp42-citation/src/citation_finding.rs b/crates/sp42-citation/src/citation_finding.rs index 3f06a660..765ad142 100644 --- a/crates/sp42-citation/src/citation_finding.rs +++ b/crates/sp42-citation/src/citation_finding.rs @@ -293,6 +293,7 @@ mod tests { claim: "claim".to_string(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: 1, } } diff --git a/crates/sp42-citation/src/citation_page_report.rs b/crates/sp42-citation/src/citation_page_report.rs index 21ee41fc..aced9a04 100644 --- a/crates/sp42-citation/src/citation_page_report.rs +++ b/crates/sp42-citation/src/citation_page_report.rs @@ -250,8 +250,23 @@ fn finding_line(finding: &CitationFinding) -> String { None => String::new(), }; + let book = finding.book_scan.as_ref().map_or_else(String::new, |scan| { + use std::fmt::Write as _; + let mut extra = format!(" scan={}", scan.ocaid); + if let Some(page) = scan.scanned_page { + let _ = write!(extra, " scanned_page={page}"); + } + if let Some(cited) = &scan.cited_page { + let _ = write!(extra, " cited_page={cited}"); + } + if let Some(note) = &scan.note { + let _ = write!(extra, " note=\"{note}\""); + } + extra + }); + format!( - "#{ordinal} ref={ref_id} {verdict}{status} url={url}{archive} claim=\"{claim}\"", + "#{ordinal} ref={ref_id} {verdict}{status} url={url}{archive}{book} claim=\"{claim}\"", ordinal = finding.use_site_ordinal, verdict = format_verdict(finding.verdict), url = finding.provenance.url, @@ -349,6 +364,7 @@ mod tests { claim: claim.to_string(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: 1, } } @@ -499,6 +515,22 @@ mod tests { )); } + #[test] + fn finding_line_shows_book_scan_details() { + let mut report = sample_report(); + report.findings[0].book_scan = Some(crate::BookScanProvenance { + ocaid: "matilda00dahl".to_string(), + scanned_page: Some(32), + cited_page: Some("42".to_string()), + note: Some("cited page had no match; whole-book search".to_string()), + }); + let text = render_page_verification_text(&report); + assert!(text.contains( + "scan=matilda00dahl scanned_page=32 cited_page=42 \ + note=\"cited page had no match; whole-book search\"" + )); + } + #[test] fn book_line_reports_misses_and_failures_distinctly() { let mut report = sample_report(); diff --git a/crates/sp42-citation/src/lib.rs b/crates/sp42-citation/src/lib.rs index c8728eeb..5cc8446d 100644 --- a/crates/sp42-citation/src/lib.rs +++ b/crates/sp42-citation/src/lib.rs @@ -35,7 +35,8 @@ pub use citation::citoid::{ }; pub use citation::concurrency::map_with_concurrency; pub use citation::extract::{ - BlockFailure, CitationUseSite, ExtractOutcome, SkippedReason, SkippedRef, extract_use_sites, + BlockFailure, BookUseSite, CitationUseSite, ExtractOutcome, SkippedReason, SkippedRef, + extract_use_sites, }; pub use citation::locate_quote::{FuzzyLocate, locate_quote, locate_quote_fuzzy}; pub use citation::openlibrary::{ @@ -52,6 +53,12 @@ pub use citation::parsing::{ }; pub use citation::prompts::ClaimContext; pub use citation::prompts::{build_repair_prompt, build_verify_prompt}; +pub use citation::search_inside::{ + ARCHIVE_METADATA_BASE, BookGroundingPreparation, BookSnippetBody, ItemLocation, + SearchInsideMatch, SearchInsideResult, build_item_metadata_request, + build_search_inside_request, extract_ocaid, parse_item_metadata, parse_search_inside, + prepare_book_grounding, scan_deep_link, search_query, +}; pub use citation::segment::{Sentence, segment_sentences}; pub use citation::source_fetch::{html_to_text, looks_like_html, recover_wayback_body}; pub use citation::storage::{ @@ -64,11 +71,12 @@ pub use citation::urls::{ }; pub use citation::verdict::{CitationFindingKind, CitationVerdict, SupportLevel, Verdict}; pub use citation::verify::{ - CitationFinding, CitationVerificationRequest, FetchedSource, GroundingAssertion, - GroundingStatus, LocatedPassage, ModelVerdict, ModelVote, SourceProvenance, + BookScanProvenance, CitationFinding, CitationVerificationRequest, FetchedSource, + GroundingAssertion, GroundingStatus, LocatedPassage, ModelVerdict, ModelVote, SourceProvenance, SourceUnavailableReason, VerificationOutcome, VerifyModelInputs, VerifyOptions, - assemble_citation_finding, build_model_votes, execute_citation_verify, fetch_source, - is_groundable_support, sha256_hex, verify_citation_use_site, + assemble_citation_finding, book_scan_unavailable_outcome, book_searched_not_supported_outcome, + build_model_votes, execute_citation_verify, fetch_source, is_groundable_support, sha256_hex, + verify_citation_use_site, }; pub use citation::voting::{BinaryVote, NClassVote, PanelAgreement, binary_vote, n_class_vote}; pub use citation_finding::{ diff --git a/crates/sp42-cli/src/main.rs b/crates/sp42-cli/src/main.rs index 37c7b6e7..d3aec62a 100644 --- a/crates/sp42-cli/src/main.rs +++ b/crates/sp42-cli/src/main.rs @@ -915,6 +915,7 @@ fn prefetched_from_body(text: String) -> FetchedSource { status: 200, content_type: "text/plain".to_string(), raw_html: None, + book_snippet: false, } } @@ -1584,6 +1585,7 @@ mod verify_tests { claim: String::new(), preceding_context: Vec::new(), archive_of: None, + book_scan: None, schema_version: 1, } } diff --git a/crates/sp42-mcp/src/verify.rs b/crates/sp42-mcp/src/verify.rs index 3ca2f48e..40ca34ff 100644 --- a/crates/sp42-mcp/src/verify.rs +++ b/crates/sp42-mcp/src/verify.rs @@ -58,6 +58,7 @@ where // usability gate's structured-paywall check (grounding uses `text`). content_type: "text/plain".to_owned(), raw_html: None, + book_snippet: false, }); } let outcome = verify_citation_use_site( diff --git a/docs/STATUS.md b/docs/STATUS.md index 17d89a17..57b4aba3 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -76,14 +76,18 @@ PWA packaging and offline installability are now effectively complete for local (text/markdown/json), defaulting to the latest revision. `frwiki`, `enwiki`, and `testwiki` are registered. The browser Citations tab that renders the same report is in review (PR #81). -- book-citation grounding (PRD-0009, ADR-0018) has its read-only resolve lane: - validated book identifiers (ISBN/OCLC/LCCN/OLID) are extracted from cite-template - `data-mw`, the page report distinguishes "book identifier present" from plain - non-URL refs, and `verify-page` now resolves each book ref through the +- book-citation grounding (PRD-0009, ADR-0018) has its read-only resolve and + grounding lanes: validated book identifiers (ISBN/OCLC/LCCN/OLID) are extracted + from cite-template `data-mw`, `verify-page` resolves each book ref through the side-effect-free Open Library lookups (Books API catalog + Read API - exact-vs-similar scan availability), rendering a Books report section with - resolved records, honest misses, and scan status; search-inside grounding - (Layer 2) is the next slice + exact-vs-similar scan availability), and a resolved book with an exact-edition + scan is grounded against Internet Archive search-inside snippets — the snippet + body feeds the existing verdict panel (with a provenance-scoped short-body + bypass), cited-page-first with whole-book fallback, page-anchored deep links, + and the honest `not_supported` vs `SourceUnavailable` split; unresolved books + stay skipped with a refined reason and a Books report section shows every + resolution; the enrichment lane (Layer 3) awaits the Open Library + apply-contract ADR ## Current Verification From 8a9ac132ea131262310e80c66d68692491443c44 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 04:50:39 +0000 Subject: [PATCH 4/9] Record Open Library apply-path research (PRD-0009 Layer 3 prerequisite) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answers the open question blocking the apply-contract ADR: what write mechanism an ordinary operator account can actually use in production. Key findings (primary sources: openlibrary-client, infogami's API plugin, the addbook.py form handler, the Writing Bots policy, upstream issues): - The PRD's "REST save PUT is internal/localhost-only" premise was wrong in its reason, right in its conclusion: the PUT and /api/save_many DO run against production (the official client and the whole bot ecosystem use them), but infogami's can_write() gates every API write on membership in /usergroup/api or /usergroup/admin — an ordinary account gets 403 even for a single-record save. - Ordinary accounts write through the website form path (POST /books/OL...M/edit, SaveBookHelper): logged-in + per-record permission only, edition./work. field prefixes, _comment preserved, no CSRF token today, a v revision parameter but no server-side refuse-on-conflict (drift protection must be client-side). - The API usergroup is obtainable by a sanctioned human process (ask the admins; bot naming conventions for bulk accounts), and membership is publicly readable at /usergroup/api.json — a write-free capability probe. - Auth for either lane: POST /account/login with credentials or per-operator IA S3 keys -> session cookie. No published rate limits; the policy is an open upstream discussion. Implication recorded for the ADR: a two-lane apply contract — the authenticated form POST as the default lane every operator has, and the REST PUT as a privileged lane for API-usergroup members — both per-operator, both proposal-only until the ADR and a one-time live form spike land. PRD-0009's three localhost-only mentions corrected to the verified usergroup-gating mechanism. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- ...-07-08-open-library-apply-path-research.md | 159 ++++++++++++++++++ ...n-grounding-and-open-library-enrichment.md | 21 ++- 2 files changed, 171 insertions(+), 9 deletions(-) create mode 100644 docs/design-plans/2026-07-08-open-library-apply-path-research.md diff --git a/docs/design-plans/2026-07-08-open-library-apply-path-research.md b/docs/design-plans/2026-07-08-open-library-apply-path-research.md new file mode 100644 index 00000000..57ee1a24 --- /dev/null +++ b/docs/design-plans/2026-07-08-open-library-apply-path-research.md @@ -0,0 +1,159 @@ +# Open Library apply-path research (PRD-0009 Layer 3 prerequisite) + +**Date:** 2026-07-08 +**Author:** Luis Villa (researched by Claude Code) +**Status:** Research note — feeds the future Open Library apply-contract ADR +(PRD-0009 resolved Q6(b)). No write code exists or is enabled; Layers 1–2 +(ADR-0018) are unaffected. + +## The question + +PRD-0009's enrichment lane (Layer 3) needs a **production write mechanism an +ordinary operator account can actually use** before the apply-contract ADR can +be drafted. The PRD deferred with: "the documented REST save `PUT` path is +internal/localhost-only," flagging the mechanism as unknown. This note answers: +what can a normal Open Library account write, through what endpoint, with what +authentication, and under what policy? + +## Method + +Read the primary sources, not summaries: the official +`internetarchive/openlibrary-client` library (login + save implementation and +README), the **server-side permission code** (infogami `plugins/api/code.py`, +which implements the RESTful save and `save_many`), the website edit-form +handler (`openlibrary/plugins/upstream/addbook.py`, `SaveBookHelper`), the +Writing Bots policy page, and the open rate-limiting issues +(internetarchive/openlibrary#8534, #10585, plus the #11628 `save_many` 403 +report). + +## Findings + +### 1. The PRD's "localhost-only" premise is wrong — the real gate is a usergroup + +The RESTful save (`PUT /books/OL…M.json`, `PUT /authors/OL…A.json`) and the +bulk `POST /api/save_many` **do run against production +`https://openlibrary.org`** — the official `openlibrary-client` targets +production by default and the whole `openlibrary-bots` ecosystem writes +through it. The docs' localhost examples are dev-instance examples, not a +deployment restriction. + +The actual restriction is **server-side permission**: in infogami's API plugin +(`infogami/plugins/api/code.py`), *every* API write — the single-document PUT +and `save_many` alike — runs `can_write()`, which passes only when the +logged-in account is a member of **`/usergroup/api`** or `/usergroup/admin`. +An ordinary account gets `403 Permission Denied` (exactly what +openlibrary#11628 reports for `save_many`). There is no volume carve-out: the +machine-API lane is closed to ordinary accounts even for a single edit. + +Corrected statement for the PRD/ADR: *the REST save path is +production-real but usergroup-gated; it is not localhost-only.* The PRD's +conclusion (don't assume it) stands; its reason changes. + +### 2. Ordinary accounts write through the website form path + +Human editors — the wiki model the PRD leans on — save through +`POST /books/OL…M/edit` (handler `book_edit` → `SaveBookHelper`; same shape +for `/works/OL…W/edit`). Verified properties from the handler source: + +- **Permission: a logged-in account + per-record `web.ctx.site.can_write`** — + the ordinary editability every human editor already has, no usergroup. +- **Field contract:** flattened form fields with `edition.*` / `work.*` + prefixes (`SaveBookHelper.process_input` splits them); a work edit rides an + edition edit only when the work key matches. +- **Edit comment:** `_comment` field, committed with `action="edit-book"` — + attribution and comment survive exactly as PRD-0009 requires. +- **No CSRF token** in the current handler (nothing to mint, but also a + contract that could change under us — see risks). +- **Versioning:** the form carries a `v` (revision) parameter and the handler + loads that revision, but there is **no explicit refuse-on-conflict**; drift + protection must be SP42-side (compare the record's `revision`/`last_modified` + read at proposal time against a re-read immediately before apply, and refuse + if moved — the ADR-0010 discipline implemented by the client, not the + server). + +### 3. The API usergroup is obtainable, by a human process, framed for bots + +The sanctioned path into `/usergroup/api`: open a GitHub issue / write to the +admins describing what the account will do; bulk-cleanup accounts are expected +to be **separate accounts named `…Bot`**, and the recommended tooling is +`openlibrary-client`. Membership is public — `GET /usergroup/api.json` lists +members — so SP42 can **capability-probe without a write**: read the +usergroup, check whether the operator's key is in it. + +For SP42 this is a plausible *per-operator opt-in* (an operator doing +sustained enrichment asks for membership, possibly via a paired bot account), +not an assumption the product can make for every operator. + +### 4. Authentication (either lane): session cookie via `/account/login` + +`POST /account/login` accepts form-encoded username/password **or** JSON +Internet Archive **S3 keys** (`access`/`secret` — any account can mint them at +`archive.org/account/s3.php`); either yields the session cookie all writes +ride. S3 keys are the better operator story (no password storage, revocable), +are what `openlibrary-client` uses, and remain strictly **per-operator** +(PRD-0009 resolved Q1). + +### 5. Rate limits: none published; policy is an open upstream discussion + +There are no published write limits; formal rate-limit policy is an open +upstream issue (openlibrary#8534, #10585 — the only number in play is a +*proposal* to limit unidentified traffic). Etiquette expectations: identify +via User-Agent, keep frequency low. SP42's posture already covers this: the +guarded fetch edge's UA and pacing (ADR-0015), ≤3 concurrency on third-party +REST, and Layer 3 being operator-confirmed one-field-at-a-time makes writes +human-paced by construction. + +## Implication for the apply-contract ADR + +The ADR can now be drafted with a **two-lane apply contract**, both +per-operator: + +1. **Default lane — authenticated form POST** (`/books/OL…M/edit`): works for + every ordinary operator account today. SP42 renders the proposal, the + operator confirms, SP42 replays the record's current fields + the confirmed + field change + `_comment` through the form contract, with an SP42-side + refuse-on-drift (re-read `revision` before apply). This is exactly the + "controlled browser/form-backed submit path" PRD-0009 reserved. +2. **Privileged lane — REST `PUT` with `_comment`** for operators whose + account is in `/usergroup/api` (capability derived by reading + `/usergroup/api.json`, no probe write): cleaner JSON contract, the + officially recommended machine lane. + +Both lanes keep Layer 3 proposal-only until the ADR lands and the form-lane +spike (below) passes. + +## Risks / open items for the ADR + +- **The form contract is not a published API.** Field names come from + templates and can change without notice; the ADR must treat the form lane + as a versioned adapter with fixture-replay tests and a fail-closed posture + (any contract drift → refuse and fall back to proposal-only). +- **One-time live spike needed** (manual, testable account): confirm the form + POST's exact field set for a minimal single-field edit, confirm `_comment` + lands in history, confirm behavior when `v` is stale (merge vs overwrite) — + this decides how strict the SP42-side drift refusal must be. +- **CSRF could be added upstream** at any time; the adapter must read the + edit form first (GET) and replay any hidden fields it finds, rather than + posting blind. +- **Community posture:** SP42 assisted edits are operator-confirmed, not bulk; + still, telling the Open Library team about SP42 (the same channel as the + usergroup request) is cheap goodwill and may yield a sanctioned path — do it + before enabling any write lane (mirrors PRD-0008's frwiki-gate posture). +- `/api/import` remains out of scope (enrich-existing-only, resolved Q2). + +## Sources + +- `internetarchive/openlibrary-client` — `olclient/openlibrary.py` (login via + `POST /account/login` with credentials or S3 keys; `PUT /books/{olid}.json` + with `_comment`; `POST /api/save_many`; default host + `https://openlibrary.org`) and README (S3 key configuration). +- `internetarchive/infogami` — `infogami/plugins/api/code.py` (`can_write()` + gating both PUT and `save_many` on `/usergroup/api` ∪ `/usergroup/admin`). +- `internetarchive/openlibrary` — `openlibrary/plugins/upstream/addbook.py` + (`book_edit`/`work_edit`/`SaveBookHelper`: logged-in + per-record + `can_write`, `edition.*`/`work.*` field split, `_comment`, + `action="edit-book"`, `v` revision parameter, no CSRF token, no server-side + conflict refusal); issues #11628 (`save_many` 403 in production), #8534 and + #10585 (rate-limit policy open). +- Open Library *Writing Bots* documentation (API-usergroup grant process, bot + account naming, `openlibrary-client` recommendation). diff --git a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md index 7073b164..e20d9c5d 100644 --- a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md +++ b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md @@ -57,9 +57,10 @@ Yes, and it is two distinct systems: `/works/OL…W.json` / `/books/OL…M.json` by record id); the Read API is an availability view over readable/borrowable volumes, not the catalog lookup. Records are editable by logged-in Open Library users, but the machine apply path is - **not assumed here**: the documented REST save `PUT` path is internal/localhost-only, - and `/api/import` is an import surface that requires the appropriate production - privileges. The enrichment lane therefore requires a separate Open Library + **not assumed here**: the REST save `PUT` path works against production but is + usergroup-gated (writes require membership in the API/admin usergroups; an ordinary + account gets 403 — see the 2026-07-08 apply-path research note), and `/api/import` + is an import surface that requires the appropriate production privileges. The enrichment lane therefore requires a separate Open Library apply-contract ADR before writes are enabled. (The `/isbn/{isbn}.json` path is *not* purely read-only — it is documented under *Import by ISBN* and can import-on-miss — so the resolve lane @@ -244,8 +245,9 @@ Open Library record and SP42 holds **sourced** values that record is missing: path before writes are enabled: either a supported API path for operators with the required Open Library privileges/API usergroup, or a controlled browser/form-backed submit path that preserves SP42's exact proposal, confirmation, edit comment, and - drift checks. The generic REST save `PUT` path is **not** assumed, because Open - Library documents it as internal/localhost-only; `/api/import` remains out of scope + drift checks. The generic REST save `PUT` path is **not** assumed, because production gates it + on the API usergroup (an ordinary operator account gets 403; see the 2026-07-08 + apply-path research note); `/api/import` remains out of scope for enriching existing records. If no supported apply path is available, Layer 3 remains proposal-only and produces no Open Library write. Any enabled write is still under the operator's own per-operator Open Library identity, never a shared/baked-in @@ -424,8 +426,8 @@ Open Library / Internet Archive responses — no live network in tests (ADR-0009 rubber-stamp, which is the reason its provenance is shown inline. - **Credential and capability handling for Open Library.** The write lane needs the operator's Open Library identity (resolved Q1: per-operator, never a shared/baked-in - key) **and** a supported apply capability. A normal account session alone may not be - enough for machine writes: Open Library's documented REST save path is internal, and + key) **and** a supported apply capability. A normal account session alone is not + enough for machine writes: the REST save path requires API-usergroup membership, and import-style API writes require the appropriate privileges. Mitigation: the apply ADR must prove the chosen production path and capability gate; an operator without that path sees proposal-only output (the read-only Resolve/Ground lanes still work). @@ -487,8 +489,9 @@ they remain open to reviewer reaction until acceptance. transfers, but its **mechanism** is MediaWiki-specific (`WikitextNodeLocator` + `replacement_wikitext` + `baserevid` + wiki session/CSRF) and does **not** map to a field-level Open Library JSON change guarded by the OL record's own revision under an - OL identity. The ADR must also pick a supported apply path; Open Library's documented - REST save `PUT` is internal/localhost-only, and `/api/import` is out of scope here. If + OL identity. The ADR must also pick a supported apply path; Open Library's + REST save `PUT` is usergroup-gated in production (not usable by an ordinary + operator account), and `/api/import` is out of scope here. If no supported production apply path exists, Layer 3 stays proposal-only. **Not "reuse ADR-0010 as-is."** This matches the header's "a second ADR … against a non-wiki target." From 8b23b68e18fffff60916bf6691dad204bbbf5e4f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 06:27:59 +0000 Subject: [PATCH 5/9] Draft ADR-0019: Open Library apply contract (PRD-0009 Layer 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The apply-contract ADR the enrichment lane was waiting on, built on the 2026-07-08 apply-path research: - Two apply lanes selected by derived capability, never configuration: the authenticated edit-form POST every ordinary operator account has, and the REST PUT for operators in the API usergroup (membership read from the public /usergroup/api.json — a write-free probe). - Per-operator session via IA S3-key login, held under the ADR-0002 local-session posture; no shared identity. - ADR-0010's propose/confirm/refuse-on-drift discipline re-implemented client-side around the record revision, since upstream refuses nothing on conflict; one field per apply; full audit of every attempt. - The form lane is a versioned adapter that GETs the form first, replays hidden fields, and fails closed on any contract drift (proposal-only rather than a best-guess POST to a public catalog). - The write lane ships disabled behind an explicit enablement gate: a one-time live form spike, upstream courtesy contact, and the operator capability report. Cross-references updated: PRD-0009 spawned-ADRs header, the research note's status, and the STATUS.md book-lane bullet. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- docs/STATUS.md | 6 +- ...-07-08-open-library-apply-path-research.md | 10 +- .../adr/0019-open-library-apply-contract.md | 225 ++++++++++++++++++ ...n-grounding-and-open-library-enrichment.md | 6 +- 4 files changed, 237 insertions(+), 10 deletions(-) create mode 100644 docs/domains/references/adr/0019-open-library-apply-contract.md diff --git a/docs/STATUS.md b/docs/STATUS.md index 57b4aba3..db1b867d 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -86,8 +86,10 @@ PWA packaging and offline installability are now effectively complete for local bypass), cited-page-first with whole-book fallback, page-anchored deep links, and the honest `not_supported` vs `SourceUnavailable` split; unresolved books stay skipped with a refined reason and a Books report section shows every - resolution; the enrichment lane (Layer 3) awaits the Open Library - apply-contract ADR + resolution; the enrichment lane (Layer 3) has its + apply contract drafted (ADR-0019: form-lane + REST-lane applies behind a + derived capability, client-side refuse-on-drift, write lane disabled until + its enablement gate) and awaits implementation ## Current Verification diff --git a/docs/design-plans/2026-07-08-open-library-apply-path-research.md b/docs/design-plans/2026-07-08-open-library-apply-path-research.md index 57ee1a24..45c89a5f 100644 --- a/docs/design-plans/2026-07-08-open-library-apply-path-research.md +++ b/docs/design-plans/2026-07-08-open-library-apply-path-research.md @@ -2,9 +2,9 @@ **Date:** 2026-07-08 **Author:** Luis Villa (researched by Claude Code) -**Status:** Research note — feeds the future Open Library apply-contract ADR -(PRD-0009 resolved Q6(b)). No write code exists or is enabled; Layers 1–2 -(ADR-0018) are unaffected. +**Status:** Research note — the findings ADR-0019 (Open Library apply +contract, PRD-0009 resolved Q6(b)) is drafted from. No write code exists or +is enabled; Layers 1–2 (ADR-0018) are unaffected. ## The question @@ -105,8 +105,8 @@ human-paced by construction. ## Implication for the apply-contract ADR -The ADR can now be drafted with a **two-lane apply contract**, both -per-operator: +*(Adopted: ADR-0019 records this contract.)* The ADR can be drafted with a +**two-lane apply contract**, both per-operator: 1. **Default lane — authenticated form POST** (`/books/OL…M/edit`): works for every ordinary operator account today. SP42 renders the proposal, the diff --git a/docs/domains/references/adr/0019-open-library-apply-contract.md b/docs/domains/references/adr/0019-open-library-apply-contract.md new file mode 100644 index 00000000..31065a13 --- /dev/null +++ b/docs/domains/references/adr/0019-open-library-apply-contract.md @@ -0,0 +1,225 @@ +# ADR-0019: Open Library apply contract (operator-confirmed enrichment writes) + +**Status:** Proposed +**Date:** 2026-07-08 +**Author:** Luis Villa (drafted by Claude Code) + +Spawned by PRD-0009 resolved Q6(b): the **apply-contract** ADR for the +enrichment lane (Layer 3). ADR-0018 owns the read side (resolve + grounding); +this ADR owns how a confirmed field-level improvement to an existing Open +Library record actually reaches openlibrary.org — and under what gates it is +allowed to. It transfers ADR-0010's propose/confirm/refuse-on-drift +**discipline** to a non-MediaWiki target whose **mechanism** is entirely +different (no `WikitextNodeLocator`, no `baserevid`, no CSRF-token API). + +Grounding for every mechanism claim here is the 2026-07-08 apply-path +research note (`docs/design-plans/2026-07-08-open-library-apply-path-research.md`), +which read the server-side permission code rather than the docs' examples. + +## Context + +What the research established, in one paragraph: Open Library's machine write +API (`PUT /books/OL…M.json`, `POST /api/save_many`) runs against production +but is **usergroup-gated** — infogami's `can_write()` rejects any account not +in `/usergroup/api` or `/usergroup/admin` with a 403, even for a single-record +save. Ordinary accounts write through the **website form path** +(`POST /books/OL…M/edit`, `SaveBookHelper`), which requires only a logged-in +account with per-record permission, carries the edit comment as `_comment`, +currently has no CSRF token, and does **no server-side conflict refusal** (a +stale `v` revision parameter is loaded, not rejected). Authentication for +either lane is a session cookie from `POST /account/login`, which accepts +per-operator Internet Archive S3 keys. API-usergroup membership is granted by +a human process and is publicly readable at `/usergroup/api.json`. + +The contract below has to reconcile three constraints: PRD-0009's posture +(per-operator identity, operator confirms every field, enrich-existing-only), +ADR-0010's discipline (a proposal is inert; apply replays exactly what was +confirmed; drift refuses), and an upstream surface where the only +ordinary-account lane is a **form contract that is not a published API**. + +## Decision + +### 1. Two apply lanes, selected by derived capability — never configured by hand + +- **Form lane (default).** The apply executes as the website edit form does: + `GET /books/OL…M/edit` (or `/works/OL…W/edit`), parse the form, replay its + fields with the single confirmed field changed and `_comment` set, `POST` + back. This is the lane every ordinary operator account has today. +- **REST lane (privileged).** When the operator's account is a member of + `/usergroup/api`, the apply is a `PUT /books/OL…M.json` of the full current + record with the confirmed field changed and `_comment` attached — the + officially recommended machine lane, structurally cleaner than form replay. +- **Capability is derived, not declared:** SP42 reads the public + `/usergroup/api.json` membership list and checks the operator's key — a + write-free probe. Members get the REST lane; everyone else gets the form + lane. There is no configuration knob to force a lane, so a misconfiguration + can never route a write through a path the account cannot use. + +### 2. Per-operator session via S3-key login; no shared identity, ever + +- The operator authenticates with **their own** Internet Archive S3 key pair + (mintable by any account), exchanged at `POST /account/login` for the + session cookie both lanes ride. S3 keys avoid password storage and are + independently revocable; this is PRD-0009 resolved Q1 made concrete. +- The Open Library session is held per-operator server-side, alongside the + operator's wiki session, under the same localhost dev-auth posture as + ADR-0002 — it is never baked into config, never shared between operators, + and its absence simply means the enrichment lane is proposal-only for that + operator. + +### 3. ADR-0010's discipline, re-implemented client-side around the record revision + +The upstream server refuses nothing on conflict, so SP42 enforces the +discipline itself: + +- **Propose:** the proposal is computed against a record read + (`/books/OL…M.json`) whose `revision` is **pinned into the proposal**. A + proposal names exactly one field, its current value, its proposed value, + the value's verbatim source (per PRD-0009's provenance rules), and the edit + comment. +- **Confirm:** the operator confirms that exact proposal; the confirmation + token binds to the proposal's content hash (same replay-what-was-confirmed + rule as ADR-0010 — apply never recomputes the change). +- **Apply:** immediately before writing, SP42 re-reads the record. If + `revision` moved since the proposal, the apply **refuses** and offers + re-proposal against the current record. Only an unmoved revision proceeds + to the lane's submit. This is `baserevid`-by-hand: weaker than a server-side + conditional write (a race in the read→submit window is possible), and + accepted as such — the window is milliseconds, the stakes are a wiki-model + record with full history/revert, and the alternative is no ordinary-account + lane at all. +- **One field per apply.** A confirmed proposal writes exactly one field; a + record needing three improvements is three proposals. This keeps every + Open Library history entry small, legible, and individually revertable, + and keeps the operator's confirmation meaningful. + +### 4. The form lane is a versioned adapter that fails closed + +The form contract belongs to Open Library's templates, not to any API +promise. Accordingly: + +- The adapter **always GETs the edit form first** and replays every field it + finds — including any hidden fields that appear in the future (this is also + the CSRF-proofing: if a token is added upstream, the adapter carries it + automatically rather than posting blind). +- The adapter validates the fetched form against its **expected contract + version** (the `edition.*`/`work.*` field families it knows how to edit). + Any surprise — a missing expected field, an unparseable form, a submit + response that is not the success shape — is **contract drift**: the apply + refuses, reports "form contract changed; enrichment is proposal-only until + the adapter is updated", and *never* submits a best-guess POST. A wrong + write to a public catalog is strictly worse than no write. +- Adapter behavior is covered by fixture-replay tests (recorded form HTML + + submit responses; ADR-0009 discipline, no live network in tests), with the + fixtures refreshed by the enablement spike (Decision 6). + +### 5. House transport, etiquette, and audit + +- Both lanes are pure `build_*`/`parse_*` pairs over the injected + `HttpClient`, executed through the guarded `sp42-fetch` source face + (ADR-0015): identified User-Agent, timeouts, caps, no proxy surprises. + Writes are POST/PUT, so they use the fetch edge's session-bearing execution + path (as MediaWiki actions do), not the GET/HEAD-only citation fetcher. +- Pacing needs no new machinery: applies are operator-confirmed + one-field-at-a-time, so the write rate is human. Reads around the apply + (capability probe, form GET, drift re-read) stay within the existing ≤ 3 + third-party concurrency posture. +- Every apply — attempted, refused, succeeded — lands in the existing action + history/audit trail with the proposal it replayed, the lane used, the + record revision it was applied against, and the resulting response, so an + Open Library edit is as reconstructable as a wiki edit. + +### 6. The write lane ships disabled, behind an explicit enablement gate + +Layer 3 remains **proposal-only** until all of the following are recorded in +the enabling PR (this is the PRD-0009 DoD item made operational): + +1. **Live form spike (one-time, manual, a test account):** confirm the form + lane's minimal single-field edit end-to-end against production — the exact + field set the form requires, `_comment` visible in the record's history, + and the observed behavior of a stale `v` submit (merge vs overwrite), + which calibrates how paranoid the drift refusal must stay. Record the + target record, before/after, and the resulting revision. +2. **Upstream courtesy:** Open Library's team has been told what SP42 does + (assisted, operator-confirmed, one field at a time) via their documented + contact channel — the same channel as an API-usergroup request. If they + object or require a different path, the lane stays disabled (the PRD-0008 + frwiki-gate posture). +3. **Capability report:** the operator-facing capability panel shows which + lane (if any) the operator's session has, and an operator with no session + or no usable lane sees proposal-only output with zero write affordances. + +Until the gate passes, everything above exists as mechanism + tests only. + +## Consequences + +- Every ordinary operator gets a real apply path (the form lane) without + asking Open Library for anything; operators who obtain API-usergroup + membership transparently upgrade to the cleaner REST lane. No third + configuration state exists. +- SP42 takes on a template-coupled adapter with a standing maintenance + liability: upstream form changes turn the write lane off (fail-closed) + until the adapter is updated. This is the deliberate price of writing as an + ordinary account; the drift refusal converts "silently broken" into + "honestly disabled". +- The refuse-on-drift guarantee is client-side and therefore has a small race + window neither lane can close (no conditional-write primitive upstream). + Accepted: wiki-model history bounds the damage, and the one-field rule + bounds the blast radius. +- The full-record replay in both lanes (form fields / JSON body) means an + apply rewrites the whole record with one field changed — so the drift + re-read is load-bearing, not cosmetic: applying over a moved revision would + silently revert someone else's edit. The refusal exists precisely to make + that impossible. +- Credential surface grows by one per-operator secret (the S3 key pair), + handled under the existing dev-auth storage posture; revocation is the + operator's kill switch. + +## Alternatives considered + +- **REST lane only (require API-usergroup membership).** Cleanest contract, + but it makes enrichment unusable for every ordinary operator and turns a + courtesy process into a hard onboarding dependency. Rejected as the sole + lane; kept as the privileged lane. +- **Form lane only.** Avoids dual mechanisms, but deliberately ignores the + officially recommended machine path for accounts that have it, and couples + *all* writes to the template contract. Rejected; the capability probe makes + dual lanes cheap. +- **Ask Open Library for a sanctioned SP42-wide bot account.** Violates + PRD-0009 resolved Q1 (per-operator identity, never a shared key) and + centralizes attribution away from the human who confirmed the edit. + Rejected. +- **Browser automation (drive the real edit page headlessly).** Survives + template drift better than form replay, but imports a browser runtime into + the server, is far harder to fixture-test deterministically, and still has + no conflict primitive. Rejected while the form contract stays this simple; + it is the fallback direction if upstream adds heavy client-side machinery. +- **Skip drift checking (history makes everything revertable).** Rejected: + full-record replay over a moved revision silently reverts third-party + edits; "revertable" does not excuse causing the mess (ADR-0010's refusal + rule exists for exactly this). +- **`/api/import` for missing editions.** Out of scope by PRD-0009 resolved + Q2 (enrich-existing-only); restated here so the apply lane is never bent + into an import lane. + +## Out of scope / non-goals + +- **What is proposable** (field provenance rules, the synthesized-description + gate and its rich-context requirement) — that is PRD-0009 Layer 3 itself; + this ADR only carries a confirmed proposal to the site. +- **archive.org item-metadata writes** (IA-S3 JSON-Patch) — excluded by + PRD-0009's scope boundary. +- **Author-record enrichment** — Open Library's own Wikidata integration owns + author-level sync (PRD-0009 de-duplication rule). +- **Work/edition creation** — no lane here may create a record; a resolve + miss stays a miss (ADR-0018 Decision 2). + +## References + +- PRD-0009 (Layer 3, resolved Q1/Q2/Q6), ADR-0010 (propose/confirm + discipline), ADR-0018 (read contract), ADR-0015 (fetch edge), ADR-0002 + (per-operator local session posture), ADR-0009 (fixture replay). +- `docs/design-plans/2026-07-08-open-library-apply-path-research.md` — the + primary-source findings every mechanism claim above rests on (infogami + `can_write()` gating; `SaveBookHelper` form contract; `/account/login` S3 + login; `/usergroup/api.json` capability probe; upstream rate-limit issues). diff --git a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md index e20d9c5d..137d0b8c 100644 --- a/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md +++ b/docs/domains/references/prd/0009-book-citation-grounding-and-open-library-enrichment.md @@ -7,9 +7,9 @@ **Discussion:** design conversation 2026-07-01 (Internet Archive editable book metadata → SP42 integration); no tracking issue yet. **Spawned ADRs:** ADR-0018 (Open Library / Internet Archive read contract: -resolve + full-text grounding). If the enrichment lane ships, a second ADR -reusing ADR-0010's propose/confirm apply discipline against a non-wiki target -is still expected. +resolve + full-text grounding) and ADR-0019 (Open Library apply contract — +ADR-0010's propose/confirm discipline against the non-wiki target, two apply +lanes behind an enablement gate). ## Scope boundary From 1e6a9023550269a489fd2631a0ee9e41f1ca9c3e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 15:26:28 +0000 Subject: [PATCH 6/9] Amend ADR-0019: lane selection by 403-fallback, not usergroup probe Review outcome: the usergroup-list derivation guarded against a failure that is already benign (a pre-mutation 403) while adding a second unofficial contract that mirrors server permission internals. Replaced with the server's own answer: attempt the REST lane, fall back to the form lane on 403 within the same apply, cache the discovered lane per operator session. Members never pay an extra request; non-members pay one refused request per session; the 403 handler is the permanent correctness mechanism and the cache only an optimization, so no state can be wrong. The fallback re-runs the drift re-read before the form submit. Also strengthened the fail-closed form adapter with a post-apply read-back (verify the confirmed field holds the proposed value; surface anything else loudly), updated the capability-report wording for discovery-on-first-apply, and recorded both rejected selection mechanisms (membership-list derivation, operator config knob) in Alternatives. Research note's implication section updated to match. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- ...-07-08-open-library-apply-path-research.md | 9 ++- .../adr/0019-open-library-apply-contract.md | 77 ++++++++++++++----- 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/docs/design-plans/2026-07-08-open-library-apply-path-research.md b/docs/design-plans/2026-07-08-open-library-apply-path-research.md index 45c89a5f..a682fd65 100644 --- a/docs/design-plans/2026-07-08-open-library-apply-path-research.md +++ b/docs/design-plans/2026-07-08-open-library-apply-path-research.md @@ -115,9 +115,12 @@ human-paced by construction. refuse-on-drift (re-read `revision` before apply). This is exactly the "controlled browser/form-backed submit path" PRD-0009 reserved. 2. **Privileged lane — REST `PUT` with `_comment`** for operators whose - account is in `/usergroup/api` (capability derived by reading - `/usergroup/api.json`, no probe write): cleaner JSON contract, the - officially recommended machine lane. + account is in `/usergroup/api`: cleaner JSON contract, the officially + recommended machine lane. (As adopted, ADR-0019 selects the lane from the + server's own pre-mutation 403 answer with per-session caching, rather + than reading the `/usergroup/api.json` membership list — the list read + remains a valid display-only capability hint, but the write path never + depends on it.) Both lanes keep Layer 3 proposal-only until the ADR lands and the form-lane spike (below) passes. diff --git a/docs/domains/references/adr/0019-open-library-apply-contract.md b/docs/domains/references/adr/0019-open-library-apply-contract.md index 31065a13..76135abc 100644 --- a/docs/domains/references/adr/0019-open-library-apply-contract.md +++ b/docs/domains/references/adr/0019-open-library-apply-contract.md @@ -39,21 +39,32 @@ ordinary-account lane is a **form contract that is not a published API**. ## Decision -### 1. Two apply lanes, selected by derived capability — never configured by hand - -- **Form lane (default).** The apply executes as the website edit form does: - `GET /books/OL…M/edit` (or `/works/OL…W/edit`), parse the form, replay its - fields with the single confirmed field changed and `_comment` set, `POST` - back. This is the lane every ordinary operator account has today. -- **REST lane (privileged).** When the operator's account is a member of - `/usergroup/api`, the apply is a `PUT /books/OL…M.json` of the full current - record with the confirmed field changed and `_comment` attached — the - officially recommended machine lane, structurally cleaner than form replay. -- **Capability is derived, not declared:** SP42 reads the public - `/usergroup/api.json` membership list and checks the operator's key — a - write-free probe. Members get the REST lane; everyone else gets the form - lane. There is no configuration knob to force a lane, so a misconfiguration - can never route a write through a path the account cannot use. +### 1. Two apply lanes, selected by the server's own answer — no knob, no probe + +- **Form lane (universal).** The apply executes as the website edit form + does: `GET /books/OL…M/edit` (or `/works/OL…W/edit`), parse the form, + replay its fields with the single confirmed field changed and `_comment` + set, `POST` back. This is the lane every ordinary operator account has + today. +- **REST lane (privileged).** For an account in the API usergroup, the apply + is a `PUT /books/OL…M.json` of the full current record with the confirmed + field changed and `_comment` attached — the officially recommended machine + lane, structurally cleaner than form replay. +- **Lane selection is the server's own answer, cached per session.** An + apply attempts the REST lane first. Infogami's `can_write()` refuses a + non-member with a 403 **before any processing** (verified in the permission + code), so the refusal is a side-effect-free capability answer straight from + the only authoritative source. On a 403 the *same* apply falls back to the + form lane and the operator's session caches "form lane"; a member's first + `PUT` simply succeeds and caches "REST lane". Members never pay an extra + request; non-members pay one refused request per login session; and there + is no state that can be *wrong* — a stale cache in either direction is + benign (the form lane works for every account, and a revoked REST lane + 403s into the same fallback and re-caches). The 403 handler is the + permanent correctness mechanism; the cache is only an optimization on top + of it. There is no configuration knob and no reading of the + `/usergroup/api.json` membership list on the write path (see Alternatives + for both rejections). ### 2. Per-operator session via S3-key login; no shared identity, ever @@ -83,7 +94,9 @@ discipline itself: - **Apply:** immediately before writing, SP42 re-reads the record. If `revision` moved since the proposal, the apply **refuses** and offers re-proposal against the current record. Only an unmoved revision proceeds - to the lane's submit. This is `baserevid`-by-hand: weaker than a server-side + to the lane's submit. A REST→form fallback within one apply (Decision 1) + **re-runs the drift re-read** before the form submit — the refusal window + restarts with the lane. This is `baserevid`-by-hand: weaker than a server-side conditional write (a race in the read→submit window is possible), and accepted as such — the window is milliseconds, the stakes are a wiki-model record with full history/revert, and the alternative is no ordinary-account @@ -109,6 +122,12 @@ promise. Accordingly: refuses, reports "form contract changed; enrichment is proposal-only until the adapter is updated", and *never* submits a best-guess POST. A wrong write to a public catalog is strictly worse than no write. +- After a form submit, the adapter **reads the record back** + (`/books/OL…M.json`) and verifies the confirmed field now holds the + proposed value, recording the new revision in the audit entry. A read-back + that shows anything else — the field unchanged, or a different value — is + surfaced loudly as an apply failure (and, if other fields moved, as a + suspected adapter defect), never silently logged as success. - Adapter behavior is covered by fixture-replay tests (recorded form HTML + submit responses; ADR-0009 discipline, no live network in tests), with the fixtures refreshed by the enablement spike (Decision 6). @@ -145,9 +164,11 @@ the enabling PR (this is the PRD-0009 DoD item made operational): contact channel — the same channel as an API-usergroup request. If they object or require a different path, the lane stays disabled (the PRD-0008 frwiki-gate posture). -3. **Capability report:** the operator-facing capability panel shows which - lane (if any) the operator's session has, and an operator with no session - or no usable lane sees proposal-only output with zero write affordances. +3. **Capability report:** the operator-facing capability panel shows the + Open Library session state and the lane the session has discovered + ("undecided — determined at first apply" before any write), and an + operator with no session sees proposal-only output with zero write + affordances. Until the gate passes, everything above exists as mechanism + tests only. @@ -155,8 +176,8 @@ Until the gate passes, everything above exists as mechanism + tests only. - Every ordinary operator gets a real apply path (the form lane) without asking Open Library for anything; operators who obtain API-usergroup - membership transparently upgrade to the cleaner REST lane. No third - configuration state exists. + membership transparently use the cleaner REST lane from their first apply. + No lane configuration exists at all, so no lane configuration can be wrong. - SP42 takes on a template-coupled adapter with a standing maintenance liability: upstream form changes turn the write lane off (fail-closed) until the adapter is updated. This is the deliberate price of writing as an @@ -177,6 +198,20 @@ Until the gate passes, everything above exists as mechanism + tests only. ## Alternatives considered +- **Derive the lane by reading `/usergroup/api.json`** (this ADR's first + draft). Rejected in review: it adds a second unofficial contract to + maintain and mirrors server permission internals (the api ∪ admin union in + `can_write()`) that can drift silently, while guarding against a failure — + routing a write through a lane the account cannot use — that is already + benign (a clean pre-mutation 403). The server's own refusal is the only + answer that cannot go stale. +- **An operator-set lane configuration knob.** Considered in review: + maximally transparent and avoids even the one refused request, but it adds + a knob, an operator-facing concept ("are you in the API usergroup?"), and + a "config says REST, server says 403" error path — all of which the + 403-fallback deletes rather than documents. The 403 handler must exist + regardless (permissions can be revoked upstream), and once it exists the + knob buys nothing. - **REST lane only (require API-usergroup membership).** Cleanest contract, but it makes enrichment unusable for every ordinary operator and turns a courtesy process into a hard onboarding dependency. Rejected as the sole From 811b020ddaa4008885b0fd83ce7f725638221b00 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 15:50:13 +0000 Subject: [PATCH 7/9] Implement the ADR-0019 apply mechanism (PRD-0009 Layer 3, writes disabled) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The enrichment lane exists as mechanism + fixture tests only: no server route or CLI surface performs a write, and ENRICHMENT_WRITE_LANE_ENABLED is the constant (false, with a tripwire test) any future surface must check per the ADR's enablement gate. - sp42-citation enrich: pure proposal computation. Deterministic ISBN-completion candidates (ISBN-13 from ISBN-10 and back, checksum arithmetic on values the record or citation already carries — never a verbatim relay of wiki content); raw-record parsing that pins the revision; EnrichmentProposal with the replacement value, provenance, comment, and the content hash a confirmation binds to. A gap that is closed on the raw record yields no proposal. - sp42-citation openlibrary_apply: the two-lane apply. S3-key login -> session cookie; REST PUT carrying the full updated record + _comment; the form-lane adapter that GETs the edit form, parses it fail-closed (unknown widgets, ambiguous selects, a missing target or _comment slot, or an occupied gap all refuse as contract drift), replays every field with one changed, and checks the redirect success shape; the executor with drift re-read, 403 fallback that restarts the drift window and caches the discovered lane per session, and a post-apply read-back that verifies the exact proposed value landed before reporting success. - Books report: resolved entries now list the read-only enrichment candidates (propose=isbn_13:...), PRD-0009's proposal-listing surface. Covered by tests: lane discovery and caching (no PUT after a cached form lane), refusal-before-write on a moved revision, contract-drift refusal with nothing posted, read-back mismatch surfacing as failure, and the complete-record-yields-no-proposal rule. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- crates/sp42-citation/src/citation.rs | 2 + crates/sp42-citation/src/citation/enrich.rs | 419 ++++++++ .../sp42-citation/src/citation/openlibrary.rs | 7 + .../src/citation/openlibrary_apply.rs | 920 ++++++++++++++++++ crates/sp42-citation/src/citation/page.rs | 9 +- .../sp42-citation/src/citation_page_report.rs | 22 +- crates/sp42-citation/src/lib.rs | 11 + docs/STATUS.md | 11 +- 8 files changed, 1394 insertions(+), 7 deletions(-) create mode 100644 crates/sp42-citation/src/citation/enrich.rs create mode 100644 crates/sp42-citation/src/citation/openlibrary_apply.rs diff --git a/crates/sp42-citation/src/citation.rs b/crates/sp42-citation/src/citation.rs index 022c2717..eb522948 100644 --- a/crates/sp42-citation/src/citation.rs +++ b/crates/sp42-citation/src/citation.rs @@ -18,9 +18,11 @@ pub mod body_classifier; pub mod citoid; pub mod concurrency; +pub mod enrich; pub mod extract; pub mod locate_quote; pub mod openlibrary; +pub mod openlibrary_apply; pub mod page; pub mod parsing; pub mod prompts; diff --git a/crates/sp42-citation/src/citation/enrich.rs b/crates/sp42-citation/src/citation/enrich.rs new file mode 100644 index 00000000..f36397c1 --- /dev/null +++ b/crates/sp42-citation/src/citation/enrich.rs @@ -0,0 +1,419 @@ +//! Open Library enrichment proposals (PRD-0009 Layer 3, ADR-0019). +//! +//! Pure proposal computation — no I/O, no writes. Two stages: +//! +//! 1. **Candidates** ([`enrichment_candidates`]): field-level gaps in a +//! resolved edition that SP42 can fill from a *deterministic, named +//! source*. The MVP proposes only identifier completion — an ISBN-13 +//! derived from an ISBN-10 (and vice versa for `978-` ISBN-13s), the +//! conversion being pure arithmetic on a value the record or the citation +//! already carries verbatim. Fields needing external sourced context +//! (subjects, cover, description) are deliberately not produced here; the +//! synthesized description in particular is gated on rich Wikidata +//! book-level context per PRD-0009 resolved Q3 and is future work. +//! 2. **Proposals** ([`propose_from_candidate`]): a candidate bound to a raw +//! record read — the record key and `revision` pinned in (ADR-0019 +//! Decision 3), the exact replacement value for the field, and the edit +//! comment. A proposal is inert data; only the apply mechanism +//! ([`crate::citation::openlibrary_apply`]) can carry one to the site, +//! and only after operator confirmation bound to [`EnrichmentProposal::content_hash`]. +//! +//! A candidate whose gap turns out to be closed on the raw record (the +//! Books API view and the raw record can drift) yields **no** proposal — a +//! record already complete is never touched (a PRD-0009 `DoD` item). + +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use crate::citation::openlibrary::OpenLibraryEdition; +use crate::citation::verify::sha256_hex; +use crate::wikitext_editor::BookIdentifier; + +/// A field-level enrichment candidate: what could be added, from where. +/// Display-ready strings — the report's read-only proposal listing +/// (PRD-0009's "read-only proposal listing" surface) renders these directly. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EnrichmentCandidate { + /// The raw-record field the candidate fills (e.g. `isbn_13`). + pub field: String, + /// The proposed value, verbatim. + pub proposed: String, + /// The named source the value traces to, human-readable + /// (e.g. `derived from ISBN-10 0140328726 (record)`). + pub source: String, +} + +/// The ISBN-13 for an ISBN-10: `978` + the nine data digits + the EAN-13 +/// check digit. Pure arithmetic; `None` for anything that is not a +/// checksum-valid ISBN-10. +#[must_use] +pub fn isbn13_from_isbn10(isbn10: &str) -> Option { + let BookIdentifier::Isbn(valid) = BookIdentifier::isbn(isbn10)? else { + return None; + }; + if valid.len() != 10 { + return None; + } + let mut digits = String::with_capacity(13); + digits.push_str("978"); + digits.push_str(&valid[..9]); + let check = ean13_check_digit(digits.as_bytes())?; + digits.push(char::from(b'0' + check)); + Some(digits) +} + +/// The ISBN-10 for a `978`-prefixed ISBN-13: the nine data digits + the +/// mod-11 check digit (which can be `X`). `None` for a non-`978` prefix +/// (ISBN-979 books have no ISBN-10 form) or an invalid ISBN-13. +#[must_use] +pub fn isbn10_from_isbn13(isbn13: &str) -> Option { + let BookIdentifier::Isbn(valid) = BookIdentifier::isbn(isbn13)? else { + return None; + }; + if valid.len() != 13 || !valid.starts_with("978") { + return None; + } + let mut digits: String = valid[3..12].to_string(); + let check = isbn10_check_digit(digits.as_bytes())?; + digits.push(check); + Some(digits) +} + +/// EAN-13 check digit over the first 12 digits. +fn ean13_check_digit(first_twelve: &[u8]) -> Option { + if first_twelve.len() != 12 || !first_twelve.iter().all(u8::is_ascii_digit) { + return None; + } + let sum: u32 = first_twelve + .iter() + .enumerate() + .map(|(i, b)| u32::from(b - b'0') * if i % 2 == 0 { 1 } else { 3 }) + .sum(); + Some(u8::try_from((10 - (sum % 10)) % 10).expect("mod 10 fits u8")) +} + +/// ISBN-10 check digit (mod 11, `X` for 10) over the first nine digits. +fn isbn10_check_digit(first_nine: &[u8]) -> Option { + if first_nine.len() != 9 || !first_nine.iter().all(u8::is_ascii_digit) { + return None; + } + let sum: u32 = first_nine + .iter() + .enumerate() + .map(|(i, b)| (10 - u32::try_from(i).unwrap_or(0)) * u32::from(b - b'0')) + .sum(); + let check = (11 - (sum % 11)) % 11; + Some(if check == 10 { + 'X' + } else { + char::from(b'0' + u8::try_from(check).expect("mod 11 fits u8")) + }) +} + +/// Compute the enrichment candidates for a resolved edition (PRD-0009 +/// Layer 3, identifier completion only). `cited` is the citation's own +/// validated identifiers — a source SP42 already trusts for resolution. +/// A record that already carries both ISBN forms yields no candidates. +#[must_use] +pub fn enrichment_candidates( + edition: &OpenLibraryEdition, + cited: &[BookIdentifier], +) -> Vec { + let cited_isbns = cited.iter().filter_map(|identifier| match identifier { + BookIdentifier::Isbn(value) => Some(value.as_str()), + _ => None, + }); + + // Known ISBN-10s / ISBN-13s, record values first (prefer the record's own + // data as the derivation source), then the citation's. + let mut known_10: Vec<(&str, &str)> = Vec::new(); + let mut known_13: Vec<(&str, &str)> = Vec::new(); + for value in &edition.isbn_10 { + known_10.push((value.as_str(), "record")); + } + for value in &edition.isbn_13 { + known_13.push((value.as_str(), "record")); + } + for value in cited_isbns { + match value.len() { + 10 => known_10.push((value, "citation")), + 13 => known_13.push((value, "citation")), + _ => {} + } + } + + let mut candidates = Vec::new(); + if edition.isbn_13.is_empty() + && let Some((converted, source_value, origin)) = known_10 + .iter() + .find_map(|(value, origin)| isbn13_from_isbn10(value).map(|c| (c, *value, *origin))) + { + candidates.push(EnrichmentCandidate { + field: "isbn_13".to_string(), + proposed: converted, + source: format!("derived from ISBN-10 {source_value} ({origin})"), + }); + } + if edition.isbn_10.is_empty() + && let Some((converted, source_value, origin)) = known_13 + .iter() + .find_map(|(value, origin)| isbn10_from_isbn13(value).map(|c| (c, *value, *origin))) + { + candidates.push(EnrichmentCandidate { + field: "isbn_10".to_string(), + proposed: converted, + source: format!("derived from ISBN-13 {source_value} ({origin})"), + }); + } + candidates +} + +/// A raw Open Library record read (`/books/OL…M.json`): the revision the +/// drift discipline pins against, plus the full document for verbatim +/// replay by the apply lanes. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct OpenLibraryRecord { + /// Record key, e.g. `/books/OL7826547M`. + pub key: String, + /// The record revision this read observed. + pub revision: u64, + /// The full raw document. + pub raw: Value, +} + +/// Parse a raw record read. `None` for an unparseable body or one missing +/// the key/revision the discipline depends on. +#[must_use] +pub fn parse_record(body: &[u8]) -> Option { + let raw: Value = serde_json::from_slice(body).ok()?; + let key = raw.get("key")?.as_str()?.to_string(); + let revision = raw.get("revision")?.as_u64()?; + Some(OpenLibraryRecord { key, revision, raw }) +} + +/// One confirmed-or-confirmable field change, pinned to the record revision +/// it was computed against (ADR-0019 Decision 3). Inert data: nothing here +/// performs I/O, and the apply mechanism replays exactly this. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EnrichmentProposal { + /// The record the proposal targets, e.g. `/books/OL7826547M`. + pub record_key: String, + /// The revision the proposal was computed against; apply refuses if the + /// record has moved past it. + pub base_revision: u64, + /// The raw-record field the proposal sets. + pub field: String, + /// The field's current raw value (`None` = absent), echoed so the + /// operator sees exactly what changes. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub current: Option, + /// The complete replacement value for the field (not a delta) — the + /// apply writes exactly this. + pub proposed: Value, + /// The named source of the proposed value (provenance, shown to the + /// operator). + pub source: String, + /// The edit comment the apply carries as `_comment`. + pub comment: String, +} + +impl EnrichmentProposal { + /// Content hash the operator's confirmation binds to (ADR-0019 + /// Decision 3): any change to what would be written changes the hash, + /// so a confirmation can never authorize anything but this proposal. + #[must_use] + pub fn content_hash(&self) -> String { + let identity = serde_json::json!({ + "record_key": self.record_key, + "base_revision": self.base_revision, + "field": self.field, + "proposed": self.proposed, + "comment": self.comment, + }); + sha256_hex(identity.to_string().as_bytes()) + } + + /// The single-value string form of the proposed value, for the form + /// lane's input field. `None` when the value has no such form (the form + /// lane then refuses rather than guessing). + #[must_use] + pub fn proposed_form_value(&self) -> Option { + match &self.proposed { + Value::String(value) => Some(value.clone()), + Value::Array(values) if values.len() == 1 => { + values[0].as_str().map(ToString::to_string) + } + _ => None, + } + } +} + +/// Bind a candidate to a raw record read, re-checking the gap against the +/// raw document (the Books API view can lag). `None` when the field is +/// already populated — a closed gap is never re-proposed. +#[must_use] +pub fn propose_from_candidate( + record: &OpenLibraryRecord, + candidate: &EnrichmentCandidate, +) -> Option { + let current = record.raw.get(&candidate.field); + let occupied = current.is_some_and(|value| match value { + Value::Array(items) => !items.is_empty(), + Value::Null => false, + Value::String(s) => !s.trim().is_empty(), + _ => true, + }); + if occupied { + return None; + } + Some(EnrichmentProposal { + record_key: record.key.clone(), + base_revision: record.revision, + field: candidate.field.clone(), + current: current.cloned(), + proposed: Value::Array(vec![Value::String(candidate.proposed.clone())]), + source: candidate.source.clone(), + comment: format!( + "Add {field} {value} ({source}) — SP42 assisted edit, operator-confirmed", + field = candidate.field, + value = candidate.proposed, + source = candidate.source, + ), + }) +} + +#[cfg(test)] +mod tests { + use super::{ + EnrichmentCandidate, OpenLibraryRecord, enrichment_candidates, isbn10_from_isbn13, + isbn13_from_isbn10, parse_record, propose_from_candidate, + }; + use crate::citation::openlibrary::OpenLibraryEdition; + use crate::wikitext_editor::BookIdentifier; + use serde_json::json; + + #[test] + fn isbn_conversion_round_trips_with_correct_check_digits() { + // Matilda: ISBN-10 0140328726 <-> ISBN-13 9780140328721. + assert_eq!( + isbn13_from_isbn10("0-14-032872-6").as_deref(), + Some("9780140328721") + ); + assert_eq!( + isbn10_from_isbn13("978-0-14-032872-1").as_deref(), + Some("0140328726") + ); + // An X check digit survives the 13 -> 10 direction. + let thirteen = isbn13_from_isbn10("080442957X").expect("valid isbn-10"); + assert_eq!(isbn10_from_isbn13(&thirteen).as_deref(), Some("080442957X")); + // Invalid inputs and 979-prefixed ISBN-13s convert to nothing. + assert_eq!(isbn13_from_isbn10("0140328720"), None, "bad checksum"); + assert_eq!(isbn10_from_isbn13("9791036700248"), None, "979 has no -10"); + } + + fn edition(isbn_10: &[&str], isbn_13: &[&str]) -> OpenLibraryEdition { + OpenLibraryEdition { + isbn_10: isbn_10.iter().map(ToString::to_string).collect(), + isbn_13: isbn_13.iter().map(ToString::to_string).collect(), + ..OpenLibraryEdition::default() + } + } + + #[test] + fn missing_isbn13_is_proposed_from_the_record_isbn10() { + let candidates = enrichment_candidates(&edition(&["0140328726"], &[]), &[]); + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].field, "isbn_13"); + assert_eq!(candidates[0].proposed, "9780140328721"); + assert!(candidates[0].source.contains("0140328726")); + assert!(candidates[0].source.contains("(record)")); + } + + #[test] + fn missing_isbn10_is_proposed_from_the_cited_isbn13() { + // The record has no ISBNs at all; the citation's validated ISBN-13 + // supplies the ISBN-10 derivation, marked as citation-sourced. + // (Only conversions are proposed — never a verbatim relay of the + // citation's own value into the record.) + let cited = [BookIdentifier::isbn("978-0-14-032872-1").expect("valid")]; + let candidates = enrichment_candidates(&edition(&[], &[]), &cited); + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].field, "isbn_10"); + assert_eq!(candidates[0].proposed, "0140328726"); + assert!(candidates[0].source.contains("(citation)")); + } + + #[test] + fn complete_record_yields_no_candidates() { + // PRD-0009 DoD: a record already complete yields no proposal. + let candidates = enrichment_candidates( + &edition(&["0140328726"], &["9780140328721"]), + &[BookIdentifier::isbn("9780140328721").expect("valid")], + ); + assert!(candidates.is_empty()); + } + + #[test] + fn record_parse_pins_key_and_revision() { + let record = parse_record( + br#"{"key": "/books/OL7826547M", "revision": 7, "title": "Matilda", "isbn_10": ["0140328726"]}"#, + ) + .expect("parses"); + assert_eq!(record.key, "/books/OL7826547M"); + assert_eq!(record.revision, 7); + // Missing revision or key is unusable for the drift discipline. + assert_eq!(parse_record(br#"{"key": "/books/OL1M"}"#), None); + assert_eq!(parse_record(b"not json"), None); + } + + fn candidate() -> EnrichmentCandidate { + EnrichmentCandidate { + field: "isbn_13".to_string(), + proposed: "9780140328721".to_string(), + source: "derived from ISBN-10 0140328726 (record)".to_string(), + } + } + + #[test] + fn proposal_binds_revision_and_replacement_value() { + let record = OpenLibraryRecord { + key: "/books/OL7826547M".to_string(), + revision: 7, + raw: json!({"key": "/books/OL7826547M", "revision": 7, "isbn_10": ["0140328726"]}), + }; + let proposal = propose_from_candidate(&record, &candidate()).expect("open gap"); + assert_eq!(proposal.base_revision, 7); + assert_eq!(proposal.field, "isbn_13"); + assert_eq!(proposal.proposed, json!(["9780140328721"])); + assert_eq!(proposal.current, None); + assert!(proposal.comment.contains("9780140328721")); + assert_eq!( + proposal.proposed_form_value().as_deref(), + Some("9780140328721") + ); + // The confirmation hash moves with anything that would be written. + let mut other = proposal.clone(); + other.proposed = json!(["9999999999999"]); + assert_ne!(proposal.content_hash(), other.content_hash()); + assert_eq!(proposal.content_hash(), proposal.clone().content_hash()); + } + + #[test] + fn closed_gap_on_the_raw_record_yields_no_proposal() { + // The Books API view said the field was missing, but the raw record + // already has it (view lag): never re-propose a closed gap. + let record = OpenLibraryRecord { + key: "/books/OL7826547M".to_string(), + revision: 8, + raw: json!({"key": "/books/OL7826547M", "revision": 8, "isbn_13": ["9780140328721"]}), + }; + assert_eq!(propose_from_candidate(&record, &candidate()), None); + // An explicitly empty array is an OPEN gap. + let record_empty = OpenLibraryRecord { + key: "/books/OL7826547M".to_string(), + revision: 8, + raw: json!({"key": "/books/OL7826547M", "revision": 8, "isbn_13": []}), + }; + assert!(propose_from_candidate(&record_empty, &candidate()).is_some()); + } +} diff --git a/crates/sp42-citation/src/citation/openlibrary.rs b/crates/sp42-citation/src/citation/openlibrary.rs index 64a82e7f..53b5a59b 100644 --- a/crates/sp42-citation/src/citation/openlibrary.rs +++ b/crates/sp42-citation/src/citation/openlibrary.rs @@ -26,6 +26,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use url::Url; +use crate::citation::enrich::EnrichmentCandidate; use crate::types::{HttpMethod, HttpRequest}; use crate::wikitext_editor::{BookIdentifier, BookSource}; use sp42_types::HttpClient; @@ -335,6 +336,12 @@ pub struct BookResolution { #[serde(default, skip_serializing_if = "Option::is_none")] pub cited_page: Option, pub outcome: BookResolutionOutcome, + /// Read-only enrichment candidates for a resolved record (PRD-0009 + /// Layer 3's proposal listing): deterministic field-gap fills the + /// operator could confirm once the write lane is enabled (ADR-0019). + /// Always empty for unresolved outcomes. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub enrichment_candidates: Vec, } /// Resolve one book source: try its identifiers in template order against the diff --git a/crates/sp42-citation/src/citation/openlibrary_apply.rs b/crates/sp42-citation/src/citation/openlibrary_apply.rs new file mode 100644 index 00000000..3f7532b3 --- /dev/null +++ b/crates/sp42-citation/src/citation/openlibrary_apply.rs @@ -0,0 +1,920 @@ +//! The Open Library apply mechanism (ADR-0019). **The write lane is +//! disabled**: no server route or CLI surface calls [`execute_enrichment_apply`] +//! until ADR-0019's enablement gate passes (live form spike, upstream +//! courtesy contact, capability report) — until then this module is +//! mechanism + fixture tests only, and [`ENRICHMENT_WRITE_LANE_ENABLED`] +//! is the constant any future surface must check. +//! +//! Mechanics, per the ADR: +//! - **Session** (Decision 2): `POST /account/login` with the operator's own +//! IA S3 keys → session cookie. Per-operator, never shared. +//! - **Lane selection** (Decision 1): attempt the REST `PUT`; a 403 is +//! infogami's pre-mutation permission refusal, so the same apply falls +//! back to the form lane and the discovered lane is cached per session by +//! the caller. No knob, no membership probe. +//! - **Drift** (Decision 3): re-read the record before writing; a moved +//! `revision` refuses. The REST→form fallback re-runs the re-read. +//! - **Form adapter** (Decision 4): GET the edit form, replay every field it +//! carries with only the confirmed field changed, and treat any surprise +//! as contract drift — refuse, never guess. After a submit, read the +//! record back and verify exactly the proposed value landed. +//! +//! The form-lane field naming (`edition--{field}--0`, infogami's `--` +//! unflatten convention) is **adapter contract v0**: pinned by synthetic +//! fixtures until the ADR-0019 enablement spike replays the real form and +//! refreshes them. + +use std::collections::BTreeMap; +use std::sync::LazyLock; + +use regex::Regex; +use serde_json::Value; +use url::Url; + +use crate::citation::enrich::{EnrichmentProposal, OpenLibraryRecord, parse_record}; +use crate::types::{HttpMethod, HttpRequest, HttpResponse}; +use sp42_types::HttpClient; + +/// ADR-0019 Decision 6: the write lane ships disabled. Any future surface +/// that wires [`execute_enrichment_apply`] to an operator action must gate +/// on this constant, which flips only in the PR that records the enablement +/// gate (live form spike + upstream contact + capability report). +pub const ENRICHMENT_WRITE_LANE_ENABLED: bool = false; + +/// The Open Library origin every apply-lane request targets. +pub const OPEN_LIBRARY_ORIGIN: &str = "https://openlibrary.org"; + +/// The origin parsed once, for the (unreachable) fallback when a built URL +/// somehow fails to parse — keeps the builders panic-free. +static ORIGIN_BASE: LazyLock = + LazyLock::new(|| OPEN_LIBRARY_ORIGIN.parse().expect("static origin parses")); + +/// The operator's Open Library session: the cookie from `/account/login`, +/// held per-operator (ADR-0019 Decision 2). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OpenLibrarySession { + /// The `Cookie:` header value subsequent requests carry. + pub cookie: String, +} + +/// The lane an operator session discovered (ADR-0019 Decision 1). Cached by +/// the caller per session; a stale cache is benign in both directions. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ApplyLane { + /// `PUT /books/OL…M.json` — API-usergroup members. + Rest, + /// The website edit form — every logged-in account. + Form, +} + +/// Build the (state-changing POST) S3-key login request. The keys are the +/// operator's own, minted at archive.org; SP42 never holds a shared key. +#[must_use] +pub fn build_login_request(access_key: &str, secret_key: &str) -> HttpRequest { + let url = format!("{OPEN_LIBRARY_ORIGIN}/account/login") + .parse() + .unwrap_or_else(|_| ORIGIN_BASE.clone()); + let body = serde_json::json!({ "access": access_key, "secret": secret_key }); + HttpRequest { + method: HttpMethod::Post, + url, + headers: BTreeMap::from([("content-type".to_string(), "application/json".to_string())]), + body: body.to_string().into_bytes(), + } +} + +/// Parse a login response into a session: a 2xx with a `set-cookie` header +/// carrying the `session` cookie. `None` = login failed, lane stays +/// proposal-only. +#[must_use] +pub fn parse_login_response(response: &HttpResponse) -> Option { + if !(200..300).contains(&response.status) { + return None; + } + let set_cookie = response.headers.get("set-cookie")?; + let session_pair = set_cookie + .split(',') + .flat_map(|part| part.split(';')) + .map(str::trim) + .find(|pair| pair.starts_with("session="))?; + Some(OpenLibrarySession { + cookie: session_pair.to_string(), + }) +} + +/// Build the (read-only GET) raw-record read: the drift re-read, the propose +/// base, and the post-apply read-back all use this. +#[must_use] +pub fn build_record_request(record_key: &str) -> HttpRequest { + let url = format!("{OPEN_LIBRARY_ORIGIN}{record_key}.json") + .parse() + .unwrap_or_else(|_| ORIGIN_BASE.clone()); + HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::new(), + body: Vec::new(), + } +} + +/// The record document with exactly the proposal's field replaced — the +/// verbatim replay both lanes write (ADR-0019 Decision 3: apply never +/// recomputes the change). +#[must_use] +pub fn apply_proposal_to_record( + record: &OpenLibraryRecord, + proposal: &EnrichmentProposal, +) -> Value { + let mut raw = record.raw.clone(); + if let Value::Object(map) = &mut raw { + map.insert(proposal.field.clone(), proposal.proposed.clone()); + } + raw +} + +/// Build the REST-lane apply: `PUT {key}.json` of the full updated document +/// plus `_comment`, under the operator's session. +#[must_use] +pub fn build_rest_apply_request( + session: &OpenLibrarySession, + record: &OpenLibraryRecord, + proposal: &EnrichmentProposal, +) -> HttpRequest { + let mut body = apply_proposal_to_record(record, proposal); + if let Value::Object(map) = &mut body { + map.insert( + "_comment".to_string(), + Value::String(proposal.comment.clone()), + ); + } + let url = format!("{OPEN_LIBRARY_ORIGIN}{key}.json", key = record.key) + .parse() + .unwrap_or_else(|_| ORIGIN_BASE.clone()); + HttpRequest { + method: HttpMethod::Put, + url, + headers: BTreeMap::from([ + ("content-type".to_string(), "application/json".to_string()), + ("cookie".to_string(), session.cookie.clone()), + ]), + body: body.to_string().into_bytes(), + } +} + +/// Build the (read-only GET) edit-form fetch for the form lane. +#[must_use] +pub fn build_edit_form_request(session: &OpenLibrarySession, record_key: &str) -> HttpRequest { + let url = format!("{OPEN_LIBRARY_ORIGIN}{record_key}/edit") + .parse() + .unwrap_or_else(|_| ORIGIN_BASE.clone()); + HttpRequest { + method: HttpMethod::Get, + url, + headers: BTreeMap::from([("cookie".to_string(), session.cookie.clone())]), + body: Vec::new(), + } +} + +/// The parsed edit form: where it posts and every field it carries, in +/// document order. Replaying all of them (with one changed) is what keeps +/// the full-record form save from blanking anything. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EditForm { + /// The form's `action` path (posted back on the same origin). + pub action_path: String, + /// Every field the form carries: `(name, value)`, later entries + /// overriding earlier ones on submit-encode is NOT assumed — names are + /// kept in order and posted as-is. + pub fields: Vec<(String, String)>, +} + +static FORM_OPEN: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?is)]*\baction="([^"]+)"[^>]*>"#).expect("valid regex") +}); +static INPUT_TAG: LazyLock = + LazyLock::new(|| Regex::new(r"(?is)<(input|textarea|select)\b[^>]*>").expect("valid regex")); +static ATTR_NAME: LazyLock = + LazyLock::new(|| Regex::new(r#"(?is)\bname="([^"]*)""#).expect("valid regex")); +static ATTR_VALUE: LazyLock = + LazyLock::new(|| Regex::new(r#"(?is)\bvalue="([^"]*)""#).expect("valid regex")); +static ATTR_TYPE: LazyLock = + LazyLock::new(|| Regex::new(r#"(?is)\btype="([^"]*)""#).expect("valid regex")); +static TEXTAREA_BLOCK: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?is)]*\bname="([^"]*)"[^>]*>(.*?)"#) + .expect("valid regex") +}); +static SELECT_BLOCK: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?is)]*\bname="([^"]*)"[^>]*>(.*?)"#).expect("valid regex") +}); +static SELECTED_OPTION: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?is)]*\bselected\b[^>]*\bvalue="([^"]*)""#).expect("valid regex") +}); + +/// Parse the edition/work edit form out of the page, fail-closed: `None` +/// for anything that is not recognizably the edit form SP42 knows how to +/// replay — a login redirect body, a redesigned page, an unparseable form, +/// or a ` + + + + + + +"#; + + fn response(status: u16, body: &str) -> HttpResponse { + HttpResponse { + status, + headers: BTreeMap::new(), + body: body.as_bytes().to_vec(), + } + } + + fn redirect(location: &str) -> HttpResponse { + HttpResponse { + status: 303, + headers: BTreeMap::from([("location".to_string(), location.to_string())]), + body: Vec::new(), + } + } + + #[test] + fn write_lane_ships_disabled() { + // ADR-0019 Decision 6: flipped only by the enablement-gate PR. + // A constant on purpose: this test is the tripwire that makes + // flipping the gate a visible, reviewed act. + #[allow(clippy::assertions_on_constants)] + // https://github.com/schiste/SP42 ADR-0019 enablement-gate tripwire + { + assert!(!ENRICHMENT_WRITE_LANE_ENABLED); + } + } + + #[test] + fn login_request_and_session_parse() { + let request = build_login_request("AK", "SK"); + assert_eq!(request.method, HttpMethod::Post); + assert_eq!( + request.url.as_str(), + "https://openlibrary.org/account/login" + ); + let body: serde_json::Value = serde_json::from_slice(&request.body).expect("json body"); + assert_eq!(body["access"], "AK"); + assert_eq!(body["secret"], "SK"); + + let ok = HttpResponse { + status: 200, + headers: BTreeMap::from([( + "set-cookie".to_string(), + "session=/people/tester%2C2026; Path=/; HttpOnly".to_string(), + )]), + body: Vec::new(), + }; + let session = parse_login_response(&ok).expect("session"); + assert_eq!(session.cookie, "session=/people/tester%2C2026"); + // A failed login yields no session. + let denied = HttpResponse { + status: 401, + headers: BTreeMap::new(), + body: Vec::new(), + }; + assert_eq!(parse_login_response(&denied), None); + } + + #[test] + fn rest_apply_request_carries_comment_and_field() { + let record = parse_record(RECORD_REV7.as_bytes()).expect("record"); + let request = build_rest_apply_request(&session(), &record, &proposal()); + assert_eq!(request.method, HttpMethod::Put); + assert_eq!( + request.url.as_str(), + "https://openlibrary.org/books/OL7826547M.json" + ); + assert_eq!( + request.headers.get("cookie").map(String::as_str), + Some(session().cookie.as_str()) + ); + let body: serde_json::Value = serde_json::from_slice(&request.body).expect("json"); + assert_eq!(body["isbn_13"], json!(["9780140328721"])); + assert_eq!( + body["isbn_10"], + json!(["0140328726"]), + "untouched fields replay" + ); + assert_eq!(body["_comment"], json!(proposal().comment)); + } + + #[test] + fn edit_form_parses_and_fills_fail_closed() { + let form = parse_edit_form(EDIT_FORM_HTML, "/books/OL7826547M").expect("known form"); + assert_eq!(form.action_path, "/books/OL7826547M/edit"); + // All non-submit fields replayed, including the hidden v. + assert!(form.fields.iter().any(|(n, v)| n == "v" && v == "7")); + assert!( + form.fields + .iter() + .any(|(n, v)| n == "edition--title--0" && v == "Matilda") + ); + + let filled = fill_edit_form(&form, &proposal()).expect("fills"); + assert!( + filled + .fields + .iter() + .any(|(n, v)| n == &form_field_name("isbn_13") && v == "9780140328721") + ); + assert!( + filled + .fields + .iter() + .any(|(n, v)| n == "_comment" && v == &proposal().comment) + ); + + // Fail-closed: a form missing the target slot or the comment slot + // refuses rather than inventing fields. + let missing_slot = EDIT_FORM_HTML.replace("edition--isbn_13--0", "edition--renamed--0"); + let form = parse_edit_form(&missing_slot, "/books/OL7826547M").expect("parses"); + assert_eq!(fill_edit_form(&form, &proposal()), None); + // Fail-closed: an occupied target slot is a closed gap. + let occupied = EDIT_FORM_HTML.replace( + r#"name="edition--isbn_13--0" value="""#, + r#"name="edition--isbn_13--0" value="9789999999991""#, + ); + let form = parse_edit_form(&occupied, "/books/OL7826547M").expect("parses"); + assert_eq!(fill_edit_form(&form, &proposal()), None); + // Fail-closed: not the edit form at all (login page), or widgets the + // adapter cannot replay faithfully. + assert_eq!( + parse_edit_form("Please log in", "/books/OL7826547M"), + None + ); + let with_checkbox = EDIT_FORM_HTML.replace( + r#""#, + r#""#, + ); + assert_eq!(parse_edit_form(&with_checkbox, "/books/OL7826547M"), None); + } + + #[test] + fn form_submit_encodes_all_fields_and_checks_success_shape() { + let form = EditForm { + action_path: "/books/OL7826547M/edit".to_string(), + fields: vec![ + ("v".to_string(), "7".to_string()), + ("_comment".to_string(), "a comment".to_string()), + ], + }; + let request = build_form_submit_request(&session(), &form); + assert_eq!(request.method, HttpMethod::Post); + assert_eq!( + request.url.as_str(), + "https://openlibrary.org/books/OL7826547M/edit" + ); + let body = String::from_utf8(request.body).expect("utf8"); + assert!(body.contains("v=7")); + assert!(body.contains("_comment=a+comment")); + + let ok = redirect("https://openlibrary.org/books/OL7826547M/Matilda"); + assert!(form_submit_succeeded(&ok, "/books/OL7826547M")); + let wrong = response(200, "error"); + assert!(!form_submit_succeeded(&wrong, "/books/OL7826547M")); + } + + #[test] + fn rest_lane_applies_and_caches_on_success() { + // read (drift) -> PUT 200 -> read-back shows the value. + let client = StubHttpClient::new([ + Ok(response(200, RECORD_REV7)), + Ok(response(200, r#"{"status": "ok"}"#)), + Ok(response(200, RECORD_REV8_APPLIED)), + ]); + let mut lane_cache = None; + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + assert_eq!( + outcome, + ApplyOutcome::Applied { + lane: ApplyLane::Rest, + new_revision: 8 + } + ); + assert_eq!(lane_cache, Some(ApplyLane::Rest)); + } + + #[test] + fn rest_403_falls_back_to_the_form_lane_within_one_apply() { + // read (drift) -> PUT 403 (capability answer) -> read (drift restart) + // -> form GET -> form POST redirect -> read-back. + let client = StubHttpClient::new([ + Ok(response(200, RECORD_REV7)), + Ok(response(403, r#"{"error": "permission_denied"}"#)), + Ok(response(200, RECORD_REV7)), + Ok(response(200, EDIT_FORM_HTML)), + Ok(redirect("https://openlibrary.org/books/OL7826547M/Matilda")), + Ok(response(200, RECORD_REV8_APPLIED)), + ]); + let mut lane_cache = None; + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + assert_eq!( + outcome, + ApplyOutcome::Applied { + lane: ApplyLane::Form, + new_revision: 8 + } + ); + assert_eq!(lane_cache, Some(ApplyLane::Form), "discovery cached"); + } + + #[test] + fn cached_form_lane_skips_the_rest_attempt() { + // With the lane cached, the request sequence has NO PUT: + // read (drift) -> form GET -> form POST -> read-back. + let client = StubHttpClient::new([ + Ok(response(200, RECORD_REV7)), + Ok(response(200, EDIT_FORM_HTML)), + Ok(redirect("https://openlibrary.org/books/OL7826547M/Matilda")), + Ok(response(200, RECORD_REV8_APPLIED)), + ]); + let mut lane_cache = Some(ApplyLane::Form); + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + assert!(matches!( + outcome, + ApplyOutcome::Applied { + lane: ApplyLane::Form, + .. + } + )); + } + + #[test] + fn moved_revision_refuses_before_any_write() { + // The record advanced to revision 9 since the proposal: exactly one + // request (the drift read) is issued, then refusal. + let client = StubHttpClient::new([Ok(response( + 200, + r#"{"key": "/books/OL7826547M", "revision": 9, "isbn_10": ["0140328726"]}"#, + ))]); + let mut lane_cache = None; + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + assert_eq!( + outcome, + ApplyOutcome::RefusedDrift { + base_revision: 7, + current_revision: 9 + } + ); + assert_eq!(lane_cache, None, "no lane discovered by a refusal"); + } + + #[test] + fn contract_drift_refuses_and_never_posts() { + // The form fetch returns something unrecognizable: the queue holds + // NOTHING after it, so any attempted POST would error the test. + let client = StubHttpClient::new([ + Ok(response(200, RECORD_REV7)), + Ok(response(403, "denied")), + Ok(response(200, RECORD_REV7)), + Ok(response( + 200, + "a redesigned page with no known form", + )), + ]); + let mut lane_cache = None; + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + assert!(matches!(outcome, ApplyOutcome::RefusedContractDrift { .. })); + } + + #[test] + fn read_back_mismatch_is_a_loud_failure_not_success() { + // The PUT reports 200 but the read-back does not show the value. + let client = StubHttpClient::new([ + Ok(response(200, RECORD_REV7)), + Ok(response(200, r#"{"status": "ok"}"#)), + Ok(response(200, RECORD_REV7)), // unchanged! + ]); + let mut lane_cache = None; + let outcome = block_on(execute_enrichment_apply( + &client, + &session(), + &proposal(), + &mut lane_cache, + )); + let ApplyOutcome::Failed { message } = outcome else { + panic!("expected Failed, got {outcome:?}"); + }; + assert!(message.contains("read-back")); + } + + #[test] + fn read_back_verifies_the_exact_proposed_value() { + let record = parse_record(RECORD_REV8_APPLIED.as_bytes()).expect("record"); + assert!(verify_applied(&record, &proposal())); + let unchanged = parse_record(RECORD_REV7.as_bytes()).expect("record"); + assert!(!verify_applied(&unchanged, &proposal())); + } + + #[test] + fn record_request_targets_the_json_read() { + assert_eq!( + build_record_request("/books/OL7826547M").url.as_str(), + "https://openlibrary.org/books/OL7826547M.json" + ); + } +} diff --git a/crates/sp42-citation/src/citation/page.rs b/crates/sp42-citation/src/citation/page.rs index b6da17aa..2383540c 100644 --- a/crates/sp42-citation/src/citation/page.rs +++ b/crates/sp42-citation/src/citation/page.rs @@ -543,13 +543,20 @@ where M: ModelClient + ?Sized, { let outcome = resolve_book_source(fetch_client, &site.book).await; - let resolution = BookResolution { + let mut resolution = BookResolution { ref_id: site.ref_id.clone(), block_ordinal: site.block_ordinal, identifiers: site.book.identifiers.clone(), cited_page: site.book.cited_page.clone(), outcome, + enrichment_candidates: Vec::new(), }; + if let BookResolutionOutcome::Resolved { edition, .. } = &resolution.outcome { + // Read-only proposal listing (PRD-0009 Layer 3): what an operator + // could confirm once ADR-0019's write lane is enabled. + resolution.enrichment_candidates = + crate::citation::enrich::enrichment_candidates(edition, &resolution.identifiers); + } let verdict = match &resolution.outcome { BookResolutionOutcome::Resolved { edition, scan, .. } => { diff --git a/crates/sp42-citation/src/citation_page_report.rs b/crates/sp42-citation/src/citation_page_report.rs index aced9a04..e559cd98 100644 --- a/crates/sp42-citation/src/citation_page_report.rs +++ b/crates/sp42-citation/src/citation_page_report.rs @@ -182,8 +182,19 @@ fn book_line(resolution: &BookResolution) -> String { .as_deref() .map(|u| format!(" url={u}")) .unwrap_or_default(); + let propose = if resolution.enrichment_candidates.is_empty() { + String::new() + } else { + let joined = resolution + .enrichment_candidates + .iter() + .map(|candidate| format!("{}:{}", candidate.field, candidate.proposed)) + .collect::>() + .join(","); + format!(" propose={joined}") + }; format!( - "{base} resolved={identifier}{title}{authors}{url} scan={scan}", + "{base} resolved={identifier}{title}{authors}{url} scan={scan}{propose}", scan = scan_label(scan.as_ref()) ) } @@ -439,6 +450,11 @@ mod tests { similar: vec![], }), }, + enrichment_candidates: vec![crate::EnrichmentCandidate { + field: "isbn_13".to_string(), + proposed: "9780140328721".to_string(), + source: "derived from ISBN-10 0140328726 (record)".to_string(), + }], }], stats: PageVerificationStats { refs_seen: 4, @@ -511,7 +527,7 @@ mod tests { assert!(text.contains( "ref=cite_isbn block=5 resolved=isbn:9780140328721 title=\"Matilda\" \ authors=\"Roald Dahl\" url=https://openlibrary.org/books/OL7826547M/Matilda \ - scan=exact" + scan=exact propose=isbn_13:9780140328721" )); } @@ -541,6 +557,7 @@ mod tests { identifiers: vec![crate::BookIdentifier::Isbn("9780140328721".to_string())], cited_page: None, outcome: crate::BookResolutionOutcome::NotFound, + enrichment_candidates: vec![], }, crate::BookResolution { ref_id: "cite_lccn".to_string(), @@ -550,6 +567,7 @@ mod tests { outcome: crate::BookResolutionOutcome::LookupFailed { message: "openlibrary unreachable".to_string(), }, + enrichment_candidates: vec![], }, ]; let text = render_page_verification_text(&report); diff --git a/crates/sp42-citation/src/lib.rs b/crates/sp42-citation/src/lib.rs index 5cc8446d..8efa9f64 100644 --- a/crates/sp42-citation/src/lib.rs +++ b/crates/sp42-citation/src/lib.rs @@ -34,6 +34,10 @@ pub use citation::citoid::{ CitoidMetadata, build_citoid_header, build_citoid_request, parse_citoid_response, }; pub use citation::concurrency::map_with_concurrency; +pub use citation::enrich::{ + EnrichmentCandidate, EnrichmentProposal, OpenLibraryRecord, enrichment_candidates, + isbn10_from_isbn13, isbn13_from_isbn10, parse_record, propose_from_candidate, +}; pub use citation::extract::{ BlockFailure, BookUseSite, CitationUseSite, ExtractOutcome, SkippedReason, SkippedRef, extract_use_sites, @@ -45,6 +49,13 @@ pub use citation::openlibrary::{ build_scan_availability_request, parse_catalog_lookup, parse_scan_availability, resolve_book_source, }; +pub use citation::openlibrary_apply::{ + ApplyLane, ApplyOutcome, ENRICHMENT_WRITE_LANE_ENABLED, EditForm, OPEN_LIBRARY_ORIGIN, + OpenLibrarySession, apply_proposal_to_record, build_edit_form_request, + build_form_submit_request, build_login_request, build_record_request, build_rest_apply_request, + execute_enrichment_apply, fill_edit_form, form_field_name, form_submit_succeeded, + parse_edit_form, parse_login_response, verify_applied, +}; pub use citation::page::{ PageVerificationReport, PageVerificationRequest, PageVerificationStats, verify_page, }; diff --git a/docs/STATUS.md b/docs/STATUS.md index db1b867d..ae5d7eb6 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -86,10 +86,13 @@ PWA packaging and offline installability are now effectively complete for local bypass), cited-page-first with whole-book fallback, page-anchored deep links, and the honest `not_supported` vs `SourceUnavailable` split; unresolved books stay skipped with a refined reason and a Books report section shows every - resolution; the enrichment lane (Layer 3) has its - apply contract drafted (ADR-0019: form-lane + REST-lane applies behind a - derived capability, client-side refuse-on-drift, write lane disabled until - its enablement gate) and awaits implementation + resolution; the enrichment lane (Layer 3) is + implemented as mechanism + fixture tests per ADR-0019 — deterministic + ISBN-completion candidates listed read-only in the Books section, and the + apply machinery (per-operator S3-key login, REST lane with 403 fallback to + a fail-closed edit-form adapter, per-session lane cache, client-side + refuse-on-drift, post-apply read-back) — with the write lane disabled and + unwired until the ADR-0019 enablement gate passes ## Current Verification From c9001b8c0f0f8d7fc1ec0153adc5cc490c8cb505 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 17:44:13 +0000 Subject: [PATCH 8/9] Add the live Open Library / IA read-contract smoke script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual, network-touching, never CI (ADR-0009 keeps tests on fixtures). Walks the exact read chain verify-page uses — Books API catalog lookup, Read API scan availability, archive.org item metadata, BookReader search-inside — with read-only GETs and asserts precisely the response fields the PRD-0009 parsers read. A pass means production still matches the fixtures; a failure names the drifted contract and keeps the real payload for fixture refresh. By construction it never touches the import-on-miss /isbn/{isbn}.json path or any write endpoint. Header notes that Open Library 403s datacenter IPs (observed from this environment), so the smoke runs from a developer connection — the same caveat applies to any future cloud-hosted SP42 deployment. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- docs/STATUS.md | 3 + scripts/openlibrary-contract-smoke.sh | 186 ++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100755 scripts/openlibrary-contract-smoke.sh diff --git a/docs/STATUS.md b/docs/STATUS.md index ae5d7eb6..643956b4 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -110,3 +110,6 @@ The workspace is currently kept green with: - README/STATUS drift checks in CI - `bash scripts/local-operator-smoke.sh` for the local operator flow - targeted multi-user coordination validation inside the local operator smoke path +- `bash scripts/openlibrary-contract-smoke.sh` for the live Open Library / + Internet Archive read-contract check (manual, network-touching, never CI; + asserts exactly the response fields the PRD-0009 parsers read) diff --git a/scripts/openlibrary-contract-smoke.sh b/scripts/openlibrary-contract-smoke.sh new file mode 100755 index 00000000..85944460 --- /dev/null +++ b/scripts/openlibrary-contract-smoke.sh @@ -0,0 +1,186 @@ +#!/usr/bin/env bash +# Live read-contract smoke for the PRD-0009 book lane (ADR-0018). +# +# MANUAL, NETWORK-TOUCHING — never wired into CI (ADR-0009: tests replay +# fixtures; this script is how a human refreshes confidence in them). It +# walks the exact read chain `verify-page` uses, read-only GETs only: +# +# 1. Books API catalog lookup (openlibrary.rs::build_catalog_lookup_request) +# 2. Read API scan availability (openlibrary.rs::build_scan_availability_request) +# 3. archive.org item metadata (search_inside.rs::build_item_metadata_request) +# 4. BookReader search-inside (search_inside.rs::build_search_inside_request) +# +# and asserts precisely the response fields the parsers read, so: +# PASS -> production still matches our fixtures/parsers; +# FAIL -> the printed payload (kept in the temp dir) is the refreshed +# fixture material and names the drifted contract. +# +# By construction this script never touches the import-on-miss +# `/isbn/{isbn}.json` path or any write endpoint (ADR-0018 Decision 2). +# +# Note: Open Library fronts with bot protection that can 403 datacenter IPs. +# Run this from a residential/developer connection; a Cloudflare challenge +# page here predicts the same for any cloud-hosted SP42 deployment. +# +# Usage: scripts/openlibrary-contract-smoke.sh [ISBN13] ["search terms"] + +set -euo pipefail + +isbn="${1:-9780140328721}" # Matilda (Puffin 1988): held, scanned, indexed. +query="${2:-Matilda parents}" +ua="SP42/0.1.0 (+https://github.com/schiste/SP42; read-contract smoke)" + +tmp="$(mktemp -d)" +keep_tmp=0 +cleanup() { + if [[ "$keep_tmp" == 1 ]]; then + echo "payloads kept in $tmp for fixture refresh" + else + rm -rf "$tmp" + fi +} +trap cleanup EXIT + +fetch() { + local url="$1" out="$2" + printf 'GET %s\n' "$url" + /usr/bin/curl -fsS --max-time 30 -A "$ua" "$url" -o "$out" + sleep 1 # politeness between third-party calls +} + +check() { + local file="$1" step="$2" + shift 2 + if ! python3 - "$file" "$@" <<'PY'; then +import json +import sys + +path = sys.argv[1] +checks = sys.argv[2:] +with open(path, "rb") as handle: + document = json.load(handle) + +def resolve(node, dotted): + for part in dotted.split("."): + if isinstance(node, list): + node = node[int(part)] + else: + node = node[part] + return node + +for spec in checks: + # spec: "dotted.path" (exists), "dotted.path=literal", or + # "dotted.path:type" with type in {str,int,list,dict}. + if "=" in spec: + dotted, expected = spec.split("=", 1) + actual = resolve(document, dotted) + if str(actual) != expected: + sys.exit(f"{dotted} = {actual!r}, expected {expected!r}") + elif ":" in spec: + dotted, kind = spec.rsplit(":", 1) + actual = resolve(document, dotted) + expected_type = {"str": str, "int": int, "list": list, "dict": dict}[kind] + if not isinstance(actual, expected_type): + sys.exit(f"{dotted} is {type(actual).__name__}, expected {kind}") + else: + resolve(document, spec) +PY + keep_tmp=1 + echo "CONTRACT DRIFT at step: $step (payload: $file)" >&2 + exit 1 + fi + echo "ok: $step" +} + +echo "== 1. Books API catalog lookup (side-effect-free resolve) ==" +fetch "https://openlibrary.org/api/books?bibkeys=ISBN%3A${isbn}&jscmd=data&format=json" "$tmp/books.json" +# parse_catalog_lookup reads: the bibkey top key, then key/url/title, +# authors[].name, publishers[].name, identifiers.isbn_10/isbn_13, cover. +check "$tmp/books.json" "Books API record shape" \ + "ISBN:${isbn}:dict" \ + "ISBN:${isbn}.key:str" \ + "ISBN:${isbn}.title:str" + +echo "== 2. Read API scan availability ==" +fetch "https://openlibrary.org/api/volumes/brief/isbn/${isbn}.json" "$tmp/read.json" +# parse_scan_availability reads: items[].match / itemURL / status. +check "$tmp/read.json" "Read API items shape" "items:list" +ocaid="$(python3 - "$tmp/read.json" <<'PY' +import json +import sys +from urllib.parse import urlparse + +with open(sys.argv[1], "rb") as handle: + document = json.load(handle) +for item in document.get("items", []): + if item.get("match") != "exact": + continue + url = urlparse(item["itemURL"]) + parts = [p for p in url.path.split("/") if p] + if url.hostname in ("archive.org", "www.archive.org") and parts[:1] == ["details"]: + print(parts[1]) + break +PY +)" +if [[ -z "$ocaid" ]]; then + # Not drift by itself (a book can lose its scan), but this smoke needs one. + keep_tmp=1 + echo "no exact-match scan for ISBN ${isbn}; pick a scanned edition (payload: $tmp/read.json)" >&2 + exit 1 +fi +echo "ok: exact-match scan -> ocaid=${ocaid}" + +echo "== 3. archive.org item metadata ==" +fetch "https://archive.org/metadata/${ocaid}" "$tmp/metadata.json" +# parse_item_metadata reads: server, dir, metadata.mediatype == texts. +check "$tmp/metadata.json" "item metadata shape" \ + "server:str" "dir:str" "metadata.mediatype=texts" + +echo "== 4. BookReader search-inside ==" +search_url="$(python3 - "$tmp/metadata.json" "$ocaid" "$query" <<'PY' +import json +import sys +from urllib.parse import urlencode + +with open(sys.argv[1], "rb") as handle: + document = json.load(handle) +params = urlencode({ + "item_id": sys.argv[2], + "doc": sys.argv[2], + "path": document["dir"], + "q": sys.argv[3], +}) +print(f"https://{document['server']}/fulltext/inside.php?{params}") +PY +)" +fetch "$search_url" "$tmp/inside.json" +# parse_search_inside reads: matches[].text (with {{{...}}} markers) and +# matches[].par[0].page; `indexed` when present. +check "$tmp/inside.json" "search-inside shape" "matches:list" +if ! python3 - "$tmp/inside.json" <<'PY'; then +import json +import sys + +with open(sys.argv[1], "rb") as handle: + document = json.load(handle) +matches = document.get("matches", []) +if not matches: + # An indexed scan with zero matches is a legitimate outcome; only an + # un-JSON or shapeless response is drift. Still worth eyeballing. + print("note: zero matches for this query; try broader terms") + sys.exit(0) +first = matches[0] +if not isinstance(first.get("text"), str): + sys.exit("matches[0].text is not a string") +par = first.get("par") +if not (isinstance(par, list) and par and isinstance(par[0].get("page"), int)): + sys.exit("matches[0].par[0].page is not an integer") +print(f"first match p.{par[0]['page']}: {first['text'][:100]!r}") +PY + keep_tmp=1 + echo "CONTRACT DRIFT at step: search-inside match shape (payload: $tmp/inside.json)" >&2 + exit 1 +fi + +echo +echo "Open Library / Internet Archive read-contract smoke passed." From 83f678c8b5ac9e09e5edff74fd73b5c8bc586abf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 17:56:45 +0000 Subject: [PATCH 9/9] ADR-0019: restructure the form spike as capture-then-write, no local OL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review outcome: no local Open Library instance will be set up. The enablement spike's form-contract validation moves to a zero-write production capture — the edit-form GET is read-only, so the real HTML can be fetched under a session cookie and committed as the adapter's fixture, putting parse/fill against real markup in the normal test suite. POST semantics (success shape, _comment in history, stale-v behavior) stay in the one-write phase of the spike as before. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012u5MVp3vAN44KhsVg3vroY --- .../adr/0019-open-library-apply-contract.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/domains/references/adr/0019-open-library-apply-contract.md b/docs/domains/references/adr/0019-open-library-apply-contract.md index 76135abc..8cbdb23f 100644 --- a/docs/domains/references/adr/0019-open-library-apply-contract.md +++ b/docs/domains/references/adr/0019-open-library-apply-contract.md @@ -153,12 +153,19 @@ promise. Accordingly: Layer 3 remains **proposal-only** until all of the following are recorded in the enabling PR (this is the PRD-0009 DoD item made operational): -1. **Live form spike (one-time, manual, a test account):** confirm the form - lane's minimal single-field edit end-to-end against production — the exact - field set the form requires, `_comment` visible in the record's history, - and the observed behavior of a stale `v` submit (merge vs overwrite), - which calibrates how paranoid the drift refusal must stay. Record the - target record, before/after, and the resulting revision. +1. **Live form spike (one-time, manual, a test account), in two phases.** + No local Open Library instance is assumed at any point. + **(a) Zero-write form capture:** log in and `GET` a real edition edit + form under the session cookie (a read-only request); commit the captured + HTML as the adapter's fixture and make `parse_edit_form`/`fill_edit_form` + pass against it, replacing the synthetic contract-v0 fixture. This + validates the field-naming contract with no write anywhere. + **(b) The single write:** the minimal single-field edit end-to-end + against production — the exact field set the submit requires, `_comment` + visible in the record's history, and the observed behavior of a stale `v` + submit (merge vs overwrite), which calibrates how paranoid the drift + refusal must stay. Record the target record, before/after, and the + resulting revision. 2. **Upstream courtesy:** Open Library's team has been told what SP42 does (assisted, operator-confirmed, one field at a time) via their documented contact channel — the same channel as an API-usergroup request. If they