Skip to content

Conversation

@rclarke0
Copy link
Contributor

@rclarke0 rclarke0 commented Dec 9, 2025

Overview

Generate hopper full error on fill and store steps

Test Plan and Hands on Testing

  • unit tests passed, need to wait to smoke test

Changelog

  • added a helper function to determine if the hopper has space for an additional labware

Risk assessment

low

Partially closes EXEC-1448

@rclarke0 rclarke0 requested review from jerader and ncdiehl11 December 9, 2025 16:03
@rclarke0 rclarke0 changed the title feat(step-generation): Generate hopper full error on fill and store stacker steps feat(protocol-designer, step-generation): Generate hopper full error on fill and store stacker steps Dec 9, 2025
@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.08%. Comparing base (78ef965) to head (3493d39).
⚠️ Report is 27 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #20329      +/-   ##
==========================================
- Coverage   25.58%   25.08%   -0.51%     
==========================================
  Files        3636     3635       -1     
  Lines      302933   302887      -46     
  Branches    42346    42423      +77     
==========================================
- Hits        77520    75972    -1548     
- Misses     225389   226887    +1498     
- Partials       24       28       +4     
Flag Coverage Δ
step-generation 5.63% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
...tion/src/commandCreators/atomic/flexStackerFill.ts 100.00% <100.00%> (ø)
...ion/src/commandCreators/atomic/flexStackerStore.ts 100.00% <100.00%> (ø)
step-generation/src/errorCreators.ts 71.64% <100.00%> (+0.43%) ⬆️
step-generation/src/types.ts 100.00% <ø> (ø)
step-generation/src/utils/misc.ts 62.12% <100.00%> (+0.23%) ⬆️

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


export const flexStackerHopperFull = (): CommandCreatorError => {
return {
type: 'HOPPER_FULL',
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 add this to the i18n file as well? I think it is alert.json

...(count != null ? [`count=${count}`] : []),
...(message != null ? [`message=${formatPyStr(message)}`] : []),
]
// TODO: add mismatched labware error ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh ya we probably do want the mismatch labware error in here too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

…for mismatched labware and description for full hopper
@rclarke0 rclarke0 requested a review from jerader December 9, 2025 18:53
const maximumAllowedLabware = stackerState?.maxPoolCount ?? 0
const labwareStored = stackerState?.labwareInHopper
const numberOfLabwareStored = labwareStored?.length ?? 0
return numberOfLabwareStored + 1 < maximumAllowedLabware
Copy link
Collaborator

@jerader jerader Dec 9, 2025

Choose a reason for hiding this comment

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

i think we don't need this +1. We're checking if there is space in the hopper at all at this point. With the +1, you could add 1 too many labwares, right?

Suggested change
return numberOfLabwareStored + 1 < maximumAllowedLabware
return numberOfLabwareStored < maximumAllowedLabware

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes you are right over thought this one!

Comment on lines 30 to 46
if (labwareToStore) {
for (const labware of labwareToStore) {
if (
labwareMatchesLabwareInHopper(
labware.primaryLabwareId,
invariantContext,
flexStackerState
)
) {
continue
} else {
return {
errors: [errorCreators.flexStackerLabwareTypeMismatch()],
}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this way totally works! but it is a bit cleaner, imo, to use .every()

Suggested change
if (labwareToStore) {
for (const labware of labwareToStore) {
if (
labwareMatchesLabwareInHopper(
labware.primaryLabwareId,
invariantContext,
flexStackerState
)
) {
continue
} else {
return {
errors: [errorCreators.flexStackerLabwareTypeMismatch()],
}
}
}
}
if (labwareToStore) {
const allMatch = labwareToStore.every(labware =>
labwareMatchesLabwareInHopper(
labware.primaryLabwareId,
invariantContext,
flexStackerState
)
)
if (!allMatch) {
return {
errors: [errorCreators.flexStackerLabwareTypeMismatch()],
}
}
}

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.

nice job! just left a few minor comments

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