fix(chemistry): treat trailing letters in SamplePointID as a sample point

A PointID ending in letters is a sample point, never a base well id:
WL-0434A is a sample point on well WL-0434. The ingest treated the whole
value as the base, so a workbook naming WL-0434A either failed to
resolve a well (the common case, since wells are named WL-0434) or, if
such a Thing existed, appended a second letter and produced WL-0434AA.

Split the supplied PointID into base and suffix, resolve the well from
the base, and compare any supplied letter against the next free
incrementor. The computed letter wins because it cannot collide with an
existing sample point, but a disagreement is now reported so a human can
reconcile it. Lowercase endings are not incrementors, so a well legitimately
named "Test Well" is unaffected.

Reporting a disagreement needed a non-fatal channel: validation_errors
aborts the file, which is too blunt for a letter that is merely
unexpected. Add a warnings list to the payload, surfaced by the CLI and
counted in the manifest's existing validation_errors_or_warnings field,
leaving the exit code at 0.

Verified against a real LIMS export (NMT_260503): 104 rows, 98 imported
after collapsing Fe/Mn/Sr duplicate methods, loading as WL-0433A and
WL-0434A rather than WL-0433AA/WL-0434AA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: jirhiker

cli/cli.py

@@ -991,6 +991,7 @@ def water_chemistry_bulk_upload(
     payload = result.payload if isinstance(result.payload, dict) else {}
     summary = payload.get("summary", {})
     validation_errors = payload.get("validation_errors", [])
+    warnings = payload.get("warnings", [])
     created_samples = payload.get("created_samples", [])
     skipped_duplicates = payload.get("skipped_duplicates", [])
 
@@ -1046,6 +1047,12 @@ def water_chemistry_bulk_upload(
             )
         typer.echo()
 
+    if warnings:
+        typer.secho("WARNINGS (loaded, but check these)", fg=colors["field"], bold=True)
+        for entry in warnings:
+            typer.secho(f"  - {entry}", fg=colors["field"])
+        typer.echo()
+
     if validation_errors:
         typer.secho("VALIDATION", fg=colors["accent"], bold=True)
         typer.secho(

services/chemistry_lims.py

@@ -362,6 +362,23 @@ def keyf(r: dict) -> tuple[str, str, str]:
 
 _SUFFIX_RE_TEMPLATE = r"^{base}([A-Z]+)$"
 
+# A PointID ending in letters is a *sample point* id, never a base well id:
+# ``WL-0434A`` is a sample point on well ``WL-0434``. The base must therefore
+# end in a non-letter (``WL-0434``, ``MG-030``) for the trailing letters to
+# count as an incrementor.
+_POINTID_SUFFIX_RE = re.compile(r"^(?P<base>.*[^A-Z])(?P<suffix>[A-Z]+)$")
+
+
+def split_pointid(pointid: str) -> tuple[str, str | None]:
+    """Split a PointID into its base well id and any supplied letter suffix.
+
+    ``WL-0434A`` -> ``("WL-0434", "A")``; ``WL-0434`` -> ``("WL-0434", None)``.
+    """
+    match = _POINTID_SUFFIX_RE.match(pointid)
+    if not match:
+        return pointid, None
+    return match.group("base"), match.group("suffix")
+
 
 def _resolve_thing_id(session: Session, pointid: str) -> int | None:
     things = session.scalars(select(Thing).where(Thing.name == pointid)).all()
@@ -496,7 +513,22 @@ def bulk_upload_chemistry(
 
     prepped = dedupe_records(prepped)
 
+    warnings: list[str] = []
+
     with session_ctx() as session:
+        # A workbook's SamplePointID may already carry a sample-point letter
+        # (WL-0434A). The well is always the base (WL-0434), so strip it before
+        # resolving; the supplied letter is checked against the computed one
+        # below.
+        supplied_suffixes: dict[str, str | None] = {}
+        for rec in prepped:
+            base, suffix = split_pointid(rec["samplepointid"])
+            rec["samplepointid"] = base
+            # Keep the first supplied suffix seen for the well; a workbook
+            # should not disagree with itself, and if it does the mismatch
+            # warning below still fires.
+            supplied_suffixes.setdefault(base, suffix)
+
         # Resolve every distinct (base) sample point to a Thing up front.
         base_pointids = sorted({r["samplepointid"] for r in prepped})
         thing_ids: dict[str, int | None] = {
@@ -549,7 +581,19 @@ def bucket_key(r: dict) -> tuple[str, str | None]:
                 max(used_suffixes[thing_id]) + 1 if used_suffixes[thing_id] else 1
             )
             used_suffixes[thing_id].add(next_int)
-            sample_point_id = f"{base}{_int_to_suffix(next_int)}"
+            computed_suffix = _int_to_suffix(next_int)
+            sample_point_id = f"{base}{computed_suffix}"
+
+            # The workbook may have supplied its own letter. The computed one
+            # wins (it cannot collide with an existing sample point), but a
+            # disagreement is surfaced so a human can reconcile it.
+            supplied = supplied_suffixes.get(base)
+            if supplied is not None and supplied != computed_suffix:
+                warnings.append(
+                    f"{base}: workbook supplied sample point {base}{supplied}, "
+                    f"but the next free incrementor is {computed_suffix}; "
+                    f"loaded as {sample_point_id}."
+                )
 
             collection_date = next(
                 (r["sample_date"] for r in recs if r["sample_date"]), None
@@ -585,6 +629,7 @@ def bucket_key(r: dict) -> tuple[str, str | None]:
         validation_errors=validation_errors,
         skipped_duplicates=skipped_duplicates,
         created=created,
+        warnings=warnings,
     )
 
 
@@ -595,8 +640,10 @@ def _result(
     validation_errors: list[str],
     skipped_duplicates: list[dict],
     created: list[dict],
+    warnings: list[str] | None = None,
 ) -> ChemistryUploadResult:
-    rows_with_issues = len(validation_errors) + len(skipped_duplicates)
+    warnings = warnings or []
+    rows_with_issues = len(validation_errors) + len(skipped_duplicates) + len(warnings)
     payload = {
         "summary": {
             "total_rows_processed": processed,
@@ -606,12 +653,15 @@ def _result(
             "samples_skipped": len(skipped_duplicates),
         },
         "validation_errors": validation_errors,
+        "warnings": warnings,
         "skipped_duplicates": skipped_duplicates,
         "created_samples": created,
     }
     stderr_parts: list[str] = []
     if validation_errors:
         stderr_parts.append("\n".join(validation_errors))
+    if warnings:
+        stderr_parts.append("\n".join(warnings))
     if skipped_duplicates:
         dupes = ", ".join(
             f"{d['pointid']} (WCLab_ID {d['wclab_id']})" for d in skipped_duplicates

tests/test_chemistry_lims.py

@@ -33,6 +33,7 @@
     bulk_upload_chemistry,
     dedupe_records,
     prep_record,
+    split_pointid,
 )
 
 LIMS_HEADER = [
@@ -204,6 +205,65 @@ def test_bulk_upload_skips_duplicate_lab_sample(
     assert len(rows_ca) == 1  # not duplicated
 
 
+@pytest.mark.parametrize(
+    "pointid,expected",
+    [
+        ("WL-0434", ("WL-0434", None)),
+        ("WL-0434A", ("WL-0434", "A")),
+        ("WL-0434AB", ("WL-0434", "AB")),
+        ("MG-030", ("MG-030", None)),
+        ("MG-030A", ("MG-030", "A")),
+        # Lowercase is not an incrementor, so a name ending in one is a base.
+        ("Test Well", ("Test Well", None)),
+        ("Test WellA", ("Test Well", "A")),
+    ],
+)
+def test_split_pointid(pointid, expected):
+    """A PointID ending in capitals is a sample point; the well is the base."""
+    assert split_pointid(pointid) == expected
+
+
+def test_bulk_upload_strips_supplied_suffix_to_find_the_well(
+    tmp_path, water_well_thing, _cleanup_chemistry
+):
+    """A workbook naming sample point 'Test WellA' resolves to well 'Test Well'."""
+    _write_workbook(
+        tmp_path / "lims.xlsx",
+        [_lims_row("calcium", "12.5", pointid="Test WellA", SampleNumber="LAB-1")],
+    )
+
+    result = bulk_upload_chemistry(tmp_path / "lims.xlsx")
+
+    assert result.exit_code == 0, result.stderr
+    # Not 'Test WellAA' -- the supplied letter is not doubled.
+    assert result.payload["created_samples"][0]["sample_point_id"] == "Test WellA"
+    assert result.payload["warnings"] == []
+
+
+def test_bulk_upload_warns_when_supplied_suffix_disagrees(
+    tmp_path, water_well_thing, _cleanup_chemistry
+):
+    """Computed letter wins; the disagreement is reported but does not fail."""
+    _write_workbook(
+        tmp_path / "first.xlsx",
+        [_lims_row("calcium", "12.5", pointid="Test WellA", SampleNumber="LAB-1")],
+    )
+    bulk_upload_chemistry(tmp_path / "first.xlsx")
+
+    # A second lab sample still labelled 'A', though 'B' is the next free one.
+    _write_workbook(
+        tmp_path / "second.xlsx",
+        [_lims_row("calcium", "9.9", pointid="Test WellA", SampleNumber="LAB-2")],
+    )
+    result = bulk_upload_chemistry(tmp_path / "second.xlsx")
+
+    assert result.exit_code == 0, result.stderr
+    assert result.payload["created_samples"][0]["sample_point_id"] == "Test WellB"
+    warnings = result.payload["warnings"]
+    assert len(warnings) == 1
+    assert "Test WellA" in warnings[0] and "Test WellB" in warnings[0]
+
+
 def test_bulk_upload_appends_new_lab_sample_with_next_suffix(
     tmp_path, water_well_thing, _cleanup_chemistry
 ):