Merge pull request #2639 from github/robertbrignull/data_progress

Remove custom progress bar and use normal VS Code notification for progress

extensions/ql-vscode/src/common/interface-types.ts

@@ -500,13 +500,6 @@ interface SetExternalApiUsagesMessage {
   externalApiUsages: ExternalApiUsage[];
 }
 
-export interface ShowProgressMessage {
-  t: "showProgress";
-  step: number;
-  maxStep: number;
-  message: string;
-}
-
 interface LoadModeledMethodsMessage {
   t: "loadModeledMethods";
   modeledMethods: Record<string, ModeledMethod>;
@@ -558,7 +551,6 @@ interface ModelDependencyMessage {
 export type ToDataExtensionsEditorMessage =
   | SetExtensionPackStateMessage
   | SetExternalApiUsagesMessage
-  | ShowProgressMessage
   | LoadModeledMethodsMessage
   | AddModeledMethodsMessage;
 

extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts

@@ -14,11 +14,7 @@ import {
   FromDataExtensionsEditorMessage,
   ToDataExtensionsEditorMessage,
 } from "../common/interface-types";
-import {
-  ProgressCallback,
-  ProgressUpdate,
-  withProgress,
-} from "../common/vscode/progress";
+import { ProgressCallback, withProgress } from "../common/vscode/progress";
 import { QueryRunner } from "../query-server";
 import {
   showAndLogExceptionWithTelemetry,
@@ -225,203 +221,205 @@ export class DataExtensionsEditorView extends AbstractWebview<
   }
 
   protected async loadExternalApiUsages(): Promise<void> {
-    const cancellationTokenSource = new CancellationTokenSource();
+    await withProgress(
+      async (progress) => {
+        try {
+          const cancellationTokenSource = new CancellationTokenSource();
+          const queryResult = await runQuery(
+            this.mode === Mode.Framework
+              ? "frameworkModeQuery"
+              : "applicationModeQuery",
+            {
+              cliServer: this.cliServer,
+              queryRunner: this.queryRunner,
+              databaseItem: this.databaseItem,
+              queryStorageDir: this.queryStorageDir,
+              progress: (update) => progress({ ...update, maxStep: 1500 }),
+              token: cancellationTokenSource.token,
+            },
+          );
+          if (!queryResult) {
+            return;
+          }
 
-    try {
-      const queryResult = await runQuery(
-        this.mode === Mode.Framework
-          ? "frameworkModeQuery"
-          : "applicationModeQuery",
-        {
-          cliServer: this.cliServer,
-          queryRunner: this.queryRunner,
-          databaseItem: this.databaseItem,
-          queryStorageDir: this.queryStorageDir,
-          progress: (progressUpdate: ProgressUpdate) => {
-            void this.showProgress(progressUpdate, 1500);
-          },
-          token: cancellationTokenSource.token,
-        },
-      );
-      if (!queryResult) {
-        await this.clearProgress();
-        return;
-      }
+          progress({
+            message: "Decoding results",
+            step: 1100,
+            maxStep: 1500,
+          });
 
-      await this.showProgress({
-        message: "Decoding results",
-        step: 1100,
-        maxStep: 1500,
-      });
+          const bqrsChunk = await readQueryResults({
+            cliServer: this.cliServer,
+            bqrsPath: queryResult.outputDir.bqrsPath,
+          });
+          if (!bqrsChunk) {
+            return;
+          }
 
-      const bqrsChunk = await readQueryResults({
-        cliServer: this.cliServer,
-        bqrsPath: queryResult.outputDir.bqrsPath,
-      });
-      if (!bqrsChunk) {
-        await this.clearProgress();
-        return;
-      }
+          progress({
+            message: "Finalizing results",
+            step: 1450,
+            maxStep: 1500,
+          });
 
-      await this.showProgress({
-        message: "Finalizing results",
-        step: 1450,
-        maxStep: 1500,
-      });
+          const externalApiUsages = decodeBqrsToExternalApiUsages(bqrsChunk);
 
-      const externalApiUsages = decodeBqrsToExternalApiUsages(bqrsChunk);
-
-      await this.postMessage({
-        t: "setExternalApiUsages",
-        externalApiUsages,
-      });
-
-      await this.clearProgress();
-    } catch (err) {
-      void showAndLogExceptionWithTelemetry(
-        this.app.logger,
-        this.app.telemetry,
-        redactableError(
-          asError(err),
-        )`Failed to load external API usages: ${getErrorMessage(err)}`,
-      );
-    }
+          await this.postMessage({
+            t: "setExternalApiUsages",
+            externalApiUsages,
+          });
+        } catch (err) {
+          void showAndLogExceptionWithTelemetry(
+            this.app.logger,
+            this.app.telemetry,
+            redactableError(
+              asError(err),
+            )`Failed to load external API usages: ${getErrorMessage(err)}`,
+          );
+        }
+      },
+      { cancellable: false },
+    );
   }
 
   protected async generateModeledMethods(): Promise<void> {
-    const tokenSource = new CancellationTokenSource();
+    await withProgress(
+      async (progress) => {
+        const tokenSource = new CancellationTokenSource();
 
-    let addedDatabase: DatabaseItem | undefined;
+        let addedDatabase: DatabaseItem | undefined;
 
-    // In application mode, we need the database of a specific library to generate
-    // the modeled methods. In framework mode, we'll use the current database.
-    if (this.mode === Mode.Application) {
-      addedDatabase = await this.promptImportDatabase((update) =>
-        this.showProgress(update),
-      );
-      if (!addedDatabase) {
-        return;
-      }
-    }
-
-    await this.showProgress({
-      step: 0,
-      maxStep: 4000,
-      message: "Generating modeled methods for library",
-    });
-
-    try {
-      await generateFlowModel({
-        cliServer: this.cliServer,
-        queryRunner: this.queryRunner,
-        queryStorageDir: this.queryStorageDir,
-        databaseItem: addedDatabase ?? this.databaseItem,
-        onResults: async (modeledMethods) => {
-          const modeledMethodsByName: Record<string, ModeledMethod> = {};
-
-          for (const modeledMethod of modeledMethods) {
-            modeledMethodsByName[modeledMethod.signature] = modeledMethod;
+        // In application mode, we need the database of a specific library to generate
+        // the modeled methods. In framework mode, we'll use the current database.
+        if (this.mode === Mode.Application) {
+          addedDatabase = await this.promptImportDatabase(progress);
+          if (!addedDatabase) {
+            return;
           }
+        }
 
-          await this.postMessage({
-            t: "addModeledMethods",
-            modeledMethods: modeledMethodsByName,
+        progress({
+          step: 0,
+          maxStep: 4000,
+          message: "Generating modeled methods for library",
+        });
+
+        try {
+          await generateFlowModel({
+            cliServer: this.cliServer,
+            queryRunner: this.queryRunner,
+            queryStorageDir: this.queryStorageDir,
+            databaseItem: addedDatabase ?? this.databaseItem,
+            onResults: async (modeledMethods) => {
+              const modeledMethodsByName: Record<string, ModeledMethod> = {};
+
+              for (const modeledMethod of modeledMethods) {
+                modeledMethodsByName[modeledMethod.signature] = modeledMethod;
+              }
+
+              await this.postMessage({
+                t: "addModeledMethods",
+                modeledMethods: modeledMethodsByName,
+              });
+            },
+            progress,
+            token: tokenSource.token,
           });
-        },
-        progress: (update) => this.showProgress(update),
-        token: tokenSource.token,
-      });
-    } catch (e: unknown) {
-      void showAndLogExceptionWithTelemetry(
-        this.app.logger,
-        this.app.telemetry,
-        redactableError(
-          asError(e),
-        )`Failed to generate flow model: ${getErrorMessage(e)}`,
-      );
-    }
+        } catch (e: unknown) {
+          void showAndLogExceptionWithTelemetry(
+            this.app.logger,
+            this.app.telemetry,
+            redactableError(
+              asError(e),
+            )`Failed to generate flow model: ${getErrorMessage(e)}`,
+          );
+        }
 
-    if (addedDatabase) {
-      // After the flow model has been generated, we can remove the temporary database
-      // which we used for generating the flow model.
-      await this.showProgress({
-        step: 3900,
-        maxStep: 4000,
-        message: "Removing temporary database",
-      });
-      await this.databaseManager.removeDatabaseItem(addedDatabase);
-    }
-
-    await this.clearProgress();
+        if (addedDatabase) {
+          // After the flow model has been generated, we can remove the temporary database
+          // which we used for generating the flow model.
+          progress({
+            step: 3900,
+            maxStep: 4000,
+            message: "Removing temporary database",
+          });
+          await this.databaseManager.removeDatabaseItem(addedDatabase);
+        }
+      },
+      { cancellable: false },
+    );
   }
 
   private async generateModeledMethodsFromLlm(
     externalApiUsages: ExternalApiUsage[],
     modeledMethods: Record<string, ModeledMethod>,
   ): Promise<void> {
-    const maxStep = 3000;
+    await withProgress(
+      async (progress) => {
+        const maxStep = 3000;
 
-    await this.showProgress({
-      step: 0,
-      maxStep,
-      message: "Retrieving usages",
-    });
+        progress({
+          step: 0,
+          maxStep,
+          message: "Retrieving usages",
+        });
 
-    const usages = await getAutoModelUsages({
-      cliServer: this.cliServer,
-      queryRunner: this.queryRunner,
-      queryStorageDir: this.queryStorageDir,
-      databaseItem: this.databaseItem,
-      progress: (update) => this.showProgress(update, maxStep),
-    });
+        const usages = await getAutoModelUsages({
+          cliServer: this.cliServer,
+          queryRunner: this.queryRunner,
+          queryStorageDir: this.queryStorageDir,
+          databaseItem: this.databaseItem,
+          progress: (update) => progress({ ...update, maxStep }),
+        });
 
-    await this.showProgress({
-      step: 1800,
-      maxStep,
-      message: "Creating request",
-    });
+        progress({
+          step: 1800,
+          maxStep,
+          message: "Creating request",
+        });
 
-    const request = createAutoModelRequest(
-      this.databaseItem.language,
-      externalApiUsages,
-      modeledMethods,
-      usages,
-      this.mode,
+        const request = createAutoModelRequest(
+          this.databaseItem.language,
+          externalApiUsages,
+          modeledMethods,
+          usages,
+          this.mode,
+        );
+
+        progress({
+          step: 2000,
+          maxStep,
+          message: "Sending request",
+        });
+
+        const response = await this.callAutoModelApi(request);
+        if (!response) {
+          return;
+        }
+
+        progress({
+          step: 2500,
+          maxStep,
+          message: "Parsing response",
+        });
+
+        const predictedModeledMethods = parsePredictedClassifications(
+          response.predicted || [],
+        );
+
+        progress({
+          step: 2800,
+          maxStep,
+          message: "Applying results",
+        });
+
+        await this.postMessage({
+          t: "addModeledMethods",
+          modeledMethods: predictedModeledMethods,
+        });
+      },
+      { cancellable: false },
     );
-
-    await this.showProgress({
-      step: 2000,
-      maxStep,
-      message: "Sending request",
-    });
-
-    const response = await this.callAutoModelApi(request);
-    if (!response) {
-      return;
-    }
-
-    await this.showProgress({
-      step: 2500,
-      maxStep,
-      message: "Parsing response",
-    });
-
-    const predictedModeledMethods = parsePredictedClassifications(
-      response.predicted || [],
-    );
-
-    await this.showProgress({
-      step: 2800,
-      maxStep,
-      message: "Applying results",
-    });
-
-    await this.postMessage({
-      t: "addModeledMethods",
-      modeledMethods: predictedModeledMethods,
-    });
-
-    await this.clearProgress();
   }
 
   private async modelDependency(): Promise<void> {
@@ -482,46 +480,12 @@ export class DataExtensionsEditorView extends AbstractWebview<
     return addedDatabase;
   }
 
-  /*
-   * Progress in this class is a bit weird. Most of the progress is based on running the query.
-   * Query progress is always between 0 and 1000. However, we still have some steps that need
-   * to be done after the query has finished. Therefore, the maximum step is 1500. This captures
-   * that there's 1000 steps of the query progress since that takes the most time, and then
-   * an additional 500 steps for the rest of the work. The progress doesn't need to be 100%
-   * accurate, so this is just a rough estimate.
-   *
-   * For generating the modeled methods for an external library, the max step is 4000. This is
-   * based on the following steps:
-   * - 1000 for the summary model
-   * - 1000 for the sink model
-   * - 1000 for the source model
-   * - 1000 for the neutral model
-   */
-  private async showProgress(update: ProgressUpdate, maxStep?: number) {
-    await this.postMessage({
-      t: "showProgress",
-      step: update.step,
-      maxStep: maxStep ?? update.maxStep,
-      message: update.message,
-    });
-  }
-
-  private async clearProgress() {
-    await this.showProgress({
-      step: 0,
-      maxStep: 0,
-      message: "",
-    });
-  }
-
   private async callAutoModelApi(
     request: ModelRequest,
   ): Promise<ModelResponse | null> {
     try {
       return await autoModel(this.app.credentials, request);
     } catch (e) {
-      await this.clearProgress();
-
       if (e instanceof RequestError && e.status === 429) {
         void showAndLogExceptionWithTelemetry(
           this.app.logger,

extensions/ql-vscode/src/view/data-extensions-editor/DataExtensionsEditor.tsx

@@ -1,9 +1,6 @@
 import * as React from "react";
 import { useCallback, useEffect, useMemo, useState } from "react";
-import {
-  ShowProgressMessage,
-  ToDataExtensionsEditorMessage,
-} from "../../common/interface-types";
+import { ToDataExtensionsEditorMessage } from "../../common/interface-types";
 import { VSCodeButton } from "@vscode/webview-ui-toolkit/react";
 import styled from "styled-components";
 import { ExternalApiUsage } from "../../data-extensions-editor/external-api-usage";
@@ -46,17 +43,6 @@ const ButtonsContainer = styled.div`
   margin-bottom: 1rem;
 `;
 
-type ProgressBarProps = {
-  completion: number;
-};
-
-const ProgressBar = styled.div<ProgressBarProps>`
-  height: 10px;
-  width: ${(props) => props.completion * 100}%;
-
-  background-color: var(--vscode-progressBar-background);
-`;
-
 type Props = {
   initialViewState?: DataExtensionEditorViewState;
   initialExternalApiUsages?: ExternalApiUsage[];
@@ -82,11 +68,6 @@ export function DataExtensionsEditor({
   const [modeledMethods, setModeledMethods] = useState<
     Record<string, ModeledMethod>
   >(initialModeledMethods);
-  const [progress, setProgress] = useState<Omit<ShowProgressMessage, "t">>({
-    step: 0,
-    maxStep: 0,
-    message: "",
-  });
 
   useEffect(() => {
     const listener = (evt: MessageEvent) => {
@@ -99,9 +80,6 @@ export function DataExtensionsEditor({
           case "setExternalApiUsages":
             setExternalApiUsages(msg.externalApiUsages);
             break;
-          case "showProgress":
-            setProgress(msg);
-            break;
           case "loadModeledMethods":
             setModeledMethods((oldModeledMethods) => {
               return {
@@ -244,89 +222,71 @@ export function DataExtensionsEditor({
     });
   }, [viewState?.mode]);
 
-  if (viewState === undefined) {
+  if (viewState === undefined || externalApiUsages.length === 0) {
     return <LoadingContainer>Loading...</LoadingContainer>;
   }
 
   return (
     <DataExtensionsEditorContainer>
-      {progress.maxStep > 0 && (
-        <p>
-          <ProgressBar completion={progress.step / progress.maxStep} />{" "}
-          {progress.message}
-        </p>
-      )}
-
-      {externalApiUsages.length > 0 && (
-        <>
-          <ViewTitle>
-            {getLanguageDisplayName(viewState.extensionPack.language)}
-          </ViewTitle>
-          <DetailsContainer>
-            <LinkIconButton onClick={onOpenExtensionPackClick}>
-              <span slot="start" className="codicon codicon-package"></span>
-              {viewState.extensionPack.name}
-            </LinkIconButton>
+      <ViewTitle>
+        {getLanguageDisplayName(viewState.extensionPack.language)}
+      </ViewTitle>
+      <DetailsContainer>
+        <LinkIconButton onClick={onOpenExtensionPackClick}>
+          <span slot="start" className="codicon codicon-package"></span>
+          {viewState.extensionPack.name}
+        </LinkIconButton>
+        <div>{percentFormatter.format(modeledPercentage / 100)} modeled</div>
+        <div>
+          {percentFormatter.format(unModeledPercentage / 100)} unmodeled
+        </div>
+        {viewState.enableFrameworkMode && (
+          <>
             <div>
-              {percentFormatter.format(modeledPercentage / 100)} modeled
+              Mode:{" "}
+              {viewState.mode === Mode.Framework ? "Framework" : "Application"}
             </div>
             <div>
-              {percentFormatter.format(unModeledPercentage / 100)} unmodeled
+              <LinkIconButton onClick={onSwitchModeClick}>
+                <span slot="start" className="codicon codicon-library"></span>
+                Switch mode
+              </LinkIconButton>
             </div>
-            {viewState.enableFrameworkMode && (
-              <>
-                <div>
-                  Mode:{" "}
-                  {viewState.mode === Mode.Framework
-                    ? "Framework"
-                    : "Application"}
-                </div>
-                <div>
-                  <LinkIconButton onClick={onSwitchModeClick}>
-                    <span
-                      slot="start"
-                      className="codicon codicon-library"
-                    ></span>
-                    Switch mode
-                  </LinkIconButton>
-                </div>
-              </>
-            )}
-          </DetailsContainer>
+          </>
+        )}
+      </DetailsContainer>
 
-          <EditorContainer>
-            <ButtonsContainer>
-              <VSCodeButton
-                onClick={onSaveAllClick}
-                disabled={modifiedSignatures.size === 0}
-              >
-                Save all
-              </VSCodeButton>
-              {viewState.enableFrameworkMode && (
-                <VSCodeButton appearance="secondary" onClick={onRefreshClick}>
-                  Refresh
-                </VSCodeButton>
-              )}
-              {viewState.mode === Mode.Framework && (
-                <VSCodeButton onClick={onGenerateFromSourceClick}>
-                  Generate
-                </VSCodeButton>
-              )}
-            </ButtonsContainer>
-            <ModeledMethodsList
-              externalApiUsages={externalApiUsages}
-              modeledMethods={modeledMethods}
-              modifiedSignatures={modifiedSignatures}
-              viewState={viewState}
-              onChange={onChange}
-              onSaveModelClick={onSaveModelClick}
-              onGenerateFromLlmClick={onGenerateFromLlmClick}
-              onGenerateFromSourceClick={onGenerateFromSourceClick}
-              onModelDependencyClick={onModelDependencyClick}
-            />
-          </EditorContainer>
-        </>
-      )}
+      <EditorContainer>
+        <ButtonsContainer>
+          <VSCodeButton
+            onClick={onSaveAllClick}
+            disabled={modifiedSignatures.size === 0}
+          >
+            Save all
+          </VSCodeButton>
+          {viewState.enableFrameworkMode && (
+            <VSCodeButton appearance="secondary" onClick={onRefreshClick}>
+              Refresh
+            </VSCodeButton>
+          )}
+          {viewState.mode === Mode.Framework && (
+            <VSCodeButton onClick={onGenerateFromSourceClick}>
+              Generate
+            </VSCodeButton>
+          )}
+        </ButtonsContainer>
+        <ModeledMethodsList
+          externalApiUsages={externalApiUsages}
+          modeledMethods={modeledMethods}
+          modifiedSignatures={modifiedSignatures}
+          viewState={viewState}
+          onChange={onChange}
+          onSaveModelClick={onSaveModelClick}
+          onGenerateFromLlmClick={onGenerateFromLlmClick}
+          onGenerateFromSourceClick={onGenerateFromSourceClick}
+          onModelDependencyClick={onModelDependencyClick}
+        />
+      </EditorContainer>
     </DataExtensionsEditorContainer>
   );
 }