Skip to content

Conversation

@ddcc4
Copy link
Collaborator

@ddcc4 ddcc4 commented Nov 25, 2025

Overview

A customer reported that a parameterized protocol works with a short CSV file but fails with their longer CSV file: https://opentrons.slack.com/archives/C389UCULX/p1763376654727119

The root cause seems to be that in parse_as_csv(), we were truncating the CSV file to 1024 bytes before passing it to the csv.Sniffer. If that chops up a line at an inopportune place, it would cause the sniffer to fail with Could not determine delimiter.

We should just pass the whole CSV file to the sniffer.

From our meeting this morning, we said we would fix this in edge rather than trying to get it into RS 8.8.0.

Test Plan and Hands on Testing

Added a test case with a long CSV file derived from the customer report.

Risk assessment

Low.

I guess there's a small risk that there could be some horrifying junk in the CSV file after the first 1024 bytes, that the sniffer previously wouldn't see, that would now be visible to the sniffer. But I think overall, it's more correct to let the sniffer see the whole file, rather than arbitrarily cutting it off at 1024 bytes.

@ddcc4 ddcc4 requested a review from jbleon95 November 25, 2025 22:03
Copy link
Contributor

@jbleon95 jbleon95 left a comment

Choose a reason for hiding this comment

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

Looks good, I honestly can't remember why I limited it to the first kilobyte other than the fear of a giant CSV slowing things down, but perhaps that was unfounded. Fine with this going into edge without version control since it'll give us time to ensure nothing breaks.

@ddcc4 ddcc4 merged commit 7c675c1 into edge Dec 2, 2025
47 checks passed
@ddcc4 ddcc4 deleted the dc-csv-sniff-len branch December 2, 2025 18:10
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