diff --git a/openagent_eval/cli/commands/audit.py b/openagent_eval/cli/commands/audit.py index 8f984cb..a7ce3d8 100644 --- a/openagent_eval/cli/commands/audit.py +++ b/openagent_eval/cli/commands/audit.py @@ -20,17 +20,77 @@ from openagent_eval import __version__ from openagent_eval.cli.context import get_context +from openagent_eval.cli.utils.helpers import load_config_from_path +from openagent_eval.config.models import CorpusConfig from openagent_eval.corpus.auditor import CorpusAuditor from openagent_eval.corpus.models import AuditReport, IssueSeverity +from openagent_eval.exceptions import ConfigurationError from openagent_eval.exceptions.corpus import CorpusAuditError, CorpusNotFoundError console = Console() +# Built-in defaults, used when neither a CLI flag nor a config value is given. +# Kept in sync with the CorpusAuditor / CorpusConfig defaults. +_DEFAULT_STALENESS_DAYS = 365 +_DEFAULT_SIMILARITY_THRESHOLD = 0.92 +_DEFAULT_MAX_DOCUMENTS = 1000 + + +def _resolve_audit_settings( + corpus_cfg: CorpusConfig | None, + *, + corpus_path: str | None, + checks: str | None, + staleness_days: int | None, + similarity_threshold: float | None, + max_documents: int | None, +) -> tuple[str | None, str | None, int, float, int]: + """Merge CLI flags with an optional ``corpus:`` config section. + + Precedence (highest first): an explicitly-provided CLI flag, then the + value from the config file's ``corpus:`` section, then the built-in + default. A CLI flag counts as "explicitly provided" when it is not + ``None`` (the sentinel default the command declares for each option), + mirroring how ``run`` treats its override flags. + + Returns: + ``(corpus_path, checks, staleness_days, similarity_threshold, + max_documents)`` with the tuning knobs resolved to concrete values. + """ + if corpus_cfg is not None: + if corpus_path is None: + corpus_path = corpus_cfg.path + if checks is None and corpus_cfg.checks: + checks = ",".join(check.value for check in corpus_cfg.checks) + if staleness_days is None: + staleness_days = corpus_cfg.staleness_days + if similarity_threshold is None: + similarity_threshold = corpus_cfg.similarity_threshold + if max_documents is None: + max_documents = corpus_cfg.max_documents + + if staleness_days is None: + staleness_days = _DEFAULT_STALENESS_DAYS + if similarity_threshold is None: + similarity_threshold = _DEFAULT_SIMILARITY_THRESHOLD + if max_documents is None: + max_documents = _DEFAULT_MAX_DOCUMENTS + + return corpus_path, checks, staleness_days, similarity_threshold, max_documents + def audit_command( - corpus_path: str = typer.Argument( - ..., - help="Path to the corpus directory or file to audit.", + corpus_path: str | None = typer.Argument( + None, + help="Path to the corpus directory or file to audit. " + "Falls back to the config file's 'corpus.path' when omitted.", + ), + config_path: str | None = typer.Option( + None, + "--config", + "-C", + help="Path to a config file whose 'corpus:' section provides audit " + "settings. Explicit CLI flags override the file's values.", ), checks: str | None = typer.Option( None, @@ -38,20 +98,21 @@ def audit_command( "-c", help="Comma-separated checks to perform (contradiction, staleness, duplicate, coverage).", ), - staleness_days: int = typer.Option( - 365, + staleness_days: int | None = typer.Option( + None, "--staleness-days", - help="Days threshold for staleness detection.", + help="Days threshold for staleness detection. [default: 365 or config value]", ), - similarity_threshold: float = typer.Option( - 0.92, + similarity_threshold: float | None = typer.Option( + None, "--similarity-threshold", - help="Similarity threshold for duplicate detection (0.0-1.0).", + help="Similarity threshold for duplicate detection (0.0-1.0). " + "[default: 0.92 or config value]", ), - max_documents: int = typer.Option( - 1000, + max_documents: int | None = typer.Option( + None, "--max-documents", - help="Maximum documents to audit.", + help="Maximum documents to audit. [default: 1000 or config value]", ), output: str | None = typer.Option( None, @@ -84,6 +145,40 @@ def audit_command( if verbose: ctx.verbose = True + # Load the optional corpus config section from a config file. Explicit CLI + # flags take precedence over the file's values (see _resolve_audit_settings). + corpus_cfg: CorpusConfig | None = None + if config_path is not None: + try: + config = load_config_from_path(Path(config_path)) + except ConfigurationError as e: + console.print(f"[red]Error:[/red] {e.message}") + raise typer.Exit(code=2) from e + corpus_cfg = config.corpus + if corpus_cfg is None: + console.print( + f"[red]Error:[/red] No 'corpus:' section found in {config_path}" + ) + raise typer.Exit(code=2) + + corpus_path, checks, staleness_days, similarity_threshold, max_documents = ( + _resolve_audit_settings( + corpus_cfg, + corpus_path=corpus_path, + checks=checks, + staleness_days=staleness_days, + similarity_threshold=similarity_threshold, + max_documents=max_documents, + ) + ) + + if corpus_path is None: + console.print( + "[red]Error:[/red] No corpus path provided. Pass it as an argument " + "or set 'corpus.path' in the config file." + ) + raise typer.Exit(code=2) + if not ctx.quiet: console.print(f"[bold blue]OpenAgent Eval[/bold blue] v{__version__}") console.print(f"[dim]Corpus: {corpus_path}[/dim]\n") diff --git a/tests/unit/test_cli/test_audit_config.py b/tests/unit/test_cli/test_audit_config.py new file mode 100644 index 0000000..1a0ab78 --- /dev/null +++ b/tests/unit/test_cli/test_audit_config.py @@ -0,0 +1,167 @@ +"""Tests for the ``audit`` command reading settings from a config file (#121). + +The audit command must accept a ``--config`` pointing at a config file whose +``corpus:`` section supplies the audit settings, while explicit CLI flags keep +precedence over the file's values. +""" + +from __future__ import annotations + +from pathlib import Path + +import yaml +from typer.testing import CliRunner + +from openagent_eval.cli.main import app +from openagent_eval.corpus.models import AuditReport + +runner = CliRunner() + + +def _write_config(tmp_path: Path, corpus_dir: Path, **corpus_overrides: object) -> Path: + """Write a minimal config.yaml with a ``corpus:`` section.""" + corpus: dict[str, object] = { + "path": str(corpus_dir), + "checks": ["contradiction", "staleness"], + "staleness_days": 180, + "similarity_threshold": 0.5, + "max_documents": 50, + } + corpus.update(corpus_overrides) + config = { + "dataset": {"path": "data.json"}, + "llm": {"provider": "openai", "model": "gpt-4o"}, + "corpus": corpus, + } + config_path = tmp_path / "config.yaml" + config_path.write_text(yaml.dump(config), encoding="utf-8") + return config_path + + +class _CaptureAuditor: + """Stand-in for ``CorpusAuditor`` that records constructor kwargs. + + Patching this at the audit-command boundary keeps the heavy embedding / + LLM machinery from running while still asserting how the command wires + resolved settings into the auditor. + """ + + captured: dict[str, object] = {} + + def __init__(self, **kwargs: object) -> None: + type(self).captured = dict(kwargs) + + async def audit(self, corpus_path: str) -> AuditReport: + type(self).captured["corpus_path"] = corpus_path + return AuditReport(corpus_path=corpus_path, total_documents=0) + + +def _patch_auditor(monkeypatch) -> type[_CaptureAuditor]: + _CaptureAuditor.captured = {} + monkeypatch.setattr( + "openagent_eval.cli.commands.audit.CorpusAuditor", _CaptureAuditor + ) + return _CaptureAuditor + + +def test_audit_uses_config_file_values_when_flags_absent(tmp_path, monkeypatch): + """With --config and no tuning flags, config values reach the auditor.""" + corpus_dir = tmp_path / "kb" + corpus_dir.mkdir() + auditor = _patch_auditor(monkeypatch) + config_path = _write_config(tmp_path, corpus_dir) + + result = runner.invoke(app, ["audit", "--config", str(config_path)]) + + assert result.exit_code == 0, result.output + assert auditor.captured["staleness_days"] == 180 + assert auditor.captured["similarity_threshold"] == 0.5 + assert auditor.captured["max_documents"] == 50 + assert auditor.captured["checks"] == ["contradiction", "staleness"] + assert auditor.captured["corpus_path"] == str(corpus_dir) + + +def test_audit_flag_overrides_config_value(tmp_path, monkeypatch): + """Explicit CLI flags win over the config file's corpus values.""" + corpus_dir = tmp_path / "kb" + corpus_dir.mkdir() + auditor = _patch_auditor(monkeypatch) + config_path = _write_config(tmp_path, corpus_dir) + + result = runner.invoke( + app, + [ + "audit", + "--config", + str(config_path), + "--staleness-days", + "30", + "--max-documents", + "7", + ], + ) + + assert result.exit_code == 0, result.output + # Flags override the file... + assert auditor.captured["staleness_days"] == 30 + assert auditor.captured["max_documents"] == 7 + # ...but the un-flagged setting still comes from the file. + assert auditor.captured["similarity_threshold"] == 0.5 + + +def test_audit_positional_path_overrides_config_path(tmp_path, monkeypatch): + """An explicit positional corpus path wins over corpus.path in the file.""" + config_dir = tmp_path / "from_config" + config_dir.mkdir() + cli_dir = tmp_path / "from_cli" + cli_dir.mkdir() + auditor = _patch_auditor(monkeypatch) + config_path = _write_config(tmp_path, config_dir) + + result = runner.invoke( + app, ["audit", str(cli_dir), "--config", str(config_path)] + ) + + assert result.exit_code == 0, result.output + assert auditor.captured["corpus_path"] == str(cli_dir) + + +def test_audit_without_config_uses_builtin_defaults(tmp_path, monkeypatch): + """Without --config the command keeps its original built-in defaults.""" + corpus_dir = tmp_path / "kb" + corpus_dir.mkdir() + auditor = _patch_auditor(monkeypatch) + + result = runner.invoke(app, ["audit", str(corpus_dir)]) + + assert result.exit_code == 0, result.output + assert auditor.captured["staleness_days"] == 365 + assert auditor.captured["similarity_threshold"] == 0.92 + assert auditor.captured["max_documents"] == 1000 + assert auditor.captured["checks"] is None + + +def test_audit_errors_when_no_path_and_no_config(monkeypatch): + """No positional path and no config is an error, not a crash.""" + _patch_auditor(monkeypatch) + + result = runner.invoke(app, ["audit"]) + + assert result.exit_code == 2 + assert "No corpus path" in result.output + + +def test_audit_errors_when_config_missing_corpus_section(tmp_path, monkeypatch): + """A config file without a corpus: section is a clear error.""" + _patch_auditor(monkeypatch) + config = { + "dataset": {"path": "data.json"}, + "llm": {"provider": "openai", "model": "gpt-4o"}, + } + config_path = tmp_path / "config.yaml" + config_path.write_text(yaml.dump(config), encoding="utf-8") + + result = runner.invoke(app, ["audit", "--config", str(config_path)]) + + assert result.exit_code == 2 + assert "corpus:" in result.output diff --git a/tests/unit/test_config/test_models.py b/tests/unit/test_config/test_models.py index 4d58c5d..53561ea 100644 --- a/tests/unit/test_config/test_models.py +++ b/tests/unit/test_config/test_models.py @@ -10,6 +10,8 @@ from openagent_eval.config.loader import load_config from openagent_eval.config.models import ( Config, + CorpusCheckType, + CorpusConfig, DatasetConfig, LLMConfig, MetricsConfig, @@ -164,3 +166,78 @@ def test_load_config_warns_on_dropped_metrics(self, tmp_path: Path, caplog) -> N load_config(config_path) assert "totally_fake_metric" in caplog.text + + +class TestCorpusConfigIntegration: + """Issue #121: the corpus: section is part of the main Config.""" + + def test_corpus_defaults_to_none(self) -> None: + """A Config without a corpus section leaves corpus unset.""" + config = Config( + dataset=DatasetConfig(path="data.json"), + llm=LLMConfig(provider="openai", model="gpt-4o"), + ) + assert config.corpus is None + + def test_load_config_parses_corpus_section(self, tmp_path: Path) -> None: + """A corpus: section in config.yaml parses into a CorpusConfig.""" + config_dict = { + "dataset": {"path": "data.json"}, + "llm": {"provider": "openai", "model": "gpt-4o"}, + "corpus": { + "path": "./knowledge_base/", + "checks": ["contradiction", "staleness"], + "staleness_days": 180, + "similarity_threshold": 0.8, + "max_documents": 50, + }, + } + config_path = tmp_path / "with_corpus.yaml" + config_path.write_text(yaml.dump(config_dict), encoding="utf-8") + + config = load_config(config_path) + + assert isinstance(config.corpus, CorpusConfig) + assert config.corpus.path == "./knowledge_base/" + assert config.corpus.checks == [ + CorpusCheckType.CONTRADICTION, + CorpusCheckType.STALENESS, + ] + assert config.corpus.staleness_days == 180 + assert config.corpus.similarity_threshold == 0.8 + assert config.corpus.max_documents == 50 + + def test_load_config_without_corpus_leaves_it_none(self, tmp_path: Path) -> None: + """Omitting the corpus section keeps config.corpus as None.""" + config_dict = { + "dataset": {"path": "data.json"}, + "llm": {"provider": "openai", "model": "gpt-4o"}, + } + config_path = tmp_path / "no_corpus.yaml" + config_path.write_text(yaml.dump(config_dict), encoding="utf-8") + + config = load_config(config_path) + + assert config.corpus is None + + def test_load_config_corpus_unknown_key_is_ignored(self, tmp_path: Path) -> None: + """Unknown keys in corpus: are silently ignored, matching sibling + sections (pydantic ``extra="ignore"``); they neither error nor drop + the known fields.""" + config_dict = { + "dataset": {"path": "data.json"}, + "llm": {"provider": "openai", "model": "gpt-4o"}, + "corpus": { + "path": "./kb/", + "staleness_days": 90, + "totally_fake_corpus_key": 123, + }, + } + config_path = tmp_path / "corpus_unknown.yaml" + config_path.write_text(yaml.dump(config_dict), encoding="utf-8") + + config = load_config(config_path) + + assert isinstance(config.corpus, CorpusConfig) + assert config.corpus.staleness_days == 90 + assert not hasattr(config.corpus, "totally_fake_corpus_key")