[PR #2] [MERGED] Shrink the results checkout and make comparison figures deduplicate #1

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: storage-hygiene

📝 Commits (3)

b40f424 Use a blobless sparse checkout for the results branch and regenerate derived files on push retry
0d98b28 Make comparison figures content-addressable and skip them for inherited failures
dad058f Keep worker error details and fix delay units in run records

📊 Changes

14 files changed (+626 additions, -42 deletions)

View changed files

📝 .github/workflows/regression.yml (+1 -1)
➕ .github/workflows/scripts-tests.yml (+22 -0)
📝 .gitignore (+1 -0)
📝 README.md (+19 -0)
📝 scripts/compare.py (+16 -5)
📝 scripts/gcs_publish.py (+5 -0)
📝 scripts/run_regression.py (+71 -17)
📝 scripts/storage.py (+163 -16)
📝 scripts/submit_and_poll.py (+30 -3)
➕ scripts/tests/conftest.py (+5 -0)
➕ scripts/tests/test_compare.py (+55 -0)
➕ scripts/tests/test_run_regression.py (+42 -0)
➕ scripts/tests/test_storage.py (+128 -0)
➕ scripts/tests/test_submit_and_poll.py (+68 -0)

📄 Description

Why

Three things were growing or fragile on the results branch:

Comparison figures embedded the candidate commit sha in their titles, so every run of the current FLUX drift re-rendered a unique 427 KB pair. Figures are 86% of the results branch today even though the output PNGs themselves deduplicate almost perfectly (459 files, 3 unique).
Every publish and every 30-minute poll fetched and checked out the entire results tree (~775 MB of mostly PNGs) on a 14 GB runner.
GitHubStorage.finalize recovered from a rejected push by byte-restoring the files it had written, which would silently clobber any shared aggregate file (needed for the upcoming run index).

What

storage.py: blobless partial clone + cone sparse-checkout of the small aggregate directories; run directories are materialised on first use, behind a lock (the orchestrator reads and writes from worker threads). Derived writers (register_derived) regenerate aggregate files from the newest remote tree inside the push retry loop (8 attempts, jittered); a failing writer is a warning and its partial output is discarded, primary results always land. A fetch failure other than "branch does not exist" is fatal instead of publishing against an empty tree; staging uses --sparse. list_json/read_blob_json read committed files through git without checking them out; the GCS backend gets the matching list helper.
compare.py: stable figure titles (golden <tag> / previous vs candidate), no matplotlib Software metadata, reuse of precomputed metrics, figures=False switch.
run_regression.py: compares against the previous tested run first and classifies a failure vs golden as new_drift (outputs changed at this commit) or inherited (bit-identical to a previous run that already failed against the same golden); inherited failures skip the golden figures and say so (figures: false). Summaries gain schema_version: 2, has_infra_error, and per-workflow detail, previous_verdict, figures, output_sha256. run.json no longer leaks the runner-local outputs_dir.
submit_and_poll.py: keeps the handler's error detail (the RunPod SDK lifts it to the job level as a JSON string; published run.json files showed error: null), records delay_ms and a converted delay_s, classifies bad_request as infrastructure without retrying it.
regression.yml: the skip check fetches blobless too.
New scripts/tests (pytest, 16 tests) and a Script tests workflow (read-only token).

All new fields are additive; the 230 existing runs and both goldens are untouched.

Verification

python -m pytest scripts/tests -q: storage backend against a local bare repo (sparse materialisation, two concurrent publishers both landing their derived files, failing derived writer never blocks primary results, fatal fetch errors, first publish to a missing branch), figure byte-identity for identical candidates, drift classification, error decoding / delay units / no retry for bad_request.
After merge: the next tested run's results-branch commit should add no figures/ for the inherited FLUX failure, and the runner log should show a sub-second checkout instead of a ~775 MB one.

Stacked: this PR, then #3, then #4.

_{🔄 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/2 **Author:** [@marawan206](https://github.com/marawan206) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/8/2026 **Merged by:** [@alexisrolland](https://github.com/alexisrolland) **Base:** `main` ← **Head:** `storage-hygiene` --- ### 📝 Commits (3) - [`b40f424`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/b40f424f12ca5a595d6d905355dbd0679f1875d1) Use a blobless sparse checkout for the results branch and regenerate derived files on push retry - [`0d98b28`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/0d98b2818f3730b239733c2250619aa09c7707aa) Make comparison figures content-addressable and skip them for inherited failures - [`dad058f`](https://github.com/Comfy-Org/comfyci-runpod-worker/commit/dad058fbf829c4a5c393e17d8b6feaa123e131da) Keep worker error details and fix delay units in run records ### 📊 Changes **14 files changed** (+626 additions, -42 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/regression.yml` (+1 -1) ➕ `.github/workflows/scripts-tests.yml` (+22 -0) 📝 `.gitignore` (+1 -0) 📝 `README.md` (+19 -0) 📝 `scripts/compare.py` (+16 -5) 📝 `scripts/gcs_publish.py` (+5 -0) 📝 `scripts/run_regression.py` (+71 -17) 📝 `scripts/storage.py` (+163 -16) 📝 `scripts/submit_and_poll.py` (+30 -3) ➕ `scripts/tests/conftest.py` (+5 -0) ➕ `scripts/tests/test_compare.py` (+55 -0) ➕ `scripts/tests/test_run_regression.py` (+42 -0) ➕ `scripts/tests/test_storage.py` (+128 -0) ➕ `scripts/tests/test_submit_and_poll.py` (+68 -0) </details> ### 📄 Description ## Why Three things were growing or fragile on the results branch: - Comparison figures embedded the candidate commit sha in their titles, so every run of the current FLUX drift re-rendered a unique 427 KB pair. Figures are 86% of the results branch today even though the output PNGs themselves deduplicate almost perfectly (459 files, 3 unique). - Every publish and every 30-minute poll fetched and checked out the entire results tree (~775 MB of mostly PNGs) on a 14 GB runner. - `GitHubStorage.finalize` recovered from a rejected push by byte-restoring the files it had written, which would silently clobber any shared aggregate file (needed for the upcoming run index). ## What - `storage.py`: blobless partial clone + cone sparse-checkout of the small aggregate directories; run directories are materialised on first use, behind a lock (the orchestrator reads and writes from worker threads). Derived writers (`register_derived`) regenerate aggregate files from the newest remote tree inside the push retry loop (8 attempts, jittered); a failing writer is a warning and its partial output is discarded, primary results always land. A fetch failure other than "branch does not exist" is fatal instead of publishing against an empty tree; staging uses `--sparse`. `list_json`/`read_blob_json` read committed files through git without checking them out; the GCS backend gets the matching list helper. - `compare.py`: stable figure titles (`golden <tag>` / `previous` vs `candidate`), no matplotlib Software metadata, reuse of precomputed metrics, `figures=False` switch. - `run_regression.py`: compares against the previous tested run first and classifies a failure vs golden as `new_drift` (outputs changed at this commit) or `inherited` (bit-identical to a previous run that already failed against the same golden); inherited failures skip the golden figures and say so (`figures: false`). Summaries gain `schema_version: 2`, `has_infra_error`, and per-workflow `detail`, `previous_verdict`, `figures`, `output_sha256`. `run.json` no longer leaks the runner-local `outputs_dir`. - `submit_and_poll.py`: keeps the handler's error detail (the RunPod SDK lifts it to the job level as a JSON string; published `run.json` files showed `error: null`), records `delay_ms` and a converted `delay_s`, classifies `bad_request` as infrastructure without retrying it. - `regression.yml`: the skip check fetches blobless too. - New `scripts/tests` (pytest, 16 tests) and a `Script tests` workflow (read-only token). All new fields are additive; the 230 existing runs and both goldens are untouched. ## Verification - `python -m pytest scripts/tests -q`: storage backend against a local bare repo (sparse materialisation, two concurrent publishers both landing their derived files, failing derived writer never blocks primary results, fatal fetch errors, first publish to a missing branch), figure byte-identity for identical candidates, drift classification, error decoding / delay units / no retry for `bad_request`. - After merge: the next tested run's results-branch commit should add no `figures/` for the inherited FLUX failure, and the runner log should show a sub-second checkout instead of a ~775 MB one. Stacked: this PR, then #3, then #4. --- <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:09 +00:00