CL-6477: fix embeddinggemma escaping the embedding-model filter - #209
Merged
Merged
Conversation
CL-6477: embeddinggemma:300m has no delimiter between "embedding" and the "gemma" family name, so the current EMBEDDING_MODEL_NAME_PATTERN (which requires a trailing delimiter) never catches it. It then wins the alphabetical default-model tiebreak and answers every chat turn with "does not support chat".
…imiter The embedding-model name filter required a delimiter after every family it recognized, including embed(ding)?. That's correct for the short abbreviations (minilm, bge, gte, e5, arctic-embed), which are prone to false positives as bare substrings, but embeddinggemma:300m has none between "embedding" and "gemma" and slipped through. Capability data (when a deployment carries any) still takes priority in isCompletionCapable; this pattern is only the belt-and-braces fallback for the uncataloged case.
An overnight run across the candidate Ollama models found gpt-oss:20b correct and tool-obedient, and 12-25x faster than the prior curated default qwen3.8:27b (8s vs 100-216s for the same task). Leads the curated list now; qwen3.8:27b and qwen3.5:9b-mlx stay as fallbacks. Kept as its own commit so it can be reverted independently of the embedding-model filter fix.
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
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.
Summary
EMBEDDING_MODEL_NAME_PATTERNinpackages/hub-client/src/model-capability.tsrequired a trailing delimiter after every family it matched, includingembed(ding)?. Google'sembeddinggemma:300mhas none between "embedding" and "gemma", so it survived the filter, then won the alphabetical default-model tiebreak — every Myra-created agent on a box with this model pulled answered every chat turn with"embeddinggemma:300m" does not support chat.embed(ding)?specifically (the short abbreviationsminilm/bge/gte/e5/arctic-embedkeep the stricter, delimiter-bound match — they're prone to false positives as bare substrings). Capability data, when a deployment carries any, already takes priority over this name check inisCompletionCapable— this pattern is only ever the belt-and-braces fallback for the uncataloged case (an unprobed local Ollama pull).gpt-oss:20bas the new curated Ollama default inpackages/hub-client/src/catalog-seed-data.ts— an overnight run found it correct, tool-obedient, and 12-25x faster than the prior defaultqwen3.8:27b(8s vs 100-216s for the same task). Kept in its own commit so it can be reverted independently.What I verified
packages/hub-client/src/model-capability.test.tscover:embeddinggemma:300mis never selected as a chat default even with no delimiter after "embedding"; the other embedding names (nomic-embed-text,all-minilm,bge-m3,qwen3-embedding) still get filtered (no regression); a chat-capable model is selected when both kinds of candidate are present; and probed capability data (when present) still wins over the name fallback.bun testgreen acrosspackages/hub-client,packages/chat,packages/inference-settings,packages/onboarding,packages/inference-catalog,packages/provider-pricing(1046 pass, 0 fail) — updated one existing onboarding test (complete-credential.test.ts) that hard-coded the old curated-default name.tsc --noEmitclean forpackages/hub-clientandpackages/onboarding.bun run lintfrom the repo root: 0 errors (12 pre-existing warnings in unrelated files, not introduced by this change).What I did not verify
bun run check(typecheck across every workspace) — scoped to the affected packages plus repo-root lint, given the timebox.OLLAMA_CAPABILITY_MAPinpackages/hub-client/src/credential-test.tshas no entry for Ollama's own"embedding"capability (onlycompletion/tools/visionare translated), so a genuinely-probed embedding-only Ollama model whose name doesn't match the name-filter at all would still collapse to an empty capability list — indistinguishable from "never probed" — and fall through to the (now-fixed) name heuristic. Closing that gap for real would need a new signal that Interchange's ownCapabilityenum (@intx/types, never to be modified here) has no room for, likely a schema/DB-level change — bigger than this hotfix's scope. Flagging for a follow-up ticket rather than expanding this PR.Not merging
Per instructions for this lane, not merging — a peer reviewer should merge after checking the above.