fix(app): keep the Changes diff on screen when its panel reopens

Hiding the Changes panel releases its diff subscription. Reopening it starts a
new stream, and since #8907 a new stream first publishes a loading snapshot
(entries, no bodies) before its first revision commits. That snapshot replaced
the settled diff in the query cache, so the panel showed the full loading
spinner for about a second and then repainted the same diff.

A loading snapshot now only counts on the first load. Once a diff has settled,
shareCheckoutDiff keeps it until the new stream's ready revision lands, which
replaces it in one step when the content changed.
This commit is contained in:
Mohamed Boudra
2026-11-10 02:25:35 +01:00
parent ab64bebe14
commit 156462e74f
3 changed files with 232 additions and 0 deletions
@@ -306,6 +306,24 @@ test("streamed Changes shows the tree before completion and applies a one-file r
await page.screenshot({ path: testInfo.outputPath("refreshed.png") });
});
test("reopening compact Changes keeps the painted diff while it resubscribes", async ({ page }) => {
const workspace = await createWorkspaceWithMountedTabDiff();
const stream = await holdDiffCompletion(page);
const explorer = await openCompactChanges(page, workspace);
stream.finish();
const firstFile = explorer.getByTestId("diff-file-0");
await expect(firstFile).toBeVisible({ timeout: 30_000 });
const painted = await firstFile.getAttribute("aria-label");
await explorer.getByTestId("workspace-tab-files").click();
stream.hold();
await explorer.getByTestId("workspace-tab-changes_tree").click();
await expect.poll(() => stream.held, { timeout: 30_000 }).toBeGreaterThan(0);
await expect(page.getByRole("progressbar", { name: "Loading..." })).toHaveCount(0);
await expect(firstFile).toHaveAccessibleName(painted ?? "");
stream.finish();
});
async function createLargeStreamedWorkspace(): Promise<DirtyWorkspace> {
const workspace = await createWorkspaceWithManyTinyDiffs(252);
await Promise.all(
@@ -371,6 +389,12 @@ async function holdDiffCompletion(page: Page) {
get completed() {
return completed;
},
get held() {
return completions.length;
},
hold: () => {
holding = true;
},
finish: () => {
holding = false;
completions.splice(0).forEach((send) => send());
+200
View File
@@ -2,6 +2,8 @@ import type { DiffState, OwnedSubscription, SubscriptionObserver } from "@getpas
import { QueryClient, QueryObserver, skipToken } from "@tanstack/react-query";
import { describe, expect, it } from "vitest";
import type { MutableDaemonConfig, SessionOutboundMessage } from "@getpaseo/protocol/messages";
import { deriveCheckoutDiffResult, type CheckoutDiffSnapshot } from "@/git/diff-snapshot";
import { shareCheckoutDiff } from "@/git/diff-sharing";
import { checkoutDiffQueryKey } from "@/git/query-keys";
import { buildTerminalsQueryKey } from "@/screens/workspace/terminals/state";
import { daemonConfigQueryKey } from "@/data/daemon-config";
@@ -34,6 +36,7 @@ type RouterMessage =
type RouterMessageType = RouterMessage["type"];
type RouterHandler = (message: RouterMessage) => void;
type RouterClient = Parameters<typeof mountServerDataPushRouter>[0]["client"];
type DiffFile = SubscribeCheckoutDiffResponseMessage["payload"]["files"][number];
const daemonConfig: MutableDaemonConfig = {
relay: { enabled: false },
@@ -233,6 +236,49 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {}
};
}
/** Matches the real diff observation: nothing is delivered until the stream publishes. */
type SubscribedDiffState = DiffState & { subscriptionId: string };
function createDiffStreamClient() {
const subscriptions: Array<{ publish(state: DiffState): void }> = [];
const noSubscription = {
subscriptionId: "none",
ready: Promise.resolve(undefined as never),
subscribe: () => () => {},
release: async () => {},
};
const client: RouterClient = {
async getProvidersSnapshot() {
throw new Error("Unexpected snapshot pull");
},
observeEvents: () => noSubscription,
observeTerminals: () => noSubscription,
observeDiff() {
const observers = new Set<SubscriptionObserver<SubscribedDiffState>>();
let released = false;
const subscriptionId = `diff-${subscriptions.length + 1}`;
subscriptions.push({
publish: (state) => {
if (released) return;
for (const observer of observers) observer.snapshot({ ...state, subscriptionId });
},
});
return {
subscriptionId,
ready: new Promise<never>(() => {}),
subscribe: (observer: SubscriptionObserver<SubscribedDiffState>) => {
observers.add(observer);
return () => observers.delete(observer);
},
release: async () => {
released = true;
},
};
},
};
return { client, subscriptions, latest: () => subscriptions[subscriptions.length - 1]! };
}
function providerUpdate(generatedAt: string): ProvidersSnapshotUpdateMessage {
return {
type: "providers_snapshot_update",
@@ -388,6 +434,160 @@ describe("server data push router", () => {
}
});
describe("a diff reopened from a retained panel", () => {
const serverId = "server-1";
const cwd = "/repo";
const queryKey = checkoutDiffQueryKey(serverId, cwd, "uncommitted", undefined, false);
function diffFile(content: string): DiffFile {
return {
path: "README.md",
isNew: false,
isDeleted: false,
additions: 1,
deletions: 0,
hunks: [
{
oldStart: 1,
oldCount: 1,
newStart: 1,
newCount: 1,
lines: [{ type: "add", content, tokens: [{ text: content, style: null }] }],
},
],
};
}
function loadingState(revision: number): DiffState {
return {
phase: "loading",
revision,
entries: [
{
path: "README.md",
oldPath: null,
change: "modified",
content: "text",
additions: 1,
deletions: 0,
},
],
};
}
function readyState(revision: number, files: DiffFile[]): DiffState {
return {
phase: "ready",
revision,
resolved: { baseSha: "", headSha: "", mergeBaseSha: "" },
files,
diffTooLarge: false,
};
}
function mountChangesPanel(fake: ReturnType<typeof createDiffStreamClient>) {
const queryClient = new QueryClient();
const route = (enabled: boolean) => ({
queryKey,
queryFn: skipToken as typeof skipToken,
gcTime: Infinity,
structuralSharing: shareCheckoutDiff,
meta: checkoutDiffPushRoute({
enabled,
serverId,
cwd,
subscriptionId: `checkoutDiff:${JSON.stringify(queryKey)}`,
compare: { mode: "uncommitted", ignoreWhitespace: false },
}),
});
const observer = new QueryObserver(queryClient, route(true));
const unsubscribeObserver = observer.subscribe(() => {});
const unmountRouter = mountServerDataPushRouter({
queryClient,
client: fake.client,
serverId,
});
return {
hide: () => observer.setOptions(route(false)),
show: () => observer.setOptions(route(true)),
// What the Changes pane renders from.
diff: () =>
deriveCheckoutDiffResult(
queryClient.getQueryData<CheckoutDiffSnapshot>(queryKey) ?? null,
),
unmount: () => {
unsubscribeObserver();
unmountRouter();
queryClient.clear();
},
};
}
it("keeps the painted diff through the reload when the daemon returns the same content", () => {
const fake = createDiffStreamClient();
const panel = mountChangesPanel(fake);
try {
fake.latest().publish(readyState(1, [diffFile("same")]));
const painted = panel.diff().files;
expect(fake.subscriptions).toHaveLength(1);
panel.hide();
panel.show();
expect(fake.subscriptions).toHaveLength(2);
fake.latest().publish(loadingState(1));
expect(panel.diff()).toMatchObject({ isLoading: false, entries: null });
expect(panel.diff().files).toBe(painted);
fake.latest().publish(readyState(1, [diffFile("same")]));
expect(panel.diff().isLoading).toBe(false);
expect(panel.diff().files).toBe(painted);
} finally {
panel.unmount();
}
});
it("replaces the painted diff in one step when the daemon returns changed content", () => {
const fake = createDiffStreamClient();
const panel = mountChangesPanel(fake);
try {
fake.latest().publish(readyState(1, [diffFile("before")]));
const painted = panel.diff().files;
panel.hide();
panel.show();
fake.latest().publish(loadingState(1));
expect(panel.diff().isLoading).toBe(false);
expect(panel.diff().files).toBe(painted);
const changed = [diffFile("after")];
fake.latest().publish(readyState(1, changed));
expect(panel.diff().isLoading).toBe(false);
expect(panel.diff().files).toEqual(changed);
} finally {
panel.unmount();
}
});
it("reports loading with the announced entries when no content has been painted", () => {
const fake = createDiffStreamClient();
const panel = mountChangesPanel(fake);
try {
expect(panel.diff().isLoading).toBe(true);
fake.latest().publish(loadingState(1));
expect(panel.diff()).toMatchObject({ isLoading: true, files: [] });
expect(panel.diff().entries).toHaveLength(1);
fake.latest().publish(readyState(1, [diffFile("first")]));
expect(panel.diff()).toMatchObject({ isLoading: false, entries: null });
expect(panel.diff().files).toHaveLength(1);
} finally {
panel.unmount();
}
});
});
it("routes provider snapshot and daemon config payloads until detached", async () => {
const queryClient = new QueryClient();
const fake = createFakeClient();
+8
View File
@@ -25,6 +25,10 @@ export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPay
const next = incoming as DiffPayload;
const old = previous as DiffPayload | undefined;
if (!old || old === next) return next;
// A new subscription announces its entries before the bodies arrive. That is
// progress on the first load only: once a diff has settled it stays on screen
// until the next revision commits, so an unchanged diff never repaints.
if (isLoadingSnapshot(next) && !isLoadingSnapshot(old)) return old;
const files = retainDiffFiles(old.files, next.files);
const error = replaceEqualDeep(old.error, next.error);
return files === old.files &&
@@ -37,6 +41,10 @@ export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPay
: { ...next, files, error };
}
function isLoadingSnapshot(payload: DiffPayload): boolean {
return payload.entries !== undefined;
}
export function shareCommitFileDiff(
previous: unknown,
incoming: unknown,