[napetrov]

Part of #4.
Follow-up to #44 (its "then RunSettings and a thin CLI" line).

Summary

analyze, --estimate-only, atoms and probe each mapped the tier-controlled flags (--max-atoms, --all-atoms, --no-*) to settings by hand. Now one resolver does it:

 cmd_novelty_analyze / atoms / probe
-  tiers.resolve(tier, flags)                      # only when --tier was given
-  flag_settings = {"paraphrases": 0 if args.no_paraphrase else 1, ...}   # analyze
-  estimate_calls(n, {...rebuilt again...})        # --estimate-only
-  None if args.all_atoms else _atom_cap(args); not args.no_verify        # atoms
-  0 if args.no_paraphrase else 1; not args.no_reversal                   # probe
+  _settings(args) -> tiers.resolve_settings(tier, flags) -> RunSettings
+    .config_fields()   -> AnalyzeConfig(..., tier=, tier_overrides=)
+    .estimate(n_spans) -> the --estimate-only bill
+    .tier_record(), .warnings()   # analyze() builds RunSettings.of(config)

The CLI now parses, calls and prints. The domain logic moved next to the code it belongs to:

dokimi/novelty/
├── tiers.py    RunSettings, resolve_settings, DEFAULT_SETTINGS (AnalyzeConfig defaults read it)
├── measure.py  parse_arms                                   (was commands._arms)
├── items.py    load_probe_cells, item_analysis              (was inline in `novelty items`)
├── routing.py  load_candidate, catalog_competitors, check_field, load_queries
└── client.py   default_judge                                (was commands._default_judge)

The probe record already moved to report.build_probe_record in #44.

	main	this PR
places that map a flag to a setting	5	1 (resolve_settings)
direct reads of tier-controlled flags in commands/novelty.py	22	0
commands/novelty.py lines	977	827
private CLI helpers that tests call (not counting the _client stub seam)	4 helpers, 14 tests	1 (_clients), 9 tests

Evidence

This is a refactor, so the evidence is that nothing changed.

• Golden test: tests/test_novelty_settings_parity.py runs 40 CLI cases with scripted clients, so nothing goes over the network. The cases cover --estimate-only for every tier, overrides (including a flag that matches the tier), --all-atoms beating --max-atoms, every --no-*, budgets, arm counts, rejected arm sets and --max-usd. They also cover full analyze runs, atoms and probe. For each case the test stores the exit status, stdout, stderr, model calls per role, run_config (including tier), warnings, the atom cap and the probe kinds. The golden file tests/fixtures/novelty_settings_main.json was captured on main at f76a760, and the module only uses interfaces that exist there.
  Before: 41 passed on main. After: 41 passed on this branch.

  test_no_setting_bill_or_tier_record_differs_from_main[estimate/triage-max-atoms-90]
    _capture(...) == golden[...]   # stdout bill, tier record, stderr forfeits: same bytes
  

• Wider capture, not committed: a harness ran every novelty subcommand (97 cases, 146 invocations), including items, routing, shorten, view and all their error paths. It recorded stdout, stderr, the exit or exception, calls per role and every file written (novelty.json, .md, annotated views, atoms.json, probes.json, routing.json, items.json). The capture from this branch is byte-identical to the one from main. Error ordering is kept on purpose:
  • routing warnings print before a later competitor fails to load;
  • --arms is still checked where it was checked before;
  • a non-JSON, missing or non-UTF-8 input file still raises as before. The new domain errors are ValueError subclasses (ProbeFileError, RoutingInputError) and the CLI catches them by name.
• New interface tests:
  • RunSettings: config round trip, estimate, warnings, and tier= matching config.tier;
  • parse_arms, load_probe_cells / item_analysis, check_field / load_queries / catalog_competitors, default_judge;
  • test_every_tier_controlled_flag_is_read_through_the_resolver, which fails if commands/novelty.py reads one of those flags directly again.
• Local checks:
  • full suite under two pytest-randomly seeds: 5235 passed;
  • mypy (strict config) clean, no exemption added;
  • ruff and pre-commit on the changed files pass;
  • make ci-local --offline: 13 passed, 7 skipped (network/docker), 1 failed: changelog, waiting for the label below;
  • diff-cover: 98% of changed lines. The misses are the catalog and entry-point "no description" warnings, which the skill loader cannot reach today; they were untested on main too.
• CI: see the checks on this PR.

Merge Danger

Door: two-way. Everything is code, with no persisted-format or schema change. A revert restores the old call sites.

Blast Radius: none

• CLI: output, artifacts, numbers, warnings and exit codes are unchanged (shown above). No new flags.

• Library, additive: AnalyzeConfig gains tier and tier_overrides, both optional (None / {}). run_config is written exactly as before. analyze(..., tier={...}) still works. It now rebuilds the record from the dict's name and overrides, so other keys in a hand-built dict are no longer copied into run_config.tier. The CLI never passed any.

• Library, removed:

  • dokimi.novelty.tiers.resolve is replaced by resolve_settings. It is not exported from dokimi.novelty, and its only callers were the CLI and tests.
  • The private CLI helpers _arms, _atom_cap, _default_judge and _catalog_competitors are removed.

• Changelog: skip-changelog. There is no user-visible effect: CLI output is byte-identical, and the public API change is additive.

• Overlap:

  • fix(novelty): a partial conflict check is not a cleared harmful_atoms pass #72 touches analyze.py and test_novelty_analyze.py. git merge-tree merges it cleanly, and the 624 novelty tests pass on the merged tree.
  • feat(judge-validity): --context stored — gate the judge the way arms are scored #73 shares no files.

• Not in this PR. Each of these would change behaviour:

  • probe --tier: probe writes no tier record, and the max_atoms/verify/classify/shorten forfeits don't describe a probe run.
  • --max-usd stays outside RunSettings: it caps the clients, not what is measured, and it is not in run_config.
  • probe checks --arms only after generating wordings (2 calls are spent before it refuses).
  • analyze with an unknown arm fails with a traceback after extraction.
  • probe accepts --max-atoms/--all-atoms and ignores them.

  All of these are candidate follow-ups on Architecture cleanup: fix live bugs, then deepen the LLM, eval-record, novelty, arms and process modules #4.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A): N/A, no documented interface or output changed
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml): the new test module has it; the JSON golden is exempt

🤖 Generated with Claude Code

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 98.04878% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.40%. Comparing base (f76a760) to head (05f18d8).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/commands/novelty.py	93.33%	2 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
+ Coverage   90.26%   90.40%   +0.14%     
==========================================
  Files         206      206              
  Lines       24289    24488     +199     
  Branches     3906     3944      +38     
==========================================
+ Hits        21924    22138     +214     
+ Misses       1700     1690      -10     
+ Partials      665      660       -5     

Files with missing lines	Coverage Δ
dokimi/dokimi/novelty/analyze.py	95.13% <100.00%> (+1.01%)	⬆️
dokimi/dokimi/novelty/client.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/items.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/measure.py	93.24% <100.00%> (+0.21%)	⬆️
dokimi/dokimi/novelty/routing.py	98.76% <100.00%> (+0.34%)	⬆️
dokimi/dokimi/novelty/tiers.py	100.00% <100.00%> (+4.08%)	⬆️
dokimi/dokimi/commands/novelty.py	93.54% <93.33%> (+1.33%)	⬆️

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[Copilot]

🟢 Approval recommended

No blocking issues were identified, and the parity evidence supports preservation of existing CLI behavior.

No open issues 🎉

What changed in this PR

Continues #4 and #44 by centralizing novelty run settings and moving domain logic out of the CLI.

Changes:

• Adds RunSettings for flag resolution, estimates, warnings, and tier records.
• Moves arm parsing, input loading, item analysis, and judge selection into domain modules.
• Adds golden parity tests and updates interface tests.

File	Description
dokimi/​tests/​test_novelty_tiers.py	Tests settings resolution and round trips.
dokimi/​tests/​test_novelty_settings_parity.py	Adds 40 CLI parity cases.
dokimi/​tests/​test_novelty_routing.py	Tests extracted routing helpers.
dokimi/​tests/​test_novelty_measure.py	Uses domain arm parsing.
dokimi/​tests/​test_novelty_items.py	Tests probe loading and analysis.
dokimi/​tests/​test_novelty_cli.py	Checks CLI validation and warnings.
dokimi/​tests/​test_novelty_analyze.py	Tests config-carried tier metadata.
dokimi/​tests/​test_default_branches.py	Uses relocated judge selection.
dokimi/​tests/​fixtures/​novelty_settings_main.json	Stores baseline parity expectations.
dokimi/​dokimi/​novelty/​tiers.py	Centralizes settings and derived records.
dokimi/​dokimi/​novelty/​routing.py	Adds routing input helpers.
dokimi/​dokimi/​novelty/​measure.py	Adds paired-arm parsing.
dokimi/​dokimi/​novelty/​items.py	Adds probe loading and analysis assembly.
dokimi/​dokimi/​novelty/​client.py	Houses default judge selection.
dokimi/​dokimi/​novelty/​analyze.py	Integrates shared defaults and settings.
dokimi/​dokimi/​commands/​novelty.py	Delegates domain logic to shared helpers.

_{Review effort: Balanced}

[antonwolfy]

The refactor holds up: I found no behaviour change against the base. Two test gaps are worth closing before merge, because the lines they cover are new code that nothing pins.

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

• make ci-local: 18 passed, 1 failed, 4 skipped. The failure is changelog, which locally cannot see the skip-changelog label; the PR has it and the check passes on GitHub, so it is not a finding. 5236 tests passed, changed-line coverage 98% (misses: commands/novelty.py:282,293, the no-description warnings).
• 20 CLI scenarios on the base and on the head (estimate-only with and without a tier, --all-atoms vs --max-atoms, bad --arms, routing and items error paths): stdout, stderr and exit status are byte-identical. The LLM transport was mocked.
• A mutation run on the new code: 148 mutants, 108 killed. The survivors that matter are in the two inline comments below.
• The golden fixture: the parity test and novelty_settings_main.json pass on the base (f76a760) as well, and regenerating the fixture on the head gives the same bytes.

Checked, no issues

• Precedence: --all-atoms still beats --max-atoms, with and without a tier. A flag equal to the tier's value is not recorded as an override, and overrides stay {} without a tier.
• Defaults: DEFAULT_SETTINGS holds scalars only, resolve_settings copies it, and mutating a resolved values does not touch it.
• Removed helpers: no reference to tiers.resolve, _arms, _atom_cap, _default_judge or _catalog_competitors is left in code, tests or docs.
• Error handling: ProbeFileError and RoutingInputError are caught only around their own calls, and error ordering matches the base.
• Repo rules: SPDX header on the new test module, the JSON fixture is exempt, no schema change, tests run offline.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• Catching TypeError together with KeyError and reporting it as “missing {exc}” can yield… · New
• The regex-based scan can produce false positives (e.g., args.<flag> appearing in comments,… · New
• names.count(n) inside a set comprehension makes this O(n²) over competitor count. Using… · New

_{Review effort: Lite}

[antonwolfy]

Review at aa4a428. The new tests close the kind_agreement half of my arm thread. The other two threads are still open: two of the three mutants I reported survive the new tests. The docstring thread is resolved.

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

• make ci-local: 18 passed, 1 failed, 4 skipped. The failure is changelog, which locally cannot see the skip-changelog label, so the label covers it. 5239 tests passed, changed-line coverage 98%.
• A mutation check of the three mutants from my earlier review against the new tests, with test_novelty_items.py, test_novelty_routing.py, test_novelty_cli.py and test_novelty_settings_parity.py run together.

Still open from my earlier review:

• dokimi/tests/test_novelty_items.py:301: nothing pins arm=arm in the discrimination call. Hardcoding arm="full" in kind_agreement is now caught.
• dokimi/tests/test_novelty_routing.py:439: the exclusion set {gold.name, *named competitors} in commands/novelty.py:290 is still unpinned by any test.

Checked, no issues

• Docstring: the tier= text at analyze.py:158 matches the code. A truthy tier replaces both the config's tier and its overrides, with no merge.
• Source change: only that docstring changed since e8e52f3. The commits since are one docstring edit and the new tests.

[xaleryb]

• Ran: CLI differential of every novelty flag combination (base vs head, main vs main+PR), and the suite on main+PR.
• novelty items --probes F with {"runs": null}, {"runs": "abc"}, {"runs": {}}, {"runs": {"a": 1}} or {"runs": [1, 2]} -> the head prints different error text from main (exit 1 on both trees). The body says "CLI output is byte-identical" and "no user-visible effect", which is the reason given for skip-changelog -> either the body and the changelog decision cover this change, or the items.py hunk of 12d1675 moves to a follow-up.
• Before our commit, removing either isinstance guard in load_probe_cells gave a TypeError traceback while the suite stayed green.
• We pushed a4419d1 test(novelty): pin the runs/row type guards in load_probe_cells: it adds {"runs": null} and {"runs": [1]} to test_a_file_that_is_not_probe_output_is_refused, so removing either guard now fails a test.
• Left for the author: the body/changelog item above.

[antonwolfy]

LGTM. Both pins from my earlier review now hold, and the new type guards in load_probe_cells are tested. Two nits inline.

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

• make ci-local: 18 passed, 1 failed, 4 skipped. The failure is changelog, which locally cannot see the skip-changelog label, so the label covers it. 5242 tests passed, changed-line coverage 99%.
• A mutation run on the delta since aa4a428: 10 mutants, all killed. They were: each isinstance guard removed, arm= dropped from discrimination, kind_agreement hardcoded to "full", the catalog exclusion reduced to {gold.name}, two check_field duplicate-check mutants, and three direct args.<flag> reads added to commands/novelty.py (plain, no_verify, inside a lambda).

I agree with @xaleryb that "byte-identical" no longer holds for novelty items --probes. New evidence: {"runs": null} now reports has `runs` as NoneType, not a list instead of 'NoneType' object is not iterable. The new text is better; reword the description.

Checked, no issues

• Arm pass-through: the rewritten test uses three models, so discrimination has variance, and it fails when arm= is dropped.
• Catalog exclusion: test_a_competitor_that_is_also_in_the_catalog_is_counted_once fails when only gold.name is excluded.
• Guards: except KeyError is enough after the isinstance checks. ProbeRun takes any field value, so no TypeError is reachable.
• check_field: the Counter version returns the same duplicate set as names.count.

[antonwolfy]

LGTM. Both nits from my earlier review are fixed in 05f18d8, and the delta since a4419d1 is three small changes. One nit on the description.

What I ran, on a Linux review host at 05f18d8:

• make ci-local: 18 passed, 1 failed, 4 skipped. The failure is changelog, which locally cannot see the skip-changelog label, so the label covers it. 5243 tests passed, changed-line coverage 99% (misses: commands/novelty.py:282,293, the no-description warnings).
• A mutation check on the delta: removing the isinstance(data, dict) guard in load_probe_cells fails the new [1] case with AttributeError: 'list' object has no attribute 'get'. novelty items --probes on files holding "x", 5, null, 1.5, true and [] exits 1 with the ProbeFileError text and no traceback.

Nit on the description. It lists _arms among the removed CLI helpers, but _arms is still in commands/novelty.py:798, now a thin wrapper that turns the ValueError from parse_arms into a SystemExit. It has three callers (lines 66, 84, 190). The behaviour is right; reword the "removed" line.

Checked, no issues

• Delta: the isinstance guard raises ProbeFileError before data.get, the function-local Counter import is gone, and check_field uses the module-level import (routing.py:37).
• Tests: test_novelty_items.py, test_novelty_routing.py and test_novelty_cli.py pass together.