ci: run model-scoped pypto-serving accuracy guards#759
Conversation
Generalize the pypto-serving test action with model-name and model-dir inputs so the same integration path can execute Qwen3-14B or DeepSeek V4 coverage. The action checks out pypto-serving main, replaces its pypto-lib submodule with the PR checkout, installs the shared serving dependencies, and dispatches the selected model guard. Use one PR-only serving job to share the NPU environment setup and PyPTO build. Detect Qwen and DeepSeek source changes independently, invoke only the affected model actions, run both sequentially when a PR touches both model trees, and retain one orphan-task and build-artifact cleanup path. Bound both Qwen and DeepSeek task submissions with a 20-minute allocation wait and 30-minute runtime limit. Run the DeepSeek V4 HTTP accuracy guard with /data/l00955553/model/dsv4-flash-w8a8 using automatic eight-device allocation outside the configured whitelist, while preserving the existing Qwen device-selection behavior and the validated model-specific ring and runtime settings.
📝 WalkthroughWalkthroughServing CI now selects Qwen3-14B or DeepSeek V4 tests based on changed model paths. The composite action adds model selection, model-specific environments, updated task submission settings, and fixed serving checkout behavior. ChangesServing test CI
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
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 updates the GitHub Action pypto-serving-tests to support running different model tests dynamically based on a new model-name input, specifically adding support for DeepSeek-V4 accuracy tests alongside the existing Qwen3 tests. Feedback points out a copy-paste error in the newly added DeepSeek-V4 test step, where TASK_DEVICE is redundantly self-assigned instead of correctly setting DEVICE_ID to propagate the active device ID to the test execution.
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.
| source "$GITHUB_WORKSPACE/$PYPTO_LIB_CHECKOUT/activate.sh" | ||
| run_cmd="source $GITHUB_WORKSPACE/$PYPTO_LIB_CHECKOUT/activate.sh" | ||
| run_cmd="$run_cmd && cd $GITHUB_WORKSPACE/$PYPTO_SERVING_CHECKOUT" | ||
| run_cmd="$run_cmd && TASK_DEVICE=\$TASK_DEVICE" |
There was a problem hiding this comment.
There is a copy-paste error here. In the Qwen test step, DEVICE_ID is set to $TASK_DEVICE (i.e., DEVICE_ID=\$TASK_DEVICE) so that the active Ascend device ID is correctly propagated to the pytest command. Here, TASK_DEVICE=\$TASK_DEVICE is a redundant self-assignment since TASK_DEVICE is already set by task-submit. It should be changed to DEVICE_ID=\$TASK_DEVICE to ensure the correct device ID environment variable is passed to the DeepSeek test run.
run_cmd="$run_cmd && DEVICE_ID=\\$TASK_DEVICE"
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
310-375: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a
permissionsblock to the serving job.zizmor flags excessive default permissions due to no
permissionsblock. The serving job only runs tests and doesn't need write access. Adding an explicit least-privilege block prevents the job from inheriting overly broad repo-default token permissions.🔒️ Proposed addition
serving: needs: detect-changes + permissions: + contents: read # PR-only: run one shared setup, then the guards selected by model changes. if: >-🤖 Prompt for 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. In @.github/workflows/ci.yml around lines 310 - 375, Add an explicit least-privilege permissions block to the serving job, alongside its existing job-level settings, granting only the read access required by its checkout/actions and leaving all write permissions disabled. Do not change the serving job’s triggers, steps, or action inputs.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 310-375: Add an explicit least-privilege permissions block to the
serving job, alongside its existing job-level settings, granting only the read
access required by its checkout/actions and leaving all write permissions
disabled. Do not change the serving job’s triggers, steps, or action inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9538e0f1-c4db-4217-b8d8-c7a8b3c01070
📒 Files selected for processing (2)
.github/actions/pypto-serving-tests/action.yml.github/workflows/ci.yml
2 participants
Summary
Validation
Depends on the DeepSeek accuracy guard merged in hw-native-sys/pypto-serving#67.