Skip to content

add(skills): pr-description-skill -- anchored, self-sufficient PR bodies for microsoft/apm#884

Open
danielmeppiel wants to merge 5 commits intomainfrom
feat/pr-description-skill
Open

add(skills): pr-description-skill -- anchored, self-sufficient PR bodies for microsoft/apm#884
danielmeppiel wants to merge 5 commits intomainfrom
feat/pr-description-skill

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

@danielmeppiel danielmeppiel commented Apr 23, 2026

add(skills): pr-description-skill — anchored, validated PR bodies for microsoft/apm

TL;DR

PR #882 shipped a high-bar PR body (anchored quotes, mermaid diagrams, structured trade-offs) hand-crafted by an apm-primitives-architect subagent — that quality bar had no reusable carrier, so the next PR was one author's discipline away from regressing. This PR captures the meta-pattern as a Claude-style skill bundle (SKILL.md + two assets/), mirrored to .github/skills/, with mandatory mmdc validation and a 150–220-line ceiling. This PR's body is itself generated by following the just-hardened skill end-to-end — including the harden cycle described under Trade-offs.

Note

Predecessor: #882. This PR builds the carrier; that PR was the worked example.

Problem (WHY)

  • PR harden(apm-review-panel): one-comment discipline + Hybrid E auth routing + apm-primitives-architect persona #882 had to be hand-crafted by a specialist subagent because no reusable scaffold existed; the next author drafting a PR starts from a blank gh pr create editor.
  • Existing PR bodies on the repository regularly reduce to commit-message rehashes, with no Problem / Approach / Trade-offs structure and no anchored quotes from PROSE or Agent Skills.
  • [!] Each new PR re-litigates section order, depth, and what counts as evidence; reviewers cannot pattern-match across PRs.

Why these matter: the bespoke specialist-per-PR shape violates "Favor small, chainable primitives over monolithic frameworks." — the PR-body shape is itself a primitive. Commit-message rehashes violate "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action." — a rehash carries no anchor a reviewer can follow. Variable section order across PRs degrades "agents pattern-match well against concrete structures" for both human reviewers and downstream agents.

Approach (WHAT)

Three artifacts behind one activation contract; template and rubric load only at the steps that need them.

# Fix Principle Source
1 Skill activates on PR-drafting intents and gathers a 9-row activation contract before any draft. "Context arrives just-in-time, not just-in-case." PROSE — Progressive Disclosure
2 assets/pr-body-template.md carries the 11-section skeleton; loaded only at synthesis. "store them in assets/ and reference them from SKILL.md so they only load when needed." Agent Skills — Templates
3 assets/section-rubric.md is a fresh-eyes self-check pass loaded only after a draft exists. "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes." Agent Skills — Validation loops
4 Cite-or-omit: every WHY-claim is a verbatim quoted phrase wrapped in a hyperlink, capped at 3 anchors in Problem. "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action." PROSE — Reduced Scope
5 Mandatory mmdc validation of every mermaid block before save, with a "common pitfalls" list (semicolons in classDiagram labels, () in flowchart labels, note right of closure). "agents pattern-match well against concrete structures" Agent Skills — Concrete structures
6 Output is UTF-8 GFM (alerts, collapsibles, task lists, em dashes); only the bundle source files stay ASCII per the repo's encoding rule. "When you find yourself covering every edge case, consider whether most are better handled by the agent's own judgment." Agent Skills — Spending context
7 Hard 150–220 line ceiling with per-section caps (TL;DR ≤ 4 sentences, Problem ≤ 6 bullets, Trade-offs ≤ 5 entries). "Match task size to context capacity." PROSE — Reduced Scope

Implementation (HOW)

  • .apm/skills/pr-description-skill/SKILL.md (347 lines / 16 KB) — activation contract, 7 anchored core principles, 10-step execution checklist with explicit asset-load gating, mandatory mmdc validation step with pitfalls list, "Output charset rule" that distinguishes bundle-source ASCII from PR-body GFM, anti-patterns refusal list.
  • .apm/skills/pr-description-skill/assets/pr-body-template.md (145 lines / 4.2 KB) — 11-section skeleton with <PLACEHOLDER> markers, an example > [!NOTE] alert, an example <details> block, and a generic flowchart so the orchestrator generates a diagram describing this PR rather than copying the example.
  • .apm/skills/pr-description-skill/assets/section-rubric.md (156 lines / 6.2 KB) — per-section acceptance / refuse pairs, body-level ceilings, "GFM features the body SHOULD use" list, and a final-pass checklist that includes "Every mermaid block validated by mmdc."
  • .github/skills/pr-description-skill/... — byte-for-byte mirror of the .apm/ source-of-truth, regenerated by apm install --target copilot. No anchor needed; mechanical.
  • apm.lock.yaml — one added local_deployed_files entry for the mirrored bundle. Incidental: the hash for .github/instructions/cicd.instructions.md was rewritten to match on-disk content (pre-existing drift on main that the lock-write step picked up; mechanical, no behavior change).
  • CHANGELOG.md — one line under [Unreleased] -> Added describing the bundle and the self-application validation strategy.

Diagrams

Legend: orchestrator path through the skill — rectangles are actions, diamonds are gates that loop back to the relevant draft step until satisfied. The two assets load at distinct steps (F and I), never together; the new mmdc gate (M) is the last hard refusal before save.

flowchart TD
    A[User intent: write PR description] --> B{Activation contract complete?}
    B -- no --> C[Collect missing inputs]
    C --> B
    B -- yes --> D[Read full diff, per-file intent]
    D --> E[Pick PROSE dimensions touched]
    E --> F[Load assets/pr-body-template.md]
    F --> G[Fill template from contract inputs]
    G --> H[Generate 1-3 mermaid diagrams + legends]
    H --> I[Load assets/section-rubric.md, run self-check]
    I --> J{All section gates pass?}
    J -- no --> G
    J -- yes --> M{mmdc validates every block?}
    M -- no --> H
    M -- yes --> N[Write single GFM file, return path]
Loading

Legend: bundle layout and load order — solid arrows are step-gated reference loads from SKILL.md; the mirror arrows are the apm install --target copilot deployment path that produces the .github/ copy and updates the lockfile.

flowchart LR
    SKILL["SKILL.md (activation contract + checklist + anti-patterns)"]
    TPL["assets/pr-body-template.md (11-section skeleton)"]
    RUB["assets/section-rubric.md (acceptance tests + final pass)"]
    LOCK["apm.lock.yaml (deployed-files entry)"]
    MIRROR[".github/skills/pr-description-skill/ (regenerated mirror)"]
    SKILL -- step 4 loads --> TPL
    SKILL -- step 7 loads --> RUB
    SKILL -- mirror sync --> MIRROR
    TPL -- mirror sync --> MIRROR
    RUB -- mirror sync --> MIRROR
    MIRROR -- recorded in --> LOCK
Loading

Trade-offs

  • v1 over-applied the source-code ASCII rule to PR-body output and shipped no mermaid validation — both fixed in this same PR before merge. The first draft of SKILL.md mandated ASCII-only output, which produced a 219-line flat body with no alerts, no collapsibles, no em dashes — reviewers had to scroll through hundreds of un-shaped lines. A maintainer pointed out that PR comments are rendered by GitHub's Primer engine, not by a Windows cp1252 terminal, so the encoding rule does not apply to the PR-body output. The skill was hardened in the same session: split charset rule (bundle source = ASCII; PR body = UTF-8 GFM), added > [!NOTE] / <details> / task list guidance, added a mandatory mmdc validation step with a pitfalls list, added a hard 150–220 line ceiling. This validates that the skill is amenable to fast iteration; it also means v1 readers should ignore the original draft.
  • Three-file split (SKILL + template + rubric). Chose split; rejected single-file. Rationale: template loads at synthesis, rubric at self-check; collapsing forces both into context at activation, defeating "Context arrives just-in-time, not just-in-case.". Cost: one extra mirror file, mitigated by the existing apm install --target copilot flow.
  • Opinionated 11-section order, not flexible. Chose fixed; rejected per-PR reordering. Rationale: stable shape across PRs is the reviewer's pattern-matching primitive. Cost: occasional verbosity on small PRs (a one-line Implementation per file is allowed, but the header still appears).
  • Mermaid prescribed (vs ASCII art / no diagrams), with deterministic mmdc gate. Chose mermaid + mmdc validation. Rationale: ASCII art is brittle; "no diagrams" loses the pattern-matching anchor. Cost: mermaid renders only on surfaces that support it; legend requirement compensates.
  • CHANGELOG line still mentions "ASCII-only output". That line was written for the v1 skill. Surgical scope kept; the CHANGELOG copy will be tightened in the same release pass.

Benefits

  1. Future PR bodies in microsoft/apm can be produced by any orchestrator (no apm-primitives-architect dispatch). Verifiable by: presence of all 11 sections in the next PRs that activate the skill.
  2. Mermaid diagrams in PR bodies render cleanly on first paste. Verifiable by: zero Syntax error in graph boxes on the next 5 PRs that use the skill.
  3. PR descriptions stop being commit-message rehashes. Verifiable by: presence of an anchored Problem section and a Trade-offs section with at least one rejected option per non-trivial decision.
  4. Body length stays scannable. Verifiable by: line count between 150 and 220 for typical PRs; long evidence inside <details>.

Validation

uv run apm audit --ci:

                                [>] APM Policy Compliance
Status     Check                    Message
[+]        lockfile-exists          No dependencies declared -- lockfile not required
[+]        ref-consistency          All dependency refs match lockfile
[+]        deployed-files-present   All deployed files present on disk
[+]        no-orphaned-packages     No orphaned packages in lockfile
[+]        config-consistency       No MCP configs to check
[+]        content-integrity        No critical hidden Unicode characters detected

[*] All 6 check(s) passed
Mirror parity (.apm/ vs .github/) and mmdc validation transcript
$ diff -q .apm/skills/pr-description-skill/SKILL.md .github/skills/pr-description-skill/SKILL.md
$ diff -q .apm/skills/pr-description-skill/assets/pr-body-template.md .github/skills/pr-description-skill/assets/pr-body-template.md
$ diff -q .apm/skills/pr-description-skill/assets/section-rubric.md .github/skills/pr-description-skill/assets/section-rubric.md
$ echo MIRROR OK
MIRROR OK

mmdc validation of the two mermaid blocks in this body (per the skill's mandatory step):

validating /tmp/mermaid-pr884/diag1.mmd
validating /tmp/mermaid-pr884/diag2.mmd
(no errors; SVGs written)

How to test

  • git fetch origin feat/pr-description-skill && git checkout feat/pr-description-skill — branch fetches cleanly.
  • uv run apm audit --ci — all 6 checks pass.
  • diff -qr .apm/skills/pr-description-skill .github/skills/pr-description-skill — no output (mirror parity).
  • Spot-check 3 anchored quotes by clicking through to https://danielmeppiel.github.io/awesome-ai-native/docs/prose/ and https://agentskills.io/skill-creation/best-practices and searching the verbatim phrase.
  • Self-application check: in a fresh session, ask any orchestrator "draft the PR body for the current branch"; confirm it activates pr-description-skill, gathers the 9-row activation contract before drafting, runs mmdc on every mermaid block, and emits a single GFM file in the 150–220-line range.

Captures the meta-pattern from PR #882 as a reusable Claude-style
skill bundle so future PR bodies in microsoft/apm meet the same bar
(anchored verbatim quotes, mermaid diagrams, honest PROSE alignment
matrix, explicit trade-offs, ASCII-only output) without per-PR
specialist subagent intervention.

Bundle layout (3 files):
- SKILL.md: activation contract, 11-section body shape, cite-or-omit
  rule, output contract, anti-patterns, gotchas.
- assets/pr-body-template.md: heading skeleton with inline acceptance
  hints; loaded only at synthesis (Progressive Disclosure).
- assets/section-rubric.md: per-section quality bar + acceptance
  tests; loaded only at the self-check pass.

Self-applied: this PR's body was generated by following the skill
end-to-end. The skill's own section rubric was run against the
draft and all 11 sections passed before opening the PR.

Validation: apm audit --ci passes all 6 baseline checks; ASCII
purity verified across all 3 new files; .apm/ <-> .github/ mirror
parity confirmed. apm.lock.yaml also includes an incidental
correction of a stale cicd.instructions.md hash entry that was
drift on main (file content matches new hash, lock had old hash).

Predecessor: #882

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 23, 2026 21:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new reusable pr-description-skill bundle to standardize high-quality, self-sufficient PR bodies for microsoft/apm, with a mirrored .github/ deployment and lockfile/changelog updates.

Changes:

  • Add pr-description-skill (SKILL.md + template + rubric) under .apm/skills/pr-description-skill/.
  • Mirror the skill bundle to .github/skills/pr-description-skill/ and record it in apm.lock.yaml.
  • Add an Unreleased changelog entry describing the new skill.
Show a summary per file
File Description
apm.lock.yaml Records the new locally deployed .github/skills/pr-description-skill directory and updates the CICD instructions hash.
CHANGELOG.md Adds an Unreleased entry under ### Added for the new PR description skill.
.apm/skills/pr-description-skill/SKILL.md Defines activation triggers, principles, required PR-body structure, and the execution checklist/output contract.
.apm/skills/pr-description-skill/assets/pr-body-template.md Provides the 11-section PR body template used at synthesis time.
.apm/skills/pr-description-skill/assets/section-rubric.md Provides per-section acceptance tests and a final-pass checklist for self-checking.
.github/skills/pr-description-skill/SKILL.md Generated mirror of the .apm/ skill definition for Copilot target deployment.
.github/skills/pr-description-skill/assets/pr-body-template.md Generated mirror of the .apm/ PR body template asset.
.github/skills/pr-description-skill/assets/section-rubric.md Generated mirror of the .apm/ section rubric asset.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 3

Comment thread .apm/skills/pr-description-skill/assets/section-rubric.md Outdated
Comment thread .apm/skills/pr-description-skill/SKILL.md Outdated
Comment thread CHANGELOG.md Outdated
danielmeppiel and others added 2 commits April 23, 2026 23:40
…n, tighten ceilings

Same-PR hardening in response to maintainer review of #882
body produced by the v1 skill. Three findings, three fixes:

1. v1 over-applied the source-code ASCII rule to PR-comment output,
   producing flat 400-line walls with no GFM features. PR comments
   are rendered by GitHub's Primer engine, not Windows cp1252
   terminals -- the encoding rule does not govern them. SKILL.md
   now explicitly distinguishes: bundle source files MUST stay ASCII
   (subject to .github/instructions/encoding.instructions.md); the
   PR body OUTPUT is UTF-8 GFM. Encourages alerts (> [!NOTE]),
   <details> collapsibles, task lists, aligned tables, em dashes.

2. v1 had no deterministic mermaid validation. PR #882's third
   diagram (classDiagram) shipped with a parser error: a semicolon
   in a link label broke the lexer. Added a mandatory step in the
   execution checklist that extracts every mermaid block via awk
   and validates each one with mmdc (npx --yes -p
   @mermaid-js/mermaid-cli). The skill MUST NOT save the draft
   until every block validates. Added a Common mermaid pitfalls
   subsection covering semicolons in classDiagram, note-end-note
   in stateDiagram-v2, parens in flowchart labels.

3. v1 produced 220-400 line bodies. Tightened section caps (max 3
   anchored quotes in Problem, max 5 trade-offs, max 5 alignment-
   matrix rows, max 5 numbered How-to-test steps). Total target
   150-220 lines. Long content lives in <details> so the body
   stays scannable in the 60-second budget reviewers actually have.

Bundle source files remain ASCII-pure; apm audit --ci 6/6 pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per maintainer feedback: the alignment matrix is only meaningful for the
narrow class of PRs that introduce or modify agent primitives (skills,
agents, instructions). Adding it as default guidance for every PR pushes
authors toward filler -- either inflated all-5 columns without 'why not 5'
justification, or matrices that have no anchored Before/After to compare.

Removed from all 6 bundle files (source + mirror, byte-identical):
- SKILL.md: dropped frontmatter mention, principle #5 'Honest alignment
  matrix', the section row in 'Required body structure', the checklist
  step 'Decide which PROSE dimensions', the anti-pattern 'Unsupported
  alignment scores', the gotcha about inflation. Renumbered downstream
  principles, sections, checklist steps.
- pr-body-template.md: removed entire '## PROSE alignment matrix'
  section including the example table.
- section-rubric.md: removed the section-7 acceptance test and
  renumbered downstream sections; tightened final-pass checklist.

Mirrors verified identical via diff -r. ASCII purity preserved on
bundle source.

Net: SKILL.md 347->329, template 145->128, rubric 156->143.

Kept: anchored-quote / cite-or-omit rule, GFM output rule, mandatory
mmdc validation, concision ceilings, Trade-offs and Benefits sections
(general-purpose, not alignment-matrix-specific).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread .apm/skills/pr-description-skill/assets/pr-body-template.md Outdated
Comment thread .apm/skills/pr-description-skill/assets/pr-body-template.md Outdated
Comment thread .apm/skills/pr-description-skill/assets/pr-body-template.md Outdated
…, drop model frontmatter

Address 6 review comments on PR #884 (3 maintainer, 3 Copilot bot).

M1 (template L37, danielmeppiel): the 'Why these matter' line previously
anchored every failure to a verbatim PROSE / Agent Skills quote. That
hard-codes a citation type that only fits APM's primitives PRs. Per the
Reduced Scope principle and the templates-as-assets rule (templates
should accept the anchor type the PR naturally affords, not enforce
one), replace with a placeholder that signals 'anchor each claim to the
most credible source available -- doc URL, file path, prior PR, CLI
excerpt, or omitted'.

M2 (template L48, danielmeppiel): the Approach table over-specified
columns ('Principle' / 'Source' as verbatim quote + URL) and forced
4 columns even for trivial fixes. Same Reduced Scope principle: collapse
to a 2-column 'Fix (and why, if non-obvious)' table by default, with
explicit guidance that 3 columns is opt-in and the anchor column should
be dropped when most rows have none. Reduces reviewer cognitive load.

M3 (template L72, danielmeppiel): the literal flowchart example
(A[Start] -> B{Decision} -> C/D -> E[End]) is the same failure mode the
python-architect.agent.md fix from PR #882 addressed -- agents follow
verbatim and produce the trivial diagram. Per Calibrated Control,
replace the example block with an imperative placeholder: real names
from the diff, real branches, real side-effect markers; explicit
guidance to pick diagram type (flowchart / sequenceDiagram /
stateDiagram-v2 / classDiagram / erDiagram) by change shape; explicit
permission to omit the entire Diagrams section when no relationship is
genuinely non-trivial. Show-don't-tell is reserved for the shape we
actually want copied.

C1 (rubric L141-143, Copilot): the trailer-line checklist item used
inline backticks across a hard line break, which Markdown renders as
literal backticks. Collapsed to single-line inline code per the bot's
suggestion.

C2 (SKILL.md L13, Copilot): removed 'model: claude-opus-4.7' from
frontmatter. Verified parser at src/apm_cli/primitives/parser.py only
consumes name/description from SKILL.md frontmatter (the model field is
silently dropped). Cross-checked .apm/skills/*: zero other SKILL.md
files use model:. The 7 .apm/agents/*.agent.md files do use model:, but
that field is consumed by Claude Code's subagent dispatch (an agent.md
becomes an invokable subagent that selects its own model); SKILL.md
files are loaded as content into the active context, with no equivalent
model-selection hook. Keeping it on SKILL.md would set a non-standard
convention.

C3 (CHANGELOG L13, Copilot): rewrote the entry to reflect current
shape: 10 sections (the alignment matrix was removed at 72654f0),
single bullet, ending with (#884) per Keep-a-Changelog convention.
Dropped the 'Self-applied' clause that no longer reflects the current
PR body.

Mirror regenerated via 'apm install --target copilot'; .github/skills/
pr-description-skill matches .apm/skills/pr-description-skill exactly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants