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
2 changes: 1 addition & 1 deletion bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"@vercel/ai-sdk-openai-websocket-fetch": "^1.0.0",
"@xterm/addon-serialize": "^0.14.0",
"@xterm/headless": "^6.0.0",
"ai": "^6.0.175",
"ai": "6.0.175",
Comment thread
ammar-agent marked this conversation as resolved.
"ai-tokenizer": "^1.0.6",
"chalk": "^5.6.2",
"comlink": "^4.4.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ describe("ImmersiveMinimap", () => {
/>
);

expect(view.getByTestId("immersive-minimap").className).toContain("h-full");
// Synchronous render — no waitFor needed since parseDiffLines runs during render
expect(view.getByTestId("immersive-minimap-canvas")).toBeTruthy();
});
Expand Down Expand Up @@ -133,6 +134,41 @@ describe("ImmersiveMinimap", () => {
expect(parseSpy).toHaveBeenCalledWith(updatedContent);
});

test("redraws from the latest scroll position when parent bumps redraw nonce", () => {
const stableLineCategories: immersiveMinimapMath.LineCategory[] = ["add", "remove", "context"];
spyOn(immersiveMinimapMath, "parseDiffLines").mockImplementation(() => stableLineCategories);
const getThumbMetricsSpy = spyOn(immersiveMinimapMath, "getThumbMetrics");
const scrollFixture = createScrollContainerFixture();
const scrollContainerRef = { current: scrollFixture.element };
const commentLineIndices = new Set<number>();
const onSelectLineIndex = mock(() => undefined);

const renderMinimap = (redrawNonce: number) => (
<ImmersiveMinimap
content="+line a\n-line b\n context"
scrollContainerRef={scrollContainerRef}
activeLineIndex={null}
redrawNonce={redrawNonce}
onSelectLineIndex={onSelectLineIndex}
commentLineIndices={commentLineIndices}
/>
);

const view = render(renderMinimap(0));
const canvas = view.getByTestId("immersive-minimap-canvas") as HTMLCanvasElement;
Object.defineProperty(canvas, "clientWidth", { configurable: true, get: () => 48 });
Object.defineProperty(canvas, "clientHeight", { configurable: true, get: () => 120 });
getThumbMetricsSpy.mockClear();

scrollFixture.element.scrollTop = 375;
view.rerender(renderMinimap(1));

// Parent reveal scroll happens outside the minimap. The nonce is the contract
// that forces a hidden pre-reveal canvas redraw using the new scrollTop even
// when React Compiler keeps parsed line categories stable across renders.
expect(getThumbMetricsSpy).toHaveBeenCalledWith(375, 1000, 250, 120);
});

test("clicking minimap dispatches the mapped line index", () => {
const scrollFixture = createScrollContainerFixture();
const onSelectLineIndex = mock(() => undefined);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useRef } from "react";
import React, { useCallback, useEffect, useLayoutEffect, useRef } from "react";

import { clamp } from "@/common/utils/clamp";
import {
Expand All @@ -15,6 +15,8 @@ interface ImmersiveMinimapProps {
onSelectLineIndex: (lineIndex: number) => void;
/** Diff line indices where review comments exist (drawn as yellow indicators) */
commentLineIndices: ReadonlySet<number>;
/** Bump after parent-owned scroll positioning so the canvas reads fresh scrollTop before reveal. */
redrawNonce?: number;
}

const DEFAULT_ADD_COLOR = "rgba(34, 197, 94, 0.85)";
Expand Down Expand Up @@ -133,9 +135,11 @@ export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
}
}, [lineCategories, props.activeLineIndex, props.commentLineIndices, props.scrollContainerRef]);

useEffect(() => {
useLayoutEffect(() => {
// Canvas pixels are visible layout state; redraw before paint so hydration
// swaps do not show one frame of the previous minimap/scroll thumb.
redrawCanvas();
}, [redrawCanvas]);
}, [redrawCanvas, props.redrawNonce]);

useEffect(() => {
const scrollContainer = props.scrollContainerRef.current;
Expand Down Expand Up @@ -280,7 +284,7 @@ export const ImmersiveMinimap: React.FC<ImmersiveMinimapProps> = (props) => {
}

return (
<div className="w-6 shrink-0 bg-[var(--color-bg-dark)]" data-testid="immersive-minimap">
<div className="h-full w-6 shrink-0 bg-[var(--color-bg-dark)]" data-testid="immersive-minimap">
<canvas
ref={canvasRef}
className="h-full w-full cursor-pointer"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
import { cleanup, fireEvent, render, waitFor } from "@testing-library/react";
import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react";
import { GlobalWindow } from "happy-dom";
import { useEffect, useState, type ComponentProps } from "react";

Expand Down Expand Up @@ -32,7 +32,7 @@ void mock.module("@/browser/contexts/API", () => ({
}),
}));

import { ImmersiveReviewView } from "./ImmersiveReviewView";
import { ImmersiveReviewView, shouldPreserveImmersiveContextCursor } from "./ImmersiveReviewView";

function createHunk(overrides: Partial<DiffHunk> = {}): DiffHunk {
return {
Expand Down Expand Up @@ -114,6 +114,36 @@ function renderImmersiveReview(
);
}

function installManualAnimationFrame() {
let nextFrameId = 1;
const callbacks = new Map<number, FrameRequestCallback>();

const requestAnimationFrameMock = mock((callback: FrameRequestCallback) => {
const frameId = nextFrameId;
nextFrameId += 1;
callbacks.set(frameId, callback);
return frameId;
}) as unknown as typeof globalThis.requestAnimationFrame;
const cancelAnimationFrameMock = mock((frameId: number) => {
callbacks.delete(frameId);
}) as unknown as typeof globalThis.cancelAnimationFrame;

globalThis.requestAnimationFrame = requestAnimationFrameMock;
globalThis.cancelAnimationFrame = cancelAnimationFrameMock;
globalThis.window.requestAnimationFrame = requestAnimationFrameMock;
globalThis.window.cancelAnimationFrame = cancelAnimationFrameMock;

return {
flush() {
const pendingCallbacks = Array.from(callbacks.values());
callbacks.clear();
for (const callback of pendingCallbacks) {
callback(performance.now());
}
},
};
}

describe("ImmersiveReviewView", () => {
let originalWindow: typeof globalThis.window;
let originalDocument: typeof globalThis.document;
Expand Down Expand Up @@ -193,6 +223,116 @@ describe("ImmersiveReviewView", () => {
});
});

test("keeps hydrated full-file context behind the reveal gate until positioned", async () => {
type ExecuteBashValue = Awaited<ReturnType<MockApiClient["workspace"]["executeBash"]>>;
let resolveRead: ((value: ExecuteBashValue) => void) | undefined;
const pendingRead = new Promise<ExecuteBashValue>((resolve) => {
resolveRead = resolve;
});
mockApi.workspace.executeBash = mock(() => pendingRead);
const animationFrame = installManualAnimationFrame();

const hunk = createHunk({
newStart: 3,
oldStart: 3,
header: "@@ -3 +3 @@",
content: "-old selected line\n+new selected line",
});

const view = renderImmersiveReview({
fileTree: createFileTree(hunk.filePath),
hunks: [hunk],
allHunks: [hunk],
selectedHunkId: hunk.id,
isTouchImmersive: false,
});

await waitFor(() => expect(mockApi.workspace.executeBash).toHaveBeenCalledTimes(1));
expect(view.getByTestId("immersive-diff-reveal-overlay").textContent ?? "").toContain(
"Loading file"
);
expect(view.getByTestId("immersive-diff-reveal-stage").className).toContain("invisible");

act(() => animationFrame.flush());
await waitFor(() => expect(view.queryByTestId("immersive-diff-reveal-overlay")).toBeNull());
expect(view.getByTestId("immersive-diff-reveal-stage").className).not.toContain("invisible");

const resolveLoadedContent = resolveRead;
if (!resolveLoadedContent) {
throw new Error("Read promise resolver was not captured");
}

act(() => {
resolveLoadedContent({
success: true,
data: {
success: true,
output: encodeFileReadOutput(
["context before selected 1", "context before selected 2", "new selected line"].join(
"\n"
)
),
exitCode: 0,
},
});
});

await waitFor(() =>
expect(view.getByTestId("immersive-diff-reveal-overlay").textContent ?? "").toContain(
"Preparing diff"
)
);
expect(view.container.textContent ?? "").toContain("context before selected 1");
expect(view.getByTestId("immersive-diff-reveal-stage").className).toContain("invisible");
expect(view.getByTestId("immersive-minimap-reveal-stage").className).toContain("h-full");
expect(view.getByTestId("immersive-minimap-reveal-stage").className).toContain("invisible");

act(() => animationFrame.flush());
await waitFor(() => expect(view.queryByTestId("immersive-diff-reveal-overlay")).toBeNull());
expect(view.getByTestId("immersive-diff-reveal-stage").className).not.toContain("invisible");
expect(view.getByTestId("immersive-minimap-reveal-stage").className).not.toContain("invisible");
});

test("only preserves context cursors while overlay content is unchanged", () => {
const previousRange = { startIndex: 0, endIndex: 1 };

expect(
shouldPreserveImmersiveContextCursor({
cursorLineIndex: 2,
previousRange,
previousHunkId: "hunk-selected",
currentHunkId: "hunk-selected",
previousOverlayContent: "compact overlay",
currentOverlayContent: "compact overlay",
})
).toBe(true);

// Compact hunk overlays and hydrated full-file overlays use different
// numeric row indices. Do not carry an out-of-hunk compact cursor across
// hydration, or the reveal can replay a stale context/separator row.
expect(
shouldPreserveImmersiveContextCursor({
cursorLineIndex: 2,
previousRange,
previousHunkId: "hunk-selected",
currentHunkId: "hunk-selected",
previousOverlayContent: "compact overlay",
currentOverlayContent: "hydrated full-file overlay",
})
).toBe(false);

expect(
shouldPreserveImmersiveContextCursor({
cursorLineIndex: 1,
previousRange,
previousHunkId: "hunk-selected",
currentHunkId: "hunk-selected",
previousOverlayContent: "compact overlay",
currentOverlayContent: "compact overlay",
})
).toBe(false);
});

test("skips full-file reads when the selected hunk starts beyond the render budget", () => {
const farHunk = createHunk({
id: "hunk-far",
Expand Down
Loading
Loading