Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 28 additions & 22 deletions apps/web/src/components/ChatView.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<TurnDiffSummary, "status" | "files">;
const unchangedCheckpoint = {
status: "ready",
files: [],
} satisfies Pick<TurnDiffSummary, "status" | "files">;
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<TurnDiffSummary, "status" | "files">;

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 = {
Expand Down
6 changes: 5 additions & 1 deletion apps/web/src/components/ChatView.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
86 changes: 58 additions & 28 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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({
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -4756,9 +4783,12 @@ export default function ChatView(props: ChatViewProps) {
isServerThread,
latestTurnSettled,
linkedThreadPullRequest,
linkedThreadPullRequestKey,
proactivePullRequestsKey,
hasLinkedPullRequestDetail,
onDiffPanelOpen,
pullRequestsCapabilityKnown,
pullRequestsSurfaceAvailable,
visiblePullRequestCount,
settings.proactivePanelsEnabled,
shouldUseRightPanelSheet,
supportsPullRequests,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/components/settings/SettingsPanels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2528,7 +2528,7 @@ export function GeneralSettingsPanel() {

<SettingsRow
{...searchableSetting("proactive-panels")}
description="Open linked pull requests when found and turn diffs when work changes files."
description="Open linked pull requests first. Otherwise, open turn diffs for changes to at least 3 files or 50 lines."
resetAction={
settings.proactivePanelsEnabled !== DEFAULT_UNIFIED_SETTINGS.proactivePanelsEnabled ? (
<SettingResetButton
Expand Down
24 changes: 16 additions & 8 deletions apps/web/src/rightPanelStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,27 @@ describe("rightPanelStore", () => {
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");
Expand Down Expand Up @@ -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,
);
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/rightPanelStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ interface RightPanelStoreState {
*/
openProactive: (
ref: ScopedThreadRef,
surface: Extract<RightPanelSurface, { kind: "diff" | "pull-request" }>,
surface: Extract<RightPanelSurface, { kind: "diff" | "pull-request" | "pull-requests" }>,
expectedUserActionRevision: number,
) => boolean;
open: (
Expand Down Expand Up @@ -496,7 +496,8 @@ export const useRightPanelStore = create<RightPanelStoreState>()(
// 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;
}
Expand Down
Loading