[PR #316] [MERGED] test(workflow-pins): fold the cursor-review and refresh-reviewers input-docs suites into the drift harness #317

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/316
Author: @mattmillerai
Created: 9/18/2026
Status: ✅ Merged
Merged: 9/19/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-15260-fold-input-docs-drift

📝 Commits (4)

6303b2c test(workflow-pins): fold the cursor-review and refresh-reviewers input-docs suites into the drift harness
9bb0ddc test(workflow-pins): assert both dir_readme paths stay in test-workflow-pins.yml's paths: filters
5822413 test(workflow-pins): close the drift harness's own vacuity gaps from review
2d122d3 Merge remote-tracking branch 'origin/main' into matt/be-15260-fold-input-docs-drift

📊 Changes

7 files changed (+356 additions, -704 deletions)

View changed files

➖ .github/cursor-review/tests/test_workflow_inputs_docs.py (+0 -141)
➖ .github/refresh-reviewers/tests/test_inputs_docs.py (+0 -536)
📝 .github/workflow-pins/README.md (+15 -1)
📝 .github/workflow-pins/tests/test_inputs_docs_drift.py (+309 -14)
📝 .github/workflows/test-cursor-review-scripts.yml (+8 -10)
📝 .github/workflows/test-refresh-reviewers.yml (+6 -2)
📝 .github/workflows/test-workflow-pins.yml (+18 -0)

📄 Description

ELI-5

Two little test files each checked, by hand, that one workflow's documented knobs match the knobs it actually has. A previous PR replaced both with a single table-driven check covering all 16 reusable workflows, but deliberately left the two originals alive. This deletes them and moves their last two extras — the directory-README knob tables and one Default-column comparison — into the shared table, then points CI's path filters at the files the shared check now reads.

What changed

One parser, not three. .github/cursor-review/tests/test_workflow_inputs_docs.py (141 lines) and .github/refresh-reviewers/tests/test_inputs_docs.py (536 lines) are deleted. Everything they asserted is carried by two columns on their rows in .github/workflow-pins/tests/test_inputs_docs_drift.py:

row	`dir_readme`	`readme_heading`	`readme_mode`	`check_defaults`
`cursor-review`	`.github/cursor-review/README.md`	`## Configuration knobs`	`two-way`	`False`
`refresh-reviewers`	`.github/refresh-reviewers/README.md`	`## Knob defaults (and why)`	`one-way`	`True`

The asymmetry is deliberate and both halves were verified rather than assumed. cursor-review's README table really is complete, so two-way set equality is what the deleted suite asserted and what the row now asserts. refresh-reviewers' table is intentionally partial (it omits reviewer_config_path, map_exclude, extra_exclude_paths and workflows_ref), so set equality there would be permanently red — phantom direction only. And check_defaults stays off for cursor-review for a reason I measured rather than repeated: turning it on produces exactly one mismatch, extra_generated_globs, whose folded >- default renders with   separators in the guide's Default cell and does not canonicalise to the workflow's space-joined value. That is a correct doc the check cannot equate, not drift.

CI path filters follow the parser. .github/cursor-review/README.md and .github/refresh-reviewers/README.md are now explicit entries in both paths: lists in test-workflow-pins.yml — they sit outside docs/callers/**, so without them a README-only PR deleting a knob row matches no filter, runs no job and lands green, which is the #31 scenario on the very file the row was added to police. In the other direction, docs/callers/cursor-review.md and docs/callers/refresh-reviewers.md come out of test-cursor-review-scripts.yml and test-refresh-reviewers.yml: I grepped both directories and no remaining test in either reads a caller guide (grep -rln callers over both tests/ trees returns nothing), so those entries only bought spurious runs. cursor-review.yml stays in the cursor-review filter — test_workflow_job_isolation.py and catalog-drift.py still parse it. The stale comment block in test-cursor-review-scripts.yml that described the deleted suite reading the guide is rewritten to say where the check went, and test-refresh-reviewers.yml gains the equivalent note.

Three guards I added beyond the ticket, because each new column fails quietly rather than loudly, which is the failure mode this file guards against everywhere else:

A dir_readme pointing at a moved file surfaced as a FileNotFoundError traceback; it is now an existence assertion in test_files_exist_for_every_row that also names the paths: obligation.
readme_mode is compared by string equality against "two-way", so a typo ("twoway") would silently downgrade the cursor-review pin from set equality to the phantom direction alone with the suite still green. test_readme_columns_are_internally_consistent rejects any other value, and also rejects a readme_heading with no dir_readme to read it (that row's README check just skips).
Nothing in the repo asserts a CI paths: filter, so test_dir_readmes_are_in_this_suites_ci_path_filters requires each dir_readme in both of test-workflow-pins.yml's lists — adding it to pull_request and forgetting push still fails.

No docs content changed, and check_workflow_pins.py's workflows_ref cross-check is untouched (both explicitly out of scope).

Verification

Each new assertion was falsified against a deliberately broken tree and then restored, not just observed green:

injected fault	result
fake `totally_fake_knob` row in `.github/cursor-review/README.md`	`InputsDocsDrift_cursor_review.test_directory_readme_knob_table` FAILS (phantom)
real `workflows_ref` row deleted from that same README	same test FAILS (two-way direction — proves `readme_mode="two-way"` is live)
fake `totally_fake_knob` row in `.github/refresh-reviewers/README.md`	`InputsDocsDrift_refresh_reviewers.test_directory_readme_knob_table` FAILS
`top_k` Default cell in the refresh-reviewers guide changed to `999`	`test_guide_default_column_matches_workflow` FAILS `'999' != '4'` (proves `check_defaults=True` is live)
`readme_mode` typo'd to `twoway`	`test_readme_columns_are_internally_consistent` FAILS
`dir_readme` pointed at a nonexistent file	`test_files_exist_for_every_row` FAILS
one README entry removed from the `push` filter	`test_files_read_outside_the_globs_are_in_this_suites_ci_path_filters` FAILS, naming the file and the event
`README.md` duplicated under `pull_request`, absent from `push`	same test FAILS (the old `found >= 2` count passed this — see review round below)
`workflows_ref:` dropped from the refresh-reviewers guide fence	`test_example_with_blocks_are_declared_inputs` FAILS
`workflows_ref:` dropped from the refresh-reviewers header example	same test FAILS (independently)
`readme_mode="two-way"` on a row with no `dir_readme`	`test_readme_columns_are_internally_consistent` FAILS
`expect_sentinel_in_examples=True` with `expect_header_with=False`	`test_sentinel_pin_has_an_example_to_pin` FAILS

Both directory READMEs and the guide were restored byte-for-byte afterwards (git diff --stat on each returned empty).

Residual

Nothing in the ticket's scope is left undone, and every artifact it names was exercised. Two things a reader should know anyway:

check_defaults remains off for cursor-review, so its guide's Default column is still unpinned. This is what the ticket asked for, and the reason is real (measured above: extra_generated_globs'  -rendered folded scalar), but the consequence is that a stale Default value for any other cursor-review input can still drift silently. Closing it needs a canonicaliser that folds   to whitespace on the guide side — a change to shared parsing that would affect all 16 rows, and therefore its own ticket rather than a column flip here.
The ticket's line-number references are slightly stale and I worked from content, not offsets. ## Configuration knobs is at .github/cursor-review/README.md:259, not :250. Everything else it cited resolved as described.
Unexercised artifacts: the parent epic and the originating spike issue are named in the source ticket by identifier only; their descriptions and comment threads were not fetched and are not reachable from this environment, so this PR was built from the implementation plan alone. No attachments were named.

Provenance

Authored by: agent-work loop
Verified: on the merged tree (main @ f22ad8f merged in): python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py': 502 tests, OK (29 skipped); .github/cursor-review/tests: 598 tests, OK; .github/refresh-reviewers/tests: 51 tests, OK; python3 .github/workflow-pins/check_workflow_pins.py: OK, 11 workflows declaring workflows_ref, 0 exempt, 155 uses: refs SHA-pinned; python3 .github/agents-md-integrity/check_agents_md.py --root .: passed (2 pre-existing warnings, unrelated to this diff); plus every falsification run tabulated above, each restored afterwards.
Deviations: none against the plan. Beyond it, a review round (5822413) closed six panel findings against this PR's own harness, all in the vacuity class the harness exists to police: the repo-root README.md was read by SharedDocExampleCallersTest but matched no paths: glob; the CI-filter guard counted - lines file-wide so two entries in one list stood in for the other event's missing one; the deleted refresh-reviewers suite's workflows_ref assertion on both example callers was not carried across (restored behind a new expect_sentinel_in_examples column); readme_mode could sit at "two-way" on a row whose knob-table test skipped; test_guide_default_column_matches_workflow did not subtract KNOWN_UNDOCUMENTED; and two docs claims were wrong. main was merged in afterwards and the merged result re-verified above — no semantic conflict with #307.

_{🔄 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/316 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/18/2026 **Status:** ✅ Merged **Merged:** 9/19/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-15260-fold-input-docs-drift` --- ### 📝 Commits (4) - [`6303b2c`](https://github.com/Comfy-Org/github-workflows/commit/6303b2cef0d9510ce1d1437238c41046783165d0) test(workflow-pins): fold the cursor-review and refresh-reviewers input-docs suites into the drift harness - [`9bb0ddc`](https://github.com/Comfy-Org/github-workflows/commit/9bb0ddce611d49c0bf2b4e0317a7754853b541d8) test(workflow-pins): assert both dir_readme paths stay in test-workflow-pins.yml's paths: filters - [`5822413`](https://github.com/Comfy-Org/github-workflows/commit/582241350666aa8307db9ec5c901942c7f29320a) test(workflow-pins): close the drift harness's own vacuity gaps from review - [`2d122d3`](https://github.com/Comfy-Org/github-workflows/commit/2d122d361f2b61cd56b6d9a07ab98abc888fc36c) Merge remote-tracking branch 'origin/main' into matt/be-15260-fold-input-docs-drift ### 📊 Changes **7 files changed** (+356 additions, -704 deletions) <details> <summary>View changed files</summary> ➖ `.github/cursor-review/tests/test_workflow_inputs_docs.py` (+0 -141) ➖ `.github/refresh-reviewers/tests/test_inputs_docs.py` (+0 -536) 📝 `.github/workflow-pins/README.md` (+15 -1) 📝 `.github/workflow-pins/tests/test_inputs_docs_drift.py` (+309 -14) 📝 `.github/workflows/test-cursor-review-scripts.yml` (+8 -10) 📝 `.github/workflows/test-refresh-reviewers.yml` (+6 -2) 📝 `.github/workflows/test-workflow-pins.yml` (+18 -0) </details> ### 📄 Description ## ELI-5 Two little test files each checked, by hand, that one workflow's documented knobs match the knobs it actually has. A previous PR replaced both with a single table-driven check covering all 16 reusable workflows, but deliberately left the two originals alive. This deletes them and moves their last two extras — the directory-README knob tables and one Default-column comparison — into the shared table, then points CI's path filters at the files the shared check now reads. ## What changed **One parser, not three.** `.github/cursor-review/tests/test_workflow_inputs_docs.py` (141 lines) and `.github/refresh-reviewers/tests/test_inputs_docs.py` (536 lines) are deleted. Everything they asserted is carried by two columns on their rows in `.github/workflow-pins/tests/test_inputs_docs_drift.py`: | row | `dir_readme` | `readme_heading` | `readme_mode` | `check_defaults` | | --- | --- | --- | --- | --- | | `cursor-review` | `.github/cursor-review/README.md` | `## Configuration knobs` | `two-way` | `False` | | `refresh-reviewers` | `.github/refresh-reviewers/README.md` | `## Knob defaults (and why)` | `one-way` | `True` | The asymmetry is deliberate and both halves were verified rather than assumed. `cursor-review`'s README table really is complete, so two-way set equality is what the deleted suite asserted and what the row now asserts. `refresh-reviewers`' table is intentionally partial (it omits `reviewer_config_path`, `map_exclude`, `extra_exclude_paths` and `workflows_ref`), so set equality there would be permanently red — phantom direction only. And `check_defaults` stays **off** for `cursor-review` for a reason I measured rather than repeated: turning it on produces exactly one mismatch, `extra_generated_globs`, whose folded `>-` default renders with ` ` separators in the guide's Default cell and does not canonicalise to the workflow's space-joined value. That is a correct doc the check cannot equate, not drift. **CI path filters follow the parser.** `.github/cursor-review/README.md` and `.github/refresh-reviewers/README.md` are now explicit entries in **both** `paths:` lists in `test-workflow-pins.yml` — they sit outside `docs/callers/**`, so without them a README-only PR deleting a knob row matches no filter, runs no job and lands green, which is the #31 scenario on the very file the row was added to police. In the other direction, `docs/callers/cursor-review.md` and `docs/callers/refresh-reviewers.md` come out of `test-cursor-review-scripts.yml` and `test-refresh-reviewers.yml`: I grepped both directories and no remaining test in either reads a caller guide (`grep -rln callers` over both `tests/` trees returns nothing), so those entries only bought spurious runs. `cursor-review.yml` stays in the cursor-review filter — `test_workflow_job_isolation.py` and `catalog-drift.py` still parse it. The stale comment block in `test-cursor-review-scripts.yml` that described the deleted suite reading the guide is rewritten to say where the check went, and `test-refresh-reviewers.yml` gains the equivalent note. **Three guards I added beyond the ticket**, because each new column fails *quietly* rather than loudly, which is the failure mode this file guards against everywhere else: 1. A `dir_readme` pointing at a moved file surfaced as a `FileNotFoundError` traceback; it is now an existence assertion in `test_files_exist_for_every_row` that also names the `paths:` obligation. 2. `readme_mode` is compared by string equality against `"two-way"`, so a typo (`"twoway"`) would silently downgrade the cursor-review pin from set equality to the phantom direction alone with the suite still green. `test_readme_columns_are_internally_consistent` rejects any other value, and also rejects a `readme_heading` with no `dir_readme` to read it (that row's README check just skips). 3. Nothing in the repo asserts a CI `paths:` filter, so `test_dir_readmes_are_in_this_suites_ci_path_filters` requires each `dir_readme` in **both** of `test-workflow-pins.yml`'s lists — adding it to `pull_request` and forgetting `push` still fails. No docs content changed, and `check_workflow_pins.py`'s `workflows_ref` cross-check is untouched (both explicitly out of scope). ## Verification Each new assertion was falsified against a deliberately broken tree and then restored, not just observed green: | injected fault | result | | --- | --- | | fake `totally_fake_knob` row in `.github/cursor-review/README.md` | `InputsDocsDrift_cursor_review.test_directory_readme_knob_table` FAILS (phantom) | | real `workflows_ref` row deleted from that same README | same test FAILS (two-way direction — proves `readme_mode="two-way"` is live) | | fake `totally_fake_knob` row in `.github/refresh-reviewers/README.md` | `InputsDocsDrift_refresh_reviewers.test_directory_readme_knob_table` FAILS | | `top_k` Default cell in the refresh-reviewers guide changed to `999` | `test_guide_default_column_matches_workflow` FAILS `'999' != '4'` (proves `check_defaults=True` is live) | | `readme_mode` typo'd to `twoway` | `test_readme_columns_are_internally_consistent` FAILS | | `dir_readme` pointed at a nonexistent file | `test_files_exist_for_every_row` FAILS | | one README entry removed from the `push` filter | `test_files_read_outside_the_globs_are_in_this_suites_ci_path_filters` FAILS, naming the file and the event | | `README.md` duplicated under `pull_request`, absent from `push` | same test FAILS (the old `found >= 2` count passed this — see review round below) | | `workflows_ref:` dropped from the refresh-reviewers guide fence | `test_example_with_blocks_are_declared_inputs` FAILS | | `workflows_ref:` dropped from the refresh-reviewers header example | same test FAILS (independently) | | `readme_mode="two-way"` on a row with no `dir_readme` | `test_readme_columns_are_internally_consistent` FAILS | | `expect_sentinel_in_examples=True` with `expect_header_with=False` | `test_sentinel_pin_has_an_example_to_pin` FAILS | Both directory READMEs and the guide were restored byte-for-byte afterwards (`git diff --stat` on each returned empty). ## Residual Nothing in the ticket's scope is left undone, and every artifact it names was exercised. Two things a reader should know anyway: - **`check_defaults` remains off for `cursor-review`, so its guide's Default column is still unpinned.** This is what the ticket asked for, and the reason is real (measured above: `extra_generated_globs`' ` `-rendered folded scalar), but the consequence is that a stale Default value for any *other* cursor-review input can still drift silently. Closing it needs a canonicaliser that folds ` ` to whitespace on the guide side — a change to shared parsing that would affect all 16 rows, and therefore its own ticket rather than a column flip here. - **The ticket's line-number references are slightly stale and I worked from content, not offsets.** `## Configuration knobs` is at `.github/cursor-review/README.md:259`, not `:250`. Everything else it cited resolved as described. - **Unexercised artifacts:** the parent epic and the originating spike issue are named in the source ticket by identifier only; their descriptions and comment threads were not fetched and are not reachable from this environment, so this PR was built from the implementation plan alone. No attachments were named. ## Provenance - **Authored by:** agent-work loop - **Verified:** on the merged tree (`main` @ f22ad8f merged in): `python3 -m unittest discover -s .github/workflow-pins/tests -p 'test_*.py'`: 502 tests, OK (29 skipped); `.github/cursor-review/tests`: 598 tests, OK; `.github/refresh-reviewers/tests`: 51 tests, OK; `python3 .github/workflow-pins/check_workflow_pins.py`: OK, 11 workflows declaring `workflows_ref`, 0 exempt, 155 `uses:` refs SHA-pinned; `python3 .github/agents-md-integrity/check_agents_md.py --root .`: passed (2 pre-existing warnings, unrelated to this diff); plus every falsification run tabulated above, each restored afterwards. - **Deviations:** none against the plan. Beyond it, a review round (5822413) closed six panel findings against this PR's own harness, all in the vacuity class the harness exists to police: the repo-root `README.md` was read by `SharedDocExampleCallersTest` but matched no `paths:` glob; the CI-filter guard counted `- ` lines file-wide so two entries in one list stood in for the other event's missing one; the deleted refresh-reviewers suite's `workflows_ref` assertion on both example callers was not carried across (restored behind a new `expect_sentinel_in_examples` column); `readme_mode` could sit at `"two-way"` on a row whose knob-table test skipped; `test_guide_default_column_matches_workflow` did not subtract `KNOWN_UNDOCUMENTED`; and two docs claims were wrong. `main` was merged in afterwards and the merged result re-verified above — no semantic conflict with #307. --- 🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

bsakaguchi added the

pull-request

label

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

bsakaguchi referenced this issue

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

[PR #317] [MERGED] docs(callers): document the 13 undocumented workflow_call inputs and drain KNOWN_UNDOCUMENTED #318

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

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

[PR #319] [MERGED] fix(pr-risk): publish the Check Run on a manual dispatch regardless of enabled #319