Refresh the Bias impact tab when a report is produced
The list fetched only when executionId changed. The experiment and compare dialogs are global hosts rendered over the still-mounted aside, so the sequence that actually happens - open the tab, run an experiment, read the report in the dialog, close it - returned the user to a tab still claiming there were no reports. There was no refresh either: retry() is rendered only in the error branch. A small shared signal announces that a report now exists. The dialogs raise it rather than the viewer, because they are what knows one was actually produced - a failed experiment produces none - and it keeps the viewer out of a path it has no part in. A reload of the same execution keeps an open report open; only a change of execution closes it, since that is a different subject. 500 frontend tests green. The reload assertion fails when the revision is ignored again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c35bbc0ccf
commit
53ac229790
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
|
|
|
|||
|
|
@ -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> = {}): 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();
|
||||
|
|
|
|||
|
|
@ -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<string | null>(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);
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue