ci: add DeepSeek V4 generation accuracy guard#67
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds a DeepSeek V4 HTTP completion accuracy test and integrates it into the unit-test CI job with extended time limits, required packages, task submission, and orphan cleanup. ChangesDeepSeek V4 accuracy validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI as unit-tests CI job
participant Task as task-submit
participant Test as DeepSeek accuracy test
participant Server as DeepSeek HTTP server
participant API as HTTP completion API
CI->>Task: submit accuracy command with device and timeout settings
Task->>Test: execute pytest test
Test->>Server: start server
Test->>Server: poll /health
Test->>API: POST /v1/completions
API-->>Test: return generated completion
Test->>Test: validate expected response
Test->>Server: terminate process group
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test (tests/test_deepseek_v4_accuracy.py) to verify the HTTP generation accuracy of DeepSeek V4 in CI. The test launches a local server, waits for health checks, performs a completion request, and asserts the output. The review feedback highlights two important improvement opportunities: first, handling urllib.error.HTTPError explicitly in _request_completion to capture detailed error bodies for better debuggability, and second, optimizing _print_server_log to read only the tail of the log file instead of loading the entire file into memory, which prevents potential Out Of Memory (OOM) issues in CI.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_deepseek_v4_accuracy.py`:
- Around line 171-199: Update _stop_process_group so cleanup never propagates a
subprocess.TimeoutExpired or other process-wait failure from the final
process.wait call after SIGKILL. Catch and suppress the timeout (and ensure
cleanup exceptions do not replace the original test failure), while preserving
the existing graceful termination and forced-kill sequence.
- Around line 156-168: Update the RuntimeError raised when the DeepSeek server
exits during generation in the queue.Empty handling path to explicitly use
exception chaining with from None. Keep the existing error message and
surrounding completion logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85b2141f-fbbb-46d1-83f4-ec43396d3697
📒 Files selected for processing (2)
.github/workflows/ci.ymltests/test_deepseek_v4_accuracy.py
ndleslx
force-pushed
the
dsv4-ci
branch
3 times, most recently
from
794006a to
45bb4ee
Compare
Add an end-to-end DeepSeek V4 HTTP completion test that starts the serving command with the documented TP=8 configuration, sends the prompt 'Huawei is' with greedy decoding, and requires the exact six-token completion ' a leading global provider of ICT'. Bound server startup and generation, preserve HTTP error response bodies, and print only a bounded server-log tail on failure. Make process-group shutdown best-effort so stuck or failed waits cannot mask the original accuracy failure. Add focused CPU coverage for HTTP error diagnostics, bounded log reads, and the forced-kill timeout path. Run the DeepSeek guard through task-submit with automatic eight-device allocation outside the configured whitelist, a 20-minute allocation wait, a 30-minute runtime limit, the CI model at /data/l00955553/model/dsv4-flash-w8a8, and the runtime settings required by the documented server configuration. Move both Qwen guards into their own automatic task-submit reservations and add a run-scoped marker so cancelled CI jobs can reap orphaned submissions. Make serving workers close executor-owned runtime resources on every exit path, including initialization and inference failures, so distributed workers do not retain NPU memory after tests finish. Cover normal, exceptional, and repeated cleanup behavior with focused unit tests, and extend the NPU job timeout and dependencies for the HTTP guard.
## Summary - generalize the pypto-serving test action with model-name and model-dir inputs - use one PR-only serving job with model-specific Qwen and DeepSeek change detection - run the merged pypto-serving DeepSeek V4 accuracy guard with automatic eight-device allocation and whitelist bypass - apply 20-minute allocation and 30-minute runtime limits to both Qwen and DeepSeek task submissions ## Validation - parsed the composite action and workflow YAML locally - passed header, English-only, Ruff, and diff checks - verified Qwen-only, DeepSeek-only, combined, unrelated, and non-PR selection behavior - verified the DeepSeek PTO2 ring inputs and task-submit arguments are forwarded explicitly Depends on the DeepSeek accuracy guard merged in hw-native-sys/pypto-serving#67.
2 participants
Summary
Validation
python -m pytest tests/test_deepseek_v4.py -q(45 passed)python -m pytest tests/test_parallel.py -q(9 passed)