diff --git a/dokimi/changelog.d/20261030_120000_napetrov_arch-loader.md b/dokimi/changelog.d/20261030_120000_napetrov_arch-loader.md new file mode 100644 index 000000000..9ab1e419a --- /dev/null +++ b/dokimi/changelog.d/20261030_120000_napetrov_arch-loader.md @@ -0,0 +1,23 @@ +### Changed + +- **Bad input exits 2 in the `novelty` and `report model-compare` commands** — a + file that `novelty` cannot read or that is not the artifact it names (`--artifact`, + `--atoms`, `--probes`, `--competitors`, `--queries-by-skill`), and the runs that + `report model-compare` refuses (a missing, unreadable, non-JSON or non-object file; + runs that disagree on the treatment, prompt or judge), and the skill fields that + `novelty routing` refuses (a missing competitor, a skill listed twice, a skill compared + with itself, an all-vs-all catalog that holds one skill twice with different content), + exit 2 with `Error: ` on stderr. For a bad file the message names the file and + the reason; for a refused field it names the skill or option at fault. The `novelty` + file refusals keep the exit code 2 they had, `--artifact` and `--atoms` among them; the + `model-compare`, `routing` and `view` refusals keep that code too. Exit 1 stays the verdict of the gates + (`--strict`, `eval judge-validity`) and the code of a model failure (an extractor or judge + that errors), so a script that branches on the code must treat 2 as "fix the input". + `subjects run --novelty-artifact` still prints its own `Error loading novelty artifact:` message + (now with the new reason text) and exits 2, as before. `novelty view` + also checks the `bundle` of an atoms file, so a malformed bundle is refused with exit 2. Two `report model-compare` messages changed wording: + `File not found: ` is now `cannot read : No such file or directory`, and + `Invalid JSON in ` is now ` is not valid JSON`. An invalid artifact named by + `novelty view` says which part is wrong: `is not valid JSON`, `is not UTF-8 text`, + `is not a novelty artifact: expected a JSON object`, or `is not a valid novelty artifact: + `. diff --git a/dokimi/docs/architecture.md b/dokimi/docs/architecture.md index 5aafd5a88..8cb2aae49 100644 --- a/dokimi/docs/architecture.md +++ b/dokimi/docs/architecture.md @@ -420,6 +420,10 @@ Guide: [run-consistency.md](run-consistency.md). decides who owns each pipe, so an interrupted launch closes its streams exactly once and Windows reader threads close theirs at EOF. `FakeRunner` is its test adapter: code that spawns takes a `runner` argument. +- Bad input: a library loader raises `dokimi.utils.InputError`, naming the file, and + never ends the process. `cli.main` prints `Error: ` and exits 2; exit 1 stays + the verdict of a gate. `tests/test_import_architecture.py` allows an exit only in the + frontend. - `dokimi/llm.py` — the entry point for LLM calls. `LLM` (one model at one provider: `complete`/`ask`/`chat`, retry, the response cache, usage) sits on one adapter seam: `LiteLLMAdapter` in production, `FakeAdapter` in tests (the diff --git a/dokimi/dokimi/artifacts.py b/dokimi/dokimi/artifacts.py index 55fd25cf6..af7e318fa 100644 --- a/dokimi/dokimi/artifacts.py +++ b/dokimi/dokimi/artifacts.py @@ -34,6 +34,8 @@ from pathlib import Path from typing import Any +from dokimi.utils import InputError, read_json_input + logger = logging.getLogger(__name__) SCHEMA_DIR = Path(__file__).resolve().parent / "schemas" @@ -207,11 +209,18 @@ def _write_target(path: Path) -> Path: return target -def load_artifact(kind: str, path: Path, *, validate: bool = True) -> dict: - """Read and (optionally) validate an artifact from ``path``.""" - data = json.loads(Path(path).read_text(encoding="utf-8")) +def load_artifact(kind: str, path: Path | str, *, validate: bool = True) -> dict: + """Read and (optionally) validate an artifact of ``kind`` from ``path``. + + Every failure is an :class:`~dokimi.utils.InputError` naming ``path``: a file that is + missing, unreadable, not UTF-8 or JSON, not a JSON object, or that fails the schema. + """ + data = read_json_input(path) if not isinstance(data, dict): - raise ValueError(f"Artifact must be a JSON object, got {type(data).__name__}") + raise InputError(f"{path} is not a {kind} artifact: expected a JSON object, got {type(data).__name__}") if validate: - validate_artifact(kind, data) + try: + validate_artifact(kind, data) + except ArtifactValidationError as exc: + raise InputError(f"{path} is not a valid {kind} artifact: {exc}") from exc return data diff --git a/dokimi/dokimi/cli.py b/dokimi/dokimi/cli.py index 225112db7..44b82b8af 100755 --- a/dokimi/dokimi/cli.py +++ b/dokimi/dokimi/cli.py @@ -193,15 +193,14 @@ def main() -> None: process.install_signal_handlers() # SIGTERM/SIGHUP: kill the process groups we started, then exit tolerate_console_encoding() args = parse_args() - from dokimi.eval.question_text import EmptyQuestionError from dokimi.utils import InputError try: args.func(args) - except (EmptyQuestionError, InputError, llm_cli.SamplingError) as exc: # bad input, not a bug: no traceback + except (InputError, llm_cli.SamplingError) as exc: # bad input, not a bug: no traceback print(f"Error: {exc}", file=sys.stderr) - # 2, as the commands' other input errors: 1 is the verdict of `eval judge-validity` - # and of the --strict gates, and a malformed file must not read as a failed gate. + # 2 is the one code for bad input: 1 is the verdict of `eval judge-validity` and of the + # --strict gates, and a malformed file must not read as a failed gate. sys.exit(2) diff --git a/dokimi/dokimi/commands/_output.py b/dokimi/dokimi/commands/_output.py index 1d72ba552..518a522a3 100644 --- a/dokimi/dokimi/commands/_output.py +++ b/dokimi/dokimi/commands/_output.py @@ -20,3 +20,16 @@ def exit_on_unwritable(path: Path): except OSError as exc: print(f"Error: cannot write {path}: {exc}", file=sys.stderr) sys.exit(EXIT_USAGE) + + +def write_text_or_exit(path: Path, text: str, code: int = EXIT_USAGE) -> None: + """Write ``text`` to ``path`` (creating parents), or exit ``code`` naming ``path`` when it cannot be written. + + A command helper: library code that cannot write raises, and the command decides the exit. + """ + try: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + except OSError as exc: + print(f"Error: cannot write {path}: {exc}", file=sys.stderr) + sys.exit(code) diff --git a/dokimi/dokimi/commands/arms.py b/dokimi/dokimi/commands/arms.py index 3890cbd01..70c46861d 100644 --- a/dokimi/dokimi/commands/arms.py +++ b/dokimi/dokimi/commands/arms.py @@ -14,7 +14,7 @@ from dokimi.exit_codes import EXIT_FAILURE, EXIT_USAGE from dokimi.llm import ANSWER_PROVIDERS -from dokimi.utils import question_id +from dokimi.utils import InputError, question_id from dokimi.eval.judge import JUDGE_PROVIDERS from dokimi.defaults import ( DEFAULT_ANSWER_MODEL, @@ -123,8 +123,8 @@ def _load_matrix_inputs(config: ArmsRunConfig, arms: str, questions: str, matrix cells = load_matrix_cells(matrix_cells) _validate_unique_cell_filenames(cells) arms_run.check_matrix(config, cells, specs) - except EmptyQuestionError: - raise # a bad input record, not a cell error: cli.main reports it (exit 2) + except InputError: # EmptyQuestionError is one + raise # a bad input record or a refused cell: cli.main reports it (exit 2) except arms_run.ProbeError as exc: print(f"Error loading matrix run inputs: {exc}", file=sys.stderr) sys.exit(EXIT_FAILURE) diff --git a/dokimi/dokimi/commands/evaluate.py b/dokimi/dokimi/commands/evaluate.py index 1549002ab..e7d25b259 100644 --- a/dokimi/dokimi/commands/evaluate.py +++ b/dokimi/dokimi/commands/evaluate.py @@ -23,8 +23,9 @@ ) from dokimi.eval.judge import JUDGE_PROVIDERS from dokimi.eval.judge_validity import DEFAULT_MIN_ACCURACY, NoUsablePairs -from dokimi.utils import same_evaluator, write_text_or_exit +from dokimi.commands._output import write_text_or_exit from dokimi.exit_codes import EXIT_USAGE +from dokimi.utils import same_evaluator def _warn_judge_independence( @@ -47,14 +48,10 @@ def _warn_judge_independence( def _load_answers(path: str): - """The answers artifact at ``path`` (read by dokimi/records.py), or a clean exit.""" - from dokimi.records import RecordError, load + """The answers artifact at ``path`` (read by dokimi/records.py); a RecordError (an InputError) if not.""" + from dokimi.records import load - try: - return load(path, section="answers") - except RecordError as exc: - print(f"Error: {exc}", file=sys.stderr) - sys.exit(2) + return load(path, section="answers") def _write_json_or_exit(path: Path, payload: dict, code: int = EXIT_USAGE) -> None: diff --git a/dokimi/dokimi/commands/explore.py b/dokimi/dokimi/commands/explore.py index ee7ec0ead..f31630234 100644 --- a/dokimi/dokimi/commands/explore.py +++ b/dokimi/dokimi/commands/explore.py @@ -133,7 +133,7 @@ def cmd_explore_run(args: argparse.Namespace) -> None: if args.out_md: from dokimi.report.exploration_report import render_explore_report - from dokimi.utils import write_text_or_exit + from dokimi.commands._output import write_text_or_exit write_text_or_exit(Path(args.out_md), render_explore_report(output)) print(f"Saved exploration report: {args.out_md}") diff --git a/dokimi/dokimi/commands/judge_reliability.py b/dokimi/dokimi/commands/judge_reliability.py index e78d07959..724ff45dc 100644 --- a/dokimi/dokimi/commands/judge_reliability.py +++ b/dokimi/dokimi/commands/judge_reliability.py @@ -19,34 +19,28 @@ ) from dokimi.eval.judge import JUDGE_PROVIDERS from dokimi.eval.judge_validity import DEFAULT_MIN_ACCURACY +from dokimi.utils import InputError, read_json_input +from dokimi.commands._output import write_text_or_exit from dokimi.exit_codes import EXIT_FAILURE, EXIT_USAGE -from dokimi.utils import write_text_or_exit if TYPE_CHECKING: from dokimi.eval.cross_judge import Measure def _read_json(path: str): - """The parsed JSON at ``path``, or exit 2 (bad input, not a failed gate) naming the file.""" - try: - return json.loads(Path(path).read_text(encoding="utf-8")) - except OSError as exc: - print(f"Error: cannot read {path}: {exc}", file=sys.stderr) - except (ValueError, RecursionError) as exc: # JSONDecodeError, bad UTF-8, deep nesting - print(f"Error: {path} is not valid JSON: {exc}", file=sys.stderr) - sys.exit(2) + """The parsed JSON at ``path``; an InputError naming the file when it cannot be read (exit 2).""" + return read_json_input(path) def _load_json_object(path: str, key: str, kind: type = dict) -> dict: - """The JSON object at ``path``, or exit 2 naming the file. + """The JSON object at ``path``, or InputError naming the file. The object must also hold a ``kind`` (object or list) under ``key``. """ data = _read_json(path) if not isinstance(data, dict) or not isinstance(data.get(key), kind): what = f"a JSON object with a `{key}` {'list' if kind is list else 'object'}" - print(f"Error: {path} is not {what}", file=sys.stderr) - sys.exit(2) + raise InputError(f"{path} is not {what}") return data @@ -57,7 +51,7 @@ def _judge_name(doc: dict, path: str) -> str: def _result_measure(path: str, key: str, kind: type, build): - """The measure an `eval pairwise` or `eval checklist` file gives, or exit 2 naming the file. + """The measure an `eval pairwise` or `eval checklist` file gives, or an InputError naming the file. *build* is ``(doc, name) -> Measure``; it raises ValueError on an entry of the wrong shape. """ @@ -65,32 +59,25 @@ def _result_measure(path: str, key: str, kind: type, build): try: return build(doc, _judge_name(doc, path)) except ValueError as exc: - print(f"Error: {path}: {exc}", file=sys.stderr) - sys.exit(2) + raise InputError(f"{path}: {exc}") from exc def _load_answers(path: str): - """The parsed file and its answers records (dokimi/records.py), or a clean exit.""" - from dokimi.records import RecordError, read + """The parsed file and its answers records (dokimi/records.py); a RecordError (an InputError) if not.""" + from dokimi.records import read data = _read_json(path) - try: - answers = list(read(data, source=path, section="answers").records) - except RecordError as exc: - print(f"Error: {exc}", file=sys.stderr) - sys.exit(2) # 1 is the verdict code of judge-validity and the --strict gates - return data, answers + return data, list(read(data, source=path, section="answers").records) def _load_points(path: str): - """question_id -> required points from a questions file, or exit 2 naming the file.""" + """question_id -> required points from a questions file, or an InputError naming the file.""" from dokimi.eval.checklist import load_points try: return load_points(path) except (OSError, ValueError) as exc: # ValueError covers bad JSON, bad UTF-8 and a refused set - print(f"Error: cannot read questions {path}: {exc}", file=sys.stderr) - sys.exit(2) + raise InputError(f"cannot read questions {path}: {exc}") from exc def _require_arms(path: str, records, command: str) -> None: @@ -102,10 +89,9 @@ def _require_arms(path: str, records, command: str) -> None: from dokimi.records import has_arms if not has_arms(records): - print(f"Error: {path} has no arms answers (no record has an `arms` field); " - f"eval {command} reads an arms run (arms run --out-json), not a binary " - "with_docs/without_docs answers file", file=sys.stderr) - sys.exit(2) + raise InputError(f"{path} has no arms answers (no record has an `arms` field); " + f"eval {command} reads an arms run (arms run --out-json), not a binary " + "with_docs/without_docs answers file") def _progress(label: str = ""): diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index ae625fd9c..4a87422de 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -20,6 +20,7 @@ from typing import TYPE_CHECKING, NoReturn from dokimi.commands.inputs import placebo_text_for_arms_or_exit +from dokimi.commands.novelty_routing_inputs import load_all_vs_all_field, load_routing_field from dokimi.defaults import ( DEFAULT_ANSWER_MODEL, DEFAULT_ANSWER_PROVIDER, @@ -28,8 +29,8 @@ DEFAULT_JUDGE_MODEL, DEFAULT_JUDGE_PROVIDER, ) -from dokimi.exit_codes import usage_exit from dokimi.llm import ANSWER_PROVIDERS as PROVIDERS +from dokimi.utils import InputError if TYPE_CHECKING: from dokimi.novelty.tiers import RunSettings @@ -119,8 +120,8 @@ def cmd_novelty_analyze(args: argparse.Namespace) -> None: extractor=extractor, answerer=answerer, judge=judge, artifact_bundle=artifact_bundle) except SelfJudgeError as exc: # the judge is the answering model: the flags contradict - usage_exit(str(exc)) - except (ExtractionError, ValueError) as exc: # a model failure under way + raise InputError(str(exc)) from exc + except (ExtractionError, ValueError) as exc: # a model failure under way: exit 1, not bad input raise SystemExit(str(exc)) from None out_dir = Path(args.out_dir) if args.out_dir else _default_dir(args.artifact) @@ -155,7 +156,7 @@ def cmd_novelty_atoms(args: argparse.Namespace) -> None: extraction = ExtractionReport() try: atoms = extract_atoms(spans, client=extractor, cap=cap, report=extraction) - except ExtractionError as exc: + except ExtractionError as exc: # a model failure under way: exit 1, not bad input raise SystemExit(str(exc)) from None # The cap warning analyze records; probe and shorten, which read atoms.json, do not repeat it. reach = [cap_warning(cap, extraction.reached)] if extraction.reached else [] @@ -172,17 +173,17 @@ def cmd_novelty_atoms(args: argparse.Namespace) -> None: def cmd_novelty_probe(args: argparse.Namespace) -> None: """Run the paired probes against a saved atom set and print the 2×2.""" - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record from dokimi.novelty.bundle import check_answer_key from dokimi.novelty.measure import SelfJudgeError, check_self_judge, run_measurement from dokimi.novelty.probes import build_probe_set from dokimi.novelty.report import build_probe_record - atoms, saved_spans, saved = load_atoms_or_exit(args.atoms) + atoms, saved_spans, saved = load_atoms_record(args.atoms) _warn_unread(saved_spans, "the atoms probed here leave out whatever those spans assert") verdict = check_answer_key(args.artifact, saved.get("bundle"), source=args.atoms) if not verdict.ok: - usage_exit(verdict.message) + raise InputError(verdict.message) for note in verdict.notes: print(note, file=sys.stderr) settings = _settings(args) @@ -195,7 +196,7 @@ def cmd_novelty_probe(args: argparse.Namespace) -> None: try: # before the paraphrase calls below, which are paid check_self_judge(judge, answerer, args.allow_self_judge) except SelfJudgeError as exc: - usage_exit(str(exc)) + raise InputError(str(exc)) from exc paraphrases, reversals = settings.values["paraphrases"], settings.values["reversals"] probe_set = build_probe_set( atoms, client=extractor if paraphrases or reversals else None, @@ -214,7 +215,7 @@ def cmd_novelty_probe(args: argparse.Namespace) -> None: allow_self_judge=args.allow_self_judge, seed=args.seed, placebo_text=placebo_text, ) - except ValueError as exc: # a model failure under way + except ValueError as exc: # a model failure under way: exit 1, not bad input raise SystemExit(str(exc)) from None payload = build_probe_record( @@ -242,7 +243,7 @@ def cmd_novelty_items(args: argparse.Namespace) -> None: try: cells, warnings = load_probe_cells(args.probes) except ProbeFileError as exc: - usage_exit(str(exc)) + raise InputError(str(exc)) from exc for warning in warnings: print(f"WARNING: {warning}", file=sys.stderr) @@ -283,99 +284,28 @@ def cmd_novelty_items(args: argparse.Namespace) -> None: f"{agreement['n_atoms_multi_kind']} atoms asked by several kinds") -def _routing_card(ref: str): - """An artifact or competitor reference as the selector sees it: ``(candidate, body)``.""" - from dokimi.novelty.routing import load_candidate - - candidate, body, warning = load_candidate(ref) - if warning: - print(f"WARNING: {warning}", file=sys.stderr) - return candidate, body - - -def _routing_field(args: argparse.Namespace, exclude: set[str], - left_out: list[str] | None = None) -> list: - """The field ``--competitors`` and ``--catalog`` name, without the names in ``exclude``. - - What was left out of the field, and why, is appended to ``left_out`` when given. - - Raises: - OSError, ValueError, RecursionError: A reference or the catalog cannot be loaded. - """ - from dokimi.novelty.routing import ( - catalog_competitors, - disables_model_invocation, - non_skill_reason, - skill_file, - ) - - competitors = [] - for ref in args.competitors or []: - reason = non_skill_reason(ref) - if reason: - print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) - if left_out is not None: - left_out.append(f"{ref}: {reason}") - continue - candidate = _routing_card(ref)[0] - ref_file = skill_file(ref) - if (ref_file and not args.include_disabled - and disables_model_invocation(ref_file)): - print(f"WARNING: skipped competitor {ref}: disable-model-invocation " - "(--include-disabled keeps it)", file=sys.stderr) - if left_out is not None: - left_out.append(f"{ref}: disable-model-invocation") - continue - competitors.append(candidate) - if args.catalog: - notes: list[str] = [] - catalog = catalog_competitors(args.catalog, - exclude | {c.name for c in competitors}, notes, - include_disabled=args.include_disabled) - for note in notes: - print(f"WARNING: {note}", file=sys.stderr) - if left_out is not None: - left_out.extend(notes) - no_description = [c.name for c in catalog if not c.description] - if no_description: - print(f"WARNING: {len(no_description)} catalog skill(s) have no description, " - f"so their card is a name only: {', '.join(sorted(no_description))}", - file=sys.stderr) - print(f"--catalog {args.catalog}: {len(catalog)} competitors", file=sys.stderr) - competitors += catalog - return competitors - - def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None: """Every skill in the field is the gold in turn: confusion matrix and attractors.""" from dokimi.novelty.all_vs_all import all_vs_all, load_queries_by_skill - from dokimi.novelty.routing import RoutingInputError, tokenize + from dokimi.novelty.routing import tokenize _check_all_vs_all_args(args) left_out: list[str] = [] + field = load_all_vs_all_field(args, left_out) try: - field = _routing_field(args, set(), left_out) - except RoutingInputError as exc: - usage_exit(str(exc)) - except (OSError, ValueError, RecursionError) as exc: - usage_exit(f"cannot load the field: {exc}") - try: - try: - queries_by_skill = load_queries_by_skill(args.queries_by_skill) - except (OSError, UnicodeDecodeError) as exc: - usage_exit(f"cannot read the queries: {exc}") - selector = _client(args.model, args.provider, "selector", args) if args.model else None - if selector is not None: - # One call per query with a content word, each sending every card in the field. - calls = sum(bool(tokenize(q)) for qs in queries_by_skill.values() for q in qs) - print( - f"all-vs-all: --model makes {calls} model calls, one per query, over " - f"{len(field)} cards each", - file=sys.stderr, - ) - result = all_vs_all(field, queries_by_skill, client=selector, seed=args.seed) - except RoutingInputError as exc: - usage_exit(str(exc)) + queries_by_skill = load_queries_by_skill(args.queries_by_skill) + except (OSError, UnicodeDecodeError) as exc: + raise InputError(f"cannot read the queries: {exc}") from exc + selector = _client(args.model, args.provider, "selector", args) if args.model else None + if selector is not None: + # One call per query with a content word, each sending every card in the field. + calls = sum(bool(tokenize(q)) for qs in queries_by_skill.values() for q in qs) + print( + f"all-vs-all: --model makes {calls} model calls, one per query, over " + f"{len(field)} cards each", + file=sys.stderr, + ) + result = all_vs_all(field, queries_by_skill, client=selector, seed=args.seed) without = [c.name for c in field if c.name not in queries_by_skill] if without: print(f"WARNING: {len(without)} skill(s) in the field have no queries, so they only " @@ -415,16 +345,16 @@ def _field_record(args: argparse.Namespace, competitors: list, left_out: list[st def _check_all_vs_all_args(args: argparse.Namespace) -> None: """Refuse the flag combinations ``--all-vs-all`` cannot run with.""" if args.artifact: - usage_exit("--all-vs-all takes no --artifact: every skill in the field is the gold in turn") + raise InputError("--all-vs-all takes no --artifact: every skill in the field is the gold in turn") if args.queries or args.queries_from_headings: - usage_exit("--all-vs-all takes --queries-by-skill, not --queries or --queries-from-headings") + raise InputError("--all-vs-all takes --queries-by-skill, not --queries or --queries-from-headings") if args.repeats > 1: - usage_exit("--all-vs-all takes no --repeats: it makes one call per query") + raise InputError("--all-vs-all takes no --repeats: it makes one call per query") if not args.queries_by_skill: - usage_exit("--all-vs-all needs --queries-by-skill : a JSON object of skill " + raise InputError("--all-vs-all needs --queries-by-skill : a JSON object of skill " "name to the requests that should select it") if not args.out: - usage_exit("--all-vs-all needs --out ") + raise InputError("--all-vs-all needs --out ") def _warn_blind_skills(result: dict, queries_by_skill: dict[str, list[str]]) -> None: @@ -470,47 +400,26 @@ def _print_attractors(result: dict) -> None: def cmd_novelty_routing(args: argparse.Namespace) -> None: """Can this artifact be selected against its competitors? Lexical floor, model on top.""" if args.repeats > 1 and not args.model: - usage_exit("--repeats needs --model: the lexical floor is deterministic, so repeating it measures nothing") + raise InputError("--repeats needs --model: the lexical floor is deterministic, so repeating it measures nothing") from dokimi.novelty.routing import ( - RoutingInputError, check_field, - disables_model_invocation, headings_as_queries, lexical_routing, load_queries, model_routing, - skill_file, ) if args.all_vs_all: return _cmd_routing_all_vs_all(args) if not args.artifact: - usage_exit("pass --artifact , or --all-vs-all for the whole field") - try: - gold, body = _routing_card(args.artifact) - artifact_file = skill_file(args.artifact) - if artifact_file and disables_model_invocation(artifact_file): - print(f"WARNING: {args.artifact} sets disable-model-invocation, so no harness lists " - "it and the model cannot select it; its routing rate is not a harness rate", - file=sys.stderr) - left_out: list[str] = [] - competitors = _routing_field(args, {gold.name}, left_out) - except RoutingInputError as exc: - usage_exit(str(exc)) - except (OSError, ValueError, RecursionError) as exc: - usage_exit(f"cannot load an artifact: {exc}") + raise InputError("pass --artifact , or --all-vs-all for the whole field") + gold, body, competitors, left_out = load_routing_field(args) # Checked before the query file is read, so the message is about the real problem. - try: - check_field(gold, competitors) - except RoutingInputError as exc: - usage_exit(str(exc)) + check_field(gold, competitors) self_derived = False if args.queries: - try: - queries = load_queries(args.queries) - except RoutingInputError as exc: - usage_exit(str(exc)) + queries = load_queries(args.queries) elif args.queries_from_headings: queries = headings_as_queries(body) self_derived = True @@ -518,7 +427,7 @@ def cmd_novelty_routing(args: argparse.Namespace) -> None: "defines both the problem scope and its own relevance, so this is triage, not " "a routing rate", file=sys.stderr) else: - usage_exit("pass --queries with requests written independently of the artifact, or " + raise InputError("pass --queries with requests written independently of the artifact, or " "--queries-from-headings for a self-derived triage run") candidates = [gold, *competitors] lexical = dict(lexical_routing(candidates, queries, gold=0), @@ -620,24 +529,24 @@ def _refuse_no_queries(args: argparse.Namespace, queries: list[str], self_derive "reason": reason, "queries_self_derived": self_derived, "n_candidates": n_candidates}, indent=2), encoding="utf-8") - usage_exit(f"no usable queries: {reason}. Pass --queries with requests written independently " + raise InputError(f"no usable queries: {reason}. Pass --queries with requests written independently " f"of the artifact, one per line or a JSON list. Wrote {out} with status no_queries") def cmd_novelty_shorten(args: argparse.Namespace) -> None: """Score shortening candidates on the five axes and optionally write the best one.""" - from dokimi.novelty.artifact import load_artifact_or_exit - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.artifact import load_artifact_ref + from dokimi.novelty.atoms import load_atoms_record from dokimi.novelty.bundle import entry_point_only from dokimi.novelty.retention import RetentionOracle, error_warning from dokimi.novelty.shorten import build_candidates, pick_best - atoms, spans, saved = load_atoms_or_exit(args.atoms) - _kind, _name, _description, frontmatter, _body = load_artifact_or_exit(args.artifact) + atoms, spans, saved = load_atoms_record(args.atoms) + _kind, _name, _description, frontmatter, _body = load_artifact_ref(args.artifact) try: spans, atoms, note = entry_point_only(saved.get("bundle"), spans, atoms) except ValueError as exc: - usage_exit(f"{args.atoms}: {exc}") + raise InputError(f"{args.atoms}: {exc}") from exc if note: print(note, file=sys.stderr) _warn_unread(spans, "every candidate keeps them, since what they carry is unknown") @@ -738,7 +647,7 @@ def _bundle_from_args(args: argparse.Namespace): max_total_words=args.max_resource_words, ) except (OSError, ValueError, RecursionError) as exc: - usage_exit(f"cannot load --artifact {args.artifact}: {exc}") + raise InputError(f"cannot load --artifact {args.artifact}: {exc}") from exc note = artifact_bundle.coverage_note() if note: print(f"NOTE: {note}", file=sys.stderr) @@ -1051,7 +960,7 @@ def _clients(args: argparse.Namespace): try: models = _role_models(args) except ValueError as exc: - usage_exit(str(exc)) + raise InputError(str(exc)) from exc return ( _client(*models["extractor"], "extractor", args), _client(*models["answerer"], "answerer", args), @@ -1126,7 +1035,7 @@ def _arms(spec: str) -> tuple[str, ...]: try: return parse_arms(spec) except ValueError as exc: - usage_exit(str(exc)) + raise InputError(str(exc)) from exc def _default_dir(ref: str) -> Path: diff --git a/dokimi/dokimi/commands/novelty_routing_inputs.py b/dokimi/dokimi/commands/novelty_routing_inputs.py new file mode 100644 index 000000000..8dd2573d4 --- /dev/null +++ b/dokimi/dokimi/commands/novelty_routing_inputs.py @@ -0,0 +1,110 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Loading the artifact, competitors and field for `novelty routing`, failing with InputError.""" + +from __future__ import annotations + +import argparse +import sys +from typing import TYPE_CHECKING + +from dokimi.utils import InputError + +if TYPE_CHECKING: + from dokimi.novelty.routing import Candidate + + +def routing_card(ref: str) -> tuple[Candidate, str]: + """An artifact or competitor reference as the selector sees it: ``(candidate, body)``.""" + from dokimi.novelty.routing import load_candidate + + candidate, body, warning = load_candidate(ref) + if warning: + print(f"WARNING: {warning}", file=sys.stderr) + return candidate, body + + +def routing_field(args: argparse.Namespace, exclude: set[str], + left_out: list[str] | None = None) -> list[Candidate]: + """The field ``--competitors`` and ``--catalog`` name, without the names in ``exclude``. + + What was left out of the field, and why, is appended to ``left_out`` when given. + + Raises: + OSError, ValueError, RecursionError: A reference or the catalog cannot be loaded. + """ + from dokimi.novelty.routing import ( + catalog_competitors, + disables_model_invocation, + non_skill_reason, + skill_file, + ) + + competitors = [] + for ref in args.competitors or []: + reason = non_skill_reason(ref) + if reason: + print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr) + if left_out is not None: + left_out.append(f"{ref}: {reason}") + continue + candidate = routing_card(ref)[0] + ref_file = skill_file(ref) + if (ref_file and not args.include_disabled + and disables_model_invocation(ref_file)): + print(f"WARNING: skipped competitor {ref}: disable-model-invocation " + "(--include-disabled keeps it)", file=sys.stderr) + if left_out is not None: + left_out.append(f"{ref}: disable-model-invocation") + continue + competitors.append(candidate) + if args.catalog: + notes: list[str] = [] + catalog = catalog_competitors(args.catalog, + exclude | {c.name for c in competitors}, notes, + include_disabled=args.include_disabled) + for note in notes: + print(f"WARNING: {note}", file=sys.stderr) + if left_out is not None: + left_out.extend(notes) + no_description = [c.name for c in catalog if not c.description] + if no_description: + print(f"WARNING: {len(no_description)} catalog skill(s) have no description, " + f"so their card is a name only: {', '.join(sorted(no_description))}", + file=sys.stderr) + print(f"--catalog {args.catalog}: {len(catalog)} competitors", file=sys.stderr) + competitors += catalog + return competitors + + +def load_all_vs_all_field(args: argparse.Namespace, left_out: list[str]) -> list[Candidate]: + """The all-vs-all field, or InputError naming the cause.""" + try: + return routing_field(args, set(), left_out) + except InputError: + raise + except (OSError, ValueError, RecursionError) as exc: + raise InputError(f"cannot load the field: {exc}") from exc + + +def load_routing_field( + args: argparse.Namespace, +) -> tuple[Candidate, str, list[Candidate], list[str]]: + """The gold card, its body, the competitors and the ones left out, or InputError naming the cause.""" + from dokimi.novelty.routing import disables_model_invocation, skill_file + + try: + gold, body = routing_card(args.artifact) + artifact_file = skill_file(args.artifact) + if artifact_file and disables_model_invocation(artifact_file): + print(f"WARNING: {args.artifact} sets disable-model-invocation, so no harness lists " + "it and the model cannot select it; its routing rate is not a harness rate", + file=sys.stderr) + left_out: list[str] = [] + competitors = routing_field(args, {gold.name}, left_out) + except InputError: + raise + except (OSError, ValueError, RecursionError) as exc: + raise InputError(f"cannot load an artifact: {exc}") from exc + return gold, body, competitors, left_out diff --git a/dokimi/dokimi/commands/novelty_view_inputs.py b/dokimi/dokimi/commands/novelty_view_inputs.py index fd45db27d..0de9e5cb1 100644 --- a/dokimi/dokimi/commands/novelty_view_inputs.py +++ b/dokimi/dokimi/commands/novelty_view_inputs.py @@ -1,36 +1,18 @@ # Copyright (C) 2026 Intel Corporation # SPDX-License-Identifier: Apache-2.0 -"""`novelty view`: read the saved novelty artifact and atoms file, or exit 2 naming the bad one.""" +"""`novelty view`: read the saved novelty artifact and atoms file (bad input raises, exit 2 by the CLI).""" from __future__ import annotations -import sys from pathlib import Path -from typing import NoReturn -from dokimi.exit_codes import EXIT_USAGE - - -def _fail(message: str) -> NoReturn: - print(f"Error: {message}", file=sys.stderr) - raise SystemExit(EXIT_USAGE) def load_view_inputs(novelty_path: str, atoms_path: str): - """``(artifact, atoms, spans)`` from the two files; an unreadable or invalid one exits 2.""" + """``(artifact, atoms, spans)`` from the two files; an unreadable or invalid one raises InputError.""" from dokimi.artifacts import load_artifact from dokimi.novelty.atoms import load_atoms - try: - artifact = load_artifact("novelty", Path(novelty_path)) - except OSError as exc: - _fail(f"cannot read {novelty_path}: {exc}") - except (ValueError, RecursionError) as exc: # not UTF-8, not JSON, or not a novelty artifact - _fail(f"{novelty_path} is not a valid novelty artifact: {exc}") - try: - atoms, spans = load_atoms(atoms_path) - except OSError as exc: - _fail(f"cannot read {atoms_path}: {exc}") - except (ValueError, RecursionError) as exc: # not UTF-8, not JSON, or not an atoms record - _fail(f"{atoms_path} is not a valid atoms file: {exc}") + artifact = load_artifact("novelty", Path(novelty_path)) + atoms, spans = load_atoms(atoms_path) return artifact, atoms, spans diff --git a/dokimi/dokimi/eval/question_text.py b/dokimi/dokimi/eval/question_text.py index 6ef8c454a..e613e6a81 100644 --- a/dokimi/dokimi/eval/question_text.py +++ b/dokimi/dokimi/eval/question_text.py @@ -17,19 +17,19 @@ from collections.abc import Mapping from typing import Any, Iterable, Optional -from dokimi.utils import question_id +from dokimi.utils import InputError, question_id # answers.v1 writes ``question_text``; ``question``/``text`` are the # question-set spellings, accepted for hand-built inputs. _KEYS = ("question_text", "question", "text") -class EmptyQuestionError(ValueError): +class EmptyQuestionError(InputError): """A record handed to a judge has no question text. - A ``ValueError`` meaning a bad input record, which the CLI reports as - ``Error: ...`` with exit 2, the code the commands use for bad input (1 is - the verdict code of ``--strict`` gates and ``eval judge-validity``). + An :class:`~dokimi.utils.InputError`: the CLI reports it as ``Error: ...`` with + exit 2, the code for bad input (1 is the verdict code of ``--strict`` gates and + ``eval judge-validity``). """ diff --git a/dokimi/dokimi/novelty/artifact.py b/dokimi/dokimi/novelty/artifact.py index 463c8c899..133d5dbb7 100644 --- a/dokimi/dokimi/novelty/artifact.py +++ b/dokimi/dokimi/novelty/artifact.py @@ -11,7 +11,8 @@ from __future__ import annotations from pathlib import Path -from dokimi.exit_codes import usage_exit + +from dokimi.utils import InputError def load_artifact_text(ref: str) -> tuple[str, str, str, str, str]: @@ -40,9 +41,10 @@ def load_artifact_text(ref: str) -> tuple[str, str, str, str, str]: return "text", path.stem, "", "", path.read_text(encoding="utf-8") -def load_artifact_or_exit(ref: str) -> tuple[str, str, str, str, str]: - """:func:`load_artifact_text`, or exit naming ``ref`` when it is missing, unreadable or not UTF-8.""" +def load_artifact_ref(ref: str) -> tuple[str, str, str, str, str]: + """:func:`load_artifact_text` for a command's ``--artifact``: an :class:`~dokimi.utils.InputError` + naming ``ref`` when it is missing, unreadable, not UTF-8 or has no frontmatter to read.""" try: return load_artifact_text(ref) except (OSError, ValueError, RecursionError) as exc: - usage_exit(f"cannot load --artifact {ref}: {exc}") + raise InputError(f"cannot load --artifact {ref}: {exc}") from exc diff --git a/dokimi/dokimi/novelty/atoms.py b/dokimi/dokimi/novelty/atoms.py index f4bf27991..dfdee0036 100644 --- a/dokimi/dokimi/novelty/atoms.py +++ b/dokimi/dokimi/novelty/atoms.py @@ -25,9 +25,10 @@ import json from dataclasses import dataclass, field +from dokimi.utils import InputError, read_json_input + from .client import LLMClient, accepting from .spans import Span -from dokimi.exit_codes import usage_exit ATOM_KINDS = ("proposition", "edge", "motif", "contract", "example") ATOM_TAGS = ( @@ -556,11 +557,11 @@ def load_atoms(path) -> tuple[list[Atom], list[Span]]: """Read back what :func:`save_atoms` wrote, with unread spans marked ``unread``. An ``atoms.json`` written before the ``extraction`` record existed marks nothing; it - cannot say whether a span went unread. + cannot say whether a span went unread. Raises :class:`~dokimi.utils.InputError`, see + :func:`load_atoms_record`. """ - from pathlib import Path - - return _atoms_from_record(json.loads(Path(path).read_text(encoding="utf-8"))) + atoms, spans, _record = load_atoms_record(path) + return atoms, spans def _is_int(value) -> bool: @@ -656,18 +657,17 @@ def _check_bundle_shape(bundle) -> None: _check_rows("bundle.excluded", bundle["excluded"], _EXCLUDED_FIELDS) -def load_atoms_or_exit(path) -> tuple[list[Atom], list[Span], dict]: - """:func:`load_atoms` plus the parsed record, for a command. +def load_atoms_record(path) -> tuple[list[Atom], list[Span], dict]: + """The atoms, the spans and the parsed record of an atoms file: the one entry point to read it. - Exits with the file named when it cannot be read or is not an atom set, including a - ``bundle`` whose fields do not have the types its readers use. + Raises :class:`~dokimi.utils.InputError` naming the file when it cannot be read, is not + JSON or UTF-8, or is not an atom set, including a ``bundle`` whose fields do not have the + types its readers use. """ - from pathlib import Path - + data = read_json_input(path) try: - data = json.loads(Path(path).read_text(encoding="utf-8")) atoms, spans = _atoms_from_record(data) _check_bundle_shape(data.get("bundle")) - except (OSError, ValueError, RecursionError) as exc: - usage_exit(f"cannot read atoms from {path}: {exc}") + except ValueError as exc: + raise InputError(f"{path} is not a valid atoms file: {exc}") from exc return atoms, spans, data diff --git a/dokimi/dokimi/novelty/measure.py b/dokimi/dokimi/novelty/measure.py index 0c67b0321..78b19491a 100644 --- a/dokimi/dokimi/novelty/measure.py +++ b/dokimi/dokimi/novelty/measure.py @@ -27,6 +27,7 @@ from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field +from dokimi.utils import InputError from .atoms import Atom from .client import LLMClient, same_client from .probes import Probe @@ -218,9 +219,9 @@ def parse_arms(spec: str) -> tuple[str, ...]: if BASELINE_ARM not in arms: # Without the baseline there is no pair, and without a pair every outcome is a # level -- which is the one thing the judge agreement does not support. - raise ValueError("--arms must include no_artifact: the 2x2 is a paired comparison") + raise InputError("--arms must include no_artifact: the 2x2 is a paired comparison") if "full" not in arms: - raise ValueError("--arms must include full: the headline 2x2 pairs it with no_artifact") + raise InputError("--arms must include full: the headline 2x2 pairs it with no_artifact") return arms @@ -228,15 +229,15 @@ def check_arms(arms: tuple[str, ...], placebo_text: str | None) -> None: """Refuse an arm set that cannot be run, before any model is called. Raises: - ValueError: An unknown arm, a set without ``full``, or ``placebo`` with no text. + InputError: An unknown arm, a set without ``full``, or ``placebo`` with no text. """ unknown = [arm for arm in arms if arm not in ARMS] if unknown: - raise ValueError(f"unknown novelty arms: {unknown}; known: {list(ARMS)}") + raise InputError(f"unknown novelty arms: {unknown}; known: {list(ARMS)}") if "full" not in arms: - raise ValueError("the arms must include full: the headline 2x2 pairs it with no_artifact") + raise InputError("the arms must include full: the headline 2x2 pairs it with no_artifact") if "placebo" in arms and not placebo_text: - raise ValueError("the placebo arm needs --placebo-text (domain-mismatched text)") + raise InputError("the placebo arm needs --placebo-text (domain-mismatched text)") def run_measurement( diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 021ba4475..a6d44f6ef 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -41,7 +41,7 @@ from pathlib import Path from dokimi.skills import Skill -from dokimi.utils import parse_frontmatter +from dokimi.utils import InputError, parse_frontmatter from .client import LLMClient, parse_json_object @@ -462,7 +462,7 @@ def disables_model_invocation(skill_md: Path) -> bool: "1", "true", "yes", "on") -class RoutingInputError(ValueError): +class RoutingInputError(InputError): """A competitor field or query file in which selection is not a measurement.""" diff --git a/dokimi/dokimi/records.py b/dokimi/dokimi/records.py index 92678d6a2..11ca283c9 100644 --- a/dokimi/dokimi/records.py +++ b/dokimi/dokimi/records.py @@ -43,7 +43,7 @@ written as and an aggregate stays the ``int`` or ``float`` it was written as, so a report that prints them prints what the file says. -Errors: :class:`RecordError` (a ``ValueError``) for a missing or unparseable file, an +Errors: :class:`RecordError` (an :class:`~dokimi.utils.InputError`, so the CLI exits 2 with its message) for a missing or unparseable file, an artifact with no record list, a record that is not an object, has no ``question_id`` or has one that is neither a string nor an integer, a condition payload that is neither an object nor ``null``, an ``arms``/``scores`` field that is not an object, an ``expected_skill`` that is not a @@ -54,13 +54,14 @@ from __future__ import annotations -import json import math from collections.abc import Iterable, Iterator, Mapping, Sequence from dataclasses import dataclass, field from pathlib import Path from typing import Any, Optional, Union +from dokimi.utils import InputError, read_json_input + #: The binary context arm, under its persisted key. CONTEXT = "with_docs" #: The binary baseline, under its persisted key. @@ -74,8 +75,8 @@ QuestionId = Union[str, int] -class RecordError(ValueError): - """An artifact or record that is not one of the shapes this module reads.""" +class RecordError(InputError): + """An artifact or record that is not one of the shapes this module reads (an :class:`InputError`).""" @dataclass(frozen=True) @@ -219,13 +220,9 @@ def __iter__(self) -> Iterator[Record]: def load(path: Union[str, Path], *, section: Optional[str] = None) -> Run: """Read an artifact file; every failure is a :class:`RecordError` naming ``path``.""" try: - data = json.loads(Path(path).read_text(encoding="utf-8")) - except FileNotFoundError: - raise RecordError(f"file not found: {path}") from None - except (OSError, UnicodeDecodeError) as exc: - raise RecordError(f"cannot read {path}: {exc}") from None - except json.JSONDecodeError as exc: - raise RecordError(f"invalid JSON in {path}: {exc}") from None + data = read_json_input(path) + except InputError as exc: + raise RecordError(str(exc)) from exc return read(data, source=str(path), section=section) diff --git a/dokimi/dokimi/report/model_compare.py b/dokimi/dokimi/report/model_compare.py index cc0f20fa0..0bb589341 100644 --- a/dokimi/dokimi/report/model_compare.py +++ b/dokimi/dokimi/report/model_compare.py @@ -7,7 +7,7 @@ ---------- check_run_consistency(runs, group_name, treatment_arm=None) Validate a list of (run_id, run_data) pairs before comparison. - Hard errors exit 2 (a usage refusal: the runs cannot be compared); returns a list of warning strings. + Raises InputError on hard errors; returns a list of warning strings. non_baseline_arms(run_data) The arm names in one artifact other than its baseline. @@ -30,7 +30,6 @@ from __future__ import annotations -import json import math import statistics from collections import defaultdict @@ -38,10 +37,10 @@ from pathlib import Path from typing import Any -from dokimi.records import Record, RecordError, arm_answers, arm_names, arm_scores, records +from dokimi.records import Record, arm_answers, arm_names, arm_scores, records from dokimi.eval.judge_context import PROTOCOL_MIXED as JUDGE_CONTEXT_MIXED from dokimi.eval import stats as arm_stats -from dokimi.exit_codes import usage_exit +from dokimi.utils import InputError, read_json_input #: Mirrors :data:`dokimi.eval.stats.HAS_SCIPY` — the hypothesis tests #: now live there, so probing for SciPy twice could only ever disagree. Still a @@ -62,21 +61,12 @@ def load_run(path: str) -> dict[str, Any]: - """Load a judged arms JSON file. Raises SystemExit with a clear message on failure.""" - try: - data = json.loads(Path(path).read_text(encoding="utf-8")) - except FileNotFoundError: - usage_exit(f"Error: File not found: {path}") - except OSError as exc: # permission denied, a directory, an I/O error - usage_exit(f"Error: cannot read {path}: {exc.strerror or exc}") - except ValueError as exc: # JSONDecodeError, or bytes that are not UTF-8 - usage_exit(f"Error: Invalid JSON in {path}: {exc}") - if isinstance(data, dict): - for section in ("answers", "evaluations"): - try: - records(data.get(section) or [], source=f"{path} {section}", kind=section) - except RecordError as exc: - usage_exit(f"Error: {exc}") + """Load a judged arms JSON file, an :class:`~dokimi.utils.InputError` naming it when it is not one.""" + data = read_json_input(path) + if not isinstance(data, dict): + raise InputError(f"{path} is not a judged arms file: expected a JSON object, got {type(data).__name__}") + for section in ("answers", "evaluations"): + records(data.get(section) or [], source=f"{path} {section}", kind=section) return data @@ -103,11 +93,8 @@ def normalize_difficulty(difficulty: Any) -> str: def _section(run_data: dict, section: str) -> list[Record]: - """The ``answers`` or ``evaluations`` records of *run_data*, or a clean exit.""" - try: - return records(run_data.get(section) or [], source=section, kind=section) - except RecordError as exc: - usage_exit(f"Error: {exc}") + """The ``answers`` or ``evaluations`` records of *run_data*; a RecordError (an InputError) if malformed.""" + return records(run_data.get(section) or [], source=section, kind=section) def non_baseline_arms(run_data: dict) -> set[str]: @@ -295,16 +282,16 @@ def _check_judge_context(runs: list[tuple[str, dict]], group_name: str) -> None: stamps = {rid: data.get("judge_context") for rid, data in runs if "evaluations" in data} mixed = sorted(rid for rid, s in stamps.items() if s == JUDGE_CONTEXT_MIXED) if mixed: - usage_exit( - f"Error [{group_name}]: {', '.join(mixed)} judged some answers on the stored " + raise InputError( + f"[{group_name}]: {', '.join(mixed)} judged some answers on the stored " "treatment material and some on report snippets (judge_context " f"{JUDGE_CONTEXT_MIXED!r}); such a run pools with nothing, not even another " "mixed one. Re-run answering and judging together, with unique question ids." ) if len(set(stamps.values())) > 1: detail = ", ".join(f"{rid}={s or ''}" for rid, s in stamps.items()) - usage_exit( - f"Error [{group_name}]: runs were judged under different judge_context " + raise InputError( + f"[{group_name}]: runs were judged under different judge_context " f"protocols ({detail}). The judge saw different evidence for the same " "answers, so the scores cannot be pooled. Re-run answering and judging " "together for the older runs: re-judging an answers file written before " @@ -322,7 +309,7 @@ def check_run_consistency( """Check runs for consistency problems; return warning strings. Hard errors (different ``baseline_arm``, different treatment arms, or - duplicate configurations without ``pool_replicates``) exit 2 through ``usage_exit`` + duplicate configurations without ``pool_replicates``) raise ``InputError`` immediately. Soft warnings (hash mismatch) are collected and returned. """ if len(runs) < 2: @@ -335,8 +322,8 @@ def check_run_consistency( unique_baselines = set(baseline_arms.values()) if len(unique_baselines) > 1: detail = ", ".join(f"{rid}={arm}" for rid, arm in baseline_arms.items()) - usage_exit( - f"Error [{group_name}]: runs have different baseline_arm values ({detail}). " + raise InputError( + f"[{group_name}]: runs have different baseline_arm values ({detail}). " "Comparing runs with different baselines produces meaningless deltas." ) @@ -369,8 +356,8 @@ def check_run_consistency( f"{rid}={'/'.join(sorted(arms)) if arms else ''}" for rid, arms in per_run.items() ) - usage_exit( - f"Error [{group_name}]: runs are scored on {len(distinct)} different " + raise InputError( + f"[{group_name}]: runs are scored on {len(distinct)} different " f"treatment arms ({detail}). Each run's delta would measure a different " "treatment, so the comparison is confounded. Compare runs that share one " "treatment, or pass --treatment-arm to pin the arm every run must be " @@ -395,8 +382,8 @@ def check_run_consistency( detail = ", ".join( f"{rid}={'/'.join(sorted(d)[:1])[:19]}…" for rid, d in by_run.items() if d ) - usage_exit( - f"Error [{group_name}]: arm '{shared_arm}' carries " + raise InputError( + f"[{group_name}]: arm '{shared_arm}' carries " f"{len(all_digests)} different prompt digests across runs ({detail}). " "The arm name comes from the profile's frontmatter id, so an edited " "profile keeps its name: these runs measured different prompts under one " @@ -436,8 +423,8 @@ def check_run_consistency( f"{key[0]} ({key[1]}) judged by {key[2] or 'n/a'} -> runs {', '.join(ids)}" for key, ids in dupes.items() ) - usage_exit( - f"Error [{group_name}]: {len(dupes)} configuration(s) appear more than " + raise InputError( + f"[{group_name}]: {len(dupes)} configuration(s) appear more than " f"once ({detail}). These are replicates of one measurement, not rival " "models; ranking them against each other reports sampling noise as a " "model difference. Pass --pool-replicates to average them into one " @@ -464,9 +451,9 @@ def extract_scores( Parsed arms JSON artifact (``arms.v1`` schema or legacy format). force_treatment_arm: When given, only scores for this arm are used as the treatment. - Raises ``SystemExit`` if the arm is absent. + Raises ``InputError`` if the arm is absent. When ``None``, the single non-baseline arm is auto-selected; raises - ``SystemExit`` if there is more than one non-baseline arm (ambiguous). + ``InputError`` if there is more than one non-baseline arm (ambiguous). """ scores = [] @@ -488,13 +475,17 @@ def extract_scores( if force_treatment_arm is not None: if force_treatment_arm not in evaluation.conditions: - usage_exit(f"Error: --treatment-arm '{force_treatment_arm}' not found in " - f"question '{qid}'. Available arms: {list(evaluation.conditions)}") + raise InputError( + f"--treatment-arm '{force_treatment_arm}' not found in " + f"question '{qid}'. Available arms: {list(evaluation.conditions)}" + ) target_arm: str | None = force_treatment_arm else: if len(treatment_arms) > 1: - usage_exit(f"Error: run has multiple non-baseline arms {treatment_arms} in " - f"question '{qid}'. Use --treatment-arm to select one explicitly.") + raise InputError( + f"run has multiple non-baseline arms {treatment_arms} in " + f"question '{qid}'. Use --treatment-arm to select one explicitly." + ) target_arm = treatment_arms[0] if treatment_arms else None if target_arm is not None and target_arm in evaluation.scores: @@ -534,13 +525,17 @@ def extract_scores( if force_treatment_arm is not None: if force_treatment_arm not in arms: - usage_exit(f"Error: --treatment-arm '{force_treatment_arm}' not found in " - f"question '{qid}'. Available arms: {list(arms)}") + raise InputError( + f"--treatment-arm '{force_treatment_arm}' not found in " + f"question '{qid}'. Available arms: {list(arms)}" + ) treatment_arm = force_treatment_arm else: if len(treatment_arms) > 1: - usage_exit(f"Error: run has multiple non-baseline arms {treatment_arms} in " - f"question '{qid}'. Use --treatment-arm to select one explicitly.") + raise InputError( + f"run has multiple non-baseline arms {treatment_arms} in " + f"question '{qid}'. Use --treatment-arm to select one explicitly." + ) treatment_arm = treatment_arms[0] if treatment_arms else None # The inline ``evaluation.aggregate_score`` of each arm (records.py). diff --git a/dokimi/dokimi/runner/compare.py b/dokimi/dokimi/runner/compare.py index 8750471d7..565b5fba3 100644 --- a/dokimi/dokimi/runner/compare.py +++ b/dokimi/dokimi/runner/compare.py @@ -11,12 +11,13 @@ from typing import Any from dokimi.gate.regression import detect_regressions +from dokimi.utils import InputError _REQUIRED_SUMMARY_KEYS = {"docs", "score", "coverage", "freshness_lite", "readability"} -class CompareInputError(ValueError): +class CompareInputError(InputError): """A snapshot or spec that ``compare`` cannot read or that has the wrong shape.""" diff --git a/dokimi/dokimi/utils.py b/dokimi/dokimi/utils.py index 56cacecae..4e92eb485 100644 --- a/dokimi/dokimi/utils.py +++ b/dokimi/dokimi/utils.py @@ -8,7 +8,6 @@ from typing import Tuple, Dict, Any from dokimi.defaults import bedrock_profile -from dokimi.exit_codes import EXIT_USAGE def parse_frontmatter(text: str) -> "Tuple[Dict[str, Any], str]": @@ -291,25 +290,31 @@ def question_id(q: Dict[str, Any]) -> Any: return None -class InputError(Exception): - """A user-supplied input file that cannot be used; ``cli.main`` prints it and exits 2.""" +class InputError(ValueError): + """A user-supplied input that cannot be used: missing, unreadable, not UTF-8 or JSON, or the wrong shape. + Library loaders raise it (or a subclass) with a message that names the file, and do not + print or exit. ``cli.main`` prints ``Error: `` and exits 2, the code for bad input; + exit 1 stays the verdict of ``--strict`` gates and ``eval judge-validity``. A ``ValueError``, + so code that already catches bad values keeps catching these. + """ -def read_json_input(path: str | os.PathLike[str], *, flag: str) -> Any: - """The parsed JSON at ``path``, or raises :class:`InputError` naming ``flag`` and the file.""" - try: - return json.loads(Path(path).read_text(encoding="utf-8")) - except (OSError, ValueError, RecursionError) as exc: # JSONDecodeError and bad UTF-8 are ValueErrors - reason = exc.strerror if isinstance(exc, OSError) and exc.strerror else exc - raise InputError(f"{flag} {path}: {reason}") from exc +def read_json_input(path: str | os.PathLike[str], *, flag: str | None = None) -> Any: + """The parsed JSON at ``path``, or :class:`InputError` naming the file and the reason. -def write_text_or_exit(path: Path, text: str, code: int = EXIT_USAGE) -> None: - """Write ``text`` to ``path`` (creating parents), or exit ``code`` naming ``path`` when it cannot be written.""" - import sys + Covers a missing or unreadable file, bytes that are not UTF-8, text that is not JSON and + JSON nested too deeply to parse. With ``flag`` (the option the file was given to) the + message is that option's own, `` : ``. + """ try: - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8") + return json.loads(Path(path).read_text(encoding="utf-8")) except OSError as exc: - print(f"Error: cannot write {path}: {exc}", file=sys.stderr) - sys.exit(code) + reason = exc.strerror or exc + raise InputError(f"{flag} {path}: {reason}" if flag else f"cannot read {path}: {reason}") from exc + except UnicodeDecodeError as exc: + raise InputError(f"{flag} {path}: {exc}" if flag else f"{path} is not UTF-8 text: {exc}") from exc + except (json.JSONDecodeError, RecursionError) as exc: # deep nesting is named as bad JSON, as on main + raise InputError(f"{flag} {path}: {exc}" if flag else f"{path} is not valid JSON: {exc}") from exc + except ValueError as exc: # e.g. "embedded null byte" in the path: pathlib raises a bare ValueError + raise InputError(f"{flag} {path}: {exc}" if flag else f"cannot read {path}: {exc}") from exc diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index b0b75b387..ab21668b6 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -1,10 +1,10 @@ { "files": { - "dokimi/commands/novelty.py": {"lines": 1189, "rationale": "estimate-only projection and W1 routing cost output (#164) from main, plus the #172 routing field, --all-vs-all, tied/unscored caveat and blind-skill warning, --repeats refused under --all-vs-all; file split tracked separately"}, + "dokimi/commands/novelty.py": {"lines": 1098, "rationale": "estimate-only projection and W1 routing cost output (#164) from main, plus the #172 routing field, --all-vs-all, tied/unscored caveat and blind-skill warning, --repeats refused under --all-vs-all; file split tracked separately"}, "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 899, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1093, "rationale": "grew on main in #180 (replay a stored retry) after #93 seeded this baseline; split it in its own PR"}, - "dokimi/report/model_compare.py": {"lines": 1199, "rationale": "grew on main in #73 (judge-validity --context stored) before the ratchet landed; split it in its own PR"}, + "dokimi/report/model_compare.py": {"lines": 1194, "rationale": "grew on main in #73 (judge-validity --context stored) before the ratchet landed; split it in its own PR"}, "tests/test_arm_runner.py": {"lines": 1175, "rationale": "pre-existing when the gate was introduced"}, "tests/test_benchflow_harness.py": {"lines": 1716, "rationale": "pre-existing when the gate was introduced"}, "tests/test_container_agent.py": {"lines": 1235, "rationale": "pre-existing when the gate was introduced"}, @@ -21,7 +21,6 @@ "dokimi/commands/catalog.py::cmd_catalog_trigger_set": {"complexity": 14}, "dokimi/commands/evaluate.py::cmd_eval_judge_validity": {"complexity": 29, "rationale": "grew on main in #73 (judge-validity --context stored) from 25 to 29 before the ratchet landed; split it in its own PR"}, "dokimi/commands/judge_reliability.py::cmd_eval_cross_judge": {"complexity": 30, "rationale": "pre-existing when the gate was introduced"}, - "dokimi/commands/novelty.py::cmd_novelty_routing": {"complexity": 13, "rationale": "W8 adds the --repeats guard after main's no_queries handling raised the file; the command is split with the rest of novelty.py in a separate PR"}, "dokimi/commands/repo.py::cmd_repo_scan": {"complexity": 12}, "dokimi/commands/repo_run.py::_context": {"complexity": 11, "rationale": "went over the cap on main in #120 (built-in review graders) after #93 seeded this baseline; split it in its own PR"}, "dokimi/commands/report.py::cmd_report_model_compare": {"complexity": 12}, diff --git a/dokimi/tests/exit_site_scan.py b/dokimi/tests/exit_site_scan.py new file mode 100644 index 000000000..81b61b165 --- /dev/null +++ b/dokimi/tests/exit_site_scan.py @@ -0,0 +1,491 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Which lines of one module end the process: the AST scan behind the exit-site rule. + +Used by tests/test_import_architecture.py and tests/test_exit_sites.py. No import is run. +""" + +from __future__ import annotations + +import ast + + +#: The exit helper: its own raise is not counted, a call to it is (like SPAWN_WRAPPER). +EXIT_HELPER = "dokimi.exit_codes" + +_EXIT_CALLS = {("sys", "exit"), ("os", "_exit"), ("builtins", "exit"), ("builtins", "quit"), + (EXIT_HELPER, "usage_exit")} +_EXIT_MODULES = {m for m, _ in _EXIT_CALLS} + + +_COMPREHENSIONS = (ast.ListComp, ast.SetComp, ast.DictComp, ast.GeneratorExp) +_SCOPE_NODES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda, ast.ClassDef, *_COMPREHENSIONS) +_FUNC_NODES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda) + + +def _header_roots(node: ast.AST) -> list[ast.AST]: + """The parts of a def, class or comprehension that Python evaluates in the enclosing scope.""" + if isinstance(node, ast.ClassDef): + return [*node.bases, *node.keywords, *node.decorator_list] + if isinstance(node, _COMPREHENSIONS): + return [node.generators[0].iter] # only the outermost iterable + args = node.args + named = [*args.posonlyargs, *args.args, *args.kwonlyargs, args.vararg, args.kwarg] + roots = [*args.defaults, *(d for d in args.kw_defaults if d is not None)] + roots += [a.annotation for a in named if a is not None and a.annotation is not None] + if not isinstance(node, ast.Lambda): + roots += [*node.decorator_list, *([node.returns] if node.returns is not None else [])] + return roots + + +def _own_nodes(root: ast.AST) -> list[ast.AST]: + """``root`` and its descendants up to a nested scope, which is listed but not entered.""" + found, stack = [], [root] + while stack: + node = stack.pop() + found.append(node) + if not isinstance(node, _SCOPE_NODES): + stack.extend(ast.iter_child_nodes(node)) + return found + + +def _scope_owners(tree: ast.Module) -> dict[int, ast.AST]: + """``id(node) -> `` the function, lambda, class, comprehension or module whose names ``node`` binds or uses.""" + owner: dict[int, ast.AST] = {} + stack: list[tuple[ast.AST, ast.AST]] = [(tree, tree)] + while stack: + node, scope = stack.pop() + for child in ast.iter_child_nodes(node): + owner[id(child)] = scope + stack.append((child, child if isinstance(child, _SCOPE_NODES) else scope)) + for scope in ast.walk(tree): # outer scopes first, so a header is moved from its final owner + if isinstance(scope, _SCOPE_NODES): + for root in _header_roots(scope): + for node in _own_nodes(root): + owner[id(node)] = owner[id(scope)] + return owner + + +#: ``(id of the scope, name) -> [(pos, kind, payload)]``; ``pos`` is where the binding takes effect. +#: The kinds and their payloads are described in the docstring of ``_bind``. +ExitTable = dict[tuple[int, str], list[tuple[tuple[int, int], str, object]]] +#: A binding moved to another scope runs when its function is called, after any statement of the module. +_LATER = (10**9, 0) + + +def _at(node: ast.AST) -> tuple[int, int]: + return (node.lineno, node.col_offset) + + +def _after(node: ast.AST) -> tuple[int, int]: + """Where the bindings of a statement or a name take effect: after its value is evaluated.""" + return (node.end_lineno, node.end_col_offset) + + +def _bind(table: ExitTable, scope: ast.AST, name: str, pos: tuple[int, int], kind: str, payload: object = None) -> None: + """Record a binding of ``name`` in ``scope`` at ``pos``. + + Kinds: ``mod`` (payload: modules), ``del`` (the name is unbound again), ``fn`` (payload: ``(module, attr)`` of an exit callable), ``sysexit`` + (SystemExit, a name for it, or an instance), ``other`` (any other binding, shadowing outer names), and + ``global`` / ``nonlocal`` (payload: the scope the name is declared in, where it is bound). + """ + table.setdefault((id(scope), name), []).append((pos, kind, payload)) + + +def _import_bindings(node: ast.Import | ast.ImportFrom, scope: ast.AST, table: ExitTable) -> None: + pos = _after(node) + if isinstance(node, ast.Import): # ``import sys as s`` binds s; ``import dokimi.exit_codes`` binds the root dokimi + for a in node.names: + name, module = (a.asname, a.name) if a.asname else (a.name.split(".")[0],) * 2 + _bind(table, scope, name, pos, "mod", frozenset({module})) + return + # ``from .exit_codes import x`` and ``from . import exit_codes`` name dokimi.exit_codes + module = node.module + if node.level: + module = {"exit_codes": EXIT_HELPER, None: "dokimi"}.get(node.module) + for a in node.names: + name = a.asname or a.name + if (module, a.name) in _EXIT_CALLS: # ``from sys import exit as bye`` + _bind(table, scope, name, pos, "fn", (module, a.name)) + elif f"{module}.{a.name}" in _EXIT_MODULES: # ``from dokimi import exit_codes as ec`` + _bind(table, scope, name, pos, "mod", frozenset({f"{module}.{a.name}"})) + elif module == "builtins" and a.name == "SystemExit" and not node.level: # ``from builtins import SystemExit as Stop`` + _bind(table, scope, name, pos, "sysexit") + else: + _bind(table, scope, name, pos, "other") + + +def _params(args: ast.arguments) -> list[str]: + named = [*args.posonlyargs, *args.args, *args.kwonlyargs, args.vararg, args.kwarg] + return [a.arg for a in named if a is not None] + + +def _late_targets(tree: ast.Module) -> dict[int, tuple[int, int]]: + """``id(name) -> pos`` for the names a for, with or augmented assignment binds: after the iterable, context or value.""" + late: dict[int, tuple[int, int]] = {} + + def mark(target: ast.AST, pos: tuple[int, int]) -> None: + late.update((id(m), pos) for m in ast.walk(target) if isinstance(m, ast.Name)) + + for n in ast.walk(tree): + if isinstance(n, (ast.For, ast.AsyncFor)): + mark(n.target, _after(n.iter)) + elif isinstance(n, (ast.With, ast.AsyncWith)): + for item in n.items: + if item.optional_vars is not None: + mark(item.optional_vars, _after(item.context_expr)) + elif isinstance(n, ast.AugAssign): + mark(n.target, _after(n.value)) + return late + + +def _capture_bindings(tree: ast.Module, owner: dict[int, ast.AST], table: ExitTable) -> None: + """``case x:``, ``case [*x]:`` and ``case {**x}:`` bind x once the pattern has matched.""" + for n in ast.walk(tree): + if isinstance(n, (ast.MatchAs, ast.MatchStar, ast.MatchMapping)): + name = n.rest if isinstance(n, ast.MatchMapping) else n.name + if name: + _bind(table, owner[id(n)], name, _after(n), "other") + + +def _shadowing_bindings(tree: ast.Module, owner: dict[int, ast.AST], table: ExitTable, assigned: set[int]) -> None: + """Every binding that is not a plain assignment of one name: imports, defs, parameters, loop, with and except targets.""" + bare = {id(n.target) for n in ast.walk(tree) if isinstance(n, ast.AnnAssign) and n.value is None} + late = _late_targets(tree) + for n in ast.walk(tree): + if isinstance(n, (ast.Import, ast.ImportFrom)): + _import_bindings(n, owner[id(n)], table) + elif isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + # a def binds after its header, which the body follows; a class binds after its body has run + at = _after(n) if isinstance(n, ast.ClassDef) else _at(n.body[0]) + _bind(table, owner[id(n)], n.name, at, "other") + elif isinstance(n, ast.ExceptHandler) and n.name: + _bind(table, owner[id(n)], n.name, _at(n.body[0]), "other") + _bind(table, owner[id(n)], n.name, _after(n), "del") # Python clears the target when the handler ends + elif isinstance(n, ast.Name) and isinstance(n.ctx, ast.Del): + _bind(table, owner[id(n)], n.id, _after(n), "del") + elif isinstance(n, ast.Name) and isinstance(n.ctx, ast.Store) and id(n) not in assigned: + # ``x: int`` binds nothing at run time outside a function, where it makes x local + if id(n) not in bare or isinstance(owner[id(n)], _FUNC_NODES): + _bind(table, owner[id(n)], n.id, late.get(id(n), _after(n)), "other") + if isinstance(n, _FUNC_NODES): # a parameter is local to its function, bound on entry + for name in _params(n.args): + _bind(table, n, name, _at(n), "other") + + +def _target_scope(scope: ast.AST, name: str, table: ExitTable) -> ast.AST: + """The scope an assignment to ``name`` in ``scope`` binds: where ``global`` or ``nonlocal`` declared it, else ``scope``.""" + for b in table.get((id(scope), name), []): + if b[1] in ("global", "nonlocal"): + return b[2] + return scope + + +#: What a name that nothing binds means: the builtin SystemExit, exit or quit, or the module sys or os. +_BUILTIN_NAMES = { + "SystemExit": ("sysexit", None), + "exit": ("fn", ("builtins", "exit")), + "quit": ("fn", ("builtins", "quit")), + "sys": ("mod", frozenset({"sys"})), + "os": ("mod", frozenset({"os"})), +} + + +def _name_bindings(name: str, scope: ast.AST, pos: tuple[int, int], table: ExitTable, + owner: dict[int, ast.AST]) -> list[tuple[str, object]]: + """The entries of ``x = name``: one for each binding that ``name`` may denote at ``pos``.""" + out: list[tuple[str, object]] = [] + for b in _candidates(name, scope, pos, table, owner): + if b is None: # the builtin of that name, where nothing binds it + out.append(_BUILTIN_NAMES.get(name, ("other", None))) + elif b[1] in ("sysexit", "fn", "mod"): + out.append((b[1], b[2])) + else: + out.append(("other", None)) + return out + + +def _value_bindings(expr: ast.expr, scope: ast.AST, pos: tuple[int, int], table: ExitTable, + owner: dict[int, ast.AST]) -> list[tuple[str, object]]: + """The (kind, payload) entries that ``x = expr`` gives x, one for each thing the value may be. + + SystemExit, an exit callable, a module, or other. A name may denote several bindings at once (a + function reads an alias after any later rebinding of its source), so each one it may denote is kept. + """ + if isinstance(expr, ast.Name): # ``stop = exit_alias`` copies every binding the name may denote + out = _name_bindings(expr.id, scope, pos, table, owner) + else: + out = [("sysexit", None)] if _is_sysexit_expr(expr, scope, pos, table, owner) else [] + if isinstance(expr, ast.Attribute): + mods = _modules(ast.unparse(expr), scope, pos, table, owner) # ``ec = dokimi.exit_codes`` + if mods: + out.append(("mod", frozenset(mods))) + for m in _modules(ast.unparse(expr.value), scope, pos, table, owner): # ``stop = sys.exit`` + if (m, expr.attr) in _EXIT_CALLS: + out.append(("fn", (m, expr.attr))) + return out or [("other", None)] + + +def _pairs(target: ast.expr, value: ast.expr) -> list[tuple[ast.Name, ast.expr]]: + """The (name, value) pairs of ``target = value`` where both unpack literally: ``a, (b, c) = x, (y, z)``. + + A target that does not unpack against a literal of the same shape gives no pairs; its names bind as other names. + """ + if isinstance(target, ast.Name): + return [(target, value)] + if not (isinstance(target, (ast.Tuple, ast.List)) and isinstance(value, (ast.Tuple, ast.List))): + return [] + if len(target.elts) != len(value.elts) or any(isinstance(e, ast.Starred) for e in [*target.elts, *value.elts]): + return [] + return [pair for t, v in zip(target.elts, value.elts, strict=True) for pair in _pairs(t, v)] + + +def _assigned(assigns: list[tuple[ast.AST, list[ast.expr]]], owner: dict[int, ast.AST], table: ExitTable): + """The ``(key, entry)`` each assignment makes against ``table``, in source order.""" + out = [] + for n, targets in sorted(assigns, key=lambda p: _after(p[0])): + scope = owner[id(n)] + if isinstance(n, ast.NamedExpr): # a walrus binds in the nearest enclosing def, lambda, class or module + while isinstance(scope, _COMPREHENSIONS): + scope = owner[id(scope)] + for t in targets: + for name, expr in _pairs(t, n.value): + target = _target_scope(scope, name.id, table) + pos = _after(n) if target is scope else _LATER + for kind, payload in _value_bindings(expr, owner[id(n)], _at(expr), table, owner): + out.append(((id(target), name.id), (pos, kind, payload))) + return out + + +def _assignment_bindings(tree: ast.Module, owner: dict[int, ast.AST], table: ExitTable) -> None: + """``x = value`` and ``(x := value)`` bind x to what the value is, once the value is evaluated. + + A value can name a binding that another assignment makes later in the source (a function may run after it), + so the entries are recomputed until they settle: each pass reads the entries of the pass before it, then replaces them. + """ + assigns: list[tuple[ast.AST, list[ast.expr]]] = [] + for n in ast.walk(tree): + if isinstance(n, ast.Assign): + assigns.append((n, n.targets)) + elif isinstance(n, (ast.AnnAssign, ast.NamedExpr)) and n.value is not None: + assigns.append((n, [n.target])) + entries: list[tuple[tuple[int, str], tuple]] = [] + for _ in range(len(assigns) + 1): + fresh = _assigned(assigns, owner, table) + for key, entry in entries: + table[key].remove(entry) + for key, entry in fresh: + table.setdefault(key, []).append(entry) + if fresh == entries: + break + entries = fresh + + +def _declared_scopes(tree: ast.Module, owner: dict[int, ast.AST], table: ExitTable, kind: str, + assigns: frozenset | set = frozenset()) -> None: + """``global x`` declares x in the module and ``nonlocal x`` in the nearest enclosing scope that binds it. + + The bindings made in the declaring scope move to that scope, where they run when the function is called. The + declaration stays in the scope, so that its own uses resolve there. ``kind`` is ``global`` or ``nonlocal``; + ``assigns`` holds the ``(scope id, name)`` that assignments bind, which also make a nonlocal binder. + """ + for n in ast.walk(tree): + if not isinstance(n, (ast.Global, ast.Nonlocal)) or (kind == "global") != isinstance(n, ast.Global): + continue + scope = owner[id(n)] + for name in n.names: + target = tree if kind == "global" else _binder_above(scope, name, table, owner, assigns) + if target is None or target is scope: + continue + # the declarations already in the scope stay there; only the bindings made in it move to the target + held = table.pop((id(scope), name), []) + table.setdefault((id(target), name), []).extend( + (_LATER, k, p) for _, k, p in held if k not in ("global", "nonlocal")) + table.setdefault((id(scope), name), []).extend( + b for b in held if b[1] in ("global", "nonlocal")) + table[(id(scope), name)].append((_at(n), kind, target)) + + +def _binder_above(scope: ast.AST, name: str, table: ExitTable, owner: dict[int, ast.AST], assigns: set) -> ast.AST | None: + outer = owner.get(id(scope)) + while outer is not None: + # a class body is no binder for nonlocal: the functions inside it do not see its names + # a scope that only declares the name (global or nonlocal) binds nothing: its bindings went elsewhere + found = table.get((id(outer), name), []) + if not isinstance(outer, ast.ClassDef) and (any(b[1] not in ("global", "nonlocal") for b in found) or ( + (id(outer), name) in assigns and not any(b[1] in ("global", "nonlocal") for b in found))): + return outer + outer = owner.get(id(outer)) + return None + + +def _exit_table(tree: ast.Module, owner: dict[int, ast.AST]) -> ExitTable: + # the targets of an assignment with a value; ``x: int`` alone binds x (local to its function) as any other name + assigned: set[int] = set() + assigned_names: list[ast.Name] = [] + for n in ast.walk(tree): + if isinstance(n, ast.Assign): + targets = n.targets + elif isinstance(n, (ast.AnnAssign, ast.NamedExpr)) and n.value is not None: + targets = [n.target] + else: + continue + value = n.value + pairs = [name for t in targets for name, _ in _pairs(t, value)] + assigned |= {id(name) for name in pairs} + assigned_names += pairs + table: ExitTable = {} + _shadowing_bindings(tree, owner, table, assigned) + _capture_bindings(tree, owner, table) + _declared_scopes(tree, owner, table, "global") + _declared_scopes(tree, owner, table, "nonlocal", {(id(owner[id(name)]), name.id) for name in assigned_names}) + _assignment_bindings(tree, owner, table) + return table + + +def _live(b, scope: ast.AST): + """The binding a lookup reads, or None (the builtin) for a ``del`` at module level. + + A ``del`` in a function leaves its local unbound: reading it raises, so it is never the builtin. A ``del`` in a + class body is not a live binding either: ``_candidates`` reads the enclosing scope instead. + """ + if b[1] != "del": + return b + return (b[0], "other", None) if isinstance(scope, _FUNC_NODES) else None + + +def _read_in_scope(scope: ast.AST, use: ast.AST, pos: tuple[int, int], found: list, here: bool) -> list | None: + """What a lookup of a name that ``scope`` binds (``found``) reads there, or None when no binding is live at ``pos``.""" + if not here: # a function may run later: each exit-kind binding it can see counts + exits = [b for b in found if b[1] != "other"] or found + return [_live(b, scope) for b in sorted(exits, key=lambda b: b[0])] + before = [b for b in found if b[0] <= pos] + if before: # every binding of the last statement that bound it: an alias may copy several + last = max(b[0] for b in before) + live = [b for b in before if b[0] == last] + if isinstance(scope, ast.ClassDef): # a class body's del unbinds its namespace: read the enclosing scope + live = [b for b in live if b[1] != "del"] + if live: + return [_live(b, scope) for b in live] + if scope is use and isinstance(use, _FUNC_NODES + _COMPREHENSIONS): # a function's local, not bound yet + return [(pos, "other", None)] + return None + + +def _candidates(name: str, use: ast.AST, pos: tuple[int, int], table: ExitTable, owner: dict[int, ast.AST]) -> list: + """The bindings that ``name`` may denote at ``pos`` in scope ``use``; a None in the list is the builtin. + + Python's scoping: the innermost scope that binds the name wins, unless it declares the name ``global`` or + ``nonlocal``, which sends the lookup to the scope that binds it. In the use's own scope the last binding at + or before ``pos`` decides, and a binding only after it leaves the name local and unbound there; a class body + then reads the enclosing names, by position, as does a comprehension. Beyond a function, lambda or generator + an enclosing scope may run after any rebinding, so every exit-kind binding of it is a candidate. A class body + is seen only from its own statements, not from the functions inside it. A module-level name read before its + first binding is the builtin of that name, since it is then unbound. A class body's del or cleared except target + leaves the name to the enclosing scope, as LOAD_NAME does, not to the builtin. + """ + scope, here = use, True + while True: + if scope is use or not isinstance(scope, ast.ClassDef): + found = table.get((id(scope), name), []) + declared = [b for b in found if b[1] in ("global", "nonlocal")] + if declared and declared[-1][2] is not scope: + scope, here = declared[-1][2], False + continue + if found: + read = _read_in_scope(scope, use, pos, found, here) + if read is not None: + return read + if id(scope) not in owner: # the module: no enclosing scope + return [None] + # a class body and a list, set or dict comprehension run in place, so the enclosing scope is read by position + here = here and isinstance(scope, (ast.ClassDef, ast.ListComp, ast.SetComp, ast.DictComp)) + scope = owner[id(scope)] + + +def _modules(text: str, scope: ast.AST, pos: tuple[int, int], table: ExitTable, owner: dict[int, ast.AST]) -> set[str]: + """The modules a dotted name may stand for at this use: its import bindings, or sys and os when nothing binds them.""" + root, _, rest = text.partition(".") + mods: set[str] = set() + for b in _candidates(root, scope, pos, table, owner): + if b is None: + mods |= {text} if text in ("sys", "os") else set() + elif b[1] == "mod": + mods |= {f"{m}.{rest}" if rest else m for m in b[2]} + return mods & _EXIT_MODULES if rest else mods # ``sys.exit`` is an attribute, not a module + + +def _is_sysexit_expr(expr: ast.expr, scope: ast.AST, pos: tuple[int, int], table: ExitTable, owner: dict[int, ast.AST]) -> bool: + """Whether ``expr`` is SystemExit, a name bound to it, or an instance of it (a call of either).""" + if isinstance(expr, ast.Call): + return _is_sysexit_expr(expr.func, scope, pos, table, owner) + if isinstance(expr, ast.Name): + return any(expr.id == "SystemExit" if b is None else b[1] == "sysexit" + for b in _candidates(expr.id, scope, pos, table, owner)) + if isinstance(expr, ast.Attribute): # builtins.SystemExit, with ``import builtins`` + return expr.attr == "SystemExit" and "builtins" in _modules(ast.unparse(expr.value), scope, pos, table, owner) + return False + + +def _calls_exit(node: ast.Call, scope: ast.AST, table: ExitTable, owner: dict[int, ast.AST]) -> bool: + func = node.func + pos = _at(node) + if isinstance(func, ast.Name): # exit(), quit(), or a name bound to sys.exit and the like + return any(func.id in ("exit", "quit") if b is None else b[1] == "fn" and b[2] in _EXIT_CALLS + for b in _candidates(func.id, scope, pos, table, owner)) + return isinstance(func, ast.Attribute) and any( + (m, func.attr) in _EXIT_CALLS for m in _modules(ast.unparse(func.value), scope, pos, table, owner)) + + +def _unevaluated(tree: ast.Module, owner: dict[int, ast.AST]) -> set[int]: + """Ids of the annotations Python never runs: a local variable's, in any function (PEP 526), and, under + ``from __future__ import annotations``, every annotation, which is kept as a string.""" + future = any(isinstance(s, ast.ImportFrom) and s.module == "__future__" and any(a.name == "annotations" for a in s.names) + for s in tree.body) + roots: list[ast.AST] = [] + for n in ast.walk(tree): + if future and isinstance(n, _FUNC_NODES): + args = n.args + roots += [a.annotation for a in [*args.posonlyargs, *args.args, *args.kwonlyargs, args.vararg, args.kwarg] + if a is not None and a.annotation is not None] + if getattr(n, "returns", None) is not None: + roots.append(n.returns) + elif isinstance(n, ast.AnnAssign) and (future or isinstance(owner[id(n)], _FUNC_NODES)): + roots.append(n.annotation) + return {id(d) for r in roots for d in ast.walk(r)} + + +def exit_sites(tree: ast.Module) -> list[int]: + """Lines that end the process: an exit call (by module attribute, through an alias, or builtin name) or a raise of SystemExit. + + The ``if __name__ == '__main__':`` block of a script is the process boundary of ``python -m``, so it is + not counted, and neither is an annotation Python does not run: a local variable's, or any under + ``from __future__ import annotations`` (its value is still scanned). Each name + resolves by scope and by source position (see ``_candidates``): an import, a parameter or an assignment in one + function does not change what another function's name means, a method does not see the names of its class + body, and a binding that follows a use on the same line does not count for it. Linear, not control-flow aware: + ``if c: e = SystemExit(2)`` then ``else: e = ValueError()`` is read as the else binding. + + Known gaps, where a real exit is missed: a module-level import followed by a later rebinding such as + ``except ImportError: sys = None`` is read as the rebinding by the code after it; a subclass of SystemExit + (``class E(SystemExit): pass``) is not known as an exit class. + """ + guarded = {id(n) for stmt in tree.body + if isinstance(stmt, ast.If) and ast.unparse(stmt.test) == "__name__ == '__main__'" + for body_stmt in stmt.body for n in ast.walk(body_stmt)} + owner = _scope_owners(tree) + guarded |= _unevaluated(tree, owner) + table = _exit_table(tree, owner) + lines: list[int] = [] + raised: set[int] = set() # the call of a raise is counted once, by the raise + for node in ast.walk(tree): + if id(node) in guarded or id(node) in raised: + continue + if isinstance(node, ast.Raise) and node.exc is not None: + if _is_sysexit_expr(node.exc, owner[id(node)], _at(node.exc), table, owner): + lines.append(node.lineno) + raised.add(id(node.exc)) + elif isinstance(node, ast.Call) and _calls_exit(node, owner[id(node)], table, owner): + lines.append(node.lineno) + return sorted(lines) diff --git a/dokimi/tests/test_eval_coverage.py b/dokimi/tests/test_eval_coverage.py index 19cc90c72..4e5a121d9 100644 --- a/dokimi/tests/test_eval_coverage.py +++ b/dokimi/tests/test_eval_coverage.py @@ -17,6 +17,7 @@ from dokimi.novelty.needs import NeedGraph, NeedNode from dokimi.novelty.needs import load_graph as novelty_load_graph from dokimi.novelty.needs import save_graph +from dokimi.utils import InputError NODES = [ {"id": "n1", "text": "convert with asnumpy before numpy.save", "path_class": "core"}, @@ -142,8 +143,9 @@ def test_graph_with_unknown_path_class_is_rejected(tmp_path): path.write_text(json.dumps({"artifact_name": "x", "nodes": [ {"id": "n1", "text": "t", "path_class": "critical"}]})) - with pytest.raises(ArtifactValidationError, match="path_class"): + with pytest.raises(InputError, match="path_class") as exc: # load_artifact wraps the schema failure load_graph(path) + assert isinstance(exc.value.__cause__, ArtifactValidationError) def test_cli_writes_artifact_and_fails_under_floor(tmp_path, capsys): diff --git a/dokimi/tests/test_eval_output_write_errors.py b/dokimi/tests/test_eval_output_write_errors.py index 0feb5e7ca..bf22dedae 100644 --- a/dokimi/tests/test_eval_output_write_errors.py +++ b/dokimi/tests/test_eval_output_write_errors.py @@ -20,7 +20,7 @@ def _blocked(tmp_path): def test_write_text_or_exit_names_the_path_and_exits_with_the_code(tmp_path, capsys): - from dokimi.utils import write_text_or_exit + from dokimi.commands._output import write_text_or_exit target = _blocked(tmp_path) with pytest.raises(SystemExit) as exc: diff --git a/dokimi/tests/test_exit_sites.py b/dokimi/tests/test_exit_sites.py new file mode 100644 index 000000000..437eb631d --- /dev/null +++ b/dokimi/tests/test_exit_sites.py @@ -0,0 +1,308 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Which lines of a module end the process, as the architecture gate counts them (``exit_sites``). + +Kept apart from test_import_architecture.py, which the growth ratchet caps at 1000 lines. +""" + +from tests.exit_site_scan import exit_sites +from tests.test_import_architecture import _package, exit_problems + + +def test_a_process_exit_outside_the_frontend_fails(tmp_path): + imports = _package(tmp_path, { + "dokimi/cli.py": "import sys\nsys.exit(2)\n", + "dokimi/commands/x.py": "import sys\nsys.exit(1)\n", + "dokimi/core.py": "import sys\n\ndef f():\n sys.exit(3)\n", + "dokimi/b.py": "def g():\n raise SystemExit('bad')\n", + "dokimi/c.py": "import os\nos._exit(0)\nexit(1)\n", + "dokimi/d.py": "import sys\n\nif __name__ == '__main__':\n sys.exit(0)\n", + "dokimi/e.py": "import sys\n\nif __name__ == '__main__':\n pass\nelse:\n sys.exit(1)\n", + }) + problems = exit_problems(imports, {"dokimi.c": (1, "one exit once")}) + assert [p.split()[0] for p in problems] == ["dokimi.b", "dokimi.c", "dokimi.core", "dokimi.e"] + assert "lower EXIT_BASELINE" not in "".join(problems) + + +def test_an_aliased_process_exit_outside_the_frontend_is_counted(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys as s\ns.exit(1)\n", + "dokimi/b.py": "from sys import exit as bye\nbye(1)\n", + "dokimi/c.py": "import os as o\no._exit(0)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [2] + assert exit_sites(imports.trees["dokimi.b"]) == [2] + assert exit_sites(imports.trees["dokimi.c"]) == [2] + + +def test_an_exit_through_the_helper_or_a_bound_raise_outside_the_frontend_fails(tmp_path): + imports = _package(tmp_path, { + "dokimi/exit_codes.py": "def usage_exit(m):\n error = SystemExit(m)\n raise error\n", + "dokimi/commands/x.py": "from dokimi.exit_codes import usage_exit\nusage_exit('x')\n", + "dokimi/a.py": "from dokimi.exit_codes import usage_exit as ue\nue('x')\n", + "dokimi/b.py": "import dokimi.exit_codes\ndokimi.exit_codes.usage_exit('x')\n", + "dokimi/c.py": "from dokimi import exit_codes as ec\nec.usage_exit('x')\n", + "dokimi/d.py": "def f():\n e = SystemExit(2)\n raise e\n", + "dokimi/e.py": "import builtins\nbuiltins.exit(1)\n", + "dokimi/f.py": "import builtins\nraise builtins.SystemExit(2)\n", + "dokimi/g.py": "from builtins import SystemExit as Stop\nraise Stop(2)\n", + "dokimi/h.py": "from builtins import SystemExit as Stop\nerror = Stop(2)\nraise error\n", + "dokimi/i.py": "import builtins\nerror = builtins.SystemExit(2)\nraise error\n", + }) + problems = exit_problems(imports, {}) + assert [p.split()[0] for p in problems] == ["dokimi.a", "dokimi.b", "dokimi.c", "dokimi.d", "dokimi.e", + "dokimi.f", "dokimi.g", "dokimi.h", "dokimi.i"] + assert [p.split(" at lines ")[1].split(";")[0] for p in problems] == ["[2]", "[2]", "[2]", "[3]", "[2]", + "[2]", "[2]", "[3]", "[3]"] + + +def test_relative_helper_imports_and_a_reused_module_alias_are_counted(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "from .exit_codes import usage_exit\nusage_exit('x')\n", + "dokimi/b.py": "from . import exit_codes as ec\nec.usage_exit('x')\n", + "dokimi/c.py": "def f():\n import sys as s\n s.exit(2)\n\ndef g():\n import os as s\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [2] + assert exit_sites(imports.trees["dokimi.b"]) == [2] + assert exit_sites(imports.trees["dokimi.c"]) == [3] + + +def test_annotated_and_scoped_bindings_of_a_raised_exit_are_counted_per_scope(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "error: SystemExit = SystemExit(2)\nraise error\n", + "dokimi/b.py": ("def f():\n error = SystemExit(2)\n return error\n\n" + "def g():\n error = ValueError('bad')\n raise error\n"), + "dokimi/c.py": "error = SystemExit(2)\nerror = ValueError('bad')\nraise error\n", + "dokimi/d.py": "def f():\n error = SystemExit(3)\n raise error\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [2] + assert exit_sites(imports.trees["dokimi.b"]) == [] + assert exit_sites(imports.trees["dokimi.c"]) == [] + assert exit_sites(imports.trees["dokimi.d"]) == [3] + + +def test_a_name_in_one_scope_does_not_change_what_another_scope_means(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "def f():\n import sys as s\n s.exit(2)\n\ndef g(s):\n s.exit()\n", + "dokimi/b.py": ("def f():\n from builtins import SystemExit as Stop\n raise Stop(2)\n\n" + "def g():\n raise Stop(3)\n"), + "dokimi/c.py": "error = SystemExit(2)\n\ndef f(error):\n raise error\n\ndef g():\n raise error\n", + "dokimi/d.py": ("error = SystemExit(2)\n\nclass C:\n error = ValueError('bad')\n\n" + " def f(self):\n raise error\n"), + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [3] + assert exit_sites(imports.trees["dokimi.c"]) == [7] + assert exit_sites(imports.trees["dokimi.d"]) == [7] + + +def test_an_annotation_alone_makes_a_name_local_to_its_function(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "error = SystemExit(2)\n\ndef f():\n error: int\n raise error\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + + +def test_comprehension_targets_and_headers_are_evaluated_where_python_evaluates_them(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\n[x for sys in []]\nsys.exit(2)\n", + "dokimi/b.py": "import sys\n[x for sys in sys.exit(2)]\n", + "dokimi/c.py": "import sys\ndef f(sys=sys.exit(2)):\n pass\n", + "dokimi/d.py": "import sys\ng = lambda sys=sys.exit(2): 0\n", + "dokimi/e.py": "import sys\n@sys.exit(2)\ndef f():\n pass\n", + "dokimi/f.py": "import sys\nclass C(sys.exit(2)):\n pass\n", + "dokimi/g.py": "import sys\ndef f():\n [sys for sys in []]\n sys.exit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [2] + assert exit_sites(imports.trees["dokimi.c"]) == [2] + assert exit_sites(imports.trees["dokimi.d"]) == [2] + assert exit_sites(imports.trees["dokimi.e"]) == [2] + assert exit_sites(imports.trees["dokimi.f"]) == [2] + assert exit_sites(imports.trees["dokimi.g"]) == [4] + + +def test_global_and_nonlocal_declarations_bind_where_they_point(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\ndef f():\n global sys\n sys.exit(2)\n sys = None\n", + "dokimi/b.py": ("def g():\n e = SystemExit\n def f():\n nonlocal e\n" + " raise e(2)\n e = None\n return f\n"), + }) + assert exit_sites(imports.trees["dokimi.a"]) == [4] + assert exit_sites(imports.trees["dokimi.b"]) == [5] + + +def test_a_class_body_reads_enclosing_names_until_it_assigns_them(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\nclass C:\n sys.exit(2)\n sys = None\n", + "dokimi/b.py": "import sys\nsys: object\nsys.exit(2)\n", + "dokimi/c.py": "import sys\nclass C:\n sys: object\n sys.exit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [3] + assert exit_sites(imports.trees["dokimi.c"]) == [4] + + +def test_a_class_name_binds_after_its_body_has_run(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\nclass sys:\n sys.exit(2)\n", + "dokimi/b.py": "E = SystemExit\nclass E:\n raise E(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [3] + + +def test_an_assigned_exit_callable_or_module_keeps_its_meaning(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\nstop = sys.exit\nstop(2)\n", + "dokimi/b.py": "import sys\ns = sys\ns.exit(2)\n", + "dokimi/c.py": "import sys\nstop: object = sys.exit\nstop(2)\n", + "dokimi/d.py": "def f():\n import sys\n stop = sys.exit\n stop(2)\n", + "dokimi/e.py": "import builtins\nstop = builtins.SystemExit\nraise stop(2)\n", + "dokimi/f.py": "import sys\nstop = None\nstop = sys.exit\n\ndef f():\n stop(1)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [3] + assert exit_sites(imports.trees["dokimi.c"]) == [3] + assert exit_sites(imports.trees["dokimi.d"]) == [4] + assert exit_sites(imports.trees["dokimi.e"]) == [3] + assert exit_sites(imports.trees["dokimi.f"]) == [6] + + +def test_a_dotted_import_resolves_through_the_binding_of_its_root(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import dokimi.exit_codes\ndef f(dokimi):\n dokimi.exit_codes.usage_exit('x')\n", + "dokimi/b.py": "import dokimi.exit_codes\ndokimi = None\ndokimi.exit_codes.usage_exit('x')\n", + "dokimi/c.py": "import dokimi.exit_codes\ndokimi.exit_codes.usage_exit('x')\n", + "dokimi/d.py": "import os.path\nos._exit(1)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + assert exit_sites(imports.trees["dokimi.b"]) == [] + assert exit_sites(imports.trees["dokimi.c"]) == [2] + assert exit_sites(imports.trees["dokimi.d"]) == [2] + + +def test_a_deleted_binding_or_a_cleared_except_target_leaves_the_builtin(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "SystemExit = ValueError\ndel SystemExit\nraise SystemExit(2)\n", + "dokimi/b.py": "try:\n 1 / 0\nexcept Exception as SystemExit:\n pass\nraise SystemExit(2)\n", + "dokimi/c.py": "SystemExit = ValueError\nraise SystemExit(2)\n", + "dokimi/d.py": "try:\n 1 / 0\nexcept Exception as SystemExit:\n raise SystemExit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [5] + assert exit_sites(imports.trees["dokimi.c"]) == [] + assert exit_sites(imports.trees["dokimi.d"]) == [] + + +def test_a_class_body_del_or_cleared_except_target_reads_the_enclosing_name(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys as s\nclass C:\n s = None\n del s\n s.exit(2)\n", + "dokimi/b.py": ("import sys as s\nclass C:\n try:\n raise ValueError\n" + " except ValueError as s:\n pass\n s.exit(2)\n"), + "dokimi/c.py": "SystemExit = ValueError\nclass C:\n SystemExit = print\n del SystemExit\n raise SystemExit(2)\n", + "dokimi/d.py": "class C:\n SystemExit = ValueError\n del SystemExit\n raise SystemExit(2)\n", + "dokimi/e.py": "import sys as s\ndel s\ns.exit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [5] + assert exit_sites(imports.trees["dokimi.b"]) == [7] + assert exit_sites(imports.trees["dokimi.c"]) == [] + assert exit_sites(imports.trees["dokimi.d"]) == [4] + assert exit_sites(imports.trees["dokimi.e"]) == [] + + +def test_a_nonlocal_assignment_reaches_the_aliases_read_in_a_sibling(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": ("import sys\ndef outer():\n x = None\n def set_exit():\n nonlocal x\n" + " x = sys.exit\n def run():\n stop = x\n stop(2)\n" + " set_exit()\n run()\nouter()\n"), + "dokimi/b.py": ("def outer():\n x = None\n def set_exit():\n nonlocal x\n" + " x = SystemExit\n def run():\n stop = x\n raise stop(2)\n" + " set_exit()\n run()\nouter()\n"), + }) + assert exit_sites(imports.trees["dokimi.a"]) == [9] + assert exit_sites(imports.trees["dokimi.b"]) == [8] + + +def test_eager_bodies_read_enclosing_names_by_position(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "E = SystemExit\nE = ValueError\nclass C:\n raise E(2)\n", + "dokimi/b.py": "E = ValueError\nE = SystemExit\nclass C:\n raise E(2)\n", + "dokimi/c.py": "import sys\ns = sys.exit\ns = print\nxs = [s(2) for _ in 'a']\n", + "dokimi/d.py": "import sys\ns = print\ns = sys.exit\nxs = [s(2) for _ in 'a']\n", + "dokimi/e.py": "E = SystemExit\nE = ValueError\ndef f():\n class C:\n raise E(2)\n", + "dokimi/f.py": "xs = [exit(2) for _ in 'a']\nexit = print\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + assert exit_sites(imports.trees["dokimi.b"]) == [4] + assert exit_sites(imports.trees["dokimi.c"]) == [] + assert exit_sites(imports.trees["dokimi.d"]) == [4] + assert exit_sites(imports.trees["dokimi.e"]) == [5] + assert exit_sites(imports.trees["dokimi.f"]) == [1] + + +def test_a_function_local_del_or_cleared_except_target_is_unbound_not_the_builtin(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "def f():\n SystemExit = ValueError\n del SystemExit\n raise SystemExit(2)\n", + "dokimi/b.py": "def f():\n try:\n 1 / 0\n except Exception as SystemExit:\n pass\n raise SystemExit(2)\n", + "dokimi/c.py": "def f():\n exit = 1\n del exit\n exit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + assert exit_sites(imports.trees["dokimi.b"]) == [] + assert exit_sites(imports.trees["dokimi.c"]) == [] + + +def test_postponed_annotations_are_not_run_but_defaults_and_decorators_are(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "from __future__ import annotations\nimport sys\ndef f(x: sys.exit(2)) -> sys.exit(3):\n pass\n", + "dokimi/b.py": "import sys\ndef f(x: sys.exit(2)):\n pass\n", + "dokimi/c.py": "from __future__ import annotations\nimport sys\ndef f(x=sys.exit(2)):\n pass\n", + "dokimi/d.py": "from __future__ import annotations\nimport sys\n@sys.exit(2)\ndef f():\n pass\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + assert exit_sites(imports.trees["dokimi.b"]) == [2] + assert exit_sites(imports.trees["dokimi.c"]) == [3] + assert exit_sites(imports.trees["dokimi.d"]) == [3] + + +def test_a_function_local_annotation_is_never_run_but_its_value_and_a_module_annotation_are(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\ndef f():\n x: sys.exit(2)\n", + "dokimi/b.py": "import sys\ndef f():\n x: sys.exit(2) = 1\n", + "dokimi/c.py": "import sys\ndef f():\n x: int = sys.exit(2)\n", + "dokimi/d.py": "import sys\ndef f():\n class C:\n x: sys.exit(2)\n", + "dokimi/e.py": "import sys\nx: sys.exit(2)\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [] + assert exit_sites(imports.trees["dokimi.b"]) == [] + assert exit_sites(imports.trees["dokimi.c"]) == [3] + assert exit_sites(imports.trees["dokimi.d"]) == [4] + assert exit_sites(imports.trees["dokimi.e"]) == [2] + + +def test_a_nonlocal_binder_skips_the_class_body_between_it_and_the_function(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": ("import sys\ndef outer():\n x = None\n class C:\n x = None\n" + " def set_exit(self):\n nonlocal x\n x = sys.exit\n" + " def run():\n x(2)\n C().set_exit()\n run()\nouter()\n"), + }) + assert exit_sites(imports.trees["dokimi.a"]) == [10] + + +def test_an_alias_rebound_after_a_call_still_names_the_exit_its_function_made(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys as s\ndef f():\n s.exit(2)\nf()\nimport os as s\n", + "dokimi/b.py": "import os\nimport sys\nstop = sys.exit\ndef f():\n stop(2)\nstop = os\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [3] + assert exit_sites(imports.trees["dokimi.b"]) == [5] + + +def test_an_alias_copied_in_a_function_keeps_every_binding_its_source_may_have(tmp_path): + imports = _package(tmp_path, { + "dokimi/a.py": "import sys\nimport os\nx = sys.exit\ndef f():\n stop = x\n stop(2)\nf()\nx = os\n", + "dokimi/b.py": "import sys\nx = SystemExit\nx = sys.exit\ndef f():\n stop = x\n stop(2)\nf()\n", + }) + assert exit_sites(imports.trees["dokimi.a"]) == [6] + assert exit_sites(imports.trees["dokimi.b"]) == [6] diff --git a/dokimi/tests/test_import_architecture.py b/dokimi/tests/test_import_architecture.py index 969eef92c..ec2aacd8c 100644 --- a/dokimi/tests/test_import_architecture.py +++ b/dokimi/tests/test_import_architecture.py @@ -36,6 +36,8 @@ import pytest +from tests.exit_site_scan import EXIT_HELPER, exit_sites + PROJECT = Path(__file__).resolve().parents[1] PACKAGE = "dokimi" @@ -406,6 +408,28 @@ def spawn_problems(imports: Imports, baseline: Mapping[str, tuple[int, str]], return problems +#: Modules that end the process (sys.exit, os._exit, exit, quit, raise SystemExit): only the frontend may. +#: A library raises InputError and cli.main exits 2. Empty: nothing outside the frontend exits. +EXIT_BASELINE: dict[str, tuple[int, str]] = {} + + +def exit_problems(imports: Imports, baseline: Mapping[str, tuple[int, str]], + helper: str = EXIT_HELPER) -> list[str]: + counts = {m: exit_sites(t) for m, t in imports.trees.items() + if layer_of(m) != "frontend" and m != helper} + problems = [] + for mod, lines in sorted(counts.items()): + allowed = baseline.get(mod, (0, ""))[0] + if len(lines) > allowed: + problems.append(f"{mod} ends the process at lines {lines}; raise InputError (dokimi.utils) " + "and let cli.main exit 2") + for mod, (allowed, _) in sorted(baseline.items()): + now = len(counts.get(mod, [])) + if now < allowed: + problems.append(f"{mod} has {now} exit site(s), baseline {allowed}: lower EXIT_BASELINE") + return problems + + # --- the package ------------------------------------------------------------------ @@ -446,6 +470,11 @@ def test_llm_provider_sdks_go_through_dokimi_llm(imports): assert not stale, f"{stale} no longer import the banned module; delete them from BANNED_BASELINE" +def test_only_the_frontend_ends_the_process(imports): + problems = exit_problems(imports, EXIT_BASELINE) + assert not problems, "\n".join(problems) + + def test_child_processes_start_through_dokimi_process(imports): problems = spawn_problems(imports, SPAWN_BASELINE) assert not problems, "\n".join(problems) @@ -453,10 +482,11 @@ def test_child_processes_start_through_dokimi_process(imports): def test_baselines_are_explained_and_name_real_modules(imports): reasons = [*LAYER_BASELINE.values(), *CYCLE_BASELINE.values(), *BANNED_BASELINE.values(), - *(reason for _, reason in SPAWN_BASELINE.values())] + *(reason for _, reason in SPAWN_BASELINE.values()), + *(reason for _, reason in EXIT_BASELINE.values())] assert all(r.strip() for r in reasons), "every baseline entry carries its reason" named = {*(m for edge in LAYER_BASELINE for m in edge), *(m for c in CYCLE_BASELINE for m in c), - *(m for m, _ in BANNED_BASELINE), *SPAWN_BASELINE} + *(m for m, _ in BANNED_BASELINE), *SPAWN_BASELINE, *EXIT_BASELINE} assert named <= imports.modules, f"{sorted(named - imports.modules)} name no module" @@ -634,3 +664,55 @@ def test_tie_between_equal_size_subsets_is_stable(): forward = cycle_problems([ab, ac, abc], {ab: "known1", ac: "known2"}) backward = cycle_problems([ab, ac, abc], {ac: "known2", ab: "known1"}) assert forward == backward + + +@pytest.mark.parametrize("source, lines", [ + # a definition's own name does not bind before its header is evaluated (the header runs in the enclosing scope) + ("import sys\ndef sys(x=sys.exit(2)):\n pass\n", [2]), + ("import sys\nclass sys(sys.exit(2)):\n pass\n", [2]), + ("import sys\nasync def sys(x=sys.exit(2)):\n pass\n", [2]), + # unshadowed builtins exit and quit, bound to another name + ("stop = exit\nstop(2)\n", [2]), + ("stop = quit\nstop(2)\n", [2]), + ("exit = print\nexit(2)\n", []), + # a function's alias of a binding made later at module level: the function runs after that binding + ("import sys\ndef f():\n stop = end\n stop(2)\nend = sys.exit\nf()\n", [4]), + ("import sys\ndef f():\n e = Stop\n raise e(2)\nStop = SystemExit\nf()\n", [4]), + # assignment expressions bind their target + ("import sys\nif (stop := sys.exit):\n stop(2)\n", [3]), + # a global declaration sends the name to the module, past the parameter that shadows it + ("import sys\ndef outer(sys):\n def inner():\n global sys\n sys.exit(2)\n return inner\n", [5]), + # a rebinding of the same line is read in source order + ("import sys; stop = sys.exit; stop(2); stop = None\n", [1]), + ("import sys; E = SystemExit; raise E(3); E = None\n", [1]), + ("import sys; s = sys; s.exit(2); s = None\n", [1]), + # a nonlocal name bound by a plain assignment in the enclosing scope: the sibling's alias reaches the exit + ("import sys\ndef outer():\n x = None\n def inner():\n nonlocal x\n x = sys.exit\n def other():\n x(2)\n return other\n", [8]), + # a name declared twice in one function: both declarations redirect to the module, and the lookup ends there + ("import sys\ndef f():\n global sys\n global sys\n sys.exit(2)\n", [5]), + # a chain of nonlocal declarations: the middle scope only declares x, so the binder is the outer one + ("import sys\ndef outer():\n x = None\n def mid():\n nonlocal x\n def inner():\n nonlocal x\n x = sys.exit\n inner()\n def other():\n x(2)\n mid()\n return other\n", [11]), + # a global rebinding made in a function runs only when it is called: module-level sys.exit is still a site + ("import sys\ndef f():\n global sys\n sys = None\nsys.exit(1)\n", [5]), + # a pattern capture, a loop, augmented or with target binds after the value that is evaluated first + ("import sys\ndef f(value):\n match value:\n case sys:\n sys.exit(2)\n", []), + ("import sys\nfor sys in sys.exit(2):\n pass\n", [2]), + ("import sys\nsys += sys.exit(2)\n", [2]), + ("import sys\nwith sys.exit(2) as sys:\n pass\n", [2]), + # in a function the loop target makes sys local, so the iterable reads an unbound name + ("import sys\nasync def f():\n async for sys in sys.exit(2):\n pass\n", []), + ("import sys\ndef f(value):\n match value:\n case [*sys]:\n sys.exit(2)\n", []), + ("import sys\ndef f(value):\n match value:\n case {**sys}:\n sys.exit(2)\n", []), + # arity mismatch and a starred target keep the names as other bindings + ("import sys\na, b = (sys.exit,)\nb(2)\n", []), + ("import sys\na, *b = sys.exit, 1\nb(2)\na(2)\n", []), + # an exit callable or SystemExit unpacked from a literal of the same shape, nested too + ("import sys\nstop, = (sys.exit,)\nstop(2)\n", [3]), + ("import sys\na, (b, c) = 1, (sys.exit, 2)\nb(2)\n", [3]), + # a module-level name is read as builtin until its own assignment runs; a function's local is unbound before it + ("raise SystemExit(2)\nSystemExit = ValueError\n", [1]), + ("exit(2)\nexit = print\nquit(2)\nquit = print\n", [1, 3]), + ("import sys\ndef f():\n exit(2)\n exit = print\n", []), +]) +def test_exit_sites_resolves_names_by_scope_and_source_order(source, lines): + assert exit_sites(ast.parse(source)) == lines diff --git a/dokimi/tests/test_load_or_fail.py b/dokimi/tests/test_load_or_fail.py new file mode 100644 index 000000000..d90cd4bab --- /dev/null +++ b/dokimi/tests/test_load_or_fail.py @@ -0,0 +1,71 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Every input loader fails the same way on a bad file: an InputError that names the file. + +The rows are the loaders, one load-or-fail entry point per artifact kind; the columns are the +ways an input file goes bad. A loader never exits the process: cli.main maps the error to exit 2. +""" + +from __future__ import annotations + +import functools + +import pytest + +from dokimi.artifacts import SCHEMA_VERSIONS, load_artifact +from dokimi.novelty.atoms import load_atoms_record +from dokimi.records import load as load_records +from dokimi.report.model_compare import load_run +from dokimi.utils import InputError + +MISSING, NOT_UTF8, BAD_JSON, NOT_AN_OBJECT, WRONG_SCHEMA = ( + "missing", "not_utf8", "bad_json", "not_an_object", "wrong_schema") + +LOADERS = { + **{f"artifact:{kind}": functools.partial(load_artifact, kind) for kind in sorted(SCHEMA_VERSIONS)}, + "atoms": load_atoms_record, + "judged-run": load_run, + "answers-records": lambda path: load_records(path, section="answers"), +} + +# An object of the loader's own kind that fails its check; other kinds get the generic one. +_WRONG_SCHEMA = { + "atoms": '{"atoms": 5, "spans": []}', + "judged-run": '{"answers": 5}', + "answers-records": '{"answers": [5]}', +} +_GENERIC_WRONG_SCHEMA = '{"schema_version": "not.a.real.version", "unexpected": 1}' + + +def _write(path, loader, mode): + if mode == MISSING: + return + if mode == NOT_UTF8: + path.write_bytes(b'\xff\xfe{"a": 1}') + elif mode == BAD_JSON: + path.write_text("{not json", encoding="utf-8") + elif mode == NOT_AN_OBJECT: + path.write_text("[1, 2]", encoding="utf-8") + else: + path.write_text(_WRONG_SCHEMA.get(loader, _GENERIC_WRONG_SCHEMA), encoding="utf-8") + + +@pytest.mark.parametrize("mode", [MISSING, NOT_UTF8, BAD_JSON, NOT_AN_OBJECT, WRONG_SCHEMA]) +@pytest.mark.parametrize("loader", list(LOADERS)) +def test_a_bad_input_file_is_an_input_error_naming_it(tmp_path, loader, mode): + # pytest.raises(InputError) fails on any other exception, a SystemExit included + path = tmp_path / "input.json" + _write(path, loader, mode) + with pytest.raises(InputError) as exc: + LOADERS[loader](path) + assert str(path) in str(exc.value) + + +def test_a_path_with_a_nul_byte_is_an_input_error_naming_it(): + # Path.read_text raises a bare ValueError here; it must not escape as a traceback. + from dokimi.utils import read_json_input + + with pytest.raises(InputError) as exc: + read_json_input("bad\x00name.json") + assert str(exc.value).startswith("cannot read bad") diff --git a/dokimi/tests/test_model_compare.py b/dokimi/tests/test_model_compare.py index 3212f845e..f708a9e63 100644 --- a/dokimi/tests/test_model_compare.py +++ b/dokimi/tests/test_model_compare.py @@ -19,6 +19,7 @@ prompt_digests, scored_arms, ) +from dokimi.utils import InputError # --------------------------------------------------------------------------- @@ -72,26 +73,26 @@ def test_load_run_valid(tmp_path): def test_load_run_missing_file(): - with pytest.raises(SystemExit, match="File not found"): + with pytest.raises(InputError, match="cannot read .*No such file"): load_run("/nonexistent/path/run.json") def test_load_run_invalid_json(tmp_path): p = tmp_path / "bad.json" p.write_text("{not valid json}", encoding="utf-8") - with pytest.raises(SystemExit, match="Invalid JSON"): + with pytest.raises(InputError, match="is not valid JSON"): load_run(str(p)) def test_load_run_non_utf8_file_names_the_file(tmp_path): p = tmp_path / "latin1.json" p.write_bytes(b'{"note": "caf\xe9"}') - with pytest.raises(SystemExit, match=r"Error: Invalid JSON in .*latin1\.json: .*utf-8"): + with pytest.raises(InputError, match=r"latin1\.json is not UTF-8 text: .*utf-8"): load_run(str(p)) def test_load_run_directory_is_an_error_message_not_a_traceback(tmp_path): - with pytest.raises(SystemExit, match=r"Error: cannot read .*Is a directory"): + with pytest.raises(InputError, match=r"cannot read .*Is a directory"): load_run(str(tmp_path)) @@ -111,7 +112,7 @@ def test_consistency_single_run_no_warnings(): def test_consistency_baseline_mismatch_raises(): - with pytest.raises(SystemExit, match="different baseline_arm"): + with pytest.raises(InputError, match="different baseline_arm"): check_run_consistency( [_run_pair("a", baseline="no-context"), _run_pair("b", baseline="baseline")], "g" ) @@ -131,7 +132,7 @@ def test_consistency_identical_model_is_an_error_unless_pooling(): # to be a soft warning while the report still ranked them against each # other, which reported sampling noise as a model difference. runs = [_run_pair("a", model="same"), _run_pair("b", model="same")] - with pytest.raises(SystemExit, match="--pool-replicates"): + with pytest.raises(InputError, match="--pool-replicates"): check_run_consistency(runs, "g") # Opting into pooling makes it acceptable. assert check_run_consistency(runs, "g", pool_replicates=True) == [] @@ -221,7 +222,7 @@ def test_null_treatment_arm_in_every_record_still_counts_as_an_arm(): {"question_id": "q1", "question_text": "Q?", "arms": {"baseline": {"answer": "b"}, "skill": {"answer": "s"}}}]} assert non_baseline_arms(run_a) == {"docs"} - with pytest.raises(SystemExit, match="different treatment arms"): + with pytest.raises(InputError, match="different treatment arms"): check_run_consistency([("a", run_a), ("b", run_b)], "regular") @@ -272,7 +273,7 @@ def _profile_run(run_id, digest, arm="profile:p", n=2, **kw): def test_consistency_rejects_runs_measuring_different_treatments(): """A `profile:` delta beside a `skill:` delta is confounded, not a model comparison.""" runs = [_armed_run("a", "profile:concise_expert"), _armed_run("b", "skill:demo-skill")] - with pytest.raises(SystemExit, match="different treatment arms"): + with pytest.raises(InputError, match="different treatment arms"): check_run_consistency(runs, "regular") @@ -313,7 +314,7 @@ def test_consistency_rejects_different_treatments_even_when_one_run_is_ambiguous _scored_run("a", "profile:concise_expert"), _scored_run("b", "skill:demo-skill", declared_extra=("docs",)), ] - with pytest.raises(SystemExit, match="different treatment arms"): + with pytest.raises(InputError, match="different treatment arms"): check_run_consistency(runs, "regular") @@ -325,7 +326,7 @@ def test_consistency_rejects_a_group_whose_third_run_is_baseline_only(): _scored_run("b", "skill:demo-skill"), ("c", {"baseline_arm": "baseline", "arms": ["baseline"], "model": "c", "provider": "p"}), ] - with pytest.raises(SystemExit, match="different treatment arms"): + with pytest.raises(InputError, match="different treatment arms"): check_run_consistency(runs, "regular") @@ -365,7 +366,7 @@ def combined(run_id, digest): }) runs = [combined("a", "sha256:" + "1" * 64), combined("b", "sha256:" + "2" * 64)] - with pytest.raises(SystemExit, match="different prompt digests"): + with pytest.raises(InputError, match="different prompt digests"): check_run_consistency(runs, "regular") @@ -382,7 +383,7 @@ def test_consistency_rejects_one_arm_name_carrying_two_prompts(): ("a", _profile_run("a", digest="sha256:" + "1" * 64)), ("b", _profile_run("b", digest="sha256:" + "2" * 64)), ] - with pytest.raises(SystemExit, match="different prompt digests"): + with pytest.raises(InputError, match="different prompt digests"): check_run_consistency(runs, "regular") @@ -559,7 +560,7 @@ def test_extract_scores_refuses_a_record_without_question_id(): {"id": "", "scores": {"baseline": {"aggregate": 60}, "with-ctx": {"aggregate": 80}}}, {"id": "Q1", "scores": {"baseline": {"aggregate": 60}, "with-ctx": {"aggregate": 80}}}, ]) - with pytest.raises(SystemExit, match=r"evaluations\[0\]: missing question_id"): + with pytest.raises(InputError, match=r"evaluations\[0\]: missing question_id"): extract_scores(run) @@ -917,7 +918,7 @@ def test_consistency_refuses_judge_context_that_differs(stamp_a, stamp_b): """Unstamped runs were judged on report snippets; pooling them with stamped runs puts two judge protocols in one comparison.""" runs = [_stamped("a", stamp_a), _stamped("b", stamp_b)] - with pytest.raises(SystemExit, match="judge_context"): + with pytest.raises(InputError, match="judge_context"): check_run_consistency(runs, "regular") @@ -925,13 +926,13 @@ def test_consistency_refuses_judge_context_that_differs(stamp_a, stamp_b): def test_consistency_refuses_a_mixed_run_even_against_another_mixed_one(other): """Two mixed runs fell back on different answers; they share a label, not evidence.""" runs = [_stamped("a", _V1 + "+snippet_fallback"), _stamped("b", other)] - with pytest.raises(SystemExit, match="pools with nothing"): + with pytest.raises(InputError, match="pools with nothing"): check_run_consistency(runs, "regular") def test_consistency_message_says_to_rerun_not_rejudge(): runs = [_stamped("a", None), _stamped("b", _V1)] - with pytest.raises(SystemExit, match="Re-run answering and judging"): + with pytest.raises(InputError, match="Re-run answering and judging"): check_run_consistency(runs, "regular") diff --git a/dokimi/tests/test_model_compare_exit_codes.py b/dokimi/tests/test_model_compare_exit_codes.py index 7366b57d1..f1f7443a2 100644 --- a/dokimi/tests/test_model_compare_exit_codes.py +++ b/dokimi/tests/test_model_compare_exit_codes.py @@ -3,15 +3,20 @@ """Runs that cannot be compared are refused with exit 2, as a missing --treatment-arm is. -The refusals are input errors: the command was given runs it cannot pool. A message match alone -would pass if a refusal went back to the default exit status 1, so each case also pins the code. +The refusals are input errors: the command was given runs it cannot pool. Each raises InputError, +which cli.main reports with exit 2; a message match alone would not pin the status. """ from __future__ import annotations +import json +import sys + import pytest +from dokimi import cli from dokimi.report.model_compare import check_run_consistency +from dokimi.utils import InputError from tests.test_model_compare import _V1, _run_pair, _stamped _NULL_ARM = {"baseline_arm": "baseline", "answers": [ @@ -29,8 +34,59 @@ @pytest.mark.parametrize("name", REFUSED_RUNS) -def test_runs_that_cannot_be_compared_exit_2(name): +def test_runs_that_cannot_be_compared_raise_input_error(name): runs, message = REFUSED_RUNS[name] - with pytest.raises(SystemExit, match=message) as exc: + with pytest.raises(InputError, match=message): check_run_consistency(runs, "regular") + + +@pytest.mark.parametrize("name", ["judge-context", "mixed-run"]) +def test_inconsistent_run_files_exit_2_through_cli_main(name, tmp_path, monkeypatch, capsys): + runs, message = REFUSED_RUNS[name] + paths = [] + for run_id, data in runs: + path = tmp_path / f"{run_id}.json" + path.write_text(json.dumps(data), encoding="utf-8") + paths.append(str(path)) + out = tmp_path / "report.md" + monkeypatch.setattr(sys, "argv", ["dokimi", "report", "model-compare", "--regular-runs", *paths, + "--run-ids", ",".join(run_id for run_id, _ in runs), "--out", str(out)]) + with pytest.raises(SystemExit) as exc: + cli.main() + assert exc.value.code == 2 + assert message in capsys.readouterr().err + assert not out.exists() + + +def _three_arm_run(path): + """Two non-baseline arms (docs, skill) on every question; baseline is declared.""" + evaluations = [{"question_id": f"q{i}", "question_text": "q", "category": "", "difficulty": "easy", + "persona": "", "scores": {"baseline": {"aggregate": 50.0}, "docs": {"aggregate": 60.0}, + "skill": {"aggregate": 55.0}}} for i in range(3)] + path.write_text(json.dumps({"baseline_arm": "baseline", "evaluations": evaluations}), encoding="utf-8") + return path + + +def _three_arm_legacy_run(path): + """The same arms in the legacy per-answer format: no top-level ``evaluations`` key.""" + answers = [{"question_id": f"q{i}", "question_text": "q", "arms": { + "baseline": {"answer": "b"}, "docs": {"answer": "d"}, "skill": {"answer": "s"}}} for i in range(3)] + path.write_text(json.dumps({"baseline_arm": "baseline", "answers": answers}), encoding="utf-8") + return path + + +@pytest.mark.parametrize("writer", [_three_arm_run, _three_arm_legacy_run]) +@pytest.mark.parametrize("extra, message", [ + ([], "multiple non-baseline arms"), + (["--treatment-arm", "missing"], "'missing' not found"), +]) +def test_treatment_arm_refusals_exit_2_through_cli_main(writer, extra, message, tmp_path, monkeypatch, capsys): + run = writer(tmp_path / "run.json") + out = tmp_path / "report.md" + monkeypatch.setattr(sys, "argv", ["dokimi", "report", "model-compare", "--regular-runs", str(run), + "--run-ids", "a", "--out", str(out), *extra]) + with pytest.raises(SystemExit) as exc: + cli.main() assert exc.value.code == 2 + assert message in capsys.readouterr().err + assert not out.exists() diff --git a/dokimi/tests/test_novelty_atoms_input.py b/dokimi/tests/test_novelty_atoms_input.py index 04ea04ae6..5111dbb74 100644 --- a/dokimi/tests/test_novelty_atoms_input.py +++ b/dokimi/tests/test_novelty_atoms_input.py @@ -9,7 +9,7 @@ import pytest -from dokimi.cli import build_parser +from dokimi.utils import InputError def _bundle(extra, end="5"): @@ -121,39 +121,41 @@ def _atoms_file(tmp_path, kind): @pytest.mark.parametrize("sub", ["probe", "shorten"]) @pytest.mark.parametrize("kind", ["missing", "invalid_json", "not_utf8", "directory", *_WRONG_SHAPES]) -def test_an_unreadable_atoms_file_exits_with_a_message_naming_it(tmp_path, sub, kind): +def test_an_unreadable_atoms_file_exits_2_with_a_message_naming_it(tmp_path, monkeypatch, capsys, sub, kind): + from dokimi import cli + skill = tmp_path / "SKILL.md" skill.write_text("---\nname: s\ndescription: d\n---\n\n# H\n\nText.\n", encoding="utf-8") atoms = _atoms_file(tmp_path, kind) - args = build_parser().parse_args( - ["novelty", sub, "--artifact", f"skill:{skill}", "--atoms", str(atoms)] + monkeypatch.setattr( + "sys.argv", ["dokimi", "novelty", sub, "--artifact", f"skill:{skill}", "--atoms", str(atoms)] ) with pytest.raises(SystemExit) as excinfo: - args.func(args) - message = str(excinfo.value) - assert message.startswith("cannot read atoms from ") - assert str(atoms) in message - assert "has no attribute" not in message + cli.main() + assert excinfo.value.code == 2 + err = capsys.readouterr().err + assert err.startswith("Error: ") and str(atoms) in err + assert "Traceback" not in err and "has no attribute" not in err @pytest.mark.parametrize("bundle", ['null', '{}', '{"files": []}', '{"files": [{"path": "SKILL.md", "sha256": null}]}']) def test_a_bundle_block_of_the_right_shape_is_not_refused(tmp_path, bundle): - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record path = tmp_path / "atoms.json" path.write_text('{"atoms": [], "spans": [], "bundle": %s}' % bundle, encoding="utf-8") - atoms, spans, record = load_atoms_or_exit(path) + atoms, spans, record = load_atoms_record(path) assert (atoms, spans, record["bundle"]) == ([], [], json.loads(bundle)) def test_an_atom_with_only_a_statement_is_accepted(tmp_path): - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record path = tmp_path / "atoms.json" span = {"id": "s0001", "kind": "sentence", "text": "S.", "start": 0, "end": 2} path.write_text(json.dumps({"atoms": [{"statement": "S."}], "spans": [span]}), encoding="utf-8") - atoms, spans, _ = load_atoms_or_exit(path) + atoms, spans, _ = load_atoms_record(path) assert [(a.statement, a.question, a.span_ids) for a in atoms] == [("S.", "", ())] assert [(s.id, s.heading_path, s.protection) for s in spans] == [("s0001", (), ())] @@ -184,12 +186,12 @@ def test_an_atom_with_only_a_statement_is_accepted(tmp_path): id="atoms_has_a_number"), ]) def test_a_wrong_span_or_atom_field_is_named_in_the_refusal(tmp_path, record, fragment): - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record path = tmp_path / "atoms.json" path.write_text(record, encoding="utf-8") - with pytest.raises(SystemExit) as exc: - load_atoms_or_exit(path) + with pytest.raises(InputError) as exc: + load_atoms_record(path) assert fragment in str(exc.value) @@ -213,17 +215,17 @@ def test_a_wrong_span_or_atom_field_is_named_in_the_refusal(tmp_path, record, fr ("bundle_excluded_words_is_a_string", "'bundle.excluded' has a row whose 'words'"), ]) def test_every_field_a_reader_uses_is_checked(tmp_path, kind, fragment): - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record - with pytest.raises(SystemExit) as exc: - load_atoms_or_exit(_atoms_file(tmp_path, kind)) + with pytest.raises(InputError) as exc: + load_atoms_record(_atoms_file(tmp_path, kind)) assert fragment in str(exc.value) def test_a_file_row_without_a_role_is_left_to_the_answer_key_check(tmp_path): - from dokimi.novelty.atoms import load_atoms_or_exit + from dokimi.novelty.atoms import load_atoms_record path = tmp_path / "atoms.json" path.write_text(json.dumps({"atoms": [], "spans": [], "bundle": { "files": [{"path": "SKILL.md", "end": 5}]}}), encoding="utf-8") - assert load_atoms_or_exit(path)[2]["bundle"]["files"][0]["path"] == "SKILL.md" + assert load_atoms_record(path)[2]["bundle"]["files"][0]["path"] == "SKILL.md" diff --git a/dokimi/tests/test_novelty_cli.py b/dokimi/tests/test_novelty_cli.py index f78f6351e..2fc732108 100644 --- a/dokimi/tests/test_novelty_cli.py +++ b/dokimi/tests/test_novelty_cli.py @@ -9,10 +9,13 @@ import json import os import re +import subprocess +import sys import pytest from dokimi.cli import build_parser +from dokimi.utils import InputError from dokimi.commands import novelty as novelty_cmd from dokimi.defaults import ( DEFAULT_ANSWER_MODEL, @@ -88,7 +91,7 @@ def test_a_judge_provider_alone_takes_the_generator_when_the_judge_is_measured() ]) def test_a_judge_provider_with_no_default_judge_there_exits(argv): args = _parse("novelty", "analyze", "--artifact", "skill:x", *argv) - with pytest.raises(SystemExit, match="--judge-model"): + with pytest.raises(InputError, match="--judge-model"): novelty_cmd._clients(args) @@ -132,13 +135,13 @@ def test_dropping_the_baseline_arm_is_refused(tmp_path): encoding="utf-8") args = _parse("novelty", "analyze", "--artifact", f"skill:{tmp_path}", "--estimate-only", "--arms", "full") - with pytest.raises(SystemExit, match="^--arms must include no_artifact: the 2x2"): + with pytest.raises(InputError, match="^--arms must include no_artifact: the 2x2"): args.func(args) def test_a_missing_artifact_exits_with_a_message(capsys): args = _parse("novelty", "analyze", "--artifact", "text:/nonexistent/skill.md") - with pytest.raises(SystemExit, match="cannot load"): + with pytest.raises(InputError, match="cannot load"): args.func(args) capsys.readouterr() @@ -485,7 +488,7 @@ def test_probe_refuses_a_stale_answer_key_before_building_a_client(tmp_path): (root / "references" / "tuning.md").write_text("Never exceed 128 threads.\n", encoding="utf-8") args = _parse("novelty", "probe", "--artifact", f"skill:{root}", "--atoms", str(atoms)) - with pytest.raises(SystemExit, match="files changed"): + with pytest.raises(InputError, match="files changed"): args.func(args) @@ -535,7 +538,7 @@ def test_shorten_exits_with_a_message_on_a_bundle_block_it_cannot_slice(tmp_path atoms.write_text(json.dumps(saved), encoding="utf-8") args = _parse("novelty", "shorten", "--artifact", f"skill:{root}", "--atoms", str(atoms), "--lexical-only") - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) assert str(exc.value) == f"{atoms}: the atoms record no span range for SKILL.md" @@ -544,7 +547,7 @@ def test_item_analysis_refuses_a_file_that_carries_no_probe_runs(tmp_path): path = tmp_path / "novelty.json" path.write_text(json.dumps({"model": "m", "paired": {}}), encoding="utf-8") args = _parse("novelty", "items", "--probes", str(path)) - with pytest.raises(SystemExit, match="no probe runs"): + with pytest.raises(InputError, match="no probe runs"): args.func(args) @@ -559,7 +562,7 @@ def test_item_analysis_exits_with_a_message_on_an_unreadable_probes_file(tmp_pat elif content is not None: path.write_bytes(content) args = _parse("novelty", "items", "--probes", str(path)) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) assert str(exc.value).startswith(str(path)) # 3.14 parses deep nesting, then refuses the list @@ -580,7 +583,7 @@ def test_item_analysis_refuses_a_probe_field_that_is_not_a_string(tmp_path, top, "verdict": "correct", **run}], **top} path.write_text(json.dumps(body), encoding="utf-8") args = _parse("novelty", "items", "--probes", str(path)) - with pytest.raises(SystemExit, match="not a string"): + with pytest.raises(InputError, match="not a string"): args.func(args) @@ -673,7 +676,7 @@ def test_items_refuses_runs_of_different_artifacts(tmp_path, field): "response": "r", "verdict": "correct_complete"}]}), encoding="utf-8") files.append(str(path)) args = _parse("novelty", "items", "--probes", *files) - with pytest.raises(SystemExit, match=f"different texts \\({field}"): + with pytest.raises(InputError, match=f"different texts \\({field}"): args.func(args) @@ -909,11 +912,11 @@ def _deep_skill(tmp_path, name="deep"): return f"skill:{root}" -def test_load_artifact_or_exit_names_a_frontmatter_nested_too_deeply(tmp_path): - from dokimi.novelty.artifact import load_artifact_or_exit +def test_load_artifact_ref_names_a_frontmatter_nested_too_deeply(tmp_path): + from dokimi.novelty.artifact import load_artifact_ref - with pytest.raises(SystemExit, match="^cannot load --artifact skill:.*maximum recursion"): - load_artifact_or_exit(_deep_skill(tmp_path)) + with pytest.raises(InputError, match="^cannot load --artifact skill:.*maximum recursion"): + load_artifact_ref(_deep_skill(tmp_path)) @pytest.mark.parametrize("sub", ["analyze", "atoms", "probe", "shorten", "routing"]) @@ -926,7 +929,7 @@ def test_a_command_given_a_frontmatter_nested_too_deeply_exits_with_a_message(tm "routing": ["--competitors", _deep_skill(tmp_path, "other")], }.get(sub, []) args = _parse("novelty", sub, "--artifact", _deep_skill(tmp_path), *extra) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) assert "maximum recursion" in str(exc.value) assert "RecursionError" not in str(exc.value) @@ -937,7 +940,7 @@ def test_all_vs_all_names_a_competitor_nested_too_deeply_and_exits_with_a_messag queries.write_text('{"deep": ["a request"]}', encoding="utf-8") args = _parse("novelty", "routing", "--all-vs-all", "--competitors", _deep_skill(tmp_path), "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="maximum recursion") as exc: + with pytest.raises(InputError, match="maximum recursion") as exc: args.func(args) assert "RecursionError" not in str(exc.value) @@ -954,7 +957,7 @@ def test_all_vs_all_names_an_unreadable_competitor_and_exits_with_a_message(tmp_ args = _parse("novelty", "routing", "--all-vs-all", "--competitors", f"skill:{root}", "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) try: - with pytest.raises(SystemExit, match="cannot load the field"): + with pytest.raises(InputError, match="cannot load the field"): args.func(args) finally: skill.chmod(0o644) @@ -969,3 +972,10 @@ def test_item_analysis_accepts_a_probe_file_that_records_no_digest(tmp_path): encoding="utf-8") cells, _ = load_probe_cells([str(path)]) assert (cells[0]["artifact"], cells[0]["ref_digest"]) == ("skill:x", None) + + +def test_importing_the_cli_does_not_load_the_novelty_package(): + # the CLI imports the novelty command at start-up: the package itself loads only when a novelty command runs + code = "import sys, dokimi.cli; print([m for m in sys.modules if m.startswith('dokimi.novelty')])" + out = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True).stdout + assert out.strip() == "[]" diff --git a/dokimi/tests/test_novelty_cli_exit.py b/dokimi/tests/test_novelty_cli_exit.py new file mode 100644 index 000000000..598e9756f --- /dev/null +++ b/dokimi/tests/test_novelty_cli_exit.py @@ -0,0 +1,46 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Exit codes of `novelty` through `cli.main`: bad input exits 2, a model failure exits 1.""" + +from __future__ import annotations + +import json +import sys + +import pytest + +from dokimi import cli +from dokimi.commands import novelty as novelty_cmd + + +def test_extraction_failure_exits_1_not_2(tmp_path, monkeypatch, capsys): + # A model failure is a runtime failure (exit 1, as before), not bad input (exit 2). + from dokimi.novelty import atoms as atoms_mod + + def failing_extract(*args, **kwargs): + raise atoms_mod.ExtractionError("service unavailable") + + skill = tmp_path / "s" / "SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text("---\nname: s\ndescription: d\n---\n\n# H\n\nDo not enable the legacy driver.\n", + encoding="utf-8") + monkeypatch.setattr(atoms_mod, "extract_atoms", failing_extract) + monkeypatch.setattr(novelty_cmd, "_client", lambda *a, **k: type("C", (), {"model": "m"})()) + monkeypatch.setattr(sys, "argv", ["dokimi", "novelty", "atoms", "--artifact", f"skill:{skill}", + "--out", str(tmp_path / "atoms.json")]) + with pytest.raises(SystemExit) as exc: + cli.main() + # SystemExit with a message exits 1; cli.main's exit 2 applies only to InputError. + assert exc.value.code == "service unavailable" + + +def test_items_bad_probes_file_exits_2_through_cli_main(tmp_path, monkeypatch, capsys): + # The command raises InputError; cli.main prints it and exits 2, the code for bad input. + path = tmp_path / "novelty.json" + path.write_text(json.dumps({"model": "m", "paired": {}}), encoding="utf-8") + monkeypatch.setattr(sys, "argv", ["dokimi", "novelty", "items", "--probes", str(path)]) + with pytest.raises(SystemExit) as exc: + cli.main() + assert exc.value.code == 2 + assert "no probe runs" in capsys.readouterr().err diff --git a/dokimi/tests/test_novelty_gate.py b/dokimi/tests/test_novelty_gate.py index fe41554ab..d4574a39d 100644 --- a/dokimi/tests/test_novelty_gate.py +++ b/dokimi/tests/test_novelty_gate.py @@ -231,12 +231,13 @@ def test_cell_warnings_reach_the_subject_warning_list(): def test_an_attached_artifact_must_validate(tmp_path): import json - from dokimi.artifacts import ArtifactValidationError + from dokimi.utils import InputError bad = tmp_path / "novelty.json" bad.write_text(json.dumps({"schema_version": "novelty.v1"}), encoding="utf-8") - with pytest.raises(ArtifactValidationError): + with pytest.raises(InputError) as exc: load_novelty_artifacts([bad]) + assert str(bad) in str(exc.value) def test_an_entry_point_only_run_of_a_bundle_artifact_is_flagged(): diff --git a/dokimi/tests/test_novelty_journey.py b/dokimi/tests/test_novelty_journey.py index 0ca364a54..60c51c096 100644 --- a/dokimi/tests/test_novelty_journey.py +++ b/dokimi/tests/test_novelty_journey.py @@ -22,6 +22,7 @@ from tests.test_novelty_analyze import SKILL, _scripted from dokimi.cli import build_parser +from dokimi.utils import InputError from dokimi.novelty import client as client_mod @@ -186,11 +187,10 @@ def test_probing_atoms_from_a_changed_artifact_is_refused(tmp_path, skill, stub_ _run("novelty", "atoms", "--artifact", f"skill:{skill}", "--include-resources", "--extract-model", "extractor-m", "--out", str(atoms_out)) (skill / "references" / "new.md").write_text("Another reference.\n", encoding="utf-8") - with pytest.raises(SystemExit, match="files changed") as exc: + with pytest.raises(InputError, match="files changed"): _run("novelty", "probe", "--artifact", f"skill:{skill}", "--atoms", str(atoms_out), "--model", "model-a", "--judge-model", "judge-m", "--out", str(tmp_path / "p.json")) - assert exc.value.code == 2 # an atoms file this artifact cannot use: an input refusal @pytest.mark.parametrize("command", ["analyze", "probe"]) @@ -212,11 +212,10 @@ def transport(prompt, system): monkeypatch.setattr(client_mod, "LLMClient", recording_client) extra = ["--atoms", str(atoms_out), "--out", str(tmp_path / "p.json")] if command == "probe" else \ ["--out-dir", str(tmp_path / "out")] - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: _run("novelty", command, "--artifact", f"skill:{skill}", "--model", "model-a", "--judge-model", "model-a", *extra) - assert exc.value.code == 2 - assert "is the answering model" in capsys.readouterr().err + assert "is the answering model" in str(exc.value) assert prompts == [], "a model was called before the self-judge check refused" @@ -320,3 +319,38 @@ def test_documented_estimate_flags_match_the_run_it_prices(): """The walkthrough's estimate commands carry the flags of the run they price (doc reference).""" estimates = [c for c in _documented_novelty_commands() if "--estimate-only" in c] assert estimates and all("--include-resources" in c for c in estimates) + + +@pytest.mark.parametrize("command", ["analyze", "probe"]) +def test_a_model_failure_exits_1_with_its_message_through_cli_main(command, tmp_path, skill, stub_models, + monkeypatch): + # A model failure is a runtime failure: SystemExit(message), which exits 1, not the bad-input 2. + import importlib + import sys + + from dokimi import cli + from dokimi.novelty import measure as measure_mod + from dokimi.novelty.atoms import ExtractionError + + common = ["--artifact", f"skill:{skill}", "--model", "model-a", "--judge-model", "judge-m"] + if command == "analyze": + def failing_analyze(*args, **kwargs): + raise ExtractionError("extractor unavailable") + # importlib: ``dokimi.novelty.analyze`` the attribute is the function, not the module + monkeypatch.setattr(importlib.import_module("dokimi.novelty.analyze"), "analyze", failing_analyze) + argv = ["novelty", "analyze", *common, "--out-dir", str(tmp_path / "out")] + message = "extractor unavailable" + else: + atoms_out = tmp_path / "atoms.json" + _run("novelty", "atoms", "--artifact", f"skill:{skill}", "--extract-model", "extractor-m", + "--out", str(atoms_out)) + + def failing_measurement(*args, **kwargs): + raise ValueError("answerer unavailable") + monkeypatch.setattr(measure_mod, "run_measurement", failing_measurement) + argv = ["novelty", "probe", *common, "--atoms", str(atoms_out), "--out", str(tmp_path / "p.json")] + message = "answerer unavailable" + monkeypatch.setattr(sys, "argv", ["dokimi", *argv]) + with pytest.raises(SystemExit) as exc: + cli.main() + assert exc.value.code == message diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index f209122ba..6c5343d71 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -10,6 +10,7 @@ import pytest +from dokimi.utils import InputError from dokimi.cli import build_parser from dokimi.novelty.client import LLMClient from dokimi.novelty.routing import ( @@ -219,7 +220,7 @@ def test_routing_against_a_single_candidate_is_refused(tmp_path): # Selecting from one candidate measures tool use rather than this artifact winning. args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), "--queries", "x") - with pytest.raises(SystemExit, match="needs competitors"): + with pytest.raises(InputError, match="needs competitors"): args.func(args) @@ -267,7 +268,7 @@ def test_an_unreadable_queries_file_exits_with_a_message(tmp_path, content): queries.write_bytes(content) args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), "--competitors", _skill(tmp_path, "b", "e"), "--queries", str(queries)) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) assert str(exc.value).startswith(f"cannot read --queries {queries}: ") @@ -275,7 +276,7 @@ def test_an_unreadable_queries_file_exits_with_a_message(tmp_path, content): def test_a_catalog_that_is_not_a_directory_is_refused(tmp_path): args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "d"), "--catalog", str(tmp_path / "nope"), "--queries-from-headings") - with pytest.raises(SystemExit, match="--catalog is not a directory"): + with pytest.raises(InputError, match="--catalog is not a directory"): args.func(args) @@ -283,7 +284,7 @@ def test_routing_without_a_query_source_is_refused(tmp_path): args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", _skill(tmp_path, "b", "tune a database")) - with pytest.raises(SystemExit, match="--queries"): + with pytest.raises(InputError, match="--queries"): args.func(args) @@ -330,7 +331,7 @@ def test_queries_value_must_be_a_list_of_strings(tmp_path, content): "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries", str(queries)) - with pytest.raises(SystemExit, match="list of strings"): + with pytest.raises(InputError, match="list of strings"): args.func(args) @@ -341,7 +342,7 @@ def test_duplicate_competitors_are_refused_or_deduped(tmp_path): args = _parse("novelty", "routing", "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", competitor, competitor, "--queries", str(queries)) - with pytest.raises(SystemExit, match="more than once"): + with pytest.raises(InputError, match="more than once"): args.func(args) @@ -385,7 +386,7 @@ def test_the_artifact_cannot_compete_with_itself(tmp_path): ref = _skill(tmp_path, "perf", "Profile a binary") args = _parse("novelty", "routing", "--artifact", ref, "--competitors", ref, "--queries", "x") - with pytest.raises(SystemExit, match="against itself"): + with pytest.raises(InputError, match="against itself"): args.func(args) @@ -689,7 +690,7 @@ def test_repeats_without_a_model_are_refused_on_the_cli(tmp_path, repeats): "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries", str(queries), "--repeats", repeats, "--out", str(out)) - with pytest.raises(SystemExit, match="needs --model"): + with pytest.raises(InputError, match="needs --model"): args.func(args) assert not out.exists() @@ -809,7 +810,7 @@ def test_headings_inside_code_only_refuse_the_run_and_say_why(tmp_path): body="```\n## only in code\n```\n"), "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries-from-headings", "--out", str(out)) - with pytest.raises(SystemExit, match="code fences.*--queries"): + with pytest.raises(InputError, match="code fences.*--queries"): args.func(args) payload = json.loads(out.read_text(encoding="utf-8")) assert payload["status"] == "no_queries" @@ -824,7 +825,7 @@ def test_a_query_file_with_nothing_in_it_refuses_the_run(tmp_path): "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries", str(queries), "--out", str(out)) - with pytest.raises(SystemExit, match="no non-blank query"): + with pytest.raises(InputError, match="no non-blank query"): args.func(args) assert json.loads(out.read_text(encoding="utf-8"))["status"] == "no_queries" @@ -836,7 +837,7 @@ def test_queries_without_a_content_word_refuse_the_run(tmp_path): "--artifact", _skill(tmp_path, "a", "profile a binary"), "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries", str(queries), "--out", str(tmp_path / "routing.json")) - with pytest.raises(SystemExit, match="has a content word"): + with pytest.raises(InputError, match="has a content word"): args.func(args) @@ -936,7 +937,7 @@ def test_the_model_row_says_its_query_count_is_of_all_given(tmp_path, monkeypatc def test_a_run_with_no_scorable_query_makes_no_model_call(tmp_path, monkeypatch): args, calls = _routing_with_model(tmp_path, monkeypatch, "the of a\nhow do I\n") - with pytest.raises(SystemExit, match="no usable queries"): + with pytest.raises(InputError, match="no usable queries"): args.func(args) assert calls == [] assert json.loads((tmp_path / "routing.json").read_text())["status"] == "no_queries" diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index f3076861b..75065ac4e 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -10,6 +10,7 @@ import pytest +from dokimi.utils import InputError from dokimi.cli import build_parser from dokimi.novelty.client import LLMClient from dokimi.novelty.all_vs_all import all_vs_all, load_queries_by_skill @@ -144,7 +145,7 @@ def test_a_queries_file_nested_too_deeply_exits_with_a_message(tmp_path, monkeyp _catalog_skill(root, "perf", "profile a slow binary with perf record and report") args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), "--queries-by-skill", str(path), "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="nested too deeply") as exc: + with pytest.raises(InputError, match="nested too deeply") as exc: args.func(args) assert "RecursionError" not in str(exc.value) @@ -240,13 +241,13 @@ def test_all_vs_all_takes_no_artifact(tmp_path): queries = _write_queries(tmp_path, {"linux-perf": ["q"]}) args = _parse("novelty", "routing", "--all-vs-all", "--artifact", "skill:/x", "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="takes no --artifact"): + with pytest.raises(InputError, match="takes no --artifact"): args.func(args) def test_a_single_routing_run_still_needs_an_artifact(tmp_path): args = _parse("novelty", "routing", "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="--all-vs-all for the whole field"): + with pytest.raises(InputError, match="--all-vs-all for the whole field"): args.func(args) @@ -466,7 +467,7 @@ def test_an_unreadable_queries_file_is_a_message_not_a_traceback(tmp_path, make) queries.write_bytes(b"\xff\xfe\x00bad") args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json")) - with pytest.raises(SystemExit, match="cannot read the queries"): + with pytest.raises(InputError, match="cannot read the queries"): args.func(args) @@ -562,7 +563,7 @@ def test_all_vs_all_refuses_the_single_artifact_query_flags(tmp_path, flag): queries = _write_queries(tmp_path, {"perf": ["profile"]}) args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(tmp_path), "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), *flag) - with pytest.raises(SystemExit, match="takes --queries-by-skill, not --queries"): + with pytest.raises(InputError, match="takes --queries-by-skill, not --queries"): args.func(args) @@ -580,7 +581,7 @@ def test_all_vs_all_refuses_repeats_it_would_not_make(tmp_path, monkeypatch): args = _parse("novelty", "routing", "--all-vs-all", "--catalog", str(root), "--queries-by-skill", str(queries), "--out", str(tmp_path / "o.json"), "--model", "m", "--repeats", "3") - with pytest.raises(SystemExit, match="takes no --repeats"): + with pytest.raises(InputError, match="takes no --repeats"): args.func(args) assert calls == [] and not (tmp_path / "o.json").exists() diff --git a/dokimi/tests/test_novelty_routing_field.py b/dokimi/tests/test_novelty_routing_field.py index 2de05770d..f00befc5d 100644 --- a/dokimi/tests/test_novelty_routing_field.py +++ b/dokimi/tests/test_novelty_routing_field.py @@ -9,6 +9,7 @@ import pytest +from dokimi.utils import InputError from dokimi.cli import build_parser from dokimi.novelty.routing import ( RoutingInputError, @@ -79,7 +80,7 @@ def test_a_non_skill_artifact_is_still_refused(tmp_path): args = _parse("novelty", "routing", "--artifact", f"skill:{helper}", "--competitors", _skill(tmp_path, "b", "tune a database"), "--queries", "x") - with pytest.raises(SystemExit, match="cannot load an artifact"): + with pytest.raises(InputError, match="cannot load an artifact"): args.func(args) @@ -241,7 +242,7 @@ def test_two_catalog_skills_with_one_name_and_different_content_are_refused_by_n "--out", str(out), ) - with pytest.raises(SystemExit) as refused: + with pytest.raises(InputError) as refused: args.func(args) message = str(refused.value) assert message.startswith("catalog has db at ") and "dedupe or rename" in message diff --git a/dokimi/tests/test_novelty_settings_parity.py b/dokimi/tests/test_novelty_settings_parity.py index 713ab052b..b9e0af976 100644 --- a/dokimi/tests/test_novelty_settings_parity.py +++ b/dokimi/tests/test_novelty_settings_parity.py @@ -36,6 +36,7 @@ from dokimi.cli import build_parser from dokimi.commands import novelty as novelty_cmd +from dokimi.utils import InputError from tests.test_novelty_analyze import SKILL, _scripted GOLDEN = Path(__file__).resolve().parent / "fixtures" / "novelty_settings_main.json" @@ -189,6 +190,9 @@ def scripted(model, provider, role, args): args.func(args) except SystemExit as exc: status = exc.code if isinstance(exc.code, int) else str(exc.code) + except InputError as exc: # cli.main's shape: exit 2, the message on stderr + status = 2 + err.write(f"{exc}\n") steps.append({ "status": status, "stdout": (_without_coverage_headline(out.getvalue()) diff --git a/dokimi/tests/test_novelty_shorten_artifact.py b/dokimi/tests/test_novelty_shorten_artifact.py index cb553cbc7..2718c407d 100644 --- a/dokimi/tests/test_novelty_shorten_artifact.py +++ b/dokimi/tests/test_novelty_shorten_artifact.py @@ -7,6 +7,7 @@ import pytest +from dokimi.utils import InputError from dokimi.cli import build_parser from dokimi.novelty.atoms import save_atoms from dokimi.novelty.spans import segment @@ -23,7 +24,7 @@ def _shorten(tmp_path, artifact): def test_a_missing_text_artifact_exits_naming_it(tmp_path): missing = tmp_path / "missing.md" - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: _shorten(tmp_path, f"text:{missing}") assert str(exc.value) == f"cannot load --artifact text:{missing}: artifact not found: {missing}" @@ -31,7 +32,7 @@ def test_a_missing_text_artifact_exits_naming_it(tmp_path): def test_a_non_utf8_text_artifact_exits_naming_it(tmp_path): binary = tmp_path / "bin.md" binary.write_bytes(bytes([0xFF, 0xFE, 0x00])) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: _shorten(tmp_path, f"text:{binary}") assert str(exc.value).startswith(f"cannot load --artifact text:{binary}: ") @@ -43,7 +44,7 @@ def test_a_skill_file_without_a_closing_frontmatter_fence_exits_naming_it(tmp_pa entry.write_text("---\nname: s\ndescription: d\n\n# H\n", encoding="utf-8") monkeypatch.setattr("dokimi.skills.loader.load_skill", lambda _p: SimpleNamespace( path=entry, name="s", description="d", body="# H\n")) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: _shorten(tmp_path, f"skill:{tmp_path}") assert str(exc.value) == (f"cannot load --artifact skill:{tmp_path}: " f"{entry} has no closing '---' after its frontmatter") @@ -67,6 +68,6 @@ def test_an_unreadable_artifact_exits_without_a_traceback(tmp_path, command, kin if command == "routing": argv += ["--queries-from-headings"] args = build_parser().parse_args(argv) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) assert str(exc.value).startswith("cannot load ") diff --git a/dokimi/tests/test_novelty_view_inputs.py b/dokimi/tests/test_novelty_view_inputs.py index ac96766c5..d86ad909e 100644 --- a/dokimi/tests/test_novelty_view_inputs.py +++ b/dokimi/tests/test_novelty_view_inputs.py @@ -1,10 +1,13 @@ # Copyright (C) 2026 Intel Corporation # SPDX-License-Identifier: Apache-2.0 -"""`novelty view`: an unreadable or invalid --novelty or --atoms file exits 1 with a message.""" +"""`novelty view`: an unreadable or invalid --novelty or --atoms file exits 2 with a message naming it.""" from __future__ import annotations +import sys +from unittest import mock + import pytest from dokimi.cli import build_parser @@ -19,10 +22,20 @@ def _view(novelty, atoms, out): args.func(args) +def _as_reasons(reason): + """One expected reason, or several where the Python version decides how deep nesting fails.""" + return (reason,) if isinstance(reason, str) else reason + + def _err(tmp_path, novelty, atoms, capsys): + """Run `novelty view` through cli.main, as a user does, and return what it printed.""" + from dokimi import cli + out = tmp_path / "annotated.md" - with pytest.raises(SystemExit) as exc: - _view(novelty, atoms, out) + argv = ["dokimi", "novelty", "view", "--novelty", str(novelty), "--atoms", str(atoms), + "--format", "md", "--out", str(out)] + with mock.patch.object(sys, "argv", argv), pytest.raises(SystemExit) as exc: + cli.main() assert exc.value.code == 2 assert not out.exists() return capsys.readouterr().err @@ -30,18 +43,26 @@ def _err(tmp_path, novelty, atoms, capsys): def test_a_missing_novelty_file_is_reported_as_unreadable(tmp_path, capsys): missing = tmp_path / "missing.json" - assert f"Error: cannot read {missing}" in _err(tmp_path, missing, tmp_path / "a.json", capsys) + assert f"cannot read {missing}" in _err(tmp_path, missing, tmp_path / "a.json", capsys) def test_a_directory_as_novelty_is_reported_as_unreadable(tmp_path, capsys): - assert f"Error: cannot read {tmp_path}" in _err(tmp_path, tmp_path, tmp_path / "a.json", capsys) + assert f"cannot read {tmp_path}" in _err(tmp_path, tmp_path, tmp_path / "a.json", capsys) -@pytest.mark.parametrize("payload", [b"", b"{not json", bytes([0xFF, 0xFE]), b"[1, 2]", b"{}", pytest.param(b"[" * 100000 + b"]" * 100000, id="nested_deeply")]) -def test_a_novelty_file_that_is_not_a_novelty_artifact_is_reported_as_invalid(tmp_path, capsys, payload): +@pytest.mark.parametrize("payload, reason", [ + (b"", "is not valid JSON"), + (b"{not json", "is not valid JSON"), + (bytes([0xFF, 0xFE]), "is not UTF-8 text"), + (b"[1, 2]", "is not a novelty artifact: expected a JSON object"), + (b"{}", "is not a valid novelty artifact: "), + pytest.param(b"[" * 100000 + b"]" * 100000, ("is not valid JSON", "is not a novelty artifact: expected a JSON object", "is not a valid novelty artifact: "), id="nested_deeply"), +]) +def test_a_novelty_file_that_is_not_a_novelty_artifact_is_reported_as_invalid(tmp_path, capsys, payload, reason): bad = tmp_path / "bad.json" bad.write_bytes(payload) - assert f"Error: {bad} is not a valid novelty artifact" in _err(tmp_path, bad, tmp_path / "a.json", capsys) + err = _err(tmp_path, bad, tmp_path / "a.json", capsys) + assert any(f"{bad} {r}" in err for r in _as_reasons(reason)) def test_a_missing_atoms_file_is_reported_as_unreadable(tmp_path, capsys): @@ -51,14 +72,20 @@ def test_a_missing_atoms_file_is_reported_as_unreadable(tmp_path, capsys): novelty = tmp_path / "novelty.json" save_artifact("novelty", _minimal_artifact(), novelty) missing = tmp_path / "missing-atoms.json" - assert f"Error: cannot read {missing}" in _err(tmp_path, novelty, missing, capsys) - - -@pytest.mark.parametrize("payload", [ - b"", b"{not json", bytes([0xFF, 0xFE]), b"[1, 2]", pytest.param(b"[" * 100000 + b"]" * 100000, id="nested_deeply"), - b'{"atoms": [{"unexpected": 1}]}', b'{"atoms": "x"}', b'{"spans": [1]}', + assert f"cannot read {missing}" in _err(tmp_path, novelty, missing, capsys) + + +@pytest.mark.parametrize("payload, reason", [ + (b"", "is not valid JSON"), + (b"{not json", "is not valid JSON"), + (bytes([0xFF, 0xFE]), "is not UTF-8 text"), + (b"[1, 2]", "is not a valid atoms file: "), + pytest.param(b"[" * 100000 + b"]" * 100000, ("is not valid JSON", "is not a valid atoms file: "), id="nested_deeply"), + (b'{"atoms": [{"unexpected": 1}]}', "is not a valid atoms file: "), + (b'{"atoms": "x"}', "is not a valid atoms file: "), + (b'{"spans": [1]}', "is not a valid atoms file: "), ]) -def test_an_atoms_file_that_is_not_an_atoms_record_is_reported_as_invalid(tmp_path, capsys, payload): +def test_an_atoms_file_that_is_not_an_atoms_record_is_reported_as_invalid(tmp_path, capsys, payload, reason): from dokimi.artifacts import save_artifact from tests.test_novelty_cli import _minimal_artifact @@ -66,7 +93,8 @@ def test_an_atoms_file_that_is_not_an_atoms_record_is_reported_as_invalid(tmp_pa save_artifact("novelty", _minimal_artifact(), novelty) bad = tmp_path / "atoms.json" bad.write_bytes(payload) - assert f"Error: {bad} is not a valid atoms file" in _err(tmp_path, novelty, bad, capsys) + err = _err(tmp_path, novelty, bad, capsys) + assert any(f"{bad} {r}" in err for r in _as_reasons(reason)) def test_an_atoms_error_names_the_exception_without_its_repr_quotes(tmp_path, capsys): @@ -77,7 +105,7 @@ def test_an_atoms_error_names_the_exception_without_its_repr_quotes(tmp_path, ca save_artifact("novelty", _minimal_artifact(), novelty) bad = tmp_path / "atoms.json" bad.write_text('{"atoms": [{"unexpected": 1}]}', encoding="utf-8") - assert f"Error: {bad} is not a valid atoms file: " in _err(tmp_path, novelty, bad, capsys) + assert f"{bad} is not a valid atoms file: " in _err(tmp_path, novelty, bad, capsys) @pytest.mark.parametrize("kind", [k for k in _WRONG_SHAPES if not k.startswith("bundle")]) @@ -89,4 +117,6 @@ def test_an_atoms_file_of_a_wrong_shape_is_reported_as_invalid_by_view(tmp_path, save_artifact("novelty", _minimal_artifact(), novelty) bad = tmp_path / "atoms.json" bad.write_text(_WRONG_SHAPES[kind], encoding="utf-8") - assert f"Error: {bad} is not a valid atoms file" in _err(tmp_path, novelty, bad, capsys) + reasons = ("is not valid JSON", "is not a valid atoms file: ") if "nested" in kind else ("is not a valid atoms file: ",) + err = _err(tmp_path, novelty, bad, capsys) + assert any(f"{bad} {r}" in err for r in reasons) diff --git a/dokimi/tests/test_record_readers.py b/dokimi/tests/test_record_readers.py index ee83a7108..3c6d20185 100644 --- a/dokimi/tests/test_record_readers.py +++ b/dokimi/tests/test_record_readers.py @@ -18,6 +18,7 @@ from dokimi.eval.panel import JudgePanel, PanelVerdict from dokimi.records import RecordError +from dokimi.utils import InputError from tests.artifact_fixtures import artifact_fixture @@ -164,14 +165,13 @@ def test_the_degradation_pool_refuses_a_record_without_question_id(): build_pairs([{"question_text": "Q?", "arms": {"baseline": {"answer": "x" * 2000}}}]) -def test_eval_score_names_the_malformed_record(tmp_path, capsys): +def test_eval_score_names_the_malformed_record(tmp_path): from dokimi.commands.evaluate import _load_answers path = tmp_path / "answers.json" path.write_text(json.dumps({"answers": [{"question_text": "Q?"}]}), encoding="utf-8") - with pytest.raises(SystemExit): + with pytest.raises(InputError, match=r"answers\.json answers\[0\]: missing question_id"): _load_answers(str(path)) - assert "answers.json answers[0]: missing question_id" in capsys.readouterr().err def test_the_dashboard_leaves_out_a_file_with_a_malformed_record(tmp_path, caplog): @@ -306,7 +306,7 @@ def test_model_compare_names_the_file_with_the_malformed_record(tmp_path): path = tmp_path / "run.json" path.write_text(json.dumps({"answers": [{"arms": {"baseline": {"answer": "a"}}}]}), encoding="utf-8") - with pytest.raises(SystemExit, match=r"run\.json answers\[0\]: missing question_id"): + with pytest.raises(InputError, match=r"run\.json answers\[0\]: missing question_id"): load_run(str(path)) @@ -464,10 +464,8 @@ def test_the_arm_judges_refuse_a_binary_answers_file_before_any_call(tmp_path, m extra = ["--questions", str(questions)] if command[0] == "checklist" else [] args = build_parser().parse_args(["eval", *command, "--answers", str(answers), *extra, "--output", str(tmp_path / "out.json")]) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError, match="has no arms answers"): args.func(args) - assert exc.value.code == 2 - assert "has no arms answers" in capsys.readouterr().err assert judge.mock_calls == [] assert not (tmp_path / "out.json").exists() @@ -590,9 +588,9 @@ def test_cross_judge_refuses_a_missing_result_file_before_any_judge_call(tmp_pat missing = tmp_path / "missing.json" args = _cross_judge_args(tmp_path, _material_arms_run(), "--no-stored", "--judges", "amazon-bedrock/m", flag, str(missing)) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) - assert exc.value.code == 2 and str(missing) in capsys.readouterr().err + assert str(missing) in str(exc.value) assert seen == {} # no paid judge call was made @@ -722,7 +720,7 @@ def _checklist(question): "questions[0]", id="coverage-past-float-range"), ]) def test_cross_judge_names_a_result_entry_of_the_wrong_shape_before_any_judge_call( - tmp_path, capsys, monkeypatch, flag, content, where): + tmp_path, monkeypatch, flag, content, where): from tests.test_judge_reliability import _cross_judge_args, _material_arms_run, _recording_judge seen: dict = {} @@ -731,10 +729,9 @@ def test_cross_judge_names_a_result_entry_of_the_wrong_shape_before_any_judge_ca wrong.write_text(json.dumps(content), encoding="utf-8") args = _cross_judge_args(tmp_path, _material_arms_run(), "--no-stored", "--judges", "amazon-bedrock/m", flag, str(wrong)) - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: args.func(args) - assert exc.value.code == 2 - assert capsys.readouterr().err.startswith(f"Error: {wrong}: {where} ") + assert str(exc.value).startswith(f"{wrong}: {where} ") assert seen == {} # no paid judge call was made diff --git a/dokimi/tests/test_records.py b/dokimi/tests/test_records.py index 99bfa5610..af464b2ac 100644 --- a/dokimi/tests/test_records.py +++ b/dokimi/tests/test_records.py @@ -232,11 +232,11 @@ def test_malformed_input_is_a_record_error_naming_the_record(data, message): def test_load_names_the_file_on_every_failure(tmp_path): - with pytest.raises(RecordError, match="file not found"): + with pytest.raises(RecordError, match="cannot read .*missing.json"): records.load(tmp_path / "missing.json") bad = tmp_path / "bad.json" bad.write_text("{not json", encoding="utf-8") - with pytest.raises(RecordError, match=r"invalid JSON in .*bad\.json"): + with pytest.raises(RecordError, match=r"bad\.json is not valid JSON"): records.load(bad) good = tmp_path / "answers.json" good.write_text(json.dumps(_answers(_answer("q1"))), encoding="utf-8") diff --git a/dokimi/tests/test_replicates.py b/dokimi/tests/test_replicates.py index 50addb011..3411b4aba 100644 --- a/dokimi/tests/test_replicates.py +++ b/dokimi/tests/test_replicates.py @@ -10,6 +10,7 @@ import pytest +from dokimi.utils import InputError from dokimi.report.model_compare import ( check_run_consistency, group_replicates, @@ -59,7 +60,7 @@ def test_group_replicates_separates_on_judge(): # ── duplicate detection ───────────────────────────────────────────────────── def test_duplicate_configs_are_a_hard_error_when_not_pooling(): runs = [("r1", _run("m1")), ("r2", _run("m1")), ("r3", _run("m2"))] - with pytest.raises(SystemExit) as exc: + with pytest.raises(InputError) as exc: check_run_consistency(runs, "golden") msg = str(exc.value) assert "--pool-replicates" in msg @@ -81,7 +82,7 @@ def test_all_same_model_is_an_error_not_just_a_warning(): # Previously this only produced a soft stderr warning while the report # ranked a model against itself. runs = [("r1", _run("m1")), ("r2", _run("m1"))] - with pytest.raises(SystemExit): + with pytest.raises(InputError): check_run_consistency(runs, "golden") @@ -315,7 +316,7 @@ def _model_compare(tmp_path, first, second, *extra): {"arms": ("baseline", "skill:s", "docs")}, ]) def test_runs_differing_in_plugin_set_harness_or_arms_are_not_replicates(tmp_path, other): - with pytest.raises(SystemExit, match="--pool-replicates"): + with pytest.raises(InputError, match="--pool-replicates"): _model_compare(tmp_path, _arms_v1(), _arms_v1()) assert "pools 2 replicates" in _model_compare(tmp_path, _arms_v1(), _arms_v1(), "--pool-replicates") diff --git a/dokimi/tests/test_skill_repo_shapes.py b/dokimi/tests/test_skill_repo_shapes.py index 0f01d3e0b..ecd8bb6a0 100644 --- a/dokimi/tests/test_skill_repo_shapes.py +++ b/dokimi/tests/test_skill_repo_shapes.py @@ -11,12 +11,13 @@ from __future__ import annotations import json +import sys from dataclasses import dataclass from pathlib import Path import pytest -from dokimi.cli import build_parser +from dokimi import cli from dokimi.novelty.client import LLMClient from tests.skill_repo_shapes import PROBE_QUERIES, SHAPES, build_shape, probe_artifact @@ -67,14 +68,21 @@ def _ok(*field: str) -> Expect: def _run(argv: list[str], capsys) -> tuple[int, str, str]: - """Run one dokimi command in-process; (exit code, stdout, stderr). Crashes propagate.""" - args = build_parser().parse_args(argv) + """Run one dokimi command through ``cli.main`` as the console script does; (exit code, stdout, stderr). + + Bad input is refused inside ``cli.main`` (InputError exits 2), so the command's own + exceptions are not seen here. Any other crash propagates. + """ + old_argv = sys.argv + sys.argv = ["dokimi", *argv] try: - args.func(args) + cli.main() except SystemExit as exc: code = 0 if exc.code is None else exc.code else: code = 0 + finally: + sys.argv = old_argv captured = capsys.readouterr() return code, captured.out, captured.err