Fix binding options not loaded from environment variables#2585
Merged
danielaskdd merged 2 commits intoHKUDS:mainfrom Jan 15, 2026
Merged
Fix binding options not loaded from environment variables#2585danielaskdd merged 2 commits intoHKUDS:mainfrom
danielaskdd merged 2 commits intoHKUDS:mainfrom
Conversation
• Extract binding value determination logic • Consolidate env var fallback handling • Simplify conditional option registration • Improve code readability and maintainability • Use consistent parsing for both bindings
• Override __getattribute__ vs __getattr__ • Handle __dict__ access for vars() calls • Maintain backward compatibility • Support provider config extraction • Add explicit global declarations
Collaborator
Author
|
@codex review |
This was referenced Jan 15, 2026
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
cleo-ia
added a commit
to cleo-intelligence/LightRAG-MT
that referenced
this pull request
Jan 16, 2026
Cherry-picked from HKUDS/LightRAG PR HKUDS#2585. - Fix GlobalArgsProxy to support vars() for binding options extraction - Refactor argument parsing for binding options to reduce duplication Before: OPENAI_LLM_TEMPERATURE and similar env vars were silently ignored After: Environment variables properly loaded and applied to LLM calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix binding options not loaded from environment variables
Summary
This PR fixes a critical bug where environment variables for LLM binding options (e.g.,
OPENAI_LLM_TEMPERATURE,OPENAI_LLM_MAX_TOKENS) were silently ignored, causing the server to always use default values.Problem
When using OpenAI/Azure OpenAI bindings, environment variables like
OPENAI_LLM_TEMPERATURE=0.7had no effect. The startup log would show:Root Cause: The
_GlobalArgsProxyclass did not properly supportvars()calls. Whenbinding_options.pycalledvars(args)on the proxy object, it returned an empty dict instead of the underlyingargparse.Namespaceattributes, causing all binding-specific options to be lost.Changes
Commit 1: Refactor argument parsing (
a000bdf0)--llm-bindingand--embedding-bindingparsing logic-Commit 2: Fix GlobalArgsProxy (
a555690c)__getattr__with__getattribute__to intercept__dict__accessvars(proxy)is called, it now correctly returns the underlying namespace's__dict__binding_options.options_dict()to properly extract provider-specific optionsTesting
Verified with:
Impact