fix(local-inference): graceful non-speculative fallback for a missing MTP drafter GGUF — #11517 back-compat, never brick an installed model - #11873
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…l non-speculative fallback for a missing drafter GGUF (#11517 back-compat) The Gemma-4 MTP cutover (#11517, then #11782 for 4b) added eliza-1-2b/4b to ELIZA_1_HOSTED_MTP_TIER_IDS, so the catalog now declares separate-drafter runtime.mtp for those tiers. resolveLocalInferenceLoadArgs then HARD-THREW for any bundle missing mtp/drafter-<tier>.gguf — which is every bundle installed before the cutover — so a previously-working installed model could no longer load at all (2 red active-model.test.ts cases caught exactly this). The drafter is a perf-only speculative-decoding artifact (~1.3-1.6x greedy speedup). Missing drafter now means: load the text model without MTP, emit a console warning pointing at re-download to pick the drafter up — never throw. The fallback also covers no-bundleRoot installs (external-scan/single-file), which previously leaked a half-configured speculative state (draft window + mobileSpeculative with NO draft model) into the loader. Applied to both copies of the resolver (packages/ui client mirror and plugins/plugin-local-inference runtime) and their suites: - ui active-model.test.ts: split the stale pre-cutover tests into hosted-tier-enables-MTP / unhosted-tier-ignores-stray-drafter / pre-cutover-bundle-falls-back (warn asserted); 17/17 green. - plugin mmproj-routing.test.ts: + pre-cutover fallback case; 10/10 green. - plugin load-args-drafter.fuzz.test.ts: throw assertion -> fallback assertion; external-scan case now expects a fully-unset MTP block; 22/22. Signed: nubs-cloud [cloud-frontdoor] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a83c1bc to
149eaa1
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
What
The Gemma-4 MTP cutover (#11517, extended to 4b by #11782) added
eliza-1-2b/eliza-1-4btoELIZA_1_HOSTED_MTP_TIER_IDS, so the shared catalog now declares separate-drafterruntime.mtpfor those tiers.resolveLocalInferenceLoadArgsthen hard-threw for any installed bundle missingmtp/drafter-<tier>.gguf— which is every bundle downloaded before the cutover — so a previously-working installed model could no longer load at all. Twopackages/uiactive-model.test.tscases were red on develop catching exactly this regression.Why graceful fallback (not a throw)
The drafter is a perf-only speculative-decoding artifact (measured ~1.3-1.6x greedy speedup, #11517 evidence). A missing drafter degrades speed, not correctness — bricking the user's installed, previously-working model over it is the wrong UX. The resolver now:
mobileSpeculative), andconsole.warntelling the user to re-download the model to enable the MTP drafter (a fresh download bundles it, since the catalog advertisescomponents.mtpfor hosted tiers).The fallback also closes a pre-existing hole: a no-
bundleRootinstall (external-scan / single-file GGUF) of a hosted-MTP tier previously leaked a half-configured speculative state (draftMin/draftMax/mobileSpeculativeset with NOdraftModelPath) into the loader. Now the whole MTP block is skipped whenever the drafter isn't on disk. (The capacitor-bridge bootstrap already behaved this way —plugins/plugin-capacitor-bridge/src/mobile-device-bridge-bootstrap.tsonly passes MTP args when the drafter resolves — so this aligns the desktop/UI resolvers with the mobile path.)Applied to both copies of the resolver:
packages/ui/src/services/local-inference/active-model.ts(client mirror) andplugins/plugin-local-inference/src/services/active-model.ts(runtime).Tests
packages/uiactive-model.test.ts— the two stale pre-cutover tests are replaced with the post-cutover contract: hosted tiers with the drafter bundled enable MTP (path + catalog draft window), unhosted tiers ignore a stray on-disk drafter, and a pre-cutover bundle (no drafter) falls back with the warning asserted.bun run --cwd packages/ui test active-model→ 17/17 (was 14 passed / 2 failed).plugins/plugin-local-inferencemmproj-routing.test.ts— new pre-cutover fallback case → 10/10.plugins/plugin-local-inferenceload-args-drafter.fuzz.test.ts— the throw assertion becomes the fallback assertion; the external-scan case now expects a fully-unset MTP block → 22/22.src/services/local-inference84/84, pluginactive-model*+text-bundle+ensure-assigned-model-loadedall green.typecheckgreen in both packages; biome clean on all touched files.Evidence
c5e1ca53e5):bun run --cwd packages/ui test active-model→ 2 failed (AssertionError: expected '/tmp/.../mtp/drafter-2b.gguf' to be undefinedandError: [local-inference] eliza-1-2b declares a separate-drafter MTP but no bundled drafter GGUF was found under /tmp/...).mtp/dir — byte-identical to a pre-cutover install layout) is exercised by real resolver calls against the real shared catalog in three suites, including the warn text.Fixes the 2 red
active-model.test.tscases on develop; back-compat regression from #11517.Signed: nubs-cloud [cloud-frontdoor]
🤖 Generated with Claude Code