perf(azure-devops): pull request Code tab shows its first files in about 5 s

packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts

@@ -40,6 +40,18 @@ function outputWithin(maxOutputBytes: number | undefined, response: string) {
 
 const json = (value: Record<string, unknown>) => JSON.stringify(value);
 
+/** The `az` call for a files diff, when the organization does not offer one. */
+const filesDiffRefused = () =>
+  Effect.fail(
+    new AzureDevOpsCli.AzureDevOpsCommandFailedError({
+      operation: "execute",
+      command: "az",
+      cwd: "/w",
+      argumentCount: 1,
+      cause: "the files diff is not offered here",
+    }),
+  );
+
 const pullRequestRow = {
   pullRequestId: 42,
   title: "Add the page",
@@ -594,6 +606,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
       mockedExecute
         .mockReturnValueOnce(Effect.succeed(output(json(pullRequestRow))))
         .mockReturnValueOnce(Effect.succeed(output(json(oneIteration))))
+        .mockReturnValueOnce(filesDiffRefused())
         .mockReturnValueOnce(
           Effect.succeed(
             output(
@@ -638,6 +651,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
       mockedExecute
         .mockReturnValueOnce(Effect.succeed(output(json(pullRequestRow))))
         .mockReturnValueOnce(Effect.succeed(output(json(oneIteration))))
+        .mockReturnValueOnce(filesDiffRefused())
         .mockReturnValueOnce(
           Effect.succeed(
             output(
@@ -704,6 +718,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
             ),
           ),
         )
+        .mockReturnValueOnce(filesDiffRefused())
         .mockReturnValueOnce(
           Effect.succeed(
             output(
@@ -729,7 +744,7 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
         number: 42,
       });
 
-      expect(argsOfCall(3)).toContain("includeContentMetadata=true");
+      expect(argsOfCall(4)).toContain("includeContentMetadata=true");
       expect(slice.patch).toContain("Binary files a/logo.png and b/logo.png differ");
       assert.isTrue(slice.truncated);
     }),
@@ -868,6 +883,101 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
     }),
   );
 
+  it.effect("asks for a page of files with their diffs through the preview route", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValueOnce(
+        Effect.succeed(
+          output(
+            // `to_string(@)`: the page's JSON as a string.
+            JSON.stringify(
+              json({
+                fileDiffs: [{ path: "/a.ts", originalPath: "/a.ts", changeType: "edit" }],
+              }),
+            ),
+          ),
+        ),
+      );
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      const files = yield* cli.listFilesDiff({
+        cwd: "/w",
+        location: { project: "platform", repository: "web" },
+        number: 42,
+        top: 50,
+        skip: 100,
+      });
+
+      assert.deepStrictEqual(argsOfCall(0), [
+        "devops",
+        "invoke",
+        "--detect",
+        "true",
+        "--area",
+        "git",
+        "--resource",
+        "pullRequestFilesDiff",
+        "--api-version",
+        "7.2-preview",
+        "--route-parameters",
+        "project=platform",
+        "repositoryId=web",
+        "pullRequestId=42",
+        "--query-parameters",
+        "$top=50",
+        "$skip=100",
+        "--query",
+        "to_string(@)",
+        "--only-show-errors",
+        "--output",
+        "json",
+      ]);
+      assert.deepStrictEqual(
+        files.map((file) => file.change.path),
+        ["a.ts"],
+      );
+    }),
+  );
+
+  it.effect("reads a page too large for its output limit as unreadable rather than short", () =>
+    Effect.gen(function* () {
+      const response = JSON.stringify(
+        json({
+          fileDiffs: Array.from({ length: 50 }, (_, index) => ({
+            path: `/file-${index}.ts`,
+            originalPath: `/file-${index}.ts`,
+            changeType: "add",
+            lineDiffBlocks: [
+              {
+                changeType: "add",
+                originalLineNumberStart: 0,
+                originalLinesCount: 0,
+                modifiedLineNumberStart: 1,
+                modifiedLinesCount: 1,
+                modifiedLines: ["x".repeat(400_000)],
+              },
+            ],
+          })),
+        }),
+      );
+      mockedExecute.mockImplementationOnce((input) =>
+        Effect.succeed(outputWithin(input.maxOutputBytes, response)),
+      );
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      const error = yield* Effect.flip(
+        cli.listFilesDiff({
+          cwd: "/w",
+          location: { project: "platform", repository: "web" },
+          number: 42,
+          top: 50,
+          skip: 0,
+        }),
+      );
+
+      assert.strictEqual(error._tag, "AzureDevOpsPullRequestReadError");
+    }),
+  );
+
   it.effect("reports a pull request it cannot place as its own outcome", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(
@@ -924,8 +1034,27 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
       });
 
       expect(statuses.map((status) => status.description)).toEqual(["Build"]);
-      // A status carries every identity Azure knows of it, which is most of a large response.
-      expect(argsOfCall(0)).toContain("--query");
+      expect(argsOfCall(0)).toEqual([
+        "devops",
+        "invoke",
+        "--detect",
+        "true",
+        "--area",
+        "git",
+        "--resource",
+        "pullRequestStatuses",
+        "--api-version",
+        "7.1",
+        "--route-parameters",
+        "project=platform",
+        "repositoryId=web",
+        "pullRequestId=42",
+        "--query",
+        "{value:value[].{id:id,state:state,description:description,targetUrl:targetUrl,context:context}}",
+        "--only-show-errors",
+        "--output",
+        "json",
+      ]);
     }),
   );
 

packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts

@@ -13,6 +13,7 @@ import type {
 
 import * as AzureDevOpsCli from "./AzureDevOpsCli.ts";
 import {
+  decodeFilesDiffJson,
   decodeItemContentJson,
   decodeIterationChangesJson,
   decodeIterationsJson,
@@ -23,6 +24,7 @@ import {
   decodeThreadsJson,
   decodeViewerJson,
   type AzureDevOpsChangeEntry,
+  type AzureDevOpsFileDiff,
   type AzureDevOpsItemContent,
   type AzureDevOpsIteration,
   type AzureDevOpsPolicyEvaluation,
@@ -107,7 +109,14 @@ export type AzureDevOpsPullRequestCliError =
   | AzureDevOpsViewerUnavailableError;
 
 const REST_API_VERSION = "7.1";
+/**
+ * The pull request files diff is registered as a preview. The extension rejects `7.2-preview.1`
+ * (it parses the version as a float) and Azure rejects a plain `7.2`, so only this spelling works.
+ */
+const FILES_DIFF_API_VERSION = "7.2-preview";
 const PULL_REQUEST_LIST_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
+/** A page of diffs carries line text, which `az` prints with the escapes of a JSON string. */
+const FILES_DIFF_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
 /** A 2,000-file page exceeds the default 1 MiB output budget. */
 const CHANGE_ENTRIES_MAX_OUTPUT_BYTES = 8 * 1024 * 1024;
 /** JSON escaping and base64 can exceed the decoded file's size. */
@@ -207,6 +216,15 @@ export class AzureDevOpsPullRequestCli extends Context.Service<
       readonly commit: string;
     }) => Effect.Effect<AzureDevOpsItemContent, AzureDevOpsPullRequestCliError>;
 
+    /** The latest iteration's files and diff blocks in one call, `top` files from `skip`. */
+    readonly listFilesDiff: (input: {
+      readonly cwd: string;
+      readonly location: AzureDevOpsRepositoryLocation;
+      readonly number: number;
+      readonly top: number;
+      readonly skip: number;
+    }) => Effect.Effect<ReadonlyArray<AzureDevOpsFileDiff>, AzureDevOpsPullRequestCliError>;
+
     readonly runPullRequestAction: (input: {
       readonly cwd: string;
       readonly number: number;
@@ -319,11 +337,12 @@ export const make = Effect.gen(function* () {
     readonly cwd: string;
     readonly operation: string;
     readonly resource: string;
+    readonly apiVersion?: string;
     readonly routeParameters: ReadonlyArray<string>;
     readonly queryParameters?: ReadonlyArray<string>;
+    /** A JMESPath query applied to the result before it is printed. */
+    readonly query?: string;
     readonly maxOutputBytes?: number;
-    /** A JMESPath projection `az` applies before printing, to keep large responses small. */
-    readonly jmesQuery?: string;
     readonly decode: (raw: string) => Result.Result<A, unknown>;
   }): Effect.Effect<A, AzureDevOpsPullRequestCliError> =>
     executeJson({
@@ -338,13 +357,13 @@ export const make = Effect.gen(function* () {
         "--resource",
         input.resource,
         "--api-version",
-        REST_API_VERSION,
+        input.apiVersion ?? REST_API_VERSION,
         "--route-parameters",
         ...input.routeParameters,
         ...(input.queryParameters === undefined
           ? []
           : ["--query-parameters", ...input.queryParameters]),
-        ...(input.jmesQuery === undefined ? [] : ["--query", input.jmesQuery]),
+        ...(input.query === undefined ? [] : ["--query", input.query]),
       ],
     }).pipe(
       Effect.flatMap((result) => {
@@ -604,7 +623,7 @@ export const make = Effect.gen(function* () {
           maxOutputBytes: REVIEW_HISTORY_MAX_OUTPUT_BYTES,
           // Each status carries its creator's profile and links, several times the fields read
           // here, which costs `az` seconds to print on a busy pull request.
-          jmesQuery: PULL_REQUEST_STATUS_FIELDS,
+          query: PULL_REQUEST_STATUS_FIELDS,
           decode: decodePullRequestStatusesJson,
         });
       },
@@ -683,6 +702,21 @@ export const make = Effect.gen(function* () {
       });
     }),
 
+    listFilesDiff: Effect.fn("AzureDevOpsPullRequestCli.listFilesDiff")(function (input) {
+      return invoke({
+        cwd: input.cwd,
+        operation: "listFilesDiff",
+        resource: "pullRequestFilesDiff",
+        apiVersion: FILES_DIFF_API_VERSION,
+        routeParameters: pullRequestRoute(input),
+        queryParameters: [`$top=${input.top}`, `$skip=${input.skip}`],
+        // The page as a string of JSON text, which `az` prints in ASCII whatever the code page.
+        query: "to_string(@)",
+        maxOutputBytes: FILES_DIFF_MAX_OUTPUT_BYTES,
+        decode: decodeFilesDiffJson,
+      });
+    }),
+
     setPullRequestReviewers: Effect.fn("AzureDevOpsPullRequestCli.setPullRequestReviewers")(
       function* (input) {
         if (input.reviewers.some((reviewer) => !isReviewerName(reviewer))) {