Skip to content
Merged
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
3 changes: 1 addition & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@
- [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

## Documentation Writer Review
<!-- Required for code and documentation changes after the changes and applicable validation are complete. Keep one review checkbox and one instance of each visible or hidden field. For Evidence, list changed documentation paths. For documentation-only changes, also state that the writing rules and documentation style were reviewed. For other results, explain why no documentation change is needed or why the review is blocked. For Agent, use a consistent product and surface name, such as Codex Desktop, Codex CLI, Claude Code, or Cursor. Record this PR number. After committing all review changes, put `git rev-parse --short HEAD` and `git rev-parse --short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review and refresh that metadata after any new commit. This receipt is advisory during the data-collection pilot. -->
<!-- Required for code and documentation changes after the changes and applicable validation are complete. Keep one review checkbox and one instance of each visible or hidden field. For Evidence, list changed documentation paths. For documentation-only changes, also state that the writing rules and documentation style were reviewed. For other results, explain why no documentation change is needed or why the review is blocked. For Agent, use a consistent product and surface name, such as Codex Desktop, Codex CLI, Claude Code, or Cursor. After committing all review changes, put `git rev-parse --short HEAD` and `git rev-parse --short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review and refresh that metadata after any new commit. This receipt is advisory during the data-collection pilot. -->
- [ ] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated` | `no-docs-needed` | `blocked`
- Evidence:
- Agent:
- PR: #<number>
<!-- docs-review-head-sha: -->
<!-- docs-review-agents-blob-sha: -->

Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ Follow `.agents/skills/_shared/pr-follow-up.md`: after opening or pushing to a P

- Treat `docs/` as the source of truth for user-facing documentation and follow `docs/CONTRIBUTING.md`.
- After completing code or documentation changes, run a documentation writer subagent before final handoff. Give it the changed files, change summary, and test or docs-build evidence. For documentation-only changes, ask it to verify the writing rules and documentation style.
- After the review, complete the PR template's Documentation Writer Review section. Record the result, evidence, agent surface, and PR number. Put the reviewed head SHA and current `AGENTS.md` blob SHA in the template's hidden metadata comments.
- After the review, complete the PR template's Documentation Writer Review section. Record the result, evidence, and agent surface. Put the reviewed head SHA and current `AGENTS.md` blob SHA in the template's hidden metadata comments.
- If any commit changes the pull-request head after the hidden head SHA, rerun the documentation writer review and refresh the hidden metadata. The receipt check runs again when new commits are pushed.
- For normal docs changes, include source pages under `docs/`.
- Update `.agents/skills/nemoclaw-user-guide/SKILL.md` only when the AI-agent docs routing guidance changes.
Expand Down
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ Record one result:

Record the product and surface that ran the review, such as `Codex Desktop`, `Codex CLI`, `Claude Code`, or `Cursor`.
Use the same name for the same surface across PRs so the report groups its data correctly.
Record the PR number in the visible receipt so the check can detect a receipt copied from another PR.

Commit all changes from the final review.
Then run these commands and put their values in the receipt's hidden HTML metadata comments:
Expand All @@ -428,11 +427,12 @@ git rev-parse --short HEAD
git rev-parse --short HEAD:AGENTS.md
```

The visible PR number ties the receipt to this PR, while the hidden head SHA identifies the pull-request revision that the review covered.
GitHub supplies the pull-request identity to the workflow and report.
The hidden head SHA identifies the pull-request revision that the review covered.
Rerun the review after any new commit changes the pull-request head.
Pushing a new commit runs the receipt check again and reports the review as stale until the hidden metadata is refreshed.
The Documentation Writer Review check reports an advisory finding when the receipt is missing, incomplete, or stale.
The check compares the receipt's PR number with the current PR number, the hidden head SHA with the current PR head, and the hidden `AGENTS.md` blob SHA with the current PR's file.
The check compares the hidden head SHA with the current PR head and the hidden `AGENTS.md` blob SHA with the current PR's file.

Maintainers can export receipt data from PR descriptions:

Expand All @@ -441,7 +441,7 @@ npm run docs-review:report -- --since 2026-06-12 --format csv > /tmp/nemoclaw-do
```

The report uses the authenticated GitHub CLI session and returns JSON by default.
It measures receipt coverage, PR-number integrity, head-revision freshness, review results, and agent-surface counts.
It measures receipt coverage, head-revision freshness, review results, and agent-surface counts.
The `eligiblePrs` JSON metric reports the total eligible pull requests.
The `eligibleCodePrs` and `eligibleDocsOnlyPrs` metrics report the code and documentation-only counts.
It records the `AGENTS.md` blob SHA, but only the PR check compares that SHA with the current PR's file.
Expand Down
7 changes: 3 additions & 4 deletions docs/resources/engineer-agentic-documentation.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,10 @@ NemoClaw records the following fields in the Documentation Writer Review section
- One result: `docs-updated`, `no-docs-needed`, or `blocked`.
- Evidence that names the changed documentation or explains the result.
- The agent product and surface that ran the review.
- The pull-request number.
- Hidden metadata for the reviewed head SHA and the `AGENTS.md` blob SHA.

Keep one instance of each field.
The pull-request number detects a receipt copied from another pull request.
GitHub supplies the pull-request identity to the workflow and report.
The head SHA binds the review to one pull-request revision.
The `AGENTS.md` blob SHA records which repository instruction version the review covered.

Expand All @@ -109,7 +108,7 @@ Use the following sequence for a pull request that changes code or documentation
6. Rerun the documentation review after any later commit.

The `CI / Documentation Writer Review` workflow runs when a pull request opens or reopens, when its description is edited, or when it receives a new commit.
It compares the receipt with the pull-request number, current head, and current `AGENTS.md` blob.
It compares the receipt with the current head and current `AGENTS.md` blob.
A new commit makes the previous receipt stale until the documentation writer subagent runs again and refreshes the metadata.
The checker evaluates code-changing and documentation-only pull requests.
For documentation-only changes, the receipt records the agent's review of writing rules and documentation style.
Expand Down Expand Up @@ -581,7 +580,7 @@ Before calling a documentation system agent-ready, confirm the following conditi
- Routing prompts and skills point to the source instead of copying it.
- Variant generation is deterministic and variant behavior is tested.
- Agent workflows define scope, approvals, stop conditions, and completion evidence.
- Code-changing and documentation-only pull requests record a documentation-review result tied to the pull-request number, current head, and instruction version.
- Code-changing and documentation-only pull requests record a documentation-review result tied to the current head and instruction version.
- New commits invalidate the recorded review until the documentation writer subagent runs again.
- Pull requests validate generated output, routes, links, product parity, and rendered previews.
- Release publication uses an approved immutable revision.
Expand Down
42 changes: 2 additions & 40 deletions scripts/docs-review-receipt.mts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ interface ParsedReceipt {
duplicateFields: string[];
duplicateSections: boolean;
evidence: string | null;
prNumber: number | null;
present: boolean;
result: ReviewResult | null;
reviewedHeadSha: string | null;
Expand All @@ -41,8 +40,6 @@ interface ReceiptRecord {
evidence: string | null;
headShaMatches: boolean | null;
issues: string[];
prNumber: number | null;
prNumberMatches: boolean | null;
result: ReviewResult | null;
reviewedHeadSha: string | null;
status: ReceiptStatus;
Expand Down Expand Up @@ -116,7 +113,6 @@ function runCheck(args: string[]): void {
const record = evaluateReceipt(
pullRequest.body ?? "",
classifyChangedFiles(changedFiles),
pullRequest.number ?? null,
headPrSha,
expectedAgentsBlob,
);
Expand Down Expand Up @@ -172,7 +168,6 @@ function runReport(args: string[]): void {
function evaluateReceipt(
body: string,
changes: ChangeClassification,
expectedPrNumber: number | null,
headPrSha: string,
expectedAgentsBlob?: string,
): ReceiptRecord {
Expand All @@ -191,8 +186,6 @@ function evaluateReceipt(
evidence: parsed.evidence,
headShaMatches: null,
issues,
prNumber: parsed.prNumber,
prNumberMatches: null,
result: parsed.result,
reviewedHeadSha: parsed.reviewedHeadSha,
status: "unclassified",
Expand All @@ -209,8 +202,6 @@ function evaluateReceipt(
evidence: parsed.evidence,
headShaMatches: null,
issues,
prNumber: parsed.prNumber,
prNumberMatches: null,
result: parsed.result,
reviewedHeadSha: parsed.reviewedHeadSha,
status: "not-required",
Expand Down Expand Up @@ -242,9 +233,6 @@ function evaluateReceipt(
if (!parsed.agent || looksLikePlaceholder(parsed.agent)) {
issues.push("Record the agent surface that ran the documentation writer review.");
}
if (parsed.prNumber === null) {
issues.push("Record this pull request number in the documentation review receipt.");
}
if (!parsed.reviewedHeadSha) {
issues.push("Refresh the hidden head SHA after the documentation writer review.");
}
Expand All @@ -256,14 +244,6 @@ function evaluateReceipt(
}
}

const prNumberMatches =
parsed.prNumber !== null && expectedPrNumber !== null
? parsed.prNumber === expectedPrNumber
: null;
if (prNumberMatches === false) {
issues.push("The receipt PR number does not match this pull request.");
}

const headShaMatches = parsed.reviewedHeadSha
? headPrSha.toLowerCase().startsWith(parsed.reviewedHeadSha)
: null;
Expand All @@ -289,8 +269,6 @@ function evaluateReceipt(
evidence: parsed.evidence,
headShaMatches,
issues,
prNumber: parsed.prNumber,
prNumberMatches,
result: parsed.result,
reviewedHeadSha: parsed.reviewedHeadSha,
status: parsed.present ? (issues.length === 0 ? "valid" : "invalid") : "missing",
Expand All @@ -308,7 +286,6 @@ function parseReceipt(body: string): ParsedReceipt {
duplicateFields: [],
duplicateSections: false,
evidence: null,
prNumber: null,
present: false,
result: null,
reviewedHeadSha: null,
Expand All @@ -325,7 +302,7 @@ function parseReceipt(body: string): ParsedReceipt {
/^- \[[ xX]\] Documentation writer subagent reviewed the completed (?:changes|implementation)$/u;
const completionPattern =
/^- \[[xX]\] Documentation writer subagent reviewed the completed (?:changes|implementation)$/u;
const duplicateFields = ["Result", "Evidence", "Agent", "PR"].filter(
const duplicateFields = ["Result", "Evidence", "Agent"].filter(
(name) => fieldValues(lines, name).length > 1,
);
for (const name of ["docs-review-head-sha", "docs-review-agents-blob-sha"]) {
Expand All @@ -340,7 +317,6 @@ function parseReceipt(body: string): ParsedReceipt {
resultMatch && RESULTS.has(resultMatch[1] as ReviewResult)
? (resultMatch[1] as ReviewResult)
: null;
const prNumber = parsePrNumber(fieldValue(lines, "PR"));
const reviewedHeadSha = parseSha(hiddenFieldValue(lines, "docs-review-head-sha"));
const agentsBlobSha = parseSha(hiddenFieldValue(lines, "docs-review-agents-blob-sha"));

Expand All @@ -351,7 +327,6 @@ function parseReceipt(body: string): ParsedReceipt {
duplicateFields,
duplicateSections: matches.length > 1,
evidence: nonEmpty(fieldValue(lines, "Evidence")),
prNumber,
present: true,
result,
reviewedHeadSha,
Expand Down Expand Up @@ -381,12 +356,6 @@ function hiddenFieldValues(lines: string[], name: string): string[] {
.map((line) => line.slice(prefix.length, -suffix.length).trim());
}

function parsePrNumber(value: string | null): number | null {
const normalized = value?.trim();
if (!normalized || !/^#[1-9]\d*$/u.test(normalized)) return null;
return Number(normalized.slice(1));
}

function parseSha(value: string | null): string | null {
const normalized = value?.replace(/^`|`$/gu, "").trim().toLowerCase();
return normalized && SHA_PATTERN.test(normalized) ? normalized : null;
Expand Down Expand Up @@ -414,7 +383,6 @@ function toReportRecord(pullRequest: GhPullRequest): ReportRecord {
...evaluateReceipt(
pullRequest.body ?? "",
classifyPrType(pullRequest.body ?? ""),
pullRequest.number,
pullRequest.headRefOid,
),
};
Expand Down Expand Up @@ -517,9 +485,7 @@ function buildReport(repository: string, since: string, through: string, records
);
const recorded = eligible.filter((record) => record.status !== "missing");
const valid = eligible.filter((record) => record.status === "valid");
const fresh = recorded.filter(
(record) => record.prNumberMatches === true && record.headShaMatches === true,
);
const fresh = recorded.filter((record) => record.headShaMatches === true);
const resultCounts: Record<ReviewResult, number> = {
blocked: 0,
"docs-updated": 0,
Expand Down Expand Up @@ -575,8 +541,6 @@ function renderCsv(records: ReportRecord[]): string {
"receipt_status",
"result",
"agent",
"receipt_pr_number",
"pr_number_matches",
"reviewed_head_sha",
"head_pr_sha",
"head_sha_matches",
Expand All @@ -597,8 +561,6 @@ function renderCsv(records: ReportRecord[]): string {
record.status,
record.result,
record.agent,
record.prNumber,
record.prNumberMatches,
record.reviewedHeadSha,
record.headPrSha,
record.headShaMatches,
Expand Down
21 changes: 9 additions & 12 deletions test/docs-review-receipt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ function receipt(overrides: Partial<Record<string, string>> = {}): string {
result: "`docs-updated`",
evidence: "Updated docs/get-started/quickstart.mdx.",
agent: "Codex",
prNumber: "#42",
headSha: HEAD_SHA.slice(0, 12),
agentsSha: AGENTS_BLOB_SHA.slice(0, 12),
...overrides,
Expand All @@ -29,7 +28,6 @@ function receipt(overrides: Partial<Record<string, string>> = {}): string {
- Result: ${values.result}
- Evidence: ${values.evidence}
- Agent: ${values.agent}
- PR: ${values.prNumber}
<!-- docs-review-head-sha: ${values.headSha} -->
<!-- docs-review-agents-blob-sha: ${values.agentsSha} -->
`;
Expand Down Expand Up @@ -99,8 +97,6 @@ describe("documentation writer review receipt", () => {
status: "valid",
result: "docs-updated",
agent: "Codex",
prNumber: 42,
prNumberMatches: true,
headShaMatches: true,
agentsShaMatches: true,
issues: [],
Expand Down Expand Up @@ -171,9 +167,11 @@ describe("documentation writer review receipt", () => {
expect(result.stderr).toContain("change code or documentation must include");
});

it("accepts the completed-implementation wording in historical receipts", () => {
it("accepts historical receipts with completed-implementation wording and a PR field", () => {
const result = runCheck(
receipt().replace("reviewed the completed changes", "reviewed the completed implementation"),
receipt()
.replace("reviewed the completed changes", "reviewed the completed implementation")
.replace("- Agent: Codex", "- Agent: Codex\n- PR: #999"),
["src/lib/example.ts", "docs/index.mdx"],
);

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -209,12 +207,11 @@ describe("documentation writer review receipt", () => {
);
});

it("reports a copied PR number and stale head and AGENTS.md revisions", () => {
it("reports stale head and AGENTS.md revisions", () => {
const result = runCheck(
receipt({
result: "`no-docs-needed`",
evidence: "The change affects an internal test helper only.",
prNumber: "#41",
headSha: "c".repeat(12),
agentsSha: "d".repeat(12),
}),
Expand All @@ -224,7 +221,6 @@ describe("documentation writer review receipt", () => {
expect(result.output.status).toBe("invalid");
expect(result.output.issues).toEqual(
expect.arrayContaining([
"The receipt PR number does not match this pull request.",
"The documentation writer review is stale after a new commit.",
"The reviewed AGENTS.md blob SHA does not match the pull request version.",
]),
Expand All @@ -247,15 +243,14 @@ describe("documentation writer review receipt", () => {
result: "`docs-updated` | `no-docs-needed` | `blocked`",
evidence: "",
agent: "<Codex | Claude Code | Cursor | other>",
prNumber: "#<number>",
headSha: "",
agentsSha: "",
}),
["src/lib/example.ts"],
);

expect(result.output.status).toBe("invalid");
expect(result.output.issues).toHaveLength(7);
expect(result.output.issues).toHaveLength(6);
});
});

Expand All @@ -279,7 +274,7 @@ describe("documentation writer review report", () => {

- [x] Code change with doc updates

${receipt({ evidence: "=1+1", prNumber: "#1" })}`,
${receipt({ evidence: "=1+1" })}`,
files: [{ path: "src/lib/example.ts" }, { path: "docs/index.mdx" }],
},
{
Expand Down Expand Up @@ -380,6 +375,8 @@ printf '%s' '${JSON.stringify(pullRequests)}'
);
expect(csvResult.status).toBe(0);
expect(csvResult.stdout).toContain("receipt_status");
expect(csvResult.stdout).not.toContain("receipt_pr_number");
expect(csvResult.stdout).not.toContain("pr_number_matches");
expect(csvResult.stdout).toContain("1,https://github.com/NVIDIA/NemoClaw/pull/1");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(csvResult.stdout).toContain("2,https://github.com/NVIDIA/NemoClaw/pull/2");
expect(csvResult.stdout).toContain("'=1+1");
Expand Down
Loading