From f6ecf00dfea270191abf3169bd2ae2749c60cebd Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 07:07:22 -0500 Subject: [PATCH 01/12] fix(etl): key sequencing_lab migration by name, not legacy id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lab transform predated migration 0038, which seeds the canonical labs keyed by NAME (with the D8 instrument→lab lookup FK'd to them by name). Forcing legacy ids over that seed via OVERRIDING SYSTEM VALUE + ON CONFLICT (id) collided on the sequencing_lab_name_key unique index during cutover (legacy id 1 "YSEQ" vs seeded id 5 "YSEQ"). Merge prod labs onto the seed by name instead — enrich metadata and add prod-only labs. Legacy lab ids carry no downstream FK (sequence_library resolves its lab by name string, not id), so id preservation is unneeded. Verified end-to-end on a real prod dump. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-migrate/src/transform.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) 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(()) } From 1a3e7a0361fc96b33ed5bdf2f7da0e226d24e763 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 07:10:56 -0500 Subject: [PATCH 02/12] feat(api): serve per-branch ASR allele polarity on the /full tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tree "/full" API now emits each defining variant's per-branch ancestral/derived alleles (link_ancestral/link_derived from tree.haplogroup_variant) alongside the variant's global coordinates. Descent classification must use the branch's actual ancestral→derived direction: the coordinate blob carries a single global polarity per variant that disagrees with the branch on recurrent SNPs, back-mutations and reference-frame cases, flipping ~18% of backbone calls. NULL link alleles ⇒ forward/legacy link, fall back to coordinates. Also rewrites for_dna_type_grouped from one wide join into two narrow passes stitched in-process: a recurrent SNP fans out to many branches, so the whole-tree join produced ~100k links over ~5k nodes and the planner picked a nested loop random-reading the wide core.variant heap once per link (plus a disk-spilling sort). Now: pull narrow link rows, read each distinct variant once by id, stitch + sort in Rust. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-db/src/variant.rs | 70 ++++++++++++++++++++++++------ rust/crates/du-web/src/api/dto.rs | 16 +++++++ rust/crates/du-web/src/api/mod.rs | 7 +++ rust/crates/du-web/src/api/tree.rs | 7 ++- 4 files changed, 84 insertions(+), 16 deletions(-) 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-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..821e4ee3 100644 --- a/rust/crates/du-web/src/api/mod.rs +++ b/rust/crates/du-web/src/api/mod.rs @@ -570,6 +570,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 +592,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) }) From c190ae3c7ee58ca1447abeb520ca7d24895d7d65 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 07:29:29 -0500 Subject: [PATCH 03/12] test(dev): env-gated CSRF bypass for local smoke testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DU_DISABLE_CSRF=1 skips the double-submit CSRF enforcement (the `csrf` cookie is still issued) so the native-form credential login/logout can be exercised locally. The login form submits as a non-boosted native POST and therefore never carries the X-CSRF-Token header the middleware requires, so credential auth 403s without this toggle. Dev/local-test only — never set DU_DISABLE_CSRF in production. The durable fix is to make login/logout carry the token (hidden field + form-field fallback, or HX-Redirect-boosted forms). Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-web/src/security.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 { From 30b8fc91d299aff8900f9621f6416dda9c0784d3 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 09:29:02 -0500 Subject: [PATCH 04/12] fix(naming): hs1-native display, dedup by site, and name+branch model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variant Naming Authority fixes surfaced during the cutover smoke test: - Display the hs1 (T2T-CHM13) coordinate the de-novo catalog is built on, not GRCh38 (most de-novo variants had no GRCh38 coord → blank). Show the mutation state (ancestral→derived) the curator needs to name a variant. - Dedup by SITE (locus + ancestral + derived), not position alone: two distinct SNPs can share a position with different alleles (Z12236 A>C vs Y17125 A>G) and are not duplicates. - Reuse an established (non-DU) name via a new "adopt" action instead of minting — "named by definition" — guarded against the (name, branch) unique index so a taken name routes to merge review, not a 500. - Scope the "needs a name" queue to variants that DEFINE a branch: a canonical name is (name + branch), so branch-less catalog rows are reference data, not naming work. The (canonical_name, defining_haplogroup_id) model — same SNP name canonical on each branch it defines, replacing ISOGG's L270.1/.2 suffixes — was never populated (defining_haplogroup_id NULL everywhere), collapsing the unique index to global-name-uniqueness. scripts/backfill-defining-haplogroup.sql populates it from the tree links (row-per-(name,branch), recurrence split). Durable loader support (denovo::load setting it + clear_dna deleting the recurrence copies) is the tracked follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-db/src/naming.rs | 107 ++++++++++++++++-- rust/crates/du-db/tests/variant_naming.rs | 90 +++++++++++---- rust/crates/du-web/src/routes/naming.rs | 91 ++++++++++++--- .../templates/curator/naming/detail.html | 14 ++- .../du-web/templates/curator/naming/list.html | 4 +- rust/locales/en.txt | 13 ++- rust/locales/es.txt | 13 ++- rust/locales/fr.txt | 13 ++- rust/scripts/backfill-defining-haplogroup.sql | 58 ++++++++++ 9 files changed, 345 insertions(+), 58 deletions(-) create mode 100644 rust/scripts/backfill-defining-haplogroup.sql diff --git a/rust/crates/du-db/src/naming.rs b/rust/crates/du-db/src/naming.rs index f0699467..e64b2500 100644 --- a/rust/crates/du-db/src/naming.rs +++ b/rust/crates/du-db/src/naming.rs @@ -45,8 +45,14 @@ 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. A canonical name is + // (name + the branch it defines), so only a variant that DEFINES a branch is + // nameable — branch-less catalog rows are reference data, not naming work. + // "Needs a name" = defines a branch AND has no real name yet: no name, a + // synthetic coordinate placeholder (`chrY:pos…`), or flagged for review. + _ => "v.defining_haplogroup_id IS NOT NULL \ + AND (v.canonical_name IS NULL OR v.canonical_name LIKE 'chr%:%' \ + OR v.naming_status = 'PENDING_REVIEW')", } } @@ -140,17 +146,96 @@ 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()) + .find(|s| !s.trim().is_empty() && !s.starts_with("DU")) + .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, String, Value)> = sqlx::query_as( + "SELECT canonical_name, naming_status::text, aliases FROM core.variant WHERE id = $1 FOR UPDATE", + ) + .bind(id) + .fetch_optional(&mut *tx) + .await?; + let (canon, status, aliases) = + row.ok_or_else(|| DbError::Conflict(format!("variant {id} not found")))?; + if status == "NAMED" || canon.is_some() { + 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/tests/variant_naming.rs b/rust/crates/du-db/tests/variant_naming.rs index d19776d4..3f72f717 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; + + // 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 = no-name-yet OR flagged-for-review. + // 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,28 @@ 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"); + // 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(std::iter::once(&adoptable)) { 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-web/src/routes/naming.rs b/rust/crates/du-web/src/routes/naming.rs index 355acd3b..390b95a9 100644 --- a/rust/crates/du-web/src/routes/naming.rs +++ b/rust/crates/du-web/src/routes/naming.rs @@ -24,23 +24,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(), } } @@ -60,6 +92,7 @@ struct Row { name: String, status: String, coord: String, + alleles: String, defining: String, } @@ -89,6 +122,7 @@ async fn load_list(st: &AppState, q: &ListQuery) -> Result { name: i.canonical_name.unwrap_or_else(|| "(unnamed)".into()), status: i.naming_status, coord: coord_label(&i.coordinates), + alleles: allele_label(&i.coordinates), defining: i.defining.unwrap_or_else(|| "—".into()), }) .collect(); @@ -147,10 +181,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 +203,32 @@ 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(); + let can_assign = i.naming_status != "NAMED"; + // "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() { + du_db::naming::established_name(&i.aliases) + } else { + None + }; Ok(DetailView { id: i.id, name: i.canonical_name.clone(), status: 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 +268,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/templates/curator/naming/detail.html b/rust/crates/du-web/templates/curator/naming/detail.html index 8a2af870..1b2b82d6 100644 --- a/rust/crates/du-web/templates/curator/naming/detail.html +++ b/rust/crates/du-web/templates/curator/naming/detail.html @@ -8,7 +8,8 @@ {% if let Some(n) = v.notice %}
{{ n }}
{% endif %}
-
{{ t.get("nm.coord") }}
{{ v.coord }}
+
{{ t.get("nm.coord") }}
{{ v.coord }} {{ v.coord_build }}
+
{{ t.get("nm.mut") }}
{{ v.alleles }}
{{ t.get("nm.type") }}
{{ v.mutation_type }}
{% if let Some(d) = v.defining %}
{{ t.get("nm.defines") }}
{{ d }}
{% endif %} {% if !v.aliases.is_empty() %} @@ -27,9 +28,20 @@ {% if v.can_assign %}
+ {% if let Some(name) = v.established %} + {# Named by definition: an established (ISOGG/YBrowse) name exists for this + locus + mutation state — reuse it rather than mint a new DU identifier. #} + + + {% else %} + {% endif %} {% if v.status != "PENDING_REVIEW" %}
- {% block scripts %}{% endblock %} From 24bab372d694680de4a5482404c91fedbd3ff583 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 12:10:36 -0500 Subject: [PATCH 06/12] =?UTF-8?q?fix(naming):=20pagination=20400=20?= =?UTF-8?q?=E2=80=94=20duplicate=20mode=20query=20param?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Prev/Next buttons put mode in their hx-get URL AND inherit hx-include="#nm-mode" from the parent #naming-table, so htmx appended mode a second time (?mode=X&page=N&mode=X). serde rejects the duplicate key → 400, so pagination silently failed. Drop mode from the button URLs and let the inherited hx-include supply it. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-web/templates/curator/naming/list.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/crates/du-web/templates/curator/naming/list.html b/rust/crates/du-web/templates/curator/naming/list.html index 1a213f12..e281b51a 100644 --- a/rust/crates/du-web/templates/curator/naming/list.html +++ b/rust/crates/du-web/templates/curator/naming/list.html @@ -23,11 +23,13 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} From fb67cbf1c0348594104d78fe18ff731a7f0fd7ec Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 12:38:56 -0500 Subject: [PATCH 07/12] fix(naming): treat de-novo coordinate placeholders as unnamed The de-novo loader stamps every branch-defining variant NAMED with a synthetic coordinate string as its canonical_name (e.g. "chrY:10158192 C->G") and also copies that string into common_names. The naming queue already surfaced these (canonical_name LIKE 'chr%:%'), but the detail panel keyed "already named" purely off naming_status = NAMED, so the curator saw them as named with no way to mint a DU identifier. Add du_db::naming::is_placeholder_name and thread it through: - assign_du_name / adopt_established_name accept a placeholder-named NAMED variant as still-nameable (real name blocks, placeholder doesn't) - assign_du_name purges chr placeholders from common_names on mint and never preserves a placeholder as an alias - established_name / common_names display filter skip placeholders so they aren't offered for adoption or shown as alias chips - list + detail render placeholder-named rows as UNNAMED (unnamed) Regression test covers the NAMED-placeholder mint path. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-db/src/naming.rs | 45 ++++++++++++++++++----- rust/crates/du-db/tests/variant_naming.rs | 26 ++++++++++++- rust/crates/du-web/src/routes/naming.rs | 44 ++++++++++++++++------ 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/rust/crates/du-db/src/naming.rs b/rust/crates/du-db/src/naming.rs index 97643aa7..67bf8fad 100644 --- a/rust/crates/du-db/src/naming.rs +++ b/rust/crates/du-db/src/naming.rs @@ -93,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 { @@ -119,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}', \ @@ -139,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) @@ -159,7 +181,9 @@ pub fn established_name(aliases: &Value) -> Option { .as_array()? .iter() .filter_map(|x| x.as_str()) - .find(|s| !s.trim().is_empty() && !s.starts_with("DU")) + // 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) } @@ -171,15 +195,18 @@ pub fn established_name(aliases: &Value) -> Option { /// 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, String, Value)> = sqlx::query_as( - "SELECT canonical_name, naming_status::text, aliases FROM core.variant WHERE id = $1 FOR UPDATE", + 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, status, aliases) = + let (canon, aliases) = row.ok_or_else(|| DbError::Conflict(format!("variant {id} not found")))?; - if status == "NAMED" || canon.is_some() { + // 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) diff --git a/rust/crates/du-db/tests/variant_naming.rs b/rust/crates/du-db/tests/variant_naming.rs index 3f72f717..28393342 100644 --- a/rust/crates/du-db/tests/variant_naming.rs +++ b/rust/crates/du-db/tests/variant_naming.rs @@ -120,12 +120,36 @@ async fn variant_naming_authority_flow() { // 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 (variants first — they reference the branches via defining_haplogroup_id). - for id in ids.iter().chain(std::iter::once(&adoptable)) { + 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] { diff --git a/rust/crates/du-web/src/routes/naming.rs b/rust/crates/du-web/src/routes/naming.rs index 390b95a9..0197fc7d 100644 --- a/rust/crates/du-web/src/routes/naming.rs +++ b/rust/crates/du-web/src/routes/naming.rs @@ -81,7 +81,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() } @@ -117,13 +125,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), - alleles: allele_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 }) @@ -208,18 +224,22 @@ async fn build_detail(st: &AppState, id: i64, notice: Option) -> Result< .into_iter() .map(|(_, name)| Candidate { name }) .collect(); - let can_assign = i.naming_status != "NAMED"; + // 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() { + 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), From 87e24e9fca141e61c4de57f9d3869e6383561110 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 12:56:28 -0500 Subject: [PATCH 08/12] fix(curator): pagination 400 across region/variant/pub/proposal/change-set tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same duplicate-query-param bug as the naming tool (24bab37): each of these fragment lists sits in a container with hx-include pointing at a filter control (search box or status select), and the Prev/Next buttons ALSO hard-coded that same param in their hx-get URL. htmx appends the included value on top of the URL's, producing a duplicate key (?query=&page=2&query=) which axum's Query extractor rejects with 400. Fix (matching the naming + haplogroups pattern): drop the hard-coded filter param from the pagination URL and keep only `page` — the inherited hx-include supplies the live filter value (also fixes stale-snapshot filters when paging). Tools fixed: regions (the reported one), variants, publications, proposals, change-sets. Audited all curator fragment pagers: haplogroups/naming already correct; instrument-proposals and denovo-conflicts have no hx-include so don't collide; inbox has no Prev/Next controls. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-web/templates/curator/change-sets/list.html | 6 ++++-- rust/crates/du-web/templates/curator/proposals/list.html | 6 ++++-- .../crates/du-web/templates/curator/publications/list.html | 6 ++++-- rust/crates/du-web/templates/curator/regions/list.html | 7 +++++-- rust/crates/du-web/templates/curator/variants/list.html | 6 ++++-- 5 files changed, 21 insertions(+), 10 deletions(-) diff --git a/rust/crates/du-web/templates/curator/change-sets/list.html b/rust/crates/du-web/templates/curator/change-sets/list.html index a6c434d8..7d210459 100644 --- a/rust/crates/du-web/templates/curator/change-sets/list.html +++ b/rust/crates/du-web/templates/curator/change-sets/list.html @@ -23,11 +23,13 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} diff --git a/rust/crates/du-web/templates/curator/proposals/list.html b/rust/crates/du-web/templates/curator/proposals/list.html index e269c41c..dcf1fddb 100644 --- a/rust/crates/du-web/templates/curator/proposals/list.html +++ b/rust/crates/du-web/templates/curator/proposals/list.html @@ -22,11 +22,13 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} diff --git a/rust/crates/du-web/templates/curator/publications/list.html b/rust/crates/du-web/templates/curator/publications/list.html index e05677d7..deb11cdc 100644 --- a/rust/crates/du-web/templates/curator/publications/list.html +++ b/rust/crates/du-web/templates/curator/publications/list.html @@ -21,11 +21,13 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} diff --git a/rust/crates/du-web/templates/curator/regions/list.html b/rust/crates/du-web/templates/curator/regions/list.html index e6a30d61..6a1507fe 100644 --- a/rust/crates/du-web/templates/curator/regions/list.html +++ b/rust/crates/du-web/templates/curator/regions/list.html @@ -15,11 +15,14 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} diff --git a/rust/crates/du-web/templates/curator/variants/list.html b/rust/crates/du-web/templates/curator/variants/list.html index 333cc2a7..c037a972 100644 --- a/rust/crates/du-web/templates/curator/variants/list.html +++ b/rust/crates/du-web/templates/curator/variants/list.html @@ -15,11 +15,13 @@ {% if list.total_pages > 1 %} {% else %} {{ list.total }} {{ t.get("pagination.total") }} From b7680025125c6f24e177a0f3f25febe136b9814b Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 14:22:30 -0500 Subject: [PATCH 09/12] feat(curator): duplicate-tolerant Query extractor + inbox pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the pagination-400 sweep. 1. Defense-in-depth: crate::extract::Query, a drop-in replacement for axum::extract::Query that no longer 400s on a repeated query key. axum's Query (serde_urlencoded) and axum-extra's (serde_html_form) both reject a duplicated scalar key — exactly what HTMX produces when a pagination link hard-codes a param that the container also sends via hx-include. The new extractor splits the raw query into ordered pairs (a sequence tolerates repeats), collapses duplicates keeping the last value (form semantics), then deserializes. Swapped into all 11 curator fragment route modules. Unit tests cover last-wins, plain, and empty. 2. Inbox pagination: the team inbox rendered a "Page x / y" label with no controls, so curators couldn't get past page 1. Added Prev/Next buttons (its container has no hx-include, so status is carried in the URL — now doubly safe under the new extractor). Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/Cargo.lock | 1 + rust/Cargo.toml | 3 + rust/crates/du-web/Cargo.toml | 1 + rust/crates/du-web/src/extract.rs | 117 ++++++++++++++++++ rust/crates/du-web/src/main.rs | 1 + rust/crates/du-web/src/routes/change_sets.rs | 3 +- rust/crates/du-web/src/routes/curation.rs | 3 +- rust/crates/du-web/src/routes/curator.rs | 3 +- .../crates/du-web/src/routes/curator_inbox.rs | 3 +- .../du-web/src/routes/curator_regions.rs | 3 +- .../du-web/src/routes/curator_variants.rs | 3 +- .../du-web/src/routes/denovo_conflicts.rs | 3 +- rust/crates/du-web/src/routes/naming.rs | 3 +- rust/crates/du-web/src/routes/publications.rs | 3 +- .../du-web/src/routes/reconcile_flags.rs | 3 +- rust/crates/du-web/src/routes/sequencer.rs | 3 +- .../du-web/templates/curator/inbox/list.html | 10 +- 17 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 rust/crates/du-web/src/extract.rs 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-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/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 0197fc7d..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}; 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/sequencer.rs b/rust/crates/du-web/src/routes/sequencer.rs index 46d09101..65837227 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}; diff --git a/rust/crates/du-web/templates/curator/inbox/list.html b/rust/crates/du-web/templates/curator/inbox/list.html index c3b65af7..00e4c2a8 100644 --- a/rust/crates/du-web/templates/curator/inbox/list.html +++ b/rust/crates/du-web/templates/curator/inbox/list.html @@ -21,6 +21,14 @@ {% if list.total_pages > 1 %} -

{{ t.get("inbox.page") }} {{ list.page }} / {{ list.total_pages }} · {{ list.total }}

+{# #thread-table has no hx-include, so the status filter is carried in the URL + alongside page (no duplicate-key risk). #} + {% endif %} {% endif %} From 5b629827e6603e5b6f9694467550264d35ca090b Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 15:10:01 -0500 Subject: [PATCH 10/12] =?UTF-8?q?feat(sequencer):=20"Established"=20tab=20?= =?UTF-8?q?to=20maintain=20instrument=E2=86=92lab=20associations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Sequencer-Lab tool was proposal-only — a consensus review queue that's empty until federation flows. The 36 preseeded (mig 0038) instrument→lab mappings were applied to sequencer_instrument.lab_id and served by the public lookup, but had no maintenance surface. Add a second tab that lists every instrument→lab association (search + pagination, unassigned first) and lets a curator reassign the lab, fix the model/manufacturer, and set the D2C flag directly — no proposal needed. Reuses the get-or-create-lab + audited-update path from accept_proposal, keyed by sequencer_instrument.id. du_db::sequencer: list_established, established_detail, list_labs, update_instrument_lab (audited REASSIGN_LAB). New routes under /curator/instrument-labs/* (distinct prefix — no :id route collision). Bootstrap nav-tabs on the page; each tab is its own two-panel pane. Pagination follows the ?page-only + hx-include pattern. i18n en/es/fr. DB test covers list/search/reassign/create-new-lab/audit. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-db/src/sequencer.rs | 158 +++++++++++++++++ rust/crates/du-db/tests/sequencer.rs | 70 ++++++++ rust/crates/du-web/src/routes/sequencer.rs | 160 ++++++++++++++++++ .../established-detail.html | 38 +++++ .../established-list.html | 32 ++++ .../curator/instrument-proposals/page.html | 54 +++++- rust/locales/en.txt | 10 ++ rust/locales/es.txt | 10 ++ rust/locales/fr.txt | 10 ++ 9 files changed, 534 insertions(+), 8 deletions(-) create mode 100644 rust/crates/du-web/templates/curator/instrument-proposals/established-detail.html create mode 100644 rust/crates/du-web/templates/curator/instrument-proposals/established-list.html 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/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-web/src/routes/sequencer.rs b/rust/crates/du-web/src/routes/sequencer.rs index 65837227..bf42c042 100644 --- a/rust/crates/du-web/src/routes/sequencer.rs +++ b/rust/crates/du-web/src/routes/sequencer.rs @@ -33,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 { @@ -394,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/templates/curator/instrument-proposals/established-detail.html b/rust/crates/du-web/templates/curator/instrument-proposals/established-detail.html new file mode 100644 index 00000000..dabfcde5 --- /dev/null +++ b/rust/crates/du-web/templates/curator/instrument-proposals/established-detail.html @@ -0,0 +1,38 @@ +{# Fragment: established instrument→lab edit panel. Target of #established-detail. #} +
+
+ {{ e.instrument_id }} + {% if e.unassigned %}{{ t.get("il.unassigned") }}{% else %}{{ e.lab }}{% endif %} +
+
+ {% if let Some(n) = e.notice %}
{{ n }}
{% endif %} + +
+
+ + {# Datalist: pick an existing lab or type a new name. #} + + + {% for l in e.labs %} +
+
+
+ + +
+
+ + +
+
+
+ + +
+ + {{ t.get("il.save.hint") }} +
+
+
diff --git a/rust/crates/du-web/templates/curator/instrument-proposals/established-list.html b/rust/crates/du-web/templates/curator/instrument-proposals/established-list.html new file mode 100644 index 00000000..bb4865b2 --- /dev/null +++ b/rust/crates/du-web/templates/curator/instrument-proposals/established-list.html @@ -0,0 +1,32 @@ +{# Fragment: established instrument→lab rows. Target of #established-table. #} + + + + + + + + {% for r in list.rows %} + + + + + + {% else %} + + {% endfor %} + +
{{ t.get("ip.col.instrument") }}{{ t.get("ip.col.model") }}{{ t.get("ip.col.lab") }}
{{ r.instrument_id }}{{ r.model }}{% if r.unassigned %}{{ t.get("il.unassigned") }}{% else %}{{ r.lab }}{% endif %}
{{ t.get("il.none") }}
+{% if list.total_pages > 1 %} + +{% else %} +{{ list.total }} {{ t.get("pagination.total") }} +{% endif %} diff --git a/rust/crates/du-web/templates/curator/instrument-proposals/page.html b/rust/crates/du-web/templates/curator/instrument-proposals/page.html index ff2e0b6c..ba7a9054 100644 --- a/rust/crates/du-web/templates/curator/instrument-proposals/page.html +++ b/rust/crates/du-web/templates/curator/instrument-proposals/page.html @@ -3,16 +3,54 @@ {% block content %}

{{ t.get("ip.title") }}

{{ t.get("ip.intro") }}

-
-
-
- {% include "curator/instrument-proposals/list.html" %} + +
    + + +
+ +
+ {# Consensus proposal queue (mined from the federation). #} +
+
+
+
+ {% include "curator/instrument-proposals/list.html" %} +
+
+
+

{{ t.get("ip.select") }}

+
-
-

{{ t.get("ip.select") }}

+ + {# Established instrument→lab associations (preseeded / ratified) — maintainable. #} +
+
+
+ +
+
+
+

{{ t.get("il.select") }}

+
+
{% endblock %} diff --git a/rust/locales/en.txt b/rust/locales/en.txt index b7a1aa6a..076610a5 100644 --- a/rust/locales/en.txt +++ b/rust/locales/en.txt @@ -592,6 +592,16 @@ ip.notice.accepted=Accepted — lab set: ip.notice.rejected=Proposal rejected. ip.resolved=Resolved ip.resolved.note=This proposal has been resolved. +ip.tab.proposals=Proposals +ip.tab.established=Established +il.search=Search instrument, model, or lab… +il.select=Select an instrument to maintain its lab. +il.none=No instruments. +il.unassigned=Unassigned +il.lab_name=Lab +il.save=Save +il.save.hint=Sets the instrument's lab (what the public lookup resolves). +il.notice.saved=Saved — lab set: # Per-sample public report (samples/report.html) sample.identity.title=Sample diff --git a/rust/locales/es.txt b/rust/locales/es.txt index 4dbe0a7f..37842270 100644 --- a/rust/locales/es.txt +++ b/rust/locales/es.txt @@ -453,6 +453,16 @@ ip.notice.accepted=Aceptada — laboratorio fijado: ip.notice.rejected=Propuesta rechazada. ip.resolved=Resuelta ip.resolved.note=Esta propuesta se ha resuelto. +ip.tab.proposals=Propuestas +ip.tab.established=Establecidas +il.search=Buscar instrumento, modelo o laboratorio… +il.select=Seleccione un instrumento para mantener su laboratorio. +il.none=Sin instrumentos. +il.unassigned=Sin asignar +il.lab_name=Laboratorio +il.save=Guardar +il.save.hint=Establece el laboratorio del instrumento (lo que resuelve la búsqueda pública). +il.notice.saved=Guardado — laboratorio establecido: # Informe público por muestra (samples/report.html) sample.identity.title=Muestra diff --git a/rust/locales/fr.txt b/rust/locales/fr.txt index 30ff5584..21e525c1 100644 --- a/rust/locales/fr.txt +++ b/rust/locales/fr.txt @@ -453,6 +453,16 @@ ip.notice.accepted=Acceptée — laboratoire fixé : ip.notice.rejected=Proposition rejetée. ip.resolved=Résolu ip.resolved.note=Cette proposition a été résolue. +ip.tab.proposals=Propositions +ip.tab.established=Établies +il.search=Rechercher un instrument, un modèle ou un labo… +il.select=Sélectionnez un instrument pour gérer son laboratoire. +il.none=Aucun instrument. +il.unassigned=Non attribué +il.lab_name=Laboratoire +il.save=Enregistrer +il.save.hint=Définit le laboratoire de l'instrument (ce que résout la recherche publique). +il.notice.saved=Enregistré — laboratoire défini : # Rapport public par échantillon (samples/report.html) sample.identity.title=Échantillon From 0d717543af2fdd0c2006d1b4ea708ce9ecfdb2f6 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 17:17:08 -0500 Subject: [PATCH 11/12] feat(samples): backfill reference-sample haplogroups via donor consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reference samples used as de-novo tree building blocks showed "No haplogroup call" even though they're placed in the tree. Root cause: one individual is split across several core.biosample rows — multiple publication accessions (SAMN… NCBI + SAMEA… EBI) plus a de-novo tree tip keyed by the panel id (NA18530) — and the ETL gave each its own specimen_donor while leaving the tip donor-less. The tree placement (tree.haplogroup_sample) landed on the tip, so it couldn't surface on the catalog accessions. ~2,660 individuals affected. Fix uses the donor as the linking entity (the entity above biosamples): - du_db::donor::consolidate_denovo_donors — groups biosamples by shared panel id (tip.accession == ref.alias), points every member at the richest donor, fills its gaps + canonical identifier, prunes the emptied donors. New run-once job `consolidate-donors` (preview unless --apply). Applied to the cutover DB: 2,661 groups, 2,684 biosamples repointed, 23 donors pruned. - Sample report resolves the haplogroup at the DONOR level: a new TreePlacement call origin sits between FedConsensus and Original in the precedence, resolving tree.haplogroup_sample across the donor's biosamples. Shows on every accession with a "from de-novo tree placement" badge. - De-novo loader now attaches each tip to the shared donor at creation (reuse an existing catalog donor by alias/accession, else create one keyed by the panel id) so reloads don't recreate donor-less tips. - dedup merge plan: cover genomics.biosample_str_profile (mig 0053, was stale → assert_fk_coverage would reject any merge); KeepSurvivor now handles a table unique on the repoint column alone. Tests: donor_consolidation (split→consolidate→prune→report surfaces placement on both accessions); existing dedup_merge/merge_e2e still pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-db/src/biosample.rs | 102 +++++++------ rust/crates/du-db/src/dedup.rs | 15 +- rust/crates/du-db/src/denovo.rs | 40 +++++- rust/crates/du-db/src/donor.rs | 134 ++++++++++++++++++ rust/crates/du-db/src/lib.rs | 1 + .../crates/du-db/tests/donor_consolidation.rs | 127 +++++++++++++++++ rust/crates/du-jobs/src/main.rs | 17 ++- rust/crates/du-web/src/api/mod.rs | 1 + rust/crates/du-web/src/routes/samples.rs | 5 + .../du-web/templates/samples/_pathway.html | 4 + rust/locales/en.txt | 1 + rust/locales/es.txt | 1 + rust/locales/fr.txt | 1 + 13 files changed, 396 insertions(+), 53 deletions(-) create mode 100644 rust/crates/du-db/src/donor.rs create mode 100644 rust/crates/du-db/tests/donor_consolidation.rs 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/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-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-web/src/api/mod.rs b/rust/crates/du-web/src/api/mod.rs index 821e4ee3..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(), 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/templates/samples/_pathway.html b/rust/crates/du-web/templates/samples/_pathway.html index 642ce188..0526d025 100644 --- a/rust/crates/du-web/templates/samples/_pathway.html +++ b/rust/crates/du-web/templates/samples/_pathway.html @@ -13,6 +13,10 @@ {% if pv.confidence != "" %}{{ t.get("sample.recon.confidence") }} {{ pv.confidence }}{% endif %} {% if pv.concordance != "" %}{{ t.get("sample.recon.concordance") }} {{ pv.concordance }}{% endif %}
+ {% else if pv.from_tree %} +
+ {{ t.get("sample.pathway.fromTree") }} +
{% endif %} {% if pv.call.is_none() %}
{{ t.get("sample.pathway.noCall") }}
diff --git a/rust/locales/en.txt b/rust/locales/en.txt index 076610a5..d9173d69 100644 --- a/rust/locales/en.txt +++ b/rust/locales/en.txt @@ -615,6 +615,7 @@ sample.ydna.title=Y-DNA pathway sample.mtdna.title=mtDNA pathway sample.pathway.noCall=No haplogroup call for this sample. sample.pathway.unplaced=Called haplogroup is not yet placed in the tree. +sample.pathway.fromTree=From de-novo tree placement sample.recon.consensus=Cross-technology consensus sample.recon.runs=runs sample.recon.confidence=confidence diff --git a/rust/locales/es.txt b/rust/locales/es.txt index 37842270..abeab010 100644 --- a/rust/locales/es.txt +++ b/rust/locales/es.txt @@ -476,6 +476,7 @@ sample.ydna.title=Linaje Y-DNA sample.mtdna.title=Linaje mtDNA sample.pathway.noCall=No hay haplogrupo asignado para esta muestra. sample.pathway.unplaced=El haplogrupo asignado aún no está ubicado en el árbol. +sample.pathway.fromTree=De la ubicación en el árbol de-novo sample.recon.consensus=Consenso entre tecnologías sample.recon.runs=ejecuciones sample.recon.confidence=confianza diff --git a/rust/locales/fr.txt b/rust/locales/fr.txt index 21e525c1..4f269e95 100644 --- a/rust/locales/fr.txt +++ b/rust/locales/fr.txt @@ -476,6 +476,7 @@ sample.ydna.title=Lignée Y-DNA sample.mtdna.title=Lignée mtDNA sample.pathway.noCall=Aucun haplogroupe attribué pour cet échantillon. sample.pathway.unplaced=L'haplogroupe attribué n'est pas encore placé dans l'arbre. +sample.pathway.fromTree=Depuis le placement dans l'arbre de-novo sample.recon.consensus=Consensus inter-technologies sample.recon.runs=analyses sample.recon.confidence=confiance From b4ef28605342676a555004a2ddd1ef37f42b9db0 Mon Sep 17 00:00:00 2001 From: James Kane Date: Thu, 2 Jul 2026 17:53:49 -0500 Subject: [PATCH 12/12] feat(home): add mtDNA tree CTA + ecosystem/participation sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The landing page had a single Y-DNA tree button and no explanation of what Decoding Us is. Add: - an mtDNA tree CTA alongside the Y-DNA one (both now icon buttons), keeping the variant-search button; - a "What is Decoding Us?" section — three cards (open haplogroup trees, public variant catalog, private discovery & research); - a "How to participate" section — explore / contribute your DNA / own your data / collaborate, with a link to the About page. Renamed home.cta.tree → home.cta.ytree and added home.cta.mtree + the eco.*/participate.* strings across en/es/fr (25 home.* keys, at parity). Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/crates/du-web/templates/index.html | 91 ++++++++++++++++++++++++- rust/locales/en.txt | 22 +++++- rust/locales/es.txt | 22 +++++- rust/locales/fr.txt | 22 +++++- 4 files changed, 152 insertions(+), 5 deletions(-) diff --git a/rust/crates/du-web/templates/index.html b/rust/crates/du-web/templates/index.html index 73b37666..3129e106 100644 --- a/rust/crates/du-web/templates/index.html +++ b/rust/crates/du-web/templates/index.html @@ -5,8 +5,95 @@

{{ t.get("home.heading") }}

{{ t.get("home.lead") }}

- {{ t.get("home.cta.tree") }} - {{ t.get("home.cta.variants") }} + {{ t.get("home.cta.ytree") }} + {{ t.get("home.cta.mtree") }} + {{ t.get("home.cta.variants") }}
+ +{# ── What is Decoding Us? (the ecosystem) ───────────────────────────────── #} +
+
+

{{ t.get("home.eco.title") }}

+

{{ t.get("home.eco.intro") }}

+
+
+
+
+
+

{{ t.get("home.eco.trees.title") }}

+

{{ t.get("home.eco.trees.body") }}

+
+
+
+
+
+
+
+

{{ t.get("home.eco.catalog.title") }}

+

{{ t.get("home.eco.catalog.body") }}

+
+
+
+
+
+
+
+

{{ t.get("home.eco.discovery.title") }}

+

{{ t.get("home.eco.discovery.body") }}

+
+
+
+
+
+
+ +{# ── How to participate ─────────────────────────────────────────────────── #} +
+
+

{{ t.get("home.participate.title") }}

+

{{ t.get("home.participate.intro") }}

+
+
+
+
+
+

{{ t.get("home.participate.explore.title") }}

+

{{ t.get("home.participate.explore.body") }}

+
+
+
+
+
+
+
+

{{ t.get("home.participate.contribute.title") }}

+

{{ t.get("home.participate.contribute.body") }}

+
+
+
+
+
+
+
+

{{ t.get("home.participate.own.title") }}

+

{{ t.get("home.participate.own.body") }}

+
+
+
+
+
+
+
+

{{ t.get("home.participate.collab.title") }}

+

{{ t.get("home.participate.collab.body") }}

+
+
+
+
+
+ {{ t.get("home.participate.learn") }} +
+
+
{% endblock %} diff --git a/rust/locales/en.txt b/rust/locales/en.txt index d9173d69..c3f9fce5 100644 --- a/rust/locales/en.txt +++ b/rust/locales/en.txt @@ -70,8 +70,28 @@ lang.fr=Français home.title=Decoding Us — genetic genealogy & population research home.heading=Decoding Us home.lead=A collaborative platform for genetic genealogy and population research — Y/mtDNA haplogroup trees, a public variant browser, and privacy-preserving relative discovery. -home.cta.tree=Browse the Y-DNA tree +home.cta.ytree=Browse the Y-DNA tree +home.cta.mtree=Browse the mtDNA tree home.cta.variants=Search variants +home.eco.title=What is Decoding Us? +home.eco.intro=Decoding Us is an open, collaborative platform for genetic genealogy and population research. You own your genetic data; the shared trees and catalogs are built openly from what the community contributes. +home.eco.trees.title=Open haplogroup trees +home.eco.trees.body=Y-DNA and mtDNA phylogenies rebuilt de novo from real genotypes — not curated by hand — so every branch is anchored in the sequences that define it. +home.eco.catalog.title=Public variant catalog +home.eco.catalog.body=A browsable catalog of Y and mitochondrial variants and the reference samples that anchor them, cross-referenced to the trees and the literature. +home.eco.discovery.title=Private discovery & research +home.eco.discovery.body=Find relatives and collaborate on research with privacy-preserving matching — no genotypes leave your control, and every share is by consent. +home.participate.title=How to participate +home.participate.intro=Whether you're just curious or ready to contribute your own results, there's a way in. +home.participate.explore.title=Explore +home.participate.explore.body=Browse the Y-DNA and mtDNA trees, search the variant catalog, and read the reference samples — all open, no account needed. +home.participate.contribute.title=Contribute your DNA +home.participate.contribute.body=Run your whole-genome or Y/mt sequencing data through the Navigator client to place yourself in the trees and help refine them for everyone. +home.participate.own.title=Own your data +home.participate.own.body=Your results live in your own AT Protocol repository (PDS). You decide what is shared, with whom, and can withdraw it at any time. +home.participate.collab.title=Collaborate +home.participate.collab.body=Join research projects, compare notes with the community, and discover genetic relatives — always on your terms. +home.participate.learn=Learn more about Decoding Us variants.title=Variant Browser variants.search.placeholder=Search by name or alias (e.g. M269, rs9786153)… diff --git a/rust/locales/es.txt b/rust/locales/es.txt index abeab010..01829327 100644 --- a/rust/locales/es.txt +++ b/rust/locales/es.txt @@ -22,8 +22,28 @@ lang.fr=Français home.title=Decoding Us — genealogía genética e investigación de poblaciones home.heading=Decoding Us home.lead=Una plataforma colaborativa para la genealogía genética y la investigación de poblaciones: árboles de haplogrupos Y/ADNmt, un explorador público de variantes y descubrimiento de parientes que preserva la privacidad. -home.cta.tree=Explorar el árbol Y-ADN +home.cta.ytree=Explorar el árbol Y-ADN +home.cta.mtree=Explorar el árbol de ADNmt home.cta.variants=Buscar variantes +home.eco.title=¿Qué es Decoding Us? +home.eco.intro=Decoding Us es una plataforma abierta y colaborativa para la genealogía genética y la investigación de poblaciones. Tú eres dueño de tus datos genéticos; los árboles y catálogos compartidos se construyen abiertamente a partir de lo que aporta la comunidad. +home.eco.trees.title=Árboles de haplogrupos abiertos +home.eco.trees.body=Filogenias de Y-ADN y ADNmt reconstruidas de novo a partir de genotipos reales —no curadas a mano— para que cada rama se ancle en las secuencias que la definen. +home.eco.catalog.title=Catálogo público de variantes +home.eco.catalog.body=Un catálogo navegable de variantes del cromosoma Y y mitocondriales y las muestras de referencia que las anclan, cruzadas con los árboles y la literatura. +home.eco.discovery.title=Descubrimiento privado e investigación +home.eco.discovery.body=Encuentra parientes y colabora en investigación con emparejamiento que preserva la privacidad: ningún genotipo sale de tu control y cada intercambio es con consentimiento. +home.participate.title=Cómo participar +home.participate.intro=Ya sea que solo tengas curiosidad o estés listo para aportar tus propios resultados, hay una forma de entrar. +home.participate.explore.title=Explorar +home.participate.explore.body=Explora los árboles de Y-ADN y ADNmt, busca en el catálogo de variantes y consulta las muestras de referencia: todo abierto, sin necesidad de cuenta. +home.participate.contribute.title=Aporta tu ADN +home.participate.contribute.body=Procesa tus datos de secuenciación de genoma completo o de Y/ADNmt con el cliente Navigator para ubicarte en los árboles y ayudar a refinarlos para todos. +home.participate.own.title=Sé dueño de tus datos +home.participate.own.body=Tus resultados residen en tu propio repositorio de AT Protocol (PDS). Tú decides qué se comparte, con quién, y puedes retirarlo en cualquier momento. +home.participate.collab.title=Colabora +home.participate.collab.body=Únete a proyectos de investigación, comparte impresiones con la comunidad y descubre parientes genéticos, siempre en tus términos. +home.participate.learn=Más información sobre Decoding Us variants.title=Explorador de variantes variants.search.placeholder=Buscar por nombre o alias (p. ej. M269, rs9786153)… diff --git a/rust/locales/fr.txt b/rust/locales/fr.txt index 4f269e95..cc405ce8 100644 --- a/rust/locales/fr.txt +++ b/rust/locales/fr.txt @@ -22,8 +22,28 @@ lang.fr=Français home.title=Decoding Us — généalogie génétique et recherche sur les populations home.heading=Decoding Us home.lead=Une plateforme collaborative pour la généalogie génétique et la recherche sur les populations : arbres d’haplogroupes Y/ADNmt, un explorateur public de variants et une découverte de parents respectueuse de la vie privée. -home.cta.tree=Explorer l’arbre Y-ADN +home.cta.ytree=Explorer l’arbre Y-ADN +home.cta.mtree=Explorer l’arbre ADNmt home.cta.variants=Rechercher des variants +home.eco.title=Qu’est-ce que Decoding Us ? +home.eco.intro=Decoding Us est une plateforme ouverte et collaborative dédiée à la généalogie génétique et à la recherche sur les populations. Vous êtes propriétaire de vos données génétiques ; les arbres et catalogues partagés sont construits ouvertement à partir des contributions de la communauté. +home.eco.trees.title=Arbres d’haplogroupes ouverts +home.eco.trees.body=Des phylogénies Y-ADN et ADNmt reconstruites de novo à partir de génotypes réels — non curées à la main — pour que chaque branche soit ancrée dans les séquences qui la définissent. +home.eco.catalog.title=Catalogue public de variants +home.eco.catalog.body=Un catalogue navigable des variants du chromosome Y et mitochondriaux et des échantillons de référence qui les ancrent, recoupés avec les arbres et la littérature. +home.eco.discovery.title=Découverte privée et recherche +home.eco.discovery.body=Trouvez des parents et collaborez à la recherche grâce à un appariement respectueux de la vie privée : aucun génotype ne quitte votre contrôle et chaque partage se fait avec consentement. +home.participate.title=Comment participer +home.participate.intro=Que vous soyez simplement curieux ou prêt à apporter vos propres résultats, il existe une porte d’entrée. +home.participate.explore.title=Explorer +home.participate.explore.body=Parcourez les arbres Y-ADN et ADNmt, cherchez dans le catalogue de variants et consultez les échantillons de référence — tout est ouvert, sans compte. +home.participate.contribute.title=Contribuez votre ADN +home.participate.contribute.body=Analysez vos données de séquençage du génome entier ou Y/ADNmt avec le client Navigator pour vous placer dans les arbres et aider à les affiner pour tous. +home.participate.own.title=Maîtrisez vos données +home.participate.own.body=Vos résultats résident dans votre propre dépôt AT Protocol (PDS). Vous décidez de ce qui est partagé, avec qui, et pouvez le retirer à tout moment. +home.participate.collab.title=Collaborez +home.participate.collab.body=Rejoignez des projets de recherche, échangez avec la communauté et découvrez des parents génétiques — toujours selon vos conditions. +home.participate.learn=En savoir plus sur Decoding Us variants.title=Explorateur de variants variants.search.placeholder=Rechercher par nom ou alias (p. ex. M269, rs9786153)…