Update from task d5292937-253d-4a01-87ad-27b90ca35e2b #1
Conversation
- Added `SIMPLIFIED_CONFIG_GUIDE.md` with comprehensive documentation for the new configuration system - Created `core/simple_config.py` implementing a unified, environment-variable-supported configuration class with validation - Introduced `migrate_config.py` script to automate migration from old config system - Generated `simple_config.json` as the new single configuration file with logical grouping - Updated `.gitignore` to remove RAVANA AGI-specific exclusions and standardize ignore patterns The changes streamline configuration management with a single, maintainable config file while ensuring backward compatibility through environment variables. The migration script enables smooth transition from legacy settings.
Summary of ChangesHello @Likhithsai2580, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request overhauls the configuration management for the RAVANA AGI project, moving from a complex, multi-file setup to a streamlined, single-file system. The primary goal is to enhance maintainability and readability by centralizing settings, simplifying model configurations, and providing clear documentation and migration tools. This change aims to make the system easier to configure and manage for developers. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a simplified configuration system, which is a positive step towards improving maintainability. However, there are several critical issues that need to be addressed before this can be merged. The most severe issues are multiple instances of hardcoded API keys, which pose a significant security risk. Additionally, the new .gitignore file has invalid syntax, and the core logic for loading configurations from a file is broken. I've also identified a thread-safety issue in the configuration singleton implementation and some inconsistencies in validation logic and documentation. Please review the detailed comments for suggestions on how to resolve these problems.
| ``` | ||
| # Compiled and build artifacts | ||
| *.pyc | ||
| __pycache__/ | ||
| **/__pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| # Virtual environments | ||
| *.o | ||
| *.obj | ||
| dist/ | ||
| build/ | ||
| *.so | ||
| *.dll | ||
| *.exe | ||
| *.class | ||
|
|
||
| # Dependencies | ||
| .venv/ | ||
| **/.venv/ | ||
| env/ | ||
| **/env/ | ||
| venv/ | ||
| **/venv/ | ||
| # Logs and databases | ||
| node_modules/ | ||
| .mypy_cache/ | ||
| .pytest_cache/ | ||
| target/ | ||
| .gradle/ | ||
|
|
||
| # Logs and temp files | ||
| *.log | ||
| *.db | ||
| *.sqlite3 | ||
| # Environment and lock files | ||
| .env | ||
| .env.* | ||
| .venv | ||
| Pipfile.lock | ||
| poetry.lock | ||
| uv.lock | ||
| # Ignore temporary editor files | ||
| *.tmp | ||
| *.swp | ||
| *.swo | ||
| *~ | ||
|
|
||
| # Environment | ||
| .env | ||
| .env.local | ||
| *.env.* | ||
|
|
||
| # Editors | ||
| .vscode/ | ||
| .idea/ | ||
| *.sublime-project | ||
| *.sublime-workspace | ||
| # OS-specific files | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # System files | ||
| .DS_Store | ||
| Thumbs.db | ||
| chroma_db/ | ||
|
|
||
| # Custom | ||
| .cursorignore | ||
|
|
||
| # PyInstaller | ||
| # build/ # Already included in Packaging section | ||
| # dist/ # Already included in Packaging section | ||
| # *.spec # Already included in Packaging section | ||
|
|
||
| # Jupyter Notebook checkpoints | ||
| .ipynb_checkpoints/ | ||
|
|
||
| # Coverage and testing | ||
| # Coverage | ||
| coverage/ | ||
| htmlcov/ | ||
| .coverage | ||
| .cache/ | ||
| pytest_cache/ | ||
| .tox/ | ||
|
|
||
| # Mypy and Pyright | ||
| .mypy_cache/ | ||
| .pyright/ | ||
|
|
||
| # Packaging | ||
| *.egg | ||
| *.egg-info/ | ||
| dist/ | ||
| build/ | ||
| *.spec | ||
|
|
||
| modules/knowledge_compression/compressed_memory.json | ||
|
|
||
| # RAVANA AGI specific | ||
| .qoder/ | ||
|
|
||
| knowledge_id_map.pkl | ||
| knowledge_index.faiss | ||
|
|
||
| snake_agent_state.json | ||
| shared_memory/ | ||
|
|
||
| user_data/ | ||
|
|
||
| snake_logs/ | ||
| backups/ | ||
|
|
||
| snake_vltm_storage/ | ||
| vltm_storage/ | ||
| test_snake_logs/ | ||
|
|
||
| performance_data/ | ||
| .benchmarks/ | ||
| .qwen/ | ||
|
|
||
| profiles/ | ||
|
|
||
| SNAKE_AGENT_FINAL_SUMMARY.md | ||
| SNAKE_AGENT_SUCCESS_SUMMARY.md | ||
|
|
||
| snake_agent_optimized_config.bat | ||
| snake_agent_optimized_config.sh | ||
|
|
||
| enhanced_snake_state.json | ||
| final_demonstration.py | ||
| optimize_code_snake.py | ||
| self_goals.json | ||
| .snake_index.json | ||
| # Compressed files | ||
| *.zip | ||
| *.gz | ||
| *.tar | ||
| *.tgz | ||
| *.bz2 | ||
| *.xz | ||
| *.7z | ||
| *.rar | ||
| *.zst | ||
| *.lz4 | ||
| *.lzh | ||
| *.cab | ||
| *.arj | ||
| *.rpm | ||
| *.deb | ||
| *.Z | ||
| *.lz | ||
| *.lzo | ||
| *.tar.gz | ||
| *.tar.bz2 | ||
| *.tar.xz | ||
| *.tar.zst | ||
| ``` |
There was a problem hiding this comment.
The .gitignore file is wrapped in markdown-style code fences (```), which is invalid syntax for this file type. These fences will not be treated as comments and will likely cause the gitignore rules to not be parsed correctly.
# Compiled and build artifacts
*.pyc
__pycache__/
*.o
*.obj
dist/
build/
*.so
*.dll
*.exe
*.class
# Dependencies
.venv/
venv/
node_modules/
.mypy_cache/
.pytest_cache/
target/
.gradle/
# Logs and temp files
*.log
*.tmp
*.swp
*.swo
# Environment
.env
.env.local
*.env.*
# Editors
.vscode/
.idea/
*.swp
*.swo
# System files
.DS_Store
Thumbs.db
# Coverage
coverage/
htmlcov/
.coverage
# Compressed files
*.zip
*.gz
*.tar
*.tgz
*.bz2
*.xz
*.7z
*.rar
*.zst
*.lz4
*.lzh
*.cab
*.arj
*.rpm
*.deb
*.Z
*.lz
*.lzo
*.tar.gz
*.tar.bz2
*.tar.xz
*.tar.zst
| self.api_keys = { | ||
| 'electronhub': os.environ.get("ELECTRONHUB_API_KEY", "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc"), | ||
| 'zuki': os.environ.get("ZUKI_API_KEY", "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d"), | ||
| 'gemini': os.environ.get("GEMINI_API_KEY", "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ") | ||
| } |
There was a problem hiding this comment.
Hardcoding API keys, even as default fallbacks, is a major security risk. These keys appear to be active and could be exploited if committed. Default values for secrets should be None or an empty string to force explicit configuration by the user.
| self.api_keys = { | |
| 'electronhub': os.environ.get("ELECTRONHUB_API_KEY", "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc"), | |
| 'zuki': os.environ.get("ZUKI_API_KEY", "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d"), | |
| 'gemini': os.environ.get("GEMINI_API_KEY", "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ") | |
| } | |
| self.api_keys = { | |
| 'electronhub': os.environ.get("ELECTRONHUB_API_KEY"), | |
| 'zuki': os.environ.get("ZUKI_API_KEY"), | |
| 'gemini': os.environ.get("GEMINI_API_KEY") | |
| } |
| def load_from_file(self, filepath: str) -> None: | ||
| """Load configuration from a JSON file.""" | ||
| with open(filepath, 'r', encoding='utf-8') as f: | ||
| config_dict = json.load(f) | ||
|
|
||
| # Update instance attributes from the loaded config | ||
| for key, value in config_dict.items(): | ||
| if hasattr(self, key): | ||
| setattr(self, key, value) |
There was a problem hiding this comment.
The load_from_file method is implemented incorrectly. It iterates over the top-level keys of the loaded JSON and tries to set attributes on the SimpleConfig instance. However, the simple_config.json file is nested (e.g., {"system": {"name": "..."}}), while the class attributes are flat (e.g., self.system_name). The hasattr(self, key) check will fail for nested keys like "system", and the configuration will not be loaded from the file. The method needs to be rewritten to correctly parse the nested JSON and map it to the flat class attributes.
| "api_keys": { | ||
| "electronhub": "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc", | ||
| "zuki": "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d", | ||
| "gemini": "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ" | ||
| }, |
There was a problem hiding this comment.
The migration script generates a simple_config.json file with hardcoded API keys. This is a significant security risk. The script should generate placeholder values instead, prompting the user to fill them in.
| "api_keys": { | |
| "electronhub": "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc", | |
| "zuki": "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d", | |
| "gemini": "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ" | |
| }, | |
| "api_keys": { | |
| "electronhub": "YOUR_ELECTRONHUB_API_KEY_HERE", | |
| "zuki": "YOUR_ZUKI_API_KEY_HERE", | |
| "gemini": "YOUR_GEMINI_API_KEY_HERE" | |
| }, |
| "api_keys": { | ||
| "electronhub": "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc", | ||
| "zuki": "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d", | ||
| "gemini": "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ" | ||
| }, |
There was a problem hiding this comment.
The default simple_config.json file contains hardcoded API keys. This is a major security vulnerability. Default configuration files should use placeholder values for secrets to prevent accidental exposure.
| "api_keys": { | |
| "electronhub": "ek-sVvxMYfdFQ0Kl6Aj2tmV7b8n5v0Y0sDHVsOUZWyx2vbs0AbuAc", | |
| "zuki": "985160dfa1fd499fd12af708d16552e37a8c6f77cbfb50ae400e3ff33fbd791bc7b3b82625379a1f5ca7568f1ee04eb81a0f8c06f0ba6c276d3dddfe13e9c18d", | |
| "gemini": "AIzaSyBW-aVU-x7JCjBJVVKjPGUacups0-GBHvQ" | |
| }, | |
| "api_keys": { | |
| "electronhub": "YOUR_ELECTRONHUB_API_KEY_HERE", | |
| "zuki": "YOUR_ZUKI_API_KEY_HERE", | |
| "gemini": "YOUR_GEMINI_API_KEY_HERE" | |
| }, |
| # Simplified Configuration Guide for RAVANA AGI | ||
|
|
||
| ## Overview | ||
|
|
||
| This guide explains the simplified configuration system for the RAVANA AGI project. The goal is to reduce complexity while maintaining essential functionality. | ||
|
|
||
| ## Key Changes | ||
|
|
||
| ### 1. Consolidated Configuration | ||
| - **Before**: Multiple configuration files (`core/config.json`, `analysis/config.json`, `core/config.py`) with complex nested structures | ||
| - **After**: Single simplified configuration file (`simple_config.json`) with clear, logical groupings | ||
|
|
||
| ### 2. Reduced Complexity | ||
| - Simplified model configuration from separate coding/reasoning models to a single primary model | ||
| - Consolidated API keys to essential providers only | ||
| - Streamlined settings into logical categories | ||
|
|
||
| ### 3. Environment Variable Support | ||
| - All configuration values can be overridden via environment variables | ||
| - Maintains compatibility with existing deployment practices | ||
|
|
||
| ## Configuration Structure | ||
|
|
||
| The simplified configuration is organized into these main sections: | ||
|
|
||
| ### System | ||
| - `name`: Name of the system (default: "RAVANA") | ||
| - `database_url`: Database connection string (default: "sqlite:///ravana_agi.db") | ||
| - `log_level`: Logging level (default: "INFO") | ||
|
|
||
| ### Operational | ||
| - `curiosity_chance`: Probability of curiosity-driven behavior (0.0 to 1.0) | ||
| - `reflection_chance`: Probability of reflection (0.0 to 1.0) | ||
| - `loop_sleep_duration`: Sleep duration between loops in seconds | ||
| - `max_iterations`: Maximum iterations for processes | ||
|
|
||
| ### Models | ||
| - `primary`: Configuration for the main AI model | ||
| - `provider`: AI provider (e.g., "ollama", "openai") | ||
| - `model_name`: Name of the model | ||
| - `base_url`: API endpoint | ||
| - `temperature`: Creativity parameter | ||
| - `max_tokens`: Maximum tokens to generate | ||
| - `timeout`: Request timeout in seconds | ||
|
|
||
| ### API Keys | ||
| - `electronhub`: API key for ElectronHub provider | ||
| - `zuki`: API key for Zuki provider | ||
| - `gemini`: API key for Google Gemini provider | ||
|
|
||
| ### Memory | ||
| - `embedding_model`: Model used for embeddings | ||
| - `embedding_use_cuda`: Whether to use CUDA for embeddings | ||
|
|
||
| ### Agents | ||
| - `snake_agent`: Configuration for the Snake Agent | ||
| - `enabled`: Whether the agent is enabled | ||
| - `interval`: Interval between agent runs in seconds | ||
|
|
||
| ### Services | ||
| - `blog`: Blog publishing service | ||
| - `enabled`: Whether blog publishing is enabled | ||
| - `api_url`: Blog API endpoint | ||
| - `conversational_ai`: Conversational AI service | ||
| - `enabled`: Whether conversational AI is enabled | ||
|
|
||
| ### Scheduling | ||
| - `data_collection_interval`: How often to collect data (seconds) | ||
| - `event_detection_interval`: How often to detect events (seconds) | ||
| - `knowledge_compression_interval`: How often to compress knowledge (seconds) | ||
|
|
||
| ### Persona | ||
| - `name`: Name of the AI persona | ||
| - `creativity`: Creativity level (0.0 to 1.0) | ||
|
|
||
| ### Shutdown | ||
| - `timeout`: Shutdown timeout in seconds | ||
| - `graceful_shutdown_enabled`: Whether graceful shutdown is enabled | ||
|
|
||
| ## Usage | ||
|
|
||
| ### Loading Configuration | ||
|
|
||
| ```python | ||
| from core.simple_config import get_config | ||
|
|
||
| config = get_config() | ||
|
|
||
| # Access configuration values | ||
| print(config.system_name) | ||
| print(config.primary_model['model_name']) | ||
| ``` | ||
|
|
||
| ### Environment Variables | ||
|
|
||
| All configuration values can be overridden using environment variables: | ||
|
|
||
| ```bash | ||
| export SYSTEM_NAME="MyRAVANA" | ||
| export PRIMARY_MODEL_NAME="llama3.1:70b" | ||
| export CURIOSITY_CHANCE=0.5 | ||
| ``` | ||
|
|
||
| ### Configuration File | ||
|
|
||
| The configuration can also be loaded from a JSON file: | ||
|
|
||
| ```python | ||
| from core.simple_config import SimpleConfig | ||
|
|
||
| config = SimpleConfig() | ||
| config.load_from_file("simple_config.json") | ||
| ``` | ||
|
|
||
| ## Migration Guide | ||
|
|
||
| To migrate from the old configuration system: | ||
|
|
||
| 1. Update your imports to use `core.simple_config` instead of `core.config` | ||
| 2. Replace complex model configurations with the simplified primary model | ||
| 3. Consolidate API keys to the essential providers | ||
| 4. Use the new logical groupings for configuration values | ||
|
|
||
| ## Benefits | ||
|
|
||
| 1. **Easier Maintenance**: Single configuration file with clear structure | ||
| 2. **Better Readability**: Logical groupings make it easy to find settings | ||
| 3. **Simplified Code**: Reduced complexity in configuration handling | ||
| 4. **Environment Support**: Full compatibility with environment variables | ||
| 5. **Backward Compatibility**: Existing environment variables still work | ||
|
|
||
| ## Example Configuration | ||
|
|
||
| ```json | ||
| { | ||
| "system": { | ||
| "name": "RAVANA", | ||
| "database_url": "sqlite:///ravana_agi.db", | ||
| "log_level": "INFO" | ||
| }, | ||
| "operational": { | ||
| "curiosity_chance": 0.4, | ||
| "reflection_chance": 0.15, | ||
| "loop_sleep_duration": 7, | ||
| "max_iterations": 15 | ||
| } | ||
| // ... other sections | ||
| } | ||
| ``` | ||
|
|
||
| This simplified configuration system maintains all essential functionality while reducing complexity and improving maintainability. No newline at end of file |
There was a problem hiding this comment.
There's an inconsistency between the documented configuration structure and the example usage. The example JSON configuration is nested (e.g., system: { name: ... }), but the example Python code accesses values as if they are flat attributes on the config object (e.g., config.system_name). The current implementation of SimpleConfig.load_from_file does not correctly handle this nested structure, which will lead to confusion and bugs. The documentation should be aligned with a consistent (either flat or nested) configuration approach throughout the class implementation and the JSON files.
| required_model_keys = ['model_name', 'base_url', 'temperature'] | ||
| for key in required_model_keys: | ||
| if key not in self.primary_model: | ||
| errors.append(f"PRIMARY_MODEL missing required key: {key}") |
There was a problem hiding this comment.
The validate method for primary_model only checks for model_name, base_url, and temperature. However, the __init__ method and the documentation in SIMPLIFIED_CONFIG_GUIDE.md show that provider, max_tokens, and timeout are also part of the primary_model configuration. For completeness and to prevent runtime errors, these keys should also be validated.
| required_model_keys = ['model_name', 'base_url', 'temperature'] | |
| for key in required_model_keys: | |
| if key not in self.primary_model: | |
| errors.append(f"PRIMARY_MODEL missing required key: {key}") | |
| required_model_keys = ['provider', 'model_name', 'base_url', 'temperature', 'max_tokens', 'timeout'] | |
| for key in required_model_keys: | |
| if key not in self.primary_model: | |
| errors.append(f"PRIMARY_MODEL missing required key: {key}") |
This PR was created by qwen-chat coder for task d5292937-253d-4a01-87ad-27b90ca35e2b.