diff --git a/CHANGELOG.md b/CHANGELOG.md index 08308763b..94455d060 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,16 @@ ensuring data is stored with proper parameter values. LLM streaming output. A `SessionActorNotConfiguredError` is raised with exit code 1 when no actor is configured. +### Added + +- **Automated CLI Docstring Example Validation** (#9106): Added `DocstringExampleValidator` + in `src/cleveragents/cli/docstring_validator.py` that introspects Typer command signatures + and validates `Examples:` sections to ensure positional arguments appear before option + flags. Validation runs automatically via `nox -s unit_tests` through the new Behave feature + `features/cli_docstring_example_validation.feature`. Fixed `rollback_plan` docstring in + `src/cleveragents/cli/commands/plan.py` to show correct positional argument order. + CONTRIBUTING.md updated with the required CLI docstring example style guide. + ### Fixed - **fileConfig error handling in alembic env.py** (#7874): Wrapped the `fileConfig()` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f25cb7c39..b0ec90fb3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1228,6 +1228,45 @@ These rules are enforced by the test environment hooks and CI quality gates: - A bug fix PR that closes issue `#N` where no `@tdd_issue_N` test exists in the codebase is blocked by the CI quality gate — the TDD step was skipped. +### CLI Docstring Example Style + +CLI command docstrings may include an `Examples:` section that shows users how to invoke the +command. These examples are rendered in the published documentation via MkDocs + mkdocstrings +and must accurately reflect the actual command signature. + +#### Required Style + +All `Examples:` lines must supply positional arguments **before** any option flags. This +mirrors the canonical Typer command signature where positional `Argument` parameters always +precede `Option` parameters. + +**Correct:** +``` +Examples: + agents plan rollback 01ARZ3NDEK7Z7BRRMJ98EGQGW 01BRZ4NFEK8A9CSSNJ09FHQHX + agents plan rollback 01ARZ3NDEK7Z7BRRMJ98EGQGW 01BRZ4NFEK8A9CSSNJ09FHQHX --yes +``` + +**Incorrect (options before positional args):** +``` +Examples: + agents plan rollback --yes 01ARZ3NDEK7Z7BRRMJ98EGQGW 01BRZ4NFEK8A9CSSNJ09FHQHX +``` + +#### Placeholder Usage + +Use realistic placeholder values (e.g., ULIDs for plan IDs, descriptive names for string +arguments) so that examples are immediately useful to readers. Avoid generic placeholders +like `` — use a realistic-looking value instead. + +#### Automated Validation + +The `DocstringExampleValidator` in `src/cleveragents/cli/docstring_validator.py` enforces +this style automatically. It is exercised by the Behave feature +`features/cli_docstring_example_validation.feature` which runs as part of `nox -s unit_tests`. +Any violation causes a test failure that names the offending command and shows the exact +example line. + ### Testing Tools Run tests using `nox`. Do not invoke `behave`, `robot`, or similar runners directly. If a `nox` diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index ea5b2a3b1..0840a0d10 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -28,6 +28,7 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed the plan tree JSON/YAML command envelope fix (#9163): wrapped `agents plan tree --format json/yaml` output in the spec-required command envelope structure, added summary statistics, decision_ids mapping, child_plans list, and accurate timing measurement. * HAL 9000 contributed Structural Component Output Validation (PR #11161 / issue #8164): implemented `validate_plan_tree`, `validate_decision_dict`, `validate_structured_output`, and `validate_structured_component_output` validators that replace exact-character matching with structural schema checking for plan tree nodes, decision CLI dictionaries, and structured session output envelopes. * HAMZA KHYARI has contributed the ACMS execute-phase context assembler project-level hot_max_tokens fix (PR #11036 / issue #11035): added `_resolve_effective_budget()` method that reads each linked project's `settings.hot_max_tokens` and uses the maximum override value as the pipeline budget instead of the hardcoded global 16K default. +* HAL 9000 has contributed the automated CLI docstring example validation (#9106): added `DocstringExampleValidator` to enforce positional-before-option ordering in CLI `Examples:` sections, with Behave test coverage and CONTRIBUTING.md documentation. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. * HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system. * HAL 9000 has contributed the pr-review-pool-supervisor tracking prefix documentation fix (#7891): aligned all documentation references from the outdated `AUTO-REV-POOL` prefix to the correct `AUTO-REV-SUP` prefix used in production. diff --git a/features/cli_docstring_example_validation.feature b/features/cli_docstring_example_validation.feature new file mode 100644 index 000000000..77716d147 --- /dev/null +++ b/features/cli_docstring_example_validation.feature @@ -0,0 +1,41 @@ +@cli +Feature: CLI docstring example validation + As a documentation maintainer + I want to ensure CLI docstring examples respect positional argument order + So that published documentation accurately reflects the command signature + + Scenario: Validator detects positional arguments before options + Given I have a CLI command with positional arguments and options + When I validate the command's docstring examples + Then the validation should pass for examples with positional args before options + + Scenario: Validator rejects options before positional arguments + Given I have a CLI command with options placed before positional arguments + When I validate the bad command's docstring examples + Then the validation should fail for examples with options before positional args + + Scenario: Validator handles commands without examples + Given I have a CLI command without docstring examples + When I validate the command's docstring examples + Then the validation should pass for commands without examples + + Scenario: Validator parses example lines with shlex + Given I have a CLI command with quoted arguments in examples + When I validate the command's docstring examples + Then the validation should correctly parse quoted arguments + + Scenario: Validator reports clear error messages + Given I have a CLI command with invalid docstring examples + When I validate the invalid command's docstring examples + Then the error message should identify the command and example line + And the error message should explain the positional argument order requirement + + Scenario: Validator handles multi-word command names correctly + Given I have a multi-word CLI command with positional arguments and options + When I validate the multi-word command's docstring examples + Then the validation should pass for multi-word commands with correct argument order + + Scenario: Validator scans all commands in a directory + Given I have a directory with CLI command modules + When I validate all commands in the directory + Then the validate all commands result should be True diff --git a/features/steps/cli_docstring_example_validation_steps.py b/features/steps/cli_docstring_example_validation_steps.py new file mode 100644 index 000000000..c0e0ee21b --- /dev/null +++ b/features/steps/cli_docstring_example_validation_steps.py @@ -0,0 +1,291 @@ +"""Steps for CLI docstring example validation feature.""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from typing import Annotated, Any + +import typer +from behave import given, then, when + +from cleveragents.cli.docstring_validator import DocstringExampleValidator + + +@given("I have a CLI command with positional arguments and options") +def step_have_cli_command_with_args_and_options(context: Any) -> None: + """Create a test CLI command with positional args and options.""" + + def test_command( + plan_id: Annotated[str, typer.Argument(help="Plan ID")], + checkpoint_id: Annotated[ + str | None, typer.Argument(help="Checkpoint ID") + ] = None, + yes: Annotated[bool, typer.Option("--yes", help="Skip confirmation")] = False, + ) -> None: + """Test command with positional and optional arguments. + + Examples: + agents test plan_id checkpoint_id + agents test plan_id checkpoint_id --yes + """ + pass + + context.test_command = test_command + context.validator = DocstringExampleValidator() + + +@when("I validate the command's docstring examples") +def step_validate_docstring_examples(context: Any) -> None: + """Validate the test command's docstring examples.""" + result = context.validator.validate_command( + context.test_command, + "agents test", + ) + context.validation_result = result + context.validation_errors = context.validator.get_errors() + + +@then("the validation should pass for examples with positional args before options") +def step_validation_passes_for_correct_order(context: Any) -> None: + """Assert that validation passes for correct positional argument order.""" + assert context.validation_result is True, ( + f"Validation should pass but got errors: {context.validation_errors}" + ) + + +@given("I have a CLI command with options placed before positional arguments") +def step_have_cli_command_with_bad_order(context: Any) -> None: + """Create a test CLI command with options before positional args in examples.""" + + def bad_command( + plan_id: Annotated[str, typer.Argument(help="Plan ID")], + checkpoint_id: Annotated[ + str | None, typer.Argument(help="Checkpoint ID") + ] = None, + yes: Annotated[bool, typer.Option("--yes", help="Skip confirmation")] = False, + ) -> None: + """Test command with bad example order. + + Examples: + agents test --yes plan_id checkpoint_id + """ + pass + + context.bad_command = bad_command + context.bad_validator = DocstringExampleValidator() + + +@when("I validate the bad command's docstring examples") +def step_validate_bad_command_examples(context: Any) -> None: + """Validate the bad command's docstring examples.""" + result = context.bad_validator.validate_command( + context.bad_command, + "agents test", + ) + context.bad_validation_result = result + context.bad_validation_errors = context.bad_validator.get_errors() + + +@then("the validation should fail for examples with options before positional args") +def step_validation_fails_for_incorrect_order(context: Any) -> None: + """Assert that validation fails for incorrect positional argument order.""" + assert context.bad_validation_result is False, ( + "Validation should fail for options before positional args" + ) + assert len(context.bad_validation_errors) > 0, "Should have validation errors" + + +@given("I have a CLI command without docstring examples") +def step_have_cli_command_without_examples(context: Any) -> None: + """Create a test CLI command without examples.""" + + def test_command_no_examples( + plan_id: Annotated[str, typer.Argument(help="Plan ID")], + ) -> None: + """Test command without examples section.""" + pass + + context.test_command = test_command_no_examples + context.validator = DocstringExampleValidator() + + +@then("the validation should pass for commands without examples") +def step_validation_passes_for_no_examples(context: Any) -> None: + """Assert that validation passes for commands without examples.""" + result = context.validator.validate_command( + context.test_command, + "agents test", + ) + assert result is True, ( + f"Validation should pass but got errors: {context.validator.get_errors()}" + ) + + +@given("I have a CLI command with quoted arguments in examples") +def step_have_cli_command_with_quoted_args(context: Any) -> None: + """Create a test CLI command with quoted arguments.""" + + def test_command_quoted( + name: Annotated[str, typer.Argument(help="Plan name")], + description: Annotated[str | None, typer.Argument(help="Description")] = None, + ) -> None: + """Test command with quoted arguments. + + Examples: + agents test "My Plan" "A description" + agents test "Plan Name" --format json + """ + pass + + context.test_command = test_command_quoted + context.validator = DocstringExampleValidator() + + +@then("the validation should correctly parse quoted arguments") +def step_validation_parses_quoted_args(context: Any) -> None: + """Assert that validation correctly parses quoted arguments.""" + result = context.validator.validate_command( + context.test_command, + "agents test", + ) + assert result is True, ( + f"Validation should pass for quoted arguments but got errors: " + f"{context.validator.get_errors()}" + ) + + +@given("I have a CLI command with invalid docstring examples") +def step_have_cli_command_with_invalid_examples(context: Any) -> None: + """Create a test CLI command with invalid examples.""" + + def test_command_invalid( + plan_id: Annotated[str, typer.Argument(help="Plan ID")], + checkpoint_id: Annotated[ + str | None, typer.Argument(help="Checkpoint ID") + ] = None, + ) -> None: + """Test command with invalid example order. + + Examples: + agents test --format json plan_id checkpoint_id + """ + pass + + context.invalid_command = test_command_invalid + context.invalid_validator = DocstringExampleValidator() + + +@when("I validate the invalid command's docstring examples") +def step_validate_invalid_command_examples(context: Any) -> None: + """Validate the invalid command's docstring examples.""" + result = context.invalid_validator.validate_command( + context.invalid_command, + "agents test", + ) + context.invalid_validation_result = result + context.invalid_validation_errors = context.invalid_validator.get_errors() + + +@then("the error message should identify the command and example line") +def step_error_identifies_command_and_line(context: Any) -> None: + """Assert that error message identifies the command and example line.""" + assert context.invalid_validation_result is False, "Validation should fail" + errors = context.invalid_validation_errors + assert len(errors) > 0, "Should have validation errors" + error_text = errors[0] + assert "agents test" in error_text, "Error should identify the command" + assert "agents test --format json" in error_text, ( + "Error should show the problematic example line" + ) + + +@then("the error message should explain the positional argument order requirement") +def step_error_explains_requirement(context: Any) -> None: + """Assert that error message explains the positional argument order requirement.""" + errors = context.invalid_validation_errors + assert len(errors) > 0, "Should have validation errors" + error_text = errors[0] + assert "positional" in error_text.lower(), ( + "Error should mention positional arguments" + ) + assert "before" in error_text.lower() or "order" in error_text.lower(), ( + "Error should explain the ordering requirement" + ) + + +@given("I have a multi-word CLI command with positional arguments and options") +def step_have_multi_word_cli_command(context: Any) -> None: + """Create a multi-word CLI command (e.g., 'agents plan rollback').""" + + def rollback_plan( + plan_id: Annotated[str, typer.Argument(help="Plan ID")], + checkpoint_id: Annotated[str, typer.Argument(help="Checkpoint ID")], + yes: Annotated[bool, typer.Option("--yes", help="Skip confirmation")] = False, + ) -> None: + """Rollback a plan to a checkpoint. + + Examples: + agents plan rollback 01ARZ3NDEK7Z7BRRMJ98EGQGW 01BRZ4NFEK8A9CSSNJ09FHQHX + agents plan rollback 01ARZ3NDEK7Z7BRRMJ98EGQGW 01BRZ4NFEK8A9CSSNJ09FHQHX --yes + """ + pass + + context.multi_word_command = rollback_plan + context.multi_word_validator = DocstringExampleValidator() + + +@when("I validate the multi-word command's docstring examples") +def step_validate_multi_word_command(context: Any) -> None: + """Validate the multi-word command's docstring examples.""" + result = context.multi_word_validator.validate_command( + context.multi_word_command, + "agents plan rollback", + ) + context.multi_word_result = result + context.multi_word_errors = context.multi_word_validator.get_errors() + + +@then("the validation should pass for multi-word commands with correct argument order") +def step_validation_passes_for_multi_word_command(context: Any) -> None: + """Assert that validation passes for multi-word commands with correct order.""" + assert context.multi_word_result is True, ( + f"Validation should pass for multi-word command but got errors: " + f"{context.multi_word_errors}" + ) + + +@given("I have a directory with CLI command modules") +def step_have_directory_with_cli_modules(context: Any) -> None: + """Create a temporary directory with a CLI command module for testing.""" + context.tmp_dir = tempfile.mkdtemp() + module_path = Path(context.tmp_dir) / "sample_cmd.py" + module_path.write_text( + '"""Sample CLI command module."""\n' + "from __future__ import annotations\n\n" + "def list_items(project_id: str) -> None:\n" + ' """List items in a project.\n\n' + " Examples:\n" + " agents sample_cmd list_items my-project\n" + ' """\n' + " pass\n" + ) + context.commands_dir = Path(context.tmp_dir) + context.all_validator = DocstringExampleValidator() + + +@when("I validate all commands in the directory") +def step_validate_all_commands(context: Any) -> None: + """Validate all commands in the temporary directory.""" + context.all_result = context.all_validator.validate_all_commands( + context.commands_dir + ) + context.all_errors = context.all_validator.get_errors() + + +@then("the validate all commands result should be True") +def step_validate_all_commands_passes(context: Any) -> None: + """Assert that validate_all_commands returns True for valid modules.""" + assert context.all_result is True, ( + f"validate_all_commands should return True but got errors: {context.all_errors}" + ) diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index 0563e4981..671b0d803 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -3772,8 +3772,8 @@ def rollback_plan( via the ``--to-checkpoint`` named option. Examples: - agents plan rollback --yes 01ARZ3NDEK... 01BRZ4NFEK... - agents plan rollback --to-checkpoint 01BRZ4NFEK... 01ARZ3NDEK... + agents plan rollback 01ARZ3NDEK... 01BRZ4NFEK... --yes + agents plan rollback 01ARZ3NDEK... --to-checkpoint 01BRZ4NFEK... """ from cleveragents.application.container import get_container from cleveragents.core.exceptions import ( diff --git a/src/cleveragents/cli/docstring_validator.py b/src/cleveragents/cli/docstring_validator.py new file mode 100644 index 000000000..e04e5b305 --- /dev/null +++ b/src/cleveragents/cli/docstring_validator.py @@ -0,0 +1,290 @@ +"""Validator for CLI command docstring examples. + +This module provides automated validation to ensure that CLI docstring examples +respect the positional argument order defined by the Typer command signature. +When a docstring example drifts from the actual command signature, the published +documentation becomes misleading. This validator catches such drift early. +""" + +from __future__ import annotations + +import importlib.util +import inspect +import logging +import re +import shlex +from collections.abc import Callable +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + + +class DocstringExampleValidator: + """Validates CLI command docstring examples against Typer signatures.""" + + def __init__(self) -> None: + """Initialize the validator.""" + self.errors: list[str] = [] + + def validate_command( + self, + func: Callable[..., Any], + command_name: str, + ) -> bool: + """Validate a single CLI command's docstring examples. + + Args: + func: The CLI command function to validate. + command_name: The full command name (e.g., "agents plan rollback"). + + Returns: + True if validation passes, False if errors were found. + """ + docstring = inspect.getdoc(func) + if not docstring: + return True + + # Extract Examples section + examples_match = re.search( + r"Examples:\s*\n((?:(?:\n|.)*?)(?=\n\n|\Z))", + docstring, + re.MULTILINE, + ) + if not examples_match: + return True + + examples_text = examples_match.group(1).strip() + if not examples_text: + return True + + # Parse Typer parameters to get positional args and options + positional_args = self._extract_positional_args(func) + option_flags = self._extract_option_flags(func) + + # Validate each example line + example_lines = [ + line.strip() + for line in examples_text.split("\n") + if line.strip() and not line.strip().startswith("#") + ] + + for example_line in example_lines: + if not self._validate_example_line( + example_line, + command_name, + positional_args, + option_flags, + ): + return False + + return True + + def _extract_positional_args(self, func: Callable[..., Any]) -> list[str]: + """Extract required positional argument names from function signature. + + Only arguments without a default value (or with a default of + ``inspect.Parameter.empty``) are considered required. Optional + positional arguments (those with a default) are excluded so that + examples that omit them are not incorrectly rejected. + + Args: + func: The CLI command function. + + Returns: + List of required positional argument names in order. + """ + positional_args = [] + sig = inspect.signature(func) + + for param_name, param in sig.parameters.items(): + if param_name in ("self", "cls"): + continue + + # Check if this is a Typer Argument (positional) + if param.annotation != inspect.Parameter.empty: + annotation_str = str(param.annotation) + if ( + "Argument" in annotation_str + and param.default is inspect.Parameter.empty + ): + positional_args.append(param_name) + + return positional_args + + def _extract_option_flags(self, func: Callable[..., Any]) -> set[str]: + """Extract option flag names from function signature. + + Args: + func: The CLI command function. + + Returns: + Set of option flag names (e.g., "--yes", "--format"). + """ + option_flags = set() + sig = inspect.signature(func) + + for param_name, param in sig.parameters.items(): + if param_name in ("self", "cls"): + continue + + # Check if this is a Typer Option + if param.annotation != inspect.Parameter.empty: + annotation_str = str(param.annotation) + if "Option" in annotation_str: + # Try to extract flag names from the annotation + # Common patterns: --flag, -f, --flag-name + flag_match = re.search( + r'["\'](-{1,2}[a-z0-9-]+)["\']', + annotation_str, + ) + if flag_match: + option_flags.add(flag_match.group(1)) + # Also add common flag patterns + option_flags.add(f"--{param_name.replace('_', '-')}") + + return option_flags + + def _validate_example_line( + self, + example_line: str, + command_name: str, + positional_args: list[str], + option_flags: set[str], + ) -> bool: + """Validate a single example line. + + Args: + example_line: The example command line to validate. + command_name: The full command name. + positional_args: List of positional argument names. + option_flags: Set of option flag names. + + Returns: + True if the example is valid, False otherwise. + """ + try: + # Parse the example line with shlex + tokens = shlex.split(example_line) + except ValueError as e: + self.errors.append( + f"[{command_name}] Failed to parse example: {example_line}\n" + f" Error: {e}" + ) + return False + + # Skip if it doesn't start with the command + if not tokens or not tokens[0].startswith("agents"): + return True + + # Determine how many tokens the command name occupies so we can + # correctly skip the command prefix when extracting positional values. + # For example, "agents plan rollback" occupies 3 tokens. + command_token_count = len(shlex.split(command_name)) + + # Find where positional arguments end and options begin. + # Scan from after the command prefix; the first token that starts + # with "-" marks the boundary. + positional_end_idx = command_token_count + suffix_tokens = tokens[command_token_count:] + for i, token in enumerate(suffix_tokens, start=command_token_count): + if token.startswith("-"): + break + positional_end_idx = i + 1 + + # Extract positional values from the example, skipping the command prefix + positional_values = tokens[command_token_count:positional_end_idx] + + # Check if positional arguments appear before any options. + # Validation applies only when the command has known required positional args. + if positional_args: + required_positional_count = len(positional_args) + + # Find the first option flag in the tokens (after the command prefix) + first_option_idx = None + for i, token in enumerate(suffix_tokens, start=command_token_count): + if token.startswith("-"): + first_option_idx = i + break + + # If there are options, verify that all required positional args + # appear before the first option flag. + if ( + first_option_idx is not None + and len(positional_values) < required_positional_count + ): + self.errors.append( + f"[{command_name}] Example violates positional argument" + f" order:\n" + f" {example_line}\n" + f" Expected positional arguments {positional_args} " + f"to appear before options.\n" + f" Found {len(positional_values)} positional values, " + f"expected at least {required_positional_count}." + ) + return False + + return True + + def validate_all_commands(self, commands_dir: Path) -> bool: + """Validate all CLI commands in a directory. + + Args: + commands_dir: Path to the CLI commands directory. + + Returns: + True if all validations pass, False if any errors were found. + """ + all_valid = True + + # Import all command modules + for command_file in sorted(commands_dir.glob("*.py")): + if command_file.name.startswith("_"): + continue + + module_name = command_file.stem + try: + spec = importlib.util.spec_from_file_location( + f"cleveragents.cli.commands.{module_name}", + command_file, + ) + if spec and spec.loader: + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + # Find all functions that look like CLI commands + for name, obj in inspect.getmembers(module): + if ( + inspect.isfunction(obj) + and not name.startswith("_") + and hasattr(obj, "__doc__") + ): + # Try to validate the command + cmd_suffix = name.replace("_", " ") + command_name = f"agents {module_name} {cmd_suffix}" + if not self.validate_command(obj, command_name): + all_valid = False + + except Exception as e: + # Log at DEBUG level so import failures are observable without + # blocking validation of other modules. + logger.debug("Skipping module %s: %s", module_name, e) + + return all_valid + + def get_errors(self) -> list[str]: + """Get all validation errors found. + + Returns: + List of error messages. + """ + return self.errors + + def print_errors(self) -> None: + """Print all validation errors to stdout.""" + if self.errors: + print("CLI Docstring Example Validation Errors:") + print("=" * 70) + for error in self.errors: + print(error) + print()