[vchamarthi]

Summary

questions, personas, dashboard, library, subjects and ingest docling read user-supplied files (questions, personas, topics, registry, runtime config) and write output directories without handling OS or decode errors, so a bad file or path ends in a traceback.

Changes

Each affected read or write now reports the path and reason in a one-line message and exits. 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

2 open issues

• The ValueError path (“Invalid JSON”) does not include the file path, which makes the error harder… · New
• cmd_questions_generate now imports _load_runtime_config from dokimi.commands.answers, but the… · New

What changed in this PR

Improves CLI robustness by converting decode/OS/path failures across several commands into clean, one-line error messages with non-zero exits (instead of tracebacks), and adds regression tests for these error paths.

Changes:

• Add input/output error handling for questions, personas, dashboard, library, subjects, and ingest docling.
• Refactor subjects run planning/validation to fail before creating output directories.
• Add targeted tests covering unreadable/invalid inputs and unwritable outputs.

File	Description
dokimi/​dokimi/​commands/​questions.py	Adds shared JSON read helper + personas validation; uses centralized runtime config loader; improves refine error for unreadable questions file.
dokimi/​dokimi/​commands/​personas.py	Adds structured error handling/validation for unreadable/invalid persona files.
dokimi/​dokimi/​commands/​library.py	Adds checked loaders for --registry / --intents to exit cleanly on unreadable/non-file inputs.
dokimi/​dokimi/​commands/​subjects.py	Adds _run_plan to validate descriptor and compute run plan before writing outputs.
dokimi/​dokimi/​commands/​ingest.py	Catches OSError during materialization and exits with a one-line message + exit code 2.
dokimi/​dokimi/​commands/​dashboard.py	Wraps dashboard file writes in OSError handling and exits cleanly on write failures.
dokimi/​tests/​test_subjects_run_input.py	Adds tests ensuring unrunnable subject descriptors exit 1 with a reason and do not write outputs.
dokimi/​tests/​test_questions_generate_inputs.py	Adds tests for clean exits on unusable personas/topics files.
dokimi/​tests/​test_questions_generate_config.py	Adds tests for clean exits on invalid runtime config content.
dokimi/​tests/​test_questions_artifact.py	Adds refine test for missing/dir --questions path emitting a path-naming error.
dokimi/​tests/​test_personas_approve.py	Adds tests for personas approve invalid/unreadable inputs exiting 1 without tracebacks.
dokimi/​tests/​test_library_cli_inputs.py	Adds tests for library commands naming unreadable registry/intents inputs and rejecting non-files.
dokimi/​tests/​test_ingest_cli.py	Adds docling ingest tests for unusable out-dir and conversion OS errors.
dokimi/​tests/​test_dashboard_cli.py	Adds dashboard tests for unwritable output dir and successful fresh output creation.
dokimi/​changelog.d/​20261025_120000_vchamarthi_cli-input-questions-misc.md	Documents the improved CLI error behavior.

_{Review effort: Lite}

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 94.44444% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.81%. Comparing base (dc141f0) to head (f1af248).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/commands/personas.py	81.81%	4 Missing and 2 partials ⚠️
dokimi/dokimi/commands/library.py	90.00%	1 Missing and 1 partial ⚠️
dokimi/dokimi/commands/dashboard.py	90.90%	0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
+ Coverage   92.29%   92.81%   +0.51%     
==========================================
  Files         239      241       +2     
  Lines       29969    30516     +547     
  Branches     4929     5039     +110     
==========================================
+ Hits        27661    28322     +661     
+ Misses       1627     1489     -138     
- Partials      681      705      +24     

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/_output.py	100.00% <100.00%> (ø)
dokimi/dokimi/commands/answers.py	94.68% <100.00%> (+0.49%)	⬆️
dokimi/dokimi/commands/ingest.py	80.00% <100.00%> (+30.00%)	⬆️
dokimi/dokimi/commands/questions.py	78.22% <100.00%> (+17.57%)	⬆️
dokimi/dokimi/commands/subjects.py	86.63% <100.00%> (+3.13%)	⬆️
dokimi/dokimi/commands/dashboard.py	90.90% <90.90%> (+55.90%)	⬆️
dokimi/dokimi/commands/library.py	69.79% <90.00%> (-0.58%)	⬇️
dokimi/dokimi/commands/personas.py	76.59% <81.81%> (+42.41%)	⬆️

... and 18 files 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 7b1658a. The fix is right where it applies: each new message names the path and the reason, and subjects run now validates before it creates the output directory. But three paths the PR claims to cover still end in a traceback, and one gap in the tests lets a regression through. Four points are worth fixing before merge (inline), plus two nits.

What I ran, on a Linux review host at 7b1658a:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8,249 tests passed; changed-line coverage passes the gate (personas.py 86%, library.py 95%).
• Reproductions for the code findings, with the LLM transport mocked.
• Mutation run: 52 mutants of the new code, 6 of which pass the new tests. The new tests fail on the base modules (31 failures), so they do test the change.

Copilot's point on the private _load_runtime_config import does not apply: benchmark.py already imports _load_registry from commands.library, and there is no import cycle.

Checked, no issues

• Ordering: _run_plan runs every ValueError check before mkdir, and the test asserts no results/ directory is left.
• Changelog and rules: the fragment is named and placed correctly, new tests carry SPDX headers, no schema or docs change is needed.

[antonwolfy]

I pushed a commit per nit, up to 4418c37, which fix the two nits from my review:

• 439bc68: dokimi/dokimi/commands/personas.py:88: the parse error now reads Cannot parse {persona_file}: {e}, and the tests assert the path and the offending value.
• 4418c37: dokimi/changelog.d/20261025_120000_vchamarthi_cli-input-questions-misc.md:3: the fragment now says a missing file exits 1 and notes that library no longer warns and prints an empty result for a missing --registry or --intents file.

make ci-local at 4418c37: 29 passed, 0 failed, 7 skipped.

Not approving yet. Still open: the four risks from my review.

[vchamarthi]

@exai-bot /review

[exai-bot]

Note

AI-generated review of 4418c37c1ff5, 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, 15 of 95 turns, 1.0 of 48 minutes, 436k in / 5k out tokens.

The PR turns OS, decode and parse errors on user-supplied input files into one-line stderr messages with an exit code, for questions, personas, dashboard, library, subjects and ingest docling. The input-reading changes are sound and well tested, and I found no regressions. Several write paths the title and changelog imply are covered are still unguarded, so the "unusable output path" claim is only partly true.

2 findings are on the changed lines, as comments below.

Findings outside the diff

1. Minor (confirmed, correctness): questions generate still tracebacks on an unwritable --output, after the LLM spend

dokimi/dokimi/commands/questions.py:331

• Problem: generator.save_questions(questions, output_path) is the last step of cmd_questions_generate and is unguarded. An --output that is a directory, has a file as a parent, or is unwritable raises OSError and ends in a traceback.
• Why it matters: the failure only appears after topic extraction, generation and optional validation have spent API calls, and the generated questions are lost. The changelog says these commands handle "an unusable output path".
• Fix: check the output location before the LLM work, or catch OSError around the save. questions analyze (save_report) and personas generate (save_personas) have the same unguarded write.

Suggested fix:

Create output_path.parent and probe that it is writable before generating, or wrap save_questions in except OSError and exit 1 with the path.

For a human reviewer

• Decide whether the changelog's "unusable output path" scope should cover every listed command's write path, or only dashboard and ingest as implemented.
• library now exits 1 on a missing --registry/--intents file where it used to warn and print an empty result. Confirm that scripts relying on the old behaviour are acceptable to break.

[exai-bot]

Note

AI-generated review of 9c5719ab7ab2, 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, 12 of 93 turns, 1.2 of 48 minutes, 387k in / 4k out tokens.

Re-review of 9c5719ab7ab2: 1 new finding.

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

[napetrov]

Review at 9c5719a. Earlier rounds covered the code; this note is only about landing this PR next to the others in the set.

What I ran: the nine touched test files on 9c5719a: 161 passed. ruff check: clean. git merge-tree against current main: no conflict.

Conflict with #236 in dokimi/commands/answers.py. Both PRs change the same except line in _load_runtime_config ((OSError, UnicodeDecodeError) in #236, (OSError, ValueError) here), so git merge-tree reports a one-line conflict. Keep this PR's ValueError form. It is the broader of the two, since UnicodeDecodeError is a subclass of it.

Still open from the bot review at 9c5719a: the output writes of questions generate and personas discover still end in a traceback, although the changelog says they exit with a message. This has no reply yet. Either guard the writes or narrow the changelog wording.

Commit subject: 9c5719a ("Exit with a message when the subjects scorecard ...") has no fix(subjects): prefix, unlike the other commits. That only matters if the PR is rebase-merged.

[antonwolfy]

Review at 1b5b96d. The new guards work where they are applied, and the four risks from my earlier review are fixed. Three things are worth fixing before merge: two output writes the changelog claims are covered still end in a traceback, and several of the new guards have no test that fails without them. One nit inline.

What I ran, on a Linux review host at 1b5b96d:

• make ci-local: 29 passed, 0 failed, 7 skipped; changed-line coverage passes the gate.
• Reproductions for the code findings, with the LLM transport mocked.
• Mutation run: 44 mutants of the new code, 10 of which pass the full suite. The new tests fail on the base modules.

Checked, no issues

• Guards: questions generate, questions analyze, personas discover (with --save-analysis) and subjects run exit 1 with Error: cannot write <path>: ... for an output under a file and for an output that is a directory.
• Runtime config: non-UTF-8 bytes, an unreadable path and a non-mapping products or retrieval all exit 1; removing the (OSError, ValueError) clause or the mapping check fails the tests.
• Repo rules: ruff and mypy are clean on the touched files, the growth ratchet is green, new files carry SPDX headers, no schema or docs page changes are needed.

[antonwolfy]

Review at b42ded3. The four risks from my earlier review are fixed, and the refine and panel-review guards now work. One risk is left in questions generate, plus two nits inline.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 8,284 tests passed.
• Reproductions for the code findings, with the LLM transport mocked.
• Mutation run: 45 mutants of the new code, 1 of which passes the new tests (inline).

Checked, no issues

• Output guards: removing any of the guards in questions.py, subjects.py and personas.py, or changing the exit code or dropping the path from the message, fails a test.
• Panel review: the exit_on_unwritable wrapper around review_questions does not relabel LLM errors as write errors. _call_reviewer catches every exception per vote, and the output directory is created before any LLM call.
• Repo rules: SPDX headers are present, the growth check is clean, the changelog fragment is in the right place.

[antonwolfy]

LGTM. 6d050df closes the points left open from my review at b42ded3: questions generate now turns a failed artifact validation into Error: ... and exit 1, the changelog names dashboard only for the output path, and the mapping check is pinned for falsy values. One nit inline.

What I ran, on a Linux review host at 6d050df:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8,287 tests passed; changed-line coverage passes the gate.
• Mutation run on the guards in questions.py, subjects.py, personas.py and _output.py: 48 mutants; the only survivors are in the personas approve required-key checks, which this PR re-indents but does not change.
• Reproductions for the code findings, with the LLM transport mocked.

Checked, no issues

• Output guards: every write in questions (generate, refine, analyze, panel-review), personas discover, subjects run, dashboard and ingest docling is wrapped; removing the ValueError handler in generate fails a test.
• Repo rules: ruff and mypy are clean, the growth ratchet is green, SPDX headers are present, the new tests use tmp_path and mocks only.

[antonwolfy]

LGTM. 5526326 rewords the fragment as asked, and every earlier point is fixed at this head. One nit inline.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; changed-line coverage passes the gate.
• Reproductions for the code findings, with the LLM transport mocked.
• Mutation run on the new guards in questions.py, subjects.py, dashboard.py, library.py and ingest.py: 18 further mutants, all killed by the new tests.

Checked, no issues

• Output guards: questions (generate, refine, analyze, panel-review), personas discover, subjects run and dashboard exit 1 with Error: cannot write ...; ingest docling exits 2. Each has a test that fails without the guard.
• Ordering: _run_plan still runs every check before mkdir in subjects run.
• Repo rules: ruff, mypy and the growth ratchet are green, SPDX headers are present, no schema or docs change is needed.

[napetrov]

Delta review 9c5719a..f1af248 (edited after the author pushed 6d050df..f1af248 mid-review).

Resolved at 1b5b96d / b42ded3: the open item from round 1. The output writes of questions generate, refine, analyze and panel-review, and of personas discover (personas and --save-analysis), now exit with Error: cannot write <path> through the shared commands/_output.exit_on_unwritable guard. That matches the changelog. 6d050df also turns a validation failure of the generated questions into an exit 1 with a message.

Resolved, landing: with both current heads (#236 33bb01e, this PR f1af248), git merge-tree is clean. Both now use except (OSError, ValueError) in _load_runtime_config. The merge against main is clean too.

What I ran: the five touched test files on f1af248, 58 passed. ruff check is clean.

Nit: the 6d050df, 1b5b96d and b42ded3 subjects have no fix(...) prefix. That only matters if the PR is rebase-merged.

No open items from me.

[antonwolfy]

LGTM. f1af248 is the last change since my review at 5526326: it moves ingest docling to the unusable-output-path list in the fragment, which matches ingest.py, where only the OSError around the output is caught. Every earlier point is fixed at this head.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 8,287 tests passed; changed-line coverage passes the gate.
• Mutation run on the new guards in _output.py, dashboard.py, ingest.py, library.py, personas.py, questions.py, subjects.py and answers.py: 44 mutants, 36 killed. The survivors are guards this PR does not add (_extract_questions and panel-review list checks, the novelty-artifact ValueError clause in subjects run) or the message text of the dashboard error.
• Reproductions with the LLM transport mocked.

Checked, no issues

• Output guards: every output write in questions, personas discover, subjects run and dashboard exits 1 with Error: cannot write ...; ingest docling exits 2. Removing any wrapper fails a test.
• Changelog: the fragment's command lists match the code at this head, and it is placed and named correctly.
• Repo rules: ruff, mypy and the growth ratchet are green, SPDX headers are present, no schema or docs change is needed.