mirror of
https://github.com/mattermost/mattermost.git
synced 2026-10-16 07:09:04 +08:00
docs: scope AI documentation drafts to the merged change
Draft PRs were titled by number, filled with the review loop, and restyling whole pages because pre-open review treated every line as new. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
# Docs writer
|
||||
|
||||
You draft Mattermost documentation pages on behalf of the technical writing team.
|
||||
You receive untrusted PR evidence and a gap brief; you emit MDX file blocks only.
|
||||
You receive untrusted PR evidence and a gap brief; you emit a summary fence and MDX file blocks only.
|
||||
|
||||
## Role
|
||||
|
||||
@@ -21,6 +21,30 @@ when the brief asks for one; otherwise extend the named existing page.
|
||||
Ask: what can the reader do now that they could not before? Answer in one sentence and
|
||||
write only what that sentence requires.
|
||||
|
||||
## Editing an existing page
|
||||
|
||||
When an existing page is supplied, change only the lines the brief requires. Copy every
|
||||
other line through unchanged, including heading case, spacing, grammar, typos, and code
|
||||
fence style. Conventions apply to the text you add. Reviewer feedback about a line you
|
||||
did not add is out of scope; do not apply it.
|
||||
|
||||
## How much to write
|
||||
|
||||
Match the depth of the page you are editing and of the source pull request's release
|
||||
note. Say what the reader can do, and any single constraint that changes which path
|
||||
they take.
|
||||
|
||||
Do not document exact error strings, bind addresses, firewall rules, port conflicts, or
|
||||
packaging variables. Reviewer feedback is not evidence. If a reviewer asks for behaviour
|
||||
the pull request description and the code diff do not state, leave it out.
|
||||
|
||||
## Closing the brief
|
||||
|
||||
Each recommended action is its own heading or procedure, at the same level as the
|
||||
sibling sections on that page. A sentence inside another section does not close an
|
||||
action. When the brief names more than one behaviour, document them in the order a
|
||||
reader meets them.
|
||||
|
||||
## Hard rules
|
||||
|
||||
- Claims must map to evidence in the supplied PR metadata, description, code diff, or
|
||||
@@ -33,8 +57,8 @@ write only what that sentence requires.
|
||||
`docs/vendor/**`, or `docs/main/agents/docs/**`.
|
||||
- Do not edit changelogs, important upgrade notes, or version-archive pages unless the
|
||||
brief explicitly requires it.
|
||||
- Follow `conventions.md` for frontmatter, plan availability, callouts, links, MDX
|
||||
escaping, heading case, and voice.
|
||||
- Follow `conventions.md` for text you add: frontmatter, plan availability, callouts,
|
||||
links, MDX escaping, heading case, and voice. Do not restyle existing lines to match.
|
||||
|
||||
## Observability and diagnostics
|
||||
|
||||
@@ -64,8 +88,17 @@ log formatting" when the meaning of the output is unchanged.
|
||||
|
||||
## Output format
|
||||
|
||||
Return one or more fenced code blocks, each preceded by a `path=` attribute naming the
|
||||
destination relative to the repository root. No prose between blocks.
|
||||
Return a summary fence, then one or more file fences. No other prose.
|
||||
|
||||
```summary
|
||||
- What the reader can do now, one behaviour per bullet.
|
||||
```
|
||||
|
||||
The summary is the draft pull request description. Describe the documentation change.
|
||||
Do not restate the gap analysis, and do not mention review feedback.
|
||||
|
||||
Each file fence is preceded by a `path=` attribute naming the destination relative to
|
||||
the repository root.
|
||||
|
||||
Use a **four-backtick** outer fence so any three-backtick code samples inside the page
|
||||
do not close the block early:
|
||||
|
||||
@@ -111,7 +111,9 @@ out of the comment it wrote last time, so it can tell its own label from a human
|
||||
When a PR **merges** still carrying `Docs/Needed`, `docs-writer-merged-pr.yml` reads the
|
||||
sticky comment's recommended actions, authors MDX with the matching `author` personas,
|
||||
pre-reviews in-loop, and opens `docs/pr-<n>` as a draft (authored by the Docs AI App).
|
||||
Closing that draft flips `Docs/Needed` → `Docs/Done` on the originating PR.
|
||||
The draft title comes from the source PR title. The draft body says what the
|
||||
documentation change does. The pre-open review trail stays in `trail.json`, not the
|
||||
body. Closing that draft flips `Docs/Needed` → `Docs/Done` on the originating PR.
|
||||
|
||||
Three constraints to preserve if you change the gap label logic:
|
||||
|
||||
|
||||
@@ -1,21 +1,23 @@
|
||||
/*
|
||||
* Synthesise an all-additions unified diff so pre-open persona review takes the
|
||||
* same prompt shape as docs-review.yml's git diff.
|
||||
* Diffs for persona review.
|
||||
*
|
||||
* additionsDiff marks every line as new. unifiedDiff compares an authored page
|
||||
* with the on-disk original so pre-open review sees only the introduced change.
|
||||
* A missing original is a new file and uses additionsDiff.
|
||||
*/
|
||||
|
||||
/** Trailing newline is not an extra empty line. */
|
||||
export function toLines(content) {
|
||||
const text = String(content ?? '');
|
||||
if (text === '') return [];
|
||||
const body = text.endsWith('\n') ? text.slice(0, -1) : text;
|
||||
return body.split('\n');
|
||||
}
|
||||
|
||||
export function additionsDiff(files) {
|
||||
const chunks = [];
|
||||
for (const {path, content} of files) {
|
||||
const lines = content === ''
|
||||
? []
|
||||
: (() => {
|
||||
const body = content.endsWith('\n') ? content : `${content}\n`;
|
||||
const parts = body.split('\n');
|
||||
// Trailing empty from the final newline — drop it so +++ counts match.
|
||||
if (parts.at(-1) === '') parts.pop();
|
||||
return parts;
|
||||
})();
|
||||
|
||||
const lines = toLines(content);
|
||||
const n = lines.length;
|
||||
chunks.push(
|
||||
`diff --git a/${path} b/${path}`,
|
||||
@@ -28,3 +30,152 @@ export function additionsDiff(files) {
|
||||
}
|
||||
return chunks.join('\n') + (chunks.length ? '\n' : '');
|
||||
}
|
||||
|
||||
const MAX_LCS_CELLS = 16_000_000;
|
||||
|
||||
/** LCS edit script for two line arrays. `type` is ' ', '-', or '+'. */
|
||||
function lcsDiff(a, b) {
|
||||
const n = a.length;
|
||||
const m = b.length;
|
||||
const dp = Array.from({length: n + 1}, () => new Uint32Array(m + 1));
|
||||
for (let i = n - 1; i >= 0; i--) {
|
||||
const row = dp[i];
|
||||
const next = dp[i + 1];
|
||||
for (let j = m - 1; j >= 0; j--) {
|
||||
row[j] = a[i] === b[j] ? next[j + 1] + 1 : Math.max(next[j], row[j + 1]);
|
||||
}
|
||||
}
|
||||
|
||||
const ops = [];
|
||||
let i = 0;
|
||||
let j = 0;
|
||||
while (i < n && j < m) {
|
||||
if (a[i] === b[j]) {
|
||||
ops.push({type: ' ', line: a[i]});
|
||||
i += 1;
|
||||
j += 1;
|
||||
} else if (dp[i + 1][j] >= dp[i][j + 1]) {
|
||||
ops.push({type: '-', line: a[i]});
|
||||
i += 1;
|
||||
} else {
|
||||
ops.push({type: '+', line: b[j]});
|
||||
j += 1;
|
||||
}
|
||||
}
|
||||
while (i < n) ops.push({type: '-', line: a[i++]});
|
||||
while (j < m) ops.push({type: '+', line: b[j++]});
|
||||
return ops;
|
||||
}
|
||||
|
||||
/**
|
||||
* Edit script against the existing page. Shared prefix and suffix stay as
|
||||
* context so a long page is not compared in full. A changed middle that is
|
||||
* still too large to align is emitted as a straight replace.
|
||||
*/
|
||||
export function lineDiff(before, after) {
|
||||
const a = toLines(before);
|
||||
const b = toLines(after);
|
||||
let start = 0;
|
||||
const maxStart = Math.min(a.length, b.length);
|
||||
while (start < maxStart && a[start] === b[start]) start += 1;
|
||||
|
||||
let endA = a.length;
|
||||
let endB = b.length;
|
||||
while (endA > start && endB > start && a[endA - 1] === b[endB - 1]) {
|
||||
endA -= 1;
|
||||
endB -= 1;
|
||||
}
|
||||
|
||||
const prefix = a.slice(0, start).map((line) => ({type: ' ', line}));
|
||||
const suffix = a.slice(endA).map((line) => ({type: ' ', line}));
|
||||
const midA = a.slice(start, endA);
|
||||
const midB = b.slice(start, endB);
|
||||
const mid = midA.length * midB.length > MAX_LCS_CELLS
|
||||
? [
|
||||
...midA.map((line) => ({type: '-', line})),
|
||||
...midB.map((line) => ({type: '+', line})),
|
||||
]
|
||||
: lcsDiff(midA, midB);
|
||||
return [...prefix, ...mid, ...suffix];
|
||||
}
|
||||
|
||||
function formatHunks(ops, context = 3) {
|
||||
const changeIdx = [];
|
||||
for (let i = 0; i < ops.length; i++) {
|
||||
if (ops[i].type !== ' ') changeIdx.push(i);
|
||||
}
|
||||
if (changeIdx.length === 0) return '';
|
||||
|
||||
const ranges = [];
|
||||
let start = changeIdx[0];
|
||||
let end = changeIdx[0];
|
||||
for (const idx of changeIdx.slice(1)) {
|
||||
if (idx <= end + context * 2 + 1) {
|
||||
end = idx;
|
||||
} else {
|
||||
ranges.push([start, end]);
|
||||
start = idx;
|
||||
end = idx;
|
||||
}
|
||||
}
|
||||
ranges.push([start, end]);
|
||||
|
||||
const oldAt = [];
|
||||
const newAt = [];
|
||||
let oldLine = 1;
|
||||
let newLine = 1;
|
||||
for (let i = 0; i < ops.length; i++) {
|
||||
oldAt[i] = oldLine;
|
||||
newAt[i] = newLine;
|
||||
if (ops[i].type !== '+') oldLine += 1;
|
||||
if (ops[i].type !== '-') newLine += 1;
|
||||
}
|
||||
|
||||
const parts = [];
|
||||
for (const [changeStart, changeEnd] of ranges) {
|
||||
const from = Math.max(0, changeStart - context);
|
||||
const to = Math.min(ops.length - 1, changeEnd + context);
|
||||
const slice = ops.slice(from, to + 1);
|
||||
let oldCount = 0;
|
||||
let newCount = 0;
|
||||
for (const op of slice) {
|
||||
if (op.type !== '+') oldCount += 1;
|
||||
if (op.type !== '-') newCount += 1;
|
||||
}
|
||||
let oldStart = oldAt[from];
|
||||
let newStart = newAt[from];
|
||||
if (oldCount === 0) oldStart = Math.max(0, oldStart - 1);
|
||||
if (newCount === 0) newStart = Math.max(0, newStart - 1);
|
||||
const body = slice.map((op) => `${op.type}${op.line}`).join('\n');
|
||||
parts.push(`@@ -${oldStart},${oldCount} +${newStart},${newCount} @@\n${body}`);
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Unified diff of authored pages against on-disk originals.
|
||||
* Unchanged files are omitted. Missing originals are new files.
|
||||
*/
|
||||
export function unifiedDiff(files, originals = []) {
|
||||
const byPath = new Map((originals || []).map((o) => [o.path, o.content ?? '']));
|
||||
const chunks = [];
|
||||
for (const file of files || []) {
|
||||
if (!byPath.has(file.path)) {
|
||||
const added = additionsDiff([file]);
|
||||
chunks.push(added.endsWith('\n') ? added.slice(0, -1) : added);
|
||||
continue;
|
||||
}
|
||||
const hunks = formatHunks(lineDiff(byPath.get(file.path), file.content));
|
||||
if (!hunks) continue;
|
||||
chunks.push(
|
||||
[
|
||||
`diff --git a/${file.path} b/${file.path}`,
|
||||
`--- a/${file.path}`,
|
||||
`+++ b/${file.path}`,
|
||||
hunks,
|
||||
].join('\n'),
|
||||
);
|
||||
}
|
||||
if (chunks.length === 0) return '';
|
||||
return `${chunks.join('\n')}\n`;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import {test} from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {additionsDiff} from './diff.mjs';
|
||||
import {additionsDiff, unifiedDiff} from './diff.mjs';
|
||||
|
||||
function hunkHeader(diff) {
|
||||
const m = diff.match(/@@ -0,0 \+(\d+)(?:,(\d+))? @@/);
|
||||
@@ -50,3 +50,61 @@ test('additionsDiff joins multiple files as separate chunks', () => {
|
||||
test('additionsDiff returns empty string for no files', () => {
|
||||
assert.equal(additionsDiff([]), '');
|
||||
});
|
||||
|
||||
test('unifiedDiff shows an insertion and leaves distant lines out of the hunk', () => {
|
||||
const path = 'docs/main/administration-guide/manage/command-line-tools.mdx';
|
||||
const before = ['## Compliance export', '', 'Existing export text.', '', '## Server operations', ''].join('\n');
|
||||
const after = [
|
||||
'## Compliance export',
|
||||
'',
|
||||
'Existing export text.',
|
||||
'',
|
||||
'## Read the bundled documentation',
|
||||
'',
|
||||
'Open /_documentation/ on a running server.',
|
||||
'',
|
||||
'## Server operations',
|
||||
'',
|
||||
].join('\n');
|
||||
const diff = unifiedDiff([{path, content: after}], [{path, content: before}]);
|
||||
assert.match(diff, /^\+\+\+ b\/docs\/main/m);
|
||||
assert.doesNotMatch(diff, /\/dev\/null/);
|
||||
assert.match(diff, /\+## Read the bundled documentation/);
|
||||
assert.match(diff, /\+Open \/_documentation\/ on a running server\./);
|
||||
assert.doesNotMatch(diff, /\+## Compliance export/);
|
||||
assert.doesNotMatch(diff, /-## Compliance export/);
|
||||
});
|
||||
|
||||
test('unifiedDiff still reports a real edit away from the insertion', () => {
|
||||
const path = 'docs/main/a.mdx';
|
||||
const before = ['## Compliance Export', '', 'Body.', '', '## Tail', ''].join('\n');
|
||||
const after = ['## Compliance export', '', 'Body.', '', '## Tail', '', 'Added.', ''].join('\n');
|
||||
const diff = unifiedDiff([{path, content: after}], [{path, content: before}]);
|
||||
assert.match(diff, /-## Compliance Export/);
|
||||
assert.match(diff, /\+## Compliance export/);
|
||||
assert.match(diff, /\+Added\./);
|
||||
});
|
||||
|
||||
test('unifiedDiff treats a missing original as a new file', () => {
|
||||
const file = {path: 'docs/main/new.mdx', content: 'Hello\n'};
|
||||
const diff = unifiedDiff([file], []);
|
||||
assert.equal(diff, additionsDiff([file]));
|
||||
});
|
||||
|
||||
test('unifiedDiff omits a file that did not change', () => {
|
||||
const path = 'docs/main/a.mdx';
|
||||
const content = 'Same\n';
|
||||
assert.equal(unifiedDiff([{path, content}], [{path, content}]), '');
|
||||
});
|
||||
|
||||
test('unifiedDiff on a long page reports the insertion and not the unchanged head', () => {
|
||||
const path = 'docs/main/a.mdx';
|
||||
const head = Array.from({length: 5000}, (_, i) => `line ${i}`);
|
||||
const before = [...head, 'tail'].join('\n');
|
||||
const after = [...head.slice(0, 2500), '## New', ...head.slice(2500), 'tail'].join('\n');
|
||||
const diff = unifiedDiff([{path, content: `${after}\n`}], [{path, content: before}]);
|
||||
assert.match(diff, /\+## New/);
|
||||
assert.doesNotMatch(diff, /\+line 0\b/);
|
||||
assert.doesNotMatch(diff, /\+line 4999\b/);
|
||||
assert.doesNotMatch(diff, /-line 0\b/);
|
||||
});
|
||||
|
||||
@@ -81,3 +81,13 @@ test('writer prompt states scope and observability guidance', () => {
|
||||
assert.match(prompt, /USER_PASSWORD_CHANGED/);
|
||||
assert.match(prompt, /processWidgets/);
|
||||
});
|
||||
|
||||
test('writer prompt keeps a draft to the introduced change', () => {
|
||||
const prompt = writerPrompt();
|
||||
assert.match(prompt, /Copy every\nother line through unchanged/);
|
||||
assert.match(prompt, /exact error strings/);
|
||||
assert.match(prompt, /own heading or procedure/);
|
||||
assert.match(prompt, /```summary/);
|
||||
assert.match(prompt, /Reviewer feedback is not evidence/);
|
||||
assert.match(prompt, /does not close an\naction/);
|
||||
});
|
||||
|
||||
@@ -20,11 +20,13 @@ function readIfSet(name) {
|
||||
return path && existsSync(path) ? readFileSync(path, 'utf8') : null;
|
||||
}
|
||||
|
||||
export function buildReviewUserPrompt({diff, prTitle, prBody}) {
|
||||
export function buildReviewUserPrompt({diff, prTitle, prBody, reviewNote, extra}) {
|
||||
const parts = [DATA_NOTICE, ''];
|
||||
|
||||
if (prTitle) parts.push(block('pull-request-title', prTitle, {maxChars: 500}), '');
|
||||
if (prBody) parts.push(block('pull-request-description', prBody, {maxChars: 6000}), '');
|
||||
if (extra) parts.push(extra, '');
|
||||
if (reviewNote) parts.push(reviewNote, '');
|
||||
|
||||
parts.push(
|
||||
block('diff', diff),
|
||||
@@ -56,6 +58,8 @@ export async function reviewPersona({
|
||||
diff,
|
||||
prTitle,
|
||||
prBody,
|
||||
reviewNote,
|
||||
extra,
|
||||
model = MODEL,
|
||||
completeFn = complete,
|
||||
}) {
|
||||
@@ -64,7 +68,7 @@ export async function reviewPersona({
|
||||
const {text, usage} = await completeFn({
|
||||
model,
|
||||
system: reviewSystemBlocks(personaId),
|
||||
userPrompt: buildReviewUserPrompt({diff, prTitle, prBody}),
|
||||
userPrompt: buildReviewUserPrompt({diff, prTitle, prBody, reviewNote, extra}),
|
||||
maxTokens: 2048,
|
||||
temperature: 0.2,
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import {existsSync, readFileSync} from 'node:fs';
|
||||
import {join} from 'node:path';
|
||||
import {complete, usageLine} from '../lib/anthropic.mjs';
|
||||
import {additionsDiff} from '../lib/diff.mjs';
|
||||
import {unifiedDiff} from '../lib/diff.mjs';
|
||||
import {
|
||||
alwaysOnPersonaIds,
|
||||
authorSystemBlocks,
|
||||
@@ -11,6 +11,30 @@ import {
|
||||
REPO_ROOT as DEFAULT_ROOT,
|
||||
} from '../lib/personas.mjs';
|
||||
import {DATA_NOTICE, block} from '../lib/untrusted.mjs';
|
||||
|
||||
const AUTHOR_REMINDER = [
|
||||
'Copy unchanged lines through unchanged.',
|
||||
'Give each brief action its own heading or procedure. A sentence inside another section does not close it.',
|
||||
'Do not add error strings, bind or firewall detail, or packaging variables.',
|
||||
].join(' ');
|
||||
|
||||
/** Writer-loop only. docs-review.yml does not send this. */
|
||||
export const PREOPEN_REVIEW_NOTE = [
|
||||
'This is a pre-open review of an AI documentation draft.',
|
||||
'REQUEST_CHANGES only when new text is factually wrong, or a required capability is missing as its own heading or procedure.',
|
||||
'A capability mentioned only inside another section is missing.',
|
||||
'Do not request exact error strings, bind addresses, firewall rules, port conflicts, or packaging variables. That is COMMENT at most, and only when the pull request evidence already states the behaviour.',
|
||||
'Do not request fixes to unchanged context lines.',
|
||||
].join('\n');
|
||||
|
||||
const SUMMARY_RE = /`{3,}summary[ \t]*\n([\s\S]*?)\n`{3,}/;
|
||||
|
||||
/** Summary fence from an author response. Empty when the model omits it. */
|
||||
export function parseSummaryBlock(text) {
|
||||
const match = String(text || '').match(SUMMARY_RE);
|
||||
if (!match) return '';
|
||||
return match[1].replace(/<!--\s*docs-ai-[\s\S]*?-->/g, '').trim();
|
||||
}
|
||||
import {reviewPersona} from '../review/persona-review.mjs';
|
||||
import {assertSafeMdx, assertVersionAnchors, mergeFiles, parseFileBlocks} from './files.mjs';
|
||||
import {isAllowedPath} from './paths.mjs';
|
||||
@@ -120,8 +144,8 @@ function buildAuthorUserPrompt({brief, groupPaths, input, revisionFeedback, exis
|
||||
}
|
||||
parts.push(
|
||||
revisionFeedback?.length
|
||||
? 'For paths with a current-draft block, revise that draft in place. Output the full updated file.'
|
||||
: 'For paths with an existing-page block, edit that page in place (smallest change that closes the gap). Output the full updated file.',
|
||||
? 'For paths with a current-draft block, revise that draft in place. Output the full updated file. Keep every line the review did not require.'
|
||||
: 'For paths with an existing-page block, edit that page in place. Copy every line you do not need to change through unchanged. Output the full updated file.',
|
||||
'',
|
||||
);
|
||||
}
|
||||
@@ -130,11 +154,15 @@ function buildAuthorUserPrompt({brief, groupPaths, input, revisionFeedback, exis
|
||||
parts.push(
|
||||
block('reviewer-feedback', JSON.stringify(revisionFeedback, null, 2), {maxChars: 12_000}),
|
||||
'',
|
||||
'Revise the pages to address REQUEST_CHANGES feedback. Output the full file blocks again.',
|
||||
'Revise the text this draft added. Ignore feedback about pre-existing lines. Do not add behaviour that is not in the pull request evidence.',
|
||||
'Output the summary fence, then the full file blocks.',
|
||||
AUTHOR_REMINDER,
|
||||
);
|
||||
} else {
|
||||
parts.push(
|
||||
'Draft the MDX file(s) that close the documentation gap for the target paths. Output only the file blocks.',
|
||||
'Draft the MDX file(s) that close the documentation gap for the target paths.',
|
||||
'Output the summary fence, then the file blocks. No other prose.',
|
||||
AUTHOR_REMINDER,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -179,7 +207,7 @@ async function authorPass({personaId, groupPaths, brief, input, revisionFeedback
|
||||
if (files.length === 0) {
|
||||
throw new Error(`author ${personaId ?? 'neutral'} emitted no allowed file blocks`);
|
||||
}
|
||||
return files;
|
||||
return {files, summary: parseSummaryBlock(text)};
|
||||
}
|
||||
|
||||
async function selectReviewers({diff, completeFn}) {
|
||||
@@ -208,9 +236,21 @@ async function selectReviewers({diff, completeFn}) {
|
||||
}
|
||||
}
|
||||
|
||||
async function reviewFiles({files, input, completeFn}) {
|
||||
const diff = additionsDiff(files);
|
||||
function requiredCapabilities(brief) {
|
||||
const actions = (brief?.actions ?? []).map((a) => String(a).trim()).filter(Boolean);
|
||||
if (actions.length === 0) return '';
|
||||
return block('required-capabilities', actions.join('\n'), {maxChars: 4000});
|
||||
}
|
||||
|
||||
async function reviewFiles({files, input, brief, completeFn}) {
|
||||
const originals = loadExistingPages(input.repoRoot, files.map((f) => f.path));
|
||||
const diff = unifiedDiff(files, originals);
|
||||
if (!diff.trim()) {
|
||||
const paths = files.map((f) => f.path).join(', ');
|
||||
throw new Error(`authored pages are identical to the existing files: ${paths}`);
|
||||
}
|
||||
const reviewers = await selectReviewers({diff, completeFn});
|
||||
const extra = requiredCapabilities(brief);
|
||||
const results = await Promise.all(
|
||||
reviewers.map((personaId) =>
|
||||
reviewPersona({
|
||||
@@ -218,6 +258,8 @@ async function reviewFiles({files, input, completeFn}) {
|
||||
diff,
|
||||
prTitle: input.prTitle,
|
||||
prBody: input.prBody,
|
||||
reviewNote: PREOPEN_REVIEW_NOTE,
|
||||
extra,
|
||||
model: REVIEW_MODEL,
|
||||
completeFn,
|
||||
}),
|
||||
@@ -227,7 +269,8 @@ async function reviewFiles({files, input, completeFn}) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Author → review → revise loop. Returns files + trail for the draft PR body.
|
||||
* Author → review → revise loop.
|
||||
* `summary` is the draft PR body. `trail` is kept for the workflow artifact only.
|
||||
*/
|
||||
export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
const targets =
|
||||
@@ -246,15 +289,18 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
|
||||
let files = [];
|
||||
const trail = {iterations: [], openConcerns: []};
|
||||
const summaries = [];
|
||||
|
||||
for (const group of groups) {
|
||||
let groupFiles = await authorPass({
|
||||
let authored = await authorPass({
|
||||
personaId: group.personaId,
|
||||
groupPaths: group.paths,
|
||||
brief,
|
||||
input,
|
||||
completeFn,
|
||||
});
|
||||
let groupFiles = authored.files;
|
||||
let groupSummary = authored.summary;
|
||||
const initialMissing = missingGroupTargets(groupFiles, group.paths);
|
||||
if (initialMissing.length) {
|
||||
throw new Error(
|
||||
@@ -267,7 +313,7 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
let revision = 0;
|
||||
let lastResults = [];
|
||||
for (;;) {
|
||||
const {results} = await reviewFiles({files: groupFiles, input, completeFn});
|
||||
const {results} = await reviewFiles({files: groupFiles, input, brief, completeFn});
|
||||
lastResults = results;
|
||||
trail.iterations.push({
|
||||
author: group.personaId ?? 'neutral',
|
||||
@@ -312,7 +358,7 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
basePages: groupFiles,
|
||||
completeFn,
|
||||
});
|
||||
const revisedMissing = missingGroupTargets(revised, group.paths);
|
||||
const revisedMissing = missingGroupTargets(revised.files, group.paths);
|
||||
if (revisedMissing.length) {
|
||||
// Incomplete revision must not replace a complete prior pass.
|
||||
console.error(
|
||||
@@ -326,8 +372,8 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
break;
|
||||
}
|
||||
try {
|
||||
assertVersionAnchors(revised, {milestoneVersion: input.milestoneVersion});
|
||||
assertSafeMdx(revised);
|
||||
assertVersionAnchors(revised.files, {milestoneVersion: input.milestoneVersion});
|
||||
assertSafeMdx(revised.files);
|
||||
} catch (e) {
|
||||
// Dropped anchor / unsafe MDX must not replace a complete prior pass.
|
||||
console.error(
|
||||
@@ -340,9 +386,11 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
});
|
||||
break;
|
||||
}
|
||||
groupFiles = revised;
|
||||
groupFiles = revised.files;
|
||||
if (revised.summary) groupSummary = revised.summary;
|
||||
}
|
||||
|
||||
if (groupSummary) summaries.push(groupSummary);
|
||||
files = mergeFiles(files, groupFiles);
|
||||
console.error(
|
||||
`[author-loop] ${group.personaId ?? 'neutral'}: ${groupFiles.map((f) => f.path).join(', ')} ` +
|
||||
@@ -350,19 +398,16 @@ export async function authorLoop({brief, input, completeFn = complete} = {}) {
|
||||
);
|
||||
}
|
||||
|
||||
return {files, trail};
|
||||
return {files, trail, summary: summaries.join('\n\n').trim()};
|
||||
}
|
||||
|
||||
export function renderPrBody({sourcePr, trail, brief, milestoneVersion}) {
|
||||
export function renderPrBody({sourcePr, summary, milestoneVersion}) {
|
||||
const lines = [
|
||||
'<!-- docs-ai-draft:v1 -->',
|
||||
`<!-- docs-ai-source-pr:${sourcePr} -->`,
|
||||
'',
|
||||
`AI-drafted documentation for #${sourcePr}.`,
|
||||
'',
|
||||
'Authored by audience personas from the gap analysis sticky comment, then reviewed',
|
||||
'in-loop before this PR opened. Treat as a draft pending human editorial review.',
|
||||
'',
|
||||
];
|
||||
|
||||
if (!milestoneVersion) {
|
||||
@@ -373,41 +418,11 @@ export function renderPrBody({sourcePr, trail, brief, milestoneVersion}) {
|
||||
);
|
||||
}
|
||||
|
||||
lines.push(
|
||||
'## Gap brief',
|
||||
'',
|
||||
brief.summary || '_No summary recovered from the sticky comment._',
|
||||
'',
|
||||
);
|
||||
|
||||
if (brief.actions?.length) {
|
||||
lines.push('### Recommended actions', '', ...brief.actions.map((a) => `- [ ] ${a}`), '');
|
||||
}
|
||||
|
||||
lines.push('## Pre-open review trail', '');
|
||||
for (const iter of trail.iterations) {
|
||||
lines.push(
|
||||
`### Author \`${iter.author}\` — revision ${iter.revision}`,
|
||||
'',
|
||||
`Paths: ${iter.paths.map((p) => `\`${p}\``).join(', ')}`,
|
||||
'',
|
||||
);
|
||||
for (const r of iter.reviews) {
|
||||
lines.push(`- **${r.persona}** — \`${r.verdict}\`: ${r.summary}`);
|
||||
for (const f of r.feedback ?? []) lines.push(` - ${f}`);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
|
||||
if (trail.openConcerns?.length) {
|
||||
lines.push('## Open concerns after revision cap', '');
|
||||
for (const c of trail.openConcerns) {
|
||||
lines.push(`- **${c.persona}**: ${c.summary}`);
|
||||
for (const f of c.feedback ?? []) lines.push(` - ${f}`);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
|
||||
lines.push(`Closes the docs gap filed against #${sourcePr}.`);
|
||||
let change = String(summary || '')
|
||||
.replace(/<!--\s*docs-ai-[\s\S]*?-->/g, '')
|
||||
.trim();
|
||||
if (change.length > 2000) change = `${change.slice(0, 2000).trimEnd()}...`;
|
||||
lines.push('## Summary', '', change || '_The draft did not include a change summary._', '');
|
||||
lines.push('Pending human editorial review.');
|
||||
return `${lines.join('\n')}\n`;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
const PREFIX = 'docs: AI draft for ';
|
||||
const MAX_LEN = 240;
|
||||
|
||||
/**
|
||||
* Draft PR title from the merged PR's title.
|
||||
* Strips a leading "docs:" so the prefix is not doubled, and drops control
|
||||
* characters so the title stays a single git/GitHub line.
|
||||
*/
|
||||
export function draftTitle(sourceTitle) {
|
||||
const cleaned = String(sourceTitle ?? '')
|
||||
.replace(/[\u0000-\u001f\u007f]/g, ' ')
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim()
|
||||
.replace(/^docs:\s*/i, '');
|
||||
let subject = cleaned || 'documentation update';
|
||||
const room = MAX_LEN - PREFIX.length;
|
||||
if (subject.length > room) {
|
||||
const cut = subject.slice(0, room - 3);
|
||||
const space = cut.lastIndexOf(' ');
|
||||
subject = `${(space > 40 ? cut.slice(0, space) : cut).trimEnd()}...`;
|
||||
}
|
||||
return PREFIX + subject;
|
||||
}
|
||||
@@ -13,6 +13,7 @@
|
||||
* Writes docs files into REPO_ROOT and emits:
|
||||
* .docs-ai/writer/trail.json
|
||||
* .docs-ai/writer/pr-body.md
|
||||
* .docs-ai/writer/draft-title.txt
|
||||
* .docs-ai/writer/written.json
|
||||
*/
|
||||
|
||||
@@ -22,6 +23,7 @@ import {findStickyComment} from '../lib/github.mjs';
|
||||
import {MARKER} from '../gap/gap.mjs';
|
||||
import {REPO_ROOT as DEFAULT_ROOT} from '../lib/personas.mjs';
|
||||
import {authorLoop, renderPrBody} from './author-loop.mjs';
|
||||
import {draftTitle} from './draft-title.mjs';
|
||||
import {versionFromMilestone, writeFiles} from './files.mjs';
|
||||
import {briefFromGapResult, briefFromPrEvidence, parseGapBrief} from './gap-brief.mjs';
|
||||
|
||||
@@ -104,11 +106,12 @@ async function main() {
|
||||
repoRoot,
|
||||
};
|
||||
|
||||
const {files, trail} = await authorLoop({brief, input});
|
||||
const {files, trail, summary} = await authorLoop({brief, input});
|
||||
|
||||
writeFileSync(join(outDir, 'trail.json'), `${JSON.stringify(trail, null, 2)}\n`);
|
||||
const body = renderPrBody({sourcePr: pr, trail, brief, milestoneVersion});
|
||||
const body = renderPrBody({sourcePr: pr, summary, milestoneVersion});
|
||||
writeFileSync(join(outDir, 'pr-body.md'), body);
|
||||
writeFileSync(join(outDir, 'draft-title.txt'), `${draftTitle(input.prTitle)}\n`);
|
||||
|
||||
if (process.env.DRY_RUN === '1') {
|
||||
writeFileSync(join(outDir, 'files.json'), `${JSON.stringify(files, null, 2)}\n`);
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import {test} from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {mkdtempSync, mkdirSync, rmSync, writeFileSync} from 'node:fs';
|
||||
import {tmpdir} from 'node:os';
|
||||
import {dirname, join} from 'node:path';
|
||||
import {additionsDiff} from '../lib/diff.mjs';
|
||||
import {authorForPath, groupPathsByAuthor, REPO_ROOT} from '../lib/personas.mjs';
|
||||
import {
|
||||
@@ -20,8 +23,12 @@ import {
|
||||
missingGroupTargets,
|
||||
normalizeDocsPath,
|
||||
parseMaxRevisions,
|
||||
parseSummaryBlock,
|
||||
PREOPEN_REVIEW_NOTE,
|
||||
renderPrBody,
|
||||
authorLoop,
|
||||
} from './author-loop.mjs';
|
||||
import {draftTitle} from './draft-title.mjs';
|
||||
import {MARKER, buildComment, decide} from '../gap/gap.mjs';
|
||||
|
||||
test('allowlist accepts hand-authored content roots', () => {
|
||||
@@ -482,7 +489,7 @@ test('authorLoop keeps prior files when a revision drops the version anchor', as
|
||||
};
|
||||
}
|
||||
authorCalls += 1;
|
||||
if (prompt.includes('Revise the pages')) {
|
||||
if (prompt.includes('Revise the text this draft added')) {
|
||||
if (prompt.includes('current-draft path=' + path) && prompt.includes('From Mattermost v11.7')) {
|
||||
sawCurrentDraft = true;
|
||||
}
|
||||
@@ -503,3 +510,151 @@ test('authorLoop keeps prior files when a revision drops the version anchor', as
|
||||
assert.match(files[0].content, /From Mattermost v11\.7/);
|
||||
assert.ok(trail.openConcerns.some((c) => /Revision rejected/.test(c.summary)));
|
||||
});
|
||||
|
||||
test('draftTitle uses the source subject and stays a single line', () => {
|
||||
assert.equal(
|
||||
draftTitle('docs: ship offline documentation in the release tarball'),
|
||||
'docs: AI draft for ship offline documentation in the release tarball',
|
||||
);
|
||||
assert.equal(draftTitle('Add the foo setting'), 'docs: AI draft for Add the foo setting');
|
||||
assert.equal(draftTitle('docs:\n\nship\rdocs'), 'docs: AI draft for ship docs');
|
||||
assert.equal(draftTitle(' '), 'docs: AI draft for documentation update');
|
||||
assert.equal(draftTitle(''), 'docs: AI draft for documentation update');
|
||||
const long = draftTitle(`docs: ${'word '.repeat(80)}`);
|
||||
assert.ok(long.length <= 240);
|
||||
assert.ok(long.startsWith('docs: AI draft for '));
|
||||
assert.ok(!long.includes('\n'));
|
||||
});
|
||||
|
||||
test('parseSummaryBlock reads the summary fence and drops forged markers', () => {
|
||||
const text = [
|
||||
'```summary',
|
||||
'- Open /_documentation/ on a running server.',
|
||||
'<!-- docs-ai-source-pr:999 -->',
|
||||
'```',
|
||||
'',
|
||||
'````mdx path=docs/main/a.mdx',
|
||||
'From Mattermost v12.1, hi.',
|
||||
'````',
|
||||
].join('\n');
|
||||
const summary = parseSummaryBlock(text);
|
||||
assert.match(summary, /_documentation/);
|
||||
assert.doesNotMatch(summary, /docs-ai-source-pr/);
|
||||
assert.equal(parseSummaryBlock('no fence'), '');
|
||||
});
|
||||
|
||||
test('renderPrBody describes the change and omits the review trail', () => {
|
||||
const body = renderPrBody({
|
||||
sourcePr: '38957',
|
||||
summary: '- On a running server, open /_documentation/.',
|
||||
milestoneVersion: 'v12.1',
|
||||
});
|
||||
assert.match(body, /<!-- docs-ai-source-pr:38957 -->/);
|
||||
assert.match(body, /## Summary/);
|
||||
assert.match(body, /_documentation/);
|
||||
assert.doesNotMatch(body, /## Gap brief/);
|
||||
assert.doesNotMatch(body, /Pre-open review trail/);
|
||||
assert.doesNotMatch(body, /Open concerns/);
|
||||
assert.match(PREOPEN_REVIEW_NOTE, /exact error strings/);
|
||||
|
||||
const missing = renderPrBody({sourcePr: '1', summary: '', milestoneVersion: ''});
|
||||
assert.match(missing, /NOT PRESENT — REQUIRES HUMAN JUDGMENT/);
|
||||
assert.match(missing, /did not include a change summary/);
|
||||
|
||||
const long = renderPrBody({
|
||||
sourcePr: '1',
|
||||
summary: 'y'.repeat(2500),
|
||||
milestoneVersion: 'v12.1',
|
||||
});
|
||||
assert.match(long, /y{2000}\.\.\./);
|
||||
assert.doesNotMatch(long, /y{2001}/);
|
||||
});
|
||||
|
||||
test('authorLoop reviews only the inserted section of an existing page', async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'docs-writer-'));
|
||||
const rel = 'docs/main/administration-guide/manage/command-line-tools.mdx';
|
||||
const abs = join(dir, rel);
|
||||
mkdirSync(dirname(abs), {recursive: true});
|
||||
const original = [
|
||||
'---',
|
||||
'title: "Command line tools"',
|
||||
'---',
|
||||
'',
|
||||
'## Compliance export',
|
||||
'',
|
||||
'Existing export text.',
|
||||
'',
|
||||
'## Server operations',
|
||||
'',
|
||||
'Existing server text.',
|
||||
'',
|
||||
].join('\n');
|
||||
writeFileSync(abs, original);
|
||||
|
||||
const updated = original.replace(
|
||||
'## Server operations',
|
||||
[
|
||||
'## Read the bundled documentation',
|
||||
'',
|
||||
'From Mattermost v12.1, open `/_documentation/` on a running server.',
|
||||
'',
|
||||
'## mattermost docs',
|
||||
'',
|
||||
'From Mattermost v12.1, serve the bundle when the server is down.',
|
||||
'',
|
||||
'## Server operations',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
let reviewDiff = '';
|
||||
const completeFn = async ({userPrompt}) => {
|
||||
const prompt = String(userPrompt || '');
|
||||
if (prompt.includes('Which reviewers apply?')) {
|
||||
return {text: '{"personas": []}', usage: {}};
|
||||
}
|
||||
if (prompt.includes('Review the diff above')) {
|
||||
reviewDiff = prompt;
|
||||
return {
|
||||
text: JSON.stringify({verdict: 'APPROVE', summary: 'ok', feedback: []}),
|
||||
usage: {},
|
||||
};
|
||||
}
|
||||
return {
|
||||
text: [
|
||||
'```summary',
|
||||
'- On a running server, open /_documentation/.',
|
||||
'- When the server is down, use mattermost docs.',
|
||||
'```',
|
||||
'',
|
||||
'````mdx path=' + rel,
|
||||
updated.trimEnd(),
|
||||
'````',
|
||||
].join('\n'),
|
||||
usage: {},
|
||||
};
|
||||
};
|
||||
|
||||
try {
|
||||
const {files, summary} = await authorLoop({
|
||||
brief: {
|
||||
summary: 'offline docs',
|
||||
actions: ['Document the /_documentation/ route on a running server'],
|
||||
targetPaths: [rel],
|
||||
},
|
||||
input: {milestoneVersion: 'v12.1', repoRoot: dir, prTitle: 'docs: ship offline docs'},
|
||||
completeFn,
|
||||
});
|
||||
|
||||
assert.match(summary, /mattermost docs/);
|
||||
assert.match(files[0].content, /## Read the bundled documentation/);
|
||||
assert.match(reviewDiff, /required-capabilities/);
|
||||
assert.match(reviewDiff, /Document the \/_documentation\/ route/);
|
||||
assert.match(reviewDiff, /exact error strings/);
|
||||
assert.match(reviewDiff, /\+## Read the bundled documentation/);
|
||||
assert.match(reviewDiff, /\+## mattermost docs/);
|
||||
assert.doesNotMatch(reviewDiff, /\+## Compliance export/);
|
||||
assert.doesNotMatch(reviewDiff, /\+title: "Command line tools"/);
|
||||
} finally {
|
||||
rmSync(dir, {recursive: true, force: true});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -237,7 +237,6 @@ jobs:
|
||||
id: push
|
||||
env:
|
||||
BRANCH: ${{ steps.pr.outputs.branch }}
|
||||
PR_NUMBER: ${{ steps.pr.outputs.pr_number }}
|
||||
GH_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
@@ -248,7 +247,13 @@ jobs:
|
||||
echo "::error::Writer produced no file changes."
|
||||
exit 1
|
||||
fi
|
||||
printf 'docs: AI draft for #%s\n' "$PR_NUMBER" | git commit -F -
|
||||
title_file=".docs-ai/writer/draft-title.txt"
|
||||
if [ ! -s "$title_file" ]; then
|
||||
echo "::error::Writer did not emit a draft title."
|
||||
exit 1
|
||||
fi
|
||||
TITLE=$(tr -d '\n' < "$title_file")
|
||||
printf '%s\n' "$TITLE" | git commit -F -
|
||||
if git ls-remote --exit-code origin "refs/heads/${BRANCH}" >/dev/null 2>&1; then
|
||||
git push origin "HEAD:refs/heads/${BRANCH}" --force-with-lease
|
||||
else
|
||||
@@ -260,17 +265,21 @@ jobs:
|
||||
env:
|
||||
GH_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
BRANCH: ${{ steps.pr.outputs.branch }}
|
||||
PR_NUMBER: ${{ steps.pr.outputs.pr_number }}
|
||||
REVIEWER: ${{ steps.pr.outputs.reviewer }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
EXISTING=$(gh pr list --head "$BRANCH" --state open --json number,url --jq 'first // empty')
|
||||
TITLE="docs: AI draft for #${PR_NUMBER}"
|
||||
title_file=".docs-ai/writer/draft-title.txt"
|
||||
if [ ! -s "$title_file" ]; then
|
||||
echo "::error::Writer did not emit a draft title."
|
||||
exit 1
|
||||
fi
|
||||
TITLE=$(tr -d '\n' < "$title_file")
|
||||
|
||||
if [ -n "$EXISTING" ]; then
|
||||
NUM=$(echo "$EXISTING" | jq -r .number)
|
||||
URL=$(echo "$EXISTING" | jq -r .url)
|
||||
gh pr edit "$NUM" --body-file .docs-ai/writer/pr-body.md
|
||||
gh pr edit "$NUM" --title "$TITLE" --body-file .docs-ai/writer/pr-body.md
|
||||
else
|
||||
URL=$(gh pr create \
|
||||
--base master \
|
||||
|
||||
Reference in New Issue
Block a user