Add a script to validate that all plugins are included to core's extras #266
Add a script to validate that all plugins are included to core's extras #266
Conversation
WalkthroughThis pull request introduces plugin dependency validation infrastructure. A new CLI command validates that all discovered plugins are properly registered in agents-core/pyproject.toml's optional dependencies. Multiple new plugin dependencies are added, the validation is integrated into the CI workflow and existing check command, and supporting dependencies are declared. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
pyproject.toml (1)
100-100: Considertomllibfor Python 3.11+.Since the project supports Python 3.10+, the
tomlpackage is required for 3.10 compatibility. For future consideration, Python 3.11+ includestomllibin the standard library. The current approach is acceptable.scripts/validate_core_extras.py (2)
15-20: Missing docstring format and return type.The docstring is present but the function lacks a return type annotation. Consider adding
-> Nonefor completeness.-def main(): +def main() -> None:
78-80: Missing return type annotation.The function returns a boolean but lacks a type hint.
-def _cwd_is_root(): +def _cwd_is_root() -> bool: cwd = Path.cwd() return (cwd / CORE_PACKAGE_NAME).exists() and (cwd / PLUGINS_DIR).exists()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/run_tests.yml(2 hunks)agents-core/pyproject.toml(2 hunks)pyproject.toml(1 hunks)scripts/validate_core_extras.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
scripts/validate_core_extras.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: unit / Ruff
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
- GitHub Check: unit / Ruff
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (5)
agents-core/pyproject.toml (2)
55-63: LGTM!The new optional dependencies are properly defined and all corresponding entries exist in the
all-pluginslist below.
65-91: LGTM!The expanded
all-pluginslist includes all individual plugin dependencies, which the new validation script will enforce going forward.scripts/validate_core_extras.py (3)
70-70: LGTM!Using
...(ellipsis) for empty exception class bodies is idiomatic Python.
73-75: LGTM!Clean use of
NamedTuplefor structured return type.
89-104: LGTM!Good defensive handling for missing optional dependencies with clear error messaging.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dev.py (1)
215-215: Fix step numbering in output.The step numbering is incorrect. This should be "=== 5. Unit Tests ===" since it follows four previous steps.
Apply this diff:
- click.echo("\n=== 4. Unit Tests ===") + click.echo("\n=== 5. Unit Tests ===")
🧹 Nitpick comments (3)
dev.py (3)
117-120: Add error handling for file operations and TOML parsing.The function could fail with unclear errors if the pyproject.toml file is missing, malformed, or lacks the expected keys. Consider wrapping in try-except blocks to provide better error messages.
def _get_plugin_package_name(plugin: str) -> str: - with open(Path(PLUGINS_DIR) / Path(plugin) / "pyproject.toml", "r") as f: - pyproject = toml.load(f) - return pyproject["project"]["name"] + plugin_path = Path(PLUGINS_DIR) / Path(plugin) / "pyproject.toml" + try: + with open(plugin_path, "r") as f: + pyproject = toml.load(f) + return pyproject["project"]["name"] + except FileNotFoundError: + raise click.ClickException(f"Plugin {plugin} is missing pyproject.toml at {plugin_path}") + except (toml.TomlDecodeError, KeyError) as e: + raise click.ClickException(f"Invalid pyproject.toml for plugin {plugin}: {e}")
123-136: Consider adding error handling for robustness.While the function uses
.get()with defaults, it could still fail if the core pyproject.toml file is missing or malformed. Adding error handling would provide clearer diagnostics.def _get_core_optional_dependencies() -> CoreDependencies: - with open(Path(CORE_PACKAGE_NAME) / "pyproject.toml", "r") as f: - pyproject = toml.load(f) + core_pyproject_path = Path(CORE_PACKAGE_NAME) / "pyproject.toml" + try: + with open(core_pyproject_path, "r") as f: + pyproject = toml.load(f) + except FileNotFoundError: + raise click.ClickException(f"Core pyproject.toml not found at {core_pyproject_path}") + except toml.TomlDecodeError as e: + raise click.ClickException(f"Invalid core pyproject.toml: {e}") optionals: dict[str, list[str]] = pyproject.get("project", {}).get( "optional-dependencies", {} )
11-16: Replacetomlwithtomllibfor Python 3.12 compatibility.The project targets Python 3.12, and the
tomlimport is used only for read-only operations (toml.load()in two functions). Sincetomllibis available in the standard library for Python 3.11+, switch to it to eliminate the external dependency. Replaceimport tomlwithimport tomlliband update the twotoml.load(f)calls to usetomllib.loads(f.read()).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/run_tests.yml(2 hunks)dev.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
dev.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (6)
dev.py (5)
18-21: LGTM!The constants are well-defined and improve code maintainability.
107-109: LGTM!The NamedTuple provides clear structure for managing core dependencies with appropriate type hints.
112-114: LGTM!Root directory validation is straightforward and effective.
200-202: LGTM!The integration of validation into the check workflow is well-structured. Calling
validate_extra_dependencies.callback()directly is the correct approach for invoking Click commands programmatically.
150-152: Setuptools namespace package discovery is correct.The use of
setuptools.find_namespace_packages()is appropriate for discovering PEP 420 namespace packages in the plugins directory. The logic to extract root package names viasplit(".")[0]correctly isolates individual plugins from the nested namespace structure..github/workflows/run_tests.yml (1)
32-41: LGTM!The validation job is well-structured and follows the existing patterns in the workflow. The command
uv run dev.py validate-extrascorrectly invokes the new CLI command defined in dev.py.Note: The past review comment mentioning
scripts/validate_core_extras.pyappears to be from a different approach and is not applicable to this implementation.
2 participants
scripts/validate_core_extras.pyto validate that:agents-code/pyproject.tomlall-pluginsoptionals section inagents-code/pyproject.tomlSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.