From 8b0d1432afe2a8bcc8d16204f09e00b125197588 Mon Sep 17 00:00:00 2001 From: Tessa Walsh Date: Wed, 12 Jun 2024 12:32:01 -0400 Subject: [PATCH] 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 Co-authored-by: sua yoo --- backend/btrixcloud/crawls.py | 5 +- backend/btrixcloud/pages.py | 15 +- backend/test/test_qa.py | 4 - frontend/src/components/ui/meter.ts | 2 + frontend/src/features/qa/qa-run-dropdown.ts | 8 +- .../archived-item-detail.ts | 34 ++--- .../pages/org/archived-item-detail/ui/qa.ts | 131 +++++++++--------- 7 files changed, 92 insertions(+), 107 deletions(-) diff --git a/backend/btrixcloud/crawls.py b/backend/btrixcloud/crawls.py index f13121e0..0e850c85 100644 --- 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, diff --git a/backend/btrixcloud/pages.py b/backend/btrixcloud/pages.py index 9db5fffd..b549ee47 100644 --- 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: diff --git a/backend/test/test_qa.py b/backend/test/test_qa.py index 8cc8f705..e9861b31 100644 --- 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 diff --git a/frontend/src/components/ui/meter.ts b/frontend/src/components/ui/meter.ts index da1cdf31..9263b7b1 100644 --- 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 { diff --git a/frontend/src/features/qa/qa-run-dropdown.ts b/frontend/src/features/qa/qa-run-dropdown.ts index 7bf34aad..86f6cc45 100644 --- 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 > - ${formatISODateString(selectedRun.finished)} ` + ${selectedRun.finished + ? formatISODateString(selectedRun.finished) + : msg("In progress")}` : msg("Select a QA run")} @@ -52,7 +54,9 @@ export class QaRunDropdown extends TailwindElement { ?disabled=${isSelected} ?checked=${isSelected} > - ${formatISODateString(run.finished)} + ${run.finished + ? formatISODateString(run.finished) + : msg("In progress")} - 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(); diff --git a/frontend/src/pages/org/archived-item-detail/ui/qa.ts b/frontend/src/pages/org/archived-item-detail/ui/qa.ts index 5c5e841c..25a5dd1c 100644 --- 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; 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` - ${msg( - "This crawl is being analyzed. You're currently viewing results from an older analysis run.", - )} - ` - : nothing}
@@ -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`
) => (this.qaRunId = e.detail.item.id)} @@ -566,12 +569,12 @@ export class ArchivedItemDetailQA extends TailwindElement { ${msg("Screenshots")} - ${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()} @@ -579,12 +582,12 @@ export class ArchivedItemDetailQA extends TailwindElement { ${msg("Text")} - ${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()} @@ -627,30 +630,32 @@ export class ArchivedItemDetailQA extends TailwindElement { ); const idx = threshold ? qaStatsThresholds.indexOf(threshold) : -1; - return html` - -
- ${bar.lowerBoundary === "No data" - ? msg("No Data") - : threshold?.label} -
- ${!["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")}
` - : nothing} - ${formatNumber(bar.count)} ${pluralOf("pages", bar.count)} -
-
- - `; + return bar.count !== 0 + ? html` + +
+ ${bar.lowerBoundary === "No data" + ? msg("No Data") + : threshold?.label} +
+ ${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
` + : nothing} + ${formatNumber(bar.count)} ${pluralOf("pages", bar.count)} +
+
+ + ` + : nothing; })} `;