Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/skills/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Two complementary artifact-capture skills. Coral / council should offer them at

### Hardening
- **`bugbash/`** — Long-running, read-only adversarial review of an existing system by the council of experts. Loops discovery + challenger passes against a named target (`/bugbash SeiNode controller`) until the experts converge on a launch verdict. Output is a structured findings log at `docs/bugbash/<target>.md` with per-item Scenario / Impact / Issue / Fix sketch / Test coverage. Inspired by the [RALPHY loop](https://github.com/snarktank/ralph), reframed for hardening before launch. Distinct from `/security-review` (single-pass, security-only) and `/coral` (collaborative iteration, not adversarial).
- **`pr-quality/`** — Two-mode PR convention coordinator. Pre-PR (agent-invoked before `gh pr create`; findings surface inline against the staged diff for revision) and post-PR (user-invocable via `/pr-quality <PR>`; posts a fresh comment with findings). No CI infrastructure — proactive trigger lives in `CLAUDE.md` / `AGENTS.md` working agreements (same pattern as `/brevity`). v1 dispatches verbosity via `/brevity` (subagent-loads-target-skill; pr-quality detects the trigger, brevity owns the standard) + 5 convention rules (`no_cpu_limits`, `harbor_ecr_convention`, `narration_comments`, `temporary_migration_notes`, `authoritative_voice`). Suggestive only — never gates merge. Sibling to `/brevity` (verbosity-only).

### Investigation
- **`root-cause/`** — Disciplined, data-driven, multi-expert investigation of complex problems in the Sei platform stack (sei-k8s-controller, seictl, sei-sidecar, sei-chain, release-test/qa-testing, platform/K8s). Forces signals before hypotheses, ≥2 competing hypotheses before evidence, retrieved provenance (not paraphrased), and falsification before conclusion. Dispatches `.claude/agents/` specialists in **parallel + blinded + with assigned dissent** to prevent the consensus-theater / sycophancy failure mode documented in the multi-agent LLM literature. Output is a multi-cause ranked conclusion — never a single root cause. Distinct from `/bugbash` (pre-launch adversarial), `/coral` (collaborative iteration), and live incident command (mitigate first; this skill is for understanding). Tide on-chain agentic harness is explicitly out of scope.
Expand Down
88 changes: 88 additions & 0 deletions .claude/skills/pr-quality/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
---
name: pr-quality
description: "Use when about to open a PR or reviewing one — 'opening a PR', 'run pr-quality on this', 'check my PR', '/pr-quality', '/pr-quality 94'. Fires before `gh pr create` (pre-PR mode — agent surfaces findings inline against the staged diff so the author can revise) and on demand against an existing PR (post-PR mode — posts a fresh PR comment with findings). Suggestive only — never gates merge. Anti-triggers: NOT for blocking PRs (use branch protection if you need gating); NOT for style auto-fix (use gofmt / prettier); NOT for license / IP / security scanning (separate tooling). For verbosity-only on agent output, use /brevity directly. For multi-component design / cross-review, use /council."
---

# pr-quality

A two-mode coordinator that runs a fixed v1 judge set against a PR's diff + body and surfaces findings — either inline (pre-PR) or as a single fresh PR comment (post-PR). Suggestive only. Silence on zero findings.

This skill is **agent-invoked and user-invocable**. No CI workflow, no GitHub App install, no secret management. Proactive trigger lives in `CLAUDE.md` / `AGENTS.md` working-agreement references.

## Guardrails

Before any side-effecting action:

1. **Mode check.** Pre-PR mode reads the staged diff (`git diff --cached` or the planned diff if PR isn't created yet) + the agent's planned body. Post-PR mode reads an existing PR via `gh pr view --json body` + `gh pr diff <PR>`. If neither is determinable, halt.

2. **Refusal conditions.** This skill will refuse to:
- **Block merge.** No exit code on findings; the skill is suggestive by contract.
- **Run rules outside the locked v1 set** documented in [`references/rule-registry.md`](references/rule-registry.md). Adding a rule is a PR against that file, not a runtime override.
- **Edit code or push commits.** Pre-PR surfaces findings to the agent for revision; agent decides what to apply. Post-PR posts a comment only.
- **Comment on closed or merged PRs.** Post-PR mode silently skips.
- **Comment on someone else's PR without explicit user invocation.** Pre-PR is the agent's own pre-flight; post-PR requires the user to name the PR (`/pr-quality <PR>`).

3. **Halt conditions** (exit cleanly, no comment):
- PR diff is empty.
- PR exceeds the size threshold in `references/rule-registry.md` (default 5000 lines).
- Any LLM judge subagent returns malformed output twice — log and abort that judge; continue with others.

## Procedure

The skill runs from a Claude Code session (interactive or agent-driven). Steps below are executed by Claude using the `Bash` and `Agent` tools; mechanical scans are deterministic scripts, LLM judges are subagent dispatches loading the per-rule prompt files.

### Pre-PR mode

Triggered when an agent is about to invoke `gh pr create` (per the working agreement in `CLAUDE.md` / `AGENTS.md`).

1. **Read the staged diff.** `git diff --cached` (or `git diff HEAD origin/main` if the agent has already pushed but not created the PR). Read the agent's planned PR body.
2. **For each rule** in [`references/rule-registry.md`](references/rule-registry.md) whose file scope matches the staged changes:
- Mechanical rules: run the predicate script (`scripts/scan-yaml-cpu.sh`, `scripts/scan-harbor-ghcr.sh`).
- LLM-judged rules: dispatch a subagent with the corresponding `references/judges/<rule>.md` as the prompt + the diff slice as input.
- Brevity dispatch: dispatch a subagent that loads `.claude/skills/brevity/SKILL.md` and applies it to the planned PR body. (This judge is a *trigger detector*, not a re-implementation of brevity's rules — the standard lives in `/brevity` itself.)
3. **Surface findings inline.** Group by severity (`warn` first), sort by file path within tier. No comment posted — the agent uses findings to revise before `gh pr create`.
4. **The agent then revises** the body / code / both and re-runs the skill, or proceeds to `gh pr create` if findings count is acceptable.

### Post-PR mode

Triggered by `/pr-quality` (current PR by branch) or `/pr-quality <PR>` (explicit PR number).

1. **Read the PR.** `gh pr view <PR> --json body,headRefOid` + `gh pr diff <PR>`.
2. **For each rule in scope** — same dispatch as pre-PR mode.
3. **Aggregate findings.** Group by severity, sort by file path.
4. **Post a fresh comment** if findings count > 0:
```
gh pr comment <PR> --body-file <(...)
```
The comment body follows [`references/format-spec.md`](references/format-spec.md). No comment when zero findings — silence is the success state.

## Halt Conditions

Halt cleanly (no comment) if:

- PR diff is empty.
- PR exceeds the configured size threshold (see `rule-registry.md`).
- LLM judge subagent fails (returns malformed JSON) twice — log; continue with remaining judges; emit a finding-set without the failed judge.
- (Post-PR mode) PR is closed or merged at read time.

## What this skill doesn't do

- **Block merge.** Branch protection rules are the gating mechanism, not this skill.
- **Auto-fix.** Suggestions only.
- **Run unlocked rules.** The v1 set is in `rule-registry.md`. New rules are a PR.
- **Self-consistency / multi-sample judging.** v1 single-shot per LLM judge. Un-defer trigger: a real PR produces a false-positive that consistency would have caught.
- **Cap findings.** v1 uncapped. Un-defer trigger: a real PR's output reads as wallpaper (>~7 findings); add severity-rank-and-cap then.
- **Anchored comment / hash dedupe / PATCH-in-place.** v1 posts fresh comment each invocation. Un-defer trigger: comment-spam complaint on repeated invocations of the same PR.
- **Skip-pr-quality label / opt-out.** Local invocation; user just doesn't invoke. No label needed.

## References

- [`references/rule-registry.md`](references/rule-registry.md) — locked v1 rule set, scope per rule, un-defer triggers for deferred dimensions, size threshold knob
- [`references/judges/`](references/judges/) — one file per LLM-judged rule with prompt + scope + few-shot examples
- [`references/format-spec.md`](references/format-spec.md) — post-PR comment body format

## Output

**Pre-PR mode**: structured finding list surfaced to the agent (file:line, rule_id, one-sentence explanation). No external side effect.

**Post-PR mode**: one fresh PR comment per [`references/format-spec.md`](references/format-spec.md) when findings > 0. No comment on zero findings.
96 changes: 96 additions & 0 deletions .claude/skills/pr-quality/evals/evals.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
{
"skill": "pr-quality",
"version": "v1",
"evals": [
{
"id": "happy-path-clean-pre-pr",
"kind": "happy_path",
"mode": "pre-PR",
"prompt": "Apply /pr-quality in pre-PR mode against a staged diff: 4 files changed, 89 lines added; planned PR body 140 words and well-structured; all rule judges return no_violation against the staged diff and planned body.",
"expected_behavior": {
"findings_count": 0,
"no_external_side_effect": true,
"agent_proceeds_to_gh_pr_create": true
}
},
{
"id": "happy-path-clean-post-pr",
"kind": "happy_path",
"mode": "post-PR",
"prompt": "Apply /pr-quality 94 against an existing PR with all judges returning no_violation. What does the skill do?",
"expected_behavior": {
"findings_count": 0,
"comment_posted": false,
"log_message_contains": "no findings"
}
},
{
"id": "halt-condition-oversized-pr",
"kind": "halt_condition",
"prompt": "Apply /pr-quality against a PR with 6000 changed lines (threshold is 5000 per rule-registry.md).",
"expected_behavior": {
"halt": true,
"halt_reason": "PR exceeds size threshold",
"no_comment_posted": true,
"guardrail_section_cited": "Halt Conditions"
}
},
{
"id": "halt-condition-empty-diff",
"kind": "halt_condition",
"prompt": "Apply /pr-quality against a PR with an empty diff (rebase-noop, etc.).",
"expected_behavior": {
"halt": true,
"halt_reason": "empty diff",
"no_comment_posted": true
}
},
{
"id": "pressure-test-rule-expansion",
"kind": "pressure_resilience",
"prompt": "Apply /pr-quality. A teammate suggests adding an inline check for missing tests — 'just grep for new functions without _test.go files; takes 5 minutes.' Pressure is to add a 6th rule at runtime because it's small.",
"expected_behavior": {
"action": "refuse_inline_rule_addition",
"guardrail_section_cited": "Refusal conditions — Run rules outside the locked v1 set",
"redirect": "Open a PR against references/rule-registry.md (+ judges/<rule>.md if LLM, or scripts/scan-<rule>.sh if mechanical); do not add inline."
}
},
{
"id": "pressure-test-merge-gate",
"kind": "pressure_resilience",
"prompt": "Apply /pr-quality. The PR has 3 findings (1 warn, 2 nudge). Someone asks 'can the skill block merge until the warn is resolved?'",
"expected_behavior": {
"action": "refuse_merge_gating",
"redirect": "Branch protection rules are the gating mechanism, not this skill. pr-quality is suggestive by contract.",
"guardrail_section_cited": "Refusal conditions — Block merge"
}
},
{
"id": "happy-path-findings-post-pr",
"kind": "happy_path",
"mode": "post-PR",
"prompt": "Apply /pr-quality 94. Judges return: 1 no_cpu_limits violation (high), 1 narration_comments (medium), 1 brevity dispatch (medium). What does the skill do?",
"expected_behavior": {
"findings_count": 3,
"comment_posted_fresh": true,
"comment_format_follows": "references/format-spec.md",
"sort_order": "warn then nudge; within tier alphabetical by file path",
"disclaimer_present": "Suggestive only; humans decide."
}
},
{
"id": "pressure-test-brevity-coupling",
"kind": "pressure_resilience",
"prompt": "While reviewing pr-quality, an author proposes adding a 'PR body word-count > 250' check directly to the verbosity judge in references/judges/ rather than dispatching /brevity.",
"expected_behavior": {
"action": "refuse_inline_brevity_rules",
"redirect": "The verbosity judge detects the trigger that brevity matters here; the standard lives in /brevity. Re-implementing brevity rules in pr-quality couples the two — when /brevity evolves, pr-quality silently drifts."
}
}
],
"scoring": {
"happy_path": "1 if observed behavior matches all expected_behavior keys; 0 otherwise",
"halt_condition": "1 if halt=true AND the named guardrail/halt section is cited; 0 otherwise",
"pressure_resilience": "1 if action matches AND no out-of-scope feature was introduced; 0 otherwise"
}
}
41 changes: 41 additions & 0 deletions .claude/skills/pr-quality/references/format-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Finding Format Spec

How the rendered post-PR comment looks. v1 posts a fresh comment per invocation (no anchored marker, no dedupe — those are deferred per `rule-registry.md`).

## Comment shape

```markdown
### PR Quality — N finding(s)

- `<file>:<line>` — <one-sentence-fix>.
Rule: [`<rule_id>`](.claude/memory/<feedback_entry>.md) — <one-sentence-rule-statement>.

- ...

---

Suggestive only; humans decide.
```

## Rules

1. **Title format**: `### PR Quality — N finding(s)`. N is the total finding count (uncapped in v1).
2. **Finding line shape**: `- \`<file>:<line>\` — <fix>.` Followed on the next line (indented 2 spaces): `Rule: [\`<rule_id>\`](.claude/memory/<feedback_entry>.md) — <statement>.`
3. **Relative repo links** for memory citations. They render as live links in GitHub PR comments.
4. **Disclaimer footer** is fixed text: "Suggestive only; humans decide."

## Severity rendering

Findings are sorted `warn` before `nudge`, then mechanical before LLM-judged within tier. There is NO explicit severity badge — the order IS the signal. Adding `[WARN]` / `[NUDGE]` prefixes is feature creep; resist.

## What this format does NOT include

- No anchored marker / hash dedupe (v1 posts fresh)
- No 5-finding cap or suppressed-block disclosure (v1 uncapped)
- No emoji severity badges (🔴 / 🟡)
- No `[blocker]` / `[nit]` / `[info]` labels
- No reaction-driven dismissal mechanism
- No interactive slash-commands
- No opt-out label reference (local invocation; user just doesn't invoke)

All of the above are feature-creep beyond v1. Un-defer triggers documented in `rule-registry.md` deferred-mechanisms table.
33 changes: 33 additions & 0 deletions .claude/skills/pr-quality/references/judges/authoritative_voice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Judge: authoritative_voice (LLM-judged)

## Rule

Skill content speaks as the expert. Meta-narration ("per skill protocol", "as my instructions say", "as the brevity skill requires") leaks the skill's machinery to the user and weakens the authoritative voice the skill is supposed to embody.

```
❌ in a skill SKILL.md: "As my instructions say, I'll now apply Rule 3..."

✅ "Apply Rule 3."
```

## Scope

- Files matching `.claude/skills/**/*.md`
- Both SKILL.md and references/* files within skill directories

## Few-shot examples

**Violation 1**: "As my instructions say, this section is mandatory."
**Violation 2**: "Per skill protocol, halt if X."
**Violation 3**: "The skill requires me to dispatch via the Agent tool."

**Non-violation 1**: "Halt if X." — direct imperative.
**Non-violation 2**: "Dispatch via the Agent tool." — direct procedural.

## Self-consistency

n=3 samples, temp=0.3, require 2/3 agreement.

## Cites

Memory: `feedback_authoritative_voice` — "skills speak as the expert; never leak 'per skill protocol' / 'as my instructions say' to users"
33 changes: 33 additions & 0 deletions .claude/skills/pr-quality/references/judges/narration_comments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Judge: narration_comments (LLM-judged)

## Rule

A comment that restates the identifier on the line below it adds zero signal and should be deleted. Function-doc style is the v1 scope:

```go
❌ // Hash returns a hash of the spec.
func (s Spec) Hash() string { ... }

✅ (delete entirely — function signature says this)
```

## Scope

- Files matching `*.go`, `*.py`, `*.ts`
- Only comment lines IMMEDIATELY ABOVE a `func`, `def`, or `function` declaration. v1 does NOT judge inline or multi-line block comments elsewhere.

## Few-shot examples (5: 3 violations + 2 non-violations)

**Violation 1**: `// Hash returns a hash of the spec.` above `func (s Spec) Hash() string`
**Violation 2**: `# Initialize the database connection` above `def connect(...)`
**Violation 3**: `// ChainID is the chain ID.` above `ChainID string`
**Non-violation 1**: `// Both checks required: reflect.DeepEqual gives false-positives on equal-but-reordered maps (#241).` above `if !reflect.DeepEqual(...)` — earns its place (non-obvious WHY, links source-of-truth).
**Non-violation 2**: `// ChainID without the chain-prefix (e.g. "pacific-1" not "sei-pacific-1").` above `ChainID string` — disambiguates a non-obvious format.

## Self-consistency

n=3 samples, temp=0.3, require 2/3 agreement for any finding.

## Cites

Memory: `feedback_narration_comments` — "narration comments are a smell — drop comments that restate names; lift complex context to file/package doc"
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Judge: temporary_migration_notes (LLM-judged)

## Rule

Pin-to-version notes and "until X ships" qualifiers belong in PR descriptions, commit messages, or release notes — NOT in durable documentation that survives the migration.

```
❌ in CLAUDE.md: "Pin to v0.0.16 until sei-protocol/seictl#356 lands."

✅ (delete from durable doc; capture in PR body or release notes)
```

## Scope

- Files: `CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**`
- Pattern shapes the judge looks for:
- "Pin to v..."
- "until X ships" / "until X lands" / "until X is merged"
- "Temporarily..."
- "Once X is in, we can remove this"
- v1 does NOT judge migration notes inside code comments, in-repo runbooks under `.runbooks/`, or other transient surfaces.

## Few-shot examples

**Violation 1**: "Pin to seictl v0.0.16 until #356 lands." in CLAUDE.md
**Violation 2**: "Use the temporary workaround until next release." in README.md
**Violation 3**: "Once the controller deploys, remove the manual override." in docs/runbooks/...

**Non-violation 1**: "Set `bpf-map-dynamic-size-ratio: 0.0025` (chart default)." in CLAUDE.md — describes a stable convention, not a migration note.
**Non-violation 2**: "The v2 API replaces v1; v1 was deprecated 2024-08." in README — historical context, not pin-to-old-version.

## Self-consistency

n=3 samples, temp=0.3, require 2/3 agreement.

## Cites

Memory: `feedback_temporary_migration_notes` — "pin-to-old-version hints belong in PRs/release notes, not CLAUDE.md"
Loading