From 93c5a75b27c073f59ddf2b5feec0d69b143e4039 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:49:47 -0700 Subject: [PATCH] fix(checks): preserve refresh precedence and budget --- .../src/components/right-sidebar/ChecksPanel.tsx | 3 ++- .../right-sidebar/checks-panel-empty-state.test.ts | 11 +++++++++++ .../right-sidebar/checks-panel-empty-state.ts | 6 ++++-- .../checks-panel-pr-refresh-request.test.ts | 11 +++++++++++ .../right-sidebar/checks-panel-pr-refresh-request.ts | 4 +++- 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 587855007e8..0139d1c3ac0 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -957,7 +957,8 @@ export default function ChecksPanel(): React.JSX.Element { const checksPanelReviewLookup = checksPanelReviewLookupResult.state const hasUnrenderedReviewEvidence = checksPanelReviewLookup === 'positive_unresolved' || - hostedReviewCreation?.blockedReason === 'existing_review' + (checksPanelReviewLookup !== 'found' && + hostedReviewCreation?.blockedReason === 'existing_review') const unrenderedReviewEvidenceIdentity = linkedReviewNumber ?? hostedReview?.number ?? diff --git a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts index c6770272065..e7c1daf42d5 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts @@ -167,6 +167,17 @@ describe('getChecksPanelReviewState — precedence', () => { expect(state.title).toBe('No upstream configured') }) + it('a safety blocker still owns the copy over positive evidence and an in-flight fetch', () => { + const state = getChecksPanelReviewState( + input({ + eligibilityBlockedReason: 'dirty', + reviewLookup: 'positive_unresolved', + refresh: { status: 'in-flight' } + }) + ) + expect(state.title).toBe('Commit changes first') + }) + it('hard refresh error hides the composer', () => { const state = getChecksPanelReviewState( input({ refresh: { status: 'error', errorType: 'auth' } }) diff --git a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts index c6fc27b0828..0191b27735e 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts @@ -74,12 +74,14 @@ export function getChecksPanelReviewState( const blockedReason = input.eligibilityBlockedReason - // 1.5 Keep active positive-review lookups self-updating; concurrent branch blockers still own their guidance. + // 1.5 Keep active positive-review lookups self-updating without hiding stronger safety guidance. const reviewFetchInFlight = input.refresh?.status === 'queued' || input.refresh?.status === 'in-flight' const hasPendingReviewEvidence = input.reviewLookup === 'positive_unresolved' || blockedReason === 'existing_review' - if (reviewFetchInFlight && hasPendingReviewEvidence && !isBranchBlocker(blockedReason)) { + const pendingReviewLookupOwnsState = + blockedReason === undefined || blockedReason === 'existing_review' + if (reviewFetchInFlight && hasPendingReviewEvidence && pendingReviewLookupOwnsState) { return { renderReview: false, title: translate( diff --git a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts index 2b2f024602e..1dfc1c250bd 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts @@ -45,6 +45,17 @@ describe('resolveChecksPanelPRRefreshRequest', () => { ).toEqual({ reason: 'swr', priority: 30 }) }) + it('does not force provider work when review details are already cached', () => { + expect( + resolveChecksPanelPRRefreshRequest({ + cachedHasPR: true, + cachedFetchedAt: 100, + panelVisibleSince: 200, + hasUnrenderedReviewEvidence: true + }) + ).toEqual({ reason: 'swr', priority: 30 }) + }) + it('keeps populated or unknown cache entries on the background path', () => { expect( resolveChecksPanelPRRefreshRequest({ diff --git a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts index 26157a6d846..d010c0ac67d 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts @@ -22,10 +22,12 @@ export function resolveChecksPanelPRRefreshRequest( input.cachedFetchedAt !== null && input.panelVisibleSince !== null && input.cachedFetchedAt < input.panelVisibleSince + const unresolvedEvidenceNeedsForeground = + input.hasUnrenderedReviewEvidence && input.cachedHasPR !== true if ( !input.hasRequestedForegroundRefresh && - (cachedMissPredatesVisiblePanel || input.hasUnrenderedReviewEvidence) + (cachedMissPredatesVisiblePanel || unresolvedEvidenceNeedsForeground) ) { // A stale miss or new positive evidence needs one foreground lookup to recover. return { reason: 'active', priority: 80 }