Skip to content

Litellm test bedrock optional params embeddings dimension#20262

Merged
Sameerlite merged 3 commits intomainfrom
litellm_test_bedrock_optional_params_embeddings_dimension
Feb 2, 2026
Merged

Litellm test bedrock optional params embeddings dimension#20262
Sameerlite merged 3 commits intomainfrom
litellm_test_bedrock_optional_params_embeddings_dimension

Conversation

@Sameerlite
Copy link
Collaborator

Relevant issues

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

@vercel
Copy link

vercel bot commented Feb 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 2, 2026 0:20am

Request Review

@Sameerlite Sameerlite merged commit 686aafe into main Feb 2, 2026
9 of 12 checks passed
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 2, 2026

Greptile Overview

Greptile Summary

This PR updates test expectations for Bedrock Cohere embeddings to reflect the newly added dimensions parameter support (introduced in PR #20235). The changes include:

  • Updated test_bedrock_optional_params_embeddings_dimension to expect dimensions support for bedrock/cohere.embed-multilingual-v3 (changed expected_dimensions from False to True)
  • Added JSON schema validation for the new cache_creation_input_token_cost_above_1hr_above_200k_tokens field
  • Updated Claude Sonnet model configurations with extended context window (200k → 1M tokens) and tiered pricing
  • Minor formatting improvements (import ordering, JSON indentation, float normalization)

The core change aligns test expectations with the actual behavior after dimensions support was added to Bedrock Cohere embeddings.

Confidence Score: 4/5

  • Safe to merge with minor test improvement recommended
  • The changes correctly update test expectations to match the new dimensions support for Cohere embeddings. All modifications are test/configuration updates with no runtime logic changes. The test could be strengthened by verifying the correct parameter name, but the current implementation is functionally correct.
  • tests/llm_translation/test_optional_params.py could benefit from stronger test assertions

Important Files Changed

Filename Overview
tests/llm_translation/test_optional_params.py Updated test to expect dimensions support for bedrock/cohere.embed-multilingual-v3 (changed from False to True)
tests/test_litellm/test_utils.py Added schema validation for new cache pricing field and reformatted imports (cosmetic changes)
litellm/model_prices_and_context_window_backup.json Updated Claude Sonnet pricing/limits and fixed indentation for openrouter model

Sequence Diagram

sequenceDiagram
    participant Test as test_bedrock_optional_params_embeddings_dimension
    participant Utils as get_optional_params_embeddings
    participant Config as BedrockCohereEmbeddingConfig
    participant Schema as JSON Schema Validator

    Note over Test: PR Updates Test Expectations
    Test->>Utils: Call with dimensions=20 for cohere.embed-multilingual-v3
    Utils->>Config: get_supported_openai_params()
    Config-->>Utils: ["encoding_format", "dimensions"]
    Utils->>Config: map_openai_params({"dimensions": 20})
    Config-->>Utils: {"output_dimension": 20}
    Utils-->>Test: Returns optional_params
    Test->>Test: Assert len(optional_params) == 1 ✓
    
    Note over Schema: PR Updates Schema Validation
    Schema->>Schema: Add cache_creation_input_token_cost_above_1hr_above_200k_tokens
    Schema->>Schema: Validate model_prices_and_context_window.json
    Schema-->>Test: Schema validation passes ✓
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

("bedrock/amazon.titan-embed-image-v1", True, "embeddingConfig"),
("bedrock/amazon.titan-embed-text-v2:0", True, "dimensions"),
("bedrock/cohere.embed-multilingual-v3", False, None),
("bedrock/cohere.embed-multilingual-v3", True, None),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test doesn't verify the correct parameter name. The dimensions_kwarg should be "output_dimension" (not None) to ensure dimensions are correctly mapped to Bedrock Cohere's output_dimension parameter, matching the pattern used for other models.

Suggested change
("bedrock/cohere.embed-multilingual-v3", True, None),
("bedrock/cohere.embed-multilingual-v3", True, "output_dimension"),
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/llm_translation/test_optional_params.py
Line: 227:227

Comment:
Test doesn't verify the correct parameter name. The `dimensions_kwarg` should be `"output_dimension"` (not `None`) to ensure dimensions are correctly mapped to Bedrock Cohere's `output_dimension` parameter, matching the pattern used for other models.

```suggestion
        ("bedrock/cohere.embed-multilingual-v3", True, "output_dimension"),
```

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant