(datafusion-comet) 01/01: docs: request changes for correctness and performance findings in the review skill (#6483)

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6483-11a27c36713779f8fd297e0dd72ac4911dd3e04a
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git

commit 9a41a11a1f91cec6851bbdec86bf4f5524bd07a2
Author: Andy Grove <ag...@apache.org>
AuthorDate: Thu Oct 1 11:24:51 2026 +0000

    docs: request changes for correctness and performance findings in the review skill (#6483)
    
    main needs a single approval to merge, so a correctness or performance
    finding posted as a comment does not stop another committer's approval,
    or an armed auto-merge, from merging the PR over it. A Request changes
    review from a committer does, until the same reviewer approves or the
    review is dismissed.
    
    Define the findings that must block the merge, add a Review State item
    to the output format, cover the re-review that lifts the block, and say
    how to post the review when the user asks for it.
---
 .ai/skills/review-comet-pr/SKILL.md | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/.ai/skills/review-comet-pr/SKILL.md b/.ai/skills/review-comet-pr/SKILL.md
index 435cfcc425..4d6d855861 100644
--- a/.ai/skills/review-comet-pr/SKILL.md
+++ b/.ai/skills/review-comet-pr/SKILL.md
@@ -166,6 +166,25 @@ before merge. If it is, raise it and expect a response. If it is not, cut it. Th
 - This bar is about what gets raised, not about how it is worded. Keep the tone below. A question you
   expect an answer to still counts as something the author needs to address.
 
+## Request Changes for Correctness and Performance Regressions
+
+Two kinds of finding have to block the merge until the author addresses them: a correctness problem
+that the PR introduces, and a performance regression. A correctness problem means Comet can return a
+different answer from Spark, including a result where Spark raises an error, or `Compatible()` over a
+known divergence. Crashes, hangs, leaks, and lost or corrupted data count too. A performance
+regression means an existing path gets slower or uses more memory. A tracking issue for a later fix
+does not address either one.
+
+When any finding is one of these, the review is submitted as **Request changes**. `main` needs a
+single approval to merge, and any committer's approval counts, including one given before this
+review. A **Comment** review does not stop that approval from merging the PR over the finding. A
+**Request changes** review from a reviewer with write access blocks the merge until the same reviewer
+approves the PR or someone with write access dismisses the review.
+
+Requesting changes commits the reviewer to the re-review. Once the author has addressed the findings
+behind it, the reviewer approves the PR or dismisses the review. A later **Comment** review leaves
+the block in place.
+
 ## Output Format
 
 Present your review as guidance for the reviewer:
@@ -177,6 +196,12 @@ Present your review as guidance for the reviewer:
 5. **Suggested Review Comments**, specific comments the reviewer could leave, with file and line
    references. Everything here is something you expect the author to address. Anything that did not
    clear the bar above should not appear.
+6. **Review State**, how to submit the review. Use the first case that applies:
+   - **Request changes** when any finding is a correctness problem that the PR introduces or a
+     performance regression. Name those findings.
+   - **Approve**, or dismiss the earlier review, when this is a re-review and the findings behind the
+     reviewer's earlier **Request changes** review have all been addressed.
+   - **Comment** otherwise.
 
 ## Review Tone and Style
 
@@ -203,3 +228,8 @@ Instead:
 **IMPORTANT: Never post comments or reviews on the PR directly.** This skill and all of its siblings
 are for providing guidance to a human reviewer. Present all findings and suggested comments to the
 user. The user will decide what to post.
+
+When the user tells you to post the review for them, submit it in the review state from the output.
+For **Request changes** that is
+`gh pr review <pr> --repo apache/datafusion-comet --request-changes --body-file <file>`. A PR comment
+or a **Comment** review does not block the merge.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org