Skip to content

Commit 346b780

Browse files
authored
Merge pull request #812 from atlanhq/feat/claude-review-skill
feat(ci): add Claude Code workflow with /review skill for PR reviews
2 parents d52cc3d + 97db879 commit 346b780

File tree

8 files changed

+703
-5
lines changed

8 files changed

+703
-5
lines changed

.claude/commands/review.md

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
---
2+
description: Structured code review for pull requests with confidence scoring and inline comments
3+
allowed-tools: Read, Grep, Glob, Bash(gh pr diff:*), Bash(gh pr view:*), Bash(gh api:*), Bash(git log:*), Bash(git diff:*), Bash(git show:*), Bash(git blame:*), Bash(gh pr comment:*), mcp__github_inline_comment__create_inline_comment
4+
---
5+
6+
You are a senior code reviewer for the pyatlan SDK (Atlan Python Client). Perform a structured, high-signal code review of the current pull request. No emojis. Professional tone. Only flag issues you are confident about.
7+
8+
## Step 1: Load repository context
9+
10+
Read the following files to understand the project's standards and structure. These are your evaluation criteria — do not review without them:
11+
12+
- `CLAUDE.md` (if it exists — root project guidelines)
13+
- `pyproject.toml` (project config, dependencies, tooling: ruff, mypy)
14+
- `.github/PULL_REQUEST_TEMPLATE.md` (PR checklist expectations)
15+
- `README.md` (project overview)
16+
17+
Also use Glob to find any additional guideline files:
18+
- `.cursor/rules/*.mdc` or `.cursor/*.md` (coding standards, review checklists)
19+
- Any `CLAUDE.md` files in subdirectories relevant to the changed files
20+
- `CONTRIBUTING.md` if it exists
21+
22+
## Step 2: Gather PR data
23+
24+
Run these commands in parallel:
25+
26+
- `gh pr view --json number,title,body,state,isDraft,baseRefName,headRefName,additions,deletions,changedFiles,commits,labels`
27+
- `gh pr diff --name-only` (list of changed files)
28+
- `gh pr diff` (full unified diff)
29+
- `git log --oneline -30 $(gh pr view --json baseRefName -q .baseRefName)..HEAD` (branch commit history)
30+
31+
## Step 3: Determine review scale
32+
33+
Count the number of changed files from step 2.
34+
35+
**If fewer than 100 files changed:**
36+
Review all changed files directly. Read each changed file using the Read tool to understand surrounding context beyond the diff.
37+
38+
**If 100 or more files changed:**
39+
This is a large PR. Deploy parallel sub-agents to gather context efficiently:
40+
1. Partition changed files by top-level directory (e.g. `pyatlan/`, `tests/`, `docs/`)
41+
2. Launch one Explore agent per partition to read the changed files and their surrounding context
42+
3. Consolidate findings from all agents before proceeding to review passes
43+
44+
## Step 4: Four review passes
45+
46+
Execute four independent review passes. For PRs with fewer than 100 files, do these sequentially. For large PRs, launch agents in parallel.
47+
48+
### Pass 1 + 2: Code standards and style compliance
49+
50+
Audit the changes against the project's conventions. Specifically check:
51+
52+
- Type hints: all function parameters and return values must have type annotations
53+
- Pydantic models: proper usage for data structures crossing boundaries
54+
- Naming: snake_case for functions/variables, PascalCase for classes, UPPER_SNAKE_CASE for constants
55+
- Import organization: stdlib -> third-party -> local, no unused imports
56+
- Ruff/mypy compliance: patterns that would fail ruff or mypy checks
57+
- Docstrings: public APIs should have clear documentation
58+
- Test coverage: new functionality should have corresponding tests
59+
- PR checklist: verify claims in the PR template (tests added, docs updated, etc.)
60+
61+
For each violation, be specific about what rule is being broken and where.
62+
63+
### Pass 3: Bug and security scan
64+
65+
Focus only on the diff — do not flag pre-existing issues. Check:
66+
67+
- Security: hardcoded secrets/tokens, SQL injection, missing input validation, sensitive data in logs, unsafe deserialization
68+
- Correctness: logic errors, off-by-one errors, incorrect exception handling, wrong return types
69+
- Resource management: unclosed files/connections, missing timeouts on HTTP calls, resource leaks
70+
- Python anti-patterns: mutable default arguments, blocking calls in async contexts, bare `except:` clauses, unreachable code
71+
- Pydantic: incorrect model definitions, missing validators, wrong field types
72+
- HTTP client patterns: missing error handling on httpx calls, missing retries for transient failures
73+
74+
Only flag significant issues. Ignore nitpicks and anything you cannot validate from the diff alone.
75+
76+
### Pass 4: Context and history analysis
77+
78+
Use git blame and git log on the changed files to understand:
79+
80+
- Is this a workaround or a root cause fix?
81+
- Does the change fit the existing codebase architecture?
82+
- Are there test coverage gaps for new/changed code?
83+
- Is the change backward compatible? (pyatlan supports Python 3.9+)
84+
- Are there breaking changes to the public API?
85+
- Do any dependency changes introduce security or compatibility risks?
86+
87+
## Step 5: Score and validate findings
88+
89+
For each issue found across all passes, assign a confidence score from 0 to 100:
90+
- **0**: Not confident, likely false positive
91+
- **25**: Somewhat confident, might be real
92+
- **50**: Moderately confident, real but minor
93+
- **75**: Highly confident, real and important
94+
- **100**: Absolutely certain, definitely real
95+
96+
**Validation**: For each finding scored 50 or above, verify it by:
97+
- Re-reading the relevant code in full context (not just the diff)
98+
- Checking if the pattern is intentionally used elsewhere in the codebase
99+
- For style violations: confirming the project actually enforces this rule
100+
101+
**Filter**: Discard all findings below confidence 80.
102+
103+
**Always discard (false positives):**
104+
- Pre-existing issues not introduced in this PR
105+
- Code that appears buggy but is actually correct in context
106+
- Pedantic nitpicks a senior engineer would not flag
107+
- Issues that linters (ruff, mypy) will catch — do not run the linter to verify
108+
- General code quality concerns not explicitly required by project conventions
109+
- Issues silenced in code via lint-ignore comments
110+
111+
## Step 6: Post summary comment
112+
113+
Use `gh pr comment` to post a single comment with this exact structure. Use a HEREDOC for the body. Do not use emojis anywhere.
114+
115+
```
116+
## Code Review
117+
118+
<2-3 sentence summary of what this PR does and its approach. Be specific about the technical change.>
119+
120+
### Confidence Score: X/5
121+
122+
- <Bullet explaining what the score means for this specific PR>
123+
- <Bullet listing what was checked: bugs, security, standards compliance, test coverage>
124+
- <If points were deducted, explain specifically why>
125+
126+
<details>
127+
<summary>Important Files Changed</summary>
128+
129+
| File | Change | Risk |
130+
|------|--------|------|
131+
| <path> | Added/Modified/Deleted | Low/Medium/High |
132+
133+
</details>
134+
135+
### Change Flow
136+
137+
```mermaid
138+
sequenceDiagram
139+
participant A as <Component>
140+
participant B as <Component>
141+
<interactions showing the primary flow affected by this PR>
142+
```
143+
144+
<Generate a Mermaid sequence diagram that shows the key interaction flow introduced or modified by this PR. Rules:>
145+
<- Maximum 8 participants>
146+
<- Maximum 15 interactions>
147+
<- For refactors: show before/after with labeled boxes>
148+
<- For new features: show the end-to-end flow>
149+
<- For bug fixes: show the incorrect flow crossed out and the corrected flow>
150+
<- Use descriptive labels on arrows>
151+
152+
### Findings
153+
154+
<If findings exist above threshold:>
155+
156+
| # | Severity | File | Issue |
157+
|---|----------|------|-------|
158+
| 1 | Critical/Warning/Info | `path/to/file.py:L42` | Brief description |
159+
160+
<If no findings:>
161+
162+
No issues found. Checked for bugs, security, and code standards compliance.
163+
```
164+
165+
**Confidence Score Rubric:**
166+
- **5/5**: Safe to merge — no issues, follows all standards, well-tested
167+
- **4/5**: Minor observations only — style/documentation nits, no functional risk
168+
- **3/5**: Needs attention — moderate issues that should be addressed before merge
169+
- **2/5**: Significant concerns — security, performance, or correctness issues found
170+
- **1/5**: Do not merge — critical problems requiring substantial rework
171+
172+
## Step 7: Post inline comments
173+
174+
For each finding in the Findings table, post an inline comment using `mcp__github_inline_comment__create_inline_comment`.
175+
176+
Rules for inline comments:
177+
- Maximum 10 inline comments total (prioritize by severity)
178+
- Each comment includes: severity tag, issue description, why it matters, and the suggested fix
179+
- For small, self-contained fixes (< 6 lines): include a committable suggestion block
180+
- For larger fixes: describe the issue and suggested approach without a suggestion block
181+
- Never post a committable suggestion unless committing it fully fixes the issue
182+
- Post exactly ONE comment per unique issue — no duplicates
183+
- Link format for code references: `https://github.com/<owner>/<repo>/blob/<full-sha>/path/to/file.ext#L<start>-L<end>` — always use the full SHA, never abbreviated
184+
185+
## Constraints
186+
187+
- Use `gh` CLI for all GitHub interactions. Do not use web fetch.
188+
- Never use emojis in any output.
189+
- Do not flag issues you cannot verify from the code. When in doubt, leave it out.
190+
- Do not suggest changes that would require reading code outside of the changed files and their immediate context.
191+
- Prioritize signal over completeness. A review with 3 real issues is better than one with 15 questionable ones.
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
name: "GlobalProtect VPN Connect"
2+
description: "Connect to GlobalProtect VPN using OpenConnect"
3+
author: "Atlan"
4+
5+
inputs:
6+
portal-url:
7+
description: "GlobalProtect portal URL"
8+
required: true
9+
username:
10+
description: "GlobalProtect username"
11+
required: true
12+
password:
13+
description: "GlobalProtect password"
14+
required: true
15+
max-attempts:
16+
description: "Maximum number of connection attempts"
17+
required: false
18+
default: "3"
19+
20+
outputs:
21+
vpn-connected:
22+
description: "Whether VPN connection was successful"
23+
value: ${{ steps.openconnect-connection.outputs.connected }}
24+
25+
runs:
26+
using: "composite"
27+
steps:
28+
- name: Connect via OpenConnect
29+
uses: nick-fields/retry@v3
30+
id: openconnect-connection
31+
with:
32+
max_attempts: ${{ fromJSON(inputs.max-attempts) }}
33+
timeout_minutes: 10
34+
shell: bash
35+
command: |
36+
echo "Connecting to GlobalProtect VPN using OpenConnect..."
37+
38+
# Install OpenConnect
39+
echo "Installing OpenConnect..."
40+
sudo apt-get update
41+
sudo apt-get install -y openconnect
42+
43+
# Check required inputs
44+
if [ -z "${{ inputs.portal-url }}" ]; then
45+
echo "portal-url input not provided"
46+
exit 1
47+
fi
48+
49+
if [ -z "${{ inputs.username }}" ]; then
50+
echo "username input not provided"
51+
exit 1
52+
fi
53+
54+
if [ -z "${{ inputs.password }}" ]; then
55+
echo "password input not provided"
56+
exit 1
57+
fi
58+
59+
echo "Attempting OpenConnect to: ${{ inputs.portal-url }}"
60+
61+
# Create credentials file for OpenConnect
62+
echo "${{ inputs.password }}" | sudo openconnect \
63+
--protocol=gp \
64+
--user="${{ inputs.username }}" \
65+
--passwd-on-stdin \
66+
--background \
67+
"${{ inputs.portal-url }}" || {
68+
echo "OpenConnect connection failed"
69+
exit 1
70+
}
71+
72+
# Wait a moment for connection to establish
73+
sleep 10
74+
75+
# Initial connectivity test
76+
echo "Testing initial connectivity to Phoenix AI..."
77+
time curl -k -sS https://phoenix-ai.atlan.com -o /dev/null
78+
79+
if [ $? -eq 0 ]; then
80+
echo "Initial connectivity test successful"
81+
else
82+
echo "Initial connectivity test failed"
83+
exit 1
84+
fi
85+
86+
# Set output to indicate successful connection
87+
echo "connected=true" >> $GITHUB_OUTPUT
88+
echo "VPN connection established successfully"

.github/workflows/claude.yml

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
name: Claude Code
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
pull_request_review_comment:
7+
types: [created]
8+
issues:
9+
types: [opened, assigned]
10+
11+
permissions:
12+
id-token: write
13+
contents: write
14+
issues: write
15+
pull-requests: write
16+
17+
jobs:
18+
claude:
19+
if: |
20+
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
21+
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
22+
(github.event_name == 'issues' && contains(github.event.issue.body, '@claude'))
23+
runs-on: ubuntu-latest
24+
permissions:
25+
contents: read
26+
pull-requests: write
27+
id-token: write
28+
timeout-minutes: 15
29+
env:
30+
ANTHROPIC_BASE_URL: https://llmproxy.atlan.dev
31+
CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS: '1'
32+
concurrency:
33+
group: claude-${{ github.event.issue.number || github.event.pull_request.number }}
34+
cancel-in-progress: true
35+
steps:
36+
- uses: actions/checkout@v4
37+
with:
38+
fetch-depth: 0
39+
40+
- name: Connect to VPN
41+
uses: ./.github/actions/globalprotect-connect
42+
with:
43+
portal-url: vpn2.atlan.app
44+
username: ${{ secrets.GLOBALPROTECT_USERNAME }}
45+
password: ${{ secrets.GLOBALPROTECT_PASSWORD }}
46+
47+
- uses: anthropics/claude-code-action@v1
48+
with:
49+
anthropic_api_key: ${{ secrets.LITELLM_API_KEY }}
50+
claude_args: |
51+
--model claude-sonnet-4
52+
--allowedTools "mcp__github_inline_comment__create_inline_comment"
53+
--append-system-prompt "CRITICAL INSTRUCTION: When a user asks you to review a pull request (e.g. @claude review, @claude review this PR, @claude please review), you MUST execute the /review slash command. Do NOT attempt a freeform review. Always use the /review skill — no exceptions."

0 commit comments

Comments
 (0)