diff --git a/.github/workflows/pr-body.yml b/.github/workflows/pr-body.yml new file mode 100644 index 00000000..c6251a6b --- /dev/null +++ b/.github/workflows/pr-body.yml @@ -0,0 +1,31 @@ +name: PR body + +on: + pull_request: + types: [opened, edited, reopened, synchronize] + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: "20" + + - name: install PR-drafting toolchain + run: npm ci + + - name: validate the PR's live body against draft-pr rules + env: + PR_BODY: ${{ github.event.pull_request.body }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + printf '%s\n' "$PR_BODY" > "$RUNNER_TEMP/body.md" + git diff --name-only "$PR_BASE_SHA...$PR_HEAD_SHA" > "$RUNNER_TEMP/files.txt" + node engine/skills/draft-pr/scripts/validate-pr-body.mjs \ + --body-file "$RUNNER_TEMP/body.md" \ + --changed-files-file "$RUNNER_TEMP/files.txt" diff --git a/.mergify.yml b/.mergify.yml index f11083d7..9df20ffe 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -11,3 +11,4 @@ queue_rules: merge_conditions: - check-success = lint - check-success = test + - check-success = validate diff --git a/engine/skills/draft-pr/SKILL.md b/engine/skills/draft-pr/SKILL.md index 3ccfeb3c..b22cce71 100644 --- a/engine/skills/draft-pr/SKILL.md +++ b/engine/skills/draft-pr/SKILL.md @@ -92,6 +92,10 @@ Paragraphs, not bullets, under 30 words each (configurable via `drafter.config.json`'s `prBody.summaryWordLimit`). Short sentences, everyday words; explain or cut every term coined while working. +The whole Summary is 150 words or fewer. Write it with the `diu` skill: +lead with what changed for a person, then the problem and the fix, and +nothing else. Detail goes in the sections below it. + No code names in Summary or Review Claim: no backticked text, no snake_case or camelCase words, no file paths, and no word that is the name of a changed file or folder. Say what the part does instead. Names and output belong in @@ -107,8 +111,10 @@ fails, Claude must rewrite. The problem: when Claude rewrote, both checkers stepped aside completely." `scripts/validate-pr-body.mjs` fails a Summary or Review Claim that holds a -code name and lists each one. It also blocks a Summary above reading grade 11 -or with more than 25% words of three or more syllables. +code name and lists each one. It also blocks a Summary above reading grade 11, +with more than 25% words of three or more syllables, or over 150 words. CI +runs the same check on every PR's live text, so a body edited on GitHub is +checked too. ## Review Claim diff --git a/engine/skills/draft-pr/scripts/summary-reading-grade.mjs b/engine/skills/draft-pr/scripts/summary-reading-grade.mjs index 9257fa49..703c170a 100644 --- a/engine/skills/draft-pr/scripts/summary-reading-grade.mjs +++ b/engine/skills/draft-pr/scripts/summary-reading-grade.mjs @@ -1,6 +1,7 @@ export const MAX_GRADE = 11; export const MAX_LONG_WORD_SHARE = 0.25; export const MIN_WORDS = 20; +export const MAX_SUMMARY_WORDS = 150; function syllables(word) { let w = word.toLowerCase().replace(/[^a-z]/g, ''); @@ -57,6 +58,23 @@ export function readingGradeError(score) { ); } +export function summaryWordCount(body) { + const section = sectionText(body, 'Summary'); + if (section === null) { + return { status: 'unchecked', reason: 'no ## Summary section to count' }; + } + const words = (section.match(/\S+/g) || []).filter((token) => /[A-Za-z0-9]/.test(token)).length; + return { status: words > MAX_SUMMARY_WORDS ? 'hard' : 'clean', words }; +} + +export function wordCapError(count) { + return ( + `Summary is too long: ${count.words} words (limit ${MAX_SUMMARY_WORDS}). ` + + `Rewrite it with the diu skill and cut at least ${count.words - MAX_SUMMARY_WORDS} words: ` + + 'say what changed for a person, then the problem and the fix. Detail belongs in later sections.' + ); +} + export const CODE_NAME_SECTIONS = ['Summary', 'Review Claim']; const FILE_EXTENSIONS = new Set([ diff --git a/engine/skills/draft-pr/scripts/validate-pr-body.mjs b/engine/skills/draft-pr/scripts/validate-pr-body.mjs index c8f1eb6f..70089d5a 100644 --- a/engine/skills/draft-pr/scripts/validate-pr-body.mjs +++ b/engine/skills/draft-pr/scripts/validate-pr-body.mjs @@ -1,7 +1,14 @@ #!/usr/bin/env node import { readFileSync } from 'node:fs'; import { loadDrafterConfig, validatePrBody, getPrBodyWarnings } from '@neko-catpital-labs/drafter-core'; -import { scoreSummary, readingGradeError, findCodeNames, codeNameError } from './summary-reading-grade.mjs'; +import { + scoreSummary, + readingGradeError, + summaryWordCount, + wordCapError, + findCodeNames, + codeNameError, +} from './summary-reading-grade.mjs'; function usage() { console.error(`Usage: node scripts/validate-pr-body.mjs (--body-file | --body ) [--require-visual-proof] [--changed-files-file ] [--diff-file ] [--config ]`); @@ -48,6 +55,10 @@ async function main() { if (reading.status === 'hard') errors.push(readingGradeError(reading)); if (reading.status === 'unchecked') console.error(`Summary reading grade unchecked: ${reading.reason}.`); + const count = summaryWordCount(body); + if (count.status === 'hard') errors.push(wordCapError(count)); + if (count.status === 'unchecked') console.error(`Summary word count unchecked: ${count.reason}.`); + const codeNames = findCodeNames(body, changedFiles); if (codeNames.status === 'hard') errors.push(codeNameError(codeNames)); if (codeNames.reason) console.error(`Code-name check unchecked: ${codeNames.reason}.`); diff --git a/engine/skills/draft-pr/tests/test_draft_pr_scripts.py b/engine/skills/draft-pr/tests/test_draft_pr_scripts.py index ba1a65ca..015a1e86 100644 --- a/engine/skills/draft-pr/tests/test_draft_pr_scripts.py +++ b/engine/skills/draft-pr/tests/test_draft_pr_scripts.py @@ -256,5 +256,41 @@ def test_code_names_in_later_sections_do_not_fail(self): self.assertNotIn("Code-name check unchecked", result.stderr) +WORD_CAP_ERROR = "Summary is too long" + + +def _plain_summary_of(words: int) -> str: + sentence = "The cat sat on the mat and then it ran home." + per = len(sentence.split()) + whole, rest = divmod(words, per) + paragraphs = [sentence] * whole + if rest: + paragraphs.append(" ".join(sentence.split()[:rest])) + return "\n\n".join(paragraphs) + + +class TestSummaryWordCap(unittest.TestCase): + def test_summary_over_150_words_fails_and_says_how_many_to_cut(self): + result = _run_validator(_with_summary(_plain_summary_of(170))) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn(WORD_CAP_ERROR, result.stderr) + self.assertIn("170 words", result.stderr) + self.assertIn("cut at least 20", result.stderr) + + def test_summary_of_exactly_150_words_passes(self): + result = _run_validator(_with_summary(_plain_summary_of(150))) + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + self.assertNotIn(WORD_CAP_ERROR, result.stderr) + + def test_words_in_later_sections_do_not_count(self): + body = _with_summary(_plain_summary_of(140)).replace( + "- Does not refactor the renderer.", + "- " + _plain_summary_of(200).replace("\n\n", " "), + ) + result = _run_validator(body) + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + self.assertNotIn(WORD_CAP_ERROR, result.stderr) + + if __name__ == "__main__": unittest.main()