docs(spf): describe pr skill#1624
Open
cjpillsbury wants to merge 5 commits into
Open
Conversation
Reviewer-oriented PR-description discipline for SPF-area PRs. Bucket-calibrated reading order (Runtime / Doc cascades / Agent skills), five-label depth set (scope × intensity), smoke-test section with proposal pattern (human-in-the-loop URL + observables), choice/alternative/why decisions, link discipline that survives merge. Methodology validated via #1584 and #1605 description rewrites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring merge-behaviors → spf-merge-behaviors, refactor-behavior → spf-refactor-behavior, split-behavior → spf-split-behavior into line with the existing SPF-namespace convention. Updates frontmatter `name:`, cross- references across sibling skills (spf-create-behavior, spf-document-feature, spf-document-use-case, spf-implement-feature, spf-implement-use-case, spf-update-behavior), and the README skills table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (9)
Players (5)
Skins (30)
UI Components (36)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (8)
Skins (27)
UI Components (30)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (10)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
Three refinements surfaced while running the skill on its own PR: - Bucket calibration — empty-bucket guidance for docs/skill-only PRs. Omit Bucket 1 entirely when no runtime; drop bucket numbering when only 1–2 buckets populated; the PR's load-bearing artifact gets [Skim file] regardless of which bucket it sits in. - Smoke-test threshold — sharper omit-vs-explicit-note rule. Explicit note for large PRs (~10+ files or ~500+ lines) regardless of runtime impact; omit for small docs-only PRs. - Carry-forward — when a sibling PR has already shipped, log the gap as a Reviewer callout in the current PR rather than retroactively editing the closed PR's description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three refinements informed by review feedback on #1605: - Jargon discipline — don't inherit meta-discourse labels (Tier N, Phase N, Stage A/B) from feature docs without translating to substance. Gloss table is for durable code terms only. - Reviewer comparative advantage — Look-for prose names concerns (invariants, ordering, contracts) not impl details (function signatures, ctx field lists). Human reviewers do comprehension; LLM-driven reviewers do per-line. Description should serve the human. - Section contractibility — Sections 4–8 are include-if-load-bearing, not required. Empty Reviewer callouts / Breaking / Test plan sections omitted rather than performed as "None" boilerplate; perfunctory test-plan checklists cut. Adds "Less is more" framing to Compression discipline; new pitfalls (AI-enthusiasm verbose mode, feature-doc jargon, performed empty sections, routine-impl design decisions); 4 new audit checklist items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GitHub already shows X files / +Y / −Z above every PR body, so restating the diff stat at the top of the For-reviewers section adds zero signal. The line is now opt-in: include only when paired with a weight-imbalance call-out reviewers should know about (agent-skill- heavy PR, mass rename, generated-code-heavy). Bare numerical recitation always cut. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Adds
/spf-describe-pr— a new agent skill that drafts reviewer-oriented PR descriptions for SPF-area PRs. Eight-section structure (TL;DR → How to read → Smoke test → What changed → Decisions → Callouts → Breaking → Tests), five-label depth grid (scope × intensity), smoke-test section with a human-in-the-loop proposal pattern, link discipline that survives merge. Methodology validated by applying the skill manually to rewrite #1584 and #1605 descriptions before shipping.Also "smuggles in" a long-pending rename: the SPF-specific behavior-workflow skills get a consistent
spf-prefix (refactor-behavior→spf-refactor-behavior,merge-behaviors→spf-merge-behaviors,split-behavior→spf-split-behavior), with cross-reference propagation across 6 sibling skills, the README, and 2 design docs.No runtime impact. Documentation + Claude Code skills only.
For reviewers — how to read this PR
No runtime; the substantive new content is one new skill file. Everything else is the rename cascade.
Design doc cross-ref cascade (skim or skip)
internal/design/spf/conventions/behaviors.md(+6/−6),internal/design/spf/features/clusters.md(+1/−1) — token swap propagating the skill rename; no semantic change.Agent skills (focus on the new skill)
Skim file:
.claude/skills/spf-describe-pr/SKILL.md(+770, new) — the PR's load-bearing artifact. Read prose for: eight-section structure, five-label depth grid ([Read fully]/[Targeted careful read]/[Skim file]/[Skim structure only]/[Skim or skip]), Smoke-test discipline (proposal pattern, no fabrication, no hard-coded hostname), Compression / Link / Cursor-Bugbot / Stacked-PR / Carry-forward disciplines, Audit-pattern checklist, Common-pitfalls list.Skim or skip:
spf-refactor-behavior,spf-merge-behaviors,spf-split-behavior) — directory + frontmattername:+ body token swap; no semantic changespf-create-behavior,spf-document-feature,spf-document-use-case,spf-implement-feature,spf-implement-use-case,spf-update-behavior) — token swap only.claude/skills/README.md(+6/−4) — quick-reference row + skills-table entriesSmoke test
No reviewer-actionable smoke test for this PR — skill-only / docs-only changes with no runtime surface. The skill's methodology was validated by applying it manually to the descriptions of #1584 and #1605 before this PR shipped; reviewers wanting to test-drive can run
/spf-describe-pr <pr-num>against any open SPF-area PR.What changed — by surface
/spf-describe-prskill — eight-section structure. TL;DR → For reviewers (how to read) → Smoke test → What changed → Notable design decisions → Reviewer callouts → Breaking → Test plan. Smoke test sits at section 3 (high in the order) so a reviewer who can confirm the change works in 30 seconds reads the rest with different eyes than one who can't. Each section has explicit rules for shape, scope-narrowing, and what to cut.Five-label depth grid. Per-file labels in How to read live on a 2D grid: scope of attention × intensity of attention.
[Read fully]/[Targeted careful read]/[Skim file]/[Skim structure only]plus a fifth tier[Skim or skip]. Explicit rule against medium-tier labels like[Spot-check]— they collapse into the four cells, and the per-file Look for prose carries mode-of-attention nuance.Smoke-test discipline with human-in-the-loop. The skill scans the diff for smoke-test signals (sandbox templates touched, demo presets changed, new observable behaviors, bug fixes with user-visible signatures) but cannot reliably synthesize meaningful URLs + observables. Mandates an
AskUserQuestion-based proposal pattern where the author confirms URL + source + observables before the section is written. Explicit guidance against fabricating smoke tests when none applies (skill-only / docs-only / internal refactors) — use the explicit "no smoke-test applicable" note when reviewers might expect one and look for it.Bucket-3-friendly framing for docs/skill-only PRs. The bucket calibration (Runtime / Design docs / Agent skills) anticipates PRs that mix categories; for an all-Bucket-3 PR like this one, the audit-pattern checklist and bucket-framing rules still apply but Bucket 1 is omitted entirely. This PR is itself the first worked example.
SPF-prefix rename.
merge-behaviors → spf-merge-behaviors,refactor-behavior → spf-refactor-behavior,split-behavior → spf-split-behaviorbrings the behavior-workflow skills into line with the existing SPF-namespace convention (spf-create-behavior,spf-document-feature, etc.). Token swap with no semantic change; propagates frontmattername:, cross-references in 6 sibling skills, the README skills table, and 2 design docs ininternal/design/spf/.Notable design decisions
Smoke test as section 3, not a subsection of Test plan. Reviewers can confirm "does this work in my hands?" before reading 800 lines of design narrative; that confirmation changes how they read the rest. Alternative considered: Smoke test as a subsection of Test plan (section 8). Rejected because reviewers naturally read top-down — burying the smoke-test at the bottom defeats its purpose. Authors writing only a Test-plan checklist line ("Manual sandbox verification") give reviewers a description rather than a runnable URL.
Smoke test is human-in-the-loop, not skill-synthesized. The skill scans the diff for candidates but explicitly asks the user to confirm URL + source + observables via
AskUserQuestionbefore writing the section. Alternative considered: the skill picks defaults (closest-matching sandbox template + first manifest in some source registry). Rejected because the wrong URL or observable burns reviewer time worse than a missing section, and the author always has context the skill doesn't.Path-only smoke-test URLs, no hostname. Reviewers append the path to whichever host they use — local Vite, Vercel preview, Netlify preview, etc. Alternative considered: hard-code the Vercel preview-URL convention. Rejected because deploy-preview conventions vary per project and per PR, and committed URLs rot when redeploys happen.
Rename SPF-specific skills to
spf-prefix. Bringsmerge-behaviors,refactor-behavior,split-behaviorinto line withspf-create-behavior,spf-document-feature, etc. Alternative considered: leave them as-is. Rejected because the naming inconsistency obscures the SPF-namespace boundary — a reader scanning.claude/skills/should be able to identify which skills are SPF-specific vs. cross-cutting (docs,git,commit-pr) from the file names alone.Bundle the rename with the new skill in one PR (not split). The rename ("smuggled in") rides on the same branch as the new skill. Alternative considered: two PRs (skill first, rename second). Rejected because both are documentation-only with zero merge-conflict risk, and a single squash-commit on main keeps history compact.
Reviewer callouts — known limitations
Breaking changes
The three skill renames (
/refactor-behavior→/spf-refactor-behavior, etc.) don't break any committed code but may break user muscle memory or saved local Claude Code commands. The old names are gone — no aliasing shim. SPF stays alpha; no public-API surface affected.Test plan
pnpm typecheck— clean (no code changes)pnpm check:workspace— cleanrefactor-behavior,merge-behaviors,split-behaviorwithoutspf-prefix) — zero hitsNote
Low Risk
Documentation and agent-skill changes only; slash-command renames may break saved workflows but do not affect shipped product code.
Overview
Adds
/spf-describe-pr, a large new Claude skill that tells agents how to draft reviewer-oriented SPF PR bodies (eight sections, runtime/design/skills buckets, five depth labels, smoke-test + link/compression rules, user check-in beforegh pr edit).Renames the behavior workflow skills to the shared
spf-namespace (refactor-behavior→spf-refactor-behavior,merge-behaviors→spf-merge-behaviors,split-behavior→spf-split-behavior) and propagates those command names through sibling skills,.claude/skills/README.md, andinternal/design/spf/conventions/behaviors.md/features/clusters.md.No runtime or library code — agent skills and internal design doc cross-refs only.
Reviewed by Cursor Bugbot for commit c434682. Bugbot is set up for automated code reviews on this repo. Configure here.