Move gateway experiment filtering to server-side query to fix inconsistent page sizes#21138
Conversation
…stent page sizes Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
| import type { SortingState } from '@tanstack/react-table'; | ||
| import type { TagFilter } from './useTagsFilter'; | ||
| import { isDemoExperiment } from '../../../utils/isDemoExperiment'; | ||
| import { isGatewayExperiment } from '../utils/experimentPage.common-utils'; |
There was a problem hiding this comment.
@copilot can we remove isGatewayExperiment function?
There was a problem hiding this comment.
Done in 040a5a8 — removed isGatewayExperiment and the GATEWAY value from the ExperimentSourceType enum since neither is referenced anywhere anymore.
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent experiment list page sizes caused by client-side filtering of gateway experiments after server-side pagination. By moving the gateway experiment exclusion filter to the server-side query, pagination now works correctly.
Changes:
- Moved gateway experiment filtering (
mlflow.experiment.sourceType != 'GATEWAY') from client-side to server-side query filter - Removed redundant client-side filtering logic and
isGatewayExperimentimport - Updated test to verify the filter is included in API calls rather than testing client-side filtering behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
mlflow/server/js/src/experiment-tracking/components/experiment-page/hooks/useExperimentListQuery.ts |
Adds gateway exclusion filter to server-side query and removes client-side filtering logic |
mlflow/server/js/src/experiment-tracking/components/experiment-page/hooks/useExperimentListQuery.test.tsx |
Updates gateway filtering test to verify API call includes the filter, and fixes overly broad tag filter check in pagination test |
Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
...server/js/src/experiment-tracking/components/experiment-page/hooks/useExperimentListQuery.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
|
hm, it seems "tag Is Not" search for experiments is not working in this way cuz it excludes any experiment that doesn't have the mlflow.experiment.sourceType tag at all. |
| return undefined; | ||
| } | ||
| // Exclude gateway experiments server-side to ensure consistent page sizes | ||
| filters.push(`tags.\`${EXPERIMENT_SOURCE_TYPE_TAG}\` != '${ExperimentSourceType.GATEWAY}'`); |
There was a problem hiding this comment.
Done in b30b620 — switched to tags.\mlflow.experiment.isGateway` IS NULLusing a newEXPERIMENT_IS_GATEWAY_TAGconstant exported fromexperimentPage.common-utils.ts`. This correctly includes all experiments that don't have the tag set, rather than accidentally excluding tag-less experiments.
…ents Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
|
|
||
| const EXPERIMENT_TYPE_TAG = 'mlflow.experimentType'; | ||
| const EXPERIMENT_SOURCE_TYPE_TAG = 'mlflow.experiment.sourceType'; | ||
| export const EXPERIMENT_SOURCE_TYPE_TAG = 'mlflow.experiment.sourceType'; |
There was a problem hiding this comment.
@copilot revert export change for EXPERIMENT_SOURCE_TYPE_TAG and ExperimentSourceType
There was a problem hiding this comment.
Done in 3c5df6a — reverted EXPERIMENT_SOURCE_TYPE_TAG back to const (unexported) and ExperimentSourceType back to an unexported enum with only the REPO_NOTEBOOK value.
Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…stent page sizes (mlflow#21138) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…stent page sizes (mlflow#21138) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…stent page sizes (#21138) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
Client-side filtering of gateway experiments after pagination causes inconsistent page sizes — e.g., with page size 10 and every 3rd experiment tagged as GATEWAY, users see 7, 7, 1 instead of 10, 5, 1.
What changes are proposed in this pull request?
useExperimentListQuery.ts: Move gateway experiment exclusion into the server-sidefilterparameter ingetFilters(), so the server excludes gateway experiments before paginating. Usestags.\mlflow.experiment.isGateway` IS NULL(leveraging the new tag added in #21149) to correctly include all non-gateway experiments — avoiding the previous!= 'GATEWAY'approach which incorrectly excluded experiments that lacked thesourceTypetag entirely. Removes the now-redundant client-sideisGatewayExperimentfilter and its import. Uses theEXPERIMENT_IS_GATEWAY_TAGconstant fromexperimentPage.common-utils.ts` instead of a hardcoded string.experimentPage.common-utils.ts: Export newEXPERIMENT_IS_GATEWAY_TAGconstant (mlflow.experiment.isGateway) so it can be shared across modules.EXPERIMENT_SOURCE_TYPE_TAGandExperimentSourceTyperemain unexported (internal to the file). Remove theisGatewayExperimentfunction as it is no longer referenced anywhere.useExperimentListQuery.test.tsx: Update gateway filtering test to assert theIS NULLexclusion filter is present in the API call (not client-side behavior). Fix an overly broadfilterParam?.includes('tags')check in the tags pagination test — since every query now includes a tag condition, tighten it to match the user-provided tag key ('env').How is this PR tested?
Does this PR require documentation update?
Does this PR require updating the MLflow Skills repository?
Release Notes
Is this a user-facing change?
Fix inconsistent experiment list page sizes caused by client-side filtering of gateway experiments after server-side pagination.
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev serverHow should the PR be classified in the release notes? Choose one:
rn/bug-fix- A user-facing bug fix worth mentioning in the release notesShould this PR be included in the next patch release?
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.