feat(skills): add title tag normalization maintainer skill - #2292
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
📝 WalkthroughWalkthroughA new maintainer skill for normalizing GitHub issue and PR title bracket tags is introduced, consisting of documentation, a TypeScript script that integrates with GitHub CLI to scan and optionally remove bracket-enclosed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts (3)
100-108: Consider validating the--repoformat.The
--repoargument accepts any string but the GitHub API expectsOWNER/REPOformat. Invalid formats will fail at API call time with a less clear error.💡 Optional format validation
if (arg === "--repo") { const value = argv[i + 1]; if (!value || value.startsWith("--")) { throw new Error("--repo requires OWNER/REPO"); } + if (!/^[\w.-]+\/[\w.-]+$/.test(value)) { + throw new Error("--repo must be in OWNER/REPO format"); + } options.repo = value; i += 1; continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts around lines 100 - 108, The --repo parsing currently accepts any string; add validation after assigning options.repo to ensure it matches OWNER/REPO (e.g. one non-empty segment, a single slash, then another non-empty segment). In the normalize-title-tags.ts command-line parsing block (the branch that checks arg === "--repo" and sets options.repo), validate options.repo against a simple pattern like "^[^/]+/[^/]+$" and if it fails throw a clear Error("--repo requires OWNER/REPO") (or augment the existing message) so invalid values are rejected early with a helpful message.
76-78: Consider handling JSON parse errors more gracefully.If
gh apireturns an error response or malformed JSON,JSON.parsewill throw a genericSyntaxError. Wrapping this with a try-catch to provide a more descriptive error message would improve debuggability.💡 Optional improvement
function ghJson(args: string[]): unknown { - return JSON.parse(run("gh", args)); + const output = run("gh", args); + try { + return JSON.parse(output); + } catch { + throw new Error(`Failed to parse GitHub API response: ${output.slice(0, 200)}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts around lines 76 - 78, The ghJson function currently calls JSON.parse(run("gh", args)) which will throw a generic SyntaxError on malformed or error responses; wrap the parse call in a try-catch inside ghJson, capture the raw output from run("gh", args), and on parse failure throw or return a new Error that includes a clear message with the failing args (args), the raw output, and the original error (e.g., `new Error(\`ghJson parse failed for args=${JSON.stringify(args)}: ${err.message} - output=${raw}\`)`) so callers can diagnose API errors more easily while preserving the original error information.
254-266: Consider rate limiting and partial failure handling.When applying changes to many items, rapid sequential API calls could trigger GitHub's secondary rate limits. Additionally, if a PATCH fails mid-way, the script exits without reporting which items were successfully updated.
💡 Optional: Add delay and track successful updates
-function applyMatches(options: Options, matches: Match[]): void { +function applyMatches(options: Options, matches: Match[]): number { + let updated = 0; for (const match of matches) { - run("gh", [ - "api", - "-X", - "PATCH", - `repos/${options.repo}/issues/${match.number}`, - "-f", - `title=${match.newTitle}`, - ]); - console.log(`UPDATED #${match.number}: ${match.oldTitle} -> ${match.newTitle}`); + try { + run("gh", [ + "api", + "-X", + "PATCH", + `repos/${options.repo}/issues/${match.number}`, + "-f", + `title=${match.newTitle}`, + ]); + console.log(`UPDATED #${match.number}: ${match.oldTitle} -> ${match.newTitle}`); + updated += 1; + } catch (error) { + console.error(`FAILED #${match.number}: ${error instanceof Error ? error.message : error}`); + } } + return updated; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts around lines 254 - 266, The applyMatches function currently fires run("gh", ...) calls in a tight loop which can hit GitHub secondary rate limits and stops on the first failure; modify applyMatches to perform each PATCH inside a try/catch around run, implement a small delay/throttle between iterations (e.g., sleep/backoff) to avoid rate limiting, record successes and failures (e.g., arrays of match.number or Match objects) so you can report which items were updated and which failed, and optionally implement a simple retry for transient errors before marking a failure; reference applyMatches, Match, Options, and the run call to locate where to add the delay, error handling, retry and final reporting/logging instead of unguarded console.log.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
@.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts:
- Around line 100-108: The --repo parsing currently accepts any string; add
validation after assigning options.repo to ensure it matches OWNER/REPO (e.g.
one non-empty segment, a single slash, then another non-empty segment). In the
normalize-title-tags.ts command-line parsing block (the branch that checks arg
=== "--repo" and sets options.repo), validate options.repo against a simple
pattern like "^[^/]+/[^/]+$" and if it fails throw a clear Error("--repo
requires OWNER/REPO") (or augment the existing message) so invalid values are
rejected early with a helpful message.
- Around line 76-78: The ghJson function currently calls JSON.parse(run("gh",
args)) which will throw a generic SyntaxError on malformed or error responses;
wrap the parse call in a try-catch inside ghJson, capture the raw output from
run("gh", args), and on parse failure throw or return a new Error that includes
a clear message with the failing args (args), the raw output, and the original
error (e.g., `new Error(\`ghJson parse failed for args=${JSON.stringify(args)}:
${err.message} - output=${raw}\`)`) so callers can diagnose API errors more
easily while preserving the original error information.
- Around line 254-266: The applyMatches function currently fires run("gh", ...)
calls in a tight loop which can hit GitHub secondary rate limits and stops on
the first failure; modify applyMatches to perform each PATCH inside a try/catch
around run, implement a small delay/throttle between iterations (e.g.,
sleep/backoff) to avoid rate limiting, record successes and failures (e.g.,
arrays of match.number or Match objects) so you can report which items were
updated and which failed, and optionally implement a simple retry for transient
errors before marking a failure; reference applyMatches, Match, Options, and the
run call to locate where to add the delay, error handling, retry and final
reporting/logging instead of unguarded console.log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bbb1fb54-9c20-4645-a5eb-6a0ec8941975
📒 Files selected for processing (3)
.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts.agents/skills/nemoclaw-skills-guide/SKILL.md
|
Heads up — the |
ericksoa
left a comment
There was a problem hiding this comment.
Clean utility — dry-run default, post-apply verification, case-insensitive matching. LGTM.
Summary
Add a maintainer skill for normalizing bracketed
NemoClawtags in issue and PR titles. The skill provides a dry-run workflow, a reusable TypeScript helper that matches tags case-insensitively anywhere in the title, and verification guidance so maintainers can apply the cleanup safely.Changes
.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.mdwith a dry-run-first workflow for previewing, applying, and verifying title tag cleanup.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.tsto find and remove bracketednemoclawtags case-insensitively from GitHub issue and PR titles.agents/skills/nemoclaw-skills-guide/SKILL.mdto include the new maintainer skill in the catalogType of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Documentation