Skip to content

"Community and custom models" section should be hidden if there are no catalog sources without labels #1836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
google-oss-prow merged 3 commits into from
Nov 18, 2025
Merged

"Community and custom models" section should be hidden if there are no catalog sources without labels #1836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e21754d
hiding a section when no source labels
YuliaKrimerman Nov 3, 2025
377d20b
addressed nit
YuliaKrimerman Nov 18, 2025
09cfab6
reverted the mocks change
YuliaKrimerman Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 59 additions & 7 deletions ...ts/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelCatalog/modelCatalog.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import type { CatalogSource } from '~/app/modelCatalogTypes';
import { MODEL_CATALOG_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api';
import { mockCatalogFilterOptionsList } from '~/__mocks__/mockCatalogFilterOptionsList';
import { SourceLabel } from '~/app/modelCatalogTypes';

type HandlersProps = {
sources?: CatalogSource[];
Expand Down Expand Up @@ -53,6 +54,32 @@ const initIntercepts = ({
});
});

// Intercept requests for sources without labels if they exist
const hasSourcesWithoutLabels = sources.some(
(source) =>
source.enabled !== false &&
(source.labels.length === 0 || source.labels.every((label) => !label.trim())),
);

if (hasSourcesWithoutLabels) {
cy.interceptApi(
`GET /api/:apiVersion/model_catalog/models`,
{
path: { apiVersion: MODEL_CATALOG_API_VERSION },
query: { sourceLabel: SourceLabel.other },
},
mockCatalogModelList({
items: Array.from({ length: modelsPerCategory }, (_, i) =>
mockCatalogModel({
name: `custom-model-${i + 1}`,
// eslint-disable-next-line camelcase
source_id: sources.find((s) => s.labels.length === 0)?.id || 'custom-source',
}),
),
}),
);
}

cy.interceptApi(
`GET /api/:apiVersion/model_catalog/models/filter_options`,
{
Expand Down Expand Up @@ -138,6 +165,30 @@ describe('Model Catalog Page', () => {
});

it('checkbox should work', () => {
// Calculate expected category count based on sources
const defaultSources = [
mockCatalogSource({}),
mockCatalogSource({ id: 'source-2', name: 'source 2' }),
];
Comment on lines +169 to +172
Copy link
Copy Markdown
Contributor

@mturley mturley Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you never actually use defaultSources in an intercept, I think the test only passes because it happens to match the default currently implemented in initIntercepts. In case that changes later I think you need to pass this into the initIntercepts call on line 203 here.

Copy link
Copy Markdown
Contributor Author

@YuliaKrimerman YuliaKrimerman Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mturley fixed. I think it needs /retest though :(

const uniqueLabels = new Set<string>();
defaultSources.forEach((source) => {
source.labels.forEach((label) => {
if (label.trim()) {
uniqueLabels.add(label.trim());
}
});
});

// Check if there are sources without labels
const hasSourcesWithoutLabels = defaultSources.some(
(source) =>
source.enabled !== false &&
(source.labels.length === 0 || source.labels.every((label) => !label.trim())),
);

// Expected count: unique labels + (1 if sources without labels exist)
const expectedCategoryCount = uniqueLabels.size + (hasSourcesWithoutLabels ? 1 : 0);

cy.interceptApi(
`GET /api/:apiVersion/model_catalog/models`,
{
Expand All @@ -149,17 +200,18 @@ describe('Model Catalog Page', () => {
}),
).as('getCatalogModelsBySource');

initIntercepts({});
initIntercepts({ sources: defaultSources });
modelCatalog.visit();
modelCatalog.findFilterCheckbox('Task', 'text-generation').click();
modelCatalog.findFilterCheckbox('Task', 'text-to-text').click();
modelCatalog.findFilterCheckbox('Provider', 'Google').click();
cy.wait([
'@getCatalogModelsBySource',
'@getCatalogModelsBySource',
'@getCatalogModelsBySource',
'@getCatalogModelsBySource',
]).then((interceptions) => {

// Wait for the expected number of API calls (one per category section when filters are applied)
const waitCalls = Array.from(
{ length: expectedCategoryCount },
() => '@getCatalogModelsBySource',
);
cy.wait(waitCalls).then((interceptions) => {
const lastInterception = interceptions[interceptions.length - 1];
expect(lastInterception.request.url).to.include(
'%28tasks+LIKE+%27%25%22text-generation%22%25%27+OR+tasks+LIKE+%27%25%22text-to-text%22%25%27%29+AND+provider%3D%27Google%27',
Expand Down
80 changes: 79 additions & 1 deletion ...d/src/__tests__/cypress/cypress/tests/mocked/modelCatalog/modelCatalogAllModelsView.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,25 @@ import {
import type { CatalogSource } from '~/app/modelCatalogTypes';
import { MODEL_CATALOG_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api';
import { mockCatalogFilterOptionsList } from '~/__mocks__/mockCatalogFilterOptionsList';
import { SourceLabel } from '~/app/modelCatalogTypes';

type HandlersProps = {
sources?: CatalogSource[];
modelsPerCategory?: number;
isEmpty?: boolean;
includeSourcesWithoutLabels?: boolean;
};

const initIntercepts = ({
sources = [
mockCatalogSource({ id: 'huggingface', name: 'Hugging Face', labels: ['Hugging Face'] }),
mockCatalogSource({ id: 'openvino', name: 'OpenVINO', labels: ['OpenVINO'] }),
mockCatalogSource({ id: 'community', name: 'Community', labels: ['Community'] }),
mockCatalogSource({ id: 'custom-source', name: 'Custom Source', labels: [] }),
],
modelsPerCategory = 4,
isEmpty = false,
includeSourcesWithoutLabels = true,
}: HandlersProps) => {
cy.interceptApi(
`GET /api/:apiVersion/model_catalog/sources`,
Expand Down Expand Up @@ -65,6 +69,36 @@ const initIntercepts = ({
);
});
});

// Intercept requests for sources without labels (sourceLabel=null)
if (includeSourcesWithoutLabels) {
const hasSourcesWithoutLabels = sources.some(
(source) =>
source.enabled !== false &&
(source.labels.length === 0 || source.labels.every((label) => !label.trim())),
);

if (hasSourcesWithoutLabels) {
cy.interceptApi(
`GET /api/:apiVersion/model_catalog/models`,
{
path: { apiVersion: MODEL_CATALOG_API_VERSION },
query: { sourceLabel: SourceLabel.other },
},
mockCatalogModelList({
items: isEmpty
? []
: Array.from({ length: modelsPerCategory }, (_, i) =>
mockCatalogModel({
name: `custom-model-${i + 1}`,
// eslint-disable-next-line camelcase
source_id: sources.find((s) => s.labels.length === 0)?.id || 'custom-source',
}),
),
}),
);
}
}
};

describe('Model Catalog All Models View', () => {
Expand All @@ -74,14 +108,32 @@ describe('Model Catalog All Models View', () => {
});

describe('Category Sections', () => {
it('should display all category sections', () => {
it('should display all category sections when sources without labels exist', () => {
modelCatalog.findAllModelsToggle().should('be.visible');
modelCatalog.findCategoryToggle('label-Hugging Face').should('be.visible');
modelCatalog.findCategoryToggle('label-OpenVINO').should('be.visible');
modelCatalog.findCategoryToggle('label-Community').should('be.visible');
modelCatalog.findCategoryToggle('no-labels').should('be.visible');
});

it('should hide Community and custom models section when no sources without labels exist', () => {
initIntercepts({
sources: [
mockCatalogSource({ id: 'huggingface', name: 'Hugging Face', labels: ['Hugging Face'] }),
mockCatalogSource({ id: 'openvino', name: 'OpenVINO', labels: ['OpenVINO'] }),
mockCatalogSource({ id: 'community', name: 'Community', labels: ['Community'] }),
],
includeSourcesWithoutLabels: false,
});
modelCatalog.visit();

modelCatalog.findAllModelsToggle().should('be.visible');
modelCatalog.findCategoryToggle('label-Hugging Face').should('be.visible');
modelCatalog.findCategoryToggle('label-OpenVINO').should('be.visible');
modelCatalog.findCategoryToggle('label-Community').should('be.visible');
modelCatalog.findCategoryToggle('no-labels').should('not.exist');
});

it('should show category titles', () => {
modelCatalog.findCategoryTitle('OpenVINO').should('contain.text', 'OpenVINO models');
cy.findByTestId('title Hugging Face').should('contain.text', 'Hugging Face models');
Expand All @@ -106,6 +158,32 @@ describe('Model Catalog All Models View', () => {

describe('Error Handling', () => {
it('should display error message when category fails to load', () => {
// Setup intercepts with sources without labels
initIntercepts({
sources: [
mockCatalogSource({ id: 'huggingface', name: 'Hugging Face', labels: ['Hugging Face'] }),
mockCatalogSource({ id: 'openvino', name: 'OpenVINO', labels: ['OpenVINO'] }),
mockCatalogSource({ id: 'community', name: 'Community', labels: ['Community'] }),
mockCatalogSource({ id: 'custom-source', name: 'Custom Source', labels: [] }),
],
includeSourcesWithoutLabels: false, // Don't set up success intercept
});

// Manually intercept with error response for sourceLabel=null
cy.intercept(
{
method: 'GET',
pathname: `/model-registry/api/${MODEL_CATALOG_API_VERSION}/model_catalog/models`,
query: { sourceLabel: SourceLabel.other },
},
{
statusCode: 500,
body: { error: 'Internal server error' },
},
);

modelCatalog.visit();

modelCatalog.findErrorState('null').scrollIntoView().should('be.visible');
modelCatalog
.findErrorState('null')
Expand Down
26 changes: 17 additions & 9 deletions clients/ui/frontend/src/app/pages/modelCatalog/screens/ModelCatalogAllModelsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ModelCatalogContext } from '~/app/context/modelCatalog/ModelCatalogCont
import {
filterEnabledCatalogSources,
getUniqueSourceLabels,
hasSourcesWithoutLabels,
} from '~/app/pages/modelCatalog/utils/modelCatalogUtils';
import { CategoryName, SourceLabel } from '~/app/modelCatalogTypes';
import CatalogCategorySection from './CatalogCategorySection';
Expand All @@ -20,6 +21,11 @@ const ModelCatalogAllModelsView: React.FC<ModelCatalogAllModelsViewProps> = ({ s
return getUniqueSourceLabels(enabledSources);
}, [catalogSources]);

const hasSourcesWithoutLabelsValue = React.useMemo(
() => hasSourcesWithoutLabels(catalogSources),
[catalogSources],
);

const handleShowMoreCategory = React.useCallback(
(categoryLabel: string) => {
updateSelectedSourceLabel(categoryLabel);
Expand All @@ -39,15 +45,17 @@ const ModelCatalogAllModelsView: React.FC<ModelCatalogAllModelsViewProps> = ({ s
onShowMore={handleShowMoreCategory}
/>
))}
<CatalogCategorySection
key={CategoryName.communityAndCustomModels}
label={SourceLabel.other}
searchTerm={searchTerm}
pageSize={4}
catalogSources={catalogSources}
onShowMore={handleShowMoreCategory}
displayName={CategoryName.communityAndCustomModels}
/>
{hasSourcesWithoutLabelsValue && (
<CatalogCategorySection
key={CategoryName.communityAndCustomModels}
label={SourceLabel.other}
searchTerm={searchTerm}
pageSize={4}
catalogSources={catalogSources}
onShowMore={handleShowMoreCategory}
displayName={CategoryName.communityAndCustomModels}
/>
)}
</Stack>
);
};
Expand Down
19 changes: 13 additions & 6 deletions clients/ui/frontend/src/app/pages/modelCatalog/screens/ModelCatalogSourceLabelBlocks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { CategoryName, SourceLabel } from '~/app/modelCatalogTypes';
import {
getUniqueSourceLabels,
filterEnabledCatalogSources,
hasSourcesWithoutLabels,
} from '~/app/pages/modelCatalog/utils/modelCatalogUtils';

type SourceLabelBlock = {
Expand All @@ -24,6 +25,7 @@ const ModelCatalogSourceLabelBlocks: React.FC = () => {

const enabledSources = filterEnabledCatalogSources(catalogSources);
const uniqueLabels = getUniqueSourceLabels(enabledSources);
const hasNoLabels = hasSourcesWithoutLabels(catalogSources);

const allBlock: SourceLabelBlock = {
id: 'all',
Expand All @@ -37,13 +39,18 @@ const ModelCatalogSourceLabelBlocks: React.FC = () => {
displayName: `${label} models`,
}));

const noLabelsBlock: SourceLabelBlock = {
id: 'no-labels',
label: SourceLabel.other,
displayName: `${CategoryName.communityAndCustomModels} models`,
};
const blocksToReturn: SourceLabelBlock[] = [allBlock, ...labelBlocks];

if (hasNoLabels) {
const noLabelsBlock: SourceLabelBlock = {
id: 'no-labels',
label: SourceLabel.other,
displayName: `${CategoryName.communityAndCustomModels} models`,
};
blocksToReturn.push(noLabelsBlock);
}

return [allBlock, ...labelBlocks, noLabelsBlock];
return blocksToReturn;
}, [catalogSources]);

if (!catalogSources) {
Expand Down
14 changes: 14 additions & 0 deletions clients/ui/frontend/src/app/pages/modelCatalog/utils/modelCatalogUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,20 @@ export const getUniqueSourceLabels = (catalogSources: CatalogSourceList | null):
return Array.from(allLabels);
};

export const hasSourcesWithoutLabels = (catalogSources: CatalogSourceList | null): boolean => {
if (!catalogSources) {
return false;
}

return catalogSources.items.some((source) => {
if (source.enabled !== false) {
// Check if source has no labels or only empty/whitespace labels
return source.labels.length === 0 || source.labels.every((label) => !label.trim());
}
return false;
});
};

export const getSourceFromSourceId = (
sourceId: string,
catalogSources: CatalogSourceList | null,
Expand Down