Skip to content

[feat] Filter out nlf model type from Upload Model flow #7793

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
luke-mino-altherr merged 1 commit into from
Dec 31, 2025
Merged

[feat] Filter out nlf model type from Upload Model flow #7793

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/platform/assets/composables/useModelTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ interface ModelTypeOption {
value: string // Actual tag value
}

const DISALLOWED_MODEL_TYPES = ['nlf'] as const

/**
* Composable for fetching and managing model types from the API
* Uses shared state to ensure data is only fetched once
Expand All @@ -51,6 +53,12 @@ export const useModelTypes = createSharedComposable(() => {
async (): Promise<ModelTypeOption[]> => {
const response = await api.getModelFolders()
return response
.filter(
(folder) =>
!DISALLOWED_MODEL_TYPES.includes(
folder.name as (typeof DISALLOWED_MODEL_TYPES)[number]
)
)
Comment on lines +56 to +61
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Type assertion can be removed after fixing the constant declaration.

The type assertion folder.name as (typeof DISALLOWED_MODEL_TYPES)[number] is a workaround for the type incompatibility caused by as const on line 40. Once that constant is typed as readonly string[], this simplifies to:

         .filter(
-          (folder) =>
-            !DISALLOWED_MODEL_TYPES.includes(
-              folder.name as (typeof DISALLOWED_MODEL_TYPES)[number]
-            )
+          (folder) => !DISALLOWED_MODEL_TYPES.includes(folder.name)
         )

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/platform/assets/composables/useModelTypes.ts around lines 56 to 61, the
explicit type assertion on folder.name is compensating for the constant being
declared with as const; change the DISALLOWED_MODEL_TYPES declaration (around
line 40) from using as const to a readonly string[] type (e.g. declare it as
const DISALLOWED_MODEL_TYPES: readonly string[] = [...]) so its element type is
string, then remove the unnecessary assertion in the filter and use folder.name
directly in the includes check.

Copy link
Contributor

@DrJKL DrJKL Dec 29, 2025

Choose a reason for hiding this comment

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

I'd probably do a

type DisallowedModelType = (typeof DISALLOWED_MODEL_TYPES)[number]

to make this a little cleaner.

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 29, 2025

Choose a reason for hiding this comment

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

@DrJKL That's a nice approach! Creating the DisallowedModelType type alias keeps the benefits of as const (literal type inference for 'nlf') while making the type assertion much more readable and maintainable. It's cleaner than repeating (typeof DISALLOWED_MODEL_TYPES)[number] inline and also more self-documenting.

If more disallowed types are added in the future, the type alias would automatically expand to the union type (e.g., 'nlf' | 'other'), which is a nice bonus for type safety compared to just using readonly string[].

🧠 Learnings used 
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Do not use `any` type or `as any` type assertions; fix the underlying type issue instead

.map((folder) => ({
name: formatDisplayName(folder.name),
value: folder.name
Expand Down
Loading