Add a2a3 CI job for on-device hello_world test#15
Add a2a3 CI job for on-device hello_world test#15zhangqi-chen merged 1 commit intohw-native-sys:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's continuous integration capabilities by introducing a new CI job designed to test the Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds CI/CD infrastructure for ARM64 NPU testing via GitHub Actions and refactors the example script's command-line interface from direct sys.argv parsing to argparse for cleaner argument handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 CI job and updates the hello_world.py example to use argparse for command-line argument parsing. The switch to argparse is a good improvement for handling script parameters like --sim and the new --device flag. My review includes a suggestion to improve maintainability by centralizing the default value for the device ID, which is currently duplicated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/hello_world.py (1)
115-120: Consider adding help text for CLI arguments.The argparse setup is functional, but adding
help=strings would make the CLI self-documenting when users run--help.📝 Suggested improvement
parser = argparse.ArgumentParser() - parser.add_argument("--sim", action="store_true") - parser.add_argument("--device", type=int, default=11) + parser.add_argument("--sim", action="store_true", help="Run on simulator (a2a3sim) instead of hardware") + parser.add_argument("--device", type=int, default=11, help="NPU device ID for hardware runs") args = parser.parse_args()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hello_world.py` around lines 115 - 120, Add descriptive help text to the argparse options so the CLI becomes self-documenting: update the parser.add_argument calls for "--sim" and "--device" to include help strings (e.g., describe that "--sim" toggles simulation mode and "--device" selects the device index, noting the default of 11) so that running the script with --help displays meaningful descriptions; locate and modify the parser.add_argument("--sim", action="store_true") and parser.add_argument("--device", type=int, default=11) lines to add the help= parameter..github/workflows/ci.yml (1)
84-92: Container image uses:latesttag, reducing build reproducibility.Using
:latestforci-device-py310means builds aren't pinned to a specific image version. If the image is updated, CI behavior could change unexpectedly without any PR changes.Consider pinning to a digest or versioned tag for reproducible builds.
📌 Example with digest pinning
container: - image: localhost:5000/ci-device-py310:latest + image: localhost:5000/ci-device-py310@sha256:<digest>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 84 - 92, The container image reference "localhost:5000/ci-device-py310:latest" should be pinned to a specific, immutable identifier to ensure reproducible CI runs; replace the ":latest" tag with either a versioned tag (e.g., ci-device-py310:vX.Y.Z) or a content digest (sha256:...) and update the image field accordingly so CI uses a fixed image for every run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 131-132: The workflow step "Run hello_world a2a3 test" passes
DEVICE_ID directly which may be unset and cause argparse to fail; update the CI
to either provide a safe default or validate DEVICE_ID before this step. Add a
preceding job/step (e.g., "Validate DEVICE_ID") that checks if the DEVICE_ID env
var is empty and fails with a clear error, or modify the run invocation to
fallback to a valid default (e.g., 0) when DEVICE_ID is not set; reference the
environment variable DEVICE_ID and the step name "Run hello_world a2a3 test"
when making the change.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 84-92: The container image reference
"localhost:5000/ci-device-py310:latest" should be pinned to a specific,
immutable identifier to ensure reproducible CI runs; replace the ":latest" tag
with either a versioned tag (e.g., ci-device-py310:vX.Y.Z) or a content digest
(sha256:...) and update the image field accordingly so CI uses a fixed image for
every run.
In `@examples/hello_world.py`:
- Around line 115-120: Add descriptive help text to the argparse options so the
CLI becomes self-documenting: update the parser.add_argument calls for "--sim"
and "--device" to include help strings (e.g., describe that "--sim" toggles
simulation mode and "--device" selects the device index, noting the default of
11) so that running the script with --help displays meaningful descriptions;
locate and modify the parser.add_argument("--sim", action="store_true") and
parser.add_argument("--device", type=int, default=11) lines to add the help=
parameter.
🪄 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: 6f67e9bf-59b8-499d-80b7-d9926729e70b
📒 Files selected for processing (2)
.github/workflows/ci.ymlexamples/hello_world.py
- New a2a3 job runs on self-hosted ARM64 NPU runner with privileged container and Ascend driver mounts - Uses aarch64 ptoas binary with SHA256 verification - hello_world.py switches to argparse, adds --device flag for explicit device ID passthrough from CI environment
1 participant
Summary
a2a3CI job that runs hello_world on a self-hosted ARM64 NPU runner with privileged container and Ascend driver mounts--deviceflag for explicit device ID passthrough from CI environmentTesting
--simand--deviceflags work correctlySummary by CodeRabbit
Release Notes
Chores
Examples