Skip to content

Commit e74126b

Browse files
nickmazziclaude
andcommitted
fix(automl): improve top_n and prediction_length validation UX
- Re-validate top_n automatically when task_type changes via trigger() - Restore dynamic max prop on ExperimentSettings NumberInput using useWatch - Extract MAX_PREDICTION_LENGTH constant for consistency with top_n constants - Fix and uncomment ExperimentSettings max validation test - Update switching tests to verify auto-revalidation without manual focus/blur Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e002621 commit e74126b

6 files changed

Lines changed: 41 additions & 27 deletions

File tree

packages/automl/frontend/src/app/components/configure/AutomlConfigure.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ function AutomlConfigure(): React.JSX.Element {
176176
control,
177177
setValue,
178178
getValues,
179+
trigger,
179180
formState: { isSubmitting: formIsSubmitting },
180181
} = form;
181182

@@ -189,6 +190,13 @@ function AutomlConfigure(): React.JSX.Element {
189190
// Calculate max top_n based on task type
190191
const maxTopN = isTimeseries ? MAX_TOP_N_TIMESERIES : MAX_TOP_N_TABULAR;
191192

193+
// Re-validate top_n when task type changes (max depends on task type)
194+
useEffect(() => {
195+
if (isTaskTypeSelected) {
196+
void trigger('top_n');
197+
}
198+
}, [taskType, isTaskTypeSelected, trigger]);
199+
192200
const canSelectFiles = !selectedSecret?.invalid && Boolean(trainDataSecretName);
193201
const isFileSelected = Boolean(trainDataFileKey);
194202
const showTrainingDataUploadDropzone = !isTrainingDataFileUploading && !trainDataFileKey.trim();

packages/automl/frontend/src/app/components/configure/AutomlExperimentSettings.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@ import {
1212
Title,
1313
} from '@patternfly/react-core';
1414
import React from 'react';
15-
import { Controller, useFormContext } from 'react-hook-form';
15+
import { Controller, useFormContext, useWatch } from 'react-hook-form';
1616
import {
1717
ConfigureSchema,
1818
EXPERIMENT_SETTINGS_FIELDS,
19+
MAX_TOP_N_TABULAR,
20+
MAX_TOP_N_TIMESERIES,
1921
MIN_TOP_N,
2022
} from '~/app/schemas/configure.schema';
23+
import { TASK_TYPE_TIMESERIES } from '~/app/utilities/const';
2124

2225
type AutomlExperimentSettingsProps = {
2326
isOpen: boolean;
@@ -37,6 +40,9 @@ const AutomlExperimentSettings: React.FC<AutomlExperimentSettingsProps> = ({
3740
formState: { isDirty, errors },
3841
} = useFormContext<ConfigureSchema>();
3942

43+
const taskType = useWatch({ control, name: 'task_type' });
44+
const maxTopN = taskType === TASK_TYPE_TIMESERIES ? MAX_TOP_N_TIMESERIES : MAX_TOP_N_TABULAR;
45+
4046
const hasFieldErrors = EXPERIMENT_SETTINGS_FIELDS.some((field) => errors[field]);
4147

4248
return (
@@ -63,6 +69,7 @@ const AutomlExperimentSettings: React.FC<AutomlExperimentSettingsProps> = ({
6369
id="top-n-input"
6470
value={field.value}
6571
min={MIN_TOP_N}
72+
max={maxTopN}
6673
validated={fieldState.error ? 'error' : 'default'}
6774
onMinus={() => field.onChange(field.value - 1)}
6875
onPlus={() => field.onChange(field.value + 1)}

packages/automl/frontend/src/app/components/configure/ConfigureTimeseriesForm.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import React, { useState } from 'react';
1414
import { Controller, useFormContext } from 'react-hook-form';
1515
import ConfigureFormGroup from '~/app/components/common/ConfigureFormGroup';
16+
import { MAX_PREDICTION_LENGTH } from '~/app/schemas/configure.schema';
1617
import { getTypeAcronym } from '~/app/utilities/columnUtils';
1718
import LoadingFormField from './LoadingFormField';
1819

@@ -408,7 +409,7 @@ function ConfigureTimeseriesForm({
408409
id="prediction-length-input"
409410
value={field.value}
410411
min={1}
411-
max={100}
412+
max={MAX_PREDICTION_LENGTH}
412413
validated={fieldState.error ? 'error' : 'default'}
413414
onMinus={() => field.onChange(Number(field.value) - 1)}
414415
onPlus={() => field.onChange(Number(field.value) + 1)}

packages/automl/frontend/src/app/components/configure/__tests__/AutomlConfigure.spec.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ describe('AutomlConfigure', () => {
931931
});
932932
});
933933

934-
it('should update max validation when switching from tabular to timeseries', async () => {
934+
it('should automatically show error when switching from tabular to timeseries with top N exceeding new max', async () => {
935935
renderComponent();
936936
selectSecretAndFile();
937937
selectPredictionType('binary');
@@ -945,19 +945,15 @@ describe('AutomlConfigure', () => {
945945
expect(screen.queryByText('Maximum number of top models is 10')).not.toBeInTheDocument();
946946
expect(screen.queryByText('Maximum number of top models is 7')).not.toBeInTheDocument();
947947

948-
// Switch to timeseries - should now show error (max 7)
948+
// Switch to timeseries - error should appear automatically without touching the field
949949
selectPredictionType('timeseries');
950950

951-
// Trigger revalidation by touching the field
952-
fireEvent.focus(input);
953-
fireEvent.blur(input);
954-
955951
await waitFor(() => {
956952
expect(screen.getByText('Maximum number of top models is 7')).toBeInTheDocument();
957953
});
958954
});
959955

960-
it('should update max validation when switching from timeseries to tabular', async () => {
956+
it('should automatically clear error when switching from timeseries to tabular with top N within new max', async () => {
961957
renderComponent();
962958
selectSecretAndFile();
963959
selectPredictionType('timeseries');
@@ -972,13 +968,9 @@ describe('AutomlConfigure', () => {
972968
expect(screen.getByText('Maximum number of top models is 7')).toBeInTheDocument();
973969
});
974970

975-
// Switch to binary - should now be valid (max 10)
971+
// Switch to binary - error should clear automatically without touching the field
976972
selectPredictionType('binary');
977973

978-
// Trigger revalidation by touching the field
979-
fireEvent.focus(input);
980-
fireEvent.blur(input);
981-
982974
await waitFor(() => {
983975
expect(
984976
screen.queryByText('Maximum number of top models is 10'),

packages/automl/frontend/src/app/components/configure/__tests__/AutomlExperimentSettings.spec.tsx

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
1+
/* eslint-disable camelcase */
12
import { zodResolver } from '@hookform/resolvers/zod';
23
import '@testing-library/jest-dom';
34
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
45
import React from 'react';
56
import { FormProvider, useForm } from 'react-hook-form';
67
import AutomlExperimentSettings from '~/app/components/configure/AutomlExperimentSettings';
7-
import { createConfigureSchema } from '~/app/schemas/configure.schema';
8+
import { ConfigureSchema, createConfigureSchema } from '~/app/schemas/configure.schema';
89

910
const configureSchema = createConfigureSchema();
1011

11-
const FormWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => {
12+
const FormWrapper: React.FC<{
13+
children: React.ReactNode;
14+
defaultValues?: Partial<ConfigureSchema>;
15+
}> = ({ children, defaultValues }) => {
1216
const form = useForm({
1317
mode: 'onChange',
1418
resolver: zodResolver(configureSchema.full),
15-
defaultValues: configureSchema.defaults,
19+
defaultValues: { ...configureSchema.defaults, ...defaultValues },
1620
});
1721
return <FormProvider {...form}>{children}</FormProvider>;
1822
};
@@ -24,9 +28,9 @@ const defaultProps = {
2428
saveChanges: jest.fn(),
2529
};
2630

27-
const renderComponent = (props = {}) =>
31+
const renderComponent = (props = {}, defaultValues?: Partial<ConfigureSchema>) =>
2832
render(
29-
<FormWrapper>
33+
<FormWrapper defaultValues={defaultValues}>
3034
<AutomlExperimentSettings {...defaultProps} {...props} />
3135
</FormWrapper>,
3236
);
@@ -80,14 +84,15 @@ describe('AutomlExperimentSettings', () => {
8084
});
8185
});
8286

83-
// it('should show error message when top N exceeds the maximum', async () => {
84-
// renderComponent();
85-
// changeTopN('6');
87+
it('should show error message when top N exceeds the maximum', async () => {
88+
// Provide a valid task_type so the cross-field top_n max validator runs
89+
renderComponent({}, { task_type: 'binary' });
90+
changeTopN('11');
8691

87-
// await waitFor(() => {
88-
// expect(screen.getByText('Maximum number of top models is 5')).toBeInTheDocument();
89-
// });
90-
// });
92+
await waitFor(() => {
93+
expect(screen.getByText('Maximum number of top models is 10')).toBeInTheDocument();
94+
});
95+
});
9196

9297
it('should show error message when top N is below the minimum', async () => {
9398
renderComponent();

packages/automl/frontend/src/app/schemas/configure.schema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { createSchema } from '~/app/utilities/schema';
1111
export const MIN_TOP_N = 1;
1212
export const MAX_TOP_N_TABULAR = 10;
1313
export const MAX_TOP_N_TIMESERIES = 7;
14+
export const MAX_PREDICTION_LENGTH = 100;
1415

1516
export const EXPERIMENT_SETTINGS_FIELDS = ['top_n'] as const;
1617

@@ -39,7 +40,7 @@ function createConfigureSchema() {
3940
target: z.string().default('').optional(),
4041
id_column: z.string().default('').optional(),
4142
timestamp_column: z.string().default('').optional(),
42-
prediction_length: z.int().min(1).max(100).default(1).optional(),
43+
prediction_length: z.int().min(1).max(MAX_PREDICTION_LENGTH).default(1).optional(),
4344
known_covariates_names: z.array(z.string()).default([]).optional(),
4445
}),
4546
validators: [

0 commit comments

Comments
 (0)