Skip to content
Closed
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
6 changes: 6 additions & 0 deletions engine/skills/draft-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 53 additions & 0 deletions engine/skills/draft-pr/scripts/summary-reading-grade.mjs
Original file line number Diff line number Diff line change
@@ -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).'
);
}
10 changes: 8 additions & 2 deletions engine/skills/draft-pr/scripts/validate-pr-body.mjs
Original file line number Diff line number Diff line change
@@ -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 <file> | --body <markdown>) [--require-visual-proof] [--changed-files-file <file>] [--diff-file <file>] [--config <file>]`);
Expand Down Expand Up @@ -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);
}

Expand Down
44 changes: 44 additions & 0 deletions engine/skills/draft-pr/tests/test_draft_pr_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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()
Loading