QVAC-23811 doc: add VisionPsy Nano and image_no_upscale to multimodal page - #3959
Merged
Merged
Conversation
- Add VisionPsy Nano (base and Flash) as a recommended VLM pair in the
Models section of ai-capabilities/multimodal.mdx, alongside the
existing Qwen2.5-Omni / Qwen3-VL and SmolVLM2 entries. SmolVLM2 stays
listed to remain consistent with the embedded example, which still
uses it.
- Add a "VisionPsy Nano: base vs. Flash" subsection documenting that
the two variants share the same mmproj and are selected at load time
via modelConfig.image_no_upscale ('off' = base, 'on' = Flash).
- Keep the same level of detail as the rest of the page: no per-field
reference for other modelConfig options, no sizing or usage guidance
beyond what is strictly needed to distinguish the two variants.
Contributor
Review StatusCurrent Status: ✅ APPROVED |
Contributor
License compliance — cleanNo new dependency license findings in this PR. 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):
|
iancris
reviewed
Aug 20, 2026
Address PR #3959 review feedback (@yingying0906, @iancris): - Name the exact weights + mmproj constants for each VisionPsy variant under "Recommended pairs", and flag the counterintuitive naming (un-suffixed constants are Flash; `_1`-suffixed constants are base). - Correct the "base vs. Flash" subsection: base and Flash are separate weights + `mmproj` pairs (not a shared mmproj); the variant is chosen by loading the matching pair, and `image_no_upscale` is the preprocessing-rule flag that must match the loaded pair — the two mmprojs' identical metadata cannot express this on its own. - Drop the misleading `'off' (default)` / `'on'` list. The field is optional; unset defers to the mmproj's own value. New guidance: leave unset for base, set `image_no_upscale: 'on'` for Flash. - Add an explicit warning that a mismatch between the loaded pair and the flag passes load validation (both mmprojs declare the same `preproc_image_size`) but silently uses the wrong preprocessing rule and degrades quality. - Scope `image_no_upscale` to the idefics3-style preprocessor and to a `mmproj` that declares `clip.vision.preproc_image_size`. Note that on the other recommended pairs the flag either warns and is ignored (Qwen2.5-Omni / Qwen3-VL) or is accepted but inert (SmolVLM2) — so in practice a VisionPsy-only key.
iancris
previously approved these changes
Aug 20, 2026
Co-authored-by: Law Po Ying <30721578+yingying0906@users.noreply.github.com>
iancris
approved these changes
Aug 20, 2026
iancris
approved these changes
Aug 20, 2026
yingying0906
approved these changes
Aug 20, 2026
gianni-cor
approved these changes
Aug 20, 2026
donriddo
added a commit
that referenced
this pull request
Aug 27, 2026
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.
This was referenced Aug 27, 2026
donriddo
added a commit
that referenced
this pull request
Aug 28, 2026
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
ai-capabilities/multimodalpage recommends SmolVLM2 and Qwen as the multimodal LLM pair but does not mention the new VisionPsy Nano VLM or itsimage_no_upscaleload option, which is required to distinguish the base variant from Flash (both share the samemmprojhparams).How does it solve it?
VisionPsy Nano (base or Flash) + mmproj-*as a recommended pair alongsideQwen2.5-Omni / Qwen3-VLandSmolVLM2. SmolVLM2 stays listed so the recommended pairs remain consistent with the embedded example, which still uses it.mmprojand are selected atloadModel()time viamodelConfig.image_no_upscale('off'= base,'on'= Flash).modelConfigoptions; only what is needed to distinguish the two variants. A broader "load options" reference is out of scope for this task and will land once the AI-capabilities pages get a standardized structure.