mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
## Summary Fixes three critical issues with the CI performance reporting pipeline that made perf reports useless on PRs (demonstrated by PR #9248 — deep watcher removal merged without useful perf signal). ## Changes ### 1. Fix z-score baseline variance collection (`0/5 runs`) **Root cause:** PR #9305 added z-score statistical analysis code to `perf-report.ts`, but the historical data download step was placed in the wrong workflow file. The report is generated in `pr-perf-report.yaml` (a `workflow_run`-triggered job), but the historical download was in `ci-perf-report.yaml` (the test runner) — different runners, different filesystems. **Fix:** Implement `perf-data` orphan branch storage: - On push to main: save `perf-metrics.json` to `perf-data` branch with timestamped filename - On PR report: fetch last 5 baselines from `perf-data` branch into `temp/perf-history/` - Rolling window of 20 baselines, oldest pruned automatically - Same pattern used by `github-action-benchmark` (33.7k repos) ### 2. Fix force-push comment staleness **Root cause:** `cancel-in-progress: true` kills the perf test run before it uploads artifacts. The downstream report workflow only triggers on `conclusion == 'success'` — cancelled runs are ignored, so the comment from the first successful run goes stale. **Fix:** - Change `cancel-in-progress: false` — with GitHub's queue depth of 1, rapid pushes (A,B,C,D) run A and D, skipping B and C - Add SHA validation in `pr-perf-report.yaml` — before posting, check if the workflow_run's head SHA still matches the PR's current head. Skip posting stale results. ### 3. Add permissions for baseline operations - `contents: write` on CI job (needed for pushing to perf-data branch) - `actions: read` on both workflows (needed for artifact/baseline access) ## One-time setup required After merging, create the `perf-data` orphan branch: ```bash git checkout --orphan perf-data git rm -rf . echo '# Performance Baselines' > README.md mkdir -p baselines git add README.md baselines git commit -m 'Initialize perf-data branch' git push origin perf-data ``` The first 2 pushes to main after setup will build up variance data, and z-scores will start appearing in PR reports (threshold is `historical.length >= 2`). ## Testing - YAML validated with `yaml.safe_load()` - `perf-report.ts` `loadHistoricalReports()` already reads from `temp/perf-history/<index>/perf-metrics.json` — no code changes needed - All new steps use `continue-on-error: true` for graceful degradation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9886-fix-fix-perf-CI-pipeline-z-score-baselines-force-push-staleness-baseline-storage-3226d73d365081538424c7945e71f308) by [Unito](https://www.unito.io)
149 lines
5.1 KiB
YAML
149 lines
5.1 KiB
YAML
name: 'PR: Performance Report'
|
|
|
|
on:
|
|
workflow_run:
|
|
workflows: ['CI: Performance Report']
|
|
types:
|
|
- completed
|
|
|
|
permissions:
|
|
contents: read
|
|
pull-requests: write
|
|
issues: write
|
|
actions: read
|
|
|
|
jobs:
|
|
comment:
|
|
runs-on: ubuntu-latest
|
|
if: >
|
|
github.repository == 'Comfy-Org/ComfyUI_frontend' &&
|
|
github.event.workflow_run.event == 'pull_request' &&
|
|
github.event.workflow_run.conclusion == 'success'
|
|
steps:
|
|
- name: Checkout repository
|
|
uses: actions/checkout@v6
|
|
|
|
- name: Setup Node
|
|
uses: actions/setup-node@v6
|
|
with:
|
|
node-version-file: '.nvmrc'
|
|
|
|
- name: Download PR metadata
|
|
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
|
with:
|
|
name: perf-meta
|
|
run_id: ${{ github.event.workflow_run.id }}
|
|
path: temp/perf-meta/
|
|
|
|
- name: Resolve and validate PR metadata
|
|
id: pr-meta
|
|
uses: actions/github-script@v8
|
|
with:
|
|
script: |
|
|
const fs = require('fs');
|
|
const artifactPr = Number(fs.readFileSync('temp/perf-meta/number.txt', 'utf8').trim());
|
|
const artifactBase = fs.readFileSync('temp/perf-meta/base.txt', 'utf8').trim();
|
|
|
|
// Resolve PR from trusted workflow context
|
|
let pr = context.payload.workflow_run.pull_requests?.[0];
|
|
if (!pr) {
|
|
const { data: prs } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
commit_sha: context.payload.workflow_run.head_sha,
|
|
});
|
|
pr = prs.find(p => p.state === 'open');
|
|
}
|
|
|
|
if (!pr) {
|
|
core.setFailed('Unable to resolve PR from workflow_run context.');
|
|
return;
|
|
}
|
|
|
|
if (Number(pr.number) !== artifactPr) {
|
|
core.setFailed(`Artifact PR number (${artifactPr}) does not match trusted context (${pr.number}).`);
|
|
return;
|
|
}
|
|
|
|
const trustedBase = pr.base?.ref;
|
|
if (!trustedBase || artifactBase !== trustedBase) {
|
|
core.setFailed(`Artifact base (${artifactBase}) does not match trusted context (${trustedBase}).`);
|
|
return;
|
|
}
|
|
|
|
core.setOutput('number', String(pr.number));
|
|
core.setOutput('base', trustedBase);
|
|
|
|
- name: Check if results are still current
|
|
id: sha-check
|
|
uses: actions/github-script@v8
|
|
with:
|
|
script: |
|
|
const prNumber = Number('${{ steps.pr-meta.outputs.number }}');
|
|
const { data: pr } = await github.rest.pulls.get({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
pull_number: prNumber,
|
|
});
|
|
const runSha = context.payload.workflow_run.head_sha;
|
|
const currentSha = pr.head.sha;
|
|
if (runSha !== currentSha) {
|
|
core.info(`Skipping stale report: run SHA ${runSha} != current PR SHA ${currentSha}`);
|
|
core.setOutput('stale', 'true');
|
|
} else {
|
|
core.setOutput('stale', 'false');
|
|
}
|
|
|
|
- name: Download PR perf metrics
|
|
if: steps.sha-check.outputs.stale != 'true'
|
|
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
|
with:
|
|
name: perf-metrics
|
|
run_id: ${{ github.event.workflow_run.id }}
|
|
path: test-results/
|
|
|
|
- name: Download baseline perf metrics
|
|
if: steps.sha-check.outputs.stale != 'true'
|
|
uses: dawidd6/action-download-artifact@0bd50d53a6d7fb5cb921e607957e9cc12b4ce392 # v12
|
|
with:
|
|
branch: ${{ steps.pr-meta.outputs.base }}
|
|
workflow: ci-perf-report.yaml
|
|
event: push
|
|
name: perf-metrics
|
|
path: temp/perf-baseline/
|
|
if_no_artifact_found: warn
|
|
|
|
- name: Load historical baselines from perf-data branch
|
|
if: steps.sha-check.outputs.stale != 'true'
|
|
continue-on-error: true
|
|
run: |
|
|
mkdir -p temp/perf-history
|
|
|
|
git fetch origin perf-data 2>/dev/null || {
|
|
echo "perf-data branch not found, skipping historical data"
|
|
exit 0
|
|
}
|
|
|
|
INDEX=0
|
|
for file in $(git ls-tree --name-only origin/perf-data baselines/ 2>/dev/null | sort -r | head -5); do
|
|
DIR="temp/perf-history/$INDEX"
|
|
mkdir -p "$DIR"
|
|
git show "origin/perf-data:${file}" > "$DIR/perf-metrics.json" 2>/dev/null || true
|
|
INDEX=$((INDEX + 1))
|
|
done
|
|
|
|
echo "Loaded $INDEX historical baselines"
|
|
|
|
- name: Generate perf report
|
|
if: steps.sha-check.outputs.stale != 'true'
|
|
run: npx --yes tsx scripts/perf-report.ts > perf-report.md
|
|
|
|
- name: Post PR comment
|
|
if: steps.sha-check.outputs.stale != 'true'
|
|
uses: ./.github/actions/post-pr-report-comment
|
|
with:
|
|
pr-number: ${{ steps.pr-meta.outputs.number }}
|
|
report-file: ./perf-report.md
|
|
comment-marker: '<!-- COMFYUI_FRONTEND_PERF -->'
|
|
token: ${{ secrets.GITHUB_TOKEN }}
|