ci(daily): add a5 system tests job to daily ci#994
Merged
lyfne123 merged 1 commit intohw-native-sys:mainfrom Apr 16, 2026
Merged
ci(daily): add a5 system tests job to daily ci#994lyfne123 merged 1 commit intohw-native-sys:mainfrom
lyfne123 merged 1 commit intohw-native-sys:mainfrom
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSwitched daily CI from cron to event-driven triggers (push/pull_request + workflow_dispatch), added concurrency, changed qwen3-decode to run on self-hosted Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (push/pr)
participant GH as GitHub Actions
participant Cache as actions/cache
participant Runner as Self-hosted a5 Runner
participant PTOAS as PTOAS installer
participant Task as task-submit / Decode Task
Dev->>GH: push / pull_request (main) / workflow_dispatch
GH->>GH: enforce concurrency
GH->>Runner: start qwen3-decode job
Runner->>Cache: check PTOAS cache (key: PTOAS_VERSION/SHA256)
alt cache miss
Runner->>PTOAS: download & install PTOAS
PTOAS-->>Runner: installed
end
Runner->>Runner: source ${ASCEND_HOME_PATH}/bin/setenv.bash\nexport user PATH
Runner->>Task: task-submit --device 1 --run "python <script> -p a5 -d 1"
Task->>Runner: execute decode script
Task-->>GH: report result / exit status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/daily_ci.yml:
- Around line 85-133: The a5-system-tests job block is fully commented out so
the A5 system tests (pytest tests/st/ -m a5) never run; uncomment the entire
a5-system-tests job (the job key a5-system-tests and its steps including
actions/checkout@v4, the "Install project and dependencies" step, the "Cache
ptoas binary" step, "Install ptoas", "Clone pto-isa repository", "Install
simpler (stable)" and the "Run A5 system tests" step) and ensure the run uses
the existing pytest command (pytest tests/st/ -v --forked --platform=a5
--device=1 -m a5) so the CI executes the new A5 system-test lane.
- Around line 3-6: Re-enable the daily cron trigger by uncommenting the schedule
block (the commented "schedule:" and cron line) and/or restore
"workflow_dispatch:" so the workflow runs on the original daily cron ('0 22 * *
*') or update the file to document which other workflow replaces this daily CI
entry; ensure the schedule and workflow_dispatch directives in the top of the
workflow are present or add a comment referencing the replacement workflow if
you intentionally moved the daily run.
- Around line 8-12: The qwen3-decode job currently runs on self-hosted A5
runners for pull_request events (runner labels [self-hosted, a5]) which allows
fork PRs; add a job-level guard to only run on same-repo PRs or non-PR triggers
by adding an if condition like: require either the event not be a pull_request
or that github.event.pull_request.head.repo.full_name equals github.repository
(so the job runs for pushes, scheduled runs, or PRs originating from the same
repository/trusted triggers).
- Line 62: Validate that ASCEND_HOME_PATH is set and non-empty before calling
source; replace direct uses of source ${ASCEND_HOME_PATH}/bin/setenv.bash with a
guard that checks ASCEND_HOME_PATH (e.g., if [ -z "${ASCEND_HOME_PATH}" ] || [ !
-d "${ASCEND_HOME_PATH}/bin" ]; then echo "ASCEND_HOME_PATH is not set or
invalid" >&2; exit 1; fi) and then source the quoted file path
("${ASCEND_HOME_PATH}/bin/setenv.bash"); apply the same validation/quoting to
both occurrences of the source invocation so the workflow fails with a clear
message instead of an opaque error.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d6971574-0f99-4d2b-aef1-98b9ea340a0d
📒 Files selected for processing (1)
.github/workflows/daily_ci.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/daily_ci.yml (2)
29-33: Consider removing debug step before merge.This debug step is useful for troubleshooting but adds noise to production CI logs. Consider removing it once the a5 runner migration is validated, or add a comment explaining why it's needed long-term.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/daily_ci.yml around lines 29 - 33, Remove or document the temporary debug step named "Debug git info" in the CI workflow: either delete the entire step (the run block that checks git existence and prints PATH) before merging, or keep it but replace it with a short explanatory comment explaining it's temporary for the a5 runner migration validation and why it must remain; update the step title "Debug git info" accordingly so reviewers know whether it's intentional or slated for removal.
21-21: Consider addingactionlint.yamlto suppress false positive.The static analysis warning about "a5" being unknown is a false positive since it's a custom label for self-hosted runners. To silence this in CI, add an
actionlint.yamlconfig:self-hosted-runner: labels: - a5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/daily_ci.yml at line 21, CI linter flags the custom self-hosted label "a5" used in runs-on: [self-hosted, a5]; add an actionlint.yaml config file named actionlint.yaml at the repo root that defines a "self-hosted-runner" entry with a labels array containing "a5" to suppress the false positive, so actionlint recognizes the custom label referenced by runs-on in .github/workflows/daily_ci.yml.
🤖 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/daily_ci.yml:
- Around line 86-87: The workflow step calling task-submit with task-submit
--timeout 1800 --max-time 1800 --device 1 --run "python $f -p a5 -d 1"
references non-existent example scripts; fix by either adding the missing
example scripts under examples/models/qwen3/ (qwen3_32b_decode_scope1.py,
qwen3_32b_decode_scope2.py, qwen3_32b_decode_scope3.py, qwen3_32b_decode.py) or
update the workflow to run the real test file
tests/st/runtime/test_qwen3_decode_scope3_mixed.py using pytest (replace the
legacy python $f -p a5 -d 1 invocation with a pytest command and proper flags),
and confirm whether the --device 1 / -d 1 hardware flag is required (remove
redundancy if task-submit already controls device allocation).
---
Nitpick comments:
In @.github/workflows/daily_ci.yml:
- Around line 29-33: Remove or document the temporary debug step named "Debug
git info" in the CI workflow: either delete the entire step (the run block that
checks git existence and prints PATH) before merging, or keep it but replace it
with a short explanatory comment explaining it's temporary for the a5 runner
migration validation and why it must remain; update the step title "Debug git
info" accordingly so reviewers know whether it's intentional or slated for
removal.
- Line 21: CI linter flags the custom self-hosted label "a5" used in runs-on:
[self-hosted, a5]; add an actionlint.yaml config file named actionlint.yaml at
the repo root that defines a "self-hosted-runner" entry with a labels array
containing "a5" to suppress the false positive, so actionlint recognizes the
custom label referenced by runs-on in .github/workflows/daily_ci.yml.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 547a4453-bb6b-4629-a0ca-1ad89a663b5a
📒 Files selected for processing (2)
.github/workflows/ci.yml.github/workflows/daily_ci.yml
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
luohuan19
force-pushed
the
ci-a5
branch
11 times, most recently
from
970cde1 to
68c5231
Compare
lyfne123
reviewed
Apr 16, 2026
| on: | ||
| schedule: | ||
| - cron: '0 22 * * *' # UTC 22:00 = Shanghai 06:00 | ||
| push: |
Collaborator
There was a problem hiding this comment.
do not execute in every ci task
- Upgrade ptoas v0.24 → v0.25 (update SHA256 checksum) - Extract PTOAS_VERSION/PTOAS_SHA256 as job-level env vars for clarity - Cache ptoas binary via actions/cache to avoid redundant downloads - Add new a5-system-tests job targeting [self-hosted, a5] runners: runs pytest tests/st/ with --forked --platform=a5 via task-submit
luohuan19
changed the title
luohuan19
changed the title
lyfne123
approved these changes
Apr 16, 2026
lyfne123
pushed a commit
that referenced
this pull request
Apr 17, 2026
- Upgrade daily ci ptoas v0.24 → v0.25 (update SHA256 checksum) - Extract PTOAS_VERSION/PTOAS_SHA256 as job-level env vars for clarity - Cache ptoas binary via actions/cache to avoid redundant downloads - Add new a5-system-tests job targeting [self-hosted, a5] runners: runs pytest tests/st/ with --forked --platform=a5 via task-submit
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
runs pytest tests/st/ with --forked --platform=a5 via task-submit