From b9790899f692e64419ecb7b4368bfde06215903a Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 13 Nov 2026 11:51:15 +0000 Subject: [PATCH 1/5] fix(novelty): routing estimate and permanent-error stop (#676, #677) Signed-off-by: Anton Volkov #676: `novelty routing --estimate-only` sized the selection prompt at chars / 4 and a reply at 65 tokens, which priced a measured 35-skill run about 1.55x below its bill. The input is still counted locally, now at SELECTION_CHARS_PER_TOKEN = 2.6 characters per token, and the flat reply allowance is 150 tokens, inside the measured 114 to 193. The estimate JSON carries chars_per_input_token, the stderr line names both allowances and says the old figures were about 1.55x low, and the docs say the figure is an estimate, not a bound. The estimate code moves to novelty/routing_estimate.py so routing.py stays under its growth baseline. #677: the per-call catch of the routing loop recorded every exception as one more failed row, so a missing credential ran all 270 calls. A permanent error (dokimi.llm.is_retryable is False) now stops the loop at that call through the path the spend cap uses: the rows made so far are kept, the result carries cap_stop and a partial block, and the message holds the error text and the call it stopped at. A refusal (ContentPolicyViolationError), an unparseable reply and a retryable error that ran out of retries are still rows, and the loop goes on. The exit code is unchanged. --- ..._150000_routing-estimate-permanent-stop.md | 4 + dokimi/docs/skill-novelty.md | 15 ++- .../dokimi/commands/novelty_routing_report.py | 21 ++-- dokimi/dokimi/novelty/routing.py | 55 +++-------- dokimi/dokimi/novelty/routing_estimate.py | 50 ++++++++++ dokimi/scripts/growth_baseline.json | 1 - dokimi/tests/test_novelty_routing.py | 8 +- .../tests/test_novelty_routing_all_vs_all.py | 15 +-- dokimi/tests/test_novelty_routing_cli_gaps.py | 16 +-- dokimi/tests/test_novelty_routing_cost.py | 2 +- .../test_novelty_routing_estimate_and_stop.py | 98 +++++++++++++++++++ dokimi/tests/test_novelty_routing_gaps.py | 2 +- 12 files changed, 216 insertions(+), 71 deletions(-) create mode 100644 dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md create mode 100644 dokimi/dokimi/novelty/routing_estimate.py create mode 100644 dokimi/tests/test_novelty_routing_estimate_and_stop.py diff --git a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md new file mode 100644 index 000000000..a042ba85a --- /dev/null +++ b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md @@ -0,0 +1,4 @@ +### Fixed + +- `novelty routing --estimate-only` sizes the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and prints both allowances. (#676) +- `novelty routing` stops at the first permanent provider error (authentication, permission, unknown model, no credentials) instead of failing every remaining call; the result is partial, with the error and the call it stopped at. A failure of one query is still one failed row. (#677) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 045a114eb..9d06680f5 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -725,11 +725,14 @@ others are named on stderr. `--estimate-only` prints, as JSON, the calls, input and output tokens and the list-price `expected_usd` the `--model` arm would make, and makes no model call. Without `--model` the run is lexical and the bill -is zero. Input is the selection prompt sized at chars / 4, so it is a projection, not a count. -Output is `output_tokens_per_call` (65) times the calls: the average a measured routing run billed per -selection call (360 calls drew 23.3k output tokens). The prompt asks for one `{"pick": N}` object, about -10 tokens, which underestimated that run 6.5 times, so the estimate does not use it. Another model, or a -reasoning model, can bill more or less. +is zero. Input is the selection prompt sized at `chars_per_input_token` (2.6) characters per token, counted +locally with no tokenizer, so it is a projection, not a count. Output is `output_tokens_per_call` (150) times +the calls: one flat allowance, inside the 114 to 193 tokens a call that Sonnet 5.5 and Haiku 5.5 billed. +The prompt asks for one `{"pick": N}` object, about 10 tokens, which underestimated a measured run 6.5 +times, so the estimate does not use it. Both allowances come from one measured catalog (35 skills, 90 +queries, Bedrock): with chars / 4 and 65 tokens the estimate was about 1.55x below the bills there. Another +model, a catalog of other text, or a reasoning model can bill more or less, so read the figure as an +estimate, not a bound. The command prints both allowances. Two numbers, reported side by side and never merged: @@ -820,6 +823,8 @@ so count the queries first, or pass `--max-usd` to stop the run at a spend limit A stored response in `DOKIMI_RESPONSE_CACHE` is replayed without a call, so a rerun can make fewer calls than the printed count. +**A permanent provider error stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID or missing credentials (`dokimi.llm.is_retryable` is false) would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with the error text and the call it stopped at as the message (`reason` stays `"spend_cap"` in the record). A failure of one query (a refusal, an unparseable reply, a transient error that ran out of retries) is still one failed row, and the run goes on. + **A routing run stopped by `--max-usd` keeps what it paid for.** `novelty routing --model`, single artifact or `--all-vs-all`, writes the result of the calls made before the cap (or a call with no known price) stopped it, prints the summary and `cost` lines, and still exits non-zero with the diff --git a/dokimi/dokimi/commands/novelty_routing_report.py b/dokimi/dokimi/commands/novelty_routing_report.py index 67f427ca5..9e6b5073b 100644 --- a/dokimi/dokimi/commands/novelty_routing_report.py +++ b/dokimi/dokimi/commands/novelty_routing_report.py @@ -62,7 +62,7 @@ def estimate_routing_usd(model: str, provider: str, prompts: list[int], priced on their own, then summed and multiplied by the repeats. ``None`` when a call has no list price. """ from dokimi.llm_pricing import list_cost_usd - from dokimi.novelty.routing import SELECTION_REPLY_TOKENS + from dokimi.novelty.routing_estimate import SELECTION_REPLY_TOKENS total = 0.0 for prompt in prompts: @@ -310,7 +310,11 @@ def drop_queries_left_out(queries_by_skill: dict[str, list[str]], field: list[Ca def _print_estimate(args: argparse.Namespace, candidates: list, sent: list[str], estimate: dict) -> None: """The estimate as JSON, with the list-price USD of the calls ``sent`` makes; nothing is sent.""" - from dokimi.novelty.routing import SELECTION_REPLY_TOKENS, selection_prompt_tokens + from dokimi.novelty.routing_estimate import ( + SELECTION_CHARS_PER_TOKEN, + SELECTION_REPLY_TOKENS, + selection_prompt_tokens, + ) usd: float | None = 0.0 if args.model and estimate["n_calls"]: @@ -320,9 +324,11 @@ def _print_estimate(args: argparse.Namespace, candidates: list, sent: list[str], "provider": args.provider if args.model else None, "n_candidates": len(candidates), **estimate, "expected_usd": usd}, indent=2)) - print("Estimate only, nothing was measured. Input is the selection prompt at chars / 4 per " - f"call; output is {SELECTION_REPLY_TOKENS} tokens a call, the measured average of a " - "routing run (output_tokens_per_call). Another model, or a reasoning model, can differ.", + print("Estimate only, nothing was measured. Input is the selection prompt at " + f"{SELECTION_CHARS_PER_TOKEN} characters per token (chars_per_input_token); output is " + f"{SELECTION_REPLY_TOKENS} tokens a call (output_tokens_per_call), inside the 114 to 193 " + "that two models billed. With chars / 4 and 65 the estimate was about 1.55x below the " + "bills. Another model, a tokenizer or a reasoning model can differ.", file=sys.stderr) if args.model and estimate["n_calls"] and usd is None: print(f"expected_usd is null: {args.model} has no list price in the litellm price map", @@ -334,7 +340,7 @@ def print_routing_estimate(args: argparse.Namespace, candidates: list, queries: No client is built. Without ``--model`` the run is lexical only, so the bill is zero. """ - from dokimi.novelty.routing import selection_estimate + from dokimi.novelty.routing_estimate import selection_estimate # Without --model there is no selector, so no call is made, whatever --repeats says. estimate = selection_estimate(candidates, queries, @@ -349,7 +355,8 @@ def print_all_vs_all_estimate(args: argparse.Namespace, field: list[Candidate], The model arm sends each query with a content word, every skill's queries with the whole field on each call, so those are what is counted. ``n_queries`` is every query the run would route. """ - from dokimi.novelty.routing import selection_estimate, tokenize + from dokimi.novelty.routing import tokenize + from dokimi.novelty.routing_estimate import selection_estimate sent = [query for queries in queries_by_skill.values() for query in queries if tokenize(query)] estimate = selection_estimate(field, sent, repeats=args.repeats if args.model else 0) diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 40c2d106e..07a76f507 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -42,6 +42,7 @@ from typing import TYPE_CHECKING from dokimi.external.listing import only_description_too_long +from dokimi.llm import is_retryable from dokimi.skills import Skill from dokimi.utils import InputError, parse_frontmatter @@ -265,11 +266,17 @@ def _pick_row(base: dict, reply: str, order: list[int], candidates: list[Candida "order": [candidates[i].name for i in order]} +_PERMANENT_STOP = "permanent error: " +# A permanent error that belongs to one query (a refusal), so the loop goes on. +_PER_QUERY_ERRORS = frozenset({"ContentPolicyViolationError"}) + + def _ask_once(client, prompt: str, base: dict, order: list[int], candidates: list[Candidate], gold: int) -> tuple[dict | None, str | None]: - """One call of the routing loop: its row, and the cap's message when the cap stops the run. + """One call of the routing loop: its row, and the message when the cap or a permanent error stops the run. A call the cap refused has no row; one with no price was paid for, so its reply is scored (a cut one as a decline) and the caller stops. + A permanent provider error (:func:`dokimi.llm.is_retryable` is False) has no row either and stops the run. """ try: reply = client.text(prompt, accept=partial(_is_pick_reply, n_cards=len(order)), @@ -282,6 +289,9 @@ def _ask_once(client, prompt: str, base: dict, order: list[int], candidates: lis return {**base, "picked": None, "abstained": True, "raw_reply": exc.reply[:_RAW_REPLY_CHARS]}, stop return _pick_row(base, exc.reply, order, candidates, gold), stop except Exception as exc: # noqa: BLE001 - one failed query must not lose the rest + if not is_retryable(exc) and type(exc).__name__ not in _PER_QUERY_ERRORS: + # No credential, wrong model ID or missing permission fails every later call alike (#677). + return None, _PERMANENT_STOP + (str(exc)[:200] or type(exc).__name__) return {**base, "picked": None, "error": str(exc)[:200] or type(exc).__name__}, None return _pick_row(base, reply, order, candidates, gold), None @@ -339,7 +349,9 @@ def model_routing( When the client's ``--max-usd`` cap refuses a call, or a call comes back unpriced, the loop stops and the result holds the calls made so far, with ``cap_stop`` (the cap's - message). A ``partial`` block (:func:`partial_record`) is added only when planned calls + message). A permanent provider error (``is_retryable`` False: authentication, permission, + unknown model, no credentials) stops it the same way, with the error and the calls made in + ``cap_stop``; an error of one query (a refusal, an unparseable reply) is a row. A ``partial`` block (:func:`partial_record`) is added only when planned calls were left unmade (``n_calls`` below queries x repeats). A query stopped between repeats keeps its rows but is left out of ``noise``. Without a cap that trips, there is no ``cap_stop`` key. @@ -358,6 +370,8 @@ def model_routing( groups.append(group) if stopped is not None: capped = stopped + if capped.startswith(_PERMANENT_STOP): + capped += f" (stopped at call {len(picks) + 1}; {len(picks)} earlier call(s) kept)" break answered = [p for p in picks if "correct" in p] @@ -434,43 +448,6 @@ def headings_as_queries(body: str, *, limit: int = 40) -> list[str]: return [h for h in headings if len(tokenize(h)) >= 2][:limit] -# Output tokens per selection call, measured on one run (360 calls drew 23.3k tokens). -SELECTION_REPLY_TOKENS = 65 - - -def selection_prompt_tokens(candidates: list[Candidate], queries: list[str]) -> list[int]: - """The prompt tokens (chars / 4) of each call one repeat of :func:`model_routing` makes. - - One entry per sent query, in query order. :func:`model_routing` sends every query, contentless - ones too, unless none has a content word: then the run refuses before any call, and the list - is empty. Each call carries the whole field's cards in the selection prompt. - """ - cards = "\n".join(f"{position}. {candidate.card}" - for position, candidate in enumerate(candidates, 1)) - sent = list(queries) if any(tokenize(query) for query in queries) else [] - return [len(_SELECT_PROMPT.format(query=query, cards=cards)) // 4 for query in sent] - - -def selection_estimate(candidates: list[Candidate], queries: list[str], *, - repeats: int = 1) -> dict: - """The calls and tokens :func:`model_routing` would make, counted without making them. - - Every repeat sends the same prompts (see :func:`selection_prompt_tokens`). Each reply is - billed at :data:`SELECTION_REPLY_TOKENS` output tokens, the measured figure, and - ``output_tokens_per_call`` says which. Zero repeats send nothing, so no query is sent. - """ - prompts = selection_prompt_tokens(candidates, queries) if repeats else [] - calls = len(prompts) * repeats - return { - "n_queries": len(queries), - "n_queries_sent": len(prompts), - "n_calls": calls, - "input_tokens": sum(prompts) * repeats, - "output_tokens_per_call": SELECTION_REPLY_TOKENS, - "output_tokens": calls * SELECTION_REPLY_TOKENS, - } - - def non_skill_reason(ref: str) -> str | None: """Why a ``skill:`` reference names something other than a skill, or ``None``. diff --git a/dokimi/dokimi/novelty/routing_estimate.py b/dokimi/dokimi/novelty/routing_estimate.py new file mode 100644 index 000000000..5dea58051 --- /dev/null +++ b/dokimi/dokimi/novelty/routing_estimate.py @@ -0,0 +1,50 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 +"""The offline estimate of a routing run's calls and tokens (``novelty routing --estimate-only``).""" +from __future__ import annotations + +from .routing import _SELECT_PROMPT, Candidate, tokenize + +# Characters per input token of a selection prompt: measured ~2.6 on a 35-skill catalog billed by +# Bedrock (7,949 tokens a call), where chars / 4 gave 5,126, 1.55x low (issue #676). +SELECTION_CHARS_PER_TOKEN = 2.6 +# Output tokens per selection call: the measured mean was 114 (Sonnet 5.5) to 193 (Haiku 5.5) in +# issue #676, 65 before. One flat figure in that range, as the repo holds no per-model table. +SELECTION_REPLY_TOKENS = 150 + + +def selection_prompt_tokens(candidates: list[Candidate], queries: list[str]) -> list[int]: + """The prompt tokens (chars / :data:`SELECTION_CHARS_PER_TOKEN`) of each call one repeat of + :func:`dokimi.novelty.routing.model_routing` makes. + + One entry per sent query, in query order. ``model_routing`` sends every query, contentless + ones too, unless none has a content word: then the run refuses before any call, and the list + is empty. Each call carries the whole field's cards in the selection prompt. + """ + cards = "\n".join(f"{position}. {candidate.card}" + for position, candidate in enumerate(candidates, 1)) + sent = list(queries) if any(tokenize(query) for query in queries) else [] + return [round(len(_SELECT_PROMPT.format(query=query, cards=cards)) / SELECTION_CHARS_PER_TOKEN) + for query in sent] + + +def selection_estimate(candidates: list[Candidate], queries: list[str], *, + repeats: int = 1) -> dict: + """The calls and tokens ``model_routing`` would make, counted without making them. + + Every repeat sends the same prompts (see :func:`selection_prompt_tokens`). Each reply is + billed at :data:`SELECTION_REPLY_TOKENS` output tokens; ``output_tokens_per_call`` and + ``chars_per_input_token`` say which allowances were used. Zero repeats send nothing, so no + query is sent. + """ + prompts = selection_prompt_tokens(candidates, queries) if repeats else [] + calls = len(prompts) * repeats + return { + "n_queries": len(queries), + "n_queries_sent": len(prompts), + "n_calls": calls, + "input_tokens": sum(prompts) * repeats, + "chars_per_input_token": SELECTION_CHARS_PER_TOKEN, + "output_tokens_per_call": SELECTION_REPLY_TOKENS, + "output_tokens": calls * SELECTION_REPLY_TOKENS, + } diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index fcbde4ba2..d6c5504b3 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -4,7 +4,6 @@ "dokimi/harnesses/benchflow.py": {"lines": 869}, "dokimi/harnesses/docker_solver.py": {"lines": 897, "rationale": "pre-existing when the gate was introduced"}, "dokimi/llm.py": {"lines": 1031, "rationale": "grew on main in #180 (replay a stored retry) after #93 seeded this baseline; split it in its own PR"}, - "dokimi/novelty/routing.py": {"lines": 822, "rationale": "grew with the #172 routing field, the keep-copies and dropped-query rules, the #405 over-limit warnings and the #403 one-repeat noise and measured reply tokens; file split tracked separately"}, "dokimi/report/model_compare.py": {"lines": 1161, "rationale": "grew on main in #73 (judge-validity --context stored) before the ratchet landed; split it in its own PR"}, "tests/test_arm_runner.py": {"lines": 1175, "rationale": "pre-existing when the gate was introduced"}, "tests/test_benchflow_harness.py": {"lines": 1716, "rationale": "pre-existing when the gate was introduced"}, diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 82a393033..219a732da 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -159,7 +159,7 @@ def test_one_failed_query_does_not_lose_the_others(): def transport(prompt: str, system: str | None): calls["n"] += 1 if calls["n"] == 1: - raise RuntimeError("provider refused") + raise RuntimeError("connection: provider refused") return json.dumps({"pick": 1}), None client = LLMClient(model="m", role="selector", transport=transport) @@ -434,7 +434,7 @@ def transport(prompt: str, system: str | None): def test_a_transport_error_is_still_excluded_from_the_rate(): def transport(prompt: str, system: str | None): - raise RuntimeError("provider down") + raise RuntimeError("connection: provider down") client = LLMClient(model="m", role="selector", transport=transport) result = model_routing([PERF, TBB], ["q1"], client=client) @@ -611,7 +611,7 @@ def transport(prompt: str, system: str | None): def test_a_query_that_failed_on_every_repeat_has_no_agreement_to_report(): def transport(prompt: str, system: str | None): - raise RuntimeError("provider down") + raise RuntimeError("connection: provider down") client = LLMClient(model="m", role="selector", transport=transport) noise = model_routing([PERF, TBB], ["q1"], client=client, repeats=2)["noise"] @@ -742,7 +742,7 @@ def _scripted(script): def transport(prompt: str, system: str | None): step = next(calls) if step == "!": - raise _Boom("provider down") + raise _Boom("connection: provider down") if step == "-": return json.dumps({"pick": 0}), None return _answer_as(step)(prompt), None diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index f58855968..bd320f8de 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -389,7 +389,7 @@ def test_the_model_arm_follows_each_call_not_one_fixed_card(): def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): def transport(prompt: str, system: str | None): - raise RuntimeError("boom") + raise RuntimeError("connection boom") client = LLMClient(model="m", role="selector", transport=transport) model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] @@ -398,8 +398,11 @@ def transport(prompt: str, system: str | None): def test_a_transport_that_raises_with_no_message_is_still_counted_as_failed(): + class Timeout(Exception): # litellm's transient class, which is_retryable reads by name + pass + def transport(prompt: str, system: str | None): - raise RuntimeError() + raise Timeout() client = LLMClient(model="m", role="selector", transport=transport) model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] @@ -410,7 +413,7 @@ def transport(prompt: str, system: str | None): def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): def transport(prompt: str, system: str | None): - raise RuntimeError("boom") + raise RuntimeError("connection boom") monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) @@ -772,7 +775,7 @@ def transport(prompt: str, system: str | None): def test_a_failed_call_adds_to_the_contentless_queries_of_the_same_skill(): def transport(prompt: str, system: str | None): - raise RuntimeError("boom") + raise RuntimeError("connection boom") field = [Candidate("alpha", "convert a json file to yaml"), Candidate("beta", "tune a slow database query")] @@ -786,7 +789,7 @@ def test_the_failed_call_count_is_over_the_calls_sent_not_every_query( tmp_path, monkeypatch, capsys): def transport(prompt: str, system: str | None): - raise RuntimeError("boom") + raise RuntimeError("connection boom") monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) @@ -915,7 +918,7 @@ def transport(prompt: str, system: str | None = None): try: return next(replies) except StopIteration: - raise RuntimeError("boom") from None + raise RuntimeError("connection boom") from None monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) diff --git a/dokimi/tests/test_novelty_routing_cli_gaps.py b/dokimi/tests/test_novelty_routing_cli_gaps.py index 7956534c2..011580406 100644 --- a/dokimi/tests/test_novelty_routing_cli_gaps.py +++ b/dokimi/tests/test_novelty_routing_cli_gaps.py @@ -119,7 +119,8 @@ def test_estimate_only_without_a_model_is_lexical_and_free(tmp_path, capsys): def test_a_zero_repeat_estimate_sends_no_queries(): - from dokimi.novelty.routing import Candidate, selection_estimate + from dokimi.novelty.routing import Candidate + from dokimi.novelty.routing_estimate import selection_estimate field = [Candidate("perf", "profile a slow binary"), Candidate("db", "tune a database")] estimate = selection_estimate(field, ["profile my binary", "index a table"], repeats=0) @@ -192,7 +193,8 @@ def test_estimate_only_refuses_the_run_the_real_run_refuses(tmp_path, capsys, qu def test_the_estimate_counts_the_prompts_the_run_sends(): # The estimate and the run each build the selection prompt. Every prompt the run sends must # have the length the estimate counts for it, so a drift between the two shows here. - from dokimi.novelty.routing import Candidate, model_routing, selection_prompt_tokens + from dokimi.novelty.routing import Candidate, model_routing + from dokimi.novelty.routing_estimate import selection_prompt_tokens field = [Candidate("perf", "profile a slow binary with perf"), Candidate("db", "tune a database index"), @@ -208,7 +210,7 @@ def text(self, prompt, *, system=None, accept=None, draw=0): return '{"pick": 1}' model_routing(field, queries, client=Recorder(), gold=0, repeats=1) - assert [len(prompt) // 4 for prompt in sent] == selection_prompt_tokens(field, queries) + assert [round(len(prompt) / 2.6) for prompt in sent] == selection_prompt_tokens(field, queries) def _two_skill_field(tmp_path): @@ -354,16 +356,16 @@ def test_all_vs_all_refuses_an_out_directory_before_it_calls_the_model(tmp_path, def test_the_estimate_prices_the_reply_the_model_sends_not_one_json_pick(): - from dokimi.novelty.routing import Candidate, selection_estimate + from dokimi.novelty.routing import Candidate + from dokimi.novelty.routing_estimate import selection_estimate field = [Candidate(name="a", description="profile a slow binary"), Candidate(name="b", description="tune a slow database")] - # 360 calls: the measured run drew 23.3k output tokens for them, where one pick per call - # (about 10 tokens) would have estimated 3.6k. + # Measured replies were 114 to 193 tokens a call; one pick per call (about 10) would not do. estimate = selection_estimate(field, ["profile my slow binary"] * 360, repeats=1) assert estimate["n_calls"] == 360 assert estimate["output_tokens"] == 360 * estimate["output_tokens_per_call"] - assert estimate["output_tokens"] == pytest.approx(23_300, rel=0.05) + assert 114 <= estimate["output_tokens_per_call"] <= 193 diff --git a/dokimi/tests/test_novelty_routing_cost.py b/dokimi/tests/test_novelty_routing_cost.py index 3813e77a4..d682d1d5e 100644 --- a/dokimi/tests/test_novelty_routing_cost.py +++ b/dokimi/tests/test_novelty_routing_cost.py @@ -101,7 +101,7 @@ def test_a_model_run_whose_every_call_fails_prints_no_price_blame(tmp_path, monk from dokimi import llm def failing(prompt, model, **_kwargs): - raise RuntimeError("provider refused") + raise RuntimeError("connection: provider refused") monkeypatch.setattr(llm, "llm_call_with_usage", failing) payload = _model_run(tmp_path) diff --git a/dokimi/tests/test_novelty_routing_estimate_and_stop.py b/dokimi/tests/test_novelty_routing_estimate_and_stop.py new file mode 100644 index 000000000..6ace4fd80 --- /dev/null +++ b/dokimi/tests/test_novelty_routing_estimate_and_stop.py @@ -0,0 +1,98 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 +"""The routing estimate's allowances (#676) and the stop at a permanent error (#677).""" +from __future__ import annotations + +import argparse +import json + +from dokimi.novelty.routing import Candidate, model_routing +from dokimi.novelty.routing_estimate import selection_estimate + +FIELD = [Candidate("perf", "profile a slow binary with perf"), + Candidate("db", "tune a database index")] +QUERIES = ["profile my binary", "index a table", "tune the database"] + + +class AuthenticationError(Exception): + """Named as litellm's, which is_retryable reads by class name.""" + + +class ContentPolicyViolationError(Exception): + pass + + +class Stub: + model = "m" + + def __init__(self, behaviour): + self.behaviour = behaviour + self.calls = 0 + + def text(self, prompt, *, system=None, accept=None, draw=0): + self.calls += 1 + return self.behaviour(self.calls) + + +def test_the_estimate_counts_characters_over_the_documented_ratio(): + sent: list[str] = [] + + class Recorder(Stub): + def text(self, prompt, **kwargs): + sent.append(prompt) + return '{"pick": 1}' + + model_routing(FIELD, QUERIES[:1], client=Recorder(None)) + estimate = selection_estimate(FIELD, QUERIES[:1], repeats=2) + assert estimate["chars_per_input_token"] == 2.6 + assert estimate["input_tokens"] == 2 * round(len(sent[0]) / 2.6) + assert estimate["output_tokens_per_call"] == 150 + assert estimate["output_tokens"] == 2 * 150 + + +def test_the_estimate_line_names_both_allowances(capsys): + from dokimi.commands.novelty_routing_report import _print_estimate + + args = argparse.Namespace(model="m", provider=None, artifact="a", repeats=1) + _print_estimate(args, FIELD, QUERIES, selection_estimate(FIELD, QUERIES)) + captured = capsys.readouterr() + assert json.loads(captured.out)["chars_per_input_token"] == 2.6 + assert "2.6 characters per token" in captured.err + assert "150 tokens a call" in captured.err + assert "1.55x below" in captured.err + + +def test_an_authentication_error_stops_at_the_first_call_and_keeps_the_rows_before(): + def behaviour(n): + if n <= 2: + return '{"pick": 1}' + raise AuthenticationError("Unable to locate credentials") + + stub = Stub(behaviour) + result = model_routing(FIELD, QUERIES, client=stub, repeats=2) + assert stub.calls == 3 + assert result["n_calls"] == 2 and result["n_failed"] == 0 + assert "Unable to locate credentials" in result["cap_stop"] + assert "stopped at call 3; 2 earlier call(s) kept" in result["cap_stop"] + assert result["partial"]["calls_done"] == 2 and result["partial"]["calls_planned"] == 6 + + +def test_a_first_call_permanent_error_makes_one_call(): + stub = Stub(lambda n: (_ for _ in ()).throw(AuthenticationError("no credentials"))) + result = model_routing(FIELD, QUERIES, client=stub, repeats=3) + assert stub.calls == 1 and result["per_query"] == [] + assert "permanent error: no credentials" in result["cap_stop"] + + +def test_an_error_of_one_query_is_a_row_and_the_loop_goes_on(): + def behaviour(n): + if n == 2: + raise ContentPolicyViolationError("refused") + if n == 3: + raise ConnectionError("connection reset by peer") # retryable, out of retries + return '{"pick": 1}' + + stub = Stub(behaviour) + result = model_routing(FIELD, QUERIES + ["tune the index"], client=stub) + assert stub.calls == 4 + assert result["n_failed"] == 2 and "cap_stop" not in result and "partial" not in result diff --git a/dokimi/tests/test_novelty_routing_gaps.py b/dokimi/tests/test_novelty_routing_gaps.py index dcf237271..0d9639d77 100644 --- a/dokimi/tests/test_novelty_routing_gaps.py +++ b/dokimi/tests/test_novelty_routing_gaps.py @@ -336,7 +336,7 @@ def test_all_vs_all_estimate_prices_each_call_and_sends_nothing(tmp_path, capsys estimate = json.loads(capsys.readouterr().out) assert set(estimate) == {"estimate_only", "artifact", "model", "provider", "n_candidates", "n_queries", "n_queries_sent", "n_calls", "input_tokens", - "output_tokens_per_call", "output_tokens", "expected_usd"} + "chars_per_input_token", "output_tokens_per_call", "output_tokens", "expected_usd"} assert estimate["estimate_only"] is True and estimate["artifact"] is None assert estimate["n_candidates"] == 3 # Four queries given; the contentless "the a of" is not sent by the all-vs-all model arm. From a4bef1cdd671a8e37a01099245718e3573eccff0 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 13 Nov 2026 15:23:25 +0000 Subject: [PATCH 2/5] fix(novelty): stop a routing run only at run-wide errors Settles the review points on the permanent-error stop (#677). Only an authentication, permission or not-found error, or a no-credentials message, stops the run now; any other error that is_retryable does not know (a context-length rejection, an IndexError, a non-retryable CLI error, a bare RuntimeError) is one failed row again, so the later queries are still asked. The tests that had been reworded to say "connection" are back as they were. The stop message no longer counts the calls of the current skill only (wrong in --all-vs-all); the exit line already gives calls_done of calls_planned. partial_record takes a reason, and a permanent stop is stored with "permanent_error" in routing and all-vs-all instead of "spend_cap". The predicate moves to novelty/routing_stop.py to keep routing.py under its size cap. Signed-off-by: Anton Volkov --- ..._150000_routing-estimate-permanent-stop.md | 2 +- dokimi/docs/skill-novelty.md | 4 +- dokimi/dokimi/novelty/all_vs_all.py | 4 +- dokimi/dokimi/novelty/noise.py | 7 +- dokimi/dokimi/novelty/routing.py | 25 +++---- dokimi/dokimi/novelty/routing_stop.py | 20 ++++++ dokimi/tests/test_novelty_routing.py | 8 +-- .../tests/test_novelty_routing_all_vs_all.py | 15 ++-- dokimi/tests/test_novelty_routing_cost.py | 2 +- .../test_novelty_routing_estimate_and_stop.py | 70 ++++++++++++++++++- 10 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 dokimi/dokimi/novelty/routing_stop.py diff --git a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md index a042ba85a..2cbb46a0a 100644 --- a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md +++ b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md @@ -1,4 +1,4 @@ ### Fixed - `novelty routing --estimate-only` sizes the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and prints both allowances. (#676) -- `novelty routing` stops at the first permanent provider error (authentication, permission, unknown model, no credentials) instead of failing every remaining call; the result is partial, with the error and the call it stopped at. A failure of one query is still one failed row. (#677) +- `novelty routing` stops at the first authentication, permission, unknown-model or no-credentials error instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index c96c1623c..e5b5621f5 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -823,13 +823,13 @@ so count the queries first, or pass `--max-usd` to stop the run at a spend limit A stored response in `DOKIMI_RESPONSE_CACHE` is replayed without a call, so a rerun can make fewer calls than the printed count. -**A permanent provider error stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID or missing credentials (`dokimi.llm.is_retryable` is false) would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with the error text and the call it stopped at as the message (`reason` stays `"spend_cap"` in the record). A failure of one query (a refusal, an unparseable reply, a transient error that ran out of retries) is still one failed row, and the run goes on. +**A permanent provider error that every call would meet stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID (`not found`) or missing credentials would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with `"reason": "permanent_error"` and the error text as the message (`--max-usd` is not involved; the exit line gives `calls_done` of `calls_planned`). Any other failure is one failed row and the run goes on: a refusal, a request one query makes too long, an unparseable reply, an error the client does not recognise, a transient error that ran out of retries. **A routing run stopped by `--max-usd` keeps what it paid for.** `novelty routing --model`, single artifact or `--all-vs-all`, writes the result of the calls made before the cap (or a call with no known price) stopped it, prints the summary and `cost` lines, and still exits non-zero with the cap message and the path of the file. The file carries -`"partial": {"reason": "spend_cap", "calls_done": N, "calls_planned": M, "message": ...}` at the +`"partial": {"reason": "spend_cap", "calls_done": N, "calls_planned": M, "message": ...}` (`"permanent_error"` for the stop above) at the top level (and under the model arm), and no `partial` key when the run finished. The record's `status` is then `"partial"` instead of `"ok"` (a single run; an all-vs-all file carries `status` only when partial), so a script that checks `status == "ok"` does not take a prefix of the run for diff --git a/dokimi/dokimi/novelty/all_vs_all.py b/dokimi/dokimi/novelty/all_vs_all.py index d39c8a534..b657a34be 100644 --- a/dokimi/dokimi/novelty/all_vs_all.py +++ b/dokimi/dokimi/novelty/all_vs_all.py @@ -218,6 +218,7 @@ def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str first_error = None planned = done = 0 capped: str | None = None + stop_reason = "spend_cap" for index, name in enumerate(names): if name not in queries_by_skill: continue @@ -247,6 +248,7 @@ def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str # A failed call measured nothing: it is not a pick, so it is left out of the counts. model[name] = Counter(row.get("picked") or NO_PICK for row in rows if not row.get("error")) capped = routed.get("cap_stop") + stop_reason = routed.get("stop_reason", "spend_cap") # Failed calls, and calls the cap refused: neither measured anything. unscored_here = routed["n_failed"] + (len(queries) * repeats - len(rows) if capped else 0) if unscored_here: @@ -258,7 +260,7 @@ def _model_arm(candidates: list[Candidate], queries_by_skill: dict[str, list[str model=client.model, seed=seed, repeats=repeats, noise=noise, n_failed=n_failed, n_abstained=n_abstained, first_error=first_error) if capped is not None and done < planned: # a stop on the last planned call leaves none unmade - arm["partial"] = partial_record(capped, done, planned) + arm["partial"] = partial_record(capped, done, planned, stop_reason) return arm diff --git a/dokimi/dokimi/novelty/noise.py b/dokimi/dokimi/novelty/noise.py index 8d1345da5..0329d03de 100644 --- a/dokimi/dokimi/novelty/noise.py +++ b/dokimi/dokimi/novelty/noise.py @@ -9,9 +9,10 @@ from collections import Counter -def partial_record(message: str, calls_done: int, calls_planned: int) -> dict: - """The marker of a result cut short by ``--max-usd``: what ran, of what was planned.""" - return {"reason": "spend_cap", "calls_done": calls_done, "calls_planned": calls_planned, +def partial_record(message: str, calls_done: int, calls_planned: int, + reason: str = "spend_cap") -> dict: + """The marker of a result cut short by ``--max-usd`` or a permanent error: what ran, of what was planned.""" + return {"reason": reason, "calls_done": calls_done, "calls_planned": calls_planned, "message": message} diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 07a76f507..0548814bb 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -42,13 +42,13 @@ from typing import TYPE_CHECKING from dokimi.external.listing import only_description_too_long -from dokimi.llm import is_retryable from dokimi.skills import Skill from dokimi.utils import InputError, parse_frontmatter from .catalog_keep import check_keep_copies_under, keep_past_broken_copies, resolve_kept_copies from .client import LLMClient, OutputTruncated, SpendCapReached, parse_json_object from .noise import partial_record, repeat_noise +from .routing_stop import PERMANENT_STOP, is_run_wide if TYPE_CHECKING: from dokimi.external.validate import SkillReview @@ -266,17 +266,13 @@ def _pick_row(base: dict, reply: str, order: list[int], candidates: list[Candida "order": [candidates[i].name for i in order]} -_PERMANENT_STOP = "permanent error: " -# A permanent error that belongs to one query (a refusal), so the loop goes on. -_PER_QUERY_ERRORS = frozenset({"ContentPolicyViolationError"}) - def _ask_once(client, prompt: str, base: dict, order: list[int], candidates: list[Candidate], gold: int) -> tuple[dict | None, str | None]: """One call of the routing loop: its row, and the message when the cap or a permanent error stops the run. A call the cap refused has no row; one with no price was paid for, so its reply is scored (a cut one as a decline) and the caller stops. - A permanent provider error (:func:`dokimi.llm.is_retryable` is False) has no row either and stops the run. + A run-wide provider error (:func:`dokimi.novelty.routing_stop.is_run_wide`) has no row either and stops the run. """ try: reply = client.text(prompt, accept=partial(_is_pick_reply, n_cards=len(order)), @@ -289,9 +285,9 @@ def _ask_once(client, prompt: str, base: dict, order: list[int], candidates: lis return {**base, "picked": None, "abstained": True, "raw_reply": exc.reply[:_RAW_REPLY_CHARS]}, stop return _pick_row(base, exc.reply, order, candidates, gold), stop except Exception as exc: # noqa: BLE001 - one failed query must not lose the rest - if not is_retryable(exc) and type(exc).__name__ not in _PER_QUERY_ERRORS: + if is_run_wide(exc): # No credential, wrong model ID or missing permission fails every later call alike (#677). - return None, _PERMANENT_STOP + (str(exc)[:200] or type(exc).__name__) + return None, PERMANENT_STOP +(str(exc)[:200] or type(exc).__name__) return {**base, "picked": None, "error": str(exc)[:200] or type(exc).__name__}, None return _pick_row(base, reply, order, candidates, gold), None @@ -349,9 +345,10 @@ def model_routing( When the client's ``--max-usd`` cap refuses a call, or a call comes back unpriced, the loop stops and the result holds the calls made so far, with ``cap_stop`` (the cap's - message). A permanent provider error (``is_retryable`` False: authentication, permission, - unknown model, no credentials) stops it the same way, with the error and the calls made in - ``cap_stop``; an error of one query (a refusal, an unparseable reply) is a row. A ``partial`` block (:func:`partial_record`) is added only when planned calls + message). A run-wide provider error (authentication, permission, unknown model, no + credentials) stops it the same way, with the error in ``cap_stop`` and ``stop_reason`` + ``"permanent_error"`` (else ``"spend_cap"``); any other error is a row of its query. A + ``partial`` block (:func:`partial_record`) is added only when planned calls were left unmade (``n_calls`` below queries x repeats). A query stopped between repeats keeps its rows but is left out of ``noise``. Without a cap that trips, there is no ``cap_stop`` key. @@ -370,8 +367,6 @@ def model_routing( groups.append(group) if stopped is not None: capped = stopped - if capped.startswith(_PERMANENT_STOP): - capped += f" (stopped at call {len(picks) + 1}; {len(picks)} earlier call(s) kept)" break answered = [p for p in picks if "correct" in p] @@ -407,8 +402,10 @@ def model_routing( # The stop is kept apart from ``partial``: a stop on the run's last planned call leaves # nothing unmade, but a caller (all-vs-all) still must not ask a later skill unmetered. result["cap_stop"] = capped + reason = "permanent_error" if capped.startswith(PERMANENT_STOP) else "spend_cap" + result["stop_reason"] = reason if len(picks) < len(queries) * repeats: - result["partial"] = partial_record(capped, len(picks), len(queries) * repeats) + result["partial"] = partial_record(capped, len(picks), len(queries) * repeats, reason) return result diff --git a/dokimi/dokimi/novelty/routing_stop.py b/dokimi/dokimi/novelty/routing_stop.py new file mode 100644 index 000000000..4e8ce87da --- /dev/null +++ b/dokimi/dokimi/novelty/routing_stop.py @@ -0,0 +1,20 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 +"""Which provider errors stop a routing run (#677); split from ``routing.py``, which stays under its size cap.""" +from __future__ import annotations + +from dokimi.llm import _NON_RETRYABLE_MESSAGES, is_retryable + +PERMANENT_STOP = "permanent error: " +# Errors that fail every later call alike: no key, no permission, an unknown model. Any other +# error (a context-length rejection, a refusal, an odd reply) may belong to one query (#709). +_RUN_WIDE_ERRORS = frozenset({"authenticationerror", "permissiondeniederror", "notfounderror"}) +_NO_CREDENTIALS = (*_NON_RETRYABLE_MESSAGES, "unable to locate credentials") + + +def is_run_wide(exc: Exception) -> bool: + """Whether *exc* is a permanent provider error that every later call would meet too.""" + if is_retryable(exc): + return False + return type(exc).__name__.lower() in _RUN_WIDE_ERRORS or any( + marker in str(exc).lower() for marker in _NO_CREDENTIALS) diff --git a/dokimi/tests/test_novelty_routing.py b/dokimi/tests/test_novelty_routing.py index 219a732da..82a393033 100644 --- a/dokimi/tests/test_novelty_routing.py +++ b/dokimi/tests/test_novelty_routing.py @@ -159,7 +159,7 @@ def test_one_failed_query_does_not_lose_the_others(): def transport(prompt: str, system: str | None): calls["n"] += 1 if calls["n"] == 1: - raise RuntimeError("connection: provider refused") + raise RuntimeError("provider refused") return json.dumps({"pick": 1}), None client = LLMClient(model="m", role="selector", transport=transport) @@ -434,7 +434,7 @@ def transport(prompt: str, system: str | None): def test_a_transport_error_is_still_excluded_from_the_rate(): def transport(prompt: str, system: str | None): - raise RuntimeError("connection: provider down") + raise RuntimeError("provider down") client = LLMClient(model="m", role="selector", transport=transport) result = model_routing([PERF, TBB], ["q1"], client=client) @@ -611,7 +611,7 @@ def transport(prompt: str, system: str | None): def test_a_query_that_failed_on_every_repeat_has_no_agreement_to_report(): def transport(prompt: str, system: str | None): - raise RuntimeError("connection: provider down") + raise RuntimeError("provider down") client = LLMClient(model="m", role="selector", transport=transport) noise = model_routing([PERF, TBB], ["q1"], client=client, repeats=2)["noise"] @@ -742,7 +742,7 @@ def _scripted(script): def transport(prompt: str, system: str | None): step = next(calls) if step == "!": - raise _Boom("connection: provider down") + raise _Boom("provider down") if step == "-": return json.dumps({"pick": 0}), None return _answer_as(step)(prompt), None diff --git a/dokimi/tests/test_novelty_routing_all_vs_all.py b/dokimi/tests/test_novelty_routing_all_vs_all.py index bd320f8de..f58855968 100644 --- a/dokimi/tests/test_novelty_routing_all_vs_all.py +++ b/dokimi/tests/test_novelty_routing_all_vs_all.py @@ -389,7 +389,7 @@ def test_the_model_arm_follows_each_call_not_one_fixed_card(): def test_a_transport_that_raises_is_counted_not_reported_as_a_clean_zero(): def transport(prompt: str, system: str | None): - raise RuntimeError("connection boom") + raise RuntimeError("boom") client = LLMClient(model="m", role="selector", transport=transport) model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] @@ -398,11 +398,8 @@ def transport(prompt: str, system: str | None): def test_a_transport_that_raises_with_no_message_is_still_counted_as_failed(): - class Timeout(Exception): # litellm's transient class, which is_retryable reads by name - pass - def transport(prompt: str, system: str | None): - raise Timeout() + raise RuntimeError() client = LLMClient(model="m", role="selector", transport=transport) model = all_vs_all(ALL_FIELD, ALL_QUERIES, client=client)["model"] @@ -413,7 +410,7 @@ def transport(prompt: str, system: str | None): def test_all_vs_all_exits_nonzero_when_every_model_call_fails(tmp_path, capsys, monkeypatch): def transport(prompt: str, system: str | None): - raise RuntimeError("connection boom") + raise RuntimeError("boom") monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) @@ -775,7 +772,7 @@ def transport(prompt: str, system: str | None): def test_a_failed_call_adds_to_the_contentless_queries_of_the_same_skill(): def transport(prompt: str, system: str | None): - raise RuntimeError("connection boom") + raise RuntimeError("boom") field = [Candidate("alpha", "convert a json file to yaml"), Candidate("beta", "tune a slow database query")] @@ -789,7 +786,7 @@ def test_the_failed_call_count_is_over_the_calls_sent_not_every_query( tmp_path, monkeypatch, capsys): def transport(prompt: str, system: str | None): - raise RuntimeError("connection boom") + raise RuntimeError("boom") monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) @@ -918,7 +915,7 @@ def transport(prompt: str, system: str | None = None): try: return next(replies) except StopIteration: - raise RuntimeError("connection boom") from None + raise RuntimeError("boom") from None monkeypatch.setattr(novelty_cost, "client", lambda model, provider, role, args: LLMClient( model=model, provider=provider, role=role, transport=transport)) diff --git a/dokimi/tests/test_novelty_routing_cost.py b/dokimi/tests/test_novelty_routing_cost.py index d682d1d5e..3813e77a4 100644 --- a/dokimi/tests/test_novelty_routing_cost.py +++ b/dokimi/tests/test_novelty_routing_cost.py @@ -101,7 +101,7 @@ def test_a_model_run_whose_every_call_fails_prints_no_price_blame(tmp_path, monk from dokimi import llm def failing(prompt, model, **_kwargs): - raise RuntimeError("connection: provider refused") + raise RuntimeError("provider refused") monkeypatch.setattr(llm, "llm_call_with_usage", failing) payload = _model_run(tmp_path) diff --git a/dokimi/tests/test_novelty_routing_estimate_and_stop.py b/dokimi/tests/test_novelty_routing_estimate_and_stop.py index 6ace4fd80..3b4eb6281 100644 --- a/dokimi/tests/test_novelty_routing_estimate_and_stop.py +++ b/dokimi/tests/test_novelty_routing_estimate_and_stop.py @@ -6,6 +6,9 @@ import argparse import json +import pytest + +from dokimi.novelty.all_vs_all import all_vs_all from dokimi.novelty.routing import Candidate, model_routing from dokimi.novelty.routing_estimate import selection_estimate @@ -18,6 +21,14 @@ class AuthenticationError(Exception): """Named as litellm's, which is_retryable reads by class name.""" +class PermissionDeniedError(Exception): + pass + + +class NotFoundError(Exception): + pass + + class ContentPolicyViolationError(Exception): pass @@ -73,8 +84,39 @@ def behaviour(n): assert stub.calls == 3 assert result["n_calls"] == 2 and result["n_failed"] == 0 assert "Unable to locate credentials" in result["cap_stop"] - assert "stopped at call 3; 2 earlier call(s) kept" in result["cap_stop"] assert result["partial"]["calls_done"] == 2 and result["partial"]["calls_planned"] == 6 + assert result["partial"]["reason"] == "permanent_error" + assert result["partial"]["message"] == result["cap_stop"] + + +@pytest.mark.parametrize("exc", [PermissionDeniedError("denied"), NotFoundError("no such model"), + RuntimeError("Missing credentials for the provider")]) +def test_each_run_wide_error_stops_the_run(exc): + def behaviour(n): + if n == 2: + raise exc + return '{"pick": 1}' + + stub = Stub(behaviour) + result = model_routing(FIELD, QUERIES, client=stub) + assert stub.calls == 2 and result["n_calls"] == 1 + assert result["partial"]["reason"] == "permanent_error" + + +def test_all_vs_all_stops_at_a_run_wide_error_and_counts_the_calls_of_the_run(): + def behaviour(n): + if n == 4: + raise AuthenticationError("bad key") + return '{"pick": 1}' + + stub = Stub(behaviour) + queries = {"perf": ["profile my binary", "profile a slow binary"], + "db": ["tune the database", "index a table"]} + arm = all_vs_all(FIELD, queries, client=stub)["model"] + assert stub.calls == 4 + assert arm["partial"]["calls_done"] == 3 and arm["partial"]["calls_planned"] == 4 + assert arm["partial"]["reason"] == "permanent_error" + assert arm["partial"]["message"] == "permanent error: bad key" def test_a_first_call_permanent_error_makes_one_call(): @@ -96,3 +138,29 @@ def behaviour(n): result = model_routing(FIELD, QUERIES + ["tune the index"], client=stub) assert stub.calls == 4 assert result["n_failed"] == 2 and "cap_stop" not in result and "partial" not in result + + +class BadRequestError(Exception): + """A context-length rejection of one oversized query.""" + + +class CliCallError(Exception): + retryable = False + + +@pytest.mark.parametrize("exc", [ + BadRequestError("maximum context length is 8192 tokens"), + IndexError("reply has no choices"), + CliCallError("claude exited 1"), + RuntimeError("boom"), +]) +def test_an_error_that_may_belong_to_one_query_is_a_row_and_a_later_query_is_asked(exc): + def behaviour(n): + if n == 1: + raise exc + return '{"pick": 1}' + + stub = Stub(behaviour) + result = model_routing(FIELD, QUERIES, client=stub) + assert stub.calls == 3 and result["n_calls"] == 3 and result["n_failed"] == 1 + assert "cap_stop" not in result and "partial" not in result From 86c79cbe9dd658a6f10593cf412e5725640fe0f9 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Sat, 14 Nov 2026 01:08:06 -0800 Subject: [PATCH 3/5] fix(novelty): a claude-cli login, usage-limit or unknown-model error stops routing CliCallError gains run_wide, set where llm_cli already knows every later call fails alike; is_run_wide reads it, as is_retryable reads retryable. Signed-off-by: Alexander Rybkin --- ..._150000_routing-estimate-permanent-stop.md | 2 +- dokimi/docs/skill-novelty.md | 2 +- dokimi/dokimi/llm_cli.py | 23 ++++---- dokimi/dokimi/novelty/routing_stop.py | 2 + .../test_novelty_routing_estimate_and_stop.py | 53 +++++++++++++++++++ 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md index 2cbb46a0a..40f928ba9 100644 --- a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md +++ b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md @@ -1,4 +1,4 @@ ### Fixed - `novelty routing --estimate-only` sizes the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and prints both allowances. (#676) -- `novelty routing` stops at the first authentication, permission, unknown-model or no-credentials error instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) +- `novelty routing` stops at the first authentication, permission, unknown-model or no-credentials error (with `claude-cli`, also not logged in, a usage limit or no `claude` binary) instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index c158b2d2b..2cb420029 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -827,7 +827,7 @@ so count the queries first, or pass `--max-usd` to stop the run at a spend limit A stored response in `DOKIMI_RESPONSE_CACHE` is replayed without a call, so a rerun can make fewer calls than the printed count. -**A permanent provider error that every call would meet stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID (`not found`) or missing credentials would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with `"reason": "permanent_error"` and the error text as the message (`--max-usd` is not involved; the exit line gives `calls_done` of `calls_planned`). Any other failure is one failed row and the run goes on: a refusal, a request one query makes too long, an unparseable reply, an error the client does not recognise, a transient error that ran out of retries. +**A permanent provider error that every call would meet stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID (`not found`) or missing credentials (with `claude-cli`: not logged in, a usage limit, no `claude` binary) would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with `"reason": "permanent_error"` and the error text as the message (`--max-usd` is not involved; the exit line gives `calls_done` of `calls_planned`). Any other failure is one failed row and the run goes on: a refusal, a request one query makes too long, an unparseable reply, an error the client does not recognise, a transient error that ran out of retries. **A routing run stopped by `--max-usd` keeps what it paid for.** `skill routing --model`, single artifact or `catalog routing`, writes the result of the calls made before the cap (or a call with no diff --git a/dokimi/dokimi/llm_cli.py b/dokimi/dokimi/llm_cli.py index c32bb9975..396e0271d 100644 --- a/dokimi/dokimi/llm_cli.py +++ b/dokimi/dokimi/llm_cli.py @@ -33,7 +33,7 @@ subscription is charged; it is kept as :attr:`CliResult.list_cost_usd`. - **Errors.** A failed call raises :class:`CliCallError` with ``retryable`` set: rate limits, overload, 5xx and timeouts are retried; an unknown model, a missing - login or a missing binary are not. + login or a missing binary are not. Those, and a usage limit, also set ``run_wide``. :func:`detect` reports whether a backend's binary is installed and signed in without calling a model (``cli.py backends``). @@ -94,11 +94,16 @@ def _text_retryable(lowered: str) -> bool: class CliCallError(RuntimeError): - """A CLI call that produced no reply; ``retryable`` is what :func:`dokimi.llm.is_retryable` reads.""" + """A CLI call that produced no reply; ``retryable`` is what :func:`dokimi.llm.is_retryable` reads. - def __init__(self, message: str, *, retryable: bool): + ``run_wide`` marks a failure every later call would meet too (no login, a usage limit, an unknown + model, no binary), which :func:`dokimi.novelty.routing_stop.is_run_wide` reads. + """ + + def __init__(self, message: str, *, retryable: bool, run_wide: bool = False): super().__init__(message) self.retryable = retryable + self.run_wide = run_wide @dataclass(frozen=True) @@ -176,14 +181,14 @@ def _claude_error(payload: Dict[str, Any], done: process.Completed) -> CliCallEr lowered = message.lower() if any(w in lowered for w in ("authentication", "not logged in", "/login")): return CliCallError(f"claude-cli: {message} (sign in: claude setup-token, or claude /login)", - retryable=False) + retryable=False, run_wide=True) if "usage limit" in lowered: # a subscription cap: it resets in hours, a retry cannot help - return CliCallError(f"claude-cli: {message}", retryable=False) + return CliCallError(f"claude-cli: {message}", retryable=False, run_wide=True) if status is None: # no structured status: the words, and a status the CLI printed as text retryable = _text_retryable(lowered) else: # a structured status decides: a 400 that quotes "rate_limit" is still a 400 retryable = status in _RETRYABLE_STATUS - return CliCallError(f"claude-cli: {message}", retryable=retryable) + return CliCallError(f"claude-cli: {message}", retryable=retryable, run_wide=status == 404) def _main_model(model_usage: Dict[str, Any], requested: str) -> Optional[str]: @@ -217,9 +222,9 @@ def _claude_parse(done: process.Completed, model: str, provider: str) -> CliResu # No JSON, so no status field: the transient words and the status printed as text are checked. # A subscription cap is not transient, as on the JSON path, whatever status it prints. lowered = text.lower() - retryable = "usage limit" not in lowered and _text_retryable(lowered) + capped = "usage limit" in lowered raise CliCallError(f"claude-cli: exit {done.returncode}, no JSON reply: {text[:300]}", - retryable=retryable) + retryable=not capped and _text_retryable(lowered), run_wide=capped) if payload.get("is_error") or done.returncode != 0: raise _claude_error(payload, done) u = payload.get("usage") or {} @@ -432,7 +437,7 @@ def call(provider: str, *, model: str, prompt: str, system: Optional[str] = None env=_call_env(cli, cwd), cwd=cwd, input=prompt, capture_limit=None) except FileNotFoundError as exc: raise CliCallError(f"{provider}: '{cli.binary}' is not installed or not on PATH ({exc})", - retryable=False) from exc + retryable=False, run_wide=True) from exc return cli.parse(done, model, provider) diff --git a/dokimi/dokimi/novelty/routing_stop.py b/dokimi/dokimi/novelty/routing_stop.py index 4e8ce87da..fb6ed241a 100644 --- a/dokimi/dokimi/novelty/routing_stop.py +++ b/dokimi/dokimi/novelty/routing_stop.py @@ -16,5 +16,7 @@ def is_run_wide(exc: Exception) -> bool: """Whether *exc* is a permanent provider error that every later call would meet too.""" if is_retryable(exc): return False + if isinstance(run_wide := getattr(exc, "run_wide", None), bool): # set by an error that knows (CliCallError) + return run_wide return type(exc).__name__.lower() in _RUN_WIDE_ERRORS or any( marker in str(exc).lower() for marker in _NO_CREDENTIALS) diff --git a/dokimi/tests/test_novelty_routing_estimate_and_stop.py b/dokimi/tests/test_novelty_routing_estimate_and_stop.py index 3b4eb6281..8996e1138 100644 --- a/dokimi/tests/test_novelty_routing_estimate_and_stop.py +++ b/dokimi/tests/test_novelty_routing_estimate_and_stop.py @@ -9,6 +9,7 @@ import pytest from dokimi.novelty.all_vs_all import all_vs_all +from dokimi.process import reply from dokimi.novelty.routing import Candidate, model_routing from dokimi.novelty.routing_estimate import selection_estimate @@ -164,3 +165,55 @@ def behaviour(n): result = model_routing(FIELD, QUERIES, client=stub) assert stub.calls == 3 and result["n_calls"] == 3 and result["n_failed"] == 1 assert "cap_stop" not in result and "partial" not in result + + +def _cli_reply(result: str, status) -> str: + return json.dumps({"type": "result", "is_error": True, "result": result, + "api_error_status": status, "usage": {}, "modelUsage": {}}) + + +def _cli_error(monkeypatch, tmp_path, answer): + """The error a real ``claude -p`` failure raises, through llm_cli's own classification.""" + from dokimi import llm_cli + from dokimi.process import FakeRunner + + def run(call): + if answer is FileNotFoundError: + raise FileNotFoundError("claude") + return answer + + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + with pytest.raises(llm_cli.CliCallError) as info: + llm_cli.call("claude-cli", model="sonnet", prompt="p", runner=FakeRunner(run)) + return info.value + + +def _first_call_raises(exc): + stub = Stub(lambda n: (_ for _ in ()).throw(exc)) + return stub, model_routing(FIELD, QUERIES, client=stub) + + +@pytest.mark.parametrize("answer", [ + reply(_cli_reply("Not logged in · Please run /login", None), returncode=1), + reply(_cli_reply("Authentication error · This may be a temporary network issue, " + "please try again", None), returncode=1), + reply(_cli_reply("Claude AI usage limit reached|1761660000", 429), returncode=1), + reply("", stderr="Claude AI usage limit reached. Error 429", returncode=1), + reply(_cli_reply("There's an issue with the selected model (claude-nope-1). It may not " + "exist or you may not have access to it.", 404), returncode=1), + FileNotFoundError, +], ids=["not-logged-in", "auth", "usage-limit", "usage-limit-no-json", "unknown-model", "no-binary"]) +def test_a_claude_cli_failure_every_call_meets_stops_the_run(monkeypatch, tmp_path, answer): + stub, result = _first_call_raises(_cli_error(monkeypatch, tmp_path, answer)) + assert stub.calls == 1 and result["per_query"] == [] + assert result["partial"]["reason"] == "permanent_error" + + +@pytest.mark.parametrize("answer", [ + reply(_cli_reply("API Error: 400 Bad Request: invalid request", 400), returncode=1), + reply("segfault", stderr="boom", returncode=139), + reply("", timed_out=True), +], ids=["bad-request", "no-json", "timeout"]) +def test_a_claude_cli_failure_of_one_call_is_a_row(monkeypatch, tmp_path, answer): + stub, result = _first_call_raises(_cli_error(monkeypatch, tmp_path, answer)) + assert stub.calls == 3 and result["n_failed"] == 3 and "partial" not in result From c72622adede5b39867c47a00b585989edaafb01d Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Sat, 14 Nov 2026 01:10:31 -0800 Subject: [PATCH 4/5] fix(novelty): an unsupported parameter stops a routing run Every call sends the same parameters, so UnsupportedParamsError fails each of them alike. Signed-off-by: Alexander Rybkin --- .../20261113_150000_routing-estimate-permanent-stop.md | 2 +- dokimi/docs/skill-novelty.md | 2 +- dokimi/dokimi/novelty/routing.py | 4 ++-- dokimi/dokimi/novelty/routing_stop.py | 7 ++++--- dokimi/tests/test_novelty_routing_estimate_and_stop.py | 7 ++++++- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md index 40f928ba9..8d50aefaa 100644 --- a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md +++ b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md @@ -1,4 +1,4 @@ ### Fixed - `novelty routing --estimate-only` sizes the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and prints both allowances. (#676) -- `novelty routing` stops at the first authentication, permission, unknown-model or no-credentials error (with `claude-cli`, also not logged in, a usage limit or no `claude` binary) instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) +- `novelty routing` stops at the first authentication, permission, unknown-model, unsupported-parameter or no-credentials error (with `claude-cli`, also not logged in, a usage limit or no `claude` binary) instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 2cb420029..2b3f8d39a 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -827,7 +827,7 @@ so count the queries first, or pass `--max-usd` to stop the run at a spend limit A stored response in `DOKIMI_RESPONSE_CACHE` is replayed without a call, so a rerun can make fewer calls than the printed count. -**A permanent provider error that every call would meet stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID (`not found`) or missing credentials (with `claude-cli`: not logged in, a usage limit, no `claude` binary) would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with `"reason": "permanent_error"` and the error text as the message (`--max-usd` is not involved; the exit line gives `calls_done` of `calls_planned`). Any other failure is one failed row and the run goes on: a refusal, a request one query makes too long, an unparseable reply, an error the client does not recognise, a transient error that ran out of retries. +**A permanent provider error that every call would meet stops a routing run the way the cap does.** An authentication or permission error, an unknown model ID (`not found`), a parameter the provider does not take (every call sends the same ones) or missing credentials (with `claude-cli`: not logged in, a usage limit, no `claude` binary) would fail every later call alike, so the run stops at that call, keeps the rows made before it and writes the partial result below, with `"reason": "permanent_error"` and the error text as the message (`--max-usd` is not involved; the exit line gives `calls_done` of `calls_planned`). Any other failure is one failed row and the run goes on: a refusal, a request one query makes too long, an unparseable reply, an error the client does not recognise, a transient error that ran out of retries. **A routing run stopped by `--max-usd` keeps what it paid for.** `skill routing --model`, single artifact or `catalog routing`, writes the result of the calls made before the cap (or a call with no diff --git a/dokimi/dokimi/novelty/routing.py b/dokimi/dokimi/novelty/routing.py index 1663fd2f9..e417f775f 100644 --- a/dokimi/dokimi/novelty/routing.py +++ b/dokimi/dokimi/novelty/routing.py @@ -345,8 +345,8 @@ def model_routing( When the client's ``--max-usd`` cap refuses a call, or a call comes back unpriced, the loop stops and the result holds the calls made so far, with ``cap_stop`` (the cap's - message). A run-wide provider error (authentication, permission, unknown model, no - credentials) stops it the same way, with the error in ``cap_stop`` and ``stop_reason`` + message). A run-wide provider error (:func:`~dokimi.novelty.routing_stop.is_run_wide`) + stops it the same way, with the error in ``cap_stop`` and ``stop_reason`` ``"permanent_error"`` (else ``"spend_cap"``); any other error is a row of its query. A ``partial`` block (:func:`partial_record`) is added only when planned calls were left unmade (``n_calls`` below queries x repeats). A query stopped between repeats diff --git a/dokimi/dokimi/novelty/routing_stop.py b/dokimi/dokimi/novelty/routing_stop.py index fb6ed241a..53f4e378d 100644 --- a/dokimi/dokimi/novelty/routing_stop.py +++ b/dokimi/dokimi/novelty/routing_stop.py @@ -6,9 +6,10 @@ from dokimi.llm import _NON_RETRYABLE_MESSAGES, is_retryable PERMANENT_STOP = "permanent error: " -# Errors that fail every later call alike: no key, no permission, an unknown model. Any other -# error (a context-length rejection, a refusal, an odd reply) may belong to one query (#709). -_RUN_WIDE_ERRORS = frozenset({"authenticationerror", "permissiondeniederror", "notfounderror"}) +# Errors that fail every later call alike: no key, no permission, an unknown model, a parameter every +# call sends. Any other error (a context-length rejection, a refusal, an odd reply) may be one query's. +_RUN_WIDE_ERRORS = frozenset({"authenticationerror", "permissiondeniederror", "notfounderror", + "unsupportedparamserror"}) _NO_CREDENTIALS = (*_NON_RETRYABLE_MESSAGES, "unable to locate credentials") diff --git a/dokimi/tests/test_novelty_routing_estimate_and_stop.py b/dokimi/tests/test_novelty_routing_estimate_and_stop.py index 8996e1138..b4fada490 100644 --- a/dokimi/tests/test_novelty_routing_estimate_and_stop.py +++ b/dokimi/tests/test_novelty_routing_estimate_and_stop.py @@ -30,6 +30,10 @@ class NotFoundError(Exception): pass +class UnsupportedParamsError(Exception): + pass + + class ContentPolicyViolationError(Exception): pass @@ -91,7 +95,8 @@ def behaviour(n): @pytest.mark.parametrize("exc", [PermissionDeniedError("denied"), NotFoundError("no such model"), - RuntimeError("Missing credentials for the provider")]) + RuntimeError("Missing credentials for the provider"), + UnsupportedParamsError("bedrock does not support parameters: ['seed']")]) def test_each_run_wide_error_stops_the_run(exc): def behaviour(n): if n == 2: From b81e6e89defb6f28de94885730fbfd0c5dda12af Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Sat, 14 Nov 2026 01:12:12 -0800 Subject: [PATCH 5/5] docs(changelog): routing stop exits 1, under the new command names A skill routing run whose every call failed exited 0 with status ok before this PR. Signed-off-by: Alexander Rybkin --- .../20261113_150000_routing-estimate-permanent-stop.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md index 8d50aefaa..6c232f599 100644 --- a/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md +++ b/dokimi/changelog.d/20261113_150000_routing-estimate-permanent-stop.md @@ -1,4 +1,4 @@ ### Fixed -- `novelty routing --estimate-only` sizes the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and prints both allowances. (#676) -- `novelty routing` stops at the first authentication, permission, unknown-model, unsupported-parameter or no-credentials error (with `claude-cli`, also not logged in, a usage limit or no `claude` binary) instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error. Any other failure, such as a request too long for the model, is still one failed row. (#677) +- `skill routing --estimate-only` and `catalog routing --estimate-only` size the selection prompt at 2.6 characters per token and a reply at 150 tokens (it was chars / 4 and 65, about 1.55x below measured bills), and print both allowances. (#676) +- `skill routing` and `catalog routing` stop at the first authentication, permission, unknown-model, unsupported-parameter or no-credentials error (with `claude-cli`, also not logged in, a usage limit or no `claude` binary) instead of failing every remaining call; the result is partial, with `reason` `permanent_error` and the error, and the command exits 1 (a `skill routing` run whose every call failed exited 0 with status `ok`). Any other failure, such as a request too long for the model, is still one failed row. (#677)