docs: fold PR writing into the human-facing guide and align it with the ai-facing style #29
Loading…
Reference in a new issue
No description provided.
Delete branch "docs/absorb-pr-writing"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Absorbs the standalone
pr-writing-styledraft intohuman-facing-writing-styleas a## Pull requestssection (titles, body shape, what gets cut, references) and applies the ai-facing writing guide to the whole file. PR prose is human-facing prose; two skills would give one meaning two homes.Cuts, safe because their content survives elsewhere: the
# Goal/# Reference/# Taskscaffold (precedence and output shape moved to the opening), the Self-check recap (reconstructible; its two non-derivable tests rehomed into Specificity and Revise), and cross-section duplicates — source attribution now lives only in Specificity, the backtick-code-names rule in Consistency, throat-clearing owns its substitution-table rows.The draft's employer-internal identifiers are anonymized for this public repo (ticket prefixes, a product question, environment and ingress names), and its Linear-specific ticket policy is generalized to tracker-conditional. The description trigger list is consolidated to one phrase per distinct situation and gains the PR branch.
Verified by a three-lens adversarial review (ai-facing conformance, source preservation against a cut ledger, cold stranger read); 17 of its 20 findings are applied in this diff, including three self-violations — the curly-quotes example held straight quotes, a
Note that…opener, a tailing-negation fragment.Follow-up once merged: repoint the align corpus rule
rules/ai-coding-workflow/conventional-commits.md, which still names the never-installedpr-writing-style, at this skill.Approach review: The approach is sound. Folding the standalone
pr-writing-styledraft intohuman-facing-writing-styleas a## Pull requestssection gives one meaning one home, which is exactly what the project's ownai-facing-writing-styleNon-Duplication Principle asks for — and the existingcreate-prs-with-ghskill is aboutghshell-quoting mechanics, not writing style, so there is no overlap or competing utility to prefer.The three structural edits all align with the ai-facing guide this PR says it is conforming to:
# Goal/# Reference/# Taskscaffold matches "There is no fixed template for any artifact beyond what Artifact Types and User Input Precedence call for." The one non-derivable directive ("return the revised text first") is correctly preserved in the opening paragraph rather than lost.## Pull requestssection applies the guide's established principles (Concision, Specificity) to the PR-specific context. That is "group by concept," not restatement — it does not duplicate the rest of the file.The anonymization of employer-internal identifiers and the generalization of the Linear-specific ticket policy to tracker-conditional are appropriate for a public repo and don't change the approach.
One alternative I considered and rejected: splitting the ~65-line PR section into a
references/pull-requests.mdsupporting file under the reach principle ("inline what every use of the skill needs; push behind a pointer what only some branches read"). That is a judgment call, not a clearly preferable one — the file is moderate-sized (317 lines total), and co-locating the PR applications with the principles they apply aids comprehension. Not a material improvement.The approach looks good.
Approach review by Approach Review 3 (OpenCode Wafer) (GLM-5.2)
Approach review: Folding PR writing into
human-facing-writing-styleis the right call — PR prose is human-facing prose, and the alternative (apr-writing-styleskill) would have split one style voice across two descriptions competing for the same routing decision. The deletions are well-justified: the# Goal/# Reference/# Taskscaffold and the Self-check recap are exactly what the sibling ai-facing guide's Structure and Non-Duplication sections call sediment, and the cross-section duplicates each keep one home.Two placement points, both about where the merged material lives rather than whether it should exist:
## Pull requestssection is ~7.4 KB of a 25.9 KBSKILL.md(66 of 317 lines) and only some branches of the skill read it — someone editing a README or a changelog pays for it on every invocation.skills/ai-facing-writing-style/SKILL.md:121states the split rule this repo maintains: inline what every use needs, push behind a pointer what only some branches read.references/supporting files already have precedent here (verify-unixy-cli,css-text-box-trim).create-prs-with-ghalready advertises the trigger "draft a PR description" while containing onlyghquoting mechanics, so that request can now route to the skill that has nothing to say about it. Worth adding to the follow-up list already in the description.Neither blocks the merge.
Approach review by Claude Code Opus (opus)
@ -1,17 +1,9 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".Adding "write the PR body" here creates a routing overlap with
create-prs-with-gh, whose description ends with the trigger "draft a PR description" but whose body is entirelyghquoting mechanics — no guidance on what the body should say. An agent asked to draft a PR description can now match that skill and never load this one.The two skills are complementary rather than duplicative (content vs. shell safety), so the fix is a pointer, not a merge: repoint
create-prs-with-gh's trigger at composition here, or have it name this skill. Reasonable as a follow-up alongside therules/ai-coding-workflow/conventional-commits.mdrepoint already listed in the description.@ -286,2 +250,3 @@4. Ask "what still sounds AI-generated here?" and fix the remaining tells.# Task## Pull requestsConsider moving this section to
references/pull-requests.mdbehind a strong pointer, keeping a one- or two-line summary here.The merge itself is right — the question is only inline vs. pointer. This section is ~7.4 KB of the file's 25.9 KB, and it serves one branch of the skill: editing a changelog, a README, or a blog post never reads it, but loads it every time. That is the reach test in
skills/ai-facing-writing-style/SKILL.md:121("inline what every use of the skill needs; push behind a pointer what only some branches read"), and the same guide notes supporting files cost nothing until their pointer fires. The frontmatter already names "write the PR body" as a trigger, so the branch is explicit enough to point at: "Before writing a PR title or body, readreferences/pull-requests.mdin full."verify-unixy-cliandcss-text-box-trimalready carryreferences/directories, so the layout is established here.Approach review: Folding PR guidance into
human-facing-writing-styleinstead of shippingpr-writing-styleas a second skill is the right call. PR prose is human-facing prose, and the general sections — Clarity, Concision, Specificity, Voice, the AI tells — all apply to it, so a separate skill would either have duplicated that base or required both skills to load. The cuts are sound for the same reason: the Self-check list was reconstructible from the prose above it, and the cross-section duplicates (source attribution, backticked code names, throat-clearing) now each have a single home.One placement alternative, left as a comment on the diff: the PR material is branch content living in the always-loaded
SKILL.mdbody, where this repo's ownai-facing-writing-styleguide and the existingreferences//rules/precedent both point to a pointer-gated supporting file. That preserves the "one meaning, one home" argument — it stays one skill.Secondary, not blocking and outside this diff:
create-prs-with-ghalready lists "draft a PR description" among its triggers while covering onlyghquoting mechanics, and this skill's description now adds "write the PR body". The two compete for the same request; a line in one naming the other (prose here, invocation there) would settle the routing. Worth pairing with the align-corpus repoint already noted as follow-up.Approach review by Claude Code Opus (opus)
@ -286,2 +250,3 @@4. Ask "what still sounds AI-generated here?" and fix the remaining tells.# Task## Pull requestsThe
## Pull requestsblock is ~7.5 KB of the now-26 KBSKILL.md, and it only applies on one branch: the artifact is a pull request. Every other invocation (docs, release notes, changelogs, blog posts) pays those tokens and never uses them.skills/ai-facing-writing-style/SKILL.mdstates the split this repo follows: "inline what every use of the skill needs; push behind a pointer what only some branches read" — and the layout already exists elsewhere (skills/css-text-box-trim/references/,skills/typescript-magician/rules/,skills/improve-codebase-architecture/*.md).Consider keeping the fold exactly as argued — one skill, one home for the meaning — but moving Titles / Body / What gets cut / References into
skills/human-facing-writing-style/references/pull-requests.md, gated by a strong pointer here: "When the artifact is a pull request, readreferences/pull-requests.mdin full before writing." The dependency runs one way (PR prose needs the general guide; the general guide never needs the PR rules), so the PR branch reads both files and every other branch drops ~29% of the always-loaded body. Per the same guide, the pointer has to be the imperative form — a bare "see …" would leave the branch unserved.Summary: Documentation-only change to one skill file; found 1 medium and 3 low issues, all internal-consistency defects in the new
## Pull requestssection.I audited the PR's central risk — that the removed
Self-checklist, the# Goal/# Reference/# Taskscaffold, and the deleted cross-section duplicates take content with them — by tracing each cut line to its claimed new home. That ledger holds: all 14 self-check items land in Clarity, Concision, Specificity, Voice, Consistency, Structure, or the new Revise list; "if a sentence works without a word" survives as "every word should earn its place" in the opening; "Match tone to where the text will be read" survives as "Voice is artifact-dependent"; the droppednote that/serves assubstitution rows are covered by the throat-clearing and copula bullets. The frontmatter still parses as a plain YAML scalar (no:or#in the value), the double-backtick title example on line 274 renders as intended, and no other file in the repo references the removed headings or apr-writing-styleskill, so nothing dangles.The findings are places where the new PR guidance collides with a rule elsewhere in the same file or with a repository's own instructions.
Code review by Claude Code Opus (opus)
@ -295,0 +264,4 @@- Before: `ABC-271 fix tracing`- After: `fix(ABC-271): gateway spans should continue the caller's trace instead of starting a new root`- Before: `fix(config): preconfigured questions should match the featured prompts they were built for`- After: `fix(config): update cached questions to match the UI`🟢 Low: The shape list eleven lines up prescribes "X should Y" for behavior corrections, but this
fix(...)After drops "should" entirely — and the Before it replaces is rejected only for narrating justification ("…they were built for"). An agent applying the shapes literally cannot tell whether a correction keeps "should". Either make the minimal fix (fix(config): preconfigured questions should match the featured prompts) or say that this change is a sync and therefore takes the mechanical shape.@ -295,0 +266,4 @@- Before: `fix(config): preconfigured questions should match the featured prompts they were built for`- After: `fix(config): update cached questions to match the UI`Name things as they exist. Never coin a slug that reads like a real identifier — quote the actual text instead (`the preconfigured "top 5 users by volume" answer`, not `top-users-answer`, which looks like a config key that doesn't exist).🟢 Low:
the preconfigured "top 5 users by volume" answeris prose, not a code-like name, so backticking it contradicts the Consistency rule this PR adds at line 98 (backticks for what exists verbatim in code, config, or a URL; italics for quoted user questions). It also works against the sentence's own point: the example that warns "don't coin something that reads like a real identifier" renders its good version as an identifier. Drop the backticks and italicise the quoted question instead.@ -295,0 +288,4 @@- Before: "Forward-only: `staging` rows persisted before this change keep their tagged URLs and age out; no backfill."- After: "Responses saved before this change keep their tagged URLs. Only new responses are stripped."Where the project tracks work in tickets, every PR links one; the ticket lives in the title scope, never in the body. Several small PRs may share one general ticket.🟢 Low: Two things an agent can't resolve from the text. (1) The title examples use the Conventional Commits scope for a ticket (
fix(ABC-271):) and for an area (fix(config):), while this sentence says the ticket "lives in the title scope" — in a repo that tracks tickets and scopes by area, which one goes in the parentheses? (2) "every PR links one" reads as contradicting References ("link the PR, never the ticket it came from") four sections down. The two rules govern different places; saying so removes the collision, e.g. "the ticket association rides in the title scope; links in the body point at artifacts, not the tracker."@ -295,0 +302,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes, and keep only its strongest version — when "passed all 39 runs where it made this choice" exists, "I verified by re-running the query" goes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟡 Medium: This bullet tells the agent to drop a
Testing:line "even where the repo's own guidelines ask for testing notes", and line 311 tells it to delete placeholder sections (Screenshots: N/A). Repository instructions — AGENTS.md, CONTRIBUTING, a PR template — normally outrank a general style skill, so as written this makes an agent override an explicit project requirement and ship PR bodies that fail a template check or a reviewer's expectation. Narrow it: keep the advice for repos that don't require the note, and state that a repo's own PR guidelines win, the way the opening already concedes precedence to explicit user instructions.Summary: Docs-only change to a single skill file; the mechanics check out. Frontmatter parses as valid YAML, the nested double-backtick title example on line 274 renders correctly under CommonMark (inner backticks stay literal), the intentional curly quotes on line 196 are the file's only curly quotes, and nothing in the repo still references the removed
## Self-checksection or the absorbedpr-writing-styleskill. I also spot-checked the cut ledger: the# Tasksteps and the two non-derivable Self-check tests do survive in the opening, Specificity, and Revise.Found 1 medium issue, 2 low, 1 informational. The medium one is a conflict the merge introduces:
## Voicestill tells the reader PRs can carry tangents and rhythm, while the new## Pull requestssection rules that register out for the same artifact.Code review by Claude Code Opus (opus)
@ -54,3 +45,3 @@## VoiceVoice is artifact-dependent. Blog posts, release narratives, postmortems, and PRs can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide. Match tone to where the text will be read.Voice is artifact-dependent. Blog posts, release narratives, postmortems, and PRs can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide.🟡 Medium: Voice still groups PRs with the tangent-friendly genres, which now contradicts the section this PR adds. Line 47 says PRs "can carry first person, opinion, rhythm, and the occasional tangent", while
## Pull requestssays the opposite for the same artifact: "Write each fact bare, one per sentence" (line 286), cut "War story and methodology" (line 305), cut "Colorful compression" (line 308), and a conversational aside is fine only "when the reviewer genuinely needs it. Usually they don't" (line 313).An agent reading this file top-to-bottom gets opposite instructions for a PR body depending on which section it lands in, and the ai-facing guide's alignment pass names exactly this failure ("conflicts — lines that contradict one another"). Folding the two homes into one was the point of the PR, so the merged file should not keep the older, looser ruling.
Fix: drop "and PRs" from the tangent-friendly list, or move PRs to the low-friction group with a pointer to
## Pull requests.@ -106,14 +95,15 @@ Prefer: "Expect about 5% of cases to need manual review. That's down from 30%."- Use one term per concept. Don't alternate between "user", "account", and "member" for the same thing. Synonym cycling ("protagonist" / "main character" / "central figure" / "hero" in the same paragraph) is a tell, not variety.- Keep parallel items parallel: matching grammatical structure, tense, and punctuation.- Follow conventions the reader already knows: capitalization of product names, terminology from the codebase or domain.- Enclose code-like names — anything that exists verbatim in code, config, or a URL, whatever its class: a parameter, variable, flag, function — in backticks wherever they appear; italicize quoted user questions.🟢 Low: "italicize quoted user questions" arrives with no referent. Nothing else in the guide establishes what a "user question" is — the term appears here and nowhere else, so this reads as a leftover from the employer-internal draft where product questions were a known artifact.
It also runs against the one place the merged file quotes a user question: line 269 shows
the preconfigured "top 5 users by volume" answerin straight quotes inside a code span, not italics. Either give the rule a referent and an example where quoted questions actually appear, or cut it and leave the bullet to the backticking rule.@ -295,0 +264,4 @@- Before: `ABC-271 fix tracing`- After: `fix(ABC-271): gateway spans should continue the caller's trace instead of starting a new root`- Before: `fix(config): preconfigured questions should match the featured prompts they were built for`- After: `fix(config): update cached questions to match the UI`🟢 Low: This pair teaches two lessons at once and reads as a counterexample to the rule nine lines above it. Line 258 says behavior corrections take the shape "X should Y"; here a
fix(config):title that already has that shape is rewritten into an imperative. The intended lesson is the cut of the narration ("they were built for"), but the rewrite also drops "should", so a reader can't tell whetherfix:titles should use "should" or not.If the classification is what changed — a config sync is a mechanical change, not a behavior correction — say so in the line introducing the pair. Otherwise keep the shape and cut only the narration.
@ -295,0 +294,4 @@For UI changes, the screenshots are the example: a `## Before` and a `## After` section, one capture each. The pair replaces prose description of the visual change — don't narrate what the images already show.**Operational constraints always stay.** Merge or deploy ordering ("do not merge before supergraph#240 is deployed"), state already live in production ("codifies a hotfix applied by ConfigMap edit"), and upgrade or migration consequences ("upgrading releases pick up the deny-all") — one sentence each. Nothing in "What gets cut" covers these.ℹ️ The PR description says the draft's "environment and ingress names" were anonymized for this public repo, but several concrete names survive in the examples:
supergraph#240here, andeu-west-1plus themainnetanddevnetingress hosts on line 299. Some of these may be generic enough on purpose (eu-west-1is an AWS region;mainnet/devnetare standard blockchain environment names), so this is a check rather than a finding — worth confirming no repo or host name from the source draft is still recognizable.Review round 1 dispositions (fixes pushed as
f8f0681):Applied
Declined
## Pull requestsinline is the deliberate design — it mirrors the ai-facing guide's inline per-artifact sections, and approach-3 examined the samereferences/split and rejected it as not material. Noted as a future option if the section grows.create-prs-with-ghtrigger overlap is real but outside this diff — banked for the skills triage pass.Answered
supergraph#240,eu-west-1, andmainnet/devnetare generic vocabulary (a federation term, an AWS region, standard chain environment names), kept deliberately; nothing source-internal survives.This closes the planned application round for this PR; the synchronize cycle's code re-reviews will be adjudicated, but no further edit round is planned absent a defect.
Summary: Docs-only change to one skill file; found 2 medium and 4 low issues.
The absorption is sound — I checked each cut named in the description against the surviving text, and the removed Self-check list, the
# Goal/# Reference/# Taskscaffold, the dropped substitution rows, and the Honesty source-attribution bullets are all covered elsewhere in the file. The double-backtick title example on line 274 renders as intended (verified with a CommonMark renderer).The findings are internal conflicts in the new
## Pull requestssection rather than lost content: a follow-up-PR rule that overrides the "operational constraints always stay" rule stated two lines earlier, and a thesis recipe that mandates a number in a guide whose hardest rule forbids inventing one. The rest are lower-stakes: an unresolved ticket-vs-component title scope, em-dash density that contradicts the rule the same section states, a trigger collision withcreate-prs-with-gh, and an instruction to ignore repo-committed testing-note guidelines.Code review by Claude Code Opus (opus)
@ -1,17 +1,9 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".🟢 Low: The new "write the PR body" trigger overlaps
skills/create-prs-with-gh, whose description already triggers on "draft a PR description" while covering onlyghquoting mechanics, not content. A request phrased either way now routes by whichever description the model matches first, and the mechanics skill teaches nothing about what to write. Narrowing create-prs-with-gh's trigger to the quoting situation would stop the two skills competing for one request.@ -295,0 +277,4 @@**One line is the target — and the floor.** When the title already says it all, a single sentence plus any screenshot is the whole body. But never ship an empty body: a 3,000-line schema PR with no body leaves the reviewer unable to tell read-only additions from a behavior change. When the change needs more:1. **Thesis first.** One or two sentences: what the change does and the problem it answers, in plain words, with the key number. Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence.🟡 Medium: "with the key number" makes a number mandatory in every thesis, but most PRs have none, and the guide's hardest rule is "Do not invent specifics." (line 31), reinforced by "Ground every claim" on line 284. An agent that follows this literally will manufacture a plausible-looking count for a change that was never measured. Make it conditional — "with the number when the change has one". Separately, "key" as a generic adjective is on this file's own watch list (line 203).
@ -295,0 +283,4 @@Ground every claim. Examples come from real runs and data, and every other statement says only what the diff or a run you performed supports — hedge or omit the rest. An invented value is fine only when marked as illustrative ("e.g. `INV-7KQMD`").Write each fact bare, one per sentence. Grounding means the diff or a run backs the claim — not that the evidence rides inside every sentence. Do not staple to a fact a justification or precision that only the tests need: "byte-for-byte in its original order and encoding" is test detail; "preserved" is the fact. An em-dash aside, a "while" contrast, or a colon label ("Forward-only:") means the sentence is hiding two or three. Split it. Describe effects in the reader's world, not the system's:🟢 Low: This line rules that "an em-dash aside … means the sentence is hiding two or three. Split it", and line 191 lists em-dash overuse as an AI formatting tic. The section that states both rules carries 24 em dashes across roughly 40 prose lines, against 7 in the file's other 251 lines. Lines 280, 281, 305, 313, and 317 each hold two. In a style guide the prose is the demonstration, so the density reads as a counterexample to the rule being stated.
@ -295,0 +288,4 @@- Before: "Forward-only: `staging` rows persisted before this change keep their tagged URLs and age out; no backfill."- After: "Responses saved before this change keep their tagged URLs. Only new responses are stripped."Where the project tracks work in tickets, every PR names one — in the title scope, never in the body. Several small PRs may share one general ticket.🟢 Low: "in the title scope" conflicts with the second title example on line 267,
fix(config): update cached questions to match the UI, where the scope names the component instead. Both example sets come from a project that has tickets (ABC-271,ABC-289), so a reader working in a tracked repo cannot satisfy both shapes. State which wins when a change has both a ticket and an obvious component scope.@ -295,0 +296,4 @@**Operational constraints always stay.** Merge or deploy ordering ("do not merge before supergraph#240 is deployed"), state already live in production ("codifies a hotfix applied by ConfigMap edit"), and upgrade or migration consequences ("upgrading releases pick up the deny-all") — one sentence each. Nothing in "What gets cut" covers these.**Follow-up PRs skip all of the above.** When a PR fixes, finishes, or repairs the fallout of an earlier PR, one sentence referencing that PR is the whole body. "Finishes the eu-west-1 teardown started in #1164 by dropping the `mainnet` and `devnet` ingress hosts" replaces a Cause/Fix re-explanation of the whole incident.🟡 Medium: "Follow-up PRs skip all of the above" contradicts "Operational constraints always stay." two lines up. "All of the above" literally includes that paragraph, and "one sentence referencing that PR is the whole body" leaves no room for a merge-ordering or migration note. The example is the risky case itself — a region teardown dropping ingress hosts is exactly the kind of follow-up that carries "do not merge before X is deployed". Line 297's carve-out only exempts "What gets cut", which sits below and does not cover this rule. Scope it: follow-ups skip all of the above except the operational constraints.
@ -295,0 +302,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes, and keep only its strongest version — when "passed all 39 runs where it made this choice" exists, "I verified by re-running the query" goes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟢 Low: "even where the repo's own guidelines ask for testing notes" directs the agent to override a committed contribution guideline. Where a repo enforces the note — a PR template checkbox, a bot linting the body — following this produces a PR that gets bounced. Bound it to projects where nothing enforces the note, or say to keep the line when the project requires it.
Summary: Found 3 medium issues.
Code review by Codex GPT-5.6 SOL (gpt-5.6-sol)
@ -292,3 +256,1 @@2. Scan for the AI tells in this guide and the voice issues in the Voice section.3. Rewrite while preserving meaning. Never invent specifics — if the evidence isn't supplied, cut the claim or flag the gap.4. Return the revised text first. Add notes about what changed only when they help the author.After the Conventional Commits prefix, write one plain lowercase sentence that anyone parses on first read. Pick the shape by the kind of change:🟡 Medium: This makes Conventional Commits syntax unconditional even though the skill applies to pull requests in any repository. In a repository with a different title convention, following the skill adds an invalid prefix; the later requirement to put ticket IDs in the scope can also replace a repository's component scopes. Make both title-shape rules conditional on the repository's documented convention and otherwise preserve its existing format.
@ -295,0 +296,4 @@**Operational constraints always stay.** Merge or deploy ordering ("do not merge before supergraph#240 is deployed"), state already live in production ("codifies a hotfix applied by ConfigMap edit"), and upgrade or migration consequences ("upgrading releases pick up the deny-all") — one sentence each. Nothing in "What gets cut" covers these.**Follow-up PRs skip all of the above.** When a PR fixes, finishes, or repairs the fallout of an earlier PR, one sentence referencing that PR is the whole body. "Finishes the eu-west-1 teardown started in #1164 by dropping the `mainnet` and `devnet` ingress hosts" replaces a Cause/Fix re-explanation of the whole incident.🟡 Medium: "Skip all of the above" also discards the operational constraints that the immediately preceding paragraph says must always stay. A follow-up PR can still have merge ordering, live-state, or migration consequences, so this exception can suppress safety-critical review information. Narrow the exception to the thesis/example template and explicitly retain operational constraints.
@ -295,0 +302,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes, and keep only its strongest version — when "passed all 39 runs where it made this choice" exists, "I verified by re-running the query" goes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟡 Medium: This explicitly tells the agent to omit a
Testing:note even when the repository's own guidelines require one. That makes generated PR bodies violate the applicable project instructions or PR template. Keep the advice for optional routine-testing prose, but preserve every repository-mandated testing section or note.Synchronize-cycle dispositions (fixes pushed as
1b07afa):Applied
Declined, staying open for the author
Resolved as duplicates of open conversations
Testing:-line precedence clause) — tracked there.create-prs-with-ghtrigger overlap) — tracked there, banked for the skills triage.Answered and closed
fix(ABC-289)) and an area scope (fix(config)) both fill the slot, and which wins when a change has both is the project's convention; the "where the project tracks work in tickets" conditional scopes the rule, and legislating further would invent a convention the source draft never had.Round 2 closes the edit rounds for this PR. The new synchronize cycle's findings will be adjudicated but not applied absent a showstopper.
Summary: Docs-only change to one skill file; no correctness or security issues. Found 1 medium and 3 low issues, all about internal consistency of the new
## Pull requestssection and content dropped in the fold.The ticket-in-scope rule and the Titles examples disagree about what occupies the Conventional Commits scope, the "Replaces X" cut can be read as deleting the follow-up body the guide requires, and the instruction to identify the artifact before matching voice lost both of its homes in this diff. Verified separately that nothing else in the repo references the removed
# Task/Self-checksections or the never-installedpr-writing-styleskill, that the frontmatter stays valid single-line YAML, that the double-backtick title example on line 274 renders as intended, and thatverify-readme's two references to this skill still resolve.Code review by Claude Code Opus (opus)
@ -1,17 +1,9 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".🟢 Low: The description now claims "write the PR body" as a trigger, which overlaps
create-prs-with-gh— that skill triggers on "draft a PR description" and "create a PR" but covers onlyghquoting mechanics, with nothing about body content. An agent routed there for a PR description gets no style guidance, and neither skill points at the other. A one-line pointer fromcreate-prs-with-ghto this skill's Pull requests section would make the split reachable from either entry.@ -54,3 +45,3 @@## VoiceVoice is artifact-dependent. Blog posts, release narratives, postmortems, and PRs can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide. Match tone to where the text will be read.Voice is artifact-dependent. Blog posts, release narratives, and postmortems can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide.🟢 Low: The removed trailing sentence ("Match tone to where the text will be read.") and the removed
# Taskstep 1 ("Identify the artifact (README, API doc, release note, blog post, PRD) and match voice to it") were this guide's only two directives to pick the artifact before writing. Both go in this diff, and the folded opening at line 6 carries only the precedence and output-shape halves of# Task. What remains is the descriptive "Voice is artifact-dependent" plus a list of genres — no instruction to identify which one is in hand. The cut ledger in the PR body doesn't account for this pair; if it was deliberate, the artifact list itself (README, API doc, release note, blog post, PRD) is worth keeping somewhere, since it is the concrete instance set the classification depends on.@ -295,0 +288,4 @@- Before: "Forward-only: `staging` rows persisted before this change keep their tagged URLs and age out; no backfill."- After: "Responses saved before this change keep their tagged URLs. Only new responses are stripped."Where the project tracks work in tickets, every PR names one — in the title scope, never in the body. Several small PRs may share one general ticket.🟡 Medium: This rule and the Titles examples give the scope slot two incompatible occupants, and the section never says how to choose. Here the ticket owns the scope (
fix(ABC-271),fix(ABC-289)), but the pair at lines 266–267 puts a component there (fix(config)) with no ticket anywhere in the title — in the same example set drawn from a tracker project. Conventional Commits allows one scope, so an agent applying both lines has no way to write a config sync that also names its ticket. Say which wins (e.g. the ticket takes the scope and the component moves into the sentence), and make thefix(config)pair follow it.@ -295,0 +307,4 @@- Repetition. Each fact appears once, in its best spot; a detail stated in two paragraphs loses one of them.- Colorful compression. "Scraped past the judge on partial credit" becomes "the judge still marked it as a success, even though the answer was wrong."- Bookkeeping numbers. Keep the numbers that carry the argument; drop inventory counts ("17 client upgrades sourced from 47 merged PRs, 16 customer-facing" — a weekly changelog needs only the week it covers). Never let a simplification widen a claim beyond the data.- "Replaces X" / "supersedes X" lines. The forward pointer belongs on the closed PR; the living PR stands alone.🟢 Low: "The forward pointer belongs on the closed PR" collides with the follow-up rule at line 299, which makes "Finishes the eu-west-1 teardown started in #1164…" the entire body. On Forgejo and GitHub a merged PR is also closed, so the disambiguating word doesn't disambiguate: an agent applying this bullet literally deletes the one sentence the Body section requires. Naming the case explicitly — a pointer to a superseded/abandoned PR, not to an earlier merged one — closes it.
Summary: Reviewed the fold of the
pr-writing-styledraft intohuman-facing-writing-style. The merged file is internally consistent: no section references the deleted# Goal/# Reference/# Taskscaffold or the removed## Self-checklist, nothing else in the repo referencespr-writing-style, thesee Quick substitutions belowpointer still resolves after the table lost four rows, and the cut cross-section duplicates (source attribution, backticked code names, throat-clearing rows) each survive in exactly one place. The frontmatter parses as YAML — the description is a safe plain scalar (no:, no#) at 342 characters. TheCurly quotation marksbullet now really does hold curly quotes, and they are the file's only ones. The39 of the last 90 runsexample in the Body list and thepassed all 39 runsline under What gets cut are the same running example, so they agree.Found 2 low-severity issues, both about how the new PR guidance is reached or scoped. No critical, high, or medium issues.
Code review by Claude Code Opus (opus)
@ -65,3 +56,2 @@- Show emotion through behavior, not labels. Not "this is concerning" but "I spent an afternoon chasing a bug that turned out to be a cached DNS record". Not "the team was frustrated" but "we rolled back three times before lunch". A named emotion tells the reader what to feel; a shown one lets them feel it. Specific behavior reads human; labeled emotion reads algorithmic.- Let some mess in (in essays, not docs). Tangents, asides, and half-formed thoughts are human. Perfectly regular structure feels generated.- Lead with information, not emotion. Skip "We're thrilled to announce…" or "Today, we're excited to share…". Open with what happened.- Let some mess in. Tangents, asides, and half-formed thoughts are human. Perfectly regular structure feels generated.🟢 Low: This bullet lost its
(in essays, not docs)qualifier, so read literally it now tells an agent to put tangents and half-formed thoughts into any artifact, including reference docs. The parallel bullet under Structure kept its scope marker (**Set up and pay off** (in essays and long-form pieces, not reference docs)), so the two bullets now disagree about whether artifact scope is stated inline. The section intro ten lines up does quote this exact phrase ("'Let some mess in' is good advice for a personal post, bad advice for a migration guide"), which limits the damage, but restoring the parenthetical keeps the bullet correct on its own — bullets in this guide are read and applied individually.@ -286,2 +250,3 @@4. Ask "what still sounds AI-generated here?" and fix the remaining tells.# Task## Pull requests🟢 Low: Nothing routes an agent from the PR workflow to this new section.
skills/create-prs-with-gh/SKILL.mdis the skill whose description triggers on "draft a PR description" and "create a PR", and it covers onlyghquoting mechanics — an agent that loads it to open a PR gets no signal that the title and body rules now live here. The repo already has a pattern for this:skills/verify-readme/SKILL.md:10and:160explicitly say to invokehuman-facing-writing-styleand apply its rules. Adding one sentence tocreate-prs-with-ghpointing at this section would close the same loop. The PR body's follow-up note covers only the external align-corpus rule, not this in-repo skill.Round 3 dispositions — no edits; per the announced cap, this round applies only showstoppers and found none. Head stays
1b07afa.create-prs-with-ghshould point at this section, and 40561 is right that the fix lives in this repo, following theverify-readmepattern. Not this PR: adding a second file re-opens review scope on a skill with its own banked issues. Committed as an immediate follow-up (tracked with 40413).The PR is complete: cycle green on
1b07afa, open conversations are the five held for the author (40412/40415 placement, 40413 routing follow-up, 40426/40515 precedence).Summary: Docs-only change to one skill file. No correctness or security risk; the findings are intra-file contradictions and ambiguities in prescriptive guidance an agent is meant to follow literally.
Found 2 medium, 5 low/info issues.
fix(config)example; the Conventional Commits scope can't hold both a component and a ticket, and the guide doesn't say which wins.## Before/## Afterin PR bodies collides with the newly scoped subheading rule; Voice no longer names PRs even though the skill now covers them; three phrasings ("backing, not inline citation", "the number", "needs only the week it covers") don't resolve on a first read.The cuts the PR describes check out: the
# Goal/# Reference/# Taskscaffold, the Self-check list, and the three removed substitution-table rows all survive in Concision, Specificity, Revise, or the opening paragraph. No stale cross-references remain —skills/verify-readme/SKILL.mdstill references this skill by name only. The trimmed frontmatter trigger list matchesai-facing-writing-style's own "one trigger per distinct situation" rule, and the curly-quote example correctly holds curly quotes now.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".ℹ️ Info: Adding "write the PR body" here overlaps the existing
create-prs-with-ghskill, whose description triggers on "draft a PR description" and "Use when creating or editing a pull request". The two cover different halves (that skill isghquoting mechanics, this one is the prose), but the routing signals now collide, and a request phrased as "draft a PR description" reaches the quoting skill rather than the guidance in## Pull requests. Worth a one-line pointer between them, or narrowing the other skill's situations to the delivery step.@ -18,2 +10,2 @@- Use plain, direct language. Prefer common words over jargon unless the jargon is load-bearing for the audience.- One idea per sentence. Break long sentences into shorter ones when comprehension suffers.- Use plain, direct language. Prefer common words over jargon unless the jargon is load-bearing for the audience; when an identifier is the precise name, lead with the plain words and give the identifier in parentheses: "the header that carries the request ID (`x-request-id`)".- One idea per sentence. An em-dash aside, a "while" contrast, or a colon label ("Forward-only:") means the sentence is hiding two or three. Split it.🟡 Medium: This new rule contradicts the pre-existing em-dash rule at line 206 ("Use them for genuine asides, not as default punctuation"). Line 11 says an em-dash aside is evidence the sentence hides two or three ideas and must be split; line 206 says a genuine aside is a legitimate use. An agent applying the guide literally gets opposite verdicts on the same sentence and cannot tell which wins.
The file demonstrates the problem on itself: em dashes go from 4 occurrences before this PR to 29 after, 17 of them in the new
## Pull requestssection, and most are exactly the aside form line 11 tells the reader to split (lines 269, 279, 291, 295, 301, 303, 307, 309, 315, 319).Fix: scope line 11 to the case it actually targets — an em dash splicing a second independent statement — and say so, leaving line 206's genuine-aside allowance intact; or drop the em-dash clause from line 11 and keep the "while" contrast and colon-label tests.
@ -36,3 +38,3 @@## SpecificityConcrete beats abstract. Numbers, named entities, and real examples beat vague gestures. Vague quantifiers are a tell: "many users", "significant improvements", "a number of issues". Provide a number or range. Same for vague attributions like "Industry observers have noted…", "Experts argue…", "Some critics suggest…". Name the source or drop the claim.Concrete beats abstract. Numbers, named entities, and real examples beat vague gestures. Vague quantifiers are a tell: "many users", "significant improvements", "a number of issues". Provide a number or range. Same for vague attributions: "Industry observers have noted…", "Experts argue…", "Some critics suggest…" — name the source or drop the claim ("A 2024 benchmark from Vercel found…"). A sentence that could appear in any other project's blog post is not yet specific. The numbers must carry the argument, though: drop inventory counts ("17 client upgrades sourced from 47 merged PRs, 16 customer-facing" — a weekly changelog needs only the week it covers).🟢 Low: "a weekly changelog needs only the week it covers" doesn't land. The example being cut is a set of inventory counts, so the parenthetical is presumably saying the reader needs the week's changes rather than the totals behind them — but read literally it says a changelog needs only its date range, which contradicts the rest of the paragraph's push toward concrete numbers. Restate what the reader does need ("readers want what changed, not how many PRs it took").
@ -54,3 +58,3 @@## VoiceVoice is artifact-dependent. Blog posts, release narratives, postmortems, and PRs can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide. Match tone to where the text will be read.Voice is artifact-dependent. Blog posts, release narratives, and postmortems can carry first person, opinion, rhythm, and the occasional tangent. Reference docs, READMEs, and API documentation should stay calm, concrete, and low-friction. "Let some mess in" is good advice for a personal post, bad advice for a migration guide.🟢 Low: Dropping "and PRs" from this list leaves the guide with no stated voice for the artifact it now covers end to end: PRs appear in neither the first-person/opinion/tangent list nor the calm-and-low-friction list. That reads as a deliberate exclusion, yet line 319 explicitly permits a conversational aside in a PR body and line 301 assumes first person ("a run you performed"). Either keep PRs in the first list or have the
## Pull requestssection state its own voice, so a reader picking voice for a PR body has an answer.@ -295,0 +288,4 @@- Before: `fix(ABC-289): links in agent responses should not carry utm_source tracking parameters`- After: ``fix(ABC-289): links in agent responses should not carry `utm_source` tracking parameters``Where the project tracks work in tickets, every PR names one — in the title scope, never in the body. Several small PRs may share one general ticket.🟡 Medium: This mandates the ticket in the Conventional Commits scope ("in the title scope, never in the body"), but the section's own worked example at line 284 ships
fix(config): update cached questions to match the UI— a component scope with no ticket — while lines 282 and 289 usefix(ABC-271)/fix(ABC-289)with the ticket in that same slot. A scope holds one value, so a ticket-tracking project with component scopes has no way to satisfy both, and the guide never states which wins or shows a combined form.Fix: state the precedence explicitly (ticket takes the scope) and correct or annotate the line 284 example, or show the form that carries both (e.g.
fix(config/ABC-289): …) so the rule and the examples agree.@ -295,0 +302,4 @@For removals and cleanup, state what was removed and why it is safe ("feature is fully launched, the flags are dead code") — that is all the justification a reviewer needs.For UI changes, the screenshots are the example: a `## Before` and a `## After` section, one capture each. The pair replaces prose description of the visual change — don't narrate what the images already show.🟢 Low: These are exactly the "bureaucratic labels" that line 121 rules out. The exception added on that line covers "articles and posts; reference material may use category labels" — a PR body is neither, so as written the guide mandates
## Before/## Afterhere and forbids category-label subheadings there. Widen the line 121 exception to name PR bodies (or reword it as "prose pieces" vs. structured artifacts) so the two sections agree.@ -295,0 +294,4 @@**One line is the target — and the floor.** When the title already says it all, a single sentence plus any screenshot is the whole body. But never ship an empty body: a 3,000-line schema PR with no body leaves the reviewer unable to tell read-only additions from a behavior change. When the change needs more:1. **Thesis first.** One or two sentences: what the change does and the problem it answers, with the number when the change has one. Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence.🟢 Low: "with the number when the change has one" — "the number" has no antecedent; nothing earlier in the step or section introduces a number, so the definite article points at something the reader has to guess (a metric? a count of affected rows? a ticket number?). Name it: "with the quantity that makes the change concrete, when there is one."
@ -295,0 +298,4 @@2. **One concrete example.** The actual scenario, the wrong output, and the correct output — a table when the data is tabular. Introduce it with one sentence that carries its evidence inline ("In 39 of the last 90 runs, the query grouped trades by the wallet that signed the transaction (`tx_from`) and gave the correct answer:").3. **One or two closing sentences** on what the example shows.Ground every claim in the diff or a run you performed — backing, not inline citation in every sentence; examples come from real runs and data. Cite concrete artifacts, not the tracker: when the evidence is another PR, link that PR, never the ticket it came from.🟢 Low: "backing, not inline citation in every sentence" is a verbless fragment whose subject the reader has to reconstruct, and the semicolon then joins an unrelated third claim. In a guide that tells the reader one idea per sentence (line 11), this is three. "Backing" is also undefined here — it reads as a noun for a concept the guide never introduces.
Suggested split: "Ground every claim in the diff or a run you performed. Examples come from real runs and data; you don't need a citation in every sentence."
Summary: Documentation-only change to one skill file. The merge is sound: the frontmatter still parses as a plain YAML scalar (no
:or leading quote in the value), every cut named in the description ledger has a surviving home (# Goal/# Taskin the opening paragraph, the dropped table rows in the Concision throat-clearing and copula bullets, source attribution in Specificity, the two non-derivable Self-check tests in Specificity and Revise), the intra-file forward reference to "What gets cut" resolves, the double-backtick title example is valid Markdown, andskills/verify-readme/SKILL.md's two pointers at this skill still hold. The curly-quote example now actually contains curly quotes.Found 3 low issues and 1 observation, all about rules an agent following the file literally would over-apply: an em-dash rule that contradicts the existing formatting-tics bullet, ticket-in-scope guidance with no boundary for projects that already use the scope for a component, and an unconditional override of a repo's own contribution guidelines.
Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".ℹ️ Info: This skill now owns PR body content, but the sibling
create-prs-with-gh(mechanics only) holds the literal trigger "draft a PR description" while this description offers "write the PR body". Routing is semantic, so both will usually match, but a request phrased as the sibling's trigger loads the quoting guidance without the content guidance. Adding the sibling's phrasing here, or a one-line pointer fromcreate-prs-with-ghat this skill (the patternskills/verify-readme/SKILL.md:10already uses), would close the gap.@ -18,2 +10,2 @@- Use plain, direct language. Prefer common words over jargon unless the jargon is load-bearing for the audience.- One idea per sentence. Break long sentences into shorter ones when comprehension suffers.- Use plain, direct language. Prefer common words over jargon unless the jargon is load-bearing for the audience; when an identifier is the precise name, lead with the plain words and give the identifier in parentheses: "the header that carries the request ID (`x-request-id`)".- One idea per sentence. An em-dash aside, a "while" contrast, or a colon label ("Forward-only:") means the sentence is hiding two or three. Split it.🟢 Low: This conflicts with the formatting tic at line 211 (
**Em dash overuse**), which explicitly blesses em dashes "for genuine asides, not as default punctuation". Here an em-dash aside is itself the signal that the sentence hides two or three ideas, and the verdict is "Split it." An agent applying this literally has no way to tell a genuine aside from a hidden second idea, and this file carries 29 em dashes — several in sentences it holds up as the good version (line 6, line 10, line 44). Naming the boundary would resolve it: the split applies when the aside carries a fact of its own, not when it sets off a parenthetical qualifier.@ -295,0 +288,4 @@- Before: `fix(ABC-289): links in agent responses should not carry utm_source tracking parameters`- After: ``fix(ABC-289): links in agent responses should not carry `utm_source` tracking parameters``Where the project tracks work in tickets, every PR names one — in the title scope, never in the body. Several small PRs may share one general ticket.🟢 Low: "in the title scope" has a plausible over-application with no stated boundary. Conventional Commits scopes usually name the component or area — this repo's own history uses
feat(audit-git-checkouts):— so in a project that tracks tickets and scopes by component, an agent following this line replaces a meaningful scope with a ticket ID rather than choosing between them. Consider naming the fallback for projects whose scope is already spoken for (ticket in the scope where the scope is otherwise unused, a leadingABC-271:or a trailing reference otherwise).@ -295,0 +312,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟢 Low: "even where the repo's own guidelines ask for testing notes", combined with the template-scaffolding bullet below, tells the agent to strip sections a repository requires. Some repos gate merges on template completeness (a required-checklist bot, or a reviewer who bounces the PR), so the result is a PR that fails a check rather than a leaner body. If the stance is deliberate, scope it — cut the routine
Testing:line the author would otherwise add, but keep sections a committed template or contribution guide requires, filled with the non-routine verification or an explicit "none".Round 4 dispositions — 4 applied in
6243438, 8 declined. Per the round-3 pattern: the next cycle (on6243438) applies showstoppers only.Applied:
Declined:
create-prs-with-ghrouting overlap is already committed as the immediate follow-up PR (see the round-3 note; tracked with 40413), using exactly the pointer pattern 40653 names.Summary: Docs-only change to
skills/human-facing-writing-style/SKILL.md. I checked the cut ledger the PR body claims: every removed line from# Goal/# Reference/# Task, the Self-check list, the sterile-prose symptoms, and the three substitution-table rows is recoverable from a surviving section, and the guide no longer uses any of its own banned phrases outside examples. The double-backtick title example on line 289 renders as intended.Found 1 medium and 2 low issues, all internal contradictions or first-read ambiguities in the new
## Pull requestssection, plus 1 informational note on skill integration.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".ℹ️ Info: With "write the PR body" now a trigger here, two skills fire on the same request:
skills/create-prs-with-gh/SKILL.mdtriggers on "draft a PR description" but covers onlyghquoting mechanics, so an agent routed there gets no content rules. A pointer from that skill to this one would close the gap — the same repoint the PR body plans for the align corpus rulerules/ai-coding-workflow/conventional-commits.md.@ -295,0 +276,4 @@- **Features**: name the new capability ("add invoice history to the billing page").- **Mechanical changes** (syncs, rolls, renames): state the action and keep it short.A title that narrates intent or justification ("…they were built for") is a story, not a title. When the commit type could mislead, make the kind of change unmistakable — a prompt-only tweak must not read like an app feature.🟢 Low: "(…they were built for)" quotes the Before example that only appears four lines later, so on first read the fragment has no referent — the cold-stranger problem this guide's own Clarity section targets. Either move this sentence below the example list or quote enough of the title to stand alone ("…should match the featured prompts they were built for").
@ -295,0 +288,4 @@- Before: `fix(ABC-289): links in agent responses should not carry utm_source tracking parameters`- After: ``fix(ABC-289): links in agent responses should not carry `utm_source` tracking parameters``Where the project tracks work in tickets, every PR names one — in the title scope, never in the body. Several small PRs may share one general ticket.🟡 Medium: This rule and the title examples above disagree about what the Conventional Commits scope holds. Two examples put the ticket in the scope (
fix(ABC-271):,fix(ABC-289):), while the recommended title four lines up isfix(config): update cached questions to match the UI— component in the scope, no ticket. In a project that tracks work in tickets, the guide's own example violates "every PR names one — in the title scope", and an agent following this literally has no rule for a repo whose scopes name components.State the precedence: either the ticket replaces the component in the scope where a tracker exists, or give the combined form (
fix(config/ABC-311):), and make thefix(config)example carry a ticket so the example set is consistent.@ -295,0 +306,4 @@**Operational constraints always stay.** Merge or deploy ordering ("do not merge before supergraph#240 is deployed"), state already live in production ("codifies a hotfix applied by ConfigMap edit"), and upgrade or migration consequences ("upgrading releases pick up the deny-all") — one sentence each. Nothing in "What gets cut" covers these.**Follow-up PRs skip all of the above except the operational constraints.** When a PR fixes, finishes, or repairs the fallout of an earlier PR, one sentence referencing that PR is the whole body. "Finishes the eu-west-1 teardown started in #1164 by dropping the `mainnet` and `devnet` ingress hosts" replaces a Cause/Fix re-explanation of the whole incident.🟢 Low: The two halves contradict each other. "skip all of the above except the operational constraints" keeps merge/deploy ordering, but the next sentence says "one sentence referencing that PR is the whole body" — a follow-up PR that also carries a deploy-order constraint then needs two sentences, and an agent following this literally has to pick which line wins. Suggest: "one sentence referencing that PR, plus any operational constraint, is the whole body".
Summary: Docs-only change to one skill file; the fold-in is coherent and I found no broken cross-references — nothing else in the repo names the retired
pr-writing-style,skills/verify-readme/SKILL.mdstill resolves, the removed Self-check items are each covered by a surviving section, and the nested-backtick title example on line 289 renders as intended under CommonMark (a two-backtick span is closed only by a two-backtick run).Found 1 medium and 4 low issues, mostly instruction conflicts introduced by merging the two guides: the thesis step tells the writer to defer implementation specifics that "What gets cut" forbids outright, the closing aside contradicts "alternatives considered belong in the ticket", and the new illustrative-value carve-out is unscoped inside the "Do not invent specifics" rule. Plus one cold-read fix and one cross-skill routing pointer.
Code review by Claude Code Opus (opus)
@ -39,2 +40,3 @@Concrete beats abstract. Numbers, named entities, and real examples beat vague gestures. Vague quantifiers are a tell: "many users", "significant improvements", "a number of issues". Provide a number or range. Same for vague attributions: "Industry observers have noted…", "Experts argue…", "Some critics suggest…" — name the source or drop the claim ("A 2024 benchmark from Vercel found…"). A sentence that could appear in any other project's blog post is not yet specific. The numbers must carry the argument, though: drop inventory counts ("17 client upgrades sourced from 47 merged PRs, 16 customer-facing" — the reader wants what changed, not how many PRs it took).**Do not invent specifics.** When editing someone else's text, never add numbers, sources, dates, or named examples that weren't supplied. Fabricated precision is worse than admitted vagueness: it launders a guess as a fact. If the evidence isn't there, cut the claim, keep the hedge, or leave a `[needs data]` marker for the author.**Do not invent specifics.** When editing someone else's text, never add numbers, sources, dates, or named examples that weren't supplied. Fabricated precision is worse than admitted vagueness: it launders a guess as a fact. If the evidence isn't there, cut the claim, keep the hedge, or leave a `[needs data]` marker for the author. An invented value is fine only when marked as illustrative ("e.g. `INV-7KQMD`").🟢 Low: The new carve-out has no scope, so it reads as an exception to the bold rule that opens the paragraph. The sentence before it is scoped ("When editing someone else's text, never add numbers, sources, dates, or named examples that weren't supplied"); "An invented value is fine only when marked as illustrative" is not, and an agent editing someone's runbook can use it to insert a plausible-looking
INV-7KQMDinto a doc whose real IDs have a different shape — the launder-a-guess-as-fact failure the paragraph exists to prevent.Scope it to your own writing, or to placeholder formats the author supplied: "In your own writing, an invented value is fine when marked as illustrative (…)."
@ -286,2 +265,3 @@4. Ask "what still sounds AI-generated here?" and fix the remaining tells.# Task## Pull requests🟢 Low: Routing gap now that PR guidance lives here:
skills/create-prs-with-gh/SKILL.mdtriggers on "draft a PR description" and "create a PR" but covers onlyghquoting, and nothing in it points at this skill — so the trigger most likely to fire when an agent actually writes a PR body never reaches this section.skills/verify-readme/SKILL.md:10shows the repo's convention for this ("invoke thehuman-facing-writing-styleskill"); a one-line pointer increate-prs-with-ghwould close it.@ -295,0 +276,4 @@- **Features**: name the new capability ("add invoice history to the billing page").- **Mechanical changes** (syncs, rolls, renames): state the action and keep it short.A title that narrates intent or justification ("…they were built for") is a story, not a title. When the commit type could mislead, make the kind of change unmistakable — a prompt-only tweak must not read like an app feature.🟢 Low: The quoted fragment "…they were built for" has no antecedent at this point — the title it comes from first appears four lines later, in the second Before/After pair. A reader hitting this sentence cold cannot tell what narration is being illustrated. Either move the sentence below the examples or point at the pair ("… is a story, not a title — see the second pair below").
@ -295,0 +294,4 @@**One line is the target — and the floor.** When the title already says it all, a single sentence plus any screenshot is the whole body. But never ship an empty body: a 3,000-line schema PR with no body leaves the reviewer unable to tell read-only additions from a behavior change. When the change needs more:1. **Thesis first.** One or two sentences: what the change does and the problem it answers, with the number when the change has one. Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence.🟡 Medium: This step and the "What gets cut" list give opposite instructions for the same material. Here, "Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence" tells the writer to include them, just later. Two sections down, the first two cut bullets forbid exactly that: "Anything the diff already says: file names, quoted changed text" (prop names are in the diff) and "How the system works. The reviewer can read the code; state the consequence, not the mechanism" (libraries are mechanism). An agent following the recipe literally writes a second thesis sentence that the cut list then deletes.
Pick one and say it once: either drop the trailing sentence and let "What gets cut" own implementation detail, or rewrite it as an exclusion — "Implementation specifics — libraries, prop names, the feature inventory — stay out; see What gets cut."
@ -295,0 +316,4 @@- "Replaces X" / "supersedes X" lines. The forward pointer belongs on the closed PR; the living PR stands alone.- Template scaffolding: a `## Summary` heading over a single paragraph, checkbox test plans of routine commands, placeholder sections, generated-with footers.A conversational aside — "Alternatively we could X, but Y felt better because Z" — is fine when the reviewer genuinely needs it. Usually they don't.🟢 Low: This closing line contradicts the "How the system works" bullet above it, which ends "Root-cause walk-throughs and alternatives considered belong in the ticket." That says relocate alternatives; this says an "Alternatively we could X, but Y felt better because Z" aside is fine when the reviewer needs it. Both are new in this PR, and neither points at the other, so which one applies is left to the reader.
Name the exception it carves out — e.g. "Alternatives considered belong in the ticket, with one exception: a conversational aside … is fine when the reviewer genuinely needs it" — or drop "and alternatives considered" from the cut bullet.
Round 5 dispositions — no edits, per the announced showstoppers-only cap. Head stays
6243438and the PR is handed to the author. All nine findings are banked on the author's review list rather than churned through another cycle:create-prs-with-ghpointer is already the committed immediate follow-up PR (tracked with 40413), in exactly theverify-readmepattern both cite.Final state: review cycle green on
6243438(2 code slots published, receipt-bound; approach slots opening-only). Open conversations are the five held for the author — 40412/40415 (placement), 40413 (routing follow-up), 40426 and 40515 (author stances) — everything else is resolved with a posted disposition. Ready to merge.Adjudicating the four remaining held conversations (40515 stays open pending one author decision):
## Pull requeststoreferences/pull-requests.md): declined as superseded. Both measured the pre-restructure section (~7.4 KB including its general rules and the References subsection). The integration pass moved the general content inline where every branch of the skill benefits; the remaining mechanics core no longer supports the reach-test arithmetic, and extraction would re-create the parallel-rulebook shape the restructure removed. Revisit if the file's size becomes a measured problem.create-prs-with-ghis the committed immediate follow-up PR after this merges (per the round-3 note).Closing 40515 with
38b6648, on the author's ruling:fix(ABC-271)examples to that explicit-mandate case, making the example set consistent.38b6648also applies two banked round-5 fixes: the illustrative-value carve-out is scoped to your own writing (40748), and the story-not-a-title sentence now follows the example pair it quotes (40742/40749).Summary: Docs-only change to
skills/human-facing-writing-style/SKILL.md. The fold reads well and the cut ledger holds up — I checked each removal against a surviving home: source attribution now lives in Specificity, the copula and throat-clearing rules cover the two dropped substitution-table rows, and both non-derivable Self-check tests ("could this appear in any other project's blog", the read-aloud test) reappear in Specificity and Revise. Nothing in the repo pointed at the removed sections;skills/verify-readmereferences the skill by name only. The curly-quote example is correct now that it holds real curly quotes, and the nested double-backtick title example renders as intended (checked withmarked).Found 5 low issues and 1 note: one that changes rendering (the title-shape and example bullets merge into a single list), one exemplar that drops a fact its own rule says to preserve, an example/default mismatch on ticket scopes, a literal-reading collision between "restatements of the change" and "Thesis first", and a trigger-phrase gap against
create-prs-with-gh. No blocking problems.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR body", or "human-facing writing style".🟢 Low: The trigger list has a routing gap now that PR writing lives here.
skills/create-prs-with-gh/SKILL.mdalready triggers on "draft a PR description" and covers onlyghquoting, so the phrase a user is most likely to type routes to the shell-safety skill and never loads this guide. Titles are unreachable too: the section covers them at length, but no trigger matches "write the PR title". Adding "draft a PR description" and a title phrase here would close both.@ -25,0 +19,4 @@Avoid: "Forward-only: `staging` rows persisted before this change keep theirtagged URLs and age out; no backfill."Prefer: "Responses saved before this change keep their tagged URLs. Only new🟢 Low: The rewrite drops a fact rather than relocating it. "age out" says the old rows eventually expire; the
Preferhalf says only that they keep their tagged URLs. Line 6 tells the reader to "rewrite while preserving meaning", so the exemplar for Describe effects in the reader's world teaches that a fact can vanish in translation. Carrying it over ("…keep their tagged URLs until they expire") keeps the pair faithful and still demonstrates the rule.@ -295,0 +276,4 @@- **Features**: name the new capability ("add invoice history to the billing page").- **Mechanical changes** (syncs, rolls, renames): state the action and keep it short.- Before: `ABC-271 fix tracing`🟢 Low: These four
Before:/After:bullets merge into the shape list above them. A blank line between items of the same marker doesn't end a CommonMark list, it only makes it loose, so this renders as one seven-item<ul>(verified withmarked): the examples appear as three more "kinds of change" alongside Behavior corrections, Features, and Mechanical changes, and every item gains paragraph spacing. Separate them — indent the pairs under the shape they illustrate, or introduce them with a lead-in sentence like the one before the backtick example at line 286.@ -295,0 +288,4 @@- Before: `fix(ABC-289): links in agent responses should not carry utm_source tracking parameters`- After: ``fix(ABC-289): links in agent responses should not carry `utm_source` tracking parameters``The scope names the component, following the project's convention. Put a ticket there instead only when the project's guidelines or the user explicitly ask for it — the `fix(ABC-271)` shape above. Where that applies, every PR names its ticket in the title scope, never in the body, and several small PRs may share one general ticket.🟢 Low: The default arrives after the examples that contradict it. Three of the four title examples above (lines 280, 288, 289) put a ticket in the scope, so a reader — or an agent — that mimics the worked examples lands on the exception, not on the component scope this line names as the rule. Either move this sentence above the
Before/Afterpairs, or switch the primary rewrite at line 279–280 to a component scope and keep a ticket scope only in the example that illustrates the exception.@ -295,0 +304,4 @@For UI changes, the screenshots are the example: a `## Before` and a `## After` section, one capture each. The pair replaces prose description of the visual change — don't narrate what the images already show.**Operational constraints always stay.** Merge or deploy ordering ("do not merge before supergraph#240 is deployed"), state already live in production ("codifies a hotfix applied by ConfigMap edit"), and upgrade or migration consequences ("upgrading releases pick up the deny-all") — one sentence each. Nothing in "What gets cut" covers these.ℹ️ Info: Worth a second look against the anonymization pass the PR describes —
supergraph#240is the one remaining example that names a repository. It reads as generic federation vocabulary, so it may well be fine; if it came from an employer-internal repo name, a neutral placeholder keeps the sentence working.@ -295,0 +310,4 @@### What gets cut- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.🟢 Low: "restatements of the change" reads as cutting exactly what line 297 requires — a thesis stating "what the change does". Line 269 ("what the diff cannot say — intent, evidence, and consequences") and the mechanism/consequence split in the next bullet carry the real boundary, but a reader following this line literally has a rule against the first item of the body template. Pin it to what is actually meant: a file-by-file or section-by-section enumeration of the diff.
Round 6 dispositions — 3 applied in
8ac6ae0, 3 declined. Note on the cycle itself: theforgejo-review-code-smart-2generator failed before doing any work (Forgejo Actions capability exchange failed: HTTP 400: "Validation failed"), so this head got one review instead of two; the8ac6ae0push retries the full cycle.Applied:
38b6648reorder — removing the paragraph between the shape list and the example bullets merged them into one seven-item CommonMark list. A lead-in line ("Two rewrites:") now separates them.create-prs-with-ghremains the committed follow-up PR.Declined:
fix(ABC-271)shape above"), so the stated rule governs over example mimicry; another structural reorder of this subsection risks more than it buys.Summary: Docs-only change to one skill file; no correctness, security, or integration defects. The cut ledger checks out — I traced every removed passage (the
# Goal/# Reference/# Taskscaffold, all 14 Self-check items, the three substitution rows, the two Credibility bullets, the sterile-writing recognition list) to a surviving home, and the only in-repo reference to this skill (skills/verify-readme/SKILL.md) is unaffected since the name is unchanged. Frontmatter parses, and the nested double-backtick title example on line 291 renders as intended.Found 4 low-severity issues, all in the new
## Pull requestssection or its knock-on edits: an ungeneralized "belong in the ticket" that the rest of the section made tracker-conditional, a repo-guideline override with no boundary for enforced PR templates, an ambiguous evidence exemplar, and a 4→28 jump in em dashes against the guide's own tell. Plus one routing observation on the description.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR title or body", "draft a PR description", or "human-facing writing style".ℹ️ Info: "draft a PR description" is now a verbatim trigger in both this description and
skills/create-prs-with-gh/SKILL.md. The two are complementary (prose vs.ghquoting), so both matching is likely what you want — noting it in case the routing overlap was unintended.@ -107,2 +110,4 @@- Keep parallel items parallel: matching grammatical structure, tense, and punctuation.- Follow conventions the reader already knows: capitalization of product names, terminology from the codebase or domain.- Name things as they exist. Never coin a shorthand that reads like a real identifier — quote the actual text instead (the preconfigured "top 5 users by volume" answer, not `top-users-answer`, which looks like a config key that doesn't exist).- Enclose code-like names — anything that exists verbatim in code, config, or a URL, whatever its class: a parameter, variable, flag, function — in backticks wherever they appear.🟢 Low: Em dash count goes from 4 in the base file to 28 here, ~24 of them in prose this PR adds — against the guide's own "Em dash overuse" tell at line 206 ("Use them for genuine asides, not as default punctuation") and the new Clarity rule at line 11 ("An em-dash aside that carries a fact of its own ... means the sentence is hiding two or three. Split it"). This line is the clearest instance: a 15-word aside carrying its own colon-labelled list splits "Enclose code-like names" from "in backticks". It reads cleanly as two sentences — "Enclose code-like names in backticks wherever they appear. That covers anything existing verbatim in code, config, or a URL, whatever its class: parameter, variable, flag, function." Lines 44, 269, 297, 315, and 321 have the same shape.
@ -295,0 +297,4 @@**One line is the target — and the floor.** When the title already says it all, a single sentence plus any screenshot is the whole body. But never ship an empty body: a 3,000-line schema PR with no body leaves the reviewer unable to tell read-only additions from a behavior change. When the change needs more:1. **Thesis first.** One or two sentences: what the change does and the problem it answers, with the number when the change has one. Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence.2. **One concrete example.** The actual scenario, the wrong output, and the correct output — a table when the data is tabular. Introduce it with one sentence that carries its evidence inline ("In 39 of the last 90 runs, the query grouped trades by the wallet that signed the transaction (`tx_from`) and gave the correct answer:").🟢 Low: The exemplar sentence is ambiguous in exactly the place it is teaching precision. "In 39 of the last 90 runs, the query grouped trades by
tx_fromand gave the correct answer" reads on first pass as "39 of 90 answers were correct" — a 43% success rate, which argues against the change. The intended meaning only surfaces from the Concision bullet 264 lines earlier ("passed all 39 runs where it made this choice"): 39 runs took this path, and all of them were right. Split the two facts, per the Clarity rule at line 11: "The query took this path in 39 of the last 90 runs. In all 39 it grouped trades bytx_fromand gave the correct answer."@ -295,0 +313,4 @@### What gets cut- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.🟢 Low: "belong in the ticket" isn't tracker-conditional, unlike the ticket policy at line 293 ("Put a ticket there instead only when the project's guidelines or the user explicitly ask for it"). For the majority case — no tracker in play — this bullet sends root-cause walk-throughs and alternatives to a destination that doesn't exist, leaving the agent to guess between dropping them and putting them back in the body the bullet just cut them from. Same generalization the PR body applied to the title scope: "belong in the ticket, or are dropped when there is none".
@ -295,0 +314,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟢 Low: "even where the repo's own guidelines ask for testing notes" tells the agent to override repository-level instructions, and the next bullet extends that to checkbox test plans and placeholder sections. Where a PR template is enforced (a template-lint check, a required-sections bot), following this drops a section the check requires and the PR fails. The stance itself is a fair editorial call; it needs the boundary the ai-facing guide's Precision rule asks for — something like "unless a check enforces the template".
8ac6ae06095bce5f014aSummary: Docs-only change to one skill file. The fold is coherent and I found no broken content: every cut I traced survives elsewhere (self-check items 1/4 rehomed into Revise and Specificity, item 8 already covered by the "Vary rhythm" bullet at line 66, the substitution-table
note thatrows by the throat-clearing bullet,Name your sourcesby Specificity), no other skill references the removed# Goal/# Reference/# Task/## Self-checkheadings, nothing in this repo references the never-addedpr-writing-style, and the nested-backtick title example on line 291 renders as intended (verified with a CommonMark renderer). Starting the body with prose instead of an H1 matchesai-facing-writing-styleand seven other skills.Found 3 low issues and 1 note, all in the new
## Pull requestsmaterial and the new examples.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR title or body", "draft a PR description", or "human-facing writing style".ℹ️ Info: The trigger phrase "draft a PR description" is now shared verbatim with
create-prs-with-gh's description, so a routing agent sees the same phrase on two skills. That is reasonable if both should load (this one for content, that one forghquoting), but if you want them to route distinctly, differentiate the phrasing — "write the PR title or body" on line 3 already covers this skill's half.@ -23,2 +17,4 @@Avoid: "In this post, we will discuss how migrations work."Prefer: "Codemods automate 70% of migrations. Here's how to handle the other 30%."Avoid: "Forward-only: `staging` rows persisted before this change keep their🟢 Low:
stagingis ambiguous here, and the two readings teach opposite lessons. If it names a table, "Responses" is the reader's-world translation the bullet above is after. If it names an environment (the PR description says environment names were anonymized), the rewrite drops the scope and widens a staging-only statement into one about all responses — exactly what the new Honesty rule on line 96 forbids. A stranger cannot tell which, so the example doesn't land. Disambiguate in the Avoid text ("rows in thestagingtable") so the rewrite visibly preserves scope.@ -39,2 +40,3 @@Concrete beats abstract. Numbers, named entities, and real examples beat vague gestures. Vague quantifiers are a tell: "many users", "significant improvements", "a number of issues". Provide a number or range. Same for vague attributions: "Industry observers have noted…", "Experts argue…", "Some critics suggest…" — name the source or drop the claim ("A 2024 benchmark from Vercel found…"). A sentence that could appear in any other project's blog post is not yet specific. The numbers must carry the argument, though: drop inventory counts ("17 client upgrades sourced from 47 merged PRs, 16 customer-facing" — the reader wants what changed, not how many PRs it took).**Do not invent specifics.** When editing someone else's text, never add numbers, sources, dates, or named examples that weren't supplied. Fabricated precision is worse than admitted vagueness: it launders a guess as a fact. If the evidence isn't there, cut the claim, keep the hedge, or leave a `[needs data]` marker for the author.**Do not invent specifics.** When editing someone else's text, never add numbers, sources, dates, or named examples that weren't supplied. Fabricated precision is worse than admitted vagueness: it launders a guess as a fact. If the evidence isn't there, cut the claim, keep the hedge, or leave a `[needs data]` marker for the author. In your own writing, an invented value is fine only when marked as illustrative ("e.g. `INV-7KQMD`").🟢 Low: The new carve-out ("In your own writing, an invented value is fine only when marked as illustrative") collides with the PR section, which opens with "Everything above applies at full strength" (line 269) but then requires "Ground every claim in the diff or a run you performed. Examples come from real runs and data" (line 303). An agent drafting a PR body without a real run can read line 42 as license to fabricate a marked-illustrative scenario/output pair for step 2's "One concrete example". Scope the carve-out to explanatory prose ("in your own explanatory writing"), or note the exclusion where it matters most — PR examples are evidence, so an illustrative stand-in is never one.
@ -295,0 +279,4 @@Two rewrites:- Before: `ABC-271 fix tracing`- After: `fix(ABC-271): gateway spans should continue the caller's trace instead of starting a new root`🟢 Low: This exemplar is the first title shape a reader copies, and it puts a ticket in the scope — which line 293 later says is the conditional form ("The scope names the component... Put a ticket there instead only when the project's guidelines or the user explicitly ask for it"). The second rewrite two lines down uses a component scope with no explanation of why the two differ, so between here and line 293 the section teaches the non-default as the default. Either state the precondition inline ("After:
fix(ABC-271): …— this project puts the ticket in the scope") or move the scope sentence from line 293 above "Two rewrites:" so the exemplars land after the rule they illustrate.Round 7 dispositions — 5 applied in
630cfb7, 4 declined. This is the terminal content round: the next cycle is adjudicated for showstoppers only, and the PR then goes to the author regardless.Cycle note: the
forgejo-review-code-smart-2generator has now failed its capability exchange three consecutive times (HTTP 400: "Validation failed", byte-identical, always that slot; last success 12:42Z, first failure 13:21Z today) — a persistent service-side regression being escalated separately, not a property of this PR. The working slot reviewed every head.Applied:
fix(ABC-271)After line now carries "— this project's guidelines put the ticket in the scope", closing the recurring example/default gap from the example side too.stagingwas readable as an environment, under which the Prefer half would widen scope. Now "rows in thestagingtable", so the rewrite visibly preserves scope.tx_from). All 39 gave the correct answer:".Declined:
ghquoting; the follow-up pointer PR makes the pairing explicit.Summary: Reviewed the fold of the
pr-writing-styledraft intohuman-facing-writing-style. The merge itself holds up: I traced all 14 deleted Self-check items and each has a surviving home (items 1, 4 rehomed into Revise and Specificity as the description claims; the rest were already covered by Structure, Voice, Specificity, and the tells sections), the three removed substitution-table rows are covered by the Concision bullets on lines 31-32, source attribution now lives only in Specificity, and no orphanpr-writing-styledirectory is left behind inskills/. The curly-quote example on line 211 now genuinely holds curly quotes.Found 2 medium and 3 low issues, all directive conflicts or ambiguities in the new
## Pull requestssection rather than lost content: "everything above applies at full strength" pulls the essay-voice licences into PR bodies that line 60 puts in the calm/low-friction bucket, and "What gets cut" tells the agent to override a repository's own committed guidelines.Code review by Claude Code Opus (opus)
@ -1,27 +1,26 @@---name: human-facing-writing-styledescription: Writing style guide for improving prose so humans can read and understand it with minimal effort. Use when writing, reviewing, or editing developer-facing text — documentation, release notes, changelogs, explanations, or any other human-readable content. Triggers on "writing style for humans", "improve prose", "make this clearer", "edit for clarity", "human-facing writing", "developer prose", "humanize this", "sounds AI-generated", "remove AI writing tells", or "make this sound natural".description: Writing style guide for prose humans read and understand with minimal effort — documentation, release notes, changelogs, blog posts, and pull request titles and bodies. Use when writing, reviewing, or editing developer-facing text. Triggers on "improve this prose", "sounds AI-generated", "write the PR title or body", "draft a PR description", or "human-facing writing style".🟢 Low: "draft a PR description" is already a verbatim trigger phrase in
skills/create-prs-with-gh/SKILL.md. Two model-invoked skills claiming the same literal phrase makes routing choose between them for a request each answers differently — prose shape here,ghquoting safety there. The neighbouring "write the PR title or body" already covers this situation for prose, so dropping "draft a PR description" here keeps the two descriptions distinguishable at no loss of coverage.@ -288,2 +267,3 @@## Pull requestsWhen editing someone's prose or writing your own:A PR is human-facing writing at its most compressed: the reviewer already has the diff, so the body exists only for what the diff cannot say — intent, evidence, and consequences. Everything above applies at full strength, concision and specificity hardest. What follows is the mechanics of the form.🟡 Medium: "Everything above applies at full strength" contradicts the Voice section for the very artifact this section governs. Line 60 puts PR bodies in the calm, concrete, low-friction bucket, but "everything above" also covers the essay-voice bullets — "Let some mess in. Tangents, asides, and half-formed thoughts are human" (line 70), "Vary rhythm", "Use 'I' when it fits the genre", "Acknowledge mixed feelings" — and "What gets cut" then says the opposite: "A conversational aside … Usually they don't" (line 321).
This diff also strips the
(in essays, not docs)qualifier that used to scope line 70, while the neighbouring Structure bullets keep or gain theirs (line 118(in essays and long-form pieces, not reference docs), line 121(in articles and posts; …)). So the one local guardrail against applying "let some mess in" to a PR body was removed in the same change that told the agent to apply everything above at full strength.Naming the exception fixes it, e.g. "Everything above applies at full strength, concision and specificity hardest — except the essay licences: no tangents, no half-formed thoughts."
@ -295,0 +279,4 @@Two rewrites:- Before: `ABC-271 fix tracing`- After: `fix(ABC-271): gateway spans should continue the caller's trace instead of starting a new root` — this project's guidelines put the ticket in the scope🟢 Low: "this project's guidelines put the ticket in the scope" is ambiguous in a skill delivered into arbitrary repositories — "this project" reads most naturally as the repo the agent is currently working in, which is exactly the wrong default. The rule that scopes it ("The scope names the component … Put a ticket there instead only when the project's guidelines or the user explicitly ask for it") arrives eleven lines later on line 293, so the exception is demonstrated before the default is stated.
Carrying the condition in the annotation avoids the misread: "— where the project's guidelines put the ticket in the scope".
@ -295,0 +297,4 @@**One line is the target — and the floor.** When the title already says it all, a single sentence plus any screenshot is the whole body. But never ship an empty body: a 3,000-line schema PR with no body leaves the reviewer unable to tell read-only additions from a behavior change. When the change needs more:1. **Thesis first.** One or two sentences: what the change does and the problem it answers, with the number when the change has one. Implementation specifics — libraries, prop names, the feature inventory — wait for the second sentence.2. **One concrete example.** The actual scenario, the wrong output, and the correct output — a table when the data is tabular. Introduce it with one sentence that carries its evidence inline ("In 39 of the last 90 runs, the query grouped trades by the wallet that signed the transaction (`tx_from`). All 39 gave the correct answer:").🟢 Low: the rule asks for "one sentence that carries its evidence inline", but the example demonstrating it is two sentences ("In 39 of the last 90 runs… All 39 gave the correct answer:"). An agent following the line literally will pick one of the two readings at random. Either relax the rule to "one or two sentences" or compress the example into a single sentence.
@ -295,0 +314,4 @@- Anything the diff already says: file names, quoted changed text, restatements of the change. Restatements also drift — a "## What" list can name an endpoint the merged diff never adds, leaving the body wrong where the diff is right.- How the system works. The reviewer can read the code; state the consequence, not the mechanism. "Rename mode clipped long endpoint-key names to ~20 characters" is the consequence; the walk-through of intrinsic input widths and flex ancestors is mechanism. Root-cause walk-throughs and alternatives considered belong in the ticket, or are dropped when the project has none.- War story and methodology: how the problem was found, what was verified, which tools ran. State evidence as a claim about outcomes. A one-line version bump needs no list of the releases walked. A `Testing:` line naming the repo's standard suite says nothing CI doesn't already report — even where the repo's own guidelines ask for testing notes. Mention testing only when the verification is non-routine.🟡 Medium: "even where the repo's own guidelines ask for testing notes" instructs the agent to override a repository's committed contribution guidance, and the next bullet extends that to stripping the repo's PR template ("checkbox test plans of routine commands, placeholder sections"). Two problems:
README.md:37notes that axskills "never overrides a repository's committedAGENTS.md; that file stays owned byj4k-align." A skill that tells the agent to disregardAGENTS.mdinverts that.Scoping it keeps the intent without the conflict: cut routine testing notes by default, but honour a repo template or guideline that requires them.
Round 8 dispositions — terminal round as announced: showstoppers only, none found, no edits. Head stays
630cfb7and the PR goes to the author.ghquoting — and the committed follow-up PR adds the explicit cross-pointer.Cycle note:
forgejo-review-code-smart-2failed its capability exchange for the fourth consecutive run (HTTP 400: "Validation failed", service-side, last slot success 12:42Z). The red check on this PR is that regression, not the content; every head since38b6648was fully reviewed by the working slot, and all 67 inline conversations are resolved.