diff --git a/studio/frontend/src/features/api-monitor/api-monitor-page.tsx b/studio/frontend/src/features/api-monitor/api-monitor-page.tsx index 4d998c6f88..1558c9c6eb 100644 --- a/studio/frontend/src/features/api-monitor/api-monitor-page.tsx +++ b/studio/frontend/src/features/api-monitor/api-monitor-page.tsx @@ -50,6 +50,9 @@ import { const API_INFERENCE_PREFIX_RE = /^\/api\/inference/; const V1_PREFIX_RE = /^\/v1\//; +// Tries per revision for a detail payload. Bounded because the usual failure +// is an entry that has aged out of the ring buffer and never comes back. +const DETAIL_FETCH_ATTEMPTS = 3; const STATUS_FILTERS: { value: MonitorStatusFilter; label: string }[] = [ { value: "all", label: "All requests" }, @@ -550,9 +553,16 @@ export function ApiMonitorPage(): ReactElement { const selectedUpdatedAt = selected?.updated_at ?? null; const selectedIsMissing = selectedId_ != null && details[selectedId_] == null; const lastFetchedRef = useRef(null); + const attemptsRef = useRef<{ revision: string; count: number }>({ + revision: "", + count: 0, + }); const [retryTick, setRetryTick] = useState(0); + // Flips as a fetch settles, successfully or not, which is what lets a failed + // one be noticed at all. + const detailInFlight = selectedId_ != null && loadingDetails.has(selectedId_); useEffect(() => { - if (selectedId_ == null) { + if (selectedId_ == null || detailInFlight) { return; } // `updated_at` advances per poll while streaming and settles when terminal. @@ -561,6 +571,19 @@ export function ApiMonitorPage(): ReactElement { if (!selectedIsMissing && lastFetchedRef.current === revision) { return; } + // A terminal row's revision never advances, so a fetch that failed had + // nothing left to re-run this effect and the payload stayed unavailable + // until the user picked another row. `loadingDetails` changing as the failed + // fetch settles is the trigger; the count bounds it, because the usual + // failure is an entry that aged out of the ring buffer and will never + // arrive however often it is asked for. + if (attemptsRef.current.revision !== revision) { + attemptsRef.current = { revision, count: 0 }; + } + if (attemptsRef.current.count >= DETAIL_FETCH_ATTEMPTS) { + return; + } + attemptsRef.current.count += 1; // Only remember the revision when a fetch really started; the in-flight // guard can refuse, and recording it anyway skips that revision for good. if (requestDetail(selectedId_)) { @@ -579,6 +602,7 @@ export function ApiMonitorPage(): ReactElement { selectedIsMissing, requestDetail, retryTick, + detailInFlight, ]); // The desktop webview's origin is tauri://, not the API server, and the diff --git a/studio/frontend/src/features/hub/hub-page.tsx b/studio/frontend/src/features/hub/hub-page.tsx index 2f0dcb677d..7965c6b2da 100644 --- a/studio/frontend/src/features/hub/hub-page.tsx +++ b/studio/frontend/src/features/hub/hub-page.tsx @@ -1365,6 +1365,22 @@ export function ModelsPage() { const openSelectedModelSettings = useCallback( (ggufVariant: string | null) => { if (!selectedModel) return; + // The card passes null while its own variant lookup is pending or after it + // failed, so this needs the same guard openModelSettings applies: a model + // that needs a quant cannot be configured without one, because the picker + // matches variants exactly and would never find the saved config while the + // API falls back to the bare key and would apply it. + if ( + !ggufVariant && + selectedModel.isGguf && + selectedModel.requiresVariant + ) { + toast.error("Couldn't determine which quant to configure.", { + description: + "Settings for this model are per quant. Check the connection or the model's cache, then try again.", + }); + return; + } // Share the sequence with openModelSettings: a row's variant lookup may // still be pending, and it must not land on top of this one. settingsOpenSeq.current += 1; diff --git a/studio/frontend/src/features/model-picker/model-config/per-model-config.ts b/studio/frontend/src/features/model-picker/model-config/per-model-config.ts index acc8efdefa..049b480f80 100644 --- a/studio/frontend/src/features/model-picker/model-config/per-model-config.ts +++ b/studio/frontend/src/features/model-picker/model-config/per-model-config.ts @@ -690,6 +690,13 @@ export function listPerModelConfigs(): { if (!modelId) { continue; } + // Never report a future-schema record. loadPerModelConfig refuses to apply + // one and eviction refuses to drop one, so handing it to the backfill would + // persist this client's partial reading of it server-side and let an + // API-triggered load apply settings the same client will not apply locally. + if (storedConfigVersion(raw) > STORAGE_SCHEMA_VERSION) { + continue; + } const variant = ggufVariantFromStorageKey(key); out.push({ modelId, diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 83c155a1ce..06d8a00082 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -786,3 +786,41 @@ def test_override_writes_are_ordered_per_model(): assert "previous .catch(() => {}) .then(() => sendModelOverride(" in src # Only the last writer clears the slot, or a queue still building loses order. assert "if (writesByKey.get(key) === write) { writesByKey.delete(key); }" in src + + +def test_backfill_skips_future_schema_local_records(): + """loadPerModelConfig refuses to apply a record written by a newer Studio and + eviction refuses to drop one, because this client cannot interpret that + schema. The enumeration the backfill uses had no such guard, so it would + persist this client's partial reading server-side and an API-triggered load + would then apply settings the same client will not apply locally. + """ + src = " ".join( + _read("features/model-picker/model-config/per-model-config.ts").split() + ) + listing = src[src.index("export function listPerModelConfigs()") :] + assert "storedConfigVersion(raw) > STORAGE_SCHEMA_VERSION" in listing[:900] + + +def test_detail_settings_need_a_resolved_quant(): + """The on-device card passes a null variant while its own lookup is pending + or after it failed. Opening the editor then saves a bare-model config, which + the picker never finds because it matches variants exactly, while the API's + bare-key fallback would apply it. openModelSettings already refuses; this + entry point has to refuse the same way.""" + src = " ".join(_read("features/hub/hub-page.tsx").split()) + guard = "if ( !ggufVariant && selectedModel.isGguf && selectedModel.requiresVariant )" + assert guard in src + assert src.count("Couldn't determine which quant to configure.") == 2 + + +def test_a_failed_detail_fetch_is_retried(): + """A terminal row's updated_at never advances and selectedIsMissing stays + true, so a fetch that failed had nothing left to re-run the effect and the + payload stayed unavailable until another row was selected. The retry is + bounded because the usual failure is an entry aged out of the ring buffer, + which never arrives however often it is asked for.""" + src = " ".join(_read("features/api-monitor/api-monitor-page.tsx").split()) + assert "const DETAIL_FETCH_ATTEMPTS = 3;" in src + assert "const detailInFlight = selectedId_ != null && loadingDetails.has(selectedId_);" in src + assert "if (attemptsRef.current.count >= DETAIL_FETCH_ATTEMPTS) { return; }" in src