Skip to content

Conversation

rclarke0
Copy link
Contributor

@rclarke0 rclarke0 commented Aug 5, 2025

Overview

Ensure Agreement between Top Frustum Dimensions and Well Dimensions

Test Plan and Hands on Testing

  • Reverifying inner well geometry of nest_12_reservoir_22ml and nest_8_reservoir_22ml with hardware-testing/hardware_testing/protocols/liquid_sense/lld_test_liquid_height_3mm.py to ensure it still passes

Changelog

  • Added test in labwareDefSchemaV2.test.ts to verify that the top frustum diameter, x, and y dimensions match the corresponding well dimensions (sharing the same geometryDefinitionId) within ±1 mm.
  • Fixed typo in eppendorf_96_wellplate_1000ul . The inner well geometry had radii rather than diameter
  • Updated nest_12_reservoir_22ml and nest_8_reservoir_22ml because we were given a more detailed labware drawing
  • Fixed nest_1_reservoir_195ml inner well geometry dimensions. The xDimension and yDimension were switched
  • Updated opentrons_24_aluminumblock_nest_1.5ml_snapcap diameter to match inner well geometry topDiameter and opentrons_24_tuberack_nest_1.5ml_snapcap since they are the same labware
  • Updated opentrons_24_aluminumblock_nest_2ml_snapcap diameter to match inner well geometry and opentrons_24_tuberack_nest_2ml_snapcap since they are the same labware
  • Updated opentrons_24_tuberack_eppendorf_1.5ml_safelock_snapcap to match inner well geometry and technical drawing
  • Updated opentrons_24_tuberack_eppendorf_2ml_safelock_snapcap to match inner well geometry and technical drawing
  • Updated opentrons_tough_1_reservoir_300ml yDimension and xDimension to match inner well geometry and technical drawing

Review requests

  • Should old labware corrections be made into a new version file?

Risk assessment

  • Users with currently released labware: nest_1_reservoir_195ml, opentrons_24_tuberack_eppendorf_1.5ml_safelock_snapcap, opentrons_24_tuberack_nest_1.5ml_snapcap, opentrons_24_tuberack_nest_2ml_snapcap, opentrons_24_tuberack_eppendorf_2ml_safelock_snapcap
    may experience slight shifts in their LPC values

Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.33%. Comparing base (c2c764a) to head (995b2c6).
⚠️ Report is 37 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #19126      +/-   ##
==========================================
+ Coverage   23.84%   24.33%   +0.48%     
==========================================
  Files        3373     3375       +2     
  Lines      296660   294990    -1670     
  Branches    31486    37849    +6363     
==========================================
+ Hits        70736    71778    +1042     
+ Misses     225901   223186    -2715     
- Partials       23       26       +3     
Flag Coverage Δ
app 0.97% <ø> (-0.26%) ⬇️
protocol-designer 18.92% <ø> (-0.02%) ⬇️
step-generation 5.37% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 397 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.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Nice, thank you! Mostly looks good to me, but a handful of requests:

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated nest_12_reservoir_22ml and nest_8_reservoir_22ml because we were given a more detailed labware drawing

For traceability and reviewability, could we leave some kind of breadcrumb trail to that new labware drawing? The way I usually do it is have the PR description link to a Jira ticket, and have the Jira ticket link to any internal company resources like Slack threads and Google Drive links. (And if all I have is a Slack DM that can't be linked, I just copy-paste quote the contents into the Jira ticket.)

@rclarke0 rclarke0 marked this pull request as ready for review August 6, 2025 17:56
@rclarke0 rclarke0 requested a review from SyntaxColoring August 6, 2025 17:56
@rclarke0 rclarke0 requested review from a team as code owners August 6, 2025 17:57
@rclarke0 rclarke0 requested review from ncdiehl11 and removed request for a team August 6, 2025 17:57
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

@rclarke0 is out today. At her request, I'm taking this over so it gets into v8.6.0-alpha.0.

  • eppendorf_96_wellplate_1000ul: New numbers LGTM based on publicly available information, this drawing which I got from this store page. Thank you!
  • nest_1_reservoir_195ml: Fix to swapped x/y makes sense, thank you. I'm deferring this to #19127 where it can be combined with an unrelated fix in that same definition, to avoid having too many disruptive version bumps. (Every new version counts as a "new labware" for the purposes of Labware Position Check, which can be annoying.)
  • nest_8_reservoir_22ml: I'm not confident I'm looking at the right drawings, so basically just trusting that this is correct.
  • nest_12_reservoir_22ml: Ditto.
  • opentrons_tough_1_reservoir_300ml: New numbers make sense, and I had the exact same fix in another branch. For posterity, here's how I arrived there:
    • There's a drawing "SINGLE WELL RESERVOIR SUPPORT DWG" rev A1.0 somewhere in Slack.
    • xDimension 108->108.7: The old value matched the bottom of the well, but a well's xDimension needs to represent the top of the well. 108.7 matches the drawing and the number in innerLabwareGeometry, so it seems right.
    • yDimension 72->70.7: I'm not sure where the old value of 72 came from, but 70.7 matches the drawing and the number in innerLabwareGeometry, so it seems right.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Will merge when CI passes.

@SyntaxColoring
Copy link
Contributor

macOS test runners are clogged up, but I think we're fully covered by the other ones.

@SyntaxColoring SyntaxColoring merged commit e5b41f7 into edge Aug 8, 2025
73 of 78 checks passed
@SyntaxColoring SyntaxColoring deleted the fix-eppendorf-1000ul branch August 8, 2025 20:14
@SyntaxColoring
Copy link
Contributor

SyntaxColoring commented Aug 8, 2025

Ack, this needed to go into the release branch but there was some confusing back-and-forth about when that would be cut and it looks like I missed it by a couple hours. Will need to cherry-pick this.

Edit: #19181

SyntaxColoring added a commit that referenced this pull request Aug 8, 2025
…sions

Co-authored-by: Max Marrone <[email protected]>
(cherry picked from commit e5b41f7 / PR #19126)
SyntaxColoring added a commit that referenced this pull request Aug 9, 2025
…sions (#19181)

Cherry picked from commit e5b41f7 / PR #19126.

Co-authored-by: Rhyann Clarke <[email protected]>
neo-jesse pushed a commit that referenced this pull request Aug 19, 2025
neo-jesse pushed a commit that referenced this pull request Aug 19, 2025
…sions (#19181)

Cherry picked from commit e5b41f7 / PR #19126.

Co-authored-by: Rhyann Clarke <[email protected]>
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.

2 participants