Ensure API docs don't have stale images (#2541)

Closes #2538. We only
didn't delete the API images due to
#2540.

To fix #2540, we have to
fix our skip of release note images. We also had to manually move 4
release note images for legacy release note files from the latest Qiskit
images folder to instead the 0.40 and 0.43 Box artifacts, then manually
update those release notes to point to the paths. With those fixes and
the re-uploaded Box artifacts, `npm run regen-apis` doesn't make
changes.

This change allows us to have checkStaleImages.ts no longer worry about
API docs images, which dramatically speeds up the program. It's now fast
enough to run on every PR. Closes
#2530.

With ripgrep:

```
  Time (mean ± σ):      1.705 s ±  0.022 s    [User: 1.143 s, System: 1.085 s]
  Range (min … max):    1.660 s …  1.735 s    10 runs
```

With git grep (CI):

```
  Time (mean ± σ):      4.118 s ±  0.054 s    [User: 11.290 s, System: 1.092 s]
  Range (min … max):    4.044 s …  4.200 s    10 runs
```

.github/workflows/main.yml

@@ -40,6 +40,8 @@ jobs:
         run: npm run check:internal-links
       - name: Check orphaned pages
         run: npm run check:orphan-pages
+      - name: Check stale images
+        run: npm run check:stale-images
       - name: Formatting
         run: npm run check:fmt
       - name: Typecheck

docs/api/qiskit/release-notes/0.40.mdx

@@ -56,7 +56,7 @@ Qiskit Terra 0.23.0 is a major feature release that includes a multitude of new
 >
 > *   Performance improvements to [`SabreLayout`](/api/qiskit/0.45/qiskit.transpiler.passes.SabreLayout "qiskit.transpiler.passes.SabreLayout"). The pass is now primarily written in Rust which can lead to a runtime improvement, however the bigger improvement is in the quality of the output (on average, fewer [`SwapGate`](/api/qiskit/0.45/qiskit.circuit.library.SwapGate "qiskit.circuit.library.SwapGate") gates introduced by [`SabreSwap`](/api/qiskit/0.45/qiskit.transpiler.passes.SabreSwap "qiskit.transpiler.passes.SabreSwap")). For example, running [`SabreLayout`](/api/qiskit/0.45/qiskit.transpiler.passes.SabreLayout "qiskit.transpiler.passes.SabreLayout") and [`SabreSwap`](/api/qiskit/0.45/qiskit.transpiler.passes.SabreSwap "qiskit.transpiler.passes.SabreSwap") on Bernstein Vazirani circuits targeting the [`FakeSherbrooke`](/api/qiskit/0.45/qiskit.providers.fake_provider.FakeSherbrooke "qiskit.providers.fake_provider.FakeSherbrooke") backend yields the following results:
 >
->     ![\_images/legacy\_release\_notes-2.png](/images/api/qiskit/legacy_release_notes-2.png)
+>     ![\_images/legacy\_release\_notes-2.png](/images/api/qiskit/legacy-release-notes/0.40-1.png)
 
 This release also deprecates support for running with Python 3.7. A `DeprecationWarning` will now be emitted if you run Qiskit with Python 3.7. Support for Python 3.7 will be removed as part of the 0.25.0 release (currently planned for release in July 2023), at which point you will need Python 3.8 or newer to use Qiskit.
 
@@ -346,7 +346,7 @@ This release also deprecates support for running with Python 3.7. A `Deprecation
     circuit.draw("mpl")
     ```
 
-    ![\_images/legacy\_release\_notes-3.png](/images/api/qiskit/legacy_release_notes-3.png)
+    ![\_images/legacy\_release\_notes-3.png](/images/api/qiskit/legacy-release-notes/0.40-2.png)
 
 *   The [`Clifford`](/api/qiskit/0.45/qiskit.quantum_info.Clifford "qiskit.quantum_info.Clifford") class now takes an optional `copy` keyword argument in its constructor. If set to `False`, then a `StabilizerTable` provided as input will not be copied, but will be used directly. This can have performance benefits, if the data in the table will never be mutated by any other means.
 
@@ -596,7 +596,7 @@ This release also deprecates support for running with Python 3.7. A `Deprecation
     out.draw('mpl')
     ```
 
-    ![\_images/legacy\_release\_notes-4.png](/images/api/qiskit/legacy_release_notes-4.png)
+    ![\_images/legacy\_release\_notes-4.png](/images/api/qiskit/legacy-release-notes/0.40-3.png)
 
 *   Random-circuit generation with [`qiskit.circuit.random.random_circuit()`](/api/qiskit/0.45/circuit#qiskit.circuit.random.random_circuit "qiskit.circuit.random.random_circuit") is now significantly faster for large circuits.
 

docs/api/qiskit/release-notes/0.43.mdx

@@ -592,7 +592,7 @@ This release adds a new control flow operation, the switch statement. This is im
     unroll_pass(qc).draw("mpl")
     ```
 
-    ![\_images/legacy\_release\_notes-1.png](/images/api/qiskit/legacy_release_notes-1.png)
+    ![\_images/legacy\_release\_notes-1.png](/images/api/qiskit/legacy-release-notes/0.43.png)
 
 *   Added a new parameter `max_trials` to pass [`VF2PostLayout`](/api/qiskit/0.45/qiskit.transpiler.passes.VF2PostLayout "qiskit.transpiler.passes.VF2PostLayout") which, when specified, limits the number of layouts discovered and compared when searching for the best layout. This differs from existing parameters `call_limit` and `time_limit` (which are used to limit the number of state visits performed by the VF2 algorithm and the total time spent by pass [`VF2PostLayout`](/api/qiskit/0.45/qiskit.transpiler.passes.VF2PostLayout "qiskit.transpiler.passes.VF2PostLayout"), respectively) in that it is used to place an upper bound on the time spent scoring potential layouts, which may be useful for larger devices.
 

package.json

@@ -15,7 +15,7 @@
     "fmt": "prettier --write .",
     "test": "playwright test",
     "typecheck": "tsc",
-    "check": "npm run check:qiskit-bot && npm run check:patterns-index && npm run check:images && npm run check:metadata && npm run check:spelling && npm run check:internal-links && npm run check:orphan-pages && npm run check:fmt",
+    "check": "npm run check:qiskit-bot && npm run check:patterns-index && npm run check:images && npm run check:metadata && npm run check:spelling && npm run check:internal-links && npm run check:orphan-pages && npm run check:stale-images && npm run check:fmt",
     "check:images": "tsx scripts/js/commands/checkImages.ts",
     "check:metadata": "tsx scripts/js/commands/checkMetadata.ts",
     "check:spelling": "tsx scripts/js/commands/checkSpelling.ts",

scripts/js/commands/api/updateApiDocs.ts

@@ -79,7 +79,7 @@ export async function generateVersion(
   args: Arguments,
 ): Promise<void> {
   const sphinxArtifactFolder = await prepareSphinxFolder(pkg, args);
-  await deleteExistingMarkdown(pkg);
+  await deleteExistingFiles(pkg);
 
   console.log(`Run pipeline for ${pkg.name}:${pkg.versionWithoutPatch}`);
   await runConversionPipeline(sphinxArtifactFolder, "docs", "public", pkg);
@@ -127,13 +127,18 @@ async function prepareSphinxFolder(pkg: Pkg, args: Arguments): Promise<string> {
   return `${sphinxArtifactFolder}/artifact`;
 }
 
-async function deleteExistingMarkdown(pkg: Pkg): Promise<void> {
+async function deleteExistingFiles(pkg: Pkg): Promise<void> {
   const markdownDir = pkg.outputDir("docs");
-  if (!(await pathExists(markdownDir))) return;
+  if (await pathExists(markdownDir)) {
+    await rmFilesInFolder(markdownDir);
+  }
+  const imagesDir = pkg.outputDir("public/images");
+  if (await pathExists(imagesDir)) {
+    await rmFilesInFolder(imagesDir);
+  }
   console.log(
-    `Deleting existing markdown for ${pkg.name}:${pkg.versionWithoutPatch}`,
+    `Deleted existing markdown & images for ${pkg.name}:${pkg.versionWithoutPatch}`,
   );
-  await rmFilesInFolder(markdownDir);
 }
 
 if (import.meta.url === `file://${process.argv[1]}`) {

scripts/js/commands/checkStaleImages.ts

@@ -13,7 +13,7 @@
 import { $ } from "zx";
 import { globby } from "globby";
 
-import { zxMain } from "../lib/zx.js";
+import { zxMain, isInstalled } from "../lib/zx.js";
 
 // Files that are used in closed source, so should not be removed.
 // Format: full file path, e.g. "public/images/guides/paulibasis.png"
@@ -51,7 +51,12 @@ zxMain(async () => {
 });
 
 async function getStrippedImagePaths(): Promise<Set<string>> {
-  const fullPaths = await globby("public/images/**");
+  const fullPaths = await globby([
+    "public/images/**",
+    // We don't check API images because the API pipeline will already ensure there are no
+    // stale images when regenerating API docs.
+    "!public/images/api/**",
+  ]);
   return new Set(
     fullPaths.map((fp) =>
       fp
@@ -64,8 +69,9 @@ async function getStrippedImagePaths(): Promise<Set<string>> {
 }
 
 async function isImageUnused(strippedFp: string): Promise<boolean> {
-  const grep = await $`rg -l ${strippedFp} docs/`
-    .quiet()
-    .catch((result) => result);
-  return grep.stdout === "" && !ALLOW_LIST.has(`public/${strippedFp}`);
+  const args = (await isInstalled("rg"))
+    ? ["rg", "-l", strippedFp, "docs", "-g", "!docs/api"]
+    : ["git", "grep", "-l", strippedFp, "docs", ":!docs/api"];
+  const proc = await $`${args}`.quiet().catch((result) => result);
+  return proc.stdout === "" && !ALLOW_LIST.has(`public/${strippedFp}`);
 }

scripts/js/lib/api/Pkg.ts

@@ -230,8 +230,8 @@ export class Pkg {
     return this.type == "latest";
   }
 
-  hasObjectsInv(): boolean {
-    return this.name !== "qiskit" || +this.versionWithoutPatch >= 0.45;
+  isProblematicLegacyQiskit(): boolean {
+    return this.name === "qiskit" && +this.versionWithoutPatch < 0.45;
   }
 
   hasSeparateReleaseNotes(): boolean {

scripts/js/lib/api/conversionPipeline.ts

@@ -75,9 +75,9 @@ async function determineFilePaths(
   docsBaseFolder: string,
   pkg: Pkg,
 ): Promise<[string[], string, ObjectsInv | undefined]> {
-  const maybeObjectsInv = await (pkg.hasObjectsInv()
-    ? ObjectsInv.fromFile(htmlPath)
-    : undefined);
+  const maybeObjectsInv = await (pkg.isProblematicLegacyQiskit()
+    ? undefined
+    : ObjectsInv.fromFile(htmlPath));
   const files = await globby(
     [
       "apidocs/**.html",
@@ -133,8 +133,6 @@ async function copyImages(
   publicBaseFolder: string,
   results: HtmlToMdResultWithUrl[],
 ): Promise<void> {
-  // Some historical versions don't have the `_images` folder in the artifact store in Box (https://ibm.ent.box.com/folder/246867452622)
-  if (pkg.isHistorical() && !(await pathExists(`${htmlPath}/_images`))) return;
   console.log("Saving images");
   const allImages = uniqBy(
     results.flatMap((result) => result.images),

scripts/js/lib/api/saveImages.ts

@@ -16,14 +16,18 @@ import { copyFile } from "fs/promises";
 
 import { Pkg } from "./Pkg.js";
 import { Image } from "./HtmlToMdResult.js";
-import { pathExists, rmFilesInFolder } from "../fs.js";
+import { pathExists } from "../fs.js";
 
 function skipReleaseNote(imgFileName: string, pkg: Pkg): boolean {
   const isReleaseNote =
     imgFileName.includes("release_notes") ||
     imgFileName.includes("release-notes");
   if (!isReleaseNote) return false;
 
+  // Release note images before Qiskit 0.45 were not actually used and we don't have
+  // the image files stored.
+  if (pkg.isProblematicLegacyQiskit()) return true;
+
   if (pkg.hasSeparateReleaseNotes()) {
     // If the pkg has dedicated release notes per release, we should copy its images
     // unless it's the dev version. We don't (yet) support release notes for dev versions:
@@ -44,10 +48,6 @@ export async function saveImages(
   const destFolder = pkg.outputDir(`${publicBaseFolder}/images`);
   if (!(await pathExists(destFolder))) {
     await mkdirp(destFolder);
-  } else if (pkg.isDev()) {
-    // We don't want to store images from other versions when we generate a
-    // different dev version
-    await rmFilesInFolder(destFolder);
   }
 
   await pMap(images, async (img) => {

scripts/js/lib/zx.ts

@@ -10,7 +10,7 @@
 // copyright notice, and modified files need to carry a notice indicating
 // that they have been altered from the originals.
 
-import { ProcessOutput } from "zx";
+import { ProcessOutput, $ } from "zx";
 
 export function zxMain(mainFn: () => Promise<void>) {
   enableCliColors();
@@ -29,3 +29,16 @@ export function enableCliColors() {
 export function disableCliColors() {
   process.env.FORCE_COLOR = "0";
 }
+
+export async function isInstalled(binary: string): Promise<boolean> {
+  try {
+    if (process.platform === "win32") {
+      await $`where ${binary}`.quiet();
+    } else {
+      await $`which ${binary}`.quiet();
+    }
+    return true;
+  } catch {
+    return false;
+  }
+}