Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
countUnreadAssistedHunks,
getEffectiveReviewFrontendFilters,
getEffectiveReviewIncludeUncommitted,
getReviewSubProjectPathspec,
normalizeReviewPanelAssistedHunks,
} from "./ReviewPanel";

Expand Down Expand Up @@ -117,6 +118,39 @@ describe("buildReviewDiffPathFilter", () => {
});
});

describe("getReviewSubProjectPathspec", () => {
test("derives a git-root pathspec for sub-project workspaces", () => {
expect(
getReviewSubProjectPathspec({
workspaceMetadata: {
projectPath: "/repo/app",
subProjectPath: "/repo/app/packages/api",
},
projectPath: "/repo/app",
})
).toBe("packages/api");
});

test("ignores missing or unrelated sub-project paths", () => {
expect(
getReviewSubProjectPathspec({
workspaceMetadata: {
projectPath: "/repo/app",
subProjectPath: "/repo/other/packages/api",
},
projectPath: "/repo/app",
})
).toBeNull();

expect(
getReviewSubProjectPathspec({
workspaceMetadata: { projectPath: "/repo/app" },
projectPath: "/repo/app",
})
).toBeNull();
});
});

describe("buildReviewDiffPathFilterSpecs", () => {
const workspaceMetadata = {
projects: [
Expand All @@ -125,6 +159,48 @@ describe("buildReviewDiffPathFilterSpecs", () => {
],
};

test("non-assisted broad diffs default to the sub-project pathspec", () => {
const specs = buildReviewDiffPathFilterSpecs({
isImmersive: false,
assistedOnly: false,
assistedHunks: [],
selectedFilePath: null,
selectedDiffPath: "",
workspaceMetadata: null,
projectPath: "/repo/app",
subProjectPathspec: "packages/api",
});

expect(specs).toEqual([
{
repoRootProjectPath: "/repo/app",
pathFilter: " -- 'packages/api'",
selectedFilePath: null,
},
]);
});

test("broad non-assisted diffs stay unfiltered without a sub-project pathspec", () => {
const specs = buildReviewDiffPathFilterSpecs({
isImmersive: false,
assistedOnly: false,
assistedHunks: [],
selectedFilePath: null,
selectedDiffPath: "",
workspaceMetadata: null,
projectPath: "/repo/app",
subProjectPathspec: null,
});

expect(specs).toEqual([
{
repoRootProjectPath: "/repo/app",
pathFilter: "",
selectedFilePath: null,
},
]);
});

test("assisted mode roots each multi-project pathspec in the pinned file's repository", () => {
const specs = buildReviewDiffPathFilterSpecs({
isImmersive: false,
Expand Down Expand Up @@ -160,6 +236,7 @@ describe("buildReviewDiffPathFilterSpecs", () => {
selectedDiffPath: "",
workspaceMetadata: null,
projectPath: "/repo/app",
subProjectPathspec: "packages/api",
pathContext: {
projectPath: "/repo/app",
executionRootPath: "/repo/app/packages/api",
Expand All @@ -175,23 +252,45 @@ describe("buildReviewDiffPathFilterSpecs", () => {
]);
});

test("non-assisted mode keeps selected file rooting for truncation recovery", () => {
test("non-assisted mode keeps in-scope selected file rooting for truncation recovery", () => {
const specs = buildReviewDiffPathFilterSpecs({
isImmersive: false,
assistedOnly: false,
assistedHunks: [{ path: "project-a/src/main.ts" }],
selectedFilePath: "project-b/src/user-selected.ts",
selectedDiffPath: "src/user-selected.ts",
selectedRepoRootProjectPath: "/repo/project-b",
assistedHunks: [{ path: "project-a/packages/api/src/main.ts" }],
selectedFilePath: "project-a/packages/api/src/user-selected.ts",
selectedDiffPath: "packages/api/src/user-selected.ts",
selectedRepoRootProjectPath: "/repo/project-a",
workspaceMetadata,
projectPath: "/repo/project-a",
subProjectPathspec: "packages/api",
});

expect(specs).toEqual([
{
repoRootProjectPath: "/repo/project-b",
pathFilter: " -- 'src/user-selected.ts'",
selectedFilePath: "project-b/src/user-selected.ts",
repoRootProjectPath: "/repo/project-a",
pathFilter: " -- 'packages/api/src/user-selected.ts'",
selectedFilePath: "project-a/packages/api/src/user-selected.ts",
},
]);
});

test("non-assisted mode ignores stale selected files outside the sub-project", () => {
const specs = buildReviewDiffPathFilterSpecs({
isImmersive: false,
assistedOnly: false,
assistedHunks: [],
selectedFilePath: "packages/web/src/user-selected.ts",
selectedDiffPath: "packages/web/src/user-selected.ts",
workspaceMetadata: null,
projectPath: "/repo/app",
subProjectPathspec: "packages/api",
});

expect(specs).toEqual([
{
repoRootProjectPath: "/repo/app",
pathFilter: " -- 'packages/api'",
selectedFilePath: null,
},
]);
});
Expand Down
109 changes: 90 additions & 19 deletions src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import React, {
} from "react";
import {
buildAssistedReviewPathCandidates,
deriveProjectRelativePath,
findAssistedCandidateMatch,
normalizeAssistedReviewHunks,
resolveAssistedReviewPathCandidatesForHunks,
Expand Down Expand Up @@ -473,6 +474,37 @@ function toPathFilter(pathspecs: readonly string[]): string {
: "";
}

function normalizeReviewPath(pathValue: string | null | undefined): string {
return pathValue?.replaceAll("\\", "/").replace(/^\.\//, "").replace(/\/+$/, "").trim() ?? "";
}

function reviewPathMatchesPathspec(pathValue: string, pathspec: string): boolean {
const normalizedPath = normalizeReviewPath(pathValue);
const normalizedPathspec = normalizeReviewPath(pathspec);
return (
normalizedPath === normalizedPathspec || normalizedPath.startsWith(`${normalizedPathspec}/`)
);
}

export function getReviewSubProjectPathspec(params: {
workspaceMetadata:
| Pick<FrontendWorkspaceMetadata, "projectPath" | "subProjectPath">
| null
| undefined;
projectPath: string;
}): string | null {
const projectPath = params.workspaceMetadata?.projectPath ?? params.projectPath;
const subProjectPath = params.workspaceMetadata?.subProjectPath;
const relativePath = deriveProjectRelativePath(projectPath, subProjectPath);
return relativePath && normalizeReviewPath(relativePath).length > 0
? normalizeReviewPath(relativePath)
: null;
}

export function getReviewSubProjectPathFilter(subProjectPathspec: string | null): string {
return subProjectPathspec ? toPathFilter([subProjectPathspec]) : "";
}

export function buildReviewDiffPathFilterSpecs(params: {
isImmersive: boolean;
assistedOnly: boolean;
Expand All @@ -482,21 +514,25 @@ export function buildReviewDiffPathFilterSpecs(params: {
selectedRepoRootProjectPath?: string | null;
workspaceMetadata: Pick<FrontendWorkspaceMetadata, "projects"> | null | undefined;
projectPath: string;
subProjectPathspec?: string | null;
pathContext?: ProjectRelativePathContext;
}): ReviewDiffPathFilterSpec[] {
if (!params.assistedOnly) {
const selectedFileActive =
params.selectedFilePath &&
!params.isImmersive &&
(params.subProjectPathspec == null ||
reviewPathMatchesPathspec(params.selectedDiffPath, params.subProjectPathspec));
const subProjectPathFilter = getReviewSubProjectPathFilter(params.subProjectPathspec ?? null);
return [
{
repoRootProjectPath:
params.selectedFilePath && !params.isImmersive
? (params.selectedRepoRootProjectPath ?? params.projectPath)
: params.projectPath,
pathFilter:
params.selectedFilePath && !params.isImmersive
? toPathFilter([params.selectedDiffPath])
: "",
selectedFilePath:
params.selectedFilePath && !params.isImmersive ? params.selectedFilePath : null,
repoRootProjectPath: selectedFileActive
? (params.selectedRepoRootProjectPath ?? params.projectPath)
: params.projectPath,
pathFilter: selectedFileActive
? toPathFilter([params.selectedDiffPath])
: subProjectPathFilter,
selectedFilePath: selectedFileActive ? params.selectedFilePath : null,
},
];
}
Expand Down Expand Up @@ -549,6 +585,7 @@ export function buildReviewDiffPathFilter(params: {
selectedDiffPath: string;
workspaceMetadata: Pick<FrontendWorkspaceMetadata, "projects"> | null | undefined;
repoRootProjectPath: string | null | undefined;
subProjectPathspec?: string | null;
pathContext?: ProjectRelativePathContext;
}): string {
return (
Expand Down Expand Up @@ -783,6 +820,13 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
const originFetchRef = useRef<OriginFetchState | null>(null);
const { api } = useAPI();
const { workspaceMetadata } = useWorkspaceMetadata();
const workspaceReviewMetadata = workspaceMetadata.get(workspaceId);
const subProjectPathspec = getReviewSubProjectPathspec({
workspaceMetadata: workspaceReviewMetadata,
projectPath,
});
const subProjectPathFilter = getReviewSubProjectPathFilter(subProjectPathspec);

const panelRef = useRef<HTMLDivElement>(null);
const scrollContainerRef = useRef<HTMLDivElement>(null);
const searchInputRef = useRef<HTMLInputElement>(null);
Expand Down Expand Up @@ -834,16 +878,28 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
);

const selectedRepoRootProjectPath = resolveRepoRootProjectPath(
workspaceMetadata.get(workspaceId),
workspaceReviewMetadata,
selectedFilePath
);

const selectedDiffPath = normalizeRepoRootFilePath(
workspaceMetadata.get(workspaceId),
workspaceReviewMetadata,
selectedFilePath ? extractNewPath(selectedFilePath) : null,
selectedRepoRootProjectPath
);

useEffect(() => {
if (!subProjectPathspec || selectedFilePath === null) {
return;
}

if (!reviewPathMatchesPathspec(selectedDiffPath, subProjectPathspec)) {
// Sub-project review should not show a stale parent selection; Assisted
// mode remains the explicit escape hatch for agent-pinned hunks outside this scope.
setSelectedFilePath(null);
}
}, [selectedDiffPath, selectedFilePath, setSelectedFilePath, subProjectPathspec]);

const projectDefaultBaseKey = STORAGE_KEYS.reviewDefaultBase(projectPath);
const workspaceDiffBaseKey = STORAGE_KEYS.reviewDiffBase(workspaceId);

Expand Down Expand Up @@ -1068,7 +1124,6 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
rawWorkspaceStore.getAssistedReviewHunks(workspaceId)
);

const workspaceReviewMetadata = workspaceMetadata.get(workspaceId);
const reviewPathContext = useMemo(
() => getReviewPanelPathContext({ workspaceMetadata: workspaceReviewMetadata, projectPath }),
[projectPath, workspaceReviewMetadata]
Expand Down Expand Up @@ -1306,14 +1361,14 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
const numstatCommand = buildGitDiffCommand(
filters.diffBase,
filters.includeUncommitted,
"", // No path filter for file tree
subProjectPathFilter,
"numstat"
);

const nameStatusCommand = buildGitDiffCommand(
filters.diffBase,
filters.includeUncommitted,
"", // No path filter for file tree
subProjectPathFilter,
"name-status"
);

Expand Down Expand Up @@ -1446,6 +1501,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
workspacePath,
projectPath,
workspaceMetadata,
subProjectPathFilter,
filters.diffBase,
filters.includeUncommitted,
refreshTrigger,
Expand Down Expand Up @@ -1474,8 +1530,9 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
selectedFilePath,
selectedDiffPath,
selectedRepoRootProjectPath,
workspaceMetadata: workspaceMetadata.get(workspaceId),
workspaceMetadata: workspaceReviewMetadata,
projectPath,
subProjectPathspec,
pathContext: reviewPathContext,
}).map((spec) => {
const diffCommand = buildGitDiffCommand(
Expand Down Expand Up @@ -1626,6 +1683,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
workspacePath,
projectPath,
workspaceMetadata,
workspaceReviewMetadata,
subProjectPathspec,
filters.diffBase,
filters.includeUncommitted,
filters.assistedOnly,
Expand Down Expand Up @@ -2349,6 +2408,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
workspaceId={workspaceId}
workspacePath={workspacePath}
refreshTrigger={refreshTrigger}
repoRootProjectPath={projectPath}
pathFilter={subProjectPathFilter}
onRefresh={handleRefresh}
/>

Expand Down Expand Up @@ -2438,9 +2499,19 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
<div className="text-muted flex flex-col items-center justify-start gap-3 px-6 pt-12 pb-6 text-center">
<div className="text-foreground text-base font-medium">No changes found</div>
<div className="text-[13px] leading-[1.5]">
No changes found for the selected diff base.
<br />
Try selecting a different base or make some changes.
{subProjectPathspec ? (
<>
No changes found in this sub-project for the selected diff base.
<br />
Assisted pins outside this scope still appear in Assisted mode.
</>
) : (
<>
No changes found for the selected diff base.
<br />
Try selecting a different base or make some changes.
</>
)}
</div>
{diagnosticInfo && (
<details className="bg-modal-bg border-border-light [&_summary]:text-muted mt-4 w-full max-w-96 cursor-pointer rounded border p-3 [&_summary]:flex [&_summary]:list-none [&_summary]:items-center [&_summary]:gap-1.5 [&_summary]:text-xs [&_summary]:font-medium [&_summary]:select-none [&_summary::-webkit-details-marker]:hidden [&_summary::before]:text-[10px] [&_summary::before]:transition-transform [&_summary::before]:duration-200 [&_summary::before]:content-['â–¶'] [&[open]_summary::before]:rotate-90">
Expand Down
Loading
Loading