Repository navigation
feat: connect local llama.cpp, vLLM, Ollama and LM Studio servers from Settings - #910
Anatoly (Till) Lazarev (lazarevtill) wants to merge 2 commits into
Conversation
Olga Lavrichenko (OLavrik)
left a comment
Olga Lavrichenko (OLavrik)
left a comment
There was a problem hiding this comment.
Thanks for this. The approach is solid: pi's models.json as the single source of truth, a sidecar only for what pi can't hold, and limits applied once at the runtime dispatch level so every caller is covered.
I'd like a few things addressed before merge, mostly around timeouts, the probe key and write consistency. Details are inline, marked Must fix / Should fix / Nit.
| setTests((current) => ({ ...current, [key]: { running: true } })); | ||
| const settle = (next: TestState) => setTests((current) => ({ ...current, [key]: next })); | ||
| await getTransport() | ||
| .request("localModels.test", { id: endpoint.id, modelId: model.id }) |
There was a problem hiding this comment.
Must fix: Test times out in the UI after 60 s. localModels.test uses the transport's default 60 s timeout, while testLocalModel on the server is budgeted for max(180 s, timeoutSeconds) × (retries + 1). A model that cold-loads for ~90 s (common with llama.cpp / Ollama) is reported as failed, while the server keeps running the check and holding a gate slot. Please pass a timeoutMs that covers the server budget.
There was a problem hiding this comment.
Fixed in d631bd7. Test now passes timeoutMs: localModelTestBudgetMs(endpoint) + 15 s to localModels.test. The budget helper lives in contracts, so the client and host use the same number and the client always outlasts the host. In 7b0d8ea, Test also returns skipped right away (shown as "busy") when every slot is in use, instead of queueing behind a running turn.
| export function limitedFetch(providerId: string, base: Fetch = globalThis.fetch): Fetch { | ||
| const gate = gateFor(providerId); | ||
| const wrapped = (async (input: Parameters<Fetch>[0], init?: Parameters<Fetch>[1]) => { | ||
| const release = await gate.acquire(init?.signal); |
There was a problem hiding this comment.
Must fix: queue wait counts against the request timeout. The gate waits inside the fetch handed to the OpenAI SDK, and the SDK arms its timeout before calling fetch. With Parallel = 1 and Timeout = 60 s, a compaction or subagent request queued behind a 2-minute generation is aborted while still in gate.acquire, then retried by retryProviderRequest, which puts it back at the end of the queue. The server was never slow, but the request fails. The slot should be acquired outside the timed section, or the timeout should start after acquire.
There was a problem hiding this comment.
Fixed in d631bd7. With a parallel limit, the timeout moved into the limited fetch and starts only after a slot is acquired. The SDK's own timeout is set to the maximum timer, so queue time never counts and a queued request is never aborted and re-queued by a retry. 7b0d8ea adds an idle timer between streamed chunks, so a server that stops mid-stream frees its slot. Tests: "queue wait does not count against an endpoint's request timeout" and "a queued request can be aborted and a stalled or cancelled body frees its slot".
| const rawKey = | ||
| request.apiKey === undefined && request.endpointId | ||
| ? readEndpointKey(request.endpointId) | ||
| : request.apiKey; |
There was a problem hiding this comment.
Must fix: Detect sends the saved key to an edited URL. When editing an endpoint without re-entering the key, the UI sends { endpointId } (LocalModelsSettings.tsx:267) and this attaches that provider's saved key to whatever baseUrl the request carries. Changing the URL (or a typo) and clicking Detect sends the real bearer key to the new host. readEndpointKey also reads any models.json provider, not only local endpoints. Suggest reusing the saved key only when the normalized baseUrl matches the saved one, and only for providers returned by readLocalEndpoints.
There was a problem hiding this comment.
Fixed in d631bd7. The saved key is reused only when endpointId names a listed local endpoint and the normalized URL equals its saved baseUrl; any other URL is probed without a key. 7b0d8ea resolves that key through pi (runtime.getAuth) with a closed error that never echoes the key, and refuses URLs carrying user:pass@ or a query string. Tests: "a saved key is reused only for the endpoint's own unchanged URL", "a saved key pi cannot resolve fails Detect without exposing the key".
| if (next !== source) writeModelsFile(path, next); | ||
| result.settings(); | ||
| if (next !== source) { | ||
| await runtime.refresh({ allowNetwork: false }); | ||
| publishModelsFileChange(); | ||
| } |
There was a problem hiding this comment.
Must fix: models.json and the sidecar are written non-atomically. models.json is written first, then result.settings() writes the sidecar. If the sidecar write throws, runtime.refresh and publishModelsFileChange are skipped even though the provider is already on disk: the runtime is stale, other clients aren't notified, and a retry creates a duplicate (local-x-2). Either write the sidecar first, or make sure refresh/publish still run (e.g. try/finally) once models.json has changed.
There was a problem hiding this comment.
Fixed in d631bd7. The sidecar is written first and models.json second. If the models.json write fails, the previous sidecar entry is restored, so a retry can't add a duplicate provider. 7b0d8ea also refuses to overwrite a sidecar that exists but isn't valid JSON. Tests: "a failed settings write leaves models.json untouched, so a retry adds no duplicate", "an unreadable settings file is never overwritten".
| const previous = known.get(model.id); | ||
| const contextWindow = | ||
| model.contextWindow ?? previous?.contextWindow ?? LOCAL_MODEL_CONTEXT_LIMITS.fallback; | ||
| const keepEdits = previous !== undefined && !previous.detected; |
There was a problem hiding this comment.
Should fix: re-detect discards edited context / max output. keepEdits is true only when the previous row was not detected. After the first Detect, rows the server reported have detected: true, so a second Detect resets the user's edited values. The panels SPEC says edited context and selection survive a re-detect.
There was a problem hiding this comment.
Fixed in d631bd7. Each row carries an edited flag, set on any context or max-output edit and on rows loaded from a saved endpoint, and re-detect keeps edited values. The e2e spec now edits max output to 8192, re-detects, and checks the value stays.
| function editDraft(endpoint: LocalModelEndpoint): Draft { | ||
| return { | ||
| id: endpoint.id, | ||
| kind: endpoint.kind ?? guessKind(endpoint.baseUrl), |
There was a problem hiding this comment.
Should fix: editing a hand-written provider can rewrite its base URL. For a provider with kind: null, the kind is guessed from the port, and on save normalizeLocalBaseUrl appends /v1 for any kind other than openai-compatible. Opening Edit on http://gpu:8000/openai just to change the parallel limit saves http://gpu:8000/openai/v1. Defaulting unknown entries to openai-compatible, or not re-normalizing an unchanged URL, would avoid this.
There was a problem hiding this comment.
Fixed differently in 7b0d8ea. The list now shows only providers ThinkRail created (they have a sidecar entry with their kind), so a hand-written provider is no longer listed, editable or removable here, and pi keeps serving it untouched. Our own endpoints open on their saved kind, and an unchanged baseUrl is kept exactly as written. Test: "hand-written providers stay out of the list and cannot be edited or removed".
| if (thinking) { | ||
| next.thinkingLevelMap = { | ||
| off: "off", | ||
| minimal: null, | ||
| low: null, | ||
| medium: "medium", | ||
| high: null, | ||
| xhigh: null, | ||
| }; | ||
| next.compat = { | ||
| ...(isObject(previous?.compat) ? previous.compat : {}), | ||
| thinkingFormat: "qwen-chat-template", | ||
| }; | ||
| } else if (!model.reasoning) { | ||
| delete next.thinkingLevelMap; | ||
| if (isObject(next.compat)) { | ||
| const { thinkingFormat: _dropped, ...rest } = next.compat; | ||
| if (Object.keys(rest).length > 0) next.compat = rest; | ||
| else delete next.compat; | ||
| } | ||
| } |
There was a problem hiding this comment.
Should fix: stale thinking config after changing the server kind. With reasoning on and a kind outside the chat-template kinds (Ollama, LM Studio, OpenAI-compatible), neither branch runs, so a previously written thinkingLevelMap and compat.thinkingFormat: "qwen-chat-template" survive. Switching an endpoint from llama.cpp to Ollama keeps the llama.cpp-specific thinking params.
There was a problem hiding this comment.
Fixed in d631bd7. For a kind outside llama.cpp/vLLM, or with reasoning off, the save drops the thinkingFormat: "qwen-chat-template" and level map it wrote, and keeps other compat keys. Test: "changing a thinking endpoint's kind drops the chat-template thinking settings".
| export function resolveProbeKey(apiKey: string | undefined): string | undefined { | ||
| const key = apiKey?.trim(); | ||
| if (!key || key === NO_KEY || key.startsWith("!")) return undefined; | ||
| const env = /^\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?$/.exec(key); | ||
| if (env?.[1]) return process.env[env[1]] || undefined; |
There was a problem hiding this comment.
Should fix: probe key resolution differs from pi. This handles only a whole-string $VAR / ${VAR}; pi's resolveConfigValue also interpolates templates and unescapes $$. Keys like ${PREFIX}-${TOKEN} work in chat but fail Detect with 401. Could this reuse pi's resolver (skipping !command)?
There was a problem hiding this comment.
Fixed in d631bd7. pi doesn't export resolveConfigValue, so resolveProbeKey mirrors its template rules: $VAR / ${VAR} anywhere in the value, $$ and $! escapes, and a missing variable means no key. One intended difference: Detect never runs a typed !command. 7b0d8ea records this copy in pi-compatibility.md with a removal gate (a public pi export with a no-command mode). Test: "probe keys interpolate env references like pi and never run commands".
|
|
||
| /** Applies the endpoint's saved limit, timeout and retries; saved values win over pi's global settings. */ | ||
| function withEndpointOptions<T extends RequestOptions>(providerId: string, options: T): T { | ||
| const settings = readLocalEndpointSettings()[providerId]; |
There was a problem hiding this comment.
Nit: this does a sync statSync read of the sidecar on every stream/complete call, for every provider including cloud ones, and Gate.drain re-reads it per queued entry. An in-memory snapshot updated by writeLocalEndpointSettings would keep this off the hot path.
There was a problem hiding this comment.
Fixed in d631bd7. The sidecar is read once into an in-memory snapshot that its own writer replaces, so the per-request lookup never touches the disk.
| /** | ||
| * Routes every model request of `runtime` through the per-endpoint options saved for local servers — | ||
| * the parallel-request limit (a per-request `fetch`), the request timeout and provider retries. Every | ||
| * pi caller — agent turns, subagents, compaction and one-shot completions — dispatches through these. | ||
| */ |
There was a problem hiding this comment.
Nit: per AGENTS.md, comments should be near-zero with rationale in the SPEC. This block (which also sits above DISPATCH rather than installRequestLimits) and the multi-line JSDoc in localModels.ts, localModelSettings.ts and modelsFile.ts duplicate what's already in agent/SPEC.md.
There was a problem hiding this comment.
Fixed in d736164. The narrative comments are gone from the local-model modules; the rationale lives in the server/agent SPEC.md.
8f19383 to
1e7ac0e
Compare
![jetbrains-air[bot]](https://avatars.githubusercontent.com/in/3314082?s=60&v=4){kind=small}
| slots: positiveInt(props.total_slots), | ||
| models: models.map((model) => ({ | ||
| ...model, | ||
| contextWindow: model.contextWindow ?? context, |
There was a problem hiding this comment.
Blocking:
Problem: Detection treats training context limits as the server's running context.
Failure scenario: With llama.cpp reporting meta.n_ctx_train: 131072 and /props reporting per-slot n_ctx: 8192, parseModelList already supplies a non-null context, so this line discards 8192. Ollama enrichment has the same problem: it uses model_info.*.context_length even when the model's parameters specify num_ctx 8192. Both probes reproduce a detected context of 131072. Saving those defaults lets pi retain prompts beyond the server's limit, causing truncation or rejection.
Suggested fix: Prefer reported runtime limits over training metadata; leave the context unknown/editable when the actual runtime limit cannot be established.
There was a problem hiding this comment.
Fixed in e17b4ab. Detection now uses only the context the server runs with: llama.cpp /props per-slot n_ctx (or meta.n_ctx / a router's -c when present), Ollama num_ctx from /api/show parameters, LM Studio loaded_context_length, and vLLM max_model_len. Training lengths (n_ctx_train, *.context_length, max_context_length) are ignored. When nothing is reported, the field holds an editable default and the dialog says the server didn't report it. Unit tests cover both of your scenarios (131072 training vs. 8192 runtime) plus LM Studio, and the e2e spec checks the note.
|
Thanks for the thorough work here. We'd like to take this in a simpler form. Keeping the endpoint's whole config in pi's Proposal: narrow this PR to a form over Keep:
Drop:
Why drop the limits:
Still needed in the narrowed version:
With the limits and the sidecar gone, the timeout-in-queue, non-atomic-write and Test-timeout findings no longer apply, and no |
e17b4ab to
5109a87
Compare
|
Thanks, makes sense. I've split it: this PR is now the form over Dropped from this PR:
Kept: the Add/Edit form, Detect with the per-kind extras and the tools badge, the per-kind compat defaults, context / max output / thinking, and the Your "still needed" list:
One thing I kept: a short Two fixes from the bot review also stay. Detection uses only the context the server runs with, not training lengths. A save clears the fields it manages from that model's For context on why I keep the limits in my fork: llama.cpp splits |
5109a87 to
857e782
Compare
857e782 to
4250ed2
Compare
|
Can you speed up this process? It's not working. |
…servers from Settings Add a Local models group to Settings -> Providers that edits pi's models.json directly: an Add/Edit form (server kind, name, URL, key, per-model context, max output and thinking) and Detect models, which reads /models plus the per-kind extras (/props, /api/show, /api/v0/models) for the running context and a tools badge. - Only local-* providers are listed; hand-written ones stay untouched - The server kind is form-only; an unchanged base URL is kept as written - Detect reuses a saved key only for the endpoint's own unchanged URL - Extract the shared models.json writer into modelsFile - Protocol v99: localModels.list / probe / save / remove Closes JetBrains#436 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FX43RF3RbesYkzMAXQdHf4
…, tighten Detect - Report chatTemplateThinking per endpoint so the edit dialog guesses the form-only kind from saved data; renaming a llama.cpp/vLLM server on a non-default port no longer strips its thinking settings - Refuse a save that moves an endpoint to another origin while keeping its saved key; refuse redirects on keyed probes; name non-JSON replies - Prefer llama.cpp's per-slot /props context over the listing and router -c - Validate model flags and the endpoint id; trim a trailing dash from slugs - Keep the unreported-context note across re-detects until that field is edited; keep an unchanged saved URL as written after Detect - Label the remove-key checkbox on its own, use a fieldset for the server kind, keep Detect from reusing a key marked for removal - Drop the removed sidecar's leftover test and SPEC line; correct SPEC details Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FX43RF3RbesYkzMAXQdHf4
4250ed2 to
8c04e3b
Compare
|
Olga Lavrichenko (@OLavrik) thanks for rebasing this.
The rebase also caught two version leftovers: the SPECs and the commit message said v94 while the code was at v95. |
Problem
ThinkRail can only use providers pi already knows how to sign in to. Running a model on your own hardware (llama.cpp, vLLM, Ollama, LM Studio, or any OpenAI-compatible gateway) means hand-editing
~/.pi/agent/models.json. pi's built-in/login llama.cppextension isn't loaded in ThinkRail, so even llama.cpp has no in-app path. And hand-editing still leaves the parts that make a local server usable for agent work out of reach:max_tokensover--max-model-len);developerrole, nostore,max_tokens);Approach
A form over pi's
models.json, not a second registry. Each server is aproviders.local-<slug>entry withapi: "openai-completions", and everything the feature saves lives in that one file. pi keeps serving, auth and catalog ownership, and the pi CLI sees the same models. Writes reuse themodelContextwriter: JSONC edits that preserve comments and BOM, temp-file + rename, symlink-following. That writer is now a sharedmodelsFilemodule, and both adapters serialize on one write chain.Only
local-*providers are listed. Hand-written providers (a cloud gateway, say) are neither listed nor edited, so local compat flags never land on them and pi keeps serving them unchanged.Detection reads what the server reports.
GET <base>/modelswith a 5 s timeout, plus per-kind extras:max_model_len;/propsfor the per-slotn_ctx,chat_template_caps.supports_toolsand theenable_thinkingtemplate;/api/showfornum_ctxand thetools/thinking/visioncapabilities;/api/v0/modelsfor the loaded context andtool_use.Only the context a server runs with counts; training lengths are ignored. When a server reports none, the dialog fills an editable default and says so. A tools badge shows whether the server says a model takes tool calls.
The server kind is a form-only choice. It drives Detect, URL normalization (
/v1) and the defaults, and isn't saved. An edit guesses it from the saved data (a model using the chat-template thinking format means llama.cpp/vLLM) and then the URL, so renaming a server never strips its thinking settings; an unchanged base URL is kept exactly as written, and switching to a kind that doesn't use the chat-template thinking format clearsthinkingFormat/thinkingLevelMap. Compat on create mirrors pi's own llama.cpp provider; an edit keeps the provider's existing compat.Keys. A typed probe key follows pi's
$VAR/${VAR}/$$template rules but never runs a!command. A saved key is reused for Detect only for that endpoint's own unchanged URL, and is resolved by pi itself; a keyed probe refuses redirects. A save that moves an endpoint to another address asks for the key again instead of carrying it along. Keys never cross the wire (hasApiKeyonly). URLs carrying credentials or a query string are refused.Request concurrency, timeouts and retries are left to the servers (llama.cpp slots, Ollama's queue, vLLM batching) and pi's own provider retry settings.
Trade-offs:
openai-completionsis supported. Every local server targeted here speaks it.Changes
localModels.list/probe/save/remove; typesLocalModelEndpoint(withchatTemplateThinkingfor the edit-time kind guess),LocalModelEndpointDraft,LocalModelProbe; validators for context (1K–10M) and max output (≥256 and ≤ context).agent/modelsFile(extracted shared IO) andagent/localModels;setModelContextPublisher→setModelsFilePublisher. A save clears the fields the form manages from that model'smodelOverrides, so a stale override can't shadow it.LocalModelsSettingsunder Settings → Providers: an endpoint list with Edit and Remove, and an Add/Edit dialog with server kind, name, URL, key (with "remove saved key" on edit), Detect models, and per-model context / max output / thinking. Re-detecting keeps edited values. English and zh-CN strings.server/agent,server/host,contractsandweb/panelsSPECs, the README capability list, and api-compatibility.mdentry for the mirrored key-template rules.local-*scoping (including a hand-written provider's key never reaching Detect), unchanged URLs on edit, kind changes, override clearing, key removal, a key not following a changed address, redirect refusal and validation; web unit tests for the edit-time kind guess;e2e/settings-local-models.spec.tsdrives a fake vLLM server through detect (including an unreported context that stays flagged across a re-detect) → re-detect → save → edit → chat in a workspace → remove.Screenshots
Settings → Providers, same scenario (fresh e2e lane) and viewport (1280×1000):
Add dialog after Detect models against a vLLM-compatible server:
The saved server in the Local models group:
Related issues
Closes #436. It covers adding, editing and removing a self-hosted endpoint from Settings without touching the filesystem, selecting its models in a chat, and endpoint probing (listed in the issue as a follow-up). Gaps against the issue as written:
openai-completionsAPI is offered;models.jsonaren't imported into the list (pi still serves them);Checklist
bun run lint,bun run typecheck, everycheck:*, andbun run teston 8c04e3b (rebased onmainat df7cb6d, protocol v99). The onebun run testfailure, "a read-only models.json is refused", also fails onmainhere (root ignoreschmod 0o400).bun run e2e): 650 passed on 8c04e3b. Of the 7 failures,extension-dialogs:178andmodel-picker:53pass on re-run;settings-model-context:321(root ignoreschmod) and fourtopbar-reviewtests (localgit pushis blocked) also fail onmainin my sandbox. No agent behaviour changed, soe2e:fullwas not run.SPEC.md/ top-level specs updated to reflect any boundary, contract, or behavior change🤖 Generated with Claude Code
https://claude.ai/code/session_01FX43RF3RbesYkzMAXQdHf4