Menu
Review a GitHub Pull Request for the Reihitsu repository. Triggers on "review PR", "review pull request", "check PR
Instalar com Codex ou Claude Copie este prompt, cole no Codex, Claude ou outro assistente e deixe que ele revise a página da skill e instale para você.
Baseado na classificação ocupacional SOC
Orchestrator for implementing a Reihitsu GitHub issue end-to-end in a Claude Code Cloud Agent environment. Triggers when the initial prompt references a GitHub issue (e.g. "implement
Orchestrator for implementing a Reihitsu GitHub issue end-to-end in Codex on Linux cloud or local Windows. Triggers when the initial prompt references a GitHub issue (e.g. "implement
Review a GitHub Pull Request for the Reihitsu repository. Triggers on "review PR", "review pull request", "check PR
Write or update Reihitsu analyzer rule markdown files under documentation/rules/RH####.md. Use this when asked to create, rewrite, or review user-facing rule documentation for an analyzer rule.
Create Reihitsu issue draft markdown files under plans/issues that pass scripts/upload-issues.ps1 validation and can be uploaded with the repository script.
Add or update localized resource texts in Reihitsu. Use this when introducing new analyzer or code-fix strings in the .resx-based resource system.
| name | gh-review |
| description | Review a GitHub Pull Request for the Reihitsu repository. Triggers on "review PR", "review pull request", "check PR |
You review a GitHub Pull Request and report findings. Output is strict — only a checklist, a findings table, and a verification block in chat, plus inline GitHub review comments for confirmed findings. Nothing else.
You are running in Codex on Linux cloud or local Windows. The repository checkout, required .NET 10 SDK, and authenticated gh CLI are present. Before execution, confirm the SDK with dotnet --list-sdks; do not install an SDK, modify PATH, or otherwise change the environment.
The PR identifier comes from the invoking prompt or $ARGUMENTS:
123#123https://github.com/<owner>/<repo>/pull/123If no PR id can be extracted, stop and ask. Do not guess.
gh CLIUse the authenticated gh CLI for every GitHub platform operation. Confirm the active account before creating issues or comments:
gh auth status
| Purpose | Command |
|---|---|
| PR metadata (title, body, base/head, author, URL, head SHA) | gh pr view <N> --json title,body,baseRefName,headRefName,author,url,headRefOid |
| PR diff | gh pr diff <N> |
| PR changed files | gh pr view <N> --json files |
| Existing PR/review comments | gh api repos/{owner}/{repo}/pulls/<N>/comments and gh pr view <N> --json comments,reviews |
Linked issue (Closes/Fixes/Resolves #N in PR body) | gh issue view <N> --json number,title,body,labels,state,url |
| Search for an existing issue | gh issue list --search "<query>" |
| File a follow-up issue | gh issue create --title "<title>" --body "<body>" |
| Inline review comments | gh api --method POST repos/{owner}/{repo}/pulls/<N>/reviews |
| General (non-line) PR comment | gh pr comment <N> --body "<message>" |
Read the PR metadata, diff, changed files, and existing review comments first. Fetch the linked issue only when the PR body carries a Closes/Fixes/Resolves #N; skip it otherwise. Read the repository counterpart files (analyzer ↔ formatter phase ↔ code fix) directly from the local checkout with the file tools.
The diff is not the review boundary. Reihitsu defects live in interactions: between an analyzer and its formatter twin, between a fix base and its consumers, between one pipeline phase and the next. Whenever a checklist item below names a counterpart file, open and read it even though it is absent from the diff.
Walk every item. For each, mark one of:
[x] checked and applicable[ ] N/A (with one short reason, e.g. N/A (no inheritance touched))Items 2–7 are the Reihitsu invariants. They exist because the 1.0-RC reviews traced nearly every Critical to one of them. A confirmed violation of items 2–5 is always high.
| # | Item | What to look for |
|---|---|---|
| 1 | Correctness | Logic errors, off-by-one, edge cases (null / empty / max / negative), resource leaks, concurrency |
| 2 | Trivia preservation | Any edit that joins lines, collapses a token gap, or moves/reorders/deletes tokens or members: enumerate every trivia kind that can live in every affected gap — end-of-line comments, block comments, doc comments, #if/#else/#elif/#endif, #pragma, #region/#endregion, disabled text. Each must survive at a sensible position or the edit must be refused. A guard that covers only comments does not cover directives. Silent deletion or relocation into a string/another construct is a bug, not a style issue. Priority item. |
| 3 | Semantics & compilability | A rewrite (fix or formatter transform) must compile and preserve runtime behavior for all input shapes, not just the ones in the PR's tests: target-typed expressions (FormattableString, overload selection), user-defined operators, string literal contents, language-version gates, partial types, explicit interface implementations, file-local types, modifier-order rules of the C# grammar. When flagging, name the compiler error (CSxxxx) or the behavior change concretely. |
| 4 | Fix convergence | Applying a code fix must silence its own diagnostic in one pass and must not raise other RH diagnostics in the edited span. Check FixAll: overlapping spans, positions stale after the first application. A registered code action that cannot change the document must not be offered. |
| 5 | Idempotency & termination | A formatter phase run on its own output must be a no-op — including on CRLF input. Any loop-until-stable needs an obviously decreasing measure; in particular, a guard that refuses an edit must not be reported upstream as "changed" (hang risk). Watch for oscillation between neighboring phases (insert ↔ collapse). |
| 6 | Analyzer/formatter/fix parity | For a touched analyzer, formatter phase, or fix: name its counterparts and check both directions — formatter output must not be flagged by the analyzer, and analyzer-clean code must be formatter-stable. A new analyzer whose concern no formatter phase owns creates permanent diagnostics for CLI users: flag it. Shared policy must come from one place (usually Reihitsu.Core), not a private copy. |
| 7 | Defect-class closure | When the PR fixes a bug: state the general defect class, then grep for sibling shapes with the same hazard and for surviving private copies of any helper this PR consolidates. A fix that closes only the reported instance while the class stays open is an incomplete fix — say so. |
| 8 | SRP / concern leakage | A class touched by this PR gains a responsibility that belongs to another class. Method named for one job that quietly does two. New code added to a class whose name no longer describes it. Priority item. |
| 9 | Duplication | New logic that re-implements something already present elsewhere in the diff context or repository. Copy-paste of a block from another file. Priority item. |
| 10 | Other SOLID | OCP (modifying stable code instead of extending), LSP (subtype breaks parent contract), ISP (interface forces unrelated members), DIP (high-level concrete dependency) |
| 11 | Coupling / cohesion | Unnecessary cross-module dependency introduced; related members scattered across files |
| 12 | Security | Missing input validation at trust boundaries, injection (SQL/command/path), exposed secrets, missing authorization |
| 13 | Error handling | Swallowed exceptions, missing failure modes, unclear error messages, broad catch without rethrow |
| 14 | Tests | See "Test expectations" below. For analyzer or formatter bug fixes the repository requires a regression test. Analyzer tests should be many small focused tests, not one large multi-case test. |
| 15 | Performance | Only obvious issues — hot-path allocations in tight loops, O(n²) over user-sized collections, unnecessary repeated IO, per-node GetText()/ToString() materialization. Do not nitpick. |
| 16 | Repo conventions | Diagnostic ID in correct range (RH0### Analyzer, RH1### Performance, … RH8### Documentation). helpLinkUri matches the actual rule doc under documentation/rules/. Code fixes delegate final layout to ReihitsuFormatter.FormatNodeInDocumentAsync / FormatNode — but check the delegation scope is tight (formatting a whole member/type to fix one token drags unrelated edits and inherited formatter defects into the fix). Formatter still leaves syntax-invalid and generated code untouched. New analyzer rule ships a comprehensive code fix or no fix at all. |
| 17 | Naming & docs | Names align with surrounding code. Public API XML docs added/updated. Rule doc under documentation/rules/RH####.md exists and matches the rule if a rule was added or renamed. |
| 18 | Scope discipline | No out-of-scope edits. No commented-out code. No TODO left without an issue link. |
| 19 | Issue coverage | If the PR links an issue (Closes #N), every requirement listed in the issue is addressed by the diff. Flag missing requirements explicitly. |
The recurring blind spot is code shapes the author did not imagine. For every analyzer, formatter, or fix change, pick the relevant subset of these shapes and trace the changed code against them (or run them, see Verification). If a shape is relevant and neither handled nor tested, that is a finding:
#if / #else / #endif, #pragma, #region in every gap the edit touches (between modifiers, parameters, members, accessors, attribute lists, base-list entries) — including fully inactive (#if false) blocks@class), letterless/digit-only identifiers (T1, _, unicode)partial types and members; explicit interface implementations; file-local (file) types; static interface membersasync, Task<T>/ValueTask<T>What checklist item 14 concretely demands, by change type:
Missing tests from this list are findings (severity per the model below), not hints.
Default to static tracing. CI builds the solution and runs every test project on the PR, so a review does not exist to re-run the suite. Re-running everything wastes the run and proves nothing CI has not already proven.
Reach for execution only when a specific suspicion is checkable and the answer changes a finding — a convergence question, an idempotency double-run, a suspected non-compiling rewrite. In that case:
Run dotnet --list-sdks to confirm the preinstalled .NET 10 SDK. Do not install an SDK, modify PATH, or otherwise change the environment.
Run only the targeted tests that resolve the suspicion, not the whole suite. Use a --filter scoped to the affected rule(s) (examples in AGENTS.md), e.g.:
dotnet test Reihitsu.Analyzer.Test/Reihitsu.Analyzer.Test.csproj -c Release --filter "FullyQualifiedName~RH3204"
For formatter changes, run dotnet run --project Reihitsu.Cli -- <path> twice over a file exercising the change — the second run must report no changes.
A high-severity finding should carry a concrete counterexample (a short code snippet plus what goes wrong) in the review comment. Constructing the counterexample is how a suspicion earns "high confidence" — do not discard invariant suspicions merely because they are not obvious from the diff.
If targeted execution is otherwise impossible, say so in the Verification block and state what was verified by static tracing instead. Never run the full test suite just to fill the Verification block — that is CI's job.
A suspicion whose scope exceeds the PR (policy drift across assemblies, a stale copy elsewhere, a parity question about an untouched counterpart, a defect class with more call sites than the diff) must not die with the review:
gh issue list --search "<query>".gh issue create --title "<short>" --body "<origin PR, suspicion, affected files>".Never silently drop a cross-cutting suspicion, and never block the PR on it.
Post only high-confidence findings (high, medium, low) through the authenticated gh CLI. Before posting, fetch existing PR and review comments with gh api and gh pr view, and skip any finding already raised.
gh api --method POST repos/{owner}/{repo}/pulls/<N>/reviews and a JSON payload containing every comment's path, line, side, and body. Batch all inline findings into one review.gh pr comment <N> --body "<message>".Comment body rules:
high finding may add a minimal counterexample snippet.maybe, perhaps, could potentially).Only the following block, nothing else. No preamble, no closing summary, no "Done."
## Checklist
- [x] Correctness
- [x] Trivia preservation
- [x] Semantics & compilability
- [ ] Fix convergence — N/A (no code fix touched)
- [x] Idempotency & termination
- [x] Analyzer/formatter/fix parity
- [x] Defect-class closure
- [x] SRP / concern leakage
- [x] Duplication
- [ ] Other SOLID — N/A (no inheritance touched)
- [x] Coupling / cohesion
- [ ] Security — N/A (no boundary code touched)
- [x] Error handling
- [x] Tests
- [ ] Performance — N/A
- [x] Repo conventions
- [x] Naming & docs
- [x] Scope discipline
- [x] Issue coverage
## Findings
| # | Severity | Location | Posted | Summary |
|---|----------|----------|--------|---------|
| 1 | high | Reihitsu.Formatter/Pipeline/Foo.cs:42 | yes | Line join deletes `#endif` between parameters — output does not compile (CS1027) |
| 2 | medium | Reihitsu.Analyzer/Rules/RH3204/Bar.cs:88 | yes | Method parses input and writes diagnostic; split parsing into helper |
| 3 | hint | Reihitsu.Cli/Program.cs:120 | no | Possible SRP issue (see Hints) |
## Verification
- Static tracing only for the RH3204 convergence question — CI runs the full suite; no targeted execution needed.
- Ran `Reihitsu.Formatter.Test` filtered to `RH5xxx` to settle the idempotency suspicion; double-run over fixture: second pass clean.
## Hints (not posted)
**#3** — `ProcessData` reads JSON and writes a file. Borderline SRP — depends on whether `Process` is established vocabulary in this module. Worth a reviewer judgement call rather than a posted comment. Filed #412 for the cross-assembly policy question.
Rules for the chat block:
_None._ underneath it.Posted column reads yes or no. no only for hint rows.Summary cells short — one sentence. Long prose for hint rows goes under Hints, never in the table.If, after a thorough review, there are no findings of any severity (including hints), still post the Checklist block, then ## Findings with _None._ underneath (Verification block still applies). Do not post any GitHub comments. Do not write any other prose.