Skip to content

Conversation

rohitvinnakota-codecov
Copy link
Member

@rohitvinnakota-codecov rohitvinnakota-codecov commented Jul 15, 2025

Is consumed by seer_rpc while communicating with Seer. This PR must be merged AFTER #95456 is deployed.

Screenshot 2025-07-14 at 1 06 24 PM

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 15, 2025
@rohitvinnakota-codecov rohitvinnakota-codecov marked this pull request as ready for review July 15, 2025 15:40
@rohitvinnakota-codecov rohitvinnakota-codecov requested a review from a team as a code owner July 15, 2025 15:40
Copy link
Member

@roaga roaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a confusing UX if the user has this toggle enabled, but "show generative AI features" disabled. Perhaps hide this option if gen AI features are disabled? That's how we handle Seer settings

Also, padding 👁️ 👁️ :
Image

Comment on lines 53 to 57
help: tct(
'Use AI to generate feedback and tests in pull requests [link:Learn more]',
{
link: <ExternalLink href="https://github.com/apps/seer-by-sentry/" />,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would a link to docs be more appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #95547       +/-   ##
===========================================
- Coverage   87.73%   77.28%   -10.45%     
===========================================
  Files       10548    10546        -2     
  Lines      607769   607667      -102     
  Branches    23817    23817               
===========================================
- Hits       533215   469664    -63551     
- Misses      74258   137707    +63449     
  Partials      296      296               

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: AI Feature Toggle Logic Inverted

The visible callback for the new enablePrReviewTestGeneration field returns hideAiFeatures directly. This inverts the intended logic: the field is displayed when AI features are hidden and hidden when they are enabled. Consequently, users cannot enable the "Enable PR Review and Test Generation" toggle when AI features are active. The callback should return !hideAiFeatures.

static/app/data/forms/organizationGeneralSettings.tsx#L47-L66

{
name: 'enablePrReviewTestGeneration',
type: 'boolean',
label: tct('Enable PR Review and Test Generation [badge]', {
badge: <FeatureBadge type="beta" style={{marginBottom: '2px'}} />,
}),
help: tct(
'Use AI to generate feedback and tests in pull requests [link:Learn more]',
{
link: (
<ExternalLink href="https://docs.sentry.io/product/ai-in-sentry/sentry-prevent-ai/" />
),
}
),
visible: ({model}) => {
// Show field when AI features are enabled (hideAiFeatures is false)
const hideAiFeatures = model.getValue('hideAiFeatures');
return hideAiFeatures;
},
},

Fix in CursorFix in Web


Was this report helpful? Give feedback by reacting with 👍 or 👎

Comment on lines +61 to +64
visible: ({model}) => {
// Show field when AI features are enabled (hideAiFeatures is false)
const hideAiFeatures = model.getValue('hideAiFeatures');
return hideAiFeatures;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is backwards?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very confusing. The logic was inverted at some point, you can test on the UI

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to change the variable name, but it touches a lot of other areas as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack ok

@rohitvinnakota-codecov rohitvinnakota-codecov merged commit 870dc45 into master Jul 16, 2025
46 checks passed
@rohitvinnakota-codecov rohitvinnakota-codecov deleted the rvinnakota/test-gen-pr-review-toggle branch July 16, 2025 12:26
cursor bot pushed a commit that referenced this pull request Jul 18, 2025
<!-- Describe your PR here. -->

Is consumed by `seer_rpc` while communicating with Seer. This PR must be
merged AFTER #95456 is deployed.

<img width="1011" height="558" alt="Screenshot 2025-07-14 at 1 06 24 PM"
src="https://github.com/user-attachments/assets/0822e352-0a04-473d-b477-57ef3e3a8e3f"
/>


<!--

  Sentry employees and contractors can delete or ignore the following.

-->

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.
andrewshie-sentry pushed a commit that referenced this pull request Jul 21, 2025
<!-- Describe your PR here. -->

Is consumed by `seer_rpc` while communicating with Seer. This PR must be
merged AFTER #95456 is deployed.

<img width="1011" height="558" alt="Screenshot 2025-07-14 at 1 06 24 PM"
src="https://github.com/user-attachments/assets/0822e352-0a04-473d-b477-57ef3e3a8e3f"
/>


<!--

  Sentry employees and contractors can delete or ignore the following.

-->

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants