diff --git a/apps/web/src/components/ChatView.logic.test.ts b/apps/web/src/components/ChatView.logic.test.ts index 8a7e17adee12..799bd0bde7f4 100644 --- a/apps/web/src/components/ChatView.logic.test.ts +++ b/apps/web/src/components/ChatView.logic.test.ts @@ -387,29 +387,35 @@ describe("proactive panels", () => { ).toBe(false); }); - it("opens a completed turn diff only for changed files", () => { - const changedCheckpoint = { - status: "ready", - files: [{ path: "src/app.ts", kind: "modified", additions: 1, deletions: 0 }], - } satisfies Pick; - const unchangedCheckpoint = { - status: "ready", - files: [], - } satisfies Pick; + it.each([ + { files: 0, additions: 0, deletions: 0, action: "ignore" }, + { files: 1, additions: 1, deletions: 0, action: "ignore" }, + { files: 2, additions: 12, deletions: 12, action: "ignore" }, + { files: 1, additions: 25, deletions: 24, action: "ignore" }, + { files: 1, additions: 25, deletions: 25, action: "open" }, + { files: 1, additions: 0, deletions: 50, action: "open" }, + { files: 3, additions: 1, deletions: 0, action: "open" }, + ])( + "uses change size for automatic diffs: $files files, +$additions/-$deletions", + ({ files, additions, deletions, action }) => { + const changedCheckpoint = { + status: "ready", + files: Array.from({ length: files }, (_, index) => ({ + path: `src/app-${index}.ts`, + kind: "modified" as const, + additions, + deletions, + })), + } satisfies Pick; - expect( - resolveProactiveTurnDiffAction({ - checkpoint: changedCheckpoint, - isGitRepo: true, - }), - ).toBe("open"); - expect( - resolveProactiveTurnDiffAction({ - checkpoint: unchangedCheckpoint, - isGitRepo: true, - }), - ).toBe("ignore"); - }); + expect( + resolveProactiveTurnDiffAction({ + checkpoint: changedCheckpoint, + isGitRepo: true, + }), + ).toBe(action); + }, + ); it("waits for definitive checkpoint and repository state", () => { const missingCheckpoint = { diff --git a/apps/web/src/components/ChatView.logic.ts b/apps/web/src/components/ChatView.logic.ts index f5d9e7ad2578..eae48ac5fa29 100644 --- a/apps/web/src/components/ChatView.logic.ts +++ b/apps/web/src/components/ChatView.logic.ts @@ -189,7 +189,11 @@ export function resolveProactiveTurnDiffAction(input: { ) { return "ignore"; } - return "open"; + const changedLines = input.checkpoint.files.reduce( + (total, file) => total + file.additions + file.deletions, + 0, + ); + return input.checkpoint.files.length >= 3 || changedLines >= 50 ? "open" : "ignore"; } export function codexArtifactTemplatePromptToAppend( diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 6b372437f2ae..79b2ba0a7816 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -4514,9 +4514,10 @@ export default function ChatView(props: ChatViewProps) { }, [activeThreadRef]); const supportsThreadPullRequests = serverConfig?.environment.capabilities.threadPullRequests === true; - const visiblePullRequestCount = visibleThreadPullRequests( + const visiblePullRequests = visibleThreadPullRequests( (activeThreadShell ?? activeThread)?.pullRequests ?? [], - ).length; + ); + const visiblePullRequestCount = visiblePullRequests.length; const pullRequestsSurfaceAvailable = isServerThread && supportsThreadPullRequests && visiblePullRequestCount > 0; const addPullRequestsSurface = useCallback(() => { @@ -4616,6 +4617,7 @@ export default function ChatView(props: ChatViewProps) { ); // The shell carries server PR updates even while thread detail is still loading. const activeThreadMetadata = activeThreadShell ?? activeThread; + const hasLinkedPullRequestDetail = activeThreadMetadata?.linkedPullRequest != null; const linkedThreadPullRequest = activeThreadMetadata?.linkedPullRequest ?? activeThreadMetadata?.branchPullRequest ?? null; const activeProjectRepository = activeProject?.repositoryIdentity?.displayName ?? null; @@ -4626,6 +4628,11 @@ export default function ChatView(props: ChatViewProps) { linkedThreadPullRequest.number, ]) : null; + const proactivePullRequestsKey = pullRequestsSurfaceAvailable + ? JSON.stringify( + visiblePullRequests.map((link) => [link.host, link.repository, link.number]).sort(), + ) + : linkedThreadPullRequestKey; const observedThreadPullRequestRef = useRef<{ readonly threadKey: string; readonly reference: ThreadLinkedPullRequest | null; @@ -4692,7 +4699,40 @@ export default function ChatView(props: ChatViewProps) { userActionRevision, ); } - if (!clientSettingsHydrated || threadDetailLoading) return; + if (!clientSettingsHydrated) return; + + const proactivePanelsEnabled = settings.proactivePanelsEnabled && !shouldUseRightPanelSheet; + const eligibleLink = + proactivePanelsEnabled && + shouldOpenProactivePullRequest(previousTargetKey, proactivePullRequestsKey); + const shouldDeferLink = eligibleLink && !pullRequestsCapabilityKnown; + proactivePanelObservationRef.current = { + ...observation, + targetKey: shouldDeferLink ? (previousTargetKey ?? null) : proactivePullRequestsKey, + }; + if (eligibleLink && pullRequestsCapabilityKnown) { + if ( + pullRequestsSurfaceAvailable && + (visiblePullRequestCount > 1 || !hasLinkedPullRequestDetail || !supportsPullRequests) + ) { + panels.openProactive( + activeThreadRef, + { id: "pull-requests", kind: "pull-requests" }, + userActionRevision, + ); + } else if ( + !followSelectedPullRequest && + supportsPullRequests && + linkedThreadPullRequest !== null + ) { + panels.openProactive( + activeThreadRef, + pullRequestSurface(linkedThreadPullRequest), + userActionRevision, + ); + } + } + if (threadDetailLoading) return; const settledTurnId = latestTurnSettled ? (activeLatestTurn?.turnId ?? null) : null; const newlyCompletedTurnId = shouldOpenProactiveTurnDiff({ @@ -4703,8 +4743,13 @@ export default function ChatView(props: ChatViewProps) { }) ? settledTurnId : null; - const proactivePanelsEnabled = settings.proactivePanelsEnabled && !shouldUseRightPanelSheet; - const eligibleCompletion = proactivePanelsEnabled && newlyCompletedTurnId !== null; + const eligibleCompletion = + proactivePanelsEnabled && + newlyCompletedTurnId !== null && + !( + proactivePullRequestsKey !== null && + (!pullRequestsCapabilityKnown || supportsPullRequests || pullRequestsSurfaceAvailable) + ); const completedCheckpoint = eligibleCompletion ? activeThread?.checkpoints.find((checkpoint) => checkpoint.turnId === newlyCompletedTurnId) : undefined; @@ -4714,30 +4759,12 @@ export default function ChatView(props: ChatViewProps) { isGitRepo: gitStatusQuery.data?.isRepo, }) : "ignore"; - const eligibleLink = - proactivePanelsEnabled && - shouldOpenProactivePullRequest(previousTargetKey, linkedThreadPullRequestKey); - const shouldDeferLink = eligibleLink && !pullRequestsCapabilityKnown; proactivePanelObservationRef.current = { - ...observation, - // Preserve first-entry eligibility while the checkpoint or repository is loading. - runningTurnId: diffAction === "defer" ? previousRunningTurnId : activeRunningTurnId, - targetKey: shouldDeferLink ? (previousTargetKey ?? null) : linkedThreadPullRequestKey, + ...proactivePanelObservationRef.current, + // Preserve first-entry eligibility while capabilities, checkpoint or repository load. + runningTurnId: + diffAction === "defer" || shouldDeferLink ? previousRunningTurnId : activeRunningTurnId, }; - - if ( - !followSelectedPullRequest && - eligibleLink && - pullRequestsCapabilityKnown && - supportsPullRequests && - linkedThreadPullRequest !== null - ) { - panels.openProactive( - activeThreadRef, - pullRequestSurface(linkedThreadPullRequest), - userActionRevision, - ); - } if (diffAction !== "open" || newlyCompletedTurnId === null) return; if (!panels.openProactive(activeThreadRef, { id: "diff", kind: "diff" }, userActionRevision)) { return; @@ -4756,9 +4783,12 @@ export default function ChatView(props: ChatViewProps) { isServerThread, latestTurnSettled, linkedThreadPullRequest, - linkedThreadPullRequestKey, + proactivePullRequestsKey, + hasLinkedPullRequestDetail, onDiffPanelOpen, pullRequestsCapabilityKnown, + pullRequestsSurfaceAvailable, + visiblePullRequestCount, settings.proactivePanelsEnabled, shouldUseRightPanelSheet, supportsPullRequests, diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index d2306e02a85f..430852757f46 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -2528,7 +2528,7 @@ export function GeneralSettingsPanel() { { number: 42, }); - it.each(["diff-first", "pull-request-first"])( - "prioritizes the linked pull request over browser and diff with %s delivery", - (order) => { + it.each([ + { order: "diff-first", surface: linkedPullRequest }, + { order: "pull-request-first", surface: linkedPullRequest }, + { order: "diff-first", surface: { id: "pull-requests", kind: "pull-requests" } as const }, + { + order: "pull-request-first", + surface: { id: "pull-requests", kind: "pull-requests" } as const, + }, + ])( + "prioritizes $surface.kind over browser and diff with $order delivery", + ({ order, surface }) => { const store = useRightPanelStore.getState(); store.openBrowser(refA, "existing-browser"); const revision = store.getUserActionRevision(refA); - const requests = - order === "diff-first" - ? [completedDiff, linkedPullRequest] - : [linkedPullRequest, completedDiff]; + const requests = order === "diff-first" ? [completedDiff, surface] : [surface, completedDiff]; for (const surface of requests) store.openProactive(refA, surface, revision); store.reconcileBrowserSurfaces(refA, ["existing-browser", "agent-browser"]); expect( selectActiveRightPanelSurface(useRightPanelStore.getState().byThreadKey, refA), - ).toEqual(linkedPullRequest); + ).toEqual(surface); store.open(refA, "diff"); expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff"); @@ -167,6 +172,9 @@ describe("rightPanelStore", () => { expect(store.openProactive(refA, completedDiff, revision)).toBe(false); expect(store.openProactive(refA, linkedPullRequest, revision)).toBe(false); + expect( + store.openProactive(refA, { id: "pull-requests", kind: "pull-requests" }, revision), + ).toBe(false); expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toBe( chosen, ); diff --git a/apps/web/src/rightPanelStore.ts b/apps/web/src/rightPanelStore.ts index c44c106c68c8..69b803bcd9cc 100644 --- a/apps/web/src/rightPanelStore.ts +++ b/apps/web/src/rightPanelStore.ts @@ -124,7 +124,7 @@ interface RightPanelStoreState { */ openProactive: ( ref: ScopedThreadRef, - surface: Extract, + surface: Extract, expectedUserActionRevision: number, ) => boolean; open: ( @@ -496,7 +496,8 @@ export const useRightPanelStore = create()( // always apply, and later user choices reject both proactive requests. if ( surface.kind === "diff" && - selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" + (selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" || + selectActiveRightPanel(state.byThreadKey, ref) === "pull-requests") ) { return state; }