feat(web): redesign review panels and supporting pages

apps/server/src/pullRequest/PullRequestService.ts

@@ -118,6 +118,20 @@ function supersedesReading(current: PullRequestReading | undefined, next: PullRe
   return (next.observedAt ?? -Infinity) >= (current.observedAt ?? -Infinity);
 }
 
+// Retry a shared flight canceled by its previous reader after that flight leaves the cache.
+const getShared = Effect.fn("PullRequestService.getShared")(
+  function* <Key, A, E, R>(cache: Cache.Cache<Key, A, E, R>, key: Key) {
+    return yield* Cache.get(cache, key);
+  },
+  (effect, cache, key) =>
+    effect.pipe(
+      Effect.catchCauseIf(
+        (cause) => Cause.hasInterruptsOnly(cause),
+        (_cause) => Cache.get(cache, key),
+      ),
+    ),
+);
+
 // Providers request one extra row; 99 stays within GitHub/GitLab's 100-row API limit.
 const DEFAULT_REPOSITORY_LIST_LIMIT = 99;
 const REPOSITORY_CONCURRENCY = 12;
@@ -1236,10 +1250,10 @@ export const make = Effect.gen(function* () {
           const roots =
             viewerRoots.get(host) ?? forHost.map(({ project }) => project.workspaceRoot);
           const key = JSON.stringify([host, api.kind, [...new Set(roots)].sort()]);
-          if (options?.allowPaused === true) return Cache.get(viewerFlights, key);
+          if (options?.allowPaused === true) return getShared(viewerFlights, key);
           // Check pauses outside the shared lookup so background reads cannot spawn repeated failing CLIs and extend the pause.
           return rateLimits.check({ provider: api.kind, host }).pipe(
-            Effect.flatMap(() => Cache.get(viewerFlights, key)),
+            Effect.flatMap(() => getShared(viewerFlights, key)),
             Effect.catch((error) =>
               Effect.succeed<ResolvedViewer>({
                 host,
@@ -2003,6 +2017,9 @@ export const make = Effect.gen(function* () {
                 : { overviewThreads: activity.overviewThreads }),
               commits: activity.commits,
               ...(activity.reactions === undefined ? {} : { reactions: activity.reactions }),
+              ...(activity.reviewRequests === undefined
+                ? {}
+                : { reviewRequests: activity.reviewRequests }),
             })),
           ),
       ),
@@ -3213,13 +3230,7 @@ export const make = Effect.gen(function* () {
         ? null
         : Object.entries(input.cursors).toSorted(([left], [right]) => left.localeCompare(right)),
     ]);
-    // A replacement reader can join a lookup still finishing its previous reader's cancellation.
-    return Cache.get(listCache, key).pipe(
-      Effect.catchCauseIf(
-        (cause) => Cause.hasInterruptsOnly(cause),
-        (_cause) => Cache.get(listCache, key),
-      ),
-    );
+    return getShared(listCache, key);
   };
 
   const checksCache = yield* Cache.makeWith(
@@ -3428,7 +3439,7 @@ export const make = Effect.gen(function* () {
   const detail: PullRequestService["Service"]["detail"] = (input) => {
     const key = refCacheKey(input);
     // Skip reads older than strict summaries; caching them would preserve regressions without asking the host again.
-    const read = Cache.get(detailCache, key).pipe(
+    const read = getShared(detailCache, key).pipe(
       Effect.tap((value) => {
         if (value.state === "merged") noteMerged(input);
         const summary = summaryFromDetail(value, lastGoodSummary.peek(key));
@@ -3498,15 +3509,15 @@ export const make = Effect.gen(function* () {
     return Cache.getSuccess(detailCache, key).pipe(
       Effect.flatMap(
         Option.match({
-          onNone: () => Cache.get(previewCache, key),
+          onNone: () => getShared(previewCache, key),
           onSome: (detail) => Effect.succeed(previewFields(detail)),
         }),
       ),
     );
   };
   const activity: PullRequestService["Service"]["activity"] = (input) => {
     const key = refCacheKey(input);
-    return Cache.get(activityCache, key);
+    return getShared(activityCache, key);
   };
 
   const diffCache = yield* Cache.makeWith(
@@ -3588,7 +3599,7 @@ export const make = Effect.gen(function* () {
       ),
       Effect.uninterruptible,
     );
-    const read = Cache.get(diffCache, key).pipe(
+    const read = getShared(diffCache, key).pipe(
       Effect.ensuring(evictDiffs),
       Effect.ensuring(Effect.sync(armDiffSweep)),
     );
@@ -3613,7 +3624,7 @@ export const make = Effect.gen(function* () {
   const filesViewed: PullRequestService["Service"]["filesViewed"] = (input) =>
     canonicalRef(input).pipe(
       Effect.flatMap((ref) =>
-        Cache.get(filesViewedCache, JSON.stringify([refCacheKey(ref), filesViewedEpoch(ref)])),
+        getShared(filesViewedCache, JSON.stringify([refCacheKey(ref), filesViewedEpoch(ref)])),
       ),
     );
 
@@ -3660,12 +3671,7 @@ export const make = Effect.gen(function* () {
     }
     if (missing.size === 0) return { stats: held };
     const key = statsBatchKey(missing.values());
-    const { result, at } = yield* Cache.get(listStatsCache, key).pipe(
-      Effect.catchCauseIf(
-        (cause) => Cause.hasInterruptsOnly(cause),
-        (_cause) => Cache.get(listStatsCache, key),
-      ),
-    );
+    const { result, at } = yield* getShared(listStatsCache, key);
     for (const [key, ref] of missing) {
       const stat = result.stats.find(
         (stat) =>
@@ -3952,9 +3958,9 @@ export const make = Effect.gen(function* () {
     notifyLinkedThreadsChanged,
     refreshAfterTurn,
     detail: credentialCached(detail),
-    checks: credentialCached((input) => Cache.get(checksCache, refCacheKey(input))),
+    checks: credentialCached((input) => getShared(checksCache, refCacheKey(input))),
     watchFingerprint: credentialCached((input) =>
-      Cache.get(watchFingerprintCache, refCacheKey(input)),
+      getShared(watchFingerprintCache, refCacheKey(input)),
     ),
     activity: credentialCached(activity),
     changes,

apps/server/src/vcs/GitVcsDriverCore.test.ts

@@ -5122,6 +5122,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
           path: "HEAD",
           insertions: 1,
           deletions: 0,
+          status: "modified",
         });
       }),
     );
@@ -5142,9 +5143,9 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(cwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "café notes.md", insertions: 1, deletions: 0 },
-          { path: "src/new.ts", insertions: 1, deletions: 0 },
-          { path: "src/old.ts", insertions: 0, deletions: 0 },
+          { path: "café notes.md", insertions: 1, deletions: 0, status: "modified" },
+          { path: "src/new.ts", insertions: 1, deletions: 0, status: "renamed" },
+          { path: "src/old.ts", insertions: 0, deletions: 0, status: "deleted" },
         ]);
         yield* driver.commit(cwd, "Commit selection", "", {
           stage: { filePaths: status.workingTree.files.map((file) => file.path) },
@@ -5172,9 +5173,9 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(nestedCwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "nested/new.txt", insertions: 0, deletions: 0 },
-          { path: "nested/tracked.txt", insertions: 1, deletions: 0 },
-          { path: "README.md", insertions: 1, deletions: 0 },
+          { path: "nested/new.txt", insertions: 0, deletions: 0, status: "untracked" },
+          { path: "nested/tracked.txt", insertions: 1, deletions: 0, status: "modified" },
+          { path: "README.md", insertions: 1, deletions: 0, status: "modified" },
         ]);
         // The root is what the paths above are relative to, not the nested cwd, and it keeps
         // the trailing space so the resolved editor target is the file that exists.
@@ -5207,8 +5208,8 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(nestedCwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "nested/new.txt", insertions: 0, deletions: 0 },
-          { path: "nested/tracked.txt", insertions: 1, deletions: 0 },
+          { path: "nested/new.txt", insertions: 0, deletions: 0, status: "untracked" },
+          { path: "nested/tracked.txt", insertions: 1, deletions: 0, status: "modified" },
         ]);
         assert.equal(
           NodeFS.realpathSync.native(status.workingTree.rootPath!),
@@ -5217,6 +5218,54 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
       }),
     );
 
+    it.effect("reports how each working-tree file changed", () =>
+      Effect.gen(function* () {
+        const cwd = yield* makeTmpDir();
+        yield* initRepoWithCommit(cwd);
+        yield* writeTextFile(cwd, "keep.ts", "keep\n");
+        yield* writeTextFile(cwd, "remove.ts", "remove\n");
+        yield* writeTextFile(cwd, "old name.ts", "rename me\n");
+        yield* git(cwd, ["add", "."]);
+        yield* git(cwd, ["commit", "-m", "add files"]);
+        yield* writeTextFile(cwd, "keep.ts", "keep\nedited\n");
+        yield* git(cwd, ["rm", "-q", "remove.ts"]);
+        yield* git(cwd, ["mv", "old name.ts", "new name.ts"]);
+        yield* writeTextFile(cwd, "staged.ts", "staged\n");
+        yield* git(cwd, ["add", "staged.ts"]);
+        yield* writeTextFile(cwd, "untracked.ts", "untracked\n");
+        const blob = yield* git(cwd, ["rev-parse", "HEAD:keep.ts"]);
+        const conflicts = [
+          ["AU", [2]],
+          ["UD", [1, 2]],
+          ["UA", [3]],
+          ["DU", [1, 3]],
+          ["AA", [2, 3]],
+          ["UU", [1, 2, 3]],
+        ] as const;
+        for (const [xy, stages] of conflicts) {
+          yield* writeTextFile(cwd, `${xy}.ts`, "conflicted\n");
+          yield* (yield* GitVcsDriver.GitVcsDriver).execute({
+            operation: "test.unmergedIndex",
+            cwd,
+            args: ["update-index", "--index-info"],
+            stdin: stages.map((stage) => `100644 ${blob} ${stage}\t${xy}.ts\n`).join(""),
+          });
+        }
+
+        const status = yield* (yield* GitVcsDriver.GitVcsDriver).statusDetails(cwd);
+        const statuses = Object.fromEntries(
+          status.workingTree.files.map((file) => [file.path, file.status]),
+        );
+
+        assert.equal(statuses["keep.ts"], "modified");
+        assert.equal(statuses["remove.ts"], "deleted");
+        assert.equal(statuses["new name.ts"], "renamed");
+        assert.equal(statuses["staged.ts"], "added");
+        assert.equal(statuses["untracked.ts"], "untracked");
+        for (const [xy] of conflicts) assert.equal(statuses[`${xy}.ts`], "modified", xy);
+      }),
+    );
+
     it.effect("reports default-branch delta separately from upstream delta", () =>
       Effect.gen(function* () {
         const cwd = yield* makeTmpDir();
@@ -5711,7 +5760,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
 
         assert.equal(status.isRepo, true);
         assert.deepStrictEqual(status.workingTree.files, [
-          { path: "initial.ts", insertions: 1, deletions: 0 },
+          { path: "initial.ts", insertions: 1, deletions: 0, status: "added" },
         ]);
       }),
     );

apps/server/src/vcs/GitVcsDriverCore.ts

@@ -35,6 +35,7 @@ import {
   type ReviewDiffFileStat,
   type ReviewDiffPreviewSource,
   type VcsRef,
+  type VcsWorkingTreeFileStatus,
 } from "@t3tools/contracts";
 import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git";
 import * as HostProcess from "@t3tools/shared/HostProcess";
@@ -276,6 +277,13 @@ function parseReviewNumstat(stdout: string): ReviewDiffFileStat[] {
   return files;
 }
 
+function withFileStatus<T extends { path: string }>(
+  file: T,
+  status: VcsWorkingTreeFileStatus | null | undefined,
+): T & { status?: VcsWorkingTreeFileStatus } {
+  return status ? { ...file, status } : file;
+}
+
 // Space-separated fields precede the path in each porcelain v2 record type. The path itself
 // may contain spaces, so it is everything after them.
 const PORCELAIN_V2_FIELDS_BEFORE_PATH: Record<string, number> = {
@@ -293,6 +301,30 @@ function parsePorcelainPath(record: string): string | null {
   return filePath.length > 0 ? filePath : null;
 }
 
+function parsePorcelainEntry(
+  record: string,
+): { path: string; status: VcsWorkingTreeFileStatus | null } | null {
+  const path = parsePorcelainPath(record);
+  if (path === null) return null;
+  const recordType = record[0];
+  const xy = record.split(" ")[1] ?? "";
+  const status: VcsWorkingTreeFileStatus | null =
+    recordType === "?"
+      ? "untracked"
+      : recordType === "!"
+        ? null
+        : recordType === "2"
+          ? "renamed"
+          : recordType === "u"
+            ? "modified"
+            : xy[1] === "D" || xy === "D."
+              ? "deleted"
+              : xy[0] === "A" || xy[1] === "A"
+                ? "added"
+                : "modified";
+  return { path, status };
+}
+
 function filterBranchesForListQuery(
   refs: ReadonlyArray<VcsRef>,
   query?: string,
@@ -2488,7 +2520,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
     let behindCount = 0;
     let aheadOfDefaultCount = 0;
     let hasWorkingTreeChanges = false;
-    const changedFilesWithoutNumstat = new Set<string>();
+    const changedFileStatuses = new Map<string, VcsWorkingTreeFileStatus | null>();
     const untrackedRoots: string[] = [];
 
     const statusRecords = statusStdout.split("\0");
@@ -2520,16 +2552,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
       }
       if (line.length > 0 && !line.startsWith("#")) {
         hasWorkingTreeChanges = true;
-        const pathValue = parsePorcelainPath(line);
-        if (pathValue) {
-          changedFilesWithoutNumstat.add(pathValue);
-          if (line.startsWith("? ")) untrackedRoots.push(pathValue);
+        const entry = parsePorcelainEntry(line);
+        if (entry) {
+          changedFileStatuses.set(entry.path, entry.status);
+          if (line.startsWith("? ")) untrackedRoots.push(entry.path);
         }
-        // A rename record is followed by its original path as a separate record. Listing it
-        // keeps the deletion selectable when committing only some files.
+        // Rename records carry a second NUL-delimited original path, selectable for partial commits.
         if (line.startsWith("2 ")) {
           const originalPath = statusRecords[++index];
-          if (originalPath) changedFilesWithoutNumstat.add(originalPath);
+          if (originalPath && !changedFileStatuses.has(originalPath))
+            changedFileStatuses.set(
+              originalPath,
+              line.split(" ")[1]?.includes("R") ? "deleted" : null,
+            );
         }
       }
     }
@@ -2580,12 +2615,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
     const files = Array.from(fileStatMap.entries()).map(([filePath, stat]) => {
       insertions += stat.insertions;
       deletions += stat.deletions;
-      return { path: filePath, insertions: stat.insertions, deletions: stat.deletions };
+      return withFileStatus(
+        { path: filePath, insertions: stat.insertions, deletions: stat.deletions },
+        changedFileStatuses.get(filePath),
+      );
     });
 
-    for (const filePath of changedFilesWithoutNumstat) {
+    for (const [filePath, status] of changedFileStatuses) {
       if (fileStatMap.has(filePath)) continue;
-      files.push({ path: filePath, insertions: 0, deletions: 0 });
+      files.push(withFileStatus({ path: filePath, insertions: 0, deletions: 0 }, status));
     }
     files.sort((a, b) => a.path.localeCompare(b.path));