- Notifications
You must be signed in to change notification settings - Fork87
fix: label metrics overcounting for closed issues when labels removed before closure#571
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Co-authored-by: zkoppert <6935431+zkoppert@users.noreply.github.com>
@copilot can you generate 3 more test cases like the ones you've added that test when labels are applied at creation and also test different time spans? |
…ls and varied timeframesCo-authored-by: zkoppert <6935431+zkoppert@users.noreply.github.com>
I've added 3 additional test cases as requested:
These tests cover the scenarios you requested: labels applied at creation time and different time spans (7, 21, 30, and 60 days vs the previous 5, 8, 10 day spans). All tests pass. Changes committed infc9d45a. |
@copilot can you fix the failing github actions. I think you need to run the black linter. |
Co-authored-by: zkoppert <6935431+zkoppert@users.noreply.github.com>
Fixed the failing GitHub Actions by running black linter on the code. The issue was a long function name that needed to be split across multiple lines according to black's formatting standards. All tests continue to pass (10/10). Commitbb65a49. |
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.
Pull Request Overview
This PR fixes a bug in theget_label_metrics function where time spent in labels was being overcounted for closed issues when those labels were removed before the issue was closed. The fix ensures that only labels still active at closure contribute to the final timespan calculation.
- Added guard condition to check if labels were still applied when issues closed
- Fixed typo in comment from 'unlabled' to 'unlabeled'
- Added comprehensive test coverage for various label application/removal scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| labels.py | Added guard condition to prevent overcounting metrics for removed labels and fixed spelling typo |
| test_labels.py | Added five new test cases covering different label timing scenarios for closed issues |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card.Take the survey.
| # if the issue is closed, add the time from the issue creation to the closed_at time | ||
| ifissue.state=="closed": | ||
| # Only add the final (closed_at - created_at) span if the label was still applied at closure. | ||
| iflabel_last_event_type.get(label)!="labeled": |
CopilotAIAug 16, 2025
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.
The condition logic is inverted. This should check if the last event was 'unlabeled' to skip the label, not if it wasn't 'labeled'. The condition should beif label_last_event_type.get(label) == "unlabeled": to match the logic in the open issue branch below.
| iflabel_last_event_type.get(label)!="labeled": | |
| iflabel_last_event_type.get(label)=="unlabeled": |
mlabeeb03 left a comment
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.
Fixes my issue, thanks.
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.
🚀
9f2ad7e intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Problem
The
get_label_metricsfunction was overcounting time spent in labels for closed issues when those labels were removed before the issue was closed.Example scenario:
Expected result: 5 days (time label was actually applied)
Actual result: 20 days (incorrect - adds full issue duration)
Root Cause
The closed issue branch in
get_label_metricsadds the full(closed_at - created_at)duration for any label that was ever applied, without checking if the label was still active when the issue closed:The open issue branch already had the correct guard logic to skip labels whose last event was "unlabeled", but this guard was missing from the closed issue branch.
Solution
Added the same guard condition that exists in the open issue branch to the closed issue branch:
This ensures we only add the closure timespan for labels that were still active when the issue was closed.
Changes
label_last_event_typein closed issue branchVerification
Backward Compatibility
No interface changes - this only reduces overcounted values to their correct amounts.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn moreCopilot coding agent tips in the docs.