[Tests]: Precommit Check for Spec-Dec Recipes#1527
Conversation
Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request introduces a new pre-commit hook to validate launcher YAML files and expands the existing recipe validation hook to support speculative-decoding recipe types. The launcher validation extracts path references from YAML configs and templates, resolves them to filesystem locations, and validates referenced recipe configs by calling ChangesPre-commit validation system enhancements
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1527 +/- ##
=======================================
Coverage 76.75% 76.75%
=======================================
Files 476 476
Lines 51811 51811
=======================================
+ Hits 39767 39768 +1
+ Misses 12044 12043 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
A bit confused by the scope: is the change for general launcher yaml check or for spec-dec specific recipes check? |
Both. It adds checks for spec-dec recipes and all yamls that appears in the general launcher. |
| entry: python tools/precommit/check_launcher_yaml.py | ||
| language: system | ||
| files: ^(tools/launcher/examples/.*\.yaml|modelopt_recipes/.*\.yaml|tools/precommit/check_launcher_yaml\.py)$ | ||
| pass_filenames: false |
There was a problem hiding this comment.
hmm, so this behaviour is not expected:
It seems the precommit check script gets activated once there are modifications to any file in the above pattern, but then it will scan all the files even for files that have no modifications.
We should scope to only modified files.
What does this PR do?
Type of change: new tests / tooling
Adds pre-commit validation for speculative-decoding recipes (the existing
check-modelopt-recipeshook only ran on PTQ) and for launcher YAML references into the recipe library.tools/precommit/check_modelopt_recipes.py: acceptspeculative_eagle/speculative_dflash/speculative_medusain addition toptq, so per-model spec-dec recipes (e.g.modelopt_recipes/models/Qwen3-8B/dflash.yaml) get full Pydantic validation viaload_recipe()at commit time.tools/precommit/check_launcher_yaml.py(new): scans everytools/launcher/examples/**/*.yamlfor--config <path>anddata.chat_template=<path>references, verifies the resolved files exist, and runsload_recipe()on any path undermodelopt_recipes/. Skips<<global_vars.x>>interpolation.pass_filenames: falseso recipe-side edits also re-validate all launcher references.Usage
Testing
Smoke-tested both hooks manually:
dflash_block_size: not_an_int--configpathdata.chat_templatepathBefore your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
Motivated by the per-model recipe migration in #TBD — without these hooks, broken
--configpaths and recipe schema typos surface only at CI or runtime.Summary by CodeRabbit
Release Notes