Show QA meter while analysis is running (#1854)

Fixes #1846 - Ensure meter auto-updates as new stats are ready - Switch meter to new QA run when new analysis run is started - Remove Files from QA meter (files and errors will be reported separately) Co-authored-by: emma <hi@emma.cafe> Co-authored-by: sua yoo <sua@webrecorder.org>
2024-06-12 12:32:01 -04:00 · 2024-06-12 12:32:01 -04:00 · 8b0d1432af
commit 8b0d1432af
parent 2ffb37bd14
7 changed files with 92 additions and 107 deletions
--- a/backend/btrixcloud/crawls.py
+++ b/backend/btrixcloud/crawls.py
@ -957,12 +957,11 @@ class CrawlOps(BaseCrawlOps):
        thresholds: Dict[str, List[float]],
    ) -> QARunAggregateStatsOut:
        """Get aggregate stats for QA run"""
-        file_count = await self.page_ops.get_crawl_file_count(crawl_id)
        screenshot_results = await self.page_ops.get_qa_run_aggregate_counts(
-            crawl_id, qa_run_id, thresholds, file_count, key="screenshotMatch"
+            crawl_id, qa_run_id, thresholds, key="screenshotMatch"
        )
        text_results = await self.page_ops.get_qa_run_aggregate_counts(
-            crawl_id, qa_run_id, thresholds, file_count, key="textMatch"
+            crawl_id, qa_run_id, thresholds, key="textMatch"
        )
        return QARunAggregateStatsOut(
            screenshotMatch=screenshot_results,
--- a/backend/btrixcloud/pages.py
+++ b/backend/btrixcloud/pages.py
@ -548,7 +548,6 @@ class PageOps:
        crawl_id: str,
        qa_run_id: str,
        thresholds: Dict[str, List[float]],
-        file_count: int,
        key: str = "screenshotMatch",
    ):
        """Get counts for pages in QA run in buckets by score key based on thresholds"""
@ -585,17 +584,11 @@ class PageOps:
        return_data = []

        for result in results:
-            key = str(result.get("_id"))
-            if key == "No data":
-                count = result.get("count", 0) - file_count
-                return_data.append(QARunBucketStats(lowerBoundary=key, count=count))
-            else:
-                return_data.append(
-                    QARunBucketStats(lowerBoundary=key, count=result.get("count", 0))
+            return_data.append(
+                QARunBucketStats(
+                    lowerBoundary=str(result.get("_id")), count=result.get("count", 0)
                )
-
-        # Add file count
-        return_data.append(QARunBucketStats(lowerBoundary="Files", count=file_count))
+            )

        # Add missing boundaries to result and re-sort
        for boundary in boundaries:
--- a/backend/test/test_qa.py
+++ b/backend/test/test_qa.py
@ -329,13 +329,11 @@ def test_qa_stats(
        {"lowerBoundary": "0.0", "count": 0},
        {"lowerBoundary": "0.7", "count": 0},
        {"lowerBoundary": "0.9", "count": 1},
-        {"lowerBoundary": "Files", "count": 0},
    ]
    assert data["textMatch"] == [
        {"lowerBoundary": "0.0", "count": 0},
        {"lowerBoundary": "0.7", "count": 0},
        {"lowerBoundary": "0.9", "count": 1},
-        {"lowerBoundary": "Files", "count": 0},
    ]

    # Test we get expected results with explicit 0 boundary
@ -350,13 +348,11 @@ def test_qa_stats(
        {"lowerBoundary": "0.0", "count": 0},
        {"lowerBoundary": "0.7", "count": 0},
        {"lowerBoundary": "0.9", "count": 1},
-        {"lowerBoundary": "Files", "count": 0},
    ]
    assert data["textMatch"] == [
        {"lowerBoundary": "0.0", "count": 0},
        {"lowerBoundary": "0.7", "count": 0},
        {"lowerBoundary": "0.9", "count": 1},
-        {"lowerBoundary": "Files", "count": 0},
    ]

    # Test that missing threshold values result in 422 HTTPException
--- a/frontend/src/components/ui/meter.ts
+++ b/frontend/src/components/ui/meter.ts
@ -30,6 +30,7 @@ export class MeterBar extends TailwindElement {
      height: 1rem;
      background-color: var(--background-color, var(--sl-color-blue-500));
      min-width: 4px;
+      transition: 400ms width;
    }
  `;

@ -151,6 +152,7 @@ export class Meter extends TailwindElement {
      display: flex;
      border-radius: var(--sl-border-radius-medium);
      overflow: hidden;
+      transition: 400ms width;
    }

    .labels {
--- a/frontend/src/features/qa/qa-run-dropdown.ts
+++ b/frontend/src/features/qa/qa-run-dropdown.ts
@ -39,7 +39,9 @@ export class QaRunDropdown extends TailwindElement {
                  slot="prefix"
                  hoist
                ></btrix-crawl-status>
-                ${formatISODateString(selectedRun.finished)} `
+                ${selectedRun.finished
+                  ? formatISODateString(selectedRun.finished)
+                  : msg("In progress")}`
            : msg("Select a QA run")}
        </sl-button>
        <sl-menu>
@ -52,7 +54,9 @@ export class QaRunDropdown extends TailwindElement {
                ?disabled=${isSelected}
                ?checked=${isSelected}
              >
-                ${formatISODateString(run.finished)}
+                ${run.finished
+                  ? formatISODateString(run.finished)
+                  : msg("In progress")}
                <btrix-crawl-status
                  type="qa"
                  hideLabel
--- a/frontend/src/pages/org/archived-item-detail/archived-item-detail.ts
+++ b/frontend/src/pages/org/archived-item-detail/archived-item-detail.ts
@ -92,9 +92,6 @@ export class ArchivedItemDetail extends TailwindElement {
  @state()
  private qaRunId?: string;

-  @state()
-  private lastFinishedQARunId?: string;
-
  @state()
  private isQAActive = false;

@ -187,25 +184,11 @@ export class ArchivedItemDetail extends TailwindElement {
      (changedProperties.has("qaRuns") ||
        changedProperties.has("mostRecentNonFailedQARun")) &&
      this.qaRuns &&
-      this.mostRecentNonFailedQARun
+      this.mostRecentNonFailedQARun?.id
    ) {
-      const lastFinishedQARun = this.qaRuns.find(({ state }) =>
-        finishedCrawlStates.includes(state),
-      );
-      const prevMostRecentNonFailedQARun =
-        changedProperties.get("mostRecentNonFailedQARun") ||
-        this.mostRecentNonFailedQARun;
-      const mostRecentNowFinished =
-        QA_RUNNING_STATES.includes(prevMostRecentNonFailedQARun.state) &&
-        finishedCrawlStates.includes(this.mostRecentNonFailedQARun.state);
-
-      // Update currently selected QA run if there is none,
-      // or if a QA run that was previously running is now finished:
-      if (lastFinishedQARun && (!this.qaRunId || mostRecentNowFinished)) {
-        this.qaRunId = lastFinishedQARun.id;
+      if (!this.qaRunId) {
+        this.qaRunId = this.mostRecentNonFailedQARun.id;
      }
-      // set last finished run
-      this.lastFinishedQARunId = lastFinishedQARun?.id;
    }
  }

@ -1025,10 +1008,8 @@ ${this.crawl?.description}
  }

  private readonly renderQAHeader = (qaRuns: QARun[]) => {
-    //const qaIsRunning = isActive(qaRuns[0]?.state);
-    //const qaIsAvailable = this.mostRecentNonFailedQARun && !qaIsRunning;
    const qaIsRunning = this.isQAActive;
-    const qaIsAvailable = !!this.lastFinishedQARunId;
+    const qaIsAvailable = !!this.mostRecentNonFailedQARun;

    const reviewLink =
      qaIsAvailable && this.qaRunId
@ -1345,13 +1326,14 @@ ${this.crawl?.description}

  private async startQARun() {
    try {
-      await this.api.fetch<{ started: string }>(
+      const result = await this.api.fetch<{ started: string }>(
        `/orgs/${this.orgId}/crawls/${this.crawlId}/qa/start`,
        this.authState!,
        {
          method: "POST",
        },
      );
+      this.qaRunId = result.started;

      void this.fetchQARuns();

@ -1457,6 +1439,10 @@ ${this.crawl?.description}
    );

    if (this.isQAActive) {
+      // Clear current timer, if it exists
+      if (this.timerId != null) {
+        this.stopPoll();
+      }
      // Restart timer for next poll
      this.timerId = window.setTimeout(() => {
        void this.fetchQARuns();
--- a/frontend/src/pages/org/archived-item-detail/ui/qa.ts
+++ b/frontend/src/pages/org/archived-item-detail/ui/qa.ts
@ -44,7 +44,7 @@ import { formatNumber, getLocale } from "@/utils/localization";
 import { pluralOf } from "@/utils/pluralize";

 type QAStatsThreshold = {
-  lowerBoundary: `${number}` | "No data" | "Files";
+  lowerBoundary: `${number}` | "No data";
  count: number;
 };
 type QAStats = Record<"screenshotMatch" | "textMatch", QAStatsThreshold[]>;
@ -65,11 +65,6 @@ const qaStatsThresholds = [
    cssColor: "var(--sl-color-success-500)",
    label: msg("Good Match"),
  },
-  {
-    lowerBoundary: "Files",
-    cssColor: "var(--sl-color-neutral-500)",
-    label: msg("Identical Files"),
-  },
 ];

 const notApplicable = () =>
@ -129,13 +124,28 @@ export class ArchivedItemDetailQA extends TailwindElement {
  private pages?: APIPaginatedList<ArchivedItemPage>;

  private readonly qaStats = new Task(this, {
-    task: async ([orgId, crawlId, qaRunId, authState]) => {
-      if (!qaRunId || !authState) throw new Error("Missing args");
+    // mostRecentNonFailedQARun passed as arg for reactivity so that meter will auto-update
+    // like progress bar as the analysis run finishes new pages
+    task: async ([
+      orgId,
+      crawlId,
+      qaRunId,
+      authState,
+      mostRecentNonFailedQARun,
+    ]) => {
+      if (!qaRunId || !authState || !mostRecentNonFailedQARun)
+        throw new Error("Missing args");
      const stats = await this.getQAStats(orgId, crawlId, qaRunId, authState);
      return stats;
    },
    args: () =>
-      [this.orgId!, this.crawlId!, this.qaRunId, this.authState] as const,
+      [
+        this.orgId!,
+        this.crawlId!,
+        this.qaRunId,
+        this.authState,
+        this.mostRecentNonFailedQARun,
+      ] as const,
  });

  @state()
@ -473,13 +483,6 @@ export class ArchivedItemDetailQA extends TailwindElement {
    }

    return html`
-      ${isRunning
-        ? html`<btrix-alert class="mb-3" variant="warning">
-            ${msg(
-              "This crawl is being analyzed. You're currently viewing results from an older analysis run.",
-            )}
-          </btrix-alert>`
-        : nothing}
      <btrix-card>
        <div slot="title" class="flex flex-wrap justify-between">
          <div class="flex flex-wrap items-center gap-x-3 gap-y-1">
@ -488,23 +491,23 @@ export class ArchivedItemDetailQA extends TailwindElement {
              const finishedQARuns = qaRuns.filter(({ state }) =>
                finishedCrawlStates.includes(state),
              );
-
-              if (!finishedQARuns.length) {
-                return nothing;
-              }
-
-              const mostRecentSelected =
-                this.mostRecentNonFailedQARun &&
-                this.mostRecentNonFailedQARun.id === this.qaRunId;
              const latestFinishedSelected =
-                this.qaRunId === finishedQARuns[0].id;
+                this.qaRunId === finishedQARuns[0]?.id;
+
+              const finishedAndRunningQARuns = qaRuns.filter(
+                ({ state }) =>
+                  finishedCrawlStates.includes(state) ||
+                  QA_RUNNING_STATES.includes(state),
+              );
+              const mostRecentSelected =
+                this.qaRunId === finishedAndRunningQARuns[0]?.id;

              return html`
                <div>
                  <sl-tooltip
                    content=${mostRecentSelected
                      ? msg(
-                          "You’re viewing the latest results from a finished analysis run.",
+                          "You’re viewing the latest analysis run results.",
                        )
                      : msg(
                          "You’re viewing results from an older analysis run.",
@ -522,7 +525,7 @@ export class ArchivedItemDetailQA extends TailwindElement {
                    </sl-tag>
                  </sl-tooltip>
                  <btrix-qa-run-dropdown
-                    .items=${finishedQARuns}
+                    .items=${finishedAndRunningQARuns}
                    selectedId=${this.qaRunId || ""}
                    @btrix-select=${(e: CustomEvent<SelectDetail>) =>
                      (this.qaRunId = e.detail.item.id)}
@ -566,12 +569,12 @@ export class ArchivedItemDetailQA extends TailwindElement {
                  ${msg("Screenshots")}
                </btrix-table-cell>
                <btrix-table-cell class="p-0">
-                  ${this.qaStats.render({
-                    complete: ({ screenshotMatch }) =>
-                      this.renderMeter(qaRun.stats.found, screenshotMatch),
-                    pending: () => this.renderMeter(),
-                    initial: () => this.renderMeter(),
-                  })}
+                  ${this.qaStats.value
+                    ? this.renderMeter(
+                        qaRun.stats.found,
+                        this.qaStats.value.screenshotMatch,
+                      )
+                    : this.renderMeter()}
                </btrix-table-cell>
              </btrix-table-row>
              <btrix-table-row>
@ -579,12 +582,12 @@ export class ArchivedItemDetailQA extends TailwindElement {
                  ${msg("Text")}
                </btrix-table-cell>
                <btrix-table-cell class="p-0">
-                  ${this.qaStats.render({
-                    complete: ({ textMatch }) =>
-                      this.renderMeter(qaRun.stats.found, textMatch),
-                    pending: () => this.renderMeter(),
-                    initial: () => this.renderMeter(),
-                  })}
+                  ${this.qaStats.value
+                    ? this.renderMeter(
+                        qaRun.stats.found,
+                        this.qaStats.value.textMatch,
+                      )
+                    : this.renderMeter()}
                </btrix-table-cell>
              </btrix-table-row>
            </btrix-table-body>
@ -627,30 +630,32 @@ export class ArchivedItemDetailQA extends TailwindElement {
          );
          const idx = threshold ? qaStatsThresholds.indexOf(threshold) : -1;

-          return html`
-            <btrix-meter-bar
-              value=${(bar.count / pageCount) * 100}
-              style="--background-color: ${threshold?.cssColor}"
-              aria-label=${bar.lowerBoundary}
-            >
-              <div class="text-center">
-                ${bar.lowerBoundary === "No data"
-                  ? msg("No Data")
-                  : threshold?.label}
-                <div class="text-xs opacity-80">
-                  ${!["No data", "Files"].includes(bar.lowerBoundary)
-                    ? html`${idx === 0
-                          ? `<${+qaStatsThresholds[idx + 1].lowerBoundary * 100}%`
-                          : idx === qaStatsThresholds.length - 1
-                            ? `>=${threshold ? +threshold.lowerBoundary * 100 : 0}%`
-                            : `${threshold ? +threshold.lowerBoundary * 100 : 0}-${+qaStatsThresholds[idx + 1].lowerBoundary * 100 || 100}%`}
-                        ${msg("match")} <br />`
-                    : nothing}
-                  ${formatNumber(bar.count)} ${pluralOf("pages", bar.count)}
-                </div>
-              </div>
-            </btrix-meter-bar>
-          `;
+          return bar.count !== 0
+            ? html`
+                <btrix-meter-bar
+                  value=${(bar.count / pageCount) * 100}
+                  style="--background-color: ${threshold?.cssColor}"
+                  aria-label=${bar.lowerBoundary}
+                >
+                  <div class="text-center">
+                    ${bar.lowerBoundary === "No data"
+                      ? msg("No Data")
+                      : threshold?.label}
+                    <div class="text-xs opacity-80">
+                      ${bar.lowerBoundary !== "No data"
+                        ? html`${idx === 0
+                              ? `<${+qaStatsThresholds[idx + 1].lowerBoundary * 100}%`
+                              : idx === qaStatsThresholds.length - 1
+                                ? `>=${threshold ? +threshold.lowerBoundary * 100 : 0}%`
+                                : `${threshold ? +threshold.lowerBoundary * 100 : 0}-${+qaStatsThresholds[idx + 1].lowerBoundary * 100 || 100}%`}
+                            match <br />`
+                        : nothing}
+                      ${formatNumber(bar.count)} ${pluralOf("pages", bar.count)}
+                    </div>
+                  </div>
+                </btrix-meter-bar>
+              `
+            : nothing;
        })}
      </btrix-meter>
    `;