feat(cli): let audit read corpus settings from the config file (fixes #121)#184
Merged
Merged
Conversation
…HQ#121) The corpus: section is already part of the main Config model and the loader parses it, but the audit command ignored config files entirely and only accepted CLI flags. Add a --config option to audit: it reads the corpus: section and uses those values, while explicit CLI flags keep precedence over the file (mirroring how run merges overrides). The corpus path may now come from corpus.path when the positional argument is omitted. Adds unit tests for corpus-section parsing, unknown-key handling, and the flag-vs-file precedence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🎉 Congratulations @Nitjsefnie! Your pull request has been successfully merged into main. 🚀 Thank you for contributing to OpenAgentHQ and helping improve the project. We truly appreciate your contribution and hope to see you back with more amazing PRs! Happy Open Sourcing! ❤️ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #121 — with a premise correction worth stating up front.
What the code showed vs what the issue says
Two of the issue's three asks were already implemented before it was filed:
CorpusConfigalready lives inconfig/models.py:119and is already wired ascorpus: CorpusConfig | Noneon the mainConfig(landed in8908183, Phase 9), and the loader already parses acorpus:section (plain pydantic — verified empirically). There is no competingCorpusConfigincorpus/. The genuine gap was ask #3:oaeval auditnever read any of it — required positional path, CLI flags only, noload_configcall. That's what this PR adds.Design (precedent-cited)
auditgains--config/-C; the positional corpus path becomes optional and falls back tocorpus.path.runcommand's own override pattern (flags default toNonesentinels, applied only when provided): explicit flag > config value > built-in default, via a pure_resolve_audit_settingshelper.corpus:behave exactly like every sibling section (pydanticextra="ignore"— pinned by a test). The Add validation for retriever settings field to catch typos early #116 warn-drop precedent governs the free-form retrieversettingsdict, not typed model fields, so it doesn't apply here.CorpusConfig.llm_provider(config field isstr | NonebutCorpusAuditorwants a provider object — nothing consumes it today; noted below) and cwd auto-discovery ofconfig.yaml(an explicit--configavoids silently changing existingoaeval audit ./kbbehavior — happy to switch torun-style auto-discovery if you prefer).Tests + verification
10 new tests: 4 model-integration (defaults, section parse, absent section, unknown-key-ignored) + 6 CliRunner tests with the auditor mocked at the boundary (file-values-when-flags-absent, flag-overrides-file, positional-overrides-config-path, builtin-defaults, and both error paths). Mutation-checked (implementer, then independently re-run before opening): inverting the flag/file precedence fails exactly
test_audit_flag_overrides_config_value; removing the corpus field fails all four model tests; dropping the path fallback fails the file-values tests. Gates:pytest tests/unit= 868 passed vs the 858-passed clean-main baseline, same single pre-existing BERTScore failure, nothing else. Also ran a real (unmocked)oaeval audit --config cfg.yaml -vend-to-end: the config-specified check drives the audit.Possible follow-up (observation, not fixed here):
CorpusConfig.llm_provider/model/embedding_modelparse from config but no code path consumes them, so contradiction detection can't yet be configured via file. Happy to file/take that separately.🤖 This PR was written with AI assistance (Claude), directed and verified by a human-supervised workflow.