From dcd4484c622b446689bc5f2ba32f0513df1cd5b3 Mon Sep 17 00:00:00 2001 From: James Kane Date: Tue, 25 Aug 2026 14:22:28 -0500 Subject: [PATCH 1/2] fix(grid): two empty digests must not form a quorum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A digest with no comparable call agrees with every other such digest — every field is absent on both sides. `a_shapeless_digest_agrees_with_nothing_real` already asserted exactly that and called it "equally uninformative", and I did not follow the thought through: two nodes whose analysis failed would agree, canonicalize a unit with no content, and be credited for it. `digest::has_content` now answers whether a digest is about anything, and the validator refuses to canonicalize when the winning cluster is empty. One discrete call is enough. The Navigator side is being fixed in the same pass — a node now fails its unit instead of submitting an empty digest. This guard is here anyway, because a node is untrusted by construction and that is the entire premise adaptive replication rests on. A check that only runs on the honest side is not a check. Found by a review of the Navigator branch, which traced the consequence back across the repository boundary. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SequryftjcFkLnjQRMKCx3 --- rust/crates/du-db/src/grid/digest.rs | 37 ++++++++++++++++++++++++ rust/crates/du-jobs/src/grid_validate.rs | 14 +++++++++ 2 files changed, 51 insertions(+) diff --git a/rust/crates/du-db/src/grid/digest.rs b/rust/crates/du-db/src/grid/digest.rs index 3dbcdbb..2c12836 100644 --- a/rust/crates/du-db/src/grid/digest.rs +++ b/rust/crates/du-db/src/grid/digest.rs @@ -84,6 +84,30 @@ impl Comparable { } } +impl Comparable { + /// Whether this digest carries **no** comparable call at all. + /// + /// Two such digests agree with each other, because every field is absent on both sides. That is + /// correct as a comparison and disastrous as a quorum: two nodes whose analysis failed would + /// agree on nothing, canonicalize a unit with no content, and be paid for it. + /// + /// So validation refuses to canonicalize on an empty digest. The node is expected to fail its + /// unit rather than submit one, but the AppView cannot rely on that — a node is untrusted by + /// construction, and that is the whole premise of adaptive replication. + pub fn is_empty(&self) -> bool { + self.sex.is_none() + && self.y_terminal.is_none() + && self.ancestry_superpop_argmax.is_none() + && self.coverage_bucket.is_none() + && self.callable_bucket.is_none() + } +} + +/// Whether a digest carries anything a quorum could be about. +pub fn has_content(digest: &Value) -> bool { + !Comparable::from_digest(digest).is_empty() +} + /// The major component of a semver-ish stack version: `"1.7.0"` → `"1"`. /// /// Only submissions from a compatible major are compared. A minor release that refactors a walker @@ -203,6 +227,19 @@ mod tests { ); } + /// …and "equally uninformative" is exactly why agreement is not enough on its own. Two nodes + /// whose analysis failed submit two empty digests, which agree. Without this check they would + /// canonicalize a unit with no content and be credited for it. + #[test] + fn an_empty_digest_has_no_content_to_agree_about() { + assert!(!has_content(&json!({"unexpected": true}))); + assert!(!has_content(&json!(null))); + assert!(!has_content(&json!({"calls": {}}))); + assert!(has_content(&d("XY", "R-A", "EUR", 30.0, 0.94))); + // One discrete call is enough to be about something. + assert!(has_content(&json!({"calls": {"sex": "XX"}}))); + } + /// The cross-repo byte contract: key order in the source JSON must not change the hash, or a /// node and the AppView would disagree about what was signed. #[test] diff --git a/rust/crates/du-jobs/src/grid_validate.rs b/rust/crates/du-jobs/src/grid_validate.rs index 894cfe9..7e86616 100644 --- a/rust/crates/du-jobs/src/grid_validate.rs +++ b/rust/crates/du-jobs/src/grid_validate.rs @@ -141,6 +141,20 @@ pub async fn validate(pool: &PgPool, spot_check_rate: f64) -> anyhow::Result = winner.iter().map(|&i| subs[i].did.as_str()).collect(); From 90a56abf25f720b2d3a98e948eb3412278f58f29 Mon Sep 17 00:00:00 2001 From: James Kane Date: Tue, 25 Aug 2026 16:39:19 -0500 Subject: [PATCH 2/2] feat(grid): carry the instrument on the curation manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The node cannot pick a mapper preset without it. A read set with no mate is either a long read or a single-end short read, and those need different presets — a long read mapped under a short-read preset does not fail, it produces alignments that look right and are wrong. `crawl_project` already stores ENA's instrument model on the library, so this only publishes a value we hold. The Navigator side then feeds it to `Preset::infer`, which errors rather than guesses on an instrument it does not recognise. Found by the third review of the Navigator branch: the node was choosing its preset from the mate count, so every single-end Illumina run would have been mapped as HiFi. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SequryftjcFkLnjQRMKCx3 --- rust/crates/du-db/src/grid/mod.rs | 9 ++++++++- rust/crates/du-db/tests/grid.rs | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/rust/crates/du-db/src/grid/mod.rs b/rust/crates/du-db/src/grid/mod.rs index 5860bf1..6a61888 100644 --- a/rust/crates/du-db/src/grid/mod.rs +++ b/rust/crates/du-db/src/grid/mod.rs @@ -419,6 +419,12 @@ pub struct CurationCandidate { /// With `only_new`, samples that already have a work unit are skipped. That is the nightly path. /// Passing `false` re-projects everything, which refreshes manifests after a re-crawl. /// +/// **The manifest carries the instrument**, because the node can not choose a mapper without it. +/// A read set with no mate can be a long read or a single-end short read, and those two need +/// different presets. A long read mapped under a short-read preset does not fail — it gives +/// alignments that look correct and are wrong. `crawl_project` already stores the instrument model +/// of ENA on the library, so this only carries a value that we hold. +/// /// **`est_bases` prefers the measured `base_count`** that ENA publishes on `read_run`, and falls /// back to `reads × read_length` where a row predates that column. The fallback is only ever a /// mean-length approximation and is wrong outright for variable-length long reads, so it is a @@ -460,7 +466,8 @@ pub async fn curation_candidates( 'index_url', sf.http_locations->0->>'file_index_url', \ 'md5', sf.checksums->0->>'checksum', \ 'bytes', sf.file_size_bytes, \ - 'format', sf.file_format \ + 'format', sf.file_format, \ + 'instrument', sl.instrument \ )) ORDER BY sl.id, sf.id) AS manifest, \ ( SELECT SUM(COALESCE(l2.base_count, l2.reads::bigint * l2.read_length::bigint))::bigint \ FROM genomics.sequence_library l2 WHERE l2.sample_guid = s.sample_guid ) AS est_bases, \ diff --git a/rust/crates/du-db/tests/grid.rs b/rust/crates/du-db/tests/grid.rs index 7011c43..cd2dff7 100644 --- a/rust/crates/du-db/tests/grid.rs +++ b/rust/crates/du-db/tests/grid.rs @@ -570,6 +570,10 @@ async fn curation_projects_crawled_samples_into_work_units() { "ftp.sra.ebi.ac.uk/vol1/run/ERR200/s1.cram.crai" ); assert_eq!(cram.manifest[0]["md5"], "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + assert_eq!( + cram.manifest[0]["instrument"], "Illumina NovaSeq 6000", + "the node needs the instrument to choose a mapper preset" + ); assert_eq!(cram.total_bytes, Some(12_000_000_000)); assert_eq!( cram.est_bases,