feat(providers): validate retriever settings keys to catch typos early (#116)

openagent_eval/providers/factory.py

@@ -22,6 +22,9 @@
 from openagent_eval.providers.base.retriever import Retriever
 from openagent_eval.providers.embedders.base import Embedder
 from openagent_eval.providers.models import Document, LLMResponse, TokenUsage
+from openagent_eval.providers.retrievers._validation import (
+    validate_retriever_settings,
+)
 
 # --------------------------------------------------------------------------- #
 # LLM provider registry                                                       #
@@ -115,6 +118,15 @@ def get_retriever(config: RetrieverConfig) -> Retriever:
     retriever_cls = _resolve(_RETRIEVER_PROVIDERS[key])
     settings = dict(config.settings or {})
 
+    # Catch typos/unknown keys in the free-form settings dict early, before the
+    # values reach a provider constructor (which may silently swallow unknown
+    # kwargs). Providers opt in by declaring ``SETTINGS_KEYS``.
+    allowed_keys = getattr(retriever_cls, "SETTINGS_KEYS", None)
+    if allowed_keys is not None:
+        unknown = validate_retriever_settings(key, settings, allowed_keys)
+        for bad_key in unknown:
+            settings.pop(bad_key, None)
+
     embedder = None
     if config.embedder is not None:
         embedder = get_embedder(config.embedder)

openagent_eval/providers/retrievers/_validation.py

@@ -0,0 +1,60 @@
+"""Early validation for retriever ``settings`` keys.
+
+Retriever providers accept a free-form ``settings`` dict from user config
+(``retriever.settings`` in the YAML). Without validation, a typo such as
+``collectoin_name`` is either swallowed by a provider's ``**kwargs`` or blows
+up with an opaque ``TypeError`` deep inside a constructor -- and only at
+runtime. This module surfaces unknown keys early, at provider-build time, with
+a "did you mean" suggestion.
+
+The behaviour mirrors how unknown/misspelled metric names are handled in
+:mod:`openagent_eval.config.loader` (see the metrics block around
+``loader.py:139``): the offending keys are dropped and a ``logging.warning`` is
+emitted naming them, rather than hard-failing. This keeps a single stray key
+from aborting an otherwise-valid evaluation run.
+"""
+
+from __future__ import annotations
+
+import difflib
+import logging
+from typing import TYPE_CHECKING, Any
+
+if TYPE_CHECKING:
+    from collections.abc import Iterable, Mapping
+
+logger = logging.getLogger(__name__)
+
+
+def validate_retriever_settings(
+    provider_name: str,
+    settings: Mapping[str, Any],
+    allowed_keys: Iterable[str],
+) -> list[str]:
+    """Warn about unknown ``settings`` keys for a retriever provider.
+
+    Args:
+        provider_name: Name of the retriever provider (for the log message).
+        settings: The user-supplied settings mapping to check.
+        allowed_keys: The setting keys the provider actually understands.
+
+    Returns:
+        The list of unknown keys found (in the order they appear in
+        ``settings``). Empty when every key is recognised.
+    """
+    allowed = set(allowed_keys)
+    unknown = [key for key in settings if key not in allowed]
+
+    for key in unknown:
+        match = difflib.get_close_matches(key, allowed, n=1)
+        suggestion = f" Did you mean '{match[0]}'?" if match else ""
+        logger.warning(
+            "Unknown setting '%s' for retriever '%s'; it will be ignored.%s "
+            "Known settings: %s",
+            key,
+            provider_name,
+            suggestion,
+            ", ".join(sorted(allowed)),
+        )
+
+    return unknown

openagent_eval/providers/retrievers/chroma.py

@@ -67,6 +67,13 @@ class ChromaRetriever(Retriever):
     name: str = "chroma"
     description: str = "ChromaDB vector-based document retriever"
 
+    #: Setting keys accepted via ``retriever.settings``. Used by the provider
+    #: factory to catch typos/unknown keys early (mirrors the constructor's
+    #: parameters below).
+    SETTINGS_KEYS: frozenset[str] = frozenset(
+        {"collection_name", "persist_directory", "distance_fn"}
+    )
+
     def __init__(
         self,
         collection_name: str,

openagent_eval/providers/retrievers/qdrant.py

@@ -25,6 +25,13 @@ class QdrantRetriever(Retriever):
     name: str = "qdrant"
     description: str = "Qdrant vector database retriever"
 
+    #: Setting keys accepted via ``retriever.settings``. Used by the provider
+    #: factory to catch typos/unknown keys early. Note the constructor swallows
+    #: unknown kwargs via ``**_``, so without this a typo would pass silently.
+    SETTINGS_KEYS: frozenset[str] = frozenset(
+        {"collection_name", "embedder", "url", "api_key", "prefer_grpc", "distance"}
+    )
+
     def __init__(
         self,
         collection_name: str,

tests/unit/test_providers/test_retriever_settings_validation.py

@@ -0,0 +1,97 @@
+"""Tests for early validation of retriever ``settings`` keys (issue #116)."""
+
+from __future__ import annotations
+
+import logging
+
+import pytest
+
+from openagent_eval.config.models import RetrieverConfig
+from openagent_eval.exceptions.provider import ProviderConnectionError
+from openagent_eval.providers.factory import get_retriever
+from openagent_eval.providers.retrievers._validation import (
+    validate_retriever_settings,
+)
+
+CHROMA_KEYS = {"collection_name", "persist_directory", "distance_fn"}
+
+
+class TestValidateRetrieverSettings:
+    """Unit tests for the validation helper in isolation."""
+
+    def test_unknown_key_is_reported_and_named(self, caplog):
+        with caplog.at_level(logging.WARNING):
+            unknown = validate_retriever_settings(
+                "chroma", {"collectoin_name": "docs"}, CHROMA_KEYS
+            )
+
+        assert unknown == ["collectoin_name"]
+        assert "collectoin_name" in caplog.text
+        # The suggestion points at the closest real key.
+        assert "Did you mean 'collection_name'?" in caplog.text
+
+    def test_all_known_keys_pass(self, caplog):
+        with caplog.at_level(logging.WARNING):
+            unknown = validate_retriever_settings(
+                "chroma",
+                {
+                    "collection_name": "docs",
+                    "persist_directory": "./db",
+                    "distance_fn": "cosine",
+                },
+                CHROMA_KEYS,
+            )
+
+        assert unknown == []
+        assert "Unknown setting" not in caplog.text
+
+    def test_unknown_key_without_close_match_has_no_suggestion(self, caplog):
+        with caplog.at_level(logging.WARNING):
+            unknown = validate_retriever_settings("chroma", {"zzzzzz": 1}, CHROMA_KEYS)
+
+        assert unknown == ["zzzzzz"]
+        assert "zzzzzz" in caplog.text
+        assert "Did you mean" not in caplog.text
+
+
+class TestFactorySettingsValidation:
+    """The factory wires validation into ``get_retriever`` before build."""
+
+    def test_typo_in_qdrant_settings_warns_with_suggestion(self, caplog):
+        cfg = RetrieverConfig(
+            provider="qdrant",
+            settings={"collection_name": "docs", "collectoin_name": "docs"},
+        )
+        # qdrant still errors (no embedder); validation runs first.
+        with (
+            caplog.at_level(logging.WARNING),
+            pytest.raises(ProviderConnectionError),
+        ):
+            get_retriever(cfg)
+
+        assert "collectoin_name" in caplog.text
+        assert "Did you mean 'collection_name'?" in caplog.text
+
+    def test_known_qdrant_settings_emit_no_unknown_warning(self, caplog):
+        cfg = RetrieverConfig(
+            provider="qdrant",
+            settings={
+                "collection_name": "docs",
+                "url": "http://localhost:6333",
+                "prefer_grpc": True,
+                "distance": "Cosine",
+            },
+        )
+        with (
+            caplog.at_level(logging.WARNING),
+            pytest.raises(ProviderConnectionError),
+        ):
+            get_retriever(cfg)
+
+        assert "Unknown setting" not in caplog.text
+
+    def test_chroma_declares_expected_settings_keys(self):
+        pytest.importorskip("chromadb")
+        from openagent_eval.providers.retrievers.chroma import ChromaRetriever
+
+        assert set(ChromaRetriever.SETTINGS_KEYS) == CHROMA_KEYS