The preferred_providers parameter is validated and stored, but then ignored when creating the InferenceSession. Line 322 hardcodes providers to ["CPUExecutionProvider"], overriding any user-specified providers. Either the preferred_providers functionality should be removed if it's not supported, or the hardcoded providers should be replaced with self._preferred_providers. The current implementation is misleading to users who might expect their provider preferences to be respected.

The test validates the preferred_providers parameter but doesn't test the actual functionality of using different providers. Given that the model() method hardcodes providers to ["CPUExecutionProvider"] (line 322 in onnx_embedding_function.py), these validation tests are misleading as they suggest the feature works when it actually doesn't. Consider either removing these tests or fixing the implementation to actually use the preferred providers.

The Args section of the docstring has incorrect formatting. The default values and comments are placed inline with the parameter names instead of in the description. The docstring should follow standard format with parameter names on one line and descriptions on the following lines. For example: 'model_name: Deprecated parameter. Will be removed in a future version. Currently only "all-MiniLM-L6-v2" is supported.'

The ARCHIVE_FILENAME class constant is defined but never used anywhere in the class. This appears to be leftover from a previous implementation that used tar.gz archives instead of direct file downloads. Consider removing this unused constant to avoid confusion.

The test uses pytest.importorskip but these imports are mandatory dependencies in pyproject.toml (onnxruntime>=1.19.0, tokenizers>=0.15.0, tqdm). Using importorskip is appropriate for optional dependencies, but for required dependencies, the test should fail hard if they're missing rather than skip. This inconsistency could mask installation issues.

The download_path is stored as a Path object but then used with os.path.join throughout the code (lines 147, 278, 320, 338). While os.path.join does accept Path objects in Python 3.6+, it's inconsistent to mix Path and os.path APIs. Consider either using Path methods consistently (download_path / EXTRACTED_FOLDER_NAME) or converting download_path to a string. The current approach reduces code clarity and could cause confusion.

The OnnxEmbeddingFunction class does not implement the persistence methods required by the EmbeddingFunction protocol (name(), get_config(), and build_from_config()). If this class is intended to be used directly by users as suggested in the deprecation warning in DefaultEmbeddingFunction, it should implement these methods to support collection persistence. Without these methods, collections using OnnxEmbeddingFunction directly cannot be persisted and restored.

The code modifies the _preferred_providers list in place by calling .remove("CoreMLExecutionProvider"). If this method is called multiple times (which could happen with cached_property in some edge cases), it will attempt to remove the same item twice and raise a ValueError. Consider using a list comprehension to create a new list without the unwanted provider instead: providers = [p for p in self._preferred_providers if p != "CoreMLExecutionProvider"]

The preferred_providers parameter is deprecated but still accepted in the init method. However, it's never passed to the underlying OnnxEmbeddingFunction. This means that even during the deprecation period, the parameter has no effect. If backward compatibility is desired during the deprecation period, the parameter should be passed through to OnnxEmbeddingFunction, or alternatively, the ValueError for unsupported values should be raised before the deprecation warning.

⚠️ Potential issue | 🟠 Major

cat -n src/pyseekdb/client/embedding_function.py | sed -n '150,185p'

# Find the OnnxEmbeddingFunction class definition to check its constructor
rg "class OnnxEmbeddingFunction" -A 30

# Check the constructor signature more carefully
rg "def __init__" src/pyseekdb/client/embedding_function.py -A 15 -B 2

Pass preferred_providers through to OnnxEmbeddingFunction.

The deprecation warning directs users to use OnnxEmbeddingFunction's preferred_providers argument, but the parameter is never passed through, effectively dropping it and breaking backward compatibility for existing callers who rely on provider selection.

         self._onnx = OnnxEmbeddingFunction(
             model_name=self.MODEL_NAME,
             hf_model_id=self.HF_MODEL_ID,
             dimension=self._DIMENSION,
+            preferred_providers=preferred_providers,
         )

In `@src/pyseekdb/client/embedding_function.py` around lines 164 - 176, The
constructor currently warns that preferred_providers is deprecated but does not
forward that parameter to the OnnxEmbeddingFunction, dropping caller selection;
update the OnnxEmbeddingFunction instantiation in the embedding function (the
block that creates self._onnx) to pass the preferred_providers argument through
(i.e., include preferred_providers=preferred_providers when calling
OnnxEmbeddingFunction) so existing callers retain provider selection behavior;
ensure the variable name matches the parameter on the parent constructor and
OnnxEmbeddingFunction signature.

⚠️ Potential issue | 🟠 Major

# Find the file and check its size
find . -name "onnx_embedding_function.py" -type f

# Read the relevant sections of the file
cat -n src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | head -100

# Also check lines 220-230, 300-310, 350-360 to see other violations mentioned
cat -n src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | sed -n '220,230p;300,310p;350,360p'

class CantBeNegative(Exception): ...

def foo(x):
    if x < 0:
        raise CantBeNegative(f"{x} is negative")  # TRY003

class CantBeNegative(Exception):
    def __init__(self, number: int) -> None:
        super().__init__(f"{number} is negative")

def foo(x):
    if x < 0:
        raise CantBeNegative(x)

raise CantBeNegative(f"{x} is negative")  # noqa: TRY003

# Look for ruff configuration
find . -name "pyproject.toml" -o -name "ruff.toml" | head -5

# Check if ruff config exists and look for TRY003
if [ -f pyproject.toml ]; then
  echo "=== pyproject.toml ===" 
  grep -A 20 "\[tool.ruff" pyproject.toml || echo "No [tool.ruff] section"
fi
if [ -f ruff.toml ]; then
  echo "=== ruff.toml ===" 
  cat ruff.toml
fi

# Verify that TRY003 is included in the TRY ruleset
cat -n ruff.toml | grep -A 2 -B 2 "TRY"

TRY003 violations must be fixed by moving exception messages into exception classes.

Ruff's TRY003 rule (raise-vanilla-args) is enabled via the TRY ruleset in ruff.toml and flags inline messages at lines 55, 57, 59, 72, 74, 224-226, 302-304, and 352-357. Move these messages into custom exception class __init__ methods rather than specifying them at raise sites.

+class InvalidModelNameError(ValueError):
+    def __init__(self) -> None:
+        super().__init__("model_name must be a non-empty string")
@@
-        if not model_name:
-            raise ValueError("model_name must be a non-empty string")
+        if not model_name:
+            raise InvalidModelNameError()

Also applies to: 224-226, 302-304, 352-357

In `@src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py` around
lines 54 - 74, The constructor and other raise sites in ONNXEmbeddingFunction
currently raise ValueError with inline messages; create specific exception
classes (e.g., InvalidModelNameError, InvalidHFModelIDError,
InvalidDimensionError, InvalidPreferredProvidersError) whose __init__ set the
appropriate default message, then replace raises that pass message strings (the
checks referencing model_name, hf_model_id, dimension, preferred_providers and
the other flagged raise sites later in the file) with raises of those exception
classes without arguments; ensure all other flagged raise sites in this module
use their own dedicated exception classes with messages defined in the exception
__init__ rather than inline messages at the raise call.

⚠️ Potential issue | 🟠 Major

cat -n src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | sed -n '150,200p'

# Get more context around the download function
cat -n src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | sed -n '130,220p'

# Check what exceptions httpx can raise
rg "import httpx|from httpx" src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py -A 2

grep -n "_download" src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | head -20

# Find the _download method definition
ast-grep --pattern 'def _download($_) { $$$ }'

# Search for _download method in the file
rg "def _download" src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py -A 20

python3 << 'EOF'
try:
    import httpx
    # Check exception hierarchy
    print("httpx.HTTPError exists:", hasattr(httpx, 'HTTPError'))
    print("httpx.HTTPStatusError exists:", hasattr(httpx, 'HTTPStatusError'))
    print("httpx.RequestError exists:", hasattr(httpx, 'RequestError'))
    
    # Check MRO if HTTPStatusError exists
    if hasattr(httpx, 'HTTPStatusError'):
        print("\nhttpx.HTTPStatusError MRO:")
        for cls in httpx.HTTPStatusError.__mro__:
            print(f"  - {cls}")
    
    if hasattr(httpx, 'RequestError'):
        print("\nhttpx.RequestError MRO:")
        for cls in httpx.RequestError.__mro__:
            print(f"  - {cls}")
            
except ImportError:
    print("httpx not installed in this environment")
EOF

Narrow the broad Exception catch to specific expected exceptions.

The blanket except Exception at line 181 can hide unexpected bugs and violates the BLE001 linting rule. Since _download() can raise httpx.HTTPStatusError (already caught at line 173), httpx.RequestError, and OSError from file operations, catch only those specific exceptions and let unexpected errors surface for debugging.

-                except Exception as e:
+                except (httpx.HTTPError, OSError) as e:
                     logger.warning(f"Failed to download {hf_filename} from Hugging Face: {e}")
                     # If download fails, try to delete partially downloaded file
                     if os.path.exists(local_path):
                         os.remove(local_path)
                     return False

In `@src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py` around
lines 173 - 186, The broad "except Exception" should be narrowed to the expected
failure types so unexpected bugs bubble up; replace the generic except block in
the download flow (the block after the existing except httpx.HTTPStatusError)
with an explicit except (httpx.RequestError, OSError) as e (or include
PermissionError if desired), keep the same logger.warning and local_path cleanup
logic, and ensure httpx is imported so httpx.RequestError is available; leave
the existing httpx.HTTPStatusError handler unchanged.

⚠️ Potential issue | 🟠 Major

cat -n src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | head -n 330 | tail -n +270

head -n 100 src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py

# Search for other references to _preferred_providers or preferred_providers in the file
rg "preferred_providers" src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py -n

# Search for any TODO comments or issues related to this in the entire file
cat src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py | grep -i -E "todo|fixme|issue|bug|workaround" -C 2

# Search for any related issues or discussions in code comments
rg "provider" src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py -B 2 -A 2

Use self._preferred_providers instead of hardcoding CPU provider.

The preferred_providers parameter is validated and defaults to available providers, but is completely ignored when creating the InferenceSession. Pass self._preferred_providers to the providers argument so users can leverage GPU or other accelerators as specified.

         return self.ort.InferenceSession(
             os.path.join(self.download_path, self.EXTRACTED_FOLDER_NAME, "model.onnx"),
-            # Force CPU execution provider to avoid provider issues
-            providers=["CPUExecutionProvider"],
+            providers=self._preferred_providers,
             sess_options=so,
         )

In `@src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py` around
lines 294 - 323, The InferenceSession call in onnx_embedding_function currently
hardcodes providers=["CPUExecutionProvider"] and thus ignores the validated
self._preferred_providers; update the providers argument in the
self.ort.InferenceSession(...) call to pass self._preferred_providers (ensure it
is a list/sequence) so the session respects user-selected/available providers
(note the code already removes "CoreMLExecutionProvider" from
self._preferred_providers). Use self._preferred_providers as the providers
parameter instead of the hardcoded CPUExecutionProvider.

⚠️ Potential issue | 🟡 Minor

# First, find and read the test file
find . -name "test_onnx_embedding_function.py" -type f

# Look for the OnnxEmbeddingFunction class to understand what it imports
find . -name "*.py" -type f | xargs grep -l "class OnnxEmbeddingFunction"

# Read the test file to see the current state
cat -n ./tests/unit_tests/test_onnx_embedding_function.py

# Read the OnnxEmbeddingFunction implementation
cat -n ./src/pyseekdb/utils/embedding_functions/onnx_embedding_function.py

Guard _make_onnx() with importorskip for optional deps.

OnnxEmbeddingFunction.__init__ imports optional packages (onnxruntime, tokenizers, tqdm) at lines 79-81. The tests test_dimension_property and test_call_empty_returns_empty will fail when these dependencies are absent because they call _make_onnx() without any skip guards, while test_call_generates_embeddings has them. Centralize the skips inside _make_onnx() and remove the redundant per-test skips.

 def _make_onnx() -> OnnxEmbeddingFunction:
+    pytest.importorskip("onnxruntime")
+    pytest.importorskip("tokenizers")
+    pytest.importorskip("tqdm")
     return OnnxEmbeddingFunction(
         model_name="all-MiniLM-L6-v2",
         hf_model_id="sentence-transformers/all-MiniLM-L6-v2",
         dimension=384,
         preferred_providers=None,
     )
@@
 def test_call_generates_embeddings() -> None:
-    pytest.importorskip("onnxruntime")
-    pytest.importorskip("tokenizers")
-    pytest.importorskip("tqdm")
-
     ef = _make_onnx()
     embeddings = ef("hello world")

In `@tests/unit_tests/test_onnx_embedding_function.py` around lines 12 - 18, The
helper _make_onnx() should guard optional dependencies via pytest.importorskip
so tests that call it don't fail when onnxruntime/tokenizers/tqdm are absent;
update _make_onnx() to call pytest.importorskip for the packages imported in
OnnxEmbeddingFunction.__init__ (onnxruntime, tokenizers, tqdm) before
constructing OnnxEmbeddingFunction and then remove redundant per-test
importorskip calls in test_dimension_property and test_call_empty_returns_empty
(leave test_call_generates_embeddings as-is or remove its duplicate skips),
referencing the helper _make_onnx and the tests test_dimension_property and
test_call_empty_returns_empty to locate changes.

⚠️ Potential issue | 🟡 Minor

Remove unused noqa to satisfy RUF100.

Ruff reports the # noqa: E402 is unused (E402 not enabled), so the directive should be removed.

-import pyseekdb  # noqa: E402
+import pyseekdb

In `@tests/integration_tests/conftest.py` around lines 13 - 18, The import line
"import pyseekdb  # noqa: E402" contains an unused noqa directive; remove the "
# noqa: E402" suffix so the line becomes a normal import (keep the existing
sys.path manipulation with repo_root, src_root, and sys.path.insert as-is) to
satisfy RUF100 and eliminate the unused noqa.