| author | |
| committer | |
| log | 52ad70ec43b5dcfcdc72ac79c0964da8e4d163db |
| tree | b0eeb56948c1b47b4451d2db551828b50764b050 |
| parent | aae361c4ef6478e4dcf970d574f3fd38d908e6b9 |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 111 insertions(+), 62 deletions(-)
apps/web/src/components/ChatView.logic.test.ts+28-22| ... | @@ -387,29 +387,35 @@ describe("proactive panels", () => { | ... | @@ -387,29 +387,35 @@ describe("proactive panels", () => { |
| 387 | ).toBe(false); | 387 | ).toBe(false); |
| 388 | }); | 388 | }); |
| 389 | 389 | ||
| 390 | it("opens a completed turn diff only for changed files", () => { | 390 | it.each([ |
| 391 | const changedCheckpoint = { | 391 | { files: 0, additions: 0, deletions: 0, action: "ignore" }, |
| 392 | status: "ready", | 392 | { files: 1, additions: 1, deletions: 0, action: "ignore" }, |
| 393 | files: [{ path: "src/app.ts", kind: "modified", additions: 1, deletions: 0 }], | 393 | { files: 2, additions: 12, deletions: 12, action: "ignore" }, |
| 394 | } satisfies Pick<TurnDiffSummary, "status" | "files">; | 394 | { files: 1, additions: 25, deletions: 24, action: "ignore" }, |
| 395 | const unchangedCheckpoint = { | 395 | { files: 1, additions: 25, deletions: 25, action: "open" }, |
| 396 | status: "ready", | 396 | { files: 1, additions: 0, deletions: 50, action: "open" }, |
| 397 | files: [], | 397 | { files: 3, additions: 1, deletions: 0, action: "open" }, |
| 398 | } satisfies Pick<TurnDiffSummary, "status" | "files">; | 398 | ])( |
| 399 | "uses change size for automatic diffs: $files files, +$additions/-$deletions", | ||
| 400 | ({ files, additions, deletions, action }) => { | ||
| 401 | const changedCheckpoint = { | ||
| 402 | status: "ready", | ||
| 403 | files: Array.from({ length: files }, (_, index) => ({ | ||
| 404 | path: `src/app-${index}.ts`, | ||
| 405 | kind: "modified" as const, | ||
| 406 | additions, | ||
| 407 | deletions, | ||
| 408 | })), | ||
| 409 | } satisfies Pick<TurnDiffSummary, "status" | "files">; | ||
| 399 | 410 | ||
| 400 | expect( | 411 | expect( |
| 401 | resolveProactiveTurnDiffAction({ | 412 | resolveProactiveTurnDiffAction({ |
| 402 | checkpoint: changedCheckpoint, | 413 | checkpoint: changedCheckpoint, |
| 403 | isGitRepo: true, | 414 | isGitRepo: true, |
| 404 | }), | 415 | }), |
| 405 | ).toBe("open"); | 416 | ).toBe(action); |
| 406 | expect( | 417 | }, |
| 407 | resolveProactiveTurnDiffAction({ | 418 | ); |
| 408 | checkpoint: unchangedCheckpoint, | ||
| 409 | isGitRepo: true, | ||
| 410 | }), | ||
| 411 | ).toBe("ignore"); | ||
| 412 | }); | ||
| 413 | 419 | ||
| 414 | it("waits for definitive checkpoint and repository state", () => { | 420 | it("waits for definitive checkpoint and repository state", () => { |
| 415 | const missingCheckpoint = { | 421 | const missingCheckpoint = { |
apps/web/src/components/ChatView.logic.ts+5-1| ... | @@ -189,7 +189,11 @@ export function resolveProactiveTurnDiffAction(input: { | ... | @@ -189,7 +189,11 @@ export function resolveProactiveTurnDiffAction(input: { |
| 189 | ) { | 189 | ) { |
| 190 | return "ignore"; | 190 | return "ignore"; |
| 191 | } | 191 | } |
| 192 | return "open"; | 192 | const changedLines = input.checkpoint.files.reduce( |
| 193 | (total, file) => total + file.additions + file.deletions, | ||
| 194 | 0, | ||
| 195 | ); | ||
| 196 | return input.checkpoint.files.length >= 3 || changedLines >= 50 ? "open" : "ignore"; | ||
| 193 | } | 197 | } |
| 194 | 198 | ||
| 195 | export function codexArtifactTemplatePromptToAppend( | 199 | export function codexArtifactTemplatePromptToAppend( |
apps/web/src/components/ChatView.tsx+58-28| ... | @@ -4514,9 +4514,10 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4514,9 +4514,10 @@ export default function ChatView(props: ChatViewProps) { |
| 4514 | }, [activeThreadRef]); | 4514 | }, [activeThreadRef]); |
| 4515 | const supportsThreadPullRequests = | 4515 | const supportsThreadPullRequests = |
| 4516 | serverConfig?.environment.capabilities.threadPullRequests === true; | 4516 | serverConfig?.environment.capabilities.threadPullRequests === true; |
| 4517 | const visiblePullRequestCount = visibleThreadPullRequests( | 4517 | const visiblePullRequests = visibleThreadPullRequests( |
| 4518 | (activeThreadShell ?? activeThread)?.pullRequests ?? [], | 4518 | (activeThreadShell ?? activeThread)?.pullRequests ?? [], |
| 4519 | ).length; | 4519 | ); |
| 4520 | const visiblePullRequestCount = visiblePullRequests.length; | ||
| 4520 | const pullRequestsSurfaceAvailable = | 4521 | const pullRequestsSurfaceAvailable = |
| 4521 | isServerThread && supportsThreadPullRequests && visiblePullRequestCount > 0; | 4522 | isServerThread && supportsThreadPullRequests && visiblePullRequestCount > 0; |
| 4522 | const addPullRequestsSurface = useCallback(() => { | 4523 | const addPullRequestsSurface = useCallback(() => { |
| ... | @@ -4616,6 +4617,7 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4616,6 +4617,7 @@ export default function ChatView(props: ChatViewProps) { |
| 4616 | ); | 4617 | ); |
| 4617 | // The shell carries server PR updates even while thread detail is still loading. | 4618 | // The shell carries server PR updates even while thread detail is still loading. |
| 4618 | const activeThreadMetadata = activeThreadShell ?? activeThread; | 4619 | const activeThreadMetadata = activeThreadShell ?? activeThread; |
| 4620 | const hasLinkedPullRequestDetail = activeThreadMetadata?.linkedPullRequest != null; | ||
| 4619 | const linkedThreadPullRequest = | 4621 | const linkedThreadPullRequest = |
| 4620 | activeThreadMetadata?.linkedPullRequest ?? activeThreadMetadata?.branchPullRequest ?? null; | 4622 | activeThreadMetadata?.linkedPullRequest ?? activeThreadMetadata?.branchPullRequest ?? null; |
| 4621 | const activeProjectRepository = activeProject?.repositoryIdentity?.displayName ?? null; | 4623 | const activeProjectRepository = activeProject?.repositoryIdentity?.displayName ?? null; |
| ... | @@ -4626,6 +4628,11 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4626,6 +4628,11 @@ export default function ChatView(props: ChatViewProps) { |
| 4626 | linkedThreadPullRequest.number, | 4628 | linkedThreadPullRequest.number, |
| 4627 | ]) | 4629 | ]) |
| 4628 | : null; | 4630 | : null; |
| 4631 | const proactivePullRequestsKey = pullRequestsSurfaceAvailable | ||
| 4632 | ? JSON.stringify( | ||
| 4633 | visiblePullRequests.map((link) => [link.host, link.repository, link.number]).sort(), | ||
| 4634 | ) | ||
| 4635 | : linkedThreadPullRequestKey; | ||
| 4629 | const observedThreadPullRequestRef = useRef<{ | 4636 | const observedThreadPullRequestRef = useRef<{ |
| 4630 | readonly threadKey: string; | 4637 | readonly threadKey: string; |
| 4631 | readonly reference: ThreadLinkedPullRequest | null; | 4638 | readonly reference: ThreadLinkedPullRequest | null; |
| ... | @@ -4692,7 +4699,40 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4692,7 +4699,40 @@ export default function ChatView(props: ChatViewProps) { |
| 4692 | userActionRevision, | 4699 | userActionRevision, |
| 4693 | ); | 4700 | ); |
| 4694 | } | 4701 | } |
| 4695 | if (!clientSettingsHydrated || threadDetailLoading) return; | 4702 | if (!clientSettingsHydrated) return; |
| 4703 | |||
| 4704 | const proactivePanelsEnabled = settings.proactivePanelsEnabled && !shouldUseRightPanelSheet; | ||
| 4705 | const eligibleLink = | ||
| 4706 | proactivePanelsEnabled && | ||
| 4707 | shouldOpenProactivePullRequest(previousTargetKey, proactivePullRequestsKey); | ||
| 4708 | const shouldDeferLink = eligibleLink && !pullRequestsCapabilityKnown; | ||
| 4709 | proactivePanelObservationRef.current = { | ||
| 4710 | ...observation, | ||
| 4711 | targetKey: shouldDeferLink ? (previousTargetKey ?? null) : proactivePullRequestsKey, | ||
| 4712 | }; | ||
| 4713 | if (eligibleLink && pullRequestsCapabilityKnown) { | ||
| 4714 | if ( | ||
| 4715 | pullRequestsSurfaceAvailable && | ||
| 4716 | (visiblePullRequestCount > 1 || !hasLinkedPullRequestDetail || !supportsPullRequests) | ||
| 4717 | ) { | ||
| 4718 | panels.openProactive( | ||
| 4719 | activeThreadRef, | ||
| 4720 | { id: "pull-requests", kind: "pull-requests" }, | ||
| 4721 | userActionRevision, | ||
| 4722 | ); | ||
| 4723 | } else if ( | ||
| 4724 | !followSelectedPullRequest && | ||
| 4725 | supportsPullRequests && | ||
| 4726 | linkedThreadPullRequest !== null | ||
| 4727 | ) { | ||
| 4728 | panels.openProactive( | ||
| 4729 | activeThreadRef, | ||
| 4730 | pullRequestSurface(linkedThreadPullRequest), | ||
| 4731 | userActionRevision, | ||
| 4732 | ); | ||
| 4733 | } | ||
| 4734 | } | ||
| 4735 | if (threadDetailLoading) return; | ||
| 4696 | 4736 | ||
| 4697 | const settledTurnId = latestTurnSettled ? (activeLatestTurn?.turnId ?? null) : null; | 4737 | const settledTurnId = latestTurnSettled ? (activeLatestTurn?.turnId ?? null) : null; |
| 4698 | const newlyCompletedTurnId = shouldOpenProactiveTurnDiff({ | 4738 | const newlyCompletedTurnId = shouldOpenProactiveTurnDiff({ |
| ... | @@ -4703,8 +4743,13 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4703,8 +4743,13 @@ export default function ChatView(props: ChatViewProps) { |
| 4703 | }) | 4743 | }) |
| 4704 | ? settledTurnId | 4744 | ? settledTurnId |
| 4705 | : null; | 4745 | : null; |
| 4706 | const proactivePanelsEnabled = settings.proactivePanelsEnabled && !shouldUseRightPanelSheet; | 4746 | const eligibleCompletion = |
| 4707 | const eligibleCompletion = proactivePanelsEnabled && newlyCompletedTurnId !== null; | 4747 | proactivePanelsEnabled && |
| 4748 | newlyCompletedTurnId !== null && | ||
| 4749 | !( | ||
| 4750 | proactivePullRequestsKey !== null && | ||
| 4751 | (!pullRequestsCapabilityKnown || supportsPullRequests || pullRequestsSurfaceAvailable) | ||
| 4752 | ); | ||
| 4708 | const completedCheckpoint = eligibleCompletion | 4753 | const completedCheckpoint = eligibleCompletion |
| 4709 | ? activeThread?.checkpoints.find((checkpoint) => checkpoint.turnId === newlyCompletedTurnId) | 4754 | ? activeThread?.checkpoints.find((checkpoint) => checkpoint.turnId === newlyCompletedTurnId) |
| 4710 | : undefined; | 4755 | : undefined; |
| ... | @@ -4714,30 +4759,12 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4714,30 +4759,12 @@ export default function ChatView(props: ChatViewProps) { |
| 4714 | isGitRepo: gitStatusQuery.data?.isRepo, | 4759 | isGitRepo: gitStatusQuery.data?.isRepo, |
| 4715 | }) | 4760 | }) |
| 4716 | : "ignore"; | 4761 | : "ignore"; |
| 4717 | const eligibleLink = | ||
| 4718 | proactivePanelsEnabled && | ||
| 4719 | shouldOpenProactivePullRequest(previousTargetKey, linkedThreadPullRequestKey); | ||
| 4720 | const shouldDeferLink = eligibleLink && !pullRequestsCapabilityKnown; | ||
| 4721 | proactivePanelObservationRef.current = { | 4762 | proactivePanelObservationRef.current = { |
| 4722 | ...observation, | 4763 | ...proactivePanelObservationRef.current, |
| 4723 | // Preserve first-entry eligibility while the checkpoint or repository is loading. | 4764 | // Preserve first-entry eligibility while capabilities, checkpoint or repository load. |
| 4724 | runningTurnId: diffAction === "defer" ? previousRunningTurnId : activeRunningTurnId, | 4765 | runningTurnId: |
| 4725 | targetKey: shouldDeferLink ? (previousTargetKey ?? null) : linkedThreadPullRequestKey, | 4766 | diffAction === "defer" || shouldDeferLink ? previousRunningTurnId : activeRunningTurnId, |
| 4726 | }; | 4767 | }; |
| 4727 | |||
| 4728 | if ( | ||
| 4729 | !followSelectedPullRequest && | ||
| 4730 | eligibleLink && | ||
| 4731 | pullRequestsCapabilityKnown && | ||
| 4732 | supportsPullRequests && | ||
| 4733 | linkedThreadPullRequest !== null | ||
| 4734 | ) { | ||
| 4735 | panels.openProactive( | ||
| 4736 | activeThreadRef, | ||
| 4737 | pullRequestSurface(linkedThreadPullRequest), | ||
| 4738 | userActionRevision, | ||
| 4739 | ); | ||
| 4740 | } | ||
| 4741 | if (diffAction !== "open" || newlyCompletedTurnId === null) return; | 4768 | if (diffAction !== "open" || newlyCompletedTurnId === null) return; |
| 4742 | if (!panels.openProactive(activeThreadRef, { id: "diff", kind: "diff" }, userActionRevision)) { | 4769 | if (!panels.openProactive(activeThreadRef, { id: "diff", kind: "diff" }, userActionRevision)) { |
| 4743 | return; | 4770 | return; |
| ... | @@ -4756,9 +4783,12 @@ export default function ChatView(props: ChatViewProps) { | ... | @@ -4756,9 +4783,12 @@ export default function ChatView(props: ChatViewProps) { |
| 4756 | isServerThread, | 4783 | isServerThread, |
| 4757 | latestTurnSettled, | 4784 | latestTurnSettled, |
| 4758 | linkedThreadPullRequest, | 4785 | linkedThreadPullRequest, |
| 4759 | linkedThreadPullRequestKey, | 4786 | proactivePullRequestsKey, |
| 4787 | hasLinkedPullRequestDetail, | ||
| 4760 | onDiffPanelOpen, | 4788 | onDiffPanelOpen, |
| 4761 | pullRequestsCapabilityKnown, | 4789 | pullRequestsCapabilityKnown, |
| 4790 | pullRequestsSurfaceAvailable, | ||
| 4791 | visiblePullRequestCount, | ||
| 4762 | settings.proactivePanelsEnabled, | 4792 | settings.proactivePanelsEnabled, |
| 4763 | shouldUseRightPanelSheet, | 4793 | shouldUseRightPanelSheet, |
| 4764 | supportsPullRequests, | 4794 | supportsPullRequests, |
apps/web/src/components/settings/SettingsPanels.tsx+1-1| ... | @@ -2528,7 +2528,7 @@ export function GeneralSettingsPanel() { | ... | @@ -2528,7 +2528,7 @@ export function GeneralSettingsPanel() { |
| 2528 | 2528 | ||
| 2529 | <SettingsRow | 2529 | <SettingsRow |
| 2530 | {...searchableSetting("proactive-panels")} | 2530 | {...searchableSetting("proactive-panels")} |
| 2531 | description="Open linked pull requests when found and turn diffs when work changes files." | 2531 | description="Open linked pull requests first. Otherwise, open turn diffs for changes to at least 3 files or 50 lines." |
| 2532 | resetAction={ | 2532 | resetAction={ |
| 2533 | settings.proactivePanelsEnabled !== DEFAULT_UNIFIED_SETTINGS.proactivePanelsEnabled ? ( | 2533 | settings.proactivePanelsEnabled !== DEFAULT_UNIFIED_SETTINGS.proactivePanelsEnabled ? ( |
| 2534 | <SettingResetButton | 2534 | <SettingResetButton |
apps/web/src/rightPanelStore.test.ts+16-8| ... | @@ -109,22 +109,27 @@ describe("rightPanelStore", () => { | ... | @@ -109,22 +109,27 @@ describe("rightPanelStore", () => { |
| 109 | number: 42, | 109 | number: 42, |
| 110 | }); | 110 | }); |
| 111 | 111 | ||
| 112 | it.each(["diff-first", "pull-request-first"])( | 112 | it.each([ |
| 113 | "prioritizes the linked pull request over browser and diff with %s delivery", | 113 | { order: "diff-first", surface: linkedPullRequest }, |
| 114 | (order) => { | 114 | { order: "pull-request-first", surface: linkedPullRequest }, |
| 115 | { order: "diff-first", surface: { id: "pull-requests", kind: "pull-requests" } as const }, | ||
| 116 | { | ||
| 117 | order: "pull-request-first", | ||
| 118 | surface: { id: "pull-requests", kind: "pull-requests" } as const, | ||
| 119 | }, | ||
| 120 | ])( | ||
| 121 | "prioritizes $surface.kind over browser and diff with $order delivery", | ||
| 122 | ({ order, surface }) => { | ||
| 115 | const store = useRightPanelStore.getState(); | 123 | const store = useRightPanelStore.getState(); |
| 116 | store.openBrowser(refA, "existing-browser"); | 124 | store.openBrowser(refA, "existing-browser"); |
| 117 | const revision = store.getUserActionRevision(refA); | 125 | const revision = store.getUserActionRevision(refA); |
| 118 | const requests = | 126 | const requests = order === "diff-first" ? [completedDiff, surface] : [surface, completedDiff]; |
| 119 | order === "diff-first" | ||
| 120 | ? [completedDiff, linkedPullRequest] | ||
| 121 | : [linkedPullRequest, completedDiff]; | ||
| 122 | for (const surface of requests) store.openProactive(refA, surface, revision); | 127 | for (const surface of requests) store.openProactive(refA, surface, revision); |
| 123 | store.reconcileBrowserSurfaces(refA, ["existing-browser", "agent-browser"]); | 128 | store.reconcileBrowserSurfaces(refA, ["existing-browser", "agent-browser"]); |
| 124 | 129 | ||
| 125 | expect( | 130 | expect( |
| 126 | selectActiveRightPanelSurface(useRightPanelStore.getState().byThreadKey, refA), | 131 | selectActiveRightPanelSurface(useRightPanelStore.getState().byThreadKey, refA), |
| 127 | ).toEqual(linkedPullRequest); | 132 | ).toEqual(surface); |
| 128 | 133 | ||
| 129 | store.open(refA, "diff"); | 134 | store.open(refA, "diff"); |
| 130 | expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff"); | 135 | expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff"); |
| ... | @@ -167,6 +172,9 @@ describe("rightPanelStore", () => { | ... | @@ -167,6 +172,9 @@ describe("rightPanelStore", () => { |
| 167 | 172 | ||
| 168 | expect(store.openProactive(refA, completedDiff, revision)).toBe(false); | 173 | expect(store.openProactive(refA, completedDiff, revision)).toBe(false); |
| 169 | expect(store.openProactive(refA, linkedPullRequest, revision)).toBe(false); | 174 | expect(store.openProactive(refA, linkedPullRequest, revision)).toBe(false); |
| 175 | expect( | ||
| 176 | store.openProactive(refA, { id: "pull-requests", kind: "pull-requests" }, revision), | ||
| 177 | ).toBe(false); | ||
| 170 | expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toBe( | 178 | expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toBe( |
| 171 | chosen, | 179 | chosen, |
| 172 | ); | 180 | ); |
apps/web/src/rightPanelStore.ts+3-2| ... | @@ -124,7 +124,7 @@ interface RightPanelStoreState { | ... | @@ -124,7 +124,7 @@ interface RightPanelStoreState { |
| 124 | */ | 124 | */ |
| 125 | openProactive: ( | 125 | openProactive: ( |
| 126 | ref: ScopedThreadRef, | 126 | ref: ScopedThreadRef, |
| 127 | surface: Extract<RightPanelSurface, { kind: "diff" | "pull-request" }>, | 127 | surface: Extract<RightPanelSurface, { kind: "diff" | "pull-request" | "pull-requests" }>, |
| 128 | expectedUserActionRevision: number, | 128 | expectedUserActionRevision: number, |
| 129 | ) => boolean; | 129 | ) => boolean; |
| 130 | open: ( | 130 | open: ( |
| ... | @@ -496,7 +496,8 @@ export const useRightPanelStore = create<RightPanelStoreState>()( | ... | @@ -496,7 +496,8 @@ export const useRightPanelStore = create<RightPanelStoreState>()( |
| 496 | // always apply, and later user choices reject both proactive requests. | 496 | // always apply, and later user choices reject both proactive requests. |
| 497 | if ( | 497 | if ( |
| 498 | surface.kind === "diff" && | 498 | surface.kind === "diff" && |
| 499 | selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" | 499 | (selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" || |
| 500 | selectActiveRightPanel(state.byThreadKey, ref) === "pull-requests") | ||
| 500 | ) { | 501 | ) { |
| 501 | return state; | 502 | return state; |
| 502 | } | 503 | } |