Let remove win over flag validation, and make Clear log clear shared rows
An explicit remove ran the launch-flag validation first, so a form still carrying a rejected flag raised a 400 and left the override in place. Nothing is stored on that path, so there is nothing to validate; remove now short-circuits it, which is what the branch below already claims to do. Clear log dropped only the caller's own rows, but a lifecycle row is shared: it is visible to everyone and owned by no one, so those rows survived and the reload straight after the click brought them back, leaving the button visibly ineffective. Deleting them is not an option either, since that erases another caller's history. They are now hidden per subject, so the clear is true for that caller and harmless to the rest. A shared row that is still running is live state rather than history, so it stays visible, and the hidden ids are pruned against the ring buffer so they cannot accumulate.
This commit is contained in:
parent
c8a4fa0961
commit
88bf2eacfb
4 changed files with 103 additions and 4 deletions
|
|
@ -125,6 +125,12 @@ class ApiMonitor:
|
|||
enabled: bool = True,
|
||||
):
|
||||
self._entries: deque[ApiMonitorEntry] = deque()
|
||||
# Shared rows one subject has cleared. A shared row belongs to everyone,
|
||||
# so dropping it would erase another caller's history, but leaving it
|
||||
# means "Clear log" visibly does nothing to it: the frontend reloads
|
||||
# straight after and the row comes back. Hiding it per subject is the
|
||||
# only thing that is both true for that caller and safe for the others.
|
||||
self._hidden_shared: dict[str, set[str]] = {}
|
||||
self._max_entries = max(0, max_entries)
|
||||
self._lock = threading.Lock()
|
||||
self._enabled = enabled
|
||||
|
|
@ -403,12 +409,25 @@ class ApiMonitor:
|
|||
with self._lock:
|
||||
if subject is None:
|
||||
self._entries.clear()
|
||||
self._hidden_shared.clear()
|
||||
return
|
||||
# A shared row that is still running is a load in progress, not
|
||||
# history, so it stays visible; clearing the log is about what has
|
||||
# already happened.
|
||||
hidden = self._hidden_shared.setdefault(subject, set())
|
||||
for entry in self._entries:
|
||||
if entry.shared and entry.subject != subject and entry.status != "running":
|
||||
hidden.add(entry.id)
|
||||
self._entries = deque(entry for entry in self._entries if entry.subject != subject)
|
||||
|
||||
@staticmethod
|
||||
def _visible(entry: ApiMonitorEntry, subject: Optional[str]) -> bool:
|
||||
return subject is None or entry.subject == subject or entry.shared
|
||||
def _visible(self, entry: ApiMonitorEntry, subject: Optional[str]) -> bool:
|
||||
if subject is None:
|
||||
return True
|
||||
if entry.subject == subject:
|
||||
return True
|
||||
if not entry.shared:
|
||||
return False
|
||||
return entry.id not in self._hidden_shared.get(subject, ())
|
||||
|
||||
def _find_locked(self, entry_id: str) -> Optional[ApiMonitorEntry]:
|
||||
for entry in self._entries:
|
||||
|
|
@ -427,6 +446,13 @@ class ApiMonitor:
|
|||
kept.append(entry)
|
||||
terminal_seen += 1
|
||||
self._entries = kept
|
||||
# The hidden sets only ever name rows that exist, so they stay bounded
|
||||
# by the ring buffer rather than growing for the life of the process.
|
||||
live = {entry.id for entry in kept}
|
||||
for subject, hidden in list(self._hidden_shared.items()):
|
||||
hidden &= live
|
||||
if not hidden:
|
||||
del self._hidden_shared[subject]
|
||||
|
||||
|
||||
api_monitor = ApiMonitor(enabled = not _api_monitor_disabled())
|
||||
|
|
|
|||
|
|
@ -414,7 +414,11 @@ def update_openai_auto_switch_override(
|
|||
bare_id = _bare_model_id(payload.model_id)
|
||||
if bare_id:
|
||||
requested_extra_args = get_model_override(bare_id).get("llama_extra_args")
|
||||
extra_args = validate_extra_args(requested_extra_args)
|
||||
# Not validated on an explicit remove: nothing is stored, so the only
|
||||
# effect would be a 400 that leaves the override in place, which is the
|
||||
# opposite of what remove means. A stale form still carrying a rejected
|
||||
# flag must not be able to block forgetting a model.
|
||||
extra_args = [] if payload.remove is True else validate_extra_args(requested_extra_args)
|
||||
if payload.remove is True:
|
||||
# An explicit remove wins over anything else in the payload: a stale
|
||||
# form field must not turn "forget this model" into an update that
|
||||
|
|
|
|||
|
|
@ -468,3 +468,54 @@ def test_request_rows_report_kind_request():
|
|||
monitor = ApiMonitor(max_entries = 2)
|
||||
monitor.start(endpoint = "/v1/chat/completions", method = "POST", model = "m", prompt = "hi")
|
||||
assert monitor.snapshot()[0]["kind"] == "request"
|
||||
|
||||
|
||||
def test_clear_hides_shared_lifecycle_rows_for_that_caller_only():
|
||||
"""A lifecycle row is shared, so it is visible to every caller but owned by
|
||||
none. A subject-scoped clear dropped only that subject's own rows, so the
|
||||
shared ones survived and the reload straight after "Clear log" brought them
|
||||
back: the button visibly did nothing to them. Dropping them outright is not
|
||||
an option either, since that erases another caller's history.
|
||||
"""
|
||||
monitor = ApiMonitor(max_entries = 10)
|
||||
mine = monitor.start(
|
||||
endpoint = "/v1/chat/completions",
|
||||
method = "POST",
|
||||
model = "org/A",
|
||||
prompt = "user: hi",
|
||||
subject = "alice",
|
||||
)
|
||||
monitor.finish(mine)
|
||||
shared = monitor.record_lifecycle(event = "unload", model = "org/A")
|
||||
|
||||
assert {e["id"] for e in monitor.snapshot(subject = "alice")} == {mine, shared}
|
||||
assert {e["id"] for e in monitor.snapshot(subject = "bob")} == {shared}
|
||||
|
||||
monitor.clear(subject = "alice")
|
||||
|
||||
assert monitor.snapshot(subject = "alice") == []
|
||||
# Bob's view is untouched: the row is hidden for alice, not deleted.
|
||||
assert {e["id"] for e in monitor.snapshot(subject = "bob")} == {shared}
|
||||
assert monitor.get(shared, subject = "alice") is None
|
||||
assert monitor.get(shared, subject = "bob") is not None
|
||||
|
||||
|
||||
def test_clear_leaves_a_running_shared_row_visible():
|
||||
"""A load still in progress is live state, not history, so clearing the log
|
||||
must not hide the row that shows it."""
|
||||
monitor = ApiMonitor(max_entries = 10)
|
||||
running = monitor.record_lifecycle(event = "load", model = "org/A", running = True)
|
||||
monitor.clear(subject = "alice")
|
||||
assert {e["id"] for e in monitor.snapshot(subject = "alice")} == {running}
|
||||
|
||||
|
||||
def test_hidden_shared_ids_do_not_outlive_their_entries():
|
||||
"""The hidden set names rows that exist, so it stays bounded by the ring
|
||||
buffer instead of growing for the life of the process."""
|
||||
monitor = ApiMonitor(max_entries = 2)
|
||||
monitor.record_lifecycle(event = "unload", model = "org/A")
|
||||
monitor.clear(subject = "alice")
|
||||
assert monitor._hidden_shared.get("alice")
|
||||
for i in range(5):
|
||||
monitor.record_lifecycle(event = "unload", model = f"org/M{i}")
|
||||
assert not monitor._hidden_shared.get("alice")
|
||||
|
|
|
|||
|
|
@ -4648,6 +4648,24 @@ def test_case_fallback_still_covers_repo_ids(monkeypatch):
|
|||
assert settings.get_model_override("unsloth/Qwen3-8B-GGUF:Q4_K_M")["max_seq_length"] == 8192
|
||||
|
||||
|
||||
def test_explicit_remove_is_not_blocked_by_stale_invalid_flags(monkeypatch):
|
||||
# remove is the operation discriminator, so a form still carrying a rejected
|
||||
# launch flag must not turn "forget this model" into a 400 that leaves the
|
||||
# override in place. Nothing is stored on this path, so there is nothing to
|
||||
# validate.
|
||||
import routes.settings as settings_route
|
||||
|
||||
_mock_override_store(monkeypatch)
|
||||
settings.set_model_override("unsloth/B-GGUF", max_seq_length = 4096)
|
||||
resp = settings_route.update_openai_auto_switch_override(
|
||||
settings_route.ModelOverridePayload(
|
||||
model_id = "unsloth/B-GGUF", remove = True, llama_extra_args = ["--port", "1234"]
|
||||
),
|
||||
"tester",
|
||||
)
|
||||
assert "unsloth/B-GGUF" not in resp.overrides
|
||||
|
||||
|
||||
def test_explicit_remove_wins_over_config_fields_in_the_same_payload(monkeypatch):
|
||||
# remove is the operation discriminator, so a stale form field alongside it
|
||||
# must not quietly turn "forget this model" into an update.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue