-
Notifications
You must be signed in to change notification settings - Fork 257
[CI] Add weekly overview snapshot sweep / [CI] 新增每周 overview snapshot 定时 sweep #2588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| name: Weekly Overview Snapshot | ||
| run-name: Weekly Overview Snapshot | ||
|
|
||
| # Scheduled curated-config pass (not a full sweep) so every | ||
| # /overview model×hardware cell gets a same-batch data point at least weekly | ||
| # (#2304). The config-keys list is the scope knob: pilot is dsv4+qwen3.5 on | ||
| # b200/b300; extend to mi355x then gb200/gb300 once the pilot proves the | ||
| # ingest path. `schedule` events get no event bump in ci-priority.yaml, so | ||
| # PR and main-push sweeps always preempt these jobs. | ||
|
Comment on lines
+4
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The comment's rationale is wrong: Extended reasoning...The workflow header comment (and the PR description) states: " Checking adjustments:
event:
push: 2.0There is no The actual PR-sweep entrypoint, This breaks the stated non-interference guarantee: it's not just imprecise, it's the opposite of what's claimed for the pre-merge case. Worse, the pilot configs chosen for this snapshot ( Concrete walkthrough: a contributor opens a PR that triggers To fix: correct the comment (and PR description) to scope the preemption claim to main-push sweeps only, e.g. "main-push sweeps get a +2.0 event bump and will preempt this job; PR sweeps do not get an event bump and may compete with it for runners." This is a documentation/design-rationale correction, not a functional bug — the workflow still runs correctly, and the Saturday 06:00 UTC off-peak scheduling plus |
||
|
|
||
| concurrency: | ||
| group: weekly-overview-snapshot | ||
| cancel-in-progress: false | ||
|
|
||
| on: | ||
| schedule: | ||
| - cron: "0 6 * * 6" # Saturday 06:00 UTC — weekend off-peak, avoids weekday PR-sweep contention | ||
| workflow_dispatch: | ||
|
Comment on lines
+15
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This introduces a new scheduled/manual sweep, configuration override, partial-failure policy, and production database dispatch, but neither AGENTS.md reference: AGENTS.md:L8-L8 Useful? React with 👍 / 👎. |
||
| inputs: | ||
| config-keys: | ||
| description: "Space-separated master-config keys to run (default: pilot list)" | ||
| required: false | ||
| type: string | ||
| skip-ingest: | ||
| description: "Skip the production DB ingest (smoke-test the sweep only)" | ||
| required: false | ||
| type: boolean | ||
| default: false | ||
|
|
||
| jobs: | ||
| # dsv4 on b200 deliberately uses the vllm-mtp entry: the sglang/trt entries | ||
| # pin the `b200-dsv4` virtual runner pool, whose scheduler queue is not | ||
| # currently served (jobs sit queued for days while b200-dgxc idles). | ||
| sweep: | ||
| uses: ./.github/workflows/e2e-tests.yml | ||
| secrets: inherit | ||
| with: | ||
| test-name: weekly-overview-snapshot | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The Extended reasoning...
The problem is that This is worth flagging because it creates a false impression: a reader sees Proof: (1) grep This is a pure quality/cleanup issue with no functional or correctness impact — nothing breaks, no job fails, and the ingest step and DB rows are unaffected. The fix is simply to drop the dead |
||
| generate-cli-command: >- | ||
| test-config | ||
| --config-keys ${{ inputs.config-keys || | ||
| 'dsv4-fp4-b200-vllm-mtp | ||
| dsv4-fp4-b300-sglang-mtp | ||
| qwen3.5-fp4-b200-sglang-mtp | ||
| qwen3.5-fp4-b300-sglang-mtp' }} | ||
| --no-evals | ||
| --config-files configs/nvidia-master.yaml configs/amd-master.yaml | ||
|
|
||
| # Fires even when some sweep jobs fail — ingest skips failed | ||
| # rows and a partially fresh snapshot beats an empty week. | ||
| trigger-ingest: | ||
|
Comment on lines
+35
to
+51
|
||
| needs: sweep | ||
| if: >- | ||
| always() && | ||
| needs.sweep.result != 'cancelled' && | ||
| needs.sweep.result != 'skipped' && | ||
| inputs.skip-ingest != true | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Trigger database ingest | ||
| run: | | ||
| curl -sSf -X POST \ | ||
| -H "Authorization: Bearer ${{ secrets.INFX_FRONTEND_PAT }}" \ | ||
| -H "Accept: application/vnd.github+v3+json" \ | ||
| https://api.github.com/repos/SemiAnalysisAI/InferenceX-app/dispatches \ | ||
| -d '{ | ||
| "event_type": "ingest-results", | ||
| "client_payload": { | ||
| "source-run-id": "${{ github.run_id }}", | ||
| "merge-run-id": "${{ github.run_id }}" | ||
| } | ||
| }' | ||
|
Comment on lines
+52
to
+72
Comment on lines
+50
to
+72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 This job's ingest curl block is a near-verbatim 4th copy of the repository_dispatch dispatch already implemented in run-sweep.yml (2 copies) and recover-reused-ingest.yml — same auth header, endpoint, event_type, and client_payload shape. Not blocking, but a shared composite action (e.g. Extended reasoning...The new The specific code path here is straightforward: whenever the sweep job in this new workflow completes (subject to the Nothing in the existing code prevents this duplication because there is no shared building block for it — each workflow file independently hardcodes the curl invocation inline rather than calling out to a composite action or reusable workflow. As a result, this PR's author (reasonably) copied the pattern from The impact is purely maintenance cost, not a functional bug: if the ingest contract ever changes (e.g. the auth secret is rotated to a different name, the endpoint moves, Step-by-step illustration: Suppose the app team renames the Suggested fix: Extract the dispatch into a composite action, e.g. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the scheduled run,
e2e-tests.ymlscores the matrix usinggithub.event_name, butconfigs/ci-priority.yamlboosts onlypush;scheduleandpull_requesttherefore receive the same event adjustment. Moreover, these FP4 SGLang dsv4/qwen3.5 jobs receive enough model, precision, framework, and MTP bonuses to tie or outrank PR jobs on the same runner pool, so this weekly 37-job batch can be selected ahead of the PR work it is intended to yield to. Add an explicit lower schedule priority rather than relying on the absence of an event bump.Useful? React with 👍 / 👎.