feat(embeddings): fix embedding dispatch in executor #112

b3nw · 2026-01-31T05:29:20Z

The embedding endpoint existed but was broken because the executor refactoring (Nov 2025) forgot to add litellm.aembedding() dispatch. All requests were going through litellm.acompletion(), causing failures.

Changes:

Add request_type field to RequestContext (completion/embedding)
Update RotatingClient.aembedding() to set request_type='embedding'
Add conditional dispatch in RequestExecutor._execute_non_streaming() to call litellm.aembedding() for embedding requests
Refactored dispatch logic to reduce code duplication

Tested with Mistral (mistral-embed) - returns proper 1024-dim vectors.

Important

Fix embedding dispatch in RequestExecutor by adding request_type to RequestContext and updating logic to correctly handle embedding requests.

Behavior:
- Fix embedding dispatch in RequestExecutor._execute_non_streaming() to call litellm.aembedding() for embedding requests.
- Add request_type field to RequestContext in types.py to distinguish between completion and embedding requests.
- Update RotatingClient.aembedding() to set request_type='embedding'.
Testing:
- Tested with Mistral (mistral-embed) to ensure correct 1024-dim vector results.

^{This description was created by}^{for 2c755e8. You can customize this summary. It will automatically update as commits are pushed.}

b3nw · 2026-01-31T05:34:03Z

@mirrobot-agent your PR review workflow failed, re-trigger it.

mirrobot-agent · 2026-01-31T05:34:32Z

@b3nw, I've received your request to re-trigger the review. I'm starting my analysis of the embedding dispatch fixes now and will post the results shortly.

mirrobot-agent

Overall Assessment

This PR correctly identifies and fixes the issue where embedding requests were being dispatched to litellm.acompletion instead of litellm.aembedding. The introduction of request_type in RequestContext is a solid approach and the refactor in RequestExecutor to dynamically select the call function is clean.

Architectural Feedback

The addition of request_type to RequestContext is a good way to handle different API operation types within the same executor logic. This pattern could potentially be extended to other operation types in the future.

Key Suggestions

Enhance aembedding feature parity: Currently, aembedding in RotatingClient is missing model resolution and transaction logging.
Async Consistency: Consider making aembedding an async def to match acompletion.

Warnings

None.

This review was generated by an AI assistant.

mirrobot-agent · 2026-01-31T05:36:15Z

src/rotator_library/client/rotating_client.py

            provider=provider,
            kwargs=kwargs,
            streaming=False,
+            request_type="embedding",


While adding request_type="embedding" here is correct, the aembedding method is still missing feature parity with acompletion. Specifically, it lacks model resolution and transaction logging.

Also, for consistency with the rest of the client, this should ideally be an async def and await the executor.

Suggested change

request_type="embedding",

# Resolve model ID

resolved_model = self._model_resolver.resolve_model_id(model, provider)

kwargs["model"] = resolved_model

# Create transaction logger if enabled

transaction_logger = None

if self.enable_request_logging:

transaction_logger = TransactionLogger(

provider=provider,

model=resolved_model,

enabled=True,

parent_dir=kwargs.pop("_parent_log_dir", None),

)

transaction_logger.log_request(kwargs)

# Build request context (embeddings are never streaming)

context = RequestContext(

model=resolved_model,

provider=provider,

kwargs=kwargs,

streaming=False,

request_type="embedding",

credentials=self.all_credentials.get(provider, []),

deadline=time.time() + self.global_timeout,

request=request,

pre_request_callback=pre_request_callback,

transaction_logger=transaction_logger,

)

return await self._executor.execute(context)

Note: This would also require changing the function signature to async def aembedding(...).

mirrobot-agent · 2026-01-31T05:36:19Z

@b3nw, I've completed the re-triggered review for PR #112.

Summary

The fixes for embedding dispatch are technically sound and resolve the core issue where embeddings were incorrectly using the completion endpoint. I've suggested some further enhancements to bring the aembedding method to full feature parity with acompletion.

Key Findings

Correct Dispatch: The use of request_type to distinguish between call types is a good architectural choice.
Improved Executor Logic: The dynamic selection of aembedding vs acompletion in the executor reduces code duplication.
Enhancement Opportunity: aembedding is currently missing model resolution and transaction logging, which I've highlighted in the review comments.

You can find the detailed feedback and code suggestions in the formal review.

This update was generated by an AI assistant.

The embedding endpoint existed but was broken because the executor refactoring (Nov 2025) forgot to add litellm.aembedding() dispatch. All requests were going through litellm.acompletion(), causing failures. Changes: - Add request_type field to RequestContext (completion/embedding) - Update RotatingClient.aembedding() to set request_type='embedding' - Add conditional dispatch in RequestExecutor._execute_non_streaming() to call litellm.aembedding() for embedding requests - Refactored dispatch logic to reduce code duplication - Added feature parity to aembedding: async signature, model resolution, and transaction logging (matching acompletion) Tested with Mistral (mistral-embed) - returns proper 1024-dim vectors.

b3nw · 2026-01-31T06:03:21Z

@mirrobot-agent addressed your items, re-review

mirrobot-agent · 2026-01-31T06:03:56Z

@b3nw, I've received your request for a re-review. I'll check the latest changes to ensure the embedding dispatch logic is fully aligned with the suggestions. I'll post my findings shortly! 🔍

mirrobot-agent

Overall Assessment

The updates successfully address all the points raised in the previous review. The aembedding method now has full feature parity with acompletion, and the dispatch logic in the RequestExecutor is clean and robust.

Key Changes Verified

Async Parity: aembedding is now an async def and properly awaits the executor.
Model Resolution: Added _model_resolver.resolve_model_id to ensure embedding requests use resolved model IDs.
Transaction Logging: Integrated TransactionLogger into the embedding flow, ensuring requests and responses are logged when enabled.
Clean Dispatch: The refactored logic in RequestExecutor._execute_non_streaming correctly selects between litellm.aembedding and litellm.acompletion based on the request_type.

This is a great improvement to the library's embedding support.

This review was generated by an AI assistant.

mirrobot-agent · 2026-01-31T06:04:28Z

src/rotator_library/client/rotating_client.py

@@ -375,19 +375,39 @@ def aembedding(
                f"Invalid model format or no credentials for provider: {model}"
            )

+        # Extract internal logging parameters (not passed to API)
+        parent_log_dir = kwargs.pop("_parent_log_dir", None)
+
+        # Resolve model ID
+        resolved_model = self._model_resolver.resolve_model_id(model, provider)
+        kwargs["model"] = resolved_model
+
+        # Create transaction logger if enabled
+        transaction_logger = None
+        if self.enable_request_logging:
+            transaction_logger = TransactionLogger(
+                provider=provider,
+                model=resolved_model,
+                enabled=True,
+                parent_dir=parent_log_dir,
+            )
+            transaction_logger.log_request(kwargs)
+
        # Build request context (embeddings are never streaming)
        context = RequestContext(
-            model=model,
+            model=resolved_model,
            provider=provider,
            kwargs=kwargs,
            streaming=False,
+            request_type="embedding",
            credentials=self.all_credentials.get(provider, []),
            deadline=time.time() + self.global_timeout,
            request=request,
            pre_request_callback=pre_request_callback,
+            transaction_logger=transaction_logger,
        )

-        return self._executor.execute(context)
+        return await self._executor.execute(context)


Fix verified: aembedding now has full feature parity with acompletion, including async support, model resolution, and transaction logging. This resolves my previous feedback.

mirrobot-agent · 2026-01-31T06:04:33Z

@b3nw, I've completed the re-review for PR #112.

Summary

The updates successfully address all the items identified in the previous review. The embedding support is now fully integrated with the library's core features (model resolution, transaction logging, and async execution).

Key Improvements

Full Feature Parity: aembedding now supports model resolution and transaction logging, matching the capabilities of the completion endpoint.
Improved Architecture: The introduction of request_type in RequestContext allows for clean and maintainable dispatch logic in the RequestExecutor.
Async Consistency: The aembedding method is now correctly implemented as an async def.

I've approved the PR. Great work on these fixes!

This update was generated by an AI assistant.

b3nw requested a review from Mirrowel as a code owner January 31, 2026 05:29

mirrobot-agent bot reviewed Jan 31, 2026

View reviewed changes

b3nw force-pushed the feature/embedding-support branch from 2c755e8 to 68babe7 Compare January 31, 2026 06:02

mirrobot-agent bot approved these changes Jan 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(embeddings): fix embedding dispatch in executor #112

feat(embeddings): fix embedding dispatch in executor #112

Uh oh!

b3nw commented Jan 31, 2026 •

edited by ellipsis-dev bot

Loading

Uh oh!

b3nw commented Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

mirrobot-agent bot left a comment

Uh oh!

mirrobot-agent bot Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

b3nw commented Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

mirrobot-agent bot left a comment

Uh oh!

mirrobot-agent bot Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

feat(embeddings): fix embedding dispatch in executor #112

Are you sure you want to change the base?

feat(embeddings): fix embedding dispatch in executor #112

Uh oh!

Conversation

b3nw commented Jan 31, 2026 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b3nw commented Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

mirrobot-agent bot left a comment

Choose a reason for hiding this comment

Overall Assessment

Architectural Feedback

Key Suggestions

Warnings

Uh oh!

mirrobot-agent bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Summary

Key Findings

Uh oh!

b3nw commented Jan 31, 2026

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Uh oh!

mirrobot-agent bot left a comment

Choose a reason for hiding this comment

Overall Assessment

Key Changes Verified

Uh oh!

mirrobot-agent bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

mirrobot-agent bot commented Jan 31, 2026

Summary

Key Improvements

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

b3nw commented Jan 31, 2026 •

edited by ellipsis-dev bot

Loading