Skip to content

fix: use API key environment variables during model initialization - #1250

Merged
Pouyanpi merged 1 commit into
developfrom
fix/api-key-init
Jun 30, 2025
Merged

fix: use API key environment variables during model initialization#1250
Pouyanpi merged 1 commit into
developfrom
fix/api-key-init

Conversation

@Pouyanpi

Copy link
Copy Markdown
Collaborator
  • Add _prepare_model_kwargs() helper to handle API key env vars
  • Fix duplication in _init_llms() method
  • Update output parsers to return list instead of tuple
  • Update self-check actions for new parser interface
  • Add comprehensive tests for API key functionality

Fixes issue where api_key_env_var was validated but ignored during LLM initialization

Comment thread tests/test_llmrails.py
Comment thread tests/test_llmrails.py
@Pouyanpi
Pouyanpi requested review from Copilot and cparisien June 27, 2025 12:56
@Pouyanpi Pouyanpi added this to the v0.14.1 milestone Jun 27, 2025
@Pouyanpi Pouyanpi added the bug Something isn't working label Jun 27, 2025

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses an issue where the API key environment variable was validated but ignored during LLM initialization. Key changes include:

  • Adding the _prepare_model_kwargs() helper to extract the API key from environment variables.
  • Refactoring _init_llms() to use the new helper and remove duplicated logic.
  • Updating tests to verify correct API key handling in both main and non‐main models.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/test_llmrails.py Adds tests to validate API key extraction from environment variables and graceful handling when missing.
nemoguardrails/rails/llm/llmrails.py Introduces _prepare_model_kwargs(), removes duplicate env var lookup code in _init_llms(), and updates LLM initialization accordingly.

Comment thread nemoguardrails/rails/llm/llmrails.py
@Pouyanpi

Copy link
Copy Markdown
Collaborator Author

fix related to #1221

@codecov-commenter

codecov-commenter commented Jun 27, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.54%. Comparing base (c5c7dd9) to head (209cf9d).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1250      +/-   ##
===========================================
+ Coverage    69.51%   69.54%   +0.02%     
===========================================
  Files          161      161              
  Lines        16013    16016       +3     
===========================================
+ Hits         11132    11138       +6     
+ Misses        4881     4878       -3     
Flag Coverage Δ
python 69.54% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
nemoguardrails/rails/llm/llmrails.py 88.18% <100.00%> (+0.57%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Pouyanpi Pouyanpi closed this Jun 27, 2025
@Pouyanpi Pouyanpi reopened this Jun 27, 2025
- Add _prepare_model_kwargs() helper to handle API key env vars
- Fix duplication in _init_llms() method
- Update output parsers to return list instead of tuple
- Update self-check actions for new parser interface
- Add comprehensive tests for API key functionality

Fixes issue where api_key_env_var was validated but ignored during LLM initializationix: use API key environment variables during model initialization- Add _prepare_model_kwargs() helper to handle API key env vars- Fix duplication in _init_llms() method- Update output parsers to return list instead of tuple- Update self-check actions for new parser interface- Add comprehensive tests for API key functionalityFixes issue where api_key_env_var was validated but ignored during LLM initialization
@Pouyanpi
Pouyanpi merged commit 56bf4f6 into develop Jun 30, 2025
@Pouyanpi
Pouyanpi deleted the fix/api-key-init branch June 30, 2025 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants