NO-JIRA: Improve AI skill quality based on review comment analysis - #531
NO-JIRA: Improve AI skill quality based on review comment analysis#531bryan-cox wants to merge 1 commit into
Conversation
Analyzed 78 review comments (62 actionable) from 2025-01-01 to 2026-06-05. Added anti-patterns for dead code, unrelated changes, unresolved TODOs in validation. Added pre-commit quality checks and push verification to solve.md. Strengthened push verification and restructure-commits guidance in address-reviews.md. Signed-off-by: JIRA Agent <jira-agent@hypershift.dev> Commit-Message-Assisted-by: Claude (via Claude Code)
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@bryan-cox: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
WalkthroughThis PR strengthens operational guidelines across two AI helper commands by adding explicit verification checkpoints and quality validation rules: the ChangesOperational Verification Rules
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/utils/commands/address-reviews.md`:
- Around line 289-290: Update Step 4 to stop assuming remotes and disallow
force-pushes: replace any use of "git push --force-with-lease" and any implicit
"origin" assumptions with a discovery step that lists configured remotes and
prompts the user to pick one, require explicit user confirmation before
performing any push, and remove/forbid all force-push variants; keep references
to the restructure skills ("restructure-commits" and
"restructure-hypershift-commits") but ensure they follow the new flow (discover
remotes, request permission, then use a safe non-force push).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 785db2e8-1d35-4f2b-a385-76263acd3df5
📒 Files selected for processing (2)
plugins/jira/commands/solve.mdplugins/utils/commands/address-reviews.md
| - **Never claim work is done without verifying the push**: After posting replies that say changes were made, the push in Step 4b MUST succeed and be verified in Step 4c. If the push fails or is skipped, the replies become lies — reviewers will see "Done" but find no code changes. This has been the single most repeated process failure. | ||
| - **Use the restructure-commits skill when asked**: When a reviewer requests commit reorganization (e.g., "restructure commits", "reorganize commits"), use the `restructure-commits` or `restructure-hypershift-commits` skill rather than manually rebasing. Reviewers have had to ask for this multiple times across PRs. |
There was a problem hiding this comment.
Push-verification guidance conflicts with current push mechanics; align Step 4 with repo safety rules.
These new rules are directionally correct, but the command still relies on git push --force-with-lease and origin-assumed verification in the same workflow. Please update the flow to (1) require explicit user permission before push, (2) remove all force-push variants, and (3) discover remotes before using them.
As per coding guidelines, “NEVER allow force push operations anywhere,” “Commands and skills must NEVER push without explicit user permission,” and “Code should NEVER assume git remote names like 'origin' or 'upstream' without first discovering them.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/utils/commands/address-reviews.md` around lines 289 - 290, Update
Step 4 to stop assuming remotes and disallow force-pushes: replace any use of
"git push --force-with-lease" and any implicit "origin" assumptions with a
discovery step that lists configured remotes and prompts the user to pick one,
require explicit user confirmation before performing any push, and remove/forbid
all force-push variants; keep references to the restructure skills
("restructure-commits" and "restructure-hypershift-commits") but ensure they
follow the new flow (discover remotes, request permission, then use a safe
non-force push).
|
For golang would it make sense to run |
|
lgtm |
|
/close opened this as an experiement. will follow up with an official one. |
|
@bryan-cox: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
Summary
Automated analysis of review comments from the JIRA Agent Dashboard
identified recurring patterns in PR feedback. This PR updates jira:solve
and address-reviews skills to address the most common issues.
Analysis Period
2025-01-01 to 2026-06-05 — 78 comments analyzed, 62 actionable
(after filtering nitpick/approval/unclassified)
Changes
Change 1: Anti-patterns for dead code and unrelated changes
File:
plugins/jira/commands/solve.mdPattern: style + process — 15 comments across multiple PRs
Evidence:
Reasoning: Added 4 explicit anti-patterns to the forbidden list:
unresolved TODO comments in validation, exported test-only functions,
dead code, and unrelated cosmetic changes.
Change 2: Pre-commit code quality checks
File:
plugins/jira/commands/solve.mdPattern: logic_bug + architecture_design — 28 comments total
Evidence:
Reasoning: Added a pre-commit checklist requiring agents to search
for existing patterns, verify test coverage of new exports, check for
dead code, and validate regex character classes before committing.
Change 3: Push verification in solve.md
File:
plugins/jira/commands/solve.mdPattern: process — 5 required_change comments about unpushed code
Evidence:
Reasoning: jira:solve claimed work was done but never pushed. Added
explicit push verification step matching what address-reviews already has.
Change 4: Push verification and restructure-commits in address-reviews.md
File:
plugins/utils/commands/address-reviews.mdPattern: process — 4 suggestion comments about commit organization
Evidence:
Reasoning: Reviewers had to ask for commit restructuring across 3
different PRs. Added explicit guidance to use the restructure-commits
skill when requested, and strengthened the push verification guideline.
Test Plan
🤖 Generated with Claude Code via
improve-ai-qualityskillSummary by CodeRabbit