[vlad-perevezentsev]

Part of #4: Wave 4, LLM module, item 2 of the plan in #4 (comment). The rest of #229 P1-2's fix ("delete get_llm and the five switches"); #376 fixed the bugs.

No behaviour change.

• The callers. The judge, the persona and question generators and the validator build Invoker(LLM(...)) for every provider. Before, openai and anthropic went through ChatOpenAI/ChatAnthropic, which built the same LLM on each call.
• One provider check. The answerer, the judge and the two generators each kept a copy of one 9-provider tuple and its error. Now they call llm.check_caller_provider, backed by llm.CALLER_PROVIDERS. The providers and the message are the same as before. The validator still accepts any provider.
• Removed:
  • utils.get_llm, utils.bedrock_litellm_model and utils._read_key: nothing in the package has called them since fix(llm): the judge, generators and validator route and key through dokimi.llm (#229 P1-2) #376;
  • llm._GetLLMCompat, and the LLM hooks (_route, _key, _labels) that only it overrode;
  • llm.ChatOpenAI and llm.ChatAnthropic.
• Tests.
  • The plan in Architecture cleanup: fix live bugs, then deepen the LLM, eval-record, novelty, arms and process modules #4 said these tests would move to fake_llm/provider_calls. They went two ways instead:
    • tests of a caller's parsing patch its Invoker;
    • tests of the transport (retry, error classification, cache) use a real Invoker(LLM(...)) with litellm.completion stubbed, so they go through LiteLLMAdapter as production does.
  • The judge and generator tests used to patch ChatOpenAI/ChatAnthropic with create=True. They now patch the caller's Invoker, without create=True. With create=True the old patches would have kept passing against a missing name while the call went to the network.
  • The constructor tests read the LLM the caller built (model, provider, key, sampling, route) instead of checking that a mock was called.
  • Tests that pinned only get_llm's own table are removed where LLM already has the same pin: ARN, converse route, key file, retry budget, pricing.
  • The Bedrock profile tests move to resolve_model (test_get_llm_bedrock.py → test_bedrock_model_ids.py).

Evidence

• On upstream/main, ChatOpenAI/ChatAnthropic and Invoker(LLM(...)) give the same requests, the same response-cache file names and the same usage labels. Checked for openai (gpt-6.1-sol, glm-5.3 with temperature=0.0, seed=3) and anthropic (claude-sonnet-5-5, claude-haiku-5-5 with temperature=0.0), for draw 0 and draw 1. So a cache written by main still hits; test_llm.py::test_an_entry_main_wrote_is_hit keeps its openai and anthropic entries.
• Mutations, each caught:
  • the judge drops temperature/seed;
  • the persona generator drops the key;
  • the question generator routes every provider to openai;
  • the validator ignores LANGCHAIN_AVAILABLE;
  • the judge accepts any provider;
  • the answerer, persona or question generator skips the provider check;
  • CALLER_PROVIDERS loses gemini;
  • Invoker drops draw;
  • LLM records the route as the usage model.
• growth_baseline.json is lowered: llm.py 1093 → 1032 lines, QuestionValidator.__init__ complexity 14 → 11.
• An openai or anthropic caller builds its LLM in the constructor, where the shim built one on each call. So model=None (the parameter is typed str, and no CLI default is None) raises the same TypeError at construction instead of at the first call. The other providers already did this on main.
• make ci-local: 0 failed (14849 passed); the PR-only checks (changelog fragment, comment blocks, growth against the base) pass against upstream/main.

Not in this PR

• Item 4 of the plan in Architecture cleanup: fix live bugs, then deepen the LLM, eval-record, novelty, arms and process modules #4 (redraw loops through llm.ask_parsed): there is nothing to move.
  • eval/pairwise.py already goes through judged_reply, which uses ask_parsed (refactor(judge): one judged-reply seam for judge, panel, pairwise, checklist #314).
  • The persona generator and the validator make one call with no redraw.
  • Adding redraws there would change behaviour, so it is not a refactor. The llm.py docstring no longer lists them.
• The answerer and the generators reject openai-codex, which their CLI --provider offers (ANSWER_PROVIDERS). Fixing that changes behaviour, so it is left out of this refactor; CALLER_PROVIDERS now marks it. JUDGE_PROVIDERS (the judge flags' choices) differs from it in gemini/vertex_ai, as on main.
• llm.LANGCHAIN_AVAILABLE is always True, and five test files still put stub langchain_* modules in sys.modules. Only the RAGAS step uses langchain.
• .agent-eval/tasks/legacy-keys/check.py patches judge.ChatAnthropic. The task is pinned to base 16e319f, where the shim exists and judge.Invoker does not, so patching Invoker would break it on its own base. It needs the new patch only if the task is moved to a newer base.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 31 selected item(s).

CI at 77f8aff: 15 passing, 2 skipped.

Review details

• Commit reviewed: 77f8aff against merge base 8d54c93
• Coverage: 31 files in 5 groups
• Engine: OpenCodeReview v1.13.5-exai.3.d7112371, us.openai.gpt-6.1-sol
• Usage: 407k tokens (242k from cache), 26s
• Tool calls: 27 (17 file_read, 3 ci_status, 3 code_search, 3 file_read_diff, 1 pr_threads)
• LLM API simplification and provider compatibility: 8 files
• Evaluation judge migration and behavior tests: 7 files
• Question generation, validation, and artifact persistence: 9 files
• Persona generation migration: 2 files
• Answerer and cross-caller integration coverage: 5 files

[napetrov]

Verdict: comment only. No blocking thread; two nits, both inline. Head a8a4f1a, merge base 52fb825, lane L2.

CI: ci / Static checks is red because the job stops at "origin/main is not fetched", before ruff, mypy and the changelog check run. The failing line is the checkout guard, not a changed file. Please re-run it after a fetch; no code change is asked for.

#	Finding	Label	Provenance	Scope	Probe
1	changelog ### Removed omits the removed public bedrock_litellm_model (C4)	nit	introduced	contract	probes/a1-out.txt
2	CLI offers openai-codex (--provider), every caller refuses it	nit	pre-existing	contract	probes/out-head.txt, probes/out-base.txt

Coverage: checked the diff, the provider check and its callers, the changelog fragment, the removed names on both trees, the judge, persona, generator and cache probes, and ci.md. Not looked at: the full pytest suite (litellm, regex and httpx not installed here, no network), mypy, and the changelog check script.

[napetrov]

nit: the changelog's Removed entry leaves out utils.bedrock_litellm_model, a public name this PR deletes

dokimi/changelog.d/20261104_120000_vlad-perevezentsev_drop-get-llm.md:1 (### Removed): the PR body lists utils.bedrock_litellm_model as removed (C4), but the entry names only get_llm, ChatOpenAI and ChatAnthropic. A caller that imports it gets ImportError on upgrade and finds no note about it. The only importer in the repo was its test, which this PR moves, so this is a note, not a break.

How to check: grep -l bedrock_litellm_model dokimi/changelog.d/* on head → no output; the definition is gone from dokimi/dokimi/utils.py (base: defined there; introduced; probes/a1-out.txt).
Expected: each removed public name appears in the Removed entry.
Fix: add dokimi.utils.bedrock_litellm_model to the Removed list. _read_key is private and needs no note.

[vlad-perevezentsev]

Fixed in 1f0cf48: the Removed entry names dokimi.utils.bedrock_litellm_model and points to dokimi.llm.resolve_model(m, "amazon-bedrock") in its place.

[napetrov]

nit: --provider openai-codex is offered by the CLI, and the answerer, judge and generators refuse it

dokimi/dokimi/llm.py:102 (CALLER_PROVIDERS): the tuple leaves out openai-codex, which ANSWER_PROVIDERS (the --provider choices) still offers. The code comment calls the exclusion deliberate. Base rejects it the same way, so this is not a regression.

How to check: Answerer, PersonaGenerator and QuestionGenerator with provider openai-codex → ValueError: Unsupported provider: 'openai-codex' on head (base: the same; pre-existing; probes/out-head.txt, probes/out-base.txt).
Expected: a provider the CLI offers is either accepted by its callers or not offered.
Fix: decide one: filter --provider to CALLER_PROVIDERS, or support openai-codex in the callers. Either can go to a follow-up issue.

[vlad-perevezentsev]

Agreed, and it is pre-existing: main refuses it the same way, so I kept it out of this refactor. CALLER_PROVIDERS now at least makes the gap visible in one place. I'll open a follow-up issue listing the commands whose --provider offers openai-codex while their caller refuses it, with both options (narrow the choices to CALLER_PROVIDERS, or accept it in the callers).

[codecov-commenter]

⚠️ 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 94.75%. Comparing base (8d54c93) to head (77f8aff).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
- Coverage   94.75%   94.75%   -0.01%     
==========================================
  Files         352      352              
  Lines       46507    46407     -100     
  Branches     7970     7949      -21     
==========================================
- Hits        44070    43974      -96     
+ Misses       1544     1542       -2     
+ Partials      893      891       -2     

Files with missing lines	Coverage Δ
dokimi/dokimi/eval/answerer.py	94.97% <100.00%> (-0.06%)	⬇️
dokimi/dokimi/eval/judge.py	94.19% <100.00%> (-0.15%)	⬇️
dokimi/dokimi/llm.py	98.75% <100.00%> (-0.09%)	⬇️
dokimi/dokimi/personas/generator.py	90.54% <100.00%> (-0.82%)	⬇️
dokimi/dokimi/questions/llm_gen.py	94.31% <100.00%> (-0.42%)	⬇️
dokimi/dokimi/questions/validator.py	88.83% <100.00%> (+0.59%)	⬆️
dokimi/dokimi/utils.py	91.13% <100.00%> (-1.01%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[antonwolfy]

LGTM. The refactor does what it says: the openai and anthropic callers build the same LLM the shims built, and CALLER_PROVIDERS and the error text match the four tuples they replace. One nit on the changelog.

What I ran, on a Linux review host at 1f0cf48:

• make ci-local: 29 passed, 1 failed, 7 skipped. 15,694 tests passed, 10 skipped; changed-line coverage passes. The failed step is two fixture-setup timeouts in tests/test_audit_test_quality.py (60 s limit, machine under load); the same file passes alone, 172 tests in 15 s, and the PR does not touch it.
• Reproductions with the LLM transport mocked, for the route, key and construction behaviour of the callers.
• A mutation run: 41 mutants over the judge, answerer, both generators, the validator, check_caller_provider, CALLER_PROVIDERS, Invoker and Request. 40 are caught; the one survivor only empties the remedy sentence of the unsupported-provider message.

Checked, no issues

• Request parity: the removed shims went through llm_call_with_usage, which built LLM(model, provider, api_key, temperature, seed) with the same defaults. The request, cache key and usage labels do not change.
• Provider check: the set and the message are identical to the old tuples; the judge still runs it after check_sampling.
• Bedrock: resolve_model(m, "amazon-bedrock") gives the same route as the removed bedrock_litellm_model on seven inputs (known bare name, anthropic.x, gemini/x, ARN, openai.*, foo/bar, unknown bare name).
• Removed tests: none lost a pin the surviving LLM tests do not hold (converse route, file: key, backoff, pricing).
• Leftovers: no remaining references to the removed names except the langchain classes in the ragas path, and .agent-eval/tasks/legacy-keys/check.py, which the description says is pinned to an older base.

[antonwolfy]

LGTM. I pushed a commit per nit, up to 77f8aff, which fix the nit from my review:

• 77f8aff: dokimi/changelog.d/20261104_120000_vlad-perevezentsev_drop-get-llm.md:10: added the openai route and the unreadable file: key to the list of differences, in the suggested wording.

make ci-local at 77f8aff: 30 passed, 0 failed, 7 skipped.