Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ issue_enrichment: {}
reviews:
# Explicit instructions applied to every review, including PR description validation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test Execution Plan

  • Run smoke tests: False — No Python code, fixtures, or test utilities were modified. All changes are GitHub Actions workflows and CodeRabbit config (YAML only).
  • Run gating tests: False — Same reason; no traced dependency path to any gating-marked test.
  • Affected tests to run: None — Changes are confined to .coderabbit.yaml, .github/workflows/request-coderabbit-test-instructions.yml, and .github/workflows/unresolve-coderabbit-threads.yml. No utilities/, libs/, or tests/ Python code was touched.

path_instructions:
- path: "utilities/unittests/**"
instructions: |
## Unit Tests — Relaxed Rules
Files under `utilities/unittests/` are unit tests for shared utilities.
Do NOT enforce:
- STD docstring format (Preconditions/Steps/Expected sections)
- STP link requirements
- RFE/Jira link requirements
- Coverage tracking rules
These rules apply only to feature/integration tests under `tests/`.
DO still enforce: code quality, type hints, naming conventions, and all other coding standards.

- path: "**"
instructions: |
## PR Template Validation
Expand All @@ -32,6 +44,20 @@ reviews:
If any required section is absent, or `What this PR does / why we need it:` has no content,
flag it as HIGH severity and ask the author to restore the missing template section(s).

## Approval Policy
You may approve the PR when ALL of the following are true:
- All your review comments have been addressed with either:
- a code/doc change that fixes the issue, or
- a substantive author response that justifies no code change.
Thread "resolved" state alone is not sufficient.
OR you had no review comments.
- If you posted a test execution plan comment requesting tests, and the PR author replied
with a comment explaining why the requested tests are not needed or were already covered,
treat that as an acceptable response — do not block approval on the test plan alone.
- The author's explanation must be reasonable and specific (not just "N/A" or "not needed").
Accept explanations like: "these tests were already run in CI", "this change is docs-only",
"the affected tests are quarantined", or "verified manually on cluster X".

# Assertive profile for strict enforcement of coding standards
profile: assertive

Expand Down Expand Up @@ -78,9 +104,12 @@ reviews:
mode: error
instructions: |
Check ONLY code that is newly added (not modified) in this PR's diff.
SKIP this check entirely for files under `utilities/unittests/` — unit tests
do not require STP/RFE/Jira links.

This applies to two cases:
A) A newly added test file (test_*.py)
B) A newly added test function (def test_*) in an existing test file
A) A newly added test file (test_*.py) under `tests/`
B) A newly added test function (def test_*) in an existing test file under `tests/`

For each case, at least one of the following MUST appear in the
module, class, or test function docstring:
Expand Down
39 changes: 33 additions & 6 deletions .github/workflows/request-coderabbit-test-instructions.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# description: Add comment for CodeRabbit to add tests instructions. A change-request comment will be added to the PR.
# Triggers on the 'verified' label (post-approval) or '/test-plan' comment (on-demand during review).
# description: Add comment for CodeRabbit to add test instructions. An inline informational test-plan comment will be added.

name: Request CodeRabbit tests instructions

Expand Down Expand Up @@ -50,7 +49,8 @@ jobs:

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a formal review - just post the inline comment directly.
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
Expand Down Expand Up @@ -109,19 +109,46 @@ jobs:
Do NOT include analysis step numbers (1-8) in your visible output.

**Your deliverable:**
Your change request comment will be based on the following requirements:
Your inline informational comment will be based on the following requirements:

**Test Execution Plan**

- **Run smoke tests: True / False** — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
- **Run gating tests: True / False** — If True, state the dependency path. True if any gating-marked test is in the affected set.
- **Affected tests to run** _(required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)_

Use these formats:
- `path/to/test_file.py` - When the entire test file needs verification
- `path/to/test_file.py::TestClass::test_method` - When specific test(s) needed
- `path/to/test_file.py::test_function` - When specific test(s) needed
- `-m marker` - When a marker covers multiple affected tests (e.g. `-m gating` only if ALL gating tests in scope need run)
- Tag each listed test or group with its marker when not obvious, e.g. `(gating)` or `(smoke)`

**Real test commands (MANDATORY when changes affect session/runtime code):**

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete `pytest` commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:
- A command for the **error/fix path** (the scenario the PR fixes)
- A command for the **happy path** (regression: the normal case still works)
- Use lightweight tests (e.g., `--collect-only` for startup failures,
a single small test for runtime behavior)
If the PR only changes test logic (not utilities/libs/conftest), the affected test
paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:
```
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally
```

**Guidelines:**

- Include tests affected directly OR via fixture setup/teardown, `yield from` cleanup, or transitive utility call chains (caller calls modified helper)
Expand All @@ -143,9 +170,9 @@ jobs:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

1. **YOUR ONLY DELIVERABLE**: Post an inline review comment containing the test execution plan on the first changed line
1. **YOUR ONLY DELIVERABLE**: Post one non-blocking inline comment containing the test execution plan on the first changed line
2. **THEN STOP IMMEDIATELY** - Do NOT generate any additional response
3. **FALLBACK ONLY**: If submitting the review fails after retrying, post as a regular PR comment
3. **FALLBACK ONLY**: If inline comment API calls fail after retrying, post as a regular PR comment
4. **SILENCE = SUCCESS**: After successfully submitting the review, your task is complete. No confirmation needed.

**ABSOLUTE PROHIBITIONS** (violating these creates empty/meaningless reviews):
Expand Down
137 changes: 137 additions & 0 deletions .github/workflows/unresolve-coderabbit-threads.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# description: Unresolve CodeRabbit review threads that were resolved by the PR author
# without addressing them. Runs on every push to a PR.
#
# Logic: If a CodeRabbit review thread was resolved by someone other than
# coderabbitai[bot], and the resolver's last reply does NOT contain a substantive
# response (e.g., "fixed", "addressed", code snippet, or explanation), the thread
# is unresolved so CodeRabbit can re-evaluate it.

name: Unresolve unaddressed CodeRabbit threads

on:
pull_request_target:
types: [synchronize]

concurrency:
group: unresolve-threads-${{ github.event.pull_request.number }}
cancel-in-progress: true

permissions:
# Required to unresolve review threads and add comments
pull-requests: write
contents: read

jobs:
unresolve-threads:
if: "!endsWith(github.event.pull_request.user.login, '[bot]')"
runs-on: ubuntu-latest
timeout-minutes: 5
Comment thread
coderabbitai[bot] marked this conversation as resolved.

steps:
- name: Unresolve prematurely resolved CodeRabbit threads
env:
GH_TOKEN: ${{ secrets.BOT3_TOKEN }}
PR_NUMBER: ${{ github.event.pull_request.number }}
REPO: ${{ github.repository }}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
run: |
set -euo pipefail

# Fetch resolved review threads with pagination
ALL_THREADS="[]"
CURSOR=""
while true; do
if [ -n "$CURSOR" ]; then
AFTER_ARG="-f after=$CURSOR"
else
AFTER_ARG=""
fi
PAGE=$(gh api graphql -f query='
query($owner: String!, $repo: String!, $pr: Int!, $after: String) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
reviewThreads(first: 100, after: $after) {
nodes {
id
isResolved
opening_comment: comments(first: 1) {
nodes {
author {
login
}
}
}
recent_comments: comments(last: 5) {
nodes {
author {
login
}
body
}
}
}
pageInfo {
hasNextPage
endCursor
}
}
}
}
}
' -f owner="${REPO%%/*}" -f repo="${REPO##*/}" -F pr="$PR_NUMBER" $AFTER_ARG)

# Append nodes to accumulated list
NODES=$(echo "$PAGE" | jq '.data.repository.pullRequest.reviewThreads.nodes')
ALL_THREADS=$(echo "$ALL_THREADS $NODES" | jq -s 'add')

# Check for next page
HAS_NEXT=$(echo "$PAGE" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')
if [ "$HAS_NEXT" != "true" ]; then
break
fi
CURSOR=$(echo "$PAGE" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')
done

# Process each resolved thread
echo "$ALL_THREADS" | jq -r --arg pr_author "$PR_AUTHOR" '
.[]
| select(.isResolved == true)
| select(.opening_comment.nodes[0].author.login == "coderabbitai")
| select(
# No substantive PR-author reply exists in the recent window
(any(.recent_comments.nodes[];
.author.login == $pr_author
and ((.body // "") | length) >= 15
) | not)
)
| .id
' | while read -r thread_id; do
if [ -n "$thread_id" ]; then
echo "Unresolving thread: $thread_id"
if gh api graphql -f query="
mutation {
unresolveReviewThread(input: {threadId: \"$thread_id\"}) {
thread {
id
isResolved
}
}
}
"; then
# Add a comment to the thread explaining why it was unresolved
gh api graphql -f query="
mutation {
addPullRequestReviewThreadReply(input: {pullRequestReviewThreadId: \"$thread_id\", body: \"\u26a0\ufe0f This thread was automatically unresolved because it was resolved without a substantive response. Please address the review comment and explain how it was resolved before resolving this thread again.\"}) {
comment {
id
}
}
}
" || echo " Warning: failed to add comment to $thread_id"
else
echo " Warning: failed to unresolve $thread_id"
fi
fi
done

echo "Done checking CodeRabbit threads."