Stable sample identity across search outputs

docs/development.md

@@ -60,6 +60,17 @@ shared CLI/viewer plan report and Parquet scoring-plan metadata. Other elements
 are ignored by this approximation; isotope-labelled C/S and unspecified formula
 bases cannot supply its composition counts.
 
+## Sample identity and output paths
+
+Search outputs use `<output>/<sample_id>/`, not the basename alone.
+IDs combine a parent-location hash with the sample name, distinguishing same-named
+inputs in different directories without hashing file contents.
+`sample_id` and `sample_name` are recorded in run/performance reports and Parquet
+file metadata, not repeated per row. Read these fields rather than parsing paths.
+
+Duplicate IDs within one invocation are rejected; rerunning with `--overwrite`
+still replaces prior outputs. Existing basename-only output directories are not migrated.
+
 ## Cargo features
 
 | Feature | Crate | Effect | Use case | Enable |

rust/tims_stage/src/load.rs

@@ -1,9 +1,9 @@
 //! The single raw-index composition: URI → `IndexedTimstofPeaks`.
 //!
-//! `load_raw` is the one place raw dispatch happens. It sniffs the URI via the
-//! reader registry, asks the chosen reader for its [`Manifest`] (the files it
-//! needs), materializes those -- in place for local inputs, or by fetching
-//! exactly the declared files for remote ones -- and calls `read`. Transport
+//! `PreparedSource` retains reader selection from preflight; `load_raw` selects
+//! on demand for callers without preflight. Both ask that reader for its
+//! [`Manifest`], materialize it -- in place for local inputs, or by fetching
+//! exactly the declared files for remote ones -- and call `read`. Transport
 //! never guesses vendor shape: it fetches what the reader declared, by name.
 //!
 //! Lives in `tims_stage` (one crate above `timscentroid`, where the registry
@@ -12,10 +12,13 @@
 
 use std::ffi::OsString;
 use std::path::Path;
+use std::sync::Arc;
 
 use http::Uri;
 use timscentroid::reader::{
+    BrukerTdfReader,
     Manifest,
+    RawReader,
     ReadError,
     ReaderRegistry,
     ResolvedSource,
@@ -54,6 +57,112 @@ pub enum LoadRawError {
     Stage(#[from] StageError),
 }
 
+/// Reader selection retained from preflight through naming and index loading.
+/// Standalone indexes may have no raw reader; tar transport contains Bruker .d.
+#[derive(Clone)]
+pub struct PreparedSource {
+    uri: String,
+    sample_name: String,
+    reader: Option<Arc<dyn RawReader>>,
+}
+
+impl std::fmt::Debug for PreparedSource {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("PreparedSource")
+            .field("uri", &self.uri)
+            .field("sample_name", &self.sample_name)
+            .field("reader", &self.reader.as_ref().map(|r| r.name()))
+            .finish()
+    }
+}
+
+impl PreparedSource {
+    pub fn new(uri: &str) -> Result<Self, ReadError> {
+        let uri = crate::canonical_uri(uri);
+        let location = uri.trim_end_matches('/');
+        let mut entry = location;
+        let mut wrapped = false;
+        let tar = location.to_ascii_lowercase().ends_with(".tar");
+        let idx = location.to_ascii_lowercase().ends_with(".idx");
+        loop {
+            let lower = entry.to_ascii_lowercase();
+            if lower.ends_with(".idx") || lower.ends_with(".tar") {
+                entry = &entry[..entry.len() - 4];
+                wrapped = true;
+            } else {
+                break;
+            }
+        }
+        let parsed = raw_uri(entry)?;
+        let reader = if tar {
+            // The existing tar loader is specifically a .d transport, not a
+            // second vendor-dispatch mechanism inferred from the archive name.
+            Some(Arc::new(BrukerTdfReader) as Arc<dyn RawReader>)
+        } else {
+            match ReaderRegistry::with_builtins().pick(&parsed, || None) {
+                Ok(reader) => Some(reader),
+                Err(ReadError::UnknownFormat(_)) if idx => None,
+                Err(error) => return Err(error),
+            }
+        };
+        Self::with_reader(&uri, entry, wrapped, reader)
+    }
+
+    fn with_reader(
+        uri: &str,
+        entry: &str,
+        wrapped: bool,
+        reader: Option<Arc<dyn RawReader>>,
+    ) -> Result<Self, ReadError> {
+        let name = entry.rsplit('/').next().unwrap_or(entry);
+        let sample_name = reader
+            .as_ref()
+            .and_then(|r| r.sample_name(name))
+            .or_else(|| wrapped.then_some(name))
+            .ok_or_else(|| ReadError::Build(format!("selected reader cannot name {uri:?}")))?
+            .to_owned();
+        Ok(Self {
+            uri: uri.to_owned(),
+            sample_name,
+            reader,
+        })
+    }
+
+    pub fn uri(&self) -> &str {
+        &self.uri
+    }
+
+    pub fn sample_name(&self) -> &str {
+        &self.sample_name
+    }
+
+    /// Read the raw artifact (or the .d extracted by tar staging) using the
+    /// same reader which supplied its name. No registry dispatch here.
+    pub fn read_raw(
+        &self,
+        uri: &str,
+        backend: &dyn StagingBackend,
+        cfg: &IndexingCentroidingConfig,
+    ) -> Result<RawRead, LoadRawError> {
+        let reader = self
+            .reader
+            .as_ref()
+            .ok_or_else(|| ReadError::Build("cached index has no raw reader".into()))?;
+        load_with_reader(uri, reader.as_ref(), backend, cfg)
+    }
+}
+
+fn raw_uri(uri: &str) -> Result<Uri, ReadError> {
+    if is_remote_uri(uri) {
+        uri.parse().map_err(|source| ReadError::UriParse {
+            uri: uri.to_owned(),
+            source,
+        })
+    } else {
+        local_uri(&std::path::absolute(uri)?)
+    }
+}
+
 /// Build an index from a raw URI (local path or `s3://…`), dispatching through
 /// the reader registry. Remote inputs stage exactly the reader's declared
 /// manifest into a tempdir (kept alive across the read); local inputs are read
@@ -63,16 +172,24 @@ pub fn load_raw(
     backend: &dyn StagingBackend,
     cfg: &IndexingCentroidingConfig,
 ) -> Result<RawRead, LoadRawError> {
-    let registry = ReaderRegistry::with_builtins();
+    let parsed = raw_uri(&crate::canonical_uri(uri))?;
+    let reader = ReaderRegistry::with_builtins().pick(&parsed, || None)?;
+    load_with_reader(uri, reader.as_ref(), backend, cfg)
+}
 
+fn load_with_reader(
+    uri: &str,
+    reader: &dyn RawReader,
+    backend: &dyn StagingBackend,
+    cfg: &IndexingCentroidingConfig,
+) -> Result<RawRead, LoadRawError> {
     if is_remote_uri(uri) {
         let parsed = uri.parse::<Uri>().map_err(|source| ReadError::UriParse {
             uri: uri.to_string(),
             source,
         })?;
         // No reader returns `Maybe` yet, so the magic-byte peek is never needed;
         // remote byte-sniffing (a range-GET here) is future work.
-        let reader = registry.pick(&parsed, || None)?;
         let manifest = reader.manifest(&parsed);
         // `staged` owns the tempdir and must outlive the read below.
         let staged = stage_manifest(backend, &manifest)?;
@@ -84,14 +201,9 @@ pub fn load_raw(
         });
     }
 
-    // Local: canonicalize first so RELATIVE paths resolve -- `local_uri`
-    // (sniff) and `local_in_place` (read) both require an absolute path. This
-    // is the single place all entry points funnel through, so relative inputs
-    // work uniformly (`read_index`, `load_index_auto`, the pyo3 binding, …).
+    // Resolve the actual local artifact without reselecting its reader.
     let abs = std::fs::canonicalize(uri)
         .map_err(|e| ReadError::Build(format!("cannot resolve local raw path {uri:?}: {e}")))?;
-    let parsed = local_uri(&abs)?;
-    let reader = registry.pick(&parsed, || None)?;
     let src = ResolvedSource::local_in_place(&abs)?;
     let index = reader.read(&src, cfg)?;
     Ok(RawRead {
@@ -214,6 +326,68 @@ mod tests {
     };
     use timscentroid::reader::Manifest;
 
+    #[test]
+    fn prepared_names_unwrap_transport() {
+        for name in [
+            "My-Run.D",
+            "My-Run.d.IDX",
+            "My-Run.D.TAR",
+            "My-Run.idx",
+            "My-Run.tar",
+        ] {
+            assert_eq!(PreparedSource::new(name).unwrap().sample_name(), "My-Run");
+        }
+        assert_eq!(
+            PreparedSource::new("run.raw.idx").unwrap().sample_name(),
+            "run.raw"
+        );
+        assert!(PreparedSource::new("run.raw").is_err());
+        assert!(PreparedSource::new("run.mzML.gz").is_err());
+    }
+
+    #[test]
+    fn prepared_source_names_and_reads_with_retained_reader() {
+        struct SelectedReader;
+        impl RawReader for SelectedReader {
+            fn name(&self) -> &'static str {
+                "selected"
+            }
+
+            fn sniff(&self, _: &Uri) -> timscentroid::reader::Sniff {
+                panic!("must not redispatch after selection")
+            }
+
+            fn sample_name<'a>(&self, name: &'a str) -> Option<&'a str> {
+                name.strip_suffix(".custom")
+            }
+
+            fn manifest(&self, _: &Uri) -> Manifest {
+                unreachable!("local input")
+            }
+
+            fn read(
+                &self,
+                src: &ResolvedSource,
+                _: &IndexingCentroidingConfig,
+            ) -> Result<IndexedTimstofPeaks, ReadError> {
+                assert_eq!(src.entry_path().file_name().unwrap(), "run.custom");
+                Err(ReadError::Build("retained reader called".into()))
+            }
+        }
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("run.custom");
+        std::fs::write(&path, []).unwrap();
+        let uri = path.to_str().unwrap();
+        let source =
+            PreparedSource::with_reader(uri, uri, false, Some(Arc::new(SelectedReader))).unwrap();
+        assert_eq!(source.sample_name(), "run");
+        let backend = PerRunTempdir::new(StagingConfig::default()).unwrap();
+        assert!(
+            matches!(source.read_raw(uri, &backend, &IndexingCentroidingConfig::default()),
+            Err(LoadRawError::Read(ReadError::Build(message))) if message == "retained reader called")
+        );
+    }
+
     fn u(p: &std::path::Path) -> Uri {
         // Raw path URI (tempdir paths are space-free, so no percent-encoding).
         p.to_str().unwrap().parse::<Uri>().unwrap()

rust/tims_stage/src/uri.rs

@@ -39,9 +39,10 @@ pub(crate) fn parse_uri_shape(uri: &str) -> Result<UriShape, StageError> {
     } else {
         LocKind::Local
     };
-    let name = if trimmed.ends_with(".idx") {
+    let lower = trimmed.to_ascii_lowercase();
+    let name = if lower.ends_with(".idx") {
         NameKind::Idx
-    } else if trimmed.ends_with(".tar") {
+    } else if lower.ends_with(".tar") {
         NameKind::Tar
     } else {
         // Any other suffix is a raw vendor artifact; the reader registry
@@ -144,6 +145,18 @@ pub fn canonical_uri(uri: &str) -> String {
 mod tests {
     use super::*;
 
+    #[test]
+    fn transport_suffixes_are_case_insensitive() {
+        assert_eq!(
+            parse_uri_shape("/data/run.D.IDX").unwrap().name,
+            NameKind::Idx
+        );
+        assert_eq!(
+            parse_uri_shape("/data/run.D.TAR").unwrap().name,
+            NameKind::Tar
+        );
+    }
+
     #[test]
     fn recognizes_local_dotd() {
         let s = parse_uri_shape("/tmp/sample.d").unwrap();