Skip to content

feat(novelty): usage and cost block for routing --model - #164

Open
napetrov wants to merge 12 commits into
intel:mainfrom
napetrov:fix/skill-gap-W1
Open

napetrov wants to merge 12 commits into
intel:mainfrom
napetrov:fix/skill-gap-W1

Conversation

@napetrov

Copy link
Copy Markdown
Contributor

Part of #4.

Summary

A novelty routing --model run now records what it spent.

 routing.json
   candidates / queries / lexical / model        (unchanged)
+  cost
+    llm_calls, input_tokens, output_tokens
+    cache_read_tokens, cache_write_tokens
+    usd                                         null when any call had no known price
 stdout
   model    selection 0.5 (...)  [selector-m]
+  cost     2 calls, 200 input / 10 output tokens, cache read 120 / write 60, $0.0040

The cache token counts are also added to the per-role summary that novelty analyze already prints, so its cost block reports them too. A run without --model makes no calls and writes no cost block.

Evidence

Three new tests in tests/test_novelty_routing.py. The two model-run tests fail with the intel/main version of commands/novelty.py and novelty/client.py (no cost block, no cache fields) and pass on this branch:

before: commands/novelty.py and novelty/client.py reverted to intel/main
  FAILED test_a_model_run_records_its_calls_tokens_and_cost
  FAILED test_a_model_run_with_an_unpriced_call_reports_no_usd_figure
  1 passed (test_a_lexical_run_spends_nothing_and_has_no_cost_block)

after: this branch
  all three pass (full suite below)

Local checks on the rebased head:

uv run ruff check .                              All checks passed!
uv run mypy dokimi                               Success: no issues found in 222 source files
uv run python -m dokimi.schema_check             exit 0
uv run python -m dokimi.config_check             exit 0
uv run pytest -n auto -q                         6882 passed, 22 skipped

Merge Danger

Door: two-way. Every change is additive: new keys in the routing JSON and in the analyze cost summary, and no existing key changes meaning. Reverting restores the old output.

Blast Radius: low, limited to --model routing runs and the analyze cost summary.

  • Schema. schemas/novelty.v1.json gains cache_read_tokens and cache_write_tokens in the cost summary. Readers that validate strictly will see two new optional-looking integer fields.
  • Unpriced calls. usd is null, not 0, when any call had no price, so a missing price is not read as a free run.
  • Cache accounting. Whether cache tokens are included in input_tokens depends on the provider. The docs say so; the two counts are reported separately.
  • No new flags. No user must pass anything new, and no persisted format changes beyond the additive keys.

Checklist

  • Tests added/updated and pytest passes
  • ruff, mypy, and schema checks pass
  • Docs updated: docs/skill-novelty.md (routing cost block)
  • No ad-hoc run artifacts committed
  • New code and test files carry the SPDX license header (no new code files; the changelog fragment follows the existing fragments, which carry none)

🤖 Generated with Claude Code

A model-backed `novelty routing` run wrote no record of what it spent,
so a routing rate could not be read against its cost. The run now
writes a `cost` block into routing.json (calls, input/output tokens,
cache read/write tokens, and usd, null when any call is unpriced) and
prints the same totals on stdout. Cache token totals are added to the
per-role cost summary, so novelty analyze reports them too.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Declare cache_read_tokens and cache_write_tokens (integer, minimum 0) in
the novelty.v1 cost block, which routing and analyze both write. Say in
skill-novelty.md that cache tokens are reported separately and may or
may not be inside input_tokens, depending on the provider. The routing
test stub now sets cache_write_tokens too, and asserts its total. The
stdout cost line drops the trailing model tag, which the model line
above already carries.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
@codecov-commenter

codecov-commenter commented Oct 22, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.99%. Comparing base (9a93635) to head (f4df39a).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
+ Coverage   91.98%   91.99%   +0.01%     
==========================================
  Files         223      223              
  Lines       27743    27754      +11     
  Branches     4560     4561       +1     
==========================================
+ Hits        25520    25533      +13     
+ Misses       1574     1572       -2     
  Partials      649      649              
Files with missing lines Coverage Δ
dokimi/dokimi/commands/novelty.py 95.91% <100.00%> (+0.07%) ⬆️
dokimi/dokimi/novelty/client.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.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cost block is right when every call returns a priced usage record, and the JSON totals are correct. Three things are worth fixing before merge: the stdout line when calls are missing or failed, the replayed-call accounting, and the stdout assertions.

What I ran, on a Linux review host at 14a7e93:

  • make ci-local: 20 passed, 8 failed, 7 skipped. 6883 tests passed, 21 skipped; changed-line coverage, schema-compat vs the base, wheel build and smoke all pass. The 8 failures are ruff, mypy, mkdocs, pip-audit and the other tool steps, missing from my venv because make dev stops at the tools install on this base, which #179 fixed on main. Static checks, mypy and the strict docs build pass on GitHub's CI at this head.
  • Reproductions for the code findings, with the provider call stubbed.
  • Mutation run on the new code and tests: mutants of client.py, the JSON totals and the cost block are all caught. Four mutants of the printed line pass the suite (inline comment).

Checked, no issues

  • Schema: the two new fields are additive optional integers matching their neighbours, and the cost object has no additionalProperties: false, so no version bump is needed. routing.json itself is not validated by any schema.
  • Other callers: roll_up_cost has one other caller (novelty/analyze.py); the markdown report prints only calls, tokens and usd, so it does not change.
  • Lexical runs: no cost block is written and the print sits under the same if args.model guard.

Comment thread dokimi/dokimi/commands/novelty.py Outdated
Comment thread dokimi/dokimi/commands/novelty.py
Comment thread dokimi/tests/test_novelty_routing.py Outdated
napetrov and others added 6 commits October 22, 2026 11:44
… cache replays

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…commands/novelty.py for W1

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…baseline after the extraction

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
roll_up_cost sums the new cache_read/cache_write/replayed_calls fields over
every role, but only single-client tests covered them, so a total taken from
the first role alone (what novelty analyze would then report) passed.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at d2fd404. The three earlier threads are fixed: the zero-call line, the replayed-call accounting and the full-line assertion all hold. Two things are worth fixing before merge: a change to novelty analyze's existing cost keys that the PR calls additive, and two test gaps found by mutation.

What I ran, on a Linux review host at d2fd404:

  • make ci-local: 28 passed, 1 failed, 7 skipped. 7213 tests passed, 1 skipped; changed-line coverage, schema-compat vs the base, mypy, ruff, wheel build and smoke all pass. The one failure is pip-audit, which reports a gitpython 3.2.0 advisory (fix 3.2.1) from the lock files; the PR does not touch them.
  • Reproductions with the LLM transport injected, comparing roll_up_cost at the base and at the head.
  • Mutation run on the new code against test_novelty_routing.py and test_novelty_measure.py: 23 mutants, 4 of which pass the suite (two inline comments).

This PR changes existing novelty analyze cost values, not only adds keys. analyze.py:386 calls roll_up_cost, and cost_summary (client.py:171-178) now skips replayed rows and sets usd to null when a call returned no usage. Two calls, each 100 in / 10 out / $0.01, replayed=True:

  • base: {'llm_calls': 2, 'input_tokens': 200, 'output_tokens': 20, 'usd': 0.02}
  • head: {'llm_calls': 2, 'input_tokens': 0, 'output_tokens': 0, ..., 'replayed_calls': 2, 'usd': 0}
    One live call and one call with usage=None: base usd 0.01, head usd None. So a resumed analyze run with DOKIMI_RESPONSE_CACHE now reports llm_calls that no longer matches its token and usd totals, and the Markdown report (novelty/report.py:468-474, outside this diff) prints "2 LLM calls, 0 in / 0 out, usd 0" without mentioning replays. The new rules are the right ones, and they match the spend cap. But the PR description says "no existing key changes meaning", the changelog fragment files this under Added and names analyze only for the cache keys, and the docs paragraph sits in the routing section. Suggestion: say in the changelog fragment (as Changed) and in the cost schema descriptions of llm_calls, input_tokens, output_tokens and usd that replayed calls are excluded from the totals, print replayed_calls in the report.py cost line when it is non-zero, and correct the PR description. Related: with an unpriced replayed row and a priced live row, usd is 0.01, while the docs and schema say usd is null when any call had no known price. Say "any call this run paid for".


monkeypatch.setattr(llm, "llm_call_with_usage", failing)
payload = _model_run(tmp_path)
assert payload["cost"]["llm_calls"] == 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing pins usd for a client that made no calls. This test asserts llm_calls == 0 and the printed text, but not payload["cost"]["usd"].

Mutation check: removing self.calls > 0 from priced (client.py:178) makes an idle client report usd 0.0, which reads as a free run. The full routing and measure files still pass (71 passed), because the 0 calls line hides the value.

Suggestion: add assert payload["cost"]["usd"] is None after line 545.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinned in 2594a80: test_a_model_run_whose_every_call_fails_prints_no_price_blame now asserts payload["cost"]["usd"] is None (fails with self.calls > 0 dropped).

from dokimi.metrics.usage import UsageRecord

_stub_provider(monkeypatch, UsageRecord(prompt_tokens=100, completion_tokens=5,
cost_usd=0.002, replayed=True))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replay test never gives the replayed rows cache tokens, so cache totals on replayed rows are unpinned. The UsageRecord here has no cache_read_tokens or cache_write_tokens, and the test asserts neither.

Mutation check: summing cache_read and cache_write over self.usage instead of billed (client.py:174-175) passes both files (71 passed), so replayed cache tokens would be billed as this run's.

Suggestion: build the record with cache_read_tokens=60, cache_write_tokens=30 and assert cost["cache_read_tokens"] == 0 and cost["cache_write_tokens"] == 0. Also assert "replayed" not in out in test_a_model_run_records_its_calls_tokens_and_cost: a mutant that always prints the replayed suffix also passes today.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2594a80: test_replayed_calls_are_counted_apart_and_not_billed now gives the replayed rows cache_read 60 / cache_write 30 and asserts both totals are 0 (fails when summed over self.usage); test_a_model_run_records_its_calls_tokens_and_cost asserts "replayed" not in out.

@xaleryb

xaleryb commented Oct 23, 2026

Copy link
Copy Markdown
Contributor
  • Ran: full suite, ruff, mypy and bandit on the head merged with main, plus novelty analyze with every call served from the response cache (replayed=True).
  • Open: novelty analyze with a cached rerun -> cost 0 in / 0 out, usd 0, and the md report prints 0 in / 0 out, usd 0. with no replay count -> the stored totals, as docs/benchmarking-and-comparison.md:691-693 says ("not $0/0s, so cost stays comparable across runs"). The change sits in the shared cost_summary, so it reaches analyze, not just routing. A call with no usage row now gives usd None. in the analyze md, and the changelog does not mention it.
  • Pushed: a5b007e test: roll_up_cost sums cache_read/cache_write/replayed_calls over every role (a first-role-only total passed before).
  • Left for the author: choose either stored totals plus a separate this-run spend field, or the exclusion applied to routing only; then update the doc and the report Cost line. Also state whether zero calls should give usd None or 0.0, and whether cache tokens on replayed rows are excluded, and pin both with tests.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at a5b007e. The only change since my last review is a test that pins the cross-role roll_up_cost sums, and it holds. The analyze behaviour change and both test gaps from d2fd404 are still open, plus one more wording point on usd.

What I ran, on a Linux review host at a5b007e:

  • make ci-local: 28 passed, 1 failed, 7 skipped. 7214 tests passed, 1 skipped; ruff, mypy, schema-compat vs the base, docs build, wheel build and smoke all pass. The one failure is pip-audit, which reports an advisory from the lock files; this PR does not touch them.
  • Mutation run on cost_summary, roll_up_cost and the printed routing summary against test_novelty_routing.py and test_novelty_measure.py: 33 mutants, 8 of which pass both files. Three of them are the ones in my open threads; the rest are print-only lines of _print_routing_summary that predate the PR's logic.
  • Reproductions with the LLM transport injected.

Still open from my earlier review:

  • dokimi/tests/test_novelty_routing.py:545: nothing pins usd is None for a client with no calls; dropping self.calls > 0 at client.py:178 still passes.
  • dokimi/tests/test_novelty_routing.py:569: the replay test gives the replayed rows no cache tokens; summing cache tokens over self.usage instead of billed (client.py:174-175) still passes, and so does printing the replayed suffix unconditionally.
  1. This PR still changes existing novelty analyze cost values, not only adds keys. analyze.py calls roll_up_cost, which now takes cost_summary's rules: replayed rows are left out of tokens and usd, and usd is null when a call returned no usage. Two replayed calls, each 100 in / 10 out / $0.01, give input_tokens: 0, output_tokens: 0, usd: 0, replayed_calls: 2 at the head, against 200 / 20 / 0.02 at the base. llm_calls still counts them (2), so calls and tokens describe different sets. Nothing else moved with it:

    • novelty/report.py:468-473 (outside this diff) prints only calls, tokens and usd, so a cached analyze rerun reads "2 LLM calls, 0 in / 0 out, usd 0" with no mention of replays, and usd None for a call with no usage.
    • docs/benchmarking-and-comparison.md:691-693 (outside this diff) still says a replayed entry reports the stored usage "so cost stays comparable across runs".
    • The changelog fragment files it all under Added; the usd, llm_calls, input_tokens and output_tokens descriptions in schemas/novelty.v1.json do not mention the exclusion.
      Suggestion: move the analyze part of the fragment to Changed, add the exclusion to those schema descriptions, print replayed_calls in the report.py cost line when it is non-zero, fix the benchmarking doc, and correct the PR description ("no existing key changes meaning").
  2. usd is not null "when any call had no known price" if the unpriced call was replayed. The check runs over non-replayed rows only (client.py:178). One live call (5 in, $0.1) plus one replayed call with no price gives llm_calls: 2, input_tokens: 5, replayed_calls: 1, usd: 0.1. A single unpriced replayed call gives usd: 0 and the routing line prints $0.0000. The changelog, docs/skill-novelty.md:611 and the cost_summary docstring all say "any call". Excluding replayed rows is the right rule; the wording is not. Suggestion: say "any call this run paid for" in the fragment, the doc paragraph, the docstring and the schema usd description.

@xaleryb's question on whether analyze keeps stored totals or this-run spend, and whether zero calls give usd None or 0.0, waits for the author.

Checked, no issues

  • Schema: the new cost fields are optional integers added in place, so no version bump or SCHEMA_VERSIONS change is needed.
  • roll_up_cost: zeroing any one of the calls, input, output, cache read, cache write or replayed sums fails the suite, and the new cross-role test covers the first-role-only total.
  • Routing line: the zero-calls branch, the unknown-USD text, the .4f format and swaps of read/write or input/output are all caught.

Comment thread dokimi/tests/test_novelty_measure.py
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit per nit, up to 54f4ca8, which fix the nit from my review:

  • 54f4ca8: dokimi/tests/test_novelty_measure.py:247: added the second blank line before test_a_failed_call_is_a_dropped_pair_not_a_graded_answer.

make ci-local at 54f4ca8: 28 passed, 1 failed (pip-audit, as at the reviewed head), 7 skipped.

Not approving yet. Still open: the two risks from my review; the two earlier threads of mine above nit level.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

# Conflicts:
#	dokimi/dokimi/commands/novelty.py
#	dokimi/scripts/growth_baseline.json
#	dokimi/tests/test_novelty_routing.py
…ay 'this run paid for'

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
@napetrov

Copy link
Copy Markdown
Contributor Author

Review-body items at 2594a80 (after merging main in 21a1e0d, which had conflicts in novelty.py, growth_baseline.json and test_novelty_routing.py):

  • Risk 2 (usd wording for a replayed unpriced call): the changelog fragment, docs/skill-novelty.md, the cost_summary docstring and the usd description in novelty.v1.json now say "any call this run paid for" (2594a80).
  • Risk 1 (novelty analyze totals now exclude replayed calls, report line, benchmarking doc, fragment under Changed): not changed. Whether analyze keeps stored totals or reports this-run spend is a design choice I have not made; it is waiting on the author.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

# Conflicts:
#	dokimi/scripts/growth_baseline.json

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants