Skip to content

Commit 9700655

Browse files
authored
refactor(embedder-params): drop dimensions knob; pass indexing kwargs into LiteLLM ctor (#151)
- Remove `dimensions` from the litellm whitelist in `_ACCEPTED_KWARGS`. Output dimension must be identical for indexing and query for vectors to be comparable, so it's a model-wide setting, not a per-side knob — exposing it under `indexing_params` / `query_params` invited misconfiguration. Updated comment template, README, design doc, and testing plan accordingly. - Plumb `indexing_params` into `create_embedder` and pass them as constructor kwargs to `PacedLiteLLMEmbedder`. The values land in `self._kwargs` and become defaults forwarded into every `litellm.aembedding` call — including paths that don't go through the `INDEXING_EMBED_PARAMS` context var (e.g. the dim probe in `_get_dim`). Per-call overrides (`query_params` spread at query time) still win because `_embed` overlays kwargs on top of `self._kwargs`. Sentence- transformers ignores `indexing_params` (its constructor doesn't accept arbitrary kwargs; `prompt_name` is per-call only).
1 parent ee3515f commit 9700655

8 files changed

Lines changed: 68 additions & 15 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ embedding:
397397
# `ccc init` auto-populates these for known models (e.g. Cohere, Voyage, Nvidia NIM,
398398
# nomic-ai code-retrieval models, Snowflake arctic-embed).
399399
# indexing_params:
400-
# input_type: search_document # litellm: input_type, dimensions
400+
# input_type: search_document # litellm: input_type
401401
# query_params:
402402
# input_type: search_query # sentence-transformers: prompt_name
403403

@@ -427,7 +427,7 @@ embedding:
427427

428428
OpenAI embeddings (`text-embedding-3-*`, `text-embedding-ada-002`) are intentionally not in the list: they're symmetric and have no equivalent knob.
429429

430-
**Accepted keys:** `prompt_name` (sentence-transformers), `input_type` and `dimensions` (litellm). Other keys are rejected at daemon startup with a clear error.
430+
**Accepted keys:** `prompt_name` (sentence-transformers) and `input_type` (litellm). Other keys are rejected at daemon startup with a clear error. Note: `dimensions` is intentionally not exposed here — output dimension must be identical for indexing and query, so it's a model-wide setting rather than a per-side knob.
431431

432432
**Doctor checks both sides.** `ccc doctor` exercises the model once with `indexing_params` and once with `query_params`, reporting each as a separate `Model Check (indexing)` / `Model Check (query)` entry — so a misconfiguration on one side is diagnosable without hiding behind the other.
433433

src/cocoindex_code/daemon.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ def run_daemon() -> None:
586586
handshake_warnings.append(
587587
_build_backward_compat_warning(user_settings, user_settings_path())
588588
)
589-
embedder = create_embedder(user_settings.embedding)
589+
embedder = create_embedder(user_settings.embedding, indexing_params=indexing_params)
590590
else:
591591
settings_env_keys = []
592592
embedder = None

src/cocoindex_code/embedder_params.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@
2121

2222

2323
# Accepted kwargs per provider. Intentionally minimal — we only expose knobs
24-
# that users have reason to tune. ``normalize_embeddings`` (sentence-
25-
# transformers) and ``encoding_format`` (litellm) are deliberately excluded
26-
# because other code assumes unit vectors (query._l2_to_score) and float
27-
# payloads (litellm_embedder hardcodes encoding_format="float").
24+
# that users have reason to tune AND that make sense per-side (indexing vs
25+
# query). Excluded keys:
26+
# - ``normalize_embeddings`` (sentence-transformers): query._l2_to_score
27+
# assumes unit vectors.
28+
# - ``encoding_format`` (litellm): litellm_embedder hardcodes "float".
2829
_ACCEPTED_KWARGS: dict[str, frozenset[str]] = {
2930
"sentence-transformers": frozenset({"prompt_name"}),
30-
"litellm": frozenset({"input_type", "dimensions"}),
31+
"litellm": frozenset({"input_type"}),
3132
}
3233

3334

src/cocoindex_code/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ def save_user_settings(settings: UserSettings) -> Path:
544544
"litellm": (
545545
" #\n"
546546
" # Extra kwargs passed to the embedder. Supported keys:\n"
547-
" # input_type, dimensions\n"
547+
" # input_type\n"
548548
" # indexing_params: {}\n"
549549
" # query_params: {}\n"
550550
),

src/cocoindex_code/shared.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,26 @@ async def check_embedding(
7676
return EmbeddingCheckResult(dim=None, error=msg)
7777

7878

79-
def create_embedder(settings: EmbeddingSettings) -> Embedder:
80-
"""Create and return an embedder instance based on settings."""
79+
def create_embedder(
80+
settings: EmbeddingSettings,
81+
indexing_params: dict[str, Any] | None = None,
82+
) -> Embedder:
83+
"""Create and return an embedder instance based on settings.
84+
85+
For LiteLLM embedders, *indexing_params* (e.g. ``{"input_type": "passage"}``)
86+
are passed to the constructor as default kwargs forwarded into every
87+
``litellm.aembedding`` call — including paths that don't go through
88+
:data:`INDEXING_EMBED_PARAMS` (e.g. the dimension probe in ``_get_dim``,
89+
or any helper that calls ``embed()`` with no per-side kwargs). Per-call
90+
overrides (the ``query_params`` spread at query time) still take effect
91+
because :meth:`LiteLLMEmbedder._embed` overlays kwargs on top of the
92+
constructor's ``self._kwargs``.
93+
94+
*indexing_params* is ignored for sentence-transformers — its constructor
95+
doesn't accept arbitrary kwargs; ``prompt_name`` is a per-call argument
96+
only and the indexing default is supplied at the call site via
97+
:data:`INDEXING_EMBED_PARAMS`.
98+
"""
8199
if settings.provider == "sentence-transformers":
82100
from cocoindex.ops.sentence_transformers import SentenceTransformerEmbedder
83101

@@ -103,6 +121,7 @@ def create_embedder(settings: EmbeddingSettings) -> Embedder:
103121
instance = PacedLiteLLMEmbedder(
104122
settings.model,
105123
min_interval_ms=min_interval_ms,
124+
**(dict(indexing_params) if indexing_params else {}),
106125
)
107126
logger.info(
108127
"Embedding model (LiteLLM): %s | min_interval_ms: %s",

tests/test_embedder_params.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@
1414

1515
def test_validate_params_accepts_known_keys() -> None:
1616
validate_params("sentence-transformers", {}, {"prompt_name": "query"})
17-
validate_params(
18-
"litellm", {"input_type": "passage"}, {"input_type": "query", "dimensions": 512}
19-
)
17+
validate_params("litellm", {"input_type": "passage"}, {"input_type": "query"})
18+
19+
20+
def test_validate_params_rejects_dimensions() -> None:
21+
"""`dimensions` is a model-wide setting, not a per-side knob — must be rejected."""
22+
with pytest.raises(ValueError, match="dimensions"):
23+
validate_params("litellm", {"dimensions": 512}, {})
2024

2125

2226
def test_validate_params_rejects_unknown_key() -> None:

tests/test_settings.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,4 +620,6 @@ def test_save_initial_writes_comment_template_for_unknown_litellm() -> None:
620620
assert "# indexing_params: {}" in content
621621
assert "# query_params: {}" in content
622622
assert "input_type" in content
623-
assert "dimensions" in content
623+
# `dimensions` is intentionally NOT in the litellm template — it must be
624+
# the same on both sides, so we don't expose it as a per-side knob.
625+
assert "dimensions" not in content

tests/test_shared.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,33 @@ def test_create_embedder_uses_paced_litellm_embedder() -> None:
3939
assert embedder._min_request_interval_seconds == 0.3
4040

4141

42+
def test_create_embedder_litellm_passes_indexing_params_as_constructor_default() -> None:
43+
"""Indexing params become default kwargs forwarded into every litellm call —
44+
covering paths that don't go through INDEXING_EMBED_PARAMS (dim probe, etc.).
45+
"""
46+
embedder = create_embedder(
47+
EmbeddingSettings(provider="litellm", model="cohere/embed-english-v3.0"),
48+
indexing_params={"input_type": "search_document"},
49+
)
50+
assert isinstance(embedder, PacedLiteLLMEmbedder)
51+
assert embedder._kwargs == {"input_type": "search_document"}
52+
53+
54+
def test_create_embedder_sentence_transformers_ignores_indexing_params() -> None:
55+
"""The SentenceTransformer constructor doesn't accept arbitrary kwargs;
56+
indexing_params is silently ignored for that provider.
57+
"""
58+
embedder = create_embedder(
59+
EmbeddingSettings(
60+
provider="sentence-transformers", model="sentence-transformers/all-MiniLM-L6-v2"
61+
),
62+
indexing_params={"prompt_name": "passage"},
63+
)
64+
# No exception, and prompt_name is not stashed on the constructor —
65+
# it's a per-call argument supplied via the embed() call site.
66+
assert not isinstance(embedder, PacedLiteLLMEmbedder)
67+
68+
4269
def test_is_sentence_transformers_installed_true_in_dev() -> None:
4370
# Dev env pulls in sentence-transformers via the `dev` extras group.
4471
assert is_sentence_transformers_installed() is True

0 commit comments

Comments
 (0)