papers 0.1.1

use std::{
    collections::{HashMap, HashSet},
    sync::Arc,
};

use anyhow::{anyhow, Context, Result};
use rayon::prelude::*;
use tokio::sync::RwLock;
use wikibase::mediawiki::api::Api;

use self::{
    identifiers::{GenericWorkIdentifier, GenericWorkType},
    wikidata_interaction::WikidataInteraction,
};
use crate::{
    generic_author_info::GenericAuthorInfo,
    scientific_publication_adapter::ScientificPublicationAdapter,
    wikidata_string_cache::WikidataStringCache, *,
};

pub type Spas = Box<dyn ScientificPublicationAdapter + Sync>;

lazy_static! {
    static ref SNAK_REMOVE_STATEMENT: Snak = Snak::new_no_value("P2093", SnakDataType::String);
}

/// Hard cap on the identifier-discovery convergence loop in
/// `update_from_paper_ids`. A misbehaving adapter that keeps inventing new
/// IDs each call would otherwise loop forever; the cap bounds the worst
/// case while still allowing real adapters (which converge in 2–3 passes)
/// to finish naturally.
pub(crate) const MAX_ID_DISCOVERY_PASSES: usize = 8;

pub struct EditResult {
    q: String,
    edited: bool,
}

impl EditResult {
    pub fn q(&self) -> &str {
        &self.q
    }

    pub fn edited(&self) -> bool {
        self.edited
    }
}

pub struct WikidataPapers {
    adapters: Vec<Spas>,
    cache: Arc<WikidataStringCache>,
    edit_summary: Option<String>,
    testing: bool,
    entities: entity_container::EntityContainer,
}

impl WikidataInteraction for WikidataPapers {}

impl WikidataPapers {
    pub fn new(cache: Arc<WikidataStringCache>) -> WikidataPapers {
        let mut entities = entity_container::EntityContainer::new();
        entities.allow_special_entity_data(false);
        WikidataPapers {
            adapters: vec![],
            cache,
            edit_summary: None,
            testing: false,
            entities,
        }
    }

    /// Constructor that registers every built-in provider adapter in the
    /// canonical order.
    ///
    /// The order is meaningful: `update_item_from_adapters` and
    /// `update_from_paper_ids` iterate `self.adapters` in registration
    /// order, so earlier adapters get first claim on identifier mappings
    /// and statement edits. Changing the order risks subtle behaviour
    /// shifts in production. Keep PMC → PubMed → Crossref → Semantic
    /// Scholar → ORCID → arXiv → OpenAlex → DataCite → EuropePMC in sync
    /// across binaries; this constructor is the single source of truth.
    pub fn with_default_adapters(cache: Arc<WikidataStringCache>) -> WikidataPapers {
        use crate::{
            arxiv2wikidata::Arxiv2Wikidata,
            crossref2wikidata::Crossref2Wikidata,
            datacite2wikidata::DataCite2Wikidata,
            europepmc2wikidata::EuropePMC2Wikidata,
            http_client::{HttpJsonFetcher, JsonFetcher},
            openalex2wikidata::OpenAlex2Wikidata,
            orcid2wikidata::Orcid2Wikidata,
            pmc2wikidata::PMC2Wikidata,
            pubmed2wikidata::Pubmed2Wikidata,
            semanticscholar2wikidata::Semanticscholar2Wikidata,
        };
        // One JSON fetcher shared across all DI-aware adapters. Cheap to
        // clone (the underlying reqwest::Client is a process-wide singleton).
        // Category-B SDK adapters (Pubmed, Crossref, Semanticscholar,
        // Orcid, Arxiv) construct their own clients — see audit P2-10b /
        // P2-HTTP-1 for the upstream-owner follow-up to add
        // `new_with_client` constructors.
        let fetcher: Arc<dyn JsonFetcher> = Arc::new(HttpJsonFetcher::default());
        let mut wdp = Self::new(cache);
        wdp.add_adapter(Box::new(PMC2Wikidata::new(fetcher.clone())));
        wdp.add_adapter(Box::new(Pubmed2Wikidata::new()));
        wdp.add_adapter(Box::new(Crossref2Wikidata::new()));
        wdp.add_adapter(Box::new(Semanticscholar2Wikidata::new()));
        wdp.add_adapter(Box::new(Orcid2Wikidata::new()));
        wdp.add_adapter(Box::new(Arxiv2Wikidata::new()));
        wdp.add_adapter(Box::new(OpenAlex2Wikidata::new(fetcher.clone())));
        wdp.add_adapter(Box::new(DataCite2Wikidata::new(fetcher.clone())));
        wdp.add_adapter(Box::new(EuropePMC2Wikidata::new(fetcher)));
        wdp
    }

    pub fn adapters_mut(&mut self) -> &mut Vec<Spas> {
        &mut self.adapters
    }

    pub fn add_adapter(&mut self, adapter_box: Spas) {
        self.adapters.push(adapter_box);
    }

    pub fn set_testing(&mut self, testing: bool) {
        self.testing = testing;
    }

    pub fn edit_summary(&self) -> &Option<String> {
        &self.edit_summary
    }

    pub fn set_edit_summary(&mut self, edit_summary: Option<String>) {
        self.edit_summary = edit_summary;
    }

    fn create_author_statements(&mut self, authors: &Vec<GenericAuthorInfo>, item: &mut Entity) {
        let mut seen_q_items: HashSet<String> = HashSet::new();
        let mut seen_names: HashSet<String> = HashSet::new();
        for author in authors {
            // Skip duplicate wikidata items (prevents duplicate P50 for same person)
            if let Some(q) = author.wikidata_item() {
                if !seen_q_items.insert(q.to_string()) {
                    continue;
                }
            }
            // Skip duplicate author name strings for P2093
            if author.wikidata_item().is_none() {
                if let Some(name) = author.name() {
                    let key = GenericAuthorInfo::simplify_name(&name.to_lowercase());
                    if !key.is_empty() && !seen_names.insert(key) {
                        continue;
                    }
                }
            }
            author.create_author_statement_in_paper_item(item);
        }
    }

    pub fn update_author_name_statement(
        &self,
        asn: &str,
        new_author: &GenericAuthorInfo,
        item: &mut Entity,
    ) {
        let author_q = match new_author.wikidata_item() {
            Some(q) => q,
            None => return,
        };
        if Self::get_p50s_from_item(item).contains(&format!("Q{}", author_q)) {
            return; // Had that author already
        }
        item.claims_mut()
            .par_iter_mut()
            .filter(|statement| statement.property() == "P2093")
            .filter_map(|statement| {
                let author = GenericAuthorInfo::new_from_statement(statement)?;
                Some((author, statement))
            })
            .filter(|(author, _statement)| author.name() == Some(asn))
            .for_each(|(_author, p2093_statement)| {
                let p50_statement = match &new_author.generate_author_statement() {
                    Some(p50_statement) => p50_statement.to_owned(),
                    None => return,
                };
                Self::replace_p2093_with_p50_in_place(&p50_statement, p2093_statement);
            });
        Self::remove_statements_with_no_value(item);
    }

    /// For each P2093 ("author name string") statement on `item`, tries to
    /// match it against `authors` (typically merged adapter-supplied
    /// authors) and either:
    /// - **replaces** the P2093 with a P50 ("author") pointing at the
    ///   matched Wikidata author item (via
    ///   [`Self::replace_p2093_with_p50_in_place`]), or
    /// - **removes** the P2093 if the matched author already has a P50
    ///   on the item, or if that candidate has already been used by an
    ///   earlier P2093 (prevents one matched candidate consuming
    ///   multiple P2093s and creating duplicate P50s).
    ///
    /// Despite the name `update_*`, the operation **rewrites** statements
    /// in place — see the per-method comments inside.
    fn update_author_statements(&self, authors: &[GenericAuthorInfo], item: &mut Entity) {
        let p50 = Self::get_p50s_from_item(item);
        let mut used_candidates: HashSet<usize> = HashSet::new();

        for statement in item.claims_mut().iter_mut() {
            if statement.property() != "P2093" {
                continue;
            }
            let author = match GenericAuthorInfo::new_from_statement(statement) {
                Some(a) => a,
                None => continue,
            };
            let (candidate, _points) = match author.find_best_match(authors) {
                Some(m) => m,
                None => continue,
            };
            let q = match authors[candidate].wikidata_item() {
                Some(q) => q,
                None => continue,
            };
            if p50.iter().any(|s| s == q) || used_candidates.contains(&candidate) {
                // Author already has P50 or candidate already used; remove redundant P2093
                Self::remove_p2093_statement(statement);
            } else if let Some(p50_statement) = &authors[candidate].generate_author_statement() {
                Self::replace_p2093_with_p50_in_place(p50_statement, statement);
                used_candidates.insert(candidate);
            }
        }

        Self::remove_statements_with_no_value(item);
    }

    pub fn create_or_update_author_statements(
        &mut self,
        item: &mut Entity,
        authors: &Vec<GenericAuthorInfo>,
    ) {
        // TODO check for duplicate P50/P2093
        if !item.has_claims_with_property("P50") && !item.has_claims_with_property("P2093") {
            self.create_author_statements(authors, item);
        } else {
            self.update_author_statements(authors, item);
        }
    }

    #[allow(clippy::ptr_arg)]
    fn merge_authors(
        &self,
        authors: &mut Vec<GenericAuthorInfo>,
        authors2: &Vec<GenericAuthorInfo>,
    ) {
        // Shortcut
        if authors.is_empty() {
            *authors = authors2.clone();
            return;
        }

        for author in authors2.iter() {
            match author.find_best_match(authors) {
                Some((candidate, _points)) => match authors[candidate].merge_from(author) {
                    Ok(_) => {},
                    Err(e) => tracing::warn!(?author, error = %e, "merge_authors: merge_from failed"),
                },
                None => {
                    // Only add if there's truly no overlap with any existing author.
                    // If there's a partial match (ambiguous), skip to avoid duplicates.
                    if !author.has_partial_match(authors) {
                        authors.push(author.clone());
                    }
                },
            }
        }
    }

    pub async fn update_item_from_adapters(
        &mut self,
        item: &mut Entity,
        adapter2work_id: &mut HashMap<usize, String>,
        mw_api: Arc<RwLock<Api>>,
    ) -> Result<()> {
        let mut authors: Vec<GenericAuthorInfo> = vec![];
        for adapter_id in 0..self.adapters.len() {
            let publication_id =
                match self.adapters[adapter_id].publication_id_from_item(item).await {
                    Some(id) => id,
                    _ => continue,
                };

            let adapter = &mut self.adapters[adapter_id];
            adapter2work_id.insert(adapter_id, publication_id.clone());
            adapter
                .update_statements_for_publication_id_default(
                    &publication_id,
                    item,
                    // self.cache.clone(),
                )
                .await;
            adapter.update_statements_for_publication_id(&publication_id, item).await;

            // Authors
            let authors2 = adapter.get_author_list(&publication_id).await;
            self.merge_authors(&mut authors, &authors2);
        }

        // Set P31 (instance of) based on work type from adapters, if not already set.
        // Adapters like Crossref can determine the correct type (book, article, etc.)
        if !item.has_claims_with_property("P31") {
            let work_type_q = adapter2work_id
                .iter()
                .find_map(|(adapter_id, pub_id)| self.adapters[*adapter_id].get_work_type(pub_id))
                .unwrap_or_else(|| "Q13442814".to_string()); // default: scientific article
            item.add_claim(Statement::new_normal(
                Snak::new_item("P31", &work_type_q),
                vec![],
                vec![],
            ));
        }

        // Final deduplication pass after all sources have been merged
        // CPU-bound: O(n²) author matching with regex — offload from async runtime
        let authors = tokio::task::spawn_blocking(move || {
            GenericAuthorInfo::deduplicate(&mut authors);
            authors
        })
        .await?;

        let mut futures = vec![];
        for author in &authors {
            let future =
                author.get_or_create_author_item(mw_api.clone(), self.cache.clone(), false);
            futures.push(future);
        }
        let new_authors = futures::future::join_all(futures).await;

        self.update_author_items(&new_authors, mw_api.clone()).await;
        self.create_or_update_author_statements(item, &new_authors);
        Ok(())
    }

    pub async fn update_author_items(
        &mut self,
        authors: &Vec<GenericAuthorInfo>,
        mw_api: Arc<RwLock<Api>>,
    ) {
        let qs: Vec<String> =
            authors.iter().filter_map(|a| a.wikidata_item().map(str::to_string)).collect();
        if qs.is_empty() {
            return;
        }

        let api = mw_api.read().await;
        if self.entities.load_entities(&api, &qs).await.is_err() {
            return;
        }
        drop(api);

        for author in authors {
            author.update_author_item(&mut self.entities, mw_api.clone()).await;
        }
    }

    fn update_item_with_ids(&self, item: &mut wikibase::Entity, ids: &Vec<GenericWorkIdentifier>) {
        for id in ids {
            let prop = match id.work_type() {
                GenericWorkType::Property(prop) => prop.to_owned(),
                _ => continue,
            };
            if item.has_claims_with_property(prop.as_str()) {
                // TODO use claims_with_property to check the individual values
                continue;
            }
            let id2statement = self
                .adapters
                .iter()
                .filter(|adapter| adapter.publication_property().as_ref() == Some(&prop))
                .find_map(|adapter| adapter.publication_id_for_statement(id.id()));
            if let Some(id) = id2statement {
                item.add_claim(Statement::new_normal(
                    Snak::new_external_id(prop.as_str(), &id),
                    vec![],
                    vec![],
                ))
            }
        }
    }

    pub async fn create_or_update_item_from_ids(
        &mut self,
        mw_api: Arc<RwLock<Api>>,
        ids: &Vec<GenericWorkIdentifier>,
    ) -> Result<Option<EditResult>> {
        if ids.is_empty() {
            return Ok(None);
        }
        let items = match self.testing {
            true => vec![],
            false => self.get_items_for_ids(ids).await,
        };
        self.create_or_update_item_from_items(mw_api, ids, &items).await
    }

    pub async fn create_or_update_item_from_q(
        &mut self,
        mw_api: Arc<RwLock<Api>>,
        q: &str,
    ) -> Result<Option<EditResult>> {
        let items = vec![q.to_owned()];
        self.create_or_update_item_from_items(mw_api, &vec![], &items).await
    }

    fn new_publication_item(&self) -> Entity {
        Entity::new_empty_item()
    }

    async fn create_or_update_item_from_items(
        &mut self,
        mw_api: Arc<RwLock<Api>>,
        ids: &Vec<GenericWorkIdentifier>,
        items: &[String],
    ) -> Result<Option<EditResult>> {
        let mut item: wikibase::Entity;
        let mut original_item = Entity::new_empty_item();
        match items.first() {
            Some(q) => {
                let api = mw_api.read().await;
                item = self
                    .entities
                    .load_entity(&api, q.clone())
                    .await
                    .with_context(|| format!("load_entity({q})"))?
                    .to_owned();
                drop(api);
                original_item = item.clone();
            },
            None => item = self.new_publication_item(),
        }

        self.update_item_with_ids(&mut item, ids);

        let mut adapter2work_id = HashMap::new();
        self.update_item_from_adapters(&mut item, &mut adapter2work_id, mw_api.clone())
            .await?;

        // Paranoia
        if item.claims().len() < 4 {
            tracing::warn!(?ids, claim_count = item.claims().len(), "skipping: too few claims to write");
            return Ok(None);
        }

        self.apply_diff_for_item(original_item, item, mw_api).await
    }

    /// Applies the diff between `original_item` and `item` to Wikidata.
    ///
    /// Returns:
    /// - `Ok(Some(EditResult))` for a successful write or an empty diff
    ///   against an existing item (in which case `edited == false`),
    /// - `Ok(None)` for an empty diff against an unsaved/empty original
    ///   or for the testing/dry-run branch,
    /// - `Err(_)` if the underlying Wikidata API write fails — so the
    ///   bot loop can mark the command FAILED instead of silently DUNNO.
    pub async fn apply_diff_for_item(
        &mut self,
        original_item: Entity,
        item: Entity,
        mw_api: Arc<RwLock<Api>>,
    ) -> Result<Option<EditResult>> {
        let mut params = EntityDiffParams::none();
        params.labels.add = EntityDiffParamState::All;
        params.aliases.add = EntityDiffParamState::All;
        params.claims.add = EntityDiffParamState::All;
        params.claims.remove = EntityDiffParamState::some(&vec!["P2093"]);
        params.references.list =
            vec![(EntityDiffParamState::All, EntityDiffParamState::except(&vec!["P813"]))];
        let mut diff = EntityDiff::new(&original_item, &item, &params);
        diff.set_edit_summary(self.edit_summary.to_owned());

        if diff.is_empty() {
            return Ok(match original_item.id().as_str() {
                "" => None,
                id => Some(EditResult { q: id.to_string(), edited: false }),
            });
        }

        if self.testing {
            println!("{}", diff.to_string_pretty().unwrap());
            Ok(None)
        } else {
            // Why a write lock here at all:
            // `wikibase::EntityDiff::apply_diff` takes `&mut mediawiki::Api`
            // because internally it calls `Api::get_edit_token().await`
            // which mutates the cached CSRF token. The actual edit POST
            // (`post_query_api_json`) only needs `&Api` — but the
            // upstream API doesn't expose the two halves separately, so
            // we have to hold the write lock for the whole call.
            //
            // Concurrent edits on one Api therefore serialise here; that
            // bottleneck is **upstream-bounded** and the path forward is
            // a `wikibase` PR splitting the two phases. See audits/STATUS.md P1-3.
            //
            // What we *can* do is release the lock as soon as
            // `apply_diff` returns, so the post-edit JSON parsing below
            // doesn't keep blocking other writers.
            let new_json = {
                let mut api = mw_api.write().await;
                diff.apply_diff(&mut api, &diff)
                    .await
                    .map_err(|e| anyhow!("apply_diff_for_item: apply_diff failed: {e}"))?
                // `api` (the write lock guard) drops here.
            };
            match EntityDiff::get_entity_id(&new_json) {
                Some(q) => Ok(Some(EditResult { q, edited: true })),
                None => Ok(None),
            }
        }
    }

    pub async fn update_from_paper_ids(
        &mut self,
        original_ids: &[GenericWorkIdentifier],
    ) -> Vec<GenericWorkIdentifier> {
        let mut ids: HashSet<GenericWorkIdentifier> = HashSet::new();
        original_ids.iter().filter(|id| id.is_legit()).for_each(|id| {
            ids.insert(id.to_owned());
        });
        // Hard-cap the convergence loop: a misbehaving adapter that returns
        // a new ID on every call would otherwise spin forever. Real adapters
        // converge in 2–3 passes; MAX_ID_DISCOVERY_PASSES leaves slack for
        // legitimate multi-hop discovery (DOI → PMID → PMCID etc.) without
        // letting bad data hang the bot.
        for _ in 0..MAX_ID_DISCOVERY_PASSES {
            let last_id_size = ids.len();
            for adapter_id in 0..self.adapters.len() {
                let adapter = &mut self.adapters[adapter_id];
                // The id set is tiny (typically <20) — par_iter's thread-pool
                // dispatch costs more than the work it parallelises, and would
                // also block the tokio worker. Sequential is faster here.
                let vids: Vec<GenericWorkIdentifier> = ids.iter().cloned().collect();
                adapter
                    .get_identifier_list(&vids)
                    .await
                    .iter()
                    .filter(|id| id.is_legit())
                    .for_each(|id| {
                        ids.insert(id.clone());
                    });
            }
            if last_id_size == ids.len() {
                break;
            }
        }
        ids.iter().filter(|id| id.is_legit()).cloned().collect()
    }

    pub async fn get_items_for_ids(&self, ids: &Vec<GenericWorkIdentifier>) -> Vec<String> {
        let mut items: Vec<String> = vec![];
        for id in ids {
            let r = match id.work_type() {
                GenericWorkType::Property(prop) => self.cache.get(prop.as_str(), id.id()).await,
                GenericWorkType::Item => Some(id.id().to_owned()),
            };
            if let Some(q) = r {
                items.push(q)
            }
        }
        // CPU work: sort + dedup on potentially large identifier list
        tokio::task::spawn_blocking(move || {
            items.sort();
            items.dedup();
            items
        })
        .await
        .unwrap_or_default()
    }

    pub fn entities_mut(&mut self) -> &mut entity_container::EntityContainer {
        &mut self.entities
    }

    fn get_p50s_from_item(item: &mut Entity) -> Vec<String> {
        item.claims()
            .par_iter()
            .filter(|statement| statement.property() == "P50")
            .filter_map(|statement| match statement.main_snak().data_value().as_ref()?.value() {
                Value::Entity(entity) => Some(entity.id().to_string()),
                _ => None,
            })
            .collect()
    }

    /// Merges qualifiers from a P2093 statement (already-on-Wikidata) and a
    /// generated P50 statement (from the adapter), with **P2093 taking
    /// precedence** on conflicting properties — the existing Wikidata data
    /// is authoritative.
    ///
    /// Also adds the canonical P1932 ("object named as") qualifier built
    /// from the P2093's *original* author-name-string main snak, so that
    /// any accent/formatting differences in the adapter's name don't
    /// silently lose the original spelling Wikidata had.
    fn merge_qualifiers_with_p2093_precedence(
        p2093_statement: &Statement,
        p50_statement: &Statement,
    ) -> Vec<Snak> {
        let p2093_props: HashSet<&str> =
            p2093_statement.qualifiers().iter().map(|q| q.property()).collect();

        // Start with all of P2093's qualifiers (precedence).
        let mut merged: Vec<Snak> = p2093_statement.qualifiers().to_vec();

        // Add P50's non-conflicting qualifiers, but always exclude its P1932 —
        // we rebuild that from the P2093's actual name string below.
        for q in p50_statement.qualifiers() {
            if q.property() != "P1932" && !p2093_props.contains(q.property()) {
                merged.push(q.clone());
            }
        }

        // P1932 always comes from the P2093's main-snak string value.
        if let Some(dv) = p2093_statement.main_snak().data_value() {
            if let Value::StringValue(author_name_string) = dv.value() {
                merged.push(Snak::new_string("P1932", author_name_string));
            }
        }
        merged
    }

    /// Replaces `p2093_statement` in place with a P50 statement that
    /// carries the merged qualifiers (P2093 precedence — see
    /// [`Self::merge_qualifiers_with_p2093_precedence`]) and preserves
    /// the references that were on the original P2093.
    ///
    /// The function is named `replace_*_in_place` rather than `update_*`
    /// because it literally builds a new `Statement` and assigns it via
    /// `*p2093_statement = …`; calling it "update" historically hid
    /// that the entire statement is rewritten.
    fn replace_p2093_with_p50_in_place(
        p50_statement: &Statement,
        p2093_statement: &mut Statement,
    ) {
        let merged_qualifiers =
            Self::merge_qualifiers_with_p2093_precedence(p2093_statement, p50_statement);
        // Preserve references from the original P2093 — they may carry the
        // P813 ("retrieved") timestamp we want to keep.
        let references = p2093_statement.references().to_owned();
        *p2093_statement = Statement::new_normal(
            p50_statement.main_snak().clone(),
            merged_qualifiers,
            references,
        );
    }

    fn remove_p2093_statement(p2093_statement: &mut Statement) {
        // HACK change to "no value", then remove downstream
        p2093_statement.set_main_snak(SNAK_REMOVE_STATEMENT.to_owned());
    }

    fn remove_statements_with_no_value(item: &mut Entity) {
        // Remove no-value P2093s
        item.claims_mut()
            .retain(|statement| *statement.main_snak() != *SNAK_REMOVE_STATEMENT);
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    /// Helper: create a P2093 statement with name and ordinal qualifier P1545
    fn make_p2093(name: &str, ordinal: &str) -> Statement {
        Statement::new_normal(
            Snak::new_string("P2093", name),
            vec![Snak::new_string("P1545", ordinal)],
            vec![],
        )
    }

    /// Helper: create a WikidataPapers with no adapters (for unit testing).
    /// Uses a wiremock server for the MediaWiki API so no real network calls
    /// are made.
    async fn make_wdp() -> WikidataPapers {
        use wiremock::{
            matchers::{method, query_param},
            Mock, MockServer, ResponseTemplate,
        };

        const SITEINFO: &str = include_str!("../test_data/api_siteinfo.json");

        let mock_server = MockServer::start().await;
        Mock::given(method("GET"))
            .and(query_param("meta", "siteinfo"))
            .respond_with(
                ResponseTemplate::new(200)
                    .insert_header("content-type", "application/json; charset=utf-8")
                    .set_body_string(SITEINFO),
            )
            .mount(&mock_server)
            .await;

        let mw_api = Arc::new(tokio::sync::RwLock::new(
            wikibase::mediawiki::api::Api::new(&mock_server.uri()).await.unwrap(),
        ));
        let cache = Arc::new(WikidataStringCache::new(mw_api));
        // mock_server is intentionally dropped here: the tests below do not
        // make any further HTTP calls, so the server is no longer needed.
        WikidataPapers::new(cache)
    }

    fn get_string_qualifier(statement: &Statement, property: &str) -> Option<String> {
        statement.qualifiers().iter().find_map(|q| {
            if q.property() != property {
                return None;
            }
            match q.data_value().as_ref()?.value() {
                Value::StringValue(s) => Some(s.to_string()),
                _ => None,
            }
        })
    }

    /// Extract P1545 qualifier value from a statement
    fn get_ordinal(statement: &Statement) -> Option<String> {
        get_string_qualifier(statement, "P1545")
    }

    // === Bug reproduction: ordinal swapping (issue #3) ===
    //
    // When a P2093 "Giorgio Alimonti" at ordinal #42 is matched to adapter
    // author with the same name but ordinal #44, the resulting P50 should
    // keep ordinal #42 (from Wikidata), not #44 (from the adapter).

    #[tokio::test]
    async fn update_author_statements_preserves_p2093_ordinal() {
        let wdp = make_wdp().await;
        let mut item = Entity::new_empty_item();
        item.add_claim(make_p2093("Giorgio Alimonti", "42"));

        // Adapter author with same name but different ordinal
        let mut adapter_author = GenericAuthorInfo::new();
        adapter_author.set_name(Some("Giorgio Alimonti".to_string()));
        adapter_author.set_wikidata_item(Some("Q64863661".to_string()));
        adapter_author.set_list_number(Some("44".to_string())); // different ordinal!

        wdp.update_author_statements(&[adapter_author], &mut item);

        // Should have exactly one claim: a P50 for Q64863661
        assert_eq!(item.claims().len(), 1, "Expected exactly one claim");
        let claim = &item.claims()[0];
        assert_eq!(claim.main_snak().property(), "P50", "Should be P50");

        // The ordinal should be #42 (from the original P2093), NOT #44 (from the
        // adapter)
        let ordinal = get_ordinal(claim);
        assert_eq!(
            ordinal,
            Some("42".to_string()),
            "Ordinal should be preserved from the original P2093 statement, not taken from adapter"
        );
    }

    // === Bug reproduction: duplicate Q-item assignment (issue #3) ===
    //
    // If two P2093 statements for name variants of the same person both
    // match the same adapter author, only one P50 should be created and
    // the other P2093 should be removed.

    #[tokio::test]
    async fn update_author_statements_prevents_duplicate_p50() {
        let wdp = make_wdp().await;
        let mut item = Entity::new_empty_item();
        // Two P2093s for what is effectively the same person (different name orderings)
        item.add_claim(make_p2093("Giorgio Alimonti", "42"));
        item.add_claim(make_p2093("Alimonti Giorgio", "44"));

        let mut adapter_author = GenericAuthorInfo::new();
        adapter_author.set_name(Some("Giorgio Alimonti".to_string()));
        adapter_author.set_wikidata_item(Some("Q64863661".to_string()));
        adapter_author.set_list_number(Some("42".to_string()));

        wdp.update_author_statements(&[adapter_author], &mut item);

        // Count how many P50 statements reference Q64863661
        let p50_count = item
            .claims()
            .iter()
            .filter(|s| s.property() == "P50")
            .filter(|s| match s.main_snak().data_value() {
                Some(dv) => matches!(dv.value(), Value::Entity(e) if e.id() == "Q64863661"),
                None => false,
            })
            .count();

        assert!(
            p50_count <= 1,
            "Same Q-item should not appear in multiple P50 statements, got {}",
            p50_count
        );
    }

    // === Bug reproduction: conflicting ordinals (issue #3) ===
    //
    // When adapter gives two different authors the same ordinal but Wikidata
    // has them at different ordinals, the Wikidata ordinals should be preserved.

    #[tokio::test]
    async fn update_author_statements_no_conflicting_ordinals() {
        let wdp = make_wdp().await;
        let mut item = Entity::new_empty_item();
        // Two different authors at different positions on Wikidata
        item.add_claim(make_p2093("Jahred Adelman", "15"));
        item.add_claim(make_p2093("Stephanie Zimmermann", "2879"));

        // Adapter has both authors with Q-items but erroneously gives same ordinal
        let mut author1 = GenericAuthorInfo::new();
        author1.set_name(Some("Jahred Adelman".to_string()));
        author1.set_wikidata_item(Some("Q100001".to_string()));
        author1.set_list_number(Some("15".to_string()));

        let mut author2 = GenericAuthorInfo::new();
        author2.set_name(Some("Stephanie Zimmermann".to_string()));
        author2.set_wikidata_item(Some("Q100002".to_string()));
        author2.set_list_number(Some("15".to_string())); // adapter erroneously gives same ordinal

        wdp.update_author_statements(&[author1, author2], &mut item);

        // Collect ordinals from all claims
        let ordinals: Vec<String> = item.claims().iter().filter_map(get_ordinal).collect();

        // Ordinals should not conflict - each should be unique
        let mut unique = ordinals.clone();
        unique.sort();
        unique.dedup();
        assert_eq!(
            ordinals.len(),
            unique.len(),
            "Ordinals should be unique, but got duplicates: {:?}",
            ordinals
        );
    }

    /// Extract P1932 ("object named as") qualifier value from a statement
    fn get_named_as(statement: &Statement) -> Option<String> {
        get_string_qualifier(statement, "P1932")
    }

    // === Bug reproduction: P1932 uses adapter name instead of P2093 name (issue
    // #5) ===
    //
    // When converting P2093 "José García" to P50, the P1932 ("object named as")
    // qualifier should preserve the original P2093 name, not the adapter's
    // version (which may differ in accents, formatting, etc.).

    #[tokio::test]
    async fn update_author_statements_p1932_preserves_original_name() {
        let wdp = make_wdp().await;
        let mut item = Entity::new_empty_item();
        // P2093 with accented name as it appears on Wikidata
        item.add_claim(make_p2093("José García", "5"));

        // Adapter has ASCII-ified version of the name
        let mut adapter_author = GenericAuthorInfo::new();
        adapter_author.set_name(Some("Jose Garcia".to_string())); // different from Wikidata!
        adapter_author.set_wikidata_item(Some("Q12345".to_string()));
        adapter_author.set_list_number(Some("5".to_string()));

        wdp.update_author_statements(&[adapter_author], &mut item);

        assert_eq!(item.claims().len(), 1);
        let claim = &item.claims()[0];
        assert_eq!(claim.main_snak().property(), "P50");

        // P1932 should preserve the original P2093 name "José García",
        // NOT the adapter's "Jose Garcia"
        let named_as = get_named_as(claim);
        assert_eq!(
            named_as,
            Some("José García".to_string()),
            "P1932 should preserve the original author name string from Wikidata"
        );
    }

    // Same issue but with a completely different adapter name (e.g.
    // transliteration)
    #[tokio::test]
    async fn update_author_statements_p1932_preserves_original_name_transliteration() {
        let wdp = make_wdp().await;
        let mut item = Entity::new_empty_item();
        item.add_claim(make_p2093("Smith, John A.", "1"));

        // Adapter reformats the name
        let mut adapter_author = GenericAuthorInfo::new();
        adapter_author.set_name(Some("John Smith".to_string())); // reordered, no middle initial
        adapter_author.set_wikidata_item(Some("Q99999".to_string()));
        adapter_author.set_list_number(Some("1".to_string()));

        wdp.update_author_statements(&[adapter_author], &mut item);

        assert_eq!(item.claims().len(), 1);
        let claim = &item.claims()[0];
        assert_eq!(claim.main_snak().property(), "P50");

        // P1932 must be "Smith, John A." (the original), not "John Smith" (adapter)
        let named_as = get_named_as(claim);
        assert_eq!(
            named_as,
            Some("Smith, John A.".to_string()),
            "P1932 should preserve the original author name string from Wikidata"
        );
    }

    // === Bug reproduction: wrong P31 for non-article works (issue #14) ===

    #[tokio::test]
    async fn new_publication_item_has_no_p31() {
        // new_publication_item should NOT hardcode P31, so that adapters
        // can set it based on the actual work type.
        let wdp = make_wdp().await;
        let item = wdp.new_publication_item();
        assert!(
            !item.has_claims_with_property("P31"),
            "new_publication_item should not set P31; it should be set later based on work type"
        );
    }

    // === P1-6: convergence-loop termination =====================================

    /// A fake adapter that returns a brand-new DOI on every call to
    /// `get_identifier_list`. Without the cap in `update_from_paper_ids`,
    /// driving this adapter would grow the id set forever and hang the bot.
    struct FakeGrowingAdapter {
        counter: u64,
        author_cache: HashMap<String, String>,
    }

    #[async_trait::async_trait(?Send)]
    impl crate::scientific_publication_adapter::ScientificPublicationAdapter for FakeGrowingAdapter {
        fn name(&self) -> &str {
            "FakeGrowingAdapter"
        }
        fn author_cache(&self) -> &HashMap<String, String> {
            &self.author_cache
        }
        fn author_cache_mut(&mut self) -> &mut HashMap<String, String> {
            &mut self.author_cache
        }
        async fn update_statements_for_publication_id(&self, _: &str, _: &mut Entity) {}
        async fn get_identifier_list(
            &mut self,
            _ids: &[GenericWorkIdentifier],
        ) -> Vec<GenericWorkIdentifier> {
            let n = self.counter;
            self.counter += 1;
            vec![GenericWorkIdentifier::new_prop(
                crate::identifiers::IdProp::DOI,
                &format!("10.0/test{n}"),
            )]
        }
    }

    #[tokio::test]
    async fn update_from_paper_ids_terminates_with_misbehaving_adapter() {
        // A growing adapter would loop forever without MAX_ID_DISCOVERY_PASSES.
        // We assert termination via tokio::time::timeout (well over what a
        // bounded run takes, well under what a runaway loop would burn).
        let mut wdp = make_wdp().await;
        wdp.add_adapter(Box::new(FakeGrowingAdapter {
            counter: 0,
            author_cache: HashMap::new(),
        }));

        let initial = vec![GenericWorkIdentifier::new_prop(
            crate::identifiers::IdProp::DOI,
            "10.0/initial",
        )];
        let result = tokio::time::timeout(
            std::time::Duration::from_secs(5),
            wdp.update_from_paper_ids(&initial),
        )
        .await;

        let ids = result.expect("update_from_paper_ids must terminate even with a growing adapter");
        // The loop runs at most MAX_ID_DISCOVERY_PASSES; the fake adapter
        // produces one new id per pass, so the final set is bounded by
        // 1 (initial) + MAX_ID_DISCOVERY_PASSES.
        assert!(
            ids.len() <= 1 + MAX_ID_DISCOVERY_PASSES,
            "expected at most {} ids, got {}",
            1 + MAX_ID_DISCOVERY_PASSES,
            ids.len()
        );
        // And we should make some progress: at least the initial id survives.
        assert!(!ids.is_empty(), "expected at least the initial id to survive");
    }

    #[tokio::test]
    async fn update_from_paper_ids_with_no_adapters_returns_legit_inputs() {
        // Empty adapter list: the loop exits on the first pass because
        // last_id_size == ids.len(). Output should be the legit inputs
        // (DOIs are canonicalised to uppercase by GenericWorkIdentifier).
        let mut wdp = make_wdp().await;
        let initial =
            vec![GenericWorkIdentifier::new_prop(crate::identifiers::IdProp::DOI, "10.0/a")];
        let ids = wdp.update_from_paper_ids(&initial).await;
        assert_eq!(ids.len(), 1);
        assert_eq!(ids[0].id(), "10.0/A");
    }

    #[tokio::test]
    async fn update_from_paper_ids_filters_non_legit_inputs() {
        // GenericWorkIdentifier::is_legit rejects empty and "0" ids;
        // those must not survive into the output set.
        let mut wdp = make_wdp().await;
        let initial = vec![
            GenericWorkIdentifier::new_prop(crate::identifiers::IdProp::DOI, "10.0/legit"),
            GenericWorkIdentifier::new_prop(crate::identifiers::IdProp::DOI, ""),
            GenericWorkIdentifier::new_prop(crate::identifiers::IdProp::DOI, "0"),
        ];
        let ids = wdp.update_from_paper_ids(&initial).await;
        assert_eq!(ids.len(), 1, "expected only the legit DOI to survive; got {ids:?}");
        assert_eq!(ids[0].id(), "10.0/LEGIT");
    }

    // === P2-T2: merge_authors ===============================================
    //
    // The audit specifically called out the no-overlap branch (no match
    // found, and no partial-match either → append the new author) and
    // the partial-match branch (ambiguous overlap → skip to avoid
    // duplicates). Both are private inherent methods on WikidataPapers
    // and are testable via the in-module test access.

    fn author_named(name: &str, list_num: usize) -> GenericAuthorInfo {
        GenericAuthorInfo::new_from_name_num(name, list_num)
    }

    #[tokio::test]
    async fn merge_authors_shortcut_when_existing_is_empty() {
        // Shortcut: if `authors` starts empty, just clone authors2.
        let wdp = make_wdp().await;
        let mut authors = vec![];
        let authors2 = vec![author_named("Alice Smith", 1), author_named("Bob Jones", 2)];
        wdp.merge_authors(&mut authors, &authors2);
        assert_eq!(authors.len(), 2);
        assert_eq!(authors[0].name(), Some("Alice Smith"));
        assert_eq!(authors[1].name(), Some("Bob Jones"));
    }

    #[tokio::test]
    async fn merge_authors_appends_when_no_overlap() {
        // No partial match anywhere → the new author is appended.
        // (Tests the no-overlap branch the audit named.)
        let wdp = make_wdp().await;
        let mut authors = vec![author_named("Alice Smith", 1)];
        let authors2 = vec![author_named("Bob Jones", 2)];
        wdp.merge_authors(&mut authors, &authors2);
        assert_eq!(authors.len(), 2);
        assert!(authors.iter().any(|a| a.name() == Some("Bob Jones")));
    }

    #[tokio::test]
    async fn merge_authors_merges_when_best_match_found() {
        // find_best_match returns Some → merge_from is called.
        // Verify the merge happened by checking that data from author2 was
        // absorbed into authors[0] (here: a prop2id entry was added).
        let wdp = make_wdp().await;
        let mut a1 = author_named("Alice Smith", 1);
        let mut a2 = author_named("Alice Smith", 1);
        a2.prop2id_mut().insert("P496".to_string(), "0000-0001-2345-6789".to_string());
        let mut authors = vec![a1.clone()];
        let authors2 = vec![a2];
        wdp.merge_authors(&mut authors, &authors2);
        assert_eq!(authors.len(), 1, "exact-match merge must not duplicate");
        assert_eq!(
            authors[0].prop2id().get("P496"),
            Some(&"0000-0001-2345-6789".to_string()),
            "ORCID from author2 should have merged into authors[0]"
        );
        // Suppress the unused-must-use lint on the clone we built for setup.
        let _ = &mut a1;
    }

    #[tokio::test]
    async fn merge_authors_skips_partial_match_to_avoid_duplicates() {
        // The no-best-match-but-partial-overlap branch: an author who
        // partially matches an existing one (e.g. same prop2id but
        // different name) should NOT be appended — that would create a
        // duplicate person in the paper.
        let wdp = make_wdp().await;
        let mut a1 = author_named("Alice Smith", 1);
        a1.prop2id_mut().insert("P496".to_string(), "0000-0001-2345-6789".to_string());

        // Same ORCID, different name (partial match via has_partial_match).
        let mut a2 = author_named("A. Smith", 5);
        a2.prop2id_mut().insert("P496".to_string(), "0000-0001-2345-6789".to_string());

        let mut authors = vec![a1];
        let authors2 = vec![a2];
        wdp.merge_authors(&mut authors, &authors2);
        // Partial match → either merged or skipped, depending on
        // find_best_match score. Either way, no duplicate must appear.
        assert!(
            authors.len() <= 1 || authors.iter().filter(|a| a.prop2id().get("P496").is_some()).count() <= 1,
            "ORCID-sharing authors must not appear twice; got {authors:?}",
        );
    }

    // === P2-T2: create_author_statements dedup ============================

    #[tokio::test]
    async fn create_author_statements_dedups_by_wikidata_item() {
        // Two authors with the same wikidata_item must not produce two P50s.
        let mut wdp = make_wdp().await;
        let mut a1 = author_named("Alice Smith", 1);
        a1.set_wikidata_item(Some("Q100".to_string()));
        let mut a2 = author_named("A. Smith", 2);
        a2.set_wikidata_item(Some("Q100".to_string())); // same Q!

        let mut item = Entity::new_empty_item();
        wdp.create_author_statements(&vec![a1, a2], &mut item);

        let p50_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P50").count();
        assert_eq!(p50_count, 1, "duplicate Q-item must produce only one P50");
    }

    #[tokio::test]
    async fn create_author_statements_dedups_by_name_when_no_item() {
        // Two authors with the same simplified name and NO wikidata_item
        // must not produce two P2093s.
        let mut wdp = make_wdp().await;
        let a1 = author_named("Alice Smith", 1);
        let a2 = author_named("alice smith", 2); // simplified-equivalent

        let mut item = Entity::new_empty_item();
        wdp.create_author_statements(&vec![a1, a2], &mut item);

        let p2093_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P2093").count();
        assert_eq!(p2093_count, 1, "duplicate simplified-name must produce only one P2093");
    }

    #[tokio::test]
    async fn create_author_statements_distinct_authors_both_produce_statements() {
        // Sanity: distinct names + distinct items each get their own statement.
        let mut wdp = make_wdp().await;
        let mut a1 = author_named("Alice Smith", 1);
        a1.set_wikidata_item(Some("Q100".to_string()));
        let a2 = author_named("Bob Jones", 2); // no item, will be P2093

        let mut item = Entity::new_empty_item();
        wdp.create_author_statements(&vec![a1, a2], &mut item);

        let p50_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P50").count();
        let p2093_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P2093").count();
        assert_eq!(p50_count, 1, "Alice's Q-item should produce a P50");
        assert_eq!(p2093_count, 1, "Bob's nameless author should produce a P2093");
    }

    #[tokio::test]
    async fn create_author_statements_item_and_nameless_dup_with_same_simplified_name() {
        // Edge case: a1 has wikidata_item (gets P50, skips name-dedup);
        // a2 has the same simplified name but no item (gets P2093, no
        // collision with a1's seen_q_items set, so it's added).
        // Verifies the two dedup sets (q_items, names) are independent.
        let mut wdp = make_wdp().await;
        let mut a1 = author_named("Alice Smith", 1);
        a1.set_wikidata_item(Some("Q100".to_string()));
        let a2 = author_named("Alice Smith", 2); // no item, same name

        let mut item = Entity::new_empty_item();
        wdp.create_author_statements(&vec![a1, a2], &mut item);

        let p50_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P50").count();
        let p2093_count =
            item.claims().iter().filter(|s| s.main_snak().property() == "P2093").count();
        assert_eq!(p50_count, 1);
        assert_eq!(p2093_count, 1, "nameless author with same name as a Q-item author still gets a P2093");
    }
}