Repository navigation
Conversation
…n queries - routing --estimate-only prints the calls, tokens and list-price USD that --model would make, as JSON, and makes no model call. - all-vs-all accepts --repeats with the same noise block as routing, and prints a cost block; the JSON gets the cost too. - all-vs-all refuses queries for a skill that only sets disable-model-invocation, names it, and suggests --include-disabled. - --queries-from-headings falls back to the description's sentences when the headings give no query; queries_source records which one was used. - --seed help says it fixes the card order only and points at --repeats for answer noise. - Routing console lines and query loading move to commands/novelty_routing_report.py to keep novelty.py under its ratchet. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…fuses --estimate-only, repeats count calls The model estimate skipped contentless queries that the run still sends, --all-vs-all --estimate-only made paid calls, the stderr call count left out the repeats, and the model arm's unscored and tied calls were printed as queries. Fix each and cover it with a test. Routing console output moves to commands/novelty_routing_report.py to keep commands/novelty.py under its growth cap. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ves nothing With neither a heading nor a description sentence, the run refuses with no_queries, but the warning still said the queries were derived from the artifact. Print it only when a query was derived. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…arm repeats With --repeats above 1 the model arm's sibling count is per call, so the attractor line now names its unit the way the tie and unscored line does. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
github-actions
Bot
added
the
cli-interface-change
CLI interface change detectedThis PR changes dokimi's user-facing CLI surface (commands and/or options): ~ `dokimi novelty routing` parameters changed:
+ added option `--estimate-only`Updated on every push; the |
|
🔍 OpenCodeReview found 1 issue(s) in this PR. By severity: 1 low
Review details
|
| priced = list_cost_usd(args.model, args.provider, estimate["input_tokens"], | ||
| estimate["output_tokens"]) |
There was a problem hiding this comment.
Must-fix: these are run-wide token totals, but list_cost_usd prices them as one completion. For models with context-length pricing tiers (for example Gemini 2.5 Pro), many short calls can therefore cross the long-context threshold only in the estimate and be charged the higher rate, although every real request stays below it. A short query with a sufficiently large --repeats reproduces this; changing repeats should not change the per-call tariff. Price each query's prompt and one pick separately, then sum and multiply by repeats. Add a tiered-pricing test; the current linear pricing mock cannot detect this.
There was a problem hiding this comment.
Fixed in 1c5348b. The estimate now prices each query's prompt and one pick on their own, sums them, and multiplies by the repeats, so a long-context tier applies to one call's prompt as it does in a real run. Pinned by test_estimate_prices_each_call_so_a_long_context_tier_is_not_reached_by_the_run_total, which uses a tiered stub; it failed on the previous head (0.568 against the 0.284 expected).
| f"{len(queries_by_skill)} skills with queries") | ||
| _print_attractors(result) | ||
| print_attractors(result) | ||
| print_all_vs_all_model(model_arm, payload.get("cost")) |
There was a problem hiding this comment.
Must-fix: this reporting call is unreachable whenever any model call fails, because the earlier if failed raises SystemExit. For example, the existing partial-failure test (one successful call followed by two failures), extended with --repeats 3, saves cost/noise in the file but prints neither on stdout. This hides usage from successful calls and breaks the new console-reporting promise. Move this call before the failure exit, while keeping the success-only summary after it, and extend the partial-failure test to assert cost/noise output and a non-zero exit.
There was a problem hiding this comment.
Fixed in 2ccd64b. The cost and noise lines now print before the failure exit, so a partial run shows the usage of the calls that ran. Pinned by test_a_partial_failure_still_prints_the_cost_of_the_calls_that_ran, which found no cost line before the fix. On a successful run those two lines now come before the OK Saved line instead of after the attractors.
There was a problem hiding this comment.
Correction to my earlier reply on this thread. I wrote that the success path was unchanged. It was not: a6fc22d dropped the noise and cost lines from a clean all-vs-all run.
Fixed in 8611ba8. A clean run now prints the summary, the attractors, then the noise and cost lines, as on main. A partial run prints the noise and cost lines before the failure exit, so the usage of the calls that ran is still shown.
Pinned by test_a_partial_failure_still_prints_the_cost_of_the_calls_that_ran and test_a_clean_all_vs_all_run_still_prints_its_cost_after_the_summary.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #329 +/- ##
========================================
Coverage 94.05% 94.05%
========================================
Files 297 298 +1
Lines 38018 38133 +115
Branches 6424 6443 +19
========================================
+ Hits 35756 35867 +111
- Misses 1452 1455 +3
- Partials 810 811 +1
🚀 New features to boost your workflow:
|
The estimate priced the run's summed prompt and picks as one completion. A long-context tier applies to one call's prompt, so many short calls could be charged the higher rate although no real request reaches it. Each query's prompt and one pick are now priced on their own, summed and multiplied by the repeats. Thread: PRRT_kwDOVBg7g86yTJiq Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The failure exit came before the cost and noise lines, so a run with some failed calls hid the usage of the calls that succeeded. Those lines now print before the exit; the success summary stays after it. Thread: PRRT_kwDOVBg7g86yTJix Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The per-call pricing added in 1c5348b returns None for an unpriced model; no test reached that path, which left a patch-coverage gap. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…he partial exit CI on a6fc22d failed the growth ratchet: novelty.py was 1149 lines against a baseline of 1147. The partial-failure exit moves into exit_partial_all_vs_all in novelty_routing_report.py, so the command body shrinks. The success path keeps its original order: the summary, then the attractors, then the noise and cost lines. A test pins that the cost line still follows the saved-file line on a clean run. The baseline drops to 1146 as check_growth.py --update reports. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 8611ba8. --estimate-only, --all-vs-all --repeats and the all-vs-all cost block work as described. The description fallback of --queries-from-headings needs another look: its queries are the gold card's own text, so a skill that used to give no_queries now gets a rate near 1.0.
What I ran, at 8611ba8:
make ci-local --offline: 11,990 passed, 0 failed, 58 skipped; all checks pass.- A repro of the description fallback: one-word headings, a three-sentence description, and three competitors that share its words. Lexical R@1 1.0, MRR 1.0,
queries_source: description. --estimate-onlyon an artifact the real run refuses: exit 0,n_calls: 0,expected_usd: 0.0.- An all-vs-all query file that names a disabled catalog skill: the refusal is right, but the field is built twice, so stderr shows two different
--catalogcounts.
Checked, no issues
- Estimate pricing: each call is priced on its own prompt and then multiplied by the repeats, so a long-context tier is not reached by the run total. The tiered-stub test pins this.
- Partial all-vs-all exit: the noise and cost lines print before the failure exit. A clean run prints them after the attractors, as on main.
- Repeats in all-vs-all:
model_routingreturns each query's repeats next to each other, so the slices in_model_armgroup them correctly for_noise. The call count on stderr is multiplied by the repeats. - CLI snapshot and docs:
cli-interface.json, the--seedhelp andskill-novelty.mdmatch the new flags.
| def description_as_queries(description: str, *, limit: int = 40) -> list[str]: | ||
| """Sentences of a description, for triage when the artifact has no usable heading. | ||
|
|
||
| A skill whose body is a ``#`` title and no ``##`` heading of two or more words still says | ||
| what it is for in its description. Each sentence with two or more content words is | ||
| offered, as :func:`headings_as_queries` offers headings. The same self-derived stamp applies. | ||
| """ | ||
| sentences = _SENTENCE_END.split(" ".join(description.split())) | ||
| return [s for s in sentences if len(tokenize(s)) >= 2][:limit] | ||
|
|
||
|
|
||
| def self_derived_queries(body: str, description: str) -> tuple[list[str], str]: | ||
| """The triage queries of an artifact and where they came from. | ||
|
|
||
| Headings are preferred; the description's sentences are the fallback. Returns | ||
| ``(queries, source)`` with ``source`` ``"headings"``, ``"description"``, or ``"none"`` when | ||
| neither gives a query of two or more content words. | ||
| """ | ||
| headings = headings_as_queries(body) | ||
| if headings: | ||
| return headings, "headings" | ||
| sentences = description_as_queries(description) | ||
| return sentences, "description" if sentences else "none" |
There was a problem hiding this comment.
Blocking: The description fallback routes the gold card's own text, so its rates are near 1.0 by construction.
The gold card both arms rank is name + description, and these queries are sentences of that same description, so every query appears word for word in the gold card. Headings at least come from the body, which is not on the card. A repro: gold grill with the description "Grill the user about a plan. Use when stress-testing a design. Ask hard questions about tradeoffs." and only a one-word ## Steps heading, against three competitors that share plan, design, user and questions: queries_source: description, lexical R@1 1.0, MRR 1.0. Before this PR the same skill gave status: no_queries. So the 24 of 144 skills that were "not measured" now get a near-perfect figure, with the same triage stamp as a heading run, and a corpus-wide figure like the 0.225 in the docs moves up because of them. The changelog does not say this.
Suggestion: keep the no_queries refusal for a skill with no usable heading. If the fallback stays, the rate lines and the docs should say that a description-derived rate is a self-match and not comparable with a heading-derived one, and a corpus summary should leave these skills out.
There was a problem hiding this comment.
Waiting on a decision before I change this: should the description fallback stay?
Context: your point holds. A description-derived query scores R@1 1.0 and MRR 1.0 against its own gold card (a 3-sentence description, three competitors: queries_source: description). Before this PR those skills gave no_queries.
Options:
(a) Keep the no_queries refusal for a skill with no usable heading (your first suggestion). The description queries are dropped, and the "description queries" part of the PR goes.
(b) Keep the fallback, stamped as a self-match: the rate lines and the docs say it is not comparable with a heading rate, and a corpus summary leaves those skills out.
My recommendation: (a). A rate near 1.0 by construction should not reach a corpus figure, and (b) needs changes in the summary code too. The thread stays open until the author picks.
| if args.estimate_only: | ||
| print_routing_estimate(args, candidates, queries) | ||
| return |
There was a problem hiding this comment.
--estimate-only returns before the no-usable-queries refusal, so it prices a run that would be refused as a free, valid run.
Repro: an artifact with the description Stop. and only ## Steps, run as novelty routing --queries-from-headings --model claude-sonnet-4-5 --estimate-only, prints n_calls: 0, expected_usd: 0.0 and exits 0. The same command without the flag exits non-zero with "no usable queries" and writes a no_queries record. The same happens with a --queries file of only stop words. A batch script that budgets with the estimate sees a zero-cost run instead of a refusal.
Suggestion: check tokenize on the queries (as selection_prompt_tokens already does) before the estimate branch, and exit with the same reason the real run gives.
There was a problem hiding this comment.
Fixed in 0fdc281. --estimate-only now refuses with the real run's reason when no query has a content word: exit 2 with no usable queries, nothing estimated, nothing written. Pinned by test_estimate_only_refuses_the_run_the_real_run_refuses (a --queries-from-headings case and a stop-words-only file), which failed before the change.
The PR body still says the estimate exits 0 for such a run (its "declined" bullet). That line is out of date now and needs the author's update.
| everyone = _routing_field(argparse.Namespace(**{**vars(args), "include_disabled": True}), | ||
| set()) |
There was a problem hiding this comment.
_refuse_queries_naming_disabled builds the field a second time, so every field warning prints twice, with a wrong competitor count.
Repro: a catalog with one disable-model-invocation skill and a queries file that names it. stderr shows --catalog ...: 5 competitors and then --catalog ...: 6 competitors before the refusal; the second count includes the disabled skill, which is not in the field. Skipped-competitor and no-description warnings repeat too, and a large catalog is read twice.
Suggestion: the first _routing_field call already appends <name>: disable-model-invocation to left_out (the catalog notes too); pass left_out in and take the disabled names from it instead of loading the field again.
There was a problem hiding this comment.
Fixed in 0fdc281. The field is read once. The loader reports the names it left out for disable-model-invocation (--competitors and --catalog), and the refusal takes them from that list, so each field warning prints once and the count is right. Pinned by test_a_disabled_skill_named_by_a_query_warns_once_and_counts_the_field_once, which failed before the change.
| gold=index, seed=seed + index) | ||
| gold=index, seed=seed + index, repeats=repeats) | ||
| rows = routed["per_query"] | ||
| groups += [rows[start:start + repeats] for start in range(0, len(rows), repeats)] |
There was a problem hiding this comment.
The all-vs-all noise.per_query rows pool every skill's queries but do not say which gold each row belongs to.
In a single-artifact run there is one gold, so query is enough. Here two skills can list the same request (profile my binary for perf and for profiler), and you get two rows with the same query and different n_correct, with nothing to tell them apart or to read agreement per skill.
Suggestion: add the gold skill name to each group's row (for example gold) when building groups in _model_arm.
There was a problem hiding this comment.
Fixed in 0fdc281. Each row of an all-vs-all noise block now names its gold skill, so two rows with the same query can be told apart. Pinned by test_every_noise_row_names_the_gold_skill_it_belongs_to, and docs/skill-novelty.md describes the field.
| return [h for h in headings if len(tokenize(h)) >= 2][:limit] | ||
|
|
||
|
|
||
| _SENTENCE_END = re.compile(r"(?<=[.!?])\s+") |
There was a problem hiding this comment.
Nit: The sentence split breaks after any period and space, so e.g. / i.e. / vs. cut one sentence into two queries.
"Profile code, e.g. PyTorch training loops, and report hotspots." gives "Profile code, e.g." and "PyTorch training loops, and report hotspots."; both have two content words, so both are routed.
Suggestion: do not split after a short lowercase token ending in a period, or skip common abbreviations.
There was a problem hiding this comment.
Fixed in 0fdc281. The sentence split no longer breaks after e.g., i.e., vs. or cf., so "Profile code, e.g. PyTorch training loops, and report hotspots." stays one query. Pinned by test_a_sentence_with_e_g_or_i_e_is_one_description_query, which failed before the change.
|
|
||
| from .client import LLMClient | ||
| from .routing import BM25, Candidate, RoutingInputError, model_routing, tokenize | ||
| from .routing import BM25, Candidate, RoutingInputError, _noise, model_routing, tokenize |
There was a problem hiding this comment.
Nit: all_vs_all.py now imports the private _noise from routing.py.
Someone changing a _ helper in routing.py will assume it has no users outside the module.
Suggestion: rename it to a public name (for example repeat_noise) now that two modules use it.
There was a problem hiding this comment.
Fixed in 0fdc281. The helper is public as repeat_noise in novelty/routing.py, and both modules import that name.
- --estimate-only refuses a run the real run would refuse (no query with a content word), with the same reason, and writes nothing. - The disabled-skill refusal reads the field once: the loader reports the names it left out, so field warnings no longer print twice. - Each all-vs-all noise row names its gold skill. - A sentence split does not break after e.g., i.e., vs. or cf. - _noise is public as repeat_noise: two modules use it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
There was a problem hiding this comment.
🟡 Changes recommended
A lexical-only estimate incorrectly reports a query as sent when no selector calls occur because zero repeats still populate the prompt list.
1 open issue
What changed in this PR
1 new finding.
This PR extends dokimi’s routing evaluation with cost estimates, repeated all-vs-all measurements, and description-derived triage queries.
Changes:
- Adds single-artifact
--estimate-onlyand all-vs-all repeat, noise, and cost reporting. - Adds description fallback queries, clearer disabled-skill refusals, and precise seed help.
- Extracts routing reporting helpers, expands regression coverage, and updates documentation.
| File | Description |
|---|---|
dokimi/tests/test_novelty_routing.py |
Updates noise helper and no-query assertions. |
dokimi/tests/test_novelty_routing_field.py |
Checks disabled-skill warnings are not duplicated. |
dokimi/tests/test_novelty_routing_cli_gaps.py |
Covers estimates, description queries, and help text. |
dokimi/tests/test_novelty_routing_all_vs_all.py |
Covers repeats, costs, refusals, and noise attribution. |
dokimi/scripts/growth_baseline.json |
Lowers command-module growth baselines. |
dokimi/dokimi/novelty/routing.py |
Adds query derivation and estimation helpers. |
dokimi/dokimi/novelty/all_vs_all.py |
Pools repeated model results and labels noise rows. |
dokimi/dokimi/commands/novelty.py |
Wires CLI options and delegates reporting. |
dokimi/dokimi/commands/novelty_routing_report.py |
Centralizes query loading, estimates, refusals, and output. |
dokimi/docs/skill-novelty.md |
Documents the expanded routing behavior. |
dokimi/docs/reference/cli-interface.json |
Records the new estimate flag. |
dokimi/changelog.d/20261031_120000_napetrov_routing-cli-gaps.md |
Summarizes routing additions and fixes. |
Review effort: Balanced
| Every repeat sends the same prompts (see :func:`selection_prompt_tokens`). The reply is one | ||
| JSON pick, :data:`PICK_REPLY_TOKENS`. | ||
| """ | ||
| prompts = selection_prompt_tokens(candidates, queries) |
| f"({model_arm['first_error']}); the partial result, with those queries " | ||
| f"left out of every rate, is in {out}") |
There was a problem hiding this comment.
Must-fix: With --all-vs-all --repeats 2, one query can have a successful first call and a failed second call. _model_arm retains the successful call in the confusion matrix and own_rate, but this exit message says the query is left out of every rate. Describe failed calls as excluded from model-arm scoring, and distinguish that from repeat-noise rates, which exclude the entire query if any repeat fails. Update the matching warning in _cmd_routing_all_vs_all too, and cover a mixed-success repeat in a CLI test.
There was a problem hiding this comment.
Fixed in 12f8c62. The exit message and the WARNING now say failed calls are left out of model-arm scoring, and a query with a failed repeat is left out of the repeat-noise rates. A query's good repeat stays counted. Red test: test_a_repeat_that_fails_after_a_good_one_keeps_the_good_pick_in_model_scoring (new file, test_novelty_routing_repeat_failures.py) failed on the old wording and the old estimate.
…lls leave out --estimate-only without --model passed repeats=0, which still counted the prompts, so n_queries_sent was non-zero for a run that sends nothing. The failed-call message said the queries were left out of every rate. A call that failed is only left out of the model-arm counts; a query with a failed repeat is left out of the repeat-noise rates. Say both. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 12f8c62. Three things are worth fixing before merge, plus one nit inline, and the merge waits for a decision on the description fallback.
@vlad-perevezentsev's blocking point on the description fallback is still open at this head: the fallback queries are sentences of the gold card's own description, so the rate is near 1.0 by construction. The author has handed the choice back and it waits for a decision. New evidence: the docs and the changelog still do not say a description run is a self-match, and skill-novelty.md puts the 0.225 corpus figure for self-derived queries next to the fallback text.
What I ran, on a Linux review host at 12f8c62:
make ci-local: 30 passed, 0 failed, 7 skipped; 11,996 tests; the changed-line coverage step passes.- A mutation run over the new code: 114 mutants, 38 of which pass the five routing test files. Most only change a message. Two decide reported values and are in the inline comments.
- Reproductions for the code findings, with the LLM transport mocked.
Findings that cannot be anchored inline:
- The "Cost of
--modelin all-vs-all" paragraph is stale after this PR.docs/skill-novelty.md:693-698says the model arm makes one call per query, that the call count is the number of queries with a content word, and quotes the stderr lineall-vs-all: --model makes N model calls, one per query, .... With--repeats 5the run makes five times that, and the code (novelty.py:394-396) multiplies byargs.repeatsand prints "one per query and repeat". A reader who sizes a budget from this paragraph under-counts by the repeat factor. Suggestion: say "one per query and repeat" and update the quoted line.
Checked, no issues
- Estimate vs. run: the estimate prices every query, contentless ones included, once per repeat, and refuses exactly when the real run refuses (no query has a content word).
--all-vs-all --estimate-onlyis a usage error. - All-vs-all repeats: groups are query-major and repeat-minor,
n_unscoredis multiplied by the repeats, and the noise rows pair with their gold throughzip(..., strict=True). The partial-failure exit prints noise and cost before it exits. - Disabled-skill refusal: it runs before any client is built, the field is loaded once, and
--include-disabledbypasses it. - CLI snapshot:
cli-interface.jsonmatches a fresh dump, and--estimate-onlyis the only entry added. All commits are signed off.
Bot points I checked that no longer apply at this head (4)
- Copilot's point on the lexical-only estimate reporting a query as sent does not apply: at this head
selection_estimatetakes no prompts at zero repeats, son_queries_sentandn_callsare 0. - exai-bot's point on pricing run-wide token totals as one completion does not apply: the estimate now prices each call's prompt and one pick separately, then multiplies by the repeats.
- exai-bot's point that the cost and noise lines are unreachable after a failed call does not apply: they print before the failure exit, and a clean run prints them after the attractors.
- exai-bot's point on the partial-failure wording does not apply: the exit message and the warning now say failed calls are left out of model-arm scoring, and a good repeat stays counted.
| 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] |
There was a problem hiding this comment.
The estimate's input_tokens, output_tokens and expected_usd are not tied to the prompt the run sends.
Mutation check: changing // 4 to // 3 here passes the 221 tests in test_novelty_routing.py, _all_vs_all.py, _cli_gaps.py and _repeat_failures.py. So do PICK_REPLY_TOKENS = 12 and joining the cards with "\n\n". The estimate assertions in test_novelty_routing_cli_gaps.py only check > 200_000 and that usd equals the tokens times the stub price. Nothing compares the estimate with the prompt model_routing builds, so a drift between them gives a wrong budget with a green suite.
Suggestion: add a test that records the prompts a mock client receives for a small field and asserts len(prompt) // 4 == selection_prompt_tokens(candidates, queries)[i] for each call.
| gold=index, seed=seed + index, repeats=repeats) | ||
| rows = routed["per_query"] | ||
| groups += [rows[start:start + repeats] for start in range(0, len(rows), repeats)] | ||
| golds += [name] * len(queries) |
There was a problem hiding this comment.
The gold label on the noise rows is only tested with one query per skill.
Mutation check: golds += [name] (one label per skill, not per query) passes all 221 routing tests. With two or more sent queries and --repeats 2, the label list is shorter than the noise rows, so zip(..., strict=True) at line 217 raises ValueError after the model calls were paid for. Dropping strict=True instead truncates the rows and loses their gold.
Suggestion: add an all-vs-all test with one skill that has two queries and repeats=2, and assert each noise.per_query row's gold and query.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
|
|
||
| _SENTENCE_END = re.compile(r"(?<=[.!?])\s+") | ||
| # A period that ends an abbreviation, not a sentence: "e.g.", "i.e.", "vs.", "cf.". | ||
| _ABBREVIATION_END = re.compile(r"(?:^|\s)(?:e\.g|i\.e|vs|cf)\.$", re.IGNORECASE) |
There was a problem hiding this comment.
Nit: The abbreviation fix still splits after parenthesized abbreviations. description_as_queries("Profile code (e.g. PyTorch training loops) and report hotspots.") produces "Profile code (e.g." and "PyTorch training loops) and report hotspots.", rather than one query, because ( does not satisfy (?:^|\s). Recognize opening punctuation before these abbreviations and add this case to the regression test; otherwise common description wording becomes two incomplete routing requests.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 94f84fa, which fix the nit from my review:
94f84fa:dokimi/commands/novelty.py:824,docs/skill-novelty.md:621and the changelog entry now say the list-priceexpected_usdinstead ofusd.
make ci-local at 94f84fa: 30 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the three risks from my review; @vlad-perevezentsev on description fallback routes the gold card's own text, so its rates are near 1.0 by construction; unfixed, docs and changelog silent; @vlad-perevezentsev on keep the description fallback (stamped as a self-match) or drop it: the author handed the choice back.
Summary
A rescan of public skill catalogs (10-30) found five gaps in
novelty routing. This PR closes them:Self-derived queries stay labelled as triage (
triage: self-derived,queries_self_derived: true), andqueries_sourceisheadings,description, ornone. When neither gives a query the run still refuses withno_queries, and the warning no longer claims a derivation that did not happen.Routing console lines and query loading move to
commands/novelty_routing_report.py, socommands/novelty.pyshrinks from 1171 to 1147 lines. The growth baseline is lowered, not raised.Evidence
Red on intel/main: nine tests fail, six in
tests/test_novelty_routing_cli_gaps.py, two intests/test_novelty_routing_all_vs_all.py, andtests/test_novelty_routing.py::test_headings_inside_code_only_refuse_the_run_and_say_why. The two tests added after the first commit (no-derivation warning, attractor unit) were each shown red on the commit before their fix.Local checks on this branch (
aa9fcbeb):Self-review (pr-review local mode, 3 medium passes, diff and description only). Pass 2 found one blocker (the attractor line counted calls under the word "queries"); fixed in
aa9fcbeb, and pass 3 found none. Lenses and verify were not run.Declined review findings
--estimate-onlyexits 0 withn_calls 0where the real run exits 2 withno_queries(no usable query). The estimate prices calls and refuses nothing; a refusal would be a separate change._SENTENCE_ENDsplits after abbreviations such as "e.g.", so a self-derived query can be a fragment. Only the triage source is affected, and it is stamped as triage. Not measured.[a-z0-9]+) splits non-English words and gives CJK no tokens. It predates this PR, andheadings_as_querieshas the same limit. The new description fallback inherits it; a CJK-only description is refused asnone.--estimate-onlyis the spelling the rescan brief asked for.dry_runis the registry concept (dokimi/cli_flags.py);tests/data/cli_flags_allowlist.txtalready admits this spelling fornovelty analyze. Reviewers may prefer--dry-run; that is a rename, not a fix.Not in this PR
description_too_long,name_not_dirandtitle_case_nameare flipped by fix(catalog): flag name conflicts, fail an empty root, keep spec errors out of routing #328, not by this PR.Merge Danger
Door: two-way. The new flag and keys are additive (
queries_source,cost,noise,estimate_only). Reverting restores the old refusals.Blast Radius: low. Routing only; no catalog loading or field building is touched.
Ramifications:
--all-vs-allnow accepts--repeatsand makesNtimes the calls. Scripts that relied on the refusal will see model calls.--queries-from-headingsruns on skills it used to refuse; those runs are triage, stamped as such.docs/reference/cli-interface.jsonis regenerated for the new option.🤖 Generated with Claude Code