[vchamarthi]

Summary

novelty subcommands read user-supplied files (--placebo-text, --probes, --atoms, --novelty, --queries, --artifact) without handling read errors, so a missing, unreadable, non-UTF-8 or invalid file ends in a traceback.

Changes

Each affected read now reports the flag, path and reason in a one-line message and exits. --placebo-text is read before any model client is built, so a typo costs no model calls. One commit per command.

Risk

Error paths only.

Testing

make ci-local (offline), Python 3.13: everything passes except mypy (1 error) and the 10 tests that also fail on main (process, sandbox, container probes; macOS). Load-flaky tests pass alone.

Checklist

• Tests added/updated and pytest passes (apart from the 10 pre-existing failures)
• ruff, mypy, and schema checks pass (mypy: the one pre-existing error)
• Docs updated (or N/A): N/A
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[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

5 open issues

• load_view_inputs() only treats ValueError as 'invalid novelty artifact'. If load_artifact()… · New
• load_artifact_or_exit() won’t prevent tracebacks for some 'invalid artifact' cases. For example,… · New
• This re-raise uses from exc, which preserves exception chaining. If ProbeFileError ever escapes… · New
• Using {exc!r} will print exception class names and quoted strings (e.g.,… · New
• The and tmp_path / \"bin.txt\" pattern relies on write_bytes() returning a truthy int, which is… · New

What changed in this PR

Improve novelty CLI UX by converting file read/parse failures into short, user-facing error messages (exit 1) instead of tracebacks across multiple subcommands.

Changes:

• Add shared helpers to read inputs (placebo text, view inputs) with explicit error messages and SystemExit(1).
• Update novelty subcommands to use the new helpers (--placebo-text, --atoms, --artifact, --novelty), and improve routing query loading errors.
• Add/adjust tests to assert exit code/message for unreadable/invalid inputs; update changelog and growth baseline.

File	Description
dokimi/​dokimi/​commands/​novelty.py	Switches subcommands to helper-based input reading and early exits on bad inputs.
dokimi/​dokimi/​commands/​inputs.py	New shared helper for --flag file reads that exits with a one-line message.
dokimi/​dokimi/​commands/​novelty_view_inputs.py	New helper to load novelty view inputs with friendly failures.
dokimi/​dokimi/​novelty/​routing.py	Wraps --queries read errors into RoutingInputError with a clear message.
dokimi/​dokimi/​novelty/​items.py	Centralizes probe-file JSON read/parse errors into ProbeFileError.
dokimi/​dokimi/​novelty/​atoms.py	Adds load_atoms_or_exit for command-friendly atoms loading failures.
dokimi/​dokimi/​novelty/​artifact.py	Adds load_artifact_or_exit for command-friendly artifact loading failures.
dokimi/​tests/​test_novelty_cli.py	Adds CLI-level coverage for unreadable --placebo-text and probes file failures.
dokimi/​tests/​test_novelty_routing.py	Adds coverage for unreadable/missing --queries and updates expectations.
dokimi/​tests/​test_novelty_view_inputs.py	New tests for novelty view failure modes (unreadable/invalid files).
dokimi/​tests/​test_novelty_shorten_artifact.py	New tests for shorten’s --artifact load failures.
dokimi/​tests/​test_novelty_atoms_input.py	New tests for --atoms failures in probe/shorten.
dokimi/​changelog.d/​20261025_120000_vchamarthi_cli-input-novelty.md	Changelog entry describing the improved error handling.
dokimi/​scripts/​growth_baseline.json	Updates baseline line count for dokimi/commands/novelty.py.

_{Review effort: Lite}

[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 92.99%. Comparing base (75f86af) to head (b3dd46c).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   92.97%   92.99%   +0.02%     
==========================================
  Files         242      244       +2     
  Lines       30745    30868     +123     
  Branches     5069     5093      +24     
==========================================
+ Hits        28585    28706     +121     
- Misses       1459     1460       +1     
- Partials      701      702       +1     

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/inputs.py	100.00% <100.00%> (ø)
dokimi/dokimi/commands/novelty.py	96.28% <100.00%> (-0.01%)	⬇️
dokimi/dokimi/commands/novelty_view_inputs.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/artifact.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/atoms.py	98.12% <100.00%> (+0.59%)	⬆️
dokimi/dokimi/novelty/bundle.py	95.22% <100.00%> (ø)
dokimi/dokimi/novelty/items.py	100.00% <100.00%> (ø)
dokimi/dokimi/novelty/routing.py	99.01% <100.00%> (+0.01%)	⬆️

... 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]

Review at 7591501. The helpers do what the PR says for the common cases: missing file, directory, non-UTF-8, bad JSON. Three things are worth fixing before merge: one input class that still ends in a traceback, and two gaps in the new tests.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 8248 tests passed; the changed-line coverage check passes.
• Reproductions for the code finding, with the LLM transport mocked.
• A mutation run over the new code: 38 mutants, 6 of which pass the five test files (findings 2 and 3).

Not caught by this PR (outside this diff, predates it): novelty probe and novelty analyze with --artifact skill:<dir> where <dir>/SKILL.md is a directory still end in IsADirectoryError. check_answer_key (bundle.py:520) and _bundle_from_args catch (FileNotFoundError, ValueError), not OSError. shorten now exits with a message for the same input, so the commands disagree.

Checked, no issues

• Placebo order: --placebo-text is read before _clients(args) in analyze and probe; --estimate-only returns before it, as on the base.
• Callers: load_probe_cells and load_queries each have one caller, and it catches the new error type. The only test expectation that changed is FileNotFoundError to RoutingInputError for a missing queries file.
• Bot points: Copilot's point on load_view_inputs() and {} does not apply: load_artifact raises ValueError for that shape, and novelty view --novelty on {}, [], an empty file and bad JSON exits 1 with Error: ... is not a valid novelty artifact. Copilot's point on StopIteration in load_artifact_or_exit does not apply: a skill without the closing --- reaches the missing a 'description' error, which is caught, and the next(...) is not reached. Copilot's point on from exc in _read_probe_file does not apply: cmd_novelty_items catches ProbeFileError and exits with the message, so the chain never reaches a traceback.

[vchamarthi]

@exai-bot /review

[exai-bot]

Note

AI-generated review of 7591501c56b6, requested by @vchamarthi, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
Model us.anthropic.claude-sonnet-5-5, effort default, 9 of 93 turns, 0.5 of 48 minutes, 191k in / 3k out tokens.

The PR turns read and parse failures on the novelty input flags into one-line exits. It covers --placebo-text, --probes, --atoms, --novelty, --queries and the shorten --artifact. I traced each change against its callers and found no defects. The exception sets match what the readers raise: ArtifactValidationError and JSONDecodeError are both ValueError, and ProbeFileError and RoutingInputError are caught by their existing handlers. The tests exercise the real commands. I could not run them here because pytest isn't installed, but CI is green.

1 finding is on the changed lines, as comments below.

[exai-bot]

Note

AI-generated review of 92792f18a38f, requested by @vchamarthi, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
Model us.anthropic.claude-sonnet-5-5, effort default, 11 of 94 turns, 0.7 of 48 minutes, 244k in / 4k out tokens.

Re-review of 92792f18a38f: 2 new findings.

1 finding is on the changed lines, as comments below.

Findings outside the diff

1. Minor (confirmed, correctness): novelty probe --artifact still tracebacks on an unreadable entry point

dokimi/dokimi/novelty/bundle.py:520

The changelog says --artifact exits with a message, but only shorten was switched to load_artifact_or_exit. probe goes through check_answer_key, which catches only FileNotFoundError and ValueError. A SKILL.md that exists but cannot be read (PermissionError) would still end in a traceback. This is an edge case; the common missing-file and non-UTF-8 cases are handled.

Suggested fix:

Catch OSError in check_answer_key, or narrow the changelog wording to novelty shorten.

[napetrov]

Review at 92792f18, follow-up commits pushed up to d2c81f34.

Summary. The earlier rounds (antonwolfy, exai-bot r1, Copilot) are addressed in 26a9916..92792f1. I pushed two small commits for the two exai-bot round-2 items:

• fdc190ee (minor, pre-existing code but it contradicts the changelog line for --artifact): check_answer_key in novelty/bundle.py caught only FileNotFoundError/ValueError, so novelty probe --artifact with an entry point that exists but cannot be read still ended in a traceback. Reproduced with SKILL.md as a directory (IsADirectoryError); both except clauses now catch OSError. New test test_a_key_for_an_entry_point_that_cannot_be_read_is_refused fails before the change and passes after.
• d2c81f34 (nit): test_novelty_atoms_input.py imports json at module level instead of using __import__("json").

Land-time note. dokimi/scripts/growth_baseline.json is also changed by #233, #236 and #239. Regenerate it on the final base when this lands rather than resolving the conflict by hand.

Test evidence (Linux, uv run): ruff check dokimi tests clean; test_novelty_bundle_key.py test_novelty_atoms_input.py test_novelty_bundle_guards.py test_novelty_cli.py test_novelty_shorten_artifact.py: 144 passed.

From my side this is ready once CI is green on d2c81f34.

[antonwolfy]

Review at d2c81f3. The new input handling does what the PR says for --placebo-text, --probes, --atoms, --novelty and --queries, and the follow-ups since 7591501 (the atoms bundle check, the OSError in check_answer_key, the placebo-order test) hold. Two things are worth fixing before merge: one --artifact path that still ends in a traceback although the changelog covers it, and one test gap in the new atoms check.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 8269 tests passed; the changed-line coverage check passes.
• Reproductions for the code finding, with the LLM transport mocked.
• A mutation run over the new code: 61 mutants, 11 of which pass the six test files (finding 2 is the one that matters). The six test files all fail against the PR's base source.

Checked, no issues

• Placebo order: --placebo-text is read before _clients(args) in analyze and probe; --estimate-only still returns before it, as on the base. The swap is now caught by the new tests.
• Callers: load_atoms keeps its behaviour for other callers (_atoms_from_record is a pure refactor); load_probe_cells and load_queries each have one caller, and it catches the new error type.
• Messages and exit codes: a missing file, a directory, non-UTF-8 and bad JSON give a one-line message and exit 1 for --placebo-text, --probes, --atoms, --novelty, --queries, and --artifact in probe and shorten.
• Repo rules: SPDX headers on the new files, changelog fragment present, growth_baseline.json matches the 916 lines of commands/novelty.py and test_growth_ratchet.py passes.

[antonwolfy]

Review at 3709700. The OSError widening at the two --artifact sites and the non-object check in load_atoms_or_exit hold, and the new tests pin them. One thing is worth fixing before merge: an atoms file can still end in a traceback through the bundle record's inner values and through deeply nested JSON (inline).

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 8281 tests passed; the changed-line coverage check passes.
• Reproductions for the code finding, with the LLM transport mocked.
• A mutation run over the changes since d2c81f3: reverting OSError to FileNotFoundError at commands/novelty.py:300 or :527, or dropping the isinstance(data, dict) check, each fails a new test.

Checked, no issues

• Earlier threads: both points from my review at d2c81f3 are fixed. analyze, atoms and routing exit with cannot load ... for a SKILL.md that is a directory, and [], null and 5 as an atoms file exit with not an object.
• Other reads: I tried a directory, a non-UTF-8 file and a mode-000 file on every --placebo-text, --probes, --atoms, --novelty, --queries and --artifact/--competitors/--catalog read in the novelty subcommands. Each exits 1 with a message.
• Repo rules: SPDX headers, changelog fragment and growth_baseline.json (916 lines for commands/novelty.py) are in order.

[antonwolfy]

Review at 8c7555e. The new bundle type checks and the RecursionError handling in load_atoms_or_exit and novelty view hold, and the tests added for them fail against the previous source. The claim in the changelog still does not hold for a few inputs: deeply nested JSON in --probes and --queries, and values inside the bundle.excluded rows and the span and atom records of an --atoms file. Three of these end in a traceback (inline). One test gap is also inline.

What I ran, on a Linux review host at 8c7555e:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8299 tests passed; the changed-line coverage check passes.
• Reproductions for the code findings, with the LLM transport mocked.
• A mutation run over the changes since 3709700: 17 mutants, 7 of which pass both new test files (6 are in the test-gap comment; the seventh is equivalent).

Also still tracebacks in --atoms (same class as the excluded comment, outside the bundle block): the span and atom fields are not type-checked in _atoms_from_record (atoms.py:530).

• A span with "text": 5 in a shorten run raises TypeError: object of type 'int' has no len() at spans.py:88.
• A span with "start": "a" or null in a bundle record raises TypeError in entry_point_only (bundle.py:621).
• An atom with "span_ids": [[1]] raises TypeError: unhashable type: 'list' (bundle.py:624, shorten.py:158).
• An atom with "statement": 5 raises AttributeError: 'int' object has no attribute 'strip' (retention.py:260).

Suggestion: validate these fields once in _atoms_from_record (it raises ValueError, which load_atoms_or_exit and load_view_inputs already turn into the exit), instead of adding one check per reader.

Checked, no issues

• Earlier thread: the bundle budgets, resource_roles, row end/words/role/path and RecursionError points from my review at 3709700 are fixed for probe, shorten and view; the new cases fail against the previous source.
• Bundle values: negative, zero and huge max_* and words, unknown resource_roles, odd sha256/artifact_name values, a one-row or empty-row files list all exit cleanly or run.
• Repo rules: SPDX headers, changelog fragments, growth_baseline.json (916 lines for commands/novelty.py; test_growth_ratchet.py passes) and ruff on the changed files are in order.

[antonwolfy]

Review at 4813252. The three fixes since 8c7555e hold: a non-string bundle.excluded path, RecursionError in --probes, and RecursionError in --queries, and each new test fails when its guard is reverted. The changelog claim still does not hold for three inputs, one of them the span and atom fields from my last review, which this push did not touch.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; the changed-line coverage check passes.
• Reproductions for the code findings, with the LLM transport mocked.
• A mutation run over the changes since 8c7555e: 18 mutants, 16 killed. The two survivors are the nit inline and an excluded that is an empty non-list, which is equivalent in effect.

Still open from my review at 8c7555e (not a thread): the span and atom fields of an --atoms file are still not type-checked. See the first inline comment.

Checked, no issues

• Messages and exit codes: a missing file, a directory, non-UTF-8 and [ * 100000 + ] * 100000 give a message and exit 1 for --placebo-text, --probes, --atoms, --novelty, --competitors and --catalog in all novelty subcommands (85 cases).
• Queries fallback: a --queries file nested too deeply to parse is read one query per line, like [urgent] build fails, and the new test pins it.
• Repo rules: SPDX headers, changelog fragments, growth_baseline.json (916 lines for commands/novelty.py) and ruff on the changed files are in order.

[antonwolfy]

Review at 495d4d1. The three fixes since 4813252 hold: RecursionError at the four artifact loaders, ValueError in load_queries, and the span and atom field checks in _check_record_fields. The changelog claim still does not hold for three inputs: a novelty probe artifact case (a bug) and two input classes that still end in a traceback. The new checks also have a test gap (all inline).

What I ran, on a Linux review host at 495d4d1:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8357 tests passed; the changed-line coverage check passes.
• Reproductions for the code findings, with the LLM transport mocked.
• A mutation run over the changes since 4813252: 22 mutants, 6 of which pass the six test files (the test-gap comment). The six test files fail against the PR's base source (140 failed).

Checked, no issues

• Earlier points: the --atoms span and atom fields, the long-integer --queries file, deeply nested SKILL.md frontmatter in analyze, atoms, routing, shorten and the bundle-less probe path, and the boolean budgets are fixed and pinned. Dropping RecursionError at each of the four artifact sites, or narrowing the load_queries clause, fails a test.
• Round trip: an atoms file written by save_atoms still loads through load_atoms and load_atoms_or_exit; the new checks reject only shapes the tool never writes.
• Repo rules: SPDX headers, changelog fragments, growth_baseline.json (916 lines for commands/novelty.py; test_growth_ratchet.py passes) and ruff are in order.

[antonwolfy]

A bundle file row with no path still ends in a traceback in novelty shorten, and the other new bundle checks do not cover it. The changes since 495d4d1 hold: the span and atom field checks, the probe-file string checks and RecursionError in check_answer_key each fail a test when reverted, and the six touched test files fail against the PR's base source (186 of 410 tests). One bug and one nit remain, both inline.

What I ran, on a Linux review host at 37360b3:

• make ci-local: 29 passed, 0 failed, 7 skipped; the changed-line coverage check passes.
• Reproduction for the code finding, with the LLM transport mocked.
• A mutation run over the changes since 495d4d1: 38 mutants, 2 of which pass the touched test files (one is the nit inline; the other is a span without id or kind, which the reader rejects with a bare 'id' message).

Checked, no issues

• Earlier points: the deeply nested recorded SKILL.md in probe, the non-string --probes fields, and the span id, kind, heading_path and protection types are fixed and pinned.
• Round trip: an atoms file written by save_atoms still loads through load_atoms and load_atoms_or_exit; an atom with only a statement is accepted, and so are probe files without artifact or ref_digest.
• Sweep: about 5,000 single-field mutations of --atoms, --novelty and --probes files, plus missing, directory, mode-000, non-UTF-8, empty and deeply nested inputs on every file flag, ended in a message or a clean run. The one traceback is the inline bug.