[PR #296] [MERGED] docs(refresh-reviewers): add its own caller guide so the workflows_ref cross-check covers it #296

New issue

Closed

opened 2026-10-26 22:38:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:38:11 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/296
Author: @mattmillerai
Created: 9/16/2026
Status: ✅ Merged
Merged: 9/17/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-14743-refresh-reviewers-docs-guide

📝 Commits (2)

7e5f4d3 docs(refresh-reviewers): add its own caller guide so the workflows_ref cross-check covers it
fc9f7ac docs(refresh-reviewers): tighten guide accuracy from cursor-review panel

📊 Changes

6 files changed (+168 additions, -11 deletions)

View changed files

📝 .github/bump-callers/README.md (+1 -0)
📝 .github/workflow-pins/check_workflow_pins.py (+9 -4)
📝 .github/workflow-pins/tests/test_check_workflow_pins.py (+28 -2)
📝 README.md (+1 -1)
📝 docs/callers/README.md (+6 -4)
➕ docs/callers/refresh-reviewers.md (+123 -0)

📄 Description

STACKED — merging lands on matt/be-6508-docs-callers-workflows-ref-cross-check (owned by mattmillerai), NOT main. This PR is based on the open cross-check PR #292, whose code is not on main yet. Do NOT merge to main until #292 lands; GitHub auto-retargets this PR to main when #292 merges (if #292 squash-merges, git merge origin/main into this branch to recompute the net diff — no force-push). "Ready to merge" here means ready once #292 has landed.

ELI-5

The pin-checker learned to catch a docs page that promises a "default" for workflows_ref the workflow itself forbids. But it finds the page by the workflow's own filename — and refresh-reviewers.yml had no page named after it, so the checker just skipped it. This PR gives refresh-reviewers.yml its own setup guide (with the correct no-default row), lists it in both catalog indexes, fixes two code comments that wrongly claimed it was "documented under assign-reviewers.md", and adds tests proving a workflow now gets cross-checked against its own-name page.

What changed

New docs/callers/refresh-reviewers.md — a caller guide mirroring pr-risk.md's structure (What it does / Prerequisites / Caller / Required permissions / Inputs / Gotchas). The Inputs table is populated from the workflow's own on.workflow_call.inputs, and the workflows_ref row uses the compliant — (**required**) no-default marker.
Indexed the new page in docs/callers/README.md and in the root README.md catalog (its Setup cell was a bare — precisely because no guide existed).
Reworded two inaccurate comments (check_workflow_pins.py and tests/test_check_workflow_pins.py) that asserted refresh-reviewers is "documented under assign-reviewers.md". That premise is false — assign-reviewers.yml is a different reusable that does not declare workflows_ref (README lists it among the non-declarers), so a workflows_ref row there would document a phantom input. The comments now use a generic internal bump-*/ci-*/test-* example (workflows that legitimately have no caller guide).
Two new DocsCrossCheckTests proving a reusable IS cross-checked via its OWN-name page: refresh-foo.yml + a refresh-foo.md whose workflows_ref row claims main yields one BE-6508 error naming refresh-foo.md; the companion asserts a — (**required**) row on refresh-foo.md is clean.

I did not add a workflows_ref row to assign-reviewers.md or introduce an alias map — the original suggestion to do so was based on the false premise above.

Verification

python3 .github/workflow-pins/check_workflow_pins.py --workflows-dir .github/workflows → OK — the output now lists checked refresh-reviewers.yml (previously skipped).
python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py' → 341 tests, OK.
Sanity: temporarily setting the new page's workflows_ref default to main failed the lint with ::error file=docs/callers/refresh-reviewers.md,line=85::… See BE-6508.; restoring returned green.

Provenance

Authored by: agent-work loop
Verified: the two commands above plus the negative sanity check, all run against this branch (stacked on #292); the new page's workflows_ref row was confirmed to normalize to a no-default marker by both the passing lint and the failing/restored sanity run.
Deviations: the spec's input-name list (history_months, max_experts_per_rule) was stale relative to the workflow — the Inputs table documents the actual declared names (window_months, top_k, floor, …), per the repo's "document only inputs that exist" rule. Also updated the root README.md catalog Setup cell for refresh-reviewers (beyond the spec's four items) to keep the two catalogs in sync, as AGENTS.md requires.

Residual

Not fixed / out of scope: nothing outstanding from the requested change — all four items are implemented and verified.
Unexercised artifacts: the originating spike issue and the related tickets named in the spec's duplicate-check (the reviewers-workflow and workflow-pins-parser tickets) live in Linear and are not reachable from this environment, so their content was not read; the reasoning here rests on the in-repo evidence (the workflow declaration, the cross-check code, and the docs catalogs), all of which was exercised directly.
Stacked-base caveat: this PR carries commits from #292 only by being based on it; if #292 squash-merges, re-run git merge origin/main here so the three-dot diff shows only this PR's changes.

_{🔄 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/296 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/16/2026 **Status:** ✅ Merged **Merged:** 9/17/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-14743-refresh-reviewers-docs-guide` --- ### 📝 Commits (2) - [`7e5f4d3`](https://github.com/Comfy-Org/github-workflows/commit/7e5f4d3ef5814645f38b7f96b4e90aad91196206) docs(refresh-reviewers): add its own caller guide so the workflows_ref cross-check covers it - [`fc9f7ac`](https://github.com/Comfy-Org/github-workflows/commit/fc9f7ac3dd01fbcc857add10c8a4c3ef9e59edbf) docs(refresh-reviewers): tighten guide accuracy from cursor-review panel ### 📊 Changes **6 files changed** (+168 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+1 -0) 📝 `.github/workflow-pins/check_workflow_pins.py` (+9 -4) 📝 `.github/workflow-pins/tests/test_check_workflow_pins.py` (+28 -2) 📝 `README.md` (+1 -1) 📝 `docs/callers/README.md` (+6 -4) ➕ `docs/callers/refresh-reviewers.md` (+123 -0) </details> ### 📄 Description **STACKED — merging lands on `matt/be-6508-docs-callers-workflows-ref-cross-check` (owned by mattmillerai), NOT `main`.** This PR is based on the open cross-check PR #292, whose code is not on `main` yet. Do NOT merge to `main` until #292 lands; GitHub auto-retargets this PR to `main` when #292 merges (if #292 squash-merges, `git merge origin/main` into this branch to recompute the net diff — no force-push). "Ready to merge" here means ready *once #292 has landed*. ## ELI-5 The pin-checker learned to catch a docs page that promises a "default" for `workflows_ref` the workflow itself forbids. But it finds the page by the workflow's own filename — and `refresh-reviewers.yml` had no page named after it, so the checker just skipped it. This PR gives `refresh-reviewers.yml` its own setup guide (with the correct no-default row), lists it in both catalog indexes, fixes two code comments that wrongly claimed it was "documented under assign-reviewers.md", and adds tests proving a workflow now gets cross-checked against its own-name page. ## What changed 1. **New `docs/callers/refresh-reviewers.md`** — a caller guide mirroring `pr-risk.md`'s structure (What it does / Prerequisites / Caller / Required permissions / Inputs / Gotchas). The Inputs table is populated from the workflow's own `on.workflow_call.inputs`, and the `workflows_ref` row uses the compliant `— (**required**)` no-default marker. 2. **Indexed the new page** in `docs/callers/README.md` and in the root `README.md` catalog (its Setup cell was a bare `—` precisely because no guide existed). 3. **Reworded two inaccurate comments** (`check_workflow_pins.py` and `tests/test_check_workflow_pins.py`) that asserted refresh-reviewers is "documented under assign-reviewers.md". That premise is false — `assign-reviewers.yml` is a different reusable that does **not** declare `workflows_ref` (README lists it among the non-declarers), so a `workflows_ref` row there would document a phantom input. The comments now use a generic internal `bump-*`/`ci-*`/`test-*` example (workflows that legitimately have no caller guide). 4. **Two new `DocsCrossCheckTests`** proving a reusable IS cross-checked via its OWN-name page: `refresh-foo.yml` + a `refresh-foo.md` whose `workflows_ref` row claims `main` yields one BE-6508 error naming `refresh-foo.md`; the companion asserts a `— (**required**)` row on `refresh-foo.md` is clean. I did **not** add a `workflows_ref` row to `assign-reviewers.md` or introduce an alias map — the original suggestion to do so was based on the false premise above. ## Verification - `python3 .github/workflow-pins/check_workflow_pins.py --workflows-dir .github/workflows` → **OK** — the output now lists `checked refresh-reviewers.yml` (previously skipped). - `python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py'` → **341 tests, OK**. - Sanity: temporarily setting the new page's `workflows_ref` default to `main` failed the lint with `::error file=docs/callers/refresh-reviewers.md,line=85::… See BE-6508.`; restoring returned green. ## Provenance - **Authored by:** agent-work loop - **Verified:** the two commands above plus the negative sanity check, all run against this branch (stacked on #292); the new page's `workflows_ref` row was confirmed to normalize to a no-default marker by both the passing lint and the failing/restored sanity run. - **Deviations:** the spec's input-name list (`history_months`, `max_experts_per_rule`) was stale relative to the workflow — the Inputs table documents the **actual** declared names (`window_months`, `top_k`, `floor`, …), per the repo's "document only inputs that exist" rule. Also updated the root `README.md` catalog Setup cell for refresh-reviewers (beyond the spec's four items) to keep the two catalogs in sync, as AGENTS.md requires. ## Residual - **Not fixed / out of scope:** nothing outstanding from the requested change — all four items are implemented and verified. - **Unexercised artifacts:** the originating spike issue and the related tickets named in the spec's duplicate-check (the reviewers-workflow and workflow-pins-parser tickets) live in Linear and are not reachable from this environment, so their content was not read; the reasoning here rests on the in-repo evidence (the workflow declaration, the cross-check code, and the docs catalogs), all of which was exercised directly. - **Stacked-base caveat:** this PR carries commits from #292 only by being based on it; if #292 squash-merges, re-run `git merge origin/main` here so the three-dot diff shows only this PR's changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:38:11 +00:00