[PR #158] [MERGED] fix(cursor-review): catalog-drift footer describes the report it is on (BE-6912) #168

New issue

Closed

opened 2026-10-26 22:36:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:36:58 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/158
Author: @mattmillerai
Created: 8/12/2026
Status: ✅ Merged
Merged: 8/12/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-6912-catalog-drift-footer

📝 Commits (3)

5b646e4 fix(cursor-review): catalog-drift footer describes the report it is on (BE-6912)
93e05dc fix(cursor-review): make the drift footer's claims survive the repin it describes (BE-6912)
0ca95c8 fix(cursor-review): gate the drift footer's claims on the action each one describes (BE-6912)

📊 Changes

3 files changed (+476 additions, -8 deletions)

View changed files

📝 .github/cursor-review/catalog-drift.py (+151 -4)
📝 .github/cursor-review/tests/test_catalog_drift.py (+324 -3)
📝 README.md (+1 -1)

📄 Description

ELI-5

Every week a robot checks whether the four AI models our PR-review panel uses still exist in Cursor's catalog. It writes what it finds into one issue that it keeps rewriting. The bottom of that issue said "this closes itself once there's nothing to report" — but it never can, because the catalog always lists extra reasoning tiers we don't pin, and those count as "something to report". So the issue sits open forever while telling you it shouldn't, which reads as "nobody has looked at this in weeks." That matters, because the same issue is where a genuinely broken pin gets reported. Now the bottom of the issue tells you which kind of report you're looking at: act on this now, or nothing here is urgent.

What changed

render_body()'s trailing footer was one fixed string; it is now _footer(report), rendered from the report in the same classes the report already separates internally:

urgent (delisted or zdr_risk) — "Act on this now:", names the 🚨 sections, says that is why the run is red, and states plainly that repinning will not close the issue (the next run just rewrites it without the 🚨 section).
findings, not urgent — "Nothing here is urgent:", the run is green, everything above is advisory, the issue stays open indefinitely, and an open issue here is not a sign anyone is ignoring it — plus a pointer to what an urgent report looks like, so a reader can tell the two apart without ever having seen one.
no findings — the only arm that promises a close, because it is the only state cursor-review-catalog-drift.yml actually closes on.

analyze() is untouched — no behavior change. Proof rather than assertion: running the CLI on the real cursor-review.yml and the captured 2026-08-10 catalog against origin/main's copy of the script produces a byte-identical report JSON (urgent, has_findings, every list), and the only line that differs in the rendered body is the footer.

Why the old promise could never be kept (falsification, not just reasoning)

The diff asserts a capability does not happen, so it was checked empirically rather than argued from the code:

Re-running analyze() on the fixture captured from run 31363137595 with that run's real pins: 192 catalog ids, 178 unpinned same-lab, 3 unpinned families, 0 delisted, 0 NO-ZDR, audit fresh → urgent=False, has_findings=True. The close arm needs has_findings=False.
GitHub state: repos/.../issues?state=all&labels=cursor-review-catalog-drift returns exactly one issue, #144, open since 2026-08-10, closed_at: null. No drift issue has ever been closed by the check.
That is now pinned by a test that reads the real catalog fixture and asserts the advisory arm renders (test_the_real_catalog_run_that_prompted_this_gets_the_advisory_footer).

Tests

New FooterTest (7 cases) covering the urgent arm (both delisted and NO-ZDR), the advisory arm, the clean arm, the real-catalog report, the invariant lead across all arms, and a guard that the exact old promise string never reappears on a report with findings. It also pins the fixture→state mapping, so if urgent/has_findings ever move, the tests stop silently testing the wrong arm. Three pre-existing assertions on the old footer text ("This issue is sticky", used as "the sections below the fold survived") were re-anchored to "Filed by the weekly", which is invariant across arms.

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' -v — 182 tests, OK.

Judgment calls

A fourth sub-arm the ticket did not ask for. A report whose only finding is a stale audit date has no standing list holding it open — it genuinely closes once the date is refreshed. Telling that reader "stays open indefinitely" would be this footer's own bug in miniature, so the persistence clause is conditional on unpinned or unpinned_labs actually being present. The stale-audit path itself is untouched, per the ticket's out-of-scope note; only the footer's description of it. Covered by test_a_findings_report_with_no_standing_list_is_not_told_it_stays_open_forever.
One clause of README.md beyond "one rendered string plus a test." The workflow catalog carried the same unreachable promise ("closed automatically on a clean run"). It is technically true of the mechanism, but it leaves the identical wrong impression in the second place it is stated, so it now says the close is rare in practice and points at the footer. Revert that hunk if you would rather keep the diff to the one file.
The urgent arm no longer says "close it by hand." An earlier draft did; that is bad advice, because the sticky issue is rewritten in place — once the pins are fixed the next run simply rewrites this issue as the advisory report, whereas closing it by hand makes the next run file a fresh one.

Out of scope and deliberately not touched, per the ticket: removing unpinned from has_findings (that is a decision — it would mean the weekly advisory never surfaces as an issue at all), what makes a run urgent, and the workflow's open/update/close branch.

Unrelated observation, not fixed here: python3 .github/agents-md-integrity/check_agents_md.py --root . fails on this repo's own AGENTS.md (227 lines, ceiling 200) on main. This repo has no ci-agents-md-integrity.yml caller, so nothing gates it.

Closes BE-6912.

_{🔄 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/github-workflows/pull/158 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/12/2026 **Status:** ✅ Merged **Merged:** 8/12/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-6912-catalog-drift-footer` --- ### 📝 Commits (3) - [`5b646e4`](https://github.com/Comfy-Org/github-workflows/commit/5b646e4fc7f707adff946ca9bfe8481c96383c34) fix(cursor-review): catalog-drift footer describes the report it is on (BE-6912) - [`93e05dc`](https://github.com/Comfy-Org/github-workflows/commit/93e05dce69be1f971d769b5b94c5a18aa18da345) fix(cursor-review): make the drift footer's claims survive the repin it describes (BE-6912) - [`0ca95c8`](https://github.com/Comfy-Org/github-workflows/commit/0ca95c8754dad6a165e9616850680db71841a043) fix(cursor-review): gate the drift footer's claims on the action each one describes (BE-6912) ### 📊 Changes **3 files changed** (+476 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/catalog-drift.py` (+151 -4) 📝 `.github/cursor-review/tests/test_catalog_drift.py` (+324 -3) 📝 `README.md` (+1 -1) </details> ### 📄 Description ## ELI-5 Every week a robot checks whether the four AI models our PR-review panel uses still exist in Cursor's catalog. It writes what it finds into one issue that it keeps rewriting. The bottom of that issue said "this closes itself once there's nothing to report" — but it never can, because the catalog always lists extra reasoning tiers we don't pin, and those count as "something to report". So the issue sits open forever while telling you it shouldn't, which reads as "nobody has looked at this in weeks." That matters, because the same issue is where a genuinely broken pin gets reported. Now the bottom of the issue tells you which kind of report you're looking at: **act on this now**, or **nothing here is urgent**. ## What changed `render_body()`'s trailing footer was one fixed string; it is now `_footer(report)`, rendered from the report in the same classes the report already separates internally: - **`urgent`** (`delisted or zdr_risk`) — "**Act on this now:**", names the 🚨 sections, says that is why the run is red, and states plainly that repinning will not close the issue (the next run just rewrites it without the 🚨 section). - **findings, not urgent** — "**Nothing here is urgent:**", the run is green, everything above is advisory, the issue stays open indefinitely, and **an open issue here is not a sign anyone is ignoring it** — plus a pointer to what an urgent report looks like, so a reader can tell the two apart without ever having seen one. - **no findings** — the only arm that promises a close, because it is the only state `cursor-review-catalog-drift.yml` actually closes on. `analyze()` is untouched — no behavior change. Proof rather than assertion: running the CLI on the real `cursor-review.yml` and the captured 2026-08-10 catalog against `origin/main`'s copy of the script produces a **byte-identical report JSON** (`urgent`, `has_findings`, every list), and the **only** line that differs in the rendered body is the footer. ## Why the old promise could never be kept (falsification, not just reasoning) The diff asserts a capability does *not* happen, so it was checked empirically rather than argued from the code: - Re-running `analyze()` on the fixture captured from run [31363137595](https://github.com/Comfy-Org/github-workflows/actions/runs/31363137595) with that run's real pins: **192 catalog ids, 178 unpinned same-lab, 3 unpinned families, 0 delisted, 0 NO-ZDR, audit fresh** → `urgent=False`, `has_findings=True`. The close arm needs `has_findings=False`. - GitHub state: `repos/.../issues?state=all&labels=cursor-review-catalog-drift` returns exactly one issue, [#144](https://github.com/Comfy-Org/github-workflows/issues/144), open since 2026-08-10, `closed_at: null`. No drift issue has ever been closed by the check. - That is now pinned by a test that reads the real catalog fixture and asserts the advisory arm renders (`test_the_real_catalog_run_that_prompted_this_gets_the_advisory_footer`). ## Tests New `FooterTest` (7 cases) covering the urgent arm (both delisted and NO-ZDR), the advisory arm, the clean arm, the real-catalog report, the invariant lead across all arms, and a guard that the exact old promise string never reappears on a report with findings. It also pins the fixture→state mapping, so if `urgent`/`has_findings` ever move, the tests stop silently testing the wrong arm. Three pre-existing assertions on the old footer text (`"This issue is sticky"`, used as "the sections below the fold survived") were re-anchored to `"Filed by the weekly"`, which is invariant across arms. `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' -v` — **182 tests, OK.** ## Judgment calls 1. **A fourth sub-arm the ticket did not ask for.** A report whose *only* finding is a stale audit date has no standing list holding it open — it genuinely closes once the date is refreshed. Telling that reader "stays open indefinitely" would be this footer's own bug in miniature, so the persistence clause is conditional on `unpinned or unpinned_labs` actually being present. The stale-audit *path itself* is untouched, per the ticket's out-of-scope note; only the footer's description of it. Covered by `test_a_findings_report_with_no_standing_list_is_not_told_it_stays_open_forever`. 2. **One clause of `README.md` beyond "one rendered string plus a test."** The workflow catalog carried the same unreachable promise ("closed automatically on a clean run"). It is technically true of the mechanism, but it leaves the identical wrong impression in the second place it is stated, so it now says the close is rare in practice and points at the footer. Revert that hunk if you would rather keep the diff to the one file. 3. **The urgent arm no longer says "close it by hand."** An earlier draft did; that is bad advice, because the sticky issue is rewritten in place — once the pins are fixed the next run simply rewrites this issue as the advisory report, whereas closing it by hand makes the next run file a fresh one. Out of scope and deliberately not touched, per the ticket: removing `unpinned` from `has_findings` (that is a decision — it would mean the weekly advisory never surfaces as an issue at all), what makes a run `urgent`, and the workflow's open/update/close branch. Unrelated observation, not fixed here: `python3 .github/agents-md-integrity/check_agents_md.py --root .` fails on this repo's own `AGENTS.md` (227 lines, ceiling 200) on `main`. This repo has no `ci-agents-md-integrity.yml` caller, so nothing gates it. Closes BE-6912. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:36:58 +00:00