Skip to content

fetch the expected number of rows, instead of stopping at the expecte…#29

Merged
severo merged 1 commit into
mainfrom
28-change-fetch-stop-criteria
Dec 11, 2025
Merged

fetch the expected number of rows, instead of stopping at the expecte…#29
severo merged 1 commit into
mainfrom
28-change-fetch-stop-criteria

Conversation

@severo

@severo severo commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

…d byte offset

fixes #28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request modifies the CSV data frame fetching logic to fetch a specific number of rows rather than stopping at an estimated byte offset. This change aims to improve the accuracy of row fetching when byte offset estimates are imprecise, particularly in CSVs with variable row sizes.

Key changes:

  • Replace byte-offset-based termination with row-count-based termination
  • Add stats.valid counter to track parsed non-empty rows
  • Update test expectations to reflect the new fetching behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/dataframe.ts Modified fetch logic to count rows instead of checking byte offsets; added stats.valid counter and changed loop termination condition
test/dataframe.test.ts Updated test expectations for resolve event counts and adjusted comments to reflect that row size estimation "might" refresh (previously said "refreshes")

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dataframe.ts
@severo severo merged commit 36a3767 into main Dec 11, 2025
10 checks passed
@severo severo deleted the 28-change-fetch-stop-criteria branch December 11, 2025 15:09
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.

Ensure at least the expected number of rows have been fetched

2 participants