Repository navigation
Conversation
|
Thanks for the PR @prodmanpd; we'll review it |
|
Hi @prodmanpd BlockerB1 — The proxy endpoint is not configurable; LiteLLM is locked to localhost
That undercuts the main reason teams adopt LiteLLM: a shared, centralised The documentation compounds it by omission. Ask: add an operator-settable Should-fixS1 — UI provider list not updated, breaking a stated invariant
The enterprise runtime override derives its supported set from
S2 — Embeddings gap for an operator who standardises on LiteLLM
Unlike deepseek/claude — deliberately unwired because they have no embedding |
…lm embeddings Addresses review feedback on the LiteLLM provider PR: - B1: add agent.ai.base_url (env AGENT_AI_BASE_URL) wired through AgentAIConfig into the chat + embedder requests, so LiteLLM (and other OpenAI-compatible backends) can target a shared/remote gateway instead of being locked to the localhost:4000 sidecar. The test-only Options.BaseURL seam still wins in tests. - S1: add 'litellm' to the UI AIProvider union and AI_PROVIDERS list so the AI-settings dropdown mirrors eino.SupportedProviders again. - S2: register a litellm embedder (reuses the OpenAI-compatible /v1/embeddings path with the local-proxy default) so RAG works when provider=litellm. Docs + sample config updated to describe the base_url knob. Adds regression tests: cfg.BaseURL routes chat egress, and NewEmbedder(litellm) embeds.
|
I'm concerned about the supply-chain boundary of this LiteLLM integration. The upstream incident report confirms compromised PyPI releases 1.82.7/1.82.8 in March 2026; those releases were removed. This reviewed change uses a proxy-compatible integration; it does not itself add a Python LiteLLM package installation. The deployment questions are who operates the proxy, which verified server version runs there, and whether credentials remain isolated from other providers. Please document those trust boundaries for users. This is one of 49 observed LiteLLM integration PRs from the same account, whose author acknowledged the cross-project effort. The pattern prompted this review; it does not establish malicious intent or connect the author to the incident. The consolidated questions and corrections are in YouDub #130; this PR's review snapshot preserves the revision and scope. |
… into feat/add-litellm-provider # Conflicts: # pkg/agent/ai/eino/chatmodel.go # pkg/agent/ai/eino/embedder.go # src/agent/configuration.md
What does this PR do?
Adds LiteLLM as a first class chat provider for the AI SRE agent, registered in the
einomodel builder registry alongsideopenai,deepseek,qwen,ollama,claude, andgemini. Setagent.ai.provider: litellmto route detect and analyze through a LiteLLM proxy, one OpenAI compatible endpoint that fans out to 100+ upstream providers, reusing the already vendoredeino-ext/components/model/openaiwith no new dependency.Changes:
pkg/agent/ai/eino/provider.go:buildLiteLLMChatModelplus the"litellm"registry entry.pkg/agent/ai/eino/provider_test.go: registry, exported validator, and egress tests for litellm.pkg/config/agent.go,pkg/agent/aisettings.go,pkg/agent/ai/eino/holder.go: provider list doc comments.config/config.yaml,pkg/config/default_config.yaml: provider comment plus a litellm usage note.src/agent/configuration.md: provider table row plus a LiteLLM gateway section.Why?
This adds gateway support so operators can centralise keys, spend caps, caching, and fallbacks in one proxy while Versus keeps a single stable config. The proxy speaks the OpenAI chat completions wire format, so it slots into the registry as a real discrete builder rather than a generic base url toggle, which means
SupportedProviders()(the enterprise write boundary validator) and the runtime fail fast path both cover it for free. Prior art: #160 proposed an OpenAI compatiblebase_urlfield naming Gemini, LiteLLM, and OpenRouter; the AI layer then moved to the Eino framework with discrete first class builders, and this PR follows that direction with a realbuildLiteLLMChatModel.How to test
Sample config:
modelis the alias defined in your proxy config (not a raw provider id); the endpoint defaults to a local sidecar athttp://localhost:4000/v1.Unit tests,
go test -race ./pkg/agent/ai/eino/...:The egress test spins up an httptest OpenAI compatible server and asserts that the virtual key rides the
BearerAuthorization header, that JSON mode sendsresponse_format:{type:json_object}, and that the reply round trips throughdetect.ParseFinding.Live end to end through a real LiteLLM proxy.
NewToolCallingChatModel(provider=litellm)buildsbuildLiteLLMChatModel, which calls the OpenAI SDK, which calls the LiteLLM proxy, which calls the upstream:Proxy side confirmation of the same call (Azure OpenAI upstream through the proxy):
Bug caught and fixed during the deep dive: the proxy path shares the OpenAI SDK, which validates the beta limitation client side, so for reasoning families (
gpt-5.*, o series) an explicittemperatureis rejected before the request leaves. Naively forwarding the configured temperature broke agpt-5alias. The builder now reuses the sameisFixedSamplingModelomission asbuildOpenAIChatModel, so an operator aliasinggpt-5through the proxy does not have to settemperature: -1by hand. The egress test asserts thattemperatureis absent from the wire for agpt-5alias.Build, vet, and format are clean:
go build ./...,go vet ./pkg/agent/ai/eino/... ./pkg/config/..., andgofmt -l.go test -race ./pkg/...is green for every package that builds locally, includingpkg/agent/ai/eino,pkg/agent/ai/detect,pkg/agent/ai/analyze,pkg/config, andpkg/core.pkg/controllersandpkg/routesonly fail to build locally because they//go:embed ui/dist, which needs the frontend build and is untouched by this PR.Type of change
Checklist
go test ./...passes locallygo vet ./...is cleangofmt'dsrc/if behavior changesROADMAP.mdif this closes a roadmap item