Skip to content

feat(lemonade): native Lemonade server integration#8319

Open
abn wants to merge 1 commit into
janhq:mainfrom
abn:feat/add-lemonade-provider
Open

feat(lemonade): native Lemonade server integration#8319
abn wants to merge 1 commit into
janhq:mainfrom
abn:feat/add-lemonade-provider

Conversation

@abn

@abn abn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Describe Your Changes

Adds Lemonade as a built-in model provider for local AI inference. Models and their capabilities (vision, tools, audio) are discovered automatically from the running server. Supports both OpenAI and Anthropic wire formats, and optionally registers Lemonade's MCP gateway per project.

Screencast.From.2026-06-18.02-08-25.mp4

PS: Hope this is a welcome addition, happy to field any review comments.

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed (N/A)

@tokamak-pm

tokamak-pm Bot commented Jun 18, 2026

Copy link
Copy Markdown

PR Review: feat(lemonade) — native Lemonade server integration

This PR adds Lemonade (a local AMD/llama.cpp inference server) as a first-class built-in provider alongside llamacpp and mlx. It wires up automatic model discovery via the /v1/models endpoint, capability inference from Lemonade's label taxonomy, optional MCP gateway registration, and both OpenAI and Anthropic wire-format switching. Documentation and tests are included.


What's good

  • Follows existing patterns well. The BUILTIN_CAPS, LOCAL_PROVIDER_IDS, getProviderLogo, getProviderTitle, and supportsRemoteCatalog extension points are all touched in the expected places.
  • inferLemonadeCapabilities is well-designed. The NON_CHAT_LABELS guard-first pattern mirrors what inferOpenAICapabilities does, and returning null to signal "skip this model" is idiomatic for the catalog normalizer.
  • No TOP_N cap for Lemonade — correct, since the server only exposes what is installed locally; slicing to 10 would silently hide models.
  • Auth header omitted when api_key is empty — the existing buildHeaders already handles this correctly; the tests confirm the right behaviour.
  • Test coverage is solid. Unit tests cover capability inference, the filtering of non-chat model types, the MCP toggle path, and the api-formatapi_type update path.

Concerns / issues to address

1. useMCPServers.getState() called inside a React event handler (Rules of Hooks violation risk)

In $providerName.tsx the new Lemonade-specific branches call useMCPServers.getState() directly inside the onChange callback of a rendered DynamicControllerSetting. This is fine at runtime (Zustand's getState() is not a hook), but the import at the top of the file (import { useMCPServers } from '@/hooks/useMCPServers') combined with naming the export useMCPServers (a hook-shaped name) can confuse linters and future readers. This pattern works, but it is worth a comment clarifying it is called as a store singleton, not as a hook.

2. Duplicated MCP URL construction logic

The base-URL-change handler and the mcp-enabled toggle handler both independently derive mcpUrl from new URL(...).origin + '/mcp' with a fallback. If either copy drifts, MCP will be registered at the wrong endpoint. Extract a small helper (lemonadeMcpUrl(baseUrl: string): string) and call it from both branches.

3. isPredefinedRemoteProvider returns true for lemonade after this PR

The providerCaps.ts patch adds lemonade: CUSTOM_PERMISSIVE to BUILTIN_CAPS but also adds 'lemonade' to LOCAL_PROVIDER_IDS, which correctly makes isPredefinedRemoteProvider return false. However, the test confirms this expectation. What is missing is a check that CUSTOM_PERMISSIVE is the right caps profile: Lemonade speaks the standard OpenAI chat-completions schema and supports all common samplers, so CUSTOM_PERMISSIVE (everything in maybe) is reasonable, but there is no comment explaining why it was chosen over LLAMACPP (which is already in BUILTIN_CAPS). A one-line comment would help future maintainers.

4. providerCanFetchWithoutKey is a hard-coded string comparison

The function providerCanFetchWithoutKey(providerName: string): boolean { return providerName === 'lemonade' } works but sets a precedent of adding more string literals here as new key-optional providers arrive (e.g., future Ollama integration). Consider storing a Set<string> of key-optional providers, consistent with how LOCAL_PROVIDER_IDS works in providerCaps.ts. Not a blocker, but worth flagging now.

5. api-key setting is rendered hidden for lemonade but still present in the settings array

The provider detail component skips rendering the api-key card item when provider.provider === 'lemonade', which is good UX. However the setting remains in the persisted settings array and the api_key field is still an empty string. If a user later sets a key via the API or CLI they will see no way to clear it through the UI. The placeholder text already hints this is optional — consider leaving the field visible but unfilled rather than hiding it entirely, consistent with how the field is documented ("Leave empty if not configured").

6. Missing auto-catalog trigger for Lemonade on initial load

The useEffect that auto-triggers handleRefreshModels when the provider page opens was patched for the guard condition (!providerHasRemoteApiKeys && !providerCanFetchWithoutKey), but its dependency array only watches provider?.api_key and provider?.models.length. For Lemonade, api_key is always '' so the effect fires once on mount — which is correct — but only if the user navigates to the Lemonade settings page. The Lemonade models therefore appear in the model picker only after that visit. This is consistent with how OpenAI works before a key is entered, but it means first-run UX requires a manual visit. Consider documenting this in the MDX or adding a note in the settings description.

7. Minor: handleStopModel comment removal

The diff removes a stale // Original: stopModel(modelId)... comment. This is a drive-by cleanup — fine, just noting it is intentional.


Summary table

Area Status
Correctness Good — capability inference, catalog fetching, MCP wiring all sound
Architecture fit Good — follows established provider extension patterns
Duplicated MCP URL logic Needs cleanup
providerCanFetchWithoutKey scalability Minor nit
Hidden API key field inconsistency Minor UX concern
Tests Good coverage across unit and integration
Docs Present and accurate

Recommendation: improve needed

@abn abn force-pushed the feat/add-lemonade-provider branch from eb3632b to 5197389 Compare June 18, 2026 10:54
@abn

abn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in the latest commit:

  • MCP URL duplication — extracted lemonadeMcpUrl(baseUrl) helper used by both handlers; also fixes a missing try/catch in the base-url branch
  • useMCPServers.getState() clarity — added inline comment at both call sites noting it's a Zustand store singleton
  • CUSTOM_PERMISSIVE rationale — one-line comment in providerCaps.ts explaining the choice vs LLAMACPP
  • providerCanFetchWithoutKey — converted to KEY_OPTIONAL_PROVIDERS = new Set([...]) matching the LOCAL_PROVIDER_IDS pattern
  • Auto-catalog UX — added a <Callout> in the docs noting models are fetched on first settings page visit

Finding 5 (api-key field hidden): The field is actually already visible for Lemonade — the condition returns null only when the provider is not in {llamacpp, mlx, lemonade}, so Lemonade's api-key renders inside the settings card. The dedicated multi-key rotation card is intentionally hidden (that's for paid cloud providers). No change needed there.

@abn abn force-pushed the feat/add-lemonade-provider branch from 5197389 to bcfa3ec Compare June 19, 2026 01:17
@abn

abn commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Updated to ensure model's context size flows through to Jan.

@tokamak-pm

tokamak-pm Bot commented Jun 19, 2026

Copy link
Copy Markdown

Follow-up Review — All feedback addressed

Re-reviewing after author's updates (Jun 18-19 comments).

Previous review items — status

# Item Status
1 useMCPServers.getState() clarity ✓ Fixed — inline comment added at both call sites
2 Duplicated MCP URL construction ✓ Fixed — extracted lemonadeMcpUrl(baseUrl) helper with added try/catch
3 CUSTOM_PERMISSIVE rationale ✓ Fixed — one-line comment explaining why not LLAMACPP
4 providerCanFetchWithoutKey scalability ✓ Fixed — converted to KEY_OPTIONAL_PROVIDERS = new Set(['lemonade'])
5 Hidden API key field ✓ Clarified — our review misread the condition; the code shows the api-key field for lemonade and hides the multi-key rotation card. No change needed
6 Auto-catalog trigger documentation ✓ Fixed — <Callout> added to docs noting first-visit fetch
7 handleStopModel comment removal ✓ Acknowledged as intentional cleanup

Additional improvement

The author added context length flow-through in custom-chat-transport.ts — falls back to selectedModel?.contextLength when max_context_tokens is not explicitly set. This prevents silent context overflow for models that report their window size. Clean defensive enhancement.

Test coverage

Comprehensive tests added:

  • remoteModelCatalog.test.ts — 142 lines covering context length, label mapping, exclusion logic
  • providerCaps.test.ts — CUSTOM_PERMISSIVE verification, provider classification
  • provider-api-keys.test.ts — Set-based key-optional logic
  • $providerName.test.tsx — 167 lines covering api-format update, MCP toggle, URL construction

Recommendation: can merge

All 7 review items resolved. Code follows existing provider extension patterns consistently. Test coverage is thorough. The additional context-length improvement is a quality enhancement. No remaining blockers.

@abn

abn commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@qnixsynapse would be awesome to get your take on this. I was not sure what the general policy around new providers are. This would really help folks using self hosted models via Lemonade. Especially the label syncing and mcp.

I am happy to field any review feedback you might have.

@qnixsynapse

Copy link
Copy Markdown
Contributor

@abn Wouldn't custom providers using the + button work with this provider?

@abn

abn commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Since it has OpenAI compatible endpoints in addition to others, yes it will technically work as a custom addition.

The biggest motivation for having this as a first-class provider is the ability to auto sync capabilities like tool, vision etc. It is a bitt challenging to go do that manually for all the available models and new ones that get added etc 😅. Especially since (like in the example) quite a lot might be available.

This bit me when I was attempt to use the browser mcp, as it needed the models to be configured as being vision capable. I had to do this manually before every switch.

The other aspects that I believe make the case are the ability to switch wire protocol and also the integration of the MCP server as a native feature.

Hope that helps clarify your question.

@tokamak-pm

tokamak-pm Bot commented Jun 22, 2026

Copy link
Copy Markdown

Follow-up Review -- Architectural discussion (built-in vs custom provider)

Catching up on the maintainer discussion between @abn and @qnixsynapse since our last review.

The question: could the custom providers + button handle this?

@qnixsynapse raises a valid point. Since Lemonade exposes OpenAI-compatible endpoints, a user can add it as a generic custom provider today. This is the lowest-maintenance path for the Jan team -- zero new code to own.

However, @abn's response outlines concrete capabilities that the custom-provider path cannot deliver:

Capability Custom provider + First-class integration
Basic chat completions Yes Yes
Auto-discover models from /v1/models Yes (generic fetch) Yes, with capability inference (vision, tools, audio parsed from Lemonade's label taxonomy)
Wire-format switching (OpenAI / Anthropic) No -- custom providers are OpenAI-only Yes -- api-format setting toggles api_type
MCP gateway registration No -- requires custom UI toggle and useMCPServers integration Yes -- one-click toggle per project
Context length flow-through Partial -- user must set manually per model Yes -- auto-read from server metadata
Capability-gated features (e.g., browser MCP needing vision) No -- user must manually flag each model Yes -- inferred automatically from labels

The auto-capability-inference point is the strongest argument. As @abn noted, without it a user has to manually mark every model as vision/tool-capable before features like browser MCP work, and redo that configuration on every model switch. That is a meaningful UX gap.

Trade-offs for the Jan team

In favor of merging:

  • The implementation follows existing provider patterns faithfully (same extension points as llamacpp/mlx)
  • Maintenance surface is modest: ~690 lines added across 16 files, with good test coverage
  • Lemonade is an active open-source project with a stable API surface
  • The MCP gateway integration is genuinely novel -- no other built-in provider offers this

Arguments for caution:

  • Each built-in provider is a long-term maintenance commitment (API changes, bug reports, docs updates)
  • If this sets a precedent, other local inference servers (Ollama, vLLM, LocalAI, etc.) may request similar treatment
  • The Jan team would need to decide on a policy for when a provider "earns" first-class status vs using the custom + flow

Code quality assessment

Regardless of the architectural decision, the implementation quality is solid:

  • All 7 items from our initial review were addressed
  • Test coverage is comprehensive (unit tests for capability inference, provider classification, MCP toggle, URL construction)
  • The lemonadeMcpUrl helper extraction and KEY_OPTIONAL_PROVIDERS Set pattern improved code structure beyond the original submission
  • Context length flow-through was added as a bonus improvement
  • No new commits since our last review on June 19 -- the code is stable

No new commits

The head SHA remains bcfa3ec0 (last amended June 19). No additional code changes since our "can merge" review.

Recommendation: can merge (from a code quality standpoint)

The code is clean, well-tested, and follows established patterns. The outstanding question is purely architectural -- whether the Jan team wants to accept a new built-in provider. That decision is @qnixsynapse's (and the broader team's) call based on their provider policy. From a technical review perspective, this is ready.

@abn

abn commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@qnixsynapse wanted to check-in to see if you had any further thoughts on this. It would definitely be a huge quality of life improvement for Lemonade users.

Adds Lemonade as a built-in model provider for local AI inference.
Models and their capabilities (vision, tools, audio) are discovered
automatically from the running server. Supports both OpenAI and
Anthropic wire formats, and optionally registers Lemonade's MCP
gateway per project.
@abn abn force-pushed the feat/add-lemonade-provider branch from bcfa3ec to 82abe56 Compare June 25, 2026 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants