refactor(evolve-lite): replace EVOLVE_ENTITIES_DIR with EVOLVE_DIR#148
refactor(evolve-lite): replace EVOLVE_ENTITIES_DIR with EVOLVE_DIR#148visahak merged 6 commits intoAgentToolkit:mainfrom
Conversation
Introduce EVOLVE_DIR as the root for all evolve data (entities, trajectories, config, etc.) instead of pointing directly at the entities subdirectory. Adds get_evolve_dir() helper as the single resolution point for the env var vs cwd fallback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces per-entity env var with a single EVOLVE_DIR pointing at the .evolve root; adds get_evolve_dir(), simplifies entity discovery to {EVOLVE_DIR}/entities (or ./.evolve/entities by default), updates installer export, and adds pytest coverage for the new resolution behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform-integrations/claude/plugins/evolve-lite/lib/entity_io.py (1)
68-79: Docstring mentions "search order" but there's now only one candidate.The docstring describes a two-step "search order" (env var vs cwd), but the implementation delegates to
get_evolve_dir()which handles both cases. The numbered list implies two distinct checks when really there's just one path computed differently based on env var presence.📝 Suggested docstring simplification
def find_entities_dir(): """Locate the entities directory. - Search order: - 1. ``{EVOLVE_DIR}/entities/`` (from env var) - 2. ``.evolve/entities/`` (cwd) + Returns ``{get_evolve_dir()}/entities/`` if the directory exists. + See :func:`get_evolve_dir` for path resolution details. Returns: Path to the directory if it exists, else ``None``. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-integrations/claude/plugins/evolve-lite/lib/entity_io.py` around lines 68 - 79, The docstring for find_entities_dir is misleading: it lists a two-step "search order" but the function only computes a single path via get_evolve_dir() and returns it if it exists. Update the docstring of find_entities_dir to remove the numbered search list and instead clearly state that it uses get_evolve_dir() to determine the base directory and returns the "entities" subdirectory Path if it exists or None otherwise; reference get_evolve_dir in the text so readers know where the actual lookup logic lives.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platform-integrations/claude/plugins/evolve-lite/lib/entity_io.py`:
- Around line 68-79: The docstring for find_entities_dir is misleading: it lists
a two-step "search order" but the function only computes a single path via
get_evolve_dir() and returns it if it exists. Update the docstring of
find_entities_dir to remove the numbered search list and instead clearly state
that it uses get_evolve_dir() to determine the base directory and returns the
"entities" subdirectory Path if it exists or None otherwise; reference
get_evolve_dir in the text so readers know where the actual lookup logic lives.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58991294-b8de-4df0-8ba7-eb1a09fe3451
📒 Files selected for processing (3)
platform-integrations/claude/plugins/evolve-lite/README.mdplatform-integrations/claude/plugins/evolve-lite/lib/entity_io.pyplatform-integrations/install.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/platform_integrations/test_entity_io.py (1)
8-9: Consider a cleaner import approach.Manipulating
sys.pathat module level before importing can be fragile and affects other tests if run in the same process. Aconftest.pyfixture or usingimportlibwith explicit path would be more maintainable.♻️ Alternative using importlib
import importlib.util from pathlib import Path _lib_path = Path(__file__).parent.parent.parent / "platform-integrations/claude/plugins/evolve-lite/lib/entity_io.py" _spec = importlib.util.spec_from_file_location("entity_io", _lib_path) entity_io = importlib.util.module_from_spec(_spec) _spec.loader.exec_module(entity_io)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/platform_integrations/test_entity_io.py` around lines 8 - 9, The test currently mutates sys.path at module import time (sys.path.insert(...) then import entity_io) which is fragile; instead load entity_io without altering global import state by either providing a fixture in conftest.py that imports the module from its file location and yields it to tests, or use importlib.util.spec_from_file_location to load "entity_io" from the repo path inside the test setup/fixture; update tests in tests/platform_integrations/test_entity_io.py to remove the sys.path modification and reference the provided fixture or the imported module object (entity_io) so other tests aren’t affected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/platform_integrations/test_entity_io.py`:
- Around line 1-9: Add the pytest marker for platform integrations by defining a
module-level pytestmark variable in this test module: import pytest if not
already present and add pytestmark = pytest.mark.platform_integrations near the
top of tests/platform_integrations/test_entity_io.py so all tests in the file
are marked (alternatively decorate individual test functions with
`@pytest.mark.platform_integrations` if you prefer function-level marking); ensure
the symbol pytestmark is used and pytest is imported.
---
Nitpick comments:
In `@tests/platform_integrations/test_entity_io.py`:
- Around line 8-9: The test currently mutates sys.path at module import time
(sys.path.insert(...) then import entity_io) which is fragile; instead load
entity_io without altering global import state by either providing a fixture in
conftest.py that imports the module from its file location and yields it to
tests, or use importlib.util.spec_from_file_location to load "entity_io" from
the repo path inside the test setup/fixture; update tests in
tests/platform_integrations/test_entity_io.py to remove the sys.path
modification and reference the provided fixture or the imported module object
(entity_io) so other tests aren’t affected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0372d1fc-304d-4c4d-b9ae-09f3a3ad9eb6
📒 Files selected for processing (2)
platform-integrations/claude/plugins/evolve-lite/lib/entity_io.pytests/platform_integrations/test_entity_io.py
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-integrations/claude/plugins/evolve-lite/lib/entity_io.py
Addresses CodeRabbit review finding: Add @pytest.mark.platform_integrations marker to all test functions
Keep EVOLVE_DIR-based implementation from evolvedir branch, which supersedes the EVOLVE_ENTITIES_DIR approach removed in public/main (AgentToolkit#147).
Introduce EVOLVE_DIR as the root for all evolve data (entities, trajectories, config, etc.) instead of pointing directly at the entities subdirectory. Adds get_evolve_dir() helper as the single resolution point for the env var vs cwd fallback.
Summary by CodeRabbit
Documentation
Refactor
Installer
Tests