Prefer AICHAT_API_KEY in default lookup#56
Conversation
Change default key resolution in AI::Chat and generate_schema! to check AICHAT_API_KEY first and fall back to OPENAI_API_KEY when the first value is missing or empty. Keep explicit configuration behavior unchanged: - api_key: still has highest precedence. - api_key_env_var: still targets exactly one env var. Add unit coverage for precedence and fallback rules, update integration test setup to accept either env var, and refresh README examples to match runtime behavior. Bump gem version to 0.5.7 and add a changelog entry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cfda555f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| it "accepts a custom environment variable name" do | ||
| ENV["CUSTOM_OPENAI_KEY"] = ENV["OPENAI_API_KEY"] | ||
| ENV["CUSTOM_OPENAI_KEY"] = ENV["AICHAT_API_KEY"] || ENV["OPENAI_API_KEY"] |
There was a problem hiding this comment.
Treat empty AICHAT_API_KEY as missing in integration key setup
Using ENV["AICHAT_API_KEY"] || ENV["OPENAI_API_KEY"] does not implement the intended fallback when AICHAT_API_KEY is present but empty, because Ruby treats "" as truthy. In environments where AICHAT_API_KEY="" and OPENAI_API_KEY is valid, this assigns an empty key to CUSTOM_OPENAI_KEY, so the custom-env integration test uses an invalid credential and can fail even though the runtime default lookup is designed to fall back to OPENAI_API_KEY for empty values.
Useful? React with 👍 / 👎.
In integration API key tests, treats empty strings as present and skips fallback. Use explicit empty checks before falling back to OPENAI_API_KEY for custom env-var and direct api_key test setup. This aligns test setup with runtime behavior and addresses the PR #56 review finding.
Summary
This PR updates default API key lookup to prefer
AICHAT_API_KEYwhilepreserving backward compatibility with
OPENAI_API_KEY.What Changed
AI::Chat.newandAI::Chat.generate_schema!default keyresolution:
AICHAT_API_KEYOPENAI_API_KEYwhenAICHAT_API_KEYis missing orempty
api_key:still has highest precedenceapi_key_env_var:still uses that exact environment variableAICHAT_API_KEYfallbackapi_key_env_var:behaviorapi_key:behaviorAICHAT_API_KEYorOPENAI_API_KEYis setOpenAI::Clientexample to matchruntime behavior
0.5.6to0.5.70.5.7Compatibility
OPENAI_API_KEYcontinue to work unchanged.AICHAT_API_KEYwithoutchanging app-wide key names.
Verification
bundle exec standardrb lib/ai/chat.rb spec/unit/chat_spec.rb spec/integration/ai_chat_integration_spec.rb spec/support/integration_helper.rbbundle exec rspec spec/unit/chat_spec.rbbundle exec rspec spec/integration/ai_chat_integration_spec.rb:227All commands passed.