From 5ff5088b0a3978f35b49e35c71add4764697c257 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 22 May 2026 14:07:44 -0500 Subject: docs(skills): scope review-code's CI-trust and CLAUDE.md-citation rules Two clarifications to review-code where it appeared to contradict other rules. The "trust CI, don't run builds" rule read as a blanket license to skip verification. I scoped it to reviewing a diff, not shipping one. A pre-commit or pre-push flow still owes the local verification verification.md requires. Reading a PR doesn't duplicate CI. Producing one doesn't get to skip it. The CLAUDE.md-adherence audit could put a CLAUDE.md citation into a team-visible PR comment, which commits.md says not to do. I added two modes. A private review cites CLAUDE.md directly. A public review translates the rule into the engineering reason and doesn't name the file, since a teammate can act on the reason but not on a file they can't reach. --- review-code/SKILL.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/review-code/SKILL.md b/review-code/SKILL.md index 7a1d175..95ac053 100644 --- a/review-code/SKILL.md +++ b/review-code/SKILL.md @@ -182,7 +182,7 @@ For each issue surfaced by any perspective or criterion, self-scrutinize: **am I Do **not** flag any of these as issues: - **Pre-existing issues** on unmodified lines — note separately as "for follow-up," don't block this PR -- **Lint / typecheck / test failures** — CI handles those; don't run builds yourself +- **Lint / typecheck / test failures** — CI handles those; don't run builds yourself. This applies to *reviewing* a diff, not to shipping one: a pre-commit or pre-push flow still owes the local verification `verification.md` requires (run the suite, or state "not run because..."). Reading a PR doesn't duplicate CI; producing a PR doesn't get to skip it. - **Nitpicks a senior engineer wouldn't call out** — unless CLAUDE.md explicitly requires them - **Style issues** — formatters and linters handle these - **Issues explicitly silenced in code** (e.g., `# type: ignore[...]` with a reason, lint ignore comments) unless the silencing is unjustified @@ -401,3 +401,10 @@ Bad (too terse — leaves the reader to fill in the gaps): ## Content scope Output this skill produces that gets committed or shared with the team must follow the *Content scope for public artifacts* rule in [`commits.md`](../claude-rules/commits.md): no local paths, no private repo names, no personal tooling references. + +This skill audits `CLAUDE.md` adherence, which creates a tension: a finding may be grounded in a `CLAUDE.md` rule, but `CLAUDE.md` is personal/project tooling a teammate can't always reach. Resolve it by where the review lands: + +- **Private or internal review** (local report, your own repo, output not posted to a shared PR) — cite `CLAUDE.md` directly. "`CLAUDE.md` requires `cast()` over `# type: ignore`; this diff adds three." +- **Public or team review** (posted to a team-visible PR) — translate the rule into the engineering reason it encodes, and don't name the rules file. "Prefer `cast()` here so the type stays checked rather than silenced" beats "violates `CLAUDE.md`." The teammate can act on the reason; they can't act on a file they don't have. + +The underlying rule is the same one `commits.md` states for citing personal tooling in public artifacts: surface the reason, not the rule's source. -- cgit v1.2.3