refactor(robot-server): Use last run command for `/runs/:runId/curren…

…tState` links (#16557)

api-client/src/runs/types.ts

@@ -99,7 +99,7 @@ export interface RunsLinks {
 }
 
 export interface RunCommandLink {
-  current: CommandLinkNoMeta
+  lastCompleted: CommandLinkNoMeta
 }
 
 export interface CommandLinkNoMeta {

api/src/opentrons/protocol_runner/run_orchestrator.py

@@ -257,6 +257,22 @@ def get_current_command(self) -> Optional[CommandPointer]:
         """Get the "current" command, if any."""
         return self._protocol_engine.state_view.commands.get_current()
 
+    def get_most_recently_finalized_command(self) -> Optional[CommandPointer]:
+        """Get the most recently finalized command, if any."""
+        most_recently_finalized_command = (
+            self._protocol_engine.state_view.commands.get_most_recently_finalized_command()
+        )
+        return (
+            CommandPointer(
+                command_id=most_recently_finalized_command.command.id,
+                command_key=most_recently_finalized_command.command.key,
+                created_at=most_recently_finalized_command.command.createdAt,
+                index=most_recently_finalized_command.index,
+            )
+            if most_recently_finalized_command
+            else None
+        )
+
     def get_command_slice(
         self, cursor: Optional[int], length: int, include_fixit_commands: bool
     ) -> CommandSlice:

robot-server/robot_server/runs/router/base_router.py

@@ -132,9 +132,9 @@ class AllRunsLinks(BaseModel):
 class CurrentStateLinks(BaseModel):
     """Links returned with the current state of a run."""
 
-    current: Optional[CommandLinkNoMeta] = Field(
+    lastCompleted: Optional[CommandLinkNoMeta] = Field(
         None,
-        description="Path to the current command when current state was reported, if any.",
+        description="Path to the last completed command when current state was reported, if any.",
     )
 
 
@@ -564,7 +564,7 @@ async def get_run_commands_error(
         """
     ),
     responses={
-        status.HTTP_200_OK: {"model": SimpleBody[RunCurrentState]},
+        status.HTTP_200_OK: {"model": Body[RunCurrentState, CurrentStateLinks]},
         status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]},
     },
 )
@@ -590,17 +590,18 @@ async def get_current_state(
             for pipetteId, nozzle_map in active_nozzle_maps.items()
         }
 
-        current_command = run_data_manager.get_current_command(run_id=runId)
+        last_completed_command = run_data_manager.get_last_completed_command(
+            run_id=runId
+        )
     except RunNotCurrentError as e:
         raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT)
 
-    # TODO(jh, 03-11-24): Use `last_completed_command` instead of `current_command` to avoid concurrency gotchas.
     links = CurrentStateLinks.construct(
-        current=CommandLinkNoMeta.construct(
-            id=current_command.command_id,
-            href=f"/runs/{runId}/commands/{current_command.command_id}",
+        lastCompleted=CommandLinkNoMeta.construct(
+            id=last_completed_command.command_id,
+            href=f"/runs/{runId}/commands/{last_completed_command.command_id}",
         )
-        if current_command is not None
+        if last_completed_command is not None
         else None
     )
 

robot-server/robot_server/runs/run_data_manager.py

@@ -456,10 +456,20 @@ def get_current_command(self, run_id: str) -> Optional[CommandPointer]:
         if self._run_orchestrator_store.current_run_id == run_id:
             return self._run_orchestrator_store.get_current_command()
         else:
-            # todo(mm, 2024-05-20):
-            # For historical runs to behave consistently with the current run,
-            # this should be the most recently completed command, not `None`.
-            return None
+            return self._get_historical_run_last_command(run_id=run_id)
+
+    def get_last_completed_command(self, run_id: str) -> Optional[CommandPointer]:
+        """Get the "last" command, if any.
+
+        See `ProtocolEngine.state_view.commands.get_most_recently_finalized_command()` for the definition of "last."
+
+        Args:
+            run_id: ID of the run.
+        """
+        if self._run_orchestrator_store.current_run_id == run_id:
+            return self._run_orchestrator_store.get_most_recently_finalized_command()
+        else:
+            return self._get_historical_run_last_command(run_id=run_id)
 
     def get_recovery_target_command(self, run_id: str) -> Optional[CommandPointer]:
         """Get the current error recovery target.
@@ -554,3 +564,22 @@ def _get_run_time_parameters(self, run_id: str) -> List[RunTimeParameter]:
             return self._run_orchestrator_store.get_run_time_parameters()
         else:
             return self._run_store.get_run_time_parameters(run_id=run_id)
+
+    def _get_historical_run_last_command(self, run_id: str) -> Optional[CommandPointer]:
+        command_slice = self._run_store.get_commands_slice(
+            run_id=run_id, cursor=None, length=1, include_fixit_commands=True
+        )
+        if not command_slice.commands:
+            return None
+        command = command_slice.commands[-1]
+
+        return (
+            CommandPointer(
+                command_id=command.id,
+                command_key=command.key,
+                created_at=command.createdAt,
+                index=command_slice.cursor,
+            )
+            if command
+            else None
+        )

robot-server/robot_server/runs/run_orchestrator_store.py

@@ -335,9 +335,13 @@ def get_run_time_parameters(self) -> List[RunTimeParameter]:
         return self.run_orchestrator.get_run_time_parameters()
 
     def get_current_command(self) -> Optional[CommandPointer]:
-        """Get the current running command."""
+        """Get the current running command, if any."""
         return self.run_orchestrator.get_current_command()
 
+    def get_most_recently_finalized_command(self) -> Optional[CommandPointer]:
+        """Get the most recently finalized command, if any."""
+        return self.run_orchestrator.get_most_recently_finalized_command()
+
     def get_command_slice(
         self, cursor: Optional[int], length: int, include_fixit_commands: bool
     ) -> CommandSlice:

robot-server/tests/runs/router/test_base_router.py

@@ -876,10 +876,12 @@ async def test_get_current_state_success(
     decoy.when(mock_run_data_manager.get_nozzle_maps(run_id=run_id)).then_return(
         mock_nozzle_maps
     )
-    decoy.when(mock_run_data_manager.get_current_command(run_id=run_id)).then_return(
+    decoy.when(
+        mock_run_data_manager.get_last_completed_command(run_id=run_id)
+    ).then_return(
         CommandPointer(
-            command_id="current-command-id",
-            command_key="current-command-key",
+            command_id="last-command-id",
+            command_key="last-command-key",
             created_at=datetime(year=2024, month=4, day=4),
             index=101,
         )
@@ -901,9 +903,9 @@ async def test_get_current_state_success(
         }
     )
     assert result.content.links == CurrentStateLinks(
-        current=CommandLinkNoMeta(
-            href="/runs/test-run-id/commands/current-command-id",
-            id="current-command-id",
+        lastCompleted=CommandLinkNoMeta(
+            href="/runs/test-run-id/commands/last-command-id",
+            id="last-command-id",
         )
     )
 

robot-server/tests/runs/test_run_data_manager.py

@@ -1004,12 +1004,105 @@ def test_get_current_command_not_current_run(
     subject: RunDataManager,
     mock_run_store: RunStore,
     mock_run_orchestrator_store: RunOrchestratorStore,
+    run_command: commands.Command,
 ) -> None:
-    """Should return None because the run is not current."""
+    """Should get the last command from the run store for a historical run."""
+    last_command_slice = commands.WaitForResume(
+        id="command-id-1",
+        key="command-key",
+        createdAt=datetime(year=2021, month=1, day=1),
+        status=commands.CommandStatus.SUCCEEDED,
+        params=commands.WaitForResumeParams(message="Hello"),
+    )
+
+    expected_last_command = CommandPointer(
+        command_id="command-id-1",
+        command_key="command-key",
+        created_at=datetime(year=2021, month=1, day=1),
+        index=0,
+    )
+
+    command_slice = CommandSlice(
+        commands=[last_command_slice], cursor=0, total_length=1
+    )
+
     decoy.when(mock_run_orchestrator_store.current_run_id).then_return("not-run-id")
+    decoy.when(
+        mock_run_store.get_commands_slice(
+            run_id="run-id", cursor=None, length=1, include_fixit_commands=True
+        )
+    ).then_return(command_slice)
     result = subject.get_current_command("run-id")
 
-    assert result is None
+    assert result == expected_last_command
+
+
+def test_get_last_completed_command_current_run(
+    decoy: Decoy,
+    subject: RunDataManager,
+    mock_run_orchestrator_store: RunOrchestratorStore,
+    run_command: commands.Command,
+) -> None:
+    """Should get the last command from the engine store for the current run."""
+    run_id = "current-run-id"
+    expected_last_command = CommandPointer(
+        command_id=run_command.id,
+        command_key=run_command.key,
+        created_at=run_command.createdAt,
+        index=1,
+    )
+
+    decoy.when(mock_run_orchestrator_store.current_run_id).then_return(run_id)
+    decoy.when(
+        mock_run_orchestrator_store.get_most_recently_finalized_command()
+    ).then_return(expected_last_command)
+
+    result = subject.get_last_completed_command(run_id)
+
+    assert result == expected_last_command
+
+
+def test_get_last_completed_command_not_current_run(
+    decoy: Decoy,
+    subject: RunDataManager,
+    mock_run_orchestrator_store: RunOrchestratorStore,
+    mock_run_store: RunStore,
+    run_command: commands.Command,
+) -> None:
+    """Should get the last command from the run store for a historical run."""
+    run_id = "historical-run-id"
+
+    last_command_slice = commands.WaitForResume(
+        id="command-id-1",
+        key="command-key",
+        createdAt=datetime(year=2021, month=1, day=1),
+        status=commands.CommandStatus.SUCCEEDED,
+        params=commands.WaitForResumeParams(message="Hello"),
+    )
+
+    expected_last_command = CommandPointer(
+        command_id="command-id-1",
+        command_key="command-key",
+        created_at=datetime(year=2021, month=1, day=1),
+        index=1,
+    )
+
+    decoy.when(mock_run_orchestrator_store.current_run_id).then_return(
+        "different-run-id"
+    )
+
+    command_slice = CommandSlice(
+        commands=[last_command_slice], cursor=1, total_length=1
+    )
+    decoy.when(
+        mock_run_store.get_commands_slice(
+            run_id=run_id, cursor=None, length=1, include_fixit_commands=True
+        )
+    ).then_return(command_slice)
+
+    result = subject.get_last_completed_command(run_id)
+
+    assert result == expected_last_command
 
 
 def test_get_command_from_engine(