Add component test of augmentation parameters and add minor fixes #1353

romanowska · 2025-10-02T09:39:23Z

📝 Description

Addressing comments from Support augmentation advanced settings #1250
change type of minValue from number to number | null

✨ Type of Change

Select the type of change your PR introduces:

🐞 Bug fix – Non-breaking change which fixes an issue
🚀 New feature – Non-breaking change which adds functionality
🔨 Refactor – Non-breaking change which refactors the code base
💥 Breaking change – Changes that break existing functionality
📚 Documentation update
🔒 Security update
🧪 Tests

🧪 Testing Scenarios

Describe how the changes were tested and how reviewers can test them too:

✅ Tested manually
🤖 Run automated end-to-end tests

✅ Checklist

Before submitting the PR, ensure the following:

🔍 PR title is clear and meaningful
✍️ PR description clearly explains the changes and their reason
📝 I have linked the PR to the corresponding GitHub Issues, if any
💬 I have commented my code, especially in hard-to-understand areas
📄 I have made corresponding changes to the documentation
✅ I have added tests that prove my fix is effective or my feature works

Copilot

Pull Request Overview

Refactor to allow configurable numeric parameters to have nullable minimum values and update related UI components, tests, and training configuration mocks accordingly. Also expands augmentation configuration options and adjusts range slider accessibility labels.

Add support for minValue/min_value being null across DTOs, services, and UI components.
Update NumberParameterField and RangeParameterField logic and associated tests for new nullable constraints and revised aria-labels.
Replace and expand augmentation configuration mocks (center_crop removed; new augmentations and parameters added).

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
web_ui/tests/fixtures/page-objects/train-model-dialog-page.ts	Adjust page object method to require external slider Locator.
web_ui/tests/features/project-models/train-model.spec.ts	Update tests to use new method signature and interact with new augmentation controls.
web_ui/tests/features/project-models/mocks.ts	Overhaul augmentation configuration mocks and expected payload structure.
web_ui/src/shared/components/header/active-learning-configuration/training-settings/required-annotations.component.tsx	Support nullable minValue by passing undefined when null.
web_ui/src/pages/project-details/components/project-models/train-model-dialog/advanced-settings/ui/range-parameter-field.test.tsx	Update tests to match new aria-labels and rendering helper.
web_ui/src/pages/project-details/components/project-models/train-model-dialog/advanced-settings/ui/range-parameter-field.component.tsx	Change slider aria-label and wrap in Flex with aria-label.
web_ui/src/pages/project-details/components/project-models/train-model-dialog/advanced-settings/ui/number-parameter-field.component.tsx	Support nullable min/max and modify step calculation logic.
web_ui/src/pages/project-details/components/project-models/train-model-dialog/advanced-settings/data-management/filters/filters-options.component.tsx	Pass undefined when minValue is null.
web_ui/src/core/configurable-parameters/services/configuration.interface.ts	Change minValue type to number
web_ui/src/core/configurable-parameters/dtos/configuration.interface.ts	Change min_value type to number

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

.../project-models/train-model-dialog/advanced-settings/ui/number-parameter-field.component.tsx

Copilot · 2025-10-02T09:41:10Z

web_ui/tests/fixtures/page-objects/train-model-dialog-page.ts

+    async changeSubsetRange(slider: Locator, range: 'start' | 'end', iterations: number) {
+        await slider.getByLabel(range === 'start' ? 'Start range' : 'End range').click();


[nitpick] Adding a required slider Locator shifts locator responsibility to every test and duplicates retrieval logic; consider keeping the page object self-contained by resolving the slider internally (or offering an overload) to reduce coupling and future selector churn.

Suggested change

async changeSubsetRange(slider: Locator, range: 'start' | 'end', iterations: number) {

await slider.getByLabel(range === 'start' ? 'Start range' : 'End range').click();

async changeSubsetRange(range: 'start' | 'end', iterations: number) {

// Assume the slider can be found by label, e.g., "Start range" or "End range"

const slider = this.page.getByLabel(range === 'start' ? 'Start range' : 'End range');

await slider.click();

Copilot · 2025-10-02T09:41:10Z

web_ui/tests/features/project-models/train-model.spec.ts

+            const distributionSlider = page.getByText('Distribution 70/20/10%');
+            await trainModelPage.changeSubsetRange(distributionSlider, 'start', 10);
+
+            const refreshedDistributionSlider = page.getByText('Distribution 60/30/10%');


[nitpick] Using getByText with the full dynamic distribution string is brittle (any formatting or recalculation change will break the test); prefer a stable aria-label/role or a test id (e.g. getByLabel('Training subsets distribution slider')) to improve selector resilience.

Suggested change

const distributionSlider = page.getByText('Distribution 70/20/10%');

await trainModelPage.changeSubsetRange(distributionSlider, 'start', 10);

const refreshedDistributionSlider = page.getByText('Distribution 60/30/10%');

const distributionSlider = page.getByTestId('training-subsets-distribution-slider');

await trainModelPage.changeSubsetRange(distributionSlider, 'start', 10);

const refreshedDistributionSlider = page.getByTestId('training-subsets-distribution-slider');

Copilot · 2025-10-02T09:41:10Z

web_ui/tests/features/project-models/mocks.ts

+                    name: 'Enable random horizontal flip',
+                    type: 'bool',
+                    description:
+                        'Whether to apply random flip images horizontally along the vertical axis (swap left and right)',


[nitpick] The phrasing is ungrammatical; consider revising to "Whether to randomly flip images horizontally along the vertical axis (swap left and right)" and "Whether to randomly flip images vertically along the horizontal axis (swap top and bottom)" for clarity.

Suggested change

'Whether to apply random flip images horizontally along the vertical axis (swap left and right)',

'Whether to randomly flip images horizontally along the vertical axis (swap left and right)',

Copilot · 2025-10-02T09:41:11Z

web_ui/tests/features/project-models/mocks.ts

+                    description:
+                        'Whether to apply random flip images vertically along the horizontal axis (swap top and bottom)',


[nitpick] The phrasing is ungrammatical; consider revising to "Whether to randomly flip images horizontally along the vertical axis (swap left and right)" and "Whether to randomly flip images vertically along the horizontal axis (swap top and bottom)" for clarity.

Ensure both single task training and task chain training tests use the same mocks and behavior, fixing inconsistencies that previously caused test failures.

dwesolow · 2025-10-09T06:51:42Z

...s/project-models/train-model-dialog/advanced-settings/ui/range-parameter-field.component.tsx


    return (
-        <Flex gap={'size-100'}>
+        <Flex gap={'size-100'} aria-label={name}>


IIRC flex does not have aria-label, could u please double check it?

it doesn't. updated

dwesolow · 2025-10-09T06:52:11Z

web_ui/src/core/configurable-parameters/dtos/configuration.interface.ts

    type: 'int' | 'float';
    value: number;
-    min_value: number;
+    min_value: number | null;


When min_value can be null?

for Random Affine Maximum shear degree, Gaussian noise mean

dwesolow · 2025-10-09T06:55:01Z

web_ui/tests/features/project-models/train-model.spec.ts

+            await brightnessStart.click();
+
+            const brightnessSlider = page.getByLabel('Change Brightness range value');
+            brightnessSlider.click();


Missing await?

dwesolow · 2025-10-09T06:57:49Z

web_ui/tests/features/project-models/train-model.spec.ts

+            await brightnessStart.click();
+
+            const brightnessSlider = page.getByLabel('Change Brightness range value');
+            brightnessSlider.click();


Same here, await?

…-platform/geti into kromano/add_augs_tests

romanowska added 6 commits September 26, 2025 10:25

use providerRender function instead of RequiredProviders

b16a3b7

update mocks

b3148d2

update NumberParameter interface min value to allow null

17f2cda

update minValue type to number | null

f902a06

update aria labels

0f0fd86

add and update component tests

6db31d2

Copilot AI review requested due to automatic review settings October 2, 2025 09:39

romanowska requested review from ActiveChooN, MarkRedeman, camiloHimura, dwesolow, jpggvilaca and pplaskie as code owners October 2, 2025 09:39

github-actions bot added the UI label Oct 2, 2025

romanowska changed the title ~~Add~~ Add component test of augmentation parameters and add minor fixes Oct 2, 2025

update condition for values update

43a96f4

Copilot AI reviewed Oct 2, 2025

View reviewed changes

romanowska added the ready for review label Oct 2, 2025

romanowska self-assigned this Oct 2, 2025

romanowska removed the ready for review label Oct 2, 2025

romanowska and others added 2 commits October 3, 2025 08:47

fix getStep fn

8c17244

fix(tests): Align mocks and behavior in component tests

091ccf4

Ensure both single task training and task chain training tests use the same mocks and behavior, fixing inconsistencies that previously caused test failures.

romanowska added the ready for review label Oct 7, 2025

dwesolow reviewed Oct 9, 2025

View reviewed changes

pplaskie previously approved these changes Oct 9, 2025

View reviewed changes

romanowska added 2 commits October 16, 2025 09:36

Merge branch 'main' into kromano/add_augs_tests

0f289aa

Merge branch 'kromano/add_augs_tests' of https://github.com/open-edge…

66e5efd

…-platform/geti into kromano/add_augs_tests

romanowska added 2 commits October 16, 2025 09:40

add missing awaits

059a5d3

remove unnecessary props

760a7b5

romanowska dismissed pplaskie’s stale review via 760a7b5 October 16, 2025 07:44

fixed resolved merge conflict

681e007

romanowska linked an issue Oct 16, 2025 that may be closed by this pull request

Introduce configurable augmentations for all supervised tasks #978

Closed

pplaskie approved these changes Oct 17, 2025

View reviewed changes

MarkRedeman approved these changes Oct 20, 2025

View reviewed changes

		async changeSubsetRange(slider: Locator, range: 'start' \| 'end', iterations: number) {
		await slider.getByLabel(range === 'start' ? 'Start range' : 'End range').click();

-    async changeSubsetRange(slider: Locator, range: 'start' | 'end', iterations: number) {
-        await slider.getByLabel(range === 'start' ? 'Start range' : 'End range').click();
+    async changeSubsetRange(range: 'start' | 'end', iterations: number) {
+        // Assume the slider can be found by label, e.g., "Start range" or "End range"
+        const slider = this.page.getByLabel(range === 'start' ? 'Start range' : 'End range');
+        await slider.click();

	'Whether to apply random flip images horizontally along the vertical axis (swap left and right)',
	'Whether to randomly flip images horizontally along the vertical axis (swap left and right)',

		description:
		'Whether to apply random flip images vertically along the horizontal axis (swap top and bottom)',

Add component test of augmentation parameters and add minor fixes #1353

Are you sure you want to change the base?

Add component test of augmentation parameters and add minor fixes #1353

Uh oh!

Conversation

romanowska commented Oct 2, 2025

📝 Description

✨ Type of Change

🧪 Testing Scenarios

✅ Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

dwesolow Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

romanowska Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

dwesolow Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

romanowska Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwesolow Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

romanowska Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

dwesolow Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

romanowska Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

romanowska Oct 16, 2025 •

edited

Loading