diff --git a/README.md b/README.md index 01f6161..078d2f0 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ read at runtime from a small **project profile** you fill in during onboarding. | Layer | Contents | How it's delivered | |---|---|---| -| **Skills** (`skills/`) | `rig-doctor`, `rig-debug`, `rig-spike`, `rig-tidy`, `rig-issue`, `rig-worktree`, `rig-review` (`find`/`fix`), `rig-plan`, `rig-task`, `rig-sprint`, `rig-epic` | Copied into `/.claude/skills/` (or `.agents/skills/` for non-Claude agents — see "Works with your agent" below) | +| **Skills** (`skills/`) | `rig-doctor`, `rig-debug`, `rig-spike`, `rig-tidy`, `rig-issue`, `rig-worktree`, `rig-review` (`find`/`fix`), `rig-proof` (`find`/`fix`), `rig-plan`, `rig-task`, `rig-sprint`, `rig-epic` | Copied into `/.claude/skills/` (or `.agents/skills/` for non-Claude agents — see "Works with your agent" below) | | **Agents** (`agents/`) | `rig-debugger`, `rig-reviewer`, `rig-architect`, `rig-qa`, `rig-coder` | Copied into `/.claude/agents/` | | **pi adapter** (`pi/`) | Per-target persona frontmatter (`pi/agents/*.yml`, assembled onto the shared bodies) + the `/rig` dispatcher prompt | Copied into `/.pi/{agents,prompts}/` — see [`docs/pi.md`](docs/pi.md) | | **Support docs** (`templates/`) | starter `REVIEWER.md` (+ `REVIEWER.scope-template.md`), `STYLE.md`, `label-mapping.md` | Copied into `/.claude/` (only if absent) | diff --git a/docs/config.md b/docs/config.md index eb9dca5..2dd0ec4 100644 --- a/docs/config.md +++ b/docs/config.md @@ -64,7 +64,7 @@ Set `provider: "none"` to strip all ticket steps from `ticket`/`sprint`/review f ## `style` | Key | Default | Meaning | |---|---|---| -| `guideFile` | `.claude/STYLE.md` | The house style for prose agents write — PR bodies, tickets, review findings, plans, hand-backs. Rig ships a starter based on the [Google developer documentation style guide](https://developers.google.com/style). Every persona reads it before writing; the rules also hold inline if the file is missing. | +| `guideFile` | `.claude/STYLE.md` | The house style for prose agents write — PR bodies, tickets, review findings, plans, hand-backs. Rig ships a starter based on the [Google developer documentation style guide](https://developers.google.com/style). Every persona reads it before writing; the rules also hold inline if the file is missing. `rig-proof` walks it against a draft, and `scripts/check-style.ts` harvests its banned-term lists to grep for mechanically — so the guide is the only place style rules live. | ## `agents` Optional map from the kit's canonical role → the agent name registered in your diff --git a/install.sh b/install.sh index 2de6faa..6b7eaa0 100644 --- a/install.sh +++ b/install.sh @@ -65,7 +65,7 @@ if [[ "$TARGET" == "$RIG_DIR" ]]; then exit 2 fi -DEFAULT_SKILLS=(rig-doctor rig-debug rig-spike rig-tidy rig-review rig-issue rig-worktree rig-task rig-plan rig-sprint rig-epic) +DEFAULT_SKILLS=(rig-doctor rig-debug rig-spike rig-tidy rig-review rig-proof rig-issue rig-worktree rig-task rig-plan rig-sprint rig-epic) if [[ ${#SKILLS[@]} -eq 0 ]]; then SKILLS=("${DEFAULT_SKILLS[@]}") fi diff --git a/scripts/check-style.test.ts b/scripts/check-style.test.ts new file mode 100644 index 0000000..b63618d --- /dev/null +++ b/scripts/check-style.test.ts @@ -0,0 +1,307 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "bun:test"; +import { + bannedSide, + checkText, + findLongSentences, + findTermHits, + guideSections, + isTermLike, + maskCode, + maskNonProse, + normalizeQuotes, + parseGuide, + resolveGuidePath, + termRegExp, + type Term, +} from "./check-style.ts"; + +/** A miniature guide with the same shape as templates/STYLE.md. */ +const GUIDE = `# Writing style + +Preamble prose with a \`backticked\` word that is not a rule. + +## Rules + +### 7. Concrete nouns + +| Instead of | Write | +|---|---| +| \`leverage\`, \`utilize\` | \`use\` | +| \`robust\` | say what it does | + +### 8. Cut filler + +Delete words that survive their own removal: \`basically\`, \`in order to\`, +\`it's worth noting that\`. Delete hedge stacks too — \`it seems like it might be\` +is \`it might be\`, or better, go check. + +### 9. No jargon + +| Instead of | Write | +|---|---| +| \`blast radius\` | \`affected callers\` | + +Never call work \`easy\` or \`trivial\`. + +- **Do:** \`The handler calls getByToken.\` +- **Don't:** \`getByToken is called by the handler.\` +`; + +describe("isTermLike", () => { + it("accepts word-shaped runs", () => { + for (const t of ["basically", "blast radius", "low-hanging fruit", "it's worth noting that"]) { + expect(isTermLike(t)).toBe(true); + } + }); + it("rejects identifiers, flags, and example markup", () => { + for (const t of ["session.tenantId", "--filter", "click [here](…)", "tests: 412 pass", "api/invoices.ts:88"]) { + expect(isTermLike(t)).toBe(false); + } + }); + it("rejects runs too long to be a term", () => { + expect(isTermLike("a".repeat(40))).toBe(false); + }); +}); + +describe("guideSections", () => { + it("splits on numbered rule headings and drops the preamble", () => { + const rules = guideSections(GUIDE).map((s) => s.rule); + expect(rules).toEqual(["7. Concrete nouns", "8. Cut filler", "9. No jargon"]); + }); + it("keeps a rule's body with its heading", () => { + const filler = guideSections(GUIDE).find((s) => s.rule.startsWith("8."))!; + expect(filler.body).toContain("basically"); + expect(filler.body).not.toContain("blast radius"); + }); + it("returns nothing for a guide with no rule sections", () => { + expect(guideSections("# Title\n\nJust prose.\n")).toEqual([]); + }); +}); + +describe("bannedSide", () => { + it("takes the tail when the marker points forward", () => { + expect(bannedSide("Descriptive link text: `good`, never `bad`.")).toContain("`bad`"); + expect(bannedSide("Descriptive link text: `good`, never `bad`.")).not.toContain("`good`"); + }); + it("takes the head when the marker points backward", () => { + const s = "Delete hedge stacks — `it seems like it might be` is `it might be`."; + expect(bannedSide(s)).toContain("it seems like it might be"); + expect(bannedSide(s)).not.toContain("`it might be`"); + }); + it("keeps the whole sentence when there is no marker", () => { + const s = "Delete these: `basically`, `very`."; + expect(bannedSide(s)).toBe(s); + }); + it("honors whichever marker comes first", () => { + const s = "Write `a`, not `b` — `c` is `d`."; + // The forward marker appears first, so everything after it is in scope. + expect(bannedSide(s)).toContain("`b`"); + expect(bannedSide(s)).not.toContain("`a`"); + }); +}); + +describe("parseGuide", () => { + const terms = parseGuide(GUIDE); + const find = (t: string) => terms.find((x) => x.term === t); + + it("harvests table terms with their replacement", () => { + expect(find("leverage")).toEqual({ term: "leverage", suggestion: "use", rule: "7. Concrete nouns" }); + expect(find("utilize")?.suggestion).toBe("use"); + expect(find("robust")?.suggestion).toBe("say what it does"); + }); + it("harvests a prose list introduced by a cue verb", () => { + expect(find("basically")).toBeDefined(); + expect(find("in order to")).toBeDefined(); + expect(find("it's worth noting that")).toBeDefined(); + }); + it("harvests the bad half of a hedge sentence, not the recommended form", () => { + expect(find("it seems like it might be")).toBeDefined(); + expect(find("it might be")).toBeUndefined(); + }); + it("does not harvest the guide's own Do/Don't examples", () => { + expect(terms.some((t) => t.term.includes("getbytoken"))).toBe(false); + }); + it("attributes each term to the rule it came from", () => { + expect(find("blast radius")?.rule).toBe("9. No jargon"); + expect(find("easy")?.rule).toBe("9. No jargon"); + }); + it("ignores backticks in the preamble", () => { + expect(find("backticked")).toBeUndefined(); + }); + it("returns terms sorted and deduped", () => { + expect(terms.map((t) => t.term)).toEqual([...new Set(terms.map((t) => t.term))].sort()); + }); +}); + +describe("normalizeQuotes", () => { + it("folds curly apostrophes so a term still matches", () => { + expect(normalizeQuotes("it’s")).toBe("it's"); + }); +}); + +describe("maskCode", () => { + it("blanks fenced blocks, inline spans, and link targets", () => { + const masked = maskCode("a `leverage` b\n```\nleverage\n```\n[x](http://very-simple)"); + expect(masked).not.toContain("leverage"); + expect(masked).not.toContain("very-simple"); + }); + it("preserves length and line structure so offsets still hold", () => { + const src = "a `bb` c\n```\nx\n```\nd"; + expect(maskCode(src).length).toBe(src.length); + expect(maskCode(src).split("\n").length).toBe(src.split("\n").length); + }); +}); + +describe("maskNonProse", () => { + it("blanks headings, table rows, and blockquotes", () => { + const masked = maskNonProse("## robust\n| robust |\n> robust\nrobust"); + expect(masked.split("\n").filter((l) => l.includes("robust"))).toEqual(["robust"]); + }); + it("blanks YAML frontmatter, which is metadata rather than prose", () => { + const src = "---\nname: x\ndescription: robust robust robust\n---\n\nrobust\n"; + const masked = maskNonProse(src); + expect(masked.split("\n").filter((l) => l.includes("robust"))).toEqual(["robust"]); + }); + it("only blanks frontmatter at the very top of the file", () => { + const src = "text\n\n---\ndescription: robust\n---\n"; + expect(maskNonProse(src)).toContain("robust"); + }); + it("preserves length", () => { + const src = "## h\n| a |\ntext"; + expect(maskNonProse(src).length).toBe(src.length); + const fm = "---\na: b\n---\nc"; + expect(maskNonProse(fm).length).toBe(fm.length); + }); +}); + +describe("termRegExp", () => { + it("matches common inflections of a single word", () => { + for (const form of ["leverage", "leverages", "leveraged", "leveraging"]) { + expect(form.match(termRegExp("leverage"))).not.toBeNull(); + } + }); + it("does not match a term embedded in a longer word", () => { + expect("adjust the readjustment".match(termRegExp("just"))).toBeNull(); + }); + it("matches a multi-word term that wrapped across lines", () => { + expect("the blast\nradius grew".match(termRegExp("blast radius"))).not.toBeNull(); + }); +}); + +describe("findTermHits", () => { + const terms: Term[] = [ + { term: "basically", rule: "8. Cut filler" }, + { term: "leverage", suggestion: "use", rule: "7. Concrete nouns" }, + ]; + + it("reports line, column, and the guide's replacement", () => { + const hits = findTermHits("ok line\nwe basically leverage it\n", terms); + expect(hits.map((h) => [h.line, h.column, h.match])).toEqual([ + [2, 4, "basically"], + [2, 14, "leverage"], + ]); + expect(hits[1].message).toBe('"leverage" — write use'); + expect(hits[0].message).toBe('cut "basically"'); + }); + it("ignores hits inside code", () => { + expect(findTermHits("`basically` and\n```\nleverage\n```\n", terms)).toEqual([]); + }); + it("flattens a wrapped match into one line of output", () => { + const hits = findTermHits("we\nbasically ship", terms); + expect(hits[0].match).toBe("basically"); + }); +}); + +describe("findLongSentences", () => { + const words = (n: number) => Array.from({ length: n }, (_, i) => `w${i}`).join(" "); + + it("catches a sentence that wraps across lines", () => { + const text = `${words(20)}\n${words(20)}.`; + expect(findLongSentences(text, 30)).toHaveLength(1); + expect(findLongSentences(text, 30)[0].match).toBe("40 words"); + }); + it("leaves a sentence at the limit alone", () => { + expect(findLongSentences(`${words(30)}.`, 30)).toEqual([]); + }); + it("treats a list marker as a sentence break even with no period before it", () => { + expect(findLongSentences(`- ${words(20)}\n- ${words(20)}\n`, 30)).toEqual([]); + }); + it("treats a blank line as a sentence break", () => { + expect(findLongSentences(`${words(20)}\n\n${words(20)}\n`, 30)).toEqual([]); + }); + it("does not flag a wide table row or a long heading", () => { + expect(findLongSentences(`| ${words(60)} |\n`, 30)).toEqual([]); + expect(findLongSentences(`## ${words(60)}\n`, 30)).toEqual([]); + }); + it("is off at 0", () => { + expect(findLongSentences(`${words(200)}.`, 0)).toEqual([]); + }); +}); + +describe("checkText", () => { + it("returns both passes ordered by position", () => { + const terms: Term[] = [{ term: "basically", rule: "8. Cut filler" }]; + const long = Array.from({ length: 40 }, (_, i) => `w${i}`).join(" "); + const found = checkText(`${long}.\nwe basically ship.\n`, terms, 30); + expect(found.map((f) => f.line)).toEqual([1, 2]); + }); +}); + +describe("resolveGuidePath", () => { + const exists = (paths: string[]) => (p: string) => paths.includes(p); + + it("prefers an explicit override", () => { + expect(resolveGuidePath("docs/S.md", () => undefined, exists(["docs/S.md", ".claude/STYLE.md"]))).toBe("docs/S.md"); + }); + it("resolves to nothing when the override is missing, rather than checking a different guide", () => { + expect(resolveGuidePath("nope.md", () => undefined, exists([".claude/STYLE.md"]))).toBeUndefined(); + }); + it("reads style.guideFile from the profile", () => { + const cfg = () => JSON.stringify({ style: { guideFile: "docs/house.md" } }); + expect(resolveGuidePath(undefined, cfg, exists(["docs/house.md"]))).toBe("docs/house.md"); + }); + it("falls back to the conventional locations when the profile points nowhere", () => { + const cfg = () => JSON.stringify({ style: { guideFile: "gone.md" } }); + expect(resolveGuidePath(undefined, cfg, exists([".rig/STYLE.md"]))).toBe(".rig/STYLE.md"); + }); + it("prefers .claude/ over .rig/ when both exist", () => { + expect(resolveGuidePath(undefined, () => undefined, exists([".claude/STYLE.md", ".rig/STYLE.md"]))).toBe( + ".claude/STYLE.md", + ); + }); + it("survives a malformed profile", () => { + expect(resolveGuidePath(undefined, () => "{not json", exists([".claude/STYLE.md"]))).toBe(".claude/STYLE.md"); + }); + it("resolves to nothing when there is no guide anywhere", () => { + expect(resolveGuidePath(undefined, () => undefined, exists([]))).toBeUndefined(); + }); +}); + +// Guards the shipped guide against a restructure that silently breaks harvesting: +// the script's whole contract is that its rules come from STYLE.md, so a parse +// that quietly yields nothing would look exactly like clean prose. +describe("the shipped templates/STYLE.md", () => { + const shipped = readFileSync(join(import.meta.dir, "..", "templates", "STYLE.md"), "utf8"); + const terms = parseGuide(shipped); + + it("still yields a substantial term list", () => { + expect(terms.length).toBeGreaterThan(20); + }); + it("covers the filler, vagueness, and jargon rules", () => { + const byRule = new Set(terms.map((t) => t.rule.replace(/\..*/, ""))); + expect(byRule.has("7")).toBe(true); + expect(byRule.has("8")).toBe(true); + expect(byRule.has("9")).toBe(true); + }); + it("harvests the terms the personas name explicitly", () => { + const names = terms.map((t) => t.term); + for (const t of ["basically", "simply", "just", "leverage", "functionality", "blast radius", "low-hanging fruit"]) { + expect(names).toContain(t); + } + }); + it("is itself clean against its own mechanical rules", () => { + expect(checkText(shipped, terms, 30)).toEqual([]); + }); +}); diff --git a/scripts/check-style.ts b/scripts/check-style.ts new file mode 100755 index 0000000..75c9e19 --- /dev/null +++ b/scripts/check-style.ts @@ -0,0 +1,437 @@ +#!/usr/bin/env bun +/** + * check-style.ts — the deterministic half of the rig-proof skill: find the + * writing-style violations that don't need a model. + * + * Rig's writing-style guide (`style.guideFile` in .rig/config.json, default + * `.claude/STYLE.md`) is the single source of truth for how agents write prose. + * A useful share of it is mechanically checkable: the banned filler and jargon + * terms live in the guide as backticked words, either in `Instead of | Write` + * tables or in sentences that tell you to cut something. This script HARVESTS + * THOSE TERMS FROM THE GUIDE ITSELF and greps the target text for them, so it + * can never drift from what the personas were told. Prune a rule from the + * guide and this stops enforcing it; add a row to a table and it starts. + * + * It carries no style opinions of its own. The only judgment baked in here is + * sentence length (guide rule "one idea per sentence"), and that threshold is + * a flag. + * + * Everything else — buried conclusions, passive voice, hedge stacks, claims + * with no file:line behind them — needs a reader, and is the model's half of + * the rig-proof pass. Running this first means the model spends its attention + * on judgment instead of on word-spotting. + * + * Usage: + * check-style.ts [ ...] [--json] [--guide ] + * check-style.ts --stdin [--json] [--guide ] (text on stdin) + * check-style.ts --terms [--json] (dump what it loaded) + * + * --guide override the guide (default: style.guideFile, else + * .claude/STYLE.md, else .rig/STYLE.md) + * --max-sentence flag sentences longer than n words (default 30; 0 off) + * --strict exit 1 when there are findings (default: always 0 — + * the rig-proof skill is the gate, not this script) + * + * Code is never flagged: fenced blocks, inline code spans (including ones that + * wrap across lines), link targets, and YAML frontmatter are masked before + * matching, so `leverage` in a snippet or a URL stays quiet. Expects prose — + * point it at Markdown or piped text, not at a source file. + */ +import { existsSync, readFileSync } from "node:fs"; + +/** A banned term harvested from the guide, with where it came from. */ +export interface Term { + /** The literal term to look for, lowercased. */ + term: string; + /** What the guide says to write instead, when it offers a replacement. */ + suggestion?: string; + /** The guide rule this came from, e.g. "8. Cut filler". */ + rule: string; +} + +export interface Finding { + line: number; + column: number; + /** The text as it appears in the target. */ + match: string; + rule: string; + message: string; +} + +/** + * Curly quotes and dashes break literal matching: a guide that says + * `it's worth noting that` must still match prose written with U+2019. Fold + * both sides to ASCII before comparing. + */ +export function normalizeQuotes(s: string): string { + return s.replace(/[\u2018\u2019\u02BC]/g, "'").replace(/[\u201C\u201D]/g, '"'); +} + +/** + * Is this harvested string actually a TERM, rather than an example sentence or + * a code identifier that happened to be backticked? + * + * The guide backticks a lot of things — flags (`--filter`), identifiers + * (`session.tenantId`), whole example sentences, link markup. Only word-shaped + * runs are terms worth grepping for. This filter is what keeps the harvest + * clean without the guide needing a machine-readable annex. + */ +export function isTermLike(s: string): boolean { + return /^[a-z][a-z' -]{1,34}$/i.test(s) && !/\s{2,}/.test(s); +} + +/** Every backticked span in a string, in order. */ +function backticked(s: string): string[] { + return [...s.matchAll(/`([^`]+)`/g)].map((m) => m[1].trim()); +} + +/** + * The guide's rule sections, keyed by their heading label ("8. Cut filler"). + * Rules live under `### . `; anything before the first one is + * preamble and carries no terms. + */ +export function guideSections(guide: string): { rule: string; body: string }[] { + const out: { rule: string; body: string }[] = []; + const re = /^###\s+(\d+\.\s*.+?)\s*$/gm; + const heads = [...guide.matchAll(re)]; + for (let i = 0; i < heads.length; i++) { + const start = heads[i].index! + heads[i][0].length; + const end = i + 1 < heads.length ? heads[i + 1].index! : guide.length; + out.push({ rule: heads[i][1].trim(), body: guide.slice(start, end) }); + } + return out; +} + +/** Sentences that mark an example rather than state a rule. */ +const EXAMPLE_MARKER = /\*\*(Do|Don't)/i; +/** Verbs that introduce something the guide is telling you not to write. */ +const CUE = /\b(delete|cut|no|never|avoid|skip|omit|replace)\b/i; +/** + * A cue sentence usually names the bad form AND the good one, so harvesting + * every backtick in it would ban the guide's own recommendation. Two markers + * separate them, pointing opposite ways: + * + * "…, never `click [here]`" → bad form FOLLOWS + * "`it seems like it might be` is `it might be`" → bad form PRECEDES + * + * Take the first marker that appears and keep the side it points at. + */ +const BAD_FOLLOWS = /,\s*(?:not|never|rather than)\s+/i; +const BAD_PRECEDES = /\s+(?:is|becomes)\s+/i; + +/** The slice of a cue sentence that holds the banned form. */ +export function bannedSide(sentence: string): string { + const after = sentence.match(BAD_FOLLOWS); + const before = sentence.match(BAD_PRECEDES); + if (after && (!before || after.index! < before.index!)) return sentence.slice(after.index! + after[0].length); + if (before) return sentence.slice(0, before.index!); + return sentence; +} + +/** Terms from the `Instead of | Write` tables in a rule section. */ +function tableTerms(body: string, rule: string): Term[] { + const out: Term[] = []; + for (const line of body.split("\n")) { + const t = line.trim(); + if (!t.startsWith("|") || /^\|[\s|:-]+\|?$/.test(t)) continue; + const cells = t.replace(/^\|/, "").replace(/\|$/, "").split("|"); + if (cells.length < 2) continue; + const suggestion = cells[1].trim().replace(/`/g, "") || undefined; + for (const term of backticked(cells[0])) { + if (isTermLike(term)) out.push({ term: term.toLowerCase(), suggestion, rule }); + } + } + return out; +} + +/** + * Terms from prose that tells you to cut something ("Delete words that survive + * their own removal: `basically`, `essentially`, …"). + */ +function proseTerms(body: string, rule: string): Term[] { + const out: Term[] = []; + // Drop table rows (handled above) and flatten wrapped lines so a sentence + // split across three lines is still one sentence. + const flat = body + .split("\n") + .filter((l) => !l.trim().startsWith("|")) + .join(" "); + // Split on sentence-final punctuation only. NOT on ":" — the guide's filler + // list reads "Delete words that survive their own removal: `basically`, …", + // and splitting at the colon strands the list from the cue verb that bans it. + for (const raw of flat.split(/(?<=[.!?])\s+/)) { + const sentence = raw.trim(); + if (!sentence || EXAMPLE_MARKER.test(sentence) || !CUE.test(sentence)) continue; + for (const term of backticked(bannedSide(sentence))) { + if (isTermLike(term)) out.push({ term: term.toLowerCase(), rule }); + } + } + return out; +} + +/** + * Every banned term the guide defines, deduped (first mention wins, so a term + * with a replacement beats a bare mention of the same word). + */ +export function parseGuide(guide: string): Term[] { + const seen = new Map<string, Term>(); + for (const { rule, body } of guideSections(normalizeQuotes(guide))) { + for (const t of [...tableTerms(body, rule), ...proseTerms(body, rule)]) { + const existing = seen.get(t.term); + if (!existing) seen.set(t.term, t); + else if (!existing.suggestion && t.suggestion) seen.set(t.term, t); + } + } + return [...seen.values()].sort((a, b) => a.term.localeCompare(b.term)); +} + +/** + * Blank out code and link targets, preserving every character position so + * line/column arithmetic on the result still describes the original. + */ +export function maskCode(text: string): string { + const blank = (m: string) => m.replace(/[^\n]/g, " "); + return text + .replace(/```[\s\S]*?```/g, blank) // fenced blocks + .replace(/~~~[\s\S]*?~~~/g, blank) + // Inline spans, INCLUDING ones that wrap across lines — a quoted example + // sentence is routinely broken over three lines, and leaving it unmasked + // reads it as the author's own prose. Bounded so one stray backtick can't + // swallow the rest of the document. + .replace(/`[^`]{1,400}`/g, blank) + .replace(/\]\([^)\n]*\)/g, blank); // link targets +} + +/** Line and 1-based column of an absolute offset. */ +function position(text: string, offset: number): { line: number; column: number } { + const before = text.slice(0, offset); + const line = before.split("\n").length; + const column = offset - (before.lastIndexOf("\n") + 1) + 1; + return { line, column }; +} + +/** + * A term becomes a whitespace-tolerant, word-bounded, case-insensitive regex. + * + * Whitespace is flexible so a term that wrapped across two lines still matches, + * and a short suffix is allowed so the guide banning `leverage` also catches + * "leveraged" and "leveraging" without the guide having to list every form. + */ +export function termRegExp(term: string): RegExp { + const escape = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&").replace(/[\s-]+/g, "[\\s-]+"); + const lead = /^[a-z]/i.test(term) ? "\\b" : ""; + if (!/[a-z]$/i.test(term)) return new RegExp(`${lead}${escape(term)}`, "gi"); + + // English drops the final "e" and turns "y" into "i" before a suffix, so a + // literal stem+suffix would miss "leveraging" and "easily". Both branches + // require one alternative, and the original spelling is among them. + let body: string; + if (/e$/i.test(term)) body = `${escape(term.slice(0, -1))}(?:e|es|ed|ing|ely)`; + else if (/y$/i.test(term)) body = `${escape(term.slice(0, -1))}(?:y|ies|ied|ily)`; + else body = `${escape(term)}(?:s|es|d|ed|ing|ly)?`; + return new RegExp(`${lead}${body}\\b`, "gi"); +} + +/** Every banned-term hit in the text. */ +export function findTermHits(text: string, terms: Term[]): Finding[] { + const masked = maskCode(normalizeQuotes(text)); + const out: Finding[] = []; + for (const t of terms) { + for (const m of masked.matchAll(termRegExp(t.term))) { + const { line, column } = position(masked, m.index!); + const flat = m[0].replace(/\s+/g, " "); + out.push({ + line, + column, + match: m[0].replace(/\s+/g, " "), + rule: t.rule, + message: t.suggestion ? `"${flat}" — write ${t.suggestion}` : `cut "${flat}"`, + }); + } + } + return out; +} + +/** + * Blank out lines that are labels rather than prose: headings, table rows, + * blockquote markers. A 12-column table row is not a run-on sentence, and a + * heading has no verb to be long-winded with. Length-preserving, like maskCode. + */ +export function maskNonProse(text: string): string { + // YAML frontmatter is metadata. A skill's `description` is a deliberate + // keyword-and-trigger list, not a sentence, and reading it as prose flags + // every well-written skill file. + const withoutFrontmatter = text.replace(/^---\n[\s\S]*?\n---(?=\n|$)/, (m) => + m.replace(/[^\n]/g, " "), + ); + return withoutFrontmatter + .split("\n") + .map((line) => (/^\s*(#{1,6}\s|\||>)/.test(line) ? line.replace(/[^\n]/g, " ") : line)) + .join("\n"); +} + +/** Split on a separator while keeping each piece's absolute offset. */ +function splitWithOffsets(text: string, sep: RegExp): { text: string; offset: number }[] { + const re = new RegExp(sep.source, sep.flags.includes("g") ? sep.flags : `${sep.flags}g`); + const out: { text: string; offset: number }[] = []; + let last = 0; + let m: RegExpExecArray | null; + while ((m = re.exec(text)) !== null) { + out.push({ text: text.slice(last, m.index), offset: last }); + last = m.index + m[0].length; + if (m[0].length === 0) re.lastIndex++; + } + out.push({ text: text.slice(last), offset: last }); + return out; +} + +/** + * Where one sentence ends and the next begins: + * + * 1. terminal punctuation followed by something that can start a sentence + * (the capital-letter lookahead keeps "e.g. foo" and "1.5" intact), + * 2. a blank line, so a paragraph with no period can't swallow the next one, + * 3. a newline into a list marker — a bullet is its own sentence even when + * the bullet before it never reached a period. + */ +const SENTENCE_BREAK = /(?<=[.!?])[ \t]*\n?[ \t]*(?=[A-Z"`(\[]|$)|\n[ \t]*\n|\n(?=[ \t]*(?:[-*+]\s|\d+[.)]\s))/; + +/** + * Sentences over `max` words (guide rule: one idea per sentence). + * + * Sentences wrap across lines, so this scans the whole text rather than line by + * line — a 55-word sentence spread over four lines is exactly the case worth + * catching. A blank line ends a sentence too, so a paragraph that never reaches + * a period doesn't swallow the one after it. + */ +export function findLongSentences(text: string, max: number): Finding[] { + if (max <= 0) return []; + const masked = maskNonProse(maskCode(normalizeQuotes(text))); + const out: Finding[] = []; + for (const s of splitWithOffsets(masked, SENTENCE_BREAK)) { + const words = s.text.trim().split(/\s+/).filter(Boolean); + if (words.length <= max) continue; + // Point at the first word, not at the leading whitespace. + const lead = s.text.length - s.text.replace(/^\s+/, "").length; + const { line, column } = position(masked, s.offset + lead); + out.push({ + line, + column, + match: `${words.length} words`, + rule: "one idea per sentence", + message: `${words.length}-word sentence — split it (limit ${max})`, + }); + } + return out; +} + +/** Both mechanical passes, ordered by position. */ +export function checkText(text: string, terms: Term[], maxSentence: number): Finding[] { + return [...findTermHits(text, terms), ...findLongSentences(text, maxSentence)].sort( + (a, b) => a.line - b.line || a.column - b.column, + ); +} + +/** + * Where the guide lives: an explicit override, then `style.guideFile` from the + * project profile, then the two locations rig installs to. + */ +export function resolveGuidePath( + override: string | undefined, + readConfig: () => string | undefined, + exists: (p: string) => boolean, +): string | undefined { + // An override that isn't there resolves to nothing rather than falling back: + // silently checking a different guide than the one you named is worse than + // saying you couldn't find it. + if (override) return exists(override) ? override : undefined; + const raw = readConfig(); + if (raw) { + try { + const configured = JSON.parse(raw)?.style?.guideFile; + if (typeof configured === "string" && exists(configured)) return configured; + } catch { + // A malformed profile is not this script's problem — fall through to the + // conventional locations rather than failing the whole pass. + } + } + return [".claude/STYLE.md", ".rig/STYLE.md"].find(exists); +} + +function main(): void { + const args = process.argv.slice(2); + const json = args.includes("--json"); + const strict = args.includes("--strict"); + const wantTerms = args.includes("--terms"); + const useStdin = args.includes("--stdin"); + const guideIdx = args.indexOf("--guide"); + const maxIdx = args.indexOf("--max-sentence"); + const maxSentence = maxIdx >= 0 ? Number(args[maxIdx + 1]) : 30; + // Consume flag values BY POSITION. Matching on value would swallow a target + // that happens to be the same path as --guide (checking the guide itself). + const consumed = new Set([guideIdx, maxIdx].filter((i) => i >= 0).map((i) => i + 1)); + const files = args.filter((a, i) => !a.startsWith("--") && !consumed.has(i)); + + const guidePath = resolveGuidePath( + guideIdx >= 0 ? args[guideIdx + 1] : undefined, + () => (existsSync(".rig/config.json") ? readFileSync(".rig/config.json", "utf8") : undefined), + existsSync, + ); + if (!guidePath) { + const msg = + guideIdx >= 0 + ? `guide not found: ${args[guideIdx + 1]}` + : "no writing-style guide found — set style.guideFile, or pass --guide <path>"; + if (json) console.log(JSON.stringify({ error: msg, terms: 0, findings: [] }, null, 2)); + else console.log(msg); + return; + } + + const terms = parseGuide(readFileSync(guidePath, "utf8")); + + if (wantTerms) { + if (json) console.log(JSON.stringify({ guide: guidePath, terms }, null, 2)); + else { + console.log(`# ${terms.length} term(s) harvested from ${guidePath}\n`); + for (const t of terms) console.log(` ${t.term}${t.suggestion ? ` → ${t.suggestion}` : ""} [${t.rule}]`); + } + return; + } + + const targets: { name: string; text: string }[] = useStdin + ? [{ name: "(stdin)", text: readFileSync(0, "utf8") }] + : files.filter(existsSync).map((f) => ({ name: f, text: readFileSync(f, "utf8") })); + + if (!targets.length) { + const msg = "nothing to check — pass one or more files, or --stdin"; + if (json) console.log(JSON.stringify({ guide: guidePath, terms: terms.length, error: msg, findings: [] }, null, 2)); + else console.log(msg); + return; + } + + const results = targets.map((t) => ({ file: t.name, findings: checkText(t.text, terms, maxSentence) })); + const total = results.reduce((n, r) => n + r.findings.length, 0); + + if (json) { + console.log(JSON.stringify({ guide: guidePath, terms: terms.length, total, files: results }, null, 2)); + } else if (!terms.length) { + // Loud, because a silent zero-term parse looks exactly like clean prose. + console.log(`⚠ ${guidePath} yielded 0 terms — the guide's rule sections may have been restructured.`); + } else if (!total) { + console.log(`✓ mechanical style pass clean (${terms.length} terms from ${guidePath})`); + } else { + console.log(`${total} mechanical style finding(s) — ${terms.length} terms from ${guidePath}\n`); + for (const r of results) { + if (!r.findings.length) continue; + console.log(`----- ${r.file} -----`); + for (const f of r.findings) console.log(` ${f.line}:${f.column} ${f.message} [${f.rule}]`); + console.log(""); + } + console.log("These are the cheap ones. The rig-proof model pass covers buried"); + console.log("conclusions, passive voice, hedging, and unanchored claims."); + } + + if (strict && total > 0) process.exit(1); +} + +if (import.meta.main) main(); diff --git a/skills/rig-issue/SKILL.md b/skills/rig-issue/SKILL.md index 74e4365..cbf62f6 100644 --- a/skills/rig-issue/SKILL.md +++ b/skills/rig-issue/SKILL.md @@ -87,7 +87,9 @@ filed it. Write the title as an imperative outcome — `Add rate limiting to /invoices`, not `Rate limiting`. Write the body to `style.guideFile` (default `.claude/STYLE.md`): goal first, then acceptance criteria as a checklist someone can verify item by item, then the files to touch and the ordered steps. One idea -per sentence, active voice, present tense, no filler. +per sentence, active voice, present tense, no filler. Before filing a +description longer than a couple of sentences, run `/rig-proof find` on the +draft and apply what it returns. ### `move <id> <status>` diff --git a/skills/rig-proof/SKILL.md b/skills/rig-proof/SKILL.md new file mode 100644 index 0000000..08e8a0e --- /dev/null +++ b/skills/rig-proof/SKILL.md @@ -0,0 +1,172 @@ +--- +name: rig-proof +description: "Proofread agent-written prose against the project's writing-style guide. `find` (default): flag buried conclusions, passive voice, hedging, filler, jargon, and unanchored claims in a PR body, ticket, review finding, plan, writeup, or changed Markdown — read-only. `fix`: rewrite it in place. Triggers on: 'proofread', 'check the writing', 'is this readable', 'tighten this up', 'check style', 'style check', 'review the PR body', 'clean up this ticket', 'make this clearer', 'rewrite this so it reads well'." +argument-hint: "[find | fix] [<file> | <PR> | --stdin | --base <ref>] — default 'find' (read-only); 'fix' rewrites" +allowed-tools: [Bash, Read, Edit, Grep, Glob] +--- + +# rig-proof — proofread what an agent wrote + +Two verbs over one source of truth: + +- **`find`** (default) — check prose against the project's writing-style guide + and return findings with line references. **Read-only.** +- **`fix`** — apply them, then re-check. + +**The guide is the only rulebook.** Its path is `style.guideFile` in +`.rig/config.json` (default `.claude/STYLE.md`). This skill has no style +opinions of its own: read the guide, walk its rules, cite them by number. When a +project prunes or extends the guide, this skill follows automatically — the same +relationship `/rig-review` has to `REVIEWER.md`. + +## Why this exists + +The personas already tell agents how to write. Instructions decay under load: an +agent 40 tool-calls deep, writing the PR body last, has spent its attention +elsewhere. This is the gate that catches what the instruction missed — the same +reason `/rig-task` runs a pre-PR self-review instead of trusting that the coder +internalized the review catalog. + +## Configuration + +Reads `.rig/config.json`: + +- `style.guideFile` — the writing-style guide, and the only rule source + (default `.claude/STYLE.md`, then `.rig/STYLE.md`). **If no guide is found, + say so and stop** — don't substitute your own preferences. +- `vcs.baseRef` — diff base when checking changed Markdown (default + `origin/main`). +- `project.repo` — `owner/name` for `gh` calls when the target is a PR. + +## Scope — what to check, and what to leave alone + +Default to **one named target**. This skill is a proofreader, not a repo-wide +linter; pointing it at every document in the tree produces a finding pile nobody +asked for. + +Resolve `$ARGUMENTS` to a target in this order: + +| Argument | Target | +|---|---| +| a file path | that file | +| `--stdin`, or prose pasted into the request | that text | +| a PR number, or `pr` | the PR body (`gh pr view <n> --json body -q .body`) | +| an issue/ticket ID | that ticket's description | +| `--base <ref>`, or `diff` | Markdown files changed vs `<ref>` | +| nothing | ask what to check — don't guess, and don't default to the repo | + +**Never check code.** Source files are out of scope: identifiers and comments +follow the conventions of the code around them, not a prose guide. Skip +generated files, vendored directories, and `CHANGELOG.md`. + +**Only sweep the whole repo when the user explicitly asks for it.** Report +per-file counts first and let them pick where to start. Don't dump every +finding at once. + +--- +# `find` — the read-only pass + +1. **Read the guide.** Resolve `style.guideFile`, read it, and keep its rule + numbers — every finding cites one. + +2. **Run the mechanical pass first.** It costs nothing and it removes the + word-spotting work from your plate: + + ```bash + <SCRIPT> --json <target-file> # or: … --stdin (text on stdin) + ``` + + where `<SCRIPT>` = `.claude/scripts/check-style.ts` if present, else + `.rig/scripts/check-style.ts`, else `<RIG_DIR>/scripts/check-style.ts` (run + with `bun`). It harvests the banned terms **from the guide itself** and greps + for them, plus flags over-long sentences. Code, fenced blocks, and link + targets are masked, so it never flags a snippet. + + Its findings are **candidates, not verdicts.** `just`, `simple`, and + `obvious` have legitimate uses; a 34-word sentence can be the clearest way to + say something. Triage each one — you're the judgment the script doesn't have. + Drop a candidate that reads fine and say nothing about it. + + If the script is missing or reports 0 harvested terms, do the whole pass + yourself and note that the mechanical half didn't run. + +3. **Read the prose yourself** for what no grep catches. This is the half that + matters: + + - **Buried conclusion.** Does the first sentence carry the answer, or does + the reader wade through process narration to reach it? This is the most + common defect and the most expensive. + - **Passive voice** where the actor matters — especially in a sentence the + reader has to act on. + - **Hedge stacks.** `it seems like it might possibly` — either go check, or + say plainly that you didn't. + - **Unanchored claims.** An assertion about the code with no `file:line`, + command, count, or SHA behind it. + - **Unmarked guesses.** Something inferred, presented as observed. + - **Structure.** Three-plus parallel items still in a paragraph; a + comparison that wants a table; ordered steps in prose. + - **Self-narration** — preamble, apology, closing offer of further help. + - **Untestable acceptance criteria**, when the target is a ticket. + - Anything else the guide's rules call for that the script can't see. + +4. **Report.** Lead with the verdict, then findings in document order. One entry + per finding, each citing the guide rule and the line: + + ``` + 3 findings — 2 that change how it reads, 1 nit + + 12:1 [rule 1: answer first] The verdict is in the last paragraph. Move + "the filter breaks the webhook path" to the first sentence. + 18:34 [rule 3: active voice] "the lookup should be scoped" — say who + scopes it. + 24:7 [rule 8: cut filler] "it's worth noting that" — delete. + ``` + + Rank by how much each one costs a reader: a buried conclusion outranks a + filler word. If the prose is clean, say so in one line and stop — don't pad + the report to look thorough. Then offer `fix`. + +--- +# `fix` — apply the findings + +1. **Get findings.** Use the caller's `find` output if passed; else run `find`. +2. **Rewrite.** Apply the changes to the target, smallest edit that fixes each + finding. + - **Preserve meaning exactly.** Rewriting prose must not change a claim, a + number, a file path, a severity, or a conclusion. If a sentence is unclear + because the *underlying fact* is unclear, that's not a writing problem — + report it and leave the sentence alone. + - **Never touch code, code spans, or fenced blocks.** Not the identifiers in + them, not the commands. + - Keep the artifact's required structure. A PR body still needs its tracker + link and `## Architecture` note; a ticket still needs its acceptance + criteria. +3. **Re-check.** Run `find` again. Report what changed and what you left, with + the reason for each thing you left. +4. **Show the diff for a durable artifact.** A local file you may edit + directly. When the target is already published — a PR body, a filed ticket — + show the rewrite and get a yes first, then push it (`gh pr edit --body`, + `gh issue edit --body`, Linear `save_issue`). + +## Calling it from another skill + +`find` is cheap and read-only, so the flows run it **before** the artifact +lands, not after: + +- **`/rig-task` Step 5** — on the PR body, before `gh pr create`. +- **`/rig-issue create`** — on the ticket body, before filing. +- **`/rig-spike`** — on the writeup, before posting it back to the ticket. + +A caller may thread `{target}` or pipe the draft text in on stdin. Return the +finding list, and the rewritten text when called with `fix`. + +## Notes + +- **Read-only by default.** `find` reports; only `fix` edits. +- **Degrades:** no guide → say so and stop. No `check-style.ts` → model-only + pass. Neither is a hard failure. +- **Not a bug hunter.** Wrong claims are `/rig-review`'s job; this checks how + the writing reads, not whether it's true. If you notice a false claim while + proofreading, say so — but don't go looking. +- **Don't proofread the same artifact twice.** If `find` came back clean once + and the text hasn't changed, there's nothing to add. diff --git a/skills/rig-spike/SKILL.md b/skills/rig-spike/SKILL.md index d48d41c..dabf227 100644 --- a/skills/rig-spike/SKILL.md +++ b/skills/rig-spike/SKILL.md @@ -150,6 +150,10 @@ into logging"). Confirm the time-box before starting. - **Open questions** — anything still unknown that a later spike or the implementation would need to resolve. + Proof it before it goes anywhere: run `/rig-proof find` on the draft and + apply what comes back. The writeup is the deliverable, and it outlives the + spike. + If a tracking ticket was created in step 2, post the writeup back to it (`mcp__claude_ai_Linear__save_comment` for Linear, `gh issue comment` for GitHub). Surface it in chat regardless. diff --git a/skills/rig-task/SKILL.md b/skills/rig-task/SKILL.md index a79be16..0a8ad36 100644 --- a/skills/rig-task/SKILL.md +++ b/skills/rig-task/SKILL.md @@ -241,6 +241,9 @@ don't pay a round-trip on. Delegate so the gate lives in one place: in the imperative under ~72 characters, PR summary that answers *what changed and why* in its first sentence, a test plan another person could run. Don't narrate the diff — the diff is right there. + - **Proof the draft body before creating the PR** — `/rig-proof find` on it + (pipe the draft in on stdin) and apply what comes back. Every reviewer reads + this body; rewriting it after it's posted notifies all of them again. 4. **Link the PR + move to In Review — adaptively** (works with *or* without a live tracker↔GitHub integration; treat `tracker.githubIntegration` as a hint, not a gate — it may claim `true` while nothing is actually connected): diff --git a/templates/STYLE.md b/templates/STYLE.md index 0649d85..621237a 100644 --- a/templates/STYLE.md +++ b/templates/STYLE.md @@ -11,8 +11,8 @@ guide](https://developers.google.com/style) — the version. Where this file and Google disagree, this file wins; where this file is silent, Google decides. -This is a starter. Keep it, prune it, or add the conventions your team actually -argues about in review. +This is a starter. Keep it, prune it, or add the conventions your team argues +about in review. > **Scope.** Every rig persona and skill writes to this style. It governs prose, > not code: source code follows the conventions of the code around it. @@ -159,8 +159,8 @@ it's red. ## Shapes for the artifacts rig produces **Commit message.** Conventional-commit subject in the imperative, under ~72 -characters, no trailing period. Body only when the *why* isn't obvious from the -diff. +characters, no trailing period. Add a body only when the diff doesn't already +show the *why*. `fix(auth): scope token lookup to the requesting tenant`