ceos/unsloth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Tighten comments in the new sidebar and delete-guard code

Browse source
This commit is contained in:
shimmyshimmer 2026-07-25 01:59:03 -07:00
commit 1cfb516d1b
8 changed files with 57 additions and 96 deletions
Download patch file Download diff file Expand all files Collapse all files

28
studio/backend/hub/services/models/deletion.py
View file

@ -589,10 +589,9 @@ def _inference_backend_blocks_delete(repo_id: str) -> bool:
def _diffusion_blocks_delete(repo_id: str) -> Optional[str]:
"""The 400 detail if the Images backend holds *repo_id*, else None.
Checks the ACTIVE engine (diffusers or native sd_cpp): on a native selection the diffusers
singleton reports unloaded while sd-cli still generates from the cached GGUF, so checking it
alone would let files be deleted mid-use. Same fail-open-on-acquire /
surface-on-query contract as :func:`_llama_cpp_blocks_delete`.
Queries the ACTIVE engine: on a native selection the diffusers singleton reports
unloaded while sd-cli still generates from the cached GGUF. Same
fail-open-on-acquire contract as :func:`_llama_cpp_blocks_delete`.
"""
try:
from core.inference.diffusion_engine_router import get_active_diffusion_engine
@ -604,16 +603,13 @@ def _diffusion_blocks_delete(repo_id: str) -> Optional[str]:
if status.get("loaded") and status.get("repo_id"):
if _loaded_id_matches_repo(str(status["repo_id"]), repo_id):
return "Unload the model before deleting"
# The native sd.cpp engine re-reads companion VAE / text-encoder files from the HF cache
# every generation, so deleting a companion repo (e.g. comfyanonymous/flux_text_encoders)
# while a native GGUF is loaded bricks the next generation. status().repo_id covers only
# the main GGUF, so also refuse the committed companion repos read from disk.
# sd.cpp re-reads companion VAE / text-encoder files every generation, and
# status().repo_id covers only the main GGUF, so refuse the companions too.
for lid in getattr(engine, "loaded_repo_ids", tuple)():
if _loaded_id_matches_repo(str(lid), repo_id):
return "Unload the model before deleting"
# Also refuse while a background image load is DOWNLOADING this repo (or its companion
# base): status().loaded is still False then, but deleting would remove blobs from under
# the in-flight download/assembly.
# A downloading repo still reports loaded=False, but deleting would pull blobs
# from under the in-flight fetch.
for lid in getattr(engine, "loading_repo_ids", tuple)():
if _loaded_id_matches_repo(str(lid), repo_id):
return "An Images model load is using this repo; wait for it to finish"
@ -621,11 +617,10 @@ def _diffusion_blocks_delete(repo_id: str) -> Optional[str]:
def _video_blocks_delete(repo_id: str) -> Optional[str]:
"""The 400 detail if the Video backend holds (or is downloading) *repo_id*, else None.
"""The 400 detail if the Video backend holds or is fetching *repo_id*, else None.
Cached non-GGUF video repos surface in the Video picker with a delete action, so without
this a loaded/loading Wan / LTX / Hunyuan pipeline could lose its HF snapshot from under
it. Mirrors :func:`_diffusion_blocks_delete`.
Video repos share the On Device delete action, so a live Wan / LTX / Hunyuan
pipeline could otherwise lose its snapshot. Mirrors :func:`_diffusion_blocks_delete`.
"""
try:
from core.inference.video import get_video_backend
@ -673,8 +668,7 @@ async def delete_cached_model_response(
):
blocks_detail = "Unload the model before deleting"
else:
# The chat guards above are chat-only; the Images / Video engines hold their own
# pipelines (and companion repos) whose GGUFs must not vanish from under them.
# The guards above are chat-only; Images / Video hold their own pipelines.
blocks_detail = _diffusion_blocks_delete(repo_id) or _video_blocks_delete(repo_id)
except Exception as e:
logger.warning(f"Load-state verification failed for {repo_id}; refusing delete: {e}")

10
studio/backend/routes/settings.py
View file

@ -735,8 +735,7 @@ SIDEBAR_MENU_ITEM_DEFAULTS = {
}
# Navigable sidebar rows the user can pin/reorder; the boolean is each id's
# default pin state, matching the shipped layout. Unpinned rows collect in the
# "More" flyout client-side; a single unpinned row is hidden there instead.
# default pin state, matching the shipped layout.
SIDEBAR_NAV_ITEM_DEFAULTS = {
"projects": True,
"hub": True,
@ -840,8 +839,7 @@ class PersonalizationCustomization(BaseModel):
default_factory = _default_sidebar_menu,
max_length = MAX_SIDEBAR_MENU_INPUT_ITEMS,
)
# Order matters here: it is the sidebar's render order, so the validator
# preserves the client's sequence and only appends ids it didn't send.
# Order is the sidebar's render order, so the validator keeps the client's.
sidebarNav: list[PersonalizationSidebarNavItem] = Field(
default_factory = _default_sidebar_nav,
max_length = MAX_SIDEBAR_NAV_INPUT_ITEMS,
@ -866,9 +864,7 @@ class PersonalizationCustomization(BaseModel):
def _validate_sidebar_nav(
cls, value: list[PersonalizationSidebarNavItem]
) -> list[PersonalizationSidebarNavItem]:
# Same contract as sidebarMenu: drop duplicate ids (keep the first) and
# re-append missing ones, so the stored list covers every nav row exactly
# once while keeping the client's order.
# Like sidebarMenu, but order is preserved: dedupe, then append missing.
seen: set[str] = set()
items = [item for item in value if not (item.id in seen or seen.add(item.id))]
for item_id, pinned in SIDEBAR_NAV_ITEM_DEFAULTS.items():

10
studio/backend/tests/test_personalization_settings.py
View file

@ -151,8 +151,7 @@ def _sidebar_nav(items):
def test_customization_sidebar_nav_defaults_match_shipped_layout():
# An untouched payload pins the rows the sidebar ships with and leaves the
# rest for the "More" flyout, so a fresh account looks unchanged.
# A fresh account must look like the shipped sidebar.
c = PersonalizationPayload().appearance.customization
assert [(i.id, i.pinned) for i in c.sidebarNav] == [
("projects", True),
@ -175,8 +174,7 @@ def test_customization_sidebar_nav_preserves_order_and_normalizes():
]
)
)
# Order is the sidebar's render order, so the client's sequence survives:
# duplicates keep the first entry and unsent ids are appended with defaults.
# Client order survives; duplicates keep the first, unsent ids are appended.
assert [(i.id, i.pinned) for i in p.appearance.customization.sidebarNav] == [
("video", True),
("hub", False),
@ -443,8 +441,8 @@ def test_personalization_route_roundtrip_real_shape(monkeypatch):
{"id": "chat", "visible": False},
{"id": "connections", "visible": False},
],
# Reordered and partly unpinned, so the round-trip proves the
# sidebar's render order survives a save unchanged.
# Reordered and partly unpinned, so the round-trip proves order
# survives a save.
"sidebarNav": [
{"id": "images", "pinned": True},
{"id": "video", "pinned": True},

49
studio/frontend/src/components/app-sidebar.tsx
View file

@ -205,8 +205,8 @@ const SETTINGS_TAB_MENU_ITEMS: Record<
connections: { icon: CloudIcon, labelKey: "settings.tabs.connections" },
};
// A navigable sidebar row. The same definition renders either as a top-level
// NavItem or as a MoreMenuItem, depending on the user's pin preference.
// One navigable row, rendered as a NavItem or a MoreMenuItem depending on its
// pin state.
type NavRowDef = {
icon: typeof ZapIcon;
label: string;
@ -293,9 +293,7 @@ function preloadSilently(request: Promise<unknown>): void {
void request.catch(() => undefined);
}
// Small "New" pill for recently shipped tabs. Same recipe as the brand "beta"
// badge (nav-badge font, --ui-font-scale sizing, nav token colours) so the two
// read as one design language.
// "New" pill for recent tabs. Same recipe as the brand "beta" badge.
function NavBadge({ label, className }: { label: string; className?: string }) {
return (
<span
@ -374,8 +372,7 @@ function NavItem({
);
}
// One row inside the "More" flyout: same affordances as a NavItem (disabled
// hint, "New" pill, route preloading) in dropdown-item form.
// A NavItem's affordances in dropdown-item form, for the "More" flyout.
function MoreMenuItem({
icon,
label,
@ -487,8 +484,8 @@ export function AppSidebar() {
const isStudioRoute = pathname === "/studio" || pathname.startsWith("/studio/");
const [chatOpen, setChatOpen] = useState(true);
// "More" flyout (Video / Recipes / Export). Opens on click or hover; the close
// is delayed so the pointer can cross the gap between row and panel.
// "More" flyout. Opens on click or hover; close is delayed so the pointer can
// cross the gap to the panel.
const [moreOpen, setMoreOpen] = useState(false);
const moreCloseTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
const openMore = useCallback(() => {
@ -689,8 +686,7 @@ export function AppSidebar() {
const chatDisabled = trainingInProgress;
// One definition per navigable row, so the pinned list and the More flyout
// render the same row from the same source and can't drift apart.
// One definition per row, so pinned rows and the flyout can't drift apart.
const navRows: Record<SidebarNavItemId, NavRowDef> = {
projects: {
icon: Folder01Icon,
@ -704,8 +700,7 @@ export function AppSidebar() {
preloadSilently(router.preloadRoute({ to: "/projects" }));
},
className: "group/projects-item relative",
// The inline "new project" affordance only makes sense on a real row; in
// the flyout the row is just a link.
// The inline "new project" affordance only fits a real row.
children: (
<button
type="button"
@ -769,9 +764,8 @@ export function AppSidebar() {
preloadSilently(router.preloadRoute({ to: "/studio" }));
},
},
// Video is diffusers-only (no native CPU engine), so a chat-only host can
// never load it; disable with a hint instead of bouncing off the root
// guard's redirect.
// Video is diffusers-only, so a chat-only host can't load it. Disable with a
// hint instead of bouncing off the root guard's redirect.
video: {
icon: FlimSlateIcon,
label: t("shell.navigation.video"),
@ -828,9 +822,8 @@ export function AppSidebar() {
const unpinnedNavIds = sidebarNav
.filter((item) => !item.pinned)
.map((item) => item.id);
// A flyout wrapping a single row costs a click and earns nothing, so More only
// appears once it would hold two or more. With exactly one row unpinned, both
// the menu and that row are dropped -- the page stays reachable by URL.
// More needs two or more rows to be worth a click. With exactly one unpinned,
// the menu and that row are both dropped.
const overflowNavIds = unpinnedNavIds.length > 1 ? unpinnedNavIds : [];
const inlineNavIds = sidebarNav
.filter((item) => item.pinned)
@ -1613,9 +1606,8 @@ export function AppSidebar() {
<SidebarGroup data-tour="navbar" className="group-data-[collapsible=icon]:px-0 pl-1.5 pr-2 py-0 shrink-0">
<SidebarGroupContent>
<SidebarMenu>
{/* Rows come from the saved pin order (Settings -> Appearance ->
Sidebar navigation). Pinned ids render here in order; the rest
fall into the More flyout below, so nothing is unreachable. */}
{/* Order and pin state come from Settings -> Appearance ->
Sidebar navigation. */}
{inlineNavIds.map((id) => {
const row = navRows[id];
return (
@ -1636,8 +1628,7 @@ export function AppSidebar() {
</NavItem>
);
})}
{/* Secondary destinations behind one row, so the primary nav stays short.
Hover or click opens it; the panel flies out to the right. */}
{/* Unpinned destinations, behind one row. */}
{overflowNavIds.length > 0 && (
<SidebarMenuItem
onPointerEnter={openMore}
@ -1648,11 +1639,9 @@ export function AppSidebar() {
onOpenChange={setMoreOpen}
modal={false}
>
{/* No `tooltip` prop on the button: with it, SidebarMenuButton
returns a Tooltip root and DropdownMenuTrigger asChild would
hand its ref/handlers to that instead of a DOM node, leaving
the trigger dead. Wrap it here instead, so both triggers
compose onto the same button. */}
{/* Tooltip wraps the trigger rather than using the button's
`tooltip` prop: that returns a Tooltip root, so
DropdownMenuTrigger asChild would miss the DOM node. */}
<Tooltip>
<TooltipPrimitive.Trigger asChild>
<DropdownMenuTrigger asChild>
@ -1673,7 +1662,7 @@ export function AppSidebar() {
</SidebarMenuButton>
</DropdownMenuTrigger>
</TooltipPrimitive.Trigger>
{/* Only the collapsed rail needs it; expanded rows show their label. */}
{/* Collapsed rail only; expanded rows show their label. */}
<TooltipContent
side="right"
align="center"

27
studio/frontend/src/features/model-picker/components/model-selector/pickers.tsx
View file

@ -1882,10 +1882,8 @@ export function HubModelPicker({
const deviceType = usePlatformStore((s) => s.deviceType);
const isMac = deviceType === "mac";
// Drop models Unsloth can't run for chat (diffusion / image / video / etc.) using the Hub's
// classifier on the tags the listing already carries. When the picker is task-scoped (e.g.
// Images asks for text-to-image), models matching that task are exactly what we want -- keep
// them even though the chat classifier marks image tasks "unsupported".
// Drop models Unsloth can't run for chat. A task-scoped picker wants exactly the tasks
// the chat classifier calls unsupported, so it gates on the task instead.
const isChatSupported = useCallback(
(r: HfModelResult) => {
// Image/Video tab (task set): only task-matching, non-editing results.
@ -1906,9 +1904,8 @@ export function HubModelPicker({
const all = dedupe([...models.map((model) => model.id), value ?? ""])
.filter((id) => !isHiddenModelId(id))
.filter((id) => !downloadedSet.has(id.toLowerCase()))
// Images/Video (task set) load single-file GGUF only, so don't surface non-GGUF rows the
// page can't load. Otherwise chat-only keeps runnable formats: GGUF anywhere, plus
// MLX/safetensors on Mac (matches the Recommended view so search stays consistent).
// Task-scoped pages load single-file GGUF only; chat-only keeps runnable formats
// (GGUF anywhere, plus MLX/safetensors on Mac).
.filter((id) =>
task
? isKnownGgufRepo(id)
@ -1961,11 +1958,9 @@ export function HubModelPicker({
formatFilter === "all"
? rows.filter((r) => isRecommendableFormat(r.id, r.isGguf, isMac))
: rows.filter((r) => matchesFormatFilter(r.id, r.isGguf, formatFilter));
// Images/Video (task set) load single-file GGUF only, so never surface non-GGUF rows the
// page can't load.
// Task-scoped pages load single-file GGUF only.
if (task) rows = rows.filter((r) => r.isGguf);
// A catalog group already renders its member repos as one canonical row; drop the members
// so they don't appear twice.
// Members already render under their canonical group row.
if (catalog) rows = rows.filter((r) => !groupForRepoId(r.id, catalog));
// The "recommended" sort always applies the device-fit filter; the shared
// "Fits on device" tick extends it to the other sorts too.
@ -2070,8 +2065,7 @@ export function HubModelPicker({
}, [results, recommendedSearch.results]);
// Ordered by the On Device dropdown (recent/download date/size/name).
// The task gate keeps the Images/Video picker to diffusion GGUFs and, conversely, hides those
// diffusion GGUFs from the chat picker (where they aren't loadable models).
// The gate keeps diffusion GGUFs in the Images/Video picker and out of chat.
const sortedCachedGguf = useMemo(
() =>
sortCachedRepos(
@ -2111,11 +2105,8 @@ export function HubModelPicker({
),
[cachedModels, downloadedSort, loadTimes, task, catalog],
);
// Variant expanders and format lists follow a single-device budget when task-scoped
// (Images/Video): the diffusion and video loaders place the whole pipeline on one device, so
// sorting/recommending quants against the summed multi-GPU total would mark variants as fitting
// that OOM at load. Chat pickers keep the summed total, where llama.cpp splits layers across
// devices.
// Task-scoped loads put the whole pipeline on one device, so quant fit must use the
// largest device, not the multi-GPU sum. Chat keeps the sum (llama.cpp splits layers).
const expanderGpuGb = gpu.available
? task
? gpu.maxDeviceMemoryGb

15
studio/frontend/src/features/settings/components/sidebar-nav-customizer.tsx
View file

@ -58,8 +58,7 @@ function MovableRow({ item }: { item: SidebarNavItemPref }) {
dragListener={false}
dragControls={controls}
layout="position"
// Rows sit flat on the dialog surface; the dragged row lifts above its
// siblings so it stays readable while crossing them.
// The dragged row lifts above its siblings so it stays readable.
whileDrag={{
backgroundColor: "var(--popover)",
boxShadow: "0 4px 16px rgb(0 0 0 / 0.18)",
@ -107,11 +106,9 @@ function MovableRow({ item }: { item: SidebarNavItemPref }) {
}
/**
* Pin and reorder the sidebar navigation rows. Rows with their switch off collect
* in the "More" flyout. A single unpinned row is hidden outright instead: More
* would be a menu of one, so neither it nor the row is drawn (the page stays
* reachable by URL). New chat renders as a static row: it is an action pinned to
* the top, not a destination.
* Pin and reorder the sidebar nav rows. Unpinned rows collect in the "More"
* flyout; a single unpinned row is hidden instead of getting a menu of one.
* New chat is static: it is an action, not a destination.
*/
export function SidebarNavCustomizer() {
const t = useT();
@ -137,9 +134,7 @@ export function SidebarNavCustomizer() {
<MovableRow key={item.id} item={item} />
))}
</Reorder.Group>
{/* Only meaningful at two or more: with everything pinned there is no More
row, and a single unpinned row is hidden rather than sitting behind a
menu built for one item. */}
{/* Mirrors the sidebar: More only exists at two or more. */}
{unpinnedCount > 1 && (
<>
<div className="mx-2 my-1 border-t border-border/70" />

13
studio/frontend/src/features/settings/stores/appearance-custom-store.ts
View file

@ -83,11 +83,9 @@ export const SIDEBAR_MENU_DEFAULT_VISIBLE: Record<SidebarMenuItemId, boolean> =
};
/**
* Sidebar NAVIGATION rows the user can pin and reorder (distinct from the
* profile-menu entries above). New chat stays fixed at the top: it is an action,
* not a destination. Array order is render order; an unpinned row moves into the
* "More" flyout. A single unpinned row is hidden outright rather than getting a
* flyout of one; the page stays reachable by URL.
* Sidebar NAVIGATION rows the user can pin and reorder, distinct from the
* profile-menu entries above. Array order is render order. Unpinned rows go to
* the "More" flyout, except a lone one, which is hidden.
*/
export const SIDEBAR_NAV_ITEM_IDS = [
"projects",
@ -103,7 +101,7 @@ export type SidebarNavItemId = (typeof SIDEBAR_NAV_ITEM_IDS)[number];
export type SidebarNavItemPref = {
id: SidebarNavItemId;
/** true = a top-level sidebar row; false = inside the "More" flyout. */
/** true = top-level row; false = under "More". */
pinned: boolean;
};
@ -277,8 +275,7 @@ function sanitizeSidebarNav(value: unknown): SidebarNavItemPref[] {
seen.add(source.id);
items.push({ id: source.id, pinned: source.pinned !== false });
}
// Ids added after the payload was written land at the end with their default
// pin state, so a new tab shows up where the shipped layout puts it.
// Ids added after the payload was written land at the end with their default.
for (const id of SIDEBAR_NAV_ITEM_IDS) {
if (!seen.has(id)) items.push({ id, pinned: SIDEBAR_NAV_DEFAULT_PINNED[id] });
}

1
studio/frontend/src/features/settings/tabs/appearance-tab.tsx
View file

@ -152,6 +152,7 @@ export function AppearanceTab() {
</SettingsRow>
</SettingsSection>
{/* Nav shape first, then the profile menu inside it. */}
<SettingsSection
title={t("settings.appearance.sidebarNav.title")}
description={t("settings.appearance.sidebarNav.description")}