diff --git a/AGENTS.md b/AGENTS.md index ca6b0e2c..16ad3d41 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,7 +87,8 @@ Registries, bundled in the package (`dokimi/dokimi/`) so they ship in the wheel: - Code that reads answers, eval or arms records goes through `dokimi/dokimi/records.py` (`read`/`load`/`records`), not `.get("with_docs")` on the dict: it is the one place that knows the keys - the writers write. + the writers write. `dokimi/tests/test_records_guard.py` fails a new such read + and lists the exceptions, which only shrink. ## What goes in the tree diff --git a/dokimi/changelog.d/20261023_160100_antonwolfy_records-reader-guard.md b/dokimi/changelog.d/20261023_160100_antonwolfy_records-reader-guard.md new file mode 100644 index 00000000..edfc2bd6 --- /dev/null +++ b/dokimi/changelog.d/20261023_160100_antonwolfy_records-reader-guard.md @@ -0,0 +1,7 @@ +### Changed + +- **Pipeline summary reads through `dokimi.records`** — the per-condition + averages and the delta average that `EvaluationPipeline.run` prints and returns + are now read with `dokimi.records` instead of the `with_docs`/`without_docs` + keys of the evaluation dicts. The numbers are unchanged. A test now holds every + other reader of those keys to a list of exceptions that only shrinks. diff --git a/dokimi/dokimi/orchestrator/pipeline.py b/dokimi/dokimi/orchestrator/pipeline.py index a732b71f..5badfda3 100644 --- a/dokimi/dokimi/orchestrator/pipeline.py +++ b/dokimi/dokimi/orchestrator/pipeline.py @@ -11,6 +11,7 @@ from dokimi.defaults import DEFAULT_ANSWER_MODEL, DEFAULT_ANSWER_PROVIDER, DEFAULT_JUDGE_MODEL, DEFAULT_JUDGE_PROVIDER, DEFAULT_RETRIEVAL_TOP_K from dokimi.mcp.factory import create_doc_source_client +from dokimi.records import BASELINE, CONTEXT, records as read_records logger = logging.getLogger(__name__) @@ -380,9 +381,10 @@ def run(self, concurrency: int = 5) -> Dict[str, Any]: } # Compute summary stats - with_scores = [e["with_docs"]["aggregate"] for e in evaluations if e.get("with_docs")] - without_scores = [e["without_docs"]["aggregate"] for e in evaluations if e.get("without_docs")] - deltas = [e["delta"] for e in evaluations if e.get("delta") is not None] + scored = read_records(evaluations, source="evaluations", kind="evaluations") + with_scores = [s for s in (r.score(CONTEXT) for r in scored) if s is not None] + without_scores = [s for s in (r.score(BASELINE) for r in scored) if s is not None] + deltas = [r.delta for r in scored if r.delta is not None] if with_scores and without_scores and deltas: results["steps"]["evaluation"]["summary"] = { diff --git a/dokimi/tests/test_pipeline_summary.py b/dokimi/tests/test_pipeline_summary.py new file mode 100644 index 00000000..f07b2beb --- /dev/null +++ b/dokimi/tests/test_pipeline_summary.py @@ -0,0 +1,57 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""The summary ``EvaluationPipeline.run`` returns for the judged questions.""" + +from __future__ import annotations + +import json + +import pytest + +from dokimi.orchestrator.pipeline import EvaluationPipeline + + +def _evaluation(qid, with_score, without_score): + delta = None if with_score is None or without_score is None else with_score - without_score + return { + "question_id": qid, + "question_text": f"Question {qid}?", + "with_docs": None if with_score is None else {"aggregate": with_score}, + "without_docs": None if without_score is None else {"aggregate": without_score}, + "delta": delta, + } + + +def _run(tmp_path, monkeypatch: pytest.MonkeyPatch, evaluations): + """Run the pipeline on a cached question set with the LLM steps replaced.""" + monkeypatch.setattr(EvaluationPipeline, "_generate_answers", lambda self, *a, **k: []) + monkeypatch.setattr(EvaluationPipeline, "_evaluate_answers", lambda self, *a, **k: evaluations) + monkeypatch.setattr(EvaluationPipeline, "_generate_report", lambda self, *a, **k: "report") + output_dir = tmp_path / "run" + (output_dir / "personas").mkdir(parents=True) + (output_dir / "questions").mkdir() + (output_dir / "personas" / "oneTBB.json").write_text(json.dumps({"personas": []}), encoding="utf-8") + questions = [{"id": e["question_id"], "question": e["question_text"], "source_type": "manual"} + for e in evaluations] + (output_dir / "questions" / "oneTBB.json").write_text(json.dumps({"questions": questions}), encoding="utf-8") + pipeline = EvaluationPipeline(product="oneTBB", output_dir=output_dir, description="Threading Building Blocks") + return pipeline.run(concurrency=1) + + +def test_summary_averages_the_judged_scores_and_skips_a_failed_question(tmp_path, monkeypatch): + """Each average is over the questions that have that score; a question whose judge + failed (null conditions, no delta) adds to none of them.""" + result = _run(tmp_path, monkeypatch, [ + _evaluation("q1", 80.5, 50), + _evaluation("q2", 59.5, 40), + _evaluation("q3", None, None), + ]) + + assert result["steps"]["evaluation"]["summary"] == {"with_avg": 70.0, "without_avg": 45.0, "delta_avg": 25.0} + + +def test_summary_is_left_out_when_no_question_was_scored(tmp_path, monkeypatch): + result = _run(tmp_path, monkeypatch, [_evaluation("q1", None, None)]) + + assert "summary" not in result["steps"]["evaluation"] diff --git a/dokimi/tests/test_records_guard.py b/dokimi/tests/test_records_guard.py new file mode 100644 index 00000000..03c8efff --- /dev/null +++ b/dokimi/tests/test_records_guard.py @@ -0,0 +1,121 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Answer and eval records are read through ``dokimi.records``, not by their persisted keys. + +``dokimi/records.py`` is the one reader of the ``with_docs`` and ``without_docs`` conditions of +an answers, eval or arms record (see ``AGENTS.md``). This test counts, in each module of +``dokimi/`` but that one, the reads of either key: an ``x["with_docs"]`` subscript that is not +assigned to, and an ``x.get("with_docs")``, ``.pop`` or ``.setdefault`` call. A dict-literal key +and an assignment are writes and do not match. + +:data:`READ_BASELINE` holds the modules whose reads may stay, with their count and the reason +they are not reads of a persisted record. More reads than the count fail, so a new read fails +wherever it is; fewer fail as stale, so the baseline only shrinks. Never raise a count to make a +new read pass: read the record through ``dokimi.records``. + +Not covered: a read through the constants (``rec[records.CONTEXT]``), a ``"with_docs" in x`` +test, and ``.raw`` reads of record fields, which a pattern cannot tell from other ``.raw`` +attributes (``Task.raw`` of ``dokimi.repos`` is a task file, not a record). +""" + +from __future__ import annotations + +import re +from collections.abc import Mapping +from pathlib import Path + +PACKAGE = Path(__file__).resolve().parents[1] / "dokimi" +READER = "dokimi.records" + +_KEY = r"""["'](?:with|without)_docs["']""" +#: A subscript of a condition key not followed by ``=`` (``==`` is a read), or a dict method call on one. +READ = re.compile(rf"\[\s*{_KEY}\s*\](?!\s*=[^=])|\.(?:get|pop|setdefault)\(\s*{_KEY}") + +#: ``module -> (reads, reason)`` for the reads that may stay. Only shrinks. +READ_BASELINE: dict[str, tuple[int, str]] = { + "dokimi.eval.answerer": (2, "a writer counting the answers it just built, before they are persisted"), + "dokimi.eval.question_text": ( + 2, "takes the persisted record dict its judges pass as Record.raw; moving it onto Record changes " + "every judge's call"), + "dokimi.report.generator": (6, "reads the statistics dict it computed from the evaluations, not a record"), + "dokimi.report.model_compare": ( + 17, "reads the per-question score rows extract_scores built, which use the binary score names"), +} + + +def reads(source: str) -> list[int]: + """The line of each read of a condition key in ``source``.""" + return [source.count("\n", 0, m.start()) + 1 for m in READ.finditer(source)] + + +def package_reads(package: Path = PACKAGE, reader: str = READER) -> dict[str, list[int]]: + """``module -> lines`` of every read of a condition key outside ``reader``.""" + found: dict[str, list[int]] = {} + for path in sorted(package.rglob("*.py")): + rel = path.relative_to(package.parent).with_suffix("") + module = ".".join(rel.parts[:-1] if rel.name == "__init__" else rel.parts) + if module != reader and (lines := reads(path.read_text(encoding="utf-8"))): + found[module] = lines + return found + + +def problems(baseline: Mapping[str, tuple[int, str]], found: Mapping[str, list[int]]) -> list[str]: + """A module with more reads than its baseline allows (0 if not listed), or fewer than it counts.""" + out = [] + for module in sorted(set(baseline) | set(found)): + lines, allowed = found.get(module, []), baseline.get(module, (0, ""))[0] + if len(lines) > allowed: + out.append(f"{module}:{lines} reads a record's with_docs/without_docs key ({len(lines)} reads, " + f"{allowed} allowed); read the record through {READER} (records()/record(), Record.score)") + elif len(lines) < allowed: + fix = f"lower its count to {len(lines)}" if lines else "delete its entry" + out.append(f"{module} has {len(lines)} of its {allowed} baselined reads left; {fix} in READ_BASELINE") + return out + + +def test_records_are_read_through_the_records_module(): + found = package_reads() + assert found, f"the scan found no read under {PACKAGE}: it would pass every check" + found_problems = problems(READ_BASELINE, found) + assert not found_problems, "\n".join(found_problems) + + +def test_every_baseline_entry_has_a_reason_and_names_a_real_module(): + for module, (count, reason) in READ_BASELINE.items(): + assert count >= 1 and reason.strip(), module + assert (PACKAGE.parent / (module.replace(".", "/") + ".py")).is_file(), f"{module} names no module" + + +def test_subscript_loads_and_dict_method_calls_are_reads(): + source = ('a = r["with_docs"]["aggregate"]\nb = r.get("without_docs")\nc = r.pop(\'with_docs\')\n' + 'd = r.setdefault("without_docs", {})\nif r[\n "with_docs"\n] == x:\n pass\n') + assert reads(source) == [1, 2, 3, 4, 5] + + +def test_assignments_dict_literals_and_other_keys_are_not_reads(): + source = ('r["with_docs"] = {"answer": "a"}\nrow = {"with_docs": None, "without_docs": None}\n' + 'r["aggregate"], r.get("delta"), r.get(key), r[key]\n') + assert reads(source) == [] + + +def test_the_reader_module_is_exempt_and_a_package_read_is_found(tmp_path): + package = tmp_path / "pkg" + (package / "sub").mkdir(parents=True) + (package / "records.py").write_text("def f(r):\n return r.get('with_docs')\n") + (package / "sub" / "__init__.py").write_text("def g(r):\n return r['without_docs']\n") + (package / "sub" / "mod.py").write_text("X = 1\n") + assert package_reads(package, reader="pkg.records") == {"pkg.sub": [2]} + + +def test_a_new_read_an_extra_read_and_a_stale_count_are_reported(): + baseline = {"pkg.a": (1, "x"), "pkg.b": (2, "y"), "pkg.c": (1, "z")} + found = {"pkg.a": [3, 7], "pkg.b": [1], "pkg.new": [9]} + assert problems(baseline, found) == [ + "pkg.a:[3, 7] reads a record's with_docs/without_docs key (2 reads, 1 allowed); " + "read the record through dokimi.records (records()/record(), Record.score)", + "pkg.b has 1 of its 2 baselined reads left; lower its count to 1 in READ_BASELINE", + "pkg.c has 0 of its 1 baselined reads left; delete its entry in READ_BASELINE", + "pkg.new:[9] reads a record's with_docs/without_docs key (1 reads, 0 allowed); " + "read the record through dokimi.records (records()/record(), Record.score)", + ]