From 3f46c1a6dce8c9cd3a78c320f7977b43a7954466 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 01:08:19 -0700 Subject: [PATCH] Block PR summaries that read too hard A PR-authoring agent shipped a Summary the user could not read ("Bare repository references ... now fail the existing provenance gate"). The plain-English rule already existed in draft-pr and invoker-make-pr prose; nothing checked it, so it passed. validate-pr-body.mjs now scores the ## Summary with the Flesch-Kincaid grade (Kincaid et al., 1975) and blocks it above grade 11 or with more than 25% words of three or more syllables. Code spans count as one short word. A Summary under 20 words, or a body with no Summary, prints "reading grade unchecked" with its reason instead of passing silently; that case fails open because drafter-core already enforces the section. Backtest on the last 25 catstack PR bodies: the shipped Summary scores grade 12.9 / 36% and is blocked; 18 others score 3.0-9.9 / 1-16% and pass; 6 have no Summary section and print unchecked. draft-pr's Summary guidance now points at diu and names the limit. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DZzDkkFWa87pWyfCuaUaPE --- engine/skills/draft-pr/SKILL.md | 6 +++ .../scripts/summary-reading-grade.mjs | 53 +++++++++++++++++++ .../draft-pr/scripts/validate-pr-body.mjs | 10 +++- .../draft-pr/tests/test_draft_pr_scripts.py | 44 +++++++++++++++ 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 engine/skills/draft-pr/scripts/summary-reading-grade.mjs diff --git a/engine/skills/draft-pr/SKILL.md b/engine/skills/draft-pr/SKILL.md index 1bd06c6a..2ddc103e 100644 --- a/engine/skills/draft-pr/SKILL.md +++ b/engine/skills/draft-pr/SKILL.md @@ -87,6 +87,12 @@ Plain-English explanation of what changed and why. Paragraphs, not bullets, under 30 words each (configurable via `drafter.config.json`'s `prBody.summaryWordLimit`). One idea per paragraph. +Write it the way the `diu` skill says: for someone who never saw the code. +First sentence says what a person sees change. Short sentences, everyday +words. Explain or cut every term coined while working ("provenance gate", +"bare reference"). `scripts/validate-pr-body.mjs` blocks a Summary above +reading grade 11 or with more than 25% words of three or more syllables. + ## Review Claim State the one thing the reviewer is being asked to approve. diff --git a/engine/skills/draft-pr/scripts/summary-reading-grade.mjs b/engine/skills/draft-pr/scripts/summary-reading-grade.mjs new file mode 100644 index 00000000..909144aa --- /dev/null +++ b/engine/skills/draft-pr/scripts/summary-reading-grade.mjs @@ -0,0 +1,53 @@ +export const MAX_GRADE = 11; +export const MAX_LONG_WORD_SHARE = 0.25; +export const MIN_WORDS = 20; + +function syllables(word) { + let w = word.toLowerCase().replace(/[^a-z]/g, ''); + if (!w) return 0; + if (w.length <= 3) return 1; + w = w.replace(/(?:[^laeiouy]es|ed|[^laeiouy]e)$/, '').replace(/^y/, ''); + return Math.max(1, (w.match(/[aeiouy]{1,2}/g) || []).length); +} + +export function summaryText(body) { + const match = /^## Summary[ \t]*\n([\s\S]*?)(?=^## |(?![\s\S]))/m.exec(body || ''); + if (!match) return null; + return match[1] + .replace(/```[\s\S]*?```/g, ' ') + .replace(/`[^`\n]*`/g, 'X') + .replace(/\[([^\]]*)\]\([^)]*\)/g, '$1') + .trim(); +} + +export function scoreSummary(body) { + const text = summaryText(body); + if (text === null) { + return { status: 'unchecked', reason: 'no ## Summary section to score' }; + } + const words = text.match(/[A-Za-z][A-Za-z'-]*/g) || []; + if (words.length < MIN_WORDS) { + return { + status: 'unchecked', + reason: `Summary has ${words.length} words; under ${MIN_WORDS} the score is too noisy to trust`, + }; + } + const sentences = text.split(/[.!?;:]+(?:\s|$)|\n\s*\n/).filter((s) => /\w/.test(s)); + const counts = words.map(syllables); + const totalSyllables = counts.reduce((a, b) => a + b, 0); + const grade = 0.39 * (words.length / sentences.length) + 11.8 * (totalSyllables / words.length) - 15.59; + const longShare = counts.filter((n) => n >= 3).length / words.length; + const roundedGrade = Math.round(grade * 10) / 10; + const percent = Math.round(longShare * 100); + const hard = roundedGrade > MAX_GRADE || longShare > MAX_LONG_WORD_SHARE; + return { status: hard ? 'hard' : 'clean', grade: roundedGrade, longPercent: percent }; +} + +export function readingGradeError(score) { + return ( + `Summary is too hard to read: reading grade ${score.grade} (limit ${MAX_GRADE}), ` + + `${score.longPercent}% words of three or more syllables (limit ${Math.round(MAX_LONG_WORD_SHARE * 100)}%). ` + + 'Rewrite it for someone who never saw the code: short sentences, everyday words, ' + + 'and explain or cut every term coined while working (see the diu skill).' + ); +} diff --git a/engine/skills/draft-pr/scripts/validate-pr-body.mjs b/engine/skills/draft-pr/scripts/validate-pr-body.mjs index e9f55e14..db4999d3 100644 --- a/engine/skills/draft-pr/scripts/validate-pr-body.mjs +++ b/engine/skills/draft-pr/scripts/validate-pr-body.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import { readFileSync } from 'node:fs'; import { loadDrafterConfig, validatePrBody, getPrBodyWarnings } from '@neko-catpital-labs/drafter-core'; +import { scoreSummary, readingGradeError } 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 ]`); @@ -41,10 +42,15 @@ async function main() { const result = await validatePrBody(body, { requiresVisualProof: args.requiresVisualProof, changedFiles, diffText, config }); const warnings = getPrBodyWarnings(body, { changedFiles, diffText, config }); + const errors = [...result.errors]; - if (result.errors.length > 0) { + const reading = scoreSummary(body); + if (reading.status === 'hard') errors.push(readingGradeError(reading)); + if (reading.status === 'unchecked') console.error(`Summary reading grade unchecked: ${reading.reason}.`); + + if (errors.length > 0) { console.error('PR body validation failed:'); - for (const error of result.errors) console.error(`- ${error}`); + for (const error of errors) console.error(`- ${error}`); process.exit(1); } 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 f107e88f..840a08af 100644 --- a/engine/skills/draft-pr/tests/test_draft_pr_scripts.py +++ b/engine/skills/draft-pr/tests/test_draft_pr_scripts.py @@ -76,6 +76,26 @@ "## Review Unit\n\nproduct", "## Review Unit\n\ntotally-bogus-unit-xyz" ) +VALID_SUMMARY = "Fixes a bug where the widget renderer crashed on empty input." + +HARD_SUMMARY = """Bare repository references such as `(#322)` now fail the existing provenance gate. Rule text should explain requirements and consequences; commit messages retain repository history. + +The detector lifecycle playbook and owning skill lose local PR citations while retaining their instructions. An external Invoker reference gains an explicit project name. + +Regression tests cover rejected references, preserved external citations, and resolvable playbook names. A skill-usage-log test now checks its temporary state directory.""" + +PLAIN_SUMMARY = """Rule files in this repo can no longer point at old PRs with a bare number like `#322`. A test now fails if they do. + +"See `#322`" only tells a reader where to dig. Each rule should say what to do and why, in its own words. + +This PR also rewrites the old PR numbers in the detector playbook, so each rule there explains itself. + +Numbers that name outside work, like "Cook `#3`", still pass.""" + + +def _with_summary(summary: str) -> str: + return VALID_BODY.replace(VALID_SUMMARY, summary) + def _run_validator(body_text: str) -> subprocess.CompletedProcess: with tempfile.TemporaryDirectory() as tmp: @@ -102,5 +122,29 @@ def test_invalid_review_unit_fails_closed(self): self.assertIn("Invalid review unit", result.stderr) +class TestSummaryReadingGrade(unittest.TestCase): + def test_hard_summary_is_blocked(self): + result = _run_validator(_with_summary(HARD_SUMMARY)) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("Summary is too hard to read", result.stderr) + self.assertIn("grade 12.9", result.stderr) + + def test_plain_summary_passes(self): + result = _run_validator(_with_summary(PLAIN_SUMMARY)) + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + self.assertNotIn("too hard to read", result.stderr) + + def test_short_summary_is_reported_unchecked_not_passed(self): + result = _run_validator(VALID_BODY) + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + self.assertIn("reading grade unchecked", result.stderr) + + def test_code_spans_do_not_count_as_long_words(self): + spans = " ".join(f"`engine/skills/draft-pr/scripts/validate_{i}.mjs`" for i in range(12)) + summary = PLAIN_SUMMARY + f"\n\nThe files are {spans}." + result = _run_validator(_with_summary(summary)) + self.assertEqual(result.returncode, 0, result.stderr + result.stdout) + + if __name__ == "__main__": unittest.main()