Comfy-Org / Comfy-Org/github-workflows

[groom] reviewers.yml has two hand-ported parsers whose parity is asserted by two independent suites and no shared corpus

Đang mở
#286 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
groom
Ngôn ngữ chính
Shell
Star
6
Fork
1
Merge trung bình
14 giờ 4 phút
Pull request đã merge (30 ngày)
98

Mô tả

**Groom auto-builder** — Comfy-Org/github-workflows: this CONFIRMED finding could not be auto-built (patch modifies a CI-privileged or dataset-of-record path (per .github/groom/patch_policy.py — workflow/action defs, lockfiles, build/test config that executes in pre-review CI, graded eval cases under a suites/**/cases/ tree, or a symlink into such a tree) — a human must author these changes), so it is filed for a human. · [run](https://github.com/Comfy-Org/github-workflows/actions/runs/34828869870)

## Problem

parseReviewerConfig (JS, inline at assign-reviewers.yml:424) and parse_reviewer_config (Python, generate.py:172, under a header at 131 reading "parity with parseReviewerConfig") are a line-by-line port: same comment-stripping quote-state machine, same unquote, same flow-sequence parser, same indent helper, same dash/list-key walk. globToRegExp is ported the same way (generate.py:96, docstring "Port of assign-reviewers.yml's globToRegExp"). Parity is asserted only by two independent hand-written suites (.github/assign-reviewers/tests/assignment.test.cjs and .github/refresh-reviewers/tests/test_generate.py); there is no shared corpus, so a case exercised on one side proves nothing about the other.

Candidate divergences visible on a read, offered as corpus seeds rather than as claims: the JS splits lines on a CR-optional newline regex while the Python splits on a bare newline; the JS unquote has no length guard, so a one-character double-quote would slice to empty, while the Python _unquote requires length >= 2.

## The harness is fragile, exactly as described

assignment.test.cjs:7 recovers the shipped script by splitting the workflow text on a literal 10-space-indented "script: |" line and on the exact downstream step name "Publish reviewer history manifest", then stripping 12 leading spaces per line. Verified: assign-reviewers.yml contains exactly ONE "script: |" (line 166) and the anchor step is at line 710, so it works today -- but it is load-bearing on there never being a second "script: |" at that indentation, on that step name never changing, and on the indentation staying at 12. Any of those would silently capture the wrong span rather than fail.

## The unification half is correctly dropped

Grepping assign-reviewers.yml for actions/checkout or workflows_ref returns NOTHING -- this workflow deliberately has no checkout at all, so moving the JS to a file would add a workflows_ref pin to every existing caller, a breaking contract change for a cosmetic gain. And the Python genuinely computes something the JS does not: parse_reviewer_config returns a (config, locations) pair recording flow/block line positions so the drift generator can rewrite only those bytes. They are not the same function. Do NOT collapse them.

## Scope (test-only)

1. Add one shared fixture corpus under .github/assign-reviewers/ -- JSON mapping config text to expected parsed shape, and glob+path to expected match -- and have BOTH assignment.test.cjs and test_generate.py iterate it. The Python side asserts only the config half of its return; locations stays covered by its own suite.
2. Re-anchor the extraction in assignment.test.cjs on a sentinel comment emitted inside the inline script, rather than on indentation plus an unrelated downstream step name, and fail loudly if the sentinel is not found.

## Value and risk

The config format is a contract read by the runtime and REWRITTEN by the drift generator, so a divergence could mean the generator edits lists that the runtime reads differently -- nothing today could catch that. Risk is low: test-only, no shipped code changes. Not marked security: this decides who is ASSIGNED to review, not who may approve or what may execute.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Read .github/assign-reviewers/tests/assignment.test.cjs and .github/refresh-reviewers/tests/test_generate.py, then inspect the parser entry points in assign-reviewers.yml:424 and generate.py:172. Add the shared JSON corpus under .github/assign-reviewers/, make both suites iterate it, and re-anchor script extraction on an in-script sentinel that fails loudly when absent. Keep Python location assertions in its existing suite and run both test suites.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
github-actions, javascript, python
Lĩnh vực
ci-cd, testing
Loại issue
Tái cấu trúc
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
74/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.