fix(dflash): resolve quality failures and PR feedback for Laguna converter - #941
Closed
shotsan wants to merge 4 commits into
Closed
Conversation
…0.0,<=5.0.1 (vllm-project#917) Updates the requirements on [datasets](https://github.com/huggingface/datasets) to permit the latest version. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/huggingface/datasets/releases">datasets's releases</a>.</em></p> <blockquote> <h2>5.0.1</h2> <h2>Bug fixes</h2> <ul> <li>Fix version string in <strong>init</strong>.py by <a href="https://github.com/qgallouedec"><code>@qgallouedec</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8244">huggingface/datasets#8244</a></li> <li>fix conda build by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8250">huggingface/datasets#8250</a></li> <li>Fix JSON loader schema inference for files starting with a UTF-8 BOM (<a href="https://redirect.github.com/huggingface/datasets/issues/8241">#8241</a>) by <a href="https://github.com/archievi"><code>@archievi</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8243">huggingface/datasets#8243</a></li> <li>Support hermes traces by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8255">huggingface/datasets#8255</a></li> <li>Fix batch(by_column=...) crashing after shard/shuffle/split by <a href="https://github.com/pkooij"><code>@pkooij</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8259">huggingface/datasets#8259</a></li> <li>fix traces streaming by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8277">huggingface/datasets#8277</a></li> <li>support droid agent traces by <a href="https://github.com/cfahlgren1"><code>@cfahlgren1</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8263">huggingface/datasets#8263</a></li> <li>Fix CI: commit operation equality (hfh 1.20.0) and pytest parametrize collection error by <a href="https://github.com/Wauplin"><code>@Wauplin</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8283">huggingface/datasets#8283</a></li> <li>Fix lance auth by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8301">huggingface/datasets#8301</a></li> <li>Fix symlink-following arbitrary file write in archive extraction by <a href="https://github.com/AAtomical"><code>@AAtomical</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8303">huggingface/datasets#8303</a></li> <li>Bug Fix: Resuming Twice Resets the Dataloader by <a href="https://github.com/francesco-bertolotti"><code>@francesco-bertolotti</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8295">huggingface/datasets#8295</a></li> <li>Bump fsspec and simpler wds compr by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8337">huggingface/datasets#8337</a></li> <li>fix: validate Arrow IPC record batches by <a href="https://github.com/XciD"><code>@XciD</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8350">huggingface/datasets#8350</a></li> <li>Make the dataset fingerprint independent of Arrow chunking by <a href="https://github.com/SuryanshSS1011"><code>@SuryanshSS1011</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8339">huggingface/datasets#8339</a></li> <li>Fix casting a nullable LargeList to a different inner type by <a href="https://github.com/vineethsaivs"><code>@vineethsaivs</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8346">huggingface/datasets#8346</a></li> <li>docs: replace AutoFeatureExtractor with AutoImageProcessor in image preprocessing docs by <a href="https://github.com/gautamkishore"><code>@gautamkishore</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8326">huggingface/datasets#8326</a></li> <li>Fix column drop in Arrow path of axis=1 concatenation by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8342">huggingface/datasets#8342</a></li> <li>Raise on length mismatch in batched IterableDataset.map by <a href="https://github.com/sohumt123"><code>@sohumt123</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8332">huggingface/datasets#8332</a></li> <li>Fix require_storage_embed recursing into require_storage_cast by <a href="https://github.com/vineethsaivs"><code>@vineethsaivs</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8349">huggingface/datasets#8349</a></li> <li>Fix path traversal via metadata file_name in folder-based builders by <a href="https://github.com/Kaif10"><code>@Kaif10</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8325">huggingface/datasets#8325</a></li> <li>Support batched=True in Dataset.to_dict by <a href="https://github.com/vineethsaivs"><code>@vineethsaivs</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8333">huggingface/datasets#8333</a></li> <li>Fix hdf5 external files by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8355">huggingface/datasets#8355</a></li> <li>remove bad require_storage test by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8357">huggingface/datasets#8357</a></li> <li>ensure fiels are in repo by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8356">huggingface/datasets#8356</a></li> <li>Keep flat numeric columns with nulls numeric in numpy format by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8352">huggingface/datasets#8352</a></li> <li>Fix bucket dataset card handling and push metadata accounting by <a href="https://github.com/pjh4993"><code>@pjh4993</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8354">huggingface/datasets#8354</a></li> <li>Fix CSV loader dropping on_bad_lines/encoding_errors on pandas 2.0-2.2 by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8358">huggingface/datasets#8358</a></li> <li>Keep integers on the python read path for fixed-shape ArrayXD columns with nulls by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8363">huggingface/datasets#8363</a></li> <li>Rebatch arrow source before formatting in IterableDataset.filter to fix resume data loss by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8360">huggingface/datasets#8360</a></li> <li>Decode Json() columns in Dataset.to_pandas() by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8344">huggingface/datasets#8344</a></li> <li>fix buckets on windows by <a href="https://github.com/lhoestq"><code>@lhoestq</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8369">huggingface/datasets#8369</a></li> <li>Fix DatasetDict.push_to_hub leaving removed splits in the dataset card by <a href="https://github.com/pjh4993"><code>@pjh4993</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8367">huggingface/datasets#8367</a></li> <li>Preserve nullable integer columns in to_json/to_csv/to_sql by <a href="https://github.com/ebarkhordar"><code>@ebarkhordar</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8366">huggingface/datasets#8366</a></li> </ul> <h2>Docs</h2> <ul> <li>docs: fix duplicate "to" in IterableDataset push-to-hub example by <a href="https://github.com/DaoyuanLi2816"><code>@DaoyuanLi2816</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8252">huggingface/datasets#8252</a></li> <li>Clarify dataset creation vs loading workflows in create_dataset tutorial by <a href="https://github.com/zanvari"><code>@zanvari</code></a> in <a href="https://redirect.github.com/huggingface/datasets/pull/8235">huggingface/datasets#8235</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/DaoyuanLi2816"><code>@DaoyuanLi2816</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8252">huggingface/datasets#8252</a></li> <li><a href="https://github.com/zanvari"><code>@zanvari</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8235">huggingface/datasets#8235</a></li> <li><a href="https://github.com/archievi"><code>@archievi</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8243">huggingface/datasets#8243</a></li> <li><a href="https://github.com/pkooij"><code>@pkooij</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8259">huggingface/datasets#8259</a></li> <li><a href="https://github.com/AAtomical"><code>@AAtomical</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8303">huggingface/datasets#8303</a></li> <li><a href="https://github.com/francesco-bertolotti"><code>@francesco-bertolotti</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8295">huggingface/datasets#8295</a></li> <li><a href="https://github.com/XciD"><code>@XciD</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8350">huggingface/datasets#8350</a></li> <li><a href="https://github.com/SuryanshSS1011"><code>@SuryanshSS1011</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8339">huggingface/datasets#8339</a></li> <li><a href="https://github.com/vineethsaivs"><code>@vineethsaivs</code></a> made their first contribution in <a href="https://redirect.github.com/huggingface/datasets/pull/8346">huggingface/datasets#8346</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/huggingface/datasets/commit/921c2a7c0c9494e86fd145e03f68d12fa4ef90a2"><code>921c2a7</code></a> release: 5.0.1 (<a href="https://redirect.github.com/huggingface/datasets/issues/8370">#8370</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/c6fc5cdbf0a527b0ebd7a74fc73274a0fa3b8cb2"><code>c6fc5cd</code></a> Preserve nullable integer columns in to_json/to_csv/to_sql (<a href="https://redirect.github.com/huggingface/datasets/issues/8366">#8366</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/6747b871de3c8ae5aabc97b17251bfac2d8d9ee5"><code>6747b87</code></a> Fix DatasetDict.push_to_hub leaving removed splits in the dataset card (<a href="https://redirect.github.com/huggingface/datasets/issues/8367">#8367</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/b305031c4ec070e5e657049abaa70659883958e2"><code>b305031</code></a> fix buckets on windows (<a href="https://redirect.github.com/huggingface/datasets/issues/8369">#8369</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/030a3e540e8e5cc850c01b0ea3bc0db448255567"><code>030a3e5</code></a> Decode Json() columns in Dataset.to_pandas() (<a href="https://redirect.github.com/huggingface/datasets/issues/8344">#8344</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/0f207a0a60d63f64d985844044c33ff7cdd4d14f"><code>0f207a0</code></a> Rebatch arrow source before formatting in IterableDataset.filter to fix resum...</li> <li><a href="https://github.com/huggingface/datasets/commit/adad35d93ec636ac47d66088c6faadc7d4f0730b"><code>adad35d</code></a> Keep integers on the python read path for fixed-shape ArrayXD columns with nu...</li> <li><a href="https://github.com/huggingface/datasets/commit/89667468df606540b4da9280dd974297d9c08734"><code>8966746</code></a> Fix CSV loader dropping on_bad_lines/encoding_errors on pandas 2.0-2.2 (<a href="https://redirect.github.com/huggingface/datasets/issues/8358">#8358</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/b8e861a26be89e232ccd869f698aa08caa779148"><code>b8e861a</code></a> Fix bucket dataset card handling and push metadata accounting (<a href="https://redirect.github.com/huggingface/datasets/issues/8354">#8354</a>)</li> <li><a href="https://github.com/huggingface/datasets/commit/521a590f9b642908c3eb116eb77db9d5512126e9"><code>521a590</code></a> Keep flat numeric columns with nulls numeric in numpy format (<a href="https://redirect.github.com/huggingface/datasets/issues/8352">#8352</a>)</li> <li>Additional commits viewable in <a href="https://github.com/huggingface/datasets/compare/4.0.0...5.0.1">compare view</a></li> </ul> </details> <br /> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-project#794) # Purpose Derive the **off-policy** training loss mask from a **render boundary**, with the vLLM instance as the single tokenization source. For each assistant turn we render the prompt and the prompt-plus-turn via `/v1/chat/completions/render`, and supervise the tokens the second render adds past the first. **Then we deprecate the fallback hf tokenization & fallback regex paths** This is the same prompt/completion boundary the engine already reports for on-policy regeneration (vllm-project#729), reconstructed from the chat template. One tokenizer now produces the loss mask, the target hidden states, and the serving prompt — no separate local tokenization that can drift from serving. Design write-up (worked examples, rationale): https://claude.ai/code/artifact/6cd3bf57-3d20-4a06-b27d-5b08e5fce6b4 # What changes - **Deleted:** the two-tier local masking path — the HF `{% generation %}` tag-mask probe and the regex span-detector — plus the local render adapters and `--assistant-pattern`. - **Added:** a vLLM `/render` client and the boundary derivation. - **Unchanged:** on-policy pre-tokenized rows (vllm-project#729) still pass straight through. **Why `token_ids`, not the server mask:** `/render` can also return `assistant_tokens_mask`, but it's only populated when the template carries `{% generation %}` tags, which the common models don't ship. We request only `token_ids` and compute the mask from the boundary — so it works on every template, and isn't blocked on the unreleased `return_loss_mask` (`/render` itself shipped in v0.15.0). # Requirement `--render-endpoint` is now required for off-policy data (pre-tokenized input still works without it). Point it at the vLLM instance already used for hidden-state extraction, or a GPU-less `vllm launch render`. Single source of truth holds only if that instance's `tokenizer_mode` matches serving's. # How to review ~68% of the diff is pure deletion; the real surface is ~440 lines of source: | Bucket | What | How to read | |---|---|---| | **Deletion** (~1,560) | the old two-tier mask functions and the tests that covered them | skim | | **New** (~530) | `render_client.py`, the boundary functions, `test_render_boundary.py` | review in isolation | | **Rewiring** (~200) | `_preprocess_batch`, `build_eagle3_dataset`, `load_and_preprocess_dataset` switching to the boundary, plus the CLI swap | **the logic change** | The other preprocessing helpers are byte-identical to `main`. The delete-plus-rewire core is atomic; `render_client.py` is the one piece reviewable on its own. # Tests ```bash # Serve any model with a chat template vllm serve Qwen/Qwen3-0.6B --port 8000 # Derive masks from its render boundaries, on real data python scripts/prepare_data.py \ --model Qwen/Qwen3-0.6B \ --data sharegpt \ --render-endpoint http://localhost:8000 \ --output /tmp/prepared \ --max-samples 20 ``` It prints the derived mask inline (blue = trainable, grey = masked): ``` <|im_start|>user I have a location independent business that generates 1 miljon per year...<|im_end|> <|im_start|>assistant <think> </think> 1/5: A clear target market and understanding of their needs is essential...<|im_end|> ``` The boundary is the thing to check: everything from `<think>` on is blue, while the user turn and the `<|im_start|>assistant` header stay grey. Or automated — launches its own vLLM and asserts the masks: ```bash pytest tests/e2e/smoke/test_render_boundary.py -v ``` # Deferred / follow-ups - **Concurrency** — the boundary does 2–3 renders per turn, today riding `datasets.map(num_proc)`. A batched/async client if throughput needs it. - **Multimodal** — prefix-stability not yet validated on an image model. - **Reasoning emission** — fan-out (on-policy, more rows) vs. a future single-row mode (cheaper, off-policy at turn seams); undecided. - **Render-failure handling** — a dead endpoint currently drains the dataset to empty per-row instead of aborting fast; should distinguish endpoint failure from a single bad conversation. - **Shared tool-call parsing** with regen (vllm-project#750), once both paths speak vLLM messages. ## Checklist - [x] Purpose of the PR - [x] Test plan / results - [x] Documentation update (CLI doc updated; user-guide tutorial pending) - [x] I (a human) have reviewed this code to the best of my ability. --------- Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
…rm-start Laguna-style DFlash checkpoints use fused qkv_proj + per-head g_proj gating, while DFlashDraftModel expects separate q/k/v_proj (Qwen3-style). Add a _remap_weights step that splits fused QKV, drops incompatible keys (g_proj, aux_hidden_norms), and slices fc.weight when the source has more target layers than the model expects. Enables warm-starting DSpark training from published Laguna DFlash checkpoints. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Orestis Zambounis <orestis-z@users.noreply.github.com> Signed-off-by: shotsan <shotsan@users.noreply.github.com>
Addresses PR review feedback to validate that separated q/k/v_proj keys don't already exist when splitting a fused qkv_proj. Also includes formatting fixes to resolve the quality checks failure. Signed-off-by: shotsan <shotsan@users.noreply.github.com>
shotsan
requested review from
dsikka,
fynnsu,
orestis-z,
rahul-tuli and
shanjiaz
as code owners
August 5, 2026 19:34
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews
🔴 Require approval from approved reviewers listWaiting for any of
This rule is failing.All pull requests must have at least one approving review from a member of the approved reviewers list before merging.
|
|
This pull request has merge conflicts that must be resolved before it can be |
|
Closing in favor of a clean PR without merge conflicts. |
|
Closing as discussed — will open a clean PR without merge conflicts. |
Author
|
Closing — will open a clean PR without merge conflicts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the quality checks and implements the CodeRabbit review feedback for PR #922. It ensures mixed fused/separate projections raise a ValueError.