Skip to content

Commit e55f8b2

Browse files
jefho-rhclaudenickmazzi
authored
fix(automl): fix feature summary infinite loading for regression runs (#7234)
* fix(automl): fix feature summary infinite loading for regression runs Co-Authored-By: Claude <noreply@anthropic.com> * test(automl): add regression test for evaluation artifacts loading state Covers the isPending vs isLoading bug in useModelEvaluationArtifactsQuery where disabled queries (e.g. confusion matrix for regression runs) caused isLoading to be stuck at true indefinitely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(automl): prevent "-0.00%" display for near-zero feature importance values Clamp near-zero importance percentages to 0 before formatting, so values like -1.7e-06 display as "0.00%" instead of "-0.00%". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(automl): assert fetch call counts in evaluation artifacts tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(automl): don't style near-zero importance bars as negative Apply the same near-zero threshold to the bar's m-negative class so it doesn't appear red when the value displays as 0.00%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nicholas Mazzitelli <nickmazz@ca.ibm.com>
1 parent 4282d4c commit e55f8b2

4 files changed

Lines changed: 120 additions & 3 deletions

File tree

packages/automl/frontend/src/app/components/run-results/AutomlModelDetailsModal/tabs/FeatureSummaryTab.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,13 @@ const FeatureSummaryTab: React.FC<TabContentProps> = ({
137137
entries.map(([name, importance]) => (
138138
<Tr key={name}>
139139
<Td dataLabel="Feature name">{name}</Td>
140-
<Td dataLabel="Importance">{(importance * 100).toFixed(2)}%</Td>
140+
{/* Clamp near-zero values to 0 to avoid displaying "-0.00%" */}
141+
<Td dataLabel="Importance">
142+
{(Math.abs(importance * 100) < 0.005 ? 0 : importance * 100).toFixed(2)}%
143+
</Td>
141144
<Td>
142145
<div
143-
className={`automl-feature-importance-bar${importance !== 0 ? ' m-has-value' : ''}${importance < 0 ? ' m-negative' : ''}`}
146+
className={`automl-feature-importance-bar${importance !== 0 ? ' m-has-value' : ''}${importance < 0 && Math.abs(importance * 100) >= 0.005 ? ' m-negative' : ''}`}
144147
data-testid={`feature-importance-bar-${name}`}
145148
style={{
146149
width: `${maxImportance > 0 ? (Math.abs(importance) / maxImportance) * 100 : 0}%`,

packages/automl/frontend/src/app/components/run-results/AutomlModelDetailsModal/tabs/__tests__/FeatureSummaryTab.spec.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,29 @@ describe('FeatureSummaryTab', () => {
195195
expect(featureNames).toEqual(['feat_a', 'feat_c', 'feat_b', 'feat_d']);
196196
});
197197

198+
it('should display near-zero negative values as 0.00% instead of -0.00%', () => {
199+
const nearZeroImportance: FeatureImportanceData = {
200+
importance: {
201+
large_positive: 0.75,
202+
tiny_negative: -1.7133e-6,
203+
tiny_positive: 4.066e-7,
204+
},
205+
};
206+
207+
render(<FeatureSummaryTab {...defaultProps} featureImportance={nearZeroImportance} />);
208+
209+
expect(screen.getByText('75.00%')).toBeInTheDocument();
210+
// Near-zero values should show as 0.00%, not -0.00%
211+
const rows = screen.getAllByRole('row').slice(1);
212+
const percentages = rows.map((row) => within(row).getAllByRole('cell')[1].textContent);
213+
expect(percentages).toEqual(['75.00%', '0.00%', '0.00%']);
214+
expect(screen.queryByText('-0.00%')).not.toBeInTheDocument();
215+
216+
// Near-zero negative bar should not be styled as negative
217+
const tinyNegativeBar = screen.getByTestId('feature-importance-bar-tiny_negative');
218+
expect(tinyNegativeBar).not.toHaveClass('m-negative');
219+
});
220+
198221
it('should handle a single feature with only negative importance', () => {
199222
const singleNegative: FeatureImportanceData = {
200223
importance: { only_feature: -1.0 },

packages/automl/frontend/src/app/hooks/__tests__/queries.spec.tsx

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
33
import React from 'react';
44
import {
55
useS3GetFileSchemaQuery,
6+
useModelEvaluationArtifactsQuery,
67
fetchS3File,
78
AutomlModelSchema,
89
isRawTimeseriesModel,
@@ -512,3 +513,93 @@ describe('isRawTimeseriesModel', () => {
512513
});
513514
});
514515
/* eslint-enable camelcase */
516+
517+
/* eslint-disable camelcase */
518+
describe('useModelEvaluationArtifactsQuery', () => {
519+
const mockFeatureImportance = {
520+
importance: { feature_a: 0.8, feature_b: 0.2 },
521+
};
522+
523+
const mockConfusionMatrix = {
524+
cat: { cat: 10, dog: 2 },
525+
dog: { cat: 1, dog: 15 },
526+
};
527+
528+
/**
529+
* Creates a mock fetch response that returns JSON parsed from a Blob,
530+
* matching the fetchS3Json → fetchS3File → fetch call chain.
531+
*/
532+
const mockBlobJsonResponse = (data: unknown) => {
533+
const json = JSON.stringify(data);
534+
return {
535+
ok: true,
536+
blob: async () => ({ text: async () => json }),
537+
};
538+
};
539+
540+
beforeEach(() => {
541+
jest.clearAllMocks();
542+
});
543+
544+
it('should not get stuck loading for regression runs (isClassification=false)', async () => {
545+
// The confusion matrix query is disabled for regression. Previously, using
546+
// isPending (which is true for disabled queries) caused isLoading to be
547+
// stuck at true. The fix uses isLoading instead.
548+
(global.fetch as jest.Mock).mockResolvedValueOnce(mockBlobJsonResponse(mockFeatureImportance));
549+
550+
const { result } = renderHook(
551+
() => useModelEvaluationArtifactsQuery('test-ns', 'models/best/', false),
552+
{ wrapper: createWrapper() },
553+
);
554+
555+
await waitFor(() => {
556+
expect(result.current.isLoading).toBe(false);
557+
});
558+
559+
expect(result.current.featureImportance).toEqual(mockFeatureImportance);
560+
expect(result.current.confusionMatrix).toBeUndefined();
561+
expect(global.fetch).toHaveBeenCalledTimes(1);
562+
});
563+
564+
it('should load both artifacts for classification runs (isClassification=true)', async () => {
565+
(global.fetch as jest.Mock)
566+
.mockResolvedValueOnce(mockBlobJsonResponse(mockFeatureImportance))
567+
.mockResolvedValueOnce(mockBlobJsonResponse(mockConfusionMatrix));
568+
569+
const { result } = renderHook(
570+
() => useModelEvaluationArtifactsQuery('test-ns', 'models/best/', true),
571+
{ wrapper: createWrapper() },
572+
);
573+
574+
await waitFor(() => {
575+
expect(result.current.isLoading).toBe(false);
576+
});
577+
578+
expect(result.current.featureImportance).toEqual(mockFeatureImportance);
579+
expect(result.current.confusionMatrix).toEqual(mockConfusionMatrix);
580+
expect(global.fetch).toHaveBeenCalledTimes(2);
581+
});
582+
583+
it('should be disabled when namespace is missing', () => {
584+
const { result } = renderHook(
585+
() => useModelEvaluationArtifactsQuery(undefined, 'models/best/', true),
586+
{ wrapper: createWrapper() },
587+
);
588+
589+
expect(result.current.isLoading).toBe(false);
590+
expect(result.current.featureImportance).toBeUndefined();
591+
expect(result.current.confusionMatrix).toBeUndefined();
592+
});
593+
594+
it('should be disabled when modelDirectory is missing', () => {
595+
const { result } = renderHook(
596+
() => useModelEvaluationArtifactsQuery('test-ns', undefined, true),
597+
{ wrapper: createWrapper() },
598+
);
599+
600+
expect(result.current.isLoading).toBe(false);
601+
expect(result.current.featureImportance).toBeUndefined();
602+
expect(result.current.confusionMatrix).toBeUndefined();
603+
});
604+
});
605+
/* eslint-enable camelcase */

packages/automl/frontend/src/app/hooks/queries.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ export function useModelEvaluationArtifactsQuery(
370370
combine: (results) => ({
371371
featureImportance: results[0].data,
372372
confusionMatrix: results[1].data,
373-
isLoading: results.some((r) => r.isPending),
373+
isLoading: results.some((r) => r.isLoading),
374374
}),
375375
});
376376
}

0 commit comments

Comments
 (0)