fix(answers,pipeline,judge,gate): Stack P fixes, step 2 of #228 by napetrov · Pull Request #274 · intel/dokimi

napetrov · 2026-10-29T02:56:57Z

Summary

Step 2 of #228, "Fix Stack P in place". Stack P (EvaluationPipeline + Answerer) gets five fixes and one stamp, one commit each. Each fix flips the rows it owns in the step-1 characterization table (tests/test_answering_stack_parity.py, PINNED), so the diff of this PR shows the change in numbers. No step 3 work is in this PR. eval/arms_run.py, commands/arms.py and commands/subjects.py are not touched (PR #270 owns them).

Which scores move, and why

These are the parity-table values, measured on the six-question fixture. No live benchmark was run for this PR; the size and direction of the live shift are not measured here.

Difference	What changes	Row (P, before to after)
D1 top_k	P asks the doc source for `top_k` docs (was the default 5). Retrieved context is smaller, so with-docs answers and their judge inputs change.	`retrieval.max_results` 5 to 3; `retrieval.passed` `("max_tokens",)` to `("max_results", "max_tokens")`
D2/D3 placeholder	With no docs (D2) or a raising reranker (D3), P no longer judges the placeholder with-docs answer `"[FALLBACK: No relevant documentation found]"`. It is recorded, not scored.	`judge.calls` 12 to 10; `judge.contexts.q-empty` and `q-rerank` lose the placeholder with-docs context; `delta.per_question` q-empty and q-rerank 0.0 to None
D9 statistics	Follows from D2/D3: P has 4 paired questions, below `MIN_PAIRS_FOR_TEST` (6). P gets no significance test.	`stats.delta_mean` 13.33 to 20.0 (4 pairs, was 6); `significance.n` 6 to None; `significance.significant` False to None
D8 library id	With no `context7_id`, the pipeline resolves the id through the registry before the client.	`library_id` `client/resolved` to `registry/lib`
D6 judge temperature	`benchmark run` gains `--judge-temperature`. The default is still the provider default, so no default score moves.	`cli.judge_temperature_flag` False to True
stamp	`method_version: 2` in the answers artifact and in the eval artifact of the single `Judge`. No score moves.	`artifact.method_version` 2 (P), None (A)

Direction of the live shift, from the D2/D3 change: the placeholder answer is scored near zero by a judge, so dropping those rows raises the with-docs average and the delta on any live run with retrieval misses. Old and new runs of the same product are therefore not comparable on those numbers. compare does not read method_version yet, so it will not refuse the mix (out of scope, below).

Default behaviour that changes live runs: benchmark run, orchestrate evaluate and eval score no longer judge a placeholder with-docs answer. Pass --judge-placeholder to restore the old judging.

An eval artifact made by eval score on answers from before this PR is stamped method_version: 2. The stamp records the judging method; the answers artifact keeps its own stamp (absent, so version 1). Read the two together; a single eval stamp does not say which answers it judged.

Trust gate: numbers, thresholds not changed

Thresholds are unchanged: min_retrieval_hit_rate 0.60, min_retrieval_coverage 0.80, min_questions 10, max_zero_fraction 0.20, and the rest of DEFAULTS.

What changes is the input of the checks:

min_questions counts the both-arm-scored questions. Placeholder questions (no document, not judged by default) drop out of that count. Fewer counted questions makes the minimum harder to reach, so this is a stricter number change, not a weakening. On the 4-of-14 fixture the count goes from 14 to 10, which still passes at min 10.
zero_score_fraction (fail) counts a question whose retrieval missed as unusable, in the numerator and the denominator. On main a miss was judged at 0 and counted as a zero, so this keeps the fail-severity check failing on the same run. The message says "scored 0 or had no document". Commit c517c727. Regression test: tests/test_trust_gate_unjudged_misses.py fails on the parent commit (4 of 14 passes at 0.0) and passes here.
min_with_docs_avg, min_delta and score_cv (warn) run over the both-arm-scored subset only. The placeholder zeros that main counted are gone from that subset, so these three checks are more lenient than on main. Not changed here; see the decision below.
Retrieval hit rate (warn) runs over every eval record. A question with no document is in the denominator as a miss. Before this PR it was in the rate only when judged. Under default flags the placeholder misses were judged before and counted already, so the net change is small.

Thresholds are not changed, and no retrieval check was added with fail severity.

Probe (probes/gate_probe.py in the review scratch; 14 both-arm questions, mock judge scores a FALLBACK answer 0, otherwise 60; re-run on the head after the fix):

Fixture	Check	Base (intel/main)	Head (this PR)
4 of 14 miss (29%)	`min_questions` (fail, min 10)	14, pass	10, pass
4 of 14 miss	`zero_score_fraction` (fail, max 0.20)	FAIL, 0.286	FAIL, 0.286
4 of 14 miss	`retrieval_hit_rate` (warn, min 0.60)	0.714, pass	0.714, pass
14 of 14 miss	`min_questions` (fail, min 10)	14, pass; zero FAIL 1.0; retrieval FAIL 0.0	0, FAIL; early return, no zero or retrieval check
all hit	all checks	unchanged	unchanged

Net effect: the fail-severity zero-score check gives the same answer on the same run as main (FAIL at 0.286). A run where 29% of questions get no document still fails the gate. The retrieval miss shows as a warn-severity rate that passes at 71%, as before. A run where every question misses still fails, through min_questions, and has no retrieval line.

Adding a fail-severity retrieval check, or changing its threshold, is a separate design decision. It is not in this PR.

Changes

fix(answers): the doc source is asked for top_k docs (was the default). Dead code deleted in the context arm.
fix(benchmark): benchmark run --judge-temperature is parsed and passed to the judge. Batch is not changed.
fix(pipeline): the library id goes through resolve_library_id_for_product (explicit id, then registry context7_id, then client).
fix(judge): the no-retrieval placeholder is not judged by default; --judge-placeholder on benchmark run, orchestrate evaluate and eval score restores it. Trust gate counts all records for retrieval (see above).
fix(gate): zero_score_fraction counts retrieval misses as unusable, so the fail-severity check matches main on the 4-of-14 fixture (commit c517c727, regression test tests/test_trust_gate_unjudged_misses.py). Commit 6bec8910 updates the methodology bullet and parses the evaluations once.
refactor(answerer): removed the unused self.llm client, the unreachable if not docs: branch, and their imports.
feat(artifacts): method_version (defaults.ANSWERING_METHOD_VERSION = 2) on the answers artifact and the eval artifact of the single Judge; documented in answers.v1.json and eval.v1.json. Artifacts without it read as version 1. Nothing reads it yet.
test(parity), test(answerer), test(answerer): lint fix (missing List import); the init tests that only read back attributes replaced by a provider allow-list smoke test (flagged by the test-quality audit); a direct test that top_k reaches the doc source.
docs: methodology trust-gate list says which checks run on both-arm-scored questions; the method_version comment says nothing reads it yet.
Changelog fragment changelog.d/20261028_120000_napetrov_stack-p-step2.md.

The flag registry (dokimi/cli_flags.py) does not exist on main, so the new flags were added to the parsers directly.

Evidence

Parity table: each fix commit flips its rows; the table is the red-then-green record for the numbers above. Red runs for the per-fix commits are in the author's scratch directory, not in this PR.
The new top_k test: fails on intel/main (the probe sends no max_results), passes on the head.
Full suite on the head (6bec8910; the full run was on 3685c29f, and the last commit only changes one comment line, with the targeted gate and docs tests re-run): 9703 passed, 6 skipped.
Regression test tests/test_trust_gate_unjudged_misses.py: fails on the parent commit 0fe7452d (value 0.0, expected 0.286), passes on the head.
ruff check .: clean. ruff format --check reports 482 files that would be reformatted; the project does not gate on ruff format, so this is pre-existing and not changed here.
Self-review (pr-review, local mode): candidates reviewed in review.md of the review scratch.

Merge Danger

Checked by trial merge against intel/main (no rebase needed for this branch: it contains intel/main).

fix(report): report eval, generate and pipeline use one paired arm summary (#229 P1-3) #250 (fix/report-eval-generate-agree, OPEN, BLOCKED): textual conflict in dokimi/dokimi/orchestrator/pipeline.py, the import block at intel/main lines 12 to 14. fix(report): report eval, generate and pipeline use one paired arm summary (#229 P1-3) #250 removes the create_doc_source_client import line; this PR adds resolve_library_id_for_product to it. Resolve by keeping both names. fix(report): report eval, generate and pipeline use one paired arm summary (#229 P1-3) #250 also edits pipeline.py at intel/main lines 383 to 391, near the _generate_* call sites this PR changes; no conflict there, but re-run the parity test after the merge.
feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255 (feat/claude-cli-backend, OPEN, DIRTY): textual conflicts in dokimi/dokimi/eval/answerer.py (the ChatModel import at lines 18 to 19, and the Answerer.__init__ llm block at lines 98 to 110, which this PR removes). Resolve by taking this PR's removal and feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255's provider change.
feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255 auto-merges with this PR in commands/benchmark.py, commands/evaluate.py, commands/orchestrate.py, defaults.py, eval/judge.py. Those are semantic overlaps, not textual ones: the --judge-placeholder and --judge-temperature wiring, and ANSWERING_METHOD_VERSION, sit next to feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255's backend changes. Re-run the suite after either merge.
feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255 conflicts in scripts/growth_baseline.json and tests/test_import_architecture.py against this branch. Those are not this PR's lines: feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255's base predates 50250218 on main (the import contract test). Rebasing feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI #255 onto intel/main clears them.

Both #250 and #255 are ours and the coordinator owns those conflicts; this PR does not resolve them.

Out of scope (listed, not changed here)

benchmark batch has no --judge-temperature.
compare does not read method_version yet, so it does not refuse mixed versions.
eval panel-score (JudgePanel) still judges the placeholder answer and writes its artifact without method_version. The panel's trust numbers therefore differ from the single judge's on the same answers.
A registered product without context7_id now raises ValueError for clients that require a declared id, in question generation and answer generation (the registry-first lookup). Intended, but a hard stop where the client used to be asked.
Trust-gate decisions for the maintainers: (a) the warn-severity score checks (min_with_docs_avg, min_delta, score_cv) run over the both-arm subset, so they are more lenient than on main; (b) a fail-severity retrieval check is not added (retrieval_hit_rate stays warn). Neither is changed here.

Refs #228

🤖 Generated with Claude Code

…tep 2) The answerer stored top_k and never sent it, so retrieval asked the doc source for its default max_results. It now sends max_results=top_k, the same request DocTreatment makes on Stack A. With the default top_k of 3 the reranker sees 3 raw docs, not 5, so with-docs answers and their judge scores move. Flips the PINNED rows D1 retrieval.passed and D1 retrieval.max_results. Test fakes that take the doc-source call now accept max_results. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…ntel#228 step 2) benchmark run had no judge temperature flag, so its judge always ran at the provider default while evaluate and arms run took one. The flag is added (default unset, so runs without it are unchanged) and reaches the EvaluationPipeline judge. Flips the PINNED row D6 cli.judge_temperature_flag. benchmark batch does not get the flag yet. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…#228) EvaluationPipeline took the library id from --context7-id or the client's own resolver, so a product whose products.yaml declares context7_id was still looked up by its registry key. Resolve it with the shared resolve_library_id_for_product helper, the same order the arms stack uses: explicit id, then the registry's context7_id, then the client. Question generation and answer generation both go through the helper. The registry is injectable (registry=) and defaults to the bundled products.yaml. Parity row D8 flips from client/resolved to registry/lib. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…el#228) When no document reached the prompt (no hit, or the reranker raised), the answerer records "[FALLBACK: No relevant documentation found]" as the with-docs answer, and the judge scored that text like a model answer. It inflated the with-docs side of the paired delta on questions the docs did not answer. The judge now leaves a context answer whose doc_source is a no-retrieval source (NO_RETRIEVAL_SOURCES) unjudged, so its with-docs score and delta are None and the question drops out of the paired statistics. The record and its retrieval_hit stay. Opt back in with judge_placeholder=True, or --judge-placeholder on benchmark run, orchestrate evaluate and eval score. The trust gate's retrieval_hit_rate read only the both-arms-scored evaluations. Unjudged misses would then vanish from it and an all-miss run would read as "not measured". It now counts every evaluation's retrieval_hit. The thresholds are unchanged. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

Answerer built a self.llm client in __init__ that nothing read: the answer call goes through llm_call_with_usage, which picks the provider's client itself. Only the unsupported-provider ValueError is kept; the client construction and its imports are gone. _generate_with_docs had an "if not docs" branch that could not run, because _generate_answer_pair calls it only when retrieval returned documents. The branch is deleted with its unreachable "[No documentation retrieved]" record. No parity row changes: nothing observable on either stack moved. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…intel#228) The Stack P answers artifact and the judge's evaluation output now carry method_version, from defaults.ANSWERING_METHOD_VERSION (2). The steps before this one are version 1 and have no stamp, so a later compare can refuse to put runs of different versions side by side. compare does not read the stamp yet. answers.v1 and eval.v1 document the field. The arms artifact is not stamped in this change; the new parity row records that. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

ruff F821 flagged the annotation on _paired_deltas. The module uses `from __future__ import annotations`, so the missing name did not fail at import time and the parity tests passed without it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…set-then-read tests (intel#228) With the llm construction removed from Answerer.__init__, test_init_openai and test_init_anthropic only set attributes and read them back, which the test quality audit flags (set_then_read). The new parametrized test checks what init does now: each supported provider is accepted. It is marked as a must-not-raise smoke test. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

) test_calls_mcp_client builds the Answerer with the default top_k, so a literal max_results=5 would pass it. The new test builds it with top_k=3 and asserts the doc source is asked for 3. On intel/main the same probe sends no max_results at all, so the test fails there. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…ons (intel#228) The methodology list and the method_version comment described the trust gate and the stamp more broadly than the code does. The score checks run over the both-arm-scored questions, so a question with no document leaves them. Nothing reads method_version yet; the comment now says so. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…fraction (intel#228) A question whose retrieval missed is no longer judged with docs, so it has no context score. The zero-score check counted only scored questions, so a run where most questions miss passed it: 4 misses of 14 gave 0.0 PASS, where intel/main gave 0.286 FAIL (the misses were judged at zero there). The check now counts a miss (no context score, a baseline score, and retrieval_hit False) as unusable, in the numerator and the denominator. The message says such rows had no document. Thresholds, min_questions and the early return are unchanged. The new fail-severity retrieval check is not added here. Regression test: tests/test_trust_gate_unjudged_misses.py. It fails on the parent commit (4 of 14 passes at 0.0) and passes with this change. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…ons once (intel#228) The methodology bullet still said the zero-score fraction leaves out questions with no document. It counts them now (c517c72). Update it. Also parse the evaluations once in evaluate_trust. The input may be a one-shot iterable, and the old code parsed it again for the retrieval check. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

codecov-commenter · 2026-10-29T03:08:43Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.52%. Comparing base (f0f95a2) to head (284b240).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   93.48%   93.52%   +0.03%     
==========================================
  Files         272      272              
  Lines       35363    35376      +13     
  Branches     6001     6000       -1     
==========================================
+ Hits        33060    33085      +25     
+ Misses       1489     1479      -10     
+ Partials      814      812       -2

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/benchmark.py	`83.40% <100.00%> (+1.43%)`	⬆️
dokimi/dokimi/commands/evaluate.py	`89.90% <100.00%> (+0.03%)`	⬆️
dokimi/dokimi/commands/orchestrate.py	`84.72% <100.00%> (+0.21%)`	⬆️
dokimi/dokimi/defaults.py	`100.00% <100.00%> (ø)`
dokimi/dokimi/eval/answerer.py	`95.02% <100.00%> (+0.75%)`	⬆️
dokimi/dokimi/eval/diagnoser.py	`100.00% <100.00%> (ø)`
dokimi/dokimi/eval/judge.py	`94.48% <100.00%> (+0.06%)`	⬆️
dokimi/dokimi/gate/trust_gate.py	`96.03% <100.00%> (+0.34%)`	⬆️
dokimi/dokimi/orchestrator/pipeline.py	`86.02% <100.00%> (+2.19%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-10-29T03:20:12Z

CLI interface change detected

This PR changes dokimi's user-facing CLI surface (commands and/or options):

~ `dokimi benchmark run` parameters changed:
    + added option `--judge-placeholder`
    + added option `--judge-temperature`
~ `dokimi eval score` parameters changed:
    + added option `--judge-placeholder`
~ `dokimi evaluate` parameters changed:
    + added option `--judge-placeholder`

Updated on every push; the cli-interface-change label is on while the diff is non-empty.

vlad-perevezentsev

Review at 6bec891. The five Stack P fixes do what step 2 of #228 asks, and the zero-score check keeps failing the same runs as main. Not judging the placeholder has side effects outside the gate's score checks (the retrieval coverage, the diagnosis and the pipeline summary), and the registry lookup misses a custom --registry. These are in the line comments.

What I ran, at 6bec891:

make ci-local --offline: 0 failed, 17 passed, 13 skipped (network and Docker jobs).
Trust gate with 15 judged rows and 5 judge-error rows, on the base and on the head: the retrieval check goes from 100% (15/15) to "not measured".
summarise_diagnoses on 10 hits and 4 misses judged through _evaluate_answer_pair, on the base and on the head: empty_retrieval 4 becomes 0 and insufficient_data 0 becomes 4.
resolve_library_id_for_product("my_lib", <Context7 client>) returns my_lib unchanged.

Checked, no issues

top_k: Answerer sends max_results=top_k, as DocTreatment does. evaluate and answers generate pass their --top-k, and the new test pins the value at the call site.
Zero-score check: unjudged misses count in both the numerator and the denominator, so 4 of 14 misses still fail at 0.286, as on main. The new test fails on the parent commit.
Judge temperature: benchmark run --judge-temperature reaches the Judge in single and multi-run mode.
Dead code: Answerer.llm was never read, and _generate_with_docs is only called with docs. The provider allow-list keeps the same set.
Schemas: method_version is an optional new property in answers.v1 and eval.v1. That widens the schemas, and old files still validate.

antonwolfy

Review at 6bec891. The five Stack P fixes do what step 2 of #228 asks, and the zero-score check keeps failing the same runs as the base. Three things are worth fixing before merge, all test gaps on the new code (inline). The open problems from @vlad-perevezentsev's review still hold at this head and block the merge.

I agree with @vlad-perevezentsev on the judge-error rows losing the retrieval rate, the insufficient_data diagnosis of misses, the registry lookup in benchmark run --registry, the summary averages over different questions, the unstamped --judge-placeholder runs, the 0-hit gate test and the changelog flag names. New evidence: 10 hits and 4 misses move min_delta from -17.1 (fail) to +0.0 (pass).

What I ran, on a Linux review host at 6bec891:

make ci-local: 27 passed, 1 failed, 7 skipped. The failure is pip-audit, which lists advisories on filelock in the lock files. This PR does not touch them.
9,705 tests passed, 4 skipped; the changed-line coverage step passes.
Reproductions for the code findings, with the LLM transport mocked.
A mutation run on the new code (33 mutants, 13 of which pass the related tests): _library_id, the unjudged_misses filter and the flag wiring in benchmark run, evaluate and eval score each have mutants that pass every test.

Checked, no issues

Schemas: method_version is an optional property on an open object in answers.v1.json and eval.v1.json. schema_compat against the base passes, and no version bump is needed.
top_k: every doc-source client's get_library_docs accepts max_results, so the new argument raises nowhere.
Dead code: no caller of Answerer.llm or of _generate_with_docs with empty docs remains.
Zero-score check: 10 hits and 4 misses give 4/14 = 0.286 and fail, as on the base. Judge-error rows are excluded from the new term.
Commits: all 12 carry Signed-off-by; the changelog fragment is present.

vchamarthi · 2026-10-29T14:09:38Z

🔍 OpenCodeReview found 4 issue(s) in this PR.

By severity: 4 low

Severity	Category	Location	Finding
low	test	test_pipeline_summary.py:87	Nit: no test covers the all-paired case.
low	test	test_pipeline_library_id.py:56	Nit: `_RecordingClient.asked` exists to record the fallback, but no test reaches it.
low	documentation	20261028_120000_napetrov_stack-p-step2.md:7	`benchmark run` had no `--judge-temperature` flag before this change (the diff adds it to the parser).
low	test	test_diagnoser.py:56	Nit: this covers only the `fallback_none` happy path.

Review details

Commit reviewed: 284b240 against merge base f0f95a2
Coverage: 25 files in 6 groups
Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
Usage: 1.04M tokens (785k from cache), 51s
Tool calls: 53 (33 code_search, 14 file_read, 3 code_comment, 3 file_read_diff)
Changelog and methodology docs: 2 files
Benchmark command, defaults, and registry wiring: 3 files
Evaluate/orchestrate commands and pipeline library_id: 6 files
Answerer refactor, answering stack parity, and schemas: 6 files
Judge and diagnoser changes with placeholder tests: 5 files
Trust gate and unjudged misses tests: 3 files

…rs-stack-p-step2 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/dokimi/orchestrator/pipeline.py

The retrieval coverage floor counted every judge-error row as an unrecorded outcome, so a run with more than 20% judge errors lost the retrieval check. Thread: PRRT_kwDOVBg7g86xgwoY Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

With the no-retrieval placeholder left unjudged its delta is None, and diagnose() returned insufficient_data before it looked at the doc source. Thread: PRRT_kwDOVBg7g86xgwog Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

The summary averages the questions scored in both arms (paired_summary, intel#250), so an unjudged retrieval miss is in none of the three numbers. The run result now carries paired and unpaired, and the printed line names them. Thread: PRRT_kwDOVBg7g86xgwon Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…s loaded benchmark run and batch loaded the registry for the entry, but the pipeline looked the key up in the bundled products.yaml. A custom entry without a context7_id then reached Context7 as a bare key and retrieved nothing, and a custom key that the bundled registry also has took the bundled id. Thread: PRRT_kwDOVBg7g86xgwou Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…ibrary_id Both mutants named in the review (explicit_id=None, product instead of product_key) now fail a test. Thread: PRRT_kwDOVBg7g86xiJYO Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

--judge-placeholder gives different numbers on the same answers under the same method_version. The eval output now carries judge_placeholder (declared in eval.v1), so two files of one version that are not comparable can be told apart. Thread: PRRT_kwDOVBg7g86xgwoz Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

… arms With every question an unjudged miss the gate returned after min_questions with "0 questions evaluated", pointing away from the cause. The retrieval check is now a helper and also runs on that path. The 0-hit case has its test again, and the 2-hit test is named for what it checks. Thread: PRRT_kwDOVBg7g86xgwo7 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

A legacy row, a hit that was not scored and a miss without a baseline must leave zero_score_fraction unchanged; each mutant of the filter named in the reviews now fails. The judge stub branch that only a judged miss reached is gone: the default leaves misses unjudged. Thread: PRRT_kwDOVBg7g86xiJYM Thread: PRRT_kwDOVBg7g86xlsnp Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…e-placeholder benchmark run (single and multi-run) is driven through cmd_benchmark_run with a faked pipeline; evaluate and eval score run end to end against a mocked judge transport. Each removed call-site kwarg named in the reviews now fails a test. The old test named for the handler called _run_single_library directly. Thread: PRRT_kwDOVBg7g86xiJYT Thread: PRRT_kwDOVBg7g86xlsUD Thread: PRRT_kwDOVBg7g86xlsb0 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…es differ benchmark run has no --top-k and there is no orchestrate evaluate: the command is dokimi evaluate. The fragment now says what each command asks for, that answers generate changes too, that old scores are not comparable, and covers the fixes of this round. Thread: PRRT_kwDOVBg7g86xgwo3 Thread: PRRT_kwDOVBg7g86xlsKV Thread: PRR_kwDOVBg7g88AAAABU4c24A Thread: PRR_kwDOVBg7g88AAAABU5LNng Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

dokimi.eval.answerer neither imports nor uses ChatOpenAI; the patch with create=True and its mock did nothing and suggested the init builds a client. Thread: IC_kwDOVBg7g88AAAABg14M_w Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…ft unused main's paired_summary (intel#250) replaced the BASELINE/CONTEXT/records reads in run(); the merge kept this branch's import line, which ruff F401 rejects. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

…extractor Codecov flagged the one patch line in _generate_questions that no test reached. The test asserts the extractor gets the registry's declared id; reverting the line to the client's name lookup fails it. Thread: IC_kwDOVBg7g88AAAABgqSv4w Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

napetrov · 2026-10-29T18:57:12Z

Its findings are the threads above (judge-error row, diagnoser, summary count, registry, judge_placeholder stamp, changelog flags, 0-hit gate), each answered with its own commit; the review itself raised no separate claim. Latest commit naming it: 3ef4eae.

napetrov · 2026-10-29T18:57:14Z

The three test gaps are the threads on the unjudged-miss term, _library_id and the judge flags; each is fixed in its own commit above (3ef4eae is the changelog one). The pip-audit failure is not from this PR: it touches no lock file.

napetrov · 2026-10-29T18:57:17Z

Fixed in 284b240: the one patch line codecov flagged is the library id in _generate_questions; test_question_generation_extracts_topics_for_the_declared_library now reaches it and fails if the line goes back to the client's name lookup.

napetrov · 2026-10-29T18:57:19Z

This is the CLI-surface notice listing the added options (--judge-temperature, --judge-placeholder); both are intended in this PR and covered by the handler tests, so nothing to change.

napetrov · 2026-10-29T18:57:21Z

Fixed in 87a1a0a: the patch of dokimi.eval.answerer.ChatOpenAI (with create=True) is removed from test_eval_answerer.py; the module has no such name.

vchamarthi · 2026-10-29T19:01:11Z

+  `top_k` docs the command was given: `benchmark run` asks for 3 (`DEFAULT_RETRIEVAL_TOP_K`; it
+  has no `--top-k`), `dokimi evaluate` and `answers generate` for their `--top-k` (default 3).
+  Before, they left `max_results` at the client default of 5. `benchmark run --judge-temperature`
+  now reaches the judge (it used to be dropped), and the doc library id resolves through the


benchmark run had no --judge-temperature flag before this change (the diff adds it to the parser). The flag was not accepted and silently dropped, as this says. It was rejected as an unknown argument. The entry should say the flag is new. The Fixed section repeats the claim.

vchamarthi · 2026-10-29T19:01:12Z

    assert result["label"] == EMPTY_RETRIEVAL


+def test_unjudged_retrieval_miss_is_an_empty_retrieval_not_insufficient_data():


Nit: this covers only the fallback_none happy path. Deleting not context_answer.error from the new diagnoser branch would still pass. An errored placeholder answer must stay insufficient_data, and the docstring says so. The second NO_RETRIEVAL_SOURCES member ("none") and context_answer=None are not exercised on the delta is None path either. Add a case with error set and one with doc_source="none".

vchamarthi · 2026-10-29T19:01:12Z

+    assert client.asked == []
+
+
+def test_the_registry_key_not_the_display_name_finds_the_declared_id(tmp_path, registry):


Nit: _RecordingClient.asked exists to record the fallback, but no test reaches it. A product that is not in the registry should resolve through client.resolve_library_id(product_key). A mutation that skips the fallback, or passes self.product instead of self.product_key, would pass these tests. Add a test with an unregistered product that asserts client.asked == [<key>].

vchamarthi · 2026-10-29T19:01:12Z

+    assert (step["paired"], step["unpaired"]) == (7, 3)
+    printed = capsys.readouterr().out
+    assert "Evaluated 10 answers: context=80.0, baseline=60.0, delta=+20.0" in printed
+    assert "over the 7 scored in both arms; 3 left out" in printed


Nit: no test covers the all-paired case. A mutation that always appends the "left out" suffix, or drops the if summary.unpaired guard, still passes every test in this file. Add a case with only complete pairs that asserts unpaired == 0 and that "left out" is not in the printed line.

napetrov added 12 commits October 28, 2026 16:22

napetrov requested review from antonwolfy, jharlow-intel, vchamarthi and xaleryb as code owners October 29, 2026 02:57

napetrov mentioned this pull request Oct 29, 2026

Answering stack: one implementation for evaluate/benchmark/answers and arms/subjects #228

Open

github-actions Bot added the cli-interface-change Changes dokimi's user-facing CLI surface label Oct 29, 2026

vlad-perevezentsev reviewed Oct 29, 2026

View reviewed changes

antonwolfy reviewed Oct 29, 2026

View reviewed changes

Comment thread dokimi/dokimi/gate/trust_gate.py

Comment thread dokimi/dokimi/orchestrator/pipeline.py

Comment thread dokimi/dokimi/commands/benchmark.py