Add fix-bug, add-test, and review-pr agent skills#10062
Add fix-bug, add-test, and review-pr agent skills#10062maliberty merged 5 commits intoThe-OpenROAD-Project:masterfrom
Conversation
Extend the AI agent skill set with three new cross-tool skills: - fix-bug: guides root cause analysis, fix implementation, and test creation for bug reports - add-test: handles test creation with dual CMake/Bazel registration (the most common contributor mistake) - review-pr: reviews PRs following the project's priority order (correctness > QoR > testing > architecture > style > process) Each skill supports Claude Code (.agents/skills/ via .claude/skills symlink) and Gemini CLI (.gemini/commands/*.toml). AGENTS.md updated with a skill reference table. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Minjae Kim <minjae.kim@baum-ds.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a set of AI agent skills for the OpenROAD project, including automated workflows for adding tests, fixing bugs, and reviewing pull requests. The changes also update the AGENTS.md documentation to list these new capabilities. Feedback was provided to improve the review-pr skill by ensuring the gh pr view command fetches the necessary commit SHA (headRefOid) and that the gh api call for posting comments correctly specifies the side and parameter types required by the GitHub API.
|
clang-tidy review says "All clean, LGTM! 👍" |
Address gemini-code-assist review: - Add headRefOid to gh pr view --json fields for commit_id - Use -F for line parameter (integer type) in gh api call - Add side="RIGHT" parameter for review comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Minjae Kim <minjae.kim@baum-ds.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
- add-test: fix CMake example (add module name arg), fix Bazel example (use TESTS list + comprehension pattern), fix ./regression -R usage - fix-bug: remove duplicated test instructions, reference add-test skill and docs/agents/testing.md instead - review-pr: never post to GitHub (draft locally for human to review), support URL or bare PR number, remove unused headRefOid field - AGENTS.md, .gemini: sync descriptions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: minjae <develop.minjae@gmail.com>
|
Thanks for the thorough review! All feedback has been addressed in ee685d1.
Please take another look when you have a chance. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
|
||
| ## AI Agent Skills | ||
|
|
||
| Skills are located in `.agents/skills/` (with `.claude/skills` symlink for Claude Code). |
There was a problem hiding this comment.
I don't see a .claude/skills symlink in this PR
| @@ -0,0 +1,201 @@ | |||
| --- | |||
| name: add-test | |||
There was a problem hiding this comment.
I'll accept this but I would like to see more use of c++ unit tests rather than tcl style tests. Would you follow up with something to cover that use model?
.agents/skills/fix-bug/SKILL.md
Outdated
| **Critical constraint**: Never modify files under `src/sta/`. OpenSTA is | ||
| managed upstream. If the root cause is in OpenSTA, report this finding | ||
| instead of patching it. | ||
|
|
||
| ## 3. Implement the fix | ||
|
|
||
| - Follow Google C++ Style Guide and OpenROAD coding practices | ||
| - Use modern C++20 features where appropriate | ||
| - Use `{...}` braces even for single-line statements | ||
| - Keep functions under 100 lines | ||
| - Use `int64_t` for area calculations to prevent integer overflow | ||
| - Add `const` qualifiers where possible | ||
| - Remove any unreachable code after `error()` or `throw` | ||
|
|
||
| Read `docs/contrib/CodingPractices.md` if unfamiliar with project idioms. |
There was a problem hiding this comment.
This is already covered in AGENTS.md. Is there a need to repeat it here?
|
|
||
| Read `docs/contrib/CodingPractices.md` if unfamiliar with project idioms. | ||
|
|
||
| ## 4. Create regression test |
There was a problem hiding this comment.
Could this reference the add-test skill rather than repeat it?
.agents/skills/review-pr/SKILL.md
Outdated
|
|
||
| ### Priority 6: Process | ||
|
|
||
| - DCO sign-off present? |
There was a problem hiding this comment.
Covered by the GHA so no need to review this
- add-test: expand C++ unit test section with fixture hierarchy, CMake/Bazel registration examples, correct include paths - fix-bug: remove CodingPractices.md duplicate, condense test section to add-test skill reference - review-pr: remove DCO check item (covered by GHA) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: minjae <develop.minjae@gmail.com>
|
Thanks for the review @maliberty! All feedback addressed in a63c03d:
|
|
clang-tidy review says "All clean, LGTM! 👍" |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: minjae <develop.minjae@gmail.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
3 participants
Summary
triage-issueskill.agents/skills/via.claude/skillssymlink) and Gemini CLI (.gemini/commands/*.toml)Motivation
The existing
triage-issueskill handles bug reproduction and test case minimization. These three skills cover the natural next steps in the contribution workflow:triage-issue(existing) -- reproduce and minimizefix-bug(new) -- trace root cause and fixadd-test(new) -- add tests with correct dual build system registrationreview-pr(new) -- review with OpenROAD-specific priorities