ref(seer): Bill seat-based autofix from Seer project preferences#113369
Draft
ref(seer): Bill seat-based autofix from Seer project preferences#113369
Conversation
Determine whether autofix is enabled for a repository by checking Seer project preferences instead of code mappings plus the `sentry:autofix_automation_tuning` project option. Behind `organizations:seer-project-settings-read-from-sentry`, read directly from `SeerProjectRepository`; otherwise bulk-fetch preferences from Seer and match the repository id, falling back to not incrementing on `SeerApiError`. Refs AIML-2754
github-actions
bot
added
the
Scope: Backend
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Contributor
Backend Test FailuresFailures on
|
Member
Author
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+79
to
+82
| except (JSONDecodeError, ValidationError, Exception): | ||
| sentry_sdk.capture_exception() | ||
| return False | ||
|
|
Contributor
There was a problem hiding this comment.
Bug: The broad exception handling captures ValidationError and sends it to Sentry, creating noise. A parallel function logs this error instead, which is the preferred pattern for transient API issues.
Severity: LOW
Suggested Fix
Align the error handling with the pattern in bulk_read_preferences. Specifically, catch ValidationError and JSONDecodeError separately from the generic Exception. Use logger.exception() for these expected API-related errors to log them without creating Sentry issues, and reserve sentry_sdk.capture_exception() for truly unexpected exceptions.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/seer/code_review/contributor_seats.py#L79-L82
Potential issue: The exception handling for `JSONDecodeError` and `ValidationError` in
`_is_autofix_enabled_for_repo` uses `sentry_sdk.capture_exception()`. This is
inconsistent with a parallel function, `bulk_read_preferences`, which handles the same
`ValidationError` by logging it via `logger.exception()`. This inconsistency will cause
transient API issues, such as malformed JSON responses from the Seer API, to be reported
as critical errors in Sentry, creating unnecessary noise. The function correctly falls
back by returning `False`, but the error reporting is overly aggressive for expected API
degradation.
Did we get this right? 👍 / 👎 to inform future reviews.
Comment on lines
+83
to
86
| return any( | ||
| any(repo.repository_id == repository_id for repo in pref.repositories) | ||
| for pref in resolved_preferences | ||
| ) |
Contributor
There was a problem hiding this comment.
Bug: When the feature flag is off, unresolvable repositories from Seer have repository_id=None, causing the check to silently fail and incorrectly report autofix as disabled, impacting billing.
Severity: MEDIUM
Suggested Fix
In the fallback path, after calling resolve_repository_ids, filter out any repositories from resolved_preferences where repo.repository_id is None. Additionally, consider logging a warning when an unresolvable repository is encountered during this check to make the silent failure visible to operators, similar to the logging in _write_preferences_to_sentry_db.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/seer/code_review/contributor_seats.py#L83-L86
Potential issue: In the fallback path (when the
`organizations:seer-project-settings-read-from-sentry` feature flag is off), the
function `_is_autofix_enabled_for_repo` can silently fail. If a repository is configured
in Seer but cannot be resolved to a Sentry `Repository` record (due to data
inconsistency or a missing integration), its `repository_id` will remain `None` after
`resolve_repository_ids()` is called. The subsequent check `repo.repository_id ==
repository_id` will evaluate to `False`, causing the function to incorrectly report that
autofix is disabled. This leads to a failure to bill for contributor seats.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes AIML-2754
Determine whether autofix is enabled for a repository (and whether this contributor is billable) by checking Seer project preferences instead of code mappings plus the
sentry:autofix_automation_tuningproject option. Autofix is enabled if any project has the given repository added to its Seer preferences.