diff --git a/dokimi/changelog.d/20261114_120000_empty-json-input-path.md b/dokimi/changelog.d/20261114_120000_empty-json-input-path.md new file mode 100644 index 000000000..9647af2cc --- /dev/null +++ b/dokimi/changelog.d/20261114_120000_empty-json-input-path.md @@ -0,0 +1,3 @@ +### Fixed + +- **An empty path given to a JSON input flag is refused by name** — `eval score reference --answers ""` exits 2 with `Error: --answers "": no file given`, where it read `.` and said `--answers : Is a directory`. Every other flag and argument that reads a JSON file through the shared reader gets the same refusal, and a read without a flag says `cannot read "": no file given`. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 5c1c0de52..313c6e9cc 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -1962,7 +1962,7 @@ pattern. Other slow shapes pass and can still take long on a long claim, such as `maintenance/.*a.*a.*a.*b`. When `.pre-commit-config.yaml` or one of those workflows was not read, the reason of a claim that stays `unresolved` here also names it, as ` was not read ()` after the rest, in tree order and once -even when the Action note above names it too. A link or path into a submodule is `unresolved`. A +even when the Action note above names it too. A link or path into a submodule is `unresolved`, and so is a bare name that matches only a submodule (`mod/` for `third_party/mod`). A pinned snapshot's submodules are its `.gitmodules` paths and every gitlink it holds, declared or not. A blob or tree link to `source.url` holds `/` and the ref may contain slashes. A default branch, a commit or a version tag is one segment; any other ref is tried at each slash, shortest first, and the link is `ok` if some diff --git a/dokimi/dokimi/questions/artifact.py b/dokimi/dokimi/questions/artifact.py index 93ab20027..01747bc1e 100644 --- a/dokimi/dokimi/questions/artifact.py +++ b/dokimi/dokimi/questions/artifact.py @@ -230,8 +230,9 @@ def load_questions(path: Path | str, *, validate: bool = True, flag: str | None :func:`dokimi.utils.read_json_input`: an unreadable, non-UTF-8 or non-JSON file raises :class:`~dokimi.utils.InputError` `` : ``. Without it the read is bare. """ + # the raw value goes to the reader: ``Path("")`` is ``.``, which would hide an empty path + raw = json.loads(Path(path).read_text(encoding="utf-8")) if flag is None else read_json_input(path, flag=flag) path = Path(path) - raw = json.loads(path.read_text(encoding="utf-8")) if flag is None else read_json_input(path, flag=flag) data, bare_list = _records(raw, path) data = _drop_v1_open_nulls(data, _v1_open_nulls()[0]) current = SCHEMA_VERSIONS[KIND] diff --git a/dokimi/dokimi/utils.py b/dokimi/dokimi/utils.py index 9c0aa97ec..7b9993ab9 100644 --- a/dokimi/dokimi/utils.py +++ b/dokimi/dokimi/utils.py @@ -773,7 +773,11 @@ def read_json_input(path: str | os.PathLike[str], *, flag: str | None = None) -> Covers a missing or unreadable file, a device (:func:`refuse_device`), 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, `` : ``. + An empty path is refused first, quoted (``--answers "": no file given``): ``Path("")`` is the + current directory, which would read as ``Is a directory`` with nothing after the flag. """ + if os.fspath(path) == "": + raise InputError(f'{flag} "": no file given' if flag else 'cannot read "": no file given') refuse_device(path, flag) try: return json.loads(Path(path).read_text(encoding="utf-8")) diff --git a/dokimi/tests/test_catalog_path_missing.py b/dokimi/tests/test_catalog_path_missing.py index 0051bfd83..73cfc07cf 100644 --- a/dokimi/tests/test_catalog_path_missing.py +++ b/dokimi/tests/test_catalog_path_missing.py @@ -236,6 +236,13 @@ def test_a_missing_escaped_comma_is_named_whole(tmp_path): assert finding.message.startswith("'assets/a,b.svg' (in ") +def test_a_missing_escaped_semicolon_is_named_whole(tmp_path): + # outside quotes, an escaped ``;`` is no shell operator: the name is not cut at it + review, _ = _review(tmp_path, r"Copy it with `cp assets/a\;b.svg /tmp/out`." + "\n") + [finding] = _path_findings(review) + assert finding.message.startswith("'assets/a;b.svg' (in ") + + def test_an_escaped_quote_does_not_end_a_quoted_name(tmp_path): review, _ = _review( tmp_path, diff --git a/dokimi/tests/test_cli_output.py b/dokimi/tests/test_cli_output.py index e8ee19e3a..abe3caadd 100644 --- a/dokimi/tests/test_cli_output.py +++ b/dokimi/tests/test_cli_output.py @@ -515,3 +515,44 @@ def test_a_failed_scan_with_no_incomplete_reason_still_says_what_failed(): # without --fail-on-fail, failed claims and an incomplete scan are not what failed the run assert _scan_reason({"incomplete": True, "incomplete_reasons": ["x"], "counts": {"fail": 2}}, ["bad"], False) == \ "1 --static problem(s) not merged" + + +def _leaf_with_json_and_jsonl() -> cli._LeafParser: + leaf = cli._LeafParser(prog="dokimi probe") + leaf.add_argument("source") + leaf.add_argument("--json", action="store_true") + leaf.add_argument("--jsonl") + leaf.set_defaults(emits_cli_output=True) + return leaf + + +def test_a_prefix_that_is_ambiguous_at_a_leaf_is_not_a_json_request(): + # no shipped leaf has --json next to another --js* option (catalog check's --jsonl has no --json): a probe leaf does + leaf = _leaf_with_json_and_jsonl() + assert cli._json_requested(leaf, ["--jso"]) is False + assert cli._json_requested(leaf, ["--json"]) is True + check = cli._leaf_at(cli.build_parser(), ["catalog", "check"]) + assert check is not None and cli._json_requested(check, ["--jso"]) is False + + +def test_an_ambiguous_json_prefix_at_a_leaf_prints_no_document(capsys): + cli_output.reset() + with pytest.raises(SystemExit) as exc: + _leaf_with_json_and_jsonl().parse_args(["--jso"]) + assert exc.value.code == 2 + out = capsys.readouterr() + assert out.out == "" and "ambiguous option: --jso" in out.err + + +@pytest.mark.parametrize("argv, message", [ + (["repo", "validate"], "the following arguments are required: source"), + (["repo", "validate", ".", "--cache-dir"], "argument --cache-dir: expected one argument"), +]) +def test_a_leaf_usage_error_without_json_prints_its_message_on_stderr_only(monkeypatch, capsys, argv, message): + # leaf.error runs for these (unlike an unrecognized --bogus, which the root reports): the flag gates its document + cli_output.reset() + with pytest.raises(SystemExit) as exc: + _run_main(monkeypatch, argv) + assert exc.value.code == 2 + out = capsys.readouterr() + assert out.out == "" and message in out.err diff --git a/dokimi/tests/test_load_or_fail.py b/dokimi/tests/test_load_or_fail.py index d90cd4bab..db4e2de37 100644 --- a/dokimi/tests/test_load_or_fail.py +++ b/dokimi/tests/test_load_or_fail.py @@ -69,3 +69,34 @@ def test_a_path_with_a_nul_byte_is_an_input_error_naming_it(): with pytest.raises(InputError) as exc: read_json_input("bad\x00name.json") assert str(exc.value).startswith("cannot read bad") + + +@pytest.mark.parametrize("flag, message", [ + ("--answers", '--answers "": no file given'), + (None, 'cannot read "": no file given'), +]) +def test_an_empty_path_is_refused_quoted_not_read_as_the_current_directory(flag, message): + from dokimi.utils import read_json_input + + with pytest.raises(InputError) as exc: + read_json_input("", flag=flag) + assert str(exc.value) == message + + +def test_eval_score_reference_with_an_empty_answers_value_exits_2_naming_it(monkeypatch, capsys, tmp_path): + from dokimi import cli + + monkeypatch.setattr("sys.argv", ["dokimi", "eval", "score", "reference", "--answers", "", + "--questions", "q.json", "-o", str(tmp_path / "out.json"), "--no-judge"]) + with pytest.raises(SystemExit) as exc: + cli.main() + assert exc.value.code == 2 + assert capsys.readouterr().err.strip() == 'Error: --answers "": no file given' + + +def test_load_questions_refuses_an_empty_path_by_name_not_as_the_current_directory(): + from dokimi.questions.artifact import load_questions + + with pytest.raises(InputError) as exc: + load_questions("", flag="--questions") + assert str(exc.value) == '--questions "": no file given' diff --git a/dokimi/tests/test_novelty_checks.py b/dokimi/tests/test_novelty_checks.py index 8b9325965..dde09b26e 100644 --- a/dokimi/tests/test_novelty_checks.py +++ b/dokimi/tests/test_novelty_checks.py @@ -17,7 +17,7 @@ from dokimi.novelty.client import LLMClient, SpendCap from dokimi.novelty.grounding import (add_checks, help_identifiers, help_text, identifiers_check, verify_keys) -from dokimi.novelty.measure import resolve_verdict, run_checks, run_measurement +from dokimi.novelty.measure import MeasureResult, ProbeRun, resolve_verdict, run_checks, run_measurement from dokimi.novelty.probes import build_probes from dokimi.utils import InputError @@ -702,3 +702,57 @@ def test_each_grounding_step_refuses_a_malformed_grading(step): "verify_keys": lambda: verify_keys(atoms)}[step] with pytest.raises(InputError, match="atom a1: invalid grading: grading has no checks"): run() + + +@pytest.mark.parametrize("answer", [ + "Do not use perf script but perf report --stdin.", + "Never guess flags while you run perf report --stdin.", +]) +def test_a_negation_does_not_reach_the_command_after_a_but_or_a_while(answer): + # "but" and "while" end the clause: the "not"/"never" before them negates the other command, not this one + result = grade_check(_veto(), "q", answer, None) + assert result.passed is False and "--stdin" in result.detail + + +@pytest.mark.parametrize("answer", [ + "Do not use perf script or perf report --stdin.", + "Never guess flags when you run perf report --stdin.", +]) +def test_a_negation_still_reaches_the_command_after_a_word_that_ends_no_clause(answer): + # the control of the test above: "or" and "when" are no clause end, so the same claim is negated + assert grade_check(_veto(), "q", answer, None).passed is True + + +@pytest.mark.parametrize("negated, claimed", [ + ("Avoid perf report --stdin.", "Try perf report --stdin."), + ("Avoiding perf report --stdin is wise.", "Using perf report --stdin is wise."), + ("Profile it without perf report --stdin.", "Profile it with perf report --stdin."), +]) +def test_avoid_avoiding_and_without_negate_the_command_right_after_them(negated, claimed): + # the word alone makes the mention no claim: the same sentence with a neutral word is vetoed + assert grade_check(_veto(), "q", negated, None).passed is True + result = grade_check(_veto(), "q", claimed, None) + assert result.passed is False and "--stdin" in result.detail + + +def _checked_run(question, verdict, judge, source, *checks): + return ProbeRun(atom_id="a1", probe_kind="direct", arm="full", question=question, response="r", + verdict=verdict, judge_verdict=judge, verdict_source=source, + checks=[{"role": role, "passed": passed} for role, passed in checks]) + + +def test_checks_summary_counts_every_field_over_rows_with_known_verdicts(): + runs = [ + _checked_run("vetoed", "wrong_other", "correct_complete", "check", ("veto", False)), + _checked_run("rescued", "correct_complete", "wrong_other", "check", ("sufficient", True)), + _checked_run("agreed", "correct_complete", "correct_complete", "check", ("sufficient", True)), + _checked_run("same side", "correct_complete", "partially_correct", "check", ("sufficient", True)), + _checked_run("left to judge", "partially_correct", "partially_correct", "judge", ("veto", True)), + _checked_run("no checks", "wrong_other", "wrong_other", "judge"), + _checked_run("errored", "error", "error", "judge", ("veto", True)), + ] + summary = MeasureResult(runs=runs).checks_summary() + # 5 checked runs; 3 verdicts differ from the judge's (vetoed, rescued, same side); 1 vetoed, and the judge + # had passed it; 3 decided by a sufficient check, of which 2 match the judge's lenient collapse. + assert summary == {"checked_runs": 5, "overruled": 3, "vetoed": 1, "judge_passed_vetoed": 1, + "decided": 3, "judge_agreed": 2} diff --git a/dokimi/tests/test_novelty_routing_gaps.py b/dokimi/tests/test_novelty_routing_gaps.py index a08fffb4b..e7167f82f 100644 --- a/dokimi/tests/test_novelty_routing_gaps.py +++ b/dokimi/tests/test_novelty_routing_gaps.py @@ -672,3 +672,35 @@ def test_a_keep_copies_under_that_resolves_a_name_adds_no_kept_no_copy_note(tmp_ assert any("kept catalog skill 'db' under a" in note for note in notes) assert not any("kept no copy" in note for note in notes) assert not any("kept catalog skill 'mirror'" in note for note in notes) + + +def _review_nested_in_review(root): + """Two skills named review, one inside the other's directory: no directory holds just one of them.""" + _skill(root, ".", "review", "review a change, the outer copy") + _skill(root, "review", "review", "review a change, the inner copy") + _skill(root, "skills", "cache", "cache slow lookups") + + +def test_skill_routing_says_no_flag_picks_a_copy_when_no_directory_holds_one(tmp_path): + root = tmp_path / "catalog" + _review_nested_in_review(root) + args = _parse("skill", "routing", f"skill:{root}/skills/cache", "--against", + str(root), "--queries-from-headings") + with pytest.raises(InputError) as exc: + args.func(args) + message = str(exc.value) + assert "`review`: no --against or --keep-copies-under picks one of its copies" in message + assert "dedupe or rename them" in message + + +def test_catalog_routing_says_no_keep_flag_picks_a_copy_when_no_directory_holds_one(tmp_path): + root = tmp_path / "catalog" + _review_nested_in_review(root) + queries = _queries(tmp_path, {"cache": ["cache my lookups"]}) + args = _parse("catalog", "routing", str(root), "--queries-by-skill", str(queries), + "--out", str(tmp_path / "o.json")) + with pytest.raises(InputError) as exc: + args.func(args) + message = str(exc.value) + assert "`review`: no --keep-copies-under picks one of its copies" in message + assert "--against" not in message