mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-20 14:23:55 +00:00
* fix(studio): refresh token count after model load and thread switch (#7450) Add a chat count_tokens API and call it from the frontend when a local GGUF model finishes loading or when a thread is opened, so the context usage bar reflects the current conversation without waiting for another message. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio): address Codex review on token count refresh (#7450) Defer post-load recount until modelLoading clears, restore saved usage while recount runs, walk the active parentId branch, read incognito messages from the live thread, guard on activeThreadId, and include local tool/RAG options in count_tokens so the bar matches chat sends. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add Playwright E2E for token bar after model switch (#7450) - Add @playwright/test harness with auth injection and model-picker helpers - Cover token bar visibility on load and after unload/reload without resending - Fix isInferenceLoaded to treat empty loaded[] as unloaded (Boolean([]) bug) - Restore saved context usage before background recount in refreshContextUsage - Add data-testid hooks on context bar and model selector * fix(ci): pin fsevents override and refresh fastapi scan baseline - Override fsevents to 2.3.3 so @playwright/test does not introduce a new install-script dependency (fsevents@2.3.2) blocked by security-audit CI - Update scan_packages_baseline for fastapi routing.py line shift in resolved studio deps (C2 polling false positive) * fix(ci): unblock Repo tests after main merge Replace raw px text utilities in agents-tab with scaled text-ui tokens so test_ui_font_scale_contract passes. Stub bitsandbytes in the ROCm routing test child process so unsloth_zoo can import under the HIP torch spoof on CPU-only CI. * Fix branch selection in the context-usage recount for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Recount the displayed branch and make the E2E unload real for PR #7453 Post-model-load recounts now follow the branch the runtime is showing, so stepping back to an earlier retry sibling and switching models no longer counts the newest stored sibling instead. The history adapter's own recount still mirrors the import head reset. The E2E unload helper sent an empty body, which UnloadRequest rejects with a 422, so no model was ever unloaded and the model-switch spec never exercised a switch. It now resolves the loaded model from /api/inference/status and asserts the unload succeeded. ensureLocalModelLoaded also stopped waiting out the 180s count_tokens response timeout on a brand-new chat, which never issues that request. * Refuse token recounts for image threads in PR #7453 llama-server's /apply-template replaces every image_url part with a short media marker and discards the decoded bytes, so tokenizing the rendered prompt misses the whole image embedding and the context bar would claim an image-heavy conversation still fits. Refuse the recount for those payloads instead, which leaves the caller on the usage it already had, and refuse before the auto-switch so a count never moves the loaded model. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep recounts off the model-switch path and fix the E2E token gate for PR #7453 The recount has no abort signal, so a count still naming the model that was loaded when it started could reach the backend after the user picked another one and, with OpenAI auto-switch on, drag the loaded model back. Drop the auto-switch from /chat/count_tokens: model there is informational and the active model is what gets counted. Scope the saved-usage fallback to the branch being counted, so a thread with a regeneration no longer leaves a sibling branch's usage on the bar when the recount does not land. issue-tokens.py minted a plain token, but ensure_default_admin seeds the account with must_change_password and every protected route answers 403 for a token without the desktop claim, so the Playwright suite could not reach the backend at all on a fresh install. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Count the Canvas instruction and close the recount usage-snapshot window for PR #7453 Canvas appends a system instruction to the real request, and the recount already declares the matching render_html tool, so it has to count that instruction too or every Canvas conversation reads short after a model load. The instruction text and the system-message folding now live in one helper the generation path and the recount share. The stale-recount guard sampled contextUsage after the outbound payload and tool-extras builds, both of which await storage and the project/RAG lookups. A completion landing in that window was captured by the snapshot itself, so the equality check passed and the stale count overwrote its exact usage. Sample as soon as the message list is fixed instead. * Decline recounts for audio turns and fix the second E2E token minter for PR #7453 toOpenAIMessages has no audio branch, so a stored user turn whose attachment the real request replays as audio_base64 would be counted as a text-only prompt and overwrite the bar with an undercount. Decline the recount there, matching what the endpoint already does for images. e2e_count_tokens_api.py minted a plain access token like the Playwright issuer did, so on a fresh database the bootstrap password gate answered 403 for every protected call the script makes. * Count the configured prompt in a brand-new chat for PR #7453 A chat with no persisted thread short-circuited to zero, but a configured system prompt, project instructions or enabled tools are already part of the next request, so the bar read 0 / context until the first completion. Build and count that payload instead, keeping the immediate zero as the placeholder so a failed count still leaves a number on the bar, and fall back to zero only when the payload really is empty. The completion guard now compares the captured thread against activeThreadId even when it is null, so a count started on an unsaved chat cannot land on a thread the user opened while it was in flight. * Count tool-only prompts for empty chats in PR #7453 An empty transcript with Search, Code, Canvas, Docs or MCP enabled skipped the count endpoint on the empty message list and wrote zero, even though the tool schemas and the backend's tool/RAG nudge are prompt tokens the next request pays for. Count whenever the tool extras contribute; buildLocalTokenCountExtras returns an empty object when nothing is enabled, which is the only genuinely zero case. * Clean tool history before counting and scope the runtime getter for PR #7453 The GGUF and Anthropic tool paths strip stale tool-call markup out of replayed assistant turns before rendering the prompt; the count path did not, so it priced text the next completion removes. Apply the same gated strip there. The runtime message getter is one module-level slot that every provider writes, so in compare mode an unmounting pane cleared the survivor's registration. Registration now hands back an owner-scoped disposer, and the incognito read only trusts the slot for the active thread. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Match the completion prompt shape in token recounts for PR #7453 Four alignments between the recount and the request it is predicting: Auto-Heal off leaves leaked tool markup in the real prompt, so the count now takes auto_heal_tool_calls from the client instead of forcing the strip on. System and developer turns are collapsed into one leading system message, the same normalization the completion path applies, so a runtime system turn plus the configured prompt renders identically. The endpoint is called even for an otherwise empty payload: the template has tokens of its own, and a process-level policy (unsloth run --enable-tools) can inject tool schemas the client cannot see. The runtime message getter is a stack rather than a slot, so unmounting one compare pane uncovers the provider underneath instead of leaving none. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard the token recount against retrieval and tokenizer drift for PR #7453 Decline the recount for a pending turn that the next generation would answer with retrieved documents: with search_knowledge_base selected and a thread ending in an unanswered user turn (or a tool result an interrupted loop never answered), the tool loop splices in whatever build_rag_autoinject retrieves before the first render, which the count never sees. build_rag_autoinject's entry gate moves into rag_autoinject_permitted so the decline cannot drift from the retrieval it stands in for. Return the tokenizer identity with the count. The endpoint counts against whatever is loaded now, so another API client's auto-switch is invisible to a caller that only checks its own captured checkpoint. _llama_status_model_ids now derives the (active_model, model_identifier) pair once for both /status and this endpoint, the count reports the same string the client holds as params.checkpoint, and a switch landing mid-count returns 503 rather than a total belonging to neither model. Recount when the initial status refresh first discovers a loaded local GGUF: thread history can load before that refresh has a checkpoint or a window, and nothing reloaded history afterwards, so the bar stayed blank until the thread was reopened. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Search every mounted runtime for the counted thread in PR #7453 The post-load recount asked only the topmost entry of the runtime getter stack. Compare mode keeps both panes mounted, so when the pane showing the counted thread is not the newest mount, the tail id check fails against that thread's records and the count falls back to the branch rebuilt from storage. After the user steps back through a retry that is the sibling nobody is looking at, so the bar shows a count for the wrong branch of the right thread. Walk the stack newest first and take the first getter whose tail is one of these records. Message ids are unique, so that getter is the pane displaying them; a stack with no match still falls back to storage exactly as before. A getter that throws while its provider tears down is now skipped instead of aborting the whole recount. * Scope recount cancellation to one thread in PR #7453 The generation counter that supersedes an in-flight recount was module wide, but every mounted provider recounts its own thread when its history loads. In compare mode a hidden pane's load therefore cancelled the visible thread's recount, and the pane's own result was then dropped by the activeThreadId guard, so the context bar stayed blank. Key the counter by thread id. Two calls for the same thread still supersede each other, which is what the counter is for; calls for different threads no longer see each other. A call that finishes after the user has switched away is still dropped by the unchanged activeThreadId guard. * Tighten the token-count comments for PR #7453 * Compress the token-count docstrings for PR #7453 * Decline strict token counts when the chat template will not render for PR #7453 * Compress the strict token-count comments for PR #7453 * Tighten the token-count comments for PR #7453 * Count the request the completion actually sends for PR #7453 Decide the route before applying the tool policy. With `unsloth run --enable-tools` set and a completed tool call still in history, the counter selected every built-in tool schema and appended the tool nudge, while the completion sent the same request to llama-server verbatim and sent neither: the policy turns on `_effective_enable_tools`, but the passthrough turns on an explicit per-request `enable_tools` or `mcp_enabled`, which that shape does not carry. The bar therefore priced a prompt larger than the one being generated from. The rule now lives in one helper that both the counter and the completion call, so the two cannot drift apart silently. It reads `tool_choice` and `response_format` defensively because the count endpoint takes a different request model that carries neither. Decline a runtime message getter whose owner is unknown. Registration records no thread, and compare mode keeps several providers mounted, so the newest getter can belong to a sibling pane while the caller's guard only checks the thread it asked for. Use it when it is the only one mounted, which is the single-pane case, and otherwise price the template alone rather than report another pane's total as this thread's. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Route the recount before merging user turns for PR #7453 The counter coalesced adjacent user turns unconditionally, but _openai_messages_for_passthrough does not merge them: it only drops the empty assistant sentinel. So on the passthrough route the count rendered a prompt the request never sends, and could come back clean where the real strict template rejects the unmerged history. Two user turns around an empty sentinel is the shape a thread has after a response is stopped before its first token. Coalescing is a normal-GGUF-path step, so it now happens only once the route is known, using the same helper the completion asks. The test pins the passthrough builder as the reference and asserts the route is decided before the merge. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added while reviewing Comments only, no code change, verified with comment_tools check. The call-site note in the count endpoint now points at _takes_tool_passthrough rather than restating its docstring, and the getter, tool-result and partial-scan notes lose a line each without losing the reason they exist. * Studio: stop pricing a withdrawn tool catalog in the token count tool_choice "none" with no tool history takes the request off the passthrough, and the GGUF path it lands on calls generate_chat_completion, which has no tools argument. The counter still handed payload.tools to /apply-template, and llama-server puts whatever tools reach that endpoint straight into the Jinja context regardless of tool_choice, so the bar reported schemas the completion never sends. Move the rule _build_openai_passthrough_body already applied into _passthrough_client_tools and read it from both, so the count and the body cannot drift, and clear the catalog outright on every route that renders tools from the selection instead. * Tests: stop shadowing the real structlog in the load-freeze suite sys.modules.setdefault("structlog", ModuleType("structlog")) ran before anything imported the package, so the empty placeholder won and stayed for the whole session. Any later file importing a studio module that calls structlog.get_logger at module scope then hit AttributeError, and only when this file was collected first - the new token-count parity tests passed alone and failed under pytest tests/studio, and the MLX hardware-dispatch cases failed the same way through utils.mlx_repair. Only stub when structlog is genuinely missing, and give that stub the get_logger those callers use. * Revert the scan-packages baseline edit The hunk rewrote the fastapi 0.139.2 entry into a byte-for-byte copy of the 0.140.0 entry sitting next to it, so it dropped a reviewed allowlist row and left a duplicate key. It also does not silence anything: CI resolves fastapi 0.140.6, whose evidence digest is neither of those. The three scan-packages shards fail on main for the same three findings and are not this branch's to fix. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Retract the seeded token count when the recount declines refreshContextUsage seeds zeroed usage so a chat with no persisted thread has something to show, then several paths return without counting: staged audio, a checkpoint that moved under it, and any failure of the count itself. The seed stayed on screen, so a recount that never landed rendered a confident "0 / <window>" where the bar previously just stayed hidden. Hold the seed by identity and clear it on those exits, but only while it is still the value on screen: anything else means a later count or a completion already owns the bar, and clearing there would roll back real usage. The load-without-sending e2e assertion had no minUsed, so the placeholder satisfied it on its own and a recount that never landed looked like a pass. * Recount the empty prompt when a New Chat opens against a resident model ThreadNewChatSwitch blanks the bar with setActiveThreadId(null), and the /chat?new=<uuid> view reaches none of the other recount triggers: no persisted thread for the history loader, and ActiveThreadSync is off while a nonce is present. Loading a model on that view showed a bar, arriving on it with the same model already loaded showed nothing until the first completion. Ask for the count after the switch. The template and system prompt are already in the request, and a fresh chat in a project resolves through the activeProjectId branch, so its instructions are priced too. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Count with the selected reasoning template settings for PR #7453 llama-server merges the load-time --chat-template-kwargs under whatever a request omits, so /apply-template called with only messages and tools rendered the mode the model was LAUNCHED in while the completion rendered the user's. A Preserve Thinking pill flipped on keeps every past turn's think block in the real prompt but not in the count, so the context bar under-reported the prompt it was about to send. The count now forwards enable_thinking / reasoning_effort / preserve_thinking and reuses _request_reasoning_kwargs, the same helper the completion paths build their kwargs with. * Probe structlog with find_spec instead of a bare import The availability check imported the module purely for its side effect, so the import-hoist verifier flagged it as an added-but-unused import and failed Source lint. find_spec answers the same question without binding a name. * Recount a reloaded New Chat once inference status hydrates for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Leave an existing structlog entry alone in the availability probe find_spec raises ValueError on a module already in sys.modules whose __spec__ is None, which is what a bare types.ModuleType stub is. Check sys.modules first so anything already present, real or stubbed, is left untouched and only a genuinely absent package gets stubbed. * Consolidate the token-count tests without changing behaviour for PR #7453 The three node-harness files each rebuilt the same mkdtemp plumbing, so it now lives in tests/studio/_node_harness.py. The reload-hydration cases move into test_new_chat_context_recount.py beside the New Chat cases they share a component with, and the reasoning-mode cases collapse into a parametrize. No behaviour changes. Each merged case was re-checked against the source revert that motivated it: the hydration retry still reddens at 0 == 1, the original recount at 0 == 1, the backend kwargs at 28 == 48 and 28 == 29, and the client payload at None is False. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Unload only once the chats stop generating, and honour the E2E backend port for PR #7453 * Trim the token recount backend to what the usage bar needs for PR #7453 * Trim the frontend recount to the branch it counts for PR #7453 * Fold the New Chat recount cases into the branch each one pins for PR #7453 * Compact the count endpoint tests and drop the fixtures they no longer read * Fold the reload-hydration case into the New Chat pricing test for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Recount the context bar when a pick adopts the already-resident GGUF for PR #7453 * Price the Canvas prompt instruction in the token recount for PR #7453 * Fold the Canvas recount cases into one parametrised test for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price the branch the mounted runtime shows in the token recount for PR #7453 * Reprice a thread that becomes active with a blank context bar for PR #7453 * Count the prompt in the reasoning mode the next completion asks for in PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price a New Chat as its own empty prompt, not the conversation it was opened from, for PR #7453 ThreadNewChatSwitch voids switchToNewThread() and writes setActiveThreadId(null) in the same effect, and the recount effect beside it runs in the same flush. The outgoing conversation is deliberately left running, so its runtime is still mounted and the live branch reader still returns its messages; the captured thread and the store's active thread were then both null, so the equality check matched and the empty chat was priced as the conversation being left behind. Require a non-null captured thread before reading the live branch. A recount with no thread id is an unpersisted New Chat, whose prompt is the bare template, and the stored-records fallback already yields that. Threads that need the live reader all carry an id: ActiveThreadSync writes assistant-ui's mainThreadId for incognito threads too, so the branch this reader was added for is unaffected. * Decline a token recount whose generation would retrieve documents for PR #7453 A thread left on a pending turn -- an unanswered user message, or a tool result an interrupted loop never answered -- is the one shape where the next generation runs on exactly the messages the recount sends, and the tool loop opens that turn by splicing in whatever build_rag_autoinject returns: top-K passages, or a whole thread document under the context budget. The count never saw any of it, so the refreshed bar could report that the pending turn fits when the generation it stands in for would overflow. Decline that shape exactly as the image case above already does, leaving the usage already on the bar rather than under-reporting it. Threads ending on an assistant turn are unaffected: their retrieval runs against a user message that does not exist yet, which no count could predict. A count payload never carries an autoinject override, so a scope arriving here is one the default-on gate retrieves for, and declining a scope it would have skipped only keeps the previous total. * Name the tokenizer that produced a token count for PR #7453 This endpoint deliberately never auto-switches: it counts with whatever is resident and treats the requested model as informational. So a load from another tab or API client, landing after this client's last status refresh but before its recount reaches the backend, returns a total from a tokenizer whose context window the bar is not showing. The frontend's guards compare only its own captured checkpoint, thread and usage object, none of which moved, so it published model B's count against model A's window. Report the identity with the count, in the same shape /api/inference/status publishes it (model_identifier ?? active_model), and drop the result client side when it is not the checkpoint the count was started for. The pair is now derived once in _llama_status_model_ids and used by both the status handler and this endpoint, so the string a client compares cannot drift from the one it was given. A load landing mid-count is refused outright, since that total belongs to neither model. A reply without the field is still accepted, so an older backend keeps working. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price the route the completion takes in token recounts for PR #7453 The token count has to describe the request the completion actually sends. The counter applied the process tool policy without first asking which route the request takes: with `unsloth run --enable-tools` set and a completed tool call still in history, it selected every built-in tool schema and appended the tool nudge, while the completion forwarded the same request to llama-server verbatim and sent neither. Restore the two rules the completion path already follows and have both routes read them from one helper each, so a count can never describe a route the completion does not take: - `_takes_tool_passthrough` decides the route before the process tool policy applies, and before adjacent user turns are coalesced (the passthrough does not merge them, so merging prices a prompt that route never sends). - `_passthrough_client_tools` decides which client tools reach the wire, so a caller's own catalog is priced on the passthrough and a `tool_choice: "none"` withdrawal is not. `ChatCountTokensRequest` carries `tools` again, since that catalog is what the passthrough renders. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bind _native_grant_backed in the loaded-GGUF status branch again Merging main into this branch left GET /api/inference/status raising NameError whenever a GGUF is loaded. The two edits do not touch the same lines, so git merges cleanly and neither side is wrong on its own: - this branch moved the native-path identity logic out of get_status into _llama_status_model_ids, and with it the local _native_grant_backed read; - main (#7385) added is_local_model = _loaded_is_local_model(llama_backend, _native_grant_backed, _model_id) inside that same branch. Read the flag in get_status again. Keeping the helper at its two-value contract: it reports identities, is_local_model wants provenance, and the helper is also consumed by _llama_status_checkpoint_id and the token count route. Also refuse an audio prompt in /chat/count_tokens, for the reason the image guard already exists. ChatCountTokensRequest allows extra fields, so audio_base64 was accepted, dropped, and answered with a total short by the whole clip. Studio declines client side, but the router is mounted at /v1 too, so a direct caller on /v1/chat/count_tokens could be handed that number. Tests: test_inference_status_loaded_gguf.py calls the handler with a loaded stub, which nothing did before, so neither suite could have caught this; it reddens on the NameError itself with the fix reverted. Two cases on test_openai_auto_switch.py for the audio refusal and its negative control. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added for PR #7453 Comment and docstring only. AST of routes/inference.py and test_openai_auto_switch.py is byte identical to the previous commit; the new test module differs in docstrings alone, with an identical code AST. * Do not put a non-finite token count on the context bar The count endpoint's response type is a compile-time assertion; parseJsonOrThrow returns the body unvalidated. Anything other than a matched backend answering 200 on that path (a reverse proxy, a version-skewed fork, an SPA fallback serving JSON) therefore publishes undefined as promptTokens and totalTokens. ContextUsageBar does not absorb that. Its "nothing to show" guard is 'used <= 0', and 'undefined <= 0' is false, so it renders 'undefined / 8.2k' with a NaN width, and formatTokenCountFull calls toLocaleString on it, which throws inside the tooltip render as soon as the bar is hovered. Leave the bar as it was instead, which is what every other failure of the recount already does. Six cases on test_new_chat_context_recount.py: a reply that omits input_tokens, and null, a numeric string, NaN, Infinity and an object in its place. All six publish to the bar with the guard reverted. * Close three prompt-parity gaps in the token recount All three were re-raised by review after ab2cc5835 ("Trim the token recount backend to what the usage bar needs") rewound this area; most of what that trim dropped was restored afterwards, these three were not. Strict counting no longer accepts the text-only fallback. count_chat_tokens refused it only when the prompt had non-text parts, so a text-only history whose /apply-template call timed out, 500'd or was rejected by a strict-alternation template still returned a number: message text plus serialized tools, with no role markers or special tokens, measured earlier at roughly 30 percent under a six-turn two-tool prompt. Both public count endpoints pass strict = True, so /v1/messages/count_tokens is corrected the same way; the three streaming message_start.usage callers are non-strict and keep the best-effort behaviour. The docstring already promised this. The counter now runs the same stale tool-call XML strip over replayed assistant turns that the GGUF tool path runs before rendering, gated on the same enabled tool names so a documented inactive foo[ARGS]{...} survives in both. Without it the count priced markup the completion deletes, which reads as a tool-enabled chat sitting closer to its window than it is. auto_heal_tool_calls is forwarded so the Auto-Heal-off cohort, whose real prompt keeps that markup, is not undercounted instead. A turn sent while a count is in flight now drops the count. The existing guard compares contextUsage by identity, which only catches a completion that wrote usage; a run stopped or failed before emitting any leaves it untouched, so the pre-message total published for a branch that had gained a turn and stayed there, since no dependency moves to retry. Compare the counted branch instead. Declined two other items. A bounded retry after a transient 503 would also re-poll the four deliberate declines (image, audio, no model, pending RAG turn), which are permanent per payload and carry no Retry-After, and its failure mode is publishing a wrong total rather than the blank bar it would fix. Forwarding autoinject and whole_doc for a Docs-enabled chat with auto-injection off changes nothing on its own, since the decline keys on rag_scope alone and the nudge never reads autoinject: there is no wrong number in that configuration, only a blank bar, and closing it needs the retrieval gate back plus a frontend mirror of resolveAutoInject that must stay in lockstep forever. Tests: three cases with negative controls. Reverting only the source reddens 5 of them on values, and the auto_heal_off and branch_unchanged rows stay green, so neither new guard overreaches. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments this PR added Comment, docstring and whitespace only, across the 11 files with PR-added comments: 509 comment lines down to 445. Heaviest cut in routes/inference.py (95 to 69), where a 14-line rationale for the pending-RAG-turn decline became 7 without losing the causal chain. Rules applied: delete where the code already says it, compress where the comment records a non-obvious reason, and keep every workaround or ordering note. So the "Route FIRST" ordering constraint in chat_count_tokens, the captured-null New Chat race note, the llama-server chat_template_kwargs layering quirk and the _native_grant_backed refactor history all survive, shorter. Two things constrained the pass. The python harness tests slice shipped TS between literal comment markers, so those markers are byte-identical and the CANVAS_* constant names stay unmentioned (one test asserts a count of 3). And a python docstring is an AST node, so three helper docstrings were shortened to one line rather than removed. Verified comment-only two ways: comment_tools.py check --strip-docstrings is 11/11, and prepush_gate --comment-only --allow-docstring-edits is 11/11, the latter isolating the docstring rewrites from the code. 269 tests pass, ruff clean, tsc clean, node --test 57 pass. * Drop a recount taken while its thread is still running The branch signature added in 961971eb3 catches a turn appended during a count, but a run streaming into a turn that already exists grows that message's content without moving the branch length or its last id, so the signature compares equal and the partial response is what gets published. Nothing recounts again, and if the run is then stopped or fails before writing usage the undercount stays. Reachable because the recount effect gates only on contextUsage being null and on its model/thread dependencies, with no running-state check, so returning to a thread with a background run and a blank usage cache starts a count mid-stream. Decline at publish time when the thread is running, rather than folding content into the signature: a live run writes its own usage when it lands and leaves the bar untouched if it is stopped, so nothing is lost by not publishing, and there is no hash of a whole branch on the main thread. Two cases, and reverting the guard reddens the streaming one while the idle control still publishes. * Make the recount's branch validation whole instead of patching it again Three rounds have each found another way past this guard, which is the signal that comparing a weak signature at publish time was the wrong shape rather than three separate bugs. Closing all three at the root. Text length joins the signature. A run streaming into a turn that already exists grows that message's content without moving the branch length or its last id, so neither the previous signature nor the running check saw it once the run had stopped: runningByThreadId is already false by then and the usage snapshot is still equal, so the partial published. An empty current branch now counts as a mismatch. The old current.length > 0 condition skipped the comparison entirely, so deleting the sole exchange mid-count left the old conversation's total on the emptied thread. The running check keys on "__default" when there is no thread id. A New Chat count captures a null id and has no branch to compare, and the active-thread check stays null until initialize() assigns one, so a first turn sent mid-count previously left the bare-template total on a chat that already had the turn. A first turn files its run under that key, which is the only witness available before the id exists. Six cases, three of them new. Reverting only this file reddens exactly the three new ones and leaves every control row green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Identify a counted branch by its content, not its size, for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Price the RAG tool on a new chat and validate attachments for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Validate stored-history recounts and repoint the status contract test for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Leave the context bar blank for output-only audio models for PR #7453 * Tighten the comments added by PR #7453 * Render a template for an empty chat so a new chat still counts for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Revert the empty-chat template shim for PR #7453; minja renders an empty list * Keep the token recount off the inference path for PR #7453 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refuse a token count server side while any generation is in flight The client gate added in 9bdb0f8c1 only covers Studio's own tab. A second tab, a script against /api or a count already queued could still land a recount next to a decode, so chat_count_tokens now declines with 503 when active_generations reports anything running. Checked twice: on entry, before the handler does any of the count's work, and again immediately before the llama-server round trip, since everything in between awaits and a run can begin in the gap. The refusal is deliberately coarse. _TrackedCancel registers external-provider runs too, and those never reach llama-server, so a count that was safe gets declined. Narrowing it would mean trusting a kind field to decide whether to do work beside a decode, and being wrong that way costs inference time while being wrong this way costs a later redraw. That coarseness exposed a gap on the client, which gated on localRunByThreadId alone. During an external-provider run it would issue a count, take the 503, and never retry, because only what the recount effect depends on can re-fire it and that run was not in the dependency array. The gate and the dependency are both runningByThreadId now, so the client refuses exactly what the server refuses. The client swallows the 503 and keeps the value already on the bar, so the cost is a number that arrives when the run ends rather than during it. test_a_count_taken_while_the_thread_is_running_is_dropped seeded a live run and asserted the publish guard dropped the total. The entry gate makes that state unreachable, so the case now starts idle and flips mid count, which is the only way a count can still be in flight when a run begins. * Price cached MCP schemas, and retry a New Chat recount after a background run Two review findings, both real. The count forced mcp_allowed False to keep discovery off the background path, so MCP schemas went unpriced while the completion rendered every one of them. That is an undercount, which is the dangerous direction: it tells the user room exists that the next request will not find. cached_mcp_tools() now reads the schemas the completion path already cached, with no probe, no spawn and no cool-off write, and the count declines with 503 when an enabled server has nothing cached and would have been probed. A server in its post-failure cool-off contributes nothing to the completion either, so skipping that one is exact rather than short. The tool branch also only opened on _tools_on, while MCP alone turns tools on for the completion, so a request with built-in tools off and MCP on was short by the whole catalog. It now opens on either. Separately, ThreadNewChatSwitch's recount did not observe run activity. New Chat during a background generation is a supported flow, so refreshContextUsage declined, and ThreadContextUsageRecount could not pick it up afterwards because an unpersisted New Chat has no activeThreadId to key on. The empty chat's bar stayed blank for good rather than until the run landed. The effect now takes runActive as a guard and a dependency, the same shape as the other recount. Merged current main, which had moved far enough to break the recount harness: the load path it slices verbatim gained a useChatRuntimeStore.setState call, and the harness's fake store had no such door. This was green locally and red in CI for exactly that reason. The fake now merges on setState, as the real one does. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by the token-count work Shorten the explanatory comments in the count endpoint, the recount helper and the runtime effects to the fewest lines that still carry the reasoning. No code changes: the minja empty-message note, the coarse generation refusal, the duplicated generation check, the _mcp_allowed rationale, the MCP cool-off note and the runningByThreadId dependency note all keep the fact they state. * Keep saved usage through history hydration, and stand a count down mid flight Two review findings. The history loader restores the last completion's own usage when it is for the current model and inside the window, then called refreshContextUsage unconditionally. That helper only stands down for a value that CHANGED while it was counting, not for one that was already there, so it overwrote exact totals with an estimate whose completionTokens is 0. It now runs only when nothing was restored, which is still every case it was added for: a thread opened after a model switch fails the model check and is priced as before. Kept as the narrowed value rather than a boolean flag, since the writes below it need that narrowing. The count's generation guards admit only while nothing is decoding, but admission and the work are separate steps, so a run registering in between was not prevented. Making them mutually exclusive would mean generation startup waiting on a lock held for the length of a count, up to tens of milliseconds on a long thread, which is the cost these guards exist to avoid. Instead count_chat_tokens takes an optional should_abort, polled between /apply-template and /tokenize, so a count admitted while idle drops its second round trip once a run appears. The comment claiming no overlap now describes what the code does rather than what it would like to be true. CountAborted has to escape the template except-block: swallowed, it would set apply_template_failed and the text fallback would tokenize anyway, which is the work being declined. Covered by a test against the real count_chat_tokens, not a stub, because a stub cannot see that. Adding the kwarg broke six existing count tests whose shared stub did not accept it, which the targeted runs did not show. Backend suite: 13877 passed, 14 failed, the same 14 as before these changes and a subset of clean main's 15. tests/studio 2350 passed with 3 failures that reproduce on clean main. Frontend 300 passed, tsc, build, eslint and ruff clean. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the token-count comments added since the last pass * Stop the recount doing work it can never use Four review findings, all confirmed against the code. Deep Research routes the next send to a server-side research run instead of posting this history, and the research reply carries no usage, so a total counted here describes a request that is never made and nothing later corrects it. The recount now declines while it is on. /chat/count_tokens always refuses images, because /apply-template swaps each one for a short marker. The client was finding that out by hashing the base64 into the branch signature, serialising it into a request body that can run to megabytes, and posting it, all on the UI thread, to be told 503. It now declines before the hash, which is why the signature moved below the guards. The history loader recounted for every pane. A compare pane deliberately never writes activeThreadId, so refreshContextUsage drops its total at publish: opening two panes rebuilt two branches from storage and paid for /apply-template and /tokenize twice to display neither. Gated on the same modelType/pairId condition ThreadContextUsageRecount and ActiveBranchRegistrar already use. A model load fires two triggers milliseconds apart, the explicit post-load call and the effect watching modelLoading, which resetLoadingUi clears just before it. The generation map discarded one result but neither request. Counts are now deduped per thread while one is on the wire; the publish-time branch check still drops a total the branch outgrew, so nothing is lost by skipping the second. Not fixed: making count admission atomic with generation startup. The check is still check-then-act and that is deliberate. Mutual exclusion means generation startup waiting on a lock a count holds, up to tens of milliseconds on a long thread, which is the cost these guards exist to avoid. tests/studio 2365 passed, the 3 failures in test_chat_autoload_failure_gate.py also fail on clean main. Frontend 300 passed, tsc and build clean. The one eslint error is a restricted import from #6205 and is present at the base. * Defer a skipped recount instead of dropping it The dedupe added in the previous commit turned one lost-retry bug into another. A run that starts and is stopped before it emits usage flips runActive back and fires the retry the recount effect depends on. If that retry arrives while a count is still in flight it was discarded, the in-flight count then rejected its now-stale branch, and nothing changed again, so the bar stayed blank for good. A skipped trigger is now recorded with its options and replayed once the in-flight count settles, but only if that count published nothing. That keeps the model-load case at a single count, since there the first trigger publishes and the queued one has nothing left to do, while the stopped-run case gets its retry against the branch as it stands rather than the one the first count read. The dedupe test could not tell the two apart: it awaited both calls but the replay is scheduled off the first count's finally, so a stray one landed after the assertions. It now settles first, and replaying unconditionally reddens it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by the last two fixes Collapse the multi-line explanations on the in-flight dedupe, the deferred retry, the Deep Research decline, the image decline, the compare-pane gate and the messagesContainImage predicate, keeping the reasoning each records. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
79 lines
2.9 KiB
Python
79 lines
2.9 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
"""Shared plumbing for the ``node --experimental-strip-types`` source harnesses.
|
|
|
|
``studio/frontend`` carries no JS test runner, so frontend behaviours are pinned by slicing
|
|
the real source VERBATIM into a harness module and running it under node; only the fixtures
|
|
the sliced code reads through are hand-written. Harness and runner go into a per-invocation
|
|
``mkdtemp(prefix = "run")``, so concurrent tests share no file.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
import tempfile
|
|
from collections.abc import Iterable
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
WORKDIR = Path(__file__).resolve().parents[2]
|
|
|
|
|
|
def source_path(relative_path: str) -> Path:
|
|
"""Locate a studio source both in this repo and in a vendored checkout."""
|
|
direct = WORKDIR / relative_path
|
|
if direct.exists():
|
|
return direct
|
|
return WORKDIR / "unsloth_repo" / relative_path
|
|
|
|
|
|
def read(path: Path) -> str:
|
|
return path.read_text(encoding = "utf-8")
|
|
|
|
|
|
def slice_between(text: str, start_marker: str, end_marker: str) -> str:
|
|
"""The source from ``start_marker`` up to (not including) ``end_marker``."""
|
|
start = text.index(start_marker)
|
|
end = text.index(end_marker, start + len(start_marker))
|
|
return text[start:end]
|
|
|
|
|
|
def require_node(sources: Iterable[Path]) -> None:
|
|
"""Skip unless node can strip types and every sliced source is present."""
|
|
if shutil.which("node") is None:
|
|
pytest.skip("node not available")
|
|
for path in sources:
|
|
if not Path(path).exists():
|
|
pytest.skip("studio chat sources not present")
|
|
probe = subprocess.run(
|
|
["node", "--experimental-strip-types", "--version"],
|
|
capture_output = True,
|
|
text = True,
|
|
timeout = 30,
|
|
)
|
|
if probe.returncode != 0:
|
|
pytest.skip("node --experimental-strip-types not available")
|
|
|
|
|
|
def run_harness(temp_root: Path, harness_source: str, script: str) -> dict:
|
|
"""Run ``script`` against ``harness_source`` and parse its last stdout line."""
|
|
temp_root.mkdir(parents = True, exist_ok = True)
|
|
workdir = Path(tempfile.mkdtemp(prefix = "run", dir = str(temp_root)))
|
|
(workdir / "harness.ts").write_text(harness_source, encoding = "utf-8")
|
|
(workdir / "run.mts").write_text(script, encoding = "utf-8")
|
|
result = subprocess.run(
|
|
["node", "--experimental-strip-types", "--no-warnings", "run.mts"],
|
|
cwd = str(workdir),
|
|
capture_output = True,
|
|
text = True,
|
|
timeout = 60,
|
|
env = dict(os.environ, NODE_NO_WARNINGS = "1"),
|
|
)
|
|
assert result.returncode == 0, f"stderr: {result.stderr}\nstdout: {result.stdout}"
|
|
lines = [line for line in result.stdout.strip().splitlines() if line.strip()]
|
|
return json.loads(lines[-1])
|