Skip to content

Commit

Permalink
refactor(app): Update diverged protocol behavior (#16674)
Browse files Browse the repository at this point in the history
Works toward RQA-3469 and closes EXEC-802

The way the app handles divergent runs, those runs that are associated with a non-determinisitic protocol, currently show some confusing copy in the desktop app/ODD, and at times, exhibit unexpected behavior. This commit works toward tying up those loose ends to make divergent protocol handling seem more like a feature and not an unhandled bug.
  • Loading branch information
mjhuff authored Nov 4, 2024
1 parent e63686b commit 7ce9d4b
Show file tree
Hide file tree
Showing 31 changed files with 210 additions and 143 deletions.
5 changes: 4 additions & 1 deletion app/src/assets/localization/en/error_recovery.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"continue": "Continue",
"continue_run_now": "Continue run now",
"continue_to_drop_tip": "Continue to drop tip",
"do_you_need_to_blowout": "First, do you need to blow out aspirated liquid?",
"door_open_gripper_home": "The robot door must be closed for the gripper to home its Z-axis before you can continue manually moving labware.",
"ensure_lw_is_accurately_placed": "Ensure labware is accurately placed in the slot to prevent further errors.",
"error": "Error",
Expand Down Expand Up @@ -49,13 +50,13 @@
"manually_move_lw_on_deck": "Manually move labware on deck",
"manually_replace_lw_and_retry": "Manually replace labware on deck and retry step",
"manually_replace_lw_on_deck": "Manually replace labware on deck",
"na": "N/A",
"next_step": "Next step",
"next_try_another_action": "Next, you can try another recovery action or cancel the run.",
"no_liquid_detected": "No liquid detected",
"overpressure_is_usually_caused": "Overpressure is usually caused by a tip contacting labware, a clog, or moving viscous liquid too quickly",
"pick_up_tips": "Pick up tips",
"pipette_overpressure": "Pipette overpressure",
"do_you_need_to_blowout": "First, do you need to blowout aspirated liquid?",
"proceed_to_cancel": "Proceed to cancel",
"proceed_to_tip_selection": "Proceed to tip selection",
"recovery_action_failed": "{{action}} failed",
Expand All @@ -76,6 +77,7 @@
"retry_with_new_tips": "Retry with new tips",
"retry_with_same_tips": "Retry with same tips",
"retrying_step_succeeded": "Retrying step {{step}} succeeded.",
"retrying_step_succeeded_na": "Retrying current step succeeded.",
"return_to_menu": "Return to menu",
"robot_door_is_open": "Robot door is open",
"robot_is_canceling_run": "Robot is canceling the run",
Expand All @@ -93,6 +95,7 @@
"skip_to_next_step_new_tips": "Skip to next step with new tips",
"skip_to_next_step_same_tips": "Skip to next step with same tips",
"skipping_to_step_succeeded": "Skipping to step {{step}} succeeded.",
"skipping_to_step_succeeded_na": "Skipping to next step succeeded.",
"stand_back": "Stand back, robot is in motion",
"stand_back_picking_up_tips": "Stand back, picking up tips",
"stand_back_resuming": "Stand back, resuming current step",
Expand Down
2 changes: 2 additions & 0 deletions app/src/assets/localization/en/run_details.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"module_controls": "Module Controls",
"module_slot_number": "Slot {{slot_number}}",
"move_labware": "Move Labware",
"na": "N/A",
"name": "Name",
"no_files_included": "No protocol files included",
"no_of_error": "{{count}} error",
Expand Down Expand Up @@ -144,6 +145,7 @@
"status_succeeded": "Completed",
"step": "Step",
"step_failed": "Step failed",
"step_na": "Step: N/A",
"step_number": "Step {{step_number}}:",
"steps_total": "{{count}} steps total",
"stored_labware_offset_data": "Stored Labware Offset data that applies to this protocol",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function RunHeaderModalContainer(
<ErrorRecoveryFlows
runStatus={runStatus}
runId={runId}
failedCommandByRunRecord={recoveryModalUtils.failedCommand}
unvalidatedFailedCommand={recoveryModalUtils.failedCommand}
protocolAnalysis={robotProtocolAnalysis}
/>
) : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe('RunProgressMeter', () => {
it('should show only the total count of commands in run and not show the meter when protocol is non-deterministic', () => {
vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any)
render(props)
expect(screen.getByText('Current Step ?/?:')).toBeTruthy()
expect(screen.getByText('Current Step N/A:')).toBeTruthy()
expect(screen.queryByText('MOCK PROGRESS BAR')).toBeFalsy()
})
it('should give the correct info when run status is idle', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ export function useRunProgressCopy({
if (runStatus === RUN_STATUS_IDLE) {
return `${stepType}:`
} else if (isTerminalStatus && currentStepNumber == null) {
return `${stepType}: N/A`
return `${stepType}: ${t('na')}`
} else if (hasRunDiverged) {
return `${stepType} ${t('na')}:`
} else {
const getCountString = (): string => {
const current = currentStepNumber ?? '?'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export function ErrorRecoveryWizard(
recoveryCommands,
routeUpdateActions,
} = props
const errorKind = getErrorKind(failedCommand?.byRunRecord ?? null)
const errorKind = getErrorKind(failedCommand)

useInitialPipetteHome({
hasLaunchedRecovery,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ describe('useDropTipFlowUtils', () => {

testingRender(result.current.copyOverrides.beforeBeginningTopText as any)

screen.getByText('First, do you need to blowout aspirated liquid?')
screen.getByText('First, do you need to blow out aspirated liquid?')

testingRender(result.current.copyOverrides.tipDropCompleteBtnCopy as any)

Expand Down
2 changes: 1 addition & 1 deletion app/src/organisms/ErrorRecoveryFlows/RecoverySplash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function RecoverySplash(props: RecoverySplashProps): JSX.Element | null {
resumePausedRecovery,
} = props
const { t } = useTranslation('error_recovery')
const errorKind = getErrorKind(failedCommand?.byRunRecord ?? null)
const errorKind = getErrorKind(failedCommand)
const title = useErrorName(errorKind)
const { makeToast } = useToaster()

Expand Down
2 changes: 1 addition & 1 deletion app/src/organisms/ErrorRecoveryFlows/__fixtures__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const mockPickUpTipLabware: LoadedLabware = {

// TODO: jh(08-07-24): update the "byAnalysis" mockFailedCommand.
export const mockRecoveryContentProps: RecoveryContentProps = {
failedCommandByRunRecord: mockFailedCommand,
unvalidatedFailedCommand: mockFailedCommand,
failedCommand: {
byRunRecord: mockFailedCommand,
byAnalysis: mockFailedCommand,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe('ErrorRecoveryFlows', () => {
beforeEach(() => {
props = {
runStatus: RUN_STATUS_AWAITING_RECOVERY,
failedCommandByRunRecord: mockFailedCommand,
unvalidatedFailedCommand: mockFailedCommand,
runId: 'MOCK_RUN_ID',
protocolAnalysis: null,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('getRelevantFailedLabwareCmdFrom', () => {
},
}
const result = getRelevantFailedLabwareCmdFrom({
failedCommandByRunRecord: failedLiquidProbeCommand,
failedCommand: { byRunRecord: failedLiquidProbeCommand } as any,
})
expect(result).toEqual(failedLiquidProbeCommand)
})
Expand All @@ -117,11 +117,13 @@ describe('getRelevantFailedLabwareCmdFrom', () => {

overpressureErrorKinds.forEach(([commandType, errorType]) => {
const result = getRelevantFailedLabwareCmdFrom({
failedCommandByRunRecord: {
...failedCommand,
commandType,
error: { isDefined: true, errorType },
},
failedCommand: {
byRunRecord: {
...failedCommand,
commandType,
error: { isDefined: true, errorType },
},
} as any,
runCommands,
})
expect(result).toBe(pickUpTipCommand)
Expand All @@ -138,27 +140,33 @@ describe('getRelevantFailedLabwareCmdFrom', () => {
},
}
const result = getRelevantFailedLabwareCmdFrom({
failedCommandByRunRecord: failedGripperCommand,
failedCommand: { byRunRecord: failedGripperCommand } as any,
})
expect(result).toEqual(failedGripperCommand)
})

it('should return null for GENERAL_ERROR error kind', () => {
const result = getRelevantFailedLabwareCmdFrom({
failedCommandByRunRecord: {
...failedCommand,
error: { errorType: 'literally anything else' },
},
failedCommand: {
byRunRecord: {
...failedCommand,
error: {
errorType: 'literally anything else',
},
},
} as any,
})
expect(result).toBeNull()
})

it('should return null for unhandled error kinds', () => {
const result = getRelevantFailedLabwareCmdFrom({
failedCommandByRunRecord: {
...failedCommand,
error: { errorType: 'SOME_UNHANDLED_ERROR' },
},
failedCommand: {
byRunRecord: {
...failedCommand,
error: { errorType: 'SOME_UNHANDLED_ERROR' },
},
} as any,
})
expect(result).toBeNull()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ describe('useRecoveryCommands', () => {

const props = {
runId: mockRunId,
failedCommandByRunRecord: mockFailedCommand,
failedCommand: {
byRunRecord: mockFailedCommand,
byAnalysis: mockFailedCommand,
},
unvalidatedFailedCommand: mockFailedCommand,
failedLabwareUtils: mockFailedLabwareUtils,
routeUpdateActions: mockRouteUpdateActions,
recoveryToastUtils: { makeSuccessToast: mockMakeSuccessToast } as any,
Expand Down Expand Up @@ -144,24 +148,29 @@ describe('useRecoveryCommands', () => {
'prepareToAspirate',
] as const).forEach(inPlaceCommandType => {
it(`Should move to retryLocation if failed command is ${inPlaceCommandType} and error is appropriate when retrying`, async () => {
const { result } = renderHook(() =>
useRecoveryCommands({
runId: mockRunId,
failedCommandByRunRecord: {
...mockFailedCommand,
commandType: inPlaceCommandType,
params: {
pipetteId: 'mock-pipette-id',
},
error: {
errorType: 'overpressure',
errorCode: '3006',
isDefined: true,
errorInfo: {
retryLocation: [1, 2, 3],
},
const { result } = renderHook(() => {
const failedCommand = {
...mockFailedCommand,
commandType: inPlaceCommandType,
params: {
pipetteId: 'mock-pipette-id',
},
error: {
errorType: 'overpressure',
errorCode: '3006',
isDefined: true,
errorInfo: {
retryLocation: [1, 2, 3],
},
},
}
return useRecoveryCommands({
runId: mockRunId,
failedCommand: {
byRunRecord: failedCommand,
byAnalysis: failedCommand,
},
unvalidatedFailedCommand: failedCommand,
failedLabwareUtils: mockFailedLabwareUtils,
routeUpdateActions: mockRouteUpdateActions,
recoveryToastUtils: {} as any,
Expand All @@ -171,7 +180,7 @@ describe('useRecoveryCommands', () => {
} as any,
selectedRecoveryOption: RECOVERY_MAP.RETRY_NEW_TIPS.ROUTE,
})
)
})
await act(async () => {
await result.current.retryFailedCommand()
})
Expand Down Expand Up @@ -245,7 +254,7 @@ describe('useRecoveryCommands', () => {

const testProps = {
...props,
failedCommandByRunRecord: mockFailedCmdWithPipetteId,
unvalidatedFailedCommand: mockFailedCmdWithPipetteId,
failedLabwareUtils: {
...mockFailedLabwareUtils,
failedLabware: mockFailedLabware,
Expand Down Expand Up @@ -312,7 +321,7 @@ describe('useRecoveryCommands', () => {

const testProps = {
...props,
failedCommandByRunRecord: mockFailedCommandWithError,
unvalidatedFailedCommand: mockFailedCommandWithError,
}

const { result, rerender } = renderHook(() =>
Expand Down Expand Up @@ -349,7 +358,7 @@ describe('useRecoveryCommands', () => {

const testProps = {
...props,
failedCommandByRunRecord: mockFailedCommandWithError,
unvalidatedFailedCommand: mockFailedCommandWithError,
}

mockUpdateErrorRecoveryPolicy.mockRejectedValueOnce(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ let mockMakeToast: Mock

const DEFAULT_PROPS: BuildToast = {
isOnDevice: false,
currentStepCount: 1,
stepCounts: {
currentStepNumber: 1,
hasRunDiverged: false,
totalStepCount: 1,
},
selectedRecoveryOption: RECOVERY_MAP.RETRY_SAME_TIPS.ROUTE,
commandTextData: { commands: [] } as any,
robotType: FLEX_ROBOT_TYPE,
Expand Down Expand Up @@ -187,13 +191,11 @@ describe('getStepNumber', () => {
})

it('should handle a falsy currentStepCount', () => {
expect(getStepNumber(RECOVERY_MAP.RETRY_SAME_TIPS.ROUTE, null)).toBe('?')
expect(getStepNumber(RECOVERY_MAP.RETRY_SAME_TIPS.ROUTE, null)).toBe(null)
})

it('should handle unknown recovery option', () => {
expect(getStepNumber('UNKNOWN_OPTION' as any, 3)).toBe(
'HANDLE RECOVERY TOAST OPTION EXPLICITLY.'
)
expect(getStepNumber('UNKNOWN_OPTION' as any, 3)).toBeNull()
})
})

Expand Down Expand Up @@ -234,17 +236,17 @@ describe('useRecoveryFullCommandText', () => {
expect(result.current).toBeNull()
})

it('should return stepNumber if it is a string', () => {
it('should return null if there is no current step count', () => {
const { result } = renderHook(() =>
useRecoveryFullCommandText({
robotType: FLEX_ROBOT_TYPE,
stepNumber: '?',
stepNumber: null,
commandTextData: { commands: [] } as any,
allRunDefs: [],
})
)

expect(result.current).toBe('?')
expect(result.current).toBeNull()
})

it('should truncate TC command', () => {
Expand Down
9 changes: 5 additions & 4 deletions app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export function useERUtils({
runStatus,
showTakeover,
allRunDefs,
unvalidatedFailedCommand,
labwareDefinitionsByUri,
}: ERUtilsProps): ERUtilsResults {
const { data: attachedInstruments } = useInstrumentsQuery()
Expand All @@ -100,7 +101,6 @@ export function useERUtils({
cursor: 0,
pageLength: 999,
})
const failedCommandByRunRecord = failedCommand?.byRunRecord ?? null

const stepCounts = useRunningStepCounts(runId, runCommands)

Expand All @@ -120,7 +120,7 @@ export function useERUtils({
)

const recoveryToastUtils = useRecoveryToasts({
currentStepCount: stepCounts.currentStepNumber,
stepCounts,
selectedRecoveryOption: currentRecoveryOptionUtils.selectedRecoveryOption,
isOnDevice,
commandTextData: protocolAnalysis,
Expand Down Expand Up @@ -152,7 +152,7 @@ export function useERUtils({
})

const failedLabwareUtils = useFailedLabwareUtils({
failedCommandByRunRecord,
failedCommand,
protocolAnalysis,
failedPipetteInfo,
runRecord,
Expand All @@ -161,7 +161,8 @@ export function useERUtils({

const recoveryCommands = useRecoveryCommands({
runId,
failedCommandByRunRecord,
failedCommand,
unvalidatedFailedCommand,
failedLabwareUtils,
routeUpdateActions,
recoveryToastUtils,
Expand Down
Loading

0 comments on commit 7ce9d4b

Please sign in to comment.