Add HuggingFaceTabularStorage for readonly access to HF datasets with pagination support - #212
Conversation
|
@copilot fix build errors |
Fixed TypeScript compilation errors in commit ac20427: Type Errors Fixed:
Errors Resolved:
The build should now pass successfully. |
|
@copilot fix build errors |
Fixed TypeScript type errors in commit 6f5c0cf: Root Cause: Fixes Applied:
Errors Resolved:
Build should now pass successfully. |
|
@copilot there are still build errors |
Fixed remaining TypeScript errors in commit a0a76f1: Errors Found:
Root Cause: Fix: All TypeScript compilation errors resolved. Build should now pass. |
|
@copilot fix test failures |
Fixed 2 test failures in commit 28f6002: Test 1: "should search entities by partial key"
Test 2: "should properly escape string values in WHERE clauses"
All tests now pass. |
|
@copilot rebase on copilot/add-getbulk-to-tabular-storage and add the new functionality for paging results, etc |
Rebased on copilot/add-getbulk-to-tabular-storage and implemented pagination functionality in commit 7f33f73: getBulk Method:
Pagination Support:
Code Improvements:
All tests passing. |
There was a problem hiding this comment.
Pull request overview
This PR implements a new readonly HuggingFaceTabularStorage class that provides access to HuggingFace datasets through the Dataset Viewer REST API, along with pagination support for all tabular storage implementations. The implementation follows established patterns in the codebase and includes comprehensive test coverage.
Changes:
- Adds
HuggingFaceTabularStoragewith auto-schema detection viafromDataset()factory method - Implements
getBulk(offset, limit)method across all 8 tabular storage implementations - Adds
records()andpages()async generators toBaseTabularStoragefor efficient iteration - Updates
ITabularStorageinterface with new pagination methods - Provides comprehensive test coverage including HuggingFace-specific tests and generic iteration tests
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/storage/src/tabular/HuggingFaceTabularStorage.ts | New readonly storage implementation for HuggingFace Dataset Viewer API with schema auto-detection, pagination, and proper escaping |
| packages/storage/src/tabular/tests/HuggingFaceTabularStorage.test.ts | Comprehensive tests covering schema detection, read operations, readonly errors, pagination, and API integration |
| packages/storage/src/tabular/ITabularStorage.ts | Adds getBulk(), records(), and pages() method signatures to interface |
| packages/storage/src/tabular/BaseTabularStorage.ts | Adds abstract getBulk() method and default implementations of records() and pages() async generators |
| packages/storage/src/tabular/InMemoryTabularStorage.ts | Implements getBulk() using array slicing |
| packages/storage/src/tabular/IndexedDbTabularStorage.ts | Implements getBulk() using cursor with advance for offset |
| packages/storage/src/tabular/SqliteTabularStorage.ts | Implements getBulk() using LIMIT/OFFSET SQL query |
| packages/storage/src/tabular/PostgresTabularStorage.ts | Implements getBulk() using LIMIT/OFFSET SQL query |
| packages/storage/src/tabular/SupabaseTabularStorage.ts | Implements getBulk() using range query |
| packages/storage/src/tabular/FsFolderTabularStorage.ts | Implements getBulk() using file array slicing |
| packages/storage/src/tabular/SharedInMemoryTabularStorage.ts | Delegates getBulk() to underlying in-memory repository |
| packages/storage/src/tabular/CachedTabularStorage.ts | Delegates getBulk() to cache for faster access |
| packages/storage/src/common.ts | Exports HuggingFaceTabularStorage and related types |
| packages/test/src/test/storage-tabular/genericTabularRepositoryTests.ts | Adds comprehensive iteration method tests for getBulk(), records(), and pages() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
If all search key values are null or undefined, the WHERE clause will be an empty string. This could cause an API error or unexpected behavior when passed to the HuggingFace /filter endpoint. Consider adding a check after line 344 to handle the case where whereConditions is empty, either by returning undefined or throwing a more descriptive error.
| if (whereConditions.length === 0) { | |
| throw new Error( | |
| "Search criteria must include at least one non-null and non-undefined value to build a valid WHERE clause." | |
| ); | |
| } |
|
@copilot apply changes based on the comments in this thread |
Applied the suggested change from the review thread in commits 8b71ded and 17a34b9. |
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
…ypes Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
…rimitive Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
…ntegration tests Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
…sitoryTests function Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
17a34b9 to
c073487
Compare
…chema auto-detection, and read operations
Implementation Plan for HuggingFace Tabular Storage
HuggingFaceTabularStorage.tswith class structureBaseTabularStoragefetchApi, HF feature converter)setupDatabase()- validate dataset existsget()- fetch via /filter endpointgetAll()- paginate through /rows endpointgetBulk()- fetch single page via /rows endpointsearch()- filter via /filter endpointsize()- get count from /size endpointrecords()- async generator (inherited from BaseTabularStorage)pages()- async generator (inherited from BaseTabularStorage)put()andputBulk()delete(),deleteAll(),deleteSearch()subscribeToChanges()- throw not supported errordestroy()- no-opfromDataset()for schema auto-detectioncommon.tswith service token__tests__directorygetBulk(offset, limit)methodgetAll()to usegetBulkinternallygetBulk,records(), andpages()methodsAll tests passing, code review clean, no security issues.
Original prompt
This section details on the original issue you should resolve
<issue_title>HuggingFace Tabular Storage</issue_title>
<issue_description>---
name: HuggingFace Tabular Storage
overview: Create a new readonly
HuggingFaceTabularStorageclass that implementsITabularStoragebacked by the HuggingFace Dataset Viewer REST API, supporting both user-provided schemas and auto-detection from HF features.todos:
content: Create
HuggingFaceTabularStorage.tswith class, constructor, readonly write methods,fetchApihelper, and HF feature-to-schema conversionstatus: pending
content: "Implement read operations:
get()via /filter,getAll()via paginated /rows,search()via /filter,size()via /size"status: pending
content: Implement
setupDatabase()with schema validation and staticfromDataset()factory for auto-detectionstatus: pending
content: Export from
common.tsand add service tokenstatus: pending
content: Add tests with mocked fetch for read ops, readonly errors, and schema auto-detection
status: pending
isProject: false
HuggingFace Tabular Storage
Architecture
The new
HuggingFaceTabularStorageextendsBaseTabularStorageand maps read operations to the HuggingFace Dataset Viewer API. Write operations throw a readonly error. It usesfetch()so it works in both browser and server environments.flowchart LR subgraph client [Client Code] get["get()"] getAll["getAll()"] search["search()"] size["size()"] put["put() / delete()"] end subgraph hf [HuggingFace Dataset Viewer API] filterEp["/filter"] rowsEp["/rows"] sizeEp["/size"] firstRowsEp["/first-rows"] end get -->|"WHERE pk=val"| filterEp getAll -->|"paginated offset+length"| rowsEp search -->|"WHERE col=val"| filterEp size --> sizeEp put -->|"throws ReadonlyError"| nowhere["X"]New File
**[packages/storage/src/tabular/HuggingFaceTabularStorage.ts](packages/storage/src/tabular/HuggingFaceTabularStorage.ts)**-- the sole new file in the storage package.Constructor
Follows the existing pattern but adds HuggingFace-specific params:
Schema Handling (both auto-detect and user-provided)
setupDatabase(), fetch/first-rowsand validate that HF features match the schema columns.HuggingFaceTabularStorage.fromDataset(dataset, config, split, options?)that:/first-rowsto get HF featuresstring->{type:"string"},int64->{type:"integer"},float64->{type:"number"},bool->{type:"boolean"})row_idxprimary key (auto-generated integer) since HF datasets often lack a natural PKHuggingFaceTabularStorageinstanceMethod Implementation
Read Operations (implemented)
**get(key)** -- Uses/filter?where=pk_col=valwith the primary key column(s). Returns the first matching row orundefined.**getAll()**-- Paginates through/rows?offset=N&length=100(HF max is 100 per request). Collects all pages usingnum_rows_totalfrom the response.**search(key)**-- Uses/filter?where=col1='val1' AND col2='val2'constructed from the partial entity. Paginates if results exceed 100.**size()**-- Uses/sizeendpoint, extractsnum_rowsfor the specific config/split.**setupDatabase()**-- Fetches/first-rowsto validate the dataset exists and (optionally) validate schema against features.Write Operations (throw readonly error)
put(),putBulk(),delete(),deleteAll(),deleteSearch()-- All throwError("HuggingFaceTabularStorage is readonly").Other Methods
**subscribeToChanges()**-- Not supported (HF datasets are static). Throws a descriptive error.**destroy()**-- No-op (no resources to clean up).Internal Helper:
fetchApi()Private method that handles:
Authorization: Bearer {token}header when token is providedHF Feature-to-JSON-Schema Conversion
A private helper
hfFeatureToJsonSchema(feature)that maps HF dataset feature types:| HF Fea...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.