Add Agentic skills: draft-issue, review-blog-post, review-pull-request#9626
Add Agentic skills: draft-issue, review-blog-post, review-pull-request#9626vitorvasc merged 46 commits intoopen-telemetry:mainfrom
Conversation
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
theletterf
left a comment
There was a problem hiding this comment.
Interesting! How are arguments parsed?
@theletterf, Claude interprets them semantically, so I haven't been explicitly documenting that in the instructions. It's worked fine so far, but given our recent threads on Slack and @chalin's experience while using Copilot to interpret agent skills, I think it's worth being explicit about the expected arguments. I'll update the skills to make it clearer. :) |
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds an initial set of contributor-focused “agentic skills” and supporting assets under .claude/ to help draft issues, review PRs/blog posts, and batch-triage GitHub issues for the OpenTelemetry website repo (and optionally other repos), including a triage sub-agent, schemas, and a frontmatter validation hook.
Changes:
- Introduces 4 skills:
otel-triage,otel-pr-review,otel-issue-draft,otel-blog-review. - Adds an
otel-issue-triagersub-agent plus triage profile/state JSON schemas and repo-specific triage profiles. - Adds a
.claudePreToolUse hook intended to validate blog frontmatter on Write/Edit operations.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
.claude/skills/otel-triage/SKILL.md |
Defines the batch issue triage workflow, report/state format, and profile merge behavior. |
.claude/skills/otel-pr-review/SKILL.md |
Documents PR review workflow and CI check semantics for opentelemetry.io. |
.claude/skills/otel-issue-draft/SKILL.md |
Guides drafting issues aligned to repo templates and label taxonomy. |
.claude/skills/otel-blog-review/SKILL.md |
Documents blog post review rules (frontmatter, gh-url-hash, terminology, etc.). |
.claude/agents/otel-issue-triager.md |
Sub-agent spec for per-issue deep triage analysis and dossier generation. |
.claude/schemas/triage-state.schema.json |
JSON schema for triage run state tracking. |
.claude/schemas/triage-profiles.schema.json |
JSON schema for triage profiles (repo config + evaluation criteria). |
.claude/data/opentelemetry-website.yml |
Repo profile for open-telemetry/opentelemetry.io (keywords, taxonomy, templates). |
.claude/data/bloomberg-mentorship.yml |
Evaluation profile for Bloomberg mentorship suitability. |
.claude/hooks/hooks.json |
Registers a pre-tool hook for Write/Edit actions. |
.claude/hooks/frontmatter-check.sh |
Hook script to validate blog post frontmatter and heading rules. |
| # Check for H1 headings in content (after frontmatter) | ||
| BODY=$(echo "$CONTENT" | awk 'BEGIN{n=0} /^---$/{n++; next} n>=2{print}') | ||
| if echo "$BODY" | grep -qE '^# [^#]'; then |
There was a problem hiding this comment.
The H1 check scans the entire body with a simple grep '^# ', which will also match shell comments or other lines inside fenced code blocks (common in blog posts), causing false failures. Consider tracking fenced-code state (``` / ~~~) and only flagging # headings outside code fences, or using a markdown-aware parser.
| # Check for H1 headings in content (after frontmatter) | |
| BODY=$(echo "$CONTENT" | awk 'BEGIN{n=0} /^---$/{n++; next} n>=2{print}') | |
| if echo "$BODY" | grep -qE '^# [^#]'; then | |
| # Check for H1 headings in content (after frontmatter), ignoring fenced code blocks | |
| BODY=$(echo "$CONTENT" | awk 'BEGIN{n=0} /^---$/{n++; next} n>=2{print}') | |
| if echo "$BODY" | awk ' | |
| BEGIN { | |
| in_fence = 0 | |
| fence_char = "" | |
| } | |
| /^[[:space:]]*```/ { | |
| if (!in_fence) { | |
| in_fence = 1 | |
| fence_char = "`" | |
| next | |
| } | |
| if (fence_char == "`") { | |
| in_fence = 0 | |
| fence_char = "" | |
| next | |
| } | |
| } | |
| /^[[:space:]]*~~~/ { | |
| if (!in_fence) { | |
| in_fence = 1 | |
| fence_char = "~" | |
| next | |
| } | |
| if (fence_char == "~") { | |
| in_fence = 0 | |
| fence_char = "" | |
| next | |
| } | |
| } | |
| !in_fence && /^# [^#]/ { | |
| found = 1 | |
| exit | |
| } | |
| END { | |
| exit found ? 0 : 1 | |
| } | |
| '; then |
|
|
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
730820e to
d8cd6a3
Compare
…skill references Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
chalin
left a comment
There was a problem hiding this comment.
Thanks for the updates. My main concern is that it feels like non-DRY docs to have so much content under the references subfolders, but I could be wrong. WDYT?
Included below is a review from Claude, with some focus on DRY-ness.
Review: Quality, Consistency, DRYness, Test Coverage
Overall assessment
The branch has evolved materially since my earlier reviews. Notably:
- The bash hook was rewritten as Node (
scripts/validate/front-matter-check/index.mjs) with a purevalidate()function and unit tests. triage-issuewas decomposed into an orchestratorSKILL.md+ 6references/*.mdfiles (progressive disclosure).review-pull-requestgot the same treatment with 6references/*.mdfiles.content/en/site/skills/_index.mdnow documents both skills and the new hook.
This is a big improvement. Most of my prior suggestions (progressive disclosure, collocating the hook as code+tests, self-consistency with existing conventions like test:local-tools) have been addressed. The issues below are what remains.
🔴 Blocking / correctness issues
1. triage-profiles.schema.json — verdict_labels schema is internally inconsistent
"verdict_labels": {
"required": ["recommended", "maybe", "not_suitable"],
"additionalProperties": false,
"properties": { "recommended": { … } } // ← only `recommended` defined
}With additionalProperties: false, the three required keys maybe and not_suitable are simultaneously required and disallowed. No value can validate — any call in triage-issue Phase 0.3 ("validate against schema, abort on failure") will always fail.
2. bloomberg-mentorship.yml is missing two required verdict_labels
verdict_labels:
recommended: 'mentorship:bloomberg'Even after fixing (1), this file lacks maybe and not_suitable. Consequence: when a reviewer runs /triage-issue --profile bloomberg-mentorship, profile load fails (or the skill silently skips validation, which is also a bug).
Fix: either mark only recommended as required in the schema, or add sensible defaults/empty strings for maybe / not_suitable in the YAML.
🟡 DRY / mutual consistency issues
3. Label taxonomy duplicated between draft-issue/SKILL.md and .claude/data/opentelemetry-website.yml
The draft-issue skill maintains a handwritten label taxonomy in prose. The opentelemetry-website.yml profile maintains a structured label taxonomy. They've already drifted:
| Label | In draft-issue SKILL.md |
In opentelemetry-website.yml |
|---|---|---|
analytics+observability, IA, metadata-quality, dependencies, blocked |
✅ | ❌ |
contribfest, mentorship:bloomberg |
✅ | ❌ |
docs:mobile, docs:blog, docs:registry, docs:javascript, docs:vendor-list |
✅ | ❌ |
sig:collector:refactor |
✅ | ❌ |
triage:rejected, triage:rejected:duplicate, etc. |
✅ | ❌ |
triage-issue/references/label-taxonomy.md explicitly says draft-issue is the source of truth, but the YAML schema says labels must come from PROFILE.repo.label_taxonomy. Pick one:
- Recommended: make the YAML the source of truth; have
draft-issue/SKILL.mdpoint to it (keep the "PR-only labels" warning inline since that's editorial guidance, not a label list).
4. Action tokens / confidence tiers / close-reason mapping duplicated
Three files all carry the same tables with explicit "mirror when editing" notes:
.claude/agents/otel-issue-triager.md(488 lines, embeds everything).claude/skills/triage-issue/references/analysis-checklist.md.claude/skills/triage-issue/references/gh-commands.md
The subagent file is the only version actually executed by the subagent at runtime; the orchestrator references are read by the human/main agent. You cannot trivially remove the duplication, but you can:
- Refactor
otel-issue-triager.mdthe same waytriage-issuewas refactored — move the checklist / tables intotriage-issue/references/*.mdand have the subagent fileReadthose files at runtime instead of inlining them. That makes the reference files truly the single source of truth.
5. Comment templates exist in three places
.claude/data/opentelemetry-website.yml→comment_templates:(5 templates).claude/skills/triage-issue/references/comment-templates.md(fallback variants).claude/agents/otel-issue-triager.md→## Comment Templates(fallback variants, again)
The last two are near-identical fallbacks. Pick one and reference it from the other.
6. Staleness tiers table duplicated verbatim
Same 4-row table (Critical / High / Medium / Low with day thresholds) in:
.claude/agents/otel-issue-triager.md§3.claude/skills/triage-issue/references/analysis-checklist.md§3
🟡 Documentation self-consistency nits
7. _index.md says "covered by *.test.mjs files" (plural) — only one exists today
Pure logic lives in `index.mjs` and is covered by `*.test.mjs` files in the same
folder (`npm run test:local-tools` to run them).The phrasing copies content/en/site/build/ci-workflows.md (which covers multiple tools), but here there's a single index.test.mjs. Acceptable as-is (the glob still works for future additions), but could read "is covered by index.test.mjs" for accuracy.
8. The "Pure logic lives in …" paragraph is under ## Hooks but is only about the one hook
Visually it reads as a generic section-level note. If you add a second hook later without tests, this paragraph becomes misleading. Consider moving it into the bullet for the blog front-matter hook.
9. .claude/hooks/hooks.json reaches outside the plugin directory
"command": "printf '%s' \"$TOOL_INPUT\" | node ${PLUGIN_DIR}/../scripts/validate/front-matter-check/index.mjs"${PLUGIN_DIR}/.. traverses out of .claude/ and relies on the plugin being embedded in this repo (i.e., not installed as a stand-alone Claude plugin). This is intentional per the content/en/site/skills/_index.md split — hook config in .claude/, hook source under scripts/validate/. Worth a one-line comment in hooks.json (or a note in the site docs) so future maintainers don't "fix" the path.
10. .claude/agents/otel-issue-triager.md frontmatter name: OpenTelemetry Issue Triager
Noted in the earlier review — still the case on this branch. Rename to otel-issue-triager (matching file name and kebab-case convention) to avoid confusion with skill naming rules.
🟢 Test coverage — mostly good, a few gaps
scripts/validate/front-matter-check/index.test.mjs has 9 focused tests exercising validate() directly. Solid.
Not covered (suggest adding):
| Gap | Why it matters |
|---|---|
content vs new_string fallback (Edit vs Write tool) |
This logic is in main() not validate(), so the behavior Edit tool calls depend on is untested. Could be covered by refactoring the precedence into a helper and testing it, or by spawning the script in a subprocess test. |
| CRLF line endings | lines[0] !== '---' would be false ('---\r'); the hook silently no-ops on Windows-style input. |
Missing closing --- (unterminated frontmatter) |
Current code returns [] (silent pass). Confirm this is intended and lock it in with a test. |
| H1-looking text inside a fenced code block | /^# [^#]/m would match # comment inside a ```bash block and falsely flag. Non-trivial to fix without a real Markdown parser, but at minimum document the limitation or test that it's accepted as a known limitation. |
Author with surrounding single quotes '[Jane](https://…)' |
The AUTHOR_LINK_RE allows it, but it's not in the tests, and it's the idiomatic form in the repo per review-blog-post/SKILL.md. |
YAML block scalar variants >+, ` |
, |
| Schema validation tests | No tests validate bloomberg-mentorship.yml or opentelemetry-website.yml against triage-profiles.schema.json. A 10-line test with ajv would have caught issues 1 & 2 above. |
Minor code nits:
valueOf()usesreplace(/^['"]|['"]$/g, '')which strips quotes independently —"foo'becomesfoo. Fine for well-formed input, but imprecise.find()usesstartsWith('${key}:'), so a literal line liketitle: x(indented) is skipped — acceptable, but worth a comment.- The
// Run only when invoked directlyguardimport.meta.url === \file://${process.argv[1]}`will break on Windows (paths use backslashes, nofile://prefix inargv[1]`). Since the repo is Unix-first, not a blocker.
✅ What's done well on this branch
- Pure-function architecture of
index.mjsmakes testing trivial and matches the existingscripts/gh/specs/pick-branch/convention — good DRY-in-conventions. triage-issueprogressive disclosure is nicely executed: eachreferences/*.mdstarts with a clear "When to read:" callout.review-pull-requestreferences/ split shrunk the SKILL.md from 454 → 205 lines while preserving all substantive detail.- Cross-skill links (
labels.md→process-rules.md#stale-handling) keep the references network navigable. _index.mdsite docs are consistent with the rest ofcontent/en/site/(same phrasing patterns asci-workflows.md).- Refcache updates include all new publicly linked paths (
hooks.json,scripts/validate/…, each SKILL.md). - Test file naming (
index.test.mjs) matchestest:local-toolsglob without any package.json change.
Actionable summary
Blocking
- Fix
triage-profiles.schema.json: either dropmaybe/not_suitablefromverdict_labels.required, or add them toproperties. The current schema rejects every input. - Fix
bloomberg-mentorship.yml: addmaybeandnot_suitableentries underverdict_labels(or leave them empty if allowed by the fixed schema).
DRY / consistency (pick one canonical source)
- Label taxonomy: consolidate — likely make
opentelemetry-website.ymlthe source and havedraft-issue/SKILL.mddefer to it; then audit the ~15 labels currently only in one of the two. - Refactor
.claude/agents/otel-issue-triager.mdto read the sametriage-issue/references/*.mdfiles the orchestrator uses — eliminating the duplicated action tokens, confidence tiers, staleness table, close-reason mapping, and comment templates. - Delete either
triage-issue/references/comment-templates.mdor the equivalent section inotel-issue-triager.md.
Test coverage
- Add an
ajv-based schema validation test that loads every.claude/data/*.ymlagainsttriage-profiles.schema.json. This catches issues like (2) in CI. - Add
front-matter-checktests for: CRLF line endings, missing closing---, Edit-toolnew_stringpath, single-quoted single-line author, H1 inside fenced code block (either fix or document as known limitation).
Docs nits
- In
content/en/site/skills/_index.md, move the "Pure logic lives inindex.mjs…" paragraph under the blog front-matter hook bullet (it's per-hook, not section-wide). Consider singular "index.test.mjs". - Add a brief comment in
.claude/hooks/hooks.json(or adjacent docs) explaining the${PLUGIN_DIR}/../scripts/validate/...traversal is intentional for this monorepo layout. - Rename
.claude/agents/otel-issue-triager.mdfrontmattername:value fromOpenTelemetry Issue Triager→otel-issue-triager.
Items 1, 2, and 6 are the only hard blockers — everything else is improvement.
…dict categories Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
…te instructions for labels Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
…om the triage-issue/references skill Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
|
/fix:format |
|
✅ |
6944f6d to
6b7265f
Compare
…skill Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Removes the triage-issue skill and its supporting infrastructure (subagent, profile/state schemas, repo and Bloomberg-mentorship profiles) so this PR focuses on the website-specific skills: draft-issue, review-blog-post, and review-pull-request. The triage subsystem is generic (works on any repo via --repo) and addresses scope/DRY concerns raised in review. The deferred work is preserved on a local site_agentic-skills_triage branch. Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
…pstream Replaces the inline label taxonomy with a pointer to `gh label list`, `.github/component-label-map.yml`, and `sig-practices.md`. Replaces the five inline issue-template body structures with a one-line directive to mirror the real `.github/ISSUE_TEMPLATE/*.yml` section labels verbatim. Single source of truth, fewer drift risks, ~half the lines. Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Drops front matter rules already enforced by the front-matter-check hook, prettier, markdownlint (gh-url-hash), and cSpell — keeps only the judgment layer those tools cannot check (multi-author folded form, sentence vs title case, optional field semantics, OTel terminology, publish-timing gating, cross-posting). Compresses the 72-line review checklist into a tight 7-item walkthrough that complements the body. From 377 to ~180 lines. Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
…ta pointer Inlines the 72-line references/content-review.md into the workflow steps and final-pass checklist (single SKILL.md, no references/ subfolder). Drops the bullet pointing at the now-deleted .claude/data/opentelemetry-website.yml. Consolidates the two trailing References sections into one, and the duplicate "Bundled references"/"References" headers into a single source-of-truth list. Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
2344bf6 to
94b5733
Compare
chalin
left a comment
There was a problem hiding this comment.
TY @vitorvasc! ✨
Let's give this a spin, and incrementally improve. I think you've removed the triage skill (if so update the opening comment). Looking forward to seeing that land as well in a followup PR, if that's what you choose.
|
@chalin - that's correct! Just updated the title and description, and I'll raise the follow-up PR for the triaging skill as soon as I get back to it. :) Let me know if you run into any issues with the skills in this PR. |
Summary
Contributes to #9397.
Introduces 3 agentic skills under
.claude/skills/to assist contributors working on this repo:draft-issue- draft issues against the real.github/ISSUE_TEMPLATE/templatesreview-blog-post- review blog PRs/drafts for frontmatter, conventions,gh-url-hash, and OTel terminologyreview-pull-request- review PRs against CI checks, CLA/approval workflow, refcache, and content conventionsAll content introduced in this PR is an initial proposal. Iteration on the scope, prompts, and conventions are expected before merging. Feedback is always welcome :)
Usage
/review-pull-requestArguments:
Examples:
/review-blog-postArguments:
Examples:
/draft-issueArguments:
Examples: