Repository navigation
fix(eval): infra errors and empty replies are counted as errors, never scored - #280
Conversation
An answer whose call failed was tallied with the answers no check could score, so the summary's unscored count mixed provider failures with grading gaps. It is now counted as errors, per condition in eval reference and per arm in arms summaries. Errors leave n, the pass rate and the mean out, as unscored answers do; an arms cell whose answer errored or whose judge raised is no longer a silent None. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A stored empty answer was replayed on every later run as a failed answer, so a one-off empty reply stuck. keeps() now refuses empty and whitespace-only text on store and on replay; a rerun asks the model again. A content-less reply (None, stored as null) is kept as before. Docs and changelog cover both commits of this branch. Note for replay-only runs: a cache that holds an empty reply now raises CacheMiss for that call instead of replaying the empty answer. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
keeps() treated None as acceptable, so a reply with no content was stored as null and replayed as "no answer" on every later run. It is now refused on store and on replay, like an empty or whitespace-only reply, and a rerun asks the model again. The one shared predicate is caching.is_empty_reply. No caller of complete() offers tools, and chat() (the tool-calling path) is never response-cached, so no tool-call-only reply is lost. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A single-turn arm whose reply is empty or whitespace (or content-less) was recorded as an answer: the judge's own empty check skipped it silently (no score, not counted) or judged a whitespace reply. It is now an error cell, empty_reply, as a session that stopped on an empty turn already was; the call's usage still counts. The judge treats an empty answer the same way, with the shared is_empty_reply predicate. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The judge's exception handler stored str(exc). Some exceptions have an
empty message (TimeoutError()), so the cell held {"error": ""}; the record
reader drops an empty error, and the cell was neither scored nor counted as
an error. The exception's class name is the error when the message is empty.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
eval reference passed an empty or whitespace-only reply to the checks as text, so a negated check scored it 1.0 and a plain one 0. Only a missing reply was treated as no answer. An empty reply is now an errored answer: it leaves n, the pass rate and the mean out, and is counted as an error, as an errored call is. The emptiness test is the shared is_empty_reply. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…cored The headline delta was the difference of two unpaired means, so a question one arm errored on moved the delta (the repro gave -8.3 where the paired difference over the two shared questions is -10.0). It is now the mean of arm minus baseline over the questions both arms scored, via paired_delta. The unpaired avg_aggregate is still reported. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The empty-reply cells added to arm_runner put it at 809 lines (cap 800) and pushed ArmRunner.judge past its complexity baseline. The unjudged cell (an error for an errored or empty answer, None for no answer) moves to arm_summary.unjudged_cell, and the single-turn empty-reply return is shorter. No behaviour change. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ors count, delta is paired Describes the four fixes on this branch: empty reply as an error cell in arms and eval reference, the judge exception with an empty message, the response cache refusing content-less replies, and the paired delta_vs_baseline. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…y empty In eval reference, a judge check whose call raised, or whose reply had no YES or NO, was dropped, and the checks that ran decided the pass. The answer is now an error: no score and no pass, counted in errors, with the reason on the block as `error`. The checklist report keeps its per-point behaviour; the strict mode is opt-in. Arms: emptiness is read on the model's reply before the output shapers run, so a footer added to an empty reply is not an answer. delta_vs_baseline is always present (null when no question has both arms scored). The arms summary shows the errors count in the Markdown table and the console summary. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
==========================================
+ Coverage 93.68% 93.69% +0.01%
==========================================
Files 277 277
Lines 35954 36016 +62
Branches 6092 6107 +15
==========================================
+ Hits 33683 33745 +62
Misses 1463 1463
Partials 808 808
🚀 New features to boost your workflow:
|
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 16710be. The single-turn and eval reference paths hold up: the strict judge, the errors/unscored split and the paired delta behave as described. Four things to fix, the first two before merge: the agentic arm still scores an empty reply once an output shaper is active, which is the case this PR sets out to close.
What I ran, on a Linux review host at 16710be:
make ci-local: 26 passed, 2 failed.Run tests: 9719 passed, 4 skipped, 1 failed. The failure istest_unbalanced_quote_in_hook_completes_quickly(1.11 s against a 1 s bound), which passes when run alone and sits in a file this PR does not touch.pip-auditfails because it reports known vulnerabilities in the lock files, which this PR does not touch. The changed-line coverage step passed.- Reproductions for the code findings, with the LLM transport mocked.
- A mutation run of the new tests (about 37 mutants across the reference, cache and arms code): the survivors are the replay guard inline below, the per-check
errorserialisation and the baseline'sdelta_vs_baseline: null. - All ten commits carry
Signed-off-by.
Findings outside the diff:
- Nit: three places still describe the old behaviour.
dokimi/docs/evaluating-treatments.md:720-724anddokimi/scripts/check_treatment_batch.py:15-17say the artifact'sdelta_vs_baselineaverages each arm over what it scored, so a failed call subtracts means over different questions. It is now the paired mean.dokimi/dokimi/task_formats/imports/native_logs.py:59still says a failed sample "stays unscored ineval reference"; it is counted undererrorsnow. Suggestion: reword the three to matchanswer-formats.md.
Checked, no issues
- Schemas:
errorsundersummary.per_armis an optional integer, so wideningarms.v1.jsonin place needs no version bump.delta_vs_baselinewas alreadynumber|null.schema_compatagainst the base passes. - Cache callers: the only
replay_onlyusers,judge.pyandpanel.py, passcache_if=parses, which already rejects empty text, so the newkeepsrule does not change their redraws. The only othercheck_answercaller stays non-strict. - Precedence: an answer's own error wins over
empty_reply, and a no-retrieval placeholder without an error stays unscored. Both orders are pinned. - Repo rules: growth ratchet holds (
arm_runner.pyis at 797 of 800 lines), no generated artifacts, no new doc pages, new tests write nothing outsidetmp_path.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to d13fcb3, which fix the two nits from my review:
21ddc76:dokimi/dokimi/report/arms_report.py:168: an absenterrorscount now prints—instead of0.d13fcb3:dokimi/docs/evaluating-treatments.md:720,dokimi/scripts/check_treatment_batch.py:15anddokimi/dokimi/task_formats/imports/native_logs.py:59: reworded to saydelta_vs_baselineis paired and a failed sample is counted undererrors.
make ci-local at d13fcb3: 27 passed, 1 failed (pip-audit, as at the reviewed head), 7 skipped.
Not approving yet. Still open: the bug and the risk from my review.
|
🔍 OpenCodeReview found 1 issue(s) in this PR. By severity: 1 low
Review details
|
An empty or null entry stored before the rule must not be replayed as the answer: complete() asks the model again. The test fails when the replay guard is reduced to cache_if alone. Thread: PRRT_kwDOVBg7g86xh-Dg Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
A row without an errors key reads as unknown, not as a false zero, and a real zero stays a zero. The test fails when the fallback is changed to 0. Thread: PRRT_kwDOVBg7g86xlq7F Review: PRR_kwDOVBg7g88AAAABU7EbFQ Comment: IC_kwDOVBg7g88AAAABg13P_A Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…hapers run _answer_agentic_arm applied the output shapers before any emptiness check, so a footer added to nothing made the text non-empty and the judge scored it. Read the loop's reply first, as the single-turn path does, and build both error cells with one empty_reply_cell in arm_summary.py (arm_runner.py is at its 800 line limit). Covers the answered and max_iterations stops. The test is red on the old code. Thread: PRRT_kwDOVBg7g86xh-DN Review: PRR_kwDOVBg7g88AAAABU5TpUw Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The docs nit of the review (three places describing the old behaviour) was fixed by the reviewer in d13fcb3; no change in this round. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
This review has no text of its own; its inline nit ( |
|
No change in this PR: the report says all modified and coverable lines are covered. Installing the Codecov GitHub app is an organization setting outside this PR. |
|
The one inline comment of this summary ( |
| @@ -298,7 +298,7 @@ def test_usage_values_of_every_field_type_replay(monkeypatch, tmp_path): | |||
|
|
|||
|
|
|||
| def test_null_text_is_replayed_not_a_miss(monkeypatch, tmp_path): | |||
There was a problem hiding this comment.
Nit: the test name still says the null entry is "replayed, not a miss", but the new rule is the opposite (complete() refuses to replay it via keeps). The name and the docstring now contradict each other. Rename it, e.g. test_a_null_entry_still_loads. The test also doesn't cover replay-only mode: an empty or null stored entry there now raises CacheMiss instead of replaying. Add a case in test_llm.py for it.
There was a problem hiding this comment.
Fixed in fcc5698.
Renamed the test to test_a_null_entry_still_loads.
Added test_an_empty_entry_is_a_miss_in_replay_only in test_llm.py, over None, "" and whitespace. In replay-only mode a stored empty or null entry raises CacheMiss and is not replayed. The test fails when the keeps() replay guard is removed.
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at cbb879f. The fix is right: an errored, empty or content-less reply and a failed judge call no longer become a score or a pass, in single-turn, session and agentic arms and in eval reference, and delta_vs_baseline is paired. What is left: the changelog files changes that move numbers under Fixed and does not say old runs are not comparable, and a few smaller points in the line comments.
What I ran, at cbb879f:
make ci-local --offline: 0 failed, 17 passed, 13 skipped (network and Docker jobs).- At 16710be, the changed test files on the merge-base: 19 of the new tests fail there, so they cover the new behaviour.
summarize_skill_selectionon an agentic cell that loaded the expected skill, as an empty answer before f3717a9 and as theempty_replyerror cell after it.
Checked, no issues
- Agentic empty reply: f3717a9 checks the loop's reply before the shapers and keeps the usage, the transcript and
tool_usageon the error cell, so cost and tool-use rollups still count the run. - Cache:
keeps()refuses empty andNonereplies on store and on replay; c026063 pins the replay half. - Report and docs: an absent
errorscount prints—, and the docs andcheck_treatment_batch.pynow describe the paired delta. eval referencetallies: an error grade has no score and no pass and goes toerrors, apart fromunscored; a no-retrieval placeholder stays unscored.- Schema:
errorsinarms.v1per-arm stats is a new optional integer, so widening in place is fine.
| @@ -0,0 +1,11 @@ | |||
| ### Fixed | |||
There was a problem hiding this comment.
This PR moves published numbers for the same inputs, but the fragment files everything under Fixed and does not say that old runs are not comparable.
delta_vs_baseline is now paired, so the same evaluations give a different delta whenever one arm errored on a question. In eval reference, a judge timeout or a reply with no YES/NO used to let the other checks decide the pass, and an empty reply was graded as "" (a negated check passed it); now both are errors and leave n, the pass rate and the mean. An agentic arm's empty reply is now an error cell too, which also takes it out of skill_selection_summary. changelog.d/README.md puts "anything that moves scores for the same inputs" under Changed, and the new errors fields and the Errors column are Added.
Suggestion: move the delta, eval reference judge/empty-reply and agentic bullets to a ### Changed section and say that pass rates, deltas and selection rates from earlier runs are not comparable with new ones (re-run arms judge / eval reference to compare); put the errors field and column under ### Added.
There was a problem hiding this comment.
Already fixed in b301b5a: the fragment now files the score-moving changes under Changed, says earlier pass rates, deltas and selection rates are not comparable, and keeps the errors field and column under Added.
| if is_empty_reply(raw_answer): # read before the shapers: a footer added to nothing is not an answer | ||
| return empty_reply_cell( | ||
| rec, cfg_metadata, t0, agentic=True, transcript=result["transcript"], | ||
| tool_call_count=result["tool_call_count"], tool_usage=result.get("tool_usage", {}), | ||
| iterations=result["iterations"], stopped_reason=result["stopped_reason"]) |
There was a problem hiding this comment.
An agentic empty reply is now an error cell, so summarize_skill_selection counts it as a failed run and drops the skill choice that the transcript did record.
The cell keeps tool_usage, but summarize_skill_selection skips every cell with error (arm_summary.py:150). For an agent that loaded the expected skill and then gave an empty final reply (e.g. a max_iterations stop), the selection summary goes from n: 1, n_failed: 0, selection_accuracy: 1.0 before f3717a9 to n: 0, n_failed: 1, selection_accuracy: None now (checked by calling summarize_skill_selection on both cells). Which skill was picked does not depend on the final text, so routing loses a measured data point, and selection rates move for the same transcripts.
Suggestion: either keep counting the selection of an empty_reply cell that has tool_usage (only the answer is missing), or say in the changelog that such runs now count under n_failed.
There was a problem hiding this comment.
| result = grade_check(check, question, answer or "", ask) | ||
| out.checks.append(result) | ||
| if out.error is None and result.error is not None: | ||
| out.error = result.error |
There was a problem hiding this comment.
Once a judge check fails and the grade is an error, grade() still runs every remaining judge check.
The grade can no longer change: it has no score and no pass. Each further judge check is up to two polarities times the retry draws of paid calls, so an answer with 4 judge checks whose first one timed out still sends the other 3 checks' calls, usually into the same outage.
Suggestion: after out.error is set, append the remaining judge checks as CheckResult(kind, None, None, weight, "not judged: an earlier judge check failed") without calling the judge (deterministic checks can still run).
There was a problem hiding this comment.
Already fixed in 0f31dad: after a failed judge check the remaining judge checks are recorded as not judged and make no call (test_reference_checks pins the detail string).
| "elapsed_sec": round(time.time() - started, 2), **extra} | ||
|
|
||
|
|
||
| def paired_delta(evaluations: Iterable[Any], baseline_arm: str, arm_name: str) -> Optional[float]: |
There was a problem hiding this comment.
Nit: paired_delta repeats the complete-case pairing of dokimi.eval.stats.paired_vectors.
The same rule now lives in two places, and significance.delta_mean is computed from the other one, so a later change to one (how a NaN aggregate is treated, say) makes delta_vs_baseline and delta_mean disagree.
Suggestion: build it from paired_vectors(evaluations, arm_name, baseline_arm) and take the mean of the differences.
There was a problem hiding this comment.
Already fixed in 2170fb8: paired_delta reads paired_vectors, so the complete-case rule lives in one place.
| # A content-less reply (None) is not stored: keeps() refuses it, so it is never replayed. | ||
| text = cast(str, reply.text) |
There was a problem hiding this comment.
Nit: keeps() now handles None on purpose, but it is still typed text: str and the callers cast the text to str to fit.
The docstring has to say "text is None at runtime ... whatever its annotation says", and mypy cannot see the None case, so a later change that calls cache_if(text) before the empty check type-checks and crashes on None.
Suggestion: type it text: Optional[str] (as is_empty_reply is) and drop the cast here and at line 713.
There was a problem hiding this comment.
Fixed in d3fae29: keeps() is typed text: Optional[str] and refuses None before cache_if runs, so the cast calls in complete() are gone. Reply.text is passed through and narrowed by a None test before the store. test_caching.py pins the annotation.
…erence Once a judge call fails the grade is an error, so no further judge check can change it. The remaining judge checks are now recorded as not judged instead of each sending its paid calls, usually into the same outage. Deterministic checks still run. Thread: PRRT_kwDOVBg7g86xypBx Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…on summary The loop loads its skills before the final reply comes back empty. That reply is an error cell, and the selection summary skipped every errored cell, so the choice the transcript recorded was dropped. The run still counts under n_failed; its selection now counts under n. Thread: PRRT_kwDOVBg7g86xypBu Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The complete-case rule lived in both paired_delta and stats.paired_vectors, so a later change to one could make delta_vs_baseline and the significance delta_mean disagree. paired_delta now takes its pairs from paired_vectors. The test fails until paired_delta uses it; the numbers do not change. Thread: PRRT_kwDOVBg7g86xypBy Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…mplete() keeps() was annotated text: str while its callers passed a content-less reply's None, so complete() cast the text twice to fit. It is now typed Optional[str] and refuses None before cache_if runs, and complete() passes Reply.text through, narrowed by a None test before the store. Thread: PRRT_kwDOVBg7g86xypB0 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… the null-entry test The null-entry test was named "replayed, not a miss" while the rule it pins is the opposite: the entry loads, and complete() refuses to replay it. Rename it. Add the replay-only case: an empty or null stored entry raises CacheMiss instead of replaying, so a replay-only run asks for a fresh answer. The new test fails when the keeps() replay guard is removed. Thread: PRRT_kwDOVBg7g86xtVB4 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…Changed Changes that move pass rates, deltas or selection rates for the same inputs go under Changed, as changelog.d/README.md says, with a note that earlier figures are not comparable. The errors fields and the Errors column are Added. The fragment has no test to fail; this is a prose change. Thread: PRRT_kwDOVBg7g86xypBs Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 21 selected item(s). Review details
|
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| assert caching.keeps(lambda _text: True, "") is False | ||
|
|
||
|
|
||
| def test_keeps_is_annotated_for_the_none_of_a_content_less_reply(): |
There was a problem hiding this comment.
This test asserts only the annotation, not behavior, so it checks no runtime outcome. mypy already enforces this, and test_a_content_less_reply_is_never_stored_or_replayed covers the None behavior. Consider dropping it.
There was a problem hiding this comment.
Keeping this one: it pins the Optional[str] annotation that d3fae29 relies on, so a re-added cast or a changed annotation fails a test. mypy would not notice a cast that keeps type-checking. Happy to drop it if you prefer the type-only check.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at fb60380. The delta since my last review holds up. The agentic empty reply is an error cell before the shapers, the replay half of keeps is pinned, the absent errors count reads —, and the paired delta, the judge-failure-as-error rule and the remaining-judge-check skip all behave as described. One risk on the new agentic cell: nothing pins what it keeps for the skill-selection summary.
What I ran, on a Linux review host at fb60380:
make ci-local: 28 passed, 1 failed, 7 skipped. The failure isRun tests: 10663 passed, 4 skipped, 1 failed. The failed test istest_the_regex_limit_is_shared_by_every_pattern_of_one_check, a wall-clock bound (1.53 s against 1.5 s); it passes when run alone, and the PR does not touch it. The changed-line coverage step,mypy,schema_compatand the growth ratchet passed.- Reproductions for the code findings, with the LLM transport mocked.
- A mutation run of the new tests (about 52 mutants across the reference, cache, arms and summary code). The survivor that matters is the one inline.
- All 23 non-merge commits carry
Signed-off-by.
Checked, no issues
- Changelog: the score-moving items now sit under
Changed, theerrorsfield underAdded, as the changelog README asks. - Judge failure: a judge outage gives
errors=1,n=0, and anerroron the check and on the cell; anegatecheck never turns a failed call into a pass; later judge checks make no call. - Empty replies:
"", whitespace andNoneare never graded, judged, stored or replayed; in replay-only mode a stored empty entry raisesCacheMiss. - Paired delta:
delta_vs_baselineand the significance delta both readpaired_vectors, so they use the same questions.
| if is_empty_reply(raw_answer): # read before the shapers: a footer added to nothing is not an answer | ||
| return empty_reply_cell( | ||
| rec, cfg_metadata, t0, agentic=True, transcript=result["transcript"], | ||
| tool_call_count=result["tool_call_count"], tool_usage=result.get("tool_usage", {}), |
There was a problem hiding this comment.
Nothing pins what the agentic empty_reply cell keeps for the skill-selection summary.
The point of keeping the cell's tool_usage is that summarize_skill_selection still scores the skill the agent loaded before its reply came back empty (arm_summary.py:161). Mutation check: dropping tool_usage=result.get("tool_usage", {}) from this call leaves tests/test_arm_cell_errors.py and tests/test_skill_selection.py at 75 passed. test_an_agentic_empty_reply_stays_an_error_cell_when_an_output_shaper_adds_text feeds the loop "tool_usage": {}, and tests/test_skill_selection.py:119 builds its cell by hand, so neither goes through this return.
If tool_usage is lost, first_turn_loaded is empty. An agent that loaded the right skill then counts as having loaded none, and selection_accuracy moves with no test failing.
Suggestion: give the fake loop result a tool_usage with a first-turn skill load, assert cell["tool_usage"] equals it, and run summarize_skill_selection([rec]) on the record to assert the choice is scored.
There was a problem hiding this comment.
Fixed in a3a74df: the agentic empty-reply test now gives the loop a first-turn skill load, asserts cell tool_usage equals it, and runs summarize_skill_selection on the record. Dropping tool_usage from the runner call fails it (red), and it passes on the branch (green).
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at fb60380. All six points from my last review are fixed, and the new commits bring no new problems. One test gap is still open.
Agreed with @antonwolfy: no test checks that an agentic empty_reply cell keeps its tool_usage. The agentic empty-reply test passes "tool_usage": {} (tests/test_arm_cell_errors.py:148), and the new skill-selection test builds its cell by hand.
Agreed with @exai-bot: the new test in test_caching.py only checks the annotation of keeps(), which mypy already does.
What I ran, at fb60380:
slot ci: 18 passed, 0 failed, 13 skipped.
Checked, no issues
- Changelog: the changes that move the numbers are now under
Changed,errorsis underAdded, and a line says earlier figures are not comparable and how to re-run them. - Empty agentic reply in routing metrics: an
empty_replycell now counts toward the skill-selectionn, and the run still counts inn_failed. So the same run now shows in bothnandFailedof the Skill selection table. The commit message says this is on purpose, and I agree with it. - Judge checks after a judge error: the later judge checks are recorded as not judged and are not called. The deterministic checks still run.
paired_delta: it now takes its pairs fromstats.paired_vectors.keeps()typing: it takesOptional[str], and bothcastcalls incomplete()are gone. The renamedtest_a_null_entry_still_loadsand the new replay-only case intest_llm.pytest what their names say.
… summary scores Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/tests/test_caching.py
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
LGTM
I checked d604fbb. Everything from my earlier rounds is fixed, and so are the open points from @antonwolfy and @exai-bot. An errored, empty or content-less reply and a failed judge call never become a score or a pass, and delta_vs_baseline is paired.
Merge: safe now: it merges cleanly into current main, 1 commit behind (a workflow file only). Once it is in, #255 conflicts with it in llm.py and needs a rebase.
What I ran, at d604fbb:
make ci-local --offline(lint, types, the full test suite, coverage of changed lines, docs build): 0 failed, 19 passed, 13 skipped (network and Docker jobs).
Checked, no issues
- Agentic skill choice: a3a74df runs an empty agentic reply through the runner with a first-turn skill load, so the test fails if the cell loses its
tool_usage. - Merge of main: the
test_caching.pyconflict with #178 is resolved, and the annotation-only test is gone. The behaviour tests for aNonereply stay.
Summary
Part of #220.
eval referenceand arms runs stop turning failed or empty answers into scores.errors. The other checks no longer decide the pass. The block carrieserror, as an arm cell does.errors, notunscored. An empty or content-less reply is never graded, judged or cached.delta_vs_baselineis paired over the questions both arms scored, and is always present (nullwhen there are none).Evidence
delta_vs_baselinekey missing; reference cells had noerror; the report had no errors column) and pass on this one. Files:tests/test_reference_checks.py,tests/test_arm_cell_errors.py,tests/test_arms_report.py."", an empty single-turn arm reply scored, a judge exception with an empty message dropped, content-less replies cached, an unpaired delta. Round 3: the findings above.16710beb, run directly (make ci-localwas not run; its schema, workflow and bandit gates are not covered here):ruff checkclean on the touched files;mypy:Success: no issues found in 271 source files.pytest -n auto: 9673 passed, 7 skipped, 1 failed:test_probe_reports_a_writable_docker_socket(AF_UNIX path too long under the box's temp dir).intel/maintoo (9670 passed, 1 failed). It passes alone on both trees, so this is pre-existing and environmental, not from this PR.intel/maininto this head: 9699 passed, 7 skipped.Merge Danger
eval referencegraded an empty answer as 0 (or 1.0 under a negated check), and a single-turn arm left it unscored. A judge failure used to be decided by the other checks. Rerun those artifacts before comparing pass rates.Nonereply is cached. They are replaced:test_a_content_less_reply_is_never_stored_or_replayed(tests/test_caching.py), and the refactor(llm): one LLM module behind every call; old entry points delegate #78 test, nowtest_a_reply_without_content_is_asked_again_not_replayed(tests/test_llm.py). A replay-only run that reaches an empty reply raisesCacheMissfor that call; judge and panel redraws move on to the next draw.errorsappears insummary(eval) and inper_arm(arms, schema widened).delta_vs_baselineis now always present. The arms Markdown table gains anErrorscolumn.dokimi/eval/judge.py:320anddokimi/eval/answerer.py:182still store a barestr(e), so an empty-message exception there is dropped as before. Legacy paths, not the arm path.🤖 Generated with Claude Code