Skip to content

Conversation

@narengogi
Copy link
Collaborator

No description provided.

@narengogi narengogi requested a review from sk-portkey October 21, 2025 15:29
@matter-code-review
Copy link
Contributor

Code Quality bug fix

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

Fixed falsy value handling for temperature, top_p, and top_k parameters in Bedrock provider to prevent invalid configuration when values are 0 or false. Added explicit null/undefined checks to ensure valid numerical values are passed.

🔍 Impact of the Change

Ensures correct inference configuration by preserving valid zero/false values for model parameters. Prevents incorrect fallbacks due to loose falsy checks. Improves reliability of AI generation settings in production.

📁 Total Files Changed

File ChangeLog
utils.ts src/providers/bedrock/utils.ts Added strict null/undefined checks for temperature, top_p, and top_k to preserve valid falsy values in model config

🧪 Test Added/Recommended

Recommended

  • Unit test for temperature=0 to verify it's correctly passed
  • Test case with top_p=false (if applicable) to confirm non-falsy exclusion
  • Negative test with null/undefined to ensure filtering works

🔒 Security Vulnerabilities

No security vulnerabilities detected. Input validation is defensive and type-safe.

⏳ Estimated code review effort

LOW (~7 minutes)

Tip

Quality Recommendations

  1. Add unit tests to validate edge cases (0, false) for temperature, top_p, and top_k

  2. Consider type guarding for params to enforce expected parameter shapes

♫ Tanka Poem

Zero is not false, 🌡️
Params now know truth and flow—
Config stands corrected. 🌿
Logic blooms with care, precise,
Code breathes in clarity. 🌱

Sequence Diagram

sequenceDiagram
    participant Client
    participant Utils as Bedrock Utils
    
    Client->>Utils: buildInferenceConfig(params)
    
    alt temperature is not null/undefined
        Utils->>Utils: Set inferenceConfig.temperature
    end
    
    alt top_p is not null/undefined
        Utils->>Utils: Set inferenceConfig.topP
    end
    
    alt top_k is not null/undefined in additional fields
        Utils->>Utils: Set additionalModelRequestFields.top_k
    end
    
    Utils-->>Client: Return inferenceConfig
Loading

@matter-code-review
Copy link
Contributor

✅ Reviewed the changes: Fixes falsy value handling for Bedrock parameters to correctly accept 0 as a valid input.

@VisargD VisargD merged commit 2c12cd2 into Portkey-AI:main Oct 22, 2025
1 check passed
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.

2 participants