fix: UniversalAPIEmbedder now passes embedding_dims to API calls

[RerankerGuo]

Description

Fixes #2177

The UniversalAPIEmbedder previously silently ignored the embedding_dims config field when making embeddings.create() calls. This caused models like text-embedding-3-large to always return the full default dimension embedding (e.g. 3072), making it impossible to use the dimensions parameter for reduced-dimensional embeddings.

Changes

1. Added _build_embedding_kwargs() helper — conditionally includes the dimensions parameter when embedding_dims is set in config
2. Extracted _call_embeddings_api() method — handles both primary and backup client paths with unified dimension support
3. Added graceful fallback — if the API rejects the dimensions parameter (e.g. older model versions or non-Ollama providers), automatically retries without it
4. Both primary and backup client paths now use the same dimensions-aware calling logic
5. Added comprehensive unit tests in tests/embedders/test_universal_api.py

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Refactor (improved code structure via helper extraction)

How Has This Been Tested?

• python3 -m py_compile src/memos/embedders/universal_api.py passes
• python3 -m py_compile tests/embedders/test_universal_api.py passes
• 5 logic tests for _build_embedding_kwargs (no dims / with dims / zero dims / empty list / batch)
• Fallback behavior verified: when dimensions not supported, auto-retry without
• No behavior change when embedding_dims=None (backward compatible)

Checklist

• I have performed a self-review of my own code
• I have commented my code in hard-to-understand areas (docstrings on new methods)
• I have added tests that prove my fix is effective
• I have linked the issue to this PR (Closes fix: UniversalAPIEmbedder silently ignores embedding_dims and never passes dimensions to the OpenAI API #2177)

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2180
Task: df83200ccde2f1fc
Base: main
Head: fix/issue-2177-embedding-dims

🔍 OpenCodeReview found 4 issue(s) in this PR.

---

1. src/memos/embedders/universal_api.py (L80-L96)

The fallback retry is triggered by any exception, not just those indicating an unsupported dimensions parameter. Timeouts (asyncio.TimeoutError), authentication failures (openai.AuthenticationError), and rate-limit errors (openai.RateLimitError) will all silently trigger an unnecessary second API call, masking the true failure.

Only errors that indicate the dimensions parameter is specifically rejected by the backend (typically openai.BadRequestError / HTTP 400) should trigger the fallback. Catching Exception broadly here is a correctness issue: a transient timeout on the primary call should not be retried as a different, potentially more expensive request.

Suggestion: narrow the catch to the specific exception type(s) that signal a bad request, e.g.:

from openai import BadRequestError

except BadRequestError as e:
    if embedding_dims is not None:
        # fallback
        ...
    raise

---

2. src/memos/embedders/universal_api.py (L88-L94)

A second asyncio.run() call is made inside the except block of an outer asyncio.run() call. While this is safe in a purely synchronous call stack, asyncio.run() itself creates and closes a new event loop; calling it again within the same thread after the first one completes is technically fine in CPython, but if _call_embeddings_api is ever called from within an already-running event loop (e.g., Jupyter notebooks, or any async test runner), both asyncio.run() calls will raise RuntimeError: This event loop is already running.

Additionally, there is no try/except around this fallback asyncio.run() call, meaning any error from the fallback (including asyncio.TimeoutError) propagates as a raw exception rather than a meaningful error message. The caller (embed()) wraps errors in ValueError, but that only applies to exceptions that escape _call_embeddings_api through the raise at the bottom — the fallback error here bypasses that logic entirely and surfaces as a raw asyncio.TimeoutError or similar.

Consider consolidating both API calls into a single coroutine that handles the fallback internally, then calling asyncio.run() once:

💡 Suggested Change

Before:

                fallback_kwargs = self._build_embedding_kwargs(model, texts, None)
                response = asyncio.run(
                    asyncio.wait_for(
                        client.embeddings.create(**fallback_kwargs),
                        timeout=timeout,
                    )
                )

After:

    def _call_embeddings_api(
        self, client, model: str, texts: list[str], timeout: int
    ) -> list[list[float]]:
        embedding_dims = getattr(self.config, "embedding_dims", None)

        async def _run():
            kwargs = self._build_embedding_kwargs(model, texts, embedding_dims)
            try:
                response = await asyncio.wait_for(
                    client.embeddings.create(**kwargs), timeout=timeout
                )
                return [r.embedding for r in response.data]
            except openai.BadRequestError as e:
                if embedding_dims is not None:
                    logger.warning(...)
                    fallback_kwargs = self._build_embedding_kwargs(model, texts, None)
                    response = await asyncio.wait_for(
                        client.embeddings.create(**fallback_kwargs), timeout=timeout
                    )
                    return [r.embedding for r in response.data]
                raise

        return asyncio.run(_run())

---

3. tests/embedders/test_universal_api.py (L82-L85)

The test asserts mock_create.call_args to check whether dimensions was passed, but never asserts on the return value of embedder.embed(["hello"]). If embed() raises an internally-swallowed exception or returns an incorrect result, this test would still pass as long as mock_create was called with the right arguments. Adding a result assertion (e.g., assert result == [[0.1, 0.2]]) would make the test more robust and actually validate end-to-end correctness.

Same pattern applies to test_embed_without_dims_does_not_pass_dimensions.

💡 Suggested Change

Before:

        embedder.embed(["hello"])

        _, kwargs = mock_create.call_args
        assert kwargs.get("dimensions") == 256

After:

        result = embedder.embed(["hello"])

        assert result == [[0.1, 0.2]]
        _, kwargs = mock_create.call_args
        assert kwargs.get("dimensions") == 256

---

4. tests/embedders/test_universal_api.py (L116-L127)

With embedding_dims=256 configured, _call_embeddings_api internally retries on any exception: the primary client's embeddings.create will be called twice (once with dimensions=256, once without) before the exception propagates to embed()'s outer handler, which then falls back to the backup client. This is the correct behaviour given the implementation, but it means the primary mock is called twice total — the test does not assert on primary_client.embeddings.create.await_count. Adding this assertion would prevent a regression if the retry logic is ever removed or changed.

💡 Suggested Change

Before:

        config = _make_config(
            api_key="primary",
            embedding_dims=256,
            backup_client=True,
            backup_api_key="backup-key",
            backup_base_url="https://api.example.com",
            backup_model_name_or_path="text-embedding-3-small",
        )
        embedder = UniversalAPIEmbedder(config)
        result = embedder.embed(["hello"])
        assert result == [[0.1, 0.2]]
        assert backup_create.await_count == 1

After:

        config = _make_config(
            api_key="primary",
            embedding_dims=256,
            backup_client=True,
            backup_api_key="backup-key",
            backup_base_url="https://api.example.com",
            backup_model_name_or_path="text-embedding-3-small",
        )
        embedder = UniversalAPIEmbedder(config)
        result = embedder.embed(["hello"])
        assert result == [[0.1, 0.2]]
        # Primary is called twice: once with dimensions (fails), once without (also fails → backup used)
        assert primary_client.embeddings.create.await_count == 2
        assert backup_create.await_count == 1

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (auto-generated test defect); treated as non-blocking. Manual review recommended. Details: The AI-generated tests mock client.embeddings.create with a synchronous function/exception, but the SUT wraps the call in asyncio.run(asyncio.wait_for(client.embeddings.create(...), ...)), which requires create to return an awaitable coroutine. [advisory, non-gating] AI-generated tests on branch test/auto-gen-f4fd7aa1f5c1ae47-20260728232248: 66/67 passed, 1 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/issue-2177-embedding-dims

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (auto-generated test defect); treated as non-blocking. Manual review recommended. Details: The tests mock client.embeddings.create with a synchronous function, but the code under test wraps the call in asyncio.run(asyncio.wait_for(...)), which requires an awaitable/coroutine. The tests fail to model the async contract that _call_embeddings_api expects. [advisory, non-gating] AI-generated tests on branch test/auto-gen-863b005ada4494c7-20260730092619: 67/92 passed, 25 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/issue-2177-embedding-dims

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (auto-generated test defect); treated as non-blocking. Manual review recommended. Details: All three failing tests crash inside asyncio.wait_for/ensure_future because the mocked client.embeddings.create(...) returns a plain MagicMock instead of an awaitable, so the asyncio machinery cannot schedule it as a future. [advisory, non-gating] AI-generated tests on branch test/auto-gen-1a47268566676cdb-20260731102051: 85/86 passed, 1 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/issue-2177-embedding-dims

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (9/9 executed). memos_python_core/changed-repo-python: 9/9. Duration: 5s

Branch: fix/issue-2177-embedding-dims