-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add automatic priority escalation based on comment count #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c7e84ad
65dfa00
f326d62
fa1d938
c6f3f30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| package main | ||
|
|
||
| type Priority int | ||
|
|
||
| const ( | ||
| Undefined Priority = iota | ||
| Minor | ||
| Normal | ||
| Major | ||
| Blocker | ||
| Critical | ||
| ) | ||
|
|
||
| func (p Priority) String() string { | ||
| switch p { | ||
| case Critical: | ||
| return "Critical" | ||
| case Blocker: | ||
| return "Blocker" | ||
| case Major: | ||
| return "Major" | ||
| case Normal: | ||
| return "Normal" | ||
| case Minor: | ||
| return "Minor" | ||
| default: | ||
| return "Undefined" | ||
| } | ||
| } | ||
|
|
||
| func parsePriority(name string) (Priority, bool) { | ||
| switch name { | ||
| case "Critical": | ||
| return Critical, true | ||
| case "Blocker": | ||
| return Blocker, true | ||
| case "Major": | ||
| return Major, true | ||
| case "Normal": | ||
| return Normal, true | ||
| case "Minor": | ||
| return Minor, true | ||
| case "Undefined": | ||
| return Undefined, true | ||
| default: | ||
| return Undefined, false | ||
| } | ||
| } | ||
|
|
||
| var defaultPriorityThresholds = []int{2, 10, 50, 100, 200} | ||
|
|
||
| const defaultPriorityThresholdsStr = "2,10,50,100,200" | ||
|
|
||
| // calculatePriority maps comment count to JIRA priority using default thresholds | ||
| func calculatePriority(commentCount int) Priority { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unused apart from tests? |
||
| return calculatePriorityWithThresholds(commentCount, defaultPriorityThresholds) | ||
| } | ||
|
|
||
| // calculatePriorityWithThresholds maps comment count to JIRA priority using custom thresholds | ||
| // thresholds should contain exactly 5 non-negative, strictly ascending values for: Minor, Normal, Major, Blocker, Critical | ||
| func calculatePriorityWithThresholds(commentCount int, thresholds []int) Priority { | ||
| // Validate thresholds: must have exactly 5 values, all non-negative, and strictly ascending | ||
| if len(thresholds) != 5 { | ||
| thresholds = defaultPriorityThresholds | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should cause an error instead 🤔 |
||
| } else { | ||
| valid := true | ||
| for i := 0; i < 5; i++ { | ||
| if thresholds[i] < 0 { | ||
| valid = false | ||
| break | ||
| } | ||
| if i > 0 && thresholds[i] <= thresholds[i-1] { | ||
| valid = false | ||
| break | ||
| } | ||
| } | ||
| if !valid { | ||
| thresholds = defaultPriorityThresholds | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| switch { | ||
| case commentCount >= thresholds[4]: | ||
| return Critical | ||
| case commentCount >= thresholds[3]: | ||
| return Blocker | ||
| case commentCount >= thresholds[2]: | ||
| return Major | ||
| case commentCount >= thresholds[1]: | ||
| return Normal | ||
| case commentCount >= thresholds[0]: | ||
| return Minor | ||
| default: | ||
| return Undefined | ||
| } | ||
| } | ||
|
|
||
| // maxPriority returns the higher of two priorities | ||
| func maxPriority(a, b Priority) Priority { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in recent go we should be able to use just a generix |
||
| if a > b { | ||
| return a | ||
| } | ||
| return b | ||
| } | ||
|
|
||
| // calculatePriorityWithTimeEscalation applies layered escalation based on total comments | ||
| // and recent activity (last 30 days and last 10 days). | ||
| // This catches both persistent issues (high total) and hot issues (high recent activity). | ||
| func calculatePriorityWithTimeEscalation(totalComments, last30Days, last10Days int, thresholds []int) Priority { | ||
| // Step 1: Calculate base priority from total comments | ||
| basePriority := calculatePriorityWithThresholds(totalComments, thresholds) | ||
|
|
||
| // Step 2: Check last 10 days for HOT issues (immediate escalation) | ||
| if last10Days >= 10 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps the magic values here should be defined in a block with a comment reminding to keep README in sync... |
||
| return Critical // Top 11% - extremely hot | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like critical should be reserved for at most top 1%.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that will vary between the runs. Currently it's 11% but in next 10 days it could be 0. I think there is no easy way to autotune it to keep 1% for critical so I think thresholds are ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, but then perhaps we should not advertise the percentiles as the source of the count numbers.... |
||
| } | ||
| if last10Days >= 5 { | ||
| return maxPriority(basePriority, Major) // Top 17% - very active | ||
| } | ||
|
|
||
| // Step 3: Check last 30 days for active trends | ||
| if last30Days >= 50 { | ||
| return maxPriority(basePriority, Critical) // Top 7% - sustained high | ||
| } | ||
| if last30Days >= 20 { | ||
| return maxPriority(basePriority, Blocker) // Top 13% - very active | ||
| } | ||
| if last30Days >= 10 { | ||
| return maxPriority(basePriority, Major) // Top 17% - active | ||
| } | ||
| if last30Days >= 5 { | ||
| return maxPriority(basePriority, Normal) // Top 28% - noticeable | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OTOH, I'd put the "normal" bar closer to 50%... |
||
| } | ||
|
|
||
| // Step 4: Fall back to base priority | ||
| return basePriority | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please derive one from the other so there is no room for skew?
Please also add a comment describing the unit (comment count I guess?).