Fix model picker review findings

Chat template editor never seeded its draft. Radix only calls onOpenChange
from internal events, so the seed in the nextOpen branch was dead and a model
with a saved override opened empty. Saving then cleared the override. Drop the
dead branch, treat draft as an untouched sentinel, and reset it on every close.

Uncached Hub picks could auto load a model after the user left the chat. Main
detached the staged pick on route exit and on chat context change. Carry the
context key on the pending pick and skip the load when it no longer matches.

Also clear configTarget when the picker closes, restore the onUpdated ref so
variant rows stop resubscribing on every parent render, skip the LRU write when
the entry is already most recent, import NumericValueInput relatively, and drop
the unused ModelUpdateAction barrel export.
This commit is contained in:
sneakr 2026-07-10 07:28:04 +02:00
commit c73fb954a7
7 changed files with 67 additions and 42 deletions

View file

@ -1108,6 +1108,11 @@ export function validateChatSearch(search: Record<string, unknown>): ChatSearch
};
}
type PendingHubAutoLoad = {
selection: SelectedModelInput;
contextKey: string;
};
// `search` comes from RootLayout (not useSearch) so ChatPage stays mounted off-route
// (keeping an in-flight generation alive), frozen to the last /chat search. `active`
// is false off-route: close body-portaled surfaces and stop route-specific listeners
@ -1680,8 +1685,9 @@ export function ChatPage({
}, [activeThreadId, closeArtifactSurface, selectedArtifact, view]);
const hasActiveModel = Boolean(inferenceParams.checkpoint);
const chatContextKey = `${view.mode}|${activeThreadId ?? ""}|${search.new ?? ""}|${search.project ?? ""}`;
const [pendingHubAutoLoad, setPendingHubAutoLoad] =
useState<SelectedModelInput | null>(null);
useState<PendingHubAutoLoad | null>(null);
const stageOrLoad = useCallback(
async (selection: SelectedModelInput) => {
const store = useChatRuntimeStore.getState();
@ -1733,7 +1739,7 @@ export function ChatPage({
hasGgufSource(selection) &&
!selection.isDownloaded);
if (wantManagerStage) {
setPendingHubAutoLoad(selection);
setPendingHubAutoLoad({ selection, contextKey: chatContextKey });
return;
}
const previousConfig = currentRuntimePerModelConfig({
@ -1748,24 +1754,30 @@ export function ChatPage({
previousConfig,
});
},
[selectModel, loadingModel, rememberedConfigFor],
[selectModel, loadingModel, rememberedConfigFor, chatContextKey],
);
useRepoDownload({
kind: DOWNLOAD_KIND.MODEL,
repoId: pendingHubAutoLoad?.id ?? "__hub_autoload_idle__",
activeVariant: pendingHubAutoLoad?.ggufVariant ?? null,
repoId: pendingHubAutoLoad?.selection.id ?? "__hub_autoload_idle__",
activeVariant: pendingHubAutoLoad?.selection.ggufVariant ?? null,
onComplete: (variant) => {
const pending = pendingHubAutoLoad;
if (!pending || (pending.ggufVariant ?? null) !== (variant ?? null)) {
if (
!pending ||
(pending.selection.ggufVariant ?? null) !== (variant ?? null)
) {
return;
}
setPendingHubAutoLoad(null);
void stageOrLoad({ ...pending, isDownloaded: true });
if (!active || pending.contextKey !== chatContextKey) {
return;
}
void stageOrLoad({ ...pending.selection, isDownloaded: true });
},
onError: (variant) => {
if (
pendingHubAutoLoad &&
(pendingHubAutoLoad.ggufVariant ?? null) === (variant ?? null)
(pendingHubAutoLoad.selection.ggufVariant ?? null) === (variant ?? null)
) {
setPendingHubAutoLoad(null);
}
@ -1773,7 +1785,7 @@ export function ChatPage({
onCancelled: (variant) => {
if (
pendingHubAutoLoad &&
(pendingHubAutoLoad.ggufVariant ?? null) === (variant ?? null)
(pendingHubAutoLoad.selection.ggufVariant ?? null) === (variant ?? null)
) {
setPendingHubAutoLoad(null);
}
@ -1786,9 +1798,9 @@ export function ChatPage({
void (async () => {
const outcome = await downloadManager.requestStart({
kind: DOWNLOAD_KIND.MODEL,
repoId: pending.id,
variant: pending.ggufVariant ?? null,
expectedBytes: pending.expectedBytes ?? 0,
repoId: pending.selection.id,
variant: pending.selection.ggufVariant ?? null,
expectedBytes: pending.selection.expectedBytes ?? 0,
});
if (!active) return;
if (outcome === "started") {

View file

@ -12,7 +12,7 @@ import {
} from "@/components/ui/dialog";
import { Spinner } from "@/components/ui/spinner";
import { Textarea } from "@/components/ui/textarea";
import { useMemo, useState } from "react";
import { useState } from "react";
import { validateChatTemplate } from "../api/templates";
import {
MAX_CHAT_TEMPLATE_BYTES,
@ -39,27 +39,26 @@ export function ChatTemplateEditorDialog({
onSave,
readOnly = false,
}: ChatTemplateEditorDialogProps) {
const [draft, setDraft] = useState("");
const [draft, setDraft] = useState<string | null>(null);
const [error, setError] = useState<string | null>(null);
const [validating, setValidating] = useState(false);
const initialDraft = value ?? defaultTemplate ?? "";
const renderedDraft = useMemo(
() =>
open && value == null && defaultTemplate != null && draft.length === 0
? defaultTemplate
: draft,
[open, value, defaultTemplate, draft],
);
const renderedDraft = draft ?? value ?? defaultTemplate ?? "";
const byteLength = chatTemplateByteLength(renderedDraft);
const overLimit = !isChatTemplateWithinLimit(renderedDraft);
const matchesDefault =
defaultTemplate != null && renderedDraft === defaultTemplate;
const handleClose = () => {
setDraft(null);
setError(null);
onOpenChange(false);
};
const handleSave = async () => {
if (renderedDraft.trim().length === 0 || matchesDefault) {
onSave(null);
onOpenChange(false);
handleClose();
return;
}
if (overLimit) {
@ -74,7 +73,7 @@ export function ChatTemplateEditorDialog({
return;
}
onSave(renderedDraft);
onOpenChange(false);
handleClose();
} catch {
setError("Could not validate the template.");
} finally {
@ -87,10 +86,10 @@ export function ChatTemplateEditorDialog({
open={open}
onOpenChange={(nextOpen) => {
if (nextOpen) {
setDraft(initialDraft);
setError(null);
onOpenChange(true);
return;
}
onOpenChange(nextOpen);
handleClose();
}}
>
<DialogContent className="corner-squircle dialog-soft-surface sm:max-w-3xl">
@ -135,7 +134,7 @@ export function ChatTemplateEditorDialog({
<DialogFooter className="flex-wrap gap-2 sm:justify-between">
{readOnly ? (
<div className="flex w-full justify-end">
<Button type="button" onClick={() => onOpenChange(false)}>
<Button type="button" onClick={handleClose}>
Close
</Button>
</div>
@ -157,11 +156,7 @@ export function ChatTemplateEditorDialog({
)}
</Button>
<div className="flex gap-2">
<Button
type="button"
variant="ghost"
onClick={() => onOpenChange(false)}
>
<Button type="button" variant="ghost" onClick={handleClose}>
Cancel
</Button>
<Button

View file

@ -18,7 +18,6 @@ import {
readPersistedSpeculativeType,
useChatRuntimeStore,
} from "@/features/chat";
import { NumericValueInput } from "@/features/model-picker";
import { ChevronDownStandardIcon } from "@/lib/chevron-icons";
import { toast } from "@/lib/toast";
import { ArrowLeft01Icon } from "@hugeicons/core-free-icons";
@ -44,6 +43,7 @@ import {
} from "../model-config/per-model-config";
import { ChatTemplateEditorDialog } from "./chat-template-editor-dialog";
import type { ModelPickTarget } from "./model-selector/types";
import { NumericValueInput } from "./numeric-value-input";
const ROW_CLASS = "flex min-h-8 items-center justify-between gap-3";
const LABEL_CLASS =

View file

@ -412,6 +412,10 @@ function ModelSelectorContent({
const effectiveHubSection: HubSection =
hubSection === "connected" && !hasExternal ? "recommended" : hubSection;
const [configTarget, setConfigTarget] = useState<ModelPickTarget | null>(
null,
);
// The picker below remounts on each open, but this tab state does not, so a
// persisted selection that lands in lora/external after async load would
// reopen on Hub. Re-derive the default tab on the open edge.
@ -423,6 +427,9 @@ function ModelSelectorContent({
// user has downloads, else their last section.
setHubSection(wantsConnectedDefault ? "connected" : defaultHubSection());
}
if (!open && wasOpen.current) {
setConfigTarget(null);
}
wasOpen.current = open;
}, [
open,
@ -473,9 +480,6 @@ function ModelSelectorContent({
}
}
const [configTarget, setConfigTarget] = useState<ModelPickTarget | null>(
null,
);
const visibleConfigTarget = open ? configTarget : null;
const openConfigPage = (id: string, meta: ModelSelectorChangeMeta) => {
const leaf = id.includes("/") ? id.slice(id.lastIndexOf("/") + 1) : id;

View file

@ -8,7 +8,13 @@ import {
} from "@/features/hub";
import { cn } from "@/lib/utils";
import { RefreshCw } from "lucide-react";
import { type ReactNode, useCallback, useEffect, useState } from "react";
import {
type ReactNode,
useCallback,
useEffect,
useRef,
useState,
} from "react";
import { toast } from "sonner";
interface ModelUpdateActionProps {
@ -44,16 +50,20 @@ export function ModelUpdateAction({
}: ModelUpdateActionProps) {
const [open, setOpen] = useState(false);
const onUpdatedRef = useRef(onUpdated);
useEffect(() => {
onUpdatedRef.current = onUpdated;
}, [onUpdated]);
useEffect(() => {
return subscribeJobListeners("model", repoId, {
onComplete: (completedVariant) => {
const matches = variant
? ggufVariantsMatch(completedVariant, variant)
: !completedVariant;
if (matches) onUpdated?.();
if (matches) onUpdatedRef.current?.();
},
});
}, [onUpdated, repoId, variant]);
}, [repoId, variant]);
const handleConfirm = useCallback(() => {
// Start the re-download and close the dialog; the Downloads panel owns progress + cancel.

View file

@ -4,7 +4,6 @@
export { ModelSelector } from "./components/model-selector";
export { FolderBrowser } from "./components/model-selector/folder-browser";
export { ModelDeleteAction } from "./components/model-selector/model-delete-action";
export { ModelUpdateAction } from "./components/model-selector/model-update-action";
export { hfModelFitsDevice } from "./components/model-selector/recommended-fit";
export {
NumericValueInput,

View file

@ -149,6 +149,11 @@ function deleteOldestEvictableEntry(
return null;
}
function isMostRecentEntry(map: StoredMap, key: string): boolean {
const keys = Object.keys(map);
return keys.length > 0 && keys[keys.length - 1] === key;
}
function touchEntry(map: StoredMap, key: string): void {
const value = map[key];
delete map[key];
@ -478,7 +483,7 @@ function loadPerModelConfigInternal(
return null;
}
const config = normalize(map[key]);
if (touch) {
if (touch && !isMostRecentEntry(map, key)) {
touchEntry(map, key);
writeMap(map);
}