feat: add environment variable fallback for Langfuse configuration

[jgwill]

Add environment variable fallback support for coaia.json configuration.

Changes

• Add simple .env file parser (Python 3.6 compatible, no dependencies)
• Support LANGFUSE_SECRET_KEY, LANGFUSE_PUBLIC_KEY, LANGFUSE_HOST, LANGFUSE_AUTH3
• Priority order: .env file → system environment → coaia.json → defaults
• Maintains backward compatibility with existing environment variables

Resolves

• Closes If no coaia.json found, use environment variable #26

Generated with Claude Code

[claude]

Claude finished @jgwill's task —— View job

---

Code Review Complete ✅

Reviewing PR: feat: add environment variable fallback for Langfuse configuration

Todo List:

• Read and analyze the modified code in coaiapy/coaiamodule.py
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage needs
• Verify Python 3.6 compatibility
• Provide comprehensive feedback

---

✅ Positive Aspects

Excellent Python 3.6 Compatibility

• No external dependencies added - maintains the critical Python 3.6 compatibility requirement
• Uses standard library functions that are available in Python 3.6
• Proper file handling with context managers

Clean Implementation

• Simple, focused .env parser that handles the basic use cases well
• Maintains backward compatibility with existing environment variables
• Good separation of concerns with dedicated functions

Proper Priority Order

The implementation correctly follows the documented priority: .env file → system environment → coaia.json → defaults

---

🐛 Issues Found

Critical Bug in Logic (coaiamodule.py:148, 154)

# Lines 148 and 154 - Logic error
if config["jtaleconf"]["host"] == config["jtaleconf"]["host"] or (not env_vars.get("REDIS_HOST") and not os.getenv("REDIS_HOST")):
if config["jtaleconf"]["password"] == config["jtaleconf"]["password"] or (not env_vars.get("REDIS_PASSWORD") and not os.getenv("REDIS_PASSWORD")):

Problem: config["jtaleconf"]["host"] == config["jtaleconf"]["host"] is always True, making the condition meaningless.

Fix: Should likely be:

if config["jtaleconf"]["host"] == "localhost" or (not env_vars.get("REDIS_HOST") and not os.getenv("REDIS_HOST")):
if config["jtaleconf"]["password"] == "" or (not env_vars.get("REDIS_PASSWORD") and not os.getenv("REDIS_PASSWORD")):

Quote Handling Edge Case (coaiamodule.py:67-69)

if (value.startswith('"') and value.endswith('"')) or \
   (value.startswith("'") and value.endswith("'")):
    value = value[1:-1]

Problem: This will incorrectly strip quotes from values like "don't" or 'say "hello"' (mixed quotes).

Suggested Fix:

if (value.startswith('"') and value.endswith('"')) or \
   (value.startswith("'") and value.endswith("'")):
    value = value[1:-1]

Error Handling Could Be More Specific (coaiamodule.py:71-72)

except Exception as e:
    print(f"Warning: Error loading .env file: {e}")

Issue: Catches all exceptions, making debugging difficult. Consider catching specific exceptions like IOError, PermissionError.

---

⚠️ Security Considerations

File Path Security

• ✅ Uses os.path.exists() checks before reading
• ✅ No user-controlled file paths in the .env loading
• ✅ Only reads from fixed .env location

Environment Variable Exposure

• ✅ Doesn't log sensitive values
• ✅ Proper handling of credentials in memory

---

🚀 Performance Considerations

Efficient Implementation

• ✅ Only loads .env file once during config initialization
• ✅ Uses simple string operations, minimal overhead
• ✅ Early exits and existence checks prevent unnecessary work

Minor Optimization Opportunity

The .env parsing could use str.partition('=') instead of str.split('=', 1) for slightly better performance, but this is negligible.

---

🧪 Test Coverage Assessment

Missing Test Coverage

The implementation lacks tests for:

• .env file parsing with various formats (quoted values, comments, empty lines)
• Error handling scenarios (malformed .env, permission errors)
• Priority order verification (env vars override .env, .env overrides config)
• Edge cases in quote handling

Recommended Test Cases

# Test .env parsing
def test_env_parsing():
    # Basic key=value
    # Quoted values: KEY="value with spaces"
    # Mixed quotes edge case
    # Comments and empty lines
    # Malformed lines (no =, multiple =)

def test_priority_order():
    # Verify .env overrides config defaults
    # Verify system env overrides .env
    # Verify config file overrides defaults

---

📋 Recommendations

High Priority

1. Fix the logic bug in lines 148 and 154 - this is critical
2. Add basic tests for the new functionality
3. Improve quote handling to handle mixed quote scenarios

Medium Priority

1. More specific exception handling in .env loading
2. Add docstring to load_env_file() function for consistency
3. Consider validation of Langfuse configuration values

Low Priority

1. Minor performance optimization with str.partition()
2. Add debug logging for configuration loading (when debugging is enabled)

---

✅ Overall Assessment

This is a solid implementation that successfully adds the requested functionality while maintaining Python 3.6 compatibility and existing behavior. The code is clean, follows existing patterns, and adds valuable functionality.

Primary concern: The logic bug on lines 148 and 154 needs to be fixed before merging.

Secondary concern: Adding some basic tests would greatly improve confidence in the implementation.

The implementation demonstrates good understanding of the codebase and requirements. With the critical bug fix, this would be ready for production use.

---