refactor(api): Save and load tip length calibrations from tiprack uri (…

…#14512)

# Overview

This is an effort to fix the problem described in an escalations case
[here](https://opentrons.atlassian.net/browse/RSS-468?atlOrigin=eyJpIjoiMTBmOTdlYTQ0YjU4NGRkYmFjNDkwZjY3ZmU2YmU2ZmEiLCJwIjoiamlyYS1zbGFjay1pbnQifQ).
Where the tip length data does not persist during a protocol run, but
does in LPC. @SyntaxColoring discovered that it seemed to be a result of
the labware hash not always being the same once a dict has been
converted to/from a pydantic model. We both agreed that the easiest path
forward would be to lookup tip length calibrations by labware URI
instead.

# Note
The decision was made to keep around tiprack hash for the delete
endpoint until we decide to bump the version header of the API. A TODO
was included in the endpoint.

# Test Plan
- Without modifying tip length data on a robot, ensure that behavior
does not change (i.e. robot moves to all the correct places).
- Modify the tip length calibration to be purposefully off (as described
in the ticket) and verify that the tip length persists both in LPC _and_
the protocol run.
- Delete tip length calibrations and start fresh. Ensure that behavior
is normal.


# Changelog

- Load and save tip length calibrations by tiprack uri rather than hash.
If the old format is detected, we should automatically migrate the data
shape.
- Updated relevant locations for the new data shape

# Review requests

Check out the code and make sure everything makes sense to you.

# Risk assessment

High. This is modifying a critical component of loading tip length
calibration.

api-client/src/calibration/types.ts

@@ -8,7 +8,7 @@ export interface PipOffsetDeletionParams {
 
 export interface TipLengthDeletionParams {
   calType: 'tipLength'
-  tiprack_hash: string
+  tiprack_uri: string
   pipette_id: string
 }
 export type DeleteCalRequestParams =
@@ -93,7 +93,7 @@ export interface TipLengthCalibration {
   source: CalibrationSourceType
   status: IndividualCalibrationHealthStatus
   id: string
-  uri?: string | null
+  uri: string
 }
 
 export interface AllTipLengthCalibrations {

api/src/opentrons/calibration_storage/ot2/models/v1.py

@@ -32,8 +32,12 @@ class TipLengthModel(BaseModel):
         default_factory=CalibrationStatus,
         description="The status of the calibration data.",
     )
-    uri: typing.Union[LabwareUri, Literal[""]] = Field(
-        ..., description="The tiprack URI associated with the tip length data."
+    # Old data may have a `uri` field, replaced later by `definitionHash`.
+    # uri: typing.Union[LabwareUri, Literal[""]] = Field(
+    #    ..., description="The tiprack URI associated with the tip length data."
+    # )
+    definitionHash: str = Field(
+        ..., description="The tiprack hash associated with the tip length data."
     )
 
     @validator("tipLength")

api/src/opentrons/calibration_storage/ot2/tip_length.py

@@ -7,6 +7,7 @@
 from opentrons import config
 
 from .. import file_operators as io, helpers, types as local_types
+from opentrons_shared_data.pipette.dev_types import LabwareUri
 
 from opentrons.protocols.api_support.constants import OPENTRONS_NAMESPACE
 from opentrons.util.helpers import utc_now
@@ -22,9 +23,9 @@
 # Get Tip Length Calibration
 
 
-def _conver_tip_length_model_to_dict(
-    to_dict: typing.Dict[str, v1.TipLengthModel]
-) -> typing.Dict[str, typing.Any]:
+def _convert_tip_length_model_to_dict(
+    to_dict: typing.Dict[LabwareUri, v1.TipLengthModel]
+) -> typing.Dict[LabwareUri, typing.Any]:
     # This is a workaround since pydantic doesn't have a nice way to
     # add encoders when converting to a dict.
     dict_of_tip_lengths = {}
@@ -35,17 +36,23 @@ def _conver_tip_length_model_to_dict(
 
 def tip_lengths_for_pipette(
     pipette_id: str,
-) -> typing.Dict[str, v1.TipLengthModel]:
+) -> typing.Dict[LabwareUri, v1.TipLengthModel]:
     tip_lengths = {}
     try:
         tip_length_filepath = config.get_tip_length_cal_path() / f"{pipette_id}.json"
         all_tip_lengths_for_pipette = io.read_cal_file(tip_length_filepath)
-        for tiprack, data in all_tip_lengths_for_pipette.items():
+        for tiprack_identifier, data in all_tip_lengths_for_pipette.items():
+            # We normally key these calibrations by their tip rack URI,
+            # but older software had them keyed by their tip rack hash.
+            # Migrate from the old format, if necessary.
+            if "/" not in tiprack_identifier:
+                data["definitionHash"] = tiprack_identifier
+                tiprack_identifier = data.pop("uri")
             try:
-                tip_lengths[tiprack] = v1.TipLengthModel(**data)
+                tip_lengths[LabwareUri(tiprack_identifier)] = v1.TipLengthModel(**data)
             except (json.JSONDecodeError, ValidationError):
                 log.warning(
-                    f"Tip length calibration is malformed for {tiprack} on {pipette_id}"
+                    f"Tip length calibration is malformed for {tiprack_identifier} on {pipette_id}"
                 )
                 pass
         return tip_lengths
@@ -64,10 +71,10 @@ def load_tip_length_calibration(
     :param pip_id: pipette you are using
     :param definition: full definition of the tiprack
     """
-    labware_hash = helpers.hash_labware_def(definition)
+    labware_uri = helpers.uri_from_definition(definition)
     load_name = definition["parameters"]["loadName"]
     try:
-        return tip_lengths_for_pipette(pip_id)[labware_hash]
+        return tip_lengths_for_pipette(pip_id)[labware_uri]
     except KeyError as e:
         raise local_types.TipLengthCalNotFound(
             f"Tip length of {load_name} has not been "
@@ -89,16 +96,16 @@ def get_all_tip_length_calibrations() -> typing.List[v1.TipLengthCalibration]:
         if filepath.stem == "index":
             continue
         tip_lengths = tip_lengths_for_pipette(filepath.stem)
-        for tiprack_hash, tip_length in tip_lengths.items():
+        for tiprack_uri, tip_length in tip_lengths.items():
             all_tip_lengths_available.append(
                 v1.TipLengthCalibration(
                     pipette=filepath.stem,
-                    tiprack=tiprack_hash,
+                    tiprack=tip_length.definitionHash,
                     tipLength=tip_length.tipLength,
                     lastModified=tip_length.lastModified,
                     source=tip_length.source,
                     status=tip_length.status,
-                    uri=tip_length.uri,
+                    uri=tiprack_uri,
                 )
             )
     return all_tip_lengths_available
@@ -129,28 +136,45 @@ def get_custom_tiprack_definition_for_tlc(labware_uri: str) -> "LabwareDefinitio
 # Delete Tip Length Calibration
 
 
-def delete_tip_length_calibration(tiprack: str, pipette_id: str) -> None:
+def delete_tip_length_calibration(
+    pipette_id: str,
+    tiprack_uri: typing.Optional[LabwareUri] = None,
+    tiprack_hash: typing.Optional[str] = None,
+) -> None:
     """
-    Delete tip length calibration based on tiprack hash and
-    pipette serial number
+    Delete tip length calibration based on an optional tiprack uri or
+    tiprack hash and pipette serial number.
 
-    :param tiprack: tiprack hash
+    :param tiprack_uri: tiprack uri
+    :param tiprack_hash: tiprack uri
     :param pipette: pipette serial number
     """
     tip_lengths = tip_lengths_for_pipette(pipette_id)
-
-    if tiprack in tip_lengths:
+    tip_length_dir = config.get_tip_length_cal_path()
+    if tiprack_uri in tip_lengths:
         # maybe make modify and delete same file?
-        del tip_lengths[tiprack]
-        tip_length_dir = config.get_tip_length_cal_path()
+        del tip_lengths[tiprack_uri]
+
+        if tip_lengths:
+            dict_of_tip_lengths = _convert_tip_length_model_to_dict(tip_lengths)
+            io.save_to_file(tip_length_dir, pipette_id, dict_of_tip_lengths)
+        else:
+            io.delete_file(tip_length_dir / f"{pipette_id}.json")
+    elif tiprack_hash and any(tiprack_hash in v.dict() for v in tip_lengths.values()):
+        # NOTE this is for backwards compatibilty only
+        # TODO delete this check once the tip_length DELETE router
+        # no longer depends on a tiprack hash
+        for k, v in tip_lengths.items():
+            if tiprack_hash in v.dict():
+                tip_lengths.pop(k)
         if tip_lengths:
-            dict_of_tip_lengths = _conver_tip_length_model_to_dict(tip_lengths)
+            dict_of_tip_lengths = _convert_tip_length_model_to_dict(tip_lengths)
             io.save_to_file(tip_length_dir, pipette_id, dict_of_tip_lengths)
         else:
             io.delete_file(tip_length_dir / f"{pipette_id}.json")
     else:
         raise local_types.TipLengthCalNotFound(
-            f"Tip length for hash {tiprack} has not been "
+            f"Tip length for uri {tiprack_uri} and hash {tiprack_hash} has not been "
             f"calibrated for this pipette: {pipette_id} and cannot"
             "be loaded"
         )
@@ -176,7 +200,7 @@ def create_tip_length_data(
     cal_status: typing.Optional[
         typing.Union[local_types.CalibrationStatus, v1.CalibrationStatus]
     ] = None,
-) -> typing.Dict[str, v1.TipLengthModel]:
+) -> typing.Dict[LabwareUri, v1.TipLengthModel]:
     """
     Function to correctly format tip length data.
 
@@ -197,13 +221,13 @@ def create_tip_length_data(
         lastModified=utc_now(),
         source=local_types.SourceType.user,
         status=cal_status_model,
-        uri=labware_uri,
+        definitionHash=labware_hash,
     )
 
     if not definition.get("namespace") == OPENTRONS_NAMESPACE:
         _save_custom_tiprack_definition(labware_uri, definition)
 
-    data = {labware_hash: tip_length_data}
+    data = {labware_uri: tip_length_data}
     return data
 
 
@@ -220,7 +244,7 @@ def _save_custom_tiprack_definition(
 
 def save_tip_length_calibration(
     pip_id: str,
-    tip_length_cal: typing.Dict[str, v1.TipLengthModel],
+    tip_length_cal: typing.Dict[LabwareUri, v1.TipLengthModel],
 ) -> None:
     """
     Function used to save tip length calibration to file.
@@ -235,5 +259,5 @@ def save_tip_length_calibration(
 
     all_tip_lengths.update(tip_length_cal)
 
-    dict_of_tip_lengths = _conver_tip_length_model_to_dict(all_tip_lengths)
+    dict_of_tip_lengths = _convert_tip_length_model_to_dict(all_tip_lengths)
     io.save_to_file(tip_length_dir_path, pip_id, dict_of_tip_lengths)

api/src/opentrons/hardware_control/instruments/ot2/instrument_calibration.py

@@ -130,18 +130,15 @@ def load_tip_length_for_pipette(
         pipette_id, tiprack
     )
 
-    # TODO (lc 09-26-2022) We shouldn't have to do a hash twice. We should figure out what
-    # information we actually need from the labware definition and pass it into
-    # the `load_tip_length_calibration` function.
-    tiprack_hash = helpers.hash_labware_def(tiprack)
+    tiprack_uri = helpers.uri_from_definition(tiprack)
 
     return TipLengthCalibration(
         tip_length=tip_length_data.tipLength,
         source=tip_length_data.source,
         pipette=pipette_id,
-        tiprack=tiprack_hash,
+        tiprack=tip_length_data.definitionHash,
         last_modified=tip_length_data.lastModified,
-        uri=tip_length_data.uri,
+        uri=tiprack_uri,
         status=types.CalibrationStatus(
             markedAt=tip_length_data.status.markedAt,
             markedBad=tip_length_data.status.markedBad,

api/tests/opentrons/calibration_storage/test_tip_length_ot2.py

@@ -1,9 +1,11 @@
 import pytest
-from typing import cast, Any, TYPE_CHECKING
+from typing import Any, TYPE_CHECKING
 
+from opentrons import config
 from opentrons.calibration_storage import (
     types as cs_types,
     helpers,
+    file_operators as io,
 )
 
 from opentrons.calibration_storage.ot2 import (
@@ -15,10 +17,10 @@
     clear_tip_length_calibration,
     models,
 )
+from opentrons_shared_data.pipette.dev_types import LabwareUri
 
 if TYPE_CHECKING:
     from opentrons_shared_data.labware.dev_types import LabwareDefinition
-    from opentrons_shared_data.pipette.dev_types import LabwareUri
 
 
 @pytest.fixture
@@ -38,6 +40,18 @@ def starting_calibration_data(
     save_tip_length_calibration("pip1", tip_length1)
     save_tip_length_calibration("pip2", tip_length2)
     save_tip_length_calibration("pip1", tip_length3)
+    inside_data = tip_length3[LabwareUri("dummy_namespace/minimal_labware_def/1")]
+    data = {
+        inside_data.definitionHash: {
+            "tipLength": 27,
+            "lastModified": inside_data.lastModified.isoformat(),
+            "source": inside_data.source,
+            "status": inside_data.status.dict(),
+            "uri": "dummy_namespace/minimal_labware_def/1",
+        }
+    }
+    tip_length_dir_path = config.get_tip_length_cal_path()
+    io.save_to_file(tip_length_dir_path, "pip2", data)
 
 
 def test_save_tip_length_calibration(
@@ -48,13 +62,13 @@ def test_save_tip_length_calibration(
     """
     assert tip_lengths_for_pipette("pip1") == {}
     assert tip_lengths_for_pipette("pip2") == {}
-    tip_rack_hash = helpers.hash_labware_def(minimal_labware_def)
+    tip_rack_uri = helpers.uri_from_definition(minimal_labware_def)
     tip_length1 = create_tip_length_data(minimal_labware_def, 22.0)
     tip_length2 = create_tip_length_data(minimal_labware_def, 31.0)
     save_tip_length_calibration("pip1", tip_length1)
     save_tip_length_calibration("pip2", tip_length2)
-    assert tip_lengths_for_pipette("pip1")[tip_rack_hash].tipLength == 22.0
-    assert tip_lengths_for_pipette("pip2")[tip_rack_hash].tipLength == 31.0
+    assert tip_lengths_for_pipette("pip1")[tip_rack_uri].tipLength == 22.0
+    assert tip_lengths_for_pipette("pip2")[tip_rack_uri].tipLength == 31.0
 
 
 def test_get_tip_length_calibration(
@@ -64,11 +78,12 @@ def test_get_tip_length_calibration(
     Test ability to get a tip length calibration model.
     """
     tip_length_data = load_tip_length_calibration("pip1", minimal_labware_def)
+    tip_rack_hash = helpers.hash_labware_def(minimal_labware_def)
     assert tip_length_data == models.v1.TipLengthModel(
         tipLength=22.0,
         source=cs_types.SourceType.user,
         lastModified=tip_length_data.lastModified,
-        uri=cast("LabwareUri", "opentronstest/minimal_labware_def/1"),
+        definitionHash=tip_rack_hash,
     )
 
     with pytest.raises(cs_types.TipLengthCalNotFound):
@@ -83,8 +98,8 @@ def test_delete_specific_tip_calibration(
     """
     assert len(tip_lengths_for_pipette("pip1").keys()) == 2
     assert tip_lengths_for_pipette("pip2") != {}
-    tip_rack_hash = helpers.hash_labware_def(minimal_labware_def)
-    delete_tip_length_calibration(tip_rack_hash, "pip1")
+    tip_rack_uri = helpers.uri_from_definition(minimal_labware_def)
+    delete_tip_length_calibration("pip1", tiprack_uri=tip_rack_uri)
     assert len(tip_lengths_for_pipette("pip1").keys()) == 1
     assert tip_lengths_for_pipette("pip2") != {}
 

api/tests/opentrons/hardware_control/instruments/test_instrument_calibration.py

@@ -81,7 +81,7 @@ def test_load_tip_length(
     tip_length_data = v1_models.TipLengthModel(
         tipLength=1.23,
         lastModified=datetime(year=2023, month=1, day=1),
-        uri=LabwareUri("def456"),
+        definitionHash="asdfghjk",
         source=subject.SourceType.factory,
         status=v1_models.CalibrationStatus(
             markedBad=True,
@@ -99,6 +99,9 @@ def test_load_tip_length(
     decoy.when(calibration_storage.helpers.hash_labware_def(tip_rack_dict)).then_return(
         "asdfghjk"
     )
+    decoy.when(
+        calibration_storage.helpers.uri_from_definition(tip_rack_dict)
+    ).then_return(LabwareUri("def456"))
 
     result = subject.load_tip_length_for_pipette(
         pipette_id="abc123", tiprack=tip_rack_definition

app-testing/tests/__snapshots__/analyses_snapshot_test/test_analysis_snapshot[25f79fd65e][Flex_None_None_TM_2_16_AnalysisError_ModuleInStagingAreaCol3].json

@@ -564,7 +564,7 @@
           "errorInfo": {
             "args": "('nest_1_reservoir_290ml in slot C4 prevents temperatureModuleV2 from using slot C3.',)",
             "class": "DeckConflictError",
-            "traceback": "  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocols/execution/execute_python.py\", line 69, in run_python\n    exec(\"run(__context)\", new_globs)\n\n  File \"<string>\", line 1, in <module>\n\n  File \"Flex_None_None_TM_2_16_AnalysisError_ModuleInStagingAreaCol3.py\", line 17, in run\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocols/api_support/util.py\", line 383, in _check_version_wrapper\n    return decorated_obj(*args, **kwargs)\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/protocol_context.py\", line 818, in load_module\n    module_core = self._core.load_module(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/core/engine/protocol.py\", line 435, in load_module\n    deck_conflict.check(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/core/engine/deck_conflict.py\", line 185, in check\n    wrapped_deck_conflict.check(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/motion_planning/deck_conflict.py\", line 224, in check\n    raise DeckConflictError(\n"
+            "traceback": "  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocols/execution/execute_python.py\", line 69, in run_python\n    exec(\"run(__context)\", new_globs)\n\n  File \"<string>\", line 1, in <module>\n\n  File \"Flex_None_None_TM_2_16_AnalysisError_ModuleInStagingAreaCol3.py\", line 17, in run\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocols/api_support/util.py\", line 383, in _check_version_wrapper\n    return decorated_obj(*args, **kwargs)\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/protocol_context.py\", line 818, in load_module\n    module_core = self._core.load_module(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/core/engine/protocol.py\", line 435, in load_module\n    deck_conflict.check(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/protocol_api/core/engine/deck_conflict.py\", line 190, in check\n    wrapped_deck_conflict.check(\n\n  File \"/usr/local/lib/python3.10/site-packages/opentrons/motion_planning/deck_conflict.py\", line 224, in check\n    raise DeckConflictError(\n"
           },
           "errorType": "PythonException",
           "wrappedErrors": []