diff --git a/src/app/services/bias/bias-reports-revision.ts b/src/app/services/bias/bias-reports-revision.ts new file mode 100644 index 0000000..b2251ab --- /dev/null +++ b/src/app/services/bias/bias-reports-revision.ts @@ -0,0 +1,22 @@ +import { Injectable, signal } from '@angular/core'; + +/** + * Announces that a bias impact report has just come into existence. + * + * The experiment and compare dialogs are global hosts rendered over the execution aside, which + * stays mounted underneath. Without this, running an experiment left the Bias impact tab still + * saying there were no reports, and the only way to see the new one was to leave the tab and come + * back. + * + * The dialogs notify rather than the viewer, because they are what knows a report was actually + * produced: an experiment that failed produced none. + */ +@Injectable({ providedIn: 'root' }) +export class BiasReportsRevisionService { + private readonly _revision = signal(0); + readonly revision = this._revision.asReadonly(); + + reportProduced() { + this._revision.update((current) => current + 1); + } +} diff --git a/src/app/shared/bias-compare-dialog/bias-compare-dialog.ts b/src/app/shared/bias-compare-dialog/bias-compare-dialog.ts index fdbf8be..6503189 100644 --- a/src/app/shared/bias-compare-dialog/bias-compare-dialog.ts +++ b/src/app/shared/bias-compare-dialog/bias-compare-dialog.ts @@ -4,6 +4,7 @@ import { BiasImpactReportViewerComponent } from '@shared/bias-impact-report-view import { ModalShellComponent } from '@shared/modal-shell/modal-shell'; import { BiasCompareDialogService } from '@services/dialogs/bias-compare-dialog'; import { BiasComparisonViewStateService } from '@services/bias/bias-comparison-view-state'; +import { BiasReportsRevisionService } from '@services/bias/bias-reports-revision'; import { extractBiasErrorMessage } from '@services/bias/bias-error.util'; import { TaskExecutionsService } from '@services/task-executions/task-executions'; import { BiasImpactReport } from '@models/bias-impact'; @@ -20,6 +21,7 @@ export class BiasCompareDialogHostComponent { private readonly dialog = inject(BiasCompareDialogService); private readonly executions = inject(TaskExecutionsService); private readonly comparisonViewState = inject(BiasComparisonViewStateService); + private readonly reportsRevision = inject(BiasReportsRevisionService); readonly state = this.dialog.state; readonly loading = signal(false); @@ -62,6 +64,8 @@ export class BiasCompareDialogHostComponent { next: (report) => { this.loading.set(false); this.report.set(report); + // The comparison is persisted server-side, so it is a new row the tab behind should list. + this.reportsRevision.reportProduced(); }, error: (error) => { this.loading.set(false); diff --git a/src/app/shared/bias-impact-experiment-dialog/bias-impact-experiment-dialog.ts b/src/app/shared/bias-impact-experiment-dialog/bias-impact-experiment-dialog.ts index 0d7f59c..4bacf33 100644 --- a/src/app/shared/bias-impact-experiment-dialog/bias-impact-experiment-dialog.ts +++ b/src/app/shared/bias-impact-experiment-dialog/bias-impact-experiment-dialog.ts @@ -7,6 +7,7 @@ import { ModalShellComponent } from '@shared/modal-shell/modal-shell'; import { ConfirmDialogService } from '@services/dialogs/confirm-dialog'; import { BiasImpactExperimentDialogService } from '@services/dialogs/bias-impact-experiment-dialog'; import { BiasComparisonViewStateService } from '@services/bias/bias-comparison-view-state'; +import { BiasReportsRevisionService } from '@services/bias/bias-reports-revision'; import { extractBiasErrorMessage } from '@services/bias/bias-error.util'; import { NotificationService } from '@services/notifications/notification'; import { TaskExecutionsService } from '@services/task-executions/task-executions'; @@ -27,6 +28,7 @@ export class BiasImpactExperimentDialogHostComponent { private readonly confirmation = inject(ConfirmDialogService); private readonly notifications = inject(NotificationService); private readonly comparisonViewState = inject(BiasComparisonViewStateService); + private readonly reportsRevision = inject(BiasReportsRevisionService); private readonly destroyRef = inject(DestroyRef); private pollSubscription: Subscription | null = null; @@ -129,7 +131,12 @@ export class BiasImpactExperimentDialogHostComponent { if (!nextJob.terminal) return; this.submitting.set(false); this.cancelPolling(); - if (nextJob.status === 'COMPLETED' && nextJob.report) this.report.set(nextJob.report); + if (nextJob.status === 'COMPLETED' && nextJob.report) { + this.report.set(nextJob.report); + // The Bias impact tab is mounted behind this dialog and would otherwise keep saying + // there are no reports for this execution. + this.reportsRevision.reportProduced(); + } else this.inlineError.set(nextJob.errorMessage || 'The bias impact experiment failed.'); }, error: (error) => { diff --git a/src/app/shared/bias-impact-report-list/bias-impact-report-list.spec.ts b/src/app/shared/bias-impact-report-list/bias-impact-report-list.spec.ts index 2c43d9b..7fe2e22 100644 --- a/src/app/shared/bias-impact-report-list/bias-impact-report-list.spec.ts +++ b/src/app/shared/bias-impact-report-list/bias-impact-report-list.spec.ts @@ -4,6 +4,7 @@ import { vi } from 'vitest'; import { BiasImpactReportListComponent } from './bias-impact-report-list'; import { TaskExecutionsService } from '@services/task-executions/task-executions'; import { BiasImpactReport } from '@models/bias-impact'; +import { BiasReportsRevisionService } from '@services/bias/bias-reports-revision'; function makeReport(overrides: Partial = {}): BiasImpactReport { return { @@ -52,6 +53,47 @@ describe('BiasImpactReportListComponent', () => { fixture = TestBed.createComponent(BiasImpactReportListComponent); }); + it('reloads when a dialog reports that one has just been produced', () => { + // The experiment and compare dialogs render over this still-mounted tab. Without this the tab + // kept saying there were no reports for the execution whose report the user was just reading. + fixture.componentRef.setInput('executionId', 'execution-1'); + fixture.detectChanges(); + expect(listBiasImpactReports).toHaveBeenCalledTimes(1); + + listBiasImpactReports.mockReturnValue(of([makeReport(), makeReport({ id: 'report-2' })])); + TestBed.inject(BiasReportsRevisionService).reportProduced(); + fixture.detectChanges(); + + expect(listBiasImpactReports).toHaveBeenCalledTimes(2); + expect(fixture.componentInstance.reports().map((one) => one.id)).toEqual(['report-1', 'report-2']); + }); + + it('does not refetch on a render that changed neither the execution nor the revision', () => { + fixture.componentRef.setInput('executionId', 'execution-1'); + fixture.detectChanges(); + fixture.detectChanges(); + + expect(listBiasImpactReports).toHaveBeenCalledTimes(1); + }); + + it('keeps an open report open across a reload, and closes it when the execution changes', () => { + fixture.componentRef.setInput('executionId', 'execution-1'); + fixture.detectChanges(); + fixture.componentInstance.openDetail('report-1'); + fixture.detectChanges(); + expect(fixture.componentInstance.selectedReport()).not.toBeNull(); + + // A reload of the same run must not yank away what the user is reading. + TestBed.inject(BiasReportsRevisionService).reportProduced(); + fixture.detectChanges(); + expect(fixture.componentInstance.selectedReport()).not.toBeNull(); + + // A different run is a clean slate. + fixture.componentRef.setInput('executionId', 'execution-2'); + fixture.detectChanges(); + expect(fixture.componentInstance.selectedReport()).toBeNull(); + }); + it('loads and renders the reports for the given execution', () => { fixture.componentRef.setInput('executionId', 'execution-1'); fixture.detectChanges(); diff --git a/src/app/shared/bias-impact-report-list/bias-impact-report-list.ts b/src/app/shared/bias-impact-report-list/bias-impact-report-list.ts index 3bd70ed..a680f04 100644 --- a/src/app/shared/bias-impact-report-list/bias-impact-report-list.ts +++ b/src/app/shared/bias-impact-report-list/bias-impact-report-list.ts @@ -4,6 +4,7 @@ import { MatButtonModule } from '@angular/material/button'; import { BiasImpactReport } from '@models/bias-impact'; import { TaskExecutionsService } from '@services/task-executions/task-executions'; import { BiasComparisonViewStateService } from '@services/bias/bias-comparison-view-state'; +import { BiasReportsRevisionService } from '@services/bias/bias-reports-revision'; import { BiasImpactReportViewerComponent } from '@shared/bias-impact-report-viewer/bias-impact-report-viewer'; @Component({ @@ -17,7 +18,9 @@ import { BiasImpactReportViewerComponent } from '@shared/bias-impact-report-view export class BiasImpactReportListComponent { private readonly executions = inject(TaskExecutionsService); private readonly comparisonViewState = inject(BiasComparisonViewStateService); + private readonly reportsRevision = inject(BiasReportsRevisionService); private lastExecutionId: string | null = null; + private lastReloadToken = 0; readonly executionId = input(null); @@ -36,9 +39,15 @@ export class BiasImpactReportListComponent { constructor() { effect(() => { const executionId = this.executionId(); - if (executionId === this.lastExecutionId) return; + const reloadToken = this.reportsRevision.revision(); + const executionChanged = executionId !== this.lastExecutionId; + const reloadRequested = reloadToken !== this.lastReloadToken; + if (!executionChanged && !reloadRequested) return; this.lastExecutionId = executionId; - this.closeDetail(); + this.lastReloadToken = reloadToken; + // A different execution starts from a clean slate; a reload of the same one keeps whatever + // report the user was reading open. + if (executionChanged) this.closeDetail(); this.loadReports(executionId); }); }