docs(regen): update response regeneration for multi-turn support#766
docs(regen): update response regeneration for multi-turn support#766soyr-redhat wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe response regeneration documentation now describes multi-turn, on-policy assistant generation, expanded dataset and retry options, vLLM passthrough arguments, resumability, async processing, and verification of regenerated conversation history. Response regeneration documentation
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
orestis-z
left a comment
There was a problem hiding this comment.
LGTM. All documented arguments, defaults, dataset entries, and output format descriptions verified against script.py and run_all.sh on main. Recommend approving.
🤖 Generated with Claude Code using the /pr-review skill
|
This pull request has merge conflicts that must be resolved before it can be |
|
This pull request has merge conflicts that must be resolved before it can be |
orestis-z
left a comment
There was a problem hiding this comment.
The multi-turn language, --max-retries / --max-model-len / --reasoning-parser additions, and the tutorial example are accurate and valuable. However, main has been substantially updated since this branch diverged (PRs #724, #729, #777, #771), and several sections this PR modifies now have better-maintained versions on main that should not be overwritten during the rebase. See inline comments for specifics.
🤖 Generated with Claude Code using the /pr-review skill
| #### Data Arguments | ||
|
|
||
| - **`--dataset`** (str, default: `ultrachat`, choices: `magpie`, `ultrachat`) Dataset to process. | ||
| - **`--dataset`** (str, default: `ultrachat`, choices: `magpie`, `ultrachat`, `gsm8k`) Dataset to process. |
There was a problem hiding this comment.
This hardcodes choices: magpie, ultrachat, gsm8k, but main already replaced this with a generic reference that won't go stale when new presets are added:
- **`--dataset`** (str, default: `ultrachat`) Dataset preset to process (see [Supported Datasets](#supported-datasets)).Same concern applies to --split (line 80) and --subset (line 82), which hardcode dataset-specific defaults. main uses generic wording ("Defaults to the preset's split/subset"). When rebasing, prefer main's wording for these three entries to avoid regressing the docs.
| | Dataset | HuggingFace ID | Prompt Field | Default Split | Subset | | ||
| | --------- | ------------------------------------------------- | ------------- | ------------- | ------ | | ||
| | Magpie | `Magpie-Align/Magpie-Llama-3.1-Pro-300K-Filtered` | `instruction` | `train` | | | ||
| | UltraChat | `HuggingFaceH4/ultrachat_200k` | `prompt` | `train_sft` | | | ||
| | GSM8K | `openai/gsm8k` | `question` | `train` | `main` | |
There was a problem hiding this comment.
This table lists 3 datasets, but main already lists 6 non-multimodal presets (sharegpt, ultrachat, gsm8k, magpie, nemotron, open-perfectblend) plus an explanation of why sharegpt4v_coco is excluded. When resolving the merge conflict here, keep main's table as the base — overwriting it with a 3-row version would be a regression.
Brings docs in line with shipped code from PRs vllm-project#693 (multi-turn), --reasoning-parser). Adds gsm8k to supported datasets, documents --subset arg, and adds a multi-turn output example. Signed-off-by: Sawyer Bowerman <sbowerma@redhat.com>
6d87dda to
453b200
Compare
|
Concerns have (hopefully) been addressed! Thanks! |
Purpose
Update response regeneration docs to reflect multi-turn support (#693), retry/resume changes (#716), and new
run_all.shflags (#499).Description
Descriptions, argument tables, and examples in both
response_regeneration.mdfiles still described single-turn-only behavior. Updates both the tutorial and CLI reference with multi-turn language, a multi-turn JSON example, missing arguments (--subset,--max-retries,--max-model-len,--reasoning-parser), and GSM8K in the supported datasets table.Tests
mdformat --check,ruff check,ruff format --checkpassscripts/response_regeneration/script.pyandrun_all.shI have filled in: