Skip to content
36 changes: 27 additions & 9 deletions engine/skills/draft-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,37 @@ Default to this structure (validated by `validatePrBody()` in
```md
## Summary

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.
The reader is a busy director who has never seen the code. Teach them from
zero.

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.
The first paragraph says what the part is and what it does for a person.
Then one short paragraph each for the problem, the cause, and the fix.
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.

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
later sections: Test Plan, Revert Plan, Architecture.

Before: "When diu-stop or prove-it-ship-gate block a reply and the agent
rewrites it, the rewrite is still checked for evidence. Before, both hooks
returned on `stop_hook_active` before running any check."

After: "Before Claude sends a reply, small checker scripts read it. One
checks length. Others check that every claim comes with proof. If a check
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.

## Review Claim

State the one thing the reviewer is being asked to approve.
State the one thing the reviewer is being asked to approve, in everyday
words with no code names (same rule as Summary).

## Review Lane

Expand Down
100 changes: 97 additions & 3 deletions engine/skills/draft-pr/scripts/summary-reading-grade.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ function syllables(word) {
return Math.max(1, (w.match(/[aeiouy]{1,2}/g) || []).length);
}

function sectionText(body, heading) {
const match = new RegExp(`^## ${heading}[ \\t]*\\n([\\s\\S]*?)(?=^## |(?![\\s\\S]))`, 'm').exec(body || '');
return match ? match[1] : null;
}

export function summaryText(body) {
const match = /^## Summary[ \t]*\n([\s\S]*?)(?=^## |(?![\s\S]))/m.exec(body || '');
if (!match) return null;
return match[1]
const section = sectionText(body, 'Summary');
if (section === null) return null;
return section
.replace(/```[\s\S]*?```/g, ' ')
.replace(/`[^`\n]*`/g, 'X')
.replace(/\[([^\]]*)\]\([^)]*\)/g, '$1')
Expand Down Expand Up @@ -51,3 +56,92 @@ export function readingGradeError(score) {
'and explain or cut every term coined while working (see the diu skill).'
);
}

export const CODE_NAME_SECTIONS = ['Summary', 'Review Claim'];

const FILE_EXTENSIONS = new Set([
'py', 'mjs', 'cjs', 'js', 'jsx', 'ts', 'tsx', 'md', 'mdc', 'json', 'yml', 'yaml', 'toml', 'ini', 'cfg',
'sh', 'bash', 'zsh', 'txt', 'tsv', 'csv', 'html', 'css', 'go', 'rs', 'rb', 'java', 'kt', 'swift',
'c', 'h', 'cpp', 'sql', 'lock', 'xml', 'plist',
]);

const TOKEN =
/```([\s\S]*?)```|`([^`\n]+)`|<!--[\s\S]*?-->|\]\([^)\s]*\)|https?:\/\/[^\s)>]+|<\/?[A-Za-z][^>]*>|([\w./~-]+)/g;

function changedFileNames(changedFiles) {
const names = new Map();
const add = (name, kind) => {
if (name && !names.has(name)) names.set(name, kind);
};
for (const file of changedFiles || []) {
const parts = file.split('/').filter((part) => part && part !== '.' && part !== '..');
const base = parts.pop();
for (const folder of parts) add(folder, 'changed folder name');
if (base) {
add(base, 'changed file name');
add(base.replace(/\.[^.]+$/, ''), 'changed file name');
}
}
return names;
}

function wordKind(word, changedNames) {
if (changedNames.has(word)) return changedNames.get(word);
if (word.includes('/')) return 'file path';
const extension = /\w\.([A-Za-z0-9]+)$/.exec(word);
if (extension && FILE_EXTENSIONS.has(extension[1])) return 'file path';
if (/[A-Za-z0-9]_+[A-Za-z0-9]/.test(word)) return 'snake_case';
if (/^[a-z][a-z0-9]*[A-Z]/.test(word)) return 'camelCase';
return null;
}

function sectionCodeNames(section, changedNames) {
const found = [];
const add = (name, kind) => {
if (name && !found.some((f) => f.name === name)) found.push({ name, kind });
};
for (const match of section.matchAll(TOKEN)) {
const [, fenced, inline, word] = match;
if (fenced !== undefined) {
const firstLine = fenced.replace(/^[\w-]*\n/, '').split('\n').map((l) => l.trim()).find(Boolean);
add((firstLine || 'code block').slice(0, 60), 'code block');
} else if (inline !== undefined) {
add(inline.trim(), 'in backticks');
} else if (word !== undefined) {
const cleaned = word.replace(/^(?:[-_]+|\.{2,})/, '').replace(/[._-]+$/, '');
if (!/[A-Za-z]/.test(cleaned)) continue;
const kind = wordKind(cleaned, changedNames);
if (kind) add(cleaned, kind);
}
}
return found;
}

export function findCodeNames(body, changedFiles = []) {
const changedNames = changedFileNames(changedFiles);
const found = [];
const missing = [];
for (const heading of CODE_NAME_SECTIONS) {
const section = sectionText(body, heading);
if (section === null) {
missing.push(`## ${heading}`);
continue;
}
const names = sectionCodeNames(section, changedNames);
if (names.length > 0) found.push({ section: heading, names });
}
const reason = missing.length > 0 ? `no ${missing.join(' or ')} section to read` : undefined;
if (found.length > 0) return { status: 'hard', found, reason };
if (reason) return { status: 'unchecked', found, reason };
return { status: 'clean', found };
}

export function codeNameError(result) {
const where = result.found
.map(({ section, names }) => `${section}: ${names.map(({ name, kind }) => `"${name}" (${kind})`).join(', ')}`)
.join('; ');
return (
`Summary and Review Claim must not use code names. Found in ${where}. ` +
'Say what the part does in everyday words; put the name in a later section such as Test Plan.'
);
}
7 changes: 6 additions & 1 deletion engine/skills/draft-pr/scripts/validate-pr-body.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +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';
import { scoreSummary, readingGradeError, findCodeNames, codeNameError } 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 @@ -48,6 +48,11 @@ async function main() {
if (reading.status === 'hard') errors.push(readingGradeError(reading));
if (reading.status === 'unchecked') console.error(`Summary reading grade unchecked: ${reading.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}.`);
if (!changedFiles) console.error('Code-name check did not compare against changed file and folder names: no --changed-files-file given.');

if (errors.length > 0) {
console.error('PR body validation failed:');
for (const error of errors) console.error(`- ${error}`);
Expand Down
98 changes: 95 additions & 3 deletions engine/skills/draft-pr/tests/test_draft_pr_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@

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.
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.
"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."""
Numbers that name outside work, like "Cook #3", still pass."""


def _with_summary(summary: str) -> str:
Expand All @@ -99,6 +99,25 @@ def _with_summary(summary: str) -> str:

ENGINE_BODY = VALID_BODY.replace("## Review Unit\n\nproduct", "## Review Unit\n\nengine-runtime")

BEFORE_SUMMARY = (
"When diu-stop or prove-it-ship-gate block a reply and the agent rewrites it, "
"the rewrite is still checked for evidence. Before, both hooks returned on "
"`stop_hook_active` before running any check."
)

AFTER_SUMMARY = (
"Before Claude sends a reply, small checker scripts read it. One checks length. "
"Others check that every claim comes with proof. If a check fails, Claude must "
"rewrite. The problem: when Claude rewrote, both checkers stepped aside completely."
)

HOOK_FILES = [
"engine/hooks/diu-stop/claude_stop_check.py",
"engine/hooks/prove-it-ship-gate/detect.py",
]

CODE_NAME_ERROR = "Summary and Review Claim must not use code names"


def _run_validator(body_text: str, changed_files: list[str] | None = None) -> subprocess.CompletedProcess:
with tempfile.TemporaryDirectory() as tmp:
Expand Down Expand Up @@ -158,10 +177,83 @@ def test_short_summary_is_reported_unchecked_not_passed(self):
self.assertIn("reading grade unchecked", result.stderr)

def test_code_spans_do_not_count_as_long_words(self):
"""Code spans fail the code-name check, not the reading grade."""
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.assertNotIn("too hard to read", result.stderr)
self.assertIn(CODE_NAME_ERROR, result.stderr)



class TestSummaryCodeNames(unittest.TestCase):
def _engine_body(self, summary: str) -> str:
return ENGINE_BODY.replace(VALID_SUMMARY, summary)

def test_code_names_in_summary_fail_and_are_each_named(self):
result = _run_validator(self._engine_body(BEFORE_SUMMARY), HOOK_FILES)
self.assertEqual(result.returncode, 1, result.stdout)
self.assertIn(CODE_NAME_ERROR, result.stderr)
for name in ("stop_hook_active", "diu-stop", "prove-it-ship-gate"):
self.assertIn(f'"{name}"', result.stderr)
self.assertIn("put the name in a later section such as Test Plan", result.stderr)

def test_plain_summary_passes_with_the_same_changed_files(self):
result = _run_validator(self._engine_body(AFTER_SUMMARY), HOOK_FILES)
self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
self.assertNotIn(CODE_NAME_ERROR, result.stderr)

def test_hyphenated_english_words_are_not_code_names(self):
summary = (
"This adds a brand-new word-count limit to the form. A person who types "
"too much now sees a short note that says how many words to cut."
)
result = _run_validator(_with_summary(summary))
self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
self.assertNotIn(CODE_NAME_ERROR, result.stderr)

def test_missing_summary_is_reported_unchecked_not_clean(self):
body = VALID_BODY.replace(f"## Summary\n\n{VALID_SUMMARY}\n\n", "")
self.assertNotIn("## Summary", body)
result = _run_validator(body)
self.assertIn("Code-name check unchecked: no ## Summary section to read", result.stderr)

def test_code_names_in_later_sections_do_not_fail(self):
body = self._engine_body(AFTER_SUMMARY).replace(
"- [x] `pytest tests/test_widget_renderer.py`",
"- [x] `python3 engine/hooks/diu-stop/claude_stop_check.py` exits 0 once "
"`stop_hook_active` is set; prove-it-ship-gate/detect.py too",
).replace(
"- Post-revert steps: None",
"- Post-revert steps: diu-stop and prove-it-ship-gate return on `stop_hook_active` again",
).replace(
"## Test Plan",
"""## Architecture

diu-stop and prove-it-ship-gate both read `stop_hook_active`.

### Before

```mermaid
graph TD
A["claude_stop_check.py"] --> B["return on stop_hook_active"]
```

### After

diu-stop/claude_stop_check.py keeps checking; detect.py too.

```mermaid
graph TD
A["claude_stop_check.py"] --> B["run every check"]
```

## Test Plan""",
)
result = _run_validator(body, HOOK_FILES)
self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
self.assertNotIn(CODE_NAME_ERROR, result.stderr)
self.assertNotIn("Code-name check unchecked", result.stderr)


if __name__ == "__main__":
Expand Down
Loading