Tidy up (#107)

* Add .claude/ to .gitignore

* cargo fmt

* Fix test-python: use maturin develop so tests see current Rust code

maturin build produces a wheel but does not install it into the active
venv, so tests were running against whatever was last manually installed.
Use maturin develop instead, which builds and installs into the active
venv. Requires sourcing bindings/python/.venv/bin/activate before just ci.

* Add Code newtype to types module

Newtype wraps String with compile-time distinction only — no content
validation at this layer. serde(transparent) keeps the wire format as
a plain JSON string. Canonical conversions via From<String>, From<&str>,
and From<Code> for String.

* Add NormalizedCode, Term, CodeSystemId, Contributor, CodeListId newtypes

* Migrate Provenance.contributors to IndexSet<Contributor>

CodeListError::ContributorNotFound now carries a Contributor (not
String), so remove_contributor takes Contributor by value and moves
it into the error on the not-found path — no as_str().to_string()
dance.

Python binding's add_contributor, remove_contributor, and the
Provenance::new call site in PyCodeList::new wrap incoming strings
with Contributor::from at the FFI boundary. Python-facing signatures
remain String; the binding exposes contributors as native strings
via as_str().

Author: CarolineMorton

.gitignore

@@ -26,6 +26,9 @@ Cargo.lock
 .DS_Store
 todo.txt
 
+# Claude Code
+.claude/
+
 ## Python ##
 __pycache__/
 venv/

Justfile

@@ -21,11 +21,12 @@ prettier-check:
 # CI task: check formatting and linting and all tests
 ci: fmt-check clippy prettier-check test-python test-rust
 
-# Run python tests
+# Run python tests.
+# Requires an active Python venv (run `source bindings/python/.venv/bin/activate`
+# first). `maturin develop` builds the extension and installs it into the
+# active venv so the tests exercise the current Rust code, not a stale wheel.
 test-python:
-    echo "Build python library"
-    maturin build --manifest-path bindings/python/Cargo.toml
-    echo "Run python tests"
+    maturin develop --manifest-path bindings/python/Cargo.toml
     sh bindings/python/tests/run.sh
 
 # Run rust tests

bindings/python/src/codelist.rs

@@ -10,7 +10,7 @@ use codelist_rs::{
         CategorisationAndUsage, Metadata, Provenance, PurposeAndContext, Source,
         ValidationAndReview,
     },
-    types::CodeListType,
+    types::{CodeListType, Contributor},
 };
 use codelist_validator_rs::validator::Validator;
 use indexmap::IndexSet;
@@ -63,7 +63,9 @@ impl PyCodeList {
         })?;
         // convert authors vec to IndexSet
         let authors_set = authors
-            .map(|authors| authors.into_iter().collect::<IndexSet<String>>())
+            .map(|authors| {
+                authors.into_iter().map(Contributor::from).collect::<IndexSet<Contributor>>()
+            })
             .unwrap_or_default();
         let provenance = Provenance::new(source, Some(authors_set));
         let categorisation_and_usage = CategorisationAndUsage::new(None, None, None);
@@ -114,7 +116,7 @@ impl PyCodeList {
 
     /// Add a contributor to the codelist's provenance
     fn add_contributor(&mut self, contributor: String) -> PyResult<()> {
-        self.inner.metadata.provenance.add_contributor(contributor);
+        self.inner.metadata.provenance.add_contributor(Contributor::from(contributor));
         Ok(())
     }
 
@@ -123,7 +125,7 @@ impl PyCodeList {
         self.inner
             .metadata
             .provenance
-            .remove_contributor(contributor)
+            .remove_contributor(Contributor::from(contributor))
             .map_err(|e| PyValueError::new_err(e.to_string()))?;
         Ok(())
     }
@@ -132,7 +134,7 @@ impl PyCodeList {
     fn contributors(&self, py: Python) -> PyResult<PyObject> {
         let py_set = PySet::new(py, &[] as &[String])?;
         for contributor in &self.inner.metadata.provenance.contributors {
-            py_set.add(contributor)?;
+            py_set.add(contributor.as_str())?;
         }
         Ok(py_set.into())
     }

rust/codelist-builder-rs/src/snomed_usage_data.rs

@@ -4,47 +4,65 @@
 //! Components of the file:
 //!
 //! SNOMED_Concept_ID:
-//! SNOMED concepts which have been added to a patient record in a general practice system during the reporting period.
+//! SNOMED concepts which have been added to a patient record in a general
+//! practice system during the reporting period.
 //!
 //! Description:
-//! The fully specified name associated with the SNOMED_Concept_ID on the final day of the reporting period (31 July).
+//! The fully specified name associated with the SNOMED_Concept_ID on the final
+//! day of the reporting period (31 July).
 //!
 //! Usage:
-//! The number of times that the SNOMED_Concept_ID was added into any patient record within the reporting period, rounded to the nearerst 10. Usage of 1 to 4 is displayed as *. SNOMED concepts with no code usage are not included.
+//! The number of times that the SNOMED_Concept_ID was added into any patient
+//! record within the reporting period, rounded to the nearerst 10. Usage of 1
+//! to 4 is displayed as *. SNOMED concepts with no code usage are not included.
 //! Important notes:
-//! - Data prior to 2019 was originally submitted mostly in READ V2 or CTV3, but in the usage files, these codes have been mapped to corresponding SNOMED codes using final 2020 version of the mapping tables published by NHS England.
-//! - The usage does not show how many patients had each code added to their record - each addition regardless of whether it is the same patient increments the count by 1. Therefore it is not possible to infer the number of individual patients with a particular code.
-//! - For the 2011-12 to 2017-18 data, it is stated that "Current maximum value is approximately 250,000,000" - no such maximum is stated for the 2018-19 onwards data.
+//! - Data prior to 2019 was originally submitted mostly in READ V2 or CTV3, but
+//!   in the usage files, these codes have been mapped to corresponding SNOMED
+//!   codes using final 2020 version of the mapping tables published by NHS
+//!   England.
+//! - The usage does not show how many patients had each code added to their
+//!   record - each addition regardless of whether it is the same patient
+//!   increments the count by 1. Therefore it is not possible to infer the
+//!   number of individual patients with a particular code.
+//! - For the 2011-12 to 2017-18 data, it is stated that "Current maximum value
+//!   is approximately 250,000,000" - no such maximum is stated for the 2018-19
+//!   onwards data.
 //!
 //! Active_at_Start:
-//! Active status of the SNOMED_Concept_ID on the first day of the reporting period. This is taken from the most recent UK clinical extension, or associated International extention, which was published up to the start of the reporting year (1 August).
-//! 1 = SNOMED concept was published and was active.
-//! 0 = SNOMED concept was either not yet available or was inactive.
+//! Active status of the SNOMED_Concept_ID on the first day of the reporting
+//! period. This is taken from the most recent UK clinical extension, or
+//! associated International extention, which was published up to the start of
+//! the reporting year (1 August). 1 = SNOMED concept was published and was
+//! active. 0 = SNOMED concept was either not yet available or was inactive.
 //!
 //! Active_at_End:
-//! Active status of the SNOMED_Concept_ID on the last day of the reporting period. This is taken from the most recent UK clinical extension, or associated International extention, which was published up to the end of the reporting year (31 July).
-//! 1 = SNOMED concept was published and was active.
+//! Active status of the SNOMED_Concept_ID on the last day of the reporting
+//! period. This is taken from the most recent UK clinical extension, or
+//! associated International extention, which was published up to the end of the
+//! reporting year (31 July). 1 = SNOMED concept was published and was active.
 //! 0 = SNOMED concept was either not yet available or was inactive.
 
-use std::fs;
-
-// Internal imports
-use crate::errors::CodeListBuilderError;
+use std::{collections::HashMap, fs};
 
 // External imports
 use csv;
 use reqwest;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;
+
+// Internal imports
+use crate::errors::CodeListBuilderError;
 
 /// Struct to represent a snomed usage data entry
 ///
 /// # Fields
 /// * `snomed_concept_id` - The snomed concept id
 /// * `description` - The description
-/// * `usage` - The usage. A count of 1-4 is denoted by a *. Counts above 4 are denoted by a number rounded to the nearest 10.
-/// * `active_at_start` - Whether the concept was active at the start of the usage period
-/// * `active_at_end` - Whether the concept was active at the end of the usage period
+/// * `usage` - The usage. A count of 1-4 is denoted by a *. Counts above 4 are
+///   denoted by a number rounded to the nearest 10.
+/// * `active_at_start` - Whether the concept was active at the start of the
+///   usage period
+/// * `active_at_end` - Whether the concept was active at the end of the usage
+///   period
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub struct SnomedUsageDataEntry {
     pub snomed_concept_id: String,

rust/codelist-builder-rs/tests/download_usage.rs

@@ -1,5 +1,4 @@
-use codelist_builder_rs::errors::CodeListBuilderError;
-use codelist_builder_rs::snomed_usage_data::SnomedUsageData;
+use codelist_builder_rs::{errors::CodeListBuilderError, snomed_usage_data::SnomedUsageData};
 
 #[tokio::test]
 async fn test_download_usage() -> Result<(), CodeListBuilderError> {

rust/codelist-rs/src/errors.rs

@@ -5,6 +5,8 @@ use std::io;
 use csv;
 use serde_json;
 
+use crate::types::Contributor;
+
 /// Enum to represent the different types of errors that can occur in the
 /// codelist library
 
@@ -53,7 +55,7 @@ pub enum CodeListError {
     CodeEntryTermDoesNotExist { code: String, msg: String },
 
     #[error("Contributor {contributor} not found")]
-    ContributorNotFound { contributor: String },
+    ContributorNotFound { contributor: Contributor },
 
     #[error("Invalid metadata source: {source_string}")]
     InvalidMetadataSource { source_string: String },

rust/codelist-rs/src/metadata/metadata.rs

@@ -58,7 +58,7 @@ mod tests {
     use indexmap::IndexSet;
 
     use super::*;
-    use crate::{errors::CodeListError, metadata::Source};
+    use crate::{errors::CodeListError, metadata::Source, types::Contributor};
 
     // helper function to get the time difference between the current time and the
     // given date
@@ -69,8 +69,10 @@ mod tests {
 
     #[test]
     fn test_new() -> Result<(), CodeListError> {
-        let provenance =
-            Provenance::new(Source::ManuallyCreated, Some(IndexSet::from(["Test".to_string()])));
+        let provenance = Provenance::new(
+            Source::ManuallyCreated,
+            Some(IndexSet::from([Contributor::from("Test")])),
+        );
         let categorisation_and_usage = CategorisationAndUsage::new(
             Some(HashSet::from(["tag1".to_string()])),
             Some(HashSet::from(["usage1".to_string()])),
@@ -100,7 +102,7 @@ mod tests {
         assert!(time_difference < 1000);
         let time_difference = get_time_difference(metadata.provenance.last_modified_date);
         assert!(time_difference < 1000);
-        assert_eq!(metadata.provenance.contributors, IndexSet::from(["Test".to_string()]));
+        assert_eq!(metadata.provenance.contributors, IndexSet::from([Contributor::from("Test")]));
 
         assert_eq!(metadata.categorisation_and_usage.tags, HashSet::from(["tag1".to_string()]));
         assert_eq!(metadata.categorisation_and_usage.usage, HashSet::from(["usage1".to_string()]));

rust/codelist-rs/src/metadata/provenance.rs

@@ -10,14 +10,14 @@ use serde::{Deserialize, Serialize};
 
 // Internal imports
 use crate::errors::CodeListError;
-use crate::metadata::metadata_source::Source;
+use crate::{metadata::metadata_source::Source, types::Contributor};
 
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct Provenance {
     pub source: Source,
     pub created_date: chrono::DateTime<Utc>,
     pub last_modified_date: chrono::DateTime<Utc>,
-    pub contributors: IndexSet<String>,
+    pub contributors: IndexSet<Contributor>,
 }
 
 impl Default for Provenance {
@@ -31,7 +31,7 @@ impl Provenance {
     ///
     /// # Arguments
     /// * `source` - The source of the codelist
-    pub fn new(source: Source, contributors: Option<IndexSet<String>>) -> Provenance {
+    pub fn new(source: Source, contributors: Option<IndexSet<Contributor>>) -> Provenance {
         Self {
             source,
             created_date: Utc::now(),
@@ -53,7 +53,7 @@ impl Provenance {
     /// # Arguments
     /// * `self` - The provenance to update
     /// * `contributor` - The contributor to add
-    pub fn add_contributor(&mut self, contributor: String) {
+    pub fn add_contributor(&mut self, contributor: Contributor) {
         self.contributors.insert(contributor);
     }
 
@@ -62,7 +62,7 @@ impl Provenance {
     /// # Arguments
     /// * `self` - The provenance to update
     /// * `contributor` - The contributor to remove
-    pub fn remove_contributor(&mut self, contributor: String) -> Result<(), CodeListError> {
+    pub fn remove_contributor(&mut self, contributor: Contributor) -> Result<(), CodeListError> {
         if self.contributors.shift_remove(&contributor) {
             Ok(())
         } else {
@@ -88,7 +88,7 @@ mod tests {
 
     fn create_test_provenance_with_contributors() -> Provenance {
         let mut contributors = IndexSet::new();
-        contributors.insert("Example Contributor".to_string());
+        contributors.insert(Contributor::from("Example Contributor"));
         Provenance::new(Source::LoadedFromFile, Some(contributors))
     }
 
@@ -100,14 +100,17 @@ mod tests {
         assert!(time_difference < 1000);
         let time_difference = get_time_difference(provenance.last_modified_date);
         assert!(time_difference < 1000);
-        assert_eq!(provenance.contributors, IndexSet::new());
+        assert_eq!(provenance.contributors, IndexSet::<Contributor>::new());
     }
 
     #[test]
     fn test_new_provenance_with_contributors() {
         let provenance = create_test_provenance_with_contributors();
         assert_eq!(provenance.source, Source::LoadedFromFile);
-        assert_eq!(provenance.contributors, IndexSet::from(["Example Contributor".to_string()]));
+        assert_eq!(
+            provenance.contributors,
+            IndexSet::from([Contributor::from("Example Contributor")])
+        );
         let time_difference = get_time_difference(provenance.created_date);
         assert!(time_difference < 1000);
         let time_difference = get_time_difference(provenance.last_modified_date);
@@ -125,23 +128,35 @@ mod tests {
     #[test]
     fn test_add_contributor() {
         let mut provenance = create_test_provenance_no_contributors();
-        provenance.add_contributor("Example Contributor".to_string());
-        assert_eq!(provenance.contributors, IndexSet::from(["Example Contributor".to_string()]));
+        provenance.add_contributor(Contributor::from("Example Contributor"));
+        assert_eq!(
+            provenance.contributors,
+            IndexSet::from([Contributor::from("Example Contributor")])
+        );
+    }
+
+    #[test]
+    fn add_contributor_uses_newtype() {
+        let mut p = create_test_provenance_no_contributors();
+        let c = Contributor::from("Caroline");
+        p.add_contributor(c.clone());
+        assert!(p.contributors.contains(&c));
     }
 
     #[test]
     fn test_remove_contributor() -> Result<(), CodeListError> {
         let mut provenance = create_test_provenance_with_contributors();
-        provenance.add_contributor("Example Contributor".to_string());
-        provenance.remove_contributor("Example Contributor".to_string())?;
-        assert_eq!(provenance.contributors, IndexSet::new());
+        provenance.add_contributor(Contributor::from("Example Contributor"));
+        provenance.remove_contributor(Contributor::from("Example Contributor"))?;
+        assert_eq!(provenance.contributors, IndexSet::<Contributor>::new());
         Ok(())
     }
 
     #[test]
     fn test_remove_contributor_not_found() {
         let mut provenance = create_test_provenance_no_contributors();
-        let error = provenance.remove_contributor("Example Contributor".to_string()).unwrap_err();
+        let error =
+            provenance.remove_contributor(Contributor::from("Example Contributor")).unwrap_err();
         let error_string = error.to_string();
         assert_eq!(error_string, "Contributor Example Contributor not found");
     }
@@ -150,35 +165,35 @@ mod tests {
     fn test_contributors_order_is_maintained() -> Result<(), CodeListError> {
         let mut provenance = create_test_provenance_no_contributors();
 
-        provenance.add_contributor("Example1".to_string());
+        provenance.add_contributor(Contributor::from("Example1"));
         {
             let mut iter = provenance.contributors.iter();
-            assert_eq!(iter.next(), Some(&"Example1".to_string()));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example1")));
             assert_eq!(iter.next(), None);
         }
 
-        provenance.add_contributor("Example2".to_string());
+        provenance.add_contributor(Contributor::from("Example2"));
         {
             let mut iter = provenance.contributors.iter();
-            assert_eq!(iter.next(), Some(&"Example1".to_string()));
-            assert_eq!(iter.next(), Some(&"Example2".to_string()));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example1")));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example2")));
             assert_eq!(iter.next(), None);
         }
 
-        provenance.add_contributor("Example3".to_string());
+        provenance.add_contributor(Contributor::from("Example3"));
         {
             let mut iter = provenance.contributors.iter();
-            assert_eq!(iter.next(), Some(&"Example1".to_string()));
-            assert_eq!(iter.next(), Some(&"Example2".to_string()));
-            assert_eq!(iter.next(), Some(&"Example3".to_string()));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example1")));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example2")));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example3")));
             assert_eq!(iter.next(), None);
         }
 
-        provenance.remove_contributor("Example2".to_string())?;
+        provenance.remove_contributor(Contributor::from("Example2"))?;
         {
             let mut iter = provenance.contributors.iter();
-            assert_eq!(iter.next(), Some(&"Example1".to_string()));
-            assert_eq!(iter.next(), Some(&"Example3".to_string()));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example1")));
+            assert_eq!(iter.next(), Some(&Contributor::from("Example3")));
             assert_eq!(iter.next(), None);
         }