feat: trigger CodeRabbit plan after issue assignment - #2577
Conversation
Signed-off-by: ParasSalonia <parassalonia22@gmail.com>
43be2dc to
b00731f
Compare
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours will begin in approximately 3 hours and 49 minutes (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
WalkthroughThe issue assignment flow now integrates CodeRabbit plan detection and triggering after successful assignments. Jest coverage validates duplicate suppression, trigger failures, assignment failures, and updated test discovery. ChangesCodeRabbit assignment integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Issue assignment can create duplicate CodeRabbit plans when an existing plan cannot be verified, or skip plan creation based on untrusted comment text. The PR is otherwise mergeable with explicit owner awareness and follow-up on verified plan-state handling. Sequence Diagram(s)sequenceDiagram
participant issue_assign_js
participant hasExistingCodeRabbitPlan
participant triggerCodeRabbitPlan
issue_assign_js->>hasExistingCodeRabbitPlan: Check assigned issue for an existing plan
hasExistingCodeRabbitPlan-->>issue_assign_js: Return plan existence
issue_assign_js->>triggerCodeRabbitPlan: Trigger plan when none exists
triggerCodeRabbitPlan-->>issue_assign_js: Return success or error
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The PR implements post-assignment triggering, duplicate-plan suppression, failure isolation, and related tests. The provided changes do not show removal of obsolete automatic mentor assignment or its related code, configuration, workflow steps, imports, and tests, which is an explicit requirement of issue
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9397f04a-3941-4d60-8bb0-1087eaacfdc5
📒 Files selected for processing (3)
.github/scripts/__tests__/jest/issue-assign.test.js.github/scripts/jest.config.js.github/scripts/shared/core/issue-assign.js
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| const planExists = await hasExistingCodeRabbitPlan( | ||
| github, | ||
| owner, | ||
| repoName, | ||
| issueNumber | ||
| ); | ||
|
|
||
| if (!planExists) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a verified plan state before triggering.
hasExistingCodeRabbitPlan returns false when comment lookup fails. This flow then creates a duplicate plan when an existing plan cannot be checked. The helper also accepts untrusted comment text containing @coderabbitai plan as an existing plan, so a contributor can suppress the trigger without a verified plan marker.
Return distinct existing, missing, and unknown states. Trigger only for confirmed missing state. Accept only the controlled marker, or another trusted CodeRabbit-produced signal, as existing.
As per path instructions, “Do not trust free-form text from issues, PRs, or comments” and “Use marker-based patterns to prevent duplicate actions.”
Source: Path instructions
MonaaEid
left a comment
There was a problem hiding this comment.
Hi @ParasSalonia thanks for this PR! Have you tested it in your fork? If not, I recommend testing it by merging these changes into your fork and giving it a try.. I’m happy to help if needed
parvninama
left a comment
There was a problem hiding this comment.
LGTM overall! Just a couple of structural changes I think should be addressed before moving forward:
| const { | ||
| triggerCodeRabbitPlan, | ||
| hasExistingCodeRabbitPlan, | ||
| } = require('../../coderabbit_plan_trigger.js'); | ||
|
|
There was a problem hiding this comment.
Could we move the CodeRabbit functionality into the shared helpers layer as part of this migration?
runAssignmentFlow is currently importing/mocking triggerCodeRabbitPlan and hasExistingCodeRabbitPlan from the legacy coderabbit_plan_trigger.js location. Since this issue is migrating the existing CodeRabbit behaviour into the new assignment architecture, it would be cleaner to have the reusable CodeRabbit operations under .github/scripts/shared/ (consistent with the other shared helpers) and have the assignment flow consume that implementation.
There was a problem hiding this comment.
If coderabbit_plan_trigger.js and the old CodeRabbit workflow/files are no longer referenced after the migration, could we remove those obsolete files/workflows as well?
| module.exports = { | ||
| testEnvironment: 'node', | ||
| testMatch: ['<rootDir>/__tests__/jest/**/*.test.js'], | ||
| testMatch: ['**/__tests__/jest/**/*.test.js'], |
There was a problem hiding this comment.
Out of scope? Could we remove this from the PR unless there’s a specific test-discovery issue that this change is intended to fix?
|
Linked Issue
Changes
Testing
git diff --checkpassedTesting done on Fork:
CodeRabbit plan trigger after successful assignment
◦ Issue: Test: CodeRabbit plan trigger
◦ Log: https://github.com/ParasSalonia/hiero-sdk-python/actions/runs/33009564342/job/98312006540
Already assigned : CodeRabbit plan is not triggered when the assignment is rejected.
◦ Issue: testing more than one issue assign #10
◦ Log: https://github.com/ParasSalonia/hiero-sdk-python/actions/runs/33159976369/job/98811797995
Summary
Integrates CodeRabbit plan generation into the issue assignment flow while avoiding duplicate plans and keeping the existing assignment behavior unchanged.