Repository navigation
feat(llm): claude-cli provider: run the Q&A track on a logged-in Claude Code CLI - #255
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #255 +/- ##
==========================================
+ Coverage 93.84% 93.92% +0.07%
==========================================
Files 280 284 +4
Lines 36366 36767 +401
Branches 6142 6200 +58
==========================================
+ Hits 34129 34532 +403
Misses 1437 1437
+ Partials 800 798 -2
🚀 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.
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>
|
Decision taken: a settings-file login is reported as signed in, and the label says where it comes from. Fixed in 90b984f.
The label says where the login comes from. It does not promise that a call can use it: calls still run with Tests: |
| This calls no model. A backend that is not signed in prints how to fix it. A Bedrock | ||
| route that `~/.claude/settings.json` sets is reported as `signed in (settings file)`. On a |
There was a problem hiding this comment.
Must-fix: distinguish a detected route from a login usable by this backend. With CLAUDE_CODE_USE_BEDROCK and credentials configured only in ~/.claude/settings.json, detect() runs claude auth status without --setting-sources "" and preflight accepts its third_party status. Actual model calls disable those settings sources and drop the routing environment variables, so that reported route is unavailable and a user without a separate Claude login cannot run the example. Qualify this paragraph to explain that signed in (settings file) is not a successful isolated-backend credential check and that a separate login/token is required; ideally make detection use the same settings policy as execution.
There was a problem hiding this comment.
I agree, and the code side needs it too: third_party from auth status means "unknown" for a call, not "signed in".
The call drops the CLAUDE_* env and runs with --setting-sources "", so it never uses the settings-file route. With a Claude login as well, the call runs on that login; with the settings file alone, every call fails with "Not logged in". In both cases _login_source labels it signed in (settings file), and preflight puts it in notes, not warnings, so it passes with no warning. Your own earlier proposal was "keep it signed in, but warn".
Suggestion: in _provider_checks, add a warning for source == "settings file", e.g. claude-cli: auth status reports a settings-file route (third_party); calls ignore settings files and use the CLI's own login, and say the same on this page.
There was a problem hiding this comment.
The docs part is in 5d3657c: signed in (settings file) is a label for where the route comes from, not a successful credential check, and calls ignore settings files. Waiting on a decision before I change this: should preflight and backends also warn that calls ignore settings files? Options: (a) label only, as now (the decision in my comment of 22:49Z); (b) the label plus a one-line warning in preflight and backends; (c) report the route as not signed in. My recommendation: (b), which never blocks a working setup and is what the reviewers asked for. Detection on the same settings policy as calls is a follow-up, since it changes what auth status reports. The thread stays open until then.
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 90b984f. My points from the earlier rounds are fixed, and so are the bot points I confirmed at 3aed8c8: an inherited GIT_DIR, a null model, prefixed and Bedrock-pinned ids, the matrix model check, the exit code, the CLI version cache and the system prompt, which now goes by file. One point still needs a change before merge: a settings-file login is reported as signed in, and preflight passes with no warning.
Agreed with @exai-bot on the settings-file login (I added the code side in its thread), on the structured 400 that a rate_limit word makes retryable, and on the unchecked --judge-model for a CLI judge. On chat_completion returning no response, I replied that no command reaches it with a CLI provider.
What I ran, at 90b984f:
make ci-local --offline: 19 passed, 0 failed, 13 skipped (the skips need the network or docker); pytest 10,779 passed.- With a fake CLI runner:
same_modelwith a null model givesFalse, andclaude-sonnet-5-5againstanthropic/claude-sonnet-5-5givesTrue._claude_errorwithapi_error_status: 400andrate_limitin the text givesretryable=True, and_login_source("third_party (bedrock)")givessettings file.evaluate --judge-provider claude-cli --judge-model gpt-6.1-solparses. claude --help(2.1.303) lists--system-prompt-file, the flag the call now uses.- No live
claude -pcall.
Checked, no issues
- Env and git: the child drops
GIT_DIR,GIT_WORK_TREE,GIT_COMMON_DIRandGIT_INDEX_FILEas well as theCLAUDE_*/ANTHROPIC_*variables, and git stops at the work dir's parent. - System prompt: it is written to a file in a private temp dir for each call, so the argv limit no longer applies, and a call with no system prompt still sends the short default.
- Retries: an HTTP status printed as text is read only after a status word and only when there is no structured status, and a usage limit is never retried.
- Arms: the typed
run_matrixfrom #270 checks every cell with the arm specs before the first one runs.
Resolve conflicts in cli.py and llm.py against intel/main (72d7add). Keep both register lines (backends, data). Review fixes in this merge: - yJ-io, yJ-ir: backends command and claude-cli choices follow the intel#284 route table (cli_routes.py) and the F1a flag registry. - yKThl: judge model is refused at parse time for a CLI judge when the model is not a Claude model (exit 2 via parser.error). - yKvVP: docs say a settings-file login is a route label, not a successful credential check. - Structured 400 is not retried because its message names a retryable word; status-None branch still matches on text. - chat_completion refuses a CLI provider before any call (llm_cli.refuse_chat_completion). - Growth ratchet: llm.py, arm_runner.py and test_llm_cli.py kept under their caps; identity tests moved to test_llm_cli_identity.py. - CLI snapshot regenerated (docs/reference/cli-interface.json). Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The refusal unwraps a plugin and tests the arm it wraps, so say that rather than 'a tool plugin'. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
These are usage errors (exit 2 by the exit rule), but they were ValueError, which the arms commands turned into exit 1. Raise InputError, keep its type through the matrix cell prefix, and assert the exit code through cli.main. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The merge 5d3657c carries the fixes for the three threads below. The other lines name commits of this PR that already fixed their review's points. Thread: PRRT_kwDOVBg7g86yJ-io Thread: PRRT_kwDOVBg7g86yJ-ir Thread: PRRT_kwDOVBg7g86yKThl Thread: PRR_kwDOVBg7g88AAAABVGaogw: already fixed in 772801c, 9134bca, cbefbe3, 3aed8c8, 1b6d131 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
The three points of this review are fixed: CLAUDE_CODE_GIT_BASH_PATH kept (e643423), the 1M-context model form (2881692), and git stopped at the work dir parent (d436854). Waiting on a decision before I change this: the settings-file login. The label is in 90b984f; the warning asked for in this review is open. Options: (a) label only; (b) label plus a one-line warning in preflight and backends; (c) report as not signed in. My recommendation: (b). The thread stays open until then. |
|
The five nits are answered under the review of 3aed8c8. Waiting on a decision before I change this: the settings-file login, which detect still reads only as loggedIn. Options: (a) label only; (b) label plus a one-line warning in preflight and backends; (c) report as not signed in. My recommendation: (b). The thread stays open until then. |
|
The points agreed with exai-bot are fixed in 5d3657c: the structured 400 is not retried, the CLI judge's --judge-model is refused at parse time, and chat_completion refuses a CLI provider. Waiting on a decision before I change this: the settings-file login, with the same options (a) label only, (b) label plus a one-line warning in preflight and backends, (c) report as not signed in. My recommendation: (b). The thread stays open until then. |
|
Waiting on a decision before I change this: the settings-file login. The label |
|
Waiting on a decision before I change this: the settings-file login. The label |
|
Waiting on a decision before I change this: the settings-file login. The label |
|
Same finding as the thread on dokimi/docs/subscription-backends.md:26. The docs part is in 5d3657c. Waiting on a decision before I change this: the settings-file warning, with the options and my recommendation (b) given in that thread. The thread stays open until then. |
| if "usage limit" in lowered: # a subscription cap: it resets in hours, a retry cannot help | ||
| return CliCallError(f"claude-cli: {message}", retryable=False) | ||
| if status is None: # no structured status: the words, and a status the CLI printed as text | ||
| retryable = any(w in lowered for w in _RETRYABLE_WORDS) or _text_status_retryable(lowered) |
There was a problem hiding this comment.
Must-fix: a permanent HTTP status printed in the message does not override transient words. For example, _err("API Error: 400 Bad Request: invalid field rate_limit", None) is classified as retryable, even though the explicit 400 should fail immediately; the non-JSON branch in _claude_parse has the same problem. Parse the first textual status and, when present, use its retryability exclusively, falling back to word matching only when there is no status. Add regressions for both JSON-without-status and non-JSON 400 messages containing a retry word.
| auth = runner(list(cli.auth_argv), timeout=30, env=env) | ||
| authenticated, method = cli.parse_auth(auth) |
There was a problem hiding this comment.
Must-fix: this authentication probe does not disable settings, whereas completion calls use --setting-sources "". On a machine whose only authentication is a Bedrock route in ~/.claude/settings.json, detect() returns authenticated=True, and benchmark preflight prints Preflight OK, but the subsequent completion fails with "Not logged in" because that route is disabled. The new docs explicitly describe this case, and the tests currently assert that it passes preflight. Probe authentication with the same settings isolation as completion calls, or report settings-only third-party authentication as unverified rather than a successful login; add a regression for a settings-only route.
auth status read settings.json, which a call ignores: a Bedrock route set only there passed preflight and every call failed with "Not logged in". The probe now runs claude --setting-sources "" auth status, as a call runs; measured on Claude Code 2.1.300. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
"API Error: 400 Bad Request: invalid field rate_limit" with no structured status was retried three more times: the retry words were checked before the 400. The text status now decides when there is one, as a structured status does; the words count only when the text names no status. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
arms: check_matrix(config, cells, specs) now runs inside main's _load_matrix_inputs; a CLI cell refusal stays an InputError (exit 2) and a failed probe exits 1. benchmark preflight: _provider_checks takes main's refusals list, so a missing API key exits 2; a CLI not installed or signed in stays a failed check (exit 1). novelty: check_self_judge takes the clients and uses same_client, so the pre-call check counts a CLI alias as its family's id. growth_baseline.json: lowered with scripts/check_growth.py --update after the merge. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
…not signed in The exit-code table counts a missing provider credential as exit 2, and a CLI backend's login is its credential, as an API key is for the others. Every _provider_checks error is now a refusal. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
| CLAUDE_ISOLATION = ("--tools", "", "--setting-sources", "", "--strict-mcp-config", | ||
| "--no-session-persistence") |
There was a problem hiding this comment.
Must-fix: --setting-sources "" disables settings.json sources, not CLAUDE.md instruction loading. A user with ~/.claude/CLAUDE.md (or CLAUDE.md in an ancestor of this cache directory) can still inject instructions into every completion; replacing the system prompt and disabling tools do not disable that context. This makes answers depend on local instructions absent from the response-cache key and contaminates the A/B evaluation. Isolate instruction loading separately while preserving authentication, and add an integration regression using a sentinel instruction in the user's CLAUDE.md rather than only checking the subprocess argv.
There was a problem hiding this comment.
This does not reproduce on Claude Code 2.1.300. I ran it with a sentinel in ~/.claude/CLAUDE.md (end every reply with PINEAPPLE7) and another in a CLAUDE.md in the working directory (start every reply with MANGO9), with --system-prompt set:
- With the isolation flags this PR passes (
--tools "" --setting-sources "" --strict-mcp-config --no-session-persistence), the reply had neither sentinel, in 2 of 2 runs. - With the same flags minus
--setting-sources "", the reply wasMANGO9 … PINEAPPLE7, in 2 of 2 runs.
So an empty --setting-sources also stops the user and project CLAUDE.md from loading, and the answers do not depend on local instructions. No change needed.
|
Main reached 36 docs with intel#255, and this branch adds docs/runs.md, so the merged tree has 37. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
* feat(runs): scan the runs root and tell each artifact's kind from its first bytes New package dokimi/runs, in the tracks layer. runs_root() is DOKIMI_RUNS_DIR when set, else results/ in the working directory, where most commands write by default. A missing default root means no runs yet; a DOKIMI_RUNS_DIR that does not exist, or a root that is not a directory, is an InputError. scan() walks the root once. save_artifact writes schema_version as the first key, so a file whose first 256 bytes open with the schema_version of a registered kind (current or superseded) is an artifact and is parsed; any other file is counted by why it was skipped (no leading schema_version, an unknown one, *.tmp, *.rejected, not .json, not a regular file such as a FIFO, a symlinked directory) and never parsed. A file that cannot be read, or whose body does not hold the version its head names, is listed as unreadable with its reason. Artifacts come newest first by the time they record, by mtime when they record none. No field is shared by every kind, so dokimi/runs/kinds.py names per kind where the subject, the model and the time are; a kind without one reads as empty, never as a guess from the path. A test fails when a kind of SCHEMA_VERSIONS has no row. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * feat(runs): runs list dokimi runs list prints one row per artifact under the runs root: schema_version, subject, model, recorded time (UTC, to the minute; the file's mtime marked * when the artifact records none) and the path relative to the root, newest first. Under the table a summary counts the artifacts, every file skipped by reason (unknown schema_versions by name) and the unreadable files with the first one's reason. --kind KIND (repeatable, a kind of SCHEMA_VERSIONS) and --subject (an exact match on one of the artifact's subjects) filter the rows. --json prints the rows with the #94 field names (schema_version, kind, run_id, subject, model, generated_at, path, size_bytes; run_id is null until #277) plus root, total, skipped, unknown_versions and unreadable. A missing default results/ prints "no runs" and exits 0. A DOKIMI_RUNS_DIR that does not exist or is not a directory, or a results/ that is not a directory, exits 2 through InputError. --subject is a new local flag, allowlisted with its reason; --kind was already on the list. The CLI snapshot is regenerated. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * feat(runs): runs show dokimi runs show PATH prints one artifact: its path, kind and schema_version, the result of its schema check, subject, model, time (the file's mtime when the artifact records none), size, the length of the lists it holds and the entries next to it, each .json one with the schema_version it opens with (the first 20 named, the rest counted). PATH is the path runs list prints, relative to the root, or an absolute or working-directory path that lies inside the root. The document is checked against the schema of the version it declares, so a superseded version is checked against its own schema. A failed check prints "schema: invalid, N errors" and every error and exits 0: showing is not a gate. A path outside the root, a missing file, a directory, an entry that is not a regular file (a FIFO), a file that does not open with the schema_version of a registered kind, or one that does not parse, exits 2. Control characters in the printed fields are escaped. --json prints the runs list row fields plus root, schema, schema_valid, schema_errors, contents, siblings, siblings_total and siblings_error. The kinds table gains the lists to count per kind; a test checks that each is a top-level array of the kind's schema. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * docs(runs): the runs root, runs list and runs show docs/runs.md: what the runs root is and how DOKIMI_RUNS_DIR moves it, an example of each command, which file counts as an artifact and why the rest are counted but not read, where subject, model and time come from, what is not listed yet (no run_id grouping until #277; repo-runs/, answers/ and eval/ written without --run-id, and output flags pointing outside the root), the --json fields and the exit codes. The page goes in the mkdocs nav and the docs index, and quickstart's output layout points to it. The static benchmark's max_docs rises to 36 for the new page, as its spec asks. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): print unknown versions escaped; an mtime past 9999 does not crash head_version keeps any character but an ASCII control, so an unknown schema_version could carry a Unicode line separator (U+2028) into the runs list summary and the runs show error. Both now go through the one escaping helper, plain(), moved to dokimi/runs/scan.py. tmpfs keeps an mtime past the year 9999, where datetime.fromtimestamp raises: the text of runs list and runs show crashed on an untimed artifact with such an mtime (--json did not). The time column and the show line now print it as @<seconds>. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): a root or an address that cannot be looked up exits 2 On Python 3.11-3.13 Path.exists(), is_dir() and is_file() raise PermissionError for a path under a directory that cannot be searched. RunsRoot.exists() (a DOKIMI_RUNS_DIR below such a directory) and runs show's resolve() (an address in such a directory) let it out, and the command crashed with a traceback and exit 1. Both now raise InputError with the reason, so the command prints it and exits 2. Python 3.14 answers False there, and the message is the existing "does not exist" / "no file"; the tests accept either. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): nothing outside the runs root is read runs show checked that the address lay inside the root as spelled, so results/linked -> /elsewhere let runs show linked/eval.json read a file outside the root (and list its neighbours), and runs list read a file symlink that pointed outside. #278 lists only what is under the root. The rule is now the same for both commands: a file is read only when its real path lies inside the real root. runs list counts a file symlink that leads out as skipped (linked_outside, "symlinks to files outside the root (not read)"); runs show exits 2 with "... leads outside the runs root"; a neighbour that links out is named but its head is not read. A symlink inside the root, and a root that is itself a symlink, work as before, and runs show keeps the name the address used. Symlinked directories are still not walked by runs list. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * test(runs): a time that is not ISO 8601, an artifact that cannot be opened Two documented behaviours had no test (Codecov on #307): runs list prints a recorded time it cannot parse as written, and runs show exits 2 on an artifact it may not open. The open error now takes its wording from read_error_reason, as every other read error does; the message is unchanged. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * docs(runs): only a file of a registered kind is parsed, so only it can fail to docs/runs.md said a file that does not parse is listed as unreadable. Only a file that opens with a registered schema_version is parsed; a broken file of no registered kind is counted as skipped. Say so, and that the text summary names the first unreadable file while --json lists all. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): name the address and the root escaped in error messages cli.main prints an InputError as it is, so a runs show address or a DOKIMI_RUNS_DIR with a newline or an escape sequence reached stderr raw. Every message of resolve(), _version(), show(), RunsRoot.exists() and runs show's missing-root error now names them through plain(); the paths used to read files are unchanged. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): runs list does not walk into a Windows junction A junction is not a symlink, and os.walk(followlinks=False) still descends into one on Python 3.11-3.13, so runs list read and listed the files of a junction that points outside the root (runs show already refused them by real path), and a junction back into the root would be walked twice. A directory that is a symlink or a junction (st_reparse_tag IO_REPARSE_TAG_MOUNT_POINT, which 3.11 reads too) is now pruned before the walk descends and counted as "symlinked directories and junctions (not walked)". The test runs on Windows only. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): runs show reads the file the filesystem opens for link/.. resolve() checked the address as the filesystem resolves it, then made it absolute with os.path.abspath, which folds "link/.." before the symlink is followed: with results/link -> results/nested/deep, runs show link/../eval.json checked results/nested/eval.json and read results/eval.json. The path is now made absolute with Path.absolute(), which keeps "..", and an address with ".." is shown and read by its real path. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * test(runs): raise the RecursionError instead of finding it by nesting The test nested 100 000 arrays to make json raise RecursionError. On Python 3.14 the C parser is bounded by the C stack, not the recursion limit, and parses that document, so the test failed on CI's 3.14 leg while the scanner was right. The parse now raises RecursionError through a patched load_document, and the test checks that the file is listed as unreadable with that reason, on every interpreter. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * docs(runs): the schema_version must open the first 256 bytes docs/runs.md and the changelog fragment said a file is an artifact when it opens with a registered schema_version. head_version reads only the first 256 bytes, so a file whose first key comes later (after 1000 spaces, say) is skipped. Say that; save_artifact's output always fits. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): runs show lists neighbours from inside the root only resolve() accepts an address whose real path is inside the root, and keeps the spelled path for display. With results/linked -> /elsewhere and /elsewhere/return.json -> results/real.json, runs show linked/return.json was accepted, and its neighbours were listed from /elsewhere: outside file names and schema_versions in the output. When the spelled directory does not resolve inside the root, the neighbours now come from the artifact's own (real) directory. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * fix(runs): runs show takes an absolute address under a root named with .. resolve() kept the spelled path when the address had no "..", and returned it with root.absolute(). For a root such as DOKIMI_RUNS_DIR=../results that base still holds "..", and read_artifact's lexical relative_to failed: an absolute address of a valid artifact exited 2. A ".." in the root now takes the real-path branch too, so the address is shown relative to the real root. Refs #278 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> * ci(windows): run the runs scan and show tests in windows-smoke test_a_junction_is_counted_and_not_walked skips off Windows, so no CI job ran the only test of the runs-root containment on Windows. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> * fix(benchmark): raise max_docs to 37 for docs/runs.md Main reached 36 docs with #255, and this branch adds docs/runs.md, so the merged tree has 37. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> --------- Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Co-authored-by: Alexander Rybkin <alexander.rybkin@intel.com>
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