QVAC-22567 feat[bc]: remove SDK dynamic tools mode (toolsMode) - #3380
Conversation
Review StatusCurrent Status: ✅ APPROVED |
License compliance — findings detected (warn-only)Critical: 0 · High: 2 · Medium: 0
How to resolve a blocking finding:
Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
- Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy.
- Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy.
- Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy.
32df6d9 to
7179327
Compare
- Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy.
- Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy.
d00fcb8 to
d161324
Compare
|
This draft PR is stale because it has been open 21 days and the author has not commented since opening. It is flagged for removal. Remove the stale label or comment on the PR or this will be closed in one day. |
Drop the SDK-side surface of the dynamic tools feature (companion to the addon-side tools_compact removal in PR #3373): - Remove the `toolsMode` field from the llamacpp model config schema and the `TOOLS_MODE` / `ToolsMode` root exports (breaking) - Remove `appendToolsToHistory` and the dynamic branches in completion-stream / batch-completion-stream; tools are always prepended after the system message (previous default behavior) - Drop the `tools_mode` -> `tools_compact` addon config mapping - Delete the dynamic-tools example and the dynamic e2e tests (tools-simple-function-dynamic, kv-cache-tools-dynamic-reuse) and the `tools-dynamic` consumer resources - Regenerate contract/schema.json and the sdk-python bindings (ToolsMode enums / tools_mode field removed) - opencode plugin: stop pinning `toolsMode: 'static'` (field removed; static is the only behavior) No version bump; releasing is deferred to the release flow. Introduced in #1779 (QVAC-13559).
Hash complete tool definitions for cache identity so same-named schema changes do not reuse stale primed caches. Update KV-cache docs and validate structured tool calls across cached model reloads.
b6c2eb3 to
9505724
Compare
…amacpp addon (#3373) * feat[bc]: remove dynamic tools (tools_compact) from llm-llamacpp addon Remove the dynamic tools feature (tools added mid-conversation and trimmed from the KV cache once the tool-call chain resolves). It only worked for Qwen3 via a custom dynamic template and relied on sliding-context anchor bookkeeping that does not carry over to newer models. - Delete ToolsCompactController and Qwen3ToolsDynamicTemplate - Stop parsing the `tools_compact` config key (breaking) - Strip the tools anchor plumbing from ContextSlider, ContextShifter, ReasoningBlockCompactor, Text/Mtmd contexts, LlamaModel, and the continuous-batch scheduler; general sliding/compaction is unchanged - Remove the two-pass (with/without tools) tokenization - Remove PromptLayout + validatePromptPolicy (existed only for tools_compact prompt-shape validation) - Remove runtimeDebugStats (nPastBeforeTools/toolsTrimmed) and getNPastBeforeTools - Delete feature unit/integration/mobile tests, docs, and examples; adapt shared tests to the new signatures No version bump; releasing is deferred to the release flow. SDK-side removal (toolsMode) is split into a companion PR. Introduced in #706 and #1379 (QVAC-16769). * fix: address PR #3373 review feedback - Tolerate a lingering `tools_compact` config key: erase it with a deprecation warning instead of letting it fall through to the generic CLI-arg conversion, where the unknown `--tools-compact` argument would throw InvalidArgument and fail the entire model load. Restores the pre-removal graceful behavior (no-op / ignored) and decouples merge and release ordering from the SDK-side removal (PR #3380). Adds a regression test (CommonParamsParseToolsCompactIgnored). - Remove the dead `hasKvCacheContext` block left in the continuous-batch admission path — a blocking file_size syscall performed under the scheduler lock whose only consumer (validatePromptPolicy) was deleted. - Reword two stale comments that still referenced the removed onGenerationCompletePolicy / generationStarted_ (ContinuousBatchScheduler .hpp finalizeTerminalDriver doc + .cpp notifyDone re-run rationale). - Fix the continuous-batching.md onCancel lifecycle row whose "Same policy as above" referred to the removed onGenerationCompletePolicy. * test: restore unit coverage for isQwen3Architecture exact-match The tools_compact removal deleted `SupportsToolsCompactForModelMetadata ByArchitecture`, which was the only test pinning the exact-match `qwen3` arch predicate. That predicate survives (it still drives fixed-template selection via isQwen3Model -> getChatTemplateForModel) but had lost all direct coverage, so a regression that broadened it to match `qwen35` would slip through silently. Export `isQwen3Architecture` from ChatTemplateUtils (mirroring the already-exported `isQwen3ReasoningFamilyArchitecture`) and add `IsQwen3ArchitectureExactMatch` covering qwen3 (+case-insensitive) true and qwen35/qwen3moe/llama/"" false. Addresses PR #3373 review comment. * test: restore qwen3 tool-call coverage * fix[api]: reject removed dynamic tools and validate warm tool calls * test: use schema-compatible warm tool prompt * fix[bc]: remove tools_compact config tombstone --------- Co-authored-by: Nidhin <nidhinpd811@gmail.com>
|
This draft PR is stale because it has been open 21 days and the author has not commented since opening. It is flagged for removal. Remove the stale label or comment on the PR or this will be closed in one day. |
|
This PR is still WIP. Waiting for the new LLM version release before opening this for review. |
|
This comment still references dynamic mode and its protective clamp |
|
/review |
Removing a config key has been done twice before, and neither time added strictness anywhere: toolsMode in #3380 and n_discarded in #3999 each deleted the field and stopped. The client options schema has been strict since long before this change, so a JS or TS caller passing a retired key already fails validation with code 50010; other callers get it stripped, which is what happened to toolsMode and what n_discarded will do. Holding load_mode to a different standard was not justified. Both .strict() calls go, and the exported contract and generated Python client return to their previous shape. With them go the tests that only existed to prove them: the resolver rejection case and the Python extra_forbidden case. The dispatch changes go too. Coercing a malformed modelConfig into a defaults object, and a raw ZodError escaping the error normalisation, are both real, but both predate this change and neither has anything to do with load_mode - every existing enum field behaves the same way. They belong in their own change if anyone wants them. The compile-time addon union guard goes as well. No other config field has one.
…p with load_mode (#4078) * QVAC-24073 feat[bc]: adopt fabric b10297 consumers and replace no_mmap with load_mode The llm-llamacpp addon dropped no_mmap in 0.47.0 and added load_mode in its place. An unrecognised key is not ignored: it falls through to llama.cpp's argument parser, so a config still carrying no_mmap fails the model load with "invalid argument: --no-mmap". Pin the six fabric b10297 consumer releases in packages/inference and packages/sdk. The set moves together because they share the libqvac-ggml-* backend build; a 10297 addon beside a 10069 sibling collides, and a mixed addon set crashes on iOS. A caret on a 0.x version locks the minor, so each range had to move explicitly. Replace no_mmap with load_mode in the llamacpp completion config, typed as the addon's own enum: none, mmap, mlock, mmap+mlock, dio. Left unset, the addon applies its default of mmap, so the field carries no SDK-side default and does not join LLM_CONFIG_DEFAULTS. transformLlmConfig needs no change: its camelCase-to-snake rewrite only matches all-letter keys, so load_mode reaches the addon verbatim, which is now covered by a test rather than assumed. Regenerate contract/schema.json and the Python client from the schema. Add three model-load e2e cases: load_mode 'none', explicit load_mode 'mmap', and a legacy no_mmap rejected by strict validation. None are tagged smoke; model loading already carries smoke coverage. Update the llm-llamacpp README and the website addon page, whose parameter tables still documented no_mmap. * QVAC-24073 fix: drop the stray streamx dependency and settle load-mode models through ResourceManager The streamx entry was never intended. It arrived from a local `bun update streamx` run while confirming that streamx 2.28.1 carries the Writable typing fix, and it contradicts how that fix actually reaches the SDK: streamx is transitive through tar-stream and bare-stream, whose ranges already admit 2.28.1, so a plain install resolves it with no direct dependency. The same command also reordered two addon entries; both are reverted. The load-mode e2e cases unloaded their model directly, which skips ResourceManager.evict() and with it the mobile unloadSettleMs pause. That pause exists because iOS does not release a worklet's pages promptly, and the next load can abort inside the GGML allocator on the residue. The two cases run back to back, and 'none' loads a private copy of the whole model rather than mapping it, so they are the pair most likely to hit it. Both now register their model under their own dep and evict in a finally. The legacy-no_mmap case leaked its model on the branch where validation unexpectedly admits the retired key. It now evicts before reporting failure. * QVAC-24073 feat[bc]: reject unknown llamacpp load config keys on the wire The wire schema's nested modelConfig was not strict, so the exported contract omitted additionalProperties for it and every generated client dropped an unrecognised key instead of refusing it. A caller passing the now-retired no_mmap got a default mmap load and no error, which is the opposite of what removing the field is meant to communicate. The JS client is unaffected: loadBuiltinToRequestSchema already validates modelConfig strictly, so an unknown key never reached the wire from there. What changes is the generated-client and raw-wire path, where the key now fails validation rather than being discarded. Rejection for a Python caller currently lands server-side, not in root client validation. The generated LoadModelRequest is a union, and the custom-plugin arm is guarded by a zod .refine() that excludes built-in model types; a refinement is runtime-only and does not survive the JSON Schema export, so the arm still admits llamacpp-completion and swallows the request. The server runs the same schemas with refinements intact and rejects it there. Closing that gap means changing the custom-plugin arm, which is tracked separately. Cover the config type directly: every accepted load mode, a rejected one, the retired no_mmap raising extra_forbidden, and the field staying optional. The other model types keep non-strict nested configs; aligning them is a separate change because it turns previously ignored input into errors across the whole fleet. * QVAC-24073 test: format the llamacpp load-mode python tests with black The sdk-python format check runs black over tests/, and the new file was not formatted to it. * QVAC-24073 fix: reject the retired no_mmap where the server actually resolves config The strict wire schema added earlier never sees the key. prepareRequest runs applyDeviceDefaults before requestSchema.parse, and that resolves modelConfig through MODEL_CONFIG_SCHEMAS[llamacppCompletion], which was the non-strict llmConfigSchema. Zod dropped no_mmap there, so the strict schema downstream validated an already-sanitised config and passed. A Python or raw-wire caller kept getting a default mmap load with no error - the silent behaviour this change set exists to remove. Making the resolver schema strict closes it at the layer that runs. Its only production consumer is that one registry entry, and the exported contract is generated from llmConfigBaseSchema, so nothing regenerates. Cover it through the production resolver rather than a strict schema built inside the test: the previous unit test supplied .strict() itself and kept passing when production strictness was removed. Verified by reverting the production line - the new no_mmap case fails, and passes again once restored. Tighten the e2e rejection case. It accepted any error naming no_mmap, which llama.cpp's own "invalid argument: --no-mmap" also satisfies; that error means the key reached the addon, which is the regression. It now asserts error code 50010 and fails explicitly on a native argument error. The numeric code lives on the error rather than in its message, so the handler surfaces it. Drop the explicit load_mode 'mmap' e2e case. mmap is the addon default, so it duplicated the existing model-load coverage for another model load of suite time. The 'none' case stays as the load-path compatibility check. Correct the docs sample comment: 'none' is a full buffered read, so it disables mlock and direct I/O as well as mmap, not just mmap. * QVAC-24073 fix: normalize a config-resolution rejection into a typed error Making the resolver schema strict gave applyDeviceDefaults a way to throw, and it ran outside the block that converts a ZodError into RequestValidationFailedError. A caller passing the retired no_mmap therefore got a raw ZodError instead of a typed error carrying code 50010 - unstructured over RPC, and against the rule that errors leave the SDK as structured classes. Before this change set the function could not throw, so the placement was safe; now it is not. Resolving defaults happens inside the same block, so both parses normalize through one path. Covered on the dispatch seam, and the guard was checked by reverting the placement: the new case fails, then passes once restored. * QVAC-24073 fix: make the llamacpp base config strict at its definition Strictness was applied per call site, so the two surfaces that share the base object kept dropping the retired key. `packages/sdk/src/schemas/public.ts` re-exports it as `llamacppCompletionConfigSchema`, where `parse({ no_mmap: true })` returned `{}`; and both deviceDefaults arms in `schemas/config.ts` use it, so a JSON, JS or Expo config carrying `no_mmap` was stripped during config validation before the strict resolver could refuse it. Neither raised an error, which is the behaviour this change set removes. A strict base makes one policy serve every consumer, so the per-site `.strict()` on the resolver and the llm wire schema are dropped as redundant. The exported contract is byte-identical either way, and the generated Python client is unchanged. Checked every consumer: `configKeys()` reads `.shape` and is unaffected; the plugin `loadConfigSchema` runs after the resolver, so nothing unknown reaches it; reload requests are whisper-only and never carry this config; and the shipped device patterns still resolve. A full config using all thirty documented keys still parses. Guards: the base rejects `no_mmap` without a test-applied `.strict()`, and both the alias and canonical deviceDefaults arms reject it while keeping a valid `load_mode`. * QVAC-24073 fix: reject a malformed modelConfig instead of coercing it to defaults applyDeviceDefaults merged the raw modelConfig by spreading it. Spreading a non-object yields nothing, so `false`, `42`, `''`, `[]` and `null` all collapsed into a plain defaults object that then satisfied the schema. A caller sending a wrong-typed modelConfig got a silently defaulted load rather than a type error. Making the base schema strict changed only half of that: a non-empty string or array spreads to indexed keys, which now fail as unrecognized, while the scalar and empty cases stayed silent. That split is worse than either end of it. A present modelConfig that is not a plain object is now passed through untouched, so the request schema reports the type error, normalized like any other. null and undefined still mean absent, which is what the optional field has always meant. Guarded across false, 42, 'abc', [] and ['a'], and checked by removing the guard: the case fails, then passes once restored. Also format config-resolution-e2e.test.ts to Prettier, which CI was failing on. * QVAC-24073 test: pin load_mode to the addon union at compile time The five values were copied from the addon rather than derived, so the two could drift apart silently: the SDK would keep accepting a mode the addon had dropped, or refuse one it had added, and no test would notice. `LlmLlamacpp.LlamaConfig['load_mode']` is exported and inference already depends on the package, so the union is reachable. An exact bidirectional extends check against it fails the typecheck on any divergence. Verified by removing 'dio' from the enum: `Type 'true' is not assignable to type 'false'`. * QVAC-24073 chore: drop the llm-llamacpp README from this SDK change set The addon's own README does not belong in an SDK PR. It is a separate package with its own release cycle, and correcting it here would land a doc change with no CHANGELOG entry and no version bump of its own. The stale `no_mmap` table is the addon release's to fix; the website page stays, since docs/ is not a package. Also rewrite isPlainObject without comparing to null, which the code-quality analyzer flagged on the new guard. Same behaviour: false for null, arrays and non-objects. * QVAC-24073 chore: drop the website addon page from this SDK change set The website page belongs to a `doc:` PR, not a feature one. The nearest precedent is the same kind of change: #3854 exposed `image_no_upscale` in the same config schema and shipped `inference` + `sdk` + `sdk-python` with no docs, and the page was updated afterwards by #3959 under its own ticket. Every commit that has ever touched `docs/website/content/docs/addons/llm-llamacpp/index.mdx` came from a `doc:` PR, never a feature PR. The parameter-table correction moves to its own doc PR. * QVAC-24073 fix: pass ctx_size, not n_ctx, from the python and conformance callers Eight call sites asked for a context window with `n_ctx`, which the LLM config has never declared. The key was silently stripped, so those loads ran at the default 1024 rather than the window they requested. Strict validation turns that into a load failure, so the sdk-python real-worker leg goes red and the shipped notebook quickstart raises for anyone who copies it. The declared key is `ctx_size`. Renaming is the correct change on its own terms: `test_bare_rpc_transport.py` explains that Qwen3's reasoning trace overflows the metadata default and an explicit window is needed, and that window was never actually applied. Sites: conformance `cases.json`, `test_bare_rpc_transport.py` (four), `test_notebook.py`, `examples/notebook.py`, `examples/notebook.ipynb`. Only the python conformance runner reached this: the JS runner skips `completionOrchestrate` before loadModel. Its skip comment blamed snake_case versus camelCase, but `n_ctx` is not a casing variant of anything — that half of the rationale is dropped, the worker-driven reason stands. * QVAC-24073 fix: drop the last n_ctx references The two config-reload `wrongModelType` cases passed a whisper modelId with an LLM modelType and `n_ctx: 2048`, expecting the type mismatch to be rejected. With `n_ctx` no longer a declared key, the unknown key alone is enough to reject, so the case could pass without ever exercising the mismatch it exists for. They now pass `ctx_size`, leaving the mismatch as the only reason to fail. `errors.py` told callers to raise `n_ctx` on a context overflow, naming a key the config has never accepted; the field is `ctx_size`. Same for the stale mention in the transport test's comment. * QVAC-24073 fix: narrow the strictness to the resolver and the wire schema Making the base object strict was more than this change needed, and it undid a deliberate design. The base was permissive and `.strict()` was applied at the call sites that wanted it, which is why the device-defaults surface accepts `.partial()` schemas for whisper, parakeet, ocr, diffusion and vla, and a bare `z.record(z.string(), z.unknown())` for nmtcpp and tts-ggml. Nothing on that surface was ever strict. A strict base made one retired LLM key abort initialisation for every model type, which was a consequence of removing that layering rather than a defect being fixed. Some server-side strictness is still required. On a plain object Zod strips the retired key, so the addon receives no loading mode and defaults to mmap, silently reversing `no_mmap: true`. Strict therefore stays in two places: the resolver schema, which is what dispatch parses against, and the llm wire schema, so the exported contract and the generated Python client keep `additionalProperties: false`. The base returns to `z.object`, so config files and the public `llamacppCompletionConfigSchema` export behave as before. The device-default and base-schema tests written for the strict base go with it; the resolver, dispatch, transform, contract, Python and e2e coverage is unchanged. * QVAC-24073 fix: drop the added strictness and everything it dragged in Removing a config key has been done twice before, and neither time added strictness anywhere: toolsMode in #3380 and n_discarded in #3999 each deleted the field and stopped. The client options schema has been strict since long before this change, so a JS or TS caller passing a retired key already fails validation with code 50010; other callers get it stripped, which is what happened to toolsMode and what n_discarded will do. Holding load_mode to a different standard was not justified. Both .strict() calls go, and the exported contract and generated Python client return to their previous shape. With them go the tests that only existed to prove them: the resolver rejection case and the Python extra_forbidden case. The dispatch changes go too. Coercing a malformed modelConfig into a defaults object, and a raw ZodError escaping the error normalisation, are both real, but both predate this change and neither has anything to do with load_mode - every existing enum field behaves the same way. They belong in their own change if anyone wants them. The compile-time addon union guard goes as well. No other config field has one. * QVAC-24073 chore: restore the sdk dependency ordering A bun install reordered two unrelated entries; the diff should carry the six pin bumps only. * QVAC-24073 chore: leave the conformance runner untouched The comment edit was not needed for this change.
…precedent Removing a config key has never added strictness — toolsMode (#3380), n_discarded's original #3999, and load_mode (#4078, where the same strictification was added and reverted) all deleted the field and stopped. The long-standing strict options schema still rejects the key for TS/JS callers with code 50010; other callers get it stripped, as before. The base schema, dispatch, the exported contract, and the generated Python client return to their previous shape, and the tests that only existed to prove the strictness go with them.
…4163) * QVAC-24251 feat[bc]: drop n_discarded from the llamacpp config schema The llm addon no longer consumes n_discarded (#3938), so the key would reach llama's own argument parser and fail model load as an unknown option. Remove it from the zod schema so loadModel() rejects it at validation, regenerate the exported contract and the Python client, and drop it from every e2e model config. The KV-cache guide now describes the two overflow surfaces instead of recommending sliding. * QVAC-24251 fix: parse every context-overflow wording the llm addon emits parseContextOverflowMessage matched two of the addon's numeric overflow wordings. The two warm-cache guards and the multimodal single-prompt wording parsed as nothing, so the typed error carried no sizes — the multimodal gap is live against the published 0.47.x, not only against the new wordings. Every pattern now captures the context size last and sums the leading groups, KV cells are preferred where a guard reports positions too, and isAddonContextOverflowError recognises the 'at batch prefill step' wording on the message-only fallback. Sizes are validated as finite, and the tests assert the >= floor the guards actually trigger on, since the failing requirement can equal the window. * QVAC-24251 refactor: drop the sliding-era tool-block resend from the kv-cache path toolBlockEvictable existed because the addon's discard window opened exactly where the static tool block sat, so while sliding was possible the block had to travel with every turn. Nothing evicts it any more, so a warm turn skips it whenever the prefix is known to hold a rendered one. The regression test now pins that a config still carrying the retired key does not force a resend. * QVAC-24251 test: cover the context boundary and prefill overflow in the e2e consumers A dedicated 512-token llm resource (same constant as llm, no new download) backs two completion tests on desktop, electron and mobile: a generation that fills the window must surface as the public stopReason 'length' with its produced tokens retained (predict is -1 and the prompt has no natural terminus, so the boundary is the only length source), and an oversized prefill must reject with the typed ContextOverflowError whose parsed sizes reach the window. * QVAC-24251 fix[bc]: fail the retired n_discarded closed on every ingestion path Only the TypeScript options schema was strict, so the raw wire request, the plugin's loadConfigSchema and both deviceDefaults entries stripped the retired key and loaded with sliding silently off. All four now use llmConfigBaseSchema.strict(). The contract's LLM modelConfig gains additionalProperties: false, which the Python generator turns into extra="forbid", so a Python caller constructing a config with n_discarded gets a ValidationError instead of a silent drop. Rejection tests pin the wire schema, both deviceDefaults keys and the generated Python model. * QVAC-24251 feat[api]: carry cached and required context on ContextOverflowError The parser summed cached totals — and, on multimodal guards, KV-cell counts — into the token-named promptTokens, so truncation logic could overestimate the prompt by the whole cached conversation and multimodal callers received cells under a token name. promptTokens now carries only figures the guard denominates in tokens; two new optional fields, cachedTokens and requiredTokens, carry the cached conversation and the total that failed the guard, in the same units as ctxSize. The fields serialize through toErrorResponseFields, rebuild in the SDK RPC reconstructor and the Python reconstructor, and the error message names both halves on a warm cache. * QVAC-24251 doc: correct the public stop reason in the KV-cache guide The guide promised stopReason "contextOverflow", which the public SDK never returns: the plugin maps the addon's context-boundary stop and a positive predict cutoff both to "length". Say so, and that stopReason alone does not distinguish the two. * QVAC-24251 test: grow a real warm cache into the overflow guard in e2e The parser's most substantial additions are the cached-plus-prompt guards, and nothing public exercised them. A third consumer test caches a first turn that fills most of the 512 window under a per-run key, then sends a follow-up that fits the window alone but not on top of the cache, and asserts the typed error carries a requiredTokens no smaller than the window; the cache is deleted in cleanup. The cold-prefill test now also pins ctxSize to the configured 512, and the boundary-stop comment no longer claims an early EOS is impossible. * QVAC-24251 fix: keep promptTokens unset for the retired short overflow form Both emitters of "(N tokens, max M)" format a cached total (text: nPast_ + nTokens, multimodal: cacheTokens + nTokens), so on a warm cache the figure is not the prompt alone. The short form now maps to requiredTokens only, matching the field's prompt-only contract, and the short-form test pins the unset field. Comments trimmed to a line or two and version strings dropped from source. * QVAC-24251 chore: adopt @qvac/llm-llamacpp 0.48.0 The addon release carrying the sliding-context removal. Caret on 0.x is patch-only, so the pins in inference (dependency and peer) and sdk move to ^0.48.0 explicitly. The overflow parsers keep the older wordings, so a worker still on 0.47.x keeps parsing. * QVAC-24251 fix[bc]: reject the retired key at dispatch, before the request schema Dispatch applies device defaults before the request schema runs, and the default-applying parse was non-strict, so it stripped n_discarded ahead of every strict validation added so far. llmConfigBaseSchema is now strict at the source, so that first parse and every derivation — wire, deviceDefaults, plugin loadConfigSchema, the public export — reject the key, and dispatch wraps the defaults parse so the failure surfaces as a structured RequestValidationFailedError. Regressions pin send() and the config-resolution path. * QVAC-24251 fix[bc]: exclude built-in types from the custom-plugin arm structurally The catch-all's built-in exclusion was a zod refine, which does not serialize, so the exported contract and the generated Python union accepted a built-in modelType with arbitrary config through the permissive arm. A regex over the canonical types and aliases carries the rule into both. Closing the leak exposed the wire arms it had been masking: they required modelConfig even though the server injects defaults, so their optionality now mirrors the options schemas (llm, whisper, bci, embeddings, ocr optional; nmt, tts, audiogen required), and the transport tests that validated enum-typed requests through the leak now send the wire strings. Python tests pin the union, the public load_model() (rejects before the transport; custom types still pass) and the generated model. * QVAC-24251 fix: neutral required-only overflow message and honest ctxSize docs A lone requiredTokens can be a cold multimodal prompt in KV cells or the retired short form's cached total in tokens, so the message no longer blames a 'prompt spanning N KV cells' — it says what is known: the request needs N context tokens and no longer fits. ctxSize is documented as the effective per-request ceiling (ctx_size split across slots at parallel > 1), not the configured total, in the parser, both error classes, the Python docstring and the KV-cache guide. The parser's separators are horizontal whitespace only, matching its single-line claim. Message-level regressions pin the warm and required-only shapes. * QVAC-24251 test: prove the warm path and the boundary; surface cleanup failures The warm-cache e2e now asserts the first turn ended commit-eligible (no stop reason, non-empty output) and that the error carries a positive cachedTokens, so a rolled-back first turn or a cold full-history resend fails instead of passing through the plain prefill guard. A failed cache deletion fails the test rather than leaking a named cache. The boundary test replaces predict -1 with a 480 budget above the window's remaining capacity and asserts generatedTokens lands under it, so a 'length' stop provably means the boundary. The JS RPC round-trip test pins cachedTokens and requiredTokens across the envelope. * QVAC-24251 chore: tighten comments and fix Python import order Comment blocks across the diff shrink to one or two lines each, and the tests/test_load_model.py import moves to its alphabetical position, which the Ruff import-order check requires. * QVAC-24251 fix: keep overflow wording unit-neutral and coherent at equality A lone or cached total can be KV cells, so the message says 'context units', and the guards trigger at equality — a generating request needs a free slot — so it is phrased as leaving no room to generate rather than exceeding, which read as a contradiction when the total equalled the capacity. The Python default message follows suit, and the stale class docs that still described a prompt against a configured window now describe the optional fields. Regressions pin the unit-neutral and equality wordings. * QVAC-24251 test: narrow the union for mypy and harden dispatch and warm assertions The custom-plugin regression dereferenced .root.root on a union arm mypy cannot narrow from the input dict, failing the required Python typecheck; an isinstance assertion narrows it. The dispatch regression now asserts the structured RequestValidationFailedError class, not just the message. The warm-cache executor deletes the per-run cache even when the flow throws before its inner handling. * QVAC-24251 chore: trim new comments to two lines * QVAC-24251 fix: take the larger measure on multimodal overflow guards The MtmdLlm guards trip on EITHER positions or KV cells against the same ceiling, but the mappings captured only one side, so a positions-dominant overflow reported a figure below the window — contradicting the requiredTokens >= ctxSize contract the fields document. Both figures are captured now and the larger is what failed the guard. Positions-dominant regressions pin all three multimodal wordings. * QVAC-24251 fix: accept the generated ModelType enum in load_model The enum's members are not str, so they only ever validated through the custom-plugin catch-all; with that arm excluding built-ins the public load_model() started rejecting them. The signature accepts the enum and coerces it to its wire string, with a regression pinning it. * QVAC-24251 test: widen the e2e budgets clear of the window The boundary test's 480 budget sat ~2% from the window's usable capacity, so a generous tokenizer could stop on the prediction cutoff and fail the boundary proof; 1000 is unambiguous. The warm test's first turn gets 48 tokens so a short ramble still ends on EOS and stays commit-eligible. * QVAC-24251 chore: drop redundant strict() calls and guard the exclusion regex The base schema is strict, so the six per-site strict() wrappers are no-ops. A guard test pins that every built-in type and alias stays a plain kebab identifier, since the custom-plugin exclusion regex interpolates them unescaped. * QVAC-24251 fix: floor the warm signature and neutralize the Python default message A commit rollback returns normally and turn two re-primes the cache, so a positive cachedTokens alone does not prove the first turn survived — the assertion now requires a floor well above what a system-prompt prime can hold. The Python direct-construction default said 'exceeds', which is false at the equality boundary the guards trigger on; it now matches the TS wording's neutrality, with direct-construction regressions for the equality and warm shapes. * QVAC-24251 fix: generate Python enums as value subclasses A generated enum member was not a str, so it validated only through the custom-plugin catch-all; with that arm excluding built-ins, direct LoadModelRequest construction with ModelType.X failed even though the enum ships in the same client. --use-subclass-enum makes every generated enum subclass its value type, so a member equals its wire string and routes through its own Literal arm. A regression pins direct union construction; load_model()'s signature-level acceptance stays. * QVAC-24251 fix: keep the committed cache through a pre-mutation overflow The addon's prefill guards reject before any decode or save and deliberately leave the last committed cache file intact, but the turn's unconditional rollback then unlinked it — an oversized follow-up cost the whole warm conversation. The session gains releaseTurn, which frees locks and active-refs without touching the disk cache or its recorded prefix, and the completion op routes a caught addon ContextOverflow to it. A regression pins that the retry after an overflow stays a warm delta send, and fails with the fix reverted. * QVAC-24251 test: retry the warm overflow before cleanup in e2e The warm test proved the failing request was warm but not that the committed cache survived it: the same follow-up is now retried before cleanup and must overflow warm again (cachedTokens >= 200), which a destroyed, re-primed cache cannot produce. * QVAC-24251 chore: label defensive parser probes and refresh stale comments Valid addon state keeps KV cells >= positions, so the positions-dominant fixtures are malformed-input probes for the defensive max, not emitted output — the test and pattern comments now say so. The enum-coercion comment stops claiming members are not str, which the subclass-enum generator change made false. * QVAC-24251 doc: note the parallel predict reservation and the releaseTurn cleanup With parallel >= 2 the scheduler treats a positive predict as a reservation and rejects at admission instead of stopping at the boundary; the guide says so next to the boundary behaviour. The implementation table carries the session's non-destructive releaseTurn. * QVAC-24251 fix: preserve the committed cache through scheduler admission refusals The continuous scheduler's per-sequence-cap guards (cached+tail capacity and the positive-predict reservation) reject before any decode or save, like the prefill guards, but throw the generic InvalidArgument status — so the deferred rollback still destroyed the last committed cache. A wording-anchored detector routes them to releaseTurn, which now also schedules the deferred auto-cache retention sweep that rollback runs. Regressions pin the warm retry for both refusal shapes (each fails with its branch reverted), every scheduler wording, and the session-level release contract: file, prefix and init flag survive, the active-ref frees, and a same-key waiter admits warm with no re-prime. * QVAC-24251 chore: name the constructor extras param and flow each size once The fifth positional param carried only cachedTokens/requiredTokens but was fed the whole parse result, which read as passing the same record through two channels. It is extraSizes now, admits explicit undefined so a destructured literal is assignable, and the three throw sites pass each parsed field exactly once. * QVAC-24251 doc: point the implementation table at the post-split paths Every row still named the deleted server/bare files; they now name the packages/inference sources, and the releaseTurn rows state the actual guarantee — a thrown overflow or admission refusal never persists the in-flight turn — rather than a prefill-only rationale. * QVAC-24251 feat[api]: make the sizes record the canonical ContextOverflowError form The four measurements form one record, so the constructor's canonical overload takes it whole — new ContextOverflowError(contextSizes, modelId, cause) — and the plugin throw sites pass the parser result as one expression with no extraction. The positional form stays as a deprecated overload (first-arg type discriminates; one normalizer feeds both), so every existing caller keeps working, pinned by the kept positional tests alongside new record-form ones. lunte's constructor-super rule false-positives on bodiless overload declarations; disabled per line with the repo idiom. * QVAC-24251 fix: regenerate the Python client over the merged schema The main merge brought AudioGen models generated before the subclass-enum flag, so the committed file no longer matched a fresh run of the pinned generator and the required Python check failed. Their enums subclass str now like the rest; generate.py --check is clean. * QVAC-24251 fix: cover the batcher submit refusals and require the real status The scheduler also refuses at submit with 'failed to add to batch (MultiRequestBatcher::AddStatus=N)' — no free slot, or addRequestAt plan validation — before anything persists, so those deleted the committed cache too. The detector now requires the addon's real InvalidArgument status code AND one of the enumerated, message-anchored submit forms, since it picks preserve over delete. Positive cases per form, near-miss cases (missing code, unlisted wording, wrapped message), the fakes carry the production LLM status codes, and both survival regressions assert the committed bytes are still on disk between the refusal and the retry. * QVAC-24251 fix: export the sizes record type and tighten the legacy overload ContextOverflowErrorSizes is re-exported from the inference surface and the SDK entry so consumers can type a reusable record; type-only imports in the tests pin both boundaries. The legacy overload's normalizer reads only its two extras fields instead of spreading, so a wider structurally assignable object can no longer override the positional arguments (pinned). The class JSDoc sits back on the class after the type insertion had detached it, and the session release test asserts the committed bytes are on disk, not just the registries. * QVAC-24251 doc: describe the current session layers and the parallel error contract The guide still described the retired three-layer model and the removed cachedMessageCounts map; it now names the session's actual layers, includes releaseTurn among the turn exits (source header too), points the delete-cache schema link at model-ops, and says the parallel admission refusals surface as generic InvalidArgument without typed sizes until the addon gives them a structured status. * QVAC-24251 fix: honest units on the mtmd single-prompt guard; cover generationParams refusals The single-prompt guard's 'tokens' figure is mtmd_helper_get_n_tokens — the same quantity the cached guard labels KV cells — so promptTokens stays unset there too, as the field contract already claimed. The generationParams apply step validates against local copies before touching live state (reachable at parallel = 1 via responseFormat), so its two refusal wordings join the pre-mutation set; the detector is renamed isAddonPreMutationRefusal to state the guarantee it checks. A third survival regression drives the json_schema refusal through the real completion path, and the guide names the broadened trigger set. * fix: match pre-mutation refusals on the transported message alone The async addon transport delivers exception.what() without the status code, so the detector checks a code only when one is present and matches complete, end-anchored per-guard wordings (batcher AddStatus limited to its real error values). The survival regressions and predicate tests now throw the code-less production error shape. * doc: align kv-cache docs with conditional release and tighten comments The handler-loop snippet and KV-path overview show the conditional releaseTurn/rollback unwind, the parallel note reflects the untyped transported errors, session-test wording drops the retired three-layer model, and comments added by this branch are trimmed to two lines. * QVAC-24251 test: pin the destructive rollback branch; fix stale doc claims An unrecognised addon failure must unlink the cache and force a cold retry — a new regression pins that branch (mutation-verified). The guide row no longer claims an InvalidArgument status requirement, the detector comment says 'disk save' (cursor snapshots precede the cap checks), and two weak test assertions are tightened. * QVAC-24251 fix: preserve the warm cache on a missing-attachment rejection; anchor the overflow fallback AttachmentNotFoundError is caller input rejected before the addon runs, so its unwind now takes the non-destructive release instead of deleting the committed cache. The overflow message fallback matches the guards' emitted starts (single-line) instead of an unanchored substring, the n_predict slot requires a positive value, and the byte-survival tests compare content, not just existence. * QVAC-24251 mod: restore no-op .strict() calls and pre-existing comment text The two .strict() calls on the now-strict base and the tails of two pre-existing comments were removed as redundant cleanup; the task did not require those lines to change, so they are restored verbatim (with only the factual corrections kept). No behavior change: unknown keys still reject on every path, contract unchanged. * QVAC-24251 fix: treat a present status code as authoritative in overflow detection A contradictory structured code no longer falls through to the message match, and embedded carriage returns are excluded from the overflow tails. The guide and handler comments name the missing-attachment release, the attachment regression asserts the error class, and a new test pins that attachments inside the committed prefix are never re-read from disk. * QVAC-24251 fix: reject non-string status codes in overflow detection A defined non-string code no longer falls through to the message match, mirroring the refusal detector. Structured-code fixtures use the released addon identifier, the module header states that codes survive only the synchronous throw path, and the cached-attachment fixture removes its temporary directory and registry state in a finally. * QVAC-24251 fix: drop the added strictness and follow the retired-key precedent Removing a config key has never added strictness — toolsMode (#3380), n_discarded's original #3999, and load_mode (#4078, where the same strictification was added and reverted) all deleted the field and stopped. The long-standing strict options schema still rejects the key for TS/JS callers with code 50010; other callers get it stripped, as before. The base schema, dispatch, the exported contract, and the generated Python client return to their previous shape, and the tests that only existed to prove the strictness go with them. * QVAC-24251 fix: roll back a cache the failing turn itself primed releaseTurn could not tell a committed cache from one freshly primed by the request that then failed, so repeated invalid first requests under unique custom keys accumulated orphan cache files. beginTurn records the prime on the turn state and releaseTurn takes the destructive path for it; a refused first turn now leaves no cache and the retry re-primes. Pinned at the session and handler layers. * QVAC-24251 chore: drop dead code and stale test descriptions The parser guard's zero-groups disjunct was unreachable (every pattern captures at least two), two test comments described a construction the canonical constructor no longer produces, the code-gate test title still described the retired tail-anchoring (a wrapped code is now pinned as a rejection, and an explicitly undefined code as the async shape), and the transport tests' pre-existing enum usage is restored verbatim. * QVAC-24251 doc: restore pre-existing comment content the comments pass over-trimmed The ContextOverflowError class docs in both mirrors kept their original UX-guidance and serialization/reconstructor paragraphs (only the factually changed sentences differ), and the KV-path overview keeps the original exit-path enumeration, updated for the conditional unwind. * QVAC-24251 fix: return the load-model request union to its previous shape The structural builtin exclusion, the wire arms' modelConfig optionality, and the str-subclass enum generation were consequences of the dropped strictness, not of removing n_discarded — the catch-all leak they fix predates this change and belongs in its own change, like the dispatch fixes #4078 deferred. load-model.ts, generate.py, and _api.py return to their previous shape verbatim; the contract and the generated Python client now differ from the base only by the removed field, and the tests that only existed to prove the union changes go with them. * QVAC-24251 test: pin stripping and the auto-path fresh prime; restore Black-canonical layout The transport test file returns byte-identical to its previous state (the enum restore had kept a reformatted assertion layout the pinned Black rejects). New regressions pin the deliberate retirement shape — the retired key strips on the wire, in deviceDefaults, in config resolution, and in the generated Python model — and the auto-cache branch of the fresh-prime rollback (file, init flag, and retention marker all cleared). * QVAC-24251 feat[api]: type batch capacity refusals as overflows; preserve cache on media-load failures The scheduler's per-sequence-cap refusals are the same out-of-context condition the model guards report, so their wordings join the overflow forms and the parser maps cap to ctxSize (reservation plus prompt on the n_predict form) — previously the identical conversation was typed at parallel 1 and untyped above it. Interim wording-based fix; the addon carrying a real ContextOverflow status stays the recorded follow-up. The multimodal media-load failures join the pre-mutation refusals: they reject before any decode or save, like the SDK-side missing attachment, so the committed cache now survives them. * QVAC-24251 fix: resolve error reconstructors from own keys only The reconstructor map is an object literal, so a hostile envelope name like "constructor" resolved through Object.prototype and returned a non-Error. Own-key gate plus a fall-through regression. * QVAC-24251 doc: state the true rationale on the overload and factory branches * QVAC-24251 doc: reword two comments to state behavior, not process --------- Co-authored-by: iancris <17702377+iancris@users.noreply.github.com>
🎯 What problem does it solve?
Companion to #3373 (addon-side removal). The SDK's dynamic tools mode (
toolsMode: 'dynamic') maps to the llm-llamacpptools_compactfeature, which only works for Qwen3 and relies on sliding-context anchor bookkeeping that does not carry over to newer LLM models. This PR removes the SDK-side surface of the feature. SDK surface was introduced in #1779 (QVAC-13559).📝 How is it solved?
toolsModeconfig field from the llamacpp model config schema and theTOOLS_MODE/ToolsModeroot exports (breaking)appendToolsToHistoryand the dynamic branches incompletion-stream/batch-completion-stream; tools are always prepended after the system message (the previous default)tools_mode→tools_compactaddon config mapping intransform.tstools-simple-function-dynamic,kv-cache-tools-dynamic-reuse), thetools-dynamicconsumer resources, andexamples/llamacpp-dynamic-tools.tscontract/schema.jsonand the sdk-python bindings (ToolsModeenums /tools_modefield gone)toolsMode: 'static'pin (the field no longer exists; static is the only behavior)🧪 How is it tested?
bun run build(eslint + tsc), unit tests, prettier, andcontract:checkall greenscripts/generate.py --checkgreengenerateConfigHashunit tests (both trees) pin the cache-identity contract: a changed parameter schema mints a new cache, object-key insertion order does not, and tool-array order doeskv-cache-tools-sequential-savenow also asserts a declared tool call on both rounds; sampling is pinned (temp: 0, top_k: 1, seed: 42) so the assertion does not depend on the model choosing to call the toolYes — the
TOOLS_MODEvalue andToolsModetype root exports are removed (compile-time breaking for any TypeScript consumer importing them), and thetoolsModemodel-config field is gone.Stale-config behavior: a leftover
toolsModekey is rejected, not ignored.loadModelvalidates built-in model configs throughloadBuiltinToRequestSchema, whose llm branch appliesllmConfigBaseSchema.strict(), so an unknown key fails validation and throwsRequestValidationFailedErrornamingmodelConfig.toolsModebefore the request reaches the server. Callers must remove the key. This matches the addon: #3373 landed without thetools_compacttombstone (see its final commit), and the@qvac/llm-llamacpp0.43.0 changelog states that configurations passingtools_compact"now fail model loading as an unsupported option". Both layers reject rather than degrade.One asymmetry worth knowing: SDK config files are parsed through the non-strict
llmConfigBaseSchemainsdk-config.ts, which silently strips unknown keys. AtoolsModeentry in a config file is therefore dropped and the model loads with static placement, while the same key passed directly toloadModelthrows. Both outcomes are safe — thetools_mode→tools_compactmapping is gone, so nothing stale can reach the addon — but the error surfaces only on the direct-call path.KV-cache impact (one-time, all existing caches).
generateConfigHashnow digests the complete canonical tool definitions instead of just tool names, so a tool whose parameters or description change no longer reuses a cache whose prefix holds the old tool block. The hash payload key also changed fromtoolNamestotools, so the serialized form differs from the previous release for every input — including an empty or omitted tool list. BecauseconfigHashis the.binfilename, every cache written by an earlier release is invalidated on upgrade, tool-using and plain-chat sessions alike: each re-primes once under a new filename, the old file is left behind, and since retention only reclaims auto-cache markers, caller-owned string keys keep those orphans untildeleteCacheremoves them. Going forward, tool-array order and any change to a tool's name, description, or parameters also mint a new cache, so a caller that varies its tool set per turn (as formerdynamicusers did) gets a fresh cache file rather than reusing one. Correctness-preserving throughout — the cost is a cold cache and some reclaimable disk.BEFORE:
AFTER:
📦 Release coordination (deferred, no version bump in this PR)
Per the no-version-bump plan, these are captured at the next coordinated release rather than in this PR:
TOOLS_MODE/ToolsModeexports +toolsModeconfig field, state that the key must be removed (it is rejected, not ignored), note the one-time KV-cache invalidation described above, and version the release as breaking.toolsMode: 'static'pin was dropped (behavior-neutral —staticwas the default and the key no longer exists) at the plugin's next release.@qvac/sdkrelease that advances its@qvac/llm-llamacppdependency past the addon'stools_compactremoval must include this change, so no build pairs the oldtools_mode→tools_compactmapping with a new addon. This cannot happen silently — the addon shipped the removal as a minor bump (0.43.0) and the SDK pins^0.39.3, which caret-on-zero-major caps below 0.40.0 — so the requirement binds to the PR that raises that range, not to any existing published build.