-
Notifications
You must be signed in to change notification settings - Fork 119
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
feat(analytics): filter out bot events in PR performance calculations #644
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyticsService as get_pr_performance
participant FilterLogic
Client->>AnalyticsService: Call get_pr_performance(pr_events)
AnalyticsService->>FilterLogic: Filter out events with username ending in "[bot]" or user type "Bot"
FilterLogic-->>AnalyticsService: Return pr_events_no_bots
AnalyticsService->>AnalyticsService: Sort pr_events_no_bots by created_at
AnalyticsService->>AnalyticsService: Determine first_review from sorted list
AnalyticsService-->>Client: Return updated performance metrics
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 7
🧹 Nitpick comments (1)
backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py (1)
69-116
: Consider documenting the design intention regarding bot events.While bot events are filtered out for determining the first review, they are still included in the counts for blocking and approving reviews. This appears to be intentional, but it would be helpful to add a comment explaining this design decision.
pr_events_no_bots.sort(key=lambda x: x.created_at) first_review = pr_events_no_bots[0] if pr_events_no_bots else None +# Bot events are filtered out for determining first review time, +# but still counted in blocking/approving review metrics approved_reviews = list( filter( lambda x: x.data["state"] == PullRequestEventState.APPROVED.value,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py
(1 hunks)backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py (4)
backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py (2)
CodeETLAnalyticsService
(15-186)get_pr_performance
(55-117)backend/analytics_server/mhq/utils/time.py (1)
time_now
(10-11)backend/analytics_server/tests/factories/models/code.py (1)
get_pull_request_event
(79-107)backend/analytics_server/mhq/store/models/code/enums.py (1)
PullRequestEventState
(24-27)
🪛 GitHub Actions: Unit Tests
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
[error] 211-211: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
[error] 231-231: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
[error] 253-253: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
[error] 274-274: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (5)
backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py (1)
56-68
: Implementation looks good for filtering bot events.The implementation effectively filters out bot events from PR performance calculations by:
- Excluding events where the actor's username ends with "[bot]"
- Excluding events where the user type is "Bot"
- Only using non-bot events for first review time calculation
This approach ensures more accurate metrics focused on human interactions while still counting bot blocking reviews appropriately.
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py (4)
204-221
: Test logic is correct and appropriately verifies the filtering functionality.This test correctly verifies that bot events are filtered out when calculating the first review time. Once the parameter issue is fixed, this test will ensure that the PR performance calculation only considers human events for first review time.
🧰 Tools
🪛 GitHub Actions: Unit Tests
[error] 211-211: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
224-245
: Test appropriately checks bot filtering based on user data.This test properly verifies that events are filtered out based on the user type being "Bot" in the event data. This covers the second condition in the bot filtering logic.
🧰 Tools
🪛 GitHub Actions: Unit Tests
[error] 231-231: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
247-266
: Test correctly handles edge case with only bot events.This test addresses an important edge case where only bot events are present, ensuring that the function returns -1 for first review time when no human reviews are available.
🧰 Tools
🪛 GitHub Actions: Unit Tests
[error] 253-253: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
268-285
: Test correctly verifies that bot blocking reviews are still counted.This test confirms that while bot events are filtered out for first review time, they are still included in the count of blocking reviews. This is an important distinction in the implementation.
🧰 Tools
🪛 GitHub Actions: Unit Tests
[error] 274-274: TypeError: get_pull_request_event() got an unexpected keyword argument 'actor_username'
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py (4)
backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py (2)
CodeETLAnalyticsService
(15-186)get_pr_performance
(55-117)backend/analytics_server/mhq/utils/time.py (1)
time_now
(10-11)backend/analytics_server/tests/factories/models/code.py (1)
get_pull_request_event
(79-107)backend/analytics_server/mhq/store/models/code/enums.py (1)
PullRequestEventState
(24-27)
🪛 GitHub Actions: Unit Tests
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
[error] 242-242: KeyError: 'state' in test_pr_performance_filters_out_bot_events_with_user_data
[error] 263-263: KeyError: 'state' in test_pr_performance_with_only_bot_events_returns_no_first_review
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (2)
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py (2)
204-222
: Looks good! Test verifies bot events are filtered out correctly.The test properly verifies that events from users with names ending in "[bot]" are excluded from PR performance calculations, ensuring that only human events are considered.
268-285
: Looks good! Test verifies bot blocking reviews are counted correctly.This test correctly verifies that bot events requesting changes are counted as blocking reviews. It ensures that while bot reviews are excluded from first review time calculations, they are still considered when counting blocking reviews.
This pull request enhances the
get_pr_performance
method by filtering out bot events from the pull request events and adds corresponding tests to ensure the functionality works as expected. The most important changes include modifying theget_pr_performance
method and adding several test cases to verify the new behavior.Enhancements to
get_pr_performance
method:backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py
: Modified theget_pr_performance
method to filter out bot events based on the actor's username and user data before determining the first review event.Addition of new test cases:
backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
: Addedtest_pr_performance_filters_out_bot_events
to ensure bot events are filtered out correctly.backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
: Addedtest_pr_performance_filters_out_bot_events_with_user_data
to verify bot events with user data are filtered out.backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
: Addedtest_pr_performance_with_only_bot_events_returns_no_first_review
to check that no first review is returned if only bot events are present.backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py
: Addedtest_pr_performance_counts_bot_blocking_reviews
to ensure bot events requesting changes are counted as blocking reviews.Summary by CodeRabbit
Bug Fixes
Tests