Skip to content

ci: opt-in Claude deep review via deep-review label (#273) - #278

Merged
allxsmith merged 2 commits into
mainfrom
ci/deep-review-label
Jul 11, 2026
Merged

allxsmith merged 2 commits into
mainfrom
ci/deep-review-label

Conversation

@allxsmith

@allxsmith allxsmith commented Jul 10, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Adds a second, opt-in trigger path to the one-shot Claude deep review (claude-review.yml): a triage+ user can apply the deep-review label to any same-repo PR to run the opus deep review — closing the gap where Claude-Code-authored but human-opened PRs (e.g. #267–271) never get one. The AI-loop path is logically unchanged; the labeler's live role is re-verified via the collaborators API (same belt-and-braces pattern as on-slop.yml / claude-implement.yml), and the <!-- claude-deep-review --> dedupe marker still caps usage at one review per PR ever — re-labeling never re-fires it. Documents the new label in the AI-development guide and the root CLAUDE.md.

  • bulma-ui (@allxsmith/bestax-bulma)
  • create-bestax (create-bestax)
  • docs (@allxsmith/bestax-docs)
  • Other (please specify): repo tooling (.github/workflows/claude-review.yml) + docs

Related Issue(s)

Closes #273

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Performance
  • Build tooling
  • Other (please describe):

Checklist

  • My code follows the project style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works (workflow-only change; no test surface — verified by YAML parse + simulating all four trigger event shapes)
  • I have added/updated documentation as needed
  • I have updated Storybook stories as needed (bulma-ui) (N/A — no UI change)
  • My changes require a change to the documentation (already included in this PR)
  • All new and existing tests passed
  • Any relevant dependencies are updated (N/A — no dependency changes)
  • If this PR changes commands, conventions, or package structure, the affected CLAUDE.md files are updated

Screenshots / Demos

N/A — CI workflow + docs change.

Additional Context

🤖 Generated with Claude Code

https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added an opt-in deep-review label to trigger a one-time deep review for eligible pull requests.
    • Added permission checks so the deep review runs only when the labeler has sufficient triage-level permissions.
    • Prevented duplicate deep reviews for the same pull request.
  • Documentation

    • Updated AI development guidance to describe the deep-review label, its one-shot lifecycle, and the updated label instructions in the AI development loop.

Add a second trigger path to claude-review.yml: a triage+ user can apply
the deep-review label to any same-repo PR to run the one-shot opus deep
review. The labeler's live role is re-verified (on-slop/claude-implement
pattern); the dedupe marker still caps it at one review per PR ever.
Document the label in the AI-development docs table and CLAUDE.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up
@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The Claude deep review workflow now supports a permission-verified deep-review label trigger for same-repository pull requests while retaining deduplication and existing AI-loop support. Repository instructions and documentation describe the new one-shot opt-in label.

Changes

Opt-in deep review

Layer / File(s) Summary
Workflow trigger and execution gating
.github/workflows/claude-review.yml
Adds the deep-review trigger path, verifies labeler permissions through the GitHub API, preserves one-shot deduplication, gates review steps, and generalizes the Claude prompt wording.
Label lifecycle guidance
CLAUDE.md, docs/docs/guides/getting-started/ai-development.md
Documents deep-review as a triage-plus opt-in label that runs one deep review per pull request.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubPR as GitHub PR event
  participant Workflow as claude-review.yml
  participant GitHubAPI as Collaborators API
  participant Claude as Claude deep review
  GitHubPR->>Workflow: Apply deep-review label
  Workflow->>GitHubAPI: Verify labeler permission
  GitHubAPI-->>Workflow: Return permission result
  Workflow->>Workflow: Check review deduplication marker
  Workflow->>Claude: Run one-shot deep review
Loading

Possibly related issues

Possibly related PRs

Suggested labels: released

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding opt-in deep-review label support.
Description check ✅ Passed The description covers the change summary, affected area, issue link, type, checklist, and extra context.
Linked Issues check ✅ Passed The summary shows #273's key requirements were met: label trigger, permission check, dedupe, AI-loop safety, and docs updates.
Out of Scope Changes check ✅ Passed No unrelated changes are indicated beyond the workflow and documentation updates needed for this feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/deep-review-label

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://ca7a2985.bestax.pages.dev

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/claude-review.yml (1)

67-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Permission re-check is well-reasoned defense-in-depth.

Falls back safely to allowed=false on any gh api failure (|| echo none → hits the *) catch-all), and the role_name values checked (admin|maintain|write|triage) match the GitHub collaborators-permission API's documented set. Good belt-and-braces pattern.

One optional note: this same pattern is called out in the comments as duplicated from on-slop.yml and claude-implement.yml. If this triage+ permission re-check is going to be reused a third time, consider extracting it into a small composite action to avoid drift between the three copies.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/claude-review.yml around lines 67 - 91, Consider
extracting the duplicated permission re-check logic from the workflow step
identified by “Check labeler permission” into a reusable composite action, then
replace the repeated implementations in the related workflows with that action
while preserving the existing inputs, outputs, and fail-closed behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/claude-review.yml:
- Around line 67-91: Consider extracting the duplicated permission re-check
logic from the workflow step identified by “Check labeler permission” into a
reusable composite action, then replace the repeated implementations in the
related workflows with that action while preserving the existing inputs,
outputs, and fail-closed behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a241539-b6ea-4553-91b7-952e1e477d85

📥 Commits

Reviewing files that changed from the base of the PR and between 306e3cb and f9a919b.

📒 Files selected for processing (3)
  • .github/workflows/claude-review.yml
  • CLAUDE.md
  • docs/docs/guides/getting-started/ai-development.md

@allxsmith

Copy link
Copy Markdown
Owner Author

@claude perform deep review

@claude

claude Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Claude finished @allxsmith's task in 4m 14s —— View job


Deep review

  • Gather context (PR description, CodeRabbit review, changed files)
  • Read .github/workflows/claude-review.yml diff in full
  • Read CLAUDE.md and docs/docs/guides/getting-started/ai-development.md diffs
  • Analyze trigger/gating logic for correctness and security
  • Post review feedback

No blocking defects found. The gating logic is sound; I traced the four trigger shapes (loop opened, loop labeled, deep-review labeled, unlabeled opened) plus fork PRs, and each resolves the way the PR description claims.

Overall: the design is sound. Worth calling out one thing I specifically checked because it looked suspicious at first glance, then confirmed is not a bug:

  • In Check labeler permission (.github/workflows/claude-review.yml:74-91), the live-role recheck is skipped whenever $ACTION != "labeled" (i.e. on opened) — allowed=true unconditionally, no API call. That looked like a gap for a PR opened with deep-review already attached. But the job-level if (claude-review.yml:44) already restricts to head.repo.full_name == github.repository — same-repo only. Pushing a branch into the upstream repo requires write access at minimum, which already satisfies the admin|maintain|write|triage allowlist the recheck itself uses. So a same-repo PR opener can never be below the bar the check would enforce anyway; the bypass is a no-op optimization, not a hole. Confirmed by comparing against the equivalent sender.type == 'User' + role-check pattern in on-slop.yml and claude-implement.yml — same trust model, no regression.

Two pre-existing minor items, both non-blocking:

  1. CodeRabbit's nitpick stands: this is now the third copy of the triage+ role-recheck (on-slop.yml, claude-implement.yml, now here). Worth a composite action if a fourth copy ever appears.
  2. CLAUDE.md:89 — the inserted sentence ("A triage+ user can also apply...") isn't rewrapped to match the surrounding paragraph's ~90-char lines (it runs to ~146 chars before wrapping). Prettier's default proseWrap: preserve won't flag this in format:check, so it's cosmetic only.

Verified: paren-balance in the multi-line if: expression (5 opens / 5 closes, correctly nested), dedupe marker regex unchanged, no permissions: block escalation, docs table addition is consistent with the rest of the AI-loop label lifecycle described.

Addresses the PR #278 deep-review note: the inserted sentence ran to
~142 chars; rewrap the paragraph tail to the surrounding ~90-char style.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142miDJt2AX9QUw5fNJt2xu
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://fe5eb120.bestax.pages.dev

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CLAUDE.md`:
- Around line 90-91: Clarify the `deep-review` documentation to state that it
applies only to same-repository PRs, not pull requests originating from forks.
Update the sentence describing labeling any PR while preserving the one-shot
behavior and existing kill-switch details.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e3c6886a-0697-48cf-bdf1-9b053a2091de

📥 Commits

Reviewing files that changed from the base of the PR and between f9a919b and 528ec2d.

📒 Files selected for processing (1)
  • CLAUDE.md

Comment thread CLAUDE.md
Comment on lines +90 to +91
user can also apply the opt-in `deep-review` label to any PR to run the one-shot Claude
deep review on it (once ever per PR). Kill switches: remove `ai-loop` (per PR) or set repo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document the same-repository restriction

“Any PR” is broader than the workflow contract: fork PRs are intentionally rejected. Clarify that deep-review applies to same-repository PRs so users do not expect labeling a fork PR to trigger a review.

Suggested wording
- A triage+ user can also apply the opt-in `deep-review` label to any PR to run the one-shot Claude
+ A triage+ repository collaborator can also apply the opt-in `deep-review` label to any
+ same-repository PR to run the one-shot Claude
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
user can also apply the opt-in `deep-review` label to any PR to run the one-shot Claude
deep review on it (once ever per PR). Kill switches: remove `ai-loop` (per PR) or set repo
A triage+ repository collaborator can also apply the opt-in `deep-review` label to any
same-repository PR to run the one-shot Claude
deep review on it (once ever per PR). Kill switches: remove `ai-loop` (per PR) or set repo
🧰 Tools
🪛 LanguageTool

[style] ~90-~90: Consider a different adjective to strengthen your wording.
Context: ...el to any PR to run the one-shot Claude deep review on it (once ever per PR). Kill s...

(DEEP_PROFOUND)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 90 - 91, Clarify the `deep-review` documentation to
state that it applies only to same-repository PRs, not pull requests originating
from forks. Update the sentence describing labeling any PR while preserving the
one-shot behavior and existing kill-switch details.

@allxsmith
allxsmith merged commit 2cfe53c into main Jul 11, 2026
20 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Opt-in Claude deep review via deep-review label (opus)

2 participants