Comfy-Org/github-workflows

Fork 0

mirror of https://github.com/Comfy-Org/github-workflows.git synced 2026-11-05 00:17:37 +00:00

[PR #298] [MERGED] test(refresh-reviewers): two-way input-vs-docs drift test for the caller guide #299

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-14761-refresh-reviewers-inputs-docs-drift-test

📝 Commits (4)

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
1bbe0e7 test(refresh-reviewers): pin workflow_call inputs against the docs/callers Inputs table
2321bdc test(refresh-reviewers): run the drift test on the files it parses; harden its scanners

📊 Changes

8 files changed (+320 additions, -11 deletions)

View changed files

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

📄 Description

STACKED — merging lands on matt/be-14743-refresh-reviewers-docs-guide (owned by @mattmillerai, PR #296), NOT main. This PR is based on #296's branch because the file it tests (docs/callers/refresh-reviewers.md) does not yet exist on main — it is introduced by #296. Do not treat this as unqualified "ready to merge": #296 must land first. When #296 merges, GitHub auto-retargets this PR to main; review here sees only the net diff (one new test file).

ELI-5

The refresh-reviewers reusable workflow has a list of knobs (its inputs:). Its brand-new setup guide has a table documenting those same knobs. If the two ever disagree — a knob documented but no longer real, or a real knob nobody wrote down — someone copying the guide gets a workflow that dies at startup with no logs, or a hidden feature nobody can find. This adds a tiny test that reads both lists and fails loudly the moment they drift apart, so the guide can never quietly go wrong.

What & why

Adds .github/refresh-reviewers/tests/test_inputs_docs.py: a stdlib-only (no PyYAML) two-set drift check pinning:

on.workflow_call.inputs declared in .github/workflows/refresh-reviewers.yml, against
the first-cell backticked names of the ## Inputs table in docs/callers/refresh-reviewers.md.

This is the BE-4691 failure mode applied to the new refresh-reviewers caller guide, which had no such pin: a phantom documented input gives a copying caller a zero-job startup_failure with no logs; a deleted-but-documented or added-but-undocumented input drifts silently. The test is modeled directly on the cursor-review panel's .github/cursor-review/tests/test_workflow_inputs_docs.py (same scanners, same parser-sanity guard), reduced to two sets.

Both sets currently resolve to the same 12 names: reviewer_config_path, window_months, half_life_days, top_k, floor, min_touches, min_score, floor_min_touches, map_exclude, extra_exclude_paths, pr_branch, workflows_ref. The sets are derived by scanning, not hardcoded.

Deliberately a TWO-set check

.github/refresh-reviewers/README.md's ## Knob defaults (and why) table is not included. Verified against the file: it is intentionally partial (omits reviewer_config_path, map_exclude, extra_exclude_paths, workflows_ref) and uses combined-cell rows (| \top_k` / `floor` |, | `min_touches` / `min_score` |`). The bare-name table regex would capture only 4 of the 12 declared inputs from it, so adding it as a third set would make the check permanently red or vacuous.

Parser-sanity guard

Each scan is asserted non-empty AND to contain workflows_ref, so a scanner that silently stops matching (a YAML/table structure change) fails loudly instead of comparing two empty sets and passing vacuously.

Verification

python3 -m unittest discover -s .github/refresh-reviewers/tests -p 'test_*.py' -v — 50 tests pass (48 pre-existing + 2 new), run on this stacked branch (i.e. a checkout that has #296's doc present, per the ticket's verification instruction).
Proved it bites: temporarily renaming top_k → top_kk in the doc table turned both new assertions red (missing: ['top_k'] and phantom: ['top_kk']); reverted.
Auto-discovered by test-refresh-reviewers.yml (path filter .github/refresh-reviewers/** + discover -p 'test_*.py'); no CI-wiring change needed.

Provenance

Authored by: agent-work loop
Verified: python3 -m unittest discover -s .github/refresh-reviewers/tests -p 'test_*.py' -v green (50/50) on the stacked branch; negative case (renamed doc input) confirmed red then reverted.
Deviations: none from the specified plan. One judgment call: the guide file is absent on main (introduced by open PR #296), so rather than parking this blocked, I stacked on #296's branch per the ALWAYS-STACK protocol — see the banner and Residual.

Residual

Blocked-on / stacked (not fixed by this PR): the tested file docs/callers/refresh-reviewers.md is introduced by open PR #296, not yet on main. This PR is stacked on #296's branch. If #296 is closed/abandoned rather than merged, this PR's base disappears and it should be closed too; if #296's branch is deleted on a squash-merge, re-open this against main (its diff is already main-clean — one new file). Until #296 merges, this test cannot run on main.
Drift risk while #296 is in review: if #296's ## Inputs table is edited before merge such that a first-cell input name diverges from refresh-reviewers.yml, this test will correctly go red — that is the test doing its job, not a false failure, and would flag a real doc bug in #296.
Unexercised artifacts I could not read (no Linear access from this environment): the originating Linear issues referenced by the ticket's provenance and the blocker relationship could not be opened; they are named only for traceability. The cursor-review model test and the two subject files (refresh-reviewers.yml, the docs/callers guide on #296's branch) and the README knob table were read and exercised directly.

_{🔄 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/298 **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-14761-refresh-reviewers-inputs-docs-drift-test` --- ### 📝 Commits (4) - [`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 - [`1bbe0e7`](https://github.com/Comfy-Org/github-workflows/commit/1bbe0e7b164eb58110b360aba8efb45d74280d96) test(refresh-reviewers): pin workflow_call inputs against the docs/callers Inputs table - [`2321bdc`](https://github.com/Comfy-Org/github-workflows/commit/2321bdc284f1022f5e173c5a6bbac47f9a168075) test(refresh-reviewers): run the drift test on the files it parses; harden its scanners ### 📊 Changes **8 files changed** (+320 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `.github/bump-callers/README.md` (+1 -0) ➕ `.github/refresh-reviewers/tests/test_inputs_docs.py` (+148 -0) 📝 `.github/workflow-pins/check_workflow_pins.py` (+9 -4) 📝 `.github/workflow-pins/tests/test_check_workflow_pins.py` (+28 -2) 📝 `.github/workflows/test-refresh-reviewers.yml` (+4 -0) 📝 `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-14743-refresh-reviewers-docs-guide` (owned by @mattmillerai, PR #296), NOT `main`.** This PR is based on #296's branch because the file it tests (`docs/callers/refresh-reviewers.md`) does not yet exist on `main` — it is introduced by #296. Do **not** treat this as unqualified "ready to merge": #296 must land first. When #296 merges, GitHub auto-retargets this PR to `main`; review here sees only the net diff (one new test file). ## ELI-5 The refresh-reviewers reusable workflow has a list of knobs (its `inputs:`). Its brand-new setup guide has a table documenting those same knobs. If the two ever disagree — a knob documented but no longer real, or a real knob nobody wrote down — someone copying the guide gets a workflow that dies at startup with no logs, or a hidden feature nobody can find. This adds a tiny test that reads both lists and fails loudly the moment they drift apart, so the guide can never quietly go wrong. ## What & why Adds `.github/refresh-reviewers/tests/test_inputs_docs.py`: a stdlib-only (no PyYAML) **two-set** drift check pinning: - `on.workflow_call.inputs` declared in `.github/workflows/refresh-reviewers.yml`, against - the first-cell backticked names of the `## Inputs` table in `docs/callers/refresh-reviewers.md`. This is the BE-4691 failure mode applied to the new refresh-reviewers caller guide, which had no such pin: a phantom documented input gives a copying caller a zero-job `startup_failure` with no logs; a deleted-but-documented or added-but-undocumented input drifts silently. The test is modeled directly on the cursor-review panel's `.github/cursor-review/tests/test_workflow_inputs_docs.py` (same scanners, same parser-sanity guard), reduced to two sets. Both sets currently resolve to the same 12 names: `reviewer_config_path, window_months, half_life_days, top_k, floor, min_touches, min_score, floor_min_touches, map_exclude, extra_exclude_paths, pr_branch, workflows_ref`. The sets are **derived by scanning**, not hardcoded. ### Deliberately a TWO-set check `.github/refresh-reviewers/README.md`'s `## Knob defaults (and why)` table is **not** included. Verified against the file: it is intentionally partial (omits `reviewer_config_path`, `map_exclude`, `extra_exclude_paths`, `workflows_ref`) and uses combined-cell rows (`| \`top_k\` / \`floor\` |`, `| \`min_touches\` / \`min_score\` |`). The bare-name table regex would capture only 4 of the 12 declared inputs from it, so adding it as a third set would make the check permanently red or vacuous. ### Parser-sanity guard Each scan is asserted non-empty AND to contain `workflows_ref`, so a scanner that silently stops matching (a YAML/table structure change) fails loudly instead of comparing two empty sets and passing vacuously. ## Verification - `python3 -m unittest discover -s .github/refresh-reviewers/tests -p 'test_*.py' -v` — **50 tests pass** (48 pre-existing + 2 new), run on this stacked branch (i.e. a checkout that has #296's doc present, per the ticket's verification instruction). - **Proved it bites:** temporarily renaming `top_k` → `top_kk` in the doc table turned both new assertions red (`missing: ['top_k']` and `phantom: ['top_kk']`); reverted. - Auto-discovered by `test-refresh-reviewers.yml` (path filter `.github/refresh-reviewers/**` + `discover -p 'test_*.py'`); **no CI-wiring change needed.** ## Provenance - **Authored by:** agent-work loop - **Verified:** `python3 -m unittest discover -s .github/refresh-reviewers/tests -p 'test_*.py' -v` green (50/50) on the stacked branch; negative case (renamed doc input) confirmed red then reverted. - **Deviations:** none from the specified plan. One judgment call: the guide file is absent on `main` (introduced by open PR #296), so rather than parking this blocked, I stacked on #296's branch per the ALWAYS-STACK protocol — see the banner and Residual. ## Residual - **Blocked-on / stacked (not fixed by this PR):** the tested file `docs/callers/refresh-reviewers.md` is introduced by open PR #296, not yet on `main`. This PR is stacked on #296's branch. If #296 is closed/abandoned rather than merged, this PR's base disappears and it should be closed too; if #296's branch is deleted on a squash-merge, re-open this against `main` (its diff is already `main`-clean — one new file). Until #296 merges, this test cannot run on `main`. - **Drift risk while #296 is in review:** if #296's `## Inputs` table is edited before merge such that a first-cell input name diverges from `refresh-reviewers.yml`, this test will correctly go red — that is the test doing its job, not a false failure, and would flag a real doc bug in #296. - **Unexercised artifacts I could not read (no Linear access from this environment):** the originating Linear issues referenced by the ticket's provenance and the blocker relationship could not be opened; they are named only for traceability. The cursor-review model test and the two subject files (`refresh-reviewers.yml`, the `docs/callers` guide on #296's branch) and the README knob table **were** read and exercised directly. --- <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:12 +00:00

closed this issue
added the
pull-request
label

No labels

cursor-review-catalog-drift

groom

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/github-workflows#299

No description provided.

Rows
Columns