Skip to content

feat: Add Ollama as an AI provider, implementing its API for agent in…#6

Open
vkop007 wants to merge 3 commits intosachaa:masterfrom
vkop007:feature/ollama-support
Open

feat: Add Ollama as an AI provider, implementing its API for agent in…#6
vkop007 wants to merge 3 commits intosachaa:masterfrom
vkop007:feature/ollama-support

Conversation

@vkop007
Copy link

@vkop007 vkop007 commented Feb 26, 2026

feat(ai): integrate Ollama as local AI provider

  • Added support for routing prompts and tool calls to a local Ollama instance
  • Re-architected Orchestrator invoke and compact payloads to support dynamic providers
  • Added Provider and Ollama URL configuration fields to the Settings UI
  • Ensured strict single-quote string formatting to match maintaining guidelines

…vocation, tool use, and context compaction, and introducing new UI settings for provider selection and Ollama host configuration.
@sachaa
Copy link
Owner

sachaa commented Mar 7, 2026

Thanks for the contribution! Really like the idea of adding Ollama support, local models are a great option to have.

I have a few concerns I'd like to see addressed before merging:

Architecture: Provider abstraction needed

The biggest issue is that invokeOllama/compactOllama duplicate nearly the entire structure of the Anthropic equivalents (~200 lines). If we add a third provider later (OpenAI, Gemini, etc.), we'd be triplicating everything. I'd like to see this refactored into a provider interface/strategy pattern, the tool-use loop, logging, and message posting are identical; only the API call shape and response parsing differ.

Bugs / must-fix:

  • Empty model on provider switch: switching to Ollama sets the model to '' if it was a Claude model. If the user sends a message before typing a model name, it will fail. Needs a sensible default or validation.
  • Hardcoded contextLimit: 8192, most Ollama models support much larger contexts (Llama 3 = 128k, Mistral = 32k). This will make the context usage bar inaccurate.
  • API key still sent in Ollama payloads, apiKey: this.apiKey is included in every worker message regardless of provider. Should only send credentials relevant to the active provider.

Nice to haves (not blocking):

  • Ollama has GET /api/tags to list local models, a dropdown with auto-discovery would be much better UX than a free-text input.
  • No URL validation on the Ollama host field.

Happy to pair on the provider abstraction if you'd like, I think that's the main thing to get right before this goes in.

@sachaa
Copy link
Owner

sachaa commented Mar 8, 2026

This is so much better! Nice progress on the refactor, the discriminated union types for the payloads are really clean, and the provider abstraction is a solid improvement.

Two small things to address before merging:

  1. The model discovery useEffect fires on every keystroke, ollamaUrl updates on every onChange and is in the dependency array, so it blasts fetch requests against partial URLs as the user types. Needs a debounce and an abort controller to cancel stale requests. There's also a stale closure on model (read inside the effect but missing from the dependency array).

  2. getContextLimit() still returns 200k for all models, it's no longer hardcoded to 8192, but 200k is wrong for Ollama models too. Consider querying /api/show for the actual context size, or at least using a conservative default for non-Claude models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants