Skip to content

Set row numbers on last rows#24

Merged
severo merged 19 commits intomainfrom
set-row-numbers-on-last-rows
Dec 11, 2025
Merged

Set row numbers on last rows#24
severo merged 19 commits intomainfrom
set-row-numbers-on-last-rows

Conversation

@severo
Copy link
Contributor

@severo severo commented Dec 8, 2025

Big refactoring, aiming at simplifying the concepts and the code, and having more predictability.

Improvements:

  • stability: we refine the evaluation of row numbers only if it has changed significantly (>1%)
  • stability: simplify the fetch (evaluate the byte range to parse, then fetch and parse it), avoiding previous concurrency issues
  • last rows: if the end of the file was parsed, we ensure scrolling to the end of the table shows rows sticking to the end.

@severo severo force-pushed the set-row-numbers-on-last-rows branch from 8c71bef to 4b05a16 Compare December 8, 2025 14:51
@severo severo force-pushed the set-row-numbers-on-last-rows branch from 0f189a9 to d187b2a Compare December 11, 2025 14:15
@severo severo marked this pull request as ready for review December 11, 2025 14:15
@severo severo requested a review from Copilot December 11, 2025 14:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the CSV caching and row estimation logic to improve how row numbers are assigned, particularly for rows at the end of files. The changes separate estimation concerns from caching by introducing a new Estimator class and restructuring the CSVCache to focus purely on storage.

Key Changes:

  • Introduced a new Estimator class to handle row number estimation separately from caching
  • Refactored CSVCache to remove internal row tracking (firstRow) and event emission, simplifying its responsibility to pure data storage
  • Updated the fetch logic to use byte-offset-based estimation rather than row-number-based positioning

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test/dataframe.test.ts Updated tests to reflect new estimation behavior where out-of-bound fetches no longer throw and estimated row numbers are returned even for uncached rows
test/cache.test.ts Restructured tests to cover the new RowsCache and Estimator classes, removing tests for the old row-tracking approach
src/helpers.ts Added checkInteger function for validating integer values without the non-negative constraint
src/dataframe.ts Integrated Estimator class, refactored fetch logic to use byte offset estimation, and updated event handling to refresh estimates explicitly
src/cache.ts Major refactoring: introduced RowsCache and Estimator classes, removed firstRow tracking from CSVRange, simplified CSVCache to focus on storage operations
package.json Updated baseline-browser-mapping dependency from 2.8.12 to 2.9.5
package-lock.json Lockfile update for the dependency version change

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

severo and others added 7 commits December 11, 2025 10:31
@severo severo merged commit 1e8be86 into main Dec 11, 2025
4 checks passed
@severo severo deleted the set-row-numbers-on-last-rows branch December 11, 2025 14:33
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.

1 participant