Repository navigation
fix(vllm): put omni video negative_prompt in prompt dict entry (fixes #15349) - #15402
Conversation
…15349) Signed-off-by: Charan Rathore <[email protected]>
GitHub Actions
Inactive
GitHub Actions
Inactive
|
👋 Hi charan-rathore! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
external-contribution
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe video request builder now stores ChangesVideo Prompt Handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~8 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Negative prompts are now passed in the prompt mapping, with focused coverage of the request variants. No concrete defect or required-validation omission is established, so the change has minimal merge risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Keep the minimal negative-prompt cases (absent, empty, ordinary, non-English) and one image co-existence case instead of crossing every value with both image states, per review feedback. Signed-off-by: Charan Rathore <[email protected]>
GitHub Actions
Inactive
|
Good point, done in 0ee4917. The parametrization is consolidated to the minimal negative-prompt cases (absent, empty, ordinary, non-English) plus a single image co-existence case, instead of crossing every value with both image states. Image handling remains covered by test_t2v_no_multi_modal_data_and_i2v_attaches_image. Same runner re-verified: 4 failures / 5 passes without the fix, 9/9 passes with it, and the PR body is updated to match. |
GitHub Actions
Inactive
|
/ok to test cb172e5 |
Fixes #15349.
Problem
A vLLM-Omni video generation request with
nvext.negative_promptset returns HTTP 500. InOmniHandler,promptis a dict-shapedOmniTextPrompt, but the code assignsprompt.negative_prompt = nvext.negative_promptas an attribute, which fails for this prompt type. The crash happens while the request is being prepared, before the engine starts.Change
One production line: put the negative prompt into the existing
"negative_prompt"dict entry instead. Requests without a negative prompt are unchanged, and an optional starting image still flows through.Adds five regression test cases to the existing video request tests: absent/empty/ordinary/non-English negative prompts, plus one image co-existence case.
Validation
Ran on this commit (base 2de120f), CPU only:
EngineInputs, loads the actual Pydantic video request models, and runs the new regression test method; it uses a dict-shaped stand-in forOmniTextPromptand stand-ins for the sampling/LoRA helpers, with heavy imports bypassed via AST extraction.ruff check(0.5.2),black --check(23.1.0),isort --check-only(5.12.0).No native vLLM-Omni test suite, GPU inference, generated video, or Docker run was performed.
AI disclosure: this PR was prepared with AI assistance (Instinct assisted) under my direction; I understand the change and take responsibility for it.