fix(app): Add special casing for Stacker combination fixtures in deck conflict resolution by smb2268 · Pull Request #18988 · Opentrons/opentrons

smb2268 · 2025-07-21T21:14:28Z

Overview

This PR rounds out the necessary work for the modules & fixtures deck conflict resolution section of protocol setup on desktop and ODD. See changelog for specifics!

Test Plan and Hands on Testing

On both Desktop and ODD, check the following:

For a protocol that needs a waste chute + stacker combination:

This displays correctly as a combination requirement
You can resolve this when you don't have a stacker or waste chute configured in deck config
You can resolve this when you have a waste chute OR stacker configured and need to add the other

For a protocol that needs a stacker + mag block combination:

These display correctly as individual items in the list
You can resolve these in either order without messing up the other

For a protocol that needs a mag block + staging area combination

These display correctly as individual items in the list
You can resolve these in either order without messing up the other

Verify that waste chutes and mag blocks are visible when combined with stacker fixtures

Changelog

Add new fixture images for waste chute + stacker and single slots and wire these up for display on desktop
Update render ordering in BaseDeck so that waste chutes and mag blocks that are combo fixtures with stacker appear on the deck map - currently they are hidden
Add ODDFixtureOption which is a copy of fixture option but with ODD only buttons that can’t be used in components, other fixture option is also used in Protocol Designer. Refactor relevant modals to use this component which closes EXEC-1634
Refactor fixture section of setup to show Mag Block combination fixtures as individual items. Extract some logic in the ODD and Desktop versions of this FixtureTable and SetupFixtureList to utilities so we aren't duplicating this logic.
Update SetupModulesList and ModuleTable to display Flex Stacker + waste chute combination correctly
Create patchDeckConfigForRequiredFixture that enables us to add a fixture without removing a combination fixture. For example, if you have a waste chute in D3 and need to add a Flex Stacker, this will be patched into a waste chute flex stacker combination fixture
Use NotConfiguredModal instead of AddFixtureModal for case in which you configure a non-conflicted fixture on ODD, which allows us to remove provided fixture logic in AddFixtureModal and remove deck config page of protocol setup, which seems to be a legacy page that was only shown under a modal in this specific instance. Move NotConfiguredModal to a shared location to enable this

Review requests

Take a look at the logic, especially Tamar :)

Risk assessment

Medium, but its been thoroughly tested

codecov · 2025-07-21T21:18:16Z

Codecov Report

Attention: Patch coverage is 3.41530% with 707 lines in your changes missing coverage. Please review.

Project coverage is 23.43%. Comparing base (8278911) to head (a62c2f8).
Report is 247 commits behind head on edge.

Files with missing lines	Patch %	Lines
...Modal/getFilteredDeckConfigFixtureCompatibility.ts	0.00%	104 Missing ⚠️
components/src/hardware-sim/BaseDeck/BaseDeck.tsx	0.00%	67 Missing ⚠️
...nisms/LocationConflictModal/NotConfiguredModal.tsx	0.00%	64 Missing ⚠️
...ConflictModal/patchDeckConfigForRequiredFixture.ts	0.00%	60 Missing ⚠️
...rotocolRun/SetupModuleAndDeck/SetupModulesList.tsx	0.00%	54 Missing ⚠️
...Setup/ProtocolSetupModulesAndDeck/FixtureTable.tsx	0.00%	49 Missing ⚠️
...rotocolRun/SetupModuleAndDeck/SetupFixtureList.tsx	0.00%	48 Missing ⚠️
...DeviceDetailsDeckConfiguration/AddFixtureModal.tsx	0.00%	47 Missing ⚠️
...lSetup/ProtocolSetupModulesAndDeck/ModuleTable.tsx	0.00%	37 Missing ⚠️
app/src/molecules/ODDFixtureOption/index.tsx	0.00%	35 Missing ⚠️
... and 9 more

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #18988      +/-   ##
==========================================
- Coverage   23.85%   23.43%   -0.42%     
==========================================
  Files        3354     3364      +10     
  Lines      294222   294902     +680     
  Branches    30992    31027      +35     
==========================================
- Hits        70173    69106    -1067     
- Misses     224029   225771    +1742     
- Partials       20       25       +5

Flag	Coverage Δ
protocol-designer	`18.97% <3.41%> (-0.12%)`	⬇️
step-generation	`5.32% <1.09%> (+0.06%)`	⬆️

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

Files with missing lines	Coverage Δ
.../organisms/DeviceDetailsDeckConfiguration/utils.ts	`0.00% <ø> (ø)`
app/src/organisms/ODD/ProtocolSetup/index.ts	`0.00% <ø> (ø)`
app/src/organisms/ODD/ProtocolSetup/types.ts	`100.00% <ø> (ø)`
app/src/pages/ODD/ProtocolSetup/index.tsx	`0.00% <ø> (ø)`
components/src/organisms/FixtureOption/index.tsx	`95.12% <100.00%> (+30.41%)`	⬆️
...organisms/HardwareConfigurator/AddFixtureModal.tsx	`70.88% <ø> (-0.07%)`	⬇️
shared-data/js/constants.ts	`100.00% <100.00%> (ø)`
...p/Devices/ProtocolRun/SetupModuleAndDeck/index.tsx	`0.00% <0.00%> (ø)`
...etupModulesAndDeck/ProtocolSetupModulesAndDeck.tsx	`0.00% <0.00%> (ø)`
...sources/runs/useModuleRenderInfoForProtocolById.ts	`0.00% <0.00%> (ø)`
... and 16 more

... and 234 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.

SyntaxColoring · 2025-07-21T21:57:05Z

+        const slotName =
+          getModuleType(module.model) === FLEX_STACKER_MODULE_TYPE
+            ? getStackerLocationFromSlotName(module.location.slotName)
+            : module.location.slotName


Is this right?

StackedItemsOnDeck is defined as:

export interface StackedItemsOnDeck { [slotName: string]: StackItem[] }

Which reads to me like it should always be like "A1": [stackItem1, stackItem2, ...], where "A1" is guaranteed to be a slot ID in the deck definition.

But this looks like it will return something like "STACKER A": [stackItem1, stackItem2, ...], so I'd expect downstream things to have trouble when they try to look up "STACKER A" as a slot ID in the deck def.

Yeah it is correct based on the implementation for this util which is created around UI concerns for protocol setup. Items represented under "STACKER A": [stackItem1, stackItem2, ...] are in the hopper location, which doesn't exist in the deck definition. Items under "A4" will be stacked on the shuttle location. All users of this util expect this format, but I can update the comment to document this!

Is there any way this can be made more type-safe? Like this, roughly, instead of a string?

type LocationForStackedItemsOnDeck = | { type: "stackerHopper", row: string // "A" - "D" } | { type: "offDeck" } | { type: "addressableAreaName" aaName: string }

You couldn't use these as keys for an object, but you could use them as keys for a Map. Or, instead of returning an object, you could return an array of objects where each object has a location property.

If that's not within reach, then at a minimum, yeah, some doc comments authoritatively describing the full format would be helpful. And I guess slotName in the interface should be called something other than slotName.

Yeah that's a good suggestion, I think the refactor is out of scope for this PR but I can make a follow up ticket to address this and for now rename the variable and add some better documentation!

TamarZanzouri · 2025-07-22T14:23:41Z

  compatibleCutoutFixtureIds,
  deckDef,
  robotName,
+  fakeCutoutFixtureId,


can you explain what is this prop for? is it always a fake cutout fixture? can we combine it with the cutoutFixtureId?

This prop is used in cases where we need to split a mag block combination fixture into two items, and represents the cutoutFixtureId that we should use for the display image and name in the UI. For example, if we have a flexStackerModuleV1WithMagneticBlockV1 fixture, we only want to render this as a magneticBlockV1 fixture on the UI but we want to maintain that the protocol requires the combination fixture in order to run, so we can't override the other props. I added this extra prop fakeCutoutFixtureId that in this case will equal magneticBlockV1 and if it exists, will be used for render purposes. Maybe a name like requiredCutoutFixtureIdForDisplay would make more sense here?

Renamed to partialRequiredCutoutFixtureId - let me know what you think!

TamarZanzouri · 2025-07-22T14:29:36Z

+          // if the module is a flex stacker in D4, check if it needs a waste chute
+          // combo fixture
+          if (
+            moduleDef.moduleType === FLEX_STACKER_MODULE_TYPE &&


can we check that the cutoutId === WASTE_CHUTE_CUTOUT instead of slot name?

We don't have access to the cutoutId here, we just have the slot name that the module is loaded into!

TamarZanzouri · 2025-07-22T14:31:34Z

+            const deckConfigCompatabilityD3 = deckConfigCompatibility?.find(
+              configItem => configItem.cutoutId === 'cutoutD3'
+            )
+            if (
+              deckConfigCompatabilityD3 != null &&
+              WASTE_CHUTE_FLEX_STACKER_FIXTURES.includes(
+                deckConfigCompatabilityD3?.compatibleCutoutFixtureIds[0]
+              )
+            ) {
+              const comboFixtureId =
+                deckConfigCompatabilityD3?.compatibleCutoutFixtureIds[0]
+              const comboFixtureConflict = !deckConfigCompatabilityD3?.compatibleCutoutFixtureIds.includes(
+                deckConfigCompatabilityD3.cutoutFixtureId
+              )


we can use replaceCutoutFixtureWithComboFixture if its helpful?

TamarZanzouri · 2025-07-22T15:03:58Z

  addressableAreaId: AddressableAreaNamesWithFakes,
  deckDefinition: DeckDefinition
 ): CutoutConfigMap[][] => {
-  if (providedFixtureOptions != null) {


why was this removed?

This is what we were discussing yesterday - I was able to replace the usage of AddFixtureModal with NotConfiguredModal on ODD deck hardware setup. This was the only place that the providedFixtureOptions prop was used so I removed all of the logic related to it.

TamarZanzouri · 2025-07-22T15:07:44Z

-            ...existingCutoutConfig,
-            cutoutFixtureId: replacementCutoutFixtureId,
-            opentronsModuleSerialNumber: moduleSerialNumber,
+          if (


nice! you can also use the combo fixture method but you will need the aa

TamarZanzouri · 2025-07-22T15:09:20Z

+export const isFixtureCompatible = (
+  cutoutFixtureId: CutoutFixtureId,
+  compatibleCutoutFixtureIds: CutoutFixtureId[],
+  fakeCutoutFixtureId?: CutoutFixtureId


im confused about this prop name can you elaborate?

This is the same field as I described above -- here, cutoutFixtureId is the current CutoutFixtureId in the existing deck config, compatibleCutoutFixtureIds is an array that includes all possible CutoutFixtureId that can satisfy the protocol's hardware requirements, and fakeCutoutFixtureId if it exists will be the fixture id that for the current item assessing compatibility for.

For example, if a protocol requires a mag block with staging area combination fixture we'd need to determine compatibility for each the mag block and the staging area and show a status for them in hardware setup. If you had a staging area configured but no mag block, you'd see a connected status for the staging area item and a not configured status for the mag block.

The inputs/outputs of the function in this case would be as follows:
For Mag Block -
isFixtureCompatible('stagingAreaRightSlot', ['stagingAreaSlotWithMagneticBlockV1'], 'magneticBlockV1') will evaluate to false
For Staging Area -
isFixtureCompatible('stagingAreaRightSlot', ['stagingAreaSlotWithMagneticBlockV1'], 'stagingAreaRightSlot') will evaluate to true

Added some clarifying comments to the functions here too

TamarZanzouri

overall looks great! had a few questions and suggestions

… for waste chute + stacker

…ure in odd conflict resolution

TamarZanzouri

looks great! thank you for addressing the comments!

smb2268 requested review from TamarZanzouri and ahiuchingau July 21, 2025 21:14

smb2268 self-assigned this Jul 21, 2025

smb2268 requested review from a team as code owners July 21, 2025 21:14

koji reviewed Jul 21, 2025

View reviewed changes

Comment thread components/src/organisms/FixtureOption/index.tsx Outdated

koji reviewed Jul 21, 2025

View reviewed changes

Comment thread app/src/molecules/ODDFixtureOption/index.tsx Outdated

koji reviewed Jul 21, 2025

View reviewed changes

Comment thread app/src/molecules/ODDFixtureOption/index.tsx Outdated

smb2268 requested review from a team and SyntaxColoring July 21, 2025 21:47

koji reviewed Jul 21, 2025

View reviewed changes

Comment thread app/src/molecules/ODDFixtureOption/index.tsx Outdated

SyntaxColoring reviewed Jul 21, 2025

View reviewed changes

ahiuchingau reviewed Jul 22, 2025

View reviewed changes

Comment thread app/src/assets/images/single_right_slot.png

TamarZanzouri reviewed Jul 22, 2025

View reviewed changes

Comment thread app/src/organisms/LocationConflictModal/patchDeckConfigForRequiredFixture.ts

TamarZanzouri requested changes Jul 22, 2025

View reviewed changes

smb2268 requested a review from TamarZanzouri July 22, 2025 19:44

smb2268 added 6 commits July 22, 2025 15:47

Fix bug where magblock is rendered underneath stacker

666e457

Add new images for single slot fixtures and stacker + waste chute combo

2fb21ca

Fix import

822734c

Add waste chute on top of stacker for combo fixtures

c0ecc84

Add new constant

3003213

Desktop special case for stacker + waste chute in deck hardware

8d6f453

smb2268 added 7 commits July 22, 2025 15:47

Code cleanup, final split for mag block/staging area and special case…

feebab2

… for waste chute + stacker

Small cleanups found while testing

b8addc1

linting

2fb0132

Use NotConfiguredModal instead of AddFixtureModal for adding new fixt…

79c998f

…ure in odd conflict resolution

PR cleanup

7a55fee

PR cleanups - clarifying comments, variable rename, component streamline

fa4db13

Add detail to util comments

bdb0899

smb2268 force-pushed the app_deck-conflict-resolution branch from 68ecd5a to bdb0899 Compare July 22, 2025 19:48

smb2268 added 2 commits July 22, 2025 16:01

Swap single right slot with smaller png

2ff535a

Fix staging area display locations

a62c2f8

TamarZanzouri approved these changes Jul 22, 2025

View reviewed changes

smb2268 merged commit 9317ea9 into edge Jul 22, 2025
54 of 55 checks passed

smb2268 deleted the app_deck-conflict-resolution branch July 22, 2025 21:10

Conversation

smb2268 commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Test Plan and Hands on Testing

Changelog

Review requests

Risk assessment

Uh oh!

codecov bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smb2268 Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TamarZanzouri Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TamarZanzouri Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TamarZanzouri left a comment

Choose a reason for hiding this comment

Uh oh!

TamarZanzouri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

smb2268 commented Jul 21, 2025 •

edited

Loading

codecov bot commented Jul 21, 2025 •

edited

Loading

smb2268 Jul 22, 2025 •

edited

Loading

TamarZanzouri Jul 22, 2025 •

edited

Loading

TamarZanzouri Jul 22, 2025 •

edited

Loading