mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-23 15:53:46 +00:00
258 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
shimmyshimmer
|
fb4d8a8460 |
default new installs to HTTPS downloads, and put the choice in Settings
The download transport preference defaulted to "auto", which resolves to Xet on any machine whose hf_xet looks healthy, so in practice everyone downloaded over Xet without ever choosing it. New installs now download over HTTPS: it resumes a cancelled or dropped transfer, and it needs nothing but ordinary TLS. Installs from before this default keep Auto, seeded once from positive evidence of prior use (a row the user wrote, or a manifest from an earlier download) and persisted, so updating changes nobody's downloads. Settings > General gains a Downloads section with an HTTPS / Xet / Auto control, the difference between the two spelled out, the reason Xet is unavailable when hf_xet is missing, and what Auto resolves to on this machine. use_xet defaulted to True in the download schemas, so a scripted caller that stated no transport got Xet whatever the preference said. It is optional now, and a request stating neither field follows the install's setting. |
||
Daniel Han
|
7266f1b29b
|
Stop a finished save's ingest watcher answering the next test's stub (#9200)
Reduced to the one fix of the original three that is still needed. The other two landed by other routes while this sat: the orphan-scan flake was fixed by #9280, and the extensionless `./mmproj-fallback` import is already `.ts` on main. Both are verified identical to main here rather than assumed. On #9280 taking a different approach: it pins `_pid_parent_is_alive` to a constant for the whole test, where this PR stubbed it only for the invented PID and delegated every other PID to the real implementation. That is a real difference in coverage, so I checked whether main lost anything before dropping mine. It did not: `test_reap_recorded_pid_spares_live_server` in test_llama_cpp_wait_for_vram_settle.py exercises the ownership gate for real, against a process the test genuinely owns. So the gate is still covered and #9280's version stands. What is left, and why it is still needed ------------------------------------------------------------------------ `saveMarkdownAsProjectSource` fires `watchIngestion` without awaiting it, and that watcher polls for up to 300 seconds. Watchers from earlier tests therefore outlive them and keep calling the shared fetch stub, which by then belongs to whichever test is running now. The stub answered any `/jobs/` URL and the uploads shared a filename, so a stale watcher emitted a toast byte-identical to the one the running test was waiting on, under a different project id. The test then waited on the toast and read the announce count without polling. The watcher toasts and announces with nothing awaited between them (save-markdown-source.ts, `toast.error(...)` then `announceProjectSourcesUpdated(...)`), so a tick landing in that window saw 1 instead of 2. Each job handler now answers only its own job id and 404s anything else, so a stale watcher dies quietly instead of borrowing the running test's answers. The save under test gets its own filename, and the test polls on the announce, which the watcher does last, then asserts the toast synchronously. Verified ------------------------------------------------------------------------ Gap sweep, inserting a 150 ms delay into the source between the toast and the announce to widen the window the race needs: main's version of the test fails 6 of 6 runs, this version passes 6 of 6. Both pass 6 of 6 on an idle box with no delay, which is why this reads as an intermittent CI failure rather than a reproducible one. Mutation-tested: deleting `announceProjectSourcesUpdated` from the watcher fails this test (9 passed, 1 failed), so it still proves what it did before rather than having been loosened into passing. Full frontend suite: 4087 passed, 0 failed. |
||
|
Daniel Han
|
f68cbac1f0
|
Drain the sampling suites on the store's write chains, not on node's loader (#9367)
#9332 fixed these suites by waiting on two observables: the mocked timers, which it counts exactly, and the module loader, which it raced by issuing an import of its own each round. The second one works and is still the wrong thing to assert. The loader probe rests on an assumption about node internals that I recorded in the comment and never verified: that the hooks thread serves requests in order over one port, so a reply to a request issued after the store's cannot arrive first. If that ever stops holding it fails as a stale read, which is precisely the failure mode #9332 existed to abolish -- a wait that comes up short reports "chat A minP: owed 0, shows 0.01" and sends the next reader into the store. The store already tracks the thing worth waiting on. threadSettingsWriteChains holds the live promise chain per thread, and awaitThreadScopedSettingsWrite already awaits one of them by id. What a drain needs is all of the started ones, without knowing which chats the store decided to write, so this adds awaitStartedThreadScopedSettingsWrites. It is deliberately not a flush. A debounce that has not fired yet is left alone, so a caller cannot use it to make a write happen earlier than the store would have; that would let a test pass against a store that never scheduled the write at all. It is bounded at 20 passes rather than looping until the map empties, so a write that keeps rescheduling itself surfaces as a failed assertion instead of a hang. It repeats rather than awaiting one snapshot because a chain that settles can leave a newer one behind it for the same chat. drainMockedTimers now takes that as a caller-supplied `barrier` instead of carrying its own loader probe. The timer counting is unchanged, and so is the exhaustion throw, which is the property that keeps an under-drain from ever again looking like a lost edit. The approach is the one from #9352, which I opened against the same bug in parallel and is closed in favour of this. #9332's mechanism landed first and turned Windows green; this replaces the part of it that was a guess about the runtime with a measurement of the subject. Verified ------------------------------------------------------------------------ Both suites twice on v24.14.0 and twice on v22.23.2, the version setup-node 22 resolves to: compat 16 passed, simulation 18 passed, every run. Full frontend suite 4080 passed, 0 failed. Mutation-tested on v22.23.2, which is the only runtime where this is observable: disabling the barrier and leaving the timer counting in place gives 8 failed, 10 passed. So the barrier is carrying the load, not decorating a wait that already worked. |
||
oobabooga
|
4c5423a36a
|
Studio: stop docked composer from squeezing download panel (#9334)
* Studio: stop docked composer from squeezing download panel * Floor the overlay rail card by card * Measure the rail's cards off the layout box * Tighten the new comments * Drop out-of-flow cards from the rail's measurements |
||
|
Daniel Han
|
1c3dde199b
|
Drain the sampling debounce long enough for the node CI actually runs (#9332)
* Stop Frontend CI installing Chromium's system libraries through apt every run `playwright install --with-deps chromium` runs its own `apt-get update` inside itself, so it bypassed everything CI has learned about apt: the shared retry helper's 20s transfer cap, APT_ACQUIRE_RETRIES: '0', and the archive cache. The job failed 3 of 8 runs on main. Job 96072994354 (main, 2026-08-19): 9 packages, 21.1 MB, and `fonts-wqy-zenhei [7472 kB]` alone took 5m50s off azure.archive.ubuntu.com. Both 420s attempts died mid-download. That is the same mirror and the same package that took the webkit shards down in #9289. Attempt 2 logged "Need to get 8833 kB/21.1 MB", so apt does resume partials across attempts and still could not finish. Split the way studio-ui-smoke.yml splits it: download the engine, launch it to find out whether the system libraries are actually missing, and run `install-deps` only if they are. ubuntu-latest is a browser-testing image and usually ships them, so the common path now runs no apt at all. The browser and apt-archive cache keys are deliberately identical to the chromium-only shards in studio-ui-smoke.yml (engine token `c`): same image, same Playwright version, same single engine, so the entry is shared rather than duplicated against a budget measured at 99.3% full. The step's authorised worst case doubles with the second helper call, to 2 x (2 x 420s + 125s) = 1930s, so its timeout goes 17m -> 33m and stays under the job's 40m. Both guarded calls are skipped on the common path. Guard: tests/studio/test_playwright_install_avoids_with_deps.py fails the build if `--with-deps` returns to any workflow, and is wired into workflow-trigger-lint, the only job with no paths filter. * Drain the sampling debounce long enough for the node CI actually runs Frontend CI has been red on main since #9055, not intermittently: eight consecutive main runs failed at `Unit tests`, every one on node v22.23.2. The three suites from #9055 wait for a debounced write with a fixed drain -- three rounds of tick(1000) plus six setImmediate turns -- and then assert. Three rounds is enough on node 24, which is what a dev box happens to have, and is not enough on node 22, which `setup-node: 22` resolves to. The same chain drains far fewer continuations per round there, so the write had not landed when the assertion ran. Reproduced by downloading node 22.23.2 and running the suites against both. Measured on the compat suite: rounds 3 10 30 60 failing 7 5 1 0 The compat suite reported it as a missing value (expected 1.37, actual undefined). The simulation suite reported it as an ORDERING violation -- "chat A temperature: owed 0.6, shows 1.37" -- because a scenario whose write has not landed looks exactly like one that wrote the wrong value, which is why this read as a feature bug rather than a slow test. There were three copies of the drain: one per suite plus the shared thread-sampling-world helper the simulations run through. Fixing only the two suites left the simulations red, since their scenarios drain through the helper. The bound is 200, generous rather than tuned to the observed 60, and settle() now takes an optional `until` predicate: it returns as soon as the caller can see the work, and throws naming itself if the condition never holds, so the next slow runtime reports "settle gave up" instead of an assertion on a missing value. Verified: node 22.23.2 compat 16/16 and simulation 18/18 (both were failing); node 24 full frontend suite 4060/4060. Only the simulation suite imports the helper, and no fixed three-round drain remains in tests/. * Raise the scenario drain to 600: 200 was marginal, and Windows needed more At 200 the simulations were 0 failing on one run and 1 on the next on the same machine, and the Windows runner -- slower again -- still had 2 of 120 orderings short. 600 is 0 failing across three consecutive runs, at 107s against 52s. Also records the adaptive version that was tried and is wrong here, so it is not tried again: the rows only change WHEN the write lands, so 'rows have stopped changing' is precisely the pending state being waited through. Quiescence on that observable stops early by construction and scored 4 failures where the fixed bound scored 0. * Stop the settings smoke asserting a tab count a new page invalidates Unskipping the browser smokes surfaced this immediately: the blocked-panel run failed with 'blocking the data panel took the dialog down' while its own report said dialog: True. Nothing had taken the dialog down. The check was if not state["dialog"] or state["nav"] != 12: and the keyboard-shortcuts page had made the nav 13. A stale constant, reading as an error-handling regression. The nav size is now read before the panel is blocked and compared against itself, which is the invariant that was meant: blocking a panel must not collapse the dialog, whatever size the dialog is. The same drift had a quieter half. The smoke's TABS list still had twelve entries, so keyboard-shortcuts had no browser coverage at all and the smoke stayed green without it. It is added here, and tests/studio/test_settings_smoke_covers_every_tab.py pins both directions against settings-dialog.tsx so the next page cannot go uncovered silently. It also checks the workflow's PW_CHUNK_FAIL names a tab that exists -- that value lives in studio-frontend-ci.yml, not in the smoke, and a rename would leave the run blocking nothing while still reporting PASS. Wired into workflow-trigger-lint, the only job with no paths filter, because it reads a workflow. Mutation-tested both ways: dropping the tab from TABS and restoring the literal count each turn it red. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drain the sampling suites on the loader too, and fail loudly when it gives up The fixed round count in the previous commit was still a guess, and the Windows job proved it: the SAME commit at 600 rounds passed one run and failed the next with "A1: 2 violation(s) across 120 orderings", reporting stale reads as lost edits. Raising the number again was not the answer. The pending work has a second half nothing was waiting on. The store's thread-scoped write ends in `await import("../utils/chat-history-storage")` (chat-runtime-store.ts:1326 and :1754), and these suites register() a resolver hook, which routes that import through the hooks thread. Three repeat imports of an already-loaded module: v24.14.0 no hook 1, 1, 1 turns hook registered 1, 1, 1 v22.23.2 no hook 1, 1, 1 turns hook registered 6, 3, 35 That is the whole green-locally / red-on-CI split, and it is why a loaded Windows runner fails what the same commit passed an hour earlier: the pending work is a message to another thread, so its cost is scheduling latency, not instructions. No round count is correct for that. Counting the mocked timers alone does not cover it either, which is worth recording since it is the obvious next idea. With the counter installed and 25 consecutive quiet rounds per drain, 150 macrotask turns of nothing, v22.23.2 still lost 7 orderings across A1 and A3, every one a write that had not landed. So drain on both observables. tests/helpers/mock-timer-drain.ts wraps the MOCKED setTimeout with a counter, giving an exact count of timers scheduled and not yet fired or cleared, and each round also issues its own import and waits for it, so the wait scales with the loader instead of guessing at it. The drain returns when no timer is outstanding and three consecutive rounds neither scheduled nor fired one. With the probe, three quiet rounds is green on v22 and v24 alike. The generous bound stays as a BACKSTOP that THROWS and names what was still outstanding, which is the change that matters most here. An under-drain used to be indistinguishable from the store losing an edit, so it sent the investigation into the store for two rounds. Now it says so: settle: drain exhausted after 2 rounds, with no timer pending but work still scheduling or firing within the last 3 rounds. Nothing read after this point is trustworthy: a queued write has not landed, so the store still shows the PREVIOUS value, which reads as a wrong value rather than a missing one. Fix the work or raise the backstop; do not read this as the store losing an edit. It is also much faster, because it stops when the work is done rather than after 600 rounds regardless. A typical drain now takes 4 rounds; instrumented over 840 drains in A1 the maximum was 4. simulation v24 101.7s -> 30s v22.23.2 106.5s -> 29s compat v24 0.54s -> 0.46s v22.23.2 0.61s -> 0.52s The three copies of the drain shape, one in the world helper and one in each test file, are now one helper. The dead end already recorded is kept next to the new measurements: quiescence on the store ROWS is still wrong, because the rows only change WHEN the write lands. Verified: both suites green twice each on v24.14.0 and on v22.23.2, the version setup-node 22 resolves to. Full frontend suite 4080 passed, 0 failed. The exhaustion throw was confirmed by forcing the backstop to 2, which produces the message above and no wrong-value violation. A deliberately broken sanitizeThreadScopedSettings still produces a real ordering violation on both runtimes, so the drain is not exiting early. Test files only. * Read the workflow and the dialog as UTF-8, not as the platform default test_source_read_encoding caught four read_text() calls this PR added with no encoding. It is right and they are a real defect: the guard reads settings-dialog.tsx, playwright_settings_tabs.py and studio-frontend-ci.yml, and on a Windows runner Path.read_text() uses the ANSI code page, so any non-ASCII byte in any of them raises UnicodeDecodeError. The whole point of this guard is that a settings page can be added without anyone noticing; a guard that cannot be collected on Windows fails the same way. Repo tests (CPU) was otherwise clean: 1 failed, 8781 passed. * Tell a pwsh crash apart from install.ps1 losing its exit code test_the_pwsh_filter_keeps_the_log_clean_and_the_exit_code_intact went red on a hosted ubuntu runner with completely empty stdout and pwsh's own banner: "An error has occurred that was not properly handled. Additional information is shown below. The PowerShell process will exit." The interpreter died; the script never ran. Workflow trigger lint is green on the last 8 main runs and this PR touches no PowerShell, so it is the runner, not the repo. Read as an ordinary failure it says "$LASTEXITCODE did not survive the added pipeline stages, so a failing install.ps1 would leave its step green" -- an accusation against the installer, raised by a run that produced no evidence either way. That is the same shape as the drain in this PR: an environment shortfall wearing the costume of a product regression. _run_pwsh retries only that case, and the distinction is what keeps it honest: a run that reaches the `RC=` line is returned on the first attempt whatever the value, so a genuine regression can never be retried into green. Only a run with no RC= AND the crash banner is retried, because it carries no verdict to preserve. If both attempts crash it fails with a message naming the interpreter, not install.ps1. 17 passed. Exercised _run_pwsh against a script that prints the banner and no RC=: it raises, so the branch is not vacuous. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
18b97f8b72
|
Studio: keep and search the turns rolling context evicts (#9074)
* Studio: add rolling context windows for local GGUF chat
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: return rolling context metadata for non-stream chats
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep original context when rolling fit fails
* Studio: keep instruction groups independently protected
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: preserve rolling context metadata across retries
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: count sanitized rolling context prompts
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: refit rolling context after respawn
* Studio: scope middle truncation to passthrough
* Studio: refit tool prompts after respawn
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: retain later choice truncation metadata
* Studio: report clipping-only context truncation
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: expose the turns a rolling fit evicts
Rolling context eviction currently drops turns with no way for a caller to
learn which ones went. Extract the turn grouping so the eviction unit is
reusable, add an identity-based diff of what a fit removed, and let a caller
reserve room for content it intends to add back after fitting.
The reserve deliberately does not affect whether trimming happens at all, only
how far it goes once it is already required, so a conversation that fits today
is still returned untouched.
* Studio: archive evicted turns into a per-thread rag scope
An evicted turn is currently gone for the rest of the session, so the model
will state the conversation began wherever its visible context begins. Keep the
turns the rolling window drops in a searchable scope built on the existing
store, chunker, embedder and hybrid retrieval.
The archive is cumulative: every compaction adds to it and nothing is cleared,
so a later compaction can still find what an earlier one evicted. It lives in
its own scope rather than the thread's document scope, because thread documents
are injected in full on every request and would re-inject the whole history.
Idempotent by content hash, since the same turns are evicted again on every
later request. Every entry point degrades to a no-op rather than raising.
* Studio: recall archived turns on the turn that evicted them
Given only a search tool, a model decides for itself whether to look, and
mostly does not. Measured on MRCR v2, a 35B declined on 56% of rows, scoring
0.099 when it skipped against 0.461 when it searched. Forcing one retrieval on
the compaction turn took tool-only 0.258 to 0.604, and the model then called
the tool on 0% of rows, so the common path costs nothing extra.
Recall fires at most once per request, since the tool loop refits on every
iteration. The tool loop renders it as an ordinary tool exchange through the
builder shared with document auto-inject; the plain path prefixes the latest
user message instead, because it sends no tools array and a tool role without
one breaks strict chat templates.
Only scalar counts join the context_truncated event, so no message content
reaches the wire.
* Studio: let the model search a compacted conversation
Forced recall answers the turn that evicted, but a later turn can refer to
something the forced pass had no reason to fetch. Add search_conversation so
the model can go looking, scoped to the thread's own archive and sharing the
admission slot with document search so the two cannot race for the embedder.
The tool is offered only once a thread has actually had turns archived, so an
ordinary short chat never pays for the schema, and it is classified read-only
so auto mode does not prompt on every call. A matching system-prompt note tells
the model the session was compacted, since otherwise it assumes the
conversation began where its visible context begins.
Deleting a thread now drops its archive rather than leaking a scope per chat.
* Studio: read thread_id defensively when selecting tools
_select_request_tools also serves the token-count request model, which has no
thread_id field, so the archive gate raised there.
* Studio: retrieve archived turns lexically first
Recalling your own conversation is mostly an exact-match problem: a name, a
number, an identifier someone pasted twenty turns ago. Those live or die on
rare-token matching, and hybrid fusion was losing them.
Measured on a 30-turn walkthrough of a 230k-character document at a 16k window,
where every turn shared the same wrapper text. The chunk holding the needle
ranked 3rd lexically at any k, was never returned by dense retrieval at all,
and RRF pushed it to 16th because it had 30 useless dense hits to fuse with.
End to end the model answered with the exact code once lexical leads, and could
not answer at all before.
Dense still fills whatever the lexical pass leaves, for paraphrased recall.
* Studio: keep recall on the branch the user is actually on
Editing an earlier message rewinds a thread and continues down a new branch,
but the archive is append-only and still holds everything the abandoned
continuation produced. Verified against a live build: after rewinding past a
turn, querying its distinctive text still returned it, so the model could be
handed a turn that on this branch never happened.
Recall now drops archived turns that are absent from the thread's saved
transcript. Threads with no saved transcript are left unfiltered, since an API
caller may pass a thread_id without persisting messages and an empty transcript
is absence of evidence rather than evidence the turns are gone.
Containment on a normalised prefix rather than a digest, because the archived
copy is rendered from the inference projection and the saved copy comes back
through the message store.
* Studio: show a persistent compaction notice on the turn that compacted
The only signal that a long chat had been compacted was a toast, which vanishes after
a few seconds and does not survive a reload. A user who scrolls back later has no way
to find out why the model seemed to forget the start of the conversation.
The notice renders from metadata.custom.contextTruncation inside the assistant
message's own container, so it is not part of the conversation sent to the model, is
not editable, and is not exported as content, but it stays attached to the turn it
describes. It reports how many messages were dropped and, when the conversation
archive is on, that they are still searchable and how many passages were recalled.
* Studio: show the compaction notice once, on the turn it started
A thread that has outgrown its window compacts on every turn from then on, not
just the first, so a notice per compacted turn was a notice on every reply for the
rest of the conversation. The user needs telling once.
The notice is now gated on being the first compacted assistant turn in the thread,
found by walking the thread rather than assuming the compacted turns are contiguous
or that this is the last one, so a rollback that removes the turn it was on moves it
to whichever turn now compacts first. The wording follows: it describes the state
the conversation is in from here on, and carries the counts from the turn it began
on in parentheses.
* Studio: make compaction an occasional event instead of every turn
The fit was stateless. The client re-sends the whole saved transcript on every
request, so "keep the newest N tokens" recomputes from scratch each time and slides
forward a turn or two at a time. Measured on a 40-turn thread against an 8k window,
the eviction boundary moved on 12 of 40 turns, which means every few replies quietly
lost a little more of the conversation, llama-server's prefix cache was thrown away
each time the head of the prompt moved, and there was no such thing as a compaction
event to tell the user about.
Two changes make it discrete. The fit now reads back the boundary the thread last
compacted to, from the newest assistant turn's own persisted truncation, and reapplies
it before deciding anything, so nothing new is stored and it survives a restart. And
when the boundary does have to move, the trim takes a further ROLLING_COMPACTION_
HEADROOM_RATIO of the budget out (default 0.25) rather than skimming to the brim, so
the new boundary has room to stay put. Same 40-turn thread, same window: 4 compactions
over 60 turns instead of 14, keeping about 82 percent of the usable budget.
Both are gated on the prompt not already fitting, exactly as the recall reserve is. A
conversation inside its window is never evicted to satisfy either, and a stale boundary
from a branch that was rolled back cannot evict a chat that now fits.
The notice follows: it is shown when dropped_messages rises above the last turn that
reported it, so it appears once per compaction and stays quiet in between.
* Studio: pin that the compaction notice can never become conversation
The notice is a sidecar rendered from metadata, not a message, and the ways that
could quietly stop being true are all one careless edit away: moving it inside
MessagePrimitive.Parts would make it a content part, and everything that walks parts
would then replay it to the model, copy it and export it.
Asserts it renders as a sibling of the content parts, that neither the outbound
message builder nor the assistant replay serialiser reads the key it renders from
(bounded to those function bodies, since the streaming handler reads the same key
legitimately on the way in), that the markdown export does not mention it, and that
it is suppressed while editing so it cannot be typed into the textarea and saved
back as text.
* Studio: say which part is too long when nothing can make a turn fit
If the message just sent is itself bigger than the window, no amount of eviction
helps. The fit already handled that correctly, returning the conversation untouched
so the request reaches llama-server's normal context-length error, but what the user
was then told was actively misleading: the error reports the size of the WHOLE
conversation and advises shortening it, when the history has already been evicted and
the single message is the part that does not fit. Measured at a 4096-token window: a
5000-token message produces 'Message too long: 10290 tokens ... shorten the
conversation', and shortening it cannot possibly work.
The fit now returns a fits:false diagnosis instead of a bare None, carrying what the
conversation could not be reduced below and how much of that is the latest turn.
Every consumer already gated on fits, so this is inert wherever a truncation is
treated as a compaction; the streaming paths now forward it so the client can use it.
The toast reads the diagnosis and, when the latest turn alone exceeds the window,
says so with the numbers instead of offering advice that leads nowhere. The merge
drops the diagnosis once a later refit succeeds, by delete rather than by assigning
undefined, so an ordinary response keeps exactly the shape it had before.
* Studio: fix the review findings on the conversation archive
Fifteen items, each reproduced against the code before changing anything.
Correctness in the request path. The fit reported a successful recall-capable fit
whenever the result was under the prompt budget, but protected messages can stop the
trim reaching the reserve target, and the recall then went in anyway: reproduced at
ctx 8000, the fit accepted at 6900 and recall took the request to 8948, past the
window it had just been made to fit. Recall is now sized from the room the fit
actually obtained, and skipped when there is none. Separately, the sticky boundary
describes the original transcript, so re-applying it on a later tool-loop fit evicted
another boundary-sized block of live history: measured, a second fit dropped 28 of the
30 surviving messages instead of 14, and the summed count persisted an inflated
boundary for the next request. It is now spent after the first fit of a request.
Availability and privacy. enabled() trusted RAG_AVAILABLE, which only records that
import sqlite_vec worked; rag_available() exists because the native vec0 library it
loads is a separate file a venv can lack. On such a machine the fit held a recall
reserve back, evicting extra history, and then both the archive write and the recall
failed, so the user paid for content they never got. And a temporary chat is never
written to studio.db, yet the frontend still sends its thread_id and the request
carries no incognito flag, so its turns were archived to a scope no deletion flow
could reach. Archival now requires the thread to be persisted, which is the same rule
that keeps every archive reachable by a delete. Clear-history and project deletion
drop archives too; only DELETE /threads did.
Archive integrity. The document was committed before its chunks, so a failed chunk
write left an empty row marked completed that document_by_hash then skipped forever;
both now go in one transaction. The live-branch filter accepted a turn on its first
matching line, so editing only the assistant half kept serving the old answer; the
whole turn must be present. Retrieval fetched exactly k before that filter, so stale
turns could starve live ones and recall returned nothing; it over-fetches first. Tool
turns archived only the tool name, which cannot answer what was actually run, so a
bounded rendering of the arguments and the assistant text goes in as well.
The tool surface. Studio always sends an explicit enabled_tools array and has no
reason to name an internal tool, so the allowlist filter removed search_conversation
before the archive gate ran and the tool, plus the compaction nudge gated on it, never
appeared in a Studio chat. It now follows the archive rather than the allowlist. The
forced recall rendered a tool exchange even when the tool was absent from the
catalogue, which is the strict-template hazard the plain path avoids; it picks inline
in that case, and always inline for the final-answer request, which sends no tools at
all. A model-supplied top_k reached a slice as out[:-1] and returned nearly the whole
candidate pool, so it is clamped. Both retrieval tools now share the per-turn search
cap; only the knowledge-base one was counted.
The UI. A fits:false diagnosis is the fitter reporting it could NOT fit, so toasting
that older turns were removed was untrue and burned the once-per-thread flag a later
real compaction needed. The too-long advice compared the latest turn against the raw
context length rather than the prompt budget, so a 3,500-token message in a
4,096-token window was still told to start a new chat, which fails identically.
* Studio: fix the follow-up findings on the archive fixes
Four items, three of them about last round's own fixes.
Archived tool turns had become permanently unrecallable. render_turn now writes
'assistant called X: args' and 'tool result: ...' lines, while assistant-ui persists a
tool call as a structured tool-call content part that the transcript flattener dropped;
with every archived line required to appear in that transcript, no tool turn could ever
match. The transcript now flattens toolName, args and result, and the probe strips the
'assistant called <name>:' label, which is ours rather than the stored message's.
The branch probe compared only the first 160 normalized characters of each line, so an
edit to the tail of a long answer left the stale copy eligible. Ordinary lines are now
compared whole; tool results keep a prefix, since render_turn deliberately truncates
those and the archived copy is not meant to equal the stored one.
The forced recall sized itself by dividing the remaining budget by CHUNK_TOKENS, which
is an embedding-token limit rather than the chat template's cost, and prices none of
the wrappers around the injection. It is now recounted with the same tokenizer the fit
used and dropped if it overshoots, so the estimate can no longer eat the reply reserve.
An omitted top_k on search_conversation defaulted to the clamp ceiling of eight rather
than the configured recall default, so an ordinary search could return eight archived
turns into the protected current exchange that rolling truncation cannot evict.
* Studio: filter conversation recall to the active branch
A thread's stored rows are the whole message DAG. Retry and regenerate keep the
replaced response as a sibling on purpose, so filtering recall against the whole
thread cannot tell a live turn from one the user replaced, and an archived copy of
the abandoned response could be recalled into a branch where it never happened.
Filter against the messages the request was actually sent with instead, which is
one branch by construction, and hand the same branch to search_conversation so the
model cannot ask for what the forced recall refused. Falls back to the thread-wide
blob for a caller with no branch to offer.
Also retunes two respawn-refit fixtures whose windows no longer produced two
compactions after compaction started trimming a headroom margin below the budget.
* Studio: survive a delete mid-archive, and read the boundary off the active branch
Deleting a chat cancels its generation, but cancellation is cooperative and the
chunk-and-embed pass between the archive's liveness check and its commit does not
observe it. A delete landing in that window drops the thread's rows and sweeps its
scope before the commit puts rows back, leaving content the user deleted in a scope
no later delete can reach. Re-check after the commit and drop the scope: the delete
route removes rows first and sweeps archives last, so either order converges.
The sticky compaction boundary had the same thread-wide read as recall did. The
stored rows are the whole DAG ordered by creation time, so after a Retry the newest
assistant turn can be the sibling the user switched away from, and its boundary is
sized for history the active branch does not have. Resolve it against the request's
own messages instead.
* Studio: give the safetensors loop the same conversation-search guards
search_conversation is advertised by thread, not by backend: the tool selector is
shared, so a chat compacted under a GGUF model still offers it after the user
switches to a safetensors one. The safetensors loop had neither guard the GGUF loop
applies to it. It passed no active branch, so a search there fell back to the
thread-wide rows and could answer from a branch Retry left behind, and it capped
only search_knowledge_base, so paraphrased conversation searches could append
archived passages into the protected current exchange on every iteration until the
window failed. The shared set of capped retrieval tools now lives beside the cap.
The forced recall also derived its query from the loop conversation, which on a
later iteration can end with an internal user-role re-prompt rather than anything
the user wrote. It reads the request branch's own latest user turn instead.
* Studio: widen recall past an abandoned branch, and name whose turn overflowed
One over-fetch is not enough for the live-branch filter. Rewinding or retrying a
continuation that had already been compacted leaves enough stale turns to fill any
fixed candidate window, and the whole page is then rejected while the live match
sitting just below it is never examined, so recall reports nothing although the
answer is in the archive. Widen and re-ask instead, stopping as soon as there are
enough live hits, when the archive stops yielding candidates, or at a bound.
The irreducible-fit diagnosis also carried the size of the last message without
saying whose it was. A tool loop refits with the tool result appended, so that turn
is often output the user never wrote and cannot edit, and the client told them to
shorten it. It now reports the role, and the advice splits on it.
* Studio: shrink an over-budget recall, and keep truncated tool turns on their branch
The exact recount is the right gate, but dropping the whole recall when it fails is
the wrong response: with the shipped defaults a full top-K of long turns lands just
over the reserve once the wrappers are priced, which would disable the forced
retrieval on exactly the long conversations it exists for. Halve the number of turns
and re-ask instead, down to one, before giving up.
The live-branch probe keyed its prefix rule off the tool-result label, but a long
tool result is one appended string containing many newlines, so only its first line
carries that label. Continuation lines were compared in full, including the last one,
which is the only line the truncation marker is on and can therefore never appear in
a transcript: every archived tool turn over the cap was rejected as rolled back.
Key off the marker instead, which also covers truncated tool arguments and compares
the full text everywhere else.
* Studio: retire an edited turn's whole archived copy, not just the edited chunk
A turn longer than CHUNK_TOKENS is stored as several chunks of one document, and the
live-branch filter ran per chunk. Editing the second half of a long answer therefore
retired only the chunks carrying the edit, and an untouched earlier chunk of the same
retired turn stayed eligible on its own. The unit that was archived is the turn, which
is what the filter already claimed to enforce.
Validate every chunk of the candidate's document before admitting any of it, cached
per call since candidates from one turn share a document. A query failure falls back
to the per-chunk answer rather than failing the recall.
* Studio: do not hold a recall reserve back for a chat that is never archived
archive_turns refuses a thread with no saved messages, because a temporary chat must
not be persisted into a scope no deletion flow can reach. The reserve did not follow
that rule: it was granted whenever the RAG stack was available, so an incognito chat,
or an API client sending a thread_id without saving anything, paid a full 2,048-token
reserve for content that can never arrive. The fit subtracts the reserve from its trim
target, so that room is bought with evicted history. Measured on a 4K window: 15
tokens of conversation survived a compaction instead of 1,615.
Both now ask one predicate, can_archive, backed by an existence probe rather than a
row load.
* Studio: match archived turns in order, and delete them without sqlite-vec
Independent line membership accepts a turn whose lines were merely rearranged: every
probe still occurs somewhere, so the pre-edit ordering stayed eligible and would be
served back as what happened. Match the probes in order instead.
That only works if both sides agree on the order, and they did not: render_turn writes
a tool call before any assistant text on the same message and the result after it,
while both transcript builders wrote the text first, so a tool turn carrying both was
rejected outright. One flattener now lays out a message the way render_turn does, for
the request shape and the stored shape alike, and it offers both JSON spacings for a
stored call's arguments, whose object form is not the string the model emitted.
Deletion no longer depends on the optional native extension. An archive is only written
while vec0 loads, but the library can stop loading afterwards, and a delete that quietly
did nothing left a deleted conversation's turns on disk to answer again once it loaded.
The fallback removes the text-bearing rows over a metadata connection; the embedding
rows it cannot reach carry no text and resolve through tables that are gone.
* Studio: give the provider loops the branch, and keep the thread across a respawn
The provider tool loops take their catalogue from the same selector as the local ones,
so search_conversation is advertised there too once a thread has an archive, but the
loop passed no active branch and its searches fell back to the whole stored DAG, where
Retry keeps the response it replaced. This is the third loop to need the same wiring.
The respawn retry also dropped the thread. It re-enables context_overflow on purpose,
to refit an already-compacted prompt for a replacement window, so it is the one path
that deliberately compacts a second time: without the thread those extra evictions were
archived nowhere, nothing was recalled in their place, and the fit held back no reserve
and re-applied no boundary.
* Studio: keep the reply that follows a recall, and archive respawn refits
group_turns keeps an assistant tool call, its result and the reply that follows in one
group, and the archiver rejected any group containing one of our own injections. So on
a turn that forced a recall, the model's actual answer was thrown away with it: the
question was archived from its own group and the answer was not, and a later search
could find what was asked and never what was said. The injections come out now and the
rest of the turn stays; retrieved passages are still kept out of the index they came
from.
The two respawn refits inside the tool loop also evicted without archiving. They run
against a smaller replacement window, so those turns are simply gone otherwise. They
archive only, deliberately: no reserve is held back on that path, so injecting a recall
there is what would push the retry back over the window, and the next request can still
recall what this one archived.
* Studio: bound the branch check to one turn, and anchor only what recall injected
The branch check searched one flattened transcript, so a line an edit removed could be
supplied by any later message that happened to repeat the words. Short answers repeat
constantly: an archived "Should I deploy? / No" survived its answer being edited to
"Yes" because a later turn said "No", and the stale pair stayed recallable. The
transcript is now one normalised string per message, and a turn matches only if its
lines appear in order within a run of adjacent messages no longer than the turn itself.
Recall anchoring took the last two messages of the conversation. That is right for the
tool style, which appends a synthetic pair, and wrong for the inline style, which
appends nothing and rewrites the latest user message in place: it also pinned the
assistant turn before it, and with it an eviction unit the fit was entitled to drop,
which can fail a later iteration that would otherwise have fit. The injection now
reports exactly what it added or rewrote, and only that is anchored.
* Studio: validate a turn's chunks against one run of the branch, not each on its own
The chunks of an archived turn are consecutive slices of a single rendering, and each was
checked for itself. That let a turn be reassembled out of parts that never sat together:
the head matching the question and the answer it has now, the tail matching some later
message that happens to repeat the passage an edit removed, so the stale association
stayed recallable. Reproduced before the change, with each chunk passing on its own.
All chunks of the document must now be found within one run of adjacent messages, bounded
by the document's own line count, which is at least the number of messages the turn was
rendered from, so a turn that really is still there always fits.
* Studio: budget conversation searches and disambiguate identical replies
search_conversation clamped the model's top_k only against a fixed ceiling of 8.
Eight chunks is roughly 4,000 tokens once wrapped, and the result lands in the
current tool exchange, which rolling truncation protects and cannot evict, so on
a small context the search itself made the turn unsendable. The GGUF loop now
passes the room the window actually has left, the tool clamps top_k by it, and a
search with no room says so instead of returning a result that cannot be sent.
The sticky compaction boundary took the newest on-branch assistant row, but the
branch check is textual, so two siblings whose replies read the same ("Done.")
are indistinguishable from there, and Retry is exactly what produces them.
Taking the first match applied a boundary measured on a different, deeper branch
and evicted live history. Where the text cannot separate them, the smallest
boundary is now used: too small costs one more compaction, too large loses turns
the branch still has.
* Studio: price a conversation search against the whole prompt
Three gaps in the budget the previous commit introduced.
The GGUF loop subtracted an estimate of the messages alone, leaving the tool
catalogue out of the prompt entirely. A large catalogue is thousands of tokens,
so the request could already be near its budget while the search was still told
there was room for several 500-token chunks. The preflight already prices the
request exactly, catalogue and template included, so the difference between that
count and the estimate of the same messages is carried forward and the estimate
only covers what the loop appends after it.
The safetensors loop advertises and executes search_conversation for a thread
compacted under a GGUF model, but forwarded no budget, so the clamp in the tool
was skipped there. It now passes the room this model has left. With no known
context length the argument is omitted rather than sent as zero, which would
refuse every search.
The sticky boundary filtered candidates with a substring test, which is right
for archived chunks and wrong for whole messages: an abandoned "Done" rode in on
a live "Not done yet", and having no live twin it then decided the boundary
alone. Where any candidate matches a live message exactly, only the exact ones
are considered; where none does, the old behaviour stands, since a stored row is
not always byte-identical to what the client re-sends.
* Studio: keep the overflow diagnosis on the tool path
The irreducible-overflow branch counted the newest message on its own, and a
tool loop reaches that branch with a tool result last. A tool result by itself
is not a conversation: templates that require it to follow its assistant tool
call refuse to render one, and the exception escaped the fit entirely, so the
caller fell back to the untrimmed request and the client was told nothing at
all. That is the one path this diagnosis was added for.
The count is now attempted and falls back to the estimator when the template
refuses. An approximate number is worth more here than a diagnosis that never
arrives.
* Studio: budget the conversation search in the provider loop
The third tool loop forwarded the active branch but no budget, so the clamp in
the tool was skipped there and a model-chosen top_k of 8 could append roughly 4K
tokens to a prompt this loop replays on its next call.
Studio knows no window for an external model: the request carries no context
length and there is no registry to look one up in, and a custom
OpenAI-compatible endpoint can be a small local server. So rather than a
measured budget, this path spends no more than one ordinary recall's worth,
which is the same amount the compaction turn itself is sized for.
* Studio: bound an archived turn to its own messages, and archive it once
Two problems in the archive.
The whole-document branch check bounded the run by the number of LINES the turn
produced, and gave each chunk its own start. A turn is two or three messages
however long it is, so a hundred-line answer got a hundred-message window, and
the tail of an edited answer could be satisfied by a message well outside the
turn. The run is now bounded by the messages the turn was rendered from, counted
from the labels render_turn writes, and the chunks are scanned as one pass: each
continues where the previous one stopped rather than restarting. The cursor
inside a message is deliberately not carried over, because chunks overlap and a
continuation chunk repeats the tail of the one before it.
The hash check that skips an already-archived turn ran long before the insert,
with the embedding pass in between, and the index on (scope, sha256) is not
unique. Two generations compacting the same thread both cleared it and both
wrote, so the turn was stored twice and its copies took two of the few recall
slots. The check is now repeated under a write lock immediately before the
insert. Reproduced with two concurrent archive passes: two documents before, one
after.
* Studio: record how many messages an archived turn came from
The run an archived turn is allowed to occupy on the branch was bounded by
counting the role labels in its rendered text. That counts lines the user wrote
as well as the ones the renderer did: a pasted chat log carries lines that look
exactly the same, and each one widens the run by a message, which is enough for
the message after an edited turn to supply the passage the edit removed.
The group's size is now recorded on the document when it is archived, in a
nullable column added the way the other lazy upgrades are. Archives written
before this have NULL and fall back to the label count, so nothing needs
backfilling to keep working.
* Studio: persist a compaction boundary the next request can use
The boundary was read back from dropped_messages, which counts what each fit
removed from the conversation in front of it. The tool loop refits on every
iteration and the client sums those counts, so a long agent run added the tool
exchanges the turn itself created, and the next request applied the total to its
saved transcript. Reproduced with six tool calls on a 4K window: three fits of 4
summed to 12, on a branch that only ever had 4 evictable messages.
The boundary is now carried separately, measured against the messages the
request was sent with, so it is absolute and re-sending it cannot advance it.
The client keeps the latest value rather than summing, and turns saved before
this fall back to the dropped count, which is the same number for a turn that
fit once.
The recall reserve is also dropped once archiving has failed. sqlite-vec can be
present and the thread saved while the embedder cannot start: archive_turns
swallows that and recall injects nothing, so the room the fit held back was pure
loss on every compaction, and the failure mode forgot more history than having
the feature off. A failed write marks the archive degraded and the next
successful one clears it.
* Studio: count the boundary past the system prompt, and notice it
Two faults in the boundary added in the previous commit.
It stopped at the first message still present, and a Studio request always
starts with a system prompt that a fit never evicts, so every compaction
recorded a boundary of zero. That is the same as having no boundary: the next
request would move the eviction point again and invalidate the prefix cache on
every turn. Instruction messages are now skipped rather than treated as the
front of the branch, and the newest turn is excluded because it is never evicted
and an inline recall rewrites it in place.
The compaction notice still keyed off dropped_messages, which is the
accumulated count. A tool-heavy turn reporting 12 while the boundary moved to 4
set a high-water mark that silenced the next two real advances. It now reads the
boundary, falling back to the dropped count only for turns saved before the
boundary was recorded.
* Studio: tighten comments in the conversation archive and rolling context window
* Studio: give the pasted-text import an extension on this branch too
The node test runner cannot resolve an extensionless relative import, so
delete-chat-files-preference fails to load the preferences store. main fixed
this in
|
||
alkinun
|
645ca5c7bd
|
Studio: add rolling context windows for local GGUF chat (#8961)
* Studio: add rolling context windows for local GGUF chat * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: return rolling context metadata for non-stream chats * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: keep original context when rolling fit fails * Studio: keep instruction groups independently protected * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: preserve rolling context metadata across retries * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: count sanitized rolling context prompts * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: refit rolling context after respawn * Studio: scope middle truncation to passthrough * Studio: refit tool prompts after respawn * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: retain later choice truncation metadata * Studio: report clipping-only context truncation * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: re-pin the permission read the deep-research contract lost to #8686 The contract still asserted the literal `const permissionMode = loadPermissionMode();`, which #8686 removed when it put a chat-scoped override in front of that read. This branch already carries the refactored store, so Repo tests (CPU) cannot pass here or on anything stacked above it. Ported main's replacement rather than dropping the check. The read is still the contract, and it is scoped to the deep-research setter so it cannot be satisfied by the initial-state constant, which is a different property and would keep passing if the read were deleted. * Studio: count a video part as media in the rolling preflight `_inject_video_part` writes llama.cpp's own `input_video` part into the same message list the fit is then handed, and the media predicate did not list it. Video prompts therefore ran the rolling preflight, which is skipped for media precisely because `/apply-template` token counting does not include the sampled video tokens. The prompt could be certified as fitting when it does not, or lose history it never needed to lose, and still fail with context_length_exceeded. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
Vineeth Sai Varikuntla
|
a6f267a4f5
|
Keep a sampler's std and uuid format across a save and reopen (#9225) | ||
Etherl
|
e99dfe5a2b
|
Studio: use a custom llama.cpp build (#9292)
* feat(studio): add custom llama.cpp path setting * fix(studio): harden custom llama.cpp selection * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix custom llama.cpp path resolution * Shorten llama cpp path comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Distinguish non-executable llama server paths * Reload after llama server path changes * Refresh llama consumers after path changes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
Michael Han
|
3cb8ecce78
|
Studio: add an edit_file tool so agents stop rewriting whole files (#8753)
* Studio: add an edit_file tool so agents stop rewriting whole files The tool loop had no way to change a file. ALL_TOOLS was web_search, python, terminal, render_html and search_knowledge_base, so the only way to edit anything was a whole-file `cat > f <<'EOF'` through terminal or an open(...).write(...) through python. Both re-send the entire file to change one line, and both lose whatever the model failed to reproduce verbatim. Measured on a 520-line source file: a one-line change costs 7,750 output tokens to rewrite versus 45 to patch. Over five edits with re-reads that is 79,390 tokens against 8,676, which is why tasks that should fit in 64-94K die past 100K. edit_file replaces an exact string. Not a unified diff: models corrupt @@ hunk headers far more often than they mis-copy a literal snippet, and a bad hunk header patches the wrong place instead of failing. A missing or non-unique old_string is a hard error naming the match count and writes nothing, so the retry is "add context" rather than "recover a mangled file". - Preserves CRLF line endings, UTF-8 BOM and file mode. old_string is matched against normalized text, so a snippet with plain newlines still matches a Windows-authored file instead of failing invisibly. - Atomic write via temp file and rename, so an interrupted write cannot leave a source file half-replaced. - Contained to the session workdir, checked on the realpath so a planted symlink cannot reach out. /mnt/data-style habit paths remap exactly as the python shim does. - Under Full access absolute paths resolve, and the schema says so. Otherwise the model assumes it cannot reach a real checkout and falls back to the rewrite precisely where files are largest. - Still prompts in auto mode: python's open(..., "w") already does, so the cheaper tool must not become the quiet way around that. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden edit_file against the cases raised in review Six real defects, each reproduced before the fix and covered by a test. - Receipt was bounded by diff LINES, which bounds nothing when one line is the whole file. A 200KB minified source returned a 400KB receipt, twice what the tool exists to avoid. Characters are now capped per line and over the receipt: the same edit returns 481 chars. - bool("false") is True, and models emit the JSON string, so replace_all as a string turned the multi-match guard off and rewrote every occurrence. The two spellings models actually produce are mapped, anything else is refused. - A FIFO or character device reported st_size 0 and then read forever. This path carries no timeout or cancel event, so the turn could not be recovered. Only regular files are accepted now. - An absolute path inside a workdir that itself sits under a habit prefix (/workspace/repo) had its own prefix stripped and rejoined onto itself, resolving to /workspace/repo/repo/a.py. Paths already inside the workdir skip the remap; habit paths outside it still remap as before. - Two chats sharing a project workspace could both read, both write, and the later rename discarded the earlier edit silently. The bytes the edit was computed from are compared again before the rename. - Containment was checked once at resolve time, leaving the whole read and diff as a window in which a parent could be swapped for a symlink. It is rechecked immediately before the rename. Left as is: an empty old_string still writes a zero-byte file. Refusing every existing target would strand the model, since no other old_string can match an empty file, so nothing could ever write to it. Nothing is lost with no contents, and the mode is carried over by the write. * Studio: give edit_file the terminal glyph, not the globe status_for_tool reports "Editing: name" for edit_file, and toolStatusKind only treats a "Running" prefix as local, so a file edit on this machine showed the globe, the same badge a web search gets. It is as local as python and terminal, so it takes the same glyph. * Studio: bound the edit_file receipt and make creation atomic Second review pass. Four findings, each reproduced before the fix. - The receipt was capped on output but not on what produced it: difflib was fed the whole file and its generator drained into a list. replace_all on a file at the 16MB cap allocated ~500MB and took 1.3s to return 200 characters. difflib now sees only a window around the first change and the generator is consumed lazily. Measured on the same 16MB file: 501MB -> 48MB, 1.3s -> 0.05s; a 600KB file goes 48MB -> 1MB. Hunk headers are shifted back to real file lines, since a receipt pointing at line 3 of a 9000-line file is worse than none. - Creation checked lexists() and then wrote, so two chats sharing a project workspace could both pass the check and the later write drop the earlier file. The absent case is now created with O_EXCL, and filling a zero-byte file goes through the guarded write rather than clobbering blindly. - New files came out 0600: mkstemp makes the temp file private and copymode had no source to copy from. O_EXCL creation takes the usual umask-derived mode, so a group that reads generated files still can. - enabled_tools in the public request schema still listed only web_search, python, terminal and render_html, leaving the new built-in undiscoverable to clients reading the OpenAPI schema, and bypass_permissions described only the python/terminal sandbox. Both now describe edit_file, including that Full access lifts its containment. Eight new tests, 44 in the file. * Stop the edit receipt inventing deletions, refuse non-regular targets and unpaired surrogates * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove the file a failed create just published O_EXCL publishes the name before the first byte and the payload goes out a buffer at a time, so ENOSPC or a quota partway through leaves the bytes that fit. Reproduced with a real kernel write failure: a 117780 byte create left 4096 bytes cut mid-token, and the retry the error message asks for is refused for ever, because an empty old_string refuses a non-empty target and no other old_string exists for a file the model never saw. close() can report a failure for data written earlier, so the error can arrive after most of the file is on disk. Unlinking the inode this call created puts the retry back on the create path, and keeps O_EXCL rather than mkstemp, whose 0600 would ignore the umask. * Let edit_file create an empty file Both strings empty is the documented creation call for __init__.py, py.typed and .gitkeep, but the identical-strings no-op check ran first and refused it, so there was no way to write a zero-byte file. Decide creation before that check, and stop reporting one line for a file with none. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim the review commentary --------- Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Michael Han
|
77bc10c663
|
Studio: price a partial GGUF by what is left to fetch (#8989)
* Studio: price a partial GGUF by what is left to fetch The size beside a partial variant was the variant total, so continuing a sharded download that was already 40 GB in still read "56 GB" and looked like the whole model coming down again. That is the reading behind the reports of the hub re-downloading models people already have. The variants endpoint now reports download_remaining_bytes for a partial: the plan total minus the bytes on disk a transfer can actually reuse. Reuse is per file, so a finished shard counts and an unresumable partial does not, and the card shows "16 GB left" instead of "56 GB". A one-file quant therefore still reads back its full size, because that is what continuing it costs: huggingface_hub 1.18+ refetches an interrupted file from zero, and a single-file quant has no other file to keep. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: price local and offline partial rows too The local and offline listings return before the hub-plan path, so their partial rows carried no remaining figure and fell back to the full total. The on-device card asks for exactly those (preferLocalCache), which is where a partial is most likely to be looked at. The worker writes its manifest before fetching anything, so those rows can be priced from the file list that produced them. Capped at the row's own total, since a manifest counts companions the row's size may not. The on-device card also merges local rows over remote ones, so it now carries the remaining figure through instead of dropping it. * Studio: stop capping a local remainder by the shards on disk A local listing sizes a variant by summing the shards it can see, so an early interruption makes that total smaller than the transfer: three 2 GB shards with one cached advertises 2 GB, and capping the remainder by it reported "2 GB left" when 4 GB had to be fetched. Under-reporting is the one direction this figure must never be wrong in. The cap was there to stop a companion-inclusive remainder reading larger than the row's own size, but no surface shows the two together: a partial row's size chip IS the remainder. So the cap is dropped and the manifest total stands. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price a partial by the bytes it holds, and credit a shared blob once across case-variant repo dirs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Follow the transfer in the remaining label while a download runs The live overlay carried only the expected size, so a row that says N left kept whatever the one-time variant fetch had measured, or the full total for a download started after it. A download 90 percent through read as though nothing had moved. The running job already carries its own progress, so derive the remainder from it and keep the fetched figure for every other row. * Take both terms of the live remainder from the job snapshot_progress nets completed_baseline_bytes out of expected_bytes and downloaded_bytes alike, so the job's two counters are consistent with each other and not with the catalog totals. Subtracting the job's transfer from the larger of the two scopes added that baseline straight back: 1 GB reused and 1 GB fetched of a 5 GB plan read 4 GB left rather than 3 GB. The catalog total still drives the size the row reports. * Price a pinned row against the cache root it names A row can name a snapshot in a previous, legacy or default HF cache root, and the request already scopes partial detection and the manifest read to it, but the blob scan always read the active root. Wrong in both directions: shards already in the pinned root earned no credit, and a copy of the same blob in the active root earned credit a resume into the pinned root cannot use, which reported less left than there is. The docstring called counting the active root only the safe direction to be wrong in, and it is for the first half, but not for the second. The fixture stub also had to start honouring an explicit root; ignoring it would have answered the active root for a pinned row and hidden the behaviour the new tests are about. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep a cancelled download on the measured remainder A job's progress is not bytes a resume can reuse: from huggingface_hub 1.18 the partial is process-unique, opened wb and unlinked in a finally, which is how existing_blob_bytes already prices it. Subtracting a dead job's transfer put 1.0 GB left on a cancelled 18 GB download with all 18 GB still to fetch, so the overlay now only prices running jobs. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price a fallback retry off the live transfer counter An XET run that falls back to HTTP re-claims in the same generation with a completed_baseline_bytes recomputed from disk, so the baseline now covers every blob the XET attempt finalized and the new run reports completed_bytes 0 against a shrunken total. resolveProgressUpdate holds the previous reading through that zero, so the card kept the dead run's finalized bytes while its total moved to the retry's scope, and taking the max of the two counters subtracted 3 GB from a 0.5 GB remainder: the row read 0 B left with the transfer barely started. snapshot_progress builds downloaded_bytes as completed plus in-flight and nets the same baseline out of both, so a single reading never has completed above downloaded and the max could only ever fire on a held figure. * Hold a retry's remainder off a held transfer reading resolveProgressUpdate holds the previous downloadedBytes through a poll that reported zero, and the GGUF row forwarded that held figure as the current transfer. An XET run that falls back to HTTP re-claims in the same generation with completed_baseline_bytes recomputed from the finalized blobs on disk, and snapshot_progress nets that baseline out of downloaded_bytes, completed_bytes and the total alike, so the retry's first reading is a legitimate 0 against a shrunken total. Subtracting the held 3 GB from the 0.5 GB that remained read "0 B left" until the retry moved its first byte. Carry the distinction out of resolveProgressUpdate as measuredTransfer and skip the progress-derived remainder on a held reading, falling back to the backend's own download_remaining_bytes the way the terminal row already does. * Carry the held-transfer marker across a reload measuredTransfer marks a reading the poll did not measure, so the remainder is not derived from a stale downloadedBytes. It was not persisted, so a reload restored the stale bytes beside the new run's shrunken expectedBytes with the flag reading undefined, which the selector treats as measured. That is the '0 B left' the guard exists to stop, and on app start it lasts until the first poll returns rather than a single tick. Persist and restore the flag. Absent stays absent, so a record written before this field still means never polled rather than held. * Carry the held-transfer marker through adoption too A restored fallback job is adopted by the next running-status probe, which seeds the persisted byte counters but rebuilt the job without the marker. That restored it as undefined, which reads as measured, so the remainder was derived from the dead run's bytes again and the row read 0 B left. The marker is a statement about those counters, so it travels with them: seededMeasuredTransfer joins carriesOverSeed in adopt-rules, gated on the same condition. Not carrying the seed zeroes the counters, so there is no held figure left and undefined stays the honest answer. * Do not credit a partial that has not been written yet blob_bytes_present read a missing st_blocks and a present zero as the same thing, so a partial set to its final length before its first chunk landed fell through to st_size and was credited whole -- the row read 0 B left on a download that had transferred nothing. Confirm the emptiness with SEEK_DATA rather than inferring it, so a mount that never populates st_blocks keeps the size fallback. * Read a pre-marker record's counters as held, not as measured A record written before measuredTransfer existed cannot say whether its byte counters were measured, so an absent marker there is not the never-polled it means in a current record. An upgrade landing mid-reclaim restored the dead run's held bytes with the guard reading measured, which is the 0 B left the marker exists to stop. Version the persisted state so the migration can tell the two absences apart, and read only the pre-marker one conservatively. * Trim the review commentary --------- Co-authored-by: shimmyshimmer <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
Lee Jackson
|
6fa27e20aa
|
Default preserve thinking on for Qwen3.8 (#9096)
--------- Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> |
||
|
Michael Han
|
e3260339d8
|
Revert "Say why a Xet download sits at 0% until it finishes (#9159)" (#9293)
This reverts commit
|
||
|
Michael Han
|
dc470257b6
|
studio: keep each chat's sampling params and system prompt with the chat (#9055)
* studio: keep each chat's sampling params and system prompt with the chat The composer pills, permission level and retrieval controls already travel with the thread. The sampling params and the system prompt did not, so returning to a chat started under one prompt showed whichever prompt the last chat had, and every chat shared one set of sliders. Those eight join the snapshot: temperature, top P, top K, min P, the two penalties, the system prompt and its variables. They are the only thread-scoped settings that live under params rather than as store fields of their own, so the read and apply paths route them through there and the whole snapshot is still one object on the thread row. An edit made with a chat open lands on that chat; with no chat open it moves the installation defaults, as before. A model's own recommendation is not an edit: both paths that apply one now say so, or loading a model while a chat was open would pin that chat to the model's sampling. Context and checkpoint stay out. They describe the model that loaded rather than the conversation, and a chat restoring a budget the current model cannot hold is the failure that exclusion exists to prevent. * studio: keep a chat's sampling through a model load and a held edit Three follow-ups from review on the per-chat sampling change. A model's recommendation still landed in the live params: fromModelDefaults only moved where the value was persisted. A chat with stored sampling ran the model's instead, and the next unrelated edit snapshotted that over what the chat had. setParams now puts back the keys the chat holds; one it never set still follows the model. The Qwen mode table is the model's recommendation, not a value the user picked, so both paths that apply it are marked. Loading the model and toggling Think reach the same table, so marking one and not the other pinned the chat by the other route. The held-edit flush read the sampling keys as store fields. They sit under params, so it read undefined, the sanitizer dropped them, and an edit made while the chat's snapshot was in flight was lost when the user navigated away. * studio: persist a model's sampling defaults even when the chat pins them Restoring the chat's values before the diff made every pinned key equal on both sides, so it dropped out of changedParams and never reached the installation defaults. One chat pinning temperature left every later new chat on whatever model loaded before it. The restore is a live-store concern only. Persistence diffs the model's own object again, as it did before the restore existed, so the recommendation reaches the defaults while the open chat keeps what it stored. getChangedInferenceParams bumps the mutation versions, so it stays a single call rather than one diff per purpose. * studio: keep a chat's sampling across a model switch, both directions setCheckpoint replays the destination model's remembered params itself, without going through setParams, so the restore never ran there. An external switch has no load after it to correct the result, so the chat adopted that model's prompt and sampling and kept them. The restore now runs on that path too, and persistence still reads the unrestored object so the model's own values reach the installation defaults. The other direction leaked the same way. rememberOutgoingModel snapshots the live params, which inside a chat are that chat's, and on a model with no entry it persists that snapshot whole. Filtering the incoming edit cannot undo a write that already happened, so the outgoing snapshot is now built without the values the open chat owns. What the model already remembered wins over the installation copy, so stepping off a model inside a chat does not flatten a preference the model was given outside one. persistReplayedParams writes the installation defaults on the setCheckpoint path, so it keeps the in-memory copy level the same way setParams does. * Keep a chat's sampling edits through hydration, model loads and remembered-params writes * Keep a held chat edit out of the outgoing model, and keep a default published mid-pairing withoutActiveThreadParams returned early while a chat's read was still out, so switching model in that window snapshotted the open chat's temperature and system prompt into the outgoing model's shared memory. The sibling restore path already read the held value first; this one consulted only the override. Driving the real store put a chat's own sentinel straight into paramsByModel, which the simulation harness already treats as a leak. Second, a model default published inside the pairing window was sent to the settings route but then rolled back in memory from the pre-window sample, so the session and the server disagreed and the stale value was pinned onto the next chat with no snapshot. noteThreadScopedDefaults also returned early when nothing had been applied yet, which is the common case, so the value had nowhere to land. * Put the installation defaults back when a chat is left mid-read Leaving a chat while its settings read is still out writes the edits made in it to that chat's row, but the store kept showing them, so the next chat took them: a snapshot-less chat captured the store as the installation defaults and was pinned with the previous chat's temperature and system prompt. Restore the pre-window values over the committed fields, and only when the chat is actually being left, so a retry after a failed read and a fork both leave the edit on screen. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the per-chat inference comments Comments only. Collapses the multi-line explanations in the thread-scoped sampling path to their shortest clear form and drops the restatements, keeping the reasons behind the pairing window, the held-edit capture and the outgoing-model filter. --------- Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Michael Han
|
334db4875f
|
Studio: add a keyboard shortcuts page and make the existing chords rebindable (#8948)
* Studio: add a keyboard shortcuts page and make the existing chords rebindable Settings gains a Shortcuts tab listing every shortcut, with search, a recorder to change a chord, and controls to unassign or restore a default. The four chords that already existed were hardcoded across four files. They now resolve through one registry, so an edit applies without a reload. * Keep the shortcuts tab across reloads, and decide contested chords by registry order loadInitialTab checked a hand-written list that the new tab was missing from, so picking it and reloading fell back to General. The list is now the source of the SettingsTab union, so the two cannot drift. A chord claimed by two actions is consumed by whichever window listener runs first, which followed mount order and so varied by route. Registry order owns it instead, and the tab names the row that loses. * Clear rebound shortcuts on preference reset, and translate the shortcuts tab in every locale * Lock the auth-route settings guard to its behaviour rather than one spelling * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Record nothing for a Super chord off macOS There is nowhere to put Meta on Windows or Linux: matchesBinding rejects an event carrying it, and the recorder wrote mod from ctrlKey alone, so Super+Alt+K persisted and displayed as Alt+K. That assigns the action to a chord the user did not choose, fires it on Alt+K pressed by itself, and leaves the chord they actually pressed unable to match anything. Returning null is the answer the recorder already gives while only modifiers are held, so it simply keeps listening. * Studio: render the sidebar shortcut hints from the live binding The search tooltip and the Settings menu row hard-coded the shipped chords, so after a rebind or a clear in the shortcuts tab they kept telling the user to press a chord that no longer runs anything. Both now read the resolved binding through useShortcutLabel and drop the hint when the action is unassigned. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Michael Han
|
0eb6b6c931
|
Studio: say when a scan folder cannot be read instead of showing no models (#9053)
* Studio: say when a scan folder cannot be read A folder Unsloth is denied looks exactly like an empty one: the scan catches the OSError, logs it, and moves on, so the model list is empty with no reason given. Add-time validation now opens the directory instead of trusting os.access, which reads mode bits only and passes on folders macOS TCC or a Windows ACL still refuses. The check runs after the denylist rules so a denied path is never opened. The scan keeps the error it already caught, and both scan-folders endpoints return it as a per-folder status the dialog shows with the setting that fixes it. No new work on the healthy path: 0.06us per folder per scan, and the folder list is a dict lookup with no syscalls. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: record scan folder status from the Hub inventory scan too The folders dialog reads /api/hub/scan-folders, but the scan behind it is the Hub inventory, not collect_local_models, so nothing was ever recorded for it and every row stayed "ok". Its custom-folder loop now records the same way. That alone is not enough: the Hub's _scan_models_dir catches the OSError itself and returns an empty list, so no exception reaches the loop. So an empty result is now the trigger. One opendir says whether the folder is empty, gone, or refused, and it runs only for a folder that returned no models. A folder that found models still costs nothing, with a test that fails if it ever touches the filesystem. * Studio: catch denied model subdirs, and recheck when the dialog reopens Two gaps in the folder status. A root can list fine while every model under it is denied, on a NAS mount or a drive owned by another user. The scanners skip an unreadable child silently, so that arrives as the same empty list as an empty folder and was reported as ok. The probe now also opens subdirectories, stopping at the first refusal and capped at 64, so the denied-everything case costs one extra open. The row tells the user to fix permissions and reopen the dialog, but nothing rechecked between inventory scans, so the warning stayed up after access was restored. Listing the folders now rechecks the folders marked bad, and only those. A healthy folder is not in the registry, so the list still opens nothing. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: probe two levels down, and keep the tests collectable on Windows The child probe descended one level, so <root>/<publisher>/<model> with the model denied still reported ok: both levels above it list fine and the scanners return nothing. It now walks two levels, depth first, on one shared budget of 64 opens. Depth first means a denied mount is found in three opens instead of after every publisher, and the budget bounds the cost whatever the shape of the tree. os.geteuid does not exist on Windows and a skipif condition is evaluated at import, so collecting the test file there raised AttributeError before the os.name check could skip anything. Resolved once into a shared marker, with a test that runs the module body with geteuid removed. * Studio: flag a folder with one denied model, and show status in the picker A folder holding one readable model and one denied model returned the readable one, so the scan looked successful and the denied model was silently absent. The probe now runs whether or not models were found, and reports "partial" in that case so the copy does not contradict the rows on screen by claiming the folder cannot be read. That is a real cost change on the healthy path, so it is measured rather than claimed: 104us for a folder with 8 model dirs, 0.77ms for one with 300, capped by the same 64-open budget. The end-to-end scan stays inside run-to-run variation, and the folder list still opens nothing unless a folder is already marked bad. The old zero-syscall test is replaced by the bound, which is now the guarantee that matters. The inline model selector manages the same folders and rendered only the path, so it shows the status too. * Studio: stop treating an exhausted probe budget as healthy Three fixes. A denied directory past the open budget was reported as ok. Running out of budget means the tail was never looked at, which is not the same as finding it healthy, so it now returns an internal "unknown" that is never recorded and never sent to the UI. That alone would strand a wide folder in a warning it could never clear, so the registry now remembers which directory refused. A recheck opens that one directory, which settles a fixed folder in a single open no matter how wide the folder is or where the denial sat. A folder recorded as partial kept that status after being deleted, because the recheck preserved partial for every non-ok probe. It now only holds partial against a permission result, so missing and unreadable replace it. One permission test was missing the marker that skips it as root and on Windows, where chmod 000 does not deny. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Probe scan folders off the event loop, and stop a vanished model or a Windows device error condemning the folder * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Probe the commit directory inside an HF snapshots folder, and stop calling a shut root partial Two ways the dialog told the user the wrong thing. The cache layout is models--org--name/snapshots/<commit>/, three levels under a registered root, so a probe that stops at two never opens the one directory the weights live in: a denied commit dir made the model vanish from the list while the folder still reported ok, which is the silent empty case this PR exists to remove. The extra level is bought only for a directory named snapshots, since a blanket third level would spend the open budget descending into diffusers component directories. And when the root itself becomes denied, the partial branch restored partial regardless, so a folder none of which can be read kept saying some models in it could not be read, sending the user hunting for one bad model. The cause the probe already returns is the discriminator: it is the root path itself for the root's own refusal and a nested entry path otherwise. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Michael Han
|
588405dce2
|
Desktop: make every drop zone take a drop again (#9036) (#9056)
* Desktop: make every drop zone take a drop again (#9036) Tauri delivers OS file drops window-wide and suppresses the webview's own drop events, so a zone wired only to `onDrop` does nothing in the desktop app: no drag-over border, and the file is silently ignored. Native drop routing (#8265) was only ever adopted by the shared image picker and the create-project dialog. Every other file drop zone still relied on HTML5 handlers that never fire, which is why this reads as intermittent: the same file works when it lands on the chat, covered by the window-wide handler, and does nothing anywhere else. Adds `useNativeFileDrop`, which claims the native drop for an element and returns drag-over state plus the HTML5 handlers the web build still needs, then adopts it in the zones that were dead: - Projects -> Sources, which also had no drag-over styling at all - Data Recipes unstructured seed - Diffusion training images - Video and audio reference pickers Documents upload by lease rather than an inline read, since the native reader only serves media inline, so the recipe seed route now accepts `nativePathLease` the way the RAG upload routes already do. The native path policy accepts video containers so the reference picker can register what it is given. Also stops two silent discards with the same symptom: a claimed zone that refused a drop while disabled, and compare mode disabling the window-wide handler outright. Both now say what happened. `native-dropzone-coverage.test.ts` walks src/ and fails if a zone reads files from a drag payload without either claiming the native drop or explicitly deferring to the window handler. * Desktop: refuse a dropped model in compare too, and validate before reading Two things the first pass got wrong. Keeping the window-wide listener on outside single chat also handed it model drops, so a GGUF dropped on a compare or project view would load and replace the active model. Nothing happened there before, so that is not a change this should be making. The refusal now covers every kind the handler would act on, models included. The recipe seed route also moved its extension check after the read, so a rejected 500 MB upload was pulled into memory first. Back to validating the filename before reading a byte, as it was. * Desktop: size the native video cap to the largest client-side limit 64 MB sat under the reference picker's own 72 MB, so a clip the picker accepts was refused on drop. The cap is a backstop; callers keep theirs. * Desktop: drop the diffusion zone from this pass, and bound the native read Two review findings, both correct. The diffusion dataset zone accepts .bmp, .m4v, .caption and .jsonl, which the chat attachment policy rejects outright, and .txt, which registers but cannot be read inline. Captions beside images are the documented workflow there, so wiring that zone to the attachment path would have uploaded the images and silently lost the captions. It needs its own registration and upload policy, which is more than this belongs to, so it goes back to the picker it had. The recipe seed route also read a dropped path in full before checking any limit, so a multi-gigabyte local file went into backend memory before the 413. It now refuses on the stat and bounds the read by what the block has left, in case the file grows in between. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Desktop: keep a busy drop zone hit-testable, and size the video cap to the raw limit A disabled seed zone carried pointer-events-none, which takes it out of elementFromPoint, so nativeDropTargetAt could not find the target it had just registered. The disabled message was unreachable and the drop fell through to the window handler instead. MAX_NATIVE_VIDEO_BYTES was set to the reference picker's 96 MiB, but that cap bounds the data URL, not the file: the picker's own raw limit is 75497280 bytes. Rust was reading and base64-encoding up to 96 MiB, 128 MiB across the bridge, for clips the picker then rejected. * Match the document-refusal test to the message it now returns * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: attach video to a chat, and say why when it is unavailable (#9057) * Studio: attach video to a chat, and say why when it is unavailable llama.cpp takes video through its OpenAI-compatible chat endpoint as an `input_video` content part, but only when the mmproj declares video, the binary was built with video support and ffmpeg is installed. It reports that verdict at /props under modalities.video. Nothing about the GGUF alone can tell us, so that is what Studio now reads. Frontend: video joins the drop classifier and its own pending queue beside images and audio, and a VideoAttachmentAdapter takes one clip per message from the picker or a drop. When the model cannot take video the adapter names all three possible causes instead of letting llama-server refuse the request later. Backend: video_base64 on the chat request is forwarded whole as an input_video part, since llama-server owns the frame sampling and there is nothing useful to transcode. has_video_input rides the same path as has_audio_input out to the model row. Compare mode is left out on purpose: video_base64 targets the single loaded GGUF, so at most one side could answer. Dropping a clip there now says so rather than ignoring the file. Size caps line up across the three hops (64 MB in the desktop reader, in the composer and in the route) so no hop accepts what the next refuses. * Studio: carry the video capability through, and cover the passthrough path Three review findings, all correct. syncModelCapabilities took has_video_input but never copied it into the row, and /api/models/list omits it for the active GGUF, so the adapter read false after every load and refused video even when /props reported it. The feature did not work in its main path. The tool and response_format passthrough returns before the injection and forwards an explicit field list, so a clip rode along nowhere and the model answered without it. Refused now, the way audio already is there. The size cap floored the base64 inflation, so a clip of exactly the size the composer allows was refused with a 413, and the data URI header was counted against the payload. Padded ceiling, measured after stripping. * Studio: carry the video capability through every hop, and refuse it where it cannot be served Three separate places map backend capability flags onto a model row and each one dropped the video flag: the direct status adoption, the queued-run capability Pick, and (fixed earlier) syncModelCapabilities. The adapter reads that row, so any of them leaves video refused on a model that supports it. Covered by a rule rather than three spot checks. Injection lives in the GGUF branch, so an external provider or a local transformers model answered as if no clip were attached. Both now refuse, as does token counting, which cannot inject the frames it would need to count. The size check also moved ahead of the automatic model switch so an oversized clip does not evict a working model before the 413, and video now votes in the pre-switch projector requirement alongside audio. The video drain's read-failure toast said 'audio', inherited from the audio drain it was cloned from. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: name the attached modality in the pre-switch refusal Adding video to require_vision made the shared rejection reachable for a request carrying only a clip, but its text is fixed at 'image or audio input', so the user who attached a video was told about modalities the request never carried. The label now follows what is attached, and defaults to the existing wording so the image-only callers are unchanged. * Match the document-refusal test to the message it now returns * Send the API key on the /props readback and skip video in the context recount The /props probe went out without an Authorization header, so under UNSLOTH_DIRECT_STREAM=1 llama-server answered 401 and video capability never came back. Context recount already bails on images and audio because toOpenAIMessages has no branch for them; video has the same property and was missing the bail, so the usage bar priced a text-only prompt and stringified megabytes of base64 on the UI thread. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Send a picked clip under the container its extension names The accept list carries extensions as well as mime types because the browser's answer is unreliable for mkv and some mov files, and the picker takes those files on the extension. Only an empty type was being replaced, so a clip the browser called application/octet-stream kept that type into the attachment, and the request builder recognises a file part only when its mimeType matches ^video/. The clip was attached, sent and dropped, and the model answered as though nothing were there, which is the silent drop this PR exists to remove. The table mirrors the one in native_intents.rs, so a clip read by the desktop reader and one picked in the browser reach the route the same way. --------- Co-authored-by: shimmyshimmer <182633334+shimmyshimmer@users.noreply.github.com> Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: shimmyshimmer <182633334+shimmyshimmer@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Michael Han
|
f3b0425345
|
studio: show the model a chat was started on, and offer it back (#9081)
* studio: show the model a chat was started on, and offer it back Every thread row has carried modelId since long before this, stamped at creation and read only for an image-compatibility warning. So the model a chat was started on is already recorded for chats that already exist; what was missing was showing it and offering it back. The chat list shows it on each single row, and a chat open on a different model gets a one-line notice with a Switch back button. Compare rows run two models and show none. Deliberately an offer rather than an automatic switch. For a local model, restoring on open would evict whatever is resident and spend a multi-gigabyte load on clicking a row in the sidebar. The button routes through handleCheckpointChange, the picker's own handler, so the confirmations, VRAM checks and external-provider handling are the same ones a manual switch gets. The notice stays quiet when it has nothing to offer: no stamp, already on that model, or a model since deleted and no longer selectable. The row label yields the right-hand slot to the spinner and the unread dot, and the title truncates first, so a long model name cannot squeeze it out. * Lift the chat model notice clear of the header instead of painting it underneath * Reserve the notice's height in the thread viewport so it stops covering the first message * Key the chat's model to the chat, and clear the research panel too Clearing the stored model inside the effect is a frame late: the effect is passive, so the first render for the incoming chat has already committed with the outgoing chat's model, painting the wrong notice and moving the viewport padding under it. Answering only for a matching thread id makes that render impossible rather than brief. The research panel needed the same offset the thread viewport got. The notice is an opaque bar spanning the whole chat content area, the panel column included, so it covered the first 2.25rem of the panel header: measured 36 px of overlap over the title, the status pill and the close button, with elementFromPoint returning the notice at all four. After the offset the overlap is 0 and the hit tests return the intended controls, while a chat with no notice keeps its exact previous geometry because the variable resolves to 0px. * Offer the switch in a chat that was started as New Chat A chat opened as ?new=<nonce> carries no thread in the URL and keeps none after the first send, so the notice never saw the row that send created and switching model in a chat you had just started offered nothing until you navigated away and reopened it. The store does learn the id, published once the visible thread is the one being persisted, but it still holds the previous chat's id for the first render, until ThreadNewChatSwitch blanks it in an effect. Handing that straight over would put the previous chat's notice on this one, so the id is taken only after it has been seen blanked for this nonce. Before the first send it stays an unpersisted local id and there is nothing to offer. * Offset the canvas panel below the chat model notice The notice is an absolute child of the chat content container, so it spans the canvas column too. The panel's top offset was a fixed 90px, which under custom desktop chrome left the notice covering the top 28px of the panel, including the preview/source tabs and the close control. Move both edges by the notice height, as the research panel already does, with 0px whenever no notice is on screen. * Switch back on the picker's metadata, not the id alone ChatModelNotice called handleCheckpointChange with only the model id. The picker never does: it supplies source, isLora, isDownloaded and isGguf for every row it offers. A Hub row or an external model does not need them (/api/models/list carries isGguf, isExternalModelId routes the rest), but a local or fine-tuned row is in neither, so selectModel resolved isGguf false and the /load request dropped n_parallel, n_batch, n_ubatch and llama_extra_args and sized the context down the transformers path. Resolve the same metadata the picker supplies for those rows, and carry the format for a single .gguf file out of the local inventory so it survives. A GGUF directory stays unclaimed on purpose: that row expands to pick a quant and a chat row records only modelId, so sending llama-server flags for a quant nobody chose would be worse than the plain switch. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Michael Han
|
2f80f11ccd
|
Studio: link the Read aloud TTS row to Audio and show the preview generate wait (#9093)
* Studio: link the Read aloud TTS row to Audio and show the preview generate wait The studio TTS row named the model selector without offering a way to reach it, and Preview showed Stop for the whole generate, which read as an idle button. * Route the Read aloud action to Speak, not the mode Audio was left in AudioPage stays mounted off-route and keeps its mode state, so a plain /audio navigation could land on the transcription selector. Send the task as a mode intent and handle a task without a model on the Audio side. * Tighten the comments on the Read aloud TTS row changes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: danielhanchen <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
oobabooga
|
531bc481a2
|
Studio: incrementally tokenize streaming code fences (#8935)
--------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Michael Han
|
7b58b56228
|
Studio: make the partial download affordance tell the truth (#8982)
* Studio: make the partial download affordance tell the truth The "Partial" badge's tooltip said "Click to continue", but the badge is a status dot with no handler. On the GGUF card it sits inside the quant trigger, so clicking it opens the quantization menu, and picking the partial quant there only selects it. On the safetensors and dataset cards the click does nothing at all. Every tooltip now names the button that acts. The button itself said "Redownload" for a Xet partial, which reads as a full refetch of a repo that may be tens of GB. It is not one: only incomplete blobs are purged, so snapshot_download skips every shard already on disk and Xet reuses CAS bytes through its chunk cache. The label is now "Continue", and the tooltip says completed files are kept. "Continue" was also promised for HTTP partials that nothing can continue. huggingface_hub 1.18 moved the writer to a process-unique .incomplete opened "wb", so an interrupted file is refetched from zero, and Studio pins >= 1.23 on Python 3.10+. The transport capabilities endpoint now reports whether partials are resumable at all, and only a partial that really is gets "Resume" or a pause glyph on the stop control. * Studio: lead the partial hint with the file that restarts The unit of "kept" is the file, and most quants are one file: a 27B Q4_K_M is a single 18 GB GGUF, so continuing its partial fetches all 18 GB again. Only a sharded repo keeps anything, which is why the 56 GB BF16 in #8927 looked like a resume. Leading with what survives promised something the single-file case cannot deliver, so the restart comes first. * Studio: decide Resume from the partial, not from the hub version The resume wording was derived from the row's transport plus whether the installed huggingface_hub resumes at all. Both can say yes about a partial that cannot be reopened: a cache shared with a 1.18+ environment holds nonce-named partials, and this repo's own pins produce that mix, since Python 3.10+ takes hub >= 1.23 and older takes 0.36.2 against one cache. The row promised Resume and the next attempt purged the bytes. Rows now carry partial_resumable, the conjunction of the transport they report and the registry's per-file check, which is what the transport-conflict dialog already used. The label and the tooltip read that; the installed-writer capability stays where it belongs, on the stop control for a running job, whose partial is this machine's own. Anything that cannot answer defaults to false, so no surface can promise a resume it has not verified. * Studio: scope the resume verdict to the variant that asked is_resumable_partial takes a variant but its blob scan is repo-wide, so a sibling quant's reopenable partial answered for a row whose own partial is nonce-named and about to be purged. Latent until now, since every caller passed a repo and no variant. It now intersects the resumable blobs with the ones the variant's manifest claims, which is what the variant transport-status endpoint already did. No manifest means no way to scope, and an unscoped yes is the promise this guards against, so it answers no. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: judge a companion partial by its own marker A GGUF manifest carries the shared mmproj and MTP drafter alongside the quant's own shards, but those blobs answer to .transport.companion, not to the variant marker. So a companion partial left by Xet, or one with no companion marker at all, satisfied the variant's HTTP marker and backed a Resume that prepare_cache_for_transport purges before fetching anything. The verdict now splits the manifest the way the worker splits it for the purge: main hashes are vouched for by the variant marker, companions only count while the companion marker agrees. A blob in neither set backs nothing, so an unrecognised layout understates rather than promises. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scope the resume verdict to each cache entry A repo can own several active cache directories at once: on a case-sensitive filesystem models--Org--Model sits beside models--org--model, and iter_active_repo_cache_dirs yields every one of them. is_resumable_partial unioned partial hashes across all of them, then asked read_active_transport_marker / read_active_companion_marker for the first non-null marker found in any. So a marker in one directory could vouch for a partial in another that prepare_cache_for_transport, which judges per entry, was about to purge. The row promised "Resume with HTTP to keep the progress you already have" for bytes that would not survive. The verdict is now decided per entry, the way the purge decides, so a marker only answers for partials sitting beside it. read_active_companion_marker had no other caller and is gone. * Answer the resume question from the row's own cache root partial_resume_available scoped the transport half to the row's cache root and then threw that root away for the resume half, so a row shown from a remembered or custom root was judged by the active root's blobs and the active root's manifest scope. Wrong in both directions, and both reproduced: a legacy .incomplete under the active root made a remembered-root row offer to keep progress the next run purges, and a genuinely resumable variant row read as unresumable because its manifest lives in its own root's digest scope, so the UI offered a redownload over good bytes. The root is keyword-only and defaults to None, so the two callers that mean the active root are unchanged. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: shimmyshimmer <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Michael Han
|
b8c931f9a4
|
Studio: size Xet download buffers from free RAM, not total (#9052)
* Studio: size Xet download buffers from free RAM, not total hf_xet's reconstruction buffers are sized from the machine's TOTAL RAM, so a download started while a model is loaded asks for the same multi-GB budget it would ask for on an idle box. Those buffers are the worker's RSS, not reclaimable page cache, so the request and the resident weights add up and the machine swaps. Clamp the zoo-sized budget to a quarter of free RAM by re-asking the zoo about a smaller machine, so buffer, per-file and file count scale together. A quarter of AVAILABLE always exceeds the zoo's eighth of TOTAL on an idle host, so the clamp is unreachable unless RAM is genuinely held: sizing is byte-identical when there is headroom. Also apply the zoo's existing MIN_XET_RAM_BYTES floor to free RAM when picking a transport, so a host too tight for even the clamped floor downloads over HTTP. Fixes #9032 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the free-RAM clamp reduce-only The recompute calls xet_env_overrides directly, without the throttled flag apply_xet_env threads through after a 429, so an un-throttled recompute could hand back a stream ceiling that backoff had lowered. Take the smaller of the two per key. Every derived value is monotonic in total RAM, so the result stays coherent. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: apply the free-RAM gate where Auto is actually resolved The Studio UI never sends transport_mode=auto. effectiveTransportMode() resolves Auto through get_download_transport_capabilities(probe=true) and submits the answer as an explicit xet/http, which resolve_requested_use_xet honours without calling resolve_auto_use_xet. The gate therefore never ran on the primary flow. Move the verdict into a shared free_ram_pressure_reason() and call it from the probe as well, so the UI path and an API caller that sends auto agree. Probe only, so an ordinary browse poll stays read-only and still does not load Zoo. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: let free RAM decide when health has no verdict Both early returns in resolve_auto_use_xet skipped the gate: a zoo without hf_xet_health, and a health probe that raises. Free RAM is read from hf_xet_tuning, a different module, so neither says anything about whether the machine can afford Xet right now. Fold both into one optimistic path that still consults free RAM, and move the registry probe's RAM read outside the health try for the same reason. A health verdict that already demoted keeps its own reason. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: reserve free RAM across concurrently starting Xet workers A worker allocates inside the child, after Popen returns, so free RAM does not move until well after sizing. Four downloads starting together each read the same untouched value and each took a quarter of it, promising the whole machine. Sizing now subtracts what live siblings were already promised, and the reservation is bound to the worker's pid so it frees when that worker exits. The transport gate subtracts it too: the clamp bottoms out at Xet's floor, so enough simultaneous workers would still add up past free RAM, and the next download is better served by HTTP than by Xet at its minimum. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make RAM admission and reservation one critical section The ledger read and the reserve sat in separate critical sections, so concurrent sizings could all read the same total before any of them wrote, which is the overcommit the ledger exists to stop. Hold the lock across the whole decide-and-reserve region; the recompute inside is pure arithmetic on a frozen profile, and the RAM/disk reading stays outside it. The earlier reservation tests started workers sequentially, which never entered that window. The new test races four threads through it. * Probe worker liveness without signalling on Windows, and let UNSLOTH_FORCE_XET override the free-RAM gate * Ask the backend fresh on every Auto download start The probe answer was cached for 30 seconds, so several Auto downloads begun inside that window were all admitted on the same pre-reservation verdict and each submitted an explicit transport_mode=xet, which the start path honours without re-reading free RAM. Measured on a 32 GB box with 5 GB free: four starts promised 4.25 GB of Xet buffer pools, one per worker process, where a live gate admits the first and sends the rest over HTTP. Render polls still answer from the cache; only the probe itself has to be live, and it costs no network probe because the health verdict is memoized upstream. * Credit a live worker's resident memory against its reservation The ledger charged a bound worker's whole buffer promise for its whole lifetime, but the free-RAM reading it is subtracted from has already dropped by whatever that worker allocated, so the same bytes went out twice. Measured with a real child holding real touched RSS: 4.04 GB genuinely free, reported as 2.0 GB, and the next Auto download demoted to HTTP for room that existed. The reservation exists to bridge the window between sizing and allocation, which is what the comment above it says, so credit what has materialised and keep the full promise for a worker that has not bound yet. The xet buffer is drawn through a permit semaphore rather than preallocated, which is why resident memory is the right measure of how much of the promise is already spent. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: shimmyshimmer <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Michael Han
|
489caae23f
|
Studio: confirm an Audio model swap instead of dead-ending on the backend 409 (#9092)
* Studio: confirm an Audio model swap instead of dead-ending on the backend 409 Audio called /load and /unload directly, so the 409 the backend raises when chats are still generating arrived as an error toast with nothing to act on. Route both through the same confirmation Chat uses and force from the answer. * Hold Chat's lifecycle gate across the question, and defer a load confirmed after Audio hides Without the gate a queue can materialize while the dialog is open, outside the snapshot the answer was given for. pendingTtsLoad is also still null while the dialog is open, so the deactivation effect could not abort a load confirmed from a page that had gone away; queue it for the activation replay instead. * Stop Chat queues only once the Audio load request is going out loadModel prepares the stored Hugging Face token before it sends /load, and returns without sending when that token is invalid and the user picks replace or dismisses the warning. Cancelling the accepted sends and queued prompts before that call discarded them for a swap that never happened, leaving the previous model resident and the work gone. Move both cancellations into the existing onRequestStart callback, which runs only once the request is issued. --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
alkinun
|
41b93a9169
|
Allow unlimited Deep Research time budgets (#9078)
* Allow unlimited Deep Research time budgets * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bound unlimited research request setup * Validate unlimited research timeout settings * Handle unavailable research timeout storage * Preserve research stream idle timeout * Update Deep Research dialog contract * Bound the unlimited path and restore the timeout floor for PR #9078 An unlimited budget removed the wall clock that used to backstop a queued request, so a backend that sent one queue notice and went silent held the run open until the user cancelled. Bound the gap between notices with the same value the unlimited read timeout already uses; each notice refreshes it, so genuine queueing is still not charged. - Clamp the model-load and model-switch waits: with the 3600s ceiling gone, a large finite budget could make the poll loop wait for years. - Give modelTimeoutSeconds a readable one-year ceiling instead of float max, so the 400 no longer quotes a 309-digit number, and keep the 10s floor for positive values with 0 as the only sentinel below it. - Bound the same field in the persisted chat settings. - Offer the default minutes when a dialog switches off No limit, rather than 1. * Report an unlimited stall by name instead of racing HTTPX for PR #9078 With no wall clock behind an unlimited request, HTTPX's read timeout and the first-output/idle guards were set to the same value, so which one fired was a coin flip. HTTPX won often enough in an end-to-end run against a real SSE server, and it raises ReadTimeout with an empty message, so the run failed with a blank error instead of naming the stall. Give the read timeout headroom so the named guards expire first, and convert a bare ReadTimeout that still gets through into the matching stall error. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bound the research timeout where it is set for PR #9078 Two validators disagreed with the run route, so a value could be persisted that made every later Deep Research run fail to start with nothing pointing at the setting that caused it. - The composer's minutes field had no ceiling and the mirror bound was MAX_SAFE_INTEGER, while ChatSettingsPayload and the run route both cap at one year, so a large entry was accepted, dropped from the settings patch, and then rejected on every run. Bound it where it is set: the input, the store setter and loader, and the mirror now share the backend ceiling. - The settings payload took 1 through 9, which the run route rejects. Refuse those there too, so a persisted value is one the run route will accept. * Refuse a sub-floor research timeout in the frontend too for PR #9078 The settings payload now rejects 1 through 9, but the mirror bound and the store validator still accepted them, so a stale or hand-edited local value would hydrate, be sent unchanged, and 400 every run start while its own patch was also rejected. sanitizeBoundedNumber takes an optional minPositive so a sentinel below the floor stays legal while the gap above it does not, which is the shape this field needs and not a special case in the caller. * Clamp an over-cap typed budget instead of resetting it for PR #9078 The max attribute does not stop a typed value from reaching the save handler, which passed it on to a setter that substitutes the 900 second default for anything over the cap. So someone asking for a very long run saved what looked like their limit and got a fifteen minute one, which is the failure this PR exists to remove. Clamp to the documented maximum at save time. * Bound queue gaps by the configured admission heartbeat for PR #9078 * Tighten the comments for PR #9078 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Name the Deep Research budget by its per-request scope for PR #9078 * Reject a boolean budget instead of reading it as unlimited for PR #9078 * Reject a boolean settings timeout and keep an untouched budget exact for PR #9078 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Daniel Han
|
df8fb6a03b
|
Say both load fallbacks when both fire, and repair the two suites that guard it (#9189)
* Repair the two chat auto-load suites that #9173 left red on main #9173 added a vision-projector fallback to the auto-load success toast. Both of the suites that read that code broke on it, and both were green on the commit immediately before ( |
||
Maheswar Kumar
|
6e5360ca91
|
show the local model's context window before the first token count (#8882)
Some checks failed
Backend CI / (Python 3.11) (push) Waiting to run
Backend CI / (Python 3.13) (push) Waiting to run
Backend CI / Repo tests (CPU) (push) Waiting to run
Unsloth export capability / capability (ubuntu-latest) (push) Waiting to run
Unsloth export capability / capability (windows-latest) (push) Waiting to run
Frontend CI / Frontend build + bundle sanity (push) Waiting to run
Unsloth UI CI / Chat UI Tests (chat) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (extra) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (picker) (push) Waiting to run
Unsloth UI CI / Loaded-models indicator (cross-browser) (push) Waiting to run
Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Windows Unsloth API CI / Unsloth API & Auth Tests (push) Waiting to run
Windows Unsloth GGUF CI / Unsloth install + inference without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / setup.ps1 units, no-VS resolve, VC++ round-trip (windows-latest) (push) Waiting to run
Windows Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Wheel CI / Wheel build + content sanity + import smoke (push) Waiting to run
Windows Application Control CI / installer survives a denied unsloth.exe (push) Waiting to run
Workflow trigger lint / workflow-trigger lint (pull_request_target / cache-poisoning) (push) Waiting to run
Frontend CI / Frontend unit tests (Windows) (push) Waiting to run
Unsloth GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Mac Studio UI + API + Update CI / Chat UI, API and Update Tests (push) Waiting to run
Unsloth Tauri CI / Tauri Linux debug build (no codesign) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (windows) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (macos) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (banner) (push) Waiting to run
Windows Unsloth GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2022) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection + VC++ round-trip (VS 2026) (push) Waiting to run
Mac Studio GGUF CI / GGUF inference smoke (API, tools, vision) (push) Has been cancelled
* show the local model's context window before the first token count The chat header rendered the context bar only when `contextUsage` was set, so a resident GGUF showed nothing until a token count landed. The recount that produces one is best-effort: it stands down for images, audio, Deep Research, a pending RAG turn and a busy backend, and a chat that has never been sent has no usage at all. In every one of those cases the context length was invisible exactly while the user was picking a model to fit a prompt into (#8867). The bar now renders as soon as the resident model's window is known, showing `— / 32.8k` until a count arrives. An uncounted chat is kept distinct from a counted zero: `percent` stays null, so no fill, no percentage and no near-limit warning are drawn. Reporting an unmeasured prompt as 0% of the window would be the same misreport in a new place. `hasKnownContextWindow` gates on the resident checkpoint rather than the picked one, so a load in flight, an evicted model and an API selection all keep their windows off the header. `deriveContextUsageBar` moves the prop-to-text derivation out of the component so the node suite can drive it directly. * stop counting an empty chat's template scaffolding as context usage * decide the empty-prompt count where the tool policy is known * keep a passthrough tool catalog out of the empty-prompt refusal * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
b0843382d2
|
Studio: make the chat thread stop getting slower as it fills (#8992)
* Fix two module resolution failures in the frontend test suite
* Fix the third Windows-only path failure in the frontend test suite
* Measure how the chat thread's interaction cost grows with message count
Studio's chat UI is reported as sluggish on Windows 11 and worsening as the
thread fills, while token generation is unaffected. That shape says the cost is
per-message renderer work, so the thing to establish first is the curve.
New smoke page mounts the real Thread against a synthetic local runtime, seeded
to N messages, each carrying prose plus one code fence plus one KaTeX block so
Streamdown, Shiki and KaTeX all pay their per-message price. No backend, no
auth, no router.
New harness runs four scripted actions at N in {10, 50, 200, 500} under 6x CDP
CPU throttling: one keystroke into the composer, one scroll gesture, one message
action menu opened and closed, and one message delete. Each is bracketed by
Performance.getMetrics, which separates the two families of cost: work that
grows because layout is uncontained lands in LayoutDuration, work that grows
because a listener or an export is O(messages) lands in TaskDuration alone.
It measures, it does not gate. There are no timing budgets: it prints the table
and exits 0 unless the harness itself broke. Budgets belong in a later change,
set from numbers taken on real hardware. What it does fail on is measuring
nothing, since that is the failure mode that reads as good news: a seed that did
not render, a menu that never opened, a delete that deleted nothing, a scroll
that did not move, or four columns that do not rise with N.
Measured on this tree, N=10 to N=500: menu open+close 1309ms to 33199ms,
delete 438ms to 8491ms, keystroke median 86ms to 268ms.
Every metric recorded reaches the printed table, and the harness contract test
now enforces that mechanically by parsing the recorded keys out of the source
and requiring each one in the table.
CDP CPU throttling and longtask are Chromium-only, so Firefox and WebKit runs of
this file are correctness checks and not performance ones.
* Stop the thread-weight harness charging its own cost to the app
A review of the first commit found four ways the measurement could look clean
while reporting something other than the app, and two of them were forging part
of the curve. All were reproduced before being fixed.
The local runtime does have a remote id. It synthesises `__LOCALID_...`, which is
truthy, so the per-message fork-count GET fires after all: seeding 20 messages
issued 10 requests, against a comment claiming nothing reached the network. They
were being answered by a Playwright route handler, so each one paused the
renderer for a round trip to another process, once per assistant message. The
page now answers them itself, before anything mounts, and the harness fails if a
single request escapes during a measured action.
Closing the menu was timed from after the Escape dispatch. Radix dismisses
synchronously inside it, so the layer teardown, focus restore and re-render --
the O(messages) fan-out this issue is about -- were excluded from the number
meant to capture them.
Every timing carries a ~33ms floor, since a double rAF cannot resolve faster than
two vsync intervals, and CPU throttling does not move it. An action that never
happened therefore reported ~33ms, which reads as a plausible measurement rather
than as a failure. The floor is now measured per N, printed, subtracted before
every growth ratio, and a keystroke at or under it fails the run.
The keystroke check read the DOM value back, which is what the harness itself
wrote. It now compares against the runtime's own composer state, so a keystroke
that reaches the textarea but not React is caught.
Also removed from the timed regions: a per-frame document-wide querySelector in
both poll loops, replaced by a MutationObserver flag and an isConnected check; a
counts() call per frame in the seed gate; an animated scrollIntoView still in
flight when the menu window opened; and a console warning per action-bar render,
by giving the page the router its useNavigate calls expect. Long tasks are now
read after a yield, since the observer delivers on a later task and the tail
entry was being dropped.
Corrected curve, N=10 to N=500, floors removed where they apply: menu open+close
1021ms to 33591ms, delete 297ms to 8563ms, keystroke 48ms to 283ms, scroll worst
frame 5ms to 126ms. Layout stays flat and tiny throughout; the growth is in
style recalc and task time.
* Stop mounting the assistant action bar for every message
At rest the full assistant action bar was mounted under every assistant
message. Each one carries around eight tooltips, and every tooltip holds a
useSyncExternalStore subscription to the shared modal-layer store, which
re-walks its ancestors reading style.pointerEvents whenever Radix puts the
body on the modal layer. A 500-message thread therefore mounted 250 bars and
1503 tooltip triggers, and every menu open fanned out across all of them.
autohide unmounts rather than hides (ActionBarRoot returns null on the hidden
status), so passing it removes the nodes and the subscriptions together. The
user bar has always done this.
Not unconditionally "always", though: this bar carries the only Stop reading
control, which is why it already passes hideWhenRunning={!speaking} and why
DeleteMessageButton guards the same case. With "always", moving the pointer
off a message being read aloud would take that control away. At most one
message speaks at a time, so exempting it costs nothing.
Measured with tests/studio/playwright_thread_weight.py at 6x CPU throttle,
before -> after, at 500 messages:
action bars 250 -> 0
tooltip triggers 1503 -> 3
DOM nodes 56332 -> 41082
delete ms 8595.8 -> 3642.7
scroll worst frame 159.5 -> 87.3
menu open+close ms 33624.5 -> 25279.3
keystroke median ms 316.2 -> 241.0
Note what did not move: menu style recalc, 23725.8 -> 22567.7 ms. A 27% cut
in DOM buys 5% there, so the bar is not what makes that number grow. It is the
document-wide invalidation from Radix writing pointer-events onto the body,
and it is still the dominant cost at large N.
The index.css comment is corrected in passing: it justified forcing
content-visibility: visible on every code block with "thread length is
bounded", which is the assumption this issue disproves. The rule is kept for
the flicker it was really fixing.
* Studio chat: one fork-count subscription per thread, not one per message
The fork badge registered its own CHAT_HISTORY_UPDATED_EVENT listener and issued
its own GET, and it is mounted once per message. A delete on a 200-message thread
therefore fired 200 requests before anything could repaint, and streaming raises
that event once per chunk.
Badges now share one debounced subscription per thread and one request that
returns every fork count of that thread, so the cost is flat in thread length.
* Studio chat: derive research-message ownership once per thread revision
useOwnsResearchMessage exported the whole thread from inside a per-message render
body, so one render pass over N messages exported N times and inspected N*N items.
Streaming re-renders the thread once per chunk, so that pass is hot.
The answer is a property of the thread revision, so derive it once for the message
list every message in the pass already shares. Measured on a synthetic thread: 200
exports and 0.80ms per pass becomes 1 export and 0.014ms; at 1000 messages 16.4ms
becomes 0.02ms.
* Studio chat: stop deep-cloning the thread on every delete and every save
exportedItemToRecord ran JSON.parse(JSON.stringify(...)) over every message's
content and attachments on its way to a PUT that serializes the same records
again, and syncExportedRepositoryToBackend ensured the thread row that
syncStoredChatMessages already ensures, so every save paid for GET /threads/{id}
twice.
The parts are replaced rather than mutated, so a copy of the list is snapshot
enough and the bytes on the wire are identical (asserted in the new test).
Measured on a 200-message thread of ~4KB messages: the record step drops from
1.17ms to 0.015ms, and a delete makes one thread-row read instead of two.
* Studio chat: open the message action menu non-modally
A modal Radix menu writes pointer-events:none on <body>. That is an inherited
property, so every open and close invalidates style for the whole document, and
on a long thread the recalc is the bulk of the cost. Non-modal never writes it.
Also teaches the harness the difference between a cost that was removed and a
page that never mounted one, so the after-tree does not read as broken.
* Pin the message action menu to the non-modal layer
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drop the #8980 content this branch no longer needs
* Print the hovered trigger count and keep the modal layer load-bearing in the verdict
The contract test caught both: a metric recorded but not printed, and the verdict
no longer reading body_pointer_events_while_open after the non-modal fix removed the
old check. The layer is now compared ACROSS N instead, since either mode is
legitimate but mixing them means the columns measure different mechanisms.
* Stop the rapid-submit settle wait measuring the action bar instead of the reply
This branch autohides the assistant action bar, and that turned the last
wait of the rapid-submit step into a 7-of-7 failure on Windows CI. The wait
is not what the step proves, and the clause that broke was not measuring
what it claimed.
innerText of a [data-role=assistant] root spans the whole subtree, and the
action bar sits inside it, so 'every reply has non-empty innerText' was
satisfied by button labels regardless of what the model returned.
Instrumented at that point on the CI runners, two runs on this branch's
merge base read:
content=[0, 0] innerText=[73, 73] clause held, BOTH replies empty
content=[0, 19] innerText=[73, 89] clause held, first reply empty
gemma-3-270m-it answers 'Reply with exactly: rapid-first' with an empty
completion in 3 of 8 sampled runs, on the merge base as much as here, and
the clause held every time. So the empty reply is the model, is pre-existing,
and was simply masked. With the bar autohidden the subtree is content only,
and the same empty completion now fails.
Dropped rather than repointed at the content element: an empty completion is
the model's behaviour, so a content assertion would be flakier than what it
replaces. What is left is exactly the settle this wait is for, two bubbles,
nothing streaming, nothing queued. The behaviour the step exists to prove,
that a 100 ms follow-up queues behind a held first turn, is state.queueSeen
above and is untouched.
Test-only. No Studio code changes, so the menu open+close and nodes-at-rest
wins are unaffected. Verified on Windows CI at this branch's head: 4 of 4
Chat UI Tests jobs green with this change, against 7 of 7 red without it.
* Validate the delete measurement at every size, not just the last
* Keep the thread's fork counts across the autohidden badges
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Exempt the thread-weight harness from the CI-coverage check
* Keep the newest reply's action bar in the tab order
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Load the crypto polyfill on the thread-weight smoke page
The page this branch adds was the only one in studio/frontend without
<script src="/crypto-boot.js"></script> in its head, so
crypto-uuid-boot.test.ts fails with "smoke-thread-weight.html must load
/crypto-boot.js".
The rule is not cosmetic: the polyfill has to run before the module entry
or crypto.randomUUID is missing on the older WebViews Desktop embeds, and
a smoke page without it measures a page that differs from production in
the one respect the harness is meant to hold constant.
Reproduced on this branch and verified: the named assertion fails before
the change and the file's four tests pass after it.
* Debounce the fork count refresh instead of throttling it
onHistoryUpdated returned while a timer existed, which is a leading edge
throttle rather than a debounce. Streaming raises the history event once per
chunk, so the timer expired mid stream and the next chunk armed another one,
costing a whole thread fork count fetch every 300ms for as long as the reply
ran. Fork counts cannot change during generation, so all of those were waste.
Clear and reschedule on every event, as the sidebar refresh already does.
The existing burst test fired all 20 events inside one window, where a
throttle and a debounce behave identically, which is why this survived. The
new case spreads the events across the window like a real stream: 20 chunks
cost 10 refetches before this change and none after, with one refresh in the
quiet window that follows.
* Reveal the action bar on focus, not only on hover
Unmounting the bar on every message but the newest took its controls out of
the tab order, and there was no non-pointer way to bring them back, so Copy,
Edit, Refresh, Delete, Read aloud and More were unreachable by keyboard or
screen reader on every older reply. Deferred rendering is fine while the user
can still ask for what is deferred, and tabbing is asking.
Measured on the thread weight smoke page at 20 messages, older reply: the
accessibility tree exposed no bar controls before this change and still none
after focus entered the message; it now gains Copy, Refresh, Delete, Edit
response, Read aloud and More. Three tabs from an older reply used to walk
its two code fence buttons and step to the next message without ever
entering a bar; the same walk now lands on the bar's Copy.
The library has no focus path, so this drives its own isHovering flag from
focus within the message root. Two writers share that flag, so a pointer
leaving while focus is inside re-asserts it from a listener registered in an
effect, which runs after the primitive's own mouseleave in the same dispatch
and so never renders the intermediate false that would unmount the element
holding focus. The clear is a one frame watchdog reading activeElement
rather than a relatedTarget test, because relatedTarget is null both for
browser chrome and for a portal, and no focusout fires at all when the
focused element is removed, which is how the menu closes.
At rest this stays at one mounted bar of ten, and at one again after a focus
round trip, so the weight this branch is here to remove is unchanged: 1950
DOM nodes and 9 tooltip triggers, the same as before.
* Give a plain prose reply a way into the tab order
The focus reveal only fires once focus is inside the message, and a reply
whose body is plain prose contains nothing focusable after autohide unmounts
its action bar. The earlier measurement of two focusable controls per message
was an artefact of a fixture where every reply carried a code fence, and
Streamdown ships one Copy button per fence. Seeded with a prose only reply it
is zero, and Tab walks straight past the message into the next reply, so
Copy, Edit, Refresh, Delete, Read aloud and More stay unreachable.
tabIndex on the message root rather than a visually hidden button: it adds no
DOM node, which matters for a branch that exists to cut per message weight,
and it draws nothing at rest. The app's own focus-visible rule gives it the
same 1px keyboard indicator every other focusable container already has, and
focus-visible means a mouse click still draws nothing.
Measured at rest, unchanged: one action bar of ten, 1950 DOM nodes, 9 tooltip
triggers. Outline is none with neither focus nor hover, and none after a
click. The cost is one extra tab stop per assistant message, which is the
price of the controls being reachable at all.
The fixture gains an opt in plain prose variant so the existing weight
measurements keep the exact thread they had.
* Bound how long a fork change can wait behind an unrelated stream
CHAT_HISTORY_UPDATED_EVENT fires once per streaming chunk, and the fork-count refresh was a pure
trailing-edge debounce, so a reply running in a background thread reset the timer on every chunk.
Deleting a fork from the sidebar while looking at its parent changes the displayed count, and the
refresh was postponed until the unrelated stream went quiet, which on a long or queued run is
minutes. That is starvation, not slowness.
The event is a bare Event with no detail and six other consumers, so telling fork changes apart
from chunks means changing a contract well outside this store. FORK_COUNT_REFRESH_MAX_WAIT_MS
bounds the wait inside it instead: a second timer, started by the first event of a burst and
deliberately not restarted by the ones after it, races the debounce, and whichever fires first
cancels the other. A second timer rather than a Date.now() deadline so it runs off the same clock
as the debounce and is testable without a fake Date.
2000ms because the bound costs one whole-thread fetch per window while a stream runs. At the
300ms debounce that is the per-chunk traffic this store exists to remove; at 2000 it is under a
sixth of it, and only while something is streaming.
The existing continuous-stream test asserted ZERO mid-stream fetches, which is the behaviour the
review flagged, so it now measures the price of the bound instead of claiming there is none: it
pins the count against both what the ceiling allows and what a leading-edge throttle would have
cost, so a regression in either direction is a failure.
Four assertions were made to fail on their own broken tree before being kept: the ceiling
removed, the ceiling restarted per chunk so it never expires, the losing timer left uncancelled
when the other fires, and the ceiling left running past unsubscribe. That last one was vacuous at
first, since the entries map is empty after unsubscribe and a leaked timer refreshes nothing; it
now subscribes a second thread inside the ceiling window, which is both observable and the case
that actually costs a user a request.
* Scope the popup lookup to the action bar
The watchdog treated any expanded descendant as this message's open menu. Reasoning cards and
tool-fallback cards are Radix CollapsibleTriggers and render aria-expanded=true for as long as
the reader leaves them open, which is the resting state of a message whose tool output has been
expanded. decide() therefore found a popup every frame, rescheduled itself every frame, held
focusWithinRef and the synthetic hover set, and left the bar mounted indefinitely, at the cost of
a DOM query per frame per such message. Scoped to .aui-assistant-action-bar-root, which is where
the trigger the hook has to hand focus back to actually lives.
Proving this took three attempts and the first two were wrong, which is worth recording because
the failure was in the test rather than in the fix.
isHovering has two writers. This hook is one; assistant-ui's own MessagePrimitive.Root mouseleave
handler is the other, and it writes false directly. So a phase that reveals the bar by HOVERING
and then moves the pointer away sees the bar unmount on both trees, because the library unmounted
it. On the broken tree the watchdog genuinely spins forever, and the bar still goes away. An
assertion on the mounted bar count cannot attribute that outcome to this branch, and C2 passed on
the fixed and broken trees alike.
The phase now keeps the pointer off the message entirely and reveals the bar by focus, which the
tabIndex on the message root makes possible. With no mouseleave to fire, focus is the only writer
and the bar's fate is decided by the watchdog alone. C2 is green on the fixed tree and red under
--break widepopup, 1 bar still mounted and held indefinitely. Two guards sit in front of it: one
asserts the pointer really is off the message, the other that the bar really was mounted and
focused, so the phase fails loudly rather than passing vacuously if either precondition breaks.
Also fixes a pre-existing crash the sweep was hiding. Under --break eagerclear the bar is gone by
the time A8 runs, and a bare more.focus() on an undefined element threw a Playwright TypeError
that killed the process before phases P, B and C ever ran, so the break reported fewer reds than
it earns and ended in a traceback rather than a red result. It is more?.focus() now and the
trigger's absence is folded into A8's condition, so eagerclear completes and A8 goes red on its
own merits.
Full sweep: head 14/23, notabindex 19/23, restring 21/23, focusring 22/23, leakflag 21/23,
noreassert 22/23, eagerclear 21/23, widepopup 22/23 with C2 the only red. thread.tsx checksummed
before and after all eight runs, identical every time.
* Reveal the action bar from the backward traversal too
The tabIndex on the message root only worked going FORWARD. A container is reached before its own
descendants, so Shift+Tab arriving from the message below landed on the last tabbable thing in the
message, and with the bar unmounted that is the root, which sits BEFORE the bar in DOM order.
Focusing it mounted the controls and the next Shift+Tab then stepped straight past them to the
previous message. Copy, Edit, Delete and More were reachable going forward and unreachable going
backward, which is worse than being unreachable outright, because the forward pass makes it look
solved.
A sentinel span after the bar is what makes the backward pass land inside the message: focus stops
there, the bar mounts, and the next Shift+Tab goes into the last control rather than out. It is
deliberately NOT a focus redirect to that control, which would trap the forward pass in a loop
between the last button and the sentinel. It carries no onFocus of its own because React's onFocus
is focusin and already bubbles to the root, and no role, because it performs no action; the
aria-label is what stops it being an unannounced stop.
It DOES cost one DOM node per assistant message, and this branch is about per-message weight, so
that is asserted rather than absorbed: the at-rest guard now requires exactly 1950 baseline nodes
plus one sentinel per reply and nothing else, measured 1960 for 10 replies, with the sentinel count
checked separately so the extra nodes are attributed rather than tolerated.
Three assertions, each proven red. nosentinel takes D2 and D3 red while D1 stays green, which is
precisely the reported asymmetry: focus still enters the reply, it just skips the bar. D1 says
something weaker, so it needs noentry, which removes the sentinel and the tabIndex together and
leaves nothing in the message reachable at all; D1, D2 and D3 all go red there.
Full sweep, 26 assertions: fixed 26/26, nosentinel 23/26, noentry 18/26, notabindex 24/26, head
15/26, widepopup 25/26 with C2 alone, eagerclear 21/26, noreassert 25/26, leakflag 24/26, restring
24/26, focusring 25/26. thread.tsx checksummed before and after every run, identical throughout.
* Draw a focus indicator on the backward reveal sentinel
The shared soft-outline rule is :where(div, main, section, aside, ul, ol):focus-visible, which
never matched a span, so the sentinel was a real tab stop that drew nothing: Shift+Tab into a
message made focus visibly disappear for one stop before the next press reached the action bar.
That is a focus-visible failure, not a cosmetic one.
The element stays 0x0 and the ring is drawn by outline-offset. Outlines take no part in layout,
so the indicator appears without shifting the message, which giving the span dimensions on focus
would have done. Still nothing at rest, and :focus-visible means a mouse click draws nothing
either.
Measured: with keyboard focus the ring spans 14px against the UA default's 2px on a zero-sized
span, which is the difference between an indicator and no indicator. D4 asserts the SPAN of the
drawn ring rather than merely that an outline style exists, because the broken tree still reports
outline-style auto and would satisfy a presence check while showing nothing.
Proven red by --break blindsentinel, which removes the indicator rule and leaves the tab stop
itself intact, so only D4 fails.
* Stop tracking the generated Studio test database
.studio-test-root/studio.db is written at test time by
tests/studio/install/test_selection_logic.py, which points storage_roots.studio_root
at that path. It is a mutable SQLite runtime database, not a fixture: nothing
reads it, any test run or Studio start rewrites it and dirties the checkout, a
later accidental commit could capture real local chat or settings data, and it
puts 221 KB into every clone while exercising nothing.
It was not in the tree deliberately. It arrived in the merge commit here because
that commit was staged with `git add -A` after running the suite, and no
gitignore rule covered the path. Main does not track it.
Untracked and ignored, and the file is left on disk since creating it is normal.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>
Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com>
|
||
Nilay
|
01cf3728cd
|
Studio: show MCP server display names instead of internal ids (#8557)
* Show MCP server display names in tool cards, status text, and exports * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stamp MCP display names on provisional tool events * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Import mcp-tool-name directly and drop unused provenance import * Stamp MCP names on unrun tool cards and index the display label * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the new code comments for PR #8557 * Shorten the new comments for the MCP display name change --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Nilay
|
8696c5fb3c
|
Studio: remember last used model in the backend, not per-browser (#8587)
* Studio: remember last used model in the backend, not per-browser * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pass auto-load abort signal through readLastLocalModelLoad * Last-model setting: path-length variant bound, legacy shadow write on failed PUT * Write the legacy shadow record synchronously before the PUT * Last-model shadow: keep v1 loadedAt, reconcile pending shadow on read * Last-model setting: order pending shadows by loaded_at across surfaces * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Last-model setting: re-sync newer shadows even on identity match * Last-model setting: match timestamp before clearing the pending marker * Last-model setting: ignore stale timestamped writes on the backend * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Last-model setting: clamp client stamps, reconcile pre-upgrade tab writes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Last-model setting: translate client stamps into the server clock frame * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Last-model setting: only a pending shadow outranks an unstamped backend record * Fix biome-ignore comment indent * Scope the remembered model per subject for PR #8587 The endpoint authenticates a subject but stored one shared row, so on a multi-user install whoever loaded a model last decided what everyone else auto-loaded. Key the record by a hash of the subject. A subject with no row of its own still falls back to the pre-scoping shared row, so an existing install keeps the model it already remembered and only stops following it once that subject writes its own. Rethrow an aborted read by error name rather than instanceof DOMException: authFetch goes through the Tauri network-retry wrapper, and an abort resurfaced as a plain Error would be swallowed and answered with a stale shadow record. Adds the frontend coverage this file had none of (old backend 404, shadow newer/older, pendingSync, day-scale clock skew both ways, corrupt shadow, the gguf-variant rule, abort), plus a test pinning a known limitation: client_now is stamped at send, so a PUT delayed in a retry is re-dated to arrival and an older load can outrank a newer one. Ordering that correctly needs a server-issued revision rather than a client timestamp. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the new code comments for PR #8587 * Trim the new comments in PR #8587 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> Co-authored-by: danielhanchen <unslothai@gmail.com> |
||
|
Nilay
|
1274395567
|
Studio: bulk chat management multi-select move/pin/archive/export/delete (#8745)
* Studio: add Manage chats bulk actions in Settings > Data * Format two test files to satisfy the pre-commit ruff hook * Use main's setPinned and scope the select-all label for PR #8745 main landed setPinned(ids, pinned) on the pinned-chats store while this branch carried an equivalent setManyPinned, which is what the merge conflicted on. Drop the duplicate and call main's, whose no-op early return also avoids a pointless write when nothing changes. The header checkbox toggles the visible page (20 rows behind Show more) but its label read "Select all chats" next to a total count, so on a longer list it promised more than it selected. Label it for what it does, and cover the bulk pin/unpin ordering. * Tighten the new code comments for PR #8745 * Anchor shift-select on the chat id and honor the delete-files preference for PR #8745 The shift-click anchor was a row index held in a ref, but useChatSidebarItems re-sorts by updatedAt on every history update, so a chat streaming in the background can move rows between the two clicks and the saved index then names a different chat. The range swept from there could archive or delete chats the user never selected. Anchor on the chat id and resolve it at click time, reusing rangeBetween, which the sidebar already uses and which falls back to the clicked row alone when the anchor is gone. Bulk delete also called deleteChatItems without the args object, so the API got delete_files:false and every selected chat kept its sandbox even with 'Always delete files' on. Pass the preference through, as the sidebar and single-chat paths do. * Carry the row project when opening a chat from Manage chats for PR #8745 * Tighten comments in the Manage chats view and its tests --------- Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
Wasim Yousef Said
|
077593612c
|
Fix low VRAM mmproj fallback in Studio (#9173)
* fix(studio): recover mmproj startup on CPU * fix(studio): report mmproj fallback in chat * test(studio): cover mmproj fallback messaging * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio): refine mmproj recovery guards * fix(studio): complete mmproj reload recovery --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Michael Han
|
3bf9bff5dd
|
Say why a Xet download sits at 0% until it finishes (#9159)
* Say why a Xet download sits at 0% until it finishes Xet fetches a model as parallel chunks and commits them in batches, so progress reads 0% for a while and then completes at once. That looks like a hung download, and it is the most common complaint about Xet. Toast the reason on the first three model downloads that start on Xet, and point at the HTTP transport in Model Hub for anyone who wants steadier progress. The count is stored, with an in-memory carry so a browser that refuses the write does not repeat the toast on every download. * Skip the Xet notice when the start attached to a live job The start endpoints accept a second client for a job already running and report that job's transport, which is indistinguishable from a fresh Xet start. Both now say which it was, so an attaching tab neither shows the notice nor spends one of the three. * Declare attached on the download start schemas The routes set response_model, so a key the schema does not name is dropped before it reaches the browser. The flag shipped in the handler but never in the response, leaving the notice gate reading undefined. * Skip the Xet notice on a start that is already stopping A cancel can land while the start POST is in flight: the start still comes back accepted, so the notice promised a running download the user had just stopped, and spent one of the three doing it. * Report attachment only when the start actually attached The reject branch also covers a cross-variant conflict and an in-progress delete, which join nothing. Both flags now come from the adoptable verdict. * Reserve the Xet notice slot under a cross-tab lock Two tabs starting a download at the same moment both read the count before either wrote it, so a fourth toast could appear. Web Locks make the read and the write one step; browsers without them keep the old bounded race. |
||
|
Michael Han
|
c675be1adc
|
Open the connection form when Settings > Connections has nothing to list (#9157)
* Open the connection form when Settings > Connections has nothing to list An install with no connections opened that page onto a single "No connections yet" row, which never says what a connection is or how to add one. Open the "New connection" form instead when the first sync comes back empty. The check reads the synced backend response rather than the local snapshot, so a stale empty list cannot flash the form at someone who already has connections, and it waits for a loaded provider registry so the form never opens with an empty dropdown. It fires once per visit: the back arrow, saving, and deleting the last connection all leave the user where they navigated to. * Let a manual navigation cancel the connections auto-open The Add connection row stays live while the first sync runs, so a user can open the form and go back before it lands. Only the sync consumed the one-shot, so it then pulled them back in. Every navigation the user drives consumes it now. |
||
|
Daniel Han
|
2073e0fe4d
|
Studio: load the settings tab panels when they are shown, not at launch (#8966)
* Studio: load the settings tab panels when they are shown, not at launch * Studio: keep a settings panel that fails to load from taking the app down A panel is fetched the first time it is shown, so it can now fail where it could not before: offline, or a page whose entry bundle predates an in-place rewrite of dist/ and still names chunks that have been replaced. The dialog is mounted at the app root and nothing above it catches, so the throw unmounted the whole of Studio rather than one panel. Blocking a panel's module in a browser reproduced it: the dialog, its nav and the rest of the page went. The panel area now sits in an error boundary that offers a reload, and the Suspense fallback is a delayed loading line rather than an empty pane, so a slow first open shows something and a prompt one still shows no flash. Reload rather than retry: React caches a lazy rejection for the life of the page and the browser's module map caches the failed import, so re-importing the same URL rethrows without a new request. index.html is served no-store, so a reload does pick up the current chunk names. tests/settings-tab-panel-loading.test.ts gains a case that walks the JSX and asserts every panel Suspense is inside a class that defines getDerivedStateFromError. tests/studio/playwright_settings_tabs.py drives the real dialog in a browser: all twelve tabs, deep-open, the search jump, and the blocked-module case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: consume a failed settings panel prefetch The idle prefetch warms every panel once the dialog opens, so a chunk it cannot fetch reached the page as an unhandled rejection for a tab nobody had asked for. Reproduced by blocking one panel's module in a browser: the rejection landed on window even though the boundary handled the panel that was actually on screen. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: typecheck the settings smoke entry, and stop the harness settling on a placeholder tsconfig.app.json lists the three existing smoke entries explicitly, so the new one was outside the project and npm run typecheck skipped it. Added, and the harness handle it installs on window is now optional, since app code sits in the same project and must not be able to reach a handle only the harness page installs. tsc --listFiles now names the file. The Playwright harness settled on whatever held still for 600ms. The panel renders from a deferred value, so a switch keeps the outgoing content up until the incoming panel is ready, and on a loaded machine that hand-off outlives the window: one run read the placeholder as the final panel and called a correct recovery a failure. It now refuses to settle on something with almost no content. A run that dies on a cold dev server also writes its report instead of leaving none. * Tighten the settings lazy-panel comments * Name the encoding when the settings harness writes its report tests/test_source_read_encoding.py holds every checked-in file read and write in the test trees to an explicit utf-8, so it does not depend on the platform default and break on Windows the day the file gains a non-ASCII byte. The report write was the one that did not. * Let the select's keyboard scroll settle before the font-scale wheel check Pre-existing flake in this step, not something this branch introduced. The step reads scrollTop straight after keyboard.press, but Radix scrolls the highlighted item into view off the back of that keypress, so the value is a mid-scroll sample: instrumented on the ubuntu CI image the viewport went on to settle 24-35px further down in 20 runs out of 20, on this branch and on its merge base alike. Two things break as a result. The stale sample is not the floor the wheel has to beat, which is why the failure reads '20 -> 44' as though the viewport had moved the wrong way when 44 is simply where the keyboard scroll ended up. And a wheel dispatched into a scroll Chromium is still animating can be swallowed outright, which is the actual failure: at a maximum scrollTop of 243 a working -400 wheel lands on 0 every time. So wait for the scroll to stop before taking the floor, keep the pointer inside a viewport that is not always 40px tall, and re-send the wheel on a bounded retry. A viewport that genuinely refuses the wheel still never moves and still fails, just after more tries. * Clear an unconsumed archive deep-open when settings navigates away * Run the settings tab-panel browser smoke in frontend CI * Keep an archive deep-open when the navigation lands back on Data * Keep a settings scroll target when its own tab is reselected * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Load the crypto polyfill on the settings smoke page * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim comments in the settings lazy-loading changes * Hold only the Data panel's own module in the settings deep-open test The abandoned-deep-open step routed every request through a handler that sleeps 2.5s, and that sleep runs on the driver thread, so the whole page's module load queued behind it and arrived at the main thread in one go. On a two-core runner sharing the box with a live Studio that pushed the reopen past its 15s timeout, which reads as a settings dialog that would not open when nothing was wrong with it. Route the Data module alone. The assertion is unchanged and still goes red on the pre-change store: the next ordinary visit to Data reopens the archive listing. * Name the cause when the settings smoke page has navigated away Vite dev proxies /api to 127.0.0.1:8888. With a Studio listening there and no token those calls answer 401, the app's auth handling navigates, and the harness window goes with it, after which every step times out waiting for a dialog that cannot exist. It happens on main too, where the harness is gone before the first open, so it says nothing about the panels. Report it. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> |
||
|
Daniel Han
|
e35b8a8447
|
Studio: stop a settings module reading a chat store key before it exists (#8979)
* Studio: stop a settings module reading a chat store key before it exists * Fix two module resolution failures in the frontend test suite * Fix the third Windows-only path failure in the frontend test suite * Drop the #8980 content this branch no longer needs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim the comments this PR added --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |
||
|
Daniel Han
|
c755c9e3ab
|
Studio: pin what the trailing-placeholder window does with nested placeholders (#9091)
* Pin what the trailing-placeholder window does with nested placeholders The module comment says the window's guarantee is about what is removed rather than about leaving an oversized fragment whole, and gives nested placeholders as the case where those two readings come apart. Nothing pinned it, so changing the behaviour to match the looser reading would have gone unnoticed. The test kills both directions: widening the window so the outer opener comes back into range, and declining to strip whenever an opener predates the window, which is the change a review of #9012 asked for. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the comments in the nested placeholder test --------- Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
ec18cdd29f
|
Chat: cover which messages a delete takes with it (#9087)
* Chat: cover which messages a delete takes with it * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pin which message the thread is left showing after a delete The cases so far cover which messages a delete removes and how the survivors are relinked, but not headId, and headId is not cosmetic: thread.import hands it straight to MessageRepository.resetHead, which throws on an id it cannot find. A head left naming the message that was just deleted therefore does not merely show the wrong branch, it makes the import fail after the backend prune has already run, so the server drops the message and the screen keeps it. Three cases: the head always names a message that is still there (or null once the thread is empty), deleting the reply on screen falls back to the sibling it was regenerated from rather than to the prompt, and deleting a branch nobody is looking at leaves the shown message shown. --------- Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
17665d4a85
|
Studio: pin why a Streamdown remount keeps its highlighted code (#9048)
* Studio: pin why a Streamdown remount keeps its highlighted code The incremental Markdown cache bumps `renderGeneration` when dropping retained blocks cannot be signalled through the Markdown string, and that generation is part of the <Streamdown> React key, so the block subtree unmounts and remounts. That was read as losing Shiki's highlight state and flashing every fence in the reply back to unstyled code. It does not, and the reason is worth holding in place. None of the highlighting state lives in the component tree. The wrapper in code-plugin.ts is built once at module scope in markdown-text.tsx, so its per-fence slots outlive any remount, and `@streamdown/code` keeps its highlighter instances and its tokenized results in module-scope Maps and answers a repeat request inline. A remount therefore re-asks for tokens it already has and gets them back in the same tick. Upstream reported and fixed exactly this. vercel/streamdown issue 186, "Code block flickering in virtual lists due to async Shiki highlighting", names "No caching between mounts: Highlighter state is lost when components unmount", and was closed by PR 240, merged 2025-11-21, which rebuilt the code blocks. The pinned `@streamdown/code` 1.1.1 carries that rebuild. Nothing equivalent is open on shikijs/shiki, where the highlighter is an object the consumer owns and caching it is the consumer's job. Measured on this branch's base: with a 40 line TypeScript fence the cold mount answers through its callback, and a fresh plugin standing in for the remount answers synchronously in 0.010 ms with the same 40 token lines. Two tests pin the halves of that. One drives a real highlight and then asks a second plugin instance for the same fence, which has to answer inline. The other walks markdown-text.tsx and requires the single `createCodePlugin` call to sit at module scope rather than inside a component. Both fail on a tree broken for them and only for them: returning null instead of the synchronous cache hit from the wrapper's dispatch fails the first and leaves the second passing, and moving the plugin construction into a function fails the second and leaves the first passing. No behaviour change. The key and the generation counter are left alone. On this base, driving the cache through the renderer's own stabilizeStreamingMarkdown(preprocessLaTeX(text), true) pipeline over twelve streamed fixtures covering prose, fenced code, inline and display LaTeX, currency dollars, and spans that stay open across thousands of characters: at one character per arrival, 46,181 updates produced 26 non-prefix updates, 1 retained-block drop and 0 generation bumps; at four characters per arrival, 11,550 updates produced 14 non-prefix updates, 1 drop and 0 bumps. The generation never moved off zero, so there is no remount to remove and nothing for a rendering benchmark to show. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pin the remount test to this repo's plugin, not the dependency cache The remount case built a fresh plugin wrapper, so the only thing keeping its answer synchronous was the module-scope cache inside @streamdown/code. Production hands Streamdown the same module-scope plugin across a remount, so reuse the mounted one and the test pins our wrapper instead. The threshold guard compared against a local copy of MIN_INCREMENTAL_CHARS, so raising the production value to 100000 sent every fence down the small fence shortcut with the test still green. Export the constant and import it. Await the cold ask's callback rather than polling highlight(), so the warm up loop no longer satisfies the remount assertion on its own. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |
||
|
Daniel Han
|
1f09a17868
|
Studio: keep the streaming render harness the perf PRs kept rebuilding (#8969)
* Studio: keep the streaming render harness the perf PRs kept rebuilding
* Studio: close the false-green paths in the stream pacing harness
The long-task total is the metric the budgets turn on, and it reads 0 both
when the render is free and when the observer never ran. observe({type:
"longtask"}) is specified to abort silently on an engine that lacks the
entry type rather than throw, so the try/catch around it never fired: under
firefox or webkit the harness scored a perfect zero and exited 0. Detect
support with PerformanceObserver.supportedEntryTypes, record it, and fail
the run when no long tasks were seen or when throttling was disabled.
Also:
- add the entry to tsconfig.app.json, which lists the smoke entries one by
one, so npm run typecheck actually covers the 270 lines it was reported
against
- write the JSON under logs/ like every sibling harness instead of dropping
an untracked stream-pacing.json in the repo root, and create the directory
- treat an exported-but-empty SMOKE_BASE_URL as unset, matching the siblings;
it drove "" as the base URL and burned the full readiness timeout
- register the harness in the two contract tests, which is what surfaced the
SMOKE_BASE_URL bug, and pin the new guards there
- record the second mutation: reverting #7892 moves the longest stall 4-5x
while leaving the long-task total inside the clean range, the exact
opposite of reverting #8750, so both budgets are load-bearing
* Studio: budget only the long tasks the stream itself caused
buffered: true replays whatever the performance timeline already held, so
module evaluation and the first React render landed in the budgeted total:
one entry, ~140ms, about 2.6% of a clean run here, and larger on a cold or
loaded runner. Nothing filtered by startTime and run() reset nothing, so a
slow page load read as a slow renderer.
Open the measurement window in run() and drop entries that began before it.
Pre-stream share goes 2.6% to 0.00% while the stream's own tasks are
unchanged (60 and 52 entries over two clean runs), and reverting #8750 still
fails the budget at 52,465ms.
* Studio: tighten the stream pacing harness comments
Comments and docstrings only, no code change. Every measured number, PR
reference and causal reason is kept verbatim.
* Studio: check the reply that settled, not the peak it once reached
paintedChars is a high-water mark and only ever climbs, so a completion
render that truncated the bubble would leave the peak behind and the 90%
workload floor would still pass on a DOM that no longer held the reply.
Record what is on screen at settlement and check that too. Measured equal
to the peak today (24,033 both), so this is a guard rather than a live
discrepancy, and it is pinned in the harness contract test.
Also count slow frames only inside the measurement window and reset the
counter in run(), the same rule long tasks now follow. Contamination
measured at 0 of 286 here, but an external server or a slower box need not
be 0 and the number is meant to be comparable across them.
* Studio: record a stall that never ends, and keep the task that starts the stream
Two holes left by the measurement window added in the previous commits.
A long task carries the start time of its whole task, so appending the user
message in the same task that assigned measureFrom stamped runtime startup
and the first publish as earlier than the window and dropped them as page
load. Hand the append to a later task so the work that begins the stream
sorts inside it.
longestStallMs was only ever written when a later paint closed the stall, so
a freeze that ran to the end of the stream was never recorded: the tail can
go missing inside the 90% floor and the quiet-frame loop then calls it
settled. Measure the stall in progress while text is still arriving, which is
what the number means, and not afterwards, where the settle window's own
quiet frames would read as a freeze.
Clean runs unchanged (stall 933 to 1,050ms, long tasks 4,749 to 5,159ms over
three) and both mutations still caught: #7892 reverted fails the stall at
5,233ms, #8750 reverted fails long tasks at 52,263ms.
* Studio: tighten the comments added since the first pass
Comments only, no code change. Every measured number and every causal
reason is kept.
* Studio: record a freeze that spans the end of the stream
The stall in progress was measured only while text was still arriving. A
freeze that spans the moment the stream ends blocks the frame loop across
it, so the first frame afterwards already observes a non-null
streamEndedAtMs and the whole frozen interval was skipped. With the lost
tail able to hide inside the 90% workload floor, thirty quiet frames then
settled the reply and the run reported a short longest stall, which is the
one shape this number exists to catch.
Cap the interval at the absolute stream-end timestamp instead. A freeze
across that moment is recorded in full, and the stall stops growing once
there is no more text to wait for, so the settle check's own quiet frames
are still not counted as a freeze.
The rule moves into smoke-stream-pacing-stall.ts so it can be tested
without importing the harness entry, which mounts React on import. The new
tests cover the spanning freeze, the settle-window bound, idempotence and
late tail paint; restoring the previous rule fails two of the five.
Clean runs unchanged (stall 967 to 983ms, long tasks 5,442 to 5,842ms) and
both mutations still caught: #7892 reverted fails the stall at 5,017ms,
#8750 reverted fails long tasks at 63,687ms.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Make the stream pacing smoke page load again
Merging main brought in ASSISTANT_PART_COMPONENTS, which thread.tsx builds
at module scope with Text: MarkdownText. Entering the markdown-text ->
features/chat -> chat-page -> thread cycle from markdown-text runs that
object literal while the MarkdownText binding is still in its temporal dead
zone, so the page died with Cannot access MarkdownText before initialization
and rendered nothing. Import the chat barrel first, as the app's entry does.
The page is also a new HTML entry, so it has to load the crypto polyfill
before its module entry like every other one.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>
|
||
|
Maheswar Kumar
|
ea687ef520
|
studio: add settings-managed LAN access (#8951)
* studio: add settings-managed LAN access Studio binds 127.0.0.1, so reaching it from a phone on the same Wi-Fi meant relaunching with -H 0.0.0.0, which the desktop app cannot do at all. Settings > API keys > LAN access now adds a second uvicorn listener over the running app, bound to each detected non-loopback IPv4 at the same port, on the primary server's event loop with lifespan="off". The loopback socket is untouched, so the desktop app keeps working while it is on. lan_access.py owns the listener: address detection, binding, and a teardown that waits on the sockets rather than on serve(), since uvicorn closes the passed sockets at the top of shutdown and only then drains in-flight responses. Waiting on the serve task made a Stop pressed from a LAN device wait out its own response for the full timeout. utils/lan_access_settings.py holds the launch policy and the persisted lan_access_auto_start preference. Start is blocked on Colab, on a --secure launch (which forces the loopback bind so the raw port is never exposed), on a launch that already binds the network, and until the seeded admin password has been changed. main.py's desktop SPA gate now opens for requests arriving on a LAN listener socket, identified by scope["server"] rather than any client header, so a desktop api-only backend serves its packaged web UI over LAN without changing the local api-only surface. host_policy tracks the LAN listener alongside the Cloudflare tunnel, so turning it on suspends the loopback stdio-MCP default. The startup banner reports the bound addresses when a persisted auto-start has already brought the listener up, instead of claiming loopback only. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: keep LAN listener ownership until the port is confirmed closed A stop that timed out dropped every reference to the server and sockets before returning False, so the wait could never be retried and a second stop found _server None, reported success, and cleared both the stop_timed_out error and the beyond-loopback trust flag while the port could still be accepting. The references are now retained on that path, so the status keeps offering Stop and a retry waits on the same sockets. start_lan_access also marks the LAN connector active before the listener can accept, rolling back if startup fails. A request served between the socket accepting and the flag being set would otherwise read the loopback-only stdio MCP default in core/inference/mcp_client.py. * studio: keep the LAN trust flag under the listener lock start_lan_access and stop_lan_access are sync routes, so FastAPI runs them in separate worker threads. Publishing the beyond-loopback flag from those callers left it outside lan_access._lock: a stop that observed no server could set the flag false between a concurrent start setting it true and that start acquiring the lock, leaving a live LAN listener while remote_connector_active() read false and core/inference/mcp_client.py still allowed the loopback-default stdio MCP transport. The flag now changes only where the listener state does, under the same lock: start_lan_listener sets it before the socket can accept and rolls it back if startup fails, and _release_listener_state clears it with the references. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: enumerate interfaces for LAN access and hold ownership through loop shutdown detect_lan_addresses only had the route to 8.8.8.8 and a hostname lookup. The probe yields one route-selected source address and hostname resolution is not an enumeration, so a multihomed host lost its second adapter and an isolated LAN with no default route reported no_lan_address while having a usable address, the Linux name-to-127.0.1.1 mapping making it worse. It now enumerates IPv4 addresses on every interface that is up via psutil, keeping the default-route address first so it stays the shown URL, and falls back to the hostname lookup only when psutil is unavailable. stop_lan_listener no longer releases ownership on the event-loop branch. /api/shutdown reaches it from a task on the serving loop, and uvicorn cannot close the sockets until that loop is free, which _graceful_shutdown then holds for seconds while it stops the inference, export and training subprocesses. Only _bound_addresses is dropped there, so the frontend gate closes at once while the sockets and the beyond-loopback trust flag stay owned until they really close. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: hold LAN trust until the stopped listener's requests drain Closing the listening sockets stops new connections, but uvicorn then drains the ones it already accepted. Clearing the beyond-loopback flag at that point let a LAN request still executing reach stdio_mcp_enabled() in core/inference/mcp_client.py and be treated as loopback-only, so a remote caller could spawn stdio MCP subprocesses under the local default. Stopping LAN access while a long inference from a phone is in flight is an ordinary way to hit it. The confirmed-close path now releases the listener references without the flag and hands it to a drain watcher, which clears it once server_state.connections empties. The watcher leaves the flag active if the connections never drain, and skips the clear when a new listener has since taken ownership. The Stop response itself still returns as soon as the sockets close. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: derive the LAN trust flag from listener and drain ownership A second stop arriving after the first closed the sockets but while accepted requests were still draining found _server already None and released the flag outright, so those still-remote requests passed the loopback-default stdio MCP gate. The idempotent stop path bypassed the drain watcher entirely. The flag is no longer assigned by any path. _sync_lan_trust publishes it from the authoritative state, a live listener or any stopped listener still draining, counted by _pending_drains and taken under the same lock as the references. A watcher whose connections never drain keeps its count rather than releasing, so the flag fails closed. start_lan_listener still raises it explicitly before the socket can accept. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: release bound sockets when the listener cannot be scheduled asyncio.run_coroutine_threadsafe raises RuntimeError("Event loop is closed") if the primary loop closes between _server_loop validating it and the schedule call. That escaped past the failure cleanup with every socket already bound and the trust flag already raised, so the port stayed listening with nothing serving it, status reported no listener, later starts hit bind_failed, and the flag stayed on until the process exited. The cleanup is now shared by both start failure paths in _fail_start, which closes the sockets, resyncs the trust flag from ownership and records listener_start_failed. The unscheduled coroutine is closed so the failure does not also emit a never-awaited warning. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: unblock a loop-side LAN stop and release the listener when its loop ends start_lan_listener holds _lock while waiting for the serving loop to run serve(). /api/shutdown reaches stop_lan_listener from a task on that same loop, so blocking on the lock left the two waiting each other out: the loop could not advance the scheduled listener, the start burned its full timeout and shutdown stalled with it. A stop running on the serving loop now takes the lock without blocking and reports an unconfirmed stop instead. The event-loop branch keeps ownership so uvicorn can close the sockets once the loop is free, but nothing released it afterwards. An embedded host that calls run_server again in-process therefore saw a stale _server, reported the previous addresses as online and never bound a new listener. run.py's server thread now releases the listener in the same finally that closes the loop, which reaches the loop-gone path and drops the sockets, references and trust flag. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: advertise only reachable LAN origins --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com> |
||
Long Yixing
|
31c42e872b
|
Restore crypto.randomUUID before the Studio bundle evaluates (#9075) | ||
|
Nilay
|
ea958c7f34
|
Studio: let the hub run non-GGUF image and video models (#8855) | ||
|
Vineeth Sai Varikuntla
|
a4fa451214
|
Derive warmup steps from a model default that gives a ratio (#8723)
Some checks are pending
Unsloth GGUF CI / Tool calling Tests (push) Waiting to run
Unsloth GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Unsloth GGUF CI / JSON, images (push) Waiting to run
Unsloth load-orchestrator CI / test (push) Waiting to run
Mac Studio GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15-intel) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26-intel) (push) Waiting to run
Mac Studio UI + API + Update CI / Chat UI, API and Update Tests (push) Waiting to run
Unsloth Tauri CI / Tauri Linux debug build (no codesign) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (windows) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (macos) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (push) Waiting to run
Unsloth UI CI / Loaded-models indicator (cross-browser) (push) Waiting to run
Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Windows Unsloth API CI / Unsloth API & Auth Tests (push) Waiting to run
Windows Unsloth GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Windows Unsloth GGUF CI / Unsloth install + inference without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / GPU prebuilt resolves without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / setup.ps1 unit tests (VS 2026 / CMake guard) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2022) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-2025-vs2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-latest) (push) Waiting to run
Windows Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Wheel CI / Wheel build + content sanity + import smoke (push) Waiting to run
Windows Application Control CI / installer survives a denied unsloth.exe (push) Waiting to run
Workflow trigger lint / workflow-trigger lint (pull_request_target / cache-poisoning) (push) Waiting to run
--------- Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> |
||
|
Vineeth Sai Varikuntla
|
4b18f07226
|
Pair the diffusion warmup presets with a scheduler that uses them (#8593)
--------- Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> |
||
|
Daniel Han
|
5a1655ca1b
|
Studio: run the frontend unit tests on Windows in CI (#9099)
* Run the frontend unit tests on Windows in CI, and fix the one test that cannot pass there Frontend CI is ubuntu-only, so a test that is correct on POSIX and wrong on Windows stays green forever. Thirteen such failures accumulated across three files before anyone ran the suite on a Windows runner. Add a windows-latest job that does checkout, node, npm ci and npm test, and nothing else: typecheck, build and the browser smokes cannot fail there for a reason they would not also fail on ubuntu, and repeating them would roughly triple the workflow. background-load-notice's stall-window test cannot pass on a Windows timer, and was measuring nothing on Linux either. Replace the millisecond budget with a comparison against the same loop with no healthy read. * Audit the lockfile before the Windows install, and run no install script Review: jobs run concurrently, so the ubuntu job rejecting a lockfile does not help a Windows runner that has already installed it, and npm 10 ignores the npm-11-only allowScripts policy. Run the same pre-install structural scan here, and install with --ignore-scripts so no package script runs on this runner at all. |
||
|
Daniel Han
|
677d93ae31
|
Studio: fix the Windows-only path failures in the frontend test suite, and catch the shape from Linux (#8980)
* Fix two module resolution failures in the frontend test suite
* Fix the third Windows-only path failure in the frontend test suite
* Fix the third Windows-only path failure, and catch the shape from Linux
microphone-permission-reset.test.ts builds its dynamic import specifier with
fileURLToPath, the same mistake copy-to-clipboard.test.ts made. On Windows that
is a "D:\\..." path, which node's ESM loader rejects, and the three tests that
load the module fail.
The suite now also asserts both shapes are absent, from any platform, so
Frontend CI on ubuntu can see a Windows-only path defect being reintroduced.
* Carry the portability taint on bindings, and scan every path argument
Review: a file-wide set of names let 'const path = url.pathname' in one scope
reject an unrelated 'const path = new URL(...)' read in another, and scanning
only arguments[0] missed copyFile(source, destination.pathname).
Resolve identifiers through their enclosing scopes instead, and map each fs
entry point to how many of its leading arguments are a path. Both are covered
by the file's own detector cases: reverting either fails them.
* Close three more gaps in the portability rules
Review: a value built in steps (let path = ""; path = url.pathname) was
invisible, an fs entry point imported under an alias was not recognised as one,
and the entry point list stopped at the read/write core.
Track assignment as well as initialization, resolve a callee through its import
binding so the module's own name is what is matched, and extend the list to the
stream, link, glob and metadata entry points. Each has its own detector case;
reverting any one of the three fails them.
* Match the portability rules on the module, not on the spelling
Review: an aliased fileURLToPath did not seed the native-path taint, and an
unrelated method sharing a name with an fs entry point (router.open, dom.link)
was classified as one.
A call is now resolved to its import binding and to the module that binding came
from, so node:url and node:fs decide both rules and a local spelling decides
neither. Namespace imports are resolved too. Three more detector cases cover it:
aliased fileURLToPath, fs.readFileSync through a namespace import, and a
router.open on a URL pathname that must stay quiet.
* Carry the taint across helpers and destructuring, and stop it at pathToFileURL
Review, three items on the portability rules.
A pathname crossing a local helper boundary was invisible: the argument is
tainted, the parameter it lands on was not. Taint now flows from a call argument
to the parameter of a locally declared function or arrow, which also covers the
same shape around dynamic import().
Destructuring introduced bindings the declaration table discarded, so
'const { pathname } = url' followed by readFile(pathname) passed. Binding
elements are now recorded individually, and an object binding element naming
pathname seeds the taint the way a member access does, so only that element is
tainted and its siblings stay clean.
The opposite risk, a false positive that would get the rule weakened: taint now
stops at pathToFileURL, which is the exact inverse of fileURLToPath, so the
standard round trip for a module location that starts life as a path is accepted.
Deliberately not a barrier for the pathname rule, where a Windows pathname is
'/D:/...' and converting it produces a URL for a file that does not exist.
Each has its own case, and the sanitizer has an inverse one asserting the
unrepaired forms still fire.
|
||
|
Daniel Han
|
2d8e0a00f7
|
Studio: strip the trailing template placeholder once, on the finished reply (#9101)
Fixes #9098. The trailing `${...}` strip ran on every SSE arrival and assigned its result back, so "ends with ${...}" was tested against every prefix of the reply rather than the reply. The one arrival whose buffer ended at a complete fragment was cut, and the reassignment made the cut permanent: in return `Hi, ${name}!` 21 chars out return `Hi,!` 13 chars The strip now runs once, after the stream has finished. The fragment it was added for in #4706 is still removed, because that one really is at the end of a completed answer. Rebased onto #9049. The watch it added still gates the scan, and now saves the whole reply from being flattened rather than one arrival's worth, so nothing on the arrival path can flatten the buffer at all. Abort keeps the buffer whole: that tail is a prefix again, so stripping it would be the same bug. `producedReplyText` is the same case one step in, for a continuation that finishes without a text or reasoning delta and so holds nothing but the partial it was seeded with. |
||
|
Daniel Han
|
4f0d691cfc
|
Studio: stop the streamed reply being flattened on every arrival (#9049)
* Studio: stop the streamed reply being flattened on every arrival
PR 9012 removed two whole-buffer scans from the chat stream adapter and
left about 16 ms per side behind. That remainder is not a scan. Every
`cumulativeText += delta` leaves a cons string, and the first thing that
reads it copies the whole reply flat, so a single `charCodeAt` costs the
same as a full scan. Measured over a 220,000 character reply of 55,005
arrivals, medians of 5 to 7 repetitions, paired and interleaved:
append only, nothing reads the buffer 0.84 ms
one bounded scan of the tail per arrival 1715.98 ms
Three steps ran per arrival and all three read the buffer, so all three
paid that. Each now takes what the arrival added instead.
The think-tag tracker takes the delta rather than slicing it back out of
the buffer, and keeps the seven characters in front of it itself, which
is the most a tag split across arrivals can hide behind. 1743.92 ms to
4.19 ms at 220K.
The trailing `${...}` strip cannot avoid touching the end of the reply
when it fires, so a watch decides whether it could fire at all, from the
deltas: the last non-whitespace character, the last `${`, and the two
most recent `}`. It never says no when the strip would cut, so nothing
that used to be stripped survives, and a reply that never ends in a
brace never wakes the strip. 1715.98 ms to 6.74 ms at 220K.
`parseAssistantContent` over the whole reply, which `liveAssistantContent`
runs on every arrival, becomes an incremental parse that keeps the parts
it has already produced and extends them with the delta. It holds back
the trailing characters that could still turn out to be a tag, so `<thi`
is text until `nk>` arrives and reasoning after, matching a full reparse
at every state. Runs are cut at the tool-call cursors exactly as before,
including that think state resets at a boundary. 1978.78 ms to 7.37 ms
at 220K.
The retained state describes an append-only reply. A rewritten prefix, a
removed suffix or a tool call landing behind the end shows up as a length
or boundary mismatch and reparses from the buffer, which is what this
replaces, so those paths are no slower than before. An external
continuation whose prefix `joinContinuation` may repair never uses the
incremental path at all.
Whole per-arrival path, before to after, medians of 5:
55,000 chars 13,755 arrivals 236.89 ms -> 4.06 ms 58x
110,000 chars 27,505 arrivals 795.09 ms -> 5.73 ms 139x
220,000 chars 55,005 arrivals 4882.98 ms -> 13.06 ms 374x
400,000 chars 100,005 arrivals 17060.48 ms -> 21.57 ms 791x
The factor grows with the reply because the old cost was quadratic and
steepens again past 131,072 characters, where a flattened string stops
fitting in a regular heap object.
Tests. A differential fuzz test drives random arrival streams, with tags
split at every offset, tool boundaries, unclosed tags, empty arrivals and
truncations, and requires the incremental parse to deep-equal a full
reparse after every single arrival. The think tracker and the placeholder
watch keep their existing suites, driven through the new delta API, and
the watch gains a soundness sweep: for every state of every stream, if
the strip would cut, the watch admitted it.
Two source pins from 9012 are repointed rather than dropped. The one that
asserted the exact spelling of the tracker call now asserts what that
spelling protected: the reply grows through one call, which is what keeps
all three delta-fed pieces in step. A new pin lists the forms in which
the loop may mention the buffer at all, because character counting cannot
see this defect: a `charCodeAt` and a full scan read the same characters
and cost the same, so only a rule about touching the buffer catches it.
* Put the placeholder-watch gate under a buffer longer than its reseed window
---------
Co-authored-by: danielhanchen <unslothshared@gmail.com>
|
||
|
Daniel Han
|
bf60ff8b23
|
Studio: stop the streaming prefix checks scanning the whole reply (#9038)
* Studio: stop the streaming prefix checks scanning the whole reply
Four checks on the streaming path ask whether the new text still begins
with what was already handled. All four used startsWith, which scans.
Slicing to the prefix length and comparing lets V8 reject on length and
then compare natively.
Measured over a 60,000 character stream, 1,052 comparisons, median of 5
interleaved runs:
strings sharing a parent 79.8 ms -> 1.1 ms
strings not sharing one 74.3 ms -> 1.6 ms
Both cases are given because a shared-parent microbenchmark can
exaggerate this; here it does not, the effect survives flattening.
Call sites: three in streaming-render-schedule.ts, one of them added by
the retained-prefix work, and the coalescer in markdown-text.tsx whose
own comment already put its scan at 59 ms across a 175,000 character
stream.
The substitution is exact. slice clamps, so a prefix longer than the
string yields a short slice that cannot equal it.
markdown-streaming-scheduling.test.ts asserted the literal spelling
"text.startsWith(displayed.text)", so it failed on a change that keeps
the behaviour identical. It now asserts both halves of the gate and
accepts either spelling: the length rejection has to be there, since the
compare alone would run on every arrival, and the prefix compare has to
be there, since the length check alone would pass a comparison against
the wrong string.
Testing: an equivalence fuzz, 20,000 randomised cases plus a fixed
corpus and every cut of astral text, asserting agreement with startsWith
throughout. It requires at least 4,000 of the cases to be real prefixes,
so it cannot pass by rejecting everything on length.
Mutation results, stated per mutation:
off-by-one on the slice caught, 3 of 3 fail
comparison in the wrong order caught, 3 of 3 fail
length guard removed NOT caught, and cannot be: slice clamps,
so that mutation is semantically
identical and only loses the fast path
npm test 2,946 passed, 0 failed. typecheck clean. biome adds no errors.
* Cover case in the prefix corpus and correct what the measurement showed
* Make the streaming prefix tests fail when the change is reverted
The scheduling test accepted either spelling of the coalescer gate, and the
previous code satisfies the startsWith half, so it passed unchanged on a tree
with this PR fully backed out. Pin the slice form and reject the scanning one;
verified by reverting markdown-text.tsx to the old spelling, which now fails
where it passed 5 of 5 before.
Nothing covered the three call sites in the cache at all. The two spellings
return the same answer, so no output test can separate them and reverting the
call sites was free. Assert instead that no one-argument startsWith survives on
that path; the one call the helper cannot express takes a start position and
compares a fixed block, so it does not grow with the reply and is excluded by
the same rule rather than by an exception. Verified by reverting two call sites,
which now fails.
Drop the duplicated measurement at the coalescer and point at the helper.
---------
Co-authored-by: danielhanchen <unslothshared@gmail.com>
|
||
|
Daniel Han
|
31d1524b09
|
Studio: sweep the streaming markdown corpus at every prefix (#9085)
* Sweep the streaming markdown corpus at every prefix The retained block list has to equal a whole-document Streamdown split at every prefix of a reply. That property is what the incremental cache rests on, and hand-picked fixtures do not find its counterexamples: the one that reached main needed a display span whose body spans blank lines, so the newline the rewrite inserts merges with the blank line in front of the opener. Sweep a corpus instead. One named case per construct, plus 120 generated replies in a loose and a tight form, all at every prefix, with a fixed seed so a failure reproduces from the message. Adds a cost invariant too: rebuilds of the retained prefix must not grow with the reply, since each one costs the whole reply so far. Runs in 1.8s. Checked against three deliberately broken trees: dropping the rollback window between a rewound boundary and the rewrite fails both sweeps, and disabling the rewind or retention entirely fails the growth invariant. * Make the corpus sweep reach the retained prefix, and fail on a rename Two ways the first version measured less than it claimed. A construct on its own is shorter than the rollback window, so candidateCount is zero and nothing is ever committed: 37 of the 39 named cases never retained a single block, and for those the sweep only asserted that repairTail agrees with remend on a fresh context. Sweep each case a second time behind a lead-in, and assert that the retention actually happened, so this cannot regress quietly. The growth assertion read the same private counter on both sides, so renaming the field made it undefined === undefined and the file passed while measuring nothing. Verified: renaming retainedPrefixRebuilds left all four green. Detection after, per injected defect: rollback window removed 2 fail, rewind disabled 1, never commits 2 (was 1), counter renamed 1 (was 0). The generated count drops from 120 to 40, which catches the same defects, and the file runs in 1.0s rather than 1.8s despite covering more. |