Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(protocol-designer, step-generation): update prewet behavior in PD #17433

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

ncdiehl11
Copy link
Collaborator

@ncdiehl11 ncdiehl11 commented Feb 5, 2025

Overview

This PR adds prewet to distribute, and updates the logic for when to use prewet for all compound liquid handling command creators. The prewet volume should always be set to the volume of the aspiration that is about to be called (including disposal volume for distribute). Prewet commands should be emmitted anytime a new tip is used for an aspiration. Also, I update the UI for the prewet form field in MoveLiquidTools

Closes AUTH-908

Test Plan and Hands on Testing

UI

  • create or open transfer step form, and navigate to aspirate advanced settings
  • verify that prewet form field UI is correct

Step Generation

  • Inspect logic for new prewet tip commands in distribute command creators

Changelog

  • add prewet to distribute command creator
  • update UI for prewet step form field
  • refactor ToggleStepFormField to accommodate checkbox in addition to toggle button
  • update tests

Review requests1

  • see test plan

Risk assessment

medium. touches distribute command creator

This PR adds prewet to distribute, and updates the logic for when to use prewet for all compound liquid handling command creators. The prewet volume should always be set to the volume of the aspiration that is about to be called (including disposal volume for `distribute`). Prewet commands should be emmitted anytime a new tip is used for an aspiration. Also, I update the UI for the prewet form field in MoveLiquidTools

Closes AUTH-908
@ncdiehl11 ncdiehl11 self-assigned this Feb 5, 2025
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

Attention: Patch coverage is 5.97015% with 63 lines in your changes missing coverage. Please review.

Project coverage is 18.15%. Comparing base (643cf15) to head (09c3c3b).
Report is 9 commits behind head on edge.

Files with missing lines Patch % Lines
...eration/src/commandCreators/compound/distribute.ts 16.00% 21 Missing ⚠️
step-generation/src/fixtures/commandFixtures.ts 0.00% 20 Missing ⚠️
...signer/src/molecules/ToggleStepFormField/index.tsx 0.00% 13 Missing ⚠️
...Steps/StepForm/StepTools/MoveLiquidTools/index.tsx 0.00% 6 Missing ⚠️
...tocolSteps/StepForm/PipetteFields/LabwareField.tsx 0.00% 1 Missing ⚠️
...esigner/ProtocolSteps/StepForm/StepFormToolbox.tsx 0.00% 1 Missing ⚠️
...eneration/src/commandCreators/compound/transfer.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17433      +/-   ##
==========================================
- Coverage   18.16%   18.15%   -0.01%     
==========================================
  Files        3171     3171              
  Lines      229418   229466      +48     
  Branches     6883     6886       +3     
==========================================
+ Hits        41665    41669       +4     
- Misses     187753   187797      +44     
Flag Coverage Δ
protocol-designer 17.34% <5.97%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tocolSteps/StepForm/PipetteFields/LabwareField.tsx 8.00% <0.00%> (-0.34%) ⬇️
...esigner/ProtocolSteps/StepForm/StepFormToolbox.tsx 5.07% <0.00%> (-0.02%) ⬇️
...eneration/src/commandCreators/compound/transfer.ts 53.04% <0.00%> (ø)
...Steps/StepForm/StepTools/MoveLiquidTools/index.tsx 0.60% <0.00%> (-0.01%) ⬇️
...signer/src/molecules/ToggleStepFormField/index.tsx 3.50% <0.00%> (-0.20%) ⬇️
step-generation/src/fixtures/commandFixtures.ts 0.00% <0.00%> (ø)
...eration/src/commandCreators/compound/distribute.ts 46.66% <16.00%> (-1.69%) ⬇️

@ncdiehl11 ncdiehl11 requested review from koji and syao1226 February 5, 2025 17:28
@ncdiehl11 ncdiehl11 marked this pull request as ready for review February 5, 2025 17:28
@ncdiehl11 ncdiehl11 requested a review from a team as a code owner February 5, 2025 17:28
@ncdiehl11 ncdiehl11 removed the request for review from a team February 5, 2025 17:35
@ncdiehl11 ncdiehl11 changed the title fix(protocol-designer,-step-generation): update prewet behavior in PD fix(protocol-designer, step-generation): update prewet behavior in PD Feb 5, 2025
@jerader
Copy link
Collaborator

jerader commented Feb 5, 2025

nice! This would mean a change in behavior for old protocols? lets remember to mention this in the 8.5 announcement modal

@ncdiehl11
Copy link
Collaborator Author

nice! This would mean a change in behavior for old protocols? lets remember to mention this in the 8.5 announcement modal

I think the intended behavior for distribute + prewet was missing all along inadvertently. But yes, I will make a note to include this in the 8.5 announcement!

@ncdiehl11 ncdiehl11 requested a review from jerader February 5, 2025 19:35
Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@ncdiehl11 ncdiehl11 merged commit aae2bfa into edge Feb 6, 2025
27 of 28 checks passed
@ncdiehl11 ncdiehl11 deleted the feat_pd-prewet-update branch February 6, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants