Skip to content

Commit b006c95

Browse files
authored
Merge pull request #18043 from ethereum/intl-pipeline-v7
feat(intl-pipeline): iterative patches and incremental reviews
2 parents 9d7a3aa + 6175536 commit b006c95

15 files changed

Lines changed: 1317 additions & 356 deletions

File tree

.claude/commands/review-translations.md

Lines changed: 74 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
description: Review translation imports for quality issues (full pipeline)
33
allowed-tools: Bash(git *), Bash(pnpm *), Bash(npx tsx *), Bash(gh *), Bash(cp *), Bash(pwd), Bash(ls *), Bash(test *), Read, Glob, Grep, Task, Edit, Write, AskUserQuestion
4-
argument-hint: [--pr=NUMBER (auto)] [--scope=pr|full (pr)] [--language=CODE] [--model=opus|sonnet|haiku (opus)] [--no-fix] [--build-local] [--netlify-check]
4+
argument-hint: [--pr=NUMBER] [--language=CODE] [--model=opus|sonnet|haiku (opus)] [--full] [--no-fix] [--build-local] [--netlify-check]
55
---
66

77
# Translation Review Command
@@ -14,20 +14,18 @@ Full pipeline for reviewing translation imports: worktree setup, sanitizer, AI r
1414

1515
## Modes of Operation
1616

17-
### Mode 1: PR Review (Default)
18-
Reviews files changed in a specific PR.
17+
### Mode 1: Pending Translations (Default)
18+
Reviews the open PR for the canonical pending-translations branch (`intl/pending-dev`).
1919
```
20-
/review-translations # Auto-detect PR, review all languages
21-
/review-translations --pr=16979 # Review specific PR's changed files
22-
/review-translations --scope=full # Review ALL files for languages in PR
20+
/review-translations # Open PR for intl/pending-dev, all languages
21+
/review-translations --language=hi # Same, filtered to Hindi only
2322
```
2423

25-
### Mode 2: Filtered PR Review
26-
Reviews only specific language(s) from a PR.
24+
### Mode 2: Specific PR
25+
Reviews a specific PR (e.g., a feature-branch translation PR like `intl/pending-feat-foo`).
2726
```
28-
/review-translations --language=hi # Filter auto-detected PR to Hindi only
29-
/review-translations --pr=16979 --language=hi # Review only Hindi files from PR #16979
30-
/review-translations --pr=16979 --language=hi,bn --scope=full # All Hindi+Bengali files
27+
/review-translations --pr=18040 # Specific PR
28+
/review-translations --pr=18040 --language=hi # Specific PR, Hindi only
3129
```
3230

3331
### Mode 3: Standalone Language Review
@@ -36,14 +34,18 @@ Reviews all files for a language when no PR context is available.
3634
/review-translations --language=es # On dev branch: review all Spanish files
3735
```
3836

37+
## Scope behavior
38+
39+
By default, the command reviews **only files changed since the last LLM review** of this PR (incremental). The prior-review SHA is read from the most recent submitted PR Review on the PR (`commit_id` field, see Phase 0). If no prior review exists, the full PR diff is reviewed. Pass `--full` to override and re-review the entire PR diff even when a prior review exists.
40+
3941
## Flags
4042

4143
| Flag | Description | Default |
4244
|------|-------------|---------|
43-
| `--pr=NUMBER` | Specific PR to review | auto-detect from `i18n*` branch |
44-
| `--scope=pr\|full` | `pr` = only PR changed files, `full` = all files for languages | `pr` |
45+
| `--pr=NUMBER` | Specific PR to review | open PR for `intl/pending-dev` |
4546
| `--language=CODES` | Filter to specific language(s), comma-separated | all languages in PR |
4647
| `--model=MODEL` | Model for analysis: `opus` (deep), `sonnet` (balanced), `haiku` (fast) | `opus` |
48+
| `--full` | Re-review the entire PR diff, ignoring any prior review SHA | absent (incremental) |
4749
| `--no-fix` | Skip auto-fixing critical issues; only present findings | absent (fixes applied by default) |
4850
| `--build-local` | Run a local scoped build to verify no MDX compilation errors | absent (skipped by default) |
4951
| `--netlify-check` | Check Netlify deploy preview for build failures | absent (skipped by default) |
@@ -53,9 +55,9 @@ Reviews all files for a language when no PR context is available.
5355
### Parse Flags
5456

5557
Extract from $ARGUMENTS:
56-
- `PR_NUMBER`: from `--pr=NUMBER` or auto-detect
58+
- `PR_NUMBER`: from `--pr=NUMBER` or auto-detect (see below)
5759
- `LANGUAGE_FILTER`: from `--language=CODES` (comma-separated) or empty
58-
- `SCOPE`: from `--scope=pr|full` (default: `pr`)
60+
- `FULL_REVIEW`: `true` if `--full` is present, `false` otherwise
5961
- `NO_FIX`: `true` if `--no-fix` is present, `false` otherwise
6062
- `BUILD_LOCAL`: `true` if `--build-local` is present, `false` otherwise
6163
- `NETLIFY_CHECK`: `true` if `--netlify-check` is present, `false` otherwise
@@ -64,25 +66,22 @@ Extract from $ARGUMENTS:
6466

6567
1. **Attempt PR Detection**
6668
- If `--pr=NUMBER` provided → use that PR
67-
- Otherwise, check if branch starts with `i18n`:
69+
- Otherwise, look up the open PR for the canonical pending-translations branch:
6870
```bash
69-
BRANCH=$(git branch --show-current)
70-
if [[ "$BRANCH" == i18n* ]]; then
71-
PR_NUMBER=$(gh pr view --json number -q .number 2>/dev/null)
72-
fi
71+
PR_NUMBER=$(gh pr list --head intl/pending-dev --state open --json number -q '.[0].number' 2>/dev/null)
7372
```
7473

7574
2. **Route to Mode**
7675
- If `PR_NUMBER` found → continue to **PR Mode Setup**
7776
- If no `PR_NUMBER`:
7877
- If `--language` provided → continue to **Standalone Mode Setup**
79-
- Otherwise → error: "No PR detected. Use --pr=NUMBER or --language=CODE"
78+
- Otherwise → error: "No open PR found for intl/pending-dev. Use --pr=NUMBER or --language=CODE."
8079

8180
### PR Mode Setup (Mode 1 & 2)
8281

8382
3. **Determine Languages**
84-
- If `--language=CODES` provided: Use those as filter (Mode 2)
85-
- Otherwise: Extract all languages from PR (Mode 1)
83+
- If `--language=CODES` provided: Use those as filter
84+
- Otherwise: Extract all languages from PR
8685

8786
To extract languages from PR:
8887
```bash
@@ -91,26 +90,44 @@ Extract from $ARGUMENTS:
9190
sed 's|.*translations/||;s|.*intl/||' | cut -d'/' -f1 | sort -u
9291
```
9392

94-
4. **Determine File Scope**
95-
- If `--scope=full`: Review ALL files for the determined languages
96-
- If `--scope=pr` (default): Review ONLY files changed in the PR
93+
4. **Determine Scope: Incremental vs. Full PR Diff**
94+
95+
The default behavior is **incremental** — review only files changed since the last LLM review of this PR. The prior-review SHA comes from the PR's submitted reviews. If `FULL_REVIEW` is true (i.e., `--full` was passed), skip the prior-review lookup entirely and use the full PR diff.
9796
98-
For `--scope=pr`, get the specific file list:
9997
```bash
100-
gh api repos/{owner}/{repo}/pulls/{PR}/files --paginate -q '.[].filename' | \
101-
grep -E "(translations/|intl/)" | \
102-
grep -E "/(${LANGUAGES_REGEX})/" # If language filter applied
98+
# Fetch all submitted reviews for this PR, sorted oldest -> newest
99+
# Skip this fetch entirely when FULL_REVIEW is true.
100+
REVIEWS_JSON=$(gh api "repos/{owner}/{repo}/pulls/${PR_NUMBER}/reviews" --paginate)
103101
```
104102
105-
5. **Report**:
106-
- Mode 1: "Reviewing {N} files in PR #{NUMBER} ({LANGUAGES})"
107-
- Mode 2: "Reviewing {N} {LANGUAGE} files in PR #{NUMBER}"
103+
**Identify the prior intl-pipeline review** (most recent first). If `FULL_REVIEW` is true, skip this step and proceed directly to the full-PR-diff branch below; report `"Override (--full): reviewing full PR diff ({N} files)."` This command may be invoked locally, in which case the review may have been posted under the user's GitHub identity rather than Claude's — so do **not** filter by `user.login`. Instead, scan review bodies and pick the most recent one whose body looks like a translation-quality review (heuristics: contains the heading `Translation Quality Review`, mentions multiple language codes, contains a scoring table, mentions glossary/ETHGlossary, or is signed by Claude). Bodies like "LGTM", "approved", or unrelated technical reviews must NOT match.
104+
105+
- If a matching review is found:
106+
- Set `LAST_REVIEWED_SHA = <commit_id>` from that review object (GitHub-attached, authoritative).
107+
- Compute the file list as the diff from `LAST_REVIEWED_SHA` to PR HEAD:
108+
```bash
109+
PR_HEAD_SHA=$(gh pr view ${PR_NUMBER} --json headRefOid -q .headRefOid)
110+
gh api "repos/{owner}/{repo}/compare/${LAST_REVIEWED_SHA}...${PR_HEAD_SHA}" \
111+
--jq '.files[].filename' | \
112+
grep -E "(translations/|intl/)" | \
113+
grep -E "/(${LANGUAGES_REGEX})/" # If language filter applied
114+
```
115+
- If the compare API errors (e.g., `LAST_REVIEWED_SHA` is unreachable from current HEAD due to a force-push or rebase): log a warning and fall back to the full PR diff below.
116+
- Report: "Incremental review since prior review at `${LAST_REVIEWED_SHA:0:10}` -- {N} files changed."
117+
118+
- If no matching prior review is found: review the full PR diff.
119+
```bash
120+
gh api repos/{owner}/{repo}/pulls/{PR}/files --paginate -q '.[].filename' | \
121+
grep -E "(translations/|intl/)" | \
122+
grep -E "/(${LANGUAGES_REGEX})/" # If language filter applied
123+
```
124+
Report: "No prior LLM review found -- reviewing full PR diff ({N} files)."
108125
109126
### Standalone Mode Setup (Mode 3)
110127
111128
3. **Set Languages** from `--language=CODES`
112129
113-
4. **Set Scope** to `full` (review all files for those languages on `dev` branch)
130+
4. **Scope:** Review all files for those languages on the `dev` branch.
114131
115132
5. **Report**: "Reviewing all {LANGUAGE} files on dev branch"
116133
@@ -191,30 +208,6 @@ cp .env.example .env.local
191208
pnpm install
192209
```
193210
194-
### 1d. Run Sanitizer
195-
196-
Run the PR-scoped sanitizer to fix deterministic issues before the AI review:
197-
198-
```bash
199-
cd "$WORKTREE_PATH"
200-
npx tsx src/scripts/i18n/sanitize-pr.ts --pr={PR_NUMBER}
201-
```
202-
203-
The sanitizer handles:
204-
- Brand name auto-fix in frontmatter tags
205-
- Ticker symbol corrections
206-
- MDX angle bracket escaping (`<``&lt;`)
207-
- Orphaned HTML tag removal
208-
- Cross-script contamination detection
209-
- Untranslated content detection (franc-min)
210-
211-
**Review the sanitizer output.** Stage the fixes it makes:
212-
```bash
213-
git add -A public/content/translations/ src/intl/
214-
```
215-
216-
Report to user: "Sanitizer complete. {N} files modified. Changes staged."
217-
218211
## Phase 2: Load Knowledge Base and Glossary
219212
220213
Before deploying agents, load accumulated knowledge from prior reviews:
@@ -323,12 +316,12 @@ The community has voted on these translations for key Ethereum terms. Use these
323316
324317
## Review Methodology
325318
326-
**For PR scope (`--scope=pr`):**
327-
- Focus on NEW or CHANGED content in the PR (not pre-existing content)
319+
**For PR-mode reviews (Modes 1 & 2):**
320+
- Focus on NEW or CHANGED content within the scope determined in Phase 0 (incremental since last review, or full PR diff)
328321
- Issues in unchanged lines are out of scope for this review
329322
- Read both translation AND English source files from the worktree
330323
331-
**For full scope (`--scope=full` or `--language`):**
324+
**For standalone language review (Mode 3):**
332325
- Review the entire current content of each file
333326
- Compare against English source files from the worktree
334327
@@ -338,6 +331,10 @@ Use the ETHGlossary terms fetched in Phase 2 as the authority for technical term
338331
339332
**If you skip ETHGlossary, the entire review is invalid.**
340333
334+
## On finding zero issues
335+
336+
**Reporting zero critical issues is a fully acceptable outcome.** Do not invent issues to "show your work." If you genuinely cannot find a critical problem after a thorough check, report `0 critical, N warnings` (or `0/0`) and that is a valid result. Fabricated criticals cost more reviewer time than missed minor issues.
337+
341338
## Review Checklist
342339
343340
**If agent role is "structural":**
@@ -597,27 +594,38 @@ Use AskUserQuestion to present options:
597594
**Question:** "Review complete. Found X critical issues (auto-fixed), Y warnings across N languages."
598595
599596
**Options:**
600-
1. **Post scores to PR**Post quality scores as a comment on the PR
597+
1. **Submit review to PR**Submit quality scores as a proper PR Review (not an issue comment)
601598
2. **Review warnings** — Show detailed warning list for manual review
602599
3. **Prepare commit message** — Generate commit message for all staged changes (sanitizer + review fixes)
603600
4. **Done** — End review session
604601
605-
### If "Post scores to PR" selected:
602+
### If "Submit review to PR" selected:
603+
604+
**This MUST be submitted as a proper PR Review, not an issue comment.** The next invocation of `/review-translations` reads each PR Review's GitHub-attached `commit_id` to determine the incremental scope. An issue comment (`gh pr comment`) does not carry a `commit_id` and would break the incremental flow.
606605
607-
Write the comment body to a temp file (to avoid heredoc backtick issues), then post:
606+
Write the review body to a temp file (to avoid heredoc backtick issues), then submit it via `gh pr review`, which auto-attaches the current PR HEAD SHA as `commit_id`:
608607
609608
```bash
610-
gh pr comment {PR_NUMBER} --body-file "$TMPDIR/pr-comment-{PR_NUMBER}.md"
609+
gh pr review ${PR_NUMBER} --comment --body-file "$TMPDIR/pr-review-${PR_NUMBER}.md"
611610
```
612611
613-
Comment format:
612+
Use `--approve` instead of `--comment` only when the review turned up **zero critical issues** (whether because none were found, or because all were auto-fixed in this same run). Otherwise use `--comment`. Never use `--request-changes`.
613+
614+
Review body format:
614615
```markdown
615616
## Translation Quality Review
616617
617618
**PR:** #{PR_NUMBER}
619+
**Branch HEAD:** `{PR_HEAD_SHA_FIRST_10}` (capture inline: `gh pr view ${PR_NUMBER} --json headRefOid -q .headRefOid`)
618620
**Languages:** {LANG_LIST}
619621
**Files reviewed:** {TOTAL_FILES}
620622
**Date:** {TODAY}
623+
**Fixes:** {FIXES_LINE}
624+
625+
Where `{FIXES_LINE}` is one of:
626+
- `Critical fixes applied: {N}` -- when running locally with auto-fix enabled and fixes were committed to this branch
627+
- `No fixes applied (review-only)` -- when running in GitHub Actions without `--fix`, or when `--no-fix` was passed locally
628+
- `No critical issues found` -- when there were no critical issues to fix in the first place
621629
622630
| Language | Files | Quality Score | Issues |
623631
|----------|-------|---------------|--------|

0 commit comments

Comments
 (0)