docs: correct stale prose about the pre-tokenized on-policy path#863
docs: correct stale prose about the pre-tokenized on-policy path#863WindChimeRan wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. 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:
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
|
e0d3da8 to
a24203d
Compare
Two places describe behaviour the code stopped having. The response-regeneration tutorial documents the output row as carrying `conversations`, a list of role/content messages. vllm-project#808 replaced that with `text`, a decode of `input_ids`, and updated docs/cli/response_regeneration.md but not the tutorial. Both example rows are corrected, and the `metadata` blocks gain the `sampling_params` key the script emits. scripts/prepare_data.py's module docstring claims it always applies a chat template and produces a mask. Since vllm-project#729, rows that already carry `input_ids` and `loss_mask` skip both steps; they are truncated and filtered only. Note the exception rather than leaving the docstring stating it unconditionally. No code or behaviour change. Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
a24203d to
070352e
Compare
WindChimeRan
changed the title
1 participant
Purpose
Two docs describe behaviour the code stopped having when on-policy regeneration started emitting pre-tokenized rows.
1. The output row schema in the tutorial.
scripts/response_regeneration/script.py:510writes"text", a decode ofinput_ids. The tutorial still shows"conversations", a list of role/content messages, in both example rows, and itsmetadatablocks omit thesampling_paramskey the script emits. #808 made this change and updateddocs/cli/response_regeneration.md, which is correct today; the tutorial was missed.2. The
prepare_data.pymodule docstring. It states the script applies a chat template and produces a loss mask, unconditionally. Since #729, rows carryinginput_idsandloss_masktake the pre-tokenized branch atpreprocessing.py:648and skip both steps -- they are truncated to--seq-lengthand filtered, nothing more. The docstring now notes the exception.No code or behaviour change.
Tests
Docs and one module docstring.
ruff check scripts/prepare_data.pypassesruff format --check scripts/prepare_data.pypassespython -m mdformat --check docs/user_guide/tutorials/response_regeneration.mdpassesField names verified against
script.py:500-519; the pre-tokenized branch againstpreprocessing.py:541and_passthrough_pretokenizedat:499.Checklist
I have filled in: