[antonwolfy]

Summary

novelty routing has two problems, both in dokimi/novelty/routing.py.

	on main	observed
#676: --estimate-only on a 35-skill catalog, 90 queries, 3 repeats (270 calls)	input at chars / 4 (5,126 tokens a call) and 65 output tokens a call	the bill was 7,949 input and 114 to 193 output tokens a call, about 1.55x the estimate
#677: --model claude-haiku-5-5 --provider amazon-bedrock with no Bedrock credentials	every call is recorded as a failed row	270 of 270 calls made and failed with Unable to locate credentials, then exit 1

For #676 the taken fix option is the third plus a cheap part of the first: the input stays a local count (no new network call, so --estimate-only stays offline) at a calibrated 2.6 characters per token, and the reply allowance is one flat 150 tokens, inside the measured 114 to 193, because the repo has no per-model table. A provider token counter would need a network call or a per-provider tokenizer, so it is not used. For #677 the permanent error stops the loop through the path the spend cap uses.

Closes #676
Closes #677

Part of #631: novelty analyze's estimate has the same class of error and is not touched here.

The same per-call catch in the atoms, probe and eval-arm code is tracked in #708.

PR #595 renames the routing CLI flags and edits novelty/routing.py, so it may conflict textually with this one; the CLI spelling is untouched here.

Changes

• novelty/routing_estimate.py (new): SELECTION_CHARS_PER_TOKEN = 2.6 and SELECTION_REPLY_TOKENS = 150, with their source (issue novelty routing --estimate-only prices about 1.55x below the bill: chars/4 input and a fixed 65-token reply #676), and selection_prompt_tokens / selection_estimate moved there from routing.py, which was at its growth baseline. The estimate now also carries chars_per_input_token.
• commands/novelty_routing_report.py: the stderr line of --estimate-only names both allowances and says the old figures were about 1.55x below measured bills.
• docs/skill-novelty.md: the estimate paragraph says it is an estimate, not a bound, and gives both allowances; a new paragraph describes the permanent-error stop.
• novelty/routing_stop.py (new): is_run_wide is true for an authentication, permission or not-found error, or a no-credentials message, that dokimi.llm.is_retryable calls permanent.
• novelty/routing.py _ask_once: when is_run_wide holds for the exception, the call has no row and the run stops. model_routing puts the error text in cap_stop, sets stop_reason to "permanent_error" and writes the partial block, as for the cap. Every other error is one failed row and the run goes on: a refusal, a request too long for the model (BadRequestError), an IndexError, a non-retryable CLI error, a bare RuntimeError, an unparseable reply, a retryable error that ran out of retries. The exit code is unchanged (1).
• novelty/noise.py partial_record takes a reason (default "spend_cap"); routing and all-vs-all store "permanent_error" for this stop. The message is the error text alone: the exit line gives calls_done of calls_planned, which in all-vs-all counts the whole run.
• Changelog fragment (Fixed).

Risk

--estimate-only figures rise: input by about 1.5x, output by about 2.3x. An authentication, permission or not-found error, or a no-credentials error, now stops a routing run at its first occurrence instead of failing every call; a provider that words a run-wide failure otherwise (a plain RuntimeError, say) still gets one failed row per call, as on main. The partial record's reason is "permanent_error" for this stop, so a script that reads reason does not take it for a --max-usd stop.

Testing

On a Linux review host, at a4bef1c (on main d61e7db):

• make ci-local: 31 passed, 1 failed, 13 skipped (docker smoke, the other-Python and Intel matrix legs, conda-forge, fuzz, semgrep, deploy-only jobs). The one failure is test / Run tests: 19118 passed, 19 skipped, and 2 errors that are Timeout (>60.0s) from pytest-timeout in tests/test_audit_test_quality.py (test_a_fixed_finding_leaves_the_baseline_too, test_no_new_test_that_cannot_fail), files this PR does not touch; that file run alone passes (172 passed in 5s), and a first run had the same timeout in the first of them with 19119 passed. The schema_compat step passes on the merged tree.
• New tests in tests/test_novelty_routing_estimate_and_stop.py: the estimate equals the prompt's characters over 2.6 (pinned by value), the printed line names both allowances, an authentication error on the third call makes exactly 3 calls and keeps 2 rows, one on the first call makes one call, and a refusal and a transient error are rows while the loop goes on. Against main's code 3 of the 5 fail (the other two cover the new estimate module, copied in); with the permanent-error check disabled 2 fail, with the ratio set to 4 three fail.
• Run-wide stop (tests/test_novelty_routing_estimate_and_stop.py): a permission, not-found or "Missing credentials" error on call 2 stops the run after 2 calls; BadRequestError (context length), IndexError, a retryable = False CLI error and a bare RuntimeError on call 1 are one failed row and all 3 queries are asked; in all_vs_all (2 skills x 2 queries, auth error on call 4) the partial is 3 of 4 calls, reason permanent_error, message permanent error: bad key. With is_run_wide always true 9 tests of the two routing files fail; with partial_record ignoring the reason 5 fail; with all_vs_all not passing the reason the all-vs-all test fails.
• Merged together with fix/routing-lexical-rows, docs/heading-query-artifact on main c8e707c: pytest -n auto 19082 passed, 19 skipped, mypy clean, ruff check clean.
• Not run: the other Python versions, a real model or provider.

Revert

A plain revert restores the chars / 4 and 65-token estimate and the record-every-failure loop. Nothing is stored.

Checklist

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

[exai-bot]

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

CI at b81e6e8: 25 passing, 3 skipped.

Review details

• Commit reviewed: b81e6e8 against merge base c1d7f5a
• Coverage: 13 files in 3 groups
• Engine: OpenCodeReview v1.13.5-exai.4.8f73eada, us.openai.gpt-6.1-sol
• Usage: 464k tokens (329k from cache), 1m3s
• Tool calls: 47 (29 file_read, 9 code_search, 3 file_read_diff, 3 pr_threads, 2 local_check_results, 1 ci_status)
• Novelty routing estimates and permanent-stop behavior: 8 files
• Novelty routing CLI reporting and documentation: 4 files
• Growth baseline maintenance: 1 file

[vlad-perevezentsev]

Review at b979089. The estimate change (#676) is right and the docs, the printed line and the tests follow it. The stop at a permanent error (#677) works for an auth error, but it also stops the run on errors that belong to one query; details are in the line comments.

Agreed with @exai-bot on the query-local rejection that now aborts the run; I added more cases in its thread.

What I ran, at b979089:

• make ci-local --offline: 20 passed, 1 failed, 19 skipped; tests and coverage of changed lines pass. The failure is schema_compat: it compares against the current main, where #633 widened novelty.v1.json after this branch's base. This PR does not touch schemas; a rebase clears it.
• A stub client failing call 2 of 3: IndexError, BadRequestError, CliCallError(retryable=False) and RuntimeError each stop the run; on the base each is one failed row.
• all_vs_all with 2 skills x 2 queries and an auth error on call 4: the message and partial.calls_done disagree.

Checked, no issues

• Allowances: 2.6 chars per token matches the issue's numbers (5,126 x 4 / 7,949 = 2.58); 150 sits inside the measured 114 to 193; the single and all-vs-all estimates and the USD estimate use the same constants.
• Stop path: a permanent stop always leaves the failing call unmade, so partial is always written and the command exits non-zero, in both modes; later skills in all-vs-all are not called.
• Docs and changelog: the fragment is under Fixed and says what changed; skill-novelty.md matches the code. The new files have the SPDX header, and the new tests fail on the base.

[antonwolfy]

(commented) Review at b979089. The estimate change (#676) is right and the docs, the printed line and the t

@vlad-perevezentsev (#709 (review)): The review's points are answered in the threads 4563405051, 4563405365 and 4563405377. The schema_compat failure came from main having widened novelty.v1.json; it passes on the tree merged with the current main.

[codecov-commenter]

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.73%. Comparing base (c1d7f5a) to head (b81e6e8).
⚠️ Report is 4 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #709   +/-   ##
=======================================
  Coverage   95.73%   95.73%           
=======================================
  Files         408      410    +2     
  Lines       54644    54667   +23     
  Branches     9404     9407    +3     
=======================================
+ Hits        52311    52338   +27     
+ Misses       1442     1438    -4     
  Partials      891      891           

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/novelty_routing_report.py	98.15% <100.00%> (+<0.01%)	⬆️
dokimi/dokimi/llm_cli.py	97.28% <100.00%> (+0.01%)	⬆️
dokimi/dokimi/novelty/all_vs_all.py	99.31% <100.00%> (+<0.01%)	⬆️
dokimi/dokimi/novelty/noise.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/routing.py	96.55% <100.00%> (-0.04%)	⬇️
dokimi/dokimi/novelty/routing_estimate.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/routing_stop.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[napetrov]

Change class: not computed (no receipt profile for this repo, or receipt.py failed)

Verdict: comment. No blocking. Two questions on the run-wide stop; the estimate change checks out (150 tokens is inside the measured 114 to 193). Head a4bef1c, merge base d61e7db, lane L3.
Ran: the run-wide and estimate probes on head (the pass's probes, with the catchall probe for the pre-existing case). Full pytest was not run here: pytest-timeout and pytest-socket are not installed in this sandbox.

CI: 0 failing Actions jobs on head. The timeouts in tests/test_audit_test_quality.py are the PR body's author-host report; CI on head shows no failure, so not chased.

Raised before:

• @exai-bot T1 (a query-local rejection aborts the run): fixed on head; only run-wide errors stop, the rest are rows.
• @vlad-perevezentsev T2 (the stop message counts one skill's calls): answered; the exit line counts the whole run, per the PR body. Not re-probed.
• @vlad-perevezentsev T3 (stop stored as spend_cap): fixed on head; the stop stores permanent_error.
• @vlad-perevezentsev R2 (estimate change): answered; the estimate and the docs follow it.

Coverage: checked is_run_wide and the stop on head (including the catch-all case), the estimate constants and the printed line. Not looked at: other Python versions, a real provider, the CLI-backed path.

Design: the UnsupportedParamsError scope is the author's call; a question, not a blocker. Follow-up issues: none.

[vchamarthi]

Reviewed head a4bef1c. 1 must fix, 1 nit to fix before merge. Details are in the line comments. Adds a routing stop at permanent provider errors and recalibrates the selection estimate. The stop misses two no-credentials cases (a claude-cli login error and a catch-all error class), and the rest of the change holds.

Verified

• Changed tests: The three changed test files pass on the head, 87 tests.
• Tests pin the stop: On main the new estimate and stop test file does not collect, because routing_stop and routing_estimate do not exist there.
• Other failures: The three failing tests in tests/test_repo_compare_interval.py fail on main as well, so they are not from this change.
• Merge: The branch merges onto main with no conflict.
• Stop is tested: Replacing if is_run_wide(exc): in _ask_once with if False: fails 6 of the changed tests.
• Empty error message: An AuthenticationError with an empty message stops the run with permanent error: AuthenticationError.

Must fix:

1. dokimi/dokimi/novelty/routing_stop.py:12: claude-cli with no login (Not logged in) does not stop the run, so every call still makes a failed row.

Fix before merge:

1. dokimi/dokimi/novelty/routing_stop.py:17: APIConnectionError("Unable to locate credentials") is judged transient, so the run keeps calling and writing failed rows.

Not verified

• Whether UnsupportedParamsError should stop the run. Add it to _RUN_WIDE_ERRORS with a test, or state in the comment why a rejected parameter is left to a failed row.
• Whether a claude-cli usage-limit reply should stop the run. Decide it, then match the message in is_run_wide with a test, or record in the comment why it is left out.

[xaleryb]

• Ran on Python 3.13, head merged with main: the routing tests, lint, types and security checks, plus end-to-end routing runs with no credentials.
• Single run (not --all-vs-all) where every call fails with no credentials -> exits 0 with status ok on main, exits 1 with a partial result on the head -> keep the head's behaviour (docs/skill-novelty.md says the stop exits non-zero, the way the cap does). But the body's "The exit code is unchanged (1)" holds only for --all-vs-all, and the changelog fragment does not mention the change. Suggested changelog addition: "and exits 1 (a single run whose every call failed exited 0 with status ok)".
• Still open from the thread: claude-cli Not logged in and usage limit reached -> is_run_wide False -> should stop the run. UnsupportedParamsError -> False -> should stop, since every call sends the same params.
• The branch now conflicts with main in docs/skill-novelty.md after feat(cli): skill routing and catalog routing replace novelty routing (#94 M3) #595 landed.

[xaleryb]

Pushed 4 commits (a4bef1c..b81e6e8):

• 23ef6da: merges main; docs/skill-novelty.md keeps this PR's permanent-error paragraph and main's skill routing / catalog routing names.
• 86c79cb: CliCallError carries run_wide, set where llm_cli already knows every later call fails alike (no login, a usage limit with or without JSON, an unknown model (404), a missing claude binary), and is_run_wide reads it. With a claude that answers "Not logged in", skill routing --provider claude-cli exited 0 with status ok and a failed row per call; it now stops after 0 of 2 calls and exits 1. A 400, a non-JSON reply and a timeout stay one failed row.
• c72622a: UnsupportedParamsError is in _RUN_WIDE_ERRORS, so it stops the run at the first call.
• b81e6e8: the changelog fragment uses the new command names and says a skill routing run whose every call failed now exits 1 (it exited 0 with status ok).

On the merged tree: full suite (19955 passed), ruff, mypy, bandit, docs check --strict, the CLI snapshot step and check_growth pass; 9 of 9 mutants over the new branches are killed. Left for the author: the PR body's "The exit code is unchanged (1)" holds only for catalog routing; and APIConnectionError("Unable to locate credentials") still counts as transient, as on main (threads 4569209131, 4568069518).

[xaleryb]

Re-checked at b81e6e8 (merge of main plus our three commits): a claude-cli login, usage-limit, unknown-model or missing-binary error and an UnsupportedParamsError now stop the run at the first call, a 400, non-JSON reply or timeout stay one failed row, and the changelog says a skill routing run whose every call failed exits 1; the full suite (19955 passed), ruff, mypy, bandit and the docs and growth gates pass, and CI is green. Left for the author: the body's exit-code sentence, and APIConnectionError('Unable to locate credentials') counted as transient as on main.