fix(cli): remove --evaluators, which never affected a run - #91
Merged
Conversation
aicertify_app_for_policy derives the evaluator set from the RequiredMetrics the selected policies declare: it calls discover_evaluators(metrics) and runs what that returns. _run_evaluate accepted an evaluators argument and did not forward it, so a value supplied on the command line reached nothing. The parser accepted the flag, argparse bound it, and the run proceeded identically. Wiring it through was considered and rejected. Restricting the evaluator set below what a policy requires leaves that policy's metric absent from the OPA input, and an absent metric is not a failed one: a rule that reads it goes undefined and its default stands. A flag whose plain meaning is "run fewer evaluators" would therefore turn a policy that should deny into one that reports no obstacle, which is a worse outcome than the flag doing nothing. Selecting evaluators explicitly remains available on evaluate_by_policy for callers using the API directly. Removing the registration rather than warning on it makes the change visible at the point of use. `aicertify evaluate --evaluators Fairness` now exits 2 with "unrecognized arguments" instead of accepting the flag and ignoring it. Nothing depended on its effect, because it had none; only its acceptance changes. #90 removed the flag from the documented options at the same time. Every other flag the parser registers was checked: all sixteen have a destination that is read. --evaluators was the only one that did not.
kmadan
added a commit
that referenced
this pull request
Aug 31, 2026
… drift Two things a first-time contributor hits before writing a line. The three example run.py scripts call OpenAI. DeepEval judges toxicity with an LLM and activates off the presence of OPENAI_API_KEY, so anyone with a key in their shell gets billed for running an example, and anyone with an exhausted quota watches nine retries per interaction scroll past and concludes the example is broken. The bundled demo already guards against this by popping the key unless --with-llm-metrics is passed; the examples never got the same treatment. They do now, opting in through AICERTIFY_WITH_LLM_METRICS=1, which each example README documents. customer-support-bot goes from a wall of 429s to zero OpenAI calls and still produces its report. The guard has to run before `from aicertify import ...`, because the evaluator stack reads the key at import time. That puts the import below module-level code, which ruff flags as E402; the ordering is load-bearing, so it carries a noqa and a comment saying why rather than being reordered into correctness bug. Separately, the README flag tables had drifted in both directions: --evaluators removed in #91, still documented in all four translations --params has always existed, documented in none of the five READMEs Neither shows up in a test, a lint or a build. A reader following the Japanese README gets `unrecognized arguments` and reasonably concludes the tool is broken. tests/test_readme_flags.py now asserts every documented flag exists, across all five languages, and that the English table covers every `evaluate` flag. Mutation-tested in both directions before committing. The translations are still missing a --params row. That needs a native speaker rather than a machine translation nobody can vouch for, so the test does not demand it and it stays a contribution task.
kmadan
added a commit
that referenced
this pull request
Aug 31, 2026
… drift Two things a first-time contributor hits before writing a line. The three example run.py scripts call OpenAI. DeepEval judges toxicity with an LLM and activates off the presence of OPENAI_API_KEY, so anyone with a key in their shell gets billed for running an example, and anyone with an exhausted quota watches nine retries per interaction scroll past and concludes the example is broken. The bundled demo already guards against this by popping the key unless --with-llm-metrics is passed; the examples never got the same treatment. They do now, opting in through AICERTIFY_WITH_LLM_METRICS=1, which each example README documents. customer-support-bot goes from a wall of 429s to zero OpenAI calls and still produces its report. The guard has to run before `from aicertify import ...`, because the evaluator stack reads the key at import time. That puts the import below module-level code, which ruff flags as E402; the ordering is load-bearing, so it carries a noqa and a comment saying why rather than being reordered into correctness bug. Separately, the README flag tables had drifted in both directions: --evaluators removed in #91, still documented in all four translations --params has always existed, documented in none of the five READMEs Neither shows up in a test, a lint or a build. A reader following the Japanese README gets `unrecognized arguments` and reasonably concludes the tool is broken. tests/test_readme_flags.py now asserts every documented flag exists, across all five languages, and that the English table covers every `evaluate` flag. Mutation-tested in both directions before committing. The translations are still missing a --params row. That needs a native speaker rather than a machine translation nobody can vouch for, so the test does not demand it and it stays a contribution task.
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
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.
🚀 Overview
Removes
--evaluatorsfrom theevaluatesubcommand. The flag was accepted andhad no effect on any run. Follow-up to #90, which removed it from the documented
options.
🔍 Why it never worked
aicertify_app_for_policyderives the evaluator set from theRequiredMetricsthe selected policies declare:
_run_evaluateaccepted anevaluatorsargument and did not forward it:The parser accepted the flag, argparse bound it, and the run proceeded
identically.
🎯 Why removed rather than wired through
Restricting the evaluator set below what a policy requires leaves that policy's
metric absent from the OPA input. An absent metric is not a failed one: a
rule that reads it goes undefined and its
defaultstands.A flag whose plain meaning is "run fewer evaluators" would therefore convert a
policy that should deny into one reporting no obstacle. That is a worse outcome
than the flag doing nothing, and it is the same fail-open class GOPAL's
empty-input test gate exists to catch.
Explicit evaluator selection remains available on
evaluate_by_policy(
aicertify/api/policy.py:297) for callers using the API directly.🎯 Why removed rather than warned
Removing the registration makes the change visible at the point of use:
Nothing depended on the flag's effect, because it had none. Only its acceptance
changes.
📜 Changes Made
evaluatorsparameter from_run_evaluate(dead)evaluators=args.evaluatorsat the call site (dead)ev.add_argument("--evaluators", ...)registrationThree deletions, 7 lines. No other behaviour changes.
🎯 How to Test
📄 Notes for reviewers
Every flag the parser registers was audited for the same defect. All sixteen
others have a destination that is read somewhere in
cli.py;--evaluatorswasthe only one that did not.
--with-llm-metricson thedemosubcommand, itsnearest neighbour, is genuinely wired (
cli.py:112).