[PR #6] [MERGED] Post PULSE comments on code owners' PRs when GPU outputs change #6

New issue

Closed

opened 2026-10-31 22:06:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:06:11 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfyci-runpod-worker/pull/6
Author: @marawan206
Created: 10/8/2026
Status: ✅ Merged
Merged: 10/8/2026
Merged by: @alexisrolland

Base: goldens-v2 ← Head: pulse-pr-comments

📝 Commits (9)

59c4dd4 Look up the merged pull request for each tested commit
23c8931 Write the run summary to a file and record each workflow's prior verdict
563ec75 Add PULSE PR comments for GPU output changes
94fd78c Run the PULSE PR comment step after the regression suite
775327c Document PULSE PR comments and the prior verdict field
a931c82 Pass the commit input to the target step through env and check it is a sha
bd82e28 Write the run summary only after the results are published
2828e04 Only report execution errors on workflows that ran before, and skip older-commit re-tests
6987318 Skip PR comments for runs that covered several commits

📊 Changes

11 files changed (+1105 additions, -17 deletions)

View changed files

📝 .github/workflows/regression.yml (+28 -5)
📝 README.md (+50 -3)
📝 scripts/commit_meta.py (+32 -3)
📝 scripts/index_builder.py (+33 -2)
➕ scripts/notify_pr.py (+371 -0)
📝 scripts/run_regression.py (+26 -3)
➕ scripts/tests/test_commit_meta.py (+103 -0)
📝 scripts/tests/test_golden_baseline.py (+2 -0)
📝 scripts/tests/test_index_builder.py (+17 -0)
➕ scripts/tests/test_notify_pr.py (+368 -0)
📝 scripts/tests/test_run_regression.py (+75 -1)

📄 Description

Stacked on #4 (goldens-v2), which adds the detail (new_drift / inherited), commit_meta and tested_range fields this builds on. Merge #4 first; this PR's diff is the last nine commits.

What this does

After each GPU regression run, scripts/notify_pr.py can leave one comment on the ComfyUI pull request that landed the tested commit. It is posted as the owner of PULSE_GH_TOKEN.

The first line starts with **(PULSE)**.
Below it is a compact table per workflow: verdict, new or inherited, PSNR dB, mean MSE, % pixels changed, exec s and peak VRAM.
Then the lane (python / torch / CUDA / GPU) and the run's page on ci.comfy.org.
Bodies stay under about 2 KB.

Default is dry-run. Nothing is posted until the PULSE_PR_COMMENTS repo variable is set to on. Until then the comment only appears in the job summary.

Policy

Comment when, at this commit:
- a workflow's outputs changed (detail: new_drift), or
- a workflow hit an execution error while its prior run still produced outputs.
Never comment for:
- pass, inherited drift, infra errors or missing goldens
- a workflow with no prior run (for example one just added)
- a re-test of a commit older than the previous run, which is compared with a newer run and so says nothing about its own PR
One comment per PR. It is found by a hidden marker () and the token owner's login, then edited in place. Re-running the same commit never adds a second comment. If a later run of that commit is clean, the comment becomes a short resolved note.
Code owners only. The PR author must be a user on the * rule of ComfyUI's CODEOWNERS. Today that is comfyanonymous, guill, alexisrolland, rattus128 and kijai; team entries are ignored. PULSE_ALLOWED_AUTHORS (comma-separated) replaces that list. If CODEOWNERS can't be read and no override is set, nothing is posted.
Single-PR runs only. A run that covered several new commits can't pin the change on one PR (and it may come from a non-owner's commit), so it is skipped.
Primary lane on master only.
Non-blocking. The step runs even when the suite fails and has continue-on-error: true. Any API or network problem is a warning with exit code 0.

Changes

commit_meta.fetch_commit_meta also calls GET /repos/Comfy-Org/ComfyUI/commits/{sha}/pulls.
- It stores pr_title, pr_author, pr_avatar, pr_labels, pr_merged_at and pr_url.
- pr falls back to the (#N) at the end of the commit subject.
- Existing keys are unchanged. Index entries gain pt / pa / av only when known.
run_regression.py
- New --summary-out writes the final summary once results are published, including on a failing run. A failed publish leaves no summary, so no comment links to a missing page.
- Each workflow records prior_verdict / prior_commit, read from the run index. Unlike the previous-run pointer, the index includes runs that had execution errors, so a standing error isn't reported on every following PR.
regression.yml
- The target and run steps take inputs.commit and inputs.workflows through env instead of pasting them into the shell. The commit input must be a hex sha.
- The run step writes $RUNNER_TEMP/summary.json.
- New step PR comment (PULSE).
README: new "PR comments (PULSE)" section.
Fixed a test that leaked index_builder.LANES_FILE into later tests.

How to enable

Create a personal access token for the account that should author the comments.
- Preferred: a fine-grained token limited to Comfy-Org/ComfyUI, with Issues and Pull requests set to read and write. It may need approval by an org owner.
- A classic public_repo token also works, but it can push to every public repo the account can write to.
Add it as the repo secret PULSE_GH_TOKEN.
Check the dry-run output in a few "GPU regression" job summaries.
Set the repo variable PULSE_PR_COMMENTS=on. Setting it to off stops the step at once.
Optionally, set PULSE_ALLOWED_AUTHORS to narrow or widen the author list.

Testing

python -m pytest scripts/tests passes (81 tests). With a mocked GitHub API they cover:

the code-owner gate
the policy matrix, including no prior run, re-tests of older commits and batched runs
updating the existing comment instead of creating a duplicate, and the clean-run resolved note
finding the comment across pages
dry-run, missing-token fallback and off modes
network errors exiting 0
no summary when publishing fails
body shape and size
the commit to PR lookup

A read-only dry run against a real code-owner commit (#16867) resolved the PR and author and reported "would create". Nothing was posted.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfyci-runpod-worker/pull/6 **Author:** [@marawan206](https://github.com/marawan206) **Created:** 10/8/2026 **Status:** ✅ Merged **Merged:** 10/8/2026 **Merged by:** [@alexisrolland](https://github.com/alexisrolland) **Base:** `goldens-v2` ← **Head:** `pulse-pr-comments` --- ### 📝 Commits (9) - [`59c4dd4`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/59c4dd421d7934507a4dc1f9a66e75a89a974709) Look up the merged pull request for each tested commit - [`23c8931`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/23c8931ed4a3c9511d7fa03c1868136ebf93802a) Write the run summary to a file and record each workflow's prior verdict - [`563ec75`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/563ec75e60b8fbf0ddd98847244339c539dfa21c) Add PULSE PR comments for GPU output changes - [`94fd78c`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/94fd78c204e9352feaad5a4b2c9ba29e041b57a0) Run the PULSE PR comment step after the regression suite - [`775327c`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/775327ca53a6e3a43b9ed98ba730d791d04e0108) Document PULSE PR comments and the prior verdict field - [`a931c82`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/a931c8263c6f0b0ed1dc628b8ab535d7aac047ba) Pass the commit input to the target step through env and check it is a sha - [`bd82e28`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/bd82e2828409be1cc688774959dd0066fe2f7c7e) Write the run summary only after the results are published - [`2828e04`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/2828e04bf3abe820d5c044ac29b045b11c8d1211) Only report execution errors on workflows that ran before, and skip older-commit re-tests - [`6987318`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/6987318a2f24449a230a8a2f33c8e0c6d86effcf) Skip PR comments for runs that covered several commits ### 📊 Changes **11 files changed** (+1105 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/regression.yml` (+28 -5) 📝 `README.md` (+50 -3) 📝 `scripts/commit_meta.py` (+32 -3) 📝 `scripts/index_builder.py` (+33 -2) ➕ `scripts/notify_pr.py` (+371 -0) 📝 `scripts/run_regression.py` (+26 -3) ➕ `scripts/tests/test_commit_meta.py` (+103 -0) 📝 `scripts/tests/test_golden_baseline.py` (+2 -0) 📝 `scripts/tests/test_index_builder.py` (+17 -0) ➕ `scripts/tests/test_notify_pr.py` (+368 -0) 📝 `scripts/tests/test_run_regression.py` (+75 -1) </details> ### 📄 Description Stacked on #4 (`goldens-v2`), which adds the `detail` (new_drift / inherited), `commit_meta` and `tested_range` fields this builds on. Merge #4 first; this PR's diff is the last nine commits. ## What this does After each GPU regression run, `scripts/notify_pr.py` can leave **one** comment on the ComfyUI pull request that landed the tested commit. It is posted as the owner of `PULSE_GH_TOKEN`. - The first line starts with `**(PULSE)**`. - Below it is a compact table per workflow: verdict, new or inherited, PSNR dB, mean MSE, % pixels changed, exec s and peak VRAM. - Then the lane (python / torch / CUDA / GPU) and the run's page on ci.comfy.org. - Bodies stay under about 2 KB. **Default is dry-run.** Nothing is posted until the `PULSE_PR_COMMENTS` repo variable is set to `on`. Until then the comment only appears in the job summary. ## Policy - **Comment** when, at this commit: - a workflow's outputs changed (`detail: new_drift`), or - a workflow hit an execution error while its prior run still produced outputs. - **Never comment** for: - pass, inherited drift, infra errors or missing goldens - a workflow with no prior run (for example one just added) - a re-test of a commit older than the previous run, which is compared with a newer run and so says nothing about its own PR - **One comment per PR.** It is found by a hidden marker (``) and the token owner's login, then edited in place. Re-running the same commit never adds a second comment. If a later run of that commit is clean, the comment becomes a short resolved note. - **Code owners only.** The PR author must be a user on the `*` rule of ComfyUI's `CODEOWNERS`. Today that is comfyanonymous, guill, alexisrolland, rattus128 and kijai; team entries are ignored. `PULSE_ALLOWED_AUTHORS` (comma-separated) replaces that list. If CODEOWNERS can't be read and no override is set, nothing is posted. - **Single-PR runs only.** A run that covered several new commits can't pin the change on one PR (and it may come from a non-owner's commit), so it is skipped. - **Primary lane on master only.** - **Non-blocking.** The step runs even when the suite fails and has `continue-on-error: true`. Any API or network problem is a warning with exit code 0. ## Changes - **`commit_meta.fetch_commit_meta`** also calls `GET /repos/Comfy-Org/ComfyUI/commits/{sha}/pulls`. - It stores `pr_title`, `pr_author`, `pr_avatar`, `pr_labels`, `pr_merged_at` and `pr_url`. - `pr` falls back to the `(#N)` at the end of the commit subject. - Existing keys are unchanged. Index entries gain `pt` / `pa` / `av` only when known. - **`run_regression.py`** - New `--summary-out` writes the final summary once results are published, including on a failing run. A failed publish leaves no summary, so no comment links to a missing page. - Each workflow records `prior_verdict` / `prior_commit`, read from the run index. Unlike the previous-run pointer, the index includes runs that had execution errors, so a standing error isn't reported on every following PR. - **`regression.yml`** - The target and run steps take `inputs.commit` and `inputs.workflows` through env instead of pasting them into the shell. The commit input must be a hex sha. - The run step writes `$RUNNER_TEMP/summary.json`. - New step **PR comment (PULSE)**. - **README:** new "PR comments (PULSE)" section. - Fixed a test that leaked `index_builder.LANES_FILE` into later tests. ## How to enable 1. Create a personal access token for the account that should author the comments. - Preferred: a fine-grained token limited to `Comfy-Org/ComfyUI`, with Issues and Pull requests set to read and write. It may need approval by an org owner. - A classic `public_repo` token also works, but it can push to every public repo the account can write to. 2. Add it as the repo secret `PULSE_GH_TOKEN`. 3. Check the dry-run output in a few "GPU regression" job summaries. 4. Set the repo variable `PULSE_PR_COMMENTS=on`. Setting it to `off` stops the step at once. 5. Optionally, set `PULSE_ALLOWED_AUTHORS` to narrow or widen the author list. ## Testing `python -m pytest scripts/tests` passes (81 tests). With a mocked GitHub API they cover: - the code-owner gate - the policy matrix, including no prior run, re-tests of older commits and batched runs - updating the existing comment instead of creating a duplicate, and the clean-run resolved note - finding the comment across pages - dry-run, missing-token fallback and off modes - network errors exiting 0 - no summary when publishing fails - body shape and size - the commit to PR lookup A read-only dry run against a real code-owner commit (#16867) resolved the PR and author and reported "would create". Nothing was posted. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:06:11 +00:00