Skip to content

Conversation

@vidurkhanal
Copy link
Member

  • Added aws-config and aws-sdk-bedrockruntime dependencies to enable Amazon Bedrock integration.
  • Implemented AmazonBedrockProvider with support for object generation via the Bedrock Converse API.
  • Updated AIProvider to route object generation requests to Bedrock when selected.
  • Introduced error handling for Bedrock-specific and deserialization errors.
  • Refactored provider builder for Bedrock, including region, credentials, and header management.
  • Added test scaffolding for Bedrock provider.
  • Updated error types and retry logic to handle new Bedrock error cases.

- Added `aws-config` and `aws-sdk-bedrockruntime` dependencies to enable Amazon Bedrock integration.
- Implemented `AmazonBedrockProvider` with support for object generation via the Bedrock Converse API.
- Updated `AIProvider` to route object generation requests to Bedrock when selected.
- Introduced error handling for Bedrock-specific and deserialization errors.
- Refactored provider builder for Bedrock, including region, credentials, and header management.
- Added test scaffolding for Bedrock provider.
- Updated error types and retry logic to handle new Bedrock error cases.
Implement the `generate_text` method for the `AmazonBedrockProvider`, enabling text generation via Amazon Bedrock. Add request normalization logic and integrate with the provider selection in `AIProvider`. Include unit tests for `generate_text` to verify functionality.
@vidurkhanal vidurkhanal marked this pull request as ready for review December 29, 2025 22:15
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 29, 2025

Greptile Summary

This PR adds Amazon Bedrock provider support to the AI module, implementing text and object generation via the Bedrock Converse API.

Key Changes:

  • Implemented AmazonBedrockProvider with GeneratesText and GeneratesObject traits
  • Added AmazonBedrockProviderBuilder with region and credential configuration
  • Integrated provider into AIProvider enum routing system
  • Added BedrockConverseError variant to error handling with retry logic
  • Included test scaffolding for both text and object generation

Critical Issues:

  • unsafe block used to set environment variables (lines 299-305) creates potential data races and violates Rust safety - credentials should be passed directly to AWS SDK
  • Base64 image data not decoded before passing to AWS API (line 200-202)
  • Method naming typo: normalizer_generate_object_request should be normalize_generate_object_request

Minor Issues:

  • default_headers field defined but never used
  • Cross-provider dependency on OpenAIProvider for utility method
  • Test placeholder typos and todo!() instead of unimplemented!()

Confidence Score: 2/5

  • Not safe to merge - contains critical security and logic issues
  • The unsafe environment variable manipulation creates thread-safety issues, and the base64 image handling bug will cause runtime failures. The typo in the method name will cause compilation errors.
  • Pay close attention to crates/umem_ai/src/providers/amazon_bedrock.rs - contains unsafe code block and logic errors that must be fixed

Important Files Changed

Filename Overview
crates/umem_ai/src/providers/amazon_bedrock.rs Added AmazonBedrockProvider with text/object generation support, but contains critical security issue with unsafe environment variable setting and typo in method name
crates/umem_ai/src/response_generators/mod.rs Added BedrockConverseError variant to error enum following project conventions
crates/umem_ai/src/utils.rs Added retry logic for Bedrock errors, consistent with existing error handling patterns
crates/umem_ai/src/lib.rs Integrated AmazonBedrockProvider into AIProvider enum and routing logic, following existing patterns

Sequence Diagram

sequenceDiagram
    participant Client
    participant AIProvider
    participant AmazonBedrockProvider
    participant AWSBedrockClient
    participant BedrockAPI

    Client->>AIProvider: generate_object(request)
    AIProvider->>AIProvider: match provider type
    AIProvider->>AmazonBedrockProvider: generate_object(request)
    
    AmazonBedrockProvider->>AmazonBedrockProvider: normalize_generate_object_request()
    Note over AmazonBedrockProvider: Extract system message from OpenAIProvider<br/>Build user messages with schema
    
    AmazonBedrockProvider->>AWSBedrockClient: converse().send()
    AWSBedrockClient->>BedrockAPI: HTTP Request (Converse API)
    
    alt Success
        BedrockAPI-->>AWSBedrockClient: Converse Response
        AWSBedrockClient-->>AmazonBedrockProvider: ConverseOutput
        AmazonBedrockProvider->>AmazonBedrockProvider: Extract text from output message
        AmazonBedrockProvider->>AmazonBedrockProvider: Deserialize JSON to type T
        AmazonBedrockProvider-->>AIProvider: GenerateObjectResponse<T>
        AIProvider-->>Client: Result<GenerateObjectResponse<T>>
    else Error
        BedrockAPI-->>AWSBedrockClient: SDK Error
        AWSBedrockClient-->>AmazonBedrockProvider: Error
        AmazonBedrockProvider-->>AIProvider: ResponseGeneratorError::BedrockConverseError
        AIProvider-->>Client: Err(ResponseGeneratorError)
    end
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.

6 files reviewed, 9 comments

Edit Code Review Agent Settings | Greptile

Implement the `generate_text` method for the `AmazonBedrockProvider`, enabling text generation via Amazon Bedrock. Add request normalization logic and integrate with the provider selection in `AIProvider`. Include unit tests for `generate_text` to verify functionality.
Implement the `generate_text` method for the `AmazonBedrockProvider`, enabling text generation via Amazon Bedrock. Add request normalization logic and integrate with the provider selection in `AIProvider`. Include unit tests for `generate_text` to verify functionality.
…vider

- Add `aws-smithy-types` dependency to support structured model request fields.
- Update `AmazonBedrockProvider` to set `InferenceConfiguration` (temperature, top_p, max_tokens) on Bedrock requests.
- Pass custom headers as additional model request fields using `aws_smithy_types::Document`.
- Refactor provider builder to use explicit credentials and provider name, removing reliance on environment variables and default headers.
- Use `Arc` for Bedrock client to support potential sharing.
… to Bedrock

Previously, base64-encoded image data was passed as raw bytes, which could result in invalid images being sent to AWS Bedrock. This change decodes the base64 string before constructing the ImageBlock, ensuring valid image data is used. Also improves error handling and clarifies unimplemented file handling.
…ponse context

- Update `ResponseGeneratorError::Deserialization` to include both the serde_json error and the original response string.
- Enhance error messages and tracing logs to display both the error and the problematic response, aiding debugging.
- Change Amazon Bedrock and OpenAI providers to pass the response text into deserialization errors.
- Update error handling in utils to log both error and response content for deserialization failures.
…ement

- Enhance AmazonBedrockProvider to use Bedrock's tool use API for structured JSON output.
- Add conversion utilities between serde_json::Value and aws_smithy_types::Document.
- Update error handling and deserialization logic to support tool use responses.
- Configure tool specification and input schema for JSON output in Bedrock requests.
- Bump aws-smithy-types dependency to enable required features.
- Introduce `rerank` and `structured_rerank` modules to support ranking and structured ranking of documents.
- Add `RerankRequest`, `RerankResponse`, and builder with validation for reranking text documents.
- Add `StructuredRerankRequest`, `StructuredRerankResponse`, and builder for reranking structured data with generic support.
- Expose new modules in `response_generators::mod.rs`.
- Make `generate_text` and `generate_object` requests constructible via builder methods.
- Add `serde-saphyr` dependency for structured serialization.
- Update `Cargo.lock` with new dependencies and versions.
…nk API to use top_k

- Derive Debug for AIProvider, RerankingModel, and provider structs (AmazonBedrockProvider, AzureOpenAIProvider, GoogleVertexAIProvider, GoogleCredentials, XAIProvider, StructuredRerankRequest, StructuredRerankResponse, StructuredRanking, RerankResponse, Ranking)
- Refactor rerank API to use `top_k` instead of `top_n` across request structs, builders, and provider implementations
- Update Amazon Bedrock provider to require region, fix model ARN construction, and add new rerank/structured_rerank tests
- Update Cohere provider to use `top_k` for rerank requests
@vidurkhanal vidurkhanal merged commit 478c732 into main Jan 20, 2026
1 check passed
@vidurkhanal vidurkhanal deleted the feat/aws-runtime branch January 20, 2026 04:41
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.

3 participants