Skip to content

fix(protocol-designer, shared-data, step-generation): always nick name top non-lid labware in a stack#21024

Open
rclarke0 wants to merge 5 commits intoedgefrom
AUTH-2770
Open

fix(protocol-designer, shared-data, step-generation): always nick name top non-lid labware in a stack#21024
rclarke0 wants to merge 5 commits intoedgefrom
AUTH-2770

Conversation

@rclarke0
Copy link
Contributor

Overview

Prevent nicknaming of lids and always nick names the top most non-lid labware in a stack

Test Plan and Hands on Testing

  • smoke tested by adjusting protocol attached in the bug ticket

lid smoke test.py

lid.bug.testing.mov

Changelog

  • added lid fixture object for unit tests
  • filter out lids from labwareList before getting the topLabwareId to pass into EditNickNameModal
  • adjusted getFullStackFromLabwares to return the largest stack at the end of the list of stacks to ensure the top labware in a stacker is what gets renamed.

Review requests

  • should I make a new getFullStackFromLabwares util or is this fine since all unit tests pass?

Risk assessment

  • High: adjusts widely used util for getting stack object

Closes AUTH-2770

@rclarke0 rclarke0 requested review from a team as code owners March 12, 2026 15:46
@rclarke0 rclarke0 requested review from TamarZanzouri and jerader and removed request for a team March 12, 2026 15:46
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 87.17949% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.28%. Comparing base (5aa94c0) to head (6bade75).
⚠️ Report is 12 commits behind head on edge.

Files with missing lines Patch % Lines
...rotocol-designer/src/components/organisms/utils.ts 66.66% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #21024      +/-   ##
==========================================
- Coverage   56.69%   56.28%   -0.42%     
==========================================
  Files        3967     3971       +4     
  Lines      328377   328890     +513     
  Branches    46499    46630     +131     
==========================================
- Hits       186187   185112    -1075     
- Misses     141970   143560    +1590     
+ Partials      220      218       -2     
Flag Coverage Δ
app 45.37% <2.56%> (-0.02%) ⬇️
protocol-designer 19.88% <87.17%> (+0.01%) ⬆️
step-generation 5.73% <38.46%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...onents/organisms/LabwareCardOverflowMenu/index.tsx 65.63% <100.00%> (-0.73%) ⬇️
.../src/pages/Designer/DeckSetup/SlotOverflowMenu.tsx 48.10% <100.00%> (+0.44%) ⬆️
shared-data/js/labware.ts 97.50% <ø> (ø)
shared-data/labware/fixtures/2/index.ts 100.00% <ø> (ø)
step-generation/src/utils/misc.ts 62.28% <100.00%> (+0.23%) ⬆️
...rotocol-designer/src/components/organisms/utils.ts 65.16% <66.66%> (+0.16%) ⬆️

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

},
})
const topLabwareId = labwareIds[0]
const stackOnlyHasLids =
Copy link
Contributor

Choose a reason for hiding this comment

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

this can probably move to a utils?

lw.stack.includes(HOPPER_STACKER_LOCATION) === isOnHopper
)
.sort((a, b) => b.stack.length - a.stack.length)[0]?.stack ?? []
.sort((a, b) => a.stack.length - b.stack.length)
Copy link
Contributor

Choose a reason for hiding this comment

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

will this change not affect other places that use this method?

Comment on lines +1071 to +1085
const labwareStack = Object.values(labware).filter(
lw =>
lw.stack.includes(mappedLocation) &&
(offDeckOverrideId == null || lw.stack.includes(offDeckOverrideId)) &&
lw.stack.includes(HOPPER_STACKER_LOCATION) === isOnHopper
)
const index = returnLargestStack ? -1 : 0
return (
Object.values(labware)
.filter(
lw =>
lw.stack.includes(mappedLocation) &&
(offDeckOverrideId == null || lw.stack.includes(offDeckOverrideId)) &&
lw.stack.includes(HOPPER_STACKER_LOCATION) === isOnHopper
labwareStack
.sort(
returnLargestStack
? (a, b) => a.stack.length - b.stack.length
: (a, b) => b.stack.length - a.stack.length
)
.sort((a, b) => b.stack.length - a.stack.length)[0]?.stack ?? []
.at(index)?.stack ?? []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you give some color on why this change is needed?

Comment on lines +115 to +116
const allLabwareNotLids = getAllLabwareWithoutLids(deckSetup)
const topLabwareThatIsNotALid = deckSetupLabware[allLabwareNotLids[0]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry if I'm not following, but doesn't allLabwareNotLids return all labware across the deck without a lid? And topLabwareThatIsNotALid will arbitrarily pick the first one of those un-lidded labwares?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants