Guard the backfill, the detail settings entry point and the detail retry
Three separate reports, all confirmed against head. listPerModelConfigs reported future-schema records. loadPerModelConfig refuses to apply one and eviction refuses to drop one, because this client cannot interpret that schema, so handing it to the backfill would persist a partial reading of it server-side and let an API-triggered load apply settings the same client will not apply locally. It is skipped there now, matching the other two paths. The detail view's on-device card passes a null variant while its own lookup is pending or after it failed, and this entry point opened the editor anyway. That 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 with a toast for exactly that reason; this path now refuses the same way. A failed detail fetch was never retried. The revision is recorded when the fetch starts, and on failure the entry stays missing, so selectedIsMissing does not change and a terminal row's updated_at does not advance: nothing was left to re-run the effect, and the full prompt and reply stayed unavailable until another row was selected. The in-flight flag settling is the trigger now, and the attempt count bounds it, because the usual failure is an entry that has aged out of the ring buffer and will never arrive however often it is asked for.
This commit is contained in:
parent
fc06185c12
commit
c2bcb1e6e5
4 changed files with 86 additions and 1 deletions
|
|
@ -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<string | null>(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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue