diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 8fbacaf0..1025e213 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1488,6 +1488,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serde_urlencoded", "sha2 0.10.9", "sqlx", "tokio", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 443b45dd..4c128bee 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -56,6 +56,9 @@ utoipa-swagger-ui = { version = "8", features = ["axum"] } # Serialization / JSONB payloads serde = { version = "1", features = ["derive"] } serde_json = "1" +# Query-string (de)serialization — used directly by crate::extract::Query to +# dedupe repeated keys before deserializing (axum already pulls it transitively). +serde_urlencoded = "0.7" # Common types uuid = { version = "1", features = ["v4", "serde"] } diff --git a/rust/crates/du-db/src/biosample.rs b/rust/crates/du-db/src/biosample.rs index e9c3fb88..c9524ce9 100644 --- a/rust/crates/du-db/src/biosample.rs +++ b/rust/crates/du-db/src/biosample.rs @@ -131,6 +131,10 @@ pub enum HaplogroupCallOrigin { Reconciled, /// `fed.biosample.y/mt_haplogroup` (a single Navigator call, not reconciled). FedConsensus, + /// `tree.haplogroup_sample` — the node this sample sits under in the decoding-us + /// de-novo tree. The sample was used as a tree building block, so its tip position + /// is an authoritative assignment (preferred over the raw publication call). + TreePlacement, /// `core.biosample.original_haplogroups` (per-publication original call). Original, } @@ -427,53 +431,69 @@ pub async fn report_by_guid(pool: &PgPool, guid: SampleGuid) -> Result = None; + let mut placed_mt: Option = None; + { + #[derive(sqlx::FromRow)] + struct PlaceRow { + dna_type: Option, + name: Option, + } + let rows: Vec = sqlx::query_as( + "SELECT DISTINCT ON (hs.dna_type) hs.dna_type::text AS dna_type, h.name \ + FROM tree.haplogroup_sample hs \ + JOIN core.biosample b2 ON b2.sample_guid = hs.sample_guid \ + JOIN tree.haplogroup h ON h.id = hs.haplogroup_id AND h.valid_until IS NULL \ + WHERE hs.status IN ('PLACED', 'CURATED') \ + AND (b2.sample_guid = $1 \ + OR b2.donor_id = (SELECT donor_id FROM core.biosample WHERE sample_guid = $1 AND donor_id IS NOT NULL)) \ + ORDER BY hs.dna_type, (b2.sample_guid = $1) DESC, (hs.status = 'CURATED') DESC, b2.sample_guid", + ) + .bind(guid.0) + .fetch_all(pool) + .await?; + for r in rows { + match r.dna_type.as_deref() { + Some("Y_DNA") => placed_y = r.name, + Some("MT_DNA") => placed_mt = r.name, + _ => {} + } + } + } + + // A plain (no-reliability) call from a name + origin — for the non-reconciled sources. + let plain = |name: String, dna_type: DnaType, origin: HaplogroupCallOrigin| HaplogroupCall { + name, + dna_type, + origin, + confidence: None, + run_count: None, + snp_concordance: None, + compatibility_level: None, + }; + // Call precedence: cross-technology consensus, else the single federated call, - // else the newest original publication call. + // else the de-novo tree placement, else the newest original publication call. let y = reconciled_call(recon_y, DnaType::YDna) + .or_else(|| fed_y.map(|n| plain(n, DnaType::YDna, HaplogroupCallOrigin::FedConsensus))) + .or_else(|| placed_y.map(|n| plain(n, DnaType::YDna, HaplogroupCallOrigin::TreePlacement))) .or_else(|| { - fed_y.map(|name| HaplogroupCall { - name, - dna_type: DnaType::YDna, - origin: HaplogroupCallOrigin::FedConsensus, - confidence: None, - run_count: None, - snp_concordance: None, - compatibility_level: None, - }) - }) - .or_else(|| { - pick_original_call(&idr.original_haplogroups, "y", "y_result").map(|name| HaplogroupCall { - name, - dna_type: DnaType::YDna, - origin: HaplogroupCallOrigin::Original, - confidence: None, - run_count: None, - snp_concordance: None, - compatibility_level: None, - }) + pick_original_call(&idr.original_haplogroups, "y", "y_result") + .map(|n| plain(n, DnaType::YDna, HaplogroupCallOrigin::Original)) }); let mt = reconciled_call(recon_mt, DnaType::MtDna) + .or_else(|| fed_mt.map(|n| plain(n, DnaType::MtDna, HaplogroupCallOrigin::FedConsensus))) + .or_else(|| placed_mt.map(|n| plain(n, DnaType::MtDna, HaplogroupCallOrigin::TreePlacement))) .or_else(|| { - fed_mt.map(|name| HaplogroupCall { - name, - dna_type: DnaType::MtDna, - origin: HaplogroupCallOrigin::FedConsensus, - confidence: None, - run_count: None, - snp_concordance: None, - compatibility_level: None, - }) - }) - .or_else(|| { - pick_original_call(&idr.original_haplogroups, "mt", "mt_result").map(|name| HaplogroupCall { - name, - dna_type: DnaType::MtDna, - origin: HaplogroupCallOrigin::Original, - confidence: None, - run_count: None, - snp_concordance: None, - compatibility_level: None, - }) + pick_original_call(&idr.original_haplogroups, "mt", "mt_result") + .map(|n| plain(n, DnaType::MtDna, HaplogroupCallOrigin::Original)) }); // ── Q3/Q4/Q5: federated sequencing, coverage, ancestry (only when linked) ── diff --git a/rust/crates/du-db/src/dedup.rs b/rust/crates/du-db/src/dedup.rs index 4aeb787b..b9f89ffc 100644 --- a/rust/crates/du-db/src/dedup.rs +++ b/rust/crates/du-db/src/dedup.rs @@ -386,6 +386,9 @@ struct Repoint { const REPOINTS: &[Repoint] = &[ Repoint { table: "fed.pds_submission", cols: &["biosample_guid"], action: Action::Simple }, Repoint { table: "genomics.biosample_callable_loci", cols: &["sample_guid"], action: Action::Simple }, + // STR profile is 1-per-sample (mig 0053), keyed by sample_guid — keep the + // survivor's if it already has one, else adopt the merged sample's. + Repoint { table: "genomics.biosample_str_profile", cols: &["sample_guid"], action: Action::KeepSurvivor(&[]) }, Repoint { table: "genomics.genotype_data", cols: &["sample_guid"], action: Action::Simple }, Repoint { table: "genomics.reported_variant_pangenome", cols: &["sample_guid"], action: Action::Simple }, Repoint { table: "genomics.sequence_library", cols: &["sample_guid"], action: Action::Simple }, @@ -527,14 +530,16 @@ pub async fn merge_biosamples( } Action::KeepSurvivor(others) => { let col = r.cols[0]; - let on = others + // Extra AND-clauses that make a merged row a *duplicate* of a survivor + // row. Empty ⇒ the table is unique on `col` alone (1-per-sample), so the + // mere existence of a survivor row makes the merged one a duplicate. + let extra = others .iter() - .map(|oc| format!("s.{oc} = m.{oc}")) - .collect::>() - .join(" AND "); + .map(|oc| format!(" AND s.{oc} = m.{oc}")) + .collect::(); let dropped = sqlx::query(&format!( "DELETE FROM {t} m WHERE m.{col} = $2 \ - AND EXISTS (SELECT 1 FROM {t} s WHERE s.{col} = $1 AND {on})", + AND EXISTS (SELECT 1 FROM {t} s WHERE s.{col} = $1{extra})", t = r.table )) .bind(survivor) diff --git a/rust/crates/du-db/src/denovo.rs b/rust/crates/du-db/src/denovo.rs index ebfcce34..7105bcfb 100644 --- a/rust/crates/du-db/src/denovo.rs +++ b/rust/crates/du-db/src/denovo.rs @@ -436,9 +436,31 @@ pub async fn load(pool: &PgPool, doc: &DenovoTree, keep_private: bool) -> Result _ => ("STANDARD", false), }; let attrs = json!({ "cohort": tip.cohort, "sex": tip.sex, "denovo": true }); + // Resolve the donor (the entity that links one individual's biosample rows): + // if a catalog accession already carries this panel id as its alias/accession, + // reuse that donor so the tip joins the same person instead of forming a + // donor-less orphan; else create a donor keyed by the panel id. This keeps the + // tip donor-linked across reloads (consolidate-donors still merges the case + // where the individual has more than one catalog donor). + let donor_id: i64 = sqlx::query_scalar( + "WITH existing AS ( \ + SELECT donor_id FROM core.biosample \ + WHERE donor_id IS NOT NULL AND deleted = false \ + AND (lower(alias) = lower($1) OR lower(accession) = lower($1)) \ + ORDER BY (source_attrs->>'denovo' = 'true') ASC, donor_id LIMIT 1), \ + ins AS ( \ + INSERT INTO core.specimen_donor (donor_identifier, donor_type) \ + SELECT $1, $2::core.biosample_source WHERE NOT EXISTS (SELECT 1 FROM existing) \ + RETURNING id) \ + SELECT donor_id FROM existing UNION ALL SELECT id FROM ins LIMIT 1", + ) + .bind(&tip.sample) + .bind(src) + .fetch_one(&mut *tx) + .await?; let guid: Uuid = match sqlx::query_scalar( - "INSERT INTO core.biosample (source, accession, center_name, source_attrs, is_public) \ - VALUES ($1::core.biosample_source, $2, $3, $4, $5) \ + "INSERT INTO core.biosample (source, accession, center_name, source_attrs, is_public, donor_id) \ + VALUES ($1::core.biosample_source, $2, $3, $4, $5, $6) \ ON CONFLICT (accession) WHERE accession IS NOT NULL DO NOTHING RETURNING sample_guid", ) .bind(src) @@ -446,6 +468,7 @@ pub async fn load(pool: &PgPool, doc: &DenovoTree, keep_private: bool) -> Result .bind(tip.cohort.as_deref()) .bind(&attrs) .bind(is_public) + .bind(donor_id) .fetch_optional(&mut *tx) .await? { @@ -454,10 +477,15 @@ pub async fn load(pool: &PgPool, doc: &DenovoTree, keep_private: bool) -> Result g } None => { - sqlx::query_scalar("SELECT sample_guid FROM core.biosample WHERE accession = $1") - .bind(&tip.sample) - .fetch_one(&mut *tx) - .await? + // Already present (reload): backfill a missing donor link. + sqlx::query_scalar( + "UPDATE core.biosample SET donor_id = COALESCE(donor_id, $2), updated_at = now() \ + WHERE accession = $1 RETURNING sample_guid", + ) + .bind(&tip.sample) + .bind(donor_id) + .fetch_one(&mut *tx) + .await? } }; let call = tip.terminal_label.clone().unwrap_or_else(|| tip.sample.clone()); diff --git a/rust/crates/du-db/src/donor.rs b/rust/crates/du-db/src/donor.rs new file mode 100644 index 00000000..481385b4 --- /dev/null +++ b/rust/crates/du-db/src/donor.rs @@ -0,0 +1,134 @@ +//! Specimen-donor consolidation — the entity *above* biosamples that links the +//! multiple biosample rows belonging to one individual. +//! +//! One person is routinely represented by several `core.biosample` rows: the same +//! 1000-Genomes donor appears under an NCBI BioSample accession (`SAMN…`) **and** an +//! EBI accession (`SAMEA…`) from different publications, and — when the sample was +//! used as a de-novo tree building block — again as a tree tip keyed by the raw +//! panel id (`HG00126`). The ETL gave each accession its own `specimen_donor` and +//! left the tip donor-less, so the donor entity never actually linked them, and the +//! sample's haplogroup (recorded on the tip's tree placement) couldn't surface on +//! the catalog accessions. +//! +//! [`consolidate_denovo_donors`] repairs this: it groups biosamples by their shared +//! panel id (a tip's `accession` == a reference's `alias`), points every member at a +//! single surviving donor (the richest one — most complete metadata), fills that +//! donor's gaps + canonical identifier, and prunes the emptied donor rows. The +//! sample report then resolves the haplogroup at the donor level, so every accession +//! of the individual shows the de-novo tree placement. + +use crate::DbError; +use sqlx::PgPool; + +/// Outcome of [`consolidate_denovo_donors`]. +#[derive(Debug, Clone, Default)] +pub struct DonorConsolidationReport { + /// Distinct same-individual groups (shared panel id with a de-novo tip + ≥1 ref). + pub groups: i64, + /// Biosamples repointed to a surviving donor. + pub biosamples_repointed: u64, + /// Redundant donor rows deleted (emptied by the repoint). + pub donors_pruned: u64, +} + +/// The member set: every biosample whose identity key (a de-novo tip's `accession`, +/// else a reference's `alias`) is a panel id shared between a de-novo tip and at +/// least one non-de-novo reference. `key` is lower-cased for grouping; `orig_id` +/// keeps the original-case id for the donor identifier. +const MEMBERS_CTE: &str = "\ + members AS ( \ + SELECT b.sample_guid, b.donor_id, \ + (CASE WHEN b.source_attrs->>'denovo' = 'true' THEN lower(b.accession) ELSE lower(b.alias) END) AS key, \ + (CASE WHEN b.source_attrs->>'denovo' = 'true' THEN b.accession ELSE b.alias END) AS orig_id \ + FROM core.biosample b \ + WHERE b.deleted = false \ + AND (CASE WHEN b.source_attrs->>'denovo' = 'true' THEN lower(b.accession) ELSE lower(b.alias) END) IN ( \ + SELECT lower(d.accession) FROM core.biosample d \ + WHERE d.source_attrs->>'denovo' = 'true' AND d.deleted = false AND d.accession IS NOT NULL \ + AND EXISTS (SELECT 1 FROM core.biosample r \ + WHERE r.deleted = false AND COALESCE(r.source_attrs->>'denovo','') <> 'true' \ + AND lower(r.alias) = lower(d.accession)) \ + ) \ + )"; + +/// Consolidate the donor rows of same-individual biosamples (see module docs). +/// Idempotent — once a group shares one donor there is nothing left to repoint or +/// prune. `apply = false` counts the work without mutating. +pub async fn consolidate_denovo_donors(pool: &PgPool, apply: bool) -> Result { + let groups: i64 = sqlx::query_scalar(&format!("WITH {MEMBERS_CTE} SELECT count(DISTINCT key) FROM members")) + .fetch_one(pool) + .await?; + let mut rep = DonorConsolidationReport { groups, ..Default::default() }; + if !apply { + return Ok(rep); + } + + let mut tx = pool.begin().await?; + + // 1) Materialize each member with its group's surviving donor: the donor with the + // most complete metadata (geocoord + biobank + sex), ties broken by lowest id. + sqlx::query(&format!( + "CREATE TEMP TABLE _donor_grp ON COMMIT DROP AS \ + WITH {MEMBERS_CTE}, \ + survivor AS ( \ + SELECT DISTINCT ON (m.key) m.key, sd.id AS survivor_donor \ + FROM (SELECT DISTINCT key, donor_id FROM members WHERE donor_id IS NOT NULL) m \ + JOIN core.specimen_donor sd ON sd.id = m.donor_id \ + ORDER BY m.key, \ + ((sd.geocoord IS NOT NULL)::int + (sd.origin_biobank IS NOT NULL)::int + (sd.sex IS NOT NULL)::int) DESC, \ + sd.id \ + ) \ + SELECT m.sample_guid, m.donor_id, m.key, m.orig_id, s.survivor_donor \ + FROM members m JOIN survivor s ON s.key = m.key" + )) + .execute(&mut *tx) + .await?; + + // 2) Fill the survivor donor's gaps from its group siblings + set the canonical + // identifier (original-case panel id) — do this BEFORE the repoint, while the + // other donors still hold their metadata. + sqlx::query( + "UPDATE core.specimen_donor s SET \ + sex = COALESCE(s.sex, agg.sex), \ + geocoord = COALESCE(s.geocoord, agg.geocoord), \ + origin_biobank = COALESCE(s.origin_biobank, agg.biobank), \ + donor_identifier = agg.ident, \ + updated_at = now() \ + FROM ( \ + SELECT g.survivor_donor, \ + (array_agg(sd.sex) FILTER (WHERE sd.sex IS NOT NULL))[1] AS sex, \ + (array_agg(sd.geocoord) FILTER (WHERE sd.geocoord IS NOT NULL))[1] AS geocoord, \ + (array_agg(sd.origin_biobank) FILTER (WHERE sd.origin_biobank IS NOT NULL))[1] AS biobank, \ + max(g.orig_id) AS ident \ + FROM _donor_grp g LEFT JOIN core.specimen_donor sd ON sd.id = g.donor_id \ + GROUP BY g.survivor_donor \ + ) agg \ + WHERE s.id = agg.survivor_donor", + ) + .execute(&mut *tx) + .await?; + + // 3) Repoint every group member to its surviving donor. + let repointed = sqlx::query( + "UPDATE core.biosample b SET donor_id = g.survivor_donor, updated_at = now() \ + FROM _donor_grp g WHERE b.sample_guid = g.sample_guid AND b.donor_id IS DISTINCT FROM g.survivor_donor", + ) + .execute(&mut *tx) + .await? + .rows_affected(); + + // 4) Prune donors that the repoint emptied (were in a group, now unreferenced). + let pruned = sqlx::query( + "DELETE FROM core.specimen_donor sd \ + WHERE sd.id IN (SELECT DISTINCT donor_id FROM _donor_grp WHERE donor_id IS NOT NULL) \ + AND NOT EXISTS (SELECT 1 FROM core.biosample b WHERE b.donor_id = sd.id)", + ) + .execute(&mut *tx) + .await? + .rows_affected(); + + tx.commit().await?; + rep.biosamples_repointed = repointed; + rep.donors_pruned = pruned; + Ok(rep) +} diff --git a/rust/crates/du-db/src/lib.rs b/rust/crates/du-db/src/lib.rs index 315674d2..d1ff89e3 100644 --- a/rust/crates/du-db/src/lib.rs +++ b/rust/crates/du-db/src/lib.rs @@ -19,6 +19,7 @@ pub mod coverage; pub mod dedup; pub mod denovo; pub mod discovery; +pub mod donor; pub mod exchange; pub mod fed; pub mod genome_region; diff --git a/rust/crates/du-db/src/naming.rs b/rust/crates/du-db/src/naming.rs index f0699467..67bf8fad 100644 --- a/rust/crates/du-db/src/naming.rs +++ b/rust/crates/du-db/src/naming.rs @@ -45,8 +45,18 @@ fn mode_predicate(mode: &str) -> &'static str { // The imported backlog: has a name but DU hasn't ratified it. "UNNAMED" => "v.naming_status = 'UNNAMED' AND v.canonical_name IS NOT NULL", "all" => "TRUE", - // needs_name (default) - _ => "(v.canonical_name IS NULL OR v.naming_status = 'PENDING_REVIEW')", + // needs_name (default): the actionable naming backlog. The DU authority names + // Y-DNA variants only — mtDNA variants are identified by their [anc]pos[der] + // coordinate string and never get a DU name, so they're excluded. A canonical + // name is (name + the branch it defines), so only a variant that DEFINES a + // (Y) branch is nameable — branch-less catalog rows are reference data, not + // naming work. "Needs a name" = defines a Y branch AND has no real name yet: + // no name, a synthetic coordinate placeholder (`chrY:pos…`), or flagged. + _ => "v.defining_haplogroup_id IS NOT NULL \ + AND EXISTS (SELECT 1 FROM tree.haplogroup h \ + WHERE h.id = v.defining_haplogroup_id AND h.haplogroup_type = 'Y_DNA'::core.dna_type) \ + AND (v.canonical_name IS NULL OR v.canonical_name LIKE 'chr%:%' \ + OR v.naming_status = 'PENDING_REVIEW')", } } @@ -83,6 +93,16 @@ pub async fn get(pool: &PgPool, id: i64) -> Result, DbError> .await?) } +/// Whether a `canonical_name` is a **synthetic coordinate placeholder** written by +/// the de-novo loader (`chrY:pos anc->der`) rather than a real, ratified name. The +/// loader stamps every branch-defining variant `NAMED` with such a stand-in, so the +/// authority must treat these as *unnamed* — they're eligible to mint/adopt, and +/// must not display as "already named". Mirrors the SQL `canonical_name LIKE 'chr%:%'` +/// the needs_name queue filters on. Real Y-SNP names never start with `chr`. +pub fn is_placeholder_name(name: &str) -> bool { + name.starts_with("chr") && name.contains(':') +} + /// Set a variant's naming status (e.g. flag for review or send back to unnamed). /// Does not touch the name. Returns whether a row changed. pub async fn set_status(pool: &PgPool, id: i64, status: &str) -> Result { @@ -109,13 +129,17 @@ pub async fn assign_du_name(pool: &PgPool, id: i64) -> Result { .fetch_optional(&mut *tx) .await?; let (old_name, status) = row.ok_or_else(|| DbError::Conflict(format!("variant {id} not found")))?; - if status == "NAMED" { + // A synthetic coordinate placeholder (`chrY:…`) counts as unnamed even though the + // loader marked it NAMED — only a *real* ratified name blocks re-minting. + let real_named = old_name.as_deref().is_some_and(|n| !is_placeholder_name(n)); + if status == "NAMED" && real_named { return Err(DbError::Conflict("variant is already NAMED".into())); } let du: String = sqlx::query_scalar("SELECT core.next_du_name()").fetch_one(&mut *tx).await?; - // Preserve any prior working name as a common-name alias (union, deduped). - if let Some(prev) = old_name.filter(|n| !n.trim().is_empty() && *n != du) { + // Preserve any prior *real* working name as a common-name alias (union, deduped). + // A placeholder coordinate string is not a name, so it's dropped rather than kept. + if let Some(prev) = old_name.filter(|n| !n.trim().is_empty() && *n != du && !is_placeholder_name(n)) { sqlx::query( "UPDATE core.variant SET aliases = jsonb_set( \ COALESCE(aliases, '{}'::jsonb), '{common_names}', \ @@ -129,8 +153,16 @@ pub async fn assign_du_name(pool: &PgPool, id: i64) -> Result { .execute(&mut *tx) .await?; } + // Set the DU name and, in the same write, purge any synthetic coordinate placeholders + // (`chr…:…`) the loader left in common_names — they were never real alternate names. sqlx::query( - "UPDATE core.variant SET canonical_name = $2, naming_status = 'NAMED', updated_at = now() WHERE id = $1", + "UPDATE core.variant SET canonical_name = $2, naming_status = 'NAMED', \ + aliases = jsonb_set(COALESCE(aliases, '{}'::jsonb), '{common_names}', \ + COALESCE((SELECT jsonb_agg(a) \ + FROM jsonb_array_elements_text(COALESCE(aliases->'common_names', '[]'::jsonb)) a \ + WHERE a NOT LIKE 'chr%:%'), '[]'::jsonb), true), \ + updated_at = now() \ + WHERE id = $1", ) .bind(id) .bind(&du) @@ -140,17 +172,101 @@ pub async fn assign_du_name(pool: &PgPool, id: i64) -> Result { Ok(du) } -/// **Dedup check** before minting: other *named* variants sharing this variant's -/// GRCh38 coordinate (contig + position). A non-empty result means an -/// established name likely already exists — reuse it instead of minting. -pub async fn dedup_by_coordinates(pool: &PgPool, id: i64) -> Result, DbError> { +/// The first established (non-DU) working name among a variant's `common_names` +/// aliases — an ISOGG/YBrowse-derived name the authority can **reuse** rather than +/// re-mint. Our own `DU…` mints are skipped (they aren't external definitions). +pub fn established_name(aliases: &Value) -> Option { + aliases + .get("common_names")? + .as_array()? + .iter() + .filter_map(|x| x.as_str()) + // Skip our own DU mints and synthetic coordinate placeholders (`chrY:…`) — neither + // is an external "named by definition" reference the authority can adopt. + .find(|s| !s.trim().is_empty() && !s.starts_with("DU") && !is_placeholder_name(s)) + .map(str::to_string) +} + +/// **Adopt an established name**: ratify a variant's existing ISOGG/YBrowse name +/// (a non-DU `common_names` alias) as its canonical name instead of minting a new +/// `DU` identifier. This is the "named by definition" path — when a matching locus +/// + mutation state already has a name in the source set, the authority reuses it. +/// Sets `canonical_name` to that name and marks `NAMED`. Errors if the variant is +/// already named or carries no established name. +pub async fn adopt_established_name(pool: &PgPool, id: i64) -> Result { + let mut tx = pool.begin().await?; + let row: Option<(Option, Value)> = sqlx::query_as( + "SELECT canonical_name, aliases FROM core.variant WHERE id = $1 FOR UPDATE", + ) + .bind(id) + .fetch_optional(&mut *tx) + .await?; + let (canon, aliases) = + row.ok_or_else(|| DbError::Conflict(format!("variant {id} not found")))?; + // A synthetic coordinate placeholder (`chrY:…`) doesn't count as a real canonical + // name — the loader stamps it NAMED, but the variant is still adoptable. + let real_named = canon.as_deref().is_some_and(|n| !is_placeholder_name(n)); + if real_named { + return Err(DbError::Conflict("variant already has a canonical name".into())); + } + let name = established_name(&aliases) + .ok_or_else(|| DbError::Conflict("variant has no established name to adopt".into()))?; + // Canonical identity is (name + defining branch) — the recurrence model that + // replaces ISOGG's L270.1/L270.2 suffixing: the same SNP name is canonical on + // each branch it defines, scoped by `defining_haplogroup_id`. A clash only + // exists when another row already holds this name for the *same* branch (which + // `variant_canonical_name_key` enforces, treating NULL as one bucket). Guard it + // so the write returns a clear notice instead of a 500. + let taken: Option = sqlx::query_scalar( + "SELECT o.id FROM core.variant o, core.variant me \ + WHERE me.id = $2 AND o.id <> me.id AND o.canonical_name = $1 \ + AND COALESCE(o.defining_haplogroup_id, -1) = COALESCE(me.defining_haplogroup_id, -1) LIMIT 1", + ) + .bind(&name) + .bind(id) + .fetch_optional(&mut *tx) + .await?; + if let Some(other) = taken { + return Err(DbError::Conflict(format!( + "name {name} is already canonical for this branch on variant #{other} — resolve the duplicate / recurrence in merge review before reusing it" + ))); + } + sqlx::query( + "UPDATE core.variant SET canonical_name = $2, naming_status = 'NAMED', updated_at = now() WHERE id = $1", + ) + .bind(id) + .bind(&name) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(name) +} + +/// **Dedup check** before naming: other *named* variants at the same locus **and +/// mutation state** — contig + position + ancestral + derived — on this variant's +/// preferred build (`hs1` first, else `GRCh38`). Matching by locus alone is wrong: +/// two distinct SNPs can share a position with different alleles (e.g. Z12236 A>C +/// vs Y17125 A>G), so a same-position-different-allele variant is NOT a duplicate. +pub async fn dedup_by_site(pool: &PgPool, id: i64) -> Result, DbError> { Ok(sqlx::query_as( - "WITH me AS (SELECT coordinates->'GRCh38'->>'contig' AS c, coordinates->'GRCh38'->>'position' AS p \ - FROM core.variant WHERE id = $1) \ + "WITH b AS ( \ + SELECT CASE WHEN coordinates ? 'hs1' THEN 'hs1' \ + WHEN coordinates ? 'GRCh38' THEN 'GRCh38' END AS build \ + FROM core.variant WHERE id = $1), \ + me AS ( \ + SELECT b.build, \ + v.coordinates->b.build->>'contig' AS c, \ + v.coordinates->b.build->>'position' AS p, \ + v.coordinates->b.build->>'ancestral' AS a, \ + v.coordinates->b.build->>'derived' AS d \ + FROM core.variant v, b WHERE v.id = $1) \ SELECT v.id, v.canonical_name FROM core.variant v, me \ - WHERE v.id <> $1 AND v.canonical_name IS NOT NULL AND me.c IS NOT NULL AND me.p IS NOT NULL \ - AND v.coordinates->'GRCh38'->>'contig' = me.c \ - AND v.coordinates->'GRCh38'->>'position' = me.p \ + WHERE v.id <> $1 AND v.canonical_name IS NOT NULL AND me.build IS NOT NULL \ + AND me.c IS NOT NULL AND me.p IS NOT NULL \ + AND v.coordinates->me.build->>'contig' = me.c \ + AND v.coordinates->me.build->>'position' = me.p \ + AND v.coordinates->me.build->>'ancestral' IS NOT DISTINCT FROM me.a \ + AND v.coordinates->me.build->>'derived' IS NOT DISTINCT FROM me.d \ ORDER BY v.canonical_name LIMIT 10", ) .bind(id) diff --git a/rust/crates/du-db/src/sequencer.rs b/rust/crates/du-db/src/sequencer.rs index 4cc43f8b..31439447 100644 --- a/rust/crates/du-db/src/sequencer.rs +++ b/rust/crates/du-db/src/sequencer.rs @@ -613,3 +613,161 @@ pub async fn reject_proposal( tx.commit().await?; Ok(row) } + +// ── established associations (maintenance) ──────────────────────────────────── +// +// The consensus queue above is for *new* proposals mined from the federation. +// These functions expose the already-established instrument→lab associations +// (preseeded via migration, or ratified via accept) so a curator can maintain +// them directly — reassign the lab, fix the model/manufacturer — without going +// through a proposal. + +/// An established instrument→lab association row for the maintenance view. +#[derive(Debug, Clone, sqlx::FromRow)] +pub struct EstablishedRow { + pub id: i64, + pub instrument_id: String, + pub model_name: Option, + pub manufacturer: Option, + pub lab_id: Option, + pub lab_name: Option, + pub is_d2c: Option, +} + +const ESTABLISHED_COLS: &str = "si.id, si.instrument_id, si.model_name, si.manufacturer, \ + si.lab_id, sl.name AS lab_name, sl.is_d2c"; + +/// Paginated list of every sequencer instrument and its current lab, for the +/// curator maintenance view. Optional case-insensitive substring `query` matches +/// instrument id, model, manufacturer, or lab name. Unassigned instruments (no +/// lab) sort first — they're the ones most in need of attention. +pub async fn list_established( + pool: &PgPool, + query: Option<&str>, + page: i64, + page_size: i64, +) -> Result, DbError> { + let offset = Page::<()>::offset(page, page_size); + let limit = page_size.clamp(1, 200); + let like = query + .map(str::trim) + .filter(|q| !q.is_empty()) + .map(|q| format!("%{}%", q.to_lowercase())); + const WHERE: &str = "($1::text IS NULL \ + OR lower(si.instrument_id) LIKE $1 \ + OR lower(coalesce(si.model_name, '')) LIKE $1 \ + OR lower(coalesce(si.manufacturer, '')) LIKE $1 \ + OR lower(coalesce(sl.name, '')) LIKE $1)"; + let items: Vec = sqlx::query_as(&format!( + "SELECT {ESTABLISHED_COLS} \ + FROM genomics.sequencer_instrument si \ + LEFT JOIN genomics.sequencing_lab sl ON sl.id = si.lab_id \ + WHERE {WHERE} \ + ORDER BY sl.name NULLS FIRST, si.instrument_id \ + LIMIT $2 OFFSET $3" + )) + .bind(like.as_deref()) + .bind(limit) + .bind(offset) + .fetch_all(pool) + .await?; + let total: i64 = sqlx::query_scalar(&format!( + "SELECT count(*) FROM genomics.sequencer_instrument si \ + LEFT JOIN genomics.sequencing_lab sl ON sl.id = si.lab_id WHERE {WHERE}" + )) + .bind(like.as_deref()) + .fetch_one(pool) + .await?; + Ok(Page { items, total, page: page.max(1), page_size: limit }) +} + +/// One established association, by `sequencer_instrument.id` (for the edit form). +pub async fn established_detail(pool: &PgPool, id: i64) -> Result, DbError> { + Ok(sqlx::query_as(&format!( + "SELECT {ESTABLISHED_COLS} \ + FROM genomics.sequencer_instrument si \ + LEFT JOIN genomics.sequencing_lab sl ON sl.id = si.lab_id \ + WHERE si.id = $1" + )) + .bind(id) + .fetch_optional(pool) + .await?) +} + +/// A lab the curator can reassign an instrument to (dropdown / datalist source). +#[derive(Debug, Clone, sqlx::FromRow)] +pub struct LabOption { + pub id: i64, + pub name: String, + pub is_d2c: bool, +} + +/// Every known lab, alphabetical — for the reassignment picker. +pub async fn list_labs(pool: &PgPool) -> Result, DbError> { + Ok(sqlx::query_as("SELECT id, name, is_d2c FROM genomics.sequencing_lab ORDER BY name") + .fetch_all(pool) + .await?) +} + +/// **Maintain** an established association directly (no proposal): get-or-create +/// the named lab, set the instrument's `lab_id` (what `lookup_lab` resolves) plus +/// its model/manufacturer, audited. Mirrors [`accept_proposal`] minus the proposal +/// bookkeeping. Keyed by `sequencer_instrument.id`. Returns the resolved lookup. +pub async fn update_instrument_lab( + pool: &PgPool, + user_id: Uuid, + id: i64, + lab_name: &str, + manufacturer: Option<&str>, + model: Option<&str>, + is_d2c: Option, +) -> Result { + let mut tx = pool.begin().await?; + let before: LabLookup = sqlx::query_as( + "SELECT si.instrument_id, coalesce(sl.name, '') AS lab_name, coalesce(sl.is_d2c, false) AS is_d2c, \ + sl.website_url, si.manufacturer, si.model_name \ + FROM genomics.sequencer_instrument si \ + LEFT JOIN genomics.sequencing_lab sl ON sl.id = si.lab_id \ + WHERE si.id = $1 FOR UPDATE OF si", + ) + .bind(id) + .fetch_optional(&mut *tx) + .await? + .ok_or_else(|| DbError::Conflict(format!("instrument {id} not found")))?; + + let lab_id = get_or_create_lab(&mut tx, lab_name, is_d2c.unwrap_or(false)).await?; + // Only touch an existing lab's d2c flag when the curator explicitly set it — an + // omitted flag must not silently clear a preseeded lab's is_d2c (which is shared + // by every other instrument tied to that lab). Mirrors accept_proposal. + if let Some(d2c) = is_d2c { + sqlx::query("UPDATE genomics.sequencing_lab SET is_d2c = $2 WHERE id = $1") + .bind(lab_id) + .bind(d2c) + .execute(&mut *tx) + .await?; + } + sqlx::query( + "UPDATE genomics.sequencer_instrument \ + SET lab_id = $2, manufacturer = COALESCE($3, manufacturer), model_name = COALESCE($4, model_name) \ + WHERE id = $1", + ) + .bind(id) + .bind(lab_id) + .bind(manufacturer) + .bind(model) + .execute(&mut *tx) + .await?; + let hit: LabLookup = sqlx::query_as( + "SELECT si.instrument_id, sl.name AS lab_name, sl.is_d2c, sl.website_url, si.manufacturer, si.model_name \ + FROM genomics.sequencer_instrument si JOIN genomics.sequencing_lab sl ON sl.id = si.lab_id \ + WHERE si.id = $1", + ) + .bind(id) + .fetch_one(&mut *tx) + .await?; + let old = json!({ "lab_name": before.lab_name, "manufacturer": before.manufacturer, "model_name": before.model_name }); + let new = json!({ "instrument_id": hit.instrument_id, "lab_name": hit.lab_name, "is_d2c": hit.is_d2c, "manufacturer": hit.manufacturer, "model_name": hit.model_name }); + crate::audit::log(&mut *tx, user_id, "sequencer_instrument", id, "REASSIGN_LAB", Some(&old), Some(&new), None).await?; + tx.commit().await?; + Ok(hit) +} diff --git a/rust/crates/du-db/src/variant.rs b/rust/crates/du-db/src/variant.rs index 0efc4a3e..2090164d 100644 --- a/rust/crates/du-db/src/variant.rs +++ b/rust/crates/du-db/src/variant.rs @@ -369,31 +369,73 @@ pub async fn for_haplogroup_name(pool: &PgPool, name: &str) -> Result Result, DbError> { +) -> Result, Option)>, DbError> { + use std::collections::HashMap; + + // (1) Narrow link rows: haplogroup ↔ variant with this branch's ASR alleles. No JSONB, + // no heap fetch of core.variant — just the current Y links. #[derive(sqlx::FromRow)] - struct GroupedRow { + struct LinkRow { haplogroup_id: i64, - #[sqlx(flatten)] - variant: VariantRow, + variant_id: i64, + link_ancestral: Option, + link_derived: Option, } - let rows: Vec = sqlx::query_as( - "SELECT hv.haplogroup_id, v.id, v.canonical_name, v.mutation_type::text AS mutation_type, \ - v.naming_status::text AS naming_status, v.aliases, v.coordinates, v.annotations \ - FROM core.variant v \ - JOIN tree.haplogroup_variant hv ON hv.variant_id = v.id AND hv.valid_until IS NULL \ + let links: Vec = sqlx::query_as( + "SELECT hv.haplogroup_id, hv.variant_id, \ + hv.ancestral_allele AS link_ancestral, hv.derived_allele AS link_derived \ + FROM tree.haplogroup_variant hv \ JOIN tree.haplogroup h ON h.id = hv.haplogroup_id \ - WHERE h.haplogroup_type::text = $1 AND h.valid_until IS NULL AND v.canonical_name IS NOT NULL \ - ORDER BY hv.haplogroup_id, v.canonical_name", + WHERE h.haplogroup_type::text = $1 AND h.valid_until IS NULL AND hv.valid_until IS NULL", ) .bind(pg_enum_label(&dna_type)?) .fetch_all(pool) .await?; - rows.into_iter() - .map(|r| Ok((r.haplogroup_id, r.variant.into_domain()?))) - .collect() + + // (2) Each distinct referenced variant, once. Named-only (matches the prior join's + // `v.canonical_name IS NOT NULL`); unnamed ids simply won't resolve below. + let mut ids: Vec = links.iter().map(|l| l.variant_id).collect(); + ids.sort_unstable(); + ids.dedup(); + let variant_rows: Vec = sqlx::query_as(&format!( + "{SELECT} WHERE id = ANY($1::bigint[]) AND canonical_name IS NOT NULL" + )) + .bind(&ids) + .fetch_all(pool) + .await?; + let mut by_id: HashMap = HashMap::with_capacity(variant_rows.len()); + for r in variant_rows { + by_id.insert(r.id, r.into_domain()?); + } + + // (3) Stitch links to variants (clone — a recurrent variant fans out to many branches), + // dropping links whose variant was unnamed/absent. Sort to the prior contract + // (haplogroup_id, canonical_name) in Rust — cheap vs Postgres's external-merge sort. + let mut out: Vec<(i64, Variant, Option, Option)> = links + .into_iter() + .filter_map(|l| { + by_id + .get(&l.variant_id) + .map(|v| (l.haplogroup_id, v.clone(), l.link_ancestral, l.link_derived)) + }) + .collect(); + out.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| a.1.canonical_name.cmp(&b.1.canonical_name))); + Ok(out) } /// Total NAMED variant count (for the export metadata endpoint). diff --git a/rust/crates/du-db/tests/donor_consolidation.rs b/rust/crates/du-db/tests/donor_consolidation.rs new file mode 100644 index 00000000..d8f8b357 --- /dev/null +++ b/rust/crates/du-db/tests/donor_consolidation.rs @@ -0,0 +1,127 @@ +//! Live-DB test for `du_db::donor::consolidate_denovo_donors` + the donor-level +//! tree-placement surfacing in the sample report. Models one individual split +//! across two publication accessions (different donors) plus a de-novo tree tip +//! (donor-less, holding the placement), then consolidates and checks the report +//! resolves the haplogroup on every accession. Skips when DATABASE_URL is unset. + +use sqlx::PgPool; +use uuid::Uuid; + +fn database_url() -> Option { + std::env::var("DATABASE_URL").ok().filter(|s| !s.is_empty()) +} + +async fn donor(pool: &PgPool, identifier: &str, biobank: Option<&str>) -> i64 { + sqlx::query_scalar( + "INSERT INTO core.specimen_donor (donor_identifier, origin_biobank, donor_type) \ + VALUES ($1, $2, 'STANDARD') RETURNING id", + ) + .bind(identifier) + .bind(biobank) + .fetch_one(pool) + .await + .expect("insert donor") +} + +async fn biosample(pool: &PgPool, accession: &str, alias: Option<&str>, denovo: bool, donor_id: Option) -> Uuid { + let attrs = if denovo { serde_json::json!({ "denovo": true }) } else { serde_json::json!({}) }; + sqlx::query_scalar( + "INSERT INTO core.biosample (source, accession, alias, source_attrs, donor_id, is_public) \ + VALUES ('EXTERNAL', $1, $2, $3, $4, true) RETURNING sample_guid", + ) + .bind(accession) + .bind(alias) + .bind(attrs) + .bind(donor_id) + .fetch_one(pool) + .await + .expect("insert biosample") +} + +async fn hg(pool: &PgPool, name: &str) -> i64 { + sqlx::query_scalar( + "INSERT INTO tree.haplogroup (name, haplogroup_type) VALUES ($1, 'Y_DNA'::core.dna_type) RETURNING id", + ) + .bind(name) + .fetch_one(pool) + .await + .expect("insert hg") +} + +async fn place(pool: &PgPool, guid: Uuid, hg_id: i64) { + sqlx::query( + "INSERT INTO tree.haplogroup_sample (sample_guid, dna_type, haplogroup_id, call_text, status, refreshed_at) \ + VALUES ($1, 'Y_DNA'::core.dna_type, $2, 'placed', 'PLACED', now())", + ) + .bind(guid) + .bind(hg_id) + .execute(pool) + .await + .expect("place"); +} + +#[tokio::test] +async fn consolidates_and_surfaces_placement() { + let Some(url) = database_url() else { + eprintln!("DATABASE_URL unset — skipping donor consolidation test"); + return; + }; + let db = du_db::testing::ephemeral_db(&url).await.expect("ephemeral db"); + let pool = db.pool().clone(); + + // One individual (panel id CONS_TEST_1) as: two publication accessions with + // *different* donors (one rich w/ biobank, one poor), plus a de-novo tip that is + // donor-less but carries the Y tree placement. + let rich = donor(&pool, "DONOR_CONS_TEST_1", Some("Coriell")).await; + let poor = donor(&pool, "CONS_TEST_1_ALT", None).await; + let r1 = biosample(&pool, "SAMN_CONS_1", Some("CONS_TEST_1"), false, Some(rich)).await; + let r2 = biosample(&pool, "SAMEA_CONS_1", Some("CONS_TEST_1"), false, Some(poor)).await; + let tip = biosample(&pool, "CONS_TEST_1", None, true, None).await; + let node = hg(&pool, "CONS-TEST-NODE").await; + place(&pool, tip, node).await; + + // Before: the reference shows no haplogroup call (placement is on the tip's row). + let g_r1 = du_db::biosample::resolve_guid(&pool, "SAMN_CONS_1").await.unwrap().unwrap(); + let before = du_db::biosample::report_by_guid(&pool, g_r1).await.unwrap().unwrap(); + assert!(before.y.is_none(), "reference has no call before consolidation"); + + // Consolidate. + let rep = du_db::donor::consolidate_denovo_donors(&pool, true).await.expect("consolidate"); + assert!(rep.groups >= 1, "found the group"); + assert!(rep.biosamples_repointed >= 2, "repointed the tip + the poor-donor ref"); + assert!(rep.donors_pruned >= 1, "pruned the emptied poor donor"); + + // All three biosamples now share ONE donor, and it's the rich one. + let donors: Vec> = sqlx::query_scalar( + "SELECT donor_id FROM core.biosample WHERE sample_guid = ANY($1)", + ) + .bind([r1, r2, tip]) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!(donors.iter().flatten().collect::>().len(), 1, "one shared donor"); + assert!(donors.iter().all(|d| *d == Some(rich)), "survivor is the rich donor"); + // The poor donor was pruned. + let poor_alive: Option = sqlx::query_scalar("SELECT id FROM core.specimen_donor WHERE id = $1").bind(poor).fetch_optional(&pool).await.unwrap(); + assert!(poor_alive.is_none(), "poor donor deleted"); + // Survivor's identifier normalized to the clean panel id. + let ident: Option = sqlx::query_scalar("SELECT donor_identifier FROM core.specimen_donor WHERE id = $1").bind(rich).fetch_one(&pool).await.unwrap(); + assert_eq!(ident.as_deref(), Some("CONS_TEST_1")); + + // After: the report surfaces the tree placement on BOTH accessions (via the donor). + for (label, acc) in [("SAMN ref", "SAMN_CONS_1"), ("SAMEA ref", "SAMEA_CONS_1")] { + let g = du_db::biosample::resolve_guid(&pool, acc).await.unwrap().unwrap(); + let rep = du_db::biosample::report_by_guid(&pool, g).await.unwrap().unwrap(); + let call = rep.y.unwrap_or_else(|| panic!("{label} should now have a Y call")); + assert_eq!(call.name, "CONS-TEST-NODE"); + assert_eq!(call.origin, du_db::biosample::HaplogroupCallOrigin::TreePlacement); + } + + // cleanup + for g in [r1, r2, tip] { + let _ = sqlx::query("DELETE FROM tree.haplogroup_sample WHERE sample_guid = $1").bind(g).execute(&pool).await; + let _ = sqlx::query("DELETE FROM core.biosample WHERE sample_guid = $1").bind(g).execute(&pool).await; + } + let _ = sqlx::query("DELETE FROM tree.haplogroup WHERE id = $1").bind(node).execute(&pool).await; + let _ = sqlx::query("DELETE FROM core.specimen_donor WHERE id = $1").bind(rich).execute(&pool).await; +} diff --git a/rust/crates/du-db/tests/sequencer.rs b/rust/crates/du-db/tests/sequencer.rs index 0b1ff2c6..921b2e8d 100644 --- a/rust/crates/du-db/tests/sequencer.rs +++ b/rust/crates/du-db/tests/sequencer.rs @@ -20,6 +20,17 @@ async fn lab(pool: &PgPool, name: &str, d2c: bool, web: Option<&str>) -> i64 { .expect("insert lab") } +/// A curator user — reassignments write an audit row (FK to ident.users). +async fn test_user(pool: &PgPool) -> uuid::Uuid { + sqlx::query_scalar( + "INSERT INTO ident.users (handle, display_name) VALUES ('testmaint-curator', 'Test Curator') \ + ON CONFLICT (handle) DO UPDATE SET display_name = EXCLUDED.display_name RETURNING id", + ) + .fetch_one(pool) + .await + .expect("test user") +} + async fn instrument(pool: &PgPool, iid: &str, model: Option<&str>, manuf: Option<&str>, lab_id: Option) { sqlx::query( "INSERT INTO genomics.sequencer_instrument (instrument_id, model_name, manufacturer, lab_id) \ @@ -69,6 +80,65 @@ async fn lookup_resolves_preseeded_association() { assert!(!all.iter().any(|i| i.instrument_id == "M01234")); } +/// The "Established" maintenance surface: list all instrument→lab associations +/// (incl. unassigned), open one, and reassign its lab directly (no proposal). +#[tokio::test] +async fn established_maintenance_flow() { + let Some(url) = database_url() else { + eprintln!("DATABASE_URL unset — skipping established test"); + return; + }; + let db = du_db::testing::ephemeral_db(&url).await.expect("ephemeral db"); + let pool = db.pool().clone(); + let user = test_user(&pool).await; + + let acme = lab(&pool, "Acme Test Sequencing", false, None).await; + instrument(&pool, "ACME-T1", Some("NovaSeq 6000"), Some("Illumina"), Some(acme)).await; + // An unassigned instrument — must surface in the maintenance list (unlike lookup). + instrument(&pool, "MAINT-UN1", Some("MiSeq"), Some("Illumina"), None).await; + + // Search narrows to our test instruments; the unassigned one is included. + let page = du_db::sequencer::list_established(&pool, Some("maint-un1"), 1, 50).await.expect("list"); + let un = page.items.iter().find(|r| r.instrument_id == "MAINT-UN1").expect("unassigned listed"); + assert!(un.lab_name.is_none(), "unassigned instrument has no lab"); + + // The lab picker offers our seeded/test labs. + let labs = du_db::sequencer::list_labs(&pool).await.expect("labs"); + assert!(labs.iter().any(|l| l.name == "Acme Test Sequencing")); + + // Reassign the unassigned instrument to an existing lab — sets what the public + // lookup resolves, and is auditable. + let hit = du_db::sequencer::update_instrument_lab( + &pool, user, un.id, "Acme Test Sequencing", None, Some("MiSeq v3"), None, + ) + .await + .expect("reassign"); + assert_eq!(hit.lab_name, "Acme Test Sequencing"); + // lookup now resolves it (was None before). + let resolved = du_db::sequencer::lookup_lab(&pool, "MAINT-UN1").await.unwrap().expect("now resolves"); + assert_eq!(resolved.lab_name, "Acme Test Sequencing"); + assert_eq!(resolved.model_name.as_deref(), Some("MiSeq v3"), "model updated"); + + // Typing a brand-new lab name creates it and reassigns in one step. + let a1 = du_db::sequencer::established_detail(&pool, un.id).await.unwrap().unwrap(); + let hit2 = du_db::sequencer::update_instrument_lab( + &pool, user, a1.id, "Brand New Lab Co", Some("BGI"), None, Some(true), + ) + .await + .expect("reassign to new lab"); + assert_eq!(hit2.lab_name, "Brand New Lab Co"); + assert!(hit2.is_d2c, "explicit d2c flag applied to the new lab"); + // The audit trail recorded the reassignment. + let audited: i64 = sqlx::query_scalar( + "SELECT count(*) FROM ident.audit_log WHERE entity_type = 'sequencer_instrument' AND action = 'REASSIGN_LAB' AND entity_id = $1", + ) + .bind(a1.id) + .fetch_one(&pool) + .await + .unwrap(); + assert!(audited >= 2, "two reassignments audited, got {audited}"); +} + /// The 0038 seed preloads the YDNA-Warehouse d2c instrument→lab map (n_crams>2, max-lab). #[tokio::test] async fn seed_preloads_ydna_warehouse_labs() { diff --git a/rust/crates/du-db/tests/variant_naming.rs b/rust/crates/du-db/tests/variant_naming.rs index d19776d4..28393342 100644 --- a/rust/crates/du-db/tests/variant_naming.rs +++ b/rust/crates/du-db/tests/variant_naming.rs @@ -1,7 +1,8 @@ //! Live-DB test for the Variant Naming Authority (`du_db::naming` + migration //! 0016: nullable canonical_name, du_variant_name_seq, next_du_name). Exercises -//! the queue, dedup-by-coordinate, DU minting (with old-name→alias), and the -//! NAMED guard. Re-runnable; skips when DATABASE_URL is unset. +//! the branch-scoped queue, dedup-by-site (locus + mutation state), DU minting +//! (with old-name→alias), reuse of an established name, and the NAMED guard. +//! Re-runnable; skips when DATABASE_URL is unset. use sqlx::PgPool; @@ -9,18 +10,42 @@ fn database_url() -> Option { std::env::var("DATABASE_URL").ok().filter(|s| !s.is_empty()) } -async fn mk_variant(pool: &PgPool, name: Option<&str>, status: &str, pos: Option<&str>) -> i64 { - let coords = match pos { - Some(p) => serde_json::json!({ "GRCh38": { "contig": "chrY", "position": p } }), - None => serde_json::json!({}), +/// A branch to scope canonical names to (canonical identity = name + branch). +async fn mk_hg(pool: &PgPool, name: &str) -> i64 { + sqlx::query_scalar( + "INSERT INTO tree.haplogroup (name, haplogroup_type) \ + VALUES ($1, 'Y_DNA'::core.dna_type) RETURNING id", + ) + .bind(name) + .fetch_one(pool) + .await + .expect("insert haplogroup") +} + +/// A variant at a GRCh38 site with explicit alleles, optionally scoped to a branch. +async fn mk_variant( + pool: &PgPool, + name: Option<&str>, + status: &str, + pos: Option<&str>, + alleles: Option<(&str, &str)>, + defining: Option, +) -> i64 { + let coords = match (pos, alleles) { + (Some(p), Some((a, d))) => serde_json::json!({ + "GRCh38": { "contig": "chrY", "position": p, "ancestral": a, "derived": d } + }), + (Some(p), None) => serde_json::json!({ "GRCh38": { "contig": "chrY", "position": p } }), + _ => serde_json::json!({}), }; sqlx::query_scalar( - "INSERT INTO core.variant (canonical_name, mutation_type, naming_status, coordinates) \ - VALUES ($1, 'SNP'::core.mutation_type, $2::core.naming_status, $3) RETURNING id", + "INSERT INTO core.variant (canonical_name, mutation_type, naming_status, coordinates, defining_haplogroup_id) \ + VALUES ($1, 'SNP'::core.mutation_type, $2::core.naming_status, $3, $4) RETURNING id", ) .bind(name) .bind(status) .bind(coords) + .bind(defining) .fetch_one(pool) .await .expect("insert variant") @@ -35,21 +60,30 @@ async fn variant_naming_authority_flow() { let db = du_db::testing::ephemeral_db(&url).await.expect("ephemeral db"); let pool = db.pool().clone(); - // Three test variants: an unnamed one + a named one at the SAME coord (dedup), - // and one with a working name awaiting an official DU name. - let unnamed = mk_variant(&pool, None, "UNNAMED", Some("2781234")).await; - let named_same = mk_variant(&pool, Some("TESTNAME-AT2781234"), "NAMED", Some("2781234")).await; - let working = mk_variant(&pool, Some("TESTNAME-WORK"), "PENDING_REVIEW", None).await; - let ids = [unnamed, named_same, working]; + let branch = mk_hg(&pool, "TEST-BRANCH-A").await; + let branch2 = mk_hg(&pool, "TEST-BRANCH-B").await; - // The default "needs a name" queue = no-name-yet OR flagged-for-review. + // A branch-defining unnamed variant (A>G) + a named one at the SAME site & state + // (dedup hit), a named one at the same POSITION but a DIFFERENT state (A>C — must + // NOT be a dedup hit), and a branch-defining working-named variant awaiting a DU. + let unnamed = mk_variant(&pool, None, "UNNAMED", Some("2781234"), Some(("A", "G")), Some(branch)).await; + let named_same = mk_variant(&pool, Some("TESTNAME-AG"), "NAMED", Some("2781234"), Some(("A", "G")), None).await; + let named_diff = mk_variant(&pool, Some("TESTNAME-AC"), "NAMED", Some("2781234"), Some(("A", "C")), None).await; + let working = mk_variant(&pool, Some("TESTNAME-WORK"), "PENDING_REVIEW", None, None, Some(branch)).await; + let ids = [unnamed, named_same, named_diff, working]; + + // The default "needs a name" queue = defines a branch AND has no real name yet. let q = du_db::naming::queue(&pool, "needs_name", 1, 200).await.expect("queue"); assert!(q.items.iter().any(|i| i.id == unnamed && i.canonical_name.is_none()), "unnamed in queue"); assert!(q.items.iter().any(|i| i.id == working), "pending-review in queue"); + // A branch-less named catalog row is NOT naming work. + assert!(!q.items.iter().any(|i| i.id == named_same), "branch-less named row excluded"); - // Dedup: the unnamed variant shares a coord with the named one → suggest reuse. - let dups = du_db::naming::dedup_by_coordinates(&pool, unnamed).await.expect("dedup"); - assert!(dups.iter().any(|(id, n)| *id == named_same && n == "TESTNAME-AT2781234"), "dedup finds same-coord named"); + // Dedup-by-site: matches the same locus + mutation state (A>G), NOT a same-position + // different-state variant (A>C) — the L270-vs-recurrence distinction. + let dups = du_db::naming::dedup_by_site(&pool, unnamed).await.expect("dedup"); + assert!(dups.iter().any(|(id, n)| *id == named_same && n == "TESTNAME-AG"), "dedup finds same-site named"); + assert!(!dups.iter().any(|(id, _)| *id == named_diff), "dedup ignores same-position different-state"); // Mint a DU name for the unnamed variant. let du1 = du_db::naming::assign_du_name(&pool, unnamed).await.expect("mint"); @@ -73,12 +107,52 @@ async fn variant_naming_authority_flow() { // Re-minting a NAMED variant is refused. assert!(du_db::naming::assign_du_name(&pool, unnamed).await.is_err(), "no re-mint of NAMED"); + // Adopt: a branch-defining variant that already carries an established (non-DU) + // name reuses it as canonical instead of minting — "named by definition". + let adoptable = mk_variant(&pool, None, "UNNAMED", Some("2999999"), Some(("C", "T")), Some(branch2)).await; + sqlx::query("UPDATE core.variant SET aliases = jsonb_build_object('common_names', jsonb_build_array('FGC00001')) WHERE id = $1") + .bind(adoptable).execute(&pool).await.unwrap(); + let adopted = du_db::naming::adopt_established_name(&pool, adoptable).await.expect("adopt"); + assert_eq!(adopted, "FGC00001", "reuses the established name"); + let got = du_db::naming::get(&pool, adoptable).await.unwrap().unwrap(); + assert_eq!(got.canonical_name.as_deref(), Some("FGC00001")); + assert_eq!(got.naming_status, "NAMED"); + // Adopting again (already named) is refused. + assert!(du_db::naming::adopt_established_name(&pool, adoptable).await.is_err(), "no re-adopt of NAMED"); + + // Placeholder names: the de-novo loader stamps branch-defining variants NAMED with a + // synthetic coordinate string (`chrY:pos anc->der`) — which it ALSO copies into the + // common_names aliases. The authority must treat these as *unnamed*: they belong in + // the needs_name queue, are still mintable despite the NAMED status, and the + // placeholder must not survive as an alias nor be offered as an established name. + let ph = mk_variant(&pool, Some("chrY:9999999 A->G"), "NAMED", Some("9999999"), Some(("A", "G")), Some(branch)).await; + sqlx::query("UPDATE core.variant SET aliases = jsonb_build_object('common_names', jsonb_build_array('chrY:9999999 A->G')) WHERE id = $1") + .bind(ph).execute(&pool).await.unwrap(); + assert!(du_db::naming::is_placeholder_name("chrY:9999999 A->G"), "coordinate string is a placeholder"); + assert!(!du_db::naming::is_placeholder_name("Z12335"), "a real SNP name is not a placeholder"); + let qph = du_db::naming::queue(&pool, "needs_name", 1, 500).await.expect("queue"); + assert!(qph.items.iter().any(|i| i.id == ph), "placeholder-named variant is naming work"); + // No established name to adopt — the only alias is the placeholder itself. + let phi = du_db::naming::get(&pool, ph).await.unwrap().unwrap(); + assert!(du_db::naming::established_name(&phi.aliases).is_none(), "placeholder alias is not an established name"); + // Minting succeeds despite the NAMED status, and drops the placeholder alias. + let du_ph = du_db::naming::assign_du_name(&pool, ph).await.expect("mint over placeholder"); + assert!(du_ph.starts_with("DU")); + let kept_placeholder: bool = sqlx::query_scalar( + "SELECT COALESCE(aliases->'common_names', '[]'::jsonb) ? 'chrY:9999999 A->G' FROM core.variant WHERE id = $1", + ) + .bind(ph).fetch_one(&pool).await.unwrap(); + assert!(!kept_placeholder, "placeholder is not preserved as an alias"); + // set_status drives the lifecycle. assert!(du_db::naming::set_status(&pool, named_same, "PENDING_REVIEW").await.expect("set")); assert_eq!(du_db::naming::get(&pool, named_same).await.unwrap().unwrap().naming_status, "PENDING_REVIEW"); - // cleanup (by id — unnamed→DU rows can't be found by a test-prefix name). - for id in ids { + // cleanup (variants first — they reference the branches via defining_haplogroup_id). + for id in ids.iter().chain([&adoptable, &ph]) { let _ = sqlx::query("DELETE FROM core.variant WHERE id = $1").bind(id).execute(&pool).await; } + for hg in [branch, branch2] { + let _ = sqlx::query("DELETE FROM tree.haplogroup WHERE id = $1").bind(hg).execute(&pool).await; + } } diff --git a/rust/crates/du-jobs/src/main.rs b/rust/crates/du-jobs/src/main.rs index 78be72a0..39e50538 100644 --- a/rust/crates/du-jobs/src/main.rs +++ b/rust/crates/du-jobs/src/main.rs @@ -123,6 +123,21 @@ async fn main() -> anyhow::Result<()> { let rep = du_db::tree_sample::recompute_placements(&pool, du_domain::enums::DnaType::YDna).await?; tracing::info!(placed = rep.placed, unplaced = rep.unplaced, "tree-samples-recompute complete (Y)"); } + // One-shot backfill: link every biosample of one individual — the multiple + // publication accessions AND the de-novo tree tip — under a single donor, + // pruning the redundant donor rows. The sample report then resolves the + // haplogroup at the donor level (surfacing the tip's tree placement on every + // accession). Grouped by shared panel id (tip.accession == ref.alias). + // Previews unless `--apply`. + "consolidate-donors" => { + let apply = argv.any(|a| a == "--apply"); + let rep = du_db::donor::consolidate_denovo_donors(&pool, apply).await?; + tracing::info!( + apply, groups = rep.groups, biosamples_repointed = rep.biosamples_repointed, + donors_pruned = rep.donors_pruned, + "consolidate-donors complete{}", if apply { "" } else { " (preview — pass --apply to consolidate)" } + ); + } // Bulk-load FTDNA Y-STR exports into genomics.biosample_str_profile, // matched to the cohort by kit→subject_id. Feeds the STR-variance age // model (run `branch-age` after). Previews unless `--apply`. @@ -134,7 +149,7 @@ async fn main() -> anyhow::Result<()> { ftdna_str::run(&pool, &cfg).await?; } other => anyhow::bail!( - "unknown run-once job '{other}' (known: ybrowse, reconcile, yregions, branch-age, ftdna-str, sequencer-consensus, discovery-consensus, coverage-norms, ibd-discovery-recompute, exchange-expire, tree-samples-recompute, dedup-candidates)" + "unknown run-once job '{other}' (known: ybrowse, reconcile, yregions, branch-age, ftdna-str, sequencer-consensus, discovery-consensus, coverage-norms, ibd-discovery-recompute, exchange-expire, tree-samples-recompute, dedup-candidates, consolidate-donors)" ), } return Ok(()); diff --git a/rust/crates/du-migrate/src/transform.rs b/rust/crates/du-migrate/src/transform.rs index 05fb80d7..bc1c4ccf 100644 --- a/rust/crates/du-migrate/src/transform.rs +++ b/rust/crates/du-migrate/src/transform.rs @@ -1075,19 +1075,26 @@ pub async fn sequencing_lab(legacy: &PgPool, target: &PgPool) -> anyhow::Result< .await?; let n = rows.len(); let mut tx = target.begin().await?; - for (id, name, d2c, web, desc) in rows { + for (_id, name, d2c, web, desc) in rows { + // Labs are keyed by NAME: mig 0038 preseeds the canonical labs (with the D8 + // instrument→lab lookup FK'd to them by name), so the ETL must merge prod + // labs onto that seed by name — enriching metadata and adding prod-only labs + // — rather than forcing legacy ids over the seeded rows (which collide on the + // name unique key). Legacy lab ids carry no downstream FK (sequence_library + // resolves its lab by name string, not id), so id preservation is unneeded. sqlx::query( - "INSERT INTO genomics.sequencing_lab (id, name, is_d2c, website_url, description_markdown) \ - OVERRIDING SYSTEM VALUE VALUES ($1,$2,$3,$4,$5) \ - ON CONFLICT (id) DO UPDATE SET name=EXCLUDED.name, is_d2c=EXCLUDED.is_d2c, \ - website_url=EXCLUDED.website_url, description_markdown=EXCLUDED.description_markdown", + "INSERT INTO genomics.sequencing_lab (name, is_d2c, website_url, description_markdown) \ + VALUES ($1,$2,$3,$4) \ + ON CONFLICT (name) DO UPDATE SET is_d2c=EXCLUDED.is_d2c, \ + website_url=COALESCE(EXCLUDED.website_url, genomics.sequencing_lab.website_url), \ + description_markdown=COALESCE(EXCLUDED.description_markdown, genomics.sequencing_lab.description_markdown)", ) - .bind(id).bind(name).bind(d2c).bind(web).bind(desc) + .bind(name).bind(d2c).bind(web).bind(desc) .execute(&mut *tx) .await?; } tx.commit().await?; - tracing::info!(table = "sequencing_lab", rows = n, "migrated"); + tracing::info!(table = "sequencing_lab", rows = n, "migrated (name-keyed; merged onto mig-0038 seed)"); Ok(()) } diff --git a/rust/crates/du-web/Cargo.toml b/rust/crates/du-web/Cargo.toml index eeff9ca2..984d1870 100644 --- a/rust/crates/du-web/Cargo.toml +++ b/rust/crates/du-web/Cargo.toml @@ -32,6 +32,7 @@ bcrypt = { workspace = true } reqwest = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +serde_urlencoded = { workspace = true } uuid = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/rust/crates/du-web/src/api/dto.rs b/rust/crates/du-web/src/api/dto.rs index 9865792f..db74ef77 100644 --- a/rust/crates/du-web/src/api/dto.rs +++ b/rust/crates/du-web/src/api/dto.rs @@ -42,6 +42,18 @@ pub struct VariantDto { pub rs_ids: Vec, /// Coordinates keyed by reference build: `{ "GRCh38": {contig, position, ...} }`. pub coordinates: serde_json::Value, + /// This branch's ancestral allele (per-branch ASR transition). **Prefer this over + /// `coordinates..ancestral` when present** — the coordinate blob carries a + /// single global polarity per variant that can disagree with the branch's actual + /// direction (recurrent SNPs, back-mutations, reference-frame), so classifying a + /// descendant's genotype against it flips ~18% of backbone calls. NULL ⇒ forward/ + /// legacy link; fall back to the coordinate alleles. Populated only by `/full`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub link_ancestral: Option, + /// This branch's derived allele — the state a descendant carries. Prefer over + /// `coordinates..derived`; see [`link_ancestral`](Self::link_ancestral). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub link_derived: Option, } impl From for VariantDto { @@ -55,6 +67,10 @@ impl From for VariantDto { common_names: v.aliases.common_names, rs_ids: v.aliases.rs_ids, coordinates, + // Per-branch link is not a property of the variant itself; the tree `/full` + // path fills it from `tree.haplogroup_variant`. Plain variant lists leave it None. + link_ancestral: None, + link_derived: None, } } } diff --git a/rust/crates/du-web/src/api/mod.rs b/rust/crates/du-web/src/api/mod.rs index 8b8d7d88..83e38c36 100644 --- a/rust/crates/du-web/src/api/mod.rs +++ b/rust/crates/du-web/src/api/mod.rs @@ -234,6 +234,7 @@ fn pathway_dto(call: &du_db::biosample::HaplogroupCall, p: du_db::haplogroup::Pa origin: match call.origin { HaplogroupCallOrigin::Reconciled => "RECONCILED", HaplogroupCallOrigin::FedConsensus => "FED_CONSENSUS", + HaplogroupCallOrigin::TreePlacement => "TREE_PLACEMENT", HaplogroupCallOrigin::Original => "ORIGINAL", } .to_string(), @@ -570,6 +571,9 @@ mod tests { coordinates: serde_json::json!({ "hs1": {"contig": "chrY", "position": 2_800_000, "ancestral": "A", "derived": "G"} }), + // Per-branch ASR polarity, reverse of the global coordinate here (the flip case). + link_ancestral: Some("G".into()), + link_derived: Some("A".into()), }; let node = HaplogroupNodeDto { id: 10, @@ -589,6 +593,10 @@ mod tests { assert_eq!(var["canonical_name"], "M207"); assert_eq!(var["coordinates"]["hs1"]["position"], 2_800_000); assert_eq!(var["coordinates"]["hs1"]["derived"], "G"); + // Per-branch ASR polarity is served and is the authoritative pole for descent + // classification (here it's the reverse of the coordinate's global A>G). + assert_eq!(var["link_ancestral"], "G"); + assert_eq!(var["link_derived"], "A"); } #[test] diff --git a/rust/crates/du-web/src/api/tree.rs b/rust/crates/du-web/src/api/tree.rs index 929598c7..09ed18d1 100644 --- a/rust/crates/du-web/src/api/tree.rs +++ b/rust/crates/du-web/src/api/tree.rs @@ -74,8 +74,11 @@ async fn build_tree(st: &AppState, dna: DnaType, root: Option<&str>) -> Result) -> Result { let nodes = du_db::haplogroup::subtree(&st.pool, dna, root).await?; let mut variants: HashMap> = HashMap::new(); - for (hid, v) in du_db::variant::for_dna_type_grouped(&st.pool, dna).await? { - variants.entry(hid).or_default().push(VariantDto::from(v)); + for (hid, v, link_ancestral, link_derived) in du_db::variant::for_dna_type_grouped(&st.pool, dna).await? { + // Carry the per-branch ASR polarity so the descent report classifies against the + // branch's actual ancestral→derived direction, not the variant's global coordinate + // polarity (which flips ~18% of backbone calls). See VariantDto::link_ancestral. + variants.entry(hid).or_default().push(VariantDto { link_ancestral, link_derived, ..VariantDto::from(v) }); } let counts = du_db::tree_sample::counts_by_node(&st.pool, dna).await?; Ok(TreeDto { roots: assemble_forest(nodes, &variants, &counts) }) diff --git a/rust/crates/du-web/src/extract.rs b/rust/crates/du-web/src/extract.rs new file mode 100644 index 00000000..c16d0051 --- /dev/null +++ b/rust/crates/du-web/src/extract.rs @@ -0,0 +1,117 @@ +//! Shared Axum extractors. +//! +//! [`Query`] is a **duplicate-tolerant** replacement for `axum::extract::Query`. +//! Axum's own `Query` deserializes with `serde_urlencoded`, which rejects a query +//! string that repeats a key (`?status=X&page=2&status=X`) with `400 Bad Request` +//! (`serde_html_form` / axum-extra behave the same for scalar fields — a repeated +//! key only collapses when the target field is a `Vec`). That 400 is easy to hit +//! with HTMX: a paginated fragment whose container carries `hx-include="#filter"` +//! auto-sends the filter's value, so if a pagination link *also* hard-codes that +//! same param the key arrives twice (see the naming/regions/publications pagers). +//! +//! This extractor first splits the raw query into ordered `(key, value)` pairs — +//! which tolerates repeats, since that's a *sequence*, not a map — then collapses +//! duplicate keys keeping the **last** occurrence (an `hx-include` value appended +//! after a hard-coded URL param wins, matching HTML-form semantics), and finally +//! deserializes the deduped pairs into `T`. The API mirrors `axum::extract::Query`, +//! so handlers get the resilient behaviour by importing `crate::extract::Query`. +//! Emitting the duplicate in the first place is still a template bug worth fixing; +//! this just keeps the whole class from ever surfacing as a 400 again. + +use axum::extract::FromRequestParts; +use axum::http::request::Parts; +use axum::http::StatusCode; +use axum::response::{IntoResponse, Response}; +use serde::de::DeserializeOwned; + +/// Duplicate-tolerant query extractor. See module docs. +pub struct Query(pub T); + +fn bad_request(msg: impl std::fmt::Display) -> Response { + (StatusCode::BAD_REQUEST, format!("invalid query string: {msg}")).into_response() +} + +#[axum::async_trait] +impl FromRequestParts for Query +where + T: DeserializeOwned, + S: Send + Sync, +{ + type Rejection = Response; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + let raw = parts.uri.query().unwrap_or(""); + // Parse into ordered pairs. As a *sequence* this accepts repeated keys + // (unlike deserializing straight into a struct/map, which would 400). + let pairs: Vec<(String, String)> = serde_urlencoded::from_str(raw).map_err(bad_request)?; + // Collapse duplicates, keeping the last value seen for each key. + let mut deduped: Vec<(String, String)> = Vec::with_capacity(pairs.len()); + for (k, v) in pairs { + match deduped.iter_mut().find(|(ek, _)| *ek == k) { + Some(slot) => slot.1 = v, + None => deduped.push((k, v)), + } + } + let encoded = serde_urlencoded::to_string(&deduped).map_err(bad_request)?; + let value = serde_urlencoded::from_str(&encoded).map_err(bad_request)?; + Ok(Query(value)) + } +} + +#[cfg(test)] +mod tests { + use super::Query; + use axum::body::Body; + use axum::http::{Request, StatusCode}; + use axum::routing::get; + use axum::Router; + use serde::Deserialize; + use tower::ServiceExt; + + #[derive(Deserialize)] + struct ListQ { + status: Option, + page: Option, + } + + async fn echo(Query(q): Query) -> String { + format!("{}-{}", q.status.unwrap_or_default(), q.page.unwrap_or(0)) + } + + async fn call(uri: &str) -> (StatusCode, String) { + let resp = Router::new() + .route("/", get(echo)) + .oneshot(Request::builder().uri(uri).body(Body::empty()).unwrap()) + .await + .unwrap(); + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + (status, String::from_utf8(bytes.to_vec()).unwrap()) + } + + /// The whole point: a duplicated query key (what HTMX's hx-include produces on + /// top of a hard-coded pagination param) must NOT 400. Last value wins. + #[tokio::test] + async fn tolerates_duplicate_keys_last_wins() { + // The exact shape that used to 400 the curator pagers. + let (status, body) = call("/?status=&page=2&status=open").await; + assert_eq!(status, StatusCode::OK, "duplicate key must not 400"); + assert_eq!(body, "open-2", "last value of a repeated scalar wins"); + } + + /// A plain, non-duplicated query still deserializes normally. + #[tokio::test] + async fn plain_query_still_works() { + let (status, body) = call("/?status=closed&page=3").await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "closed-3"); + } + + /// An empty query string yields all-defaults (Options are None). + #[tokio::test] + async fn empty_query_is_defaults() { + let (status, body) = call("/").await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "-0"); + } +} diff --git a/rust/crates/du-web/src/main.rs b/rust/crates/du-web/src/main.rs index c533d330..b2b32775 100644 --- a/rust/crates/du-web/src/main.rs +++ b/rust/crates/du-web/src/main.rs @@ -12,6 +12,7 @@ use tower_cookies::Key; mod api; mod auth; mod error; +mod extract; mod htmx; mod i18n; mod render; diff --git a/rust/crates/du-web/src/routes/change_sets.rs b/rust/crates/du-web/src/routes/change_sets.rs index 3880d377..74f2e29c 100644 --- a/rust/crates/du-web/src/routes/change_sets.rs +++ b/rust/crates/du-web/src/routes/change_sets.rs @@ -10,7 +10,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/curation.rs b/rust/crates/du-web/src/routes/curation.rs index 907d9b25..557e1965 100644 --- a/rust/crates/du-web/src/routes/curation.rs +++ b/rust/crates/du-web/src/routes/curation.rs @@ -9,7 +9,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; diff --git a/rust/crates/du-web/src/routes/curator.rs b/rust/crates/du-web/src/routes/curator.rs index 99ebb7d4..f1e567f4 100644 --- a/rust/crates/du-web/src/routes/curator.rs +++ b/rust/crates/du-web/src/routes/curator.rs @@ -8,7 +8,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/curator_inbox.rs b/rust/crates/du-web/src/routes/curator_inbox.rs index d2262568..269242e7 100644 --- a/rust/crates/du-web/src/routes/curator_inbox.rs +++ b/rust/crates/du-web/src/routes/curator_inbox.rs @@ -9,7 +9,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{Html, IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/curator_regions.rs b/rust/crates/du-web/src/routes/curator_regions.rs index 70550b55..086842f3 100644 --- a/rust/crates/du-web/src/routes/curator_regions.rs +++ b/rust/crates/du-web/src/routes/curator_regions.rs @@ -8,7 +8,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/curator_variants.rs b/rust/crates/du-web/src/routes/curator_variants.rs index aadbfb92..8a4da03d 100644 --- a/rust/crates/du-web/src/routes/curator_variants.rs +++ b/rust/crates/du-web/src/routes/curator_variants.rs @@ -8,7 +8,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/denovo_conflicts.rs b/rust/crates/du-web/src/routes/denovo_conflicts.rs index 17ef904b..3543fb6b 100644 --- a/rust/crates/du-web/src/routes/denovo_conflicts.rs +++ b/rust/crates/du-web/src/routes/denovo_conflicts.rs @@ -8,7 +8,8 @@ use crate::error::AppError; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Query, State}; +use crate::extract::Query; +use axum::extract::State; use axum::response::Response; use axum::routing::get; use axum::Router; diff --git a/rust/crates/du-web/src/routes/naming.rs b/rust/crates/du-web/src/routes/naming.rs index 355acd3b..112b978c 100644 --- a/rust/crates/du-web/src/routes/naming.rs +++ b/rust/crates/du-web/src/routes/naming.rs @@ -10,7 +10,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; @@ -24,23 +25,55 @@ pub fn router() -> Router { .route("/curator/naming/fragment", get(list)) .route("/curator/naming/:id/panel", get(panel)) .route("/curator/naming/:id/assign", post(assign)) + .route("/curator/naming/:id/adopt", post(adopt)) .route("/curator/naming/:id/status", post(status)) } // ── helpers ───────────────────────────────────────────────────────────────── -/// "chrY:2781234" from the GRCh38 coordinate JSONB, or "—". -fn coord_label(coords: &serde_json::Value) -> String { - let g = coords.get("GRCh38"); - match g { - Some(g) => { - let contig = g.get("contig").and_then(|v| v.as_str()); - let pos = g.get("position").and_then(|v| v.as_str().map(str::to_string).or_else(|| v.as_i64().map(|n| n.to_string()))); - match (contig, pos) { - (Some(c), Some(p)) => format!("{c}:{p}"), - _ => "—".into(), - } +/// Pick the coordinate build to display, **preferring `hs1`** — the platform-native +/// T2T-CHM13 assembly the de-novo catalog is built on — then GRCh38, then whatever +/// build is present. Returns the build name and its coordinate object. +fn pick_build(coords: &serde_json::Value) -> Option<(String, &serde_json::Value)> { + let obj = coords.as_object()?; + for b in ["hs1", "GRCh38"] { + if let Some(v) = obj.get(b) { + return Some((b.to_string(), v)); } + } + obj.iter().next().map(|(k, v)| (k.clone(), v)) +} + +/// A JSONB string-or-integer field as a string (positions may be either). +fn field_str(v: &serde_json::Value, key: &str) -> Option { + v.get(key) + .and_then(|x| x.as_str().map(str::to_string).or_else(|| x.as_i64().map(|n| n.to_string()))) +} + +/// "chrY:2781234" from the preferred-build coordinate JSONB (hs1 first), or "—". +fn coord_label(coords: &serde_json::Value) -> String { + match pick_build(coords) { + Some((_, g)) => match (g.get("contig").and_then(|v| v.as_str()), field_str(g, "position")) { + (Some(c), Some(p)) => format!("{c}:{p}"), + _ => "—".into(), + }, + None => "—".into(), + } +} + +/// The build name whose coordinate `coord_label` displayed (e.g. "hs1"), or "—". +fn coord_build(coords: &serde_json::Value) -> String { + pick_build(coords).map(|(b, _)| b).unwrap_or_else(|| "—".into()) +} + +/// The mutation state "G→A" (ancestral→derived) from the preferred build — what the +/// curator needs to name a variant — or "—". +fn allele_label(coords: &serde_json::Value) -> String { + match pick_build(coords) { + Some((_, g)) => match (field_str(g, "ancestral"), field_str(g, "derived")) { + (Some(a), Some(d)) => format!("{a}→{d}"), + _ => "—".into(), + }, None => "—".into(), } } @@ -49,7 +82,15 @@ fn common_names(aliases: &serde_json::Value) -> Vec { aliases .get("common_names") .and_then(|v| v.as_array()) - .map(|a| a.iter().filter_map(|x| x.as_str().map(str::to_string)).collect()) + .map(|a| { + a.iter() + .filter_map(|x| x.as_str()) + // Drop synthetic coordinate placeholders (`chrY:…`) the loader stashed as + // aliases — they aren't real alternate names. + .filter(|s| !du_db::naming::is_placeholder_name(s)) + .map(str::to_string) + .collect() + }) .unwrap_or_default() } @@ -60,6 +101,7 @@ struct Row { name: String, status: String, coord: String, + alleles: String, defining: String, } @@ -84,12 +126,21 @@ async fn load_list(st: &AppState, q: &ListQuery) -> Result { let rows = result .items .into_iter() - .map(|i| Row { - id: i.id, - name: i.canonical_name.unwrap_or_else(|| "(unnamed)".into()), - status: i.naming_status, - coord: coord_label(&i.coordinates), - defining: i.defining.unwrap_or_else(|| "—".into()), + .map(|i| { + // A synthetic coordinate placeholder (`chrY:…`) the loader stamped NAMED is + // shown as unnamed — it is naming work, not a ratified name. + let placeholder = i.canonical_name.as_deref().is_some_and(du_db::naming::is_placeholder_name); + Row { + id: i.id, + name: match i.canonical_name { + Some(n) if !placeholder => n, + _ => "(unnamed)".into(), + }, + status: if placeholder { "UNNAMED".into() } else { i.naming_status }, + coord: coord_label(&i.coordinates), + alleles: allele_label(&i.coordinates), + defining: i.defining.unwrap_or_else(|| "—".into()), + } }) .collect(); Ok(ListView { mode, rows, page, total, total_pages }) @@ -147,10 +198,14 @@ struct DetailView { status: String, mutation_type: String, coord: String, + coord_build: String, + alleles: String, aliases: Vec, defining: Option, dedup: Vec, can_assign: bool, + /// Established (non-DU) name to reuse, if this variant is named by definition. + established: Option, notice: Option, } @@ -165,21 +220,36 @@ async fn build_detail(st: &AppState, id: i64, notice: Option) -> Result< let i = du_db::naming::get(&st.pool, id) .await? .ok_or_else(|| AppError::NotFound(format!("variant {id}")))?; - let dedup = du_db::naming::dedup_by_coordinates(&st.pool, id) + let dedup = du_db::naming::dedup_by_site(&st.pool, id) .await? .into_iter() .map(|(_, name)| Candidate { name }) .collect(); + // A synthetic coordinate placeholder (`chrY:…`) counts as unnamed even though the + // loader marked it NAMED — the variant is still nameable, and must not display as + // "already named". + let placeholder = i.canonical_name.as_deref().is_some_and(du_db::naming::is_placeholder_name); + let can_assign = i.naming_status != "NAMED" || placeholder; + // "Named by definition": the variant already carries an established (non-DU) + // name for its locus+mutation-state — reuse it rather than mint a new DU id. + let established = if can_assign && (i.canonical_name.is_none() || placeholder) { + du_db::naming::established_name(&i.aliases) + } else { + None + }; Ok(DetailView { id: i.id, - name: i.canonical_name.clone(), - status: i.naming_status.clone(), + name: if placeholder { None } else { i.canonical_name.clone() }, + status: if placeholder { "UNNAMED".into() } else { i.naming_status.clone() }, mutation_type: i.mutation_type, coord: coord_label(&i.coordinates), + coord_build: coord_build(&i.coordinates), + alleles: allele_label(&i.coordinates), aliases: common_names(&i.aliases), defining: i.defining, dedup, - can_assign: i.naming_status != "NAMED", + can_assign, + established, notice, }) } @@ -219,6 +289,22 @@ async fn assign( changed_response(&st, locale.t, id, Some(notice)).await } +/// Reuse the variant's established ISOGG/YBrowse name as its canonical name +/// ("named by definition") instead of minting a new DU identifier. +async fn adopt( + _c: Curator, + State(st): State, + locale: Locale, + Path(id): Path, +) -> Result { + let notice = match du_db::naming::adopt_established_name(&st.pool, id).await { + Ok(name) => format!("{} {name}", locale.t.get("nm.notice.adopted")), + Err(du_db::DbError::Conflict(m)) => m, + Err(e) => return Err(e.into()), + }; + changed_response(&st, locale.t, id, Some(notice)).await +} + #[derive(Deserialize)] struct StatusForm { /// PENDING_REVIEW | UNNAMED diff --git a/rust/crates/du-web/src/routes/publications.rs b/rust/crates/du-web/src/routes/publications.rs index 6db07a2d..219bc4d5 100644 --- a/rust/crates/du-web/src/routes/publications.rs +++ b/rust/crates/du-web/src/routes/publications.rs @@ -10,7 +10,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/reconcile_flags.rs b/rust/crates/du-web/src/routes/reconcile_flags.rs index f1304963..1db495e8 100644 --- a/rust/crates/du-web/src/routes/reconcile_flags.rs +++ b/rust/crates/du-web/src/routes/reconcile_flags.rs @@ -11,7 +11,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Router}; diff --git a/rust/crates/du-web/src/routes/samples.rs b/rust/crates/du-web/src/routes/samples.rs index 5c0c64f4..67f84700 100644 --- a/rust/crates/du-web/src/routes/samples.rs +++ b/rust/crates/du-web/src/routes/samples.rs @@ -57,6 +57,9 @@ struct PathwayView { placed: bool, /// True when the call is the cross-technology reconciliation consensus. reconciled: bool, + /// True when the call comes from the sample's de-novo tree placement (it was a + /// building block) rather than a published/federated call. + from_tree: bool, /// Consensus reliability for a reconciled call (formatted; "" when absent). confidence: String, run_count: String, @@ -232,6 +235,7 @@ fn build_pathway(call: Option<&HaplogroupCall>, pathway: Option) -> Pat call: None, placed: false, reconciled: false, + from_tree: false, confidence: String::new(), run_count: String::new(), concordance: String::new(), @@ -261,6 +265,7 @@ fn build_pathway(call: Option<&HaplogroupCall>, pathway: Option) -> Pat call: Some(call.name.clone()), placed: !steps.is_empty(), reconciled: call.origin == du_db::biosample::HaplogroupCallOrigin::Reconciled, + from_tree: call.origin == du_db::biosample::HaplogroupCallOrigin::TreePlacement, confidence: pct(call.confidence), run_count: call.run_count.map(|n| n.to_string()).unwrap_or_default(), concordance: pct(call.snp_concordance), diff --git a/rust/crates/du-web/src/routes/sequencer.rs b/rust/crates/du-web/src/routes/sequencer.rs index 46d09101..bf42c042 100644 --- a/rust/crates/du-web/src/routes/sequencer.rs +++ b/rust/crates/du-web/src/routes/sequencer.rs @@ -12,7 +12,8 @@ use crate::htmx::HxHeaders; use crate::i18n::{Locale, T}; use crate::render::html; use crate::state::AppState; -use axum::extract::{Path, Query, State}; +use crate::extract::Query; +use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use axum::routing::{get, post}; use axum::{Form, Json, Router}; @@ -32,6 +33,12 @@ pub fn router() -> Router { .route("/curator/instrument-proposals/:id/panel", get(ui_panel)) .route("/curator/instrument-proposals/:id/accept", post(ui_accept)) .route("/curator/instrument-proposals/:id/reject", post(ui_reject)) + // "Established" tab: maintain already-associated instrument→lab mappings + // directly (distinct path prefix so it never collides with the `:id` routes + // above). Same page, second pane. + .route("/curator/instrument-labs/fragment", get(ui_established_list)) + .route("/curator/instrument-labs/:id/panel", get(ui_established_panel)) + .route("/curator/instrument-labs/:id", post(ui_established_update)) } fn proposal_json(p: &ProposalView) -> Value { @@ -393,6 +400,160 @@ async fn ui_reject( changed_response(&st, locale.t, id, Some(notice)).await } +// ── "Established" maintenance tab ──────────────────────────────────────────── + +const ESTABLISHED_CHANGED: &str = "established-changed"; + +struct EstablishedRow { + id: i64, + instrument_id: String, + model: String, + lab: String, + /// True when the instrument has no lab yet — flagged for the curator's eye. + unassigned: bool, +} + +struct EstablishedList { + rows: Vec, + page: i64, + total: i64, + total_pages: i64, +} + +#[derive(Deserialize)] +struct EstablishedQuery { + query: Option, + page: Option, +} + +async fn load_established(st: &AppState, q: &EstablishedQuery) -> Result { + let page = du_db::sequencer::list_established(&st.pool, q.query.as_deref(), q.page.unwrap_or(1), 25).await?; + let (cur_page, total, total_pages) = (page.page, page.total, page.total_pages()); + Ok(EstablishedList { + rows: page + .items + .into_iter() + .map(|r| EstablishedRow { + id: r.id, + instrument_id: r.instrument_id, + model: r.model_name.unwrap_or_default(), + lab: r.lab_name.clone().unwrap_or_default(), + unassigned: r.lab_name.is_none(), + }) + .collect(), + page: cur_page, + total, + total_pages, + }) +} + +#[derive(askama::Template)] +#[template(path = "curator/instrument-proposals/established-list.html")] +struct EstablishedListTemplate { + t: T, + list: EstablishedList, +} + +async fn ui_established_list( + _c: Curator, + State(st): State, + locale: Locale, + Query(q): Query, +) -> Result { + let list = load_established(&st, &q).await?; + Ok(html(&EstablishedListTemplate { t: locale.t, list })) +} + +struct EstablishedDetail { + id: i64, + instrument_id: String, + lab: String, + manufacturer: String, + model: String, + is_d2c: bool, + unassigned: bool, + /// Existing lab names for the reassignment datalist. + labs: Vec, + notice: Option, +} + +#[derive(askama::Template)] +#[template(path = "curator/instrument-proposals/established-detail.html")] +struct EstablishedDetailTemplate { + t: T, + e: EstablishedDetail, +} + +async fn build_established_detail(st: &AppState, id: i64, notice: Option) -> Result { + let r = du_db::sequencer::established_detail(&st.pool, id) + .await? + .ok_or_else(|| AppError::NotFound(format!("instrument {id}")))?; + let labs = du_db::sequencer::list_labs(&st.pool).await?.into_iter().map(|l| l.name).collect(); + Ok(EstablishedDetail { + id: r.id, + instrument_id: r.instrument_id, + lab: r.lab_name.clone().unwrap_or_default(), + manufacturer: r.manufacturer.unwrap_or_default(), + model: r.model_name.unwrap_or_default(), + is_d2c: r.is_d2c.unwrap_or(false), + unassigned: r.lab_name.is_none(), + labs, + notice, + }) +} + +async fn established_detail_response(st: &AppState, t: T, id: i64, notice: Option) -> Result { + Ok(html(&EstablishedDetailTemplate { t, e: build_established_detail(st, id, notice).await? })) +} + +async fn ui_established_panel( + _c: Curator, + State(st): State, + locale: Locale, + Path(id): Path, +) -> Result { + established_detail_response(&st, locale.t, id, None).await +} + +#[derive(Deserialize)] +struct EstablishedForm { + lab_name: String, + manufacturer: Option, + model: Option, + /// Checkbox: present only when ticked (absent ⇒ leave the lab's flag untouched). + is_d2c: Option, +} + +async fn ui_established_update( + Curator(s): Curator, + State(st): State, + locale: Locale, + Path(id): Path, + Form(f): Form, +) -> Result { + let lab_name = f.lab_name.trim(); + if lab_name.is_empty() { + return Err(AppError::BadRequest("lab_name is required".into())); + } + let clean = |o: Option| o.map(|s| s.trim().to_string()).filter(|s| !s.is_empty()); + let manufacturer = clean(f.manufacturer); + let model = clean(f.model); + let is_d2c = f.is_d2c.map(|_| true); // ticked ⇒ Some(true); absent ⇒ None + let hit = du_db::sequencer::update_instrument_lab( + &st.pool, + s.user_id, + id, + lab_name, + manufacturer.as_deref(), + model.as_deref(), + is_d2c, + ) + .await?; + let notice = format!("{} {}", locale.t.get("il.notice.saved"), hit.lab_name); + let body = established_detail_response(&st, locale.t, id, Some(notice)).await?; + Ok((HxHeaders::new().trigger(ESTABLISHED_CHANGED), body).into_response()) +} + #[cfg(test)] mod tests { use axum::body::Body; diff --git a/rust/crates/du-web/src/security.rs b/rust/crates/du-web/src/security.rs index 1cf72ac8..2cf875c3 100644 --- a/rust/crates/du-web/src/security.rs +++ b/rust/crates/du-web/src/security.rs @@ -73,6 +73,21 @@ fn is_state_changing(m: &Method) -> bool { matches!(*m, Method::POST | Method::PUT | Method::PATCH | Method::DELETE) } +/// Dev-only escape hatch: when `DU_DISABLE_CSRF` is set to a truthy value, skip CSRF +/// enforcement (the `csrf` cookie is still issued). **Never set this in production** — +/// it exists so local smoke-testing of the native-form credential login/logout works +/// while the double-submit token is header-only. Read once at startup. +fn csrf_disabled() -> bool { + use std::sync::OnceLock; + static DISABLED: OnceLock = OnceLock::new(); + *DISABLED.get_or_init(|| { + matches!( + std::env::var("DU_DISABLE_CSRF").ok().as_deref(), + Some("1") | Some("true") | Some("TRUE") | Some("yes") + ) + }) +} + /// Read the `csrf` cookie value out of the request's `Cookie` header. fn csrf_cookie(req: &Request) -> Option { req.headers() @@ -95,7 +110,7 @@ pub async fn csrf_protect(req: Request, next: Next) -> Response { let cookie_token = csrf_cookie(&req); let exempt = req.uri().path().starts_with("/api/v1/"); - if is_state_changing(req.method()) && !exempt { + if is_state_changing(req.method()) && !exempt && !csrf_disabled() { let header_token = req.headers().get("x-csrf-token").and_then(|v| v.to_str().ok()); let valid = matches!((&cookie_token, header_token), (Some(c), Some(h)) if c == h); if !valid { diff --git a/rust/crates/du-web/templates/base.html b/rust/crates/du-web/templates/base.html index a045a152..7362b982 100644 --- a/rust/crates/du-web/templates/base.html +++ b/rust/crates/du-web/templates/base.html @@ -8,6 +8,11 @@ + {# Bootstrap loads in (deferred), NOT at end of : hx-boost swaps the + whole body on every nav, and a {% block head %}{% endblock %} @@ -25,7 +30,7 @@