fix(knowledge_base): correct Ollama embedding URL and validate storage type

[chon3806]

Description

Fixes #11910 — two related bugs surface when creating a knowledge base configured with an Ollama embedding model.

Bug 1: 404 on Ollama embeddings without explicit /v1

The OpenAI SDK appends route paths (/embeddings, /chat/completions) directly onto whatever base_url the client was constructed with. Ollama exposes its OpenAI-compatible API only under /v1, so a configured value of:

embedding_model = {
    "provider": "ollama",
    "model_name": "nomic-embed-text",
    "base_url": "http://localhost:11434"
}

resolves to POST http://localhost:11434/embeddings — a route Ollama does not serve, returning 404 Not Found (which then propagates as the opaque message Problem with embedding model config: 'str' object has no attribute 'get' on older versions).

Fix: in LLMClient.__init__'s ollama branch, normalize base_url via a new _normalize_ollama_base_url helper:

• Missing base_url → default to http://localhost:11434/v1.
• User URL without a v1 path segment → transparently append /v1.
• User URL that already contains v1 → left unchanged (explicit user config wins).

The path-segment check uses urlparse so that http://host/v10 is not mistaken for v1. This mirrors the provider-specific-default approach in #12405 (reranker fix for the related issue gh-11952), but additionally heals the very-common user case from the bug report where the URL was supplied without the /v1 suffix.

Bug 2: AttributeError when storage points at a non-vector integration

Configuring storage = files.test_knowledge1 (or any other non-vector handler) previously crashed deep inside the controller with:

AttributeError: 'FileHandler' object has no attribute 'create_table'

The user has no way to tell from that message that knowledge-base storage must be a vector database.

Fix: validate up front that the resolved vector_store_handler is an instance of VectorStoreHandler (the base class extended by every in-repo vector backend — pgvector, chromadb, duckdb_faiss). If not, raise a clear, actionable ValueError that names the offending integration, the handler type, and points at valid alternatives.

The default-storage code path (storage=None) only ever provisions pgvector or duckdb_faiss, both of which extend VectorStoreHandler, so this validation cannot false-positive on the auto-storage flow.

Type of change

• 🐛 Bug fix (non-breaking change which fixes an issue)

Verification Process

• Test Location: tests/unit/interfaces/knowledge_base/test_ollama_base_url.py, tests/unit/interfaces/knowledge_base/test_storage_handler_validation.py
• Verification Steps:
  1. python -m pytest tests/unit/interfaces/knowledge_base/ -v — 15 tests pass (8 new for the ollama URL normalization rules including the v10/v1 false-positive guard, 2 new for the storage handler type validation, plus the 5 pre-existing default-storage tests still green).
  2. End-to-end: with the issue's repro CREATE KNOWLEDGE_BASE statement (Ollama running locally with nomic-embed-text pulled), the embedding probe now succeeds against http://localhost:11434 (auto-corrected to /v1) and the files.* storage misuse now fails fast with: Storage 'files' is not a vector database (handler type: FileHandler). Knowledge base storage must be a vector database integration such as pgvector, chromadb, or faiss.

Checklist

• My code follows the style guidelines (PEP 8) of MindsDB.
• I have appropriately commented on my code, especially in complex areas.
• Necessary documentation updates are either made or tracked in issues.
• Relevant unit and integration tests are updated or added.

[chon3806]

I have read the CLA Document and I hereby sign the CLA

[entelligence-ai-pr-reviews]

EntelligenceAI PR Summary

Fixes gh-11910 by adding two defensive validations in the knowledge base pipeline and corresponding regression tests.

• controller.py: Raises ValueError with actionable guidance early in KnowledgeBaseController.add when a non-VectorStoreHandler storage integration is provided, preventing a downstream AttributeError
• llm_client.py: Introduces _normalize_ollama_base_url to automatically append /v1 to bare Ollama host URLs, resolving 404 errors caused by the OpenAI SDK appending routes to an incomplete base URL
• test_ollama_base_url.py: Pins normalization logic across 7 edge cases including empty input, trailing slashes, remote hosts, sub-paths, and exact segment matching
• test_storage_handler_validation.py: Pins storage handler validation with tests for both the error case (non-vector handler) and the happy path (valid VectorStoreHandler subclass)

---

Confidence Score: 3/5 - Review Recommended

Not safe to merge without changes — the _normalize_ollama_base_url method in llm_client.py uses the str | None union type syntax (PEP 604) which raises a TypeError at class-definition time on Python 3.8 and 3.9, while the rest of the file uses typing.List indicating the codebase targets pre-3.10 runtimes. The PR itself is well-intentioned — fixing the Ollama /v1 URL normalization and adding early validation in KnowledgeBaseController.add are both solid defensive improvements — but this type annotation incompatibility would cause a hard crash on supported Python versions before the fixed logic even runs.

Key Findings:

• _normalize_ollama_base_url in llm_client.py annotates its return type as str | None (Python 3.10+ union syntax), which will raise TypeError at class-definition time on Python 3.8/3.9; the fix is trivial — use Optional[str] from typing or add from __future__ import annotations — but as-is it breaks the very feature this PR is meant to fix on older runtimes.
• The VectorStoreHandler type check added in controller.py is a good early-exit guard that prevents a confusing downstream AttributeError, and the error message provides actionable guidance — this is a clean, low-risk improvement.
• Only 2 of 4 changed files received review coverage, so it's possible additional issues exist in the untested files; the regression tests mentioned in the summary should be verified to actually exercise the Python 3.8-compatible code path once the annotation is fixed.

Files requiring special attention

• mindsdb/interfaces/knowledge_base/llm_client.py
• mindsdb/interfaces/knowledge_base/controller.py

[entelligence-ai-pr-reviews]

Fixes gh-11910 by adding two defensive validations in the knowledge base pipeline and corresponding regression tests.

• controller.py: Raises ValueError with actionable guidance early in KnowledgeBaseController.add when a non-VectorStoreHandler storage integration is provided, preventing a downstream AttributeError
• llm_client.py: Introduces _normalize_ollama_base_url to automatically append /v1 to bare Ollama host URLs, resolving 404 errors caused by the OpenAI SDK appending routes to an incomplete base URL
• test_ollama_base_url.py: Pins normalization logic across 7 edge cases including empty input, trailing slashes, remote hosts, sub-paths, and exact segment matching
• test_storage_handler_validation.py: Pins storage handler validation with tests for both the error case (non-vector handler) and the happy path (valid VectorStoreHandler subclass)

[entelligence-ai-pr-reviews]

Correctness: The str | None union type syntax requires Python 3.10+; the rest of the file imports List from typing (Python 3.8/3.9 style), so this annotation will raise a TypeError at class-definition time on any runtime below 3.10. Use Optional[str] from typing instead (or add from __future__ import annotations to defer annotation evaluation).

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In mindsdb/interfaces/knowledge_base/llm_client.py at line 122, the method signature `def _normalize_ollama_base_url(base_url: str | None) -> str:` uses the Python 3.10+ union type syntax (`str | None`). The rest of the file uses `typing.List` which is the Python 3.8/3.9 compatible style. Replace `str | None` with `Optional[str]` (importing `Optional` from `typing`) or simply remove the annotation to maintain backwards compatibility with Python 3.9 and earlier.

[chon3806]

The PEP 604 concern is a false positive: setup.py pins python_requires=">=3.10,<3.14", where str | None is native. The same syntax is already used throughout the repo, including the files this PR touches — e.g. controller.py:52 (id_column: str | None = None) and default_storage_resolver.py:7 (def _normalize_engine_name(engine: str | None) -> str | None). The annotation is consistent with existing convention, not a regression.

[entelligence-ai-pr-reviews]

@chon3806 Got it, I'll keep that in mind for future reviews. Since the repo targets Python 3.10+ and already uses PEP 604 syntax consistently, I won't flag str | None style annotations as issues.

✍️ Learning notes added:

Learnt from: chon3806
Repo Name: mindsdb/mindsdb
PR Number: 12412
Filename: mindsdb/interfaces/knowledge_base/llm_client.py
Timestamp: 2026-04-25 18:07:53
Notes: Do not flag `str | None` union type syntax as a compatibility issue when the project's setup.py explicitly requires Python 3.10+, and the syntax is already consistently used throughout the codebase.

[torrmal]

thank you!! can you please create a pull request in github.com/mindsdb/engine