Skip to content

feat: add Ollama as local embedding provider - #81

Closed
TerminalGravity wants to merge 1 commit into
mainfrom
feat/ollama-embed-provider
Closed

feat: add Ollama as local embedding provider#81
TerminalGravity wants to merge 1 commit into
mainfrom
feat/ollama-embed-provider

Conversation

@TerminalGravity

Copy link
Copy Markdown
Collaborator

Adds OllamaEmbeddingProvider using Ollama /api/embed endpoint. Fully local, no API key needed. Defaults to nomic-embed-text, 768 dims. Closes #6

Adds OllamaEmbeddingProvider that uses Ollama's /api/embed endpoint.
Supports custom base URL, model, and dimensions. Defaults to
nomic-embed-text with 768 dimensions.

Closes #6
@TerminalGravity

Copy link
Copy Markdown
Collaborator Author

Clean implementation — follows the same pattern as OpenAI, batch support via /api/embed is the right call. Defaults (nomic-embed-text, 768 dims) are sensible. CI green ✅

One thought for later: a health check on init (ping Ollama, verify model is pulled) would give users a clear error instead of a cryptic fetch failure. Not blocking though.

@TerminalGravity

Copy link
Copy Markdown
Collaborator Author

Clean implementation — the factory pattern extension is exactly right, and good call using /api/embed with batch support from the start.

A couple thoughts for follow-up:

  • Consider adding a connection health ping so users get a clear error if Ollama isn't running (instead of failing on first embed)
  • The node engine bump to >=20 in package-lock seems unrelated — intentional?
  • Worth adding a README note about supported Ollama models

Closes #6. Merging. 🎉

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean implementation. Batch endpoint usage avoids N+1 round trips — good call.

A few suggestions for follow-up:

  1. Timeout on fetch calls — if Ollama is down or slow to load a model, the MCP server will hang. Even a 30s timeout + clear error would help.
  2. Node engine bump — the package-lock bumps to >=20, worth noting since it's technically breaking for anyone on 18.
  3. Health check — a quick /api/tags ping would make it easy to surface 'Ollama not running' vs 'model not found' errors.

None block merge. Nice close on #6.

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and clean. The Ollama provider looks solid — using the batch /api/embed endpoint for both single and batch is smart.

One small thing: if Ollama isn't running, the fetch will throw a connection refused error that'll be hard to debug. Worth wrapping with a friendlier message like "Cannot connect to Ollama at ${this.baseUrl} — is it running?" in a try/catch around the fetch calls.

Also, the node engine bump from >=18 to >=20 in package-lock.json — was that intentional? If so, should match in package.json too. If not, might want to revert that line.

@TerminalGravity

Copy link
Copy Markdown
Collaborator Author

Superseded by a newer PR.

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.

Ollama embedding support

1 participant