diff --git a/api-client/src/runs/types.ts b/api-client/src/runs/types.ts index 0415367f1e6..d274152d578 100644 --- a/api-client/src/runs/types.ts +++ b/api-client/src/runs/types.ts @@ -98,7 +98,7 @@ export interface RunsLinks { } export interface RunCommandLink { - current: CommandLinkNoMeta + lastCompleted: CommandLinkNoMeta } export interface CommandLinkNoMeta { diff --git a/api/src/opentrons/protocol_runner/run_orchestrator.py b/api/src/opentrons/protocol_runner/run_orchestrator.py index 697e4a14e3a..69d9feaf524 100644 --- a/api/src/opentrons/protocol_runner/run_orchestrator.py +++ b/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: diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index 639e6d91628..788ca44aa1c 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/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 ) diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index 3edf89ef163..d30f5c33979 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/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 + ) diff --git a/robot-server/robot_server/runs/run_orchestrator_store.py b/robot-server/robot_server/runs/run_orchestrator_store.py index e05bd3bd349..efa97347ae9 100644 --- a/robot-server/robot_server/runs/run_orchestrator_store.py +++ b/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: diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index 25c91f70ade..894950343e4 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/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", ) ) diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index 869f1c1c643..5e4aed1f3e2 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/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(