Repository navigation
Add production Lance inference and async transform primitives - #259
Conversation
This reverts commit de7ea26.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2340441281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| ) = self._fill_add_columns_fragments( | ||
| lance, | ||
| source_fragment_ids, | ||
| output_schema, | ||
| lance_schema, | ||
| ) |
There was a problem hiding this comment.
When the destination advances beyond source_version before the reducer runs, this call writes fill-only Lance files for every omitted fragment, but the version check later raises before reaching the commit try/cleanup block. Because these reducer-created paths are not persisted in worker sidecars, subsequent retries cannot discover them, leaving potentially dataset-sized orphan files; validate the current version before filling or cover all post-fill validation failures with cleanup.
Useful? React with 👍 / 👎.
2340441 to
4b2d3d7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fa0a0eda8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| batch_size: int = 65_536, | ||
| num_shards: int | None = None, | ||
| row_limit: int | None = None, | ||
| max_rows: int | None = None, |
There was a problem hiding this comment.
With load_lance(..., max_rows=0) on a nonempty dataset followed by write_lance_dataset(..., mode=AddColumns()), the source plans no shards, so the writer stage emits no sidecar containing the planned output schema. The reducer consequently takes the no-metadata path and rejects the operation as missing every fragment instead of filling every omitted row, even though zero is explicitly permitted here and bounded AddColumns is intended to fill omitted rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6578e32e6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af3361b7b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Hynek Kydlíček (hynky1999)
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbfb15f488
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ddb4ea11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| `batch_map_async` processes multiple batches concurrently. Use | ||
| `max_in_flight` to bound concurrency and `preserve_order` when output order | ||
| must match input order: |
There was a problem hiding this comment.
This section introduces a new bounded async-batch scheduling model alongside worker-local factory lifetimes, but it documents only the API mechanics. Add an ending ## Internal Notes section comparing this approach with Spark, Beam/Dataflow, Daft, Hugging Face Datasets, and Ray/Ray Data, and justify why Refiner uses per-worker async windows and factories instead of those systems’ batch, actor, or stage primitives; otherwise this execution-model decision omits the repository-required architectural tradeoff analysis.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcc4d83075
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce7c036992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 107e302860
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| dtypes = getattr(step, "dtypes", None) | ||
| if dtypes: | ||
| columns.update(dtypes) |
There was a problem hiding this comment.
When one row transform declares dtypes={"value": int64()} and a later map, map_async, batch_map, or flat_map replaces value without declaring a dtype, this set retains the earlier constraint, so the Lance sink casts the final runtime strings back to integers or rejects them. Although the source-schema case is now covered, this new helper is fresh evidence that the same dynamic-map behavior still fails whenever the planned type originated in an earlier transform; only constraints applicable to the final producer of a column should be enforced.
Useful? React with 👍 / 👎.
1 participant
Summary
This consolidates the Refiner primitives needed by the production hand-feature pipeline:
max_rowsnameAddColumns(fill=...)for row-aligned partial Lance column updates, including bounded/filtered sources and retry-stable untouched-fragment synthesisThe intentionally unsafe attempted overlap of packed-blob reads/uploads remains reverted.
Validation
1204 passed, 22 skippedmax_rowsbehavior