Repository navigation
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #255 +/- ##
==========================================
+ Coverage 93.26% 93.32% +0.06%
==========================================
Files 272 276 +4
Lines 35117 35451 +334
Branches 5964 6010 +46
==========================================
+ Hits 32752 33086 +334
Misses 1547 1547
Partials 818 818
🚀 New features to boost your workflow:
|
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 563eed3. The routing, the isolation flags and the error classification in llm_cli.py hold up, and the API-provider path is unchanged. The problems are at the edges: an ordering bug in orchestrate eval, a novelty default that was missed, and test gaps that let real regressions through.
What I ran, on Linux at 563eed3:
make ci-local: 28 passed, 0 failed, 7 skipped. The changed-line coverage step passes.- Reproductions for the code findings, with the LLM transport mocked and no
claudebinary started. - A mutation run on
test_llm_cli.py: 122 mutants, 52 of which pass the full suite. Most are cosmetic; the ones that matter are in the two test comments.
Findings outside the diff:
-
orchestrate eval --judge-provider claude-cli --judge-temperature 0is refused only after personas, questions and answers have been generated.EvaluationPipeline.__init__(orchestrator/pipeline.py:376-380) only storesjudge_temperature;check_samplingruns whenJudge(...)is built in_evaluate_answers(pipeline.py:647-668). Withjudge_provider="claude-cli", judge_temperature=0.0the constructor raises nothing, andrun()printed "Discovered 0 personas", "Generated 1 questions", "Generated answers for 1 questions" before reaching the judge. A user pairing an API answerer with a CLI judge pays for the whole answer step, then gets aValueError. The commit message of f192c40 says the refusal is "before the run". Suggestion: callcheck_sampling(judge_provider, judge_temperature, None)in the pipeline constructor, or at the top ofcmd_evaluateincommands/orchestrate.py. -
novelty ... --judge-provider claude-cliwithout--judge-modelfails, although the docs and changelog list novelty as supported.defaults.default_judge_model("claude-cli")was extended (defaults.py:71), butdefault_judgeinnovelty/client.py:248-272was not: it only matches a candidate whose provider equals--judge-provideror Bedrock.default_judge("sonnet", "claude-cli")raises--judge-provider claude-cli: no default judge runs there other than the model under measurement (sonnet); pass --judge-model as well. The message is wrong for this case, since the judge is not the model under measurement. Suggestion: givedefault_judgethe same CLI branch asdefault_judge_model, and add a test next totest_the_default_judge_runs_on_the_cli.
Checked, no issues
- Provider lists: every
--providerchoice incommands/*now comes fromANSWER_PROVIDERSorJUDGE_PROVIDERS; no old tuple is left. - Sampling refusal:
arms,judge-validity,JudgeandJudgePanelrefuse before any model call. - Schema versioning: the spec change only raises
max_docsfrom 32 to 33, withinspec.schema.json's range; no version bump is needed. llm_json.py:extract_json_objectandextract_json_arrayare byte-identical to the base, andfrom dokimi.llm import ...still works.- Docs:
mkdocs buildis clean, the nav entry is present, and the flags insubscription-backends.mdmatch argparse.
| @pytest.mark.parametrize("answer, retryable, words", [ | ||
| (reply(UNKNOWN_MODEL, returncode=1), False, "selected model"), | ||
| (reply(NO_LOGIN, returncode=1), False, "setup-token"), | ||
| (reply(_err("Rate limit reached", 429), returncode=1), True, "Rate limit"), |
There was a problem hiding this comment.
The retry classification is only tested with cases that match both the status and the word list, so neither carries a case alone. Rate limit reached/429 and Overloaded/529 each match _RETRYABLE_STATUS and _RETRYABLE_WORDS at once (llm_cli.py:68-69, :139).
Mutation check: dropping any of 408, 429, 500, 502, 503 or 529 from _RETRYABLE_STATUS passes the full suite. So does emptying _RETRYABLE_WORDS, dropping overloaded or usage limit, and iserror-ignored: ignoring is_error when the exit code is non-zero, or the exit code when is_error is true (llm_cli.py:153). A typo in either table would make every real run fail on its first rate limit or 5xx.
Suggestion: parametrize with a status and an unrelated message (_err("x", 503) retryable), a matching word and no status (_err("overloaded", None) retryable), and one case each for is_error with exit 0 and exit 1 with is_error false.
There was a problem hiding this comment.
Still open at 43eafc9: the parametrize list is unchanged. One more point on the table itself: usage limit is retryable, but a subscription usage limit resets after hours, so the 2/4/8 s backoff only adds three more CLI starts per answer before it fails. I would make it non-retryable and cover it with a case that has no status code.
There was a problem hiding this comment.
Partly addressed, at 1688652.
- Status and word lists:
test_each_retryable_status_is_retried_whatever_the_message(408, 429, 500, 502, 503, 504, 529) andtest_each_retryable_word_is_retried_without_a_status(rate limit, rate_limit, overloaded, timed out) are written as literal lists, so dropping a member fromllm_cli.pyfails them. Both were added in 35c5219. usage limitis not retried (9147dfa):_claude_errorinllm_cli.pyreturns non-retryable on that text, before the status and word checks.test_a_subscription_usage_limit_is_not_retriedcovers status None and 429.- Not done: there is no test yet for the
is_errorcases (exit 0 withis_errortrue; exit 1 withis_errorfalse).
There was a problem hiding this comment.
The status and word tables are pinned by literal lists in test_each_retryable_status_is_retried_whatever_the_message and test_each_retryable_word_is_retried_without_a_status. The is_error cases are covered by test_is_error_and_the_exit_code_each_mark_a_failure, so my earlier note that they were missing was wrong. A subscription usage limit is not retried (test_a_subscription_usage_limit_is_not_retried); I removed it from the retryable word list, where it had no effect. Fixed in 090122c.
| assert row["backend"] == "claude-cli" and row["authenticated"] is True | ||
|
|
||
|
|
||
| def test_benchmark_preflight_checks_the_login_not_a_key(monkeypatch): |
There was a problem hiding this comment.
The wiring of the preflight and of the panel refusal is not tested, only the helpers. test_benchmark_preflight_checks_the_login_not_a_key calls _credential_error directly, and the sampling test goes through JudgePanel(default_provider=...).
Mutation check: these pass the full suite. In commands/benchmark.py: cmd_benchmark_preflight no longer appending the errors, no longer checking the judge provider, and the API-key branch always returning None or inverted. In eval/panel.py:207-208: running the check only when judges is None, or only against default_provider. cmd_eval_panel_score (commands/evaluate.py:131) always passes explicit judges=[JudgeConfig(...)], so that path is the one --judge-configs role:model:claude-cli takes.
Suggestion: add one test that runs cmd_benchmark_preflight with an unsigned CLI backend (fake detect) and a missing API key and asserts the exit status, and one that builds JudgePanel(judges=[JudgeConfig(..., provider="claude-cli", temperature=0.0)]) and expects the ValueError.
There was a problem hiding this comment.
Still open at 43eafc9: the preflight test still calls _credential_error directly, and the panel test still goes through default_provider, not judges=[JudgeConfig(..., provider="claude-cli", temperature=0.0)].
There was a problem hiding this comment.
Fixed at a3c2923 (on 1688652).
- Panel:
test_a_judge_panel_refuses_sampling_on_the_cli_with_explicit_judges(35c5219) buildsJudgePanel(judges=[JudgeConfig(role="a", model="sonnet", provider="claude-cli", temperature=0.0)])and expects theValueError. - Preflight helper:
test_benchmark_preflight_checks_the_login_not_a_key(af07f85) goes through_provider_checks. The unanswered case istest_an_unanswered_login_check_is_a_warning_not_a_pass(35c5219). - End to end (a3c2923):
test_benchmark_preflight_exit_status_follows_the_cli_loginrunscmd_benchmark_preflightwith a fakedclaude auth status: signed out exits 1; no answer prints the warning and passes; signed in passes; a signed-out judge with an API answerer exits 1. Dropping the provider errors, or skipping the judge's login, each fails it.
There was a problem hiding this comment.
Both are covered at a3c2923. test_a_judge_panel_refuses_sampling_on_the_cli_with_explicit_judges builds the panel with explicit judges, and test_benchmark_preflight_exit_status_follows_the_cli_login runs cmd_benchmark_preflight against a faked claude auth status. Dropping the judge's login check makes that test fail.
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 43eafc9. The core of the claude-cli provider is done with care: the routing, the isolation flags, the error classes and cost_usd=None are right. Some things still need changes before merge; they are in the line comments, and two points from the earlier review are still open at this head.
What I ran, at 43eafc9:
make ci-local --offline: 17 passed, 0 failed, 13 skipped (the skips need the network or docker).- No live
claude -pcalls. Every repro ran through a fake CLI runner: the alias against full-id independence check,ArmRunnerwithbaseline,agentand with a session,mkdirunder a read-only parent,claude auth statuswith no JSON,OPENAI_API_KEY=file:with a missing file, andanswers generate --provider claude-cliwithout--model. _envinside a Claude Code 2.1.303 session, to see which variables reach the child.- A temporary merge with #155:
select_docsfails withgolden_manifest violation: matched 34 ... more than max_docs=33, since both PRs raisemax_docsfrom 32 to 33 for a new page.
Checked, no issues
- Earlier threads: the
tmp_paththread is fixed in e79c12d, and the "32 files" comment inspec.v1.yamlis fixed in 43eafc9. - Prompt size: the system prompt goes in argv, but the agent profile is cut at 12,000 characters, far below the argv limit.
detectwithout--setting-sources: right as it is, sinceclaude auth statusdoes not accept that flag.- Artifacts: no schema or stored-key changes, and the numbers for API providers do not change.
- Docs and changelog: the new page is in
nav:and indocs/README.md, and the Added/Changed entries fit. - Files: SPDX headers are in all new files, the tests run offline with a fake runner, and no run artifacts are in the PR.
| """ | ||
| if provider == DEFAULT_JUDGE_PROVIDER or (provider == "amazon-bedrock" | ||
| # claude-cli takes Anthropic's model ids, as the anthropic provider does. | ||
| if provider in (DEFAULT_JUDGE_PROVIDER, "claude-cli") or (provider == "amazon-bedrock" |
There was a problem hiding this comment.
Blocking: With --provider claude-cli the answerer usually gets an alias (--model sonnet, as in the docs), but the default judge here is the full id claude-sonnet-5-5. The independence check compares strings (normalize_model_ref), so claude-cli/sonnet != claude-cli/claude-sonnet-5-5: eval score prints no warning and writes evaluator_independence_warning: false, and novelty's judge.model == answerer.model refusal (novelty/measure.py:243) does not fire. Please treat a CLI alias as the same model as any claude-<alias>-* id in these checks, and warn when unsure.
There was a problem hiding this comment.
Fixed in 9147dfa. The independence checks use same_model (llm_cli.py), so a CLI alias and the full id of its family are one model: sonnet and claude-sonnet-5-5, including Bedrock or prefixed ids. same_model is used by the evaluate independence check (commands/evaluate.py) and the novelty judge check (novelty/client.py).
Tests: test_an_alias_and_its_full_id_are_one_model_for_the_identity_checks and test_the_independence_warning_fires_for_an_alias_against_its_full_id (9147dfa), and test_an_alias_and_a_bedrock_or_prefixed_full_id_are_one_model_for_the_identity_checks (2c491ec). Head is 1688652.
There was a problem hiding this comment.
A CLI alias and the full id of its family are one model in both identity checks: same_model matches sonnet with claude-sonnet-5-5, including Bedrock and prefixed ids. The evaluate independence warning and the novelty judge gate both use it. Pinned by test_an_alias_and_its_full_id_are_one_model_for_the_identity_checks and test_the_independence_warning_fires_for_an_alias_against_its_full_id. Fixed in 9147dfa.
| "repository: --questions-root /path/to/content-repo/questions") | ||
| eval_p.add_argument("--model", default=DEFAULT_ANSWER_MODEL, help="LLM model for generation/answering") | ||
| eval_p.add_argument("--provider", default=DEFAULT_ANSWER_PROVIDER, choices=["openai", "anthropic", "amazon-bedrock", "google-vertex", "openrouter", "openai-codex"]) | ||
| eval_p.add_argument("--provider", default=DEFAULT_ANSWER_PROVIDER, choices=ANSWER_PROVIDERS) |
There was a problem hiding this comment.
Following up on @antonwolfy's point 1 in the review: still open at 43eafc9. cmd_evaluate has no check_sampling call and EvaluationPipeline.__init__ only stores judge_temperature, so orchestrate eval --judge-provider claude-cli --judge-temperature 0 still pays for personas, questions and answers before the judge refuses.
There was a problem hiding this comment.
Fixed in 35c5219. cmd_evaluate (commands/orchestrate.py) calls check_sampling(args.judge_provider, getattr(args, "judge_temperature", None), None) before the pipeline starts. orchestrate eval --judge-provider claude-cli --judge-temperature 0 now prints the ValueError and exits 1 before any persona, question or answer is paid for. Head is 1688652.
The call is pinned by test_a_cli_judge_given_a_temperature_stops_the_full_pipeline_before_any_work, which fails if the pipeline is constructed.
There was a problem hiding this comment.
cmd_evaluate now checks the judge's sampling flags before the pipeline is built, so a claude-cli judge with a temperature stops before any persona, question or answer is paid for. test_a_cli_judge_given_a_temperature_stops_the_full_pipeline_before_any_work fails if the pipeline is constructed. Fixed in 35c5219.
| for m in request.messages if m.get("role") == "system"] | ||
| out = llm_cli.call(request.provider, model=request.model, prompt=prompt_text(request), | ||
| system="\n\n".join(system) or None) | ||
| return Reply(text=out.text, usage=out.usage, finish_reason=out.finish_reason) |
There was a problem hiding this comment.
_cli_reply drops out.model_id, so answers, the judge record and the response-cache key only say sonnet. The alias moves with CLI updates and with ANTHROPIC_DEFAULT_SONNET_MODEL, which is passed through from the caller's env. So two runs that both say claude-cli/sonnet can be different models, and a cached reply from one is replayed in the other. Please record the resolved id (in the usage record or the run meta). Also, next(iter(modelUsage)) can be a helper model; harnesses/claude_json._main_model picks the main one.
There was a problem hiding this comment.
Partly addressed, at 1688652. The resolved model is recorded. It is not part of the cache key.
- Recorded (4937a07): each usage row carries
resolved_model, the model the CLI resolved the alias to. Sums keep every name they mix, comma-separated. Answer records get it throughas_metrics_dict(shown only when set, so API rows are unchanged), and the judge record listsjudge_resolved_models. - Main model (2c491ec):
_main_modelinllm_cli.pytakes the requested family frommodelUsage, else the entry with the most output, so a helper model is not reported. - Cache key (4937a07): for a CLI provider the key takes the CLI version (
llm_cli.cache_extra), not the resolved id. A new CLI release gives a new key, so the old model's replies are not replayed. Test:test_a_new_cli_version_misses_the_response_cache. - Known gaps: (1) the resolved id is known only after the call, so the pre-call key cannot hold it; a server-side move of the alias with no CLI release is not caught; (2) the version is read once per process, so a CLI self-update during a run is not seen; (3) when the version line cannot be read, the key takes an empty version, so all such calls share one key.
Tests: test_the_resolved_model_is_on_the_usage_row_and_in_its_metrics, test_a_sum_of_usage_rows_keeps_every_resolved_model_it_mixes, test_a_cached_cli_reply_replays_the_resolved_model, test_the_judge_record_names_the_resolved_models_it_was_judged_by.
There was a problem hiding this comment.
The resolved model is on each usage row and in the judge record, and the main model is taken from the requested family rather than a helper model. The response-cache key takes the CLI version, and the ANTHROPIC_* variables that could move the alias are no longer passed to the CLI. Known gap, accepted for this PR: a server-side move of the alias within one CLI version is not caught, because the resolved id is only known after the call.
| arms_run_p.add_argument("--model", default=DEFAULT_ANSWER_MODEL, help="LLM for answering") | ||
| arms_run_p.add_argument("--provider", default=DEFAULT_ANSWER_PROVIDER, | ||
| choices=["openai", "anthropic", "amazon-bedrock", "google-vertex", "openrouter", "openai-codex"]) | ||
| choices=ANSWER_PROVIDERS) |
There was a problem hiding this comment.
arms run --provider claude-cli (and matrix-run, line 384) is accepted with agentic arms (agent, skill-agent:, skill-catalog:, tool plugins) and with --session-turns, but the CLI path refuses tools and multi-turn chat on every call. The run starts, pays for the plain arms, and records agent_failed for every agentic answer; a session stops after its first turn. Please refuse this in _build_arms_output before any answer, as tools does by leaving CLI providers out.
There was a problem hiding this comment.
Partly addressed, at 1688652. Fixed in 9147dfa for arms run, with the import fix in 2c491ec and the entry-point test in 4e33134.
arms runwith--provider claude-clirefuses agentic arms (MCP agent, skill agent, skill catalog, and tool plugins) and--session-turns.refuse_cli_arms(commands/arms_refusals.py) runs in_build_arms_outputbefore any answer. It reaches through plugin wrappers and combined arms.matrix-runcalls the same_build_arms_outputfor each cell, so the refusal applies there too, but per cell. Not done: a matrix whose earlier cells use another provider runs those cells before the first CLI cell is refused. Pre-checking every cell before the first answer is open for the author to decide.--session-turnsdefaults to None (test_the_session_turns_flag_defaults_to_none, d14a762).- The entry-point test patches
ArmRunnerto fail on start (RunnerMustNotStart).
There was a problem hiding this comment.
arms run refuses agentic arms and session turns on claude-cli in _build_arms_output, before any answer. matrix-run now applies the same refusal to every cell before the first cell is answered, so an API cell does not run ahead of a refused CLI cell. Pinned by test_a_matrix_with_a_cli_agentic_cell_is_refused_before_the_first_cell_is_answered and test_the_arms_entry_point_refuses_a_cli_agentic_arm_before_any_answer. Fixed in 090122c. Correction: this branch has no tool-plugin kind, only prompt_middleware and output_shaper, so my earlier mention of tool plugins was wrong.
| Fixed, not fresh per call: the CLI states its working directory in the prompt, and | ||
| a random path would make every request, and its response-cache key, differ. | ||
| """ | ||
| path = Path(tempfile.gettempdir()) / "dokimi-cli" / name |
There was a problem hiding this comment.
<tmp>/dokimi-cli/claude-cli is shared by every user of the machine. If another user ran it first, mkdir fails with PermissionError for you; that is not a CliCallError and is not retried, so every answer fails. Anyone can also pre-create the directory (for example as a git repo), and the CLI puts its git status into the hidden prompt. Please use a per-user directory (for example under the user cache dir, mode 0700) and turn an OSError here into a CliCallError.
There was a problem hiding this comment.
Fixed in 35c5219, with the per-user test in 9147dfa. Head is 1688652.
The work dir is now per user: $XDG_CACHE_HOME/dokimi/cli/claude-cli, default ~/.cache/dokimi/cli/claude-cli, created with mode 0700. An OSError while creating it is raised as a permanent CliCallError that names the path, so it is not retried.
Tests: test_the_work_dir_is_under_the_users_cache_not_the_shared_temp_dir and test_an_unusable_work_dir_is_a_permanent_cli_error.
There was a problem hiding this comment.
The CLI work directory is now under the user's cache dir, created with mode 0700, so no other account can pre-create it. An OSError while creating it becomes a non-retryable CliCallError that names the path. Pinned by test_the_work_dir_is_under_the_users_cache_not_the_shared_temp_dir and test_an_unusable_work_dir_is_a_permanent_cli_error. Fixed in 35c5219.
| CLAUDE_ENV_DROP = frozenset({ | ||
| "CLAUDECODE", "CLAUDE_CODE_SESSION_ID", "CLAUDE_CODE_CHILD_SESSION", | ||
| "CLAUDE_CODE_ENTRYPOINT", "CLAUDE_CODE_SSE_PORT", | ||
| # Set, it wins over the CLI's login: the run would bill the key, not the subscription. | ||
| "ANTHROPIC_API_KEY", | ||
| }) |
There was a problem hiding this comment.
The drop list misses variables that change who is billed or which session the child talks to. ANTHROPIC_AUTH_TOKEN also wins over the CLI login, for the same reason you drop ANTHROPIC_API_KEY. In a Claude Code 2.1.303 session the child still gets CLAUDE_CODE_MESSAGING_SOCKET, CLAUDE_CODE_MESSAGING_TOKEN, CLAUDE_PID, CLAUDE_EFFORT and CLAUDE_CODE_SESSION_ATTENDED. Could you drop by prefix (CLAUDECODE, CLAUDE_*) and keep an explicit list of the login and provider variables?
There was a problem hiding this comment.
Fixed in 9147dfa. The child process drops every variable whose name starts with ANTHROPIC_, CLAUDE_ or CLAUDECODE (CLAUDE_ENV_DROP_PREFIXES). That covers ANTHROPIC_AUTH_TOKEN, CLAUDE_CODE_MESSAGING_SOCKET, CLAUDE_CODE_MESSAGING_TOKEN, CLAUDE_PID, CLAUDE_EFFORT and CLAUDE_CODE_SESSION_ATTENDED.
The keep list is explicit: CLAUDE_CONFIG_DIR and CLAUDE_CODE_OAUTH_TOKEN (CLAUDE_ENV_KEEP). The login check (detect) uses the same reduced env. Head is 1688652.
The prefix rule also drops the Bedrock and Vertex switch variables (CLAUDE_CODE_USE_BEDROCK, CLAUDE_CODE_USE_VERTEX), so the CLI uses its own login. This is intended and is in the changelog.
There was a problem hiding this comment.
The child process now drops every variable with prefix ANTHROPIC_, CLAUDE_ or CLAUDECODE, except CLAUDE_CONFIG_DIR and CLAUDE_CODE_OAUTH_TOKEN, and the login check uses the same reduced environment. Pinned by test_the_child_env_drops_billing_and_session_variables_by_prefix. Fixed in 9147dfa.
| if status.authenticated is False: | ||
| return f"{provider}: '{status.binary}' is not signed in ({status.hint})" | ||
| return None |
There was a problem hiding this comment.
When claude auth status times out or prints no JSON, detect returns authenticated=None and this returns None, so preflight passes with no message. Please report the unknown state at least as a warning, e.g. claude-cli: could not check the login (claude auth status gave no answer).
There was a problem hiding this comment.
Fixed in 35c5219. When claude auth status gives no answer, preflight now reports a warning: claude-cli: could not check the login ('claude auth status' gave no answer) (commands/benchmark.py). It is a warning, not an error, so the preflight still passes. Head is 1688652.
Test: test_an_unanswered_login_check_is_a_warning_not_a_pass.
There was a problem hiding this comment.
An unanswered login check is now reported as a warning, 'claude-cli: could not check the login', and preflight still passes. Pinned by test_an_unanswered_login_check_is_a_warning_not_a_pass. Fixed in 35c5219.
| if llm_cli.backend(request.provider) is not None: | ||
| return _cli_reply(request) |
There was a problem hiding this comment.
A CLI call still goes through LLM._key(), and api_key_env('claude-cli') falls back to OPENAI_API_KEY. With OPENAI_API_KEY=file:~/keys/openai and the file missing, every --provider claude-cli call fails with Cannot read API key from file before the CLI runs. Please skip the key for CLI providers, in _key() or in api_key_env.
There was a problem hiding this comment.
There was a problem hiding this comment.
A CLI provider never reads an API key now, so OPENAI_API_KEY=file:... with a missing file no longer fails before the CLI runs. Pinned by test_a_cli_provider_never_reads_an_api_key_file. Fixed in 35c5219.
| gen_a_p.add_argument("--output", default=None, help="Output file (default: answers/{product}.json)") | ||
| gen_a_p.add_argument("--model", default=DEFAULT_ANSWER_MODEL, help="LLM model for answering") | ||
| gen_a_p.add_argument("--provider", default=DEFAULT_ANSWER_PROVIDER, choices=["openai", "anthropic", "amazon-bedrock", "google-vertex", "openrouter", "openai-codex"]) | ||
| gen_a_p.add_argument("--provider", default=DEFAULT_ANSWER_PROVIDER, choices=ANSWER_PROVIDERS) |
There was a problem hiding this comment.
--provider claude-cli keeps the default answer model gpt-6.1-sol, so answers generate --provider claude-cli without --model sends --model gpt-6.1-sol to claude and every answer fails with an unknown-model error. The same holds for questions, personas and subjects run, where --model is hidden. Please refuse a non-Claude model for a CLI provider in parse_args, or pick a Claude default as default_judge_model does.
There was a problem hiding this comment.
Fixed in 35c5219, with the coverage test in 9147dfa. Head is 1688652.
A non-Claude model with --provider claude-cli is refused at parse time (cli.py:107): "takes a Claude model (sonnet, haiku, opus or a claude-* id)". This covers answers generate (default gpt-6.1-sol), questions, personas and subjects run, where --model is hidden.
Test: test_every_command_with_a_hidden_or_default_model_refuses_the_cli_without_a_claude_model.
There was a problem hiding this comment.
parse_args refuses a non-Claude model with --provider claude-cli, so answers generate, questions, personas and subjects fail at parse time and ask for --model. The parse-time test now includes answers generate, which it did not before, so my earlier claim was not backed by a test. Pinned by test_every_command_with_a_hidden_or_default_model_refuses_the_cli_without_a_claude_model. Fixed in 090122c.
| DEFAULT_JUDGE_MODEL, | ||
| DEFAULT_JUDGE_PROVIDER, | ||
| ) | ||
| from dokimi.llm import ANSWER_PROVIDERS as PROVIDERS |
There was a problem hiding this comment.
On @antonwolfy's point 2 (novelty default_judge on claude-cli): when you add the CLI branch, please compare by model family. With --model sonnet the bare-name check sees sonnet != claude-sonnet-5-5 and would pick the answerer itself as the judge, and novelty/measure.py:243 would not refuse it either.
There was a problem hiding this comment.
Fixed in 9147dfa. default_judge (novelty/client.py) on claude-cli compares by family through same_model: it picks the first of opus, sonnet, haiku that is not the answerer's family. So --model sonnet picks opus, not the answerer.
Test: test_the_novelty_default_judge_on_the_cli_is_another_family. Head is 1688652.
There was a problem hiding this comment.
default_judge on claude-cli now compares by family, so the judge is the first of opus, sonnet, haiku that is not the answerer's family. The test now covers full-id answerers, such as claude-opus-5-1, not only the bare alias. Pinned by test_the_novelty_default_judge_on_the_cli_is_another_family. Fixed in 9147dfa; test extended in 090122c.
github-actions
Bot
added
the
cli-interface-change
CLI interface change detectedThis PR changes dokimi's user-facing CLI surface (commands and/or options): + added command `dokimi backends`
+ option `--help/-h`
+ option `--json`
~ `dokimi answers generate` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi arms matrix-run` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi arms run` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi benchmark batch` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi benchmark preflight` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi benchmark run` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi eval checklist` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi eval judge-validity` parameters changed:
~ changed option `--escalate-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi eval judge-wording` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi eval pairwise` parameters changed:
~ changed option `--escalate-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi eval panel-score` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi eval reference` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi eval score` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ `dokimi evaluate` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi novelty analyze` parameters changed:
~ changed option `--extract-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi novelty atoms` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi novelty probe` parameters changed:
~ changed option `--extract-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi novelty routing` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi novelty shorten` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi personas discover` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi questions analyze` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi questions generate` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi questions panel-review` parameters changed:
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]
~ `dokimi subjects run` parameters changed:
~ changed option `--judge-provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google'", "'google-vertex'", "'openai'", "'openrouter'"]
~ changed option `--provider`: choices: ["'amazon-bedrock'", "'anthropic'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"] -> ["'amazon-bedrock'", "'anthropic'", "'claude-cli'", "'google-vertex'", "'openai'", "'openai-codex'", "'openrouter'"]Updated on every push; the |
… Claude Code CLI A provider in dokimi.llm_cli.BACKENDS runs one prompt through the CLI the user is signed in to instead of an API key. LiteLLMAdapter hands such a request to llm_cli, so retry, the response cache and usage totals are unchanged. Each call is isolated (no tools, no settings, no MCP, no session, never --bare), runs in one fixed empty directory and takes the prompt on stdin; the parent's Claude Code session variables are dropped. temperature/seed are reported as unsupported, cost_usd stays None (the CLI's figure is list price), and CliCallError says whether to retry. Also: cli.py backends (install/login status, no model call), benchmark preflight checks the CLI login instead of a key, claude-cli for the judge and get_llm, the --provider choices come from llm.ANSWER_PROVIDERS, and the JSON helpers move to llm_json so llm.py does not grow. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…pling, drops the API key Answerer, PersonaGenerator and QuestionGenerator kept their own provider lists, so answers generate / personas discover / questions generate --provider claude-cli raised "Unsupported provider". A judge built with --judge-temperature or a seed now fails instead of recording a value the CLI never applied. ANTHROPIC_API_KEY is no longer passed to the child: set, it wins over the login and the run bills the key. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ore the run The refusal was per call, so eval score exited 0 with an error row per answer and recorded a judge_temperature the CLI never applied. Tests now also pin --output-format json and the whole env drop list. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
JudgePanel builds its judges without Judge.__init__, so panel-score with --judge-temperature still ran and recorded the value. The check moves to llm_cli.check_sampling, called by both. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…rning - A CLI provider never reads an API key: OPENAI_API_KEY (the fallback for an unknown provider) no longer fails a call with a missing key file. - The work dir moves from the shared temp dir to the user's cache dir (0700); a dir that cannot be made is a permanent CliCallError, not an OSError. - orchestrate evaluate refuses --judge-temperature for a CLI judge before any persona, question or answer is paid for. - parse_args refuses a non-Claude --model for --provider claude-cli (the default answer model is not a Claude model). - Benchmark preflight warns when `claude auth status` gives no answer, instead of passing silently. - The env drop list also takes ANTHROPIC_AUTH_TOKEN and the Claude Code messaging, pid, effort and attended variables. - Tests: each retryable status and word alone, is_error and the exit code each alone, a judge panel with explicit judges, the work-dir failure. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
43eafc9 to
35c5219
Compare
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 35c5219. The earlier fixes hold: orchestrate eval now refuses a CLI judge's temperature before any work, the key path no longer reads OPENAI_API_KEY for claude-cli, the default answer model is refused for a CLI provider, the work dir is per user, and an unknown login now warns in preflight. One point from my earlier review is still open, and the test gaps in two earlier threads are unchanged. Three of @vlad-perevezentsev's points are also still open at this head.
What I ran, on a Linux review host at 35c5219:
make ci-local: 28 passed, 0 failed, 7 skipped. The changed-line coverage step passes.- Reproductions for the code findings, with the LLM transport mocked and no
claudebinary started. - A mutation run on
test_llm_cli.py: 27 mutants, including every entry of_RETRYABLE_STATUSand_RETRYABLE_WORDSand the preflight andcmd_evaluatewiring.
Still open from my earlier review:
dokimi/tests/test_llm_cli.py:111: the per-status and per-word tests are parametrized overllm_cli._RETRYABLE_STATUSand_RETRYABLE_WORDSthemselves, so dropping a member also drops its test. Dropping each of 408, 429, 500, 502, 503, 504 and 529, and each of the five words, leaves the suite green. Write the expected values as literals. Also pinusage limiton its own, and see the point on it in the thread.dokimi/tests/test_llm_cli.py:247:test_benchmark_preflight_checks_the_login_not_a_keystill calls_credential_errordirectly. Four mutants ofcmd_benchmark_preflightpass the suite: errors not appended, judge provider not checked, warnings not appended, and the API-key branch returningNone. The newcheck_samplingcall incmd_evaluate(commands/orchestrate.py:33) is also unpinned: replacing it withpasspasses the suite. The panel test with explicitJudgeConfigjudges is in place.
Findings outside the diff:
novelty ... --judge-provider claude-cliwithout--judge-modelstill fails.default_judgeinnovelty/client.py:263has no CLI branch.default_judge("sonnet", "claude-cli")raisesno default judge runs there other than the model under measurement (sonnet); pass --judge-model as well, although the judge is not the model under measurement.defaults.default_judge_modelwas extended, this was not, and the docs list novelty as supported. Suggestion: givedefault_judgethe same CLI branch, compare by model family (an aliassonnetandclaude-sonnet-5-5are one model), and add a test next totest_the_default_judge_runs_on_the_cli.
I agree with @vlad-perevezentsev's three open points (alias vs full id, agentic arms on claude-cli, the dropped model id), and reproduced each.
Checked, no issues
- Sampling refusal:
orchestrate eval --judge-provider claude-cli --judge-temperature 0exits before any persona is generated. - Key path: with
OPENAI_API_KEY=file:/missing, aclaude-clicall no longer fails on the key. - Answer model:
answers generate --provider claude-cliwithout--modelexits inparse_argswith a message naming the flag. - Docs and bookkeeping:
mkdocs build --strictis clean, the growth ratchet and import-architecture tests pass, every commit has aSigned-off-by, andllm_json.pyis byte-identical to the base functions.
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 fff2f19, which fix the nit from my review:
fff2f19:dokimi/docs/subscription-backends.md:66: the work dir is now named$XDG_CACHE_HOME/dokimi/cli/claude-cli, default~/.cache/dokimi/cli/claude-cli, asllm_cli.work_dirbuilds it.
make ci-local at fff2f19: 27 passed, 1 failed (host timing flake), 7 skipped.
GitHub CI has not run on these commits: the branch conflicts with main, and Actions runs no workflow on a conflicting PR. CI will check them once main is merged in.
Not approving yet. Still open: the bug from my review; the two earlier threads of mine above nit level; @vlad-perevezentsev on alias (sonnet) vs full id (claude-sonnet-5-5) treated as different models by the independence checks; reproduced; @vlad-perevezentsev on arms run/matrix-run accept agentic arms and --session-turns with claude-cli; each fails per answer; reproduced; @vlad-perevezentsev on resolved model id dropped by _cli_reply, not in usage or cache key; helper model can be picked from modelUsage; reproduced.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> # Conflicts: # dokimi/scripts/growth_baseline.json # dokimi/tests/test_import_architecture.py
… itself - Agentic arms (mcp_agent, skill_agent, skill_catalog) and --session-turns are refused on a CLI provider in arms run/matrix-run before any answer. - The child env drops ANTHROPIC_*, CLAUDE_* and CLAUDECODE by prefix, keeping CLAUDE_CONFIG_DIR and CLAUDE_CODE_OAUTH_TOKEN. - An alias and its full id are one model for the independence warning and the novelty self-judge refusal; the novelty default judge on a CLI is another family. - A usage-limit error is not retried; the reply names the main model the CLI used. - Preflight checks the login through _provider_checks; tests pin the retry lists. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
- arms: import refuse_cli_arms (a NameError on every arms run before this). - Model identity strips a Bedrock or LiteLLM prefix before the family match, so an alias and its prefixed full id are one model. The main-model fallback is not taken for a model with no family. - default_judge takes the answerer's provider: a CLI answerer with no judge provider judges on the same CLI, another family. Drops the redundant alias check. - Tests: every agentic arm kind through the wrapper and the combined arm, the arms entry point refuses before any answer, the login check uses the reduced env, the model gate names --model, and the refusal cases pass required args. - Changelog: a Bedrock or Vertex route is dropped with the rest of ANTHROPIC_/CLAUDE_. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
- The entry-point test makes any arm-runner start fail, so the refusal is checked to come before any answer machinery. - A test pins --session-turns to None by default (parse_args on arms run). - The login check test pins the reduced env (already the case at head). - A comment on the claude-cli alias list for a second CLI backend. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The arms entry point test now fails with 'the arm runner started before the CLI refusal' when the refusal is removed; before, it failed on a missing Namespace field. The session default test is renamed to what it checks. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Drop an unused loop variable, and a literal temp path in the env-drop test (bandit B108), whose value does not matter to the drop rule. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… on the CLI version - Usage rows carry resolved_model, the model the CLI resolved its alias to. Sums keep every name they mix, sorted and comma-separated; as_metrics_dict shows it when set, so non-CLI rows are unchanged. - The judge record lists judge_resolved_models. - The response-cache key takes the CLI version for a CLI provider. A new release can move an alias such as sonnet to another model, and the resolved id is only known after the call, so the version stands in for it. A move that ships with no CLI release is not caught. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… attribute exists Review round 4 (nit). The flag was only needed before the attribute existed. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The login checks were covered through _provider_checks only, so a preflight that dropped an error, skipped the judge's login, or lost the unknown-login warning still passed. The new test runs cmd_benchmark_preflight end to end with a fake claude: signed out exits 1, an unanswered login prints the warning and passes, a signed-in judge passes, and a signed-out judge with an API answerer exits 1. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Replies to the open reviews, at head a3c2923 (one test-only commit on top of 1688652). antonwolfy, "Not approving yet" (fff2f19)
vlad-perevezentsev (43eafc9 review)
antonwolfy (35c5219 review)
antonwolfy (563eed3 review)
|
…t gaps - arms matrix-run refuses a claude-cli cell with an agentic arm before any cell is answered, as the sampling check already does (_refuse_cli_cell). - drop the usage-limit entry from the retryable words: _claude_error refuses that text before the tables are read, so the entry was dead. - tests: answers generate joins the parse-time model refusal; the novelty default judge is pinned for full-id answerers (claude-opus-5-1) as well as bare aliases. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…/dokimi into feat/claude-cli-backend Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… front default_judge compared bare names, so --model sonnet on claude-cli with --judge-provider anthropic picked claude-sonnet-5-5 as judge. same_client refused it only after the paid extraction; compare by family instead, so the run stops in role selection, as it does for a full id on main. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The only test called default_judge with an explicit claude-cli provider, so dropping the CLI-answerer inheritance or the same-family skip survived the suite. Parse novelty analyze with no judge flags instead. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
arms run and matrix-run build the judge record in ArmRunner, which copied the judge's usage totals but not the models a CLI alias resolved to, so only eval score wrote judge_resolved_models as the changelog says. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
…ude model The parse-time gate reads --model only; a matrix cell sets the provider and model after it, so claude-cli/gpt-6.1-sol in a cell sent `claude --model gpt-6.1-sol` for every answer. Check the pair where the cell is applied. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
The golden_manifest ceiling conflicted in benchmarks/spec.v1.yaml (this branch raised it to 33, main to 35); it is set to 36, the measured docs/**/*.md selection of the merged tree, main's 35 plus this branch's subscription-backends.md. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
Merged main after #271 and #155 (9ad3fe1): the one conflict was |
feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI
Summary
New provider
claude-cli: the Q&A track (answers, judge, question generation, personas, novelty) runs on a logged-in Claude Code CLI instead of an API key.cost_usdisNone: the CLI'stotal_cost_usdis list price, not what a subscription costs.--temperature/--judge-temperature/ seed withclaude-clifail before the run (orchestrate evalincluded);max_tokensis ignored.--modelwith a CLI provider fails at parse time.arms run/matrix-runrefuse agentic arms and--session-turnsbefore any answer is paid for.ANTHROPIC_*,CLAUDE_*andCLAUDECODEare dropped by prefix, exceptCLAUDE_CONFIG_DIRandCLAUDE_CODE_OAUTH_TOKEN. That also drops Bedrock and Vertex switches, so the CLI uses its own login;backendsreports that login, not the route.resolved_model(the model the CLI resolved the alias to); the judge record listsjudge_resolved_models. The response-cache key takes the CLI version for a CLI provider, so a new release that movessonnetdoes not replay the old model's replies.Evidence
answers generate --provider claude-cli→ argparse error; with the choice added,AnswererraisesUnsupported provider: 'claude-cli'.After:
tests/test_llm_cli.py(60 collected items) andtests/test_judge_sampling.py(17): argv isolation, stdin prompt, env drop by prefix, retryability per error, missing binary, unusable work dir, LLM retry and metering, judge/panel/answerer/generators, sampling refused, alias vs full-id identity, default judge family, arms and session refusals (entry point, with the runner patched to fail on start), resolved model on usage rows, judge record and cache replay, cache miss on a new CLI version,detect,backends, preflight login check.TypeError/AttributeErrorbefore;'OLD' == 'NEW'before for the version key).make ci-localon the working tree before1688652d: pytest 9755 passed, 5 skipped; ruff, bandit and mypy pass. pip-audit fails on filelock advisories, which this diff does not touch (no dependency file changed).PONGwith token usage, judge call OK, bad model → non-retryable error, missing binary → "not installed") was taken before the Bedrock/Vertex drop. This box signs in through Bedrock, so that smoke cannot be reproduced here. Not tested: a subscription OAuth login, real rate limits.8d65d107..1688652d) reported no blocking findings; its nits were either fixed or declined with the reason in the review record.Known gaps
cli_version: "", shared by every such process.Follow-ups (next PRs):
codex-cli+--judge-providerper role;--provider auto/ matrix; CLI version and list cost in run metadata; one provider registry instead of per-class lists; share theclaude -pisolation recipe withharnesses/;--system-prompt-filefor system prompts over 128 KiB.Merge Danger
Door: two-way
New provider name and a new
backendscommand; nothing existing changes shape. Theclaude-cliname is a soft door (artifacts record it);claude-codeis the harness-side name for the same binary — worth settling before people store results.Blast Radius: additive
API providers take the same path as before; the only shared edits are the
--providerchoice lists (now one tuple, same members plusclaude-cli),is_retryablereading anexc.retryableattribute first, and the usage row / judge record gaining an optionalresolved_modelkey that is absent for API providers.🤖 Generated with Claude Code