fix(api): Keep command keys stable across boots

api/src/opentrons/protocol_engine/commands/hash_command_params.py

@@ -1,14 +1,14 @@
 """Hash command params into idempotent keys to track commands from analysis to run."""
+from hashlib import md5
 from typing import Optional
 
 from .command import CommandIntent
 from .command_unions import CommandCreate
 
 
-# TODO(mc, 2022-11-02): this implementation is overly simplistic
-# and exists solely for demostration purposes.
-# Give it a real implementation with tests
-# https://opentrons.atlassian.net/browse/RCORE-326
+# TODO(mm, 2023-04-28):
+# This implementation will not notice that commands are different if they have different params
+# but share the same commandType. We should also hash command params. (Jira RCORE-326.)
 def hash_command_params(
     create: CommandCreate, last_hash: Optional[str]
 ) -> Optional[str]:
@@ -28,8 +28,12 @@ def hash_command_params(
         The command hash, if the command is a protocol command.
         `None` if the command is a setup command.
     """
-    return (
-        f"{hash((last_hash, create.commandType))}"
-        if create.intent != CommandIntent.SETUP
-        else None
-    )
+    if create.intent == CommandIntent.SETUP:
+        return None
+    else:
+        # We avoid Python's built-in hash() function because it's not stable across
+        # runs of the Python interpreter. (Jira RSS-215.)
+        last_contribution = b"" if last_hash is None else last_hash.encode("ascii")
+        this_contribution = md5(create.commandType.encode("ascii")).digest()
+        to_hash = last_contribution + this_contribution
+        return md5(to_hash).hexdigest()

api/tests/opentrons/protocol_engine/commands/test_hash_command_params.py

@@ -0,0 +1,64 @@
+"""Tests for hash_command_params."""
+
+from opentrons.protocol_engine import CommandIntent
+from opentrons.protocol_engine import commands
+from opentrons.protocol_engine.commands.hash_command_params import hash_command_params
+
+
+def test_equivalent_commands() -> None:
+    """Equivalent commands should have the same hash."""
+    a = commands.BlowOutInPlaceCreate(
+        params=commands.BlowOutInPlaceParams(
+            pipetteId="abc123",
+            flowRate=123,
+        )
+    )
+    b = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123)
+    )
+    c = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123)
+    )
+
+    assert hash_command_params(b, None) == hash_command_params(c, None)
+
+    a_hash = hash_command_params(a, None)
+    assert hash_command_params(b, a_hash) == hash_command_params(c, a_hash)
+
+
+def test_nonequivalent_commands() -> None:
+    """Nonequivalent commands should have different hashes."""
+    a = commands.BlowOutInPlaceCreate(
+        params=commands.BlowOutInPlaceParams(
+            pipetteId="abc123",
+            flowRate=123,
+        )
+    )
+    b = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123)
+    )
+
+    assert hash_command_params(a, None) != hash_command_params(b, None)
+
+
+def test_repeated_commands() -> None:
+    """Repeated commands should hash differently, even though they're equivalent in isolation."""
+    a = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123)
+    )
+    b = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123)
+    )
+
+    a_hash = hash_command_params(a, None)
+    b_hash = hash_command_params(b, a_hash)
+    assert a_hash != b_hash
+
+
+def test_setup_command() -> None:
+    """Setup commands should always hash to None."""
+    setup_command = commands.WaitForDurationCreate(
+        params=commands.WaitForDurationParams(seconds=123),
+        intent=CommandIntent.SETUP,
+    )
+    assert hash_command_params(setup_command, None) is None