docs: add a pr-triage skill and document pull request triage - #5742
docs: add a pr-triage skill and document pull request triage#5742andygrove wants to merge 2 commits into
Conversation
Add `.ai/skills/pr-triage/SKILL.md`, a skill that labels the open pull request backlog with a type label and the `area:*` labels for the subsystems each PR touches, so that reviewers can filter the backlog to their area of expertise. The skill derives the area from the PR's changed files rather than its title, since titles rarely name the subsystem, and it asks a human before creating any new area label. Also document PR triage in the contributor guide and bring the area label table up to date with the labels that exist in the repository: `area:Iceberg`, `area:udf`, `area:memory`, and `area:joins`.
sunchao
left a comment
There was a problem hiding this comment.
Correctness
Comet has an issue-triage guide and skill, but no corresponding PR workflow. This change adds that workflow and documents its differences from issue triage. The type definitions preserve the distinction between repairing broken behavior and adding support where Comet previously fell back. Supporting labels distinguish silent wrong results from exceptions and crashes.
There is one P2 finding in Step 3: the prescribed changed-file query returns only the first 100 files with GitHub CLI 2.93.0. A wider PR can therefore miss area labels for subsystems that appear only in the omitted paths. The later nonempty-label check cannot detect this. Please fetch the complete file list before classifying the area, as described inline.
I reviewed head 21ef3c034b93d6cd3e0ea708b7826dd5a7a83ec5 against base 7f1e00189b1ed86f1cb5acd872d97fce694482b1. The complete discussion snapshot at 2026-09-08 03:19:10 UTC contains no reviews or comments. The head has four successful checks, including Preflight, and twelve skipped checks. The authored diff passes the whitespace check. Validation was limited to source and CLI behavior inspection. I did not run the triage commands against the backlog.
Performance
The change affects documentation and contributor tooling, so it does not alter query execution or introduce a runtime performance claim. The workflow bounds concurrent PR requests at six and batches each PR's label additions into one edit. Complete file pagination will add reads for wide PRs, which is necessary to support the area-classification contract. Spark tests and runtime benchmarks do not apply to these two changed files.
Design
The guide remains the authority for type and area definitions, while the skill provides the operational sequence: discover labels and PRs, inspect changed files, classify, apply, verify, and report. It explicitly leaves Dependabot PRs alone, allows PRs without a matching area, and requires the user's choice before creating a new area label. Its permitted PR changes are limited to labels, with the result reported to the user. These boundaries fit the stated contributor workflow.
Abstraction & complexity
The implementation adds one skill and extends the existing guide without introducing a new framework or changing the issue-triage workflow. The path mapping gives the operator useful subsystem hints, while the guide holds the shared classification rules. Label additions preserve unrelated labels, and an incorrect opposite type label is removed explicitly. The file-completeness correction belongs in the existing collection step and does not require a larger abstraction.
| 'gh pr view {} --repo apache/datafusion-comet --json files \ | ||
| --jq "[.files[].path]|join(\" \")" > '"$SCRATCH"'/files/{}.txt 2>/dev/null' \ |
There was a problem hiding this comment.
Correctness
[P2] Fetch the complete changed-file list before assigning areas
With GitHub CLI 2.93.0, gh pr view --json files uses files(first: 100), and the number lookup makes one request without paging the files. For a PR changing more than 100 files, this silently omits the remaining paths. Any subsystem touched only by those paths will be missed, even though Step 7 can still report success because the PR already has other labels. Please use a paginated file query and check that collection completed before deriving the area labels, reporting an incomplete read instead of treating the first page as the whole diff.
|
superseded by #5762 |
Which issue does this PR close?
Closes #.
Rationale for this change
Comet has around 170 open pull requests. Almost none of them carried labels, so
there was no way for a reviewer to filter the backlog down to their area of
expertise: a query like
is:pr is:open label:area:shufflereturned nothinguseful.
The project already has a documented triage process for issues
(
docs/source/contributor-guide/bug_triage.md) and abug-triageskill thatautomates it. Pull requests had no equivalent.
PR triage differs from issue triage in ways that are easy to get wrong:
perf: reuse zstd compression contexts across shuffle blocksnever says "shuffle"; the changed files do.fix:prefix does not always meanbug. A PR adding support for a typeComet previously fell back on is an enhancement, since nothing was broken.
Capturing these rules means the next triage pass is consistent with this one
rather than re-deriving the conventions from scratch.
What changes are included in this PR?
.ai/skills/pr-triage/SKILL.md. The skill enumerates open PRs, fetcheseach PR's changed files to derive its area, applies exactly one type label
(
bugorenhancement) plus the supporting labels (performance,correctness,crash,test,build,documentation) and thearea:*labels, and prints a report. It does not review, comment on, or edit PRs, and
it asks a human before creating any new area label rather than inventing one.
rules above.
already exist in the repository but were undocumented:
area:Iceberg,area:udf,area:memory, andarea:joins. Also list the pre-existingarray expressions,map expressions,json expressions, andtemporal expressionslabels as area indicators.area:memoryandarea:joinswere created while triaging the current backlog:seven open PRs cover memory pools, reservations, and OOM handling, and three
cover join operators and dynamic filter pushdown, none of which any existing
area described.
How are these changes tested?
Documentation and agent-skill changes only, with no code paths to test. The
skill was exercised against the live backlog before being written up: all 170
open non-Dependabot PRs now carry a type label, and 151 carry at least one area
label. The
remainder are plan-rule, AQE, EXPLAIN, and caching changes that the guide
explicitly says should be left without one.
prettierpasses on both changed files.