[PR #161] [MERGED] [groom] `gate-unresolved.py` is documented as orphaned dead code in three places while build-ledger.py imports it #170

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/161
Author: @cloud-code-bot[bot]
Created: 8/13/2026
Status: ✅ Merged
Merged: 8/13/2026
Merged by: @mattmillerai

Base: main ← Head: groom/github-workflows-whole-repo-github-cursor-review-31687825490-0

📝 Commits (1)

7bdff0f groom: gate-unresolved.py is documented as orphaned dead code in three places while build-ledger.py imports it

📊 Changes

2 files changed (+9 additions, -4 deletions)

View changed files

📝 .github/cursor-review/README.md (+5 -2)
📝 docs/callers/cursor-review.md (+4 -2)

📄 Description

🤖 Auto-built by the groom sweep — this is a machine-proposed refactor. It runs full CI + cursor-review and requires human review; it is NOT auto-merged. Ranked from a CONFIRMED groom finding. · run

ELI-5

One Python file in this repo, gate-unresolved.py, has two lives. It has a command-line entry point — an old "block the merge while review comments are unresolved" gate — that nothing runs any more, and it also holds a few shared pieces (a marker string, a GitHub GraphQL query, and two small helper functions) that a very much alive script, build-ledger.py, imports and uses on every review run. Three documents said flatly that the file was "orphaned" and that "nothing calls this today", which reads like an invitation to delete it. Acting on that would break the prior-review ledger the review panel consumes. This PR rewrites those three sentences so they say what is actually true: the CLI is unwired, the module is not dead code. No code, workflow, or behavior is touched.

What changed

Documentation only — three sites, no code:

.github/cursor-review/README.md:87 — the file-table row no longer says "Orphaned — nothing calls this today". It now says the CLI is unwired but the module is imported by build-ledger.py for CONSOLIDATED_MARKER, the paging reviewThreads GraphQL query, and the iter_threads / is_cursor_thread helpers, and that the file should not be deleted.
.github/cursor-review/README.md:166 — the blocking-gate regression note no longer calls the implementation "orphaned and unwired"; it now distinguishes the unwired CLI/main from the still-live module and names the importer.
docs/callers/cursor-review.md:27 — the same correction in the caller guide: the script is still in the tree with its CLI unwired, and the module itself is still imported by build-ledger.py.

AGENTS.md:206 is deliberately left alone: it cites this file as the worked example of a deleted input's leftovers, which is about the blocking: input and remains accurate.

Why

The docs described the file as fully orphaned, but build-ledger.py imports it by path through an importlib shim (the filename is hyphenated) and then depends on it structurally via CONSOLIDATED_MARKER, gate_unresolved.iter_threads(...), and gate_unresolved.is_cursor_thread(...); tests/test_build_ledger.py asserts that reuse is real. A reader trusting the old wording could delete the file as dead code, which would break the prior-review ledger that the review panel consumes — no such deletion has occurred, so this removes a trap rather than repairing an observed failure. Behavior is preserved exactly because the change is prose-only: no script, workflow, or test is modified. The optional follow-up the finding mentions — extracting the shared pieces into a normally-importable module and dropping the importlib shim — is intentionally left out to keep this diff minimal.

Verifier rationale

Verified

.github/cursor-review/README.md:87 describes the file as "Orphaned — nothing calls this today."; :166 repeats "still here, orphaned and unwired"; docs/callers/cursor-review.md:27 says it "is still in the tree, orphaned". (AGENTS.md:206 cites it as the worked example of a deleted input's leftovers — that framing is about the blocking: input and is fine.)

But .github/cursor-review/build-ledger.py:87-101 imports it by path through an importlib shim (needed because the filename is hyphenated) and then depends on it structurally: CONSOLIDATED_MARKER (:102), gate_unresolved.iter_threads(...) (:171) and gate_unresolved.is_cursor_thread(thread) (:291), with tests/test_build_ledger.py:518-544 asserting the reuse is real (it checks bl.gate_unresolved.CONSOLIDATED_MARKER is bl.CONSOLIDATED_MARKER and that the shared QUERY contains reviewThreads/databaseId).

So the file is half dead (its blocking-gate CLI/main has no caller) and half a shared library — and the docs tell a reader it is safe to delete. Deleting it would break the prior-review ledger that the review panel consumes.

Fix (in priority order)

Correct the three docs (README.md:87 and :166, docs/callers/cursor-review.md:27): say the blocking-gate CLI is unwired, and that the module is imported by build-ledger.py for the marker, the GraphQL query and the thread helpers. This alone removes the trap and is a docs-only change.
Optional: move CONSOLIDATED_MARKER + QUERY + iter_threads/is_cursor_thread into a normally-importable .github/cursor-review/review_threads.py, leaving gate-unresolved.py as the thin unwired CLI over it, which also drops the importlib-by-path shim. Keep the CLI — the blocking gate is an intentionally-preserved regression someone may re-wire.

Steelman

The shim is five lines and works, and renaming things makes the #31 regression history harder to follow. That is why step 1 (docs) is the confirmed core and step 2 is optional.

Risk

Low. One import site plus its tests, all inside this repo; no external path references the module.

_{🔄 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/161 **Author:** [@cloud-code-bot[bot]](https://github.com/apps/cloud-code-bot) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `groom/github-workflows-whole-repo-github-cursor-review-31687825490-0` --- ### 📝 Commits (1) - [`7bdff0f`](https://github.com/Comfy-Org/github-workflows/commit/7bdff0fe3b1d4df89409b07e069ee57a5fc79209) groom: `gate-unresolved.py` is documented as orphaned dead code in three places while build-ledger.py imports it ### 📊 Changes **2 files changed** (+9 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+5 -2) 📝 `docs/callers/cursor-review.md` (+4 -2) </details> ### 📄 Description > 🤖 **Auto-built by the groom sweep** — this is a machine-proposed refactor. It runs full CI + cursor-review and **requires human review; it is NOT auto-merged.** Ranked from a CONFIRMED groom finding. · [run](https://github.com/Comfy-Org/github-workflows/actions/runs/31687825490) ## ELI-5 One Python file in this repo, `gate-unresolved.py`, has two lives. It has a command-line entry point — an old "block the merge while review comments are unresolved" gate — that nothing runs any more, and it also holds a few shared pieces (a marker string, a GitHub GraphQL query, and two small helper functions) that a very much alive script, `build-ledger.py`, imports and uses on every review run. Three documents said flatly that the file was "orphaned" and that "nothing calls this today", which reads like an invitation to delete it. Acting on that would break the prior-review ledger the review panel consumes. This PR rewrites those three sentences so they say what is actually true: the CLI is unwired, the module is not dead code. No code, workflow, or behavior is touched. ## What changed Documentation only — three sites, no code: - `.github/cursor-review/README.md:87` — the file-table row no longer says "Orphaned — nothing calls this today". It now says the CLI is unwired but the module is imported by `build-ledger.py` for `CONSOLIDATED_MARKER`, the paging `reviewThreads` GraphQL query, and the `iter_threads` / `is_cursor_thread` helpers, and that the file should not be deleted. - `.github/cursor-review/README.md:166` — the blocking-gate regression note no longer calls the implementation "orphaned and unwired"; it now distinguishes the unwired CLI/`main` from the still-live module and names the importer. - `docs/callers/cursor-review.md:27` — the same correction in the caller guide: the script is still in the tree with its CLI unwired, and the module itself is still imported by `build-ledger.py`. `AGENTS.md:206` is deliberately left alone: it cites this file as the worked example of a deleted *input*'s leftovers, which is about the `blocking:` input and remains accurate. ## Why The docs described the file as fully orphaned, but `build-ledger.py` imports it by path through an `importlib` shim (the filename is hyphenated) and then depends on it structurally via `CONSOLIDATED_MARKER`, `gate_unresolved.iter_threads(...)`, and `gate_unresolved.is_cursor_thread(...)`; `tests/test_build_ledger.py` asserts that reuse is real. A reader trusting the old wording could delete the file as dead code, which would break the prior-review ledger that the review panel consumes — no such deletion has occurred, so this removes a trap rather than repairing an observed failure. Behavior is preserved exactly because the change is prose-only: no script, workflow, or test is modified. The optional follow-up the finding mentions — extracting the shared pieces into a normally-importable module and dropping the `importlib` shim — is intentionally left out to keep this diff minimal. <details> <summary><strong>Verifier rationale</strong></summary> ## Verified `.github/cursor-review/README.md:87` describes the file as "**Orphaned — nothing calls this today.**"; `:166` repeats "still here, orphaned and unwired"; `docs/callers/cursor-review.md:27` says it "is still in the tree, orphaned". (AGENTS.md:206 cites it as the worked example of a deleted input's leftovers — that framing is about the `blocking:` input and is fine.) But `.github/cursor-review/build-ledger.py:87-101` imports it by path through an `importlib` shim (needed because the filename is hyphenated) and then depends on it structurally: `CONSOLIDATED_MARKER` (:102), `gate_unresolved.iter_threads(...)` (:171) and `gate_unresolved.is_cursor_thread(thread)` (:291), with `tests/test_build_ledger.py:518-544` asserting the reuse is real (it checks `bl.gate_unresolved.CONSOLIDATED_MARKER is bl.CONSOLIDATED_MARKER` and that the shared `QUERY` contains `reviewThreads`/`databaseId`). So the file is half dead (its blocking-gate CLI/`main` has no caller) and half a shared library — and the docs tell a reader it is safe to delete. Deleting it would break the prior-review ledger that the review panel consumes. ## Fix (in priority order) 1. **Correct the three docs** (README.md:87 and :166, docs/callers/cursor-review.md:27): say the blocking-gate *CLI* is unwired, and that the module is imported by `build-ledger.py` for the marker, the GraphQL query and the thread helpers. This alone removes the trap and is a docs-only change. 2. **Optional:** move `CONSOLIDATED_MARKER` + `QUERY` + `iter_threads`/`is_cursor_thread` into a normally-importable `.github/cursor-review/review_threads.py`, leaving `gate-unresolved.py` as the thin unwired CLI over it, which also drops the `importlib`-by-path shim. Keep the CLI — the blocking gate is an intentionally-preserved regression someone may re-wire. ## Steelman The shim is five lines and works, and renaming things makes the #31 regression history harder to follow. That is why step 1 (docs) is the confirmed core and step 2 is optional. ## Risk Low. One import site plus its tests, all inside this repo; no external path references the module. </details>  --- <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